fix: restore deleted mergeable children once on revert/apply_diff/undo

[zxch3n]

Summary

After a mergeable child's map key is deleted, going back to a version where the child was visible added its content a second time. revertTo turned "hello" into "hellohello", a Counter 7 into 14, and ['keep'] into ['keep','keep']. The same happened for nested mergeable containers, in undo/redo, and in diff(a,b) + applyDiff on the same doc.

Root cause

A mergeable child that becomes visible again keeps its deterministic cid and its hidden state. Revival reports its full state, and a doc that kept the hidden state applied that full state on top of it.

Contract (backward compatible)

• Events keep main's shapes. Local transactions report the marker plus their own ops, so a plain re-ensure emits only the parent marker. Import/checkout report full state. Mirroring events therefore still requires the receiver to share the source's hidden state, as on main; to build a mirror from scratch, use diff() or a snapshot.
• applyDiff is incremental by default, as on main.
• Opt-in alignment for diff() results. Rust: LoroDoc::diff marks its batch (DiffBatch::is_full_state / set_full_state), and apply_diff aligns only such batches. JS: applyDiff(diff, { fullState: true }) (ApplyDiffOptions). A re-activated mergeable child's full state is then aligned with whatever the target holds at its cid (none, the same, or different):
  • Map: per key.
  • Counter: adds the difference.
  • Text: keeps the common prefix/suffix by content and re-marks style-only differences.
  • List / MovableList: keeps the common prefix/suffix; moved movable-list children keep their id.
  • Tree: by TreeID.
• When to opt in: when applying diff() to a doc that may keep the deleted child (the same doc, a fork, another history). Without it, such a doc gets main's behavior (appended). A doc without hidden state gets the same result either way.
• Revert and undo apply the actual change (KeptChange next to revival full states; DiffBatch::from_changes). They write only the real difference and keep char, element and TreeID identity.
• UndoManager transforms against the actual change, so a peer can still undo its own edit after a remote undo re-activates the child.

Known limitations

These are documented in context/mergeable-containers.md:

• Alignment is content-based, and each receiver writes its own ops. Two receivers aligning the same batch against the same hidden state and then merging is convergent but not idempotent.
• Counters stay additive.
• revert_to treats remote hidden edits like a root container would; undo of a delete keeps them.

Validation

• crates/loro/tests/mergeable_revert.rs (33 tests, 1 of them an ignored perf test). It covers every reproduction from the three review rounds, including reordered MovableList/List children, style-only restyle with a concurrent edit, emoji/UTF-16, re-ensure event mirroring, and cross-peer undo.
• WASM mergeable.test.ts: 17 tests.
• Public diff() matrix (6 kinds × source hidden same/modified × target absent/same/different): 36/36 with fullState; 12/12 on the default path for targets without hidden state.
• loro-js reading WASM events from a source that shares hidden state: re-ensure 3/3, revert 4/4.
• cargo test -p loro -p loro-internal: 1223 passed. pnpm check is clean. WASM: 393 passed; CJS, tsc, deno and bun pass.
• Combined with fix: reject malformed movable list move/set ops on import instead of aborting #1125 + fix: reject creating unknown-type containers instead of panicking #1142 (with fix: reject creating unknown-type containers instead of panicking #1142's own conflict against main resolved locally): 1246 passed, 0 failed.

The performance table is in the PR comment.

🤖 Generated with Claude Code

[github-actions]

WASM Size Report

• Original size: 3235.44 KB
• Gzipped size: 1073.75 KB
• Brotli size: 751.15 KB

[zxch3n]

Addressed the adversarial review of ef61c31 by redesigning the fix (b57170c; main, including #1123, is merged in).

What changed

I dropped the apply-side reconcile (handler/full_state.rs) completely. apply_diff is back to main's purely incremental semantics. The fix is now on the recording side:

• LoroDoc::diff and the undo diff closure record with DocState::mergeable_revival_as_delta set.
• In that mode, a mergeable child re-activated by the diff of a container that keeps its id (not itself revived) is not revived as full state. Its own calculator delta (or nothing) goes into the batch, because the doc the batch is applied to still holds the hidden state at the same deterministic cid.
• A mergeable child under a parent that is revived under a fresh id (e.g. a map in a deleted list element) resolves to a fresh cid and still gets full state.
• Checkout/import events are unchanged.
• Core diff is about 40 lines in state.rs / loro.rs.

Blocking items

1. Forwarding local events to applyDiff. The source now writes real CRDT deltas on the hidden state, and apply_diff applies increments. So events forwarded to a doc in the same state reproduce the source. Counter hidden 7→7 gives 7 on both docs; hidden 10→7 gives 7 on both. Text, List, Tree and nested Text are covered too, in Rust and in WASM (subscribe → applyDiff). Every scenario in the shared check harness now also asserts event forwarding.
2. Tree concurrent meta edit. Revert uses the diff calculator's delta, so it deletes only the extra node and keeps n's TreeID. B's remote meta survives the merge (revert_keeps_tree_node_for_concurrent_meta_edit).

Concurrency and semantics

• Diff-based restore keeps identity. Hidden hello! → deletes only !; [keep, extra] → deletes only extra. Two peers reverting concurrently merge to the target (Text, List, MovableList, nested Text, Tree with the original TreeID). Counter stays additive (10→7 on two peers gives 4), the same as a root counter. That is documented and asserted in the WASM test.
• revert_to now behaves like a root container: a remote edit to the hidden child that was already observed is reverted, and one that arrives later merges. Both orders are tested.
• Undo of a delete only restores the marker, so a remote edit made while the child was hidden survives in either arrival order. I chose this over "reset to the undone version": undo should only revert local operations, and the explicit version target is what revert_to is for.
• Trade-off: LoroDoc::diff output for a re-activated mergeable child is now a delta relative to its hidden state, not a full state a JSON mirror could use. That matches what local events already carry. It is documented in context/mergeable-containers.md and the changeset. loro-js fix(loro.js): include reattached child containers' state in diff() and revertTo() #1131 should follow the same rule.

Validation

• mergeable_revert.rs now has 24 tests; the same file fails 21/24 on main and 17/24 on ef61c31.
• cargo test -p loro -p loro-internal: 1198 passed, 0 failed, 15 ignored.
• pnpm check: clean.
• pnpm -C crates/loro-wasm build-dev && test: 382 passed (mergeable 11/11); CJS, tsc, deno and bun pass.
• The in_batch / full_state_targets bookkeeping is gone, so the regular path has no added per-container work.

Performance

Release build. Median of 5 builds of revert_to + commit; main and the PR were built in separate worktrees with separate CARGO_TARGET_DIRs. Two runs each.

case	main ms	PR ms	main ops	PR ops
plain Text, revert 20k inserts	2.88 / 2.45	2.84 / 2.49	20,000	20,000
5k regular Text children restored	27.4 / 21.7	25.8 / 27.0	30,000	30,000
5k mergeable Text children restored	28.0 / 29.0	12.7 / 13.1	30,000	5,000
100k-char Text, hidden equal	0.201 / 0.150	0.010 / 0.008	100,001	1
100k-char Text, extra !	0.333 / 0.272	0.174 / 0.173	100,001	2
1M-char Text, extra !	2.28 / 2.24	1.67 / 1.63	1,000,001	2
100k-item List, hidden equal	64.7 / 66.7	0.044 / 0.037	100,001	1
100k-item List, one extra	64.7 / 66.0	0.080 / 0.079	100,001	2

The regular paths vary run to run on both sides; the code they run is unchanged.

🤖 Generated with Claude Code

[zxch3n]

Round-2 review of b57170c is addressed in 1605c7b. Main is merged in up to 18924fa.

B1: public diff() must stay portable

You were right that I should not have changed the public contract. Events and public diff() are back to main's shape: a container that becomes visible carries its full state. What changed:

• KeptChange. A container revived while it kept its state (a re-activated mergeable child, and what it holds) now also records its actual change, in InternalContainerDiff and ContainerDiff::kept.
• Revert and undo apply DiffBatch::from_changes, which uses the actual change, so they keep the identity-preserving behaviour from round 1. They record with record_changes_only, an optimization that skips building full states nobody reads; the output is the same.
• Public apply_diff aligns a re-activated mergeable child's full state with whatever the target holds at its cid (handler/full_state.rs):
  • Map: per key.
  • Counter: adds the difference.
  • Text / List / MovableList: keeps the common prefix and suffix (chars with equal styles; values, containers by id) and replaces the middle.
  • Tree: by TreeID. Missing nodes are created, kept ones are moved if needed, the rest are deleted.
  • Kept children are handed down. No entry means empty.
• Local events keep (a), (b) and (c) consistent. A local transaction that writes a mergeable marker now reports the child's full state (plus the containers it holds), with the transaction's ops as its KeptChange (DocState::revive_mergeable_in_local_diff). So the same event shape as import/checkout holds everywhere, and applyDiff can always treat it as full state. This also fixes a gap on main: ensure_mergeable_* over a deleted key used to emit no child content, so JS mirrors could not follow it.

Your matrix, rerun (6 kinds × source hidden same/modified × target absent/same/different): 36/36 reach the target. Counter hidden 10→7 on a fresh doc gives 7, and nothing goes out of range.

Every scenario in the shared test harness also checks:

• event forwarding to a fork;
• event forwarding to a visible-only doc, built with apply_diff(diff([], v)), which has no hidden state;
• diff applied to a visible-only doc.

B2: a peer can undo its own edit

UndoManager::compose_remote_event now composes ContainerDiff::change(), not the full-state diff. A re-activated child with unchanged content therefore contributes nothing to the transform base, and B's retain 5, delete 1 stays in bounds.

peer_can_undo_own_edit_after_remote_undo_reactivates_child covers Text, List, MovableList and nested Text × both arrival orders. The "delete arrives first" order, which was already broken on main, is fixed too. The WASM test covers the Text case.

Validation

• mergeable_revert.rs now has 28 tests. The same file fails 25/28 on main and 17/28 on b57170c, including public_diff_*, peer_can_undo_* and local_reactivation_event_carries_full_state.
• cargo test -p loro -p loro-internal: 1208 passed, 0 failed, 15 ignored.
• pnpm check: clean.
• WASM build-dev + test: 384 passed (mergeable 13/13); CJS, tsc, deno and bun pass.
• Combined with fix: keep root-time map values when checking out a shallow doc #1124 + fix: reject malformed movable list move/set ops on import instead of aborting #1125, merged locally with no conflicts: 1221 passed, 0 failed for both fix: keep root-time map values when checking out a shallow doc #1124 heads. That covers 722777d and ae18063, the current head.
• Not done: the optional loro-js consumer check of WASM batches. The batch shape is now main's again.

Performance

Release build. Median of 5 builds; main 106d9e5 and the PR were built in separate worktrees with separate CARGO_TARGET_DIRs. Two runs each (ms):

case	main	PR	ops main → PR
plain Text, revert 20k inserts	2.82 / 2.77	2.61 / 2.56	20,000 → 20,000
5k regular Text children	30.2 / 29.2	27.6 / 27.2	30,000 → 30,000
5k mergeable Text children	33.5 / 30.2	14.4 / 14.3	30,000 → 5,000
100k-char Text, hidden equal	0.183 / 0.217	0.008 / 0.009	100,001 → 1
100k-char Text, extra !	0.282 / 0.283	0.204 / 0.190	100,001 → 2
1M-char Text, extra !	2.78 / 2.33	1.75 / 2.28	1,000,001 → 2
100k List, hidden equal	66.3 / 67.4	0.047 / 0.047	100,001 → 1
100k List, one extra	65.7 / 66.2	0.081 / 0.085	100,001 → 2
100k List, one extra, revert with subscriber	69.2 / 68.4	4.37 / 4.32	100,001 → 2
100k List, one extra, public diff+apply_diff	68.1 / 69.8	11.0 / 10.7	100,001 → 2
100k-char Text, extra !, public diff+apply_diff	0.283 / 0.287	0.621 / 0.630	100,001 → 2

The subscriber row pays for building the full-state event, which is the contract. Public apply_diff pays an O(n) alignment; for Text that is 0.6 ms against main's 0.28 ms, and main's result was duplicated content. _apply_diff only tracks per-container bookkeeping when alignment is on (public apply_diff); revert and undo skip it.

Trade-offs, also in context/mergeable-containers.md

• Aligning against a different hidden state is content-based. Identity is kept only for the shared prefix/suffix and for tree nodes by TreeID, and the edits are new ops of the applying doc.
• Counters stay additive: concurrent 10→7 reverts give 4, like a root counter.
• revert_to reverts a remote hidden edit it had already received and merges one that arrives later. Undo of a delete keeps remote hidden edits in either order.

🤖 Generated with Claude Code

[zxch3n]

Round 3 is addressed with option 1: alignment is opt-in, and event shapes stay as on main. Commits 6574efd and a3bcbbc; main is merged up to 27982a5, including #1135 and #1138.

Why opt-in

A [cid, diff][] batch cannot say whether a re-activated mergeable child's entry is its full state (diff()) or an increment on shared hidden state (events). For a plain re-ensure, "no entry" means "unchanged" in a local event but "empty" in a full-state diff. I confirmed this before switching designs: aligned apply plus main-shaped events gave a mirror {c: 0, t: "h"} against a source {c: 7, t: "hello"}.

So:

• applyDiff stays incremental by default.
• LoroDoc::diff marks its Rust batch (DiffBatch::is_full_state / set_full_state).
• JS opts in with applyDiff(diff, { fullState: true }). ApplyDiffOptions is exported in the TS types and documented on applyDiff.

B1: MovableList children

On top of #1138 (delete + re-insert of a child becomes a move), the element loss is gone. The remaining duplication (["AA","BB"]) came from moved children not being marked as full-state targets. They are marked now. A plain list recreates its middle children under fresh ids, so only its prefix/suffix children are marked.

The test covers Map/Text children × {swap, rotate, reverse, delete, insert new} × target same/absent/different hidden, plus revert_to. The WASM test covers the reported case.

B2: event shapes

Local events are main's again. revive_mergeable_in_local_diff is removed, so a re-ensure emits only the parent marker.

• Rust local_reensure_event_keeps_main_shape checks that forwarding such events to a doc sharing the hidden state reproduces the source.
• The WASM test covers the Counter/Text/List re-ensure mirror.
• loro-js, reading WASM events with containers encoded as 🦜:cid: re-ensure 3/3 (Counter stays 7, Text hello, List [keep]), local revert 4/4.

Round-2 (b) (source local events → a doc that shares the hidden state) still holds. revert_to/undo write actual changes, so their local events are increments on shared hidden state.

Non-blocking items

• Quadratic commit cost: gone with the removed local revival (numbers below).
• Style-only differences: Text alignment now matches content only. Kept chars whose style differs are re-marked (null unmarks), not deleted and re-inserted. Bold over plain hello writes the marker plus one mark, and a concurrent h→H merges to Hello (was helloH).
• 1M-char text alignment: now compares bytes and walks style runs instead of per-char vectors. Emoji, combining marks and CJK are tested in Rust and in WASM (UTF-16 units).
• Concurrent alignment is not idempotent: documented as a known limitation in context/mergeable-containers.md. For example, two receivers aligning hello over hXYZo and merging give hellello. To restore once across peers, have one peer apply and sync the updates.

Validation

• mergeable_revert.rs: 33 tests (1 ignored perf test).
• WASM mergeable: 17/17.
• cargo test -p loro -p loro-internal: 1223 passed, 0 failed, 17 ignored.
• pnpm check: clean.
• WASM build-dev + test: 393 passed; CJS, tsc, deno and bun pass.
• Round-2 matrix: 36/36 with fullState; default path on targets without hidden state 12/12.
• Combination with fix: reject malformed movable list move/set ops on import instead of aborting #1125 (50d200d) + fix: reject creating unknown-type containers instead of panicking #1142 (9bd96f3): 1246 passed, 0 failed.
  • fix: reject creating unknown-type containers instead of panicking #1142 conflicts with main by itself (GitHub reports CONFLICTING), in movable_list_apply_delta.rs, independent of this PR. For the combined run I took main's fix: apply multi-child MovableList diffs as moves without losing elements #1138 version and applied fix: reject creating unknown-type containers instead of panicking #1142's Handler::new_unattached(..)? propagation.

Performance

Release build, main 27982a5 vs this PR, separate worktrees and CARGO_TARGET_DIRs, interleaved runs; each cell is the median of 5. The machine has 18 cores; other sessions' builds kept loadavg at 9.5–18, as recorded per run in the logs. It never dropped below 4 in about 20 minutes of waiting, so treat small differences as noise.

case	main ms	PR ms	ops main → PR
plain Text, revert 20k inserts	2.40–2.56	2.37–2.62	20,000 → 20,000
5k regular Text children revert	21.5–41.6	20.5–23.4	30,000 → 30,000
5k mergeable Text children revert	24.2–55.1	10.1–11.2	30,000 → 5,000
100k Text, hidden equal, revert	0.146–0.209	0.007–0.010	100,001 → 1
1M Text, extra !, revert	2.19–2.98	1.59–1.69	1,000,001 → 2
100k List, one extra, revert	63.0–77.9	0.078–0.082	100,001 → 2
same, revert with subscriber	65.5–77.0	0.082–0.097	100,001 → 2
100k List, one extra, diff + apply_diff (full state)	63.8–71.0	9.2–11.4	100,001 → 2
100k Text, extra !, diff + apply_diff	0.27–0.35	0.28–0.48	100,001 → 2
1M Text, extra !, diff + apply_diff	2.47–3.43	2.70–3.72	1,000,001 → 2
commit 5k new mergeable children, subscribed	7.1–11.0	7.2–10.8	—
commit 20k new mergeable children, subscribed	36.2–58.2	38.9–46.5	—

The 1M-char diff + apply_diff row went from 6.4–8.6 ms in round 3 to parity with main, and main's result there is duplicated content. The subscribed-commit rows are back to main's cost; round 3 measured 1.26–2.17 s for 20k.

🤖 Generated with Claude Code

[zxch3n]

Round-4 follow-ups are in 5ff481a. main (27982a5) is already included; I re-fetched and it hasn't moved.

1. DiffBatch::compose keeps the full-state flag (internal undo.rs):
   • An empty side is the identity, so default().compose(&full) adopts full_state = true. Your hellohello repro now gives hello.
   • Two full-state batches stay full-state. A child the second batch re-activates was hidden where the batches meet, so the first has no entry for it, and the first batch's own revivals followed by increments are still full states.
   • Two incremental batches stay incremental.
   • Mixing a non-empty full-state batch with an incremental one panics with a clear message, because neither mode can apply the result. UndoManager only composes incremental change batches, so it is unaffected.
   • clear resets the flag.
   • Tests: diff_batch_compose_keeps_full_state (empty + full, full + full; the applied result is checked) and diff_batch_compose_rejects_mixed_modes.
2. Documented which batches may carry the flag, in the set_full_state Rust doc, the ApplyDiffOptions.fullState / applyDiff JS docs, the internal field doc and context/mergeable-containers.md:
   • diff() results may, including after a JSON round trip.
   • Import/checkout events may. I confirmed they revive children with full state; import_events_may_be_applied_as_full_state forwards import events and gets hello with the flag and main's hellohello without it.
   • Local-commit events must never carry it. A re-ensure has no child entry because the child is unchanged, and full-state reads that as "empty" (a counter at 7 becomes 0).

Validation:

• cargo test -p loro -p loro-internal: 1226 passed, 0 failed.
• pnpm check: clean.
• WASM build-dev + test: 393 passed; CJS, tsc, deno and bun pass.

As agreed, the #1142 interaction is left for #1142 to adapt after this merges.

🤖 Generated with Claude Code