Skip to content

fix(loro.js): match Rust's local tree create and move positions - #1148

Merged
zxch3n merged 6 commits into
mainfrom
fix/loro-js-tree-local-positions
Sep 29, 2026
Merged

zxch3n merged 6 commits into
mainfrom
fix/loro-js-tree-local-positions

Conversation

@zxch3n

@zxch3n zxch3n commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Summary

Four local Tree API differences from loro-crdt, found by the differential fuzz in lockstep with Rust:

  1. Equal fractional indexes throw. Two roots created concurrently at index 0 both get position 80. tree.createNode(undefined, 1) or moving a node between them threw fractional index bounds are not ordered. Rust gives the new node 817F80 and moves the right neighbor to 8180 in the same transaction.

  2. No-op moves record ops. tree.move(a, undefined, 0) for a node already at 0, tree.move(last, undefined) / last.move(undefined) for the last child, and b.moveAfter(a) for the node right after a each recorded a move. Rust records nothing. A redundant move can override a concurrent move of the same node, so this also changed concurrent outcomes.

  3. moveAfter/moveBefore in the same parent. The index counted the moved node itself, so a.moveAfter(c) in [a, b, c] threw tree index 3 is out of range, and other cases landed one slot off.

  4. Moving a node under its own deleted descendant threw. Rust's is_ancestor_of treats a deleted node's parent as the deleted root, so tree.move(parent, deletedChild) is allowed (the node becomes deleted). The loro.js cycle check followed the deleted node's old parent and threw cannot move a tree node below itself or its descendant. The walk is now also bounded by the node count: with a cycle left by concurrent moves (a separate, still open convergence bug), it looped forever, which is how the fuzz harness first hung.

Rationale

  • LoroTree._positionFor is now a port of Rust's NodeChildren::generate_fi_at: when the left and right neighbors share a position, it collects the right neighbor and the following siblings with that position, and spreads n + 1 positions between the left neighbor and the next different position (fractionalIndexesBetween, a port of generate_n_evenly). _treeCreate/_treeMove emit the node's op, then one move per rearranged sibling, in Rust's order.
  • _treeMove returns early when the node already has that parent and index, with an omitted index meaning the last slot, as in TreeHandler::mov/mov_with_txn.
  • LoroTreeNode.moveAfter/moveBefore apply the same index adjustment as mov_after/mov_before.
  • The local cycle check in _treeMove stops at a deleted node and at most visits every node once.

Validation

  • New loro-js/tests/tree-local-positions.test.ts; the first four tests fail on main, and the fifth (move under a deleted child) throws there. The fractional indexes, op counts, and orders asserted are exactly what loro-crdt (WASM build of this main) produces for the same calls.
  • pnpm --dir loro-js format:check, lint, typecheck, test (252 passed), build.
  • bench:complexity (1k–8k), 7 alternating rounds vs main, loadavg 10–13: 284 items, none both >1.5× and >0.3 ms slower. Total 953 ms on main, 956 ms here.

Concurrent tree moves that form a cycle, and tree event ordering, are separate issues and are not addressed here.

🤖 Generated with Claude Code

zxch3n and others added 2 commits September 28, 2026 05:19
- Port generate_fi_at's rearrangement: when both neighbors share a
  fractional index, the right neighbor and following siblings with that
  index get new positions (generate_n_evenly) through extra move ops.
  loro.js threw "fractional index bounds are not ordered" instead.
- Like mov_with_txn, a move to the node's current index, including an
  omitted index when it is already last, records nothing.
- Like mov_after/mov_before, count the target index without the moved
  node when it shares the parent.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Rust's is_ancestor_of treats a deleted node's parent as the deleted
root. loro.js kept walking a deleted node's old parent, so moving a node
under its own deleted child threw. The walk is also bounded by the node
count: a cycle left by concurrent moves made it loop forever.

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

github-actions Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

WASM Size Report

  • Original size: 3277.59 KB
  • Gzipped size: 1089.33 KB
  • Brotli size: 762.83 KB

…l-positions

# Conflicts:
#	loro-js/src/runtime/document.ts

zxch3n commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Merged the latest main (c0ff1255). The only conflict was two helpers appended at the end of document.ts; both are kept. pnpm -C loro-js check with LORO_REQUIRE_WASM_REFERENCE=1 passes (26 files, 419 tests).

Still needed on main: a createNode/move between two siblings with equal fractional indexes (e.g. after two peers each create a root at index 0) still throws fractional index bounds are not ordered. moveAfter in the same parent throws tree index 3 is out of range, and moving a node under its own deleted child throws. On this branch these match loro-crdt exactly (positions 817F80/8180, op counts, orders). After the move-under-deleted-child case, isNodeDeleted still reports false; that is the ancestor-aware liveness from #1147.

zxch3n and others added 3 commits September 29, 2026 19:25
…l-positions

# Conflicts:
#	loro-js/src/runtime/document.ts
… move

The no-op check listed every sibling to count them, so each move within a
parent was linear in the sibling count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

zxch3n commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Merged main (resolved the document.ts tail conflict with #1143 by keeping both sameOptionalCodecId and sameFrontierSet) and fixed the blocking issue in 44912ff: the same-parent no-op check now uses tree._childCount(parentId) (the sibling index's .size) instead of _childrenOf(parentId).length. New test moves within a parent in time independent of the sibling count (1k vs 16k siblings, 1,000 moves): ratio ≈ 0.65 with the fix, ≈ 15.5 without (threshold 4). pnpm -C loro-js check with the Rust reference: 32 files, 499 passed, 2 skipped.

@zxch3n
zxch3n merged commit 489088c into main Sep 29, 2026
2 checks passed
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