Skip to content

fix(loro.js): emit concurrently deleted text once in checkout events - #1140

Closed
zxch3n wants to merge 1 commit into
mainfrom
fix/loro-js-checkout-duplicate-deletes
Closed

zxch3n wants to merge 1 commit into
mainfrom
fix/loro-js-checkout-duplicate-deletes

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 27, 2026

Copy link
Copy Markdown
Member

Summary

When several peers deleted the same text concurrently, loro.js checkout events were wrong. The document state was right.

  • Restoring the text emitted it once per delete: { insert: "bccb" } instead of { insert: "bc" }. With astral characters the doubled text also split surrogate pairs.
  • A checkout that restored one such character and removed another threw event diff position 1 is out of range:
p1.getText("t").insert(0, "ab");            // everyone imports this
p4.getText("t").delete(0, 1);               // a
p2.getText("t").delete(1, 1);               // b
p3.getText("t").delete(1, 1);               // b, concurrently
// doc has everything and a subscriber
doc.checkout([{ peer: "1", counter: 1 }, { peer: "4", counter: 0 }]);  // "b"
doc.checkout([{ peer: "2", counter: 0 }, { peer: "3", counter: 0 }]);  // threw

This is the "event diff position N is out of range" failure seen in random fuzzing, including on full-history documents.

Rationale

#applyVersionTransition collects the elements touched by each delete op in a Set<SequenceElement>. Text stored in packed spans returns a new scalar view object per lookup, so an element deleted by two ops ends up in the set twice. The removal and insertion lists were computed from that set before any mutation, so both copies were emitted. Lists are unaffected because their elements are stable objects. The fix deduplicates the collected elements by ID before building the event.

Validation

  • New tests in loro-js/tests/event-diff.test.ts: restoring bc and a😀b, and the restore+double-delete checkout. All three fail on main. The expected events are what loro-crdt (WASM build of this main) emits for the same scenarios.
  • Differential fuzz (harness in a separate PR) with events mirrored into a model: on a branch with this and the other Text/List fixes, 500 Map/List/Text/Counter seeds pass the event oracle. The out-of-range throws, doubled inserts and surrogate splits were all this bug.
  • pnpm --dir loro-js format:check, lint, typecheck, test (250 passed), build.
  • bench:complexity (1k–8k), 5 alternating rounds vs main, loadavg 9–10: 284 items, none both >1.5× and >0.3 ms slower. Total 1003 ms on main, 954 ms here.

🤖 Generated with Claude Code

A version transition collects the elements each delete op touches in a
Set. Packed text spans return a new scalar view per lookup, so text that
two peers deleted concurrently was collected once per delete. Restoring
it emitted the text twice; removing it deleted twice from the event
diff and could throw "event diff position N is out of range".

Deduplicate the collected elements by ID before building the event.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

WASM Size Report

  • Original size: 3210.11 KB
  • Gzipped size: 1062.59 KB
  • Brotli size: 744.13 KB

zxch3n commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Closing: covered by #1136 (e74d64dc), which collects transition elements in a SequenceElementSet keyed by ID. That is the same deduplication this PR did at the consumption site.

Checked on main at c0ff1255: restoring concurrently deleted bc / a😀b emits the text once, and the restore-one/remove-one checkout no longer throws event diff position 1 is out of range. The events equal loro-crdt's, and all 9 tests from this PR's tests/event-diff.test.ts pass on main.

@zxch3n zxch3n closed this Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant