Mark patches committed without exact values; read only unmarked merges as patchflow 0.8 wrote them

src/merge-commit.test.ts

@@ -4,6 +4,7 @@ import { MemoryPatchStore } from "./adapters/memory-patch-store";
 import { mergeStrings3 } from "./merge3";
 import { StringCodec, StringDocument } from "./string-document";
 import { legacyPatchId } from "./patch-id";
+import { hashString } from "./value-hash";
 import type { DocCodec, Inconsistency, Patch, PatchEnvelope } from "./types";
 
 const doc = (s: string) => new StringDocument(s);
@@ -184,6 +185,25 @@ describe("a history written before merge commits recorded their merged value", (
     return all;
   }
 
+  it("is what a current session without merge3 writes: no inexact marker", async () => {
+    // The reviewer's case: a current apply-all writer, read by exact readers
+    // without changing its envelopes.
+    const [a, b] = await writers(applyAllCodec, 2);
+    const root = a.commit(doc("hello\n"));
+    b.applyRemote(root);
+    const left = a.commit(doc("helloA\n"));
+    const right = b.commit(doc("helloB\n"));
+    a.applyRemote(right);
+    const authored = `${a.getDocument()}tail\n`;
+    const last = a.commit(doc(authored));
+    a.close();
+    b.close();
+    for (const p of [root, left, right, last]) expect(p.inexact).toBeUndefined();
+    for (const c of [codec, otherCodec]) {
+      expect(graph(c, [root, left, right, last]).version(last.time).toString()).toBe(authored);
+    }
+  });
+
   it("keeps the values its authors saw, whatever the merge algorithm", async () => {
     let merges = 0;
     for (let seed = 1; seed <= 20; seed++) {
@@ -380,3 +400,51 @@ describe("replay checkpoints of a large document", () => {
     expect(applied).toBeLessThan(4 * all.length);
   });
 });
+
+describe("a merge commit made without the exact value of its parents", () => {
+  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)),
+    hash: hashString(to),
+  });
+  // 2 deletes a line that 3 edits concurrently; 4 merges them (the edit
+  // wins) and is snapshotted; 5 edits a line that 6 deletes concurrently.
+  const ps = [
+    patch(1, [], "", "a\nb\nc\n"),
+    patch(2, [1], "a\nb\nc\n", "a\nc\n"),
+    patch(3, [1], "a\nb\nc\n", "a\nB\nc\n"),
+    patch(4, [2, 3], "a\nB\nc\n", "a\nB\nc\nd\n"),
+    patch(5, [4], "a\nB\nc\nd\n", "a\nB\nC!\nd\n"),
+    patch(6, [3], "a\nB\nc\n", "a\nB\n"),
+  ];
+  const snapshot: Patch = { ...ps[3], isSnapshot: true, snapshot: "a\nB\nc\nd\n" };
+
+  it("is marked inexact, and read with merge3", async () => {
+    // A client with the snapshot, 5 and 6, but not 3 (below the snapshot):
+    // it cannot compute the exact merge of 5 and 6.
+    const s = new Session({
+      codec,
+      patchStore: new MemoryPatchStore([snapshot, ps[4], ps[5]]),
+      clientId: "late",
+    });
+    await s.init();
+    expect(s.getHeads()).toEqual([id(5), id(6)].sort());
+    const merge = s.commit(doc(`${s.getDocument()}g\n`));
+    s.close();
+    expect(merge.parents).toHaveLength(2);
+    expect(merge.hash).toBeUndefined();
+    expect(merge.mergeParent).toBeUndefined();
+    expect(merge.inexact).toBe(true);
+    // Read with merge3, like any merge commit of this version, not by
+    // replaying the history in time order (which depends on the history a
+    // client has loaded, and here differs).
+    const full = graph(codec, [...ps, snapshot, merge]);
+    const value = full.version(merge.time).toString();
+    const merged = full.exactValueOf(merge.parents!)!;
+    expect(value).toBe(codec.applyPatch(merged, merge.patch).toString());
+    const legacy = graph(codec, [...ps, snapshot, { ...merge, inexact: undefined }]);
+    expect(legacy.version(merge.time).toString()).not.toBe(value);
+  });
+});

src/patch-graph.ts

@@ -60,17 +60,21 @@ 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 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.
+// (see Patch.mergeParent), has no hash and is not marked inexact, 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). 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.
+// A merge commit written by this version without the exact value of its
+// parents is marked inexact (Patch.inexact) and read with merge3, which, unlike
+// replaying the history in time order, does not depend on how much of the
+// history a client has loaded.
 function isLegacyMerge(patch: Patch): boolean {
   return (
     (patch.parents?.length ?? 0) > 1 &&
     patch.mergeParent == null &&
     patch.hash == null &&
+    !patch.inexact &&
     patch.patch != null
   );
 }
@@ -776,6 +780,13 @@ export class PatchGraph {
     return hashString(this.codec.toString(doc));
   }
 
+  // Whether this graph computes exact values (the codec provides merge3 and
+  // the strategy is not apply-all). Otherwise every value applies all patches
+  // in time order, as patchflow 0.8 did.
+  computesExactValues(): boolean {
+    return this.codec.merge3 != null && this.mergeStrategy !== "apply-all";
+  }
+
   // The exact merged value of a set of patches (e.g. the parents of a new
   // patch), or undefined if it cannot be computed from the loaded history or
   // the graph does not compute exact values.

src/session.ts

@@ -367,6 +367,7 @@ export class Session extends EventEmitter {
       source: opts.source,
       meta: opts.meta,
       hash: parentsValue == null ? undefined : this.graph.hashOf(nextDoc),
+      ...this.inexactMarker(parentsValue),
       ...this.recordMerge(parents, parentsValue),
     };
     this.graph.add([envelope]);
@@ -405,6 +406,15 @@ export class Session extends EventEmitter {
     return best == null ? {} : { mergeParent: best.parent, mergePatch: best.patch };
   }
 
+  // A patch committed without the exact value of its parents by a session
+  // that computes exact values (e.g. before the history needed for it is
+  // loaded) is marked inexact (see Patch.inexact). A session that never
+  // computes exact values (no merge3, or apply-all) writes history the way
+  // patchflow 0.8 did, and its patches are read that way: no marker.
+  private inexactMarker(parentsValue: Document | undefined): Pick<PatchEnvelope, "inexact"> {
+    return parentsValue == null && this.graph.computesExactValues() ? { inexact: true } : {};
+  }
+
   // Merge a remote patch and refresh the current document.
   applyRemote(env: PatchEnvelope): void {
     this.applyRemoteBatch([env]);
@@ -743,6 +753,7 @@ export class Session extends EventEmitter {
       version: nextVersion,
       file: true,
       hash: parentsValue == null ? undefined : this.graph.hashOf(newDoc),
+      ...this.inexactMarker(parentsValue),
       ...this.recordMerge(parents, parentsValue),
     };
     this.graph.add([envelope]);

src/types.ts

@@ -54,6 +54,12 @@ export interface Patch {
   // patch-graph.ts.
   mergeParent?: PatchId;
   mergePatch?: unknown;
+  // Set by this version on a patch committed without the exact value of its
+  // parents (e.g. before the history needed for it was loaded): `patch` is a
+  // diff from an approximation of that value, and the patch has no hash. It
+  // tells such a patch apart from one written by a version before exact
+  // merges (see isLegacyMerge in patch-graph.ts).
+  inexact?: boolean;
 }
 
 // A value that does not match the hash its author recorded (Patch.hash).