From c48b04a6387b3cda3f462c945a6920439b37bbfb Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 19 Oct 2026 14:49:14 -0400 Subject: [PATCH 01/15] Add a repository conformance suite (#432) describeAnalysisRepositoryConformance declares the tests every AnalysisRepository implementation must pass, given a store it can open repositories over and plant a newer build's draft in. It covers the whole-draft contract: an empty draft when none is stored, round trips of every record kind and optional field within one repository and across reopening, whole replacement (with or without a prior load), a payload shared across books kept and edited as one record, model-version stamping, JSON-plain values with optional fields omitted rather than null, copy semantics in both directions, isolation between source projects, and refusal to read or overwrite a newer build's draft. Record lists are compared in id order, since the seam promises no list order; arrays inside records must keep theirs. The AnalysisRepository docs now state the empty-draft, ordering and stamping rules the suite checks. draftRepository.conformance.test.ts runs it against DraftRepository over map-backed storage, failing on any warning DraftRepository logs. The map storage and silent logger move to test-helpers. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 +- .../analysis-repository-conformance.ts | 349 ++++++++++++++++++ .../draftRepository.conformance.test.ts | 28 ++ .../services/draftRepository.test.ts | 24 +- src/__tests__/test-helpers.ts | 28 ++ .../utils/analysis-violations.test.ts | 7 +- src/types/analysis-repository.ts | 14 +- 7 files changed, 418 insertions(+), 36 deletions(-) create mode 100644 src/__tests__/analysis-repository-conformance.ts create mode 100644 src/__tests__/services/draftRepository.conformance.test.ts diff --git a/AGENTS.md b/AGENTS.md index 76021a292..b63433051 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,7 +79,9 @@ The WebView root component is assigned to `globalThis.webViewComponent` (not exp Drafts are loaded and saved through [AnalysisRepository](src/types/analysis-repository.ts), the seam a later storage backend implements. [DraftRepository](src/services/draftRepository.ts) implements it over the draft envelope and its per-book shards, and rewrites only the books that changed. It imports nothing from PAPI and reaches storage only through a `KeyValueStorage`. That lets the perf harness run it in Node, straight over the storage files ([perf/storage.ts](perf/storage.ts)). -Saved projects are listed through [ProjectSummaries](src/services/projectSummaries.ts): one record holding every project's summary, rewritten on each project write so listing reads no analysis. It is PAPI-free in the same way. +[The conformance suite](src/__tests__/analysis-repository-conformance.ts) defines the seam: every `AnalysisRepository` implementation must pass it, and [draftRepository.conformance.test.ts](src/__tests__/services/draftRepository.conformance.test.ts) runs it against `DraftRepository`. A behavior every implementation owes goes in the suite; one particular to an implementation, such as which shards a save rewrites, goes in that implementation's own tests. A later backend runs the suite by passing `describeAnalysisRepositoryConformance` a store it can open repositories over. + +Saved projects are listed through [ProjectSummaries](src/services/projectSummaries.ts): one record holding every project's summary, rewritten on each project write so listing reads no analysis. Like `DraftRepository`, it is PAPI-free. ### Components diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts new file mode 100644 index 000000000..af641b1d6 --- /dev/null +++ b/src/__tests__/analysis-repository-conformance.ts @@ -0,0 +1,349 @@ +/// + +import type { DraftProject, TextAnalysis } from 'interlinearizer'; +import type { AnalysisRepository } from '../types/analysis-repository'; +import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; +import { CURRENT_MODEL_VERSION } from '../types/model-version'; +import { FIXTURE_STAMPS, makePhraseLink } from './test-helpers'; + +/** One empty store an {@link AnalysisRepository} implementation keeps drafts in. */ +export interface ConformanceStore { + /** + * Opens a repository for `sourceProjectId`. Every repository this store opens reaches the same + * stored drafts, as one opened after a restart would. + */ + open(sourceProjectId: string): Promise; + + /** Stores a draft for `sourceProjectId` as a build with a newer model version would write it. */ + storeNewerBuildDraft(sourceProjectId: string): Promise; +} + +const SOURCE = 'source-project'; + +function tokenLink(id: string, analysisId: string, tokenRef: string, surfaceText: string) { + return { + id, + analysisId, + ...FIXTURE_STAMPS, + status: 'approved' as const, + token: { tokenRef, surfaceText }, + }; +} + +/** + * Every record kind and every optional field, spread over GEN and EXO. One payload is linked from + * both books and one from neither. Arrays inside records are out of sorted order, so a repository + * that sorts them fails. + */ +function fullAnalysis(): TextAnalysis { + return { + tokenAnalyses: [ + { id: 'ta-and', ...FIXTURE_STAMPS, surfaceText: 'and', gloss: { en: 'and', fr: 'et' } }, + { + id: 'ta-beginning', + ...FIXTURE_STAMPS, + surfaceText: 'beginning', + confidence: 'medium', + producer: 'pt9-import', + sourceUser: 'translator', + morphemes: [ + { + id: 'm-2', + form: 'begin', + writingSystem: 'en', + entryRef: { authority: 'fwlite', projectId: 'lex', entryId: 'e-begin' }, + senseRef: { authority: 'fwlite', senseId: 's-begin' }, + allomorphRef: { authority: 'fwlite', allomorphId: 'a-begin' }, + grammarRef: { authority: 'fwlite', msaId: 'msa-verb' }, + gloss: { en: 'start' }, + }, + { id: 'm-1', form: '-ning', writingSystem: 'en' }, + ], + morphemesStale: true, + pos: 'noun', + features: { number: 'singular', case: 'nominative' }, + gloss: { en: 'start' }, + glossSenseRef: { authority: 'fwlite', senseId: 's-start' }, + }, + { id: 'ta-unlinked', ...FIXTURE_STAMPS, surfaceText: 'spare', gloss: { en: 'spare' } }, + ], + tokenAnalysisLinks: [ + tokenLink('tl-gen-and', 'ta-and', 'GEN 1:1:0', 'and'), + tokenLink('tl-exo-and', 'ta-and', 'EXO 1:1:0', 'and'), + { + ...tokenLink('tl-gen-beginning', 'ta-beginning', 'GEN 1:1:9', 'beginning'), + status: 'suggested', + confidence: 'low', + }, + ], + phraseAnalyses: [ + { + id: 'pa-in-the', + ...FIXTURE_STAMPS, + surfaceText: 'in the', + gloss: { en: 'in' }, + senseRef: { authority: 'fwlite', senseId: 's-in' }, + }, + ], + phraseAnalysisLinks: [makePhraseLink('pa-in-the', ['GEN 1:1:4', 'GEN 1:1:2'], ['the', 'in'])], + segmentAnalyses: [ + { + id: 'sa-exo', + ...FIXTURE_STAMPS, + surfaceText: 'And these', + freeTranslation: { en: 'And these are the names.' }, + literalTranslation: { en: 'And these the names.' }, + }, + ], + segmentAnalysisLinks: [ + { + id: 'sl-exo', + analysisId: 'sa-exo', + ...FIXTURE_STAMPS, + status: 'approved', + segmentId: 'EXO 1:1', + }, + ], + }; +} + +/** A draft setting every optional field, around {@link fullAnalysis}. */ +function fullDraft(): DraftProject { + return { + sourceProjectId: SOURCE, + modelVersion: CURRENT_MODEL_VERSION, + analysisLanguages: ['fr', 'en'], + targetProjectId: 'target-project', + suggestedName: 'Genesis draft', + suggestedDescription: 'A draft of Genesis and Exodus.', + segmentation: { + removedVerseStarts: ['GEN 1:2'], + addedStarts: [{ tokenRef: 'GEN 1:1:4', surfaceText: 'the' }], + }, + analysis: fullAnalysis(), + dirty: true, + }; +} + +/** {@link fullDraft} with GEN's records dropped, leaving only what EXO links. */ +function exodusOnlyDraft(): DraftProject { + const { analysis } = fullDraft(); + return { + ...fullDraft(), + analysis: { + ...emptyAnalysis(), + tokenAnalyses: analysis.tokenAnalyses.filter(({ id }) => id === 'ta-and'), + tokenAnalysisLinks: analysis.tokenAnalysisLinks.filter(({ id }) => id === 'tl-exo-and'), + segmentAnalyses: analysis.segmentAnalyses, + segmentAnalysisLinks: analysis.segmentAnalysisLinks, + }, + }; +} + +/** + * The draft with each record list sorted by id. Lists of records promise no order, but the arrays + * inside a record keep theirs. + */ +function inIdOrder(draft: DraftProject): DraftProject { + const byId = (records: readonly T[]): T[] => + [...records].sort((a, b) => a.id.localeCompare(b.id)); + const { analysis } = draft; + return { + ...draft, + analysis: { + tokenAnalyses: byId(analysis.tokenAnalyses), + tokenAnalysisLinks: byId(analysis.tokenAnalysisLinks), + phraseAnalyses: byId(analysis.phraseAnalyses), + phraseAnalysisLinks: byId(analysis.phraseAnalysisLinks), + segmentAnalyses: byId(analysis.segmentAnalyses), + segmentAnalysisLinks: byId(analysis.segmentAnalysisLinks), + }, + }; +} + +/** Loads the draft through a newly opened repository, with its record lists in id order. */ +async function reload(store: ConformanceStore, sourceProjectId = SOURCE): Promise { + const repository = await store.open(sourceProjectId); + return inIdOrder(await repository.exportAll()); +} + +/** + * Declares the tests every {@link AnalysisRepository} implementation must pass, titled with the + * implementation's name. Each test runs against a fresh store from `createStore`, which must hold + * no drafts. + */ +export function describeAnalysisRepositoryConformance( + implementationName: string, + createStore: () => ConformanceStore, +): void { + describe(`${implementationName} conforms to AnalysisRepository`, () => { + let store: ConformanceStore; + + beforeEach(() => { + store = createStore(); + }); + + describe('exportAll', () => { + it('loads an empty draft for its source project when none is saved', async () => { + await expect(reload(store)).resolves.toStrictEqual(emptyDraft(SOURCE)); + }); + + it('returns only JSON-plain values, omitting absent optional fields rather than nulling them', async () => { + const minimal: DraftProject = { + ...emptyDraft(SOURCE), + analysis: { + ...emptyAnalysis(), + tokenAnalyses: [{ id: 'ta-and', ...FIXTURE_STAMPS, surfaceText: 'and' }], + tokenAnalysisLinks: [tokenLink('tl-and', 'ta-and', 'GEN 1:1:0', 'and')], + }, + }; + await (await store.open(SOURCE)).replaceAll(minimal); + + const drafts = [ + await (await store.open(SOURCE)).exportAll(), + await (await store.open('never-saved')).exportAll(), + ]; + drafts.forEach((draft) => { + expect(JSON.parse(JSON.stringify(draft))).toStrictEqual(draft); + expect(JSON.stringify(draft)).not.toMatch(/\bnull\b/); + }); + await expect(reload(store)).resolves.toStrictEqual(minimal); + }); + + it('returns a draft the caller may change without changing what is stored', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + + const loaded = await repository.exportAll(); + loaded.analysisLanguages.push('de'); + loaded.analysis.tokenAnalyses[0].surfaceText = 'changed'; + + await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( + inIdOrder(fullDraft()), + ); + }); + + it('refuses a draft written by a newer build', async () => { + await store.storeNewerBuildDraft(SOURCE); + + await expect((await store.open(SOURCE)).exportAll()).rejects.toThrow(); + }); + }); + + describe('replaceAll', () => { + it('saves every record kind and optional field for the same repository to load', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + + await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( + inIdOrder(fullDraft()), + ); + }); + + it('saves a draft for a newly opened repository to load', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + }); + + it('keeps a payload linked from several books as one record', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + + const { analysis } = await reload(store); + + expect(analysis.tokenAnalyses.filter(({ id }) => id === 'ta-and')).toHaveLength(1); + }); + + it('saves an edit to a payload linked from several books', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + const draft = await repository.exportAll(); + draft.analysis.tokenAnalyses = draft.analysis.tokenAnalyses.map((payload) => + payload.id === 'ta-and' ? { ...payload, gloss: { en: 'and then' } } : payload, + ); + + await repository.replaceAll(draft); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(draft)); + }); + + it('stamps what it saves with the current model version', async () => { + await ( + await store.open(SOURCE) + ).replaceAll({ ...fullDraft(), modelVersion: CURRENT_MODEL_VERSION - 1 }); + + const { modelVersion } = await reload(store); + + expect(modelVersion).toBe(CURRENT_MODEL_VERSION); + }); + + it('replaces the stored draft whole, dropping records the saved draft lacks', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + await repository.exportAll(); + + await repository.replaceAll(exodusOnlyDraft()); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); + }); + + it('drops records the saved draft lacks even when this repository never loaded them', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + + await (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); + }); + + it('saves an emptied analysis as empty', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + + await repository.replaceAll({ ...fullDraft(), analysis: emptyAnalysis() }); + + await expect(reload(store)).resolves.toStrictEqual({ + ...fullDraft(), + analysis: emptyAnalysis(), + }); + }); + + it('restores records a later save brings back', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + await repository.replaceAll(exodusOnlyDraft()); + + await repository.replaceAll(fullDraft()); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + }); + + it('stores what it was given, unaffected by later changes to that draft', async () => { + const draft = fullDraft(); + await (await store.open(SOURCE)).replaceAll(draft); + + draft.analysisLanguages.push('de'); + draft.analysis.tokenAnalyses[0].surfaceText = 'changed'; + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + }); + + it('keeps the drafts of different source projects apart', async () => { + const other = { ...exodusOnlyDraft(), sourceProjectId: 'other-project' }; + await (await store.open(SOURCE)).replaceAll(fullDraft()); + + await (await store.open('other-project')).replaceAll(other); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + await expect(reload(store, 'other-project')).resolves.toStrictEqual(inIdOrder(other)); + }); + + it('leaves a draft written by a newer build in place, refusing to overwrite it', async () => { + await store.storeNewerBuildDraft(SOURCE); + + await expect((await store.open(SOURCE)).replaceAll(fullDraft())).rejects.toThrow(); + + await expect((await store.open(SOURCE)).exportAll()).rejects.toThrow(); + }); + }); + }); +} diff --git a/src/__tests__/services/draftRepository.conformance.test.ts b/src/__tests__/services/draftRepository.conformance.test.ts new file mode 100644 index 000000000..548cd7506 --- /dev/null +++ b/src/__tests__/services/draftRepository.conformance.test.ts @@ -0,0 +1,28 @@ +/// + +import { DraftRepository } from '../../services/draftRepository'; +import { draftKey } from '../../services/storageKeys'; +import { emptyDraft } from '../../types/empty-factories'; +import { CURRENT_MODEL_VERSION } from '../../types/model-version'; +import { describeAnalysisRepositoryConformance } from '../analysis-repository-conformance'; +import { memoryStorage, silentLogger } from '../test-helpers'; + +const logger = silentLogger(); + +describeAnalysisRepositoryConformance('DraftRepository', () => { + const { storage } = memoryStorage(); + return { + open: async (sourceProjectId) => new DraftRepository(storage, sourceProjectId, logger), + storeNewerBuildDraft: (sourceProjectId) => + storage.write( + draftKey(sourceProjectId), + JSON.stringify({ ...emptyDraft(sourceProjectId), modelVersion: CURRENT_MODEL_VERSION + 1 }), + ), + }; +}); + +// A conforming save and load of well-formed drafts has nothing to warn about, so a warning means +// the repository found fault with records it wrote itself. +afterEach(() => { + expect(logger.warn).not.toHaveBeenCalled(); +}); diff --git a/src/__tests__/services/draftRepository.test.ts b/src/__tests__/services/draftRepository.test.ts index 9915fd9d6..5f2be32a7 100644 --- a/src/__tests__/services/draftRepository.test.ts +++ b/src/__tests__/services/draftRepository.test.ts @@ -3,31 +3,9 @@ import type { DraftProject, TextAnalysis } from 'interlinearizer'; import { DraftRepository } from '../../services/draftRepository'; import { draftAnalysisKey, draftKey } from '../../services/storageKeys'; -import type { KeyValueStorage, WarningLogger } from '../../types/analysis-repository'; import { emptyAnalysis, emptyDraft } from '../../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../../types/model-version'; -import { FIXTURE_STAMPS, makePhraseLink } from '../test-helpers'; - -/** Storage held in a map, recording every write in order. */ -function memoryStorage(initial: Record = {}) { - const records = new Map(Object.entries(initial)); - const writes: string[] = []; - const storage: KeyValueStorage = { - read: async (key) => records.get(key), - write: async (key, data) => { - writes.push(key); - records.set(key, data); - }, - delete: async (key) => { - records.delete(key); - }, - }; - return { storage, records, writes }; -} - -function silentLogger(): WarningLogger & { warn: jest.Mock } { - return { warn: jest.fn() }; -} +import { FIXTURE_STAMPS, makePhraseLink, memoryStorage, silentLogger } from '../test-helpers'; /** An analysis with a token payload shared by GEN and EXO, a phrase in GEN and a segment in EXO. */ function twoBookAnalysis(): TextAnalysis { diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index 6a003cd86..d398ebaa9 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -11,6 +11,7 @@ import { } from '../components/PhraseStripContext'; import type { SegmentPages } from '../hooks/useSegmentPages'; import { summarizeProject } from '../services/projectSummaries'; +import type { KeyValueStorage, WarningLogger } from '../types/analysis-repository'; import { emptyAnalysis } from '../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../types/model-version'; import type { InterlinearProjectSummary } from '../types/interlinear-project-summary'; @@ -546,3 +547,30 @@ export function getMockedSendCommand(papiModule: unknown): jest.Mock { export function getMockedWaitForNetworkObject(papiModule: unknown): jest.Mock { return getMockedPapiFn(papiModule, ['networkObjectStatus', 'waitForNetworkObject']); } + +/** + * Storage held in a map, recording every key written. + * + * @returns The storage, the map backing it (seeded from `initial`), and the keys written so far, in + * write order, with repeats. + */ +export function memoryStorage(initial: Record = {}) { + const records = new Map(Object.entries(initial)); + const writes: string[] = []; + const storage: KeyValueStorage = { + read: async (key) => records.get(key), + write: async (key, data) => { + writes.push(key); + records.set(key, data); + }, + delete: async (key) => { + records.delete(key); + }, + }; + return { storage, records, writes }; +} + +/** A logger whose warnings a test can inspect instead of printing. */ +export function silentLogger(): WarningLogger & { warn: jest.Mock } { + return { warn: jest.fn() }; +} diff --git a/src/__tests__/utils/analysis-violations.test.ts b/src/__tests__/utils/analysis-violations.test.ts index ca63095c9..b960fa11c 100644 --- a/src/__tests__/utils/analysis-violations.test.ts +++ b/src/__tests__/utils/analysis-violations.test.ts @@ -1,14 +1,9 @@ /// import type { TextAnalysis, TokenAnalysisLink } from 'interlinearizer'; -import type { WarningLogger } from '../../types/analysis-repository'; import { emptyAnalysis } from '../../types/empty-factories'; import { reportAnalysisViolations } from '../../utils/analysis-violations'; -import { FIXTURE_STAMPS } from '../test-helpers'; - -function silentLogger(): WarningLogger & { warn: jest.Mock } { - return { warn: jest.fn() }; -} +import { FIXTURE_STAMPS, silentLogger } from '../test-helpers'; function linkTo(analysisId: string, tokenRef: string, id = `tl-${tokenRef}`): TokenAnalysisLink { return { diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index c345bb68c..2773878e3 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -53,11 +53,12 @@ export type DraftExport = { /** * Reads and writes one source project's draft. Every call is async and takes and returns only * JSON-plain values, so an implementation may sit behind a network boundary or another language. + * Lists of records come back in no promised order. */ export interface AnalysisRepository { /** - * Loads the whole stored draft. Stored content that cannot be read loads as empty, so one damaged - * record never hides the rest. + * Loads the whole stored draft, or an empty one for the source project when none is stored. + * Stored content that cannot be read loads as empty, so one damaged record never hides the rest. * * @throws {SyntaxError} If the draft's stored value contains invalid JSON. * @throws {Error} If the stored draft was written by a newer build. @@ -76,10 +77,11 @@ export interface AnalysisRepository { exportForCopy(): Promise; /** - * Saves the whole draft, replacing the stored one. Content that lands counts as saved even when - * cleanup after it fails. A book whose stored analysis could not be read is kept as stored unless - * `options` discards it: a draft that loaded it empty never wipes it, and analysis the draft - * carries for it never stands in for what could not be read. + * Saves the whole draft, replacing the stored one, stamped with this build's model version. + * Content that lands counts as saved even when cleanup after it fails. A book whose stored + * analysis could not be read is kept as stored unless `options` discards it: a draft that loaded + * it empty never wipes it, and analysis the draft carries for it never stands in for what could + * not be read. * * @returns The books whose analysis in the draft was left out for that reason; empty when the * draft was saved whole. From 87c1b04343cca643cbaa51f86cfc686bf076b02b Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 19 Oct 2026 14:55:52 -0400 Subject: [PATCH 02/15] Check that a whole replacement drops stale draft metadata The draft that replaces the full fixture kept every metadata field, so a backend that merged the saved draft over the stored one passed. It now drops every optional field and changes the required ones. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/analysis-repository-conformance.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index af641b1d6..699a06cc9 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -125,11 +125,17 @@ function fullDraft(): DraftProject { }; } -/** {@link fullDraft} with GEN's records dropped, leaving only what EXO links. */ +/** + * {@link fullDraft} with GEN's records and every optional field dropped and the other metadata + * changed, so a save that keeps any of the stored draft shows. + */ function exodusOnlyDraft(): DraftProject { const { analysis } = fullDraft(); return { - ...fullDraft(), + sourceProjectId: SOURCE, + modelVersion: CURRENT_MODEL_VERSION, + analysisLanguages: ['en'], + dirty: false, analysis: { ...emptyAnalysis(), tokenAnalyses: analysis.tokenAnalyses.filter(({ id }) => id === 'ta-and'), From b97cfef4abb0dbd079e8703f55b3c0cc965312ed Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 19 Oct 2026 15:03:43 -0400 Subject: [PATCH 03/15] Order conformance fixture fields as their types declare them Co-Authored-By: Claude Opus 5.5 --- .../analysis-repository-conformance.ts | 52 +++++++++---------- src/__tests__/test-helpers.ts | 2 +- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index 699a06cc9..8c0f78745 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -37,6 +37,24 @@ function tokenLink(id: string, analysisId: string, tokenRef: string, surfaceText */ function fullAnalysis(): TextAnalysis { return { + segmentAnalyses: [ + { + id: 'sa-exo', + ...FIXTURE_STAMPS, + surfaceText: 'And these', + freeTranslation: { en: 'And these are the names.' }, + literalTranslation: { en: 'And these the names.' }, + }, + ], + segmentAnalysisLinks: [ + { + id: 'sl-exo', + analysisId: 'sa-exo', + ...FIXTURE_STAMPS, + status: 'approved', + segmentId: 'EXO 1:1', + }, + ], tokenAnalyses: [ { id: 'ta-and', ...FIXTURE_STAMPS, surfaceText: 'and', gloss: { en: 'and', fr: 'et' } }, { @@ -61,7 +79,7 @@ function fullAnalysis(): TextAnalysis { ], morphemesStale: true, pos: 'noun', - features: { number: 'singular', case: 'nominative' }, + features: { case: 'nominative', number: 'singular' }, gloss: { en: 'start' }, glossSenseRef: { authority: 'fwlite', senseId: 's-start' }, }, @@ -86,24 +104,6 @@ function fullAnalysis(): TextAnalysis { }, ], phraseAnalysisLinks: [makePhraseLink('pa-in-the', ['GEN 1:1:4', 'GEN 1:1:2'], ['the', 'in'])], - segmentAnalyses: [ - { - id: 'sa-exo', - ...FIXTURE_STAMPS, - surfaceText: 'And these', - freeTranslation: { en: 'And these are the names.' }, - literalTranslation: { en: 'And these the names.' }, - }, - ], - segmentAnalysisLinks: [ - { - id: 'sl-exo', - analysisId: 'sa-exo', - ...FIXTURE_STAMPS, - status: 'approved', - segmentId: 'EXO 1:1', - }, - ], }; } @@ -116,12 +116,12 @@ function fullDraft(): DraftProject { targetProjectId: 'target-project', suggestedName: 'Genesis draft', suggestedDescription: 'A draft of Genesis and Exodus.', + analysis: fullAnalysis(), + dirty: true, segmentation: { removedVerseStarts: ['GEN 1:2'], addedStarts: [{ tokenRef: 'GEN 1:1:4', surfaceText: 'the' }], }, - analysis: fullAnalysis(), - dirty: true, }; } @@ -135,14 +135,14 @@ function exodusOnlyDraft(): DraftProject { sourceProjectId: SOURCE, modelVersion: CURRENT_MODEL_VERSION, analysisLanguages: ['en'], - dirty: false, analysis: { ...emptyAnalysis(), - tokenAnalyses: analysis.tokenAnalyses.filter(({ id }) => id === 'ta-and'), - tokenAnalysisLinks: analysis.tokenAnalysisLinks.filter(({ id }) => id === 'tl-exo-and'), segmentAnalyses: analysis.segmentAnalyses, segmentAnalysisLinks: analysis.segmentAnalysisLinks, + tokenAnalyses: analysis.tokenAnalyses.filter(({ id }) => id === 'ta-and'), + tokenAnalysisLinks: analysis.tokenAnalysisLinks.filter(({ id }) => id === 'tl-exo-and'), }, + dirty: false, }; } @@ -157,12 +157,12 @@ function inIdOrder(draft: DraftProject): DraftProject { return { ...draft, analysis: { + segmentAnalyses: byId(analysis.segmentAnalyses), + segmentAnalysisLinks: byId(analysis.segmentAnalysisLinks), tokenAnalyses: byId(analysis.tokenAnalyses), tokenAnalysisLinks: byId(analysis.tokenAnalysisLinks), phraseAnalyses: byId(analysis.phraseAnalyses), phraseAnalysisLinks: byId(analysis.phraseAnalysisLinks), - segmentAnalyses: byId(analysis.segmentAnalyses), - segmentAnalysisLinks: byId(analysis.segmentAnalysisLinks), }, }; } diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index d398ebaa9..de9cc0331 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -13,8 +13,8 @@ import type { SegmentPages } from '../hooks/useSegmentPages'; import { summarizeProject } from '../services/projectSummaries'; import type { KeyValueStorage, WarningLogger } from '../types/analysis-repository'; import { emptyAnalysis } from '../types/empty-factories'; -import { CURRENT_MODEL_VERSION } from '../types/model-version'; import type { InterlinearProjectSummary } from '../types/interlinear-project-summary'; +import { CURRENT_MODEL_VERSION } from '../types/model-version'; import type { OutlineSegment } from '../types/segment-pages'; /** Minimal execution token-shaped object for tests (structural match for ExecutionToken). */ From c20a654e42cdb81431aa39de68ec585f8527418a Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Mon, 19 Oct 2026 17:42:15 -0400 Subject: [PATCH 04/15] Tighten conformance suite checks and docs per review - Keep any extra key on a loaded analysis when sorting record lists, so strict comparisons catch it. - Split the JSON-plain check into a saved-draft test and a never-saved one. - Describe exodusOnlyDraft as what it is: EXO's records, without the draft's optional fields. - Drop the two DraftRepository tests the suite already covers. Co-Authored-By: Claude Opus 5.5 --- .../analysis-repository-conformance.ts | 26 +++++++++++-------- .../services/draftRepository.test.ts | 19 -------------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index 8c0f78745..88e6405fd 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -126,8 +126,8 @@ function fullDraft(): DraftProject { } /** - * {@link fullDraft} with GEN's records and every optional field dropped and the other metadata - * changed, so a save that keeps any of the stored draft shows. + * {@link fullDraft} cut to EXO's records, with the draft's optional fields dropped and its other + * metadata changed, so a save that keeps any of the stored draft shows. */ function exodusOnlyDraft(): DraftProject { const { analysis } = fullDraft(); @@ -157,6 +157,7 @@ function inIdOrder(draft: DraftProject): DraftProject { return { ...draft, analysis: { + ...analysis, segmentAnalyses: byId(analysis.segmentAnalyses), segmentAnalysisLinks: byId(analysis.segmentAnalysisLinks), tokenAnalyses: byId(analysis.tokenAnalyses), @@ -167,6 +168,12 @@ function inIdOrder(draft: DraftProject): DraftProject { }; } +/** Expects `draft` to survive a JSON round trip unchanged and to hold no `null`. */ +function expectJsonPlain(draft: DraftProject): void { + expect(JSON.parse(JSON.stringify(draft))).toStrictEqual(draft); + expect(JSON.stringify(draft)).not.toMatch(/\bnull\b/); +} + /** Loads the draft through a newly opened repository, with its record lists in id order. */ async function reload(store: ConformanceStore, sourceProjectId = SOURCE): Promise { const repository = await store.open(sourceProjectId); @@ -194,7 +201,7 @@ export function describeAnalysisRepositoryConformance( await expect(reload(store)).resolves.toStrictEqual(emptyDraft(SOURCE)); }); - it('returns only JSON-plain values, omitting absent optional fields rather than nulling them', async () => { + it('returns a saved draft as JSON-plain values, omitting absent optional fields rather than nulling them', async () => { const minimal: DraftProject = { ...emptyDraft(SOURCE), analysis: { @@ -205,17 +212,14 @@ export function describeAnalysisRepositoryConformance( }; await (await store.open(SOURCE)).replaceAll(minimal); - const drafts = [ - await (await store.open(SOURCE)).exportAll(), - await (await store.open('never-saved')).exportAll(), - ]; - drafts.forEach((draft) => { - expect(JSON.parse(JSON.stringify(draft))).toStrictEqual(draft); - expect(JSON.stringify(draft)).not.toMatch(/\bnull\b/); - }); + expectJsonPlain(await (await store.open(SOURCE)).exportAll()); await expect(reload(store)).resolves.toStrictEqual(minimal); }); + it('returns a never-saved draft as JSON-plain values', async () => { + expectJsonPlain(await (await store.open(SOURCE)).exportAll()); + }); + it('returns a draft the caller may change without changing what is stored', async () => { const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); diff --git a/src/__tests__/services/draftRepository.test.ts b/src/__tests__/services/draftRepository.test.ts index 5f2be32a7..95885f807 100644 --- a/src/__tests__/services/draftRepository.test.ts +++ b/src/__tests__/services/draftRepository.test.ts @@ -41,25 +41,6 @@ function twoBookDraft(): DraftProject { } describe('DraftRepository', () => { - it('loads an empty draft for its source project when none is stored', async () => { - const { storage } = memoryStorage(); - - await expect(new DraftRepository(storage, 'src', silentLogger()).exportAll()).resolves.toEqual({ - draft: emptyDraft('src'), - unreadableBooks: [], - }); - }); - - it('loads in a new instance what another saved', async () => { - const { storage } = memoryStorage(); - await new DraftRepository(storage, 'src', silentLogger()).replaceAll(twoBookDraft()); - - await expect(new DraftRepository(storage, 'src', silentLogger()).exportAll()).resolves.toEqual({ - draft: twoBookDraft(), - unreadableBooks: [], - }); - }); - it('loads a payload shared by several books once', async () => { const { storage, records } = memoryStorage(); await new DraftRepository(storage, 'src', silentLogger()).replaceAll(twoBookDraft()); From 38c48afecccdcda9ab4315a896d608e84ad55bda Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Mon, 19 Oct 2026 16:36:40 -0600 Subject: [PATCH 05/15] Cover inherited fields on every record kind and document caller rules --- .../analysis-repository-conformance.ts | 17 ++++++++++++++--- src/types/analysis-repository.ts | 5 ++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index 88e6405fd..a671054d5 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -42,6 +42,9 @@ function fullAnalysis(): TextAnalysis { id: 'sa-exo', ...FIXTURE_STAMPS, surfaceText: 'And these', + confidence: 'high', + producer: 'human', + sourceUser: 'consultant', freeTranslation: { en: 'And these are the names.' }, literalTranslation: { en: 'And these the names.' }, }, @@ -52,6 +55,7 @@ function fullAnalysis(): TextAnalysis { analysisId: 'sa-exo', ...FIXTURE_STAMPS, status: 'approved', + confidence: 'high', segmentId: 'EXO 1:1', }, ], @@ -99,11 +103,19 @@ function fullAnalysis(): TextAnalysis { id: 'pa-in-the', ...FIXTURE_STAMPS, surfaceText: 'in the', + confidence: 'guess', + producer: 'eflomal', + sourceUser: 'reviewer', gloss: { en: 'in' }, senseRef: { authority: 'fwlite', senseId: 's-in' }, }, ], - phraseAnalysisLinks: [makePhraseLink('pa-in-the', ['GEN 1:1:4', 'GEN 1:1:2'], ['the', 'in'])], + phraseAnalysisLinks: [ + { + ...makePhraseLink('pa-in-the', ['GEN 1:1:4', 'GEN 1:1:2'], ['the', 'in']), + confidence: 'guess', + }, + ], }; } @@ -182,8 +194,7 @@ async function reload(store: ConformanceStore, sourceProjectId = SOURCE): Promis /** * Declares the tests every {@link AnalysisRepository} implementation must pass, titled with the - * implementation's name. Each test runs against a fresh store from `createStore`, which must hold - * no drafts. + * implementation's name. Each test runs against a fresh store from `createStore`. */ export function describeAnalysisRepositoryConformance( implementationName: string, diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index 2773878e3..30b7d649c 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -53,7 +53,10 @@ export type DraftExport = { /** * Reads and writes one source project's draft. Every call is async and takes and returns only * JSON-plain values, so an implementation may sit behind a network boundary or another language. - * Lists of records come back in no promised order. + * The caller may change a draft it passed in or got back without changing what is stored. Lists of + * records come back in no promised order. + * + * Once another repository saves the same draft, this one must load it before saving again. */ export interface AnalysisRepository { /** From f96bdc996afb89f7ce580cae999b38536e2cb809 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 20 Oct 2026 16:26:09 -0400 Subject: [PATCH 06/15] Require a reload after another save for loaded, unsaved repositories A repository that loaded a draft but never saved it still caches what each shard held, so saving after another repository's save could skip a book that save deleted. Co-Authored-By: Claude Opus 5.5 --- src/types/analysis-repository.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index 30b7d649c..ee967568e 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -56,7 +56,8 @@ export type DraftExport = { * The caller may change a draft it passed in or got back without changing what is stored. Lists of * records come back in no promised order. * - * Once another repository saves the same draft, this one must load it before saving again. + * Once another repository saves the same draft, a repository that loaded or saved it before then + * must load it again before its next save. */ export interface AnalysisRepository { /** From 4040f227e22891f589d0bbaac6654f2528c9f56b Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Tue, 20 Oct 2026 17:35:01 -0400 Subject: [PATCH 07/15] Bar overlapping calls and limit caller changes to settled calls DraftRepository reads a saved draft's contents after its first await, and overlapping saves from two repositories can store a mix of both, so the interface now promises caller changes only once a call settles and forbids overlapping calls for one draft. Co-Authored-By: Claude Opus 5.5 --- src/types/analysis-repository.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index ee967568e..5287dc320 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -53,11 +53,12 @@ export type DraftExport = { /** * Reads and writes one source project's draft. Every call is async and takes and returns only * JSON-plain values, so an implementation may sit behind a network boundary or another language. - * The caller may change a draft it passed in or got back without changing what is stored. Lists of - * records come back in no promised order. + * Once a call settles, the caller may change a draft it passed in or got back without changing what + * is stored. Lists of records come back in no promised order. * - * Once another repository saves the same draft, a repository that loaded or saved it before then - * must load it again before its next save. + * Calls for one draft must not overlap, whether on one repository or several. After one repository + * saves a draft, any other that had already loaded or saved it must load it again before its next + * save. */ export interface AnalysisRepository { /** From f4d3eb44ea6c843b63cafbde4ccfdaa627b1b217 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 21 Oct 2026 08:44:51 -0400 Subject: [PATCH 08/15] Check saved-draft ownership through the same repository too A repository that kept the caller's saved draft by reference to serve the next load would pass a check through a newly opened repository alone. - AGENTS.md: the suite takes a factory for a fresh store per test. - memoryStorage: state each fact once in its doc comment. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- src/__tests__/analysis-repository-conformance.ts | 6 +++++- src/__tests__/test-helpers.ts | 6 +++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index b63433051..fd1197177 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,7 +79,7 @@ The WebView root component is assigned to `globalThis.webViewComponent` (not exp Drafts are loaded and saved through [AnalysisRepository](src/types/analysis-repository.ts), the seam a later storage backend implements. [DraftRepository](src/services/draftRepository.ts) implements it over the draft envelope and its per-book shards, and rewrites only the books that changed. It imports nothing from PAPI and reaches storage only through a `KeyValueStorage`. That lets the perf harness run it in Node, straight over the storage files ([perf/storage.ts](perf/storage.ts)). -[The conformance suite](src/__tests__/analysis-repository-conformance.ts) defines the seam: every `AnalysisRepository` implementation must pass it, and [draftRepository.conformance.test.ts](src/__tests__/services/draftRepository.conformance.test.ts) runs it against `DraftRepository`. A behavior every implementation owes goes in the suite; one particular to an implementation, such as which shards a save rewrites, goes in that implementation's own tests. A later backend runs the suite by passing `describeAnalysisRepositoryConformance` a store it can open repositories over. +[The conformance suite](src/__tests__/analysis-repository-conformance.ts) defines the seam: every `AnalysisRepository` implementation must pass it, and [draftRepository.conformance.test.ts](src/__tests__/services/draftRepository.conformance.test.ts) runs it against `DraftRepository`. A behavior every implementation owes goes in the suite; one particular to an implementation, such as which shards a save rewrites, goes in that implementation's own tests. A later backend runs the suite by passing `describeAnalysisRepositoryConformance` a factory that returns a fresh `ConformanceStore` for each test. Saved projects are listed through [ProjectSummaries](src/services/projectSummaries.ts): one record holding every project's summary, rewritten on each project write so listing reads no analysis. Like `DraftRepository`, it is PAPI-free. diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index a671054d5..afd1f2da4 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -339,12 +339,16 @@ export function describeAnalysisRepositoryConformance( }); it('stores what it was given, unaffected by later changes to that draft', async () => { + const repository = await store.open(SOURCE); const draft = fullDraft(); - await (await store.open(SOURCE)).replaceAll(draft); + await repository.replaceAll(draft); draft.analysisLanguages.push('de'); draft.analysis.tokenAnalyses[0].surfaceText = 'changed'; + await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( + inIdOrder(fullDraft()), + ); await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); }); diff --git a/src/__tests__/test-helpers.ts b/src/__tests__/test-helpers.ts index de9cc0331..745cb1093 100644 --- a/src/__tests__/test-helpers.ts +++ b/src/__tests__/test-helpers.ts @@ -549,10 +549,10 @@ export function getMockedWaitForNetworkObject(papiModule: unknown): jest.Mock { } /** - * Storage held in a map, recording every key written. + * Storage held in a map seeded from `initial`. * - * @returns The storage, the map backing it (seeded from `initial`), and the keys written so far, in - * write order, with repeats. + * @returns The storage, the map backing it, and the keys written so far, in write order, with + * repeats. */ export function memoryStorage(initial: Record = {}) { const records = new Map(Object.entries(initial)); From ba83ef6f58042766411f023155b09deac5951075 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 21 Oct 2026 09:01:24 -0400 Subject: [PATCH 09/15] Check a save after reloading a draft another repository replaced Co-Authored-By: Claude Opus 5.5 --- src/__tests__/analysis-repository-conformance.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index afd1f2da4..b57032638 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -316,6 +316,17 @@ export function describeAnalysisRepositoryConformance( await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); }); + it('saves in full after reloading a draft another repository replaced', async () => { + const repository = await store.open(SOURCE); + await repository.replaceAll(fullDraft()); + await (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()); + await repository.exportAll(); + + await repository.replaceAll(fullDraft()); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + }); + it('saves an emptied analysis as empty', async () => { const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); From f7360a27e3f5eab9bcfc2c664e2d8aa4d8430e91 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 21 Oct 2026 20:19:35 -0400 Subject: [PATCH 10/15] Check what loads and saves see after another repository's save - After another repository replaces the draft, expect this one's load to return that draft, so serving loads from a cached copy of its own last save fails. - After changing a saved draft in place, save it again and expect the change stored, so keeping the passed draft as the baseline for the next save's diff fails. Co-Authored-By: Claude Opus 5.5 --- src/__tests__/analysis-repository-conformance.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index b57032638..6c4d66eae 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -320,7 +320,9 @@ export function describeAnalysisRepositoryConformance( const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); await (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()); - await repository.exportAll(); + await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( + inIdOrder(exodusOnlyDraft()), + ); await repository.replaceAll(fullDraft()); @@ -361,6 +363,10 @@ export function describeAnalysisRepositoryConformance( inIdOrder(fullDraft()), ); await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + + await repository.replaceAll(draft); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(draft)); }); it('keeps the drafts of different source projects apart', async () => { From bfed15cc5f2a38b1e36923e4e8dfb88928a1d876 Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Wed, 21 Oct 2026 20:29:06 -0400 Subject: [PATCH 11/15] Cover #475's repository calls in the conformance suite The interface now names the books a load could not read, keeps such a book as stored on save unless told to discard it, settles a save once its content lands, and adds exportForCopy and markClean. The suite checks each of those at the seam, so a later backend owes them too. - ConformanceStore gains hooks to damage one book or the whole draft, fail a save's cleanup after its content lands, and report leftovers. - The DraftRepository runner tolerates warnings only in tests that damage storage or fail cleanup. - DraftRepository tests the suite now covers are dropped; those that check which keys are written or what bytes stay remain. - exportForCopy's doc names the empty draft it returns when none is stored. Co-Authored-By: Claude Opus 5.5 --- .../analysis-repository-conformance.ts | 286 ++++++++++++++++-- .../draftRepository.conformance.test.ts | 62 +++- .../services/draftRepository.test.ts | 92 ------ src/types/analysis-repository.ts | 3 +- 4 files changed, 322 insertions(+), 121 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index 6c4d66eae..f2f8ae24f 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -1,7 +1,7 @@ /// import type { DraftProject, TextAnalysis } from 'interlinearizer'; -import type { AnalysisRepository } from '../types/analysis-repository'; +import type { AnalysisRepository, DraftExport } from '../types/analysis-repository'; import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../types/model-version'; import { FIXTURE_STAMPS, makePhraseLink } from './test-helpers'; @@ -16,6 +16,27 @@ export interface ConformanceStore { /** Stores a draft for `sourceProjectId` as a build with a newer model version would write it. */ storeNewerBuildDraft(sourceProjectId: string): Promise; + + /** + * Leaves the stored analysis of one book of the draft for `sourceProjectId` unreadable, the rest + * of the draft as it stands: `corrupt` keeps something unusable in its place, `lost` nothing. + */ + damageBook(sourceProjectId: string, bookCode: string, damage: 'corrupt' | 'lost'): Promise; + + /** + * Leaves the stored draft for `sourceProjectId` well-formed but unusable, so no book in it can be + * vouched for. + */ + damageDraft(sourceProjectId: string): Promise; + + /** + * Makes the cleanup each save of the draft for `sourceProjectId` does after its content lands + * fail, until the returned function is called. + */ + failCleanup(sourceProjectId: string): () => void; + + /** Whether storage holds anything for `sourceProjectId` that its stored draft no longer needs. */ + holdsLeftovers(sourceProjectId: string): Promise; } const SOURCE = 'source-project'; @@ -158,6 +179,22 @@ function exodusOnlyDraft(): DraftProject { }; } +/** {@link fullDraft} without GEN's records, as it loads once GEN's stored analysis is unreadable. */ +function fullDraftWithoutGenesis(): DraftProject { + const draft = fullDraft(); + const { analysis } = draft; + return { + ...draft, + analysis: { + ...analysis, + tokenAnalyses: analysis.tokenAnalyses.filter(({ id }) => id !== 'ta-beginning'), + tokenAnalysisLinks: analysis.tokenAnalysisLinks.filter(({ id }) => id === 'tl-exo-and'), + phraseAnalyses: [], + phraseAnalysisLinks: [], + }, + }; +} + /** * The draft with each record list sorted by id. Lists of records promise no order, but the arrays * inside a record keep theirs. @@ -180,16 +217,47 @@ function inIdOrder(draft: DraftProject): DraftProject { }; } -/** Expects `draft` to survive a JSON round trip unchanged and to hold no `null`. */ -function expectJsonPlain(draft: DraftProject): void { - expect(JSON.parse(JSON.stringify(draft))).toStrictEqual(draft); - expect(JSON.stringify(draft)).not.toMatch(/\bnull\b/); +/** Expects `value` to survive a JSON round trip unchanged and to hold no `null`. */ +function expectJsonPlain(value: unknown): void { + expect(JSON.parse(JSON.stringify(value))).toStrictEqual(value); + expect(JSON.stringify(value)).not.toMatch(/\bnull\b/); +} + +/** + * Expects `exported` to name no unreadable book, and returns its draft with record lists in id + * order. + */ +function readableDraft(exported: DraftExport): DraftProject { + expect(exported.unreadableBooks).toStrictEqual([]); + return inIdOrder(exported.draft); +} + +/** Loads the draft through `repository`, as {@link readableDraft} returns it. */ +async function load(repository: AnalysisRepository): Promise { + return readableDraft(await repository.exportAll()); } -/** Loads the draft through a newly opened repository, with its record lists in id order. */ +/** Loads the draft through a newly opened repository, as {@link readableDraft} returns it. */ async function reload(store: ConformanceStore, sourceProjectId = SOURCE): Promise { - const repository = await store.open(sourceProjectId); - return inIdOrder(await repository.exportAll()); + return load(await store.open(sourceProjectId)); +} + +/** Loads the draft for copying through a newly opened repository, with record lists in id order. */ +async function reloadForCopy(store: ConformanceStore): Promise { + const { draft, unreadableBooks } = await (await store.open(SOURCE)).exportForCopy(); + return { draft: inIdOrder(draft), unreadableBooks }; +} + +/** + * Saves {@link fullDraft}, leaves GEN's stored analysis unreadable, and returns a repository that + * has loaded the draft since. + */ +async function loadedWithUnreadableGenesis(store: ConformanceStore): Promise { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageBook(SOURCE, 'GEN', 'corrupt'); + const repository = await store.open(SOURCE); + await repository.exportAll(); + return repository; } /** @@ -235,13 +303,11 @@ export function describeAnalysisRepositoryConformance( const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); - const loaded = await repository.exportAll(); + const { draft: loaded } = await repository.exportAll(); loaded.analysisLanguages.push('de'); loaded.analysis.tokenAnalyses[0].surfaceText = 'changed'; - await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( - inIdOrder(fullDraft()), - ); + await expect(load(repository)).resolves.toStrictEqual(inIdOrder(fullDraft())); }); it('refuses a draft written by a newer build', async () => { @@ -249,6 +315,70 @@ export function describeAnalysisRepositoryConformance( await expect((await store.open(SOURCE)).exportAll()).rejects.toThrow(); }); + + it.each(['corrupt', 'lost'] as const)( + 'names a book whose stored analysis is %s, and loads the rest', + async (damage) => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageBook(SOURCE, 'GEN', damage); + + const { draft, unreadableBooks } = await (await store.open(SOURCE)).exportAll(); + + expect(unreadableBooks).toStrictEqual(['GEN']); + expect(inIdOrder(draft)).toStrictEqual(inIdOrder(fullDraftWithoutGenesis())); + }, + ); + + it('loads an unusable draft as an empty one', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageDraft(SOURCE); + + await expect(reload(store)).resolves.toStrictEqual(emptyDraft(SOURCE)); + }); + }); + + describe('exportForCopy', () => { + it('exports a saved draft whole, naming no book', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + + await expect(reloadForCopy(store)).resolves.toStrictEqual({ + draft: inIdOrder(fullDraft()), + unreadableBooks: [], + }); + }); + + it('exports an empty draft for its source project when none is saved', async () => { + await expect(reloadForCopy(store)).resolves.toStrictEqual({ + draft: emptyDraft(SOURCE), + unreadableBooks: [], + }); + }); + + it.each(['corrupt', 'lost'] as const)( + 'names a book whose stored analysis is %s, and exports the rest', + async (damage) => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageBook(SOURCE, 'GEN', damage); + + await expect(reloadForCopy(store)).resolves.toStrictEqual({ + draft: inIdOrder(fullDraftWithoutGenesis()), + unreadableBooks: ['GEN'], + }); + }, + ); + + it('refuses an unusable draft rather than exporting it empty', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageDraft(SOURCE); + + await expect((await store.open(SOURCE)).exportForCopy()).rejects.toThrow(); + }); + + it('refuses a draft written by a newer build', async () => { + await store.storeNewerBuildDraft(SOURCE); + + await expect((await store.open(SOURCE)).exportForCopy()).rejects.toThrow(); + }); }); describe('replaceAll', () => { @@ -256,9 +386,7 @@ export function describeAnalysisRepositoryConformance( const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); - await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( - inIdOrder(fullDraft()), - ); + await expect(load(repository)).resolves.toStrictEqual(inIdOrder(fullDraft())); }); it('saves a draft for a newly opened repository to load', async () => { @@ -278,7 +406,7 @@ export function describeAnalysisRepositoryConformance( it('saves an edit to a payload linked from several books', async () => { const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); - const draft = await repository.exportAll(); + const { draft } = await repository.exportAll(); draft.analysis.tokenAnalyses = draft.analysis.tokenAnalyses.map((payload) => payload.id === 'ta-and' ? { ...payload, gloss: { en: 'and then' } } : payload, ); @@ -320,9 +448,7 @@ export function describeAnalysisRepositoryConformance( const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); await (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()); - await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( - inIdOrder(exodusOnlyDraft()), - ); + await expect(load(repository)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); await repository.replaceAll(fullDraft()); @@ -359,9 +485,7 @@ export function describeAnalysisRepositoryConformance( draft.analysisLanguages.push('de'); draft.analysis.tokenAnalyses[0].surfaceText = 'changed'; - await expect(repository.exportAll().then(inIdOrder)).resolves.toStrictEqual( - inIdOrder(fullDraft()), - ); + await expect(load(repository)).resolves.toStrictEqual(inIdOrder(fullDraft())); await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); await repository.replaceAll(draft); @@ -386,6 +510,124 @@ export function describeAnalysisRepositoryConformance( await expect((await store.open(SOURCE)).exportAll()).rejects.toThrow(); }); + + it('resolves once the draft has landed even if cleanup after it fails, and a later save finishes that cleanup', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + const restore = store.failCleanup(SOURCE); + + await expect( + (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()), + ).resolves.toStrictEqual([]); + + restore(); + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); + await (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()); + await expect(store.holdsLeftovers(SOURCE)).resolves.toBe(false); + }); + }); + + describe('replaceAll after loading a book that could not be read', () => { + it('keeps the book as stored when the draft carries nothing for it, leaving out no book', async () => { + const repository = await loadedWithUnreadableGenesis(store); + + await expect(repository.replaceAll(exodusOnlyDraft())).resolves.toStrictEqual([]); + + await expect(reloadForCopy(store)).resolves.toStrictEqual({ + draft: inIdOrder(exodusOnlyDraft()), + unreadableBooks: ['GEN'], + }); + }); + + it('resolves with the book, keeping it as stored, when the draft carries analysis for it', async () => { + const repository = await loadedWithUnreadableGenesis(store); + const withoutSegments = (draft: DraftProject): DraftProject => ({ + ...draft, + analysis: { ...draft.analysis, segmentAnalyses: [], segmentAnalysisLinks: [] }, + }); + + await expect(repository.replaceAll(withoutSegments(fullDraft()))).resolves.toStrictEqual([ + 'GEN', + ]); + + await expect(reloadForCopy(store)).resolves.toStrictEqual({ + draft: inIdOrder(withoutSegments(fullDraftWithoutGenesis())), + unreadableBooks: ['GEN'], + }); + }); + + it('writes the book like any other once told to discard what could not be read', async () => { + const repository = await loadedWithUnreadableGenesis(store); + + await expect( + repository.replaceAll(fullDraft(), { discardUnreadable: true }), + ).resolves.toStrictEqual([]); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + }); + + it('wipes the book the draft carries nothing for once told to discard what could not be read', async () => { + const repository = await loadedWithUnreadableGenesis(store); + + await repository.replaceAll(exodusOnlyDraft(), { discardUnreadable: true }); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); + await expect(store.holdsLeftovers(SOURCE)).resolves.toBe(false); + }); + }); + + describe('markClean', () => { + it('records the stored draft as clean, leaving its content as it stands', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + + await (await store.open(SOURCE)).markClean(); + + await expect(reload(store)).resolves.toStrictEqual( + inIdOrder({ ...fullDraft(), dirty: false }), + ); + }); + + it('keeps a book that could not be read as stored', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageBook(SOURCE, 'GEN', 'corrupt'); + + await (await store.open(SOURCE)).markClean(); + + await expect(reloadForCopy(store)).resolves.toStrictEqual({ + draft: inIdOrder({ ...fullDraftWithoutGenesis(), dirty: false }), + unreadableBooks: ['GEN'], + }); + }); + + it('leaves an already-clean draft as it stands', async () => { + await (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()); + + await (await store.open(SOURCE)).markClean(); + + await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); + }); + + it('leaves no draft when none is saved', async () => { + await (await store.open(SOURCE)).markClean(); + + await expect(reload(store)).resolves.toStrictEqual(emptyDraft(SOURCE)); + }); + + it('refuses a draft written by a newer build, leaving it in place', async () => { + await store.storeNewerBuildDraft(SOURCE); + + await expect((await store.open(SOURCE)).markClean()).rejects.toThrow(); + + await expect((await store.open(SOURCE)).exportAll()).rejects.toThrow(); + }); + + it('refuses an unusable draft, leaving it in place', async () => { + await (await store.open(SOURCE)).replaceAll(fullDraft()); + await store.damageDraft(SOURCE); + + await expect((await store.open(SOURCE)).markClean()).rejects.toThrow(); + + await expect((await store.open(SOURCE)).exportForCopy()).rejects.toThrow(); + }); }); }); } diff --git a/src/__tests__/services/draftRepository.conformance.test.ts b/src/__tests__/services/draftRepository.conformance.test.ts index 548cd7506..1ec7262e5 100644 --- a/src/__tests__/services/draftRepository.conformance.test.ts +++ b/src/__tests__/services/draftRepository.conformance.test.ts @@ -1,7 +1,7 @@ /// import { DraftRepository } from '../../services/draftRepository'; -import { draftKey } from '../../services/storageKeys'; +import { draftAnalysisKey, draftKey } from '../../services/storageKeys'; import { emptyDraft } from '../../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../../types/model-version'; import { describeAnalysisRepositoryConformance } from '../analysis-repository-conformance'; @@ -9,20 +9,70 @@ import { memoryStorage, silentLogger } from '../test-helpers'; const logger = silentLogger(); +/** Whether the current test damaged storage or failed its cleanup, which a repository may log. */ +let warningsExpected = false; + describeAnalysisRepositoryConformance('DraftRepository', () => { - const { storage } = memoryStorage(); + const { storage: memory, records } = memoryStorage(); + warningsExpected = false; + /** The draft key whose landing starts refusals, while cleanup is set to fail. */ + let failingDraftKey: string | undefined; + let landed = false; + /** Refuses every write and deletion once the draft lands, while cleanup is set to fail. */ + const refuseAfterLanding = async (key: string, operation: () => Promise) => { + if (landed) throw new Error('cleanup refused'); + await operation(); + if (key === failingDraftKey) landed = true; + }; + const storage = { + read: memory.read, + write: (key: string, data: string) => refuseAfterLanding(key, () => memory.write(key, data)), + delete: (key: string) => refuseAfterLanding(key, () => memory.delete(key)), + }; return { open: async (sourceProjectId) => new DraftRepository(storage, sourceProjectId, logger), storeNewerBuildDraft: (sourceProjectId) => - storage.write( + memory.write( draftKey(sourceProjectId), JSON.stringify({ ...emptyDraft(sourceProjectId), modelVersion: CURRENT_MODEL_VERSION + 1 }), ), + damageBook: async (sourceProjectId, bookCode, damage) => { + warningsExpected = true; + const key = draftAnalysisKey(sourceProjectId, bookCode); + if (damage === 'lost') records.delete(key); + else records.set(key, 'not json'); + }, + damageDraft: async (sourceProjectId) => { + warningsExpected = true; + records.set(draftKey(sourceProjectId), JSON.stringify({ dirty: true })); + }, + failCleanup: (sourceProjectId) => { + warningsExpected = true; + failingDraftKey = draftKey(sourceProjectId); + return () => { + failingDraftKey = undefined; + landed = false; + }; + }, + holdsLeftovers: async (sourceProjectId) => { + const stored: { analysisBooks?: string[] } = JSON.parse( + records.get(draftKey(sourceProjectId)) ?? '{}', + ); + const needed = new Set([ + draftKey(sourceProjectId), + ...(stored.analysisBooks ?? []).map((bookCode) => + draftAnalysisKey(sourceProjectId, bookCode), + ), + ]); + return [...records.keys()].some( + (key) => key.startsWith(`${draftKey(sourceProjectId)}:`) && !needed.has(key), + ); + }, }; }); -// A conforming save and load of well-formed drafts has nothing to warn about, so a warning means -// the repository found fault with records it wrote itself. +// A conforming save and load of well-formed drafts has nothing to warn about, so a warning there +// means the repository found fault with records it wrote itself. afterEach(() => { - expect(logger.warn).not.toHaveBeenCalled(); + if (!warningsExpected) expect(logger.warn).not.toHaveBeenCalled(); }); diff --git a/src/__tests__/services/draftRepository.test.ts b/src/__tests__/services/draftRepository.test.ts index 95885f807..bf889a1b7 100644 --- a/src/__tests__/services/draftRepository.test.ts +++ b/src/__tests__/services/draftRepository.test.ts @@ -101,68 +101,7 @@ describe('DraftRepository', () => { ); }); - it('names a book whose shard is unusable, and loads the rest', async () => { - const { storage, records } = memoryStorage(); - await new DraftRepository(storage, 'src', silentLogger()).replaceAll(twoBookDraft()); - records.set(draftAnalysisKey('src', 'GEN'), 'not json'); - - const { draft, unreadableBooks } = await new DraftRepository( - storage, - 'src', - silentLogger(), - ).exportAll(); - - expect(unreadableBooks).toEqual(['GEN']); - expect(draft.analysis.tokenAnalysisLinks.map(({ id }) => id)).toEqual(['tl-EXO']); - }); - describe('exportForCopy', () => { - it('exports a readable draft whole, naming no book', async () => { - const { storage } = memoryStorage(); - await new DraftRepository(storage, 'src', silentLogger()).replaceAll(twoBookDraft()); - - await expect( - new DraftRepository(storage, 'src', silentLogger()).exportForCopy(), - ).resolves.toEqual({ draft: twoBookDraft(), unreadableBooks: [] }); - }); - - it('names a book whose shard is unusable, and exports the rest', async () => { - const { storage, records } = memoryStorage(); - await new DraftRepository(storage, 'src', silentLogger()).replaceAll(twoBookDraft()); - records.set(draftAnalysisKey('src', 'GEN'), 'not json'); - - const { draft, unreadableBooks } = await new DraftRepository( - storage, - 'src', - silentLogger(), - ).exportForCopy(); - - expect(unreadableBooks).toEqual(['GEN']); - expect(draft.analysis.tokenAnalysisLinks.map(({ id }) => id)).toEqual(['tl-EXO']); - }); - - it('names a book whose shard the manifest lists but storage lacks', async () => { - const { storage, records } = memoryStorage(); - await new DraftRepository(storage, 'src', silentLogger()).replaceAll(twoBookDraft()); - records.delete(draftAnalysisKey('src', 'EXO')); - - const { unreadableBooks } = await new DraftRepository( - storage, - 'src', - silentLogger(), - ).exportForCopy(); - - expect(unreadableBooks).toEqual(['EXO']); - }); - - it('exports a fresh empty draft when none is stored', async () => { - const { storage } = memoryStorage(); - - await expect( - new DraftRepository(storage, 'src', silentLogger()).exportForCopy(), - ).resolves.toEqual({ draft: emptyDraft('src'), unreadableBooks: [] }); - }); - it('exports an inline draft whole, naming no book', async () => { const inline = twoBookDraft(); const { storage } = memoryStorage({ [draftKey('src')]: JSON.stringify(inline) }); @@ -279,14 +218,6 @@ describe('DraftRepository', () => { expect(records.get(draftAnalysisKey('src', 'GEN'))).toBe('not json'); }); - it('leaves out no book when the draft carries nothing for an unreadable one', async () => { - const { repository, draft } = await loadedWithUnreadableGen(); - - await expect(repository.replaceAll({ ...draft, analysis: emptyAnalysis() })).resolves.toEqual( - [], - ); - }); - it('leaves out the analysis a draft carries for an unreadable book, keeping its shard', async () => { const { repository, records } = await loadedWithUnreadableGen(); @@ -295,29 +226,6 @@ describe('DraftRepository', () => { expect(records.get(draftAnalysisKey('src', 'GEN'))).toBe('not json'); }); - it('saves the rest of a draft carrying analysis for an unreadable book', async () => { - const { repository, storage } = await loadedWithUnreadableGen(); - const draft = twoBookDraft(); - - await repository.replaceAll({ - ...draft, - analysis: { ...draft.analysis, segmentAnalyses: [], segmentAnalysisLinks: [] }, - }); - - const reloaded = await new DraftRepository(storage, 'src', silentLogger()).exportForCopy(); - expect(reloaded.unreadableBooks).toEqual(['GEN']); - expect(reloaded.draft.analysis.segmentAnalysisLinks).toEqual([]); - expect(reloaded.draft.analysis.tokenAnalysisLinks.map(({ id }) => id)).toEqual(['tl-EXO']); - }); - - it('writes over an unreadable book once the draft is started over', async () => { - const { repository, records } = await loadedWithUnreadableGen(); - - await repository.replaceAll(twoBookDraft(), { discardUnreadable: true }); - - expect(records.get(draftAnalysisKey('src', 'GEN'))).not.toBe('not json'); - }); - it('wipes an unreadable book the draft carries nothing for once the draft is started over', async () => { const { repository, draft, records } = await loadedWithUnreadableGen(); diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index 5287dc320..80cd873e4 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -72,7 +72,8 @@ export interface AnalysisRepository { exportAll(): Promise; /** - * Loads the whole stored draft for copying into a project. + * Loads the whole stored draft for copying into a project, or an empty one for the source project + * when none is stored. * * @throws {SyntaxError} If the draft's stored value contains invalid JSON. * @throws {Error} If the stored draft was written by a newer build. From 71d3068426cc41c5ad1b1e670c7f3ba7c78badcc Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 22 Oct 2026 11:03:05 -0400 Subject: [PATCH 12/15] Change nested values in the ownership tests A repository that copies a draft only shallowly keeps aliases to the glosses, morphemes and segmentation inside it, so the tests that change a draft after saving or loading it now change those too. Helper and hook docs in the suite say what each returns or does rather than which helper it delegates to or what each argument value means. Co-Authored-By: Claude Opus 5.5 --- .../analysis-repository-conformance.ts | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index f2f8ae24f..34c2aead8 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -18,8 +18,8 @@ export interface ConformanceStore { storeNewerBuildDraft(sourceProjectId: string): Promise; /** - * Leaves the stored analysis of one book of the draft for `sourceProjectId` unreadable, the rest - * of the draft as it stands: `corrupt` keeps something unusable in its place, `lost` nothing. + * Leaves the stored analysis of one book of the draft for `sourceProjectId` unreadable, corrupt + * or lost outright, with the rest of the draft as it stands. */ damageBook(sourceProjectId: string, bookCode: string, damage: 'corrupt' | 'lost'): Promise; @@ -217,6 +217,24 @@ function inIdOrder(draft: DraftProject): DraftProject { }; } +/** + * Changes `draft` in place, from its own lists down to values nested in its records and its + * segmentation, so a repository still holding any part of it shows. + */ +function changeInPlace(draft: DraftProject): void { + draft.analysisLanguages.push('de'); + draft.analysis.tokenAnalyses.forEach((payload) => { + payload.surfaceText = 'changed'; + if (payload.gloss) payload.gloss.en = 'changed'; + payload.morphemes?.forEach((morpheme) => { + if (morpheme.gloss) morpheme.gloss.en = 'changed'; + }); + }); + draft.segmentation?.addedStarts.forEach((start) => { + start.surfaceText = 'changed'; + }); +} + /** Expects `value` to survive a JSON round trip unchanged and to hold no `null`. */ function expectJsonPlain(value: unknown): void { expect(JSON.parse(JSON.stringify(value))).toStrictEqual(value); @@ -232,12 +250,18 @@ function readableDraft(exported: DraftExport): DraftProject { return inIdOrder(exported.draft); } -/** Loads the draft through `repository`, as {@link readableDraft} returns it. */ +/** + * Loads the draft through `repository`, expecting every book readable, with record lists in id + * order. + */ async function load(repository: AnalysisRepository): Promise { return readableDraft(await repository.exportAll()); } -/** Loads the draft through a newly opened repository, as {@link readableDraft} returns it. */ +/** + * Loads the draft through a newly opened repository, expecting every book readable, with record + * lists in id order. + */ async function reload(store: ConformanceStore, sourceProjectId = SOURCE): Promise { return load(await store.open(sourceProjectId)); } @@ -248,10 +272,7 @@ async function reloadForCopy(store: ConformanceStore): Promise { return { draft: inIdOrder(draft), unreadableBooks }; } -/** - * Saves {@link fullDraft}, leaves GEN's stored analysis unreadable, and returns a repository that - * has loaded the draft since. - */ +/** A repository that has loaded {@link fullDraft} with GEN's stored analysis unreadable. */ async function loadedWithUnreadableGenesis(store: ConformanceStore): Promise { await (await store.open(SOURCE)).replaceAll(fullDraft()); await store.damageBook(SOURCE, 'GEN', 'corrupt'); @@ -303,9 +324,7 @@ export function describeAnalysisRepositoryConformance( const repository = await store.open(SOURCE); await repository.replaceAll(fullDraft()); - const { draft: loaded } = await repository.exportAll(); - loaded.analysisLanguages.push('de'); - loaded.analysis.tokenAnalyses[0].surfaceText = 'changed'; + changeInPlace((await repository.exportAll()).draft); await expect(load(repository)).resolves.toStrictEqual(inIdOrder(fullDraft())); }); @@ -482,8 +501,7 @@ export function describeAnalysisRepositoryConformance( const draft = fullDraft(); await repository.replaceAll(draft); - draft.analysisLanguages.push('de'); - draft.analysis.tokenAnalyses[0].surfaceText = 'changed'; + changeInPlace(draft); await expect(load(repository)).resolves.toStrictEqual(inIdOrder(fullDraft())); await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); From b5dceaa090533143a9db2dd3b0488420c183dd0b Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 22 Oct 2026 11:42:41 -0400 Subject: [PATCH 13/15] Tighten the ownership and cleanup checks per review - Change every record and link, and the values nested in them and in the segmentation, when testing what a repository holds on to. - Give saving a draft changed in place its own test. - Expect a failed cleanup to leave something behind, so a store whose failCleanup or holdsLeftovers hook is mis-wired fails. - Retitle the markClean test with no saved draft to what the seam shows. - In the DraftRepository runner, refuse only the failing draft's other records, and only between a save landing and the next call. - State in replaceAll's doc that a save leaves nothing the draft no longer needs and that a later save finishes a failed cleanup. Co-Authored-By: Claude Opus 5.5 --- .../analysis-repository-conformance.ts | 48 ++++++++++++++++--- .../draftRepository.conformance.test.ts | 14 ++++-- src/types/analysis-repository.ts | 11 +++-- 3 files changed, 57 insertions(+), 16 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index 34c2aead8..46a7f3364 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -31,7 +31,8 @@ export interface ConformanceStore { /** * Makes the cleanup each save of the draft for `sourceProjectId` does after its content lands - * fail, until the returned function is called. + * fail, leaving storage that {@link ConformanceStore.holdsLeftovers} reports, until the returned + * function is called. */ failCleanup(sourceProjectId: string): () => void; @@ -218,19 +219,44 @@ function inIdOrder(draft: DraftProject): DraftProject { } /** - * Changes `draft` in place, from its own lists down to values nested in its records and its - * segmentation, so a repository still holding any part of it shows. + * Changes `draft` in place at each level of nesting, so a repository still holding part of it + * shows. */ function changeInPlace(draft: DraftProject): void { + const { analysis, segmentation } = draft; draft.analysisLanguages.push('de'); - draft.analysis.tokenAnalyses.forEach((payload) => { - payload.surfaceText = 'changed'; + [ + ...analysis.segmentAnalyses, + ...analysis.segmentAnalysisLinks, + ...analysis.tokenAnalyses, + ...analysis.tokenAnalysisLinks, + ...analysis.phraseAnalyses, + ...analysis.phraseAnalysisLinks, + ].forEach((record) => { + record.updatedAt = '2026-02-02T00:00:00.000Z'; + }); + analysis.segmentAnalyses.forEach((payload) => { + if (payload.freeTranslation) payload.freeTranslation.en = 'changed'; + }); + analysis.tokenAnalyses.forEach((payload) => { if (payload.gloss) payload.gloss.en = 'changed'; payload.morphemes?.forEach((morpheme) => { if (morpheme.gloss) morpheme.gloss.en = 'changed'; }); }); - draft.segmentation?.addedStarts.forEach((start) => { + analysis.phraseAnalyses.forEach((payload) => { + if (payload.gloss) payload.gloss.en = 'changed'; + }); + analysis.tokenAnalysisLinks.forEach((link) => { + link.token.surfaceText = 'changed'; + }); + analysis.phraseAnalysisLinks.forEach((link) => { + link.tokens.forEach((token) => { + token.surfaceText = 'changed'; + }); + }); + segmentation?.removedVerseStarts.push('EXO 1:2'); + segmentation?.addedStarts.forEach((start) => { start.surfaceText = 'changed'; }); } @@ -505,6 +531,13 @@ export function describeAnalysisRepositoryConformance( await expect(load(repository)).resolves.toStrictEqual(inIdOrder(fullDraft())); await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); + }); + + it('stores changes made in place to a draft it saved once that draft is saved again', async () => { + const repository = await store.open(SOURCE); + const draft = fullDraft(); + await repository.replaceAll(draft); + changeInPlace(draft); await repository.replaceAll(draft); @@ -536,6 +569,7 @@ export function describeAnalysisRepositoryConformance( await expect( (await store.open(SOURCE)).replaceAll(exodusOnlyDraft()), ).resolves.toStrictEqual([]); + await expect(store.holdsLeftovers(SOURCE)).resolves.toBe(true); restore(); await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); @@ -624,7 +658,7 @@ export function describeAnalysisRepositoryConformance( await expect(reload(store)).resolves.toStrictEqual(inIdOrder(exodusOnlyDraft())); }); - it('leaves no draft when none is saved', async () => { + it('resolves when no draft is saved, which still loads as empty', async () => { await (await store.open(SOURCE)).markClean(); await expect(reload(store)).resolves.toStrictEqual(emptyDraft(SOURCE)); diff --git a/src/__tests__/services/draftRepository.conformance.test.ts b/src/__tests__/services/draftRepository.conformance.test.ts index 1ec7262e5..7a707768f 100644 --- a/src/__tests__/services/draftRepository.conformance.test.ts +++ b/src/__tests__/services/draftRepository.conformance.test.ts @@ -15,17 +15,23 @@ let warningsExpected = false; describeAnalysisRepositoryConformance('DraftRepository', () => { const { storage: memory, records } = memoryStorage(); warningsExpected = false; - /** The draft key whose landing starts refusals, while cleanup is set to fail. */ + /** The key of the draft whose cleanup fails; unset while cleanup succeeds. */ let failingDraftKey: string | undefined; let landed = false; - /** Refuses every write and deletion once the draft lands, while cleanup is set to fail. */ + /** + * Refuses writes and deletions of the failing draft's other records from the time a save of it + * lands until the draft is next read, which is how every call on it starts. + */ const refuseAfterLanding = async (key: string, operation: () => Promise) => { - if (landed) throw new Error('cleanup refused'); + if (landed && key.startsWith(`${failingDraftKey}:`)) throw new Error('cleanup refused'); await operation(); if (key === failingDraftKey) landed = true; }; const storage = { - read: memory.read, + read: async (key: string) => { + if (key === failingDraftKey) landed = false; + return memory.read(key); + }, write: (key: string, data: string) => refuseAfterLanding(key, () => memory.write(key, data)), delete: (key: string) => refuseAfterLanding(key, () => memory.delete(key)), }; diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index 80cd873e4..1e787045a 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -83,11 +83,12 @@ export interface AnalysisRepository { exportForCopy(): Promise; /** - * Saves the whole draft, replacing the stored one, stamped with this build's model version. - * Content that lands counts as saved even when cleanup after it fails. A book whose stored - * analysis could not be read is kept as stored unless `options` discards it: a draft that loaded - * it empty never wipes it, and analysis the draft carries for it never stands in for what could - * not be read. + * Saves the whole draft, replacing the stored one, stamped with this build's model version, and + * leaves nothing stored that the saved draft no longer needs. Content that lands counts as saved + * even when cleanup after it fails; a later save, from any repository, finishes that cleanup. A + * book whose stored analysis could not be read is kept as stored unless `options` discards it: a + * draft that loaded it empty never wipes it, and analysis the draft carries for it never stands + * in for what could not be read. * * @returns The books whose analysis in the draft was left out for that reason; empty when the * draft was saved whole. From 84509d90325933431969830194f11f64ca626ccb Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 22 Oct 2026 14:05:27 -0400 Subject: [PATCH 14/15] Clarify replaceAll cleanup guarantee and a conformance test title Co-Authored-By: Claude Opus 5.5 --- src/__tests__/analysis-repository-conformance.ts | 2 +- src/types/analysis-repository.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/__tests__/analysis-repository-conformance.ts b/src/__tests__/analysis-repository-conformance.ts index 46a7f3364..d708b5ee3 100644 --- a/src/__tests__/analysis-repository-conformance.ts +++ b/src/__tests__/analysis-repository-conformance.ts @@ -533,7 +533,7 @@ export function describeAnalysisRepositoryConformance( await expect(reload(store)).resolves.toStrictEqual(inIdOrder(fullDraft())); }); - it('stores changes made in place to a draft it saved once that draft is saved again', async () => { + it('stores in-place changes to a saved draft when that draft is saved again', async () => { const repository = await store.open(SOURCE); const draft = fullDraft(); await repository.replaceAll(draft); diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index 1e787045a..6f1bb8d76 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -83,12 +83,12 @@ export interface AnalysisRepository { exportForCopy(): Promise; /** - * Saves the whole draft, replacing the stored one, stamped with this build's model version, and - * leaves nothing stored that the saved draft no longer needs. Content that lands counts as saved - * even when cleanup after it fails; a later save, from any repository, finishes that cleanup. A - * book whose stored analysis could not be read is kept as stored unless `options` discards it: a - * draft that loaded it empty never wipes it, and analysis the draft carries for it never stands - * in for what could not be read. + * Saves the whole draft, replacing the stored one, stamped with this build's model version. Apart + * from the stored analysis a save keeps for unreadable books, nothing stays stored that the saved + * draft no longer needs. Content that lands counts as saved even when cleanup after it fails; a + * later save, from any repository, finishes that cleanup. A book whose stored analysis could not + * be read is kept as stored unless `options` discards it: a draft that loaded it empty never + * wipes it, and analysis the draft carries for it never stands in for what could not be read. * * @returns The books whose analysis in the draft was left out for that reason; empty when the * draft was saved whole. From 1d44ede05c405ba71a1d524a6dd7f6b7f19344ae Mon Sep 17 00:00:00 2001 From: Danny Rorabaugh Date: Thu, 22 Oct 2026 14:13:15 -0400 Subject: [PATCH 15/15] Condition replaceAll's cleanup guarantee on the cleanup succeeding Co-Authored-By: Claude Opus 5.5 --- src/types/analysis-repository.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/types/analysis-repository.ts b/src/types/analysis-repository.ts index 6f1bb8d76..ea40cca45 100644 --- a/src/types/analysis-repository.ts +++ b/src/types/analysis-repository.ts @@ -83,12 +83,13 @@ export interface AnalysisRepository { exportForCopy(): Promise; /** - * Saves the whole draft, replacing the stored one, stamped with this build's model version. Apart - * from the stored analysis a save keeps for unreadable books, nothing stays stored that the saved - * draft no longer needs. Content that lands counts as saved even when cleanup after it fails; a - * later save, from any repository, finishes that cleanup. A book whose stored analysis could not - * be read is kept as stored unless `options` discards it: a draft that loaded it empty never - * wipes it, and analysis the draft carries for it never stands in for what could not be read. + * Saves the whole draft, replacing the stored one, stamped with this build's model version. Once + * the save's cleanup succeeds, nothing the saved draft no longer needs stays stored, apart from + * the stored analysis a save keeps for unreadable books. Content that lands counts as saved even + * when that cleanup fails; a later save, from any repository, finishes the cleanup. A book whose + * stored analysis could not be read is kept as stored unless `options` discards it: a draft that + * loaded it empty never wipes it, and analysis the draft carries for it never stands in for what + * could not be read. * * @returns The books whose analysis in the draft was left out for that reason; empty when the * draft was saved whole.