fix(loro.js): stop a concurrent insert at the end of its origin's subtree

.changeset/loro-js-fugue-origin-subtree.md

@@ -0,0 +1,5 @@
+---
+"loro.js": patch
+---
+
+Fix a `loro.js` Text/List convergence bug with concurrent inserts. When an insert was concurrent with a run typed after the same character and with another concurrent element whose origin was further left, `loro.js` put the insert after that element instead of at the end of the run. Replicas could disagree with each other (depending on the order in which they received the updates) and with `loro-crdt`. `loro.js` now places these inserts where Rust does.

context/loro-js-performance.md

@@ -103,7 +103,16 @@ JavaScript constant factor.
   keep their single-child edge implicit, and `SequenceIndex` can skip an entire
   future ID run while finding the next causally included element. Ordinary local
   edits keep the smaller unindexed path. MovableList continues to use the scan
-  because moves break the origin-tree physical preorder.
+  because moves break the origin-tree physical preorder. Sibling subtrees are
+  contiguous, so the gap between two direct children belongs to the earlier
+  child. After the last child the interval can also hold concurrent elements
+  whose origin is left of `originLeft`; Rust's scan stops before them. The index
+  therefore checks whether the interval's last element descends from
+  `originLeft` and otherwise binary-searches the boundary. The descent test
+  walks origin-left links but jumps over each implicit run through per-peer
+  sorted counters of explicit (non-consecutive) elements, so it costs
+  O(explicit links · log n), like Rust's span-based scan, instead of one probe
+  per scalar in a long concurrent run.
 - Merging adjacent changes appends only the new operations and key-table entries
   to the retained record. The cached operation length, peer end, frontier set,
   operation indexes, and subscriber update slice are updated incrementally, so

loro-js/src/runtime/containers.ts

@@ -2870,6 +2870,12 @@ interface FugueOriginEntry {
 interface FugueOriginIndex {
   structureVersion: number;
   readonly explicitChildren: Map<string, FugueOriginEntry[]>;
+  /**
+   * Sorted counters, per peer, of the elements that are not the implicit child
+   * of their predecessor ID. Every other element is linked to `counter - 1`, so
+   * an origin-left walk can jump from any element to the head of its run.
+   */
+  readonly explicitCounters: Map<bigint, number[]>;
 }
 
 interface FugueInsertionResult {
@@ -3036,6 +3042,7 @@ function indexedFugueInsertion<T extends SequenceElement>(
   const parentRightIndex = directRightParentIndex(sequence, originLeft, originRight);
   let insertIndex = startIndex;
   let scanning = false;
+  let afterLastChild = false;
   for (let childIndex = 0; childIndex < candidates.length; childIndex += 1) {
     const { entry: other } = candidates[childIndex]!;
     if (sameOptionalId(other.originRight, originRight)) {
@@ -3054,7 +3061,38 @@ function indexedFugueInsertion<T extends SequenceElement>(
     }
 
     if (!scanning) {
-      insertIndex = candidates[childIndex + 1]?.index ?? originRightIndex;
+      const next = candidates[childIndex + 1];
+      insertIndex = next?.index ?? originRightIndex;
+      afterLastChild = next === undefined;
+    }
+  }
+
+  // Sibling subtrees are contiguous, so the gap between two direct children
+  // belongs to the earlier one. After the last child, the interval can also
+  // hold concurrent elements whose origin is left of `originLeft`; the scan
+  // stops before them, so the insertion must too.
+  const lastChild = candidates.at(-1);
+  if (afterLastChild && originLeft !== undefined && lastChild !== undefined) {
+    const left = sequence.findByIdRaw(originLeft);
+    const leftIndex = left === undefined ? undefined : sequence.physicalIndexOf(left);
+    if (leftIndex === undefined) return undefined;
+    const inSubtree = (index: number): boolean =>
+      fugueDescendsFrom(
+        sequence,
+        originIndex,
+        sequence.atPhysicalRaw(index)!.id,
+        originLeft,
+        leftIndex,
+      );
+    if (!inSubtree(originRightIndex - 1)) {
+      let low = lastChild.index + 1;
+      let high = originRightIndex - 1;
+      while (low < high) {
+        const middle = (low + high) >>> 1;
+        if (inSubtree(middle)) low = middle + 1;
+        else high = middle;
+      }
+      insertIndex = low;
     }
   }
 
@@ -3066,6 +3104,54 @@ function indexedFugueInsertion<T extends SequenceElement>(
   };
 }
 
+/**
+ * Whether `ancestor`, at physical index `ancestorIndex`, is on the origin-left
+ * chain of `id`. Ancestors precede their descendants, so the walk stops at the
+ * first origin left of `ancestorIndex`; runs of implicit children are skipped
+ * through `explicitCounters`.
+ */
+function fugueDescendsFrom<T extends SequenceElement>(
+  sequence: SequenceIndex<T>,
+  index: FugueOriginIndex,
+  id: CodecId,
+  ancestor: CodecId,
+  ancestorIndex: number,
+): boolean {
+  let current = id;
+  for (;;) {
+    const counters = index.explicitCounters.get(current.peer);
+    let head = current.counter;
+    if (counters !== undefined) {
+      let low = 0;
+      let high = counters.length;
+      while (low < high) {
+        const middle = (low + high) >>> 1;
+        if (counters[middle]! <= current.counter) low = middle + 1;
+        else high = middle;
+      }
+      if (low > 0) head = counters[low - 1]!;
+    }
+    if (
+      ancestor.peer === current.peer &&
+      head <= ancestor.counter &&
+      ancestor.counter < current.counter
+    ) {
+      return true;
+    }
+    const parent = sequence.findByIdRaw({
+      peer: current.peer,
+      counter: head,
+    })?.originLeft;
+    if (parent === undefined) return false;
+    if (parent.peer === ancestor.peer && parent.counter === ancestor.counter) return true;
+    const element = sequence.findByIdRaw(parent);
+    const parentIndex =
+      element === undefined ? undefined : sequence.physicalIndexOf(element);
+    if (parentIndex === undefined || parentIndex < ancestorIndex) return false;
+    current = parent;
+  }
+}
+
 function getFugueOriginIndex<T extends SequenceElement>(
   sequence: SequenceIndex<T>,
 ): FugueOriginIndex {
@@ -3079,6 +3165,7 @@ function getFugueOriginIndex<T extends SequenceElement>(
   const rebuilt: FugueOriginIndex = {
     structureVersion: sequence.structureVersion,
     explicitChildren: new Map(),
+    explicitCounters: new Map(),
   };
   sequence.forEachPhysicalRaw((element) => {
     recordFugueOriginEntry(rebuilt, element.id, element.originLeft, element.originRight);
@@ -3147,6 +3234,21 @@ function recordFugueOriginEntry(
   ) {
     return;
   }
+  const counters = index.explicitCounters.get(id.peer);
+  if (counters === undefined) {
+    index.explicitCounters.set(id.peer, [id.counter]);
+  } else if (counters.at(-1)! < id.counter) {
+    counters.push(id.counter);
+  } else {
+    let low = 0;
+    let high = counters.length;
+    while (low < high) {
+      const middle = (low + high) >>> 1;
+      if (counters[middle]! < id.counter) low = middle + 1;
+      else high = middle;
+    }
+    if (counters[low] !== id.counter) counters.splice(low, 0, id.counter);
+  }
   const key = optionalSequenceIdKey(originLeft);
   const children = index.explicitChildren.get(key);
   const entry = {

loro-js/tests/fugue-index.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, test, vi } from "vitest";
 
-import { LoroText } from "../src/index";
+import { LoroDoc, LoroText } from "../src/index";
 import { SequenceIndex } from "../src/runtime/sequence-index";
 
 describe("Fugue origin index", () => {
@@ -23,4 +23,108 @@ describe("Fugue origin index", () => {
     expect(longRunProbes).toBeLessThanOrEqual(shortRunProbes + 2);
     expect(longRunProbes).toBeLessThan(8);
   });
+
+  test("finds the end of a concurrent run under the origin without probing it", () => {
+    const probeCount = (length: number): number => {
+      const text = new LoroText();
+      text._insertFugue(0, "x", { peer: 1n, counter: 0 }, 0, new Map());
+      // A concurrent root: it sorts after x and stays after x's subtree.
+      text._insertFugue(0, "n", { peer: 4n, counter: 0 }, 0, new Map());
+      text._insertFugue(
+        1,
+        "a".repeat(length),
+        { peer: 1n, counter: 1 },
+        1,
+        new Map([[1n, 1]]),
+      );
+      expect(text.toString()).toBe(`x${"a".repeat(length)}n`);
+
+      const atPhysical = vi.spyOn(SequenceIndex.prototype, "atPhysicalRaw");
+      text._insertFugue(1, "b", { peer: 2n, counter: 0 }, 1, new Map([[1n, 1]]));
+      const probes = atPhysical.mock.calls.length;
+      atPhysical.mockRestore();
+
+      expect(text.toString()).toBe(`x${"a".repeat(length)}bn`);
+      return probes;
+    };
+
+    const shortRunProbes = probeCount(128);
+    const longRunProbes = probeCount(16_384);
+    // A binary search over the interval, not a scan of the run.
+    expect(longRunProbes).toBeLessThanOrEqual(shortRunProbes + 16);
+    expect(longRunProbes).toBeLessThan(64);
+  });
+
+  // An insert concurrent with a run under its origin must stop at the first
+  // concurrent element whose origin is further left, as Rust's scan does.
+  // The indexed path used to jump to the origin-right element instead.
+  test.each(["list", "text"] as const)(
+    "orders a concurrent %s insert before an element from an older origin",
+    (kind) => {
+      const peers = [1, 2, 3, 4].map((peer) => {
+        const doc = new LoroDoc();
+        doc.setPeerId(peer);
+        return doc;
+      });
+      const [p1, p2, p3, p4] = peers as [LoroDoc, LoroDoc, LoroDoc, LoroDoc];
+      const insert = (doc: LoroDoc, pos: number, value: string): void => {
+        if (kind === "list") doc.getList("seq").insert(pos, value);
+        else doc.getText("seq").insert(pos, value);
+        doc.commit();
+      };
+      const read = (doc: LoroDoc): string =>
+        kind === "list"
+          ? (doc.getList("seq").toArray() as string[]).join("")
+          : doc.getText("seq").toString();
+
+      insert(p1, 0, "A");
+      insert(p2, 0, "B");
+      insert(p4, 0, "N");
+      p3.import(p1.export({ mode: "update" }));
+      p3.import(p4.export({ mode: "update" }));
+      expect(read(p3)).toBe("AN");
+      // Y: origin left A, origin right N.
+      insert(p3, 1, "Y");
+      p1.import(p2.export({ mode: "update" }));
+      p1.import(p4.export({ mode: "update" }));
+      expect(read(p1)).toBe("ABN");
+      // X: origin left A, origin right B.
+      insert(p1, 1, "X");
+      p1.import(p3.export({ mode: "update" }));
+
+      const replay = new LoroDoc();
+      for (const peer of [p4, p3, p2, p1]) replay.import(peer.export({ mode: "update" }));
+      // loro-crdt gives AXYBN for every import order.
+      expect(read(replay)).toBe("AXYBN");
+      expect(read(p1)).toBe("AXYBN");
+    },
+  );
+
+  test("places an insert concurrent with a run before a concurrent root, as Rust does", () => {
+    const [p1, p2, p4] = [1, 2, 4].map((peer) => {
+      const doc = new LoroDoc();
+      doc.setPeerId(peer);
+      return doc;
+    }) as [LoroDoc, LoroDoc, LoroDoc];
+    p1.getText("text").insert(0, "x");
+    p1.commit();
+    p2.import(p1.export({ mode: "update" }));
+    p1.getText("text").insert(1, "aaaa");
+    p1.commit();
+    p4.getText("text").insert(0, "n");
+    p4.commit();
+    p2.getText("text").insert(1, "b");
+    p2.commit();
+
+    for (const order of [
+      [p1, p4, p2],
+      [p2, p4, p1],
+      [p4, p1, p2],
+    ]) {
+      const doc = new LoroDoc();
+      for (const peer of order) doc.import(peer.export({ mode: "update" }));
+      // loro-crdt gives xaaaabn; the indexed path used to give xaaaanb.
+      expect(doc.getText("text").toString()).toBe("xaaaabn");
+    }
+  });
 });