diff --git a/hub-client/src/services/attribution-runs.test.ts b/hub-client/src/services/attribution-runs.test.ts index 3f35673a..ff5f155f 100644 --- a/hub-client/src/services/attribution-runs.test.ts +++ b/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): DocHandle { + const view: ViewableHandle = { + history: () => A.topoHistoryTraversal(doc).map(h => encodeHeads([h])), + metadata: () => undefined, + doc: () => doc, + }; + return view as unknown as DocHandle; +} + +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({ text: '' }, { actor: aliceActor }); + let bob = A.load(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({ text: '' }, { actor: aliceActor }); + let bob = A.load(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({ 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); + }); +}); diff --git a/hub-client/src/services/attribution-runs.ts b/hub-client/src/services/attribution-runs.ts index a86ce3ec..569258b9 100644 --- a/hub-client/src/services/attribution-runs.ts +++ b/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; - const newChanges = getChanges(state._workDoc, doc); - const changeByHash = new Map(); - 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[0]); - let lastHeads: unknown[] = state.processedHeads; let workDoc: Doc = state._workDoc; - - for (let i = state.processedHistoryIndex; i < history.length; i++) { - const currHeads = history[i]; - const decodedCurr = decodeHeads(currHeads as Parameters[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, }; }