fix: follow text moved by remote splits, merges, and block reorders when recovering the caret - #3154
Open
christianhg wants to merge 3 commits into
Open
fix: follow text moved by remote splits, merges, and block reorders when recovering the caret#3154christianhg wants to merge 3 commits into
christianhg wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: 79f5dc7 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Bundle StatsWarning 1 significant change. @portabletext/editor🔴 All scenario measurements (7)🗺️
Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time. |
christianhg
force-pushed
the
feat/transaction-interpreter
branch
from
August 24, 2026 15:57
b3789ae to
e2b72a4
Compare
christianhg
changed the title
This was referenced Aug 26, 2026
christianhg
marked this pull request as ready for review
August 26, 2026 11:49
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
christianhg
changed the title
christianhg
force-pushed
the
feat/transaction-interpreter
branch
from
August 26, 2026 12:05
e2b72a4 to
5ae10c0
Compare
christianhg
force-pushed
the
feat/transaction-interpreter
branch
from
August 26, 2026 12:07
5ae10c0 to
dbfac00
Compare
christianhg
changed the title
christianhg
changed the title
…d nodes Point transforms judge each operation alone, so a composite edit defeats them: a block split crosses the wire as a text diff plus an unrelated block insert, the diff collapses points into the shortened block, and the insert moves nothing because keyed paths make node inserts point no-ops. The information that content *moved* was never on the wire, so no per-operation rule can act on it. `interpretTransaction(base, patches)` derives primitive steps against an evolving working copy (a text diff's offsets only mean anything against the text it applied to) and recognizes moves two ways. Text pairing: a removal and an insertion carrying the same text become a `move.text` step, deliberately conservative because a missed move degrades to today's behavior while a false move teleports the caret: exact match only, never under two characters, node insertions pair only with earlier removals, folds into existing spans pair only with whole-node removals matching current text and landing at the destination's end, and any removal or insertion with more than one eligible partner, across both kinds, pairs with nothing. Key reappearance: an unset node whose pre-existing key reappears inside a later insertion becomes a `move.node` step (a multi-span block split unsets the tail children and reinserts them, same keys, inside the new block), with symmetric ambiguity guards: a key inserted twice or removed twice pairs nothing, and key pairing never claims an insertion text pairing already consumed, since the engine's own split output leaves the same key in two blocks. Steps sit at the insertion's slot with source coordinates mapped forward, so points travel through intermediate edits exactly once. Eleven adversarial shapes from review plus the duplicate-key document state are pinned as negative tests, and a character-identity oracle sweeps split and formatting transactions across single- and multi-span seeds at every caret position (1075 cases), with a repeated-text family asserting block-stability. The wire catalogue gains the `block-split-multi-span` fixture the field failure exposed: the single-span fixture was the only split shape captured, and the recognizers were built against what was captured, not what exists. Internal machinery only; the remote boundary adopts it separately.
…hen recovering the caret Applying a remote batch transformed the selection one operation at a time, so a split's text deletion collapsed the caret to the shortened block's end and the tail insert never picked it up; a multi-span split's child unsets stranded a caret from ` baz` after `fo`; merges snapped the caret to the merged span's end; a reordered block lost the carets inside it to its own removal step. The per-operation rules were each correct and jointly wrong, because the wire never says "moved". `setupRemotePatches` now snapshots the pre-apply value and selection, interprets the origin-filtered batch into steps, and, after the apply loop and its normalization pass, maps the old selection's points through the steps, carrying `backward`. The mapped selection is applied only when a selection existed before the batch, every point survives the mapping, it resolves in the new document, and it differs from where the per-operation transforms left the caret; on any other outcome the per-operation result stands. Pinned red-on-old by the field-replica multi-span split, the flipped single-span split expectation, span merge, block merge, combined merge, key-preserving split, and block reorder scenarios, plus the negative guards: an unrelated identical delete and insert must not move the caret, the duplicate-span-key document must not teleport it, an expanded backward selection keeps its direction, and a batch with no pre-existing selection performs no recovery. Range decorations stay on the per-operation path: the decoration machine is a private subscription whose `onMoved` contract a boundary override would bypass; giving it a batch-steps event is separate, tracked work.
… caret A merge into a block holding twin keys renames the colliding spans before deleting their block (`set` on `_key`, then a whole-block `unset`, then reinsertion under the new keys). Recognition tracks each rename's identity in `renamedKeyPaths` (the node's exact path recorded at rename time, never matched by key value alone: a rename shadowing a pre-existing twin key pairs nothing) and registers exactly those descendants of a whole-block `unset` as move candidates. The container's `remove.node` survives descendant moves, relocated after them, so never-renamed siblings invalidate instead of dangling. Probes pin the refusals: a shadowed rename, a partial collision, a chained rename, and a rename whose container moved afterward (stale recorded path: pairs nothing, degrades safely). The caret-follow browser test pins the end result: a receiver's caret in the renamed span lands in the merged block at its character position.
christianhg
force-pushed
the
feat/transaction-interpreter
branch
from
August 27, 2026 14:39
1531c04 to
79f5dc7
Compare
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.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 79f5dc7. Configure here.
| } | ||
|
|
||
| return point | ||
| } |
There was a problem hiding this comment.
Move-text skips later source carets
Medium Severity
move.text remaps only offsets inside from. Carets after that range are left unshifted, unlike remove.text. assembleSteps drops the paired remove.text, so a non-suffix move (a mid-span delete paired with a matching node insert) leaves those carets at offsets past the remaining text.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 79f5dc7. Configure here.
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.
A remote transaction arrives as raw patches: text diffs, unsets, inserts. Read one at a time they describe destruction and creation, even when the user's intent was a split, a merge, or a reorder that merely moved content. The caret, transformed patch by patch, collapses to a boundary or dies at the destructive step, ending up stranded in text its owner never touched: reproduced with two editors and a multi-span block split whose captured wire shape defeats any per-patch reading.
Three commits. The first is the library:
interpretTransactionreplays a transaction's patches against a working copy of the document and reduces them to primitive steps, pairing removals with insertions that are really one move. Pairing is deliberately conservative, because a wrong pairing teleports a caret into unrelated content while a missed one only degrades it to a boundary: matches require identity (a deleted key reappearing) or near-certainty (exact text, two characters minimum, the shape and order a real move produces), and all ambiguity, even across match kinds, pairs nothing. A character-identity oracle derives expected outcomes for every caret position of every captured scenario, and adversarial probes pin each refusal.The second commit is the wiring: after a remote batch and its deferred normalization settle, the pre-batch selection is mapped through the interpreted steps, and the editor's selection is corrected only when the mapped result disagrees with what per-operation handling produced. When the interpreter refuses to recognize a shape, the per-operation result stands, so the recovery only acts where it has a positive opinion.
The third commit extends recognition to spans a colliding block merge renames (the emission shape shipped earlier:
seton_key, whole-blockunset, reinsertion under the new keys). Renames are tracked by the node's recorded path, never by key value, a rename shadowing a pre-existing twin key pairs nothing, and the merged-away block'sremove.nodesurvives so unrecognized siblings invalidate instead of dangling. A receiver's caret sitting in a renamed span now lands in the merged block at its character position.Editor selection only: range refs and decorations stay on per-operation transforms; batching their transformation is separate, tracked work.
Note
Medium Risk
Touches collaborative selection recovery with heuristic move recognition; wrong pairing could misplace carets, though ambiguous cases intentionally fall back to existing per-patch behavior.
Overview
Remote collaborative edits no longer strand the local caret at patch boundaries when a collaborator’s transaction moved text or nodes (splits, merges, block reorders, decorator/annotation carve-ups, renamed spans in duplicate-key merges).
After each remote patch batch,
interpretTransactionreplays patches on a working copy and emits primitive steps, pairing conservative delete+insert shapes intomove.text/move.node(plus existing rekey/remove steps) instead of treating every change as unrelated destruction.mapPointThroughStepsgained handlers for those move steps.setupRemotePatchessnapshots selection before apply, runs interpretation when a selection exists, and after normalize callsrecoverSelectionto remap anchor/focus through the interpreted steps andapplySelectonly when that disagrees with per-patch selection handling.Coverage adds wire-catalogue fixtures, a large
interpret-transactiontest suite (oracles + adversarial non-pairing probes),move.textunit tests, and integration tests for splits, multi-span splits, merges, reorders, and duplicate-key merge receivers.Reviewed by Cursor Bugbot for commit 79f5dc7. Bugbot is set up for automated code reviews on this repo. Configure here.