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

.changeset/loro-js-tree-local-positions.md

@@ -0,0 +1,10 @@
+---
+"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.
+- 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).

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";
@@ -3028,11 +3028,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));
@@ -3053,10 +3060,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 {
@@ -3198,12 +3224,28 @@ export class LoroTreeNode<T extends Record<string, unknown> = Record<string, unk
   }
 
   moveAfter(target: LoroTreeNode<T>): 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<T>): 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<T> | undefined): boolean {
+    const record = this.#record();
+    return (
+      !record.deleted &&
+      (parent === undefined
+        ? record.parent === undefined
+        : record.parent !== undefined && formatTreeId(record.parent) === parent.id)
+    );
   }
 
   parent(): LoroTreeNode<T> | undefined {

loro-js/src/runtime/document.ts

@@ -4229,35 +4229,59 @@ export class LoroDoc<T extends Record<string, Container> = Record<string, Contai
     if (parent !== undefined && !tree.has(parent)) {
       throw new RangeError(`tree parent ${parent} does not exist`);
     }
-    const position = tree._positionFor(parentId, index);
+    const { position, rearranged } = tree._positionFor(parentId, index);
     this.#appendAndApply(
       tree,
       { type: "tree-create", subject, parent: parentId, position },
       1,
     );
+    this.#moveRearrangedTreeNodes(tree, parentId, rearranged);
     return new LoroTreeNode(tree, subject);
   }
 
+  /** Moves the siblings that `_positionFor` gave new positions, like Rust. */
+  #moveRearrangedTreeNodes(
+    tree: LoroTree,
+    parent: CodecId | undefined,
+    rearranged: readonly { id: CodecId; position: Uint8Array }[],
+  ): void {
+    for (const { id, position } of rearranged) {
+      this.#appendAndApply(tree, { type: "tree-move", subject: id, parent, position }, 1);
+    }
+  }
+
   _treeMove(tree: LoroTree, target: TreeID, parent?: TreeID, index?: number): void {
     if (!tree.has(target)) throw new RangeError(`tree node ${target} does not exist`);
     if (parent !== undefined && !tree.has(parent)) {
       throw new RangeError(`tree parent ${parent} does not exist`);
     }
     const subject = parseTreeId(target);
     const parentId = parent === undefined ? undefined : parseTreeId(parent);
+    // Like Rust's is_ancestor_of: a deleted node's parent is the deleted
+    // root, so the walk stops there. It is bounded so that a malformed cycle
+    // cannot loop forever.
     let ancestor = parentId;
-    while (ancestor !== undefined) {
+    for (let steps = 0; ancestor !== undefined && steps <= tree._nodes.size; steps += 1) {
       if (idsEqual(ancestor, subject)) {
         throw new RangeError("cannot move a tree node below itself or its descendant");
       }
-      ancestor = tree._nodes.get(formatTreeId(ancestor))?.parent;
+      const record = tree._nodes.get(formatTreeId(ancestor));
+      ancestor = record === undefined || record.deleted ? undefined : record.parent;
     }
-    const position = tree._positionFor(parentId, index, subject);
+    // Rust's mov_with_txn: moving a node to where it already is records nothing.
+    const record = tree._nodes.get(target)!;
+    if (!record.deleted && sameOptionalCodecId(record.parent, parentId)) {
+      const current = tree._indexOf(record);
+      const siblings = tree._childCount(parentId);
+      if (current === (index ?? siblings - 1)) return;
+    }
+    const { position, rearranged } = tree._positionFor(parentId, index, subject);
     this.#appendAndApply(
       tree,
       { type: "tree-move", subject, parent: parentId, position },
       1,
     );
+    this.#moveRearrangedTreeNodes(tree, parentId, rearranged);
   }
 
   _treeDelete(tree: LoroTree, target: TreeID): void {
@@ -11316,6 +11340,15 @@ function treeNodeAtPath(
   return index === undefined ? undefined : tree._nodeAt(undefined, index);
 }
 
+function sameOptionalCodecId(
+  left: CodecId | undefined,
+  right: CodecId | undefined,
+): boolean {
+  return left === undefined || right === undefined
+    ? left === right
+    : idsEqual(left, right);
+}
+
 function sameFrontierSet(left: readonly OpId[], right: readonly OpId[]): boolean {
   if (left.length !== right.length) return false;
   const keys = new Set(left.map((id) => `${id.counter}@${id.peer}`));

loro-js/src/runtime/fractional-index.ts

@@ -25,6 +25,36 @@ export function fractionalIndexBetween(
   return terminated(between);
 }
 
+/**
+ * `n` increasing indexes between the bounds, spread by bisection. Port of
+ * Rust's `FractionalIndex::generate_n_evenly`.
+ */
+export function fractionalIndexesBetween(
+  lower: Uint8Array | undefined,
+  upper: Uint8Array | undefined,
+  n: number,
+): Uint8Array[] {
+  const output: Uint8Array[] = [];
+  const generate = (
+    low: Uint8Array | undefined,
+    high: Uint8Array | undefined,
+    count: number,
+  ): void => {
+    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]);
 }

loro-js/tests/tree-local-positions.test.ts

@@ -0,0 +1,146 @@
+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]);
+  });
+
+  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();
+    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/);
+  });
+});