From de93aeb8a66874ebb950372b7af7df46064c7cf3 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 05:19:10 +0800 Subject: [PATCH 1/3] fix(loro.js): match Rust's local tree create and move positions - 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) --- .changeset/loro-js-tree-local-positions.md | 9 ++ loro-js/src/runtime/containers.ts | 60 ++++++++++-- loro-js/src/runtime/document.ts | 33 ++++++- loro-js/src/runtime/fractional-index.ts | 30 ++++++ loro-js/tests/tree-local-positions.test.ts | 101 +++++++++++++++++++++ 5 files changed, 222 insertions(+), 11 deletions(-) create mode 100644 .changeset/loro-js-tree-local-positions.md create mode 100644 loro-js/tests/tree-local-positions.test.ts diff --git a/.changeset/loro-js-tree-local-positions.md b/.changeset/loro-js-tree-local-positions.md new file mode 100644 index 000000000..d2c826e1f --- /dev/null +++ b/.changeset/loro-js-tree-local-positions.md @@ -0,0 +1,9 @@ +--- +"loro.js": patch +--- + +Fix `loro.js` local tree moves and creates to match `loro-crdt`: + +- Creating or moving a node between two siblings that share a fractional index (for example two roots created concurrently at index 0) threw `fractional index bounds are not ordered`. It now gives the following siblings with that index new positions in the same transaction, as Rust does. +- Moving a node to the position it already has, including `move(node, parent)` without an index when it is already last, `node.move(parent)`, and `moveAfter`/`moveBefore` its current neighbor, no longer records an op. +- `moveAfter` and `moveBefore` within the same parent counted the moved node itself, so moving a node after a later sibling threw `tree index … is out of range` or landed one slot off. diff --git a/loro-js/src/runtime/containers.ts b/loro-js/src/runtime/containers.ts index 9b3979b91..2d34a00a0 100644 --- a/loro-js/src/runtime/containers.ts +++ b/loro-js/src/runtime/containers.ts @@ -1,4 +1,4 @@ -import { bytesToHex } from "../codec/bytes"; +import { bytesEqual, bytesToHex } from "../codec/bytes"; import { containerTypeFromHistoricalByte, containerTypeToHistoricalByte, @@ -7,7 +7,7 @@ import { PostcardReader, PostcardWriter } from "../codec/postcard"; import type { ContainerId as CodecContainerId, Id as CodecId } from "../codec/types"; import { formatContainerId, formatTreeId, parseContainerId } from "./ids"; import type { LoroDoc } from "./document"; -import { fractionalIndexBetween } from "./fractional-index"; +import { fractionalIndexBetween, fractionalIndexesBetween } from "./fractional-index"; import { OrderedIndex } from "./ordered-index"; import { SequenceIndex, SequenceSpan } from "./sequence-index"; import type { SequenceIdRun } from "./sequence-index"; @@ -2360,11 +2360,18 @@ export class LoroTree< ); } + /** + * Port of Rust's `generate_fi_at`: the position for a node placed at + * `index` among `parent`'s children, not counting `exclude`. When both + * neighbors share a position there is no index between them, so the right + * neighbor and every following sibling with that position get new + * positions in `rearranged`, in order. + */ _positionFor( parent: CodecId | undefined, index?: number, exclude?: CodecId, - ): Uint8Array { + ): { position: Uint8Array; rearranged: { id: CodecId; position: Uint8Array }[] } { const children = this._children.get(treeParentKey(parent)); const excluded = exclude === undefined ? undefined : this._nodes.get(formatTreeId(exclude)); @@ -2385,10 +2392,29 @@ export class LoroTree< ? siblingIndex + 1 : siblingIndex, ); - return fractionalIndexBetween( - position === 0 ? undefined : siblingAt(position - 1)!.position, - position === length ? undefined : siblingAt(position)!.position, - ); + const left = position === 0 ? undefined : siblingAt(position - 1)!.position; + const right = position === length ? undefined : siblingAt(position)!; + if (left === undefined || right === undefined || !bytesEqual(left, right.position)) { + return { position: fractionalIndexBetween(left, right?.position), rearranged: [] }; + } + const reset = [right]; + let nextRight: Uint8Array | undefined; + for (let sibling = position + 1; sibling < length; sibling += 1) { + const record = siblingAt(sibling)!; + if (!bytesEqual(record.position, left)) { + nextRight = record.position; + break; + } + reset.push(record); + } + const positions = fractionalIndexesBetween(left, nextRight, reset.length + 1); + return { + position: positions[0]!, + rearranged: reset.map((record, offset) => ({ + id: record.id, + position: positions[offset + 1]!, + })), + }; } _setRecord(record: TreeNodeRecord): void { @@ -2530,12 +2556,28 @@ export class LoroTreeNode = Record): void { + // Rust's mov_after: the index is counted without this node. const parent = target.parent(); - this.move(parent, target.index() + 1); + let index = target.index() + 1; + if (this.#hasParent(parent) && this.index() < index) index -= 1; + this.move(parent, index); } moveBefore(target: LoroTreeNode): void { - this.move(target.parent(), target.index()); + const parent = target.parent(); + let index = target.index(); + if (this.#hasParent(parent) && index >= 1 && this.index() < index) index -= 1; + this.move(parent, index); + } + + #hasParent(parent: LoroTreeNode | undefined): boolean { + const record = this.#record(); + return ( + !record.deleted && + (parent === undefined + ? record.parent === undefined + : record.parent !== undefined && formatTreeId(record.parent) === parent.id) + ); } parent(): LoroTreeNode | undefined { diff --git a/loro-js/src/runtime/document.ts b/loro-js/src/runtime/document.ts index 57f9a8a4d..dd0c9bda9 100644 --- a/loro-js/src/runtime/document.ts +++ b/loro-js/src/runtime/document.ts @@ -2569,15 +2569,27 @@ export class LoroDoc = Record = Record { + if (count === 0) return; + const middle = fractionalIndexBetween(low, high); + if (count === 1) { + output.push(middle); + return; + } + const half = Math.floor(count / 2); + generate(low, middle, half); + output.push(middle); + generate(middle, high, count - half - 1); + }; + generate(lower, upper, n); + return output; +} + function terminated(bytes: number[]): Uint8Array { return Uint8Array.from([...bytes, TERMINATOR]); } diff --git a/loro-js/tests/tree-local-positions.test.ts b/loro-js/tests/tree-local-positions.test.ts new file mode 100644 index 000000000..3b53913f8 --- /dev/null +++ b/loro-js/tests/tree-local-positions.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from "vitest"; + +import { LoroDoc } from "../src/index"; +import type { LoroTree } from "../src/index"; + +// Expected values are what loro-crdt returns for the same calls. + +function shape(tree: LoroTree): string[] { + return tree.toJSON().map((node) => `${node.id}:${node.fractional_index}`); +} + +/** Two roots created concurrently at index 0 share the position 80. */ +function equalPositions(): LoroDoc { + const p1 = new LoroDoc(); + p1.setPeerId(1); + const p2 = new LoroDoc(); + p2.setPeerId(2); + p1.getTree("tree").createNode(); + p1.commit(); + p2.getTree("tree").createNode(); + p2.commit(); + p1.import(p2.export({ mode: "update" })); + expect(shape(p1.getTree("tree"))).toEqual(["0@1:80", "0@2:80"]); + return p1; +} + +describe("local tree positions", () => { + // Rust's generate_fi_at: there is no index between two equal positions, so + // the right neighbor is moved to a new position in the same transaction. + test("rearranges equal neighbors when creating a node between them", () => { + const doc = equalPositions(); + const tree = doc.getTree("tree"); + const before = doc.opCount(); + const node = tree.createNode(undefined, 1); + doc.commit(); + expect(node.id).toBe("1@1"); + expect(doc.opCount() - before).toBe(2); + expect(shape(tree)).toEqual(["0@1:80", "1@1:817F80", "0@2:8180"]); + + const replay = new LoroDoc(); + replay.import(doc.export({ mode: "update" })); + expect(shape(replay.getTree("tree"))).toEqual(shape(tree)); + }); + + test("rearranges equal neighbors when moving a node between them", () => { + const doc = equalPositions(); + const tree = doc.getTree("tree"); + const extra = tree.createNode(); + doc.commit(); + const before = doc.opCount(); + tree.move(extra.id, undefined, 1); + doc.commit(); + expect(doc.opCount() - before).toBe(2); + expect(shape(tree)).toEqual(["0@1:80", "1@1:817F80", "0@2:8180"]); + }); + + test("records nothing for a move to the current position", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const a = tree.createNode(); + const b = tree.createNode(); + const c = tree.createNode(); + doc.commit(); + const count = (edit: () => void): number => { + const before = doc.opCount(); + edit(); + doc.commit(); + return doc.opCount() - before; + }; + + expect(count(() => tree.move(a.id, undefined, 0))).toBe(0); + // Without an index the node goes last; it already is. + expect(count(() => tree.move(c.id, undefined))).toBe(0); + expect(count(() => c.move(undefined))).toBe(0); + expect(count(() => b.moveAfter(a))).toBe(0); + expect(count(() => a.moveBefore(b))).toBe(0); + expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, b.id, c.id]); + }); + + // Rust's mov_after/mov_before count the index without the moved node. + test("moves after and before a sibling in the same parent", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const a = tree.createNode(); + const b = tree.createNode(); + const c = tree.createNode(); + doc.commit(); + + a.moveAfter(c); + doc.commit(); + expect(tree.toJSON().map((node) => node.id)).toEqual([b.id, c.id, a.id]); + a.moveBefore(b); + doc.commit(); + expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, b.id, c.id]); + b.moveAfter(c); + doc.commit(); + expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, c.id, b.id]); + }); +}); From bfa7dd8e01cb04f93357d7184624246b876a8156 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 05:31:18 +0800 Subject: [PATCH 2/3] fix(loro.js): stop the local tree cycle check at deleted nodes 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) --- .changeset/loro-js-tree-local-positions.md | 1 + loro-js/src/runtime/document.ts | 8 ++++++-- loro-js/tests/tree-local-positions.test.ts | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/.changeset/loro-js-tree-local-positions.md b/.changeset/loro-js-tree-local-positions.md index d2c826e1f..594857ff2 100644 --- a/.changeset/loro-js-tree-local-positions.md +++ b/.changeset/loro-js-tree-local-positions.md @@ -7,3 +7,4 @@ Fix `loro.js` local tree moves and creates to match `loro-crdt`: - Creating or moving a node between two siblings that share a fractional index (for example two roots created concurrently at index 0) threw `fractional index bounds are not ordered`. It now gives the following siblings with that index new positions in the same transaction, as Rust does. - Moving a node to the position it already has, including `move(node, parent)` without an index when it is already last, `node.move(parent)`, and `moveAfter`/`moveBefore` its current neighbor, no longer records an op. - `moveAfter` and `moveBefore` within the same parent counted the moved node itself, so moving a node after a later sibling threw `tree index … is out of range` or landed one slot off. +- Moving a node under one of its own deleted descendants threw `cannot move a tree node below itself or its descendant`. A deleted node's parent is the deleted root, as in Rust, so the move is allowed (and the node becomes deleted). diff --git a/loro-js/src/runtime/document.ts b/loro-js/src/runtime/document.ts index dd0c9bda9..d222822a4 100644 --- a/loro-js/src/runtime/document.ts +++ b/loro-js/src/runtime/document.ts @@ -2597,12 +2597,16 @@ export class LoroDoc = Record { doc.commit(); expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, c.id, b.id]); }); + + // Rust's is_ancestor_of treats a deleted node's parent as the deleted root. + test("moves a node under its own deleted child", () => { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const parent = tree.createNode(); + const child = tree.createNode(parent.id); + doc.commit(); + tree.delete(child.id); + doc.commit(); + const before = doc.opCount(); + tree.move(parent.id, child.id, 0); + doc.commit(); + expect(doc.opCount() - before).toBe(1); + expect(tree.toJSON()).toEqual([]); + expect(() => tree.move(child.id, child.id, 0)).toThrow(/below itself/); + }); }); From 44912ffc6d24217d7fa829b51d682ccfe023d9d9 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Tue, 29 Sep 2026 21:25:48 +0800 Subject: [PATCH 3/3] fix(loro.js): count siblings in O(1) when checking a same-parent tree 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 --- loro-js/src/runtime/document.ts | 2 +- loro-js/tests/tree-local-positions.test.ts | 27 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/loro-js/src/runtime/document.ts b/loro-js/src/runtime/document.ts index de8ec52b5..ced47c814 100644 --- a/loro-js/src/runtime/document.ts +++ b/loro-js/src/runtime/document.ts @@ -4272,7 +4272,7 @@ export class LoroDoc = Record { expect(tree.toJSON().map((node) => node.id)).toEqual([a.id, c.id, b.id]); }); + test("moves within a parent in time independent of the sibling count", () => { + /** Best of three runs of 1,000 same-parent moves, half of them no-ops. */ + function movesMs(siblings: number): number { + const doc = new LoroDoc(); + doc.setPeerId(1); + const tree = doc.getTree("tree"); + const nodes = Array.from({ length: siblings }, () => tree.createNode()); + doc.commit(); + let best = Infinity; + for (let run = 0; run < 3; run += 1) { + const started = performance.now(); + for (let step = 0; step < 500; step += 1) { + const node = nodes[(step * 7_919) % siblings]!; + tree.move(node.id, undefined, node.index()!); + tree.move(node.id, undefined, 0); + } + doc.commit(); + best = Math.min(best, performance.now() - started); + } + return best; + } + // Counting the siblings by listing them made each move linear in their + // number (16x the siblings took about 16x the time). + const ratio = movesMs(16_000) / movesMs(1_000); + expect(ratio).toBeLessThan(4); + }, 60_000); + // Rust's is_ancestor_of treats a deleted node's parent as the deleted root. test("moves a node under its own deleted child", () => { const doc = new LoroDoc();