From e9e85973ac42336ff385735395e62f2008f26258 Mon Sep 17 00:00:00 2001 From: William Stein Date: Fri, 2 Oct 2026 01:52:36 +0000 Subject: [PATCH 1/4] Merge commits record their merged value; legacy merges keep their values A patch with several parents (a merge commit) is a diff from the merged value of its parents, so computing its value reran merge3: changing merge3 changed the values of patches already in a history, and their hashes stopped matching. Now: - Session records the merged value of a merge commit's parents, as its author computed it: mergePatch, a diff from the exact value of mergeParent (the parent with the smallest diff). PatchGraph computes the value of such a patch from that parent alone, without merging, like a git merge commit records its tree. A later merge3 change only changes how new concurrent edits merge. `patch` keeps its meaning, so clients that do not know the new fields, and the apply-all fallback, read it as before. - A merge commit without them and without a hash was written by a version that applied every patch in time order (patchflow 0.8, what CoCalc runs in production) or with that fallback's value. Its value is now computed the same way, from the patches its parents descend from (legacyValueOfSet), instead of with merge3, so existing histories keep the values their authors saw. It starts from the newest snapshot that contains every older patch it needs; a snapshot made after a concurrent patch does not. - Undo of a merge commit reverts from the same recorded value. Tests (merge-commit.test.ts): merge commits record their value; a different merge3 gives the same values and matching hashes (and without the record does not); readers without the fields agree; random 3-writer histories keep every value under either merge3; 20 random histories written with apply-all keep every value under merge3 and a different merge; a snapshot taken after a concurrent patch is not a starting point; undo of a merge commit. --- README.md | 6 + src/merge-commit.test.ts | 231 ++++++++++++++++++++++++++++++++++ src/patch-graph-exact.test.ts | 10 +- src/patch-graph.ts | 134 ++++++++++++++++++-- src/session.ts | 32 +++++ src/types.ts | 12 ++ 6 files changed, 417 insertions(+), 8 deletions(-) create mode 100644 src/merge-commit.test.ts diff --git a/README.md b/README.md index a4b95ed..dc50ab7 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,12 @@ const codec = { `mergeStrings3` is a line-oriented diff3 that never relocates edits by fuzzy matching, keeps whatever either side typed (a duplicate is visible and easy to fix, lost text is not), never splices characters of different words or lines together, and is symmetric in its two sides. When the history needed for an exact value is not loaded (e.g., below a snapshot), the graph falls back to the default algorithm. +### Merge commits record their merged value + +A patch that merges concurrent heads (a merge commit, with several parents) is a diff from the merged value of its parents, like any patch. `Session` also records that merged value as its author computed it, as a diff from one of the parents (`Patch.mergeParent`, `Patch.mergePatch`), like a git merge commit records its merged tree. Computing the value of any patch in the history then needs no merge, so a later change to `merge3` only changes how new concurrent edits merge, never the values of patches already written; value hashes keep matching. Clients that do not know these fields read `patch` as before. + +Merge commits written before this (no `mergeParent` and no `hash`) were made by versions that applied every patch in time order; their values are computed the same way, from the patches their parents descend from, so the values of such a history do not change either. + ## Quickstart ```sh diff --git a/src/merge-commit.test.ts b/src/merge-commit.test.ts new file mode 100644 index 0000000..936ad6d --- /dev/null +++ b/src/merge-commit.test.ts @@ -0,0 +1,231 @@ +import { PatchGraph } from "./patch-graph"; +import { Session } from "./session"; +import { MemoryPatchStore } from "./adapters/memory-patch-store"; +import { mergeStrings3 } from "./merge3"; +import { StringCodec, StringDocument } from "./string-document"; +import { legacyPatchId } from "./patch-id"; +import type { DocCodec, Inconsistency, Patch, PatchEnvelope } from "./types"; + +const doc = (s: string) => new StringDocument(s); + +// The current merge, and a different one, as a later version might merge. +const codec: DocCodec = { + ...(StringCodec as any), + merge3: (base, a, b, ancestors) => + doc( + mergeStrings3({ + base: base.toString(), + a: a.toString(), + b: b.toString(), + ancestors: ancestors?.map(String), + }), + ), +}; +const otherCodec: DocCodec = { + ...(StringCodec as any), + merge3: (_base, a, b) => doc(`${b}${a}`), +}; +// No merge3: how versions before exact merges computed values, applying every +// patch in time order. +const applyAllCodec = StringCodec as unknown as DocCodec; + +function graph(c: DocCodec, patches: Patch[], inconsistencies: Inconsistency[] = []) { + const g = new PatchGraph({ codec: c, onInconsistency: (x) => inconsistencies.push(x) }); + g.add(patches); + return g; +} + +// Writers that exchange patches only when told to. +async function writers(c: DocCodec, n: number) { + let now = 1000; + const clock = () => now++; + const sessions: Session[] = []; + for (let i = 0; i < n; i++) { + const s = new Session({ + codec: c, + patchStore: new MemoryPatchStore(), + clock, + clientId: `c${i}`, + userId: i, + }); + await s.init(); + sessions.push(s); + } + return sessions; +} + +const strip = ({ mergeParent: _p, mergePatch: _m, ...rest }: PatchEnvelope): Patch => rest; + +describe("merge commits record their merged value", () => { + async function history() { + const [a, b] = await writers(codec, 2); + const e0 = a.commit(doc("one two three\n")); + b.applyRemote(e0); + const ea = a.commit(doc("one TWO three\n")); + const eb = b.commit(doc("one two three four\n")); + a.applyRemote(eb); + b.applyRemote(ea); + expect(a.getDocument().toString()).toBe("one TWO three four\n"); + const merge = a.commit(doc("one TWO three four\nfive\n")); + return { patches: [e0, ea, eb, merge], merge }; + } + + it("as a diff from one of its parents", async () => { + const { patches, merge } = await history(); + expect(merge.parents).toHaveLength(2); + expect(merge.parents).toContain(merge.mergeParent); + expect(merge.mergePatch).toBeDefined(); + for (const p of patches.slice(0, 3)) { + expect(p.mergeParent).toBeUndefined(); + expect(p.mergePatch).toBeUndefined(); + } + }); + + it("so a different merge algorithm does not change the value of the history", async () => { + const { patches, merge } = await history(); + const inconsistencies: Inconsistency[] = []; + const g = graph(otherCodec, patches, inconsistencies); + expect(g.version(merge.time).toString()).toBe("one TWO three four\nfive\n"); + expect(g.verifyValue(merge.time)).toBe("ok"); + expect(inconsistencies).toEqual([]); + // Without the recorded merged value it would (and the hash shows it). + const h = graph(otherCodec, patches.map(strip), inconsistencies); + expect(h.version(merge.time).toString()).not.toBe("one TWO three four\nfive\n"); + expect(h.verifyValue(merge.time)).toBe("mismatch"); + }); + + it("and readers that do not know it read the patch as before", async () => { + const { patches, merge } = await history(); + const inconsistencies: Inconsistency[] = []; + const g = graph(codec, patches.map(strip), inconsistencies); + expect(g.version(merge.time).toString()).toBe("one TWO three four\nfive\n"); + expect(inconsistencies).toEqual([]); + // So does applying every patch in time order. + expect(graph(applyAllCodec, patches).version(merge.time).toString()).toBe( + "one TWO three four\nfive\n", + ); + }); + + it("needs only the parent it records the merged value from", async () => { + const { patches, merge } = await history(); + const g = graph(otherCodec, patches); + const spy = jest.spyOn(otherCodec, "merge3"); + expect(g.version(merge.time).toString()).toBe("one TWO three four\nfive\n"); + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + + it("from random concurrent histories, whatever the merge algorithm", async () => { + let seed = 7; + const rng = () => (seed = (seed * 1103515245 + 12345) % 2 ** 31) / 2 ** 31; + let merges = 0; + for (let run = 0; run < 20; run++) { + const sessions = await writers(codec, 3); + const all: PatchEnvelope[] = []; + const seen = sessions.map(() => 0); + const values = new Map(); + for (let step = 0; step < 15; step++) { + const i = Math.floor(rng() * sessions.length); + const s = sessions[i]; + // Catch up with some of what the others wrote. + if (rng() < 0.5) { + s.applyRemoteBatch(all.slice(seen[i]).filter((e) => !e.time.endsWith(`c${i}`))); + seen[i] = all.length; + } + const words = s.getDocument().toString().split(" ").filter(Boolean); + words.splice(Math.floor(rng() * (words.length + 1)), 0, `w${run}x${step}`); + const env = s.commit(doc(words.join(" "))); + values.set(env.time, words.join(" ")); + all.push(env); + if ((env.parents?.length ?? 0) > 1) merges++; + } + for (const c of [codec, otherCodec]) { + const inconsistencies: Inconsistency[] = []; + const g = graph(c, all, inconsistencies); + for (const [time, value] of values) expect(g.version(time).toString()).toBe(value); + expect(inconsistencies).toEqual([]); + } + } + expect(merges).toBeGreaterThan(20); + }); +}); + +describe("a history written before merge commits recorded their merged value", () => { + // Written by versions that applied every patch in time order: no hashes, + // merge commits are diffs from that value of their parents. + async function legacyHistory(seed: number) { + const rng = () => (seed = (seed * 1103515245 + 12345) % 2 ** 31) / 2 ** 31; + const sessions = await writers(applyAllCodec, 3); + const all: PatchEnvelope[] = []; + const seen = sessions.map(() => 0); + const base = ["the", "cat", "sat", "on", "the", "mat"]; + for (let step = 0; step < 15; step++) { + const i = Math.floor(rng() * sessions.length); + const s = sessions[i]; + if (rng() < 0.5) { + s.applyRemoteBatch(all.slice(seen[i]).filter((e) => !e.time.endsWith(`c${i}`))); + seen[i] = all.length; + } + const words = all.length === 0 ? base.slice() : s.getDocument().toString().split(" "); + // Concurrent edits of the same words, which merge differently. + const k = Math.floor(rng() * words.length); + words[k] = `${words[k]}${step}`; + all.push(s.commit(doc(words.join(" ")))); + } + return all; + } + + it("keeps the values its authors saw, whatever the merge algorithm", async () => { + let merges = 0; + for (let seed = 1; seed <= 20; seed++) { + const all = await legacyHistory(seed); + expect(all.every((p) => p.hash == null && p.mergeParent == null)).toBe(true); + merges += all.filter((p) => (p.parents?.length ?? 0) > 1).length; + const authored = graph(applyAllCodec, all); + for (const c of [codec, otherCodec]) { + const g = graph(c, all); + for (const p of all) { + expect(g.version(p.time).toString()).toBe(authored.version(p.time).toString()); + } + } + } + expect(merges).toBeGreaterThan(20); + }); + + it("does not start from a snapshot made after a concurrent patch", () => { + const id = legacyPatchId; + const patch = (t: number, parents: number[], from: string, to: string): Patch => ({ + time: id(t), + parents: parents.map(id), + patch: doc(from).makePatch(doc(to)), + }); + // 2 and 3 edit 1 concurrently, 4 builds on 2 and is snapshotted (without + // 3), 5 merges 4 and 3. + const ps = [ + patch(1, [], "", "a\nb\nc\n"), + patch(2, [1], "a\nb\nc\n", "A\nb\nc\n"), + patch(3, [1], "a\nb\nc\n", "a\nb\nC\n"), + patch(4, [2], "A\nb\nc\n", "A\nB\nc\n"), + patch(5, [4, 3], "A\nB\nC\n", "A\nB\nC\nd\n"), + ]; + const snapshot: Patch = { ...ps[3], isSnapshot: true, snapshot: "A\nB\nc\n" }; + const g = graph(codec, [...ps.slice(0, 3), snapshot, ps[4]]); + expect(g.version(id(5)).toString()).toBe("A\nB\nC\nd\n"); + }); +}); + +describe("undo of a merge commit", () => { + it("reverts its own changes from its recorded merged value", async () => { + const [a, b] = await writers(codec, 2); + const e0 = a.commit(doc("one two three\n")); + b.applyRemote(e0); + const ea = a.commit(doc("one TWO three\n")); + const eb = b.commit(doc("one two three four\n")); + a.applyRemote(eb); + b.applyRemote(ea); + const merge = a.commit(doc("one TWO three four\nfive\n")); + const g = graph(codec, [e0, ea, eb, merge]); + // Undo only the merge commit's own edit; the merge itself stays. + expect(g.value({ withoutTimes: [merge.time] }).toString()).toBe("one TWO three four\n"); + }); +}); diff --git a/src/patch-graph-exact.test.ts b/src/patch-graph-exact.test.ts index 841da35..b27702d 100644 --- a/src/patch-graph-exact.test.ts +++ b/src/patch-graph-exact.test.ts @@ -535,7 +535,7 @@ describe("PatchGraph exact values (codec with merge3)", () => { next = lines.join("\n") + "\n"; } const t = legacyPatchId(time++); - g.add([patch(t, parents, base, next)]); + g.add([commit(g, t, parents, base, next)]); seen[c] = [t]; // Sometimes a client catches up with everything. if (rng() < 0.3) seen[Math.floor(rng() * 3)] = g.getHeads(); @@ -582,6 +582,14 @@ describe("PatchGraph exact values (codec with merge3)", () => { }); }); +// A patch as Session commits it: a merge commit records its merged value (see +// Patch.mergeParent). +function commit(g: PatchGraph, time: string, parents: string[], from: string, to: string): Patch { + const p = patch(time, parents, from, to); + if (parents.length < 2) return p; + return { ...p, mergeParent: parents[0], mergePatch: g.version(parents[0]).makePatch(doc(from)) }; +} + function parentsValue(g: PatchGraph, parents: string[]): string { // The value a client committing on top of `parents` started from. const heads = parents; diff --git a/src/patch-graph.ts b/src/patch-graph.ts index 7b39de4..d181c29 100644 --- a/src/patch-graph.ts +++ b/src/patch-graph.ts @@ -58,6 +58,35 @@ function isRoot(patch: Patch): boolean { return (patch.parents ?? []).length === 0 && !(patch.isSnapshot && patch.snapshot != null); } +// A merge commit (several parents) that does not record its merged value +// (see Patch.mergeParent) and has no hash, so was written either by a version that merged +// concurrent patches by applying them all in time order, or with the value of +// that same fallback (see Session.commit). Its value is computed the way its +// author computed the value it is a diff from (see legacyValueOfSet), not +// with merge3, so a history written before exact merges keeps its values. +function isLegacyMerge(patch: Patch): boolean { + return ( + (patch.parents?.length ?? 0) > 1 && + patch.mergeParent == null && + patch.hash == null && + patch.patch != null + ); +} + +// The parent from which a merge commit records its merged value (see +// Patch.mergeParent), if it does. +function mergeParent(patch: Patch): string | undefined { + const parent = patch.mergeParent; + if (parent == null || patch.mergePatch == null) return undefined; + return (patch.parents ?? []).includes(parent) ? parent : undefined; +} + +// The patches whose exact values a patch's value is computed from. +function valueParents(patch: Patch): string[] { + const parent = mergeParent(patch); + return parent != null ? [parent] : (patch.parents ?? []); +} + function patchCmp(a: Patch, b: Patch): number { return comparePatchId(a.time, b.time); } @@ -374,15 +403,25 @@ export class PatchGraph { const patch = this.patches.get(time)!; if (patch.isSnapshot || patch.patch == null) continue; const after = this.exactValue(time); - const parents = patch.parents ?? []; - const before = - parents.length === 0 ? this.codec.fromString("") : this.exactValueOfSet(parents); + const before = this.parentsValue(patch); if (after == null || before == null) return undefined; doc = merge3(after, doc, before); } return doc; } + // The value a patch is a diff from: the merged value of its parents, as + // its author computed it (see Patch.mergeParent and isLegacyMerge). + private parentsValue(patch: Patch): Document | undefined { + const parents = patch.parents ?? []; + if (parents.length === 0) return this.codec.fromString(""); + if (isLegacyMerge(patch)) return this.legacyValueOfSet(parents); + const recorded = mergeParent(patch); + if (recorded == null) return this.exactValueOfSet(parents); + const value = this.exactValue(recorded); + return value == null ? undefined : this.codec.applyPatch(value, patch.mergePatch); + } + // The caller knows it has every patch from the start of the history (e.g. // it began with an empty history and receives the stream in full): then a // missing parent has not arrived yet, even before the first patch has. @@ -815,7 +854,7 @@ export class PatchGraph { } if (state === "bad") rejected.push(t); const below = needed || state === "bad"; - for (const parent of patch.parents ?? []) stack.push({ t: parent, needed: below }); + for (const parent of valueParents(patch)) stack.push({ t: parent, needed: below }); } // Loaded ancestors are not enough: rebuilding a rejected snapshot's value // can also need the common ancestors of its parents (which the walk above @@ -916,7 +955,10 @@ export class PatchGraph { // patch is not loaded, the exact value is unknown until more history is // (see needsMoreHistory). if (snapshot === "bad" && patch.patch == null) return undefined; - for (const parent of patch.parents ?? []) { + // A legacy merge commit's value is computed from its parents' history, + // not from their exact values (see legacyValueOfSet). + if (isLegacyMerge(patch)) continue; + for (const parent of valueParents(patch)) { if (!this.patches.has(parent)) return undefined; uses.set(parent, (uses.get(parent) ?? 0) + 1); if (!visited.has(parent)) stack.push({ t: parent, expanded: false }); @@ -931,12 +973,19 @@ export class PatchGraph { const patch = this.patches.get(t)!; let doc: Document | undefined; const useSnapshot = this.snapshotState(patch) === "use"; - const parents = useSnapshot ? [] : (patch.parents ?? []); + const legacy = !useSnapshot && isLegacyMerge(patch); + const parents = useSnapshot || legacy ? [] : valueParents(patch); if (useSnapshot) { doc = this.codec.fromString(patch.snapshot!); } else { let base: Document | undefined; - if (parents.length === 0) { + const recorded = mergeParent(patch); + if (legacy) { + base = this.legacyValueOfSet(patch.parents!); + } else if (recorded != null) { + const value = get(recorded) ?? this.exactValue(recorded); + base = value == null ? undefined : this.codec.applyPatch(value, patch.mergePatch); + } else if (parents.length === 0) { base = this.codec.fromString(""); } else if (parents.length === 1) { base = get(parents[0]) ?? this.exactValue(parents[0]); @@ -962,6 +1011,77 @@ export class PatchGraph { return doc; } + // The value a legacy merge commit (see isLegacyMerge) is a diff from, as its + // author computed it: every patch its parents descend from applied in time + // order, as every version before exact merges computed the value of + // concurrent patches, so the merge commit gets the value its author saw. + // Starts from the newest snapshot among them that already contains every + // older one of them (a snapshot made after concurrent patches does not; + // starting from it would drop them). Undefined if a patch needed for it is + // not loaded. This must never change: it defines the values of histories + // written by those versions. + private legacyValueOfSet(times: string[]): Document | undefined { + const sorted = this.sortHeads(Array.from(new Set(times))); + const key = `legacy:${sorted.join(",")}`; + const cached = + this.pinnedMerged?.get(key) ?? + this.recentMerged.get(key) ?? + this.exactMergeCache.get(key)?.doc; + if (cached) return cached; + const skipped = new Set(); + let reachable: Set; + let snapshot: Patch | undefined; + for (;;) { + reachable = new Set(); + const stack = [...sorted]; + while (stack.length > 0) { + const t = stack.pop()!; + if (reachable.has(t)) continue; + const patch = this.patches.get(t); + if (patch == null) return undefined; + reachable.add(t); + if (this.snapshotState(patch) === "use" && !skipped.has(t)) continue; + if (patch.isSnapshot && patch.patch == null) return undefined; + stack.push(...(patch.parents ?? [])); + } + snapshot = this.latestSnapshot(Array.from(reachable).filter((t) => !skipped.has(t))); + if (snapshot == null) break; + const floor = snapshot.time; + const older = Array.from(reachable).filter((t) => comparePatchId(t, floor) < 0); + if (older.length === 0) break; + const above = this.ancestorsSince(floor, older.sort(comparePatchId)[0]); + if (older.every((t) => above.has(t))) break; + skipped.add(floor); + } + const floor = snapshot?.time; + const ordered = Array.from(reachable) + .filter((t) => floor == null || comparePatchId(t, floor) > 0) + .map((t) => this.patches.get(t)!) + .sort(patchCmp); + this.dedupFileLoads(ordered); + const patches = ordered.filter((p) => p.patch != null).map((p) => p.patch); + let doc = snapshot ? this.codec.fromString(snapshot.snapshot!) : this.codec.fromString(""); + if (patches.length > 0) doc = this.codec.applyPatchBatch(doc, patches); + this.exactMergeCache.set(key, { doc }); + remember(this.recentMerged, key, doc); + this.pinnedMerged?.set(key, doc); + return doc; + } + + // The loaded ancestors of a patch that are not older than `oldest` (parents + // are older than their children, so the walk stops there). + private ancestorsSince(time: string, oldest: string): Set { + const found = new Set(); + const stack = [...(this.patches.get(time)?.parents ?? [])]; + while (stack.length > 0) { + const t = stack.pop()!; + if (found.has(t) || comparePatchId(t, oldest) < 0) continue; + found.add(t); + stack.push(...(this.patches.get(t)?.parents ?? [])); + } + return found; + } + // Exact merged value of a set of patches (heads or a merge patch's parents): // fold them in time order, merging each into the accumulated value with // merge3 from the value of their maximal common ancestors. diff --git a/src/session.ts b/src/session.ts index 8c39bf6..a932993 100644 --- a/src/session.ts +++ b/src/session.ts @@ -37,6 +37,16 @@ export type SessionOptions = { presenceAdapter?: PresenceAdapter; }; +// Parents considered as the one a merge commit records its merged value from +// (see Session.recordMerge): each costs a diff. +const MAX_MERGE_PARENT_CANDIDATES = 8; + +// Rough size of a patch, to compare patches of the same codec. +function patchSize(patch: unknown): number { + if (patch instanceof Uint8Array) return patch.byteLength; + return JSON.stringify(patch)?.length ?? 0; +} + /** * Session orchestrates a local Document against a PatchGraph and a PatchStore. * It handles local commits, remote patches, and basic undo/redo of local changes. @@ -357,6 +367,7 @@ export class Session extends EventEmitter { source: opts.source, meta: opts.meta, hash: parentsValue == null ? undefined : this.graph.hashOf(nextDoc), + ...this.recordMerge(parents, parentsValue), }; this.graph.add([envelope]); this.maxVersion = Math.max(this.maxVersion, nextVersion); @@ -374,6 +385,26 @@ export class Session extends EventEmitter { return envelope; } + // For a patch with several parents, the merged value of its parents as a + // diff from one of them (see Patch.mergeParent), so its value never depends + // on how a later version merges. From the parent with the smallest diff, + // usually the one with the most of the merged changes. + private recordMerge( + parents: string[], + merged: Document | undefined, + ): Pick { + if (parents.length < 2 || merged == null) return {}; + let best: { parent: string; patch: unknown; size: number } | undefined; + for (const parent of parents.slice(0, MAX_MERGE_PARENT_CANDIDATES)) { + const value = this.graph.exactValueOf([parent]); + if (value == null) return {}; + const patch = this.codec.makePatch(value, merged); + const size = patchSize(patch); + if (best == null || size < best.size) best = { parent, patch, size }; + } + return best == null ? {} : { mergeParent: best.parent, mergePatch: best.patch }; + } + // Merge a remote patch and refresh the current document. applyRemote(env: PatchEnvelope): void { this.applyRemoteBatch([env]); @@ -712,6 +743,7 @@ export class Session extends EventEmitter { version: nextVersion, file: true, hash: parentsValue == null ? undefined : this.graph.hashOf(newDoc), + ...this.recordMerge(parents, parentsValue), }; this.graph.add([envelope]); this.maxVersion = Math.max(this.maxVersion, nextVersion); diff --git a/src/types.ts b/src/types.ts index e8a7288..5061d0a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -42,6 +42,18 @@ export interface Patch { // (see PatchGraphOptions.onInconsistency). A snapshot record carries the // hash of the patch it is a snapshot of. hash?: string; + // A patch with several parents (a merge commit) is, like any patch, a diff + // from the merged value of its parents. From this version on it also + // records that merged value, as its author computed it: mergePatch applied + // to the exact value of mergeParent, one of the parents. Computing the + // value of the patch then needs no merge, so changing the merge algorithm + // (DocCodec.merge3) later never changes the values of patches already in a + // history, like a git merge commit, which records its merged tree. Clients + // that do not know these fields still read `patch` as before. A merge + // commit without them, written by an earlier version, see isLegacyMerge in + // patch-graph.ts. + mergeParent?: PatchId; + mergePatch?: unknown; } // A value that does not match the hash its author recorded (Patch.hash). From 1e62282edc32d686b5a37e056165ce6cb3875b29 Mon Sep 17 00:00:00 2001 From: William Stein Date: Fri, 2 Oct 2026 02:35:31 +0000 Subject: [PATCH 2/4] Unmarked merges: the application says how they were written; legacy replays continue from checkpoints Review of #16: - P1: a merge commit without a hash or recorded merged value was not necessarily written by apply-all: patchflow 0.9 wrote them with exact merges too. Which one is known only to the application, so it says so: PatchGraphOptions/SessionOptions.unmarkedMerges, "merge3" (default, as 0.9/0.10 read them) or "apply-all" (patchflow 0.8 histories; CoCalc). Test: a history written by the v0.9.3 Session (the reviewer's repro) reads as its author saw it by default. - P2: successive legacy merges replayed every patch back to the start (or the snapshot) for each merge, quadratic over a snapshot-free history. legacyValueOfSet now keeps checkpoints of its replays, keyed by a hash of the replayed patch times from the same start, and continues from the longest one. Test: 150 rounds of fork/fork/merge read merge by merge, every value equal to its author's, apply < 4 per patch (34,125 without the checkpoints, < 1,804 with). - Contract: a patch is still held back until all its parents arrive, even when its merged value is recorded (causal delivery, as for every patch). Test added. --- README.md | 2 +- src/merge-commit.test.ts | 172 +++++++++++++++++++++++++++++++++++++-- src/patch-graph.ts | 107 +++++++++++++++++++----- src/session.ts | 6 +- src/types.ts | 4 +- 5 files changed, 261 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index dc50ab7..8647d8d 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ const codec = { A patch that merges concurrent heads (a merge commit, with several parents) is a diff from the merged value of its parents, like any patch. `Session` also records that merged value as its author computed it, as a diff from one of the parents (`Patch.mergeParent`, `Patch.mergePatch`), like a git merge commit records its merged tree. Computing the value of any patch in the history then needs no merge, so a later change to `merge3` only changes how new concurrent edits merge, never the values of patches already written; value hashes keep matching. Clients that do not know these fields read `patch` as before. -Merge commits written before this (no `mergeParent` and no `hash`) were made by versions that applied every patch in time order; their values are computed the same way, from the patches their parents descend from, so the values of such a history do not change either. +Merge commits written before this (no `mergeParent` and no `hash`) cannot tell which merge their author used, so the application says how its older history was written with the `unmarkedMerges` option of `Session`/`PatchGraph`: `"merge3"` (default; patchflow 0.9 exact merges) or `"apply-all"` (patchflow 0.8 and earlier, which applied every patch in time order). With `"apply-all"`, such a merge commit's value is computed the way its author computed it, from the patches its parents descend from, so the values of that history do not change either. ## Quickstart diff --git a/src/merge-commit.test.ts b/src/merge-commit.test.ts index 936ad6d..e7e6120 100644 --- a/src/merge-commit.test.ts +++ b/src/merge-commit.test.ts @@ -1,4 +1,4 @@ -import { PatchGraph } from "./patch-graph"; +import { PatchGraph, type PatchGraphOptions } from "./patch-graph"; import { Session } from "./session"; import { MemoryPatchStore } from "./adapters/memory-patch-store"; import { mergeStrings3 } from "./merge3"; @@ -29,8 +29,17 @@ const otherCodec: DocCodec = { // patch in time order. const applyAllCodec = StringCodec as unknown as DocCodec; -function graph(c: DocCodec, patches: Patch[], inconsistencies: Inconsistency[] = []) { - const g = new PatchGraph({ codec: c, onInconsistency: (x) => inconsistencies.push(x) }); +function graph( + c: DocCodec, + patches: Patch[], + inconsistencies: Inconsistency[] = [], + opts: Partial = {}, +) { + const g = new PatchGraph({ + codec: c, + onInconsistency: (x) => inconsistencies.push(x), + ...opts, + }); g.add(patches); return g; } @@ -150,6 +159,8 @@ describe("merge commits record their merged value", () => { }); }); +const applyAll = { unmarkedMerges: "apply-all" } as const; + describe("a history written before merge commits recorded their merged value", () => { // Written by versions that applied every patch in time order: no hashes, // merge commits are diffs from that value of their parents. @@ -183,7 +194,7 @@ describe("a history written before merge commits recorded their merged value", ( merges += all.filter((p) => (p.parents?.length ?? 0) > 1).length; const authored = graph(applyAllCodec, all); for (const c of [codec, otherCodec]) { - const g = graph(c, all); + const g = graph(c, all, [], applyAll); for (const p of all) { expect(g.version(p.time).toString()).toBe(authored.version(p.time).toString()); } @@ -209,7 +220,7 @@ describe("a history written before merge commits recorded their merged value", ( patch(5, [4, 3], "A\nB\nC\n", "A\nB\nC\nd\n"), ]; const snapshot: Patch = { ...ps[3], isSnapshot: true, snapshot: "A\nB\nc\n" }; - const g = graph(codec, [...ps.slice(0, 3), snapshot, ps[4]]); + const g = graph(codec, [...ps.slice(0, 3), snapshot, ps[4]], [], applyAll); expect(g.version(id(5)).toString()).toBe("A\nB\nC\nd\n"); }); }); @@ -229,3 +240,154 @@ describe("undo of a merge commit", () => { expect(g.value({ withoutTimes: [merge.time] }).toString()).toBe("one TWO three four\n"); }); }); + +describe("unmarked merge commits", () => { + // Written by the Session of patchflow 0.9.3 (exact merges, no hashes, no + // recorded merged value): two writers append to "hello" concurrently, then + // one merges and adds a line. Its author saw "helloA\nhelloB\ntail\n". + const authored = "helloA\nhelloB\ntail\n"; + const written: Patch[] = [ + { time: "000000000rs_A", parents: [], patch: [[[[1, "hello\n"]], 0, 0, 0, 6]] }, + { + time: "000000000ru_A", + parents: ["000000000rs_A"], + patch: [ + [ + [ + [0, "hello"], + [1, "A"], + [0, "\n"], + ], + 0, + 0, + 6, + 7, + ], + ], + }, + { + time: "000000000rw_B", + parents: ["000000000rs_A"], + patch: [ + [ + [ + [0, "hello"], + [1, "B"], + [0, "\n"], + ], + 0, + 0, + 6, + 7, + ], + ], + }, + { + time: "000000000ry_A", + parents: ["000000000ru_A", "000000000rw_B"], + patch: [ + [ + [ + [0, "\nhelloB\n"], + [1, "tail\n"], + ], + 6, + 6, + 8, + 13, + ], + ], + }, + ]; + + it("are read with merge3 by default, as patchflow 0.9 wrote them", () => { + expect(graph(codec, written).value().toString()).toBe(authored); + }); + + it("are read as patchflow 0.8 wrote them only when the application says so", () => { + // This history was not written that way, so that reading differs: which + // one is right is known only to the application. + expect(graph(codec, written, [], applyAll).value().toString()).not.toBe(authored); + }); + + it("read as patchflow 0.8 wrote them, each merge continues the previous replay", async () => { + // Rounds of: fork the last merge, one line on each side, merge. Reading + // every merge as it arrives applies each patch once, not the whole + // history again for every merge. + let now = 1000; + const clock = () => now++; + const make = async (id: string) => { + const s = new Session({ + codec: applyAllCodec, + patchStore: new MemoryPatchStore(), + clock, + clientId: id, + }); + await s.init(); + return s; + }; + const a = await make("a"); + const b = await make("b"); + const all: PatchEnvelope[] = [a.commit(doc("start\n"))]; + b.applyRemote(all[0]); + const values = new Map(); + const rounds = 150; + for (let i = 0; i < rounds; i++) { + const l = a.commit(doc(`${a.getDocument()}L${i}\n`)); + const r = b.commit(doc(`R${i}\n${b.getDocument()}`)); + a.applyRemote(r); + const m = a.commit(doc(`${a.getDocument()}M${i}\n`)); + b.applyRemoteBatch([l, m]); + all.push(l, r, m); + values.set(m.time, `${a.getDocument()}`); + } + expect(all.filter((p) => (p.parents?.length ?? 0) > 1 && p.hash == null)).toHaveLength(rounds); + let applied = 0; + const counting: DocCodec = { + ...codec, + applyPatch: (d, p) => { + applied++; + return d.applyPatch(p); + }, + applyPatchBatch: (d, ps) => { + applied += ps.length; + return d.applyPatchBatch(ps); + }, + }; + const g = new PatchGraph({ codec: counting, ...applyAll }); + g.add([all[0]]); + let checked = 0; + for (let i = 1; i < all.length; i += 3) { + const round = all.slice(i, i + 3); + g.add(round); + expect(g.value().toString()).toBe(values.get(round[2].time)); + checked++; + } + expect(checked).toBe(rounds); + // Each patch is applied a bounded number of times (by the replay and by + // the exact values of the branches), not once per later merge. + expect(applied).toBeLessThan(4 * all.length); + }); +}); + +describe("a merge commit with a parent not yet received", () => { + it("waits for it, although its merged value is recorded", async () => { + // Patches are delivered with their ancestors (see types.ts): a patch is + // held back until all its parents arrive, so every client has the same + // history below the patches it shows. + const [a, b] = await writers(codec, 2); + const e0 = a.commit(doc("base\n")); + b.applyRemote(e0); + const ea = a.commit(doc("base\nleft\n")); + const eb = b.commit(doc("base\nright\n")); + a.applyRemote(eb); + const merge = a.commit(doc(`${a.getDocument()}edit\n`)); + const other = merge.mergeParent === ea.time ? eb : ea; + const rest = [e0, ea, eb].filter((p) => p !== other); + const g = graph(codec, [...rest, merge]); + expect(g.getValueHeads()).not.toContain(merge.time); + expect(g.needsMoreHistory()).toBe(true); + g.add([other]); + expect(g.value().toString()).toBe(`${a.getDocument()}`); + }); +}); diff --git a/src/patch-graph.ts b/src/patch-graph.ts index d181c29..8a59fcd 100644 --- a/src/patch-graph.ts +++ b/src/patch-graph.ts @@ -21,6 +21,7 @@ const DEFAULT_EXACT_CACHE_MAX_ENTRIES = 2000; // of a large text would otherwise keep thousands of full copies of it. const DEFAULT_EXACT_CACHE_MAX_SIZE = 10_000_000; const RECENT_EXACT_ENTRIES = 8; +const LEGACY_CHECKPOINTS = 64; export type PatchGraphOptions = { codec: DocCodec; @@ -34,6 +35,16 @@ export type PatchGraphOptions = { // Values computed by the apply-all fallback are approximations and are not // checked. onInconsistency?: (inconsistency: Inconsistency) => void; + // How the value of an unmarked merge commit (several parents, no recorded + // merged value, no hash) is computed, which depends on what wrote the + // history; only its application knows that: + // - "merge3" (default): from the merge3 of its parents, as patchflow 0.9 + // wrote it (exact merges, before hashes); + // - "apply-all": from every patch its parents descend from applied in time + // order, as versions before exact merges (patchflow 0.8 and earlier) + // wrote it, so such a history keeps the values its authors saw (see + // legacyValueOfSet). + unmarkedMerges?: "merge3" | "apply-all"; }; // Size of a cached exact value: characters of a text, records of a database @@ -59,12 +70,10 @@ function isRoot(patch: Patch): boolean { } // A merge commit (several parents) that does not record its merged value -// (see Patch.mergeParent) and has no hash, so was written either by a version that merged -// concurrent patches by applying them all in time order, or with the value of -// that same fallback (see Session.commit). Its value is computed the way its -// author computed the value it is a diff from (see legacyValueOfSet), not -// with merge3, so a history written before exact merges keeps its values. -function isLegacyMerge(patch: Patch): boolean { +// (see Patch.mergeParent) and has no hash: written before merge commits +// recorded their merged value. Which merge its author used cannot be told +// from the patch (see PatchGraphOptions.unmarkedMerges). +function isUnmarkedMerge(patch: Patch): boolean { return ( (patch.parents?.length ?? 0) > 1 && patch.mergeParent == null && @@ -142,10 +151,19 @@ export class PatchGraph { // snapshot never change), and the inconsistencies reported. private snapshotOk = new globalThis.Map(); private reported = new Set(); + // See PatchGraphOptions.unmarkedMerges. + private unmarkedApplyAll: boolean; + // Values of legacy replays (see legacyValueOfSet) after a prefix of their + // time-ordered patches, so a replay that extends an earlier one (e.g. the + // next merge commit) continues from it. + private legacyCheckpoints = new LRUCache({ + max: LEGACY_CHECKPOINTS, + }); constructor(opts: PatchGraphOptions) { this.codec = opts.codec; this.onInconsistency = opts.onInconsistency; + this.unmarkedApplyAll = opts.unmarkedMerges === "apply-all"; const exactMax = opts.exactCacheMaxEntries ?? DEFAULT_EXACT_CACHE_MAX_ENTRIES; const exactMaxSize = opts.exactCacheMaxSize ?? DEFAULT_EXACT_CACHE_MAX_SIZE; const exactOpts = { max: exactMax, maxSize: exactMaxSize, sizeCalculation: docSize }; @@ -410,12 +428,18 @@ export class PatchGraph { return doc; } + // An unmarked merge commit whose value is computed as patchflow 0.8 did + // (see PatchGraphOptions.unmarkedMerges). + private isLegacyMerge(patch: Patch): boolean { + return this.unmarkedApplyAll && isUnmarkedMerge(patch); + } + // The value a patch is a diff from: the merged value of its parents, as // its author computed it (see Patch.mergeParent and isLegacyMerge). private parentsValue(patch: Patch): Document | undefined { const parents = patch.parents ?? []; if (parents.length === 0) return this.codec.fromString(""); - if (isLegacyMerge(patch)) return this.legacyValueOfSet(parents); + if (this.isLegacyMerge(patch)) return this.legacyValueOfSet(parents); const recorded = mergeParent(patch); if (recorded == null) return this.exactValueOfSet(parents); const value = this.exactValue(recorded); @@ -877,6 +901,7 @@ export class PatchGraph { private clearExactCaches(): void { this.hashChecked.clear(); + this.legacyCheckpoints.clear(); this.exactCache.clear(); this.exactMergeCache.clear(); this.recentExact.clear(); @@ -957,7 +982,7 @@ export class PatchGraph { if (snapshot === "bad" && patch.patch == null) return undefined; // A legacy merge commit's value is computed from its parents' history, // not from their exact values (see legacyValueOfSet). - if (isLegacyMerge(patch)) continue; + if (this.isLegacyMerge(patch)) continue; for (const parent of valueParents(patch)) { if (!this.patches.has(parent)) return undefined; uses.set(parent, (uses.get(parent) ?? 0) + 1); @@ -973,7 +998,7 @@ export class PatchGraph { const patch = this.patches.get(t)!; let doc: Document | undefined; const useSnapshot = this.snapshotState(patch) === "use"; - const legacy = !useSnapshot && isLegacyMerge(patch); + const legacy = !useSnapshot && this.isLegacyMerge(patch); const parents = useSnapshot || legacy ? [] : valueParents(patch); if (useSnapshot) { doc = this.codec.fromString(patch.snapshot!); @@ -1058,10 +1083,43 @@ export class PatchGraph { .filter((t) => floor == null || comparePatchId(t, floor) > 0) .map((t) => this.patches.get(t)!) .sort(patchCmp); - this.dedupFileLoads(ordered); - const patches = ordered.filter((p) => p.patch != null).map((p) => p.patch); - let doc = snapshot ? this.codec.fromString(snapshot.snapshot!) : this.codec.fromString(""); + // Continue from the checkpoint of the longest replayed prefix of the same + // patches from the same start. A prefix is identified by a hash of its + // patch times; patches never change, so neither does its value. + const keys: string[] = []; + let h1 = 0x811c9dc5; + let h2 = 0x01000193; + for (const patch of ordered) { + for (let i = 0; i < patch.time.length; i++) { + const c = patch.time.charCodeAt(i); + h1 = Math.imul(h1 ^ c, 0x01000193); + h2 = Math.imul(h2 ^ c, 0x5bd1e995); + } + h1 = Math.imul(h1 ^ 44, 0x01000193); + h2 = Math.imul(h2 ^ 44, 0x5bd1e995); + keys.push(`${floor ?? ""}|${keys.length + 1}|${h1 >>> 0}|${h2 >>> 0}`); + } + let start = 0; + let doc: Document | undefined; + let last: Patch | undefined; + for (let i = ordered.length; i > 0; i--) { + const checkpoint = this.legacyCheckpoints.get(keys[i - 1]); + if (checkpoint != null) { + ({ doc, last } = checkpoint); + start = i; + break; + } + } + doc ??= snapshot ? this.codec.fromString(snapshot.snapshot!) : this.codec.fromString(""); + // Identical file loads close in time apply once, as in dedupFileLoads. + const patches: unknown[] = []; + for (const patch of ordered.slice(start)) { + if (this.isDuplicateFileLoad(last, patch)) continue; + last = patch; + if (patch.patch != null) patches.push(patch.patch); + } if (patches.length > 0) doc = this.codec.applyPatchBatch(doc, patches); + if (ordered.length > 0) this.legacyCheckpoints.set(keys[ordered.length - 1], { doc, last }); this.exactMergeCache.set(key, { doc }); remember(this.recentMerged, key, doc); this.pinnedMerged?.set(key, doc); @@ -1300,6 +1358,21 @@ export class PatchGraph { return best; } + // Whether `patch` repeats the file load `last` (the previous patch kept), + // so applying it again would duplicate the file's content. + private isDuplicateFileLoad(last: Patch | undefined, patch: Patch): boolean { + return !!( + patch.file && + last && + last.file && + last.patch && + patch.patch && + decodePatchId(patch.time).timeMs - decodePatchId(last.time).timeMs <= + this.fileTimeDedupTolerance && + List(patch.patch as unknown[]).equals(List(last.patch as unknown[])) + ); + } + private dedupFileLoads(ordered: Patch[]): void { if (ordered.length < 2) return; let last: Patch | undefined; @@ -1309,15 +1382,7 @@ export class PatchGraph { last = patch; continue; } - if ( - last && - last.file && - last.patch && - patch.patch && - decodePatchId(patch.time).timeMs - decodePatchId(last.time).timeMs <= - this.fileTimeDedupTolerance && - List(patch.patch as unknown[]).equals(List(last.patch as unknown[])) - ) { + if (this.isDuplicateFileLoad(last, patch)) { ordered.splice(i, 1); i -= 1; continue; diff --git a/src/session.ts b/src/session.ts index a932993..7f0868e 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,5 +1,5 @@ import { EventEmitter } from "events"; -import { PatchGraph } from "./patch-graph"; +import { PatchGraph, type PatchGraphOptions } from "./patch-graph"; import { rebaseDraft } from "./working-copy"; import { decodePatchId, encodePatchId } from "./patch-id"; import { makeClientId } from "./client-id"; @@ -35,6 +35,9 @@ export type SessionOptions = { fileAdapter?: FileAdapter; // Optional presence adapter to publish/receive lightweight presence state. presenceAdapter?: PresenceAdapter; + // How to read merge commits written before they recorded their merged + // value; see PatchGraphOptions.unmarkedMerges. + unmarkedMerges?: PatchGraphOptions["unmarkedMerges"]; }; // Parents considered as the one a merge commit records its merged value from @@ -110,6 +113,7 @@ export class Session extends EventEmitter { this.graph = new PatchGraph({ codec: this.codec, onInconsistency: (inconsistency) => this.emit("inconsistency", inconsistency), + unmarkedMerges: opts.unmarkedMerges, }); const factory = opts.clientIdFactory ?? makeClientId; this.clientId = opts.clientId ?? factory(); diff --git a/src/types.ts b/src/types.ts index 5061d0a..d34adbb 100644 --- a/src/types.ts +++ b/src/types.ts @@ -50,8 +50,8 @@ export interface Patch { // (DocCodec.merge3) later never changes the values of patches already in a // history, like a git merge commit, which records its merged tree. Clients // that do not know these fields still read `patch` as before. A merge - // commit without them, written by an earlier version, see isLegacyMerge in - // patch-graph.ts. + // commit without them, written by an earlier version: see + // PatchGraphOptions.unmarkedMerges. mergeParent?: PatchId; mergePatch?: unknown; } From e80ff647752cdad8353a6bc43eab2069b835ac56 Mon Sep 17 00:00:00 2001 From: William Stein Date: Fri, 2 Oct 2026 03:26:00 +0000 Subject: [PATCH 3/4] Drop unmarkedMerges: a merge commit without a hash was written by patchflow 0.8 patchflow 0.9 was never used in production, so a merge commit with no hash and no recorded merged value comes from 0.8 (apply-all), as before 1e62282. Keeps the replay checkpoints and the causal-delivery test. --- README.md | 2 +- src/merge-commit.test.ts | 80 +++------------------------------------- src/patch-graph.ts | 36 +++++------------- src/session.ts | 6 +-- src/types.ts | 4 +- 5 files changed, 19 insertions(+), 109 deletions(-) diff --git a/README.md b/README.md index 8647d8d..7a72576 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ const codec = { A patch that merges concurrent heads (a merge commit, with several parents) is a diff from the merged value of its parents, like any patch. `Session` also records that merged value as its author computed it, as a diff from one of the parents (`Patch.mergeParent`, `Patch.mergePatch`), like a git merge commit records its merged tree. Computing the value of any patch in the history then needs no merge, so a later change to `merge3` only changes how new concurrent edits merge, never the values of patches already written; value hashes keep matching. Clients that do not know these fields read `patch` as before. -Merge commits written before this (no `mergeParent` and no `hash`) cannot tell which merge their author used, so the application says how its older history was written with the `unmarkedMerges` option of `Session`/`PatchGraph`: `"merge3"` (default; patchflow 0.9 exact merges) or `"apply-all"` (patchflow 0.8 and earlier, which applied every patch in time order). With `"apply-all"`, such a merge commit's value is computed the way its author computed it, from the patches its parents descend from, so the values of that history do not change either. +Merge commits written before this (no `mergeParent` and no `hash`) were made by versions that applied every patch in time order (patchflow 0.8 and earlier); their values are computed the same way, from the patches their parents descend from, so the values of such a history do not change either. ## Quickstart diff --git a/src/merge-commit.test.ts b/src/merge-commit.test.ts index e7e6120..b58dba2 100644 --- a/src/merge-commit.test.ts +++ b/src/merge-commit.test.ts @@ -159,8 +159,6 @@ describe("merge commits record their merged value", () => { }); }); -const applyAll = { unmarkedMerges: "apply-all" } as const; - describe("a history written before merge commits recorded their merged value", () => { // Written by versions that applied every patch in time order: no hashes, // merge commits are diffs from that value of their parents. @@ -194,7 +192,7 @@ describe("a history written before merge commits recorded their merged value", ( merges += all.filter((p) => (p.parents?.length ?? 0) > 1).length; const authored = graph(applyAllCodec, all); for (const c of [codec, otherCodec]) { - const g = graph(c, all, [], applyAll); + const g = graph(c, all); for (const p of all) { expect(g.version(p.time).toString()).toBe(authored.version(p.time).toString()); } @@ -220,7 +218,7 @@ describe("a history written before merge commits recorded their merged value", ( patch(5, [4, 3], "A\nB\nC\n", "A\nB\nC\nd\n"), ]; const snapshot: Patch = { ...ps[3], isSnapshot: true, snapshot: "A\nB\nc\n" }; - const g = graph(codec, [...ps.slice(0, 3), snapshot, ps[4]], [], applyAll); + const g = graph(codec, [...ps.slice(0, 3), snapshot, ps[4]]); expect(g.version(id(5)).toString()).toBe("A\nB\nC\nd\n"); }); }); @@ -241,76 +239,8 @@ describe("undo of a merge commit", () => { }); }); -describe("unmarked merge commits", () => { - // Written by the Session of patchflow 0.9.3 (exact merges, no hashes, no - // recorded merged value): two writers append to "hello" concurrently, then - // one merges and adds a line. Its author saw "helloA\nhelloB\ntail\n". - const authored = "helloA\nhelloB\ntail\n"; - const written: Patch[] = [ - { time: "000000000rs_A", parents: [], patch: [[[[1, "hello\n"]], 0, 0, 0, 6]] }, - { - time: "000000000ru_A", - parents: ["000000000rs_A"], - patch: [ - [ - [ - [0, "hello"], - [1, "A"], - [0, "\n"], - ], - 0, - 0, - 6, - 7, - ], - ], - }, - { - time: "000000000rw_B", - parents: ["000000000rs_A"], - patch: [ - [ - [ - [0, "hello"], - [1, "B"], - [0, "\n"], - ], - 0, - 0, - 6, - 7, - ], - ], - }, - { - time: "000000000ry_A", - parents: ["000000000ru_A", "000000000rw_B"], - patch: [ - [ - [ - [0, "\nhelloB\n"], - [1, "tail\n"], - ], - 6, - 6, - 8, - 13, - ], - ], - }, - ]; - - it("are read with merge3 by default, as patchflow 0.9 wrote them", () => { - expect(graph(codec, written).value().toString()).toBe(authored); - }); - - it("are read as patchflow 0.8 wrote them only when the application says so", () => { - // This history was not written that way, so that reading differs: which - // one is right is known only to the application. - expect(graph(codec, written, [], applyAll).value().toString()).not.toBe(authored); - }); - - it("read as patchflow 0.8 wrote them, each merge continues the previous replay", async () => { +describe("merge commits written before hashes", () => { + it("each merge continues the previous replay", async () => { // Rounds of: fork the last merge, one line on each side, merge. Reading // every merge as it arrives applies each patch once, not the whole // history again for every merge. @@ -354,7 +284,7 @@ describe("unmarked merge commits", () => { return d.applyPatchBatch(ps); }, }; - const g = new PatchGraph({ codec: counting, ...applyAll }); + const g = new PatchGraph({ codec: counting }); g.add([all[0]]); let checked = 0; for (let i = 1; i < all.length; i += 3) { diff --git a/src/patch-graph.ts b/src/patch-graph.ts index 8a59fcd..d86787a 100644 --- a/src/patch-graph.ts +++ b/src/patch-graph.ts @@ -35,16 +35,6 @@ export type PatchGraphOptions = { // Values computed by the apply-all fallback are approximations and are not // checked. onInconsistency?: (inconsistency: Inconsistency) => void; - // How the value of an unmarked merge commit (several parents, no recorded - // merged value, no hash) is computed, which depends on what wrote the - // history; only its application knows that: - // - "merge3" (default): from the merge3 of its parents, as patchflow 0.9 - // wrote it (exact merges, before hashes); - // - "apply-all": from every patch its parents descend from applied in time - // order, as versions before exact merges (patchflow 0.8 and earlier) - // wrote it, so such a history keeps the values its authors saw (see - // legacyValueOfSet). - unmarkedMerges?: "merge3" | "apply-all"; }; // Size of a cached exact value: characters of a text, records of a database @@ -70,10 +60,13 @@ function isRoot(patch: Patch): boolean { } // A merge commit (several parents) that does not record its merged value -// (see Patch.mergeParent) and has no hash: written before merge commits -// recorded their merged value. Which merge its author used cannot be told -// from the patch (see PatchGraphOptions.unmarkedMerges). -function isUnmarkedMerge(patch: Patch): boolean { +// (see Patch.mergeParent) and has no hash, so was written by a version that +// applied every patch in time order (patchflow 0.8 and earlier; 0.9 was never +// used in production), or with the value of that same fallback (see +// Session.commit). Its value is computed the way its author computed the value +// it is a diff from (see legacyValueOfSet), not with merge3, so a history +// written before exact merges keeps its values. +function isLegacyMerge(patch: Patch): boolean { return ( (patch.parents?.length ?? 0) > 1 && patch.mergeParent == null && @@ -151,8 +144,6 @@ export class PatchGraph { // snapshot never change), and the inconsistencies reported. private snapshotOk = new globalThis.Map(); private reported = new Set(); - // See PatchGraphOptions.unmarkedMerges. - private unmarkedApplyAll: boolean; // Values of legacy replays (see legacyValueOfSet) after a prefix of their // time-ordered patches, so a replay that extends an earlier one (e.g. the // next merge commit) continues from it. @@ -163,7 +154,6 @@ export class PatchGraph { constructor(opts: PatchGraphOptions) { this.codec = opts.codec; this.onInconsistency = opts.onInconsistency; - this.unmarkedApplyAll = opts.unmarkedMerges === "apply-all"; const exactMax = opts.exactCacheMaxEntries ?? DEFAULT_EXACT_CACHE_MAX_ENTRIES; const exactMaxSize = opts.exactCacheMaxSize ?? DEFAULT_EXACT_CACHE_MAX_SIZE; const exactOpts = { max: exactMax, maxSize: exactMaxSize, sizeCalculation: docSize }; @@ -428,18 +418,12 @@ export class PatchGraph { return doc; } - // An unmarked merge commit whose value is computed as patchflow 0.8 did - // (see PatchGraphOptions.unmarkedMerges). - private isLegacyMerge(patch: Patch): boolean { - return this.unmarkedApplyAll && isUnmarkedMerge(patch); - } - // The value a patch is a diff from: the merged value of its parents, as // its author computed it (see Patch.mergeParent and isLegacyMerge). private parentsValue(patch: Patch): Document | undefined { const parents = patch.parents ?? []; if (parents.length === 0) return this.codec.fromString(""); - if (this.isLegacyMerge(patch)) return this.legacyValueOfSet(parents); + if (isLegacyMerge(patch)) return this.legacyValueOfSet(parents); const recorded = mergeParent(patch); if (recorded == null) return this.exactValueOfSet(parents); const value = this.exactValue(recorded); @@ -982,7 +966,7 @@ export class PatchGraph { if (snapshot === "bad" && patch.patch == null) return undefined; // A legacy merge commit's value is computed from its parents' history, // not from their exact values (see legacyValueOfSet). - if (this.isLegacyMerge(patch)) continue; + if (isLegacyMerge(patch)) continue; for (const parent of valueParents(patch)) { if (!this.patches.has(parent)) return undefined; uses.set(parent, (uses.get(parent) ?? 0) + 1); @@ -998,7 +982,7 @@ export class PatchGraph { const patch = this.patches.get(t)!; let doc: Document | undefined; const useSnapshot = this.snapshotState(patch) === "use"; - const legacy = !useSnapshot && this.isLegacyMerge(patch); + const legacy = !useSnapshot && isLegacyMerge(patch); const parents = useSnapshot || legacy ? [] : valueParents(patch); if (useSnapshot) { doc = this.codec.fromString(patch.snapshot!); diff --git a/src/session.ts b/src/session.ts index 7f0868e..a932993 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1,5 +1,5 @@ import { EventEmitter } from "events"; -import { PatchGraph, type PatchGraphOptions } from "./patch-graph"; +import { PatchGraph } from "./patch-graph"; import { rebaseDraft } from "./working-copy"; import { decodePatchId, encodePatchId } from "./patch-id"; import { makeClientId } from "./client-id"; @@ -35,9 +35,6 @@ export type SessionOptions = { fileAdapter?: FileAdapter; // Optional presence adapter to publish/receive lightweight presence state. presenceAdapter?: PresenceAdapter; - // How to read merge commits written before they recorded their merged - // value; see PatchGraphOptions.unmarkedMerges. - unmarkedMerges?: PatchGraphOptions["unmarkedMerges"]; }; // Parents considered as the one a merge commit records its merged value from @@ -113,7 +110,6 @@ export class Session extends EventEmitter { this.graph = new PatchGraph({ codec: this.codec, onInconsistency: (inconsistency) => this.emit("inconsistency", inconsistency), - unmarkedMerges: opts.unmarkedMerges, }); const factory = opts.clientIdFactory ?? makeClientId; this.clientId = opts.clientId ?? factory(); diff --git a/src/types.ts b/src/types.ts index d34adbb..5061d0a 100644 --- a/src/types.ts +++ b/src/types.ts @@ -50,8 +50,8 @@ export interface Patch { // (DocCodec.merge3) later never changes the values of patches already in a // history, like a git merge commit, which records its merged tree. Clients // that do not know these fields still read `patch` as before. A merge - // commit without them, written by an earlier version: see - // PatchGraphOptions.unmarkedMerges. + // commit without them, written by an earlier version, see isLegacyMerge in + // patch-graph.ts. mergeParent?: PatchId; mergePatch?: unknown; } From bcd25e071478940e61f7be16afb6c4dbd447210a Mon Sep 17 00:00:00 2001 From: William Stein Date: Fri, 2 Oct 2026 03:34:32 +0000 Subject: [PATCH 4/4] Legacy replay checkpoints: bounded by document size; the newest kept apart Review of #16 (P2): the checkpoint cache was bounded only by count (64), so a large document kept up to 64 full versions (64M characters at 1M). Bound it by size like the exact caches (exactCacheMaxSize, docSize), and keep the newest checkpoint apart, so reading merge by merge continues from the last replay even when one version does not fit the cache. Test: a 200,000 character document with a 150,000 cache keeps no more than that and still applies under 4 patches per patch (fails with a count-only cache, and without the newest checkpoint: 1,425 applications). --- src/merge-commit.test.ts | 59 ++++++++++++++++++++++++++++++++++++++++ src/patch-graph.ts | 29 ++++++++++++++++---- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/src/merge-commit.test.ts b/src/merge-commit.test.ts index b58dba2..7e5fd7d 100644 --- a/src/merge-commit.test.ts +++ b/src/merge-commit.test.ts @@ -321,3 +321,62 @@ describe("a merge commit with a parent not yet received", () => { expect(g.value().toString()).toBe(`${a.getDocument()}`); }); }); + +describe("replay checkpoints of a large document", () => { + it("are bounded by size, and reading merge by merge still continues from the last", async () => { + let now = 1000; + const clock = () => now++; + const make = async (id: string) => { + const s = new Session({ + codec: applyAllCodec, + patchStore: new MemoryPatchStore(), + clock, + clientId: id, + }); + await s.init(); + return s; + }; + const a = await make("a"); + const b = await make("b"); + const big = "x".repeat(99) + "\n"; + const all: PatchEnvelope[] = [a.commit(doc(big.repeat(2000)))]; // 200,000 characters + b.applyRemote(all[0]); + const values = new Map(); + const rounds = 30; + for (let i = 0; i < rounds; i++) { + const l = a.commit(doc(`${a.getDocument()}L${i}\n`)); + const r = b.commit(doc(`R${i}\n${b.getDocument()}`)); + a.applyRemote(r); + const m = a.commit(doc(`${a.getDocument()}M${i}\n`)); + b.applyRemoteBatch([l, m]); + all.push(l, r, m); + values.set(m.time, `${a.getDocument()}`); + } + let applied = 0; + const counting: DocCodec = { + ...codec, + applyPatch: (d, p) => { + applied++; + return d.applyPatch(p); + }, + applyPatchBatch: (d, ps) => { + applied += ps.length; + return d.applyPatchBatch(ps); + }, + }; + // Less room than one version of the document: only the newest checkpoint, + // kept apart, lets the next merge continue. + const maxSize = 150_000; + const g = new PatchGraph({ codec: counting, exactCacheMaxSize: maxSize }); + g.add([all[0]]); + for (let i = 1; i < all.length; i += 3) { + const round = all.slice(i, i + 3); + g.add(round); + expect(g.value().toString()).toBe(values.get(round[2].time)); + let kept = 0; + for (const { doc } of (g as any).legacyCheckpoints.values()) kept += doc.toString().length; + expect(kept).toBeLessThanOrEqual(maxSize); + } + expect(applied).toBeLessThan(4 * all.length); + }); +}); diff --git a/src/patch-graph.ts b/src/patch-graph.ts index d86787a..316afbf 100644 --- a/src/patch-graph.ts +++ b/src/patch-graph.ts @@ -89,6 +89,10 @@ function valueParents(patch: Patch): string[] { return parent != null ? [parent] : (patch.parents ?? []); } +// The value after replaying a prefix of a legacy merge's patches, and the +// last patch kept (see legacyValueOfSet). +type LegacyCheckpoint = { doc: Document; last?: Patch }; + function patchCmp(a: Patch, b: Patch): number { return comparePatchId(a.time, b.time); } @@ -147,9 +151,11 @@ export class PatchGraph { // Values of legacy replays (see legacyValueOfSet) after a prefix of their // time-ordered patches, so a replay that extends an earlier one (e.g. the // next merge commit) continues from it. - private legacyCheckpoints = new LRUCache({ - max: LEGACY_CHECKPOINTS, - }); + // Bounded by size like the exact caches; the newest is also kept apart + // (like recentExact), so a document too large for the cache still continues + // from the previous replay. + private legacyCheckpoints: LRUCache; + private recentCheckpoint?: { key: string } & LegacyCheckpoint; constructor(opts: PatchGraphOptions) { this.codec = opts.codec; @@ -159,6 +165,10 @@ export class PatchGraph { const exactOpts = { max: exactMax, maxSize: exactMaxSize, sizeCalculation: docSize }; this.exactCache = new LRUCache(exactOpts); this.exactMergeCache = new LRUCache(exactOpts); + this.legacyCheckpoints = new LRUCache({ + ...exactOpts, + max: LEGACY_CHECKPOINTS, + }); this.mergeStrategy = opts.mergeStrategy ?? "three-way"; const maxSize = opts.valueCacheMaxSize ?? DEFAULT_VALUE_CACHE_MAX_SIZE; const maxEntries = opts.valueCacheMaxEntries ?? DEFAULT_VALUE_CACHE_MAX_ENTRIES; @@ -886,6 +896,7 @@ export class PatchGraph { private clearExactCaches(): void { this.hashChecked.clear(); this.legacyCheckpoints.clear(); + this.recentCheckpoint = undefined; this.exactCache.clear(); this.exactMergeCache.clear(); this.recentExact.clear(); @@ -1087,7 +1098,11 @@ export class PatchGraph { let doc: Document | undefined; let last: Patch | undefined; for (let i = ordered.length; i > 0; i--) { - const checkpoint = this.legacyCheckpoints.get(keys[i - 1]); + const key = keys[i - 1]; + const checkpoint = + this.recentCheckpoint?.key === key + ? this.recentCheckpoint + : this.legacyCheckpoints.get(key); if (checkpoint != null) { ({ doc, last } = checkpoint); start = i; @@ -1103,7 +1118,11 @@ export class PatchGraph { if (patch.patch != null) patches.push(patch.patch); } if (patches.length > 0) doc = this.codec.applyPatchBatch(doc, patches); - if (ordered.length > 0) this.legacyCheckpoints.set(keys[ordered.length - 1], { doc, last }); + if (ordered.length > 0) { + const key = keys[ordered.length - 1]; + this.legacyCheckpoints.set(key, { doc, last }); + this.recentCheckpoint = { key, doc, last }; + } this.exactMergeCache.set(key, { doc }); remember(this.recentMerged, key, doc); this.pinnedMerged?.set(key, doc);