Skip to content

Work around editByPath merge failure after split#163

Merged
hackerwins merged 1 commit into
mainfrom
fix/merge-block-split-workaround
Apr 26, 2026
Merged

Work around editByPath merge failure after split#163
hackerwins merged 1 commit into
mainfrom
fix/merge-block-split-workaround

Conversation

@hackerwins

@hackerwins hackerwins commented Apr 26, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

Summary

  • Fix backspace not merging blocks in docs editor when the blocks were created by Enter (splitLevel>=2)
  • Decompose the single cross-boundary editByPath into two same-parent operations to avoid the SDK's Phase 3 Range Narrowing bug

Problem

When pressing Enter in a paragraph (e.g., "as|df" → "as" + "df"), the split creates CRDT nodes linked via insNextID. A subsequent backspace calling editByPath([blockPath, inlineCount], [nextPath, 0]) fails silently because the SDK's Range Narrowing follows the insNextID chain and narrows the collection range to empty.

Fix

Replace the single cross-boundary edit with two same-parent operations:

  1. Delete the next block element: editByPath(nextPath, [parentPath, nextIdx+1])
  2. Insert its inline children at the end of the first block: editBulkByPath(...)

Both operations stay within the same parent element, avoiding the Range Narrowing code path.

Related SDK issue: yorkie-team/yorkie-js-sdk#1237

Test plan

  • pnpm verify:fast passes (all 622 docs + 1204 sheets + 167 frontend tests)
  • "split then merge round-trip" test continues to pass
  • Manual test: type "asdf", press Enter after "as", press Backspace → should merge back to "asdf"
  • Manual test with styled text (bold "as" + normal "df") → merge preserves styles

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed block merging to handle complex document structures more reliably. The merge operation now uses an optimized two-step process that separately handles the deletion and reinsertion of block content, ensuring better stability when working with nested block elements.

@hackerwins @claude
Work around editByPath merge failure after split
936c15b 
When blocks are created by editByPath with splitLevel>=2, the CRDT
nodes are linked via insNextID. A subsequent cross-boundary
editByPath to merge them back fails silently because the SDK's
Phase 3 Range Narrowing follows the insNextID chain and narrows
the collection range to empty.

Decompose the single cross-boundary edit into two same-parent
operations: (1) delete the next block element, (2) insert its
inline children at the end of the first block. Both edits stay
within the same parent, avoiding the Range Narrowing path.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Apr 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8f83b9d7-e3ef-4584-8da4-60a5228fc378

📥 Commits

Reviewing files that changed from the base of the PR and between f2bb764 and 936c15b.

📒 Files selected for processing (1)
  • packages/frontend/src/app/docs/yorkie-doc-store.ts
📝 Walkthrough

Walkthrough

The mergeBlock function in the YorkieDocStore has been refactored to replace a single cross-boundary editByPath operation with a two-step sequence: first deleting the next block element, then inserting its inline children into the first block. This change addresses a yorkie-js-sdk Phase 3 range narrowing issue affecting blocks with splitLevel>=2.

Changes

Cohort / File(s) Summary
Block Merge Operation Refactor
packages/frontend/src/app/docs/yorkie-doc-store.ts		 Replaced single cross-boundary editByPath operation with two-step delete-and-insert sequence using deep-cloned nodes to avoid yorkie-js-sdk range narrowing issue.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A blocks merge smooth, in two steps we go,
Delete, then insert—watch the tree flow!
No range-narrowing tricks in Phase 3's way,
Deep clones and boundaries save the day! ✨

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/merge-block-split-workaround

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@hackerwins
hackerwins merged commit c62b1a7 into main Apr 26, 2026
1 check passed
@hackerwins
hackerwins deleted the fix/merge-block-split-workaround branch April 26, 2026 10:53
@github-actions

github-actions Bot commented Apr 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Verification: verify:self

Result: ✅ PASS in 92.5s

Lane Status Duration
sheets:build ✅ pass 9.7s
docs:build ✅ pass 6.1s
verify:fast ✅ pass 45.8s
frontend:build ✅ pass 12.7s
verify:frontend:chunks ✅ pass 0.3s
backend:build ✅ pass 3.7s
cli:build ✅ pass 1.3s
verify:entropy ✅ pass 13.0s

Verification: verify:integration

Result: ✅ PASS

@codecov

codecov Bot commented Apr 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@hackerwins hackerwins mentioned this pull request May 1, 2026
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@hackerwins