Repository navigation
fix(loro.js): include reattached child containers' state in diff() and revertTo() - #1131
Conversation
A child container without ops in the range was emitted only as a reference, so applyDiff/revertTo recreated it empty. Emit its full state, as Rust does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
WASM Size Report
|
…le in diff() Emit a child's whole state only when it is unreachable at `from`, replace a partial diff for such a child, include revived tree node metadata, and order the result parent-first. Moved movable-list children and mergeable children keep only their own ops. applyDiff now moves an existing movable-list child instead of recreating it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Addressed all five P1s from the adversarial review in 08840f4. New rule.
Each item has a regression test in the new Cross-check with the Rust
Move-diff performance: MovableList
Restoring a deleted Text child is O(returned content): 1k 0.58 ms, 100k 13.5 ms, 1M 114 ms.
Checks: |
…ved-child-content
…ve tree subtrees in diff() Port Rust's MovableListHandler::apply_delta so a moved child becomes one move instead of reordering the whole list, undo movable-list moves through their recorded neighbors, and emit creates for every node that becomes reachable, including descendants of revived nodes and nodes moved out of a deleted ancestor. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Round 2 addressed in f6f28cd; B2: one move became N−1 moves. B1: undo of an applied move. B3: tree reachability through ancestors. Before the reachability pass, Cross-check with Rust/WASM. The WASM build is from this branch's tree (Rust
New tests in applyDiff on a MovableList (median of 5 runs):
The remaining 4 ms at 100k is the cost of one plain
Mergeable vs. #1134. (Edited: this paragraph described an earlier #1134 design that has since been dropped. See the round-3 comment below.) #1134 (b57170c) keeps Checks: loro-js 19 files, 256 tests; |
…oot in diff() On a shallow doc a node created before the root has no retained op at the root version; use its root-state placement instead of treating it as revived. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Addendum: 784f8e7. Running the combined tree (main + #1126→#1127→#1128 + this PR + #1133) exposed an interaction. On a shallow doc, a node created in the shallow root commit has no retained op at the root version. The fix: when a node has retained history but no op at the version, its placement is taken from the shallow root state ( Combined tree: 22 files / 292 tests pass; format, lint, typecheck, and build pass. It has two trivial conflicts:
The Rust/WASM cross-checks on the combined tree match Rust:
This branch: 19 files / 257 tests pass. |
…ved-child-content
…peated moves, and restore mergeable markers on undo applyDiff now finds moved children through their inserted ids and places every inserted item after its final-order predecessor, so unmerged delete items, several children moved out of one range, and scalar-only deletes are handled without index bookkeeping or scans. Undo reverts earlier moves of an element that only this document moved since. A wide revived subtree is walked iteratively. Undoing a mergeable key deletion re-ensures the marker. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Round 3 addressed in e7415e2; B2/B3/N2: movable-list
So several children moved out of one range work, and unplaced moved elements never shift the target indices. B1: undo of repeated moves. Undo of a move now finds that op's entry in the element's move history. Moves made later by this document do not block it; these include the inverse ops written while undoing a later move in the same group. A later remote move still blocks it, as before. B4: wide subtree. The revived subtree is walked with an explicit stack in preorder, with child indices taken from the traversal. There is no argument spread and no N1: mergeable undo. Undoing a deleted mergeable key now re-ensures the marker ( Cross-check with Rust/WASM (dev nodejs builds of the current
New tests in
MovableList
The ~4 ms at 100k is the constant of one Wide subtree revival, plain node, median of 3 (ms):
The remaining superlinear part is the output itself. With 100k appended siblings the fractional-index strings grow, and
Checks: this branch passes 19 files / 265 tests, with Combined tree: The PR description and my round-2 comment no longer describe the old #1134 design. Mergeable handling follows #1134 b57170c (delta relative to the hidden state; |
… subtree in diff() A node alive at `from` inside a subtree revived by the range (e.g. a child moved out of a deleted parent and then reverted back under it) was created again while its old copy stayed. Delete the topmost such nodes first, as Rust's diff does, then create the subtree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Fixed the P1 from the #1126–#1128 combined review ("#1131 duplicates a child that was already alive") in d8cb391. Cause. In Fix. Within a revived subtree, the nodes alive at Cross-check with Rust/WASM (
On e7415e2 the diff was The new test is "revives a parent whose child stayed alive elsewhere without duplicating it", with and without a grandchild. It checks the action sequence, Checks. This branch: 19 files / 266 tests; Performance. The only added work is an alive-at- Mergeable alignment still waits for #1134's final design (now 1605c7b/92e4780b: full-state public |
…delta validation, and linear tree revival - Undo only lets later moves the UndoManager tracks (its recorded edits and the inverses it wrote) pass; excluded-origin and remote moves block. - Each move records its physical successor; undo puts the element back there, so redo after mixed moves and inserts restores the original order. - applyDiff validates list deltas for the whole batch before applying, hydrates lazily imported lists before looking for moved children, and deletes only the gaps between kept positions. - Tree revival memoizes "alive at from" along ancestor paths and emits the deletes of moved-in live nodes first, with their index at `from`. - Undo restores a mergeable marker over a later overwrite. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ved-child-content
|
Round 4 addressed in dc5106c;
I found one existing gap that is still open: redo of an overwrite by another container (key New tests in Performance. loadavg was 14–24 during all runs, so treat single numbers with care. Median of 3 (ms):
Wide subtree, 2 alternating rounds (ms):
These overlap under this load; the only change on this path is the memoized alive check. The cost is dominated by the fractional-index strings in the output, as you measured. One Text child plus N scalars deleted (
Checks. This branch: 19 files / 274 tests; The mergeable diff/applyDiff protocol is still waiting on #1134's final design (full-state diff plus apply-side alignment vs. incremental). I haven't changed it in this round. |
…ved-child-content # Conflicts: # loro-js/src/runtime/document.ts
…not moved since The old successor may have been moved by an untracked op; prefer the old predecessor, then the successor, and write no op when the element is already there. Matches Rust in 70 of 72 neighbor-change cases (the other two: Rust revives a deleted element as an empty container). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Drop tracked ranges that end before the earliest stacked op of their peer, and all of them on clear/destroy; only later moves are ever checked. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…in apply order Put the diff's own deletes and the moved-in deletes first, resolve each parent's sibling order at `from` from history (nodes that only exist at `from` included), and count indexes with a Fenwick tree as the deletes are applied. Fixes indexes next to new roots and the quadratic rescans. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Simulate list lengths in batch order before any write: lists created by an earlier entry start empty, repeated entries see the previous result, and the lookup creates nothing (no empty root after a rejected batch). Entries whose bounds were checked skip the second validation. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ith Rust #1134 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ved-child-content
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Round 5 + Rust #1134 alignmentHead: Blocking items
Mergeable alignment with merged Rust #1134 (
|
| Benchmark | main | PR | Note |
|---|---|---|---|
diff after deleting a tree root, deep, 10k |
0.2 ms (1 item) | 13.7 ms (10,001 items) | main omits the subtree (the bug); PR is linear |
| same, wide, 10k | 0.2 ms (1 item) | 17.0 ms (10,001 items) | same |
applyDiff MovableList move-to-end, 100k |
0.08 ms / 2 ops (delete + fresh empty child) | 3.6 ms / 1 move op | The PR time is the first-move lamport index build, which a plain list.move also pays on main (first 4.1 ms, then 0.04 ms). A second move through applyDiff takes 0.18 ms. |
applyDiff MovableList move-to-end, 10k |
0.05 ms | 0.43–0.60 ms | same one-time index |
| Batch validation, 1k / 100k / 1M delta items | 0.02 / 1.50 / 5.9 ms | 0.09 / 0.97 / 10.1 ms | Simulated-length validation of the whole batch before writing; 100k is faster |
diff of a moved child, 1M-char text |
0.021 ms | 0.025 ms | noise level |
| JSON import/export (binary, movable, counter) | 23.1 / 12.6 / 5.4 ms | 22.7 / 13.2 / 5.3 ms | flat |
| b4 apply / snapshot import / update import | 248 / 116 / 175 ms | 241 / 115 / 173 ms | flat; main round 1 had a 429 ms apply outlier |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…port Pruning visited both stacks on every commit (quadratic sessions), and compared a peer's ranges only with that peer's items, so after a peer change it could drop a range a stacked move still needed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fore writing Tree diff items now apply in order, as in Rust: moves of live nodes (parent first), then the topmost deletes, then revived subtrees, with every index counted in the tree the earlier items leave. applyDiff applies tree items in order and dry-runs them with the list checks, so a failing batch writes nothing. A node hidden at the target version is emitted as a delete. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… time The reachability filter and tree revival checked each node's ancestors without a cache (quadratic in depth). Fractional indexes are now built as flat strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A create of a node that is alive here moves it, a move of a missing or deleted node creates a new one, and a delete of a deleted node does nothing, so default applyDiff on a doc holding a mergeable tree's hidden state no longer duplicates its nodes. A regular map child is always set as a new container, so its full state is not appended to a hidden copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The CI runner is several times slower than a dev machine, so the 1.5 s bound flaked; compare 8,000-deep with 2,000-deep instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 6Head: Blocking items
After NB1, Random tree oracle (300 concurrent two-peer histories, 1,200 version pairs; the target is
There are no regressions against main or Atomicity fuzz (30,000 random tree batches): main writes a partial state in 59; this PR in 0. It accepts exactly the same 29,826 batches as main, so no valid batch is rejected. Non-blocking items
Checks
PerformanceThree alternating rounds of main
🤖 Generated with Claude Code |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ved-child-content
|
Follow-up to the round-6 comment above. The head is now
Checks:
🤖 Generated with Claude Code |
…ved-child-content # Conflicts: # loro-js/src/runtime/document.ts
#1131 now revives re-attached children in #emit itself (#completeDiffEntries), so take its version and drop #reviveAttachedChildren. One case is left: its reachability exemption covers every sequence, but a listener only keeps a moved child for a movable list. For events, apply the exemption to movable lists only, so a child that a whole-value List diff (the replay fallback) deletes and re-inserts is revived. diff()/revertTo keep #1131's behavior. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…path Keep main's one-level _isContainerDeleted for getContainerById and the diff/event reachability walk, and use the ancestor walk only in isDeleted(), getPathToContainer() and getCursorPos(). Drop the guard on local edits to deleted containers: it threw on a redo that main allows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Seeds 0-119 with nested children and snapshot imports pass on main now that #1131 is in. See context/loro-js-differential-fuzz.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* test(loro.js): add a multi-peer differential fuzz harness Random multi-peer edits, update exchanges in several orders, and checkouts, checked against replay, checkout-at-version, snapshot, shallow-snapshot, and event-mirror oracles, and optionally step by step against a loro-crdt WASM build. Failures are grouped by signature, minimized, and printed as loro.js calls. The default run is a fixed 120-seed Map/List/Text/Counter set that needs no WASM build (~2 s). Seeds that fail on main are listed with the fix that makes them pass. Usage and findings: context/loro-js-differential-fuzz.md. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(loro.js): record the fuzz findings and their PRs Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(loro.js): state which PR the lockstep measurement used Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(loro.js): run nested-container fuzz seeds in CI Seeds 0-119 with nested children and snapshot imports pass on main now that #1131 is in. See context/loro-js-differential-fuzz.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ro-dev#1147) * fix(loro.js): treat containers under deleted ancestors as deleted isDeleted() only looked at a container's own parent binding, and a tree node only at its own deleted flag. A child of a removed map value, or a node under a deleted tree node, therefore counted as alive: it kept a path, getNodes() listed it, and local edits to it recorded ops on invisible state. Rust's DocState::is_deleted follows the whole chain and rejects such edits with ContainerDeleted. Walk the parent chain (and tree ancestors) for isDeleted, getPathToContainer, and a new guard in #appendAndApply that throws Rust's message. getContainerById keeps its one-level check, since Rust still resolves those IDs. nodes() now returns every node and getNodes() lists alive nodes breadth-first, as Rust does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(loro.js): keep deleted-container checks off loro-dev#1131's reachability path Keep main's one-level _isContainerDeleted for getContainerById and the diff/event reachability walk, and use the ancestor walk only in isDeleted(), getPathToContainer() and getCursorPos(). Drop the guard on local edits to deleted containers: it threw on a redo that main allows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(loro.js): list tree nodes without spreading child lists into push() getNodes() spread each child list, and with withDeleted the deleted subtrees, into one push() call, which throws a RangeError past V8's argument limit (about 125k on Node's main thread). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
Found while porting the Rust #1124 test fixture (
shallow_checkout_equal_value.rs,revert = true) toloro.js.Its
map.subis a child map{"k":"v"}that gets replaced by a string after the root, followed byrevert_to(root). Rust restores{"k":"v"};loro.jsrestores{}.Root cause
LoroDoc.diff(from, to)only emits diffs for containers that have ops in the range. When the range attaches a child container again without editing it, for example a map key set back to an older child, the parent diff references the child, but the child's own state is never emitted.applyDiff, and thereforerevertTo, then recreates the child empty.Rust's diff for the same scenario emits the parent entry, the child map with
{k: "v", text: <Text>}, and the nested Text withinsert "hi".loro.jsnow emits the same three entries.Fix
After the changed-container diffs are built, walk the child containers attached by map/list diffs. Every child that has no entry yet gets a full-state diff (
containerDiff(child, undefined)), and this recurses into nested children. Parent-before-child order is kept. The extra work is proportional to the attached children's content, which is part of the returned diff anyway.Validation
loro-js/tests/runtime.test.ts("reverts child containers whose content did not change in the range"): the diff includes the child map and its nested Text, andrevertTorestores a map child and a list child, including a nested Text. It fails onmainbecause the child has no diff entry.mainfor the same scenario:diffyields the same three entries, andrevert_togives{"map":{"sub":{"k":"v","text":"hi"}}}.pnpm --dir loro-js test(17 files, 241 tests),typecheck,lint,format:checkall pass.bench:complexity 1000,2000,4000,8000back to back againstorigin/main: one entry is above 1.5x,history-concurrent-movable-mixed-switch-subscribed@2000 at 0.345 → 0.526 ms, which does not calldiff().history-tail-diffstays at 0.12–0.40 ms. A first run under machine load showed broad slowdowns in unrelated paths (commit, import,toJSON), and they did not reproduce.Related: #1124 (the fixture that exposed it).
Current state (after review rounds 1–6, aligned with merged Rust #1134)
diff()returns the full state. A container that is unreachable atfromand reachable attocarries its whole state:creates for revived nodes with their whole subtree and metadata, including nodes moved out of a deleted ancestor;Entries are in parent-first order. Containers that are unreachable at
to(deleted, under a deleted tree node, or a hidden mergeable child) have no entry, as in Rust.Tree diffs are sequences, as in Rust. Items come in this order:
Each index is counted in the tree the earlier items leave, in O(changed · log n) regardless of the sibling count. A node hidden at
tois reported as deleted.applyDiffapplies tree items in order and resolves them as Rust'sapply_diffdoes: a create of a live node moves it, a move of a missing or deleted node creates one, and a delete of a deleted node does nothing. Details:context/loro-js-tree-diff.md.applyDiffis incremental by default.applyDiff(diff, { fullState: true })aligns each re-activated mergeable child with the receiver's state:TreeID.A full-state target with no entry is cleared.
revertToand undo apply only the actual change, so identity is kept. A regular map child is always set as a new container, as in Rust.Events match Rust main.
{ fullState: true }forimport/checkoutbatches, as with loro-crdt; with the default, it applies that full state on top of its hidden copy. This is a behaviour change from loro.js main, noted in the changeset.Batch checks before writing. Before writing anything,
applyDiffchecks:An out-of-range list delta or a tree item that cannot apply rejects the whole batch. Other malformed entries (text ranges, counter values, type mismatches) can still leave earlier entries applied, as before and as in Rust.
Undo reverts MovableList moves. The element goes next to its old physical neighbour if that neighbour has not moved since. The UndoManager prunes its tracked ranges from the stack bottoms by lamport, so a commit costs the same as on main whatever
maxUndoStepsis.Known differences:
[e0,e1]: Amove(0,1)‖ Binsert(0,"b")). The MovableList model rework (test(loro.js): add a Rust differential suite for MovableList #1132) fixes it.The review responses and measurements are in the PR comments.
🤖 Generated with Claude Code