From 829db4fe16e7c2073529cc735074b7b5b6c11c7a Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 04:29:44 +0800 Subject: [PATCH] fix(loro.js): stop a concurrent insert at the end of its origin's subtree The indexed Fugue path jumped from the last direct child of `originLeft` to the origin-right element. Rust's scan stops at the first concurrent element whose origin is further left, so the insert could land after that element. Replicas then disagreed depending on import order, and all of them could disagree with loro-crdt. Check whether the interval's last element descends from `originLeft` and otherwise binary-search the boundary. The descent test skips implicit runs through per-peer sorted explicit counters. Rationale and cost: context/loro-js-performance.md. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/loro-js-fugue-origin-subtree.md | 5 + context/loro-js-performance.md | 11 ++- loro-js/src/runtime/containers.ts | 104 +++++++++++++++++++- loro-js/tests/fugue-index.test.ts | 106 ++++++++++++++++++++- 4 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 .changeset/loro-js-fugue-origin-subtree.md diff --git a/.changeset/loro-js-fugue-origin-subtree.md b/.changeset/loro-js-fugue-origin-subtree.md new file mode 100644 index 000000000..fecda9748 --- /dev/null +++ b/.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. diff --git a/context/loro-js-performance.md b/context/loro-js-performance.md index 896482166..14dda5920 100644 --- a/context/loro-js-performance.md +++ b/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 diff --git a/loro-js/src/runtime/containers.ts b/loro-js/src/runtime/containers.ts index 9b3979b91..c6a49a6e2 100644 --- a/loro-js/src/runtime/containers.ts +++ b/loro-js/src/runtime/containers.ts @@ -2870,6 +2870,12 @@ interface FugueOriginEntry { interface FugueOriginIndex { structureVersion: number; readonly explicitChildren: Map; + /** + * 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; } interface FugueInsertionResult { @@ -3036,6 +3042,7 @@ function indexedFugueInsertion( 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( } 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( }; } +/** + * 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( + sequence: SequenceIndex, + 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( sequence: SequenceIndex, ): FugueOriginIndex { @@ -3079,6 +3165,7 @@ function getFugueOriginIndex( 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 = { diff --git a/loro-js/tests/fugue-index.test.ts b/loro-js/tests/fugue-index.test.ts index 7b59fa27c..dcac55c6c 100644 --- a/loro-js/tests/fugue-index.test.ts +++ b/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"); + } + }); });