packages/frontend/src/app/docs/yorkie-doc-store.ts (4)

621-644: Edge case: resolveBlockNodeOffset with zero inline children returns { inlineIndex: 0, charOffset: 0 }.

When inlineChildren.length === 0, lastIdx = -1, inlineChildren[-1]?.children is undefined, and the function returns { inlineIndex: Math.max(0, -1), charOffset: 0 } = { inlineIndex: 0, charOffset: 0 }. Callers then build paths like [...blockPath, 0, 0] which don't correspond to a real tree node.

In practice, blocks like horizontal-rule / page-break have no inlines and callers gate on block type before reaching here, and the code path that calls resolveBlockNodeOffset is insertText/deleteText/splitBlock, which shouldn't target empty-inline block types. So this is latent rather than exploitable today.

Worth either returning an explicit sentinel / throwing for the empty-inlines case, or documenting the precondition that callers must not invoke this for blocks without inline children.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 621 - 644,
The function resolveBlockNodeOffset currently assumes there is at least one
inline child; detect the case when inlineChildren.length === 0 and handle it
explicitly (either throw a clear error or return a sentinel) instead of
returning { inlineIndex: 0, charOffset: 0 }; update resolveBlockNodeOffset to
validate blockNode has inline children and throw e.g. "No inline children in
block" (or return a documented sentinel) so callers (insertText, deleteText,
splitBlock) won't build invalid paths—ensure the error message references the
blockNode/type to aid debugging.

---

512-545: setFooter reads childCount before getRootTreeNode() can reflect any header just inserted in this callback — but it currently doesn't matter.

Just a heads-up for future edits: setFooter reads childCount inside the update, which is correct as long as no earlier mutation in the same doc.update changed the child count. Today setFooter is called in isolation, so this is fine. If later refactors batch header + footer into one doc.update, the childCount-based positioning here ([childCount - 1] for replace, [childCount] for append) assumes the footer is the last child and will break silently if a header insertion happens in the same callback before this block runs.

Consider asserting the last child is actually a footer when hadFooter is true, or computing the footer index from a single pass over treeRoot.children that explicitly finds the footer child. Not blocking.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 512 - 545,
setFooter assumes the footer is the last child by using childCount and
[childCount-1]/[childCount] which will break if another mutation in the same
doc.update changes child order; change the logic in setFooter to compute the
footer position by scanning treeRoot.children for a node with type === 'footer'
(or assert that the last child has type 'footer' when hadFooter is true), then
use that explicit index for tree.editByPath replace/remove operations (and use
children.length only when appending a new footer); reference symbols: setFooter,
treeRoot, childCount, hadFooter, tree.editByPath.

---

680-696: updateBlock no longer verifies that the passed block.id matches id.

resolveBlockTreePath(id, ...) locates the target slot by the provided id, but then you overwrite that slot with block (which may have a different block.id). Given the region-aware cache update, that would leave a block in doc.header.blocks / doc.blocks / doc.footer.blocks whose id no longer matches, and subsequent resolveBlockTreePath calls for the old id would throw while calls for the new id might collide with existing blocks.

Probably worth either validating block.id === id up front, or forcing block.id = id before writing. Not new to this PR, but now that all three regions flow through the same helper it's easier to trip.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 680 - 696,
The updateBlock method uses resolveBlockTreePath(id, ...) to find the slot but
then writes the provided Block which may have a different block.id; fix
updateBlock by ensuring the incoming block's id matches the target id before
mutating the document — either validate and throw if block.id !== id or assign
block.id = id (do this immediately in updateBlock, prior to calling
this.doc.update and before calling setBlockByRegion); reference updateBlock,
resolveBlockTreePath, buildBlockNode and setBlockByRegion when making the change
so the tree edit and cache update remain consistent.

---

582-602: Two separate path-resolution helpers now coexist — worth consolidating eventually.

resolveBlockTreePath (new, for flat header/body/footer blocks) and resolveTableTreePath (existing, for nested table blocks) both walk the document to find a block by id, but don't share code and don't look into each other's regions. Consequences:

• resolveBlockTreePath does not descend into tables, so calling any of updateBlock / insertText / deleteText / applyStyle / splitBlock / mergeBlock / deleteBlock with a blockId that lives inside a table cell will throw Block not found: ….
• resolveTableTreePath only searches doc.blocks (the body) and won't find tables placed in the header or footer, even though the schema allows them and the backend supports nested tables in these regions.

Not a blocker for this PR, but a follow-up worth considering: unify into a single region-aware resolver that returns a path + region and optionally descends into tables.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/frontend/src/app/docs/yorkie-doc-store.ts` around lines 582 - 602,
resolveBlockTreePath currently only checks header/body/footer top-level blocks
and never descends into table cells, while resolveTableTreePath only searches
body blocks and misses tables placed in header/footer; unify or update the
resolvers so both are region-aware and search nested table contents: modify
resolveBlockTreePath and/or resolveTableTreePath to (1) search header, body, and
footer consistently, (2) when encountering a table block descend into its
rows/cells to look for blockId, and (3) return a consistent result shape { path:
number[]; region: 'header'|'body'|'footer' } used by
updateBlock/insertText/deleteText/applyStyle/splitBlock/mergeBlock/deleteBlock
so operations on blocks inside table cells or tables in header/footer do not
throw "Block not found". Ensure you reference resolveBlockTreePath and
resolveTableTreePath when updating and keep existing return contract (path +
region).

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes