packages/frontend/tests/app/docs/yorkie-doc-store-concurrent.integration.ts (2)

500-555: Placement nit: setBlockType / applyBlockStyle tests aren't table-cell tests.

Both cases operate on a plain paragraph block (makeBlock('HelloWorld')) but are nested inside describe('YorkieDocStore concurrent table cell edits', …) (line 425). Consider moving them into the YorkieDocStore concurrent inline styling suite (or a new concurrent block attribute edits suite) so the describe label reflects what's actually exercised.

🤖 Prompt for AI Agents

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

In `@packages/frontend/tests/app/docs/yorkie-doc-store-concurrent.integration.ts`
around lines 500 - 555, These two tests ("concurrent setBlockType and text
insert should both be preserved" and "concurrent applyBlockStyle and text insert
should both be preserved") are misplaced under the describe labeled
"YorkieDocStore concurrent table cell edits"; cut these it blocks out of that
describe and move them into the existing "YorkieDocStore concurrent inline
styling" describe (or create a new describe "YorkieDocStore concurrent block
attribute edits") so the describe label matches the behavior being tested;
ensure you preserve the surrounding setup/teardown context (use of makeBlock and
createTwoUserDocs and ctx.cleanup) and keep the it titles and assertions intact.

---

500-588: Strengthen convergence assertions with a length check.

Existing cell tests in this file (e.g., line 493: assert.equal(textA.length, 12, 'Insert should be preserved');) assert final length in addition to includes. The three new tests only check includes('XX'|'YY'|'ZZ'), which would pass even if insertion were duplicated or characters from the original were lost. Adding a length assertion (e.g., 'HelloWorld'.length + 2 for XX/YY, 'CellText'.length + 2 for ZZ) would catch regressions in the intent-preserving path where that's exactly the property we care about.

♻️ Example: add length check to the setBlockType test

       const textA = docA.blocks[0].inlines.map((i) => i.text).join('');
       const textB = docB.blocks[0].inlines.map((i) => i.text).join('');
       assert.equal(textA, textB, 'Text should converge');
+      assert.equal(textA.length, 'HelloWorld'.length + 2, `Expected 12 chars, got "${textA}"`);
       assert.ok(textA.includes('XX'), `Inserted text "XX" should be preserved, got "${textA}"`);

🤖 Prompt for AI Agents

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

In `@packages/frontend/tests/app/docs/yorkie-doc-store-concurrent.integration.ts`
around lines 500 - 588, Add explicit final-length assertions to the three new
tests to ensure the inserted two characters weren't duplicated or lost: in the
"concurrent setBlockType and text insert should both be preserved" and
"concurrent applyBlockStyle and text insert should both be preserved" tests
assert that textA.length and textB.length equal makeBlock('HelloWorld') length
(10) + 2 => 12 (use the local variable block to compute or hardcode 12); in the
"concurrent applyCellStyle and text insert in same cell should both be
preserved" test assert textA.length and textB.length equal cellBlock original
text length (CellText = 8) + 2 => 10 (use cellBlock to compute or hardcode 10);
place these assertions alongside the existing includes(...) checks after
computing textA/textB so failures show both content and length mismatches,
referencing ctx.storeA/ctx.storeB and the local block/cellBlock variables to
locate the assertions.

docs/tasks/archive/2026/04/20260423-block-attr-intent-preserving-lessons.md (1)

1-3: Placeholder lessons file — no review needed.

Consider populating this before archival (e.g., takeaways from styleByPath vs full-block replacement, stale-attribute handling in setBlockType, cache-order fix in applyCellStyle) so the archive entry is useful to future work.

🤖 Prompt for AI Agents

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

In `@docs/tasks/archive/2026/04/20260423-block-attr-intent-preserving-lessons.md`
around lines 1 - 3, This is a placeholder lessons file; before archiving,
populate it with concise takeaways from recent edits — summarize pros/cons of
styleByPath versus full-block replacement, note stale-attribute handling in
setBlockType, and document the cache-order bug and fix in applyCellStyle so
future readers can find the rationale and patterns used; add short examples or
links to the relevant functions (styleByPath, setBlockType, applyCellStyle) and
a brief “lessons learned” bullet list.

packages/frontend/tests/app/docs/yorkie-doc-store.test.ts (1)

835-1037: LGTM — thorough coverage for the new intent-preserving methods.

Tests for setBlockType (including stale-attribute clearing via removeStyleByPath), applyBlockStyle, applyCellStyle, and insertImageInline exercise the main paths including cell-internal blocks. The hoisted makeTableWithText helper is cleanly reused.

One optional coverage gap you may want to add later: insertImageInline at the end of a block's text (offset === totalText.length), which exercises a boundary in resolveBlockNodeOffset + split-inline path.

🤖 Prompt for AI Agents

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

In `@packages/frontend/tests/app/docs/yorkie-doc-store.test.ts` around lines 835 -
1037, Add a boundary test that inserts an image inline at the end of a block's
text (offset === totalText.length) to exercise resolveBlockNodeOffset and the
split-inline path used by insertImageInline; create a block via makeBlock (or
reuse existing helper), setDocument, call store.insertImageInline(block.id,
totalTextLength, { text: '\uFFFC', style: { image: {...} } }), then assert the
final inlines/text and that the image inline exists — this will cover the edge
case the reviewer mentioned.

Fix all unresolved CodeRabbit comments on this PR:

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