hub-client: simplify the per-keystroke attribution update

hub-client/src/services/attribution-runs.test.ts

@@ -11,11 +11,18 @@
  */
 
 import { describe, it, expect } from 'vitest';
+import { next as A } from '@automerge/automerge';
+import type { Doc } from '@automerge/automerge';
+import { encodeHeads } from '@automerge/automerge-repo';
+import type { DocHandle } from '@automerge/automerge-repo';
 
 import {
   buildCharToByteMap,
+  buildRunListAttribution,
   runsCharToByteOffsets,
+  updateRunListAttribution,
   type AttributionRun,
+  type ViewableHandle,
 } from './attribution-runs';
 
 describe('buildCharToByteMap', () => {
@@ -75,3 +82,91 @@ describe('runsCharToByteOffsets', () => {
     expect(out).toEqual(runs);
   });
 });
+
+// ---------------------------------------------------------------------------
+// Incremental ≡ from-scratch invariant
+// ---------------------------------------------------------------------------
+
+// Anchor for the incremental path: whatever shortcut `updateRunListAttribution`
+// uses to skip work, the final run list must agree character-for-character with
+// what `buildRunListAttribution` produces from `init()` on the same final doc.
+// If a future refactor breaks this — including via something subtle at the
+// Automerge boundary (history-traversal ordering, getChanges semantics, etc.)
+// — this is the test that should catch it.
+
+interface TDoc { text: string }
+
+function fakeHandle(doc: Doc<TDoc>): DocHandle<unknown> {
+  const view: ViewableHandle = {
+    history: () => A.topoHistoryTraversal(doc).map(h => encodeHeads([h])),
+    metadata: () => undefined,
+    doc: () => doc,
+  };
+  return view as unknown as DocHandle<unknown>;
+}
+
+describe('updateRunListAttribution invariant', () => {
+  it('matches a from-scratch rebuild after a concurrent merge', async () => {
+    const aliceActor = 'f'.repeat(32);
+    const bobActor = '0'.repeat(32);
+
+    let alice = A.from<TDoc>({ text: '' }, { actor: aliceActor });
+    let bob = A.load<TDoc>(A.save(alice), { actor: bobActor });
+
+    alice = A.change(alice, d => A.splice(d, ['text'], 0, 0, 'Hello'));
+    alice = A.change(alice, d => A.splice(d, ['text'], 5, 0, ' World'));
+    alice = A.change(alice, d => A.splice(d, ['text'], 11, 0, '!'));
+
+    const stateBefore = await buildRunListAttribution(fakeHandle(alice), 'text');
+    expect(stateBefore).toBeTruthy();
+
+    bob = A.change(bob, d => A.splice(d, ['text'], 0, 0, 'X'));
+    bob = A.change(bob, d => A.splice(d, ['text'], 1, 0, 'Y'));
+    bob = A.change(bob, d => A.splice(d, ['text'], 2, 0, 'Z'));
+    alice = A.merge(alice, bob);
+
+    const incremental = updateRunListAttribution(stateBefore!, fakeHandle(alice), 'text');
+    const fromScratch = await buildRunListAttribution(fakeHandle(alice), 'text');
+
+    expect(fromScratch).toBeTruthy();
+    expect(incremental.runs).toEqual(fromScratch!.runs);
+  });
+
+  it('matches a from-scratch rebuild across interleaved local and merged remote edits', async () => {
+    const aliceActor = 'f'.repeat(32);
+    const bobActor = '0'.repeat(32);
+
+    let alice = A.from<TDoc>({ text: '' }, { actor: aliceActor });
+    let bob = A.load<TDoc>(A.save(alice), { actor: bobActor });
+
+    alice = A.change(alice, d => A.splice(d, ['text'], 0, 0, 'A1'));
+    const stateBefore = await buildRunListAttribution(fakeHandle(alice), 'text');
+    expect(stateBefore).toBeTruthy();
+
+    bob = A.change(bob, d => A.splice(d, ['text'], 0, 0, 'B1'));
+    bob = A.change(bob, d => A.splice(d, ['text'], 2, 0, 'B2'));
+    alice = A.merge(alice, bob);
+    alice = A.change(alice, d => A.splice(d, ['text'], 0, 0, 'A2'));
+    alice = A.change(alice, d => A.splice(d, ['text'], 0, 0, 'A3'));
+
+    const incremental = updateRunListAttribution(stateBefore!, fakeHandle(alice), 'text');
+    const fromScratch = await buildRunListAttribution(fakeHandle(alice), 'text');
+
+    expect(fromScratch).toBeTruthy();
+    expect(incremental.runs).toEqual(fromScratch!.runs);
+  });
+
+  it('returns state unchanged when no new changes are present', async () => {
+    const aliceActor = 'f'.repeat(32);
+    let alice = A.from<TDoc>({ text: '' }, { actor: aliceActor });
+    alice = A.change(alice, d => A.splice(d, ['text'], 0, 0, 'hello'));
+
+    const stateBefore = await buildRunListAttribution(fakeHandle(alice), 'text');
+    expect(stateBefore).toBeTruthy();
+
+    // No edits between build and update — the incremental path should
+    // short-circuit and return the same runs.
+    const incremental = updateRunListAttribution(stateBefore!, fakeHandle(alice), 'text');
+    expect(incremental.runs).toEqual(stateBefore!.runs);
+  });
+});

hub-client/src/services/attribution-runs.ts

@@ -26,6 +26,7 @@ import {
   decodeChange,
   getAllChanges,
   getChanges,
+  getHeads,
   init,
 } from '@automerge/automerge';
 import type { Change, Doc, Patch } from '@automerge/automerge';
@@ -350,44 +351,35 @@ export function updateRunListAttribution(
   textFieldName: string,
 ): RunListAttribution {
   const viewable = handle as unknown as ViewableHandle;
-  const history = viewable.history();
-  if (!history) throw new HistoryCompactedError();
-  if (state.processedHistoryIndex > history.length) throw new HistoryCompactedError();
   if (!state._workDoc) throw new HistoryCompactedError();
 
-  if (state.processedHistoryIndex === history.length) {
-    return state;
-  }
-
-  // Pull just the new changes (since workDoc's heads), index by hash.
   const doc = viewable.doc() as Doc<unknown>;
-  const newChanges = getChanges(state._workDoc, doc);
-  const changeByHash = new Map<string, Change>();
-  for (const c of newChanges) {
-    changeByHash.set(decodeChange(c).hash, c);
+  let newChanges: Change[];
+  try {
+    newChanges = getChanges(state._workDoc, doc);
+  } catch {
+    // `getChanges` throws if `oldState` has changes not in `newState`.
+    // Production state machine never hits that — `_workDoc` only ever
+    // advances by applying changes from `doc` — but guard against
+    // hand-constructed state in tests and surface as a cold-rebuild signal.
+    throw new HistoryCompactedError();
   }
+  if (newChanges.length === 0) return state;
 
   const runs = state.runs.map(r => ({ ...r }));
-  let prevHeads = decodeHeads(state.processedHeads as Parameters<typeof decodeHeads>[0]);
-  let lastHeads: unknown[] = state.processedHeads;
   let workDoc: Doc<unknown> = state._workDoc;
-
-  for (let i = state.processedHistoryIndex; i < history.length; i++) {
-    const currHeads = history[i];
-    const decodedCurr = decodeHeads(currHeads as Parameters<typeof decodeHeads>[0]);
-    const newHash = newChangeHashAt(prevHeads, decodedCurr);
-    const change = newHash ? changeByHash.get(newHash) : undefined;
-    if (change) {
-      workDoc = replayChange(workDoc, change, textFieldName, runs);
-    }
-    prevHeads = decodedCurr;
-    lastHeads = Array.isArray(currHeads) ? currHeads : [currHeads];
+  for (const change of newChanges) {
+    workDoc = replayChange(workDoc, change, textFieldName, runs);
   }
 
+  // `processedHeads` / `processedHistoryIndex` are bookkeeping in the
+  // incremental path — neither is read here. Updating them keeps the state
+  // truthful for the cold-start path and for any future consumer.
+  const history = viewable.history();
   return {
     runs,
-    processedHeads: lastHeads as unknown[],
-    processedHistoryIndex: history.length,
+    processedHeads: getHeads(workDoc),
+    processedHistoryIndex: history?.length ?? state.processedHistoryIndex,
     _workDoc: workDoc,
   };
 }