fix: rename colliding keys before a range delete merges sibling blocks - #3187
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 108c2d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Bundle Stats✅ No significant changes. All scenario measurements (7)🗺️
Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time. |
christianhg
marked this pull request as ready for review
August 27, 2026 08:20
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4c6b289. Configure here.
christianhg
force-pushed
the
range-delete-merge-renames
branch
from
August 27, 2026 08:37
4c6b289 to
42dbae4
Compare
The backspace and forward-delete merge behaviors planned their colliding-key renames inside `behavior.abstract.delete.ts`, coupled to behavior actions (`raise`). The planning itself — which keys collide, what they become, which `marks` follow a renamed markDef — is layer-neutral, and the range-delete merge at the operation layer needs the same plan. Move the pure planner to `internal-utils/plan-merge-key-renames.ts`, returning the renamed block plus neutral child and markDef rename lists; the behavior builds its `raise` actions from those lists. Net behavior unchanged: colliding and non-colliding merges emit byte-identical patches, pinned by the existing `block-merge-duplicate-keys` suite and wire-catalogue fixtures, both untouched.
christianhg
force-pushed
the
range-delete-merge-renames
branch
from
August 27, 2026 12:22
45c8f02 to
0e3371b
Compare
A range delete spanning two sibling text blocks merges the end block into the start block (`mergeBlock` → `applyMergeNode`). When the blocks shared a child `_key` (legal across blocks), two mechanisms corrupted the merge. `applyMergeNode` anchored each child insert after the previous child's pre-apply `_key`, but the engine's collision backstop re-mints a colliding key at apply time and writes it back onto the operation, so the stale anchor resolved to the destination's own twin span and the next child landed in the wrong position: the merged text came out reordered. And `mergeBlock` merged the two `markDefs` arrays through a last-wins map keyed on `_key`, so the end block's def silently replaced the start block's, repointing the start block's surviving annotated spans at the wrong annotation. Both re-mints were silent on the wire: receivers saw fresh-key inserts with no rename — destroy-and-create for content the user merely merged. `mergeBlock` now plans `_key` renames against the start block's current children before the merge — including the span holding the range's start point, which survives as an empty span until deferred normalization and is a genuine collision source — and applies them to the still-living end block, so keyed `set` patches for every rename precede the merge's `unset` and inserts on the wire, mirroring the collapsed backspace/forward-delete merges. markDef renames apply as direct engine `set`s (`setNodeProperties` cannot address keyed `markDefs` descents and would silently no-op). The markDefs carry over as a plain append (collisions are renamed away, so the dedupe map is gone), and `applyMergeNode` anchors each insert on the post-apply key, immunizing it against any collision that still reaches the backstop. Byte-identical markDefs under the same `_key` dedupe instead of renaming: the merge keeps the destination's def and the migrated spans' `marks` resolve to it, preserving annotation identity the way the old map did for identical defs. Only the range-delete path dedupes: the collapsed merges keep renaming, because their `renamedBlock` feeds `insert.block`, where `parseBlock` parses the block standalone and strips any mark that doesn't resolve in the block's own `markDefs` — a deduped def's spans would lose the annotation entirely. Pinned red-on-old: child reorder, markDef overwrite, the markDef rename reaching the wire before the merge's `unset`, identical-def dedupe with no rename patch, the collapsed path still renaming identical defs (red under a flipped `dedupeEqualMarkDefs`), trimmed-empty-span collision (with the caret collapsing to the range start), inline-object collision, and merge-undo round-trips restoring every original `_key`. New wire-catalogue row `range-delete-duplicate-keys`.
christianhg
force-pushed
the
range-delete-merge-renames
branch
from
August 27, 2026 13:13
590f765 to
108c2d3
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Deleting a range that spans two sibling text blocks merges the end block into the start block. When the blocks share a child
_key(legal across blocks, and a shape real documents have), the merge comes out wrong twice over: the merged text is reordered, and a collidingmarkDefskey makes the end block's def replace the start block's, so the start block's surviving annotated spans point at the wrong annotation. Both corruptions are silent on the wire: the colliding keys are re-minted with no rename, so receivers read destroy-and-create for content that was merely merged, and carets, comments, and decorations tracking those spans lose them.The fix mirrors what #3182 gave the collapsed backspace/forward-delete merges:
mergeBlockplans_keyrenames before the merge and applies them to the still-living end block, so keyedsetpatches land on the wire ahead of the merge'sunsetand inserts. The plan is computed against the start block's mid-delete state, where the span holding the range's start point still exists as an empty span and is a genuine collision source. The rename planner is extracted from the delete behaviors into a shared helper in its own behavior-preserving commit. markDef renames apply as direct enginesets, sincesetNodePropertiescannot address keyedmarkDefsdescents and would silently no-op (caught in review; the wire test now asserts the markDef rename patch explicitly).applyMergeNodealso anchors each child insert on the post-apply key, since the engine's collision backstop re-mints keys in place; the stale pre-apply anchor was the reorder mechanism.Byte-identical markDefs under the same
_keydedupe instead of renaming: the merge keeps one def under the original key and the migrated spans resolve to it, preserving annotation identity the way the old merge did for identical defs. Only same-key-different-content defs rename. The dedupe is scoped to the range-delete path: the collapsed merges keep renaming, because their output feedsinsert.block, where the block is parsed standalone and any mark that doesn't resolve in the block's ownmarkDefsis stripped, so a deduped def's spans would lose the annotation entirely. A dedicated test pins that the collapsed path keeps renaming identical defs (it goes red if the dedupe flag is flipped there).Pinned red-on-old: the reorder, the markDef overwrite, the markDef rename reaching the wire before the block
unset, the identical-def dedupe (no rename patch on the wire), the empty-span collision (with the caret collapsing to the range start), an inline-object collision, and undo round-trips restoring every original_key. New wire-catalogue row:range-delete-duplicate-keys.Note
Medium Risk
Touches core delete/merge and patch ordering for collaboration, but the change is scoped to duplicate-key merges with broad regression coverage.
Overview
Fixes range deletes that merge two sibling blocks when those blocks reuse the same child
_keyormarkDefskey—cases that could scramble merged text, overwrite annotations, and emit patches that look like destroy/create instead of moves.Range-delete merge (
mergeBlockindelete-internal.ts) now runs the same pre-merge rename plan as collapsed backspace/forward merges: colliding keys on the end block are updated with explicitsetpatches before unset/insert, using the start block’s mid-delete children (including the surviving empty span at the range anchor) as the collision set. Byte-identical colliding markDefs are deduped on this path; differing defs are renamed and spanmarksare rewritten.Rename planning is extracted to
plan-merge-key-renames.ts; delete behaviors keep a thinplanMergeKeyRenameActionswrapper that raises behavior events (dedupeEqualMarkDefs: falseforinsert.block).applyMergeNodechains child inserts using each insert operation’s post-apply_key, so engine collision re-minting no longer mis-orders siblings.Tests cover wire order, markDef dedupe vs rename, inline-object collisions, undo/redo, and a new wire-catalogue scenario
range-delete-duplicate-keys.Reviewed by Cursor Bugbot for commit 108c2d3. Bugbot is set up for automated code reviews on this repo. Configure here.