From 14bd2548d92498e9e3d64fcf2d3a7da155aa5635 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 15 Oct 2026 15:19:09 -0600 Subject: [PATCH 1/4] Give token and segment links ids and pick ties by rule (#430) --- AGENTS.md | 2 +- perf/dataset/generate.ts | 5 + .../components/AnalysisCatalogPanel.test.tsx | 8 +- .../components/AnalysisStore.test.tsx | 19 +- .../components/ConcordancePanel.test.tsx | 1 + .../components/ConcordanceRowView.test.tsx | 1 + .../components/InterlinearizerLoader.test.tsx | 3 + .../components/StaleAnalysesReporter.test.tsx | 8 +- .../components/TokenChip.suggestions.test.tsx | 14 + src/__tests__/components/TokenChip.test.tsx | 4 + .../converters/pt9/analysisMerger.test.ts | 1 + .../pt9/tokenAnalysisDedupe.test.ts | 1 + src/__tests__/hooks/useDraftProject.test.ts | 2 + .../hooks/useWholeTextReanchor.test.ts | 1 + src/__tests__/services/projectStorage.test.ts | 142 ++++ src/__tests__/store/analysisSlice.test.ts | 630 +++++++++++++++++- src/__tests__/types/type-guards.test.ts | 130 +++- src/__tests__/utils/analysis-book.test.ts | 9 +- src/__tests__/utils/analysis-query.test.ts | 8 +- src/__tests__/utils/concordance.test.ts | 26 +- src/__tests__/utils/pick-rules.test.ts | 81 +++ .../utils/project-analysis-summary.test.ts | 9 +- src/__tests__/utils/reanchor-analysis.test.ts | 3 +- src/__tests__/utils/reanchor-draft.test.ts | 3 + src/converters/pt9/analysisMerger.ts | 1 + src/services/projectStorage.ts | 38 +- src/store/analysisSlice.ts | 301 +++++---- src/types/interlinearizer.d.ts | 18 +- src/types/type-guards.ts | 36 +- src/utils/concordance.ts | 9 +- src/utils/pick-rules.ts | 31 + 31 files changed, 1370 insertions(+), 175 deletions(-) create mode 100644 src/__tests__/utils/pick-rules.test.ts create mode 100644 src/utils/pick-rules.ts diff --git a/AGENTS.md b/AGENTS.md index 51001799d..2a806e73f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,7 +112,7 @@ The core types are: All character offsets (`charStart`, `charEnd`, `charIndex`) are zero-based UTF-16 code-unit indices — plain JavaScript string indices, not code points and not grapheme clusters — with end offsets exclusive. -Key invariants: `Segment.baselineText.slice(charStart, charEnd) === Token.surfaceText`; at most one linked analysis per token/segment may have `status: 'approved'`. Token and phrase payloads are shared by every occurrence analyzed identically, so a phrase occurrence is identified by `PhraseAnalysisLink.id`, never by its `analysisId`. `MultiString` values are keyed by BCP 47 tags. `TokenSnapshot.surfaceText` detects drift when baseline text changes. Every project and draft record carries a `modelVersion`; every write stamps `CURRENT_MODEL_VERSION`, and a record stamped higher than that is refused (`assertSupportedModelVersion`) rather than read or overwritten. +Key invariants: `Segment.baselineText.slice(charStart, charEnd) === Token.surfaceText`; at most one linked analysis per token/segment may have `status: 'approved'`. Token and phrase payloads are shared by every occurrence analyzed identically, so a link is identified by its own `id`, never by its `analysisId`. Picking one of several links or payloads goes through [pick-rules.ts](src/utils/pick-rules.ts), never list order. `MultiString` values are keyed by BCP 47 tags. `TokenSnapshot.surfaceText` detects drift when baseline text changes. Every project and draft record carries a `modelVersion`; every write stamps `CURRENT_MODEL_VERSION`, and a record stamped higher than that is refused (`assertSupportedModelVersion`) rather than read or overwritten. ### TypeScript path aliases diff --git a/perf/dataset/generate.ts b/perf/dataset/generate.ts index 9d6c2ffc5..49b1e8f6b 100644 --- a/perf/dataset/generate.ts +++ b/perf/dataset/generate.ts @@ -69,6 +69,9 @@ export function generateAnalysis( const segmentAnalyses: SegmentAnalysis[] = []; const segmentAnalysisLinks: SegmentAnalysisLink[] = []; + // Drawn apart from the shape's stream, so link ids never move a tier's shape. + const linkIds = createRng(seed, 'link-ids'); + let clock = 0; const nextStamp = (rng: Rng): string => { clock += intBetween(rng, 1, 90_000); @@ -119,6 +122,7 @@ export function generateAnalysis( const payload = tokenPayload(surfaceText, token.writingSystem, sense, rng); const stamp = nextStamp(rng); tokenAnalysisLinks.push({ + id: uuid(linkIds), analysisId: payload.id, createdAt: stamp, updatedAt: stamp, @@ -168,6 +172,7 @@ export function generateAnalysis( }, }); segmentAnalysisLinks.push({ + id: uuid(linkIds), analysisId: id, segmentId: segment.id, createdAt: stamp, diff --git a/src/__tests__/components/AnalysisCatalogPanel.test.tsx b/src/__tests__/components/AnalysisCatalogPanel.test.tsx index 7ad4da514..b234358af 100644 --- a/src/__tests__/components/AnalysisCatalogPanel.test.tsx +++ b/src/__tests__/components/AnalysisCatalogPanel.test.tsx @@ -61,7 +61,13 @@ function link( status: AssignmentStatus = 'approved', surfaceText = 'ἀρχῇ', ): TokenAnalysisLink { - return { ...FIXTURE_STAMPS, analysisId, status, token: { tokenRef, surfaceText } }; + return { + id: `${tokenRef}:${analysisId}:${status}`, + ...FIXTURE_STAMPS, + analysisId, + status, + token: { tokenRef, surfaceText }, + }; } /** The token ref {@link FocusRequestProbe} last claimed, or `undefined` when it claimed nothing. */ diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index ceef889b6..2065d6e48 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -74,6 +74,7 @@ function makeAnalysisWithGloss( gloss: { und: gloss }, }; const link: TokenAnalysisLink = { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: ta.id, status: 'approved', @@ -106,12 +107,14 @@ function makeSharedAnalysis(gloss = 'a', surfaceText = 'word'): TextAnalysis { tokenAnalyses: [ta], tokenAnalysisLinks: [ { + id: 'tl-2', ...FIXTURE_STAMPS, analysisId: ta.id, status: 'approved', token: { tokenRef: 'tok-1', surfaceText }, }, { + id: 'tl-3', ...FIXTURE_STAMPS, analysisId: ta.id, status: 'approved', @@ -207,6 +210,7 @@ describe('useGloss', () => { gloss: { en: 'hi' }, }; const link: TokenAnalysisLink = { + id: 'tl-4', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'suggested', @@ -272,6 +276,7 @@ describe('useGloss', () => { gloss: { fr: 'bonjour' }, }; const link: TokenAnalysisLink = { + id: 'tl-5', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -563,6 +568,7 @@ describe('useGlossDispatch', () => { gloss: { en: 'old' }, }; const suggestedLink: TokenAnalysisLink = { + id: 'tl-6', ...FIXTURE_STAMPS, analysisId: 'suggested-1', status: 'suggested', @@ -1010,7 +1016,7 @@ const SEGMENT_ANALYSIS_WITH_TRANSLATION: TextAnalysis = { }, ], segmentAnalysisLinks: [ - { ...FIXTURE_STAMPS, analysisId: 'sa-1', status: 'approved', segmentId: 'seg-1' }, + { id: 'sl-1', ...FIXTURE_STAMPS, analysisId: 'sa-1', status: 'approved', segmentId: 'seg-1' }, ], tokenAnalyses: [], tokenAnalysisLinks: [], @@ -1119,7 +1125,9 @@ function withSegmentTranslation(segmentId: string, status: AssignmentStatus): Te segmentAnalyses: [ { ...FIXTURE_STAMPS, id: 'sa-1', surfaceText: 'old text', freeTranslation: { und: 'old' } }, ], - segmentAnalysisLinks: [{ ...FIXTURE_STAMPS, analysisId: 'sa-1', status, segmentId }], + segmentAnalysisLinks: [ + { id: 'sl-2', ...FIXTURE_STAMPS, analysisId: 'sa-1', status, segmentId }, + ], }; } @@ -1257,6 +1265,7 @@ describe('useMorphemes', () => { ], }; const link: TokenAnalysisLink = { + id: 'tl-7', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -1385,6 +1394,7 @@ describe('useConfirmMorphemesDispatch', () => { ], tokenAnalysisLinks: [ { + id: 'tl-8', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -1435,6 +1445,7 @@ describe('useMorphemeDeleteDispatch', () => { ], }; const link: TokenAnalysisLink = { + id: 'tl-9', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -1493,6 +1504,7 @@ describe('useMorphemeGlossDispatch', () => { ], }; const link: TokenAnalysisLink = { + id: 'tl-10', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -1533,12 +1545,14 @@ describe('useMorphemeGlossDispatch', () => { ], tokenAnalysisLinks: [ { + id: 'tl-11', ...FIXTURE_STAMPS, analysisId: 'shared-analysis', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'word' }, }, { + id: 'tl-12', ...FIXTURE_STAMPS, analysisId: 'shared-analysis', status: 'approved', @@ -1891,6 +1905,7 @@ describe('useApproveAnalysisDispatch', () => { function approvedLink(analysisId: string, tokenRef: string): TokenAnalysisLink { return { + id: `${tokenRef}:${analysisId}`, ...FIXTURE_STAMPS, analysisId, token: { tokenRef, surfaceText: 'ἀρχῇ' }, diff --git a/src/__tests__/components/ConcordancePanel.test.tsx b/src/__tests__/components/ConcordancePanel.test.tsx index f116273a9..8620f69ed 100644 --- a/src/__tests__/components/ConcordancePanel.test.tsx +++ b/src/__tests__/components/ConcordancePanel.test.tsx @@ -78,6 +78,7 @@ function makeText(overrides: Partial = {}): SourceText { /** Builds an approved link from `tokenRef` to the analysis. */ function link(analysisId: string, tokenRef: string): TokenAnalysisLink { return { + id: `${tokenRef}:${analysisId}`, ...FIXTURE_STAMPS, analysisId, status: 'approved', diff --git a/src/__tests__/components/ConcordanceRowView.test.tsx b/src/__tests__/components/ConcordanceRowView.test.tsx index c69319bdd..c9a40a1fe 100644 --- a/src/__tests__/components/ConcordanceRowView.test.tsx +++ b/src/__tests__/components/ConcordanceRowView.test.tsx @@ -23,6 +23,7 @@ const localizedStrings = Object.fromEntries(CONCORDANCE_ROW_STRING_KEYS.map((k) /** Builds an approved link from `tokenRef` to the analysis. */ function link(analysisId: string, tokenRef: string): TokenAnalysisLink { return { + id: `${tokenRef}:${analysisId}`, ...FIXTURE_STAMPS, analysisId, status: 'approved', diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 52297e1b0..770fb88f8 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -325,6 +325,7 @@ function analysisApprovingAt(tokenRef: string, surfaceText: string): TextAnalysi tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText }], tokenAnalysisLinks: [ { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -3440,6 +3441,7 @@ describe('InterlinearizerLoader', () => { /** A stale place of `analysisId` at `tokenRef`, whose word read differently when glossed. */ function staleAt(analysisId: string, tokenRef: string) { return { + id: `${tokenRef}:${analysisId}`, ...FIXTURE_STAMPS, analysisId, status: 'stale' as const, @@ -3459,6 +3461,7 @@ describe('InterlinearizerLoader', () => { staleAt('ta-gen', 'GEN 1:1:0'), staleAt('ta-exo', 'EXO 1:1:0'), { + id: 'tl-2', ...FIXTURE_STAMPS, analysisId: 'ta-live', status: 'approved', diff --git a/src/__tests__/components/StaleAnalysesReporter.test.tsx b/src/__tests__/components/StaleAnalysesReporter.test.tsx index 8fe3d5616..e4996f76c 100644 --- a/src/__tests__/components/StaleAnalysesReporter.test.tsx +++ b/src/__tests__/components/StaleAnalysesReporter.test.tsx @@ -17,7 +17,13 @@ jest.mock('../../components/AnalysisStore', () => ({ })); function staleLink(analysisId: string, tokenRef: string): TokenAnalysisLink { - return { ...FIXTURE_STAMPS, analysisId, token: { tokenRef, surfaceText: 'In' }, status: 'stale' }; + return { + id: `${tokenRef}:${analysisId}`, + ...FIXTURE_STAMPS, + analysisId, + token: { tokenRef, surfaceText: 'In' }, + status: 'stale', + }; } function staleTranslation(analysisId: string, segmentId: string): StaleFreeTranslation { diff --git a/src/__tests__/components/TokenChip.suggestions.test.tsx b/src/__tests__/components/TokenChip.suggestions.test.tsx index 69d1b0699..518c3d4b7 100644 --- a/src/__tests__/components/TokenChip.suggestions.test.tsx +++ b/src/__tests__/components/TokenChip.suggestions.test.tsx @@ -34,6 +34,7 @@ function poolWithOneApproved(gloss: string | undefined, surfaceText = 'logos'): ...(gloss === undefined ? {} : { gloss: { en: gloss } }), }; const link: TokenAnalysisLink = { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -64,18 +65,21 @@ function homographBankPool(financeGloss: string | undefined): TextAnalysis { }; const links: TokenAnalysisLink[] = [ { + id: 'tl-2', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r1', surfaceText: 'bank' }, }, { + id: 'tl-3', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r2', surfaceText: 'bank' }, }, { + id: 'tl-4', ...FIXTURE_STAMPS, analysisId: 'ta-fin', status: 'approved', @@ -113,18 +117,21 @@ function sameGlossParsePool(rivalForms: readonly string[]): TextAnalysis { }; const links: TokenAnalysisLink[] = [ { + id: 'tl-5', ...FIXTURE_STAMPS, analysisId: 'ta-past', status: 'approved', token: { tokenRef: 'p1', surfaceText: 'ran' }, }, { + id: 'tl-6', ...FIXTURE_STAMPS, analysisId: 'ta-past', status: 'approved', token: { tokenRef: 'p2', surfaceText: 'ran' }, }, { + id: 'tl-7', ...FIXTURE_STAMPS, analysisId: 'ta-rival', status: 'approved', @@ -154,18 +161,21 @@ function homographTopBlankPool(): TextAnalysis { }; const links: TokenAnalysisLink[] = [ { + id: 'tl-8', ...FIXTURE_STAMPS, analysisId: 'ta-blank', status: 'approved', token: { tokenRef: 'b1', surfaceText: 'bank' }, }, { + id: 'tl-9', ...FIXTURE_STAMPS, analysisId: 'ta-blank', status: 'approved', token: { tokenRef: 'b2', surfaceText: 'bank' }, }, { + id: 'tl-10', ...FIXTURE_STAMPS, analysisId: 'ta-fin', status: 'approved', @@ -545,24 +555,28 @@ function homographMajorityPool(): TextAnalysis { }; const links: TokenAnalysisLink[] = [ { + id: 'tl-11', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r1', surfaceText: 'bank' }, }, { + id: 'tl-12', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r2', surfaceText: 'bank' }, }, { + id: 'tl-13', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r3', surfaceText: 'bank' }, }, { + id: 'tl-14', ...FIXTURE_STAMPS, analysisId: 'ta-fin', status: 'approved', diff --git a/src/__tests__/components/TokenChip.test.tsx b/src/__tests__/components/TokenChip.test.tsx index a231f7067..7d20a51a0 100644 --- a/src/__tests__/components/TokenChip.test.tsx +++ b/src/__tests__/components/TokenChip.test.tsx @@ -209,11 +209,13 @@ describe('TokenChip', () => { ], tokenAnalysisLinks: [ { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'GEN 1:1:0', surfaceText: 'hello' }, } satisfies { + id: string; analysisId: string; createdAt: string; updatedAt: string; @@ -966,11 +968,13 @@ describe('TokenChip read-only', () => { tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'hello', gloss: { und: 'in' } }], tokenAnalysisLinks: [ { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'GEN 1:1:0', surfaceText: 'hello' }, } satisfies { + id: string; analysisId: string; createdAt: string; updatedAt: string; diff --git a/src/__tests__/converters/pt9/analysisMerger.test.ts b/src/__tests__/converters/pt9/analysisMerger.test.ts index 8909d5af1..dab2a861e 100644 --- a/src/__tests__/converters/pt9/analysisMerger.test.ts +++ b/src/__tests__/converters/pt9/analysisMerger.test.ts @@ -97,6 +97,7 @@ describe('mergeLanguageAnalyses - token records', () => { ]); expect(result.tokenAnalysisLinks).toStrictEqual([ { + id: 'pt9:tl:GEN 1:1:0:0', analysisId: 'pt9:ta:GEN 1:1:0:0', createdAt: STAMP, updatedAt: STAMP, diff --git a/src/__tests__/converters/pt9/tokenAnalysisDedupe.test.ts b/src/__tests__/converters/pt9/tokenAnalysisDedupe.test.ts index c50e6de40..e95f851ee 100644 --- a/src/__tests__/converters/pt9/tokenAnalysisDedupe.test.ts +++ b/src/__tests__/converters/pt9/tokenAnalysisDedupe.test.ts @@ -30,6 +30,7 @@ function link( surfaceText = 'plovs', ): TokenAnalysisLink { return { + id: `${tokenRef}:${status}`, analysisId, createdAt: STAMP, updatedAt: STAMP, diff --git a/src/__tests__/hooks/useDraftProject.test.ts b/src/__tests__/hooks/useDraftProject.test.ts index 61d70c0a4..b144ba86b 100644 --- a/src/__tests__/hooks/useDraftProject.test.ts +++ b/src/__tests__/hooks/useDraftProject.test.ts @@ -38,6 +38,7 @@ function analysisWithToken(id: string): TextAnalysis { tokenAnalyses: [{ ...FIXTURE_STAMPS, id, surfaceText: 'word' }], tokenAnalysisLinks: [ { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: id, status: 'approved', @@ -440,6 +441,7 @@ describe('useDraftProject', () => { tokenAnalysisLinks: [ ...genToken.tokenAnalysisLinks, { + id: 'tl-2', ...FIXTURE_STAMPS, analysisId: 'mat-tok', status: 'approved', diff --git a/src/__tests__/hooks/useWholeTextReanchor.test.ts b/src/__tests__/hooks/useWholeTextReanchor.test.ts index 046f940f6..5d7feea35 100644 --- a/src/__tests__/hooks/useWholeTextReanchor.test.ts +++ b/src/__tests__/hooks/useWholeTextReanchor.test.ts @@ -24,6 +24,7 @@ function draftLinkingInto(...tokenRefs: string[]): DraftProject { ...emptyAnalysis(), tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText: 'beta' }], tokenAnalysisLinks: tokenRefs.map((tokenRef) => ({ + id: tokenRef, ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', diff --git a/src/__tests__/services/projectStorage.test.ts b/src/__tests__/services/projectStorage.test.ts index ea73c71fb..c24a546c7 100644 --- a/src/__tests__/services/projectStorage.test.ts +++ b/src/__tests__/services/projectStorage.test.ts @@ -70,6 +70,7 @@ function makeDraftSpanningBooks(sourceProjectId: string, ...bookCodes: string[]) gloss: { en: `gloss-${book}` }, }); draft.analysis.tokenAnalysisLinks.push({ + id: `link-${book}`, analysisId: `analysis-${book}`, ...FIXTURE_STAMPS, status: 'approved', @@ -98,6 +99,7 @@ function analysisWithDanglingLink(): TextAnalysis { ...emptyAnalysis(), tokenAnalysisLinks: [ { + id: 'tl-2', ...FIXTURE_STAMPS, analysisId: 'ta-missing', status: 'approved', @@ -107,6 +109,20 @@ function analysisWithDanglingLink(): TextAnalysis { }; } +/** `analysis` as stored before links carried ids. */ +function withoutLinkIds(analysis: TextAnalysis): unknown { + const stripped = (link: L): Partial => { + const copy: Partial = { ...link }; + delete copy.id; + return copy; + }; + return { + ...analysis, + tokenAnalysisLinks: analysis.tokenAnalysisLinks.map(stripped), + segmentAnalysisLinks: analysis.segmentAnalysisLinks.map(stripped), + }; +} + describe('projectStorage', () => { beforeEach(() => { resetQueuesForTesting(); @@ -443,6 +459,19 @@ describe('projectStorage', () => { expect(__mockLogger.warn).not.toHaveBeenCalled(); }); + + it('loads a project holding a link with no id with an empty analysis, and logs it', async () => { + const draft = makeDraftSpanningBooks('src-proj', 'GEN'); + const stored = { ...makeStubProject('abc'), analysis: withoutLinkIds(draft.analysis) }; + __mockReadUserData.mockResolvedValue(JSON.stringify(stored)); + + const result = await getProject(token, 'abc'); + + expect(result?.analysis).toEqual(emptyAnalysis()); + expect(__mockLogger.warn).toHaveBeenCalledWith( + 'Interlinearizer: project abc has links without ids; loading it empty', + ); + }); }); describe('listProjects', () => { @@ -1315,6 +1344,41 @@ describe('projectStorage', () => { ); }); + it('loads a book whose shard holds a link with no id as empty, and logs it', async () => { + const draft = makeDraftSpanningBooks('src-proj', 'GEN', 'JHN'); + await saveDraft(token, 'src-proj', draft); + const [, jhn] = draft.analysis.tokenAnalysisLinks; + const oldShard = withoutLinkIds({ ...emptyAnalysis(), tokenAnalysisLinks: [jhn] }); + __mockReadUserData.mockImplementation(async (_t: unknown, key: unknown) => { + if (key === 'draft:src-proj:analysis:JHN') return JSON.stringify(oldShard); + const written = __mockWriteUserData.mock.calls.findLast(([, k]) => k === key); + if (!written) throw enoentError(); + return written[2]; + }); + + const loaded = await getDraft(token, 'src-proj'); + + expect(loaded.analysis.tokenAnalysisLinks.map((l) => l.token.tokenRef)).toEqual([ + 'GEN 1:1!0', + ]); + expect(__mockLogger.warn).toHaveBeenCalledWith( + 'Interlinearizer: analysis shard draft:src-proj:analysis:JHN has links without ids; treating as empty', + ); + }); + + it('loads a draft whose inline analysis holds a link with no id as empty, and logs it', async () => { + const draft = makeDraftSpanningBooks('src-proj', 'GEN'); + const stored = { ...draft, dirty: true, analysis: withoutLinkIds(draft.analysis) }; + __mockReadUserData.mockResolvedValue(JSON.stringify(stored)); + + const result = await getDraft(token, 'src-proj'); + + expect(result).toEqual(emptyDraft('src-proj')); + expect(__mockLogger.warn).toHaveBeenCalledWith( + 'Interlinearizer: draft for source project src-proj has links without ids; loading it empty', + ); + }); + it('keeps the other books when one shard is not a valid analysis', async () => { const draft = makeDraftSpanningBooks('src-proj', 'GEN', 'JHN'); await saveDraft(token, 'src-proj', draft); @@ -1488,6 +1552,7 @@ describe('projectStorage', () => { }); ['GEN', 'EXO'].forEach((book) => { draft.analysis.tokenAnalysisLinks.push({ + id: 'tl-3', analysisId: 'shared', ...FIXTURE_STAMPS, status: 'approved', @@ -2461,5 +2526,82 @@ describe('projectStorage', () => { expect(__mockWriteUserData).not.toHaveBeenCalled(); }); }); + + describe('through save and reload', () => { + const IMPORTED: TextAnalysis = { + ...emptyAnalysis(), + tokenAnalyses: [ + { ...FIXTURE_STAMPS, id: 'pt9:ta:GEN 1:1:0:0', surfaceText: 'In', gloss: { en: 'in' } }, + ], + tokenAnalysisLinks: [ + { + ...FIXTURE_STAMPS, + id: 'pt9:tl:GEN 1:1:0:0', + analysisId: 'pt9:ta:GEN 1:1:0:0', + status: 'approved', + token: { tokenRef: 'GEN 1:1:0', surfaceText: 'In' }, + }, + ], + }; + + /** Keeps every write, serving it back to later reads. */ + function writeThroughStore(): void { + const records = new Map(); + __mockWriteUserData.mockImplementation(async (_t: unknown, key: string, value: string) => { + records.set(key, value); + }); + __mockReadUserData.mockImplementation(async (_t: unknown, key: string) => { + const value = records.get(key); + if (value === undefined) throw enoentError(); + return value; + }); + } + + /** Imports {@link IMPORTED} and makes an editable copy of it. */ + async function importAndCopy() { + writeThroughStore(); + jest + .spyOn(crypto, 'randomUUID') + .mockReturnValueOnce('00000000-0000-0000-0000-00000000000a') + .mockReturnValueOnce('00000000-0000-0000-0000-00000000000b'); + const imported = await savePt9Import( + token, + 'src-project', + 'Import', + 'Imported', + ['en'], + IMPORTED, + PT9_PROVENANCE, + ); + const copy = await createEditableCopy(token, imported.id, 'Copy'); + return { imported, copy }; + } + + it('reloads a new import with its analysis', async () => { + const { imported } = await importAndCopy(); + + expect((await getProject(token, imported.id))?.analysis).toEqual(IMPORTED); + }); + + it('reloads the editable copy with its analysis after a save', async () => { + const { copy } = await importAndCopy(); + + await updateAnalysis(token, copy.id, IMPORTED); + + expect((await getProject(token, copy.id))?.analysis).toEqual(IMPORTED); + }); + + it('reloads a draft opened from the editable copy with its analysis', async () => { + const { copy } = await importAndCopy(); + const opened = await getProject(token, copy.id); + + await saveDraft(token, 'src-project', { + ...emptyDraft('src-project'), + analysis: opened?.analysis ?? emptyAnalysis(), + }); + + expect((await getDraft(token, 'src-project')).analysis).toEqual(IMPORTED); + }); + }); }); }); diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index b5f6b4b14..8d756f4da 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -76,6 +76,7 @@ import { makePhraseLink, makeVerseBook, FIXTURE_STAMPS } from '../test-helpers'; */ function makeApprovedLink(ta: TokenAnalysis): TokenAnalysisLink { return { + id: `tok-1:${ta.id}`, ...FIXTURE_STAMPS, analysisId: ta.id, status: 'approved', @@ -90,6 +91,7 @@ function makeLink( status: AssignmentStatus, ): TokenAnalysisLink { return { + id: `${tokenRef}:${ta.id}:${status}`, ...FIXTURE_STAMPS, analysisId: ta.id, status, @@ -145,6 +147,7 @@ describe('writeGloss', () => { it('removes an orphaned approved link and creates a fresh one', () => { // Orphaned state: an approved link whose analysisId has no matching TokenAnalysis. const orphanLink: TokenAnalysisLink = { + id: 'tl-3', ...FIXTURE_STAMPS, analysisId: 'old-uuid', status: 'approved', @@ -166,6 +169,7 @@ describe('writeGloss', () => { it('produces exactly one approved link after writing a gloss over an orphaned link', () => { const orphanLink: TokenAnalysisLink = { + id: 'tl-4', ...FIXTURE_STAMPS, analysisId: 'old-uuid', status: 'approved', @@ -538,6 +542,7 @@ describe('find-or-create on write (dedupe)', () => { expect(linksOf(store, 'tok-1')).toEqual([ { + id: 'tok-1:ta-imported:suggested', analysisId: 'ta-imported', createdAt: FIXTURE_STAMPS.createdAt, updatedAt: WRITE, @@ -1506,6 +1511,40 @@ describe('phrase payload sharing', () => { ]); }); }); + + describe('picking among identical payloads', () => { + const OLDER = '2025-06-01T00:00:00.000Z'; + const OLD_NOT: PhraseAnalysis = { ...NOT, id: 'pa-old', createdAt: OLDER }; + const NEW_NOT: PhraseAnalysis = { ...NOT, id: 'pa-new' }; + const OLD_BARE: PhraseAnalysis = { ...BARE, id: 'pa-old', createdAt: OLDER }; + const NEW_BARE: PhraseAnalysis = { ...BARE, id: 'pa-new' }; + + it.each([ + ['listed first', [OLD_BARE, NEW_BARE]], + ['listed last', [NEW_BARE, OLD_BARE]], + ])('shares the earliest-created payload when it is %s', (_, payloads) => { + const store = phraseStore(payloads, []); + + store.dispatch(createPhrase(NE_PAS)); + + const [link] = store.getState().analysis.analysis.phraseAnalysisLinks; + expect(link.analysisId).toBe('pa-old'); + }); + + it.each([ + ['listed first', [OLD_NOT, NEW_NOT]], + ['listed last', [NEW_NOT, OLD_NOT]], + ])('folds an edited payload into the earliest-created twin when it is %s', (_, payloads) => { + const store = phraseStore( + [...payloads, { ...BARE, id: 'pa-edited' }], + [occurrence('occ-1', 'pa-edited', ['tok-ne', 'tok-pas'])], + ); + + store.dispatch(writePhraseGloss({ phraseId: 'occ-1', value: 'not' })); + + expect(payloadOf(store, 'occ-1')?.id).toBe('pa-old'); + }); + }); }); /** @@ -1523,6 +1562,7 @@ function makeAnalysisWithSegment( segmentAnalyses: [analysis], segmentAnalysisLinks: [ link ?? { + id: 'sl-1', ...FIXTURE_STAMPS, analysisId: analysis.id, status: 'approved', @@ -1546,6 +1586,7 @@ describe('writeSegmentFreeTranslation', () => { }); expect(segmentAnalysisLinks).toEqual([ { + id: expect.any(String), analysisId: segmentAnalyses[0].id, createdAt: segmentAnalyses[0].createdAt, updatedAt: segmentAnalyses[0].updatedAt, @@ -1666,6 +1707,7 @@ describe('writeSegmentFreeTranslation', () => { it('repairs an orphaned approved link and creates a fresh analysis', () => { const orphanLink: SegmentAnalysisLink = { + id: 'sl-2', ...FIXTURE_STAMPS, analysisId: 'old-uuid', status: 'approved', @@ -1697,7 +1739,7 @@ function segmentTranslation( ): { analysis: SegmentAnalysis; link: SegmentAnalysisLink } { return { analysis: { ...FIXTURE_STAMPS, id, surfaceText: 'old text', freeTranslation }, - link: { ...FIXTURE_STAMPS, analysisId: id, status, segmentId }, + link: { id: `${segmentId}:${id}`, ...FIXTURE_STAMPS, analysisId: id, status, segmentId }, }; } @@ -2027,7 +2069,13 @@ describe('selectSegmentFreeTranslation', () => { analysis: { ...emptyAnalysis(), segmentAnalysisLinks: [ - { ...FIXTURE_STAMPS, analysisId: 'gone', status: 'approved', segmentId: 'seg-1' }, + { + id: 'sl-4', + ...FIXTURE_STAMPS, + analysisId: 'gone', + status: 'approved', + segmentId: 'seg-1', + }, ], }, analysisLanguage: 'und', @@ -2105,7 +2153,13 @@ describe('selectFreeTranslationsBySegment', () => { analysis: { ...emptyAnalysis(), segmentAnalysisLinks: [ - { ...FIXTURE_STAMPS, analysisId: 'gone', status: 'approved', segmentId: 'seg-1' }, + { + id: 'sl-5', + ...FIXTURE_STAMPS, + analysisId: 'gone', + status: 'approved', + segmentId: 'seg-1', + }, ], }, analysisLanguage: 'und', @@ -2214,12 +2268,14 @@ describe('writeMorphemes', () => { tokenAnalyses: [single, split], tokenAnalysisLinks: [ { + id: 'tl-5', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'run' }, }, { + id: 'tl-6', ...FIXTURE_STAMPS, analysisId: 'ta-2', status: 'approved', @@ -2240,6 +2296,7 @@ describe('writeMorphemes', () => { it('removes an orphaned approved link and creates a fresh analysis', () => { const orphanLink: TokenAnalysisLink = { + id: 'tl-7', ...FIXTURE_STAMPS, analysisId: 'old-uuid', status: 'approved', @@ -2475,12 +2532,14 @@ describe('deleteMorphemes', () => { tokenAnalyses: [withBreakdown, glossOnly], tokenAnalysisLinks: [ { + id: 'tl-8', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'run' }, }, { + id: 'tl-9', ...FIXTURE_STAMPS, analysisId: 'ta-2', status: 'approved', @@ -2604,6 +2663,7 @@ describe('deleteMorphemes', () => { it('repairs an orphaned approved link by removing it', () => { const orphanLink: TokenAnalysisLink = { + id: 'tl-10', ...FIXTURE_STAMPS, analysisId: 'missing-uuid', status: 'approved', @@ -2707,6 +2767,7 @@ describe('writeMorphemeGloss', () => { it('repairs an orphaned approved link by removing it', () => { const orphanLink: TokenAnalysisLink = { + id: 'tl-11', ...FIXTURE_STAMPS, analysisId: 'missing-uuid', status: 'approved', @@ -2772,12 +2833,14 @@ describe('writeMorphemeGloss', () => { tokenAnalyses: [ta1, ta2], tokenAnalysisLinks: [ { + id: 'tl-12', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'cats' }, }, { + id: 'tl-13', ...FIXTURE_STAMPS, analysisId: 'ta-2', status: 'approved', @@ -2826,12 +2889,14 @@ describe('writeMorphemeGloss', () => { tokenAnalyses: [ta1, ta2], tokenAnalysisLinks: [ { + id: 'tl-14', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'cats' }, }, { + id: 'tl-15', ...FIXTURE_STAMPS, analysisId: 'ta-2', status: 'approved', @@ -2933,12 +2998,14 @@ describe('selectPoolIndex', () => { // "logos" — so the pool frequency is the count of distinct approving tokens. const tokenAnalysisLinks: TokenAnalysisLink[] = [ { + id: 'tl-16', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'logos' }, }, { + id: 'tl-17', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -2966,6 +3033,7 @@ describe('selectPoolIndex', () => { }; const tokenAnalysisLinks: TokenAnalysisLink[] = [ { + id: 'tl-18', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'suggested', @@ -3222,24 +3290,28 @@ describe('selectSuggestionAfterClearing', () => { }; const tokenAnalysisLinks: TokenAnalysisLink[] = [ { + id: 'tl-19', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r1', surfaceText: 'bank' }, }, { + id: 'tl-20', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r2', surfaceText: 'bank' }, }, { + id: 'tl-21', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r3', surfaceText: 'bank' }, }, { + id: 'tl-22', ...FIXTURE_STAMPS, analysisId: 'ta-fin', status: 'approved', @@ -3286,6 +3358,7 @@ describe('selectSuggestionAfterClearing', () => { tokenAnalysisLinks: [ ...pool.tokenAnalysisLinks, { + id: 'tl-23', ...FIXTURE_STAMPS, analysisId: 'ta-fin', status: 'rejected', @@ -3396,18 +3469,21 @@ describe('approveAnalysisForToken', () => { }; const tokenAnalysisLinks: TokenAnalysisLink[] = [ { + id: 'tl-24', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r1', surfaceText: 'bank' }, }, { + id: 'tl-25', ...FIXTURE_STAMPS, analysisId: 'ta-river', status: 'approved', token: { tokenRef: 'r2', surfaceText: 'bank' }, }, { + id: 'tl-26', ...FIXTURE_STAMPS, analysisId: 'ta-fin', status: 'approved', @@ -3483,12 +3559,14 @@ describe('approveAnalysisForToken', () => { tokenAnalyses: [approved, other], tokenAnalysisLinks: [ { + id: 'tl-27', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'logos' }, }, { + id: 'tl-28', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -3539,6 +3617,7 @@ describe('approveAnalysisForToken', () => { tokenAnalyses: [approved, other], tokenAnalysisLinks: [ { + id: 'tl-29', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -3578,6 +3657,7 @@ describe('approveAnalysisForToken', () => { tokenAnalyses: [approved], tokenAnalysisLinks: [ { + id: 'tl-30', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -3655,6 +3735,7 @@ describe('approveAnalysisForToken', () => { expect(linksOf(store, 'tok-1')).toEqual([ { + id: 'tok-1:ta-imported:suggested', analysisId: 'ta-imported', createdAt: FIXTURE_STAMPS.createdAt, updatedAt: APPROVAL, @@ -4914,6 +4995,7 @@ describe('analysis-keyed reducers', () => { ...overrides, }; const links: TokenAnalysisLink[] = ['tok-1', 'tok-2'].map((tokenRef) => ({ + id: `link-${tokenRef}`, ...FIXTURE_STAMPS, analysisId: shared.id, status: 'approved', @@ -4947,6 +5029,7 @@ describe('analysis-keyed reducers', () => { ]; const links: TokenAnalysisLink[] = [ { + id: 'link-source', ...FIXTURE_STAMPS, analysisId: 'ta-source', status: sourceStatus, @@ -4954,6 +5037,7 @@ describe('analysis-keyed reducers', () => { ...provenance.source, }, { + id: 'link-target', ...FIXTURE_STAMPS, analysisId: 'ta-target', status: targetStatus, @@ -5438,6 +5522,7 @@ describe('analysis-keyed reducers', () => { ], tokenAnalysisLinks: [ { + id: 'tl-33', ...FIXTURE_STAMPS, analysisId: 'ta-unused', status: 'candidate', @@ -5556,6 +5641,7 @@ describe('analysis-keyed reducers', () => { return override ? { ...payload, ...override } : payload; }); const links: TokenAnalysisLink[] = payloads.map((ta, index) => ({ + id: `link-${index}`, ...FIXTURE_STAMPS, analysisId: ta.id, status: 'approved', @@ -6186,7 +6272,7 @@ describe('analysis-keyed reducers', () => { ]); }); - it('rates a link the collapse raised by the approval it supersedes', () => { + it("keeps the approval's confidence on the link a collapse leaves", () => { const store = makeBothLinkedStore('approved', 'candidate', 'tok-1', { source: { confidence: 'high' }, target: { confidence: 'low' }, @@ -6205,7 +6291,7 @@ describe('analysis-keyed reducers', () => { ]); }); - it('drops a raised link confidence when the approval it supersedes carried none', () => { + it('leaves the link a collapse keeps unrated when the approval carried no confidence', () => { const store = makeBothLinkedStore('approved', 'candidate', 'tok-1', { target: { confidence: 'low' }, }); @@ -6401,6 +6487,7 @@ describe('analysis-keyed reducers', () => { tokenAnalysisLinks: [ ...state.analysis.tokenAnalysisLinks, { + id: 'tl-34', ...FIXTURE_STAMPS, analysisId: 'ta-shared', status: 'candidate', @@ -6528,6 +6615,7 @@ describe('analysis-keyed reducers', () => { tokenAnalyses: [unapproved], tokenAnalysisLinks: [ { + id: 'tl-35', ...FIXTURE_STAMPS, analysisId: unapproved.id, status: 'candidate', @@ -6570,12 +6658,14 @@ describe('analysis-keyed reducers', () => { tokenAnalyses: [unused, approved], tokenAnalysisLinks: [ { + id: 'tl-36', ...FIXTURE_STAMPS, analysisId: unused.id, status: 'candidate', token: { tokenRef: 'tok-1', surfaceText: 'word' }, }, { + id: 'tl-37', ...FIXTURE_STAMPS, analysisId: approved.id, status: 'approved', @@ -6606,12 +6696,15 @@ describe('analysis-keyed reducers', () => { surfaceText: 'word', gloss: { und: 'first' }, }; - const duplicated: TokenAnalysisLink[] = ['tok-1', 'tok-1', 'tok-2'].map((tokenRef) => ({ - ...FIXTURE_STAMPS, - analysisId: shared.id, - status: 'approved', - token: { tokenRef, surfaceText: 'word' }, - })); + const duplicated: TokenAnalysisLink[] = ['tok-1', 'tok-1', 'tok-2'].map( + (tokenRef, index) => ({ + id: `link-${index}`, + ...FIXTURE_STAMPS, + analysisId: shared.id, + status: 'approved', + token: { tokenRef, surfaceText: 'word' }, + }), + ); const store = createAnalysisStore({ analysis: { analysis: { @@ -6644,6 +6737,7 @@ describe('analysis-keyed reducers', () => { ], tokenAnalysisLinks: (['candidate', 'rejected', 'stale', 'suggested'] as const).map( (status, i) => ({ + id: `link-${i}`, ...FIXTURE_STAMPS, analysisId: 'ta-mixed', status, @@ -6674,7 +6768,8 @@ describe('analysis-keyed reducers', () => { tokenAnalyses: [ { ...FIXTURE_STAMPS, id: 'ta-dup', surfaceText: 'word', gloss: { und: 'first' } }, ], - tokenAnalysisLinks: ['tok-1', 'tok-1'].map((tokenRef) => ({ + tokenAnalysisLinks: ['tok-1', 'tok-1'].map((tokenRef, index) => ({ + id: `link-${index}`, ...FIXTURE_STAMPS, analysisId: 'ta-dup', status: 'candidate' as const, @@ -6702,12 +6797,14 @@ describe('analysis-keyed reducers', () => { ], tokenAnalysisLinks: [ { + id: 'tl-38', ...FIXTURE_STAMPS, analysisId: 'ta-both', status: 'approved', token: { tokenRef: 'tok-1', surfaceText: 'word' }, }, { + id: 'tl-39', ...FIXTURE_STAMPS, analysisId: 'ta-both', status: 'candidate', @@ -6882,3 +6979,512 @@ describe('setAnalysisLanguage', () => { expect(selectApprovedGloss(store.getState().analysis, 'tok-1')).toBe('hello'); }); }); + +describe('link ids', () => { + function linksAt(store: AnalysisStore, tokenRef: string): TokenAnalysisLink[] { + return store + .getState() + .analysis.analysis.tokenAnalysisLinks.filter((l) => l.token.tokenRef === tokenRef); + } + + it('gives each link a gloss creates an id of its own, though the links share a payload', () => { + const store = createAnalysisStore(); + + store.dispatch(writeGloss('tok-1', 'word', 'hello')); + store.dispatch(writeGloss('tok-2', 'word', 'hello')); + + const [first, second] = store.getState().analysis.analysis.tokenAnalysisLinks; + expect(first.analysisId).toBe(second.analysisId); + expect(first.id).toEqual(expect.any(String)); + expect(second.id).toEqual(expect.any(String)); + expect(first.id).not.toBe(second.id); + }); + + it('gives the link a breakdown creates an id', () => { + const store = createAnalysisStore(); + + store.dispatch(writeMorphemes('tok-1', 'cats', ['cat', '-s'], 'en')); + + expect(linksAt(store, 'tok-1')).toEqual([expect.objectContaining({ id: expect.any(String) })]); + }); + + it('gives the link an approval creates an id', () => { + const imported = logos('ta-imported', 'word'); + const store = createAnalysisStore(tokenState([imported], [])); + + store.dispatch( + approveAnalysisForToken({ tokenRef: 'tok-1', surfaceText: 'logos', analysisId: imported.id }), + ); + + expect(linksAt(store, 'tok-1')).toEqual([expect.objectContaining({ id: expect.any(String) })]); + }); + + it('gives the link a re-applied stale analysis creates an id', () => { + const stranded = logos('ta-stranded', 'word'); + const store = createAnalysisStore( + tokenState([stranded], [makeLink(stranded, 'tok-old', 'stale')]), + ); + + store.dispatch( + reapplyStaleAnalysis({ + analysisId: stranded.id, + staleTokenRef: 'tok-old', + tokenRef: 'tok-new', + surfaceText: 'logos', + }), + ); + + expect(linksAt(store, 'tok-new')).toEqual([ + expect.objectContaining({ id: expect.any(String) }), + ]); + }); + + it('gives the link a free translation creates an id apart from its payload', () => { + const store = createAnalysisStore(); + + store.dispatch(writeSegmentFreeTranslation('seg-1', 'In the beginning', 'au commencement')); + + const [link] = store.getState().analysis.analysis.segmentAnalysisLinks; + expect(link.id).toEqual(expect.any(String)); + expect(link.id).not.toBe(link.analysisId); + }); + + it('keeps the link id when a promotion moves the token to another payload', () => { + const held = logos('ta-held', 'word'); + const promoted = logos('ta-promoted', 'speech'); + const store = createAnalysisStore( + tokenState([held, promoted], [makeLink(held, 'tok-1', 'approved')]), + ); + + store.dispatch( + approveAnalysisForToken({ tokenRef: 'tok-1', surfaceText: 'logos', analysisId: promoted.id }), + ); + + expect(linksAt(store, 'tok-1')).toEqual([ + expect.objectContaining({ id: 'tok-1:ta-held:approved', analysisId: 'ta-promoted' }), + ]); + }); +}); + +describe('approval ties', () => { + const EARLIER = '2026-01-01T00:00:00.000Z'; + const LATER = '2026-02-01T00:00:00.000Z'; + const ALPHA = logos('ta-alpha', 'alpha'); + const BETA = logos('ta-beta', 'beta'); + + function approval(id: string, analysisId: string, updatedAt: string): TokenAnalysisLink { + return { + id, + analysisId, + createdAt: EARLIER, + updatedAt, + status: 'approved', + token: { tokenRef: 'tok-1', surfaceText: 'logos' }, + }; + } + + /** Each token payload's English gloss, by id. */ + function glossesOf(store: AnalysisStore): Map { + return new Map( + store.getState().analysis.analysis.tokenAnalyses.map((ta) => [ta.id, ta.gloss?.en]), + ); + } + + const LATEST_BETA: [string, TokenAnalysisLink[]][] = [ + [ + 'listed first', + [approval('l-beta', 'ta-beta', LATER), approval('l-alpha', 'ta-alpha', EARLIER)], + ], + [ + 'listed last', + [approval('l-alpha', 'ta-alpha', EARLIER), approval('l-beta', 'ta-beta', LATER)], + ], + ]; + + it.each(LATEST_BETA)('reads the latest-written approval when it is %s', (_, links) => { + const store = createAnalysisStore(tokenState([ALPHA, BETA], links)); + + expect(selectApprovedGloss(store.getState().analysis, 'tok-1')).toBe('beta'); + }); + + it.each(LATEST_BETA)('writes onto the latest-written approval when it is %s', (_, links) => { + const store = createAnalysisStore(tokenState([ALPHA, BETA], links)); + + store.dispatch(writeGloss('tok-1', 'logos', 'gamma')); + + expect(glossesOf(store)).toEqual( + new Map([ + ['ta-alpha', 'alpha'], + ['ta-beta', 'gamma'], + ]), + ); + }); + + it.each([ + ['listed first', [approval('l-a', 'ta-alpha', EARLIER), approval('l-b', 'ta-beta', EARLIER)]], + ['listed last', [approval('l-b', 'ta-beta', EARLIER), approval('l-a', 'ta-alpha', EARLIER)]], + ])('reads the smaller-id approval of two written together when it is %s', (_, links) => { + const store = createAnalysisStore(tokenState([ALPHA, BETA], links)); + + expect(selectApprovedGloss(store.getState().analysis, 'tok-1')).toBe('alpha'); + }); + + it('reads past a later approval naming no payload to one that names a payload', () => { + const store = createAnalysisStore( + tokenState( + [ALPHA], + [approval('l-alpha', 'ta-alpha', EARLIER), approval('l-gone', 'ta-gone', LATER)], + ), + ); + + expect(selectApprovedGloss(store.getState().analysis, 'tok-1')).toBe('alpha'); + }); + + it('drops a later approval naming no payload and writes onto the one that names a payload', () => { + const store = createAnalysisStore( + tokenState( + [ALPHA], + [approval('l-alpha', 'ta-alpha', EARLIER), approval('l-gone', 'ta-gone', LATER)], + ), + ); + + store.dispatch(writeGloss('tok-1', 'logos', 'gamma')); + + const { tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalysisLinks.map((l) => l.id)).toEqual(['l-alpha']); + expect(glossesOf(store)).toEqual(new Map([['ta-alpha', 'gamma']])); + }); + + describe('on a segment', () => { + const SA_ALPHA: SegmentAnalysis = { + ...FIXTURE_STAMPS, + id: 'sa-alpha', + surfaceText: 'text', + freeTranslation: { und: 'alpha' }, + }; + const SA_BETA: SegmentAnalysis = { + ...SA_ALPHA, + id: 'sa-beta', + freeTranslation: { und: 'beta' }, + }; + + function segmentApproval( + id: string, + analysisId: string, + updatedAt: string, + ): SegmentAnalysisLink { + return { + id, + analysisId, + createdAt: EARLIER, + updatedAt, + status: 'approved', + segmentId: 'seg-1', + }; + } + + function segmentTieStore(links: SegmentAnalysisLink[]) { + return createAnalysisStore({ + analysis: { + analysis: { + ...emptyAnalysis(), + segmentAnalyses: [SA_ALPHA, SA_BETA], + segmentAnalysisLinks: links, + }, + analysisLanguage: 'und', + }, + }); + } + + const LATEST_SA_BETA: [string, SegmentAnalysisLink[]][] = [ + [ + 'listed first', + [ + segmentApproval('l-beta', 'sa-beta', LATER), + segmentApproval('l-alpha', 'sa-alpha', EARLIER), + ], + ], + [ + 'listed last', + [ + segmentApproval('l-alpha', 'sa-alpha', EARLIER), + segmentApproval('l-beta', 'sa-beta', LATER), + ], + ], + ]; + + it.each(LATEST_SA_BETA)('reads the latest-written approval when it is %s', (_, links) => { + const store = segmentTieStore(links); + + expect(selectSegmentFreeTranslation(store.getState().analysis, 'seg-1')).toBe('beta'); + }); + + it.each(LATEST_SA_BETA)('lists the latest-written approval when it is %s', (_, links) => { + const store = segmentTieStore(links); + + expect(selectFreeTranslationsBySegment(store.getState().analysis)).toEqual( + new Map([['seg-1', 'beta']]), + ); + }); + + it.each(LATEST_SA_BETA)('writes onto the latest-written approval when it is %s', (_, links) => { + const store = segmentTieStore(links); + + store.dispatch(writeSegmentFreeTranslation('seg-1', 'text', 'gamma')); + + const { segmentAnalyses } = store.getState().analysis.analysis; + expect(new Map(segmentAnalyses.map((sa) => [sa.id, sa.freeTranslation?.und]))).toEqual( + new Map([ + ['sa-alpha', 'alpha'], + ['sa-beta', 'gamma'], + ]), + ); + }); + + it('drops a later approval naming no payload and writes onto the one that names a payload', () => { + const store = segmentTieStore([ + segmentApproval('l-alpha', 'sa-alpha', EARLIER), + segmentApproval('l-gone', 'sa-gone', LATER), + ]); + + store.dispatch(writeSegmentFreeTranslation('seg-1', 'text', 'gamma')); + + const { segmentAnalyses, segmentAnalysisLinks } = store.getState().analysis.analysis; + expect(segmentAnalysisLinks.map((l) => l.id)).toEqual(['l-alpha']); + expect(segmentAnalyses.find((sa) => sa.id === 'sa-alpha')?.freeTranslation?.und).toBe( + 'gamma', + ); + }); + }); +}); + +describe('merge survivors', () => { + const OLDER = '2025-06-01T00:00:00.000Z'; + + function twin(id: string, createdAt = FIXTURE_STAMPS.createdAt): TokenAnalysis { + return { ...logos(id, 'word'), createdAt }; + } + + /** The payload `tokenRef`'s approved link names. */ + function approvedIdAt(store: AnalysisStore, tokenRef: string): string | undefined { + return store + .getState() + .analysis.analysis.tokenAnalysisLinks.find( + (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, + )?.analysisId; + } + + /** A token glossed apart from twin payloads, each approved by a token of its own. */ + function storeWithTwins(twins: TokenAnalysis[]) { + const edited = logos('ta-edited', 'speech'); + return createAnalysisStore( + tokenState( + [edited, ...twins], + [ + makeLink(edited, 'tok-1', 'approved'), + ...twins.map((ta, i) => makeLink(ta, `tok-twin-${i}`, 'approved')), + ], + ), + ); + } + + it.each([ + ['listed first', [twin('ta-old', OLDER), twin('ta-new')]], + ['listed last', [twin('ta-new'), twin('ta-old', OLDER)]], + ])('folds an edited payload into the earliest-created twin when it is %s', (_, twins) => { + const store = storeWithTwins(twins); + + store.dispatch(writeGloss('tok-1', 'logos', 'word')); + + expect(approvedIdAt(store, 'tok-1')).toBe('ta-old'); + }); + + it.each([ + ['listed first', [twin('ta-a'), twin('ta-b')]], + ['listed last', [twin('ta-b'), twin('ta-a')]], + ])( + 'folds an edited payload into the smaller-id twin of two created together when it is %s', + (_, twins) => { + const store = storeWithTwins(twins); + + store.dispatch(writeGloss('tok-1', 'logos', 'word')); + + expect(approvedIdAt(store, 'tok-1')).toBe('ta-a'); + }, + ); + + it.each([ + ['listed first', [twin('ta-old', OLDER), twin('ta-new')]], + ['listed last', [twin('ta-new'), twin('ta-old', OLDER)]], + ])('adopts the earliest-created twin on a first gloss when it is %s', (_, twins) => { + const store = createAnalysisStore(tokenState(twins, [])); + + store.dispatch(writeGloss('tok-1', 'logos', 'word')); + + expect(approvedIdAt(store, 'tok-1')).toBe('ta-old'); + }); + + it.each([ + ['listed first', [twin('ta-old', OLDER), twin('ta-new')]], + ['listed last', [twin('ta-new'), twin('ta-old', OLDER)]], + ])('re-applies a stale analysis onto the earliest-created twin when it is %s', (_, twins) => { + const stranded: TokenAnalysis = { ...logos('ta-stranded', 'word'), surfaceText: 'logoi' }; + const store = createAnalysisStore( + tokenState([stranded, ...twins], [makeLink(stranded, 'tok-1', 'stale')]), + ); + + store.dispatch( + reapplyStaleAnalysis({ + analysisId: stranded.id, + staleTokenRef: 'tok-1', + tokenRef: 'tok-1', + surfaceText: 'logos', + }), + ); + + expect(approvedIdAt(store, 'tok-1')).toBe('ta-old'); + }); +}); + +describe('links a token is left holding one of', () => { + const EARLIER = '2026-01-01T00:00:00.000Z'; + const LATER = '2026-02-01T00:00:00.000Z'; + const SOURCE = logos('ta-source', 'first'); + const TARGET = logos('ta-target', 'second'); + + function link( + id: string, + analysisId: string, + status: AssignmentStatus, + updatedAt: string, + ): TokenAnalysisLink { + return { + id, + analysisId, + createdAt: EARLIER, + updatedAt, + status, + token: { tokenRef: 'tok-1', surfaceText: 'logos' }, + }; + } + + it.each([ + [ + 'listed first', + [ + link('l-approved', 'ta-target', 'approved', EARLIER), + link('l-candidate', 'ta-source', 'candidate', LATER), + ], + ], + [ + 'listed last', + [ + link('l-candidate', 'ta-source', 'candidate', LATER), + link('l-approved', 'ta-target', 'approved', EARLIER), + ], + ], + ])('keeps the approved link a collapse leaves beside another when it is %s', (_, links) => { + const store = createAnalysisStore(tokenState([SOURCE, TARGET], links)); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-source', value: 'second' })); + + expect(store.getState().analysis.analysis.tokenAnalysisLinks).toEqual([ + expect.objectContaining({ id: 'l-approved', status: 'approved' }), + ]); + }); + + it.each([ + [ + 'listed first', + [ + link('l-later', 'ta-source', 'candidate', LATER), + link('l-earlier', 'ta-target', 'candidate', EARLIER), + ], + ], + [ + 'listed last', + [ + link('l-earlier', 'ta-target', 'candidate', EARLIER), + link('l-later', 'ta-source', 'candidate', LATER), + ], + ], + ])( + 'keeps the latest-written of two unapproved links a collapse leaves when it is %s', + (_, links) => { + const store = createAnalysisStore(tokenState([SOURCE, TARGET], links)); + + store.dispatch(writeAnalysisGloss({ analysisId: 'ta-source', value: 'second' })); + + expect(store.getState().analysis.analysis.tokenAnalysisLinks.map((l) => l.id)).toEqual([ + 'l-later', + ]); + }, + ); + + it.each([ + [ + 'listed first', + [ + link('l-later', 'ta-target', 'suggested', LATER), + link('l-earlier', 'ta-target', 'suggested', EARLIER), + ], + ], + [ + 'listed last', + [ + link('l-earlier', 'ta-target', 'suggested', EARLIER), + link('l-later', 'ta-target', 'suggested', LATER), + ], + ], + ])('approves the latest-written of two held links when it is %s', (_, links) => { + const store = createAnalysisStore(tokenState([TARGET], links)); + + store.dispatch( + approveAnalysisForToken({ tokenRef: 'tok-1', surfaceText: 'logos', analysisId: 'ta-target' }), + ); + + const statuses = new Map( + store.getState().analysis.analysis.tokenAnalysisLinks.map((l) => [l.id, l.status]), + ); + expect(statuses).toEqual( + new Map([ + ['l-later', 'approved'], + ['l-earlier', 'suggested'], + ]), + ); + }); + + it.each([ + [ + 'listed first', + [ + { ...link('l-later', 'ta-target', 'stale', LATER), confidence: 'high' as const }, + { ...link('l-earlier', 'ta-target', 'stale', EARLIER), confidence: 'low' as const }, + ], + ], + [ + 'listed last', + [ + { ...link('l-earlier', 'ta-target', 'stale', EARLIER), confidence: 'low' as const }, + { ...link('l-later', 'ta-target', 'stale', LATER), confidence: 'high' as const }, + ], + ], + ])( + 'rates a re-application by the latest-written of two stale links at one place when it is %s', + (_, links) => { + const store = createAnalysisStore(tokenState([TARGET], links)); + + store.dispatch( + reapplyStaleAnalysis({ + analysisId: 'ta-target', + staleTokenRef: 'tok-1', + tokenRef: 'tok-1', + surfaceText: 'logos', + }), + ); + + const { tokenAnalysisLinks } = store.getState().analysis.analysis; + expect(tokenAnalysisLinks.find((l) => l.status === 'approved')?.confidence).toBe('high'); + }, + ); +}); diff --git a/src/__tests__/types/type-guards.test.ts b/src/__tests__/types/type-guards.test.ts index f28fa69b5..3f680a4f4 100644 --- a/src/__tests__/types/type-guards.test.ts +++ b/src/__tests__/types/type-guards.test.ts @@ -2,6 +2,7 @@ import type { AssignmentStatus, PhraseAnalysisLink, TextAnalysis } from 'interli import { FIXTURE_STAMPS } from '../test-helpers'; import { emptyAnalysis } from '../../types/empty-factories'; import { + hasLinkWithoutId, isPt9ImportProvenance, isTextAnalysis, validateTextAnalysis, @@ -91,6 +92,28 @@ describe('isTextAnalysis', () => { expect(isTextAnalysis({ ...emptyAnalysis(), phraseAnalysisLinks: [link] })).toBe(false); }); + it('rejects a token link that carries no id', () => { + expect( + isTextAnalysis({ + ...emptyAnalysis(), + tokenAnalysisLinks: [ + { ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: TOKEN }, + ], + }), + ).toBe(false); + }); + + it('rejects a segment link that carries no id', () => { + expect( + isTextAnalysis({ + ...emptyAnalysis(), + segmentAnalysisLinks: [ + { ...FIXTURE_STAMPS, analysisId: 'sa-1', status: 'approved', segmentId: 'GEN 1:1' }, + ], + }), + ).toBe(false); + }); + it('rejects a reference whose authority is not a string', () => { expect( isTextAnalysis(analysisWithTokenFields({ glossSenseRef: { authority: 7, senseId: 's-1' } })), @@ -139,7 +162,13 @@ describe('isTextAnalysis', () => { isTextAnalysis({ ...emptyAnalysis(), tokenAnalysisLinks: [ - { analysisId: 'ta-1', status: 'approved', confidence: 'certain', token: TOKEN }, + { + id: 'tl-1', + analysisId: 'ta-1', + status: 'approved', + confidence: 'certain', + token: TOKEN, + }, ], }), ).toBe(false); @@ -155,7 +184,13 @@ function tokenLink( status: AssignmentStatus = 'approved', tokenRef = TOKEN.tokenRef, ) { - return { ...FIXTURE_STAMPS, analysisId, status, token: { ...TOKEN, tokenRef } }; + return { + ...FIXTURE_STAMPS, + id: `${tokenRef}:${analysisId}:${status}`, + analysisId, + status, + token: { ...TOKEN, tokenRef }, + }; } function phraseLink( @@ -307,6 +342,38 @@ describe('validateTextAnalysis', () => { ).toEqual([{ kind: 'duplicateLinkId', layer: 'phrase', count: 1, sample: ['occ-1'] }]); }); + it('reports two token links that share an id', () => { + expect( + validateTextAnalysis( + tokenLayer({ + tokenAnalyses: [tokenAnalysis('ta-1')], + tokenAnalysisLinks: [ + { ...tokenLink('ta-1', 'approved', 'GEN 1:1:0'), id: 'tl-1' }, + { ...tokenLink('ta-1', 'approved', 'GEN 1:1:5'), id: 'tl-1' }, + ], + }), + ), + ).toEqual([{ kind: 'duplicateLinkId', layer: 'token', count: 1, sample: ['tl-1'] }]); + }); + + it('reports two segment links that share an id', () => { + const segmentLink = (segmentId: string) => ({ + ...FIXTURE_STAMPS, + id: 'sl-1', + analysisId: 'sa-1', + status: 'suggested' as const, + segmentId, + }); + expect( + validateTextAnalysis( + tokenLayer({ + segmentAnalyses: [tokenAnalysis('sa-1')], + segmentAnalysisLinks: [segmentLink('GEN 1:1'), segmentLink('GEN 1:2')], + }), + ), + ).toEqual([{ kind: 'duplicateLinkId', layer: 'segment', count: 1, sample: ['sl-1'] }]); + }); + it('does not report phrase occurrences sharing a payload under distinct link ids', () => { expect( validateTextAnalysis( @@ -348,7 +415,13 @@ describe('validateTextAnalysis', () => { validateTextAnalysis( tokenLayer({ segmentAnalysisLinks: [ - { ...FIXTURE_STAMPS, analysisId: 'sa-1', status: 'approved', segmentId: 'GEN 1:1' }, + { + id: 'sl-1', + ...FIXTURE_STAMPS, + analysisId: 'sa-1', + status: 'approved', + segmentId: 'GEN 1:1', + }, ], }), ), @@ -385,6 +458,57 @@ describe('validateTextAnalysis', () => { }); }); +describe('hasLinkWithoutId', () => { + it('finds a token link with no id', () => { + expect( + hasLinkWithoutId({ + ...emptyAnalysis(), + tokenAnalysisLinks: [ + { ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', token: TOKEN }, + ], + }), + ).toBe(true); + }); + + it('finds a segment link with no id', () => { + expect( + hasLinkWithoutId({ + ...emptyAnalysis(), + segmentAnalysisLinks: [ + { ...FIXTURE_STAMPS, analysisId: 'sa-1', status: 'approved', segmentId: 'GEN 1:1' }, + ], + }), + ).toBe(true); + }); + + it('finds none where every link has an id', () => { + expect( + hasLinkWithoutId( + tokenLayer({ + tokenAnalysisLinks: [tokenLink('ta-1')], + segmentAnalysisLinks: [ + { + ...FIXTURE_STAMPS, + id: 'sl-1', + analysisId: 'sa-1', + status: 'approved', + segmentId: 'GEN 1:1', + }, + ], + }), + ), + ).toBe(false); + }); + + it('finds none in a value that is no analysis', () => { + expect(hasLinkWithoutId(undefined)).toBe(false); + }); + + it('finds none in an analysis missing its link collections', () => { + expect(hasLinkWithoutId({})).toBe(false); + }); +}); + const PROVENANCE = { fileHashes: { 'Lexicon.xml': 'aaaa1111' }, importedAt: '2026-08-01T00:00:00.000Z', diff --git a/src/__tests__/utils/analysis-book.test.ts b/src/__tests__/utils/analysis-book.test.ts index cdbf73db0..3ce9ba685 100644 --- a/src/__tests__/utils/analysis-book.test.ts +++ b/src/__tests__/utils/analysis-book.test.ts @@ -28,6 +28,7 @@ function mkTokenAnalysis(id: string, surfaceText = id): TokenAnalysis { /** Creates a {@link TokenAnalysisLink} joining a token ref to an analysis id. */ function mkTokenLink(analysisId: string, tokenRef: string): TokenAnalysisLink { return { + id: `${tokenRef}:${analysisId}`, ...FIXTURE_STAMPS, analysisId, status: 'approved', @@ -45,7 +46,13 @@ function mkSegmentAnalysis(id: string, surfaceText = id): SegmentAnalysis { /** Creates a {@link SegmentAnalysisLink} joining a segment id to an analysis id. */ function mkSegmentLink(analysisId: string, segmentId: string): SegmentAnalysisLink { - return { ...FIXTURE_STAMPS, analysisId, status: 'approved', segmentId }; + return { + id: `${segmentId}:${analysisId}`, + ...FIXTURE_STAMPS, + analysisId, + status: 'approved', + segmentId, + }; } describe('bookOfRef', () => { diff --git a/src/__tests__/utils/analysis-query.test.ts b/src/__tests__/utils/analysis-query.test.ts index 9a9a445e8..775aec75e 100644 --- a/src/__tests__/utils/analysis-query.test.ts +++ b/src/__tests__/utils/analysis-query.test.ts @@ -35,7 +35,13 @@ function link( tokenRef: string, status: AssignmentStatus = 'approved', ): TokenAnalysisLink { - return { ...FIXTURE_STAMPS, analysisId, status, token: { tokenRef, surfaceText: 'word' } }; + return { + id: `${tokenRef}:${analysisId}:${status}`, + ...FIXTURE_STAMPS, + analysisId, + status, + token: { tokenRef, surfaceText: 'word' }, + }; } describe('buildCatalogRows', () => { diff --git a/src/__tests__/utils/concordance.test.ts b/src/__tests__/utils/concordance.test.ts index 5ade052a9..7e64da917 100644 --- a/src/__tests__/utils/concordance.test.ts +++ b/src/__tests__/utils/concordance.test.ts @@ -27,7 +27,13 @@ function link( surfaceText = 'word', status: TokenAnalysisLink['status'] = 'approved', ): TokenAnalysisLink { - return { ...FIXTURE_STAMPS, analysisId, status, token: { tokenRef, surfaceText } }; + return { + id: `${tokenRef}:${analysisId}:${status}`, + ...FIXTURE_STAMPS, + analysisId, + status, + token: { tokenRef, surfaceText }, + }; } /** Builds a token analysis glossed in English. */ @@ -189,13 +195,19 @@ describe('approvedAnalysisOf', () => { expect(approvedAnalysisOf(approved, entry, first)).toBe('a1'); }); - it('keeps the first approval for a token approved twice', () => { - const approved = approvedAnalysisByToken([ - link('a1', 'GEN 1:1:0', 'dark'), - link('a2', 'GEN 1:1:0', 'dark'), - ]); + it.each([ + ['listed first', ['a2', 'a1']], + ['listed last', ['a1', 'a2']], + ])('keeps the latest-written approval for a token approved twice when it is %s', (_, order) => { + const links = { + a1: link('a1', 'GEN 1:1:0', 'dark'), + a2: { ...link('a2', 'GEN 1:1:0', 'dark'), updatedAt: '2026-02-01T00:00:00.000Z' }, + }; + const approved = approvedAnalysisByToken( + order.map((id) => (id === 'a1' ? links.a1 : links.a2)), + ); - expect(approvedAnalysisOf(approved, entry, first)).toBe('a1'); + expect(approvedAnalysisOf(approved, entry, first)).toBe('a2'); }); }); diff --git a/src/__tests__/utils/pick-rules.test.ts b/src/__tests__/utils/pick-rules.test.ts new file mode 100644 index 000000000..7235966cb --- /dev/null +++ b/src/__tests__/utils/pick-rules.test.ts @@ -0,0 +1,81 @@ +/// + +import type { TokenAnalysis, TokenAnalysisLink } from 'interlinearizer'; +import { FIXTURE_STAMPS } from '../test-helpers'; +import { preferredLink, preferredPayload, prefersLink } from '../../utils/pick-rules'; + +const EARLIER = '2026-01-01T00:00:00.000Z'; +const LATER = '2026-02-01T00:00:00.000Z'; + +function link(id: string, updatedAt: string): TokenAnalysisLink { + return { + id, + analysisId: 'ta-1', + createdAt: EARLIER, + updatedAt, + status: 'approved', + token: { tokenRef: 'GEN 1:1:0', surfaceText: 'word' }, + }; +} + +function payload(id: string, createdAt: string): TokenAnalysis { + return { ...FIXTURE_STAMPS, id, createdAt, surfaceText: 'word' }; +} + +describe('prefersLink', () => { + it('prefers the later-written link', () => { + expect(prefersLink(link('b', LATER), link('a', EARLIER))).toBe(true); + }); + + it('does not prefer the earlier-written link', () => { + expect(prefersLink(link('a', EARLIER), link('b', LATER))).toBe(false); + }); + + it('prefers the smaller id between links written together', () => { + expect(prefersLink(link('a', EARLIER), link('b', EARLIER))).toBe(true); + }); + + it('does not prefer the larger id between links written together', () => { + expect(prefersLink(link('b', EARLIER), link('a', EARLIER))).toBe(false); + }); +}); + +describe('preferredLink', () => { + it('picks the latest-written link whatever the order', () => { + const latest = link('z', LATER); + + expect(preferredLink([latest, link('a', EARLIER)])).toBe(latest); + expect(preferredLink([link('a', EARLIER), latest])).toBe(latest); + }); + + it('picks the smallest id among links written together whatever the order', () => { + const smallest = link('a', EARLIER); + + expect(preferredLink([link('b', EARLIER), smallest])).toBe(smallest); + expect(preferredLink([smallest, link('b', EARLIER)])).toBe(smallest); + }); + + it('picks nothing from no links', () => { + expect(preferredLink([])).toBeUndefined(); + }); +}); + +describe('preferredPayload', () => { + it('picks the earliest-created payload whatever the order', () => { + const earliest = payload('z', EARLIER); + + expect(preferredPayload([payload('a', LATER), earliest])).toBe(earliest); + expect(preferredPayload([earliest, payload('a', LATER)])).toBe(earliest); + }); + + it('picks the smallest id among payloads created together whatever the order', () => { + const smallest = payload('a', EARLIER); + + expect(preferredPayload([payload('b', EARLIER), smallest])).toBe(smallest); + expect(preferredPayload([smallest, payload('b', EARLIER)])).toBe(smallest); + }); + + it('picks nothing from no payloads', () => { + expect(preferredPayload([])).toBeUndefined(); + }); +}); diff --git a/src/__tests__/utils/project-analysis-summary.test.ts b/src/__tests__/utils/project-analysis-summary.test.ts index a9b3db64e..ba9b4422f 100644 --- a/src/__tests__/utils/project-analysis-summary.test.ts +++ b/src/__tests__/utils/project-analysis-summary.test.ts @@ -12,6 +12,7 @@ function mkTokenAnalysis(id: string): TokenAnalysis { /** Creates a {@link TokenAnalysisLink} joining a token ref to an analysis id. */ function mkTokenLink(analysisId: string, tokenRef: string): TokenAnalysisLink { return { + id: `${tokenRef}:${analysisId}`, ...FIXTURE_STAMPS, analysisId, status: 'approved', @@ -51,7 +52,13 @@ describe('summarizeAnalysis', () => { tokenAnalysisLinks: [mkTokenLink('a1', 'GEN 1:1:0')], segmentAnalyses: [{ ...FIXTURE_STAMPS, id: 's1', surfaceText: 's1' }], segmentAnalysisLinks: [ - { ...FIXTURE_STAMPS, analysisId: 's1', status: 'approved', segmentId: 'LEV 1:1' }, + { + id: 'sl-1', + ...FIXTURE_STAMPS, + analysisId: 's1', + status: 'approved', + segmentId: 'LEV 1:1', + }, ], phraseAnalyses: [{ ...FIXTURE_STAMPS, id: 'p1', surfaceText: 'p1' }], phraseAnalysisLinks: [makePhraseLink('p1', ['NUM 1:1:0'])], diff --git a/src/__tests__/utils/reanchor-analysis.test.ts b/src/__tests__/utils/reanchor-analysis.test.ts index 8755e75c4..cfda8670c 100644 --- a/src/__tests__/utils/reanchor-analysis.test.ts +++ b/src/__tests__/utils/reanchor-analysis.test.ts @@ -34,6 +34,7 @@ function makeTokenLink( analysisId = 'ta-1', ): TokenAnalysisLink { return { + id: `${tokenRef}:${analysisId}`, analysisId, ...FIXTURE_STAMPS, status: 'approved', @@ -57,7 +58,7 @@ function analysisWithSegmentLink(segmentId: string, surfaceText: string): TextAn { id: 'sa-1', ...FIXTURE_STAMPS, surfaceText, freeTranslation: { en: 'a translation' } }, ], segmentAnalysisLinks: [ - { analysisId: 'sa-1', ...FIXTURE_STAMPS, status: 'approved', segmentId }, + { id: 'sl-1', analysisId: 'sa-1', ...FIXTURE_STAMPS, status: 'approved', segmentId }, ], }; } diff --git a/src/__tests__/utils/reanchor-draft.test.ts b/src/__tests__/utils/reanchor-draft.test.ts index 195e72970..82639ef55 100644 --- a/src/__tests__/utils/reanchor-draft.test.ts +++ b/src/__tests__/utils/reanchor-draft.test.ts @@ -16,6 +16,7 @@ function glossedOn(text: string, surfaceText: string, gloss: string): TextAnalys tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText, gloss: { und: gloss } }], tokenAnalysisLinks: [ { + id: 'tl-1', ...FIXTURE_STAMPS, analysisId: 'ta-1', status: 'approved', @@ -64,6 +65,7 @@ describe('reanchorDraftToBook', () => { segmentAnalyses: [{ id: 'sa-1', ...FIXTURE_STAMPS, surfaceText: 'beta' }], segmentAnalysisLinks: [ { + id: 'sl-1', analysisId: 'sa-1', ...FIXTURE_STAMPS, status: 'approved' as const, @@ -113,6 +115,7 @@ describe('reanchorDraftToMissingBook', () => { segmentAnalyses: [{ ...FIXTURE_STAMPS, id: 'sa-1', surfaceText: 'alpha beta' }], segmentAnalysisLinks: [ { + id: 'sl-1', ...FIXTURE_STAMPS, analysisId: 'sa-1', status: 'approved' as const, diff --git a/src/converters/pt9/analysisMerger.ts b/src/converters/pt9/analysisMerger.ts index f1700c7c5..65a120cc6 100644 --- a/src/converters/pt9/analysisMerger.ts +++ b/src/converters/pt9/analysisMerger.ts @@ -347,6 +347,7 @@ export function mergeLanguageAnalyses(args: { ...(morphemes !== undefined && { morphemes }), }); tokenAnalysisLinks.push({ + id: `pt9:tl:${entry.tokenRef}:${count}`, analysisId: id, createdAt: importedAt, updatedAt: importedAt, diff --git a/src/services/projectStorage.ts b/src/services/projectStorage.ts index df4b5884e..39b5b1151 100644 --- a/src/services/projectStorage.ts +++ b/src/services/projectStorage.ts @@ -9,7 +9,12 @@ import type { import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; import { mergeAnalyses, splitAnalysisByBook } from '../utils/analysis-book'; import { assertSupportedModelVersion, CURRENT_MODEL_VERSION } from '../types/model-version'; -import { isDraftProject, isTextAnalysis, validateTextAnalysis } from '../types/type-guards'; +import { + hasLinkWithoutId, + isDraftProject, + isTextAnalysis, + validateTextAnalysis, +} from '../types/type-guards'; import { perfTime, perfTimeAsync, perfTimeAsyncExcluding } from '../utils/perf-marks'; import { draftAnalysisKey, draftJournalKey, draftKey, projectKey } from './storageKeys'; @@ -324,12 +329,19 @@ async function readRecord(token: ExecutionToken, key: string): Promise & { * which only damage outside the extension produces, falls back to the read time and is logged, * since that date is invented rather than recovered. * + * A project holding a link with no id loads with an empty analysis, and is logged. + * * A record written by a newer build is refused outright, so it survives on disk in the shape that * build wrote. * @@ -810,9 +824,13 @@ function parseProject(raw: string, id: string): InterlinearProject { `Interlinearizer: project ${id} was stored without a creation time; dating it by the read time`, ); const createdAt = stored.createdAt ?? new Date().toISOString(); - reportAnalysisViolations(stored.analysis, `project ${id} on load`); + const lacksLinkIds = hasLinkWithoutId(stored.analysis); + if (lacksLinkIds) + logger.warn(`Interlinearizer: project ${id} has links without ids; loading it empty`); + else reportAnalysisViolations(stored.analysis, `project ${id} on load`); return { ...stored, + ...(lacksLinkIds && { analysis: emptyAnalysis() }), createdAt, updatedAt: stored.updatedAt ?? createdAt, }; @@ -1029,7 +1047,8 @@ export async function hasDraft(token: ExecutionToken, sourceProjectId: string): * * A book whose shard is missing or unusable loads as empty, and {@link saveDraft} then leaves that * shard and its manifest entry alone rather than treating the book as wiped, until a save carries - * analyses for that book again. + * analyses for that book again. A shard holding a link with no id is unusable, and a draft storing + * its analysis inline with such a link loads as a fresh empty draft; either is logged. * * A save interrupted before its envelope landed leaves shards the stored manifest does not name. A * book first analyzed in that save still reopens with its work rather than empty. @@ -1051,6 +1070,17 @@ export async function getDraft( // Refuse a newer build's draft before validating it: a later shape need not satisfy this // build's guard, and failing validation is what resets the draft to empty. assertSupportedModelVersion(parsed, `draft for source project ${sourceProjectId}`); + if ( + !!parsed && + typeof parsed === 'object' && + 'analysis' in parsed && + hasLinkWithoutId(parsed.analysis) + ) { + logger.warn( + `Interlinearizer: draft for source project ${sourceProjectId} has links without ids; loading it empty`, + ); + return emptyDraft(sourceProjectId); + } if (!isDraftProject(parsed) || parsed.sourceProjectId !== sourceProjectId) { logger.warn('Interlinearizer: stored draft failed validation; resetting to empty draft'); return emptyDraft(sourceProjectId); diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 1a059f913..2f3864656 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -22,6 +22,7 @@ import { import { buildCatalogRows, type HeadingPlacement } from '../utils/analysis-query'; import type { StaleFreeTranslation } from '../utils/stale-free-translations'; import { isEmptyMultiString } from '../utils/multi-string'; +import { preferredLink, preferredPayload, prefersLink } from '../utils/pick-rules'; import { buildPoolIndex, deriveTokenSuggestion, @@ -57,6 +58,8 @@ interface WriteGlossPayload { value: string; /** Pre-generated UUID for a new `TokenAnalysis` record, produced by the `prepare` callback. */ id: string; + /** Pre-generated UUID for a new `TokenAnalysisLink`, produced by the `prepare` callback. */ + linkId: string; /** ISO 8601 stamp for the records this write touches, produced by the `prepare` callback. */ now: string; } @@ -133,6 +136,8 @@ interface WriteSegmentFreeTranslationPayload { value: string; /** Pre-generated UUID for a new `SegmentAnalysis` record, produced by the `prepare` callback. */ id: string; + /** Pre-generated UUID for a new `SegmentAnalysisLink`, produced by the `prepare` callback. */ + linkId: string; /** ISO 8601 stamp for the records this write touches, produced by the `prepare` callback. */ now: string; /** A stale translation the value was typed over, which the write adopts when the segment has none. */ @@ -163,6 +168,8 @@ interface ReapplyStaleAnalysisPayload { surfaceText: string; /** Pre-generated UUID for a copy under the token's spelling, produced by the `prepare` callback. */ id: string; + /** Pre-generated UUID for a new approved link, produced by the `prepare` callback. */ + linkId: string; /** ISO 8601 stamp for the records this write touches, produced by the `prepare` callback. */ now: string; } @@ -277,8 +284,10 @@ function editablePhraseAnalysis( * edit left the payload unique. */ function mergeIntoIdenticalPhrase(state: AnalysisState, analysis: PhraseAnalysis): void { - const other = state.analysis.phraseAnalyses.find( - (pa) => pa.id !== analysis.id && phraseAnalysesAreIdentical(pa, analysis), + const other = preferredPayload( + state.analysis.phraseAnalyses.filter( + (pa) => pa.id !== analysis.id && phraseAnalysesAreIdentical(pa, analysis), + ), ); if (!other) return; state.analysis.phraseAnalysisLinks.forEach((l) => { @@ -310,28 +319,29 @@ function reshapePhrase( } /** - * Finds the approved `SegmentAnalysisLink` for `segmentId` together with the `SegmentAnalysis` it - * references. When the approved link references a missing analysis (an orphaned link from - * corruption or a migration), the link is removed from the draft — so the corruption never persists - * or accumulates duplicate approved links — and `undefined` is returned as if no approved link - * existed. Mirrors {@link resolveApprovedAnalysis} for the segment layer. + * Finds the approved `SegmentAnalysisLink` for `segmentId` — of several, the one the read selectors + * surface — together with the `SegmentAnalysis` it references. Approved links referencing a missing + * analysis (orphaned links from corruption or a migration) are removed from the draft first — so + * the corruption never persists or accumulates duplicate approved links — and the pick falls to the + * rest. Mirrors {@link resolveApprovedAnalysis} for the segment layer. */ function resolveApprovedSegmentAnalysis( state: AnalysisState, segmentId: string, ): { link: SegmentAnalysisLink; analysis: SegmentAnalysis } | undefined { - const link = state.analysis.segmentAnalysisLinks.find( + const analysisOf = (link: SegmentAnalysisLink) => + state.analysis.segmentAnalyses.find((sa) => sa.id === link.analysisId); + const approved = state.analysis.segmentAnalysisLinks.filter( (l) => l.status === 'approved' && l.segmentId === segmentId, ); - if (!link) return undefined; - const analysis = state.analysis.segmentAnalyses.find((sa) => sa.id === link.analysisId); - if (!analysis) { + const orphans = approved.filter((l) => !analysisOf(l)); + if (orphans.length > 0) state.analysis.segmentAnalysisLinks = state.analysis.segmentAnalysisLinks.filter( - (l) => l !== link, + (l) => !orphans.includes(l), ); - return undefined; - } - return { link, analysis }; + const link = preferredLink(approved.filter((l) => !orphans.includes(l))); + const analysis = link && analysisOf(link); + return link && analysis ? { link, analysis } : undefined; } /** @@ -419,29 +429,29 @@ function approveStaleSegmentAnalysis( /** * Finds the approved `TokenAnalysisLink` for `tokenRef` together with the `TokenAnalysis` it - * references. Uses `findLast` so that, in the data-model-violating case of multiple approved links - * for one token, the reducer mutates the same link the read selectors surface (both - * {@link selectApprovedIdByTokenRef} and the phrase-link selectors are last-wins); otherwise a write - * would land on a different link than the one those selectors read and appear to no-op. When the - * approved link references a missing analysis (an orphaned link from corruption or a migration), - * the link is removed from the draft — so the corruption never persists or accumulates duplicate - * approved links — and `undefined` is returned as if no approved link existed. Every token-analysis - * reducer resolves through this helper so they all repair orphaned links the same way. + * references. In the data-model-violating case of multiple approved links for one token it picks + * the one the read selectors surface, so a write never lands on a different link than the one they + * read and appears to no-op. Approved links referencing a missing analysis (orphaned links from + * corruption or a migration) are removed from the draft first — so the corruption never persists or + * accumulates duplicate approved links — and the pick falls to the rest. */ function resolveApprovedAnalysis( state: AnalysisState, tokenRef: string, ): { link: TokenAnalysisLink; analysis: TokenAnalysis } | undefined { - const link = state.analysis.tokenAnalysisLinks.findLast( + const analysisOf = (link: TokenAnalysisLink) => + state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); + const approved = state.analysis.tokenAnalysisLinks.filter( (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, ); - if (!link) return undefined; - const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === link.analysisId); - if (!analysis) { - state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter((l) => l !== link); - return undefined; - } - return { link, analysis }; + const orphans = approved.filter((l) => !analysisOf(l)); + if (orphans.length > 0) + state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( + (l) => !orphans.includes(l), + ); + const link = preferredLink(approved.filter((l) => !orphans.includes(l))); + const analysis = link && analysisOf(link); + return link && analysis ? { link, analysis } : undefined; } /** @@ -458,8 +468,11 @@ function approveHeldLink( surfaceText: string, now: string, ): TokenAnalysisLink | undefined { - const held = state.analysis.tokenAnalysisLinks.findLast( - (l) => l.status !== 'approved' && l.token.tokenRef === tokenRef && l.analysisId === analysisId, + const held = preferredLink( + state.analysis.tokenAnalysisLinks.filter( + (l) => + l.status !== 'approved' && l.token.tokenRef === tokenRef && l.analysisId === analysisId, + ), ); if (!held) return undefined; held.status = 'approved'; @@ -471,7 +484,8 @@ function approveHeldLink( /** * Approves the stored payload `analysisId` for a token in place of any approval it held, keeping * one approved link per token, and reclaims the payload that approval named once nothing links it. - * A no-op for an id naming no stored payload. + * A token holding no link to the payload gets one under `linkId`. A no-op for an id naming no + * stored payload. */ function approveStoredAnalysis( state: AnalysisState, @@ -479,6 +493,7 @@ function approveStoredAnalysis( surfaceText: string, analysisId: string, now: string, + linkId: string, ): void { // Approve only a payload that actually exists: an unknown id would point an approved link at // nothing, which the read selectors then have to repair as an orphan. @@ -500,6 +515,7 @@ function approveStoredAnalysis( resolved.link.updatedAt = now; } else { state.analysis.tokenAnalysisLinks.push({ + id: linkId, analysisId, createdAt: now, updatedAt: now, @@ -544,11 +560,15 @@ function appendApprovedAnalysis( analysis: TokenAnalysis, tokenRef: string, now: string, + linkId: string, ): void { - const existing = state.analysis.tokenAnalyses.find((ta) => analysesAreIdentical(ta, analysis)); + const existing = preferredPayload( + state.analysis.tokenAnalyses.filter((ta) => analysesAreIdentical(ta, analysis)), + ); if (!existing) state.analysis.tokenAnalyses.push(analysis); else if (approveHeldLink(state, tokenRef, existing.id, analysis.surfaceText, now)) return; state.analysis.tokenAnalysisLinks.push({ + id: linkId, analysisId: existing?.id ?? analysis.id, createdAt: now, updatedAt: now, @@ -627,31 +647,29 @@ function forkSharedAnalysis( } /** - * Leaves a token holding one link to `analysisId` where it held several, keeping the link the read - * selectors surface and approving it if any it supersedes was, so the "at most one approved link - * per token" invariant holds across the collapse. + * Leaves a token holding one link to `analysisId` where it held several, so the "at most one + * approved link per token" invariant holds across the collapse. The token keeps its approved link + * over any other, and which of two of the same standing it keeps never depends on list order. * - * The survivor keeps the token's first annotation date and, where it is raised to approved, the - * superseded approval's `confidence` — so a collapse retires a link without retiring what it - * recorded. + * The survivor keeps the token's first annotation date. */ -function coalesceLinksPerToken(state: AnalysisState, analysisId: string, now: string): void { - const survivorByToken = new Map(); +function coalesceLinksPerToken(state: AnalysisState, analysisId: string): void { + const keptByToken = new Map(); state.analysis.tokenAnalysisLinks.forEach((l) => { if (l.analysisId !== analysisId) return; - const superseded = survivorByToken.get(l.token.tokenRef); - if (superseded) { - if (superseded.createdAt < l.createdAt) l.createdAt = superseded.createdAt; - if (superseded.status === 'approved' && l.status !== 'approved') { - l.status = 'approved'; - l.updatedAt = now; - if (superseded.confidence === undefined) delete l.confidence; - else l.confidence = superseded.confidence; - } + const held = keptByToken.get(l.token.tokenRef); + if (!held) { + keptByToken.set(l.token.tokenRef, { link: l, createdAt: l.createdAt }); + return; } - survivorByToken.set(l.token.tokenRef, l); + const approvalDiffers = (l.status === 'approved') !== (held.link.status === 'approved'); + if (approvalDiffers ? l.status === 'approved' : prefersLink(l, held.link)) held.link = l; + if (l.createdAt < held.createdAt) held.createdAt = l.createdAt; + }); + keptByToken.forEach(({ link, createdAt }) => { + link.createdAt = createdAt; }); - const survivors = new Set(survivorByToken.values()); + const survivors = new Set([...keptByToken.values()].map(({ link }) => link)); state.analysis.tokenAnalysisLinks = state.analysis.tokenAnalysisLinks.filter( (l) => l.analysisId !== analysisId || survivors.has(l), ); @@ -686,8 +704,10 @@ function mergeIntoIdenticalPayload( now: string, settleProvenance = false, ): void { - const other = state.analysis.tokenAnalyses.find( - (ta) => ta !== analysis && analysesAreIdentical(ta, analysis), + const other = preferredPayload( + state.analysis.tokenAnalyses.filter( + (ta) => ta !== analysis && analysesAreIdentical(ta, analysis), + ), ); if (!other) return; if (settleProvenance) { @@ -702,7 +722,7 @@ function mergeIntoIdenticalPayload( state.analysis.tokenAnalysisLinks.forEach((l) => { if (l.analysisId === analysis.id) l.analysisId = other.id; }); - coalesceLinksPerToken(state, other.id, now); + coalesceLinksPerToken(state, other.id); state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter((ta) => ta !== analysis); state.lastCollapseSurvivorId = other.id; } @@ -918,12 +938,19 @@ const analysisSlice = createSlice({ reducers: { writeGloss: { /** - * Generates a UUID for a potential new `TokenAnalysis` record before the action reaches the - * reducer, keeping the reducer pure. + * Generates UUIDs for a potential new `TokenAnalysis` record and link before the action + * reaches the reducer, keeping the reducer pure. */ prepare(tokenRef: string, surfaceText: string, value: string) { return { - payload: { tokenRef, surfaceText, value, id: crypto.randomUUID(), now: nowIso() }, + payload: { + tokenRef, + surfaceText, + value, + id: crypto.randomUUID(), + linkId: crypto.randomUUID(), + now: nowIso(), + }, }; }, /** @@ -953,7 +980,7 @@ const analysisSlice = createSlice({ * gloss text, the sense belonging to the analysis rather than to the emptied language. */ reducer(state, action: PayloadAction) { - const { tokenRef, surfaceText, value, id, now } = action.payload; + const { tokenRef, surfaceText, value, id, linkId, now } = action.payload; const lang = state.analysisLanguage; const isBlank = value.trim() === ''; @@ -999,14 +1026,15 @@ const analysisSlice = createSlice({ { id, createdAt: now, updatedAt: now, surfaceText, gloss: { [lang]: value } }, tokenRef, now, + linkId, ); }, }, writeMorphemes: { /** - * Generates UUIDs for new morpheme records and a potential new `TokenAnalysis` before the - * action reaches the reducer. The token's own writing system is stored on each morpheme as - * the writing system of its form. + * Generates UUIDs for new morpheme records and a potential new `TokenAnalysis` and link + * before the action reaches the reducer. The token's own writing system is stored on each + * morpheme as the writing system of its form. */ prepare(tokenRef: string, surfaceText: string, forms: string[], writingSystem: string) { return { @@ -1015,6 +1043,7 @@ const analysisSlice = createSlice({ surfaceText, writingSystem, analysisId: crypto.randomUUID(), + linkId: crypto.randomUUID(), morphemes: forms.map((form) => ({ id: crypto.randomUUID(), form })), now: nowIso(), }, @@ -1043,11 +1072,13 @@ const analysisSlice = createSlice({ surfaceText: string; writingSystem: string; analysisId: string; + linkId: string; morphemes: Array<{ id: string; form: string }>; now: string; }>, ) { - const { tokenRef, surfaceText, writingSystem, analysisId, morphemes, now } = action.payload; + const { tokenRef, surfaceText, writingSystem, analysisId, linkId, morphemes, now } = + action.payload; const resolved = resolveApprovedAnalysis(state, tokenRef); if (resolved) { @@ -1084,6 +1115,7 @@ const analysisSlice = createSlice({ }, tokenRef, now, + linkId, ); }, }, @@ -1441,7 +1473,7 @@ const analysisSlice = createSlice({ l.updatedAt = now; } }); - coalesceLinksPerToken(state, survivorAnalysisId, now); + coalesceLinksPerToken(state, survivorAnalysisId); state.analysis.tokenAnalyses = state.analysis.tokenAnalyses.filter( (ta) => !merged.has(ta.id), ); @@ -1486,9 +1518,12 @@ const analysisSlice = createSlice({ * rather than the moment this token accepted it. */ approveAnalysisForToken: { - /** Reads the clock before the action reaches the reducer, keeping the reducer pure. */ + /** + * Generates the id a new link would take and reads the clock before the action reaches the + * reducer, keeping the reducer pure. + */ prepare(arg: { tokenRef: string; surfaceText: string; analysisId: string }) { - return { payload: { ...arg, now: nowIso() } }; + return { payload: { ...arg, linkId: crypto.randomUUID(), now: nowIso() } }; }, reducer( state, @@ -1496,11 +1531,12 @@ const analysisSlice = createSlice({ tokenRef: string; surfaceText: string; analysisId: string; + linkId: string; now: string; }>, ) { - const { tokenRef, surfaceText, analysisId, now } = action.payload; - approveStoredAnalysis(state, tokenRef, surfaceText, analysisId, now); + const { tokenRef, surfaceText, analysisId, linkId, now } = action.payload; + approveStoredAnalysis(state, tokenRef, surfaceText, analysisId, now, linkId); }, }, /** @@ -1533,19 +1569,27 @@ const analysisSlice = createSlice({ * to `analysisId` sits at `staleTokenRef`. */ reapplyStaleAnalysis: { - /** Generates the id a copy would take and reads the clock, keeping the reducer pure. */ + /** + * Generates the ids a copy and a new link would take and reads the clock, keeping the reducer + * pure. + */ prepare(arg: { analysisId: string; staleTokenRef: string; tokenRef: string; surfaceText: string; }) { - return { payload: { ...arg, id: crypto.randomUUID(), now: nowIso() } }; + return { + payload: { ...arg, id: crypto.randomUUID(), linkId: crypto.randomUUID(), now: nowIso() }, + }; }, reducer(state, action: PayloadAction) { - const { analysisId, staleTokenRef, tokenRef, surfaceText, id, now } = action.payload; - const stale = state.analysis.tokenAnalysisLinks.find((l) => - isStaleLinkAt(l, analysisId, staleTokenRef), + const { analysisId, staleTokenRef, tokenRef, surfaceText, id, linkId, now } = + action.payload; + const stale = preferredLink( + state.analysis.tokenAnalysisLinks.filter((l) => + isStaleLinkAt(l, analysisId, staleTokenRef), + ), ); const analysis = state.analysis.tokenAnalyses.find((ta) => ta.id === analysisId); if (!stale || !analysis) return; @@ -1571,8 +1615,8 @@ const analysisSlice = createSlice({ ) copy.morphemes = [{ ...only, form: surfaceText }]; if (rest.length > 0) copy.morphemesStale = true; - const identical = state.analysis.tokenAnalyses.find((ta) => - analysesAreIdentical(ta, copy), + const identical = preferredPayload( + state.analysis.tokenAnalyses.filter((ta) => analysesAreIdentical(ta, copy)), ); if (identical) approvedId = identical.id; else { @@ -1580,11 +1624,9 @@ const analysisSlice = createSlice({ approvedId = id; } } - approveStoredAnalysis(state, tokenRef, surfaceText, approvedId, now); + approveStoredAnalysis(state, tokenRef, surfaceText, approvedId, now, linkId); if (staleTokenRef === tokenRef) { - const approved = state.analysis.tokenAnalysisLinks.find( - (l) => l.status === 'approved' && l.token.tokenRef === tokenRef, - ); + const approved = resolveApprovedAnalysis(state, tokenRef)?.link; /* v8 ignore next -- the approval just made always stands */ if (approved) { if (stale.createdAt < approved.createdAt) approved.createdAt = stale.createdAt; @@ -1622,8 +1664,8 @@ const analysisSlice = createSlice({ updatedAt: now, surfaceText: phraseSurfaceText(tokens), }; - const existing = state.analysis.phraseAnalyses.find((pa) => - phraseAnalysesAreIdentical(pa, created), + const existing = preferredPayload( + state.analysis.phraseAnalyses.filter((pa) => phraseAnalysesAreIdentical(pa, created)), ); if (!existing) state.analysis.phraseAnalyses.push(created); state.analysis.phraseAnalysisLinks.push({ @@ -1754,8 +1796,8 @@ const analysisSlice = createSlice({ }, writeSegmentFreeTranslation: { /** - * Generates a UUID for a potential new `SegmentAnalysis` record before the action reaches the - * reducer, keeping the reducer pure. + * Generates UUIDs for a potential new `SegmentAnalysis` record and link before the action + * reaches the reducer, keeping the reducer pure. */ prepare( segmentId: string, @@ -1769,6 +1811,7 @@ const analysisSlice = createSlice({ surfaceText, value, id: crypto.randomUUID(), + linkId: crypto.randomUUID(), now: nowIso(), ...(adoptStaleAnalysisId !== undefined && { adoptStaleAnalysisId }), }, @@ -1792,7 +1835,8 @@ const analysisSlice = createSlice({ * it stale, and never creates a record. */ reducer(state, action: PayloadAction) { - const { segmentId, surfaceText, value, id, now, adoptStaleAnalysisId } = action.payload; + const { segmentId, surfaceText, value, id, linkId, now, adoptStaleAnalysisId } = + action.payload; const lang = state.analysisLanguage; const isBlank = value.trim() === ''; @@ -1836,6 +1880,7 @@ const analysisSlice = createSlice({ freeTranslation: { [lang]: value }, }; const newLink: SegmentAnalysisLink = { + id: linkId, analysisId: id, createdAt: now, updatedAt: now, @@ -1936,19 +1981,25 @@ const selectAnalysisById = createSelector( /** * Memoized selector that builds a `Map` from `tokenRef` to the approved `TokenAnalysis.id` for that - * token. Only the last approved link per token is indexed (the data model allows at most one). - * Recomputes only when `tokenAnalysisLinks` or `tokenAnalyses` change reference. + * token. Of several approved links on one token (the data model allows at most one), the one an + * approval tie resolves to is indexed; links naming no payload are left out. Recomputes only when + * `tokenAnalysisLinks` or `tokenAnalyses` change reference. */ const selectApprovedIdByTokenRef = createSelector( selectTokenAnalysisLinks, selectAnalysisById, - (links, byId) => - links.reduce((index, link) => { - if (link.status === 'approved' && byId.has(link.analysisId)) { - index.set(link.token.tokenRef, link.analysisId); - } - return index; - }, new Map()), + (links, byId) => { + const winners = new Map(); + const index = new Map(); + links.forEach((link) => { + if (link.status !== 'approved' || !byId.has(link.analysisId)) return; + const held = winners.get(link.token.tokenRef); + if (held && !prefersLink(link, held)) return; + winners.set(link.token.tokenRef, link); + index.set(link.token.tokenRef, link.analysisId); + }); + return index; + }, ); const NO_PENDING: readonly TokenAnalysis[] = []; @@ -2412,27 +2463,46 @@ export function selectPhraseGloss(state: AnalysisState, phraseId: string): strin return pa?.gloss?.[state.analysisLanguage] ?? ''; } +/** Projects `segmentAnalysisLinks` out of `AnalysisState` for use as a `createSelector` input. */ +const selectSegmentAnalysisLinks = (state: AnalysisState) => state.analysis.segmentAnalysisLinks; + +/** Projects `segmentAnalyses` out of `AnalysisState` for use as a `createSelector` input. */ +const selectSegmentAnalyses = (state: AnalysisState) => state.analysis.segmentAnalyses; + +/** + * Memoized selector mapping each segment to its approved `SegmentAnalysis`, the one an approval tie + * resolves to where several are approved. A link naming no payload is left out (read-only here; the + * orphan is repaired on the next write). + */ +const selectApprovedSegmentAnalysisBySegment = createSelector( + selectSegmentAnalysisLinks, + selectSegmentAnalyses, + (links, analyses) => { + const byId = new Map(analyses.map((a) => [a.id, a])); + const winners = new Map(); + const bySegment = new Map(); + links.forEach((link) => { + const analysis = byId.get(link.analysisId); + if (link.status !== 'approved' || !analysis) return; + const held = winners.get(link.segmentId); + if (held && !prefersLink(link, held)) return; + winners.set(link.segmentId, link); + bySegment.set(link.segmentId, analysis); + }); + return bySegment; + }, +); + /** * Returns the approved free-translation string for the given segment in the active analysis * language, or `''` when no approved analysis exists or it has no free translation for the active - * language. An approved link referencing a missing analysis is treated as absent (read-only here; - * the orphan is repaired on the next write). + * language. */ export function selectSegmentFreeTranslation(state: AnalysisState, segmentId: string): string { - const link = state.analysis.segmentAnalysisLinks.find( - (l) => l.status === 'approved' && l.segmentId === segmentId, - ); - if (!link) return ''; - const sa = state.analysis.segmentAnalyses.find((a) => a.id === link.analysisId); + const sa = selectApprovedSegmentAnalysisBySegment(state).get(segmentId); return sa?.freeTranslation?.[state.analysisLanguage] ?? ''; } -/** Projects `segmentAnalysisLinks` out of `AnalysisState` for use as a `createSelector` input. */ -const selectSegmentAnalysisLinks = (state: AnalysisState) => state.analysis.segmentAnalysisLinks; - -/** Projects `segmentAnalyses` out of `AnalysisState` for use as a `createSelector` input. */ -const selectSegmentAnalyses = (state: AnalysisState) => state.analysis.segmentAnalyses; - /** * Memoized selector returning the id of every segment holding an approved free translation, in * whatever language. @@ -2448,18 +2518,13 @@ export const selectSegmentsWithApprovedTranslation = createSelector( * active analysis language, keyed by segment id. */ export const selectFreeTranslationsBySegment = createSelector( - selectSegmentAnalysisLinks, - selectSegmentAnalyses, + selectApprovedSegmentAnalysisBySegment, selectAnalysisLanguage, - (links, analyses, language) => { - const translatedById = new Map( - analyses.map((a) => [a.id, a.freeTranslation?.[language] ?? '']), - ); + (approved, language) => { const bySegment = new Map(); - links.forEach((link) => { - if (link.status !== 'approved') return; - const text = translatedById.get(link.analysisId) ?? ''; - if (text !== '') bySegment.set(link.segmentId, text); + approved.forEach((analysis, segmentId) => { + const text = analysis.freeTranslation?.[language] ?? ''; + if (text !== '') bySegment.set(segmentId, text); }); return bySegment; }, diff --git a/src/types/interlinearizer.d.ts b/src/types/interlinearizer.d.ts index e148034a8..5904e2691 100644 --- a/src/types/interlinearizer.d.ts +++ b/src/types/interlinearizer.d.ts @@ -829,7 +829,8 @@ declare module 'interlinearizer' { * `status: 'approved'`. That linked analysis is the canonical segment-level analysis for * rendering; alternates are available to review workflows via the other statuses. Whatever * writes an approved link owns this invariant; nothing enforces it at runtime, and a breach is - * reported at the storage boundary rather than repaired. + * reported at the storage boundary rather than repaired. Reads and writes resolve a breach to + * the approved link with the latest `updatedAt`, then the smallest `id`. */ segmentAnalyses: SegmentAnalysis[]; @@ -853,7 +854,8 @@ declare module 'interlinearizer' { * available to review workflows via the other statuses (`'suggested'`, `'candidate'`, * `'rejected'`, `'stale'`). Whatever writes an approved link owns this invariant; nothing * enforces it at runtime, and a breach is reported at the storage boundary rather than - * repaired. + * repaired. Reads and writes resolve a breach to the approved link with the latest `updatedAt`, + * then the smallest `id`. */ tokenAnalyses: TokenAnalysis[]; @@ -899,6 +901,12 @@ declare module 'interlinearizer' { * fresh link `createdAt` while the payload keeps its older one. */ export interface AnalysisLink { + /** + * Unique within the owning `TextAnalysis` — identifies this link, which `analysisId` cannot, + * since targets analyzed identically share one payload. + */ + id: string; + /** The `Analysis.id` for the linked analysis payload record. */ analysisId: string; @@ -1192,12 +1200,6 @@ declare module 'interlinearizer' { /** Links one `PhraseAnalysis` payload record to one or more token snapshots. */ export interface PhraseAnalysisLink extends AnalysisLink { - /** - * Unique within the owning `TextAnalysis` — identifies this phrase occurrence, which - * `analysisId` cannot, since occurrences analyzed identically share one payload. - */ - id: string; - /** Ordered snapshots of tokens that compose this phrase (one or more). */ tokens: TokenSnapshot[]; } diff --git a/src/types/type-guards.ts b/src/types/type-guards.ts index ee9a56de8..c2777f47a 100644 --- a/src/types/type-guards.ts +++ b/src/types/type-guards.ts @@ -192,6 +192,8 @@ function isAnalysisLink(v: unknown): boolean { return ( !!v && typeof v === 'object' && + 'id' in v && + typeof v.id === 'string' && 'analysisId' in v && typeof v.analysisId === 'string' && 'status' in v && @@ -224,8 +226,6 @@ function isPhraseAnalysisLink(v: unknown): boolean { isAnalysisLink(v) && !!v && typeof v === 'object' && - 'id' in v && - typeof v.id === 'string' && 'tokens' in v && Array.isArray(v.tokens) && v.tokens.length > 0 && @@ -262,6 +262,24 @@ export function isTextAnalysis(value: unknown): value is TextAnalysis { ); } +function lacksId(link: unknown): boolean { + return !!link && typeof link === 'object' && !('id' in link); +} + +/** Whether `analysis` holds a token or segment link with no id, as one stored before links had ids. */ +export function hasLinkWithoutId(analysis: unknown): boolean { + return ( + !!analysis && + typeof analysis === 'object' && + (('tokenAnalysisLinks' in analysis && + Array.isArray(analysis.tokenAnalysisLinks) && + analysis.tokenAnalysisLinks.some(lacksId)) || + ('segmentAnalysisLinks' in analysis && + Array.isArray(analysis.segmentAnalysisLinks) && + analysis.segmentAnalysisLinks.some(lacksId))) + ); +} + /** * Validates a {@link SegmentationDelta} parsed from unknown JSON, so a malformed delta cannot * corrupt re-segmentation. @@ -313,7 +331,6 @@ export interface AnalysisViolation { /** * Which invariant was broken. `unreferencedAnalysis` is never reported for the token layer, whose * payloads describe a spelling rather than an occurrence and so may outrun the text. - * `duplicateLinkId` is reported only for the phrase layer, the one whose links carry an id. */ kind: | 'multipleApproved' @@ -410,25 +427,21 @@ function validateLayer( ), violation('unreferencedAnalysis', layer, unreferenced), violation('duplicateAnalysisId', layer, repeated(analyses.map((a) => a.id))), + violation('duplicateLinkId', layer, repeated(links.map((link) => link.id))), ].filter((v) => v !== undefined); } /** * Reports the invariant violations a structurally valid {@link TextAnalysis} can still carry: a * target with more than one `approved` link, a link whose `analysisId` names no payload, two - * payloads or two phrase occurrences sharing an id, and — for the layers whose payloads may not - * outrun the text — a payload no link references. Where {@link isTextAnalysis} asks whether the - * shape is readable, this asks whether the collections agree with each other. + * payloads or two links sharing an id, and — for the layers whose payloads may not outrun the text + * — a payload no link references. Where {@link isTextAnalysis} asks whether the shape is readable, + * this asks whether the collections agree with each other. * * Reporting is all it does: the returned violations leave the analysis untouched, so a corrupted * record stays readable and its corruption stays visible. */ export function validateTextAnalysis(analysis: TextAnalysis): AnalysisViolation[] { - const duplicatePhraseLinkIds = violation( - 'duplicateLinkId', - 'phrase', - repeated(analysis.phraseAnalysisLinks.map((link) => link.id)), - ); return [ ...validateLayer('segment', analysis.segmentAnalyses, analysis.segmentAnalysisLinks, { targetKey: (link) => link.segmentId, @@ -443,6 +456,5 @@ export function validateTextAnalysis(analysis: TextAnalysis): AnalysisViolation[ approvalKeys: (link) => link.tokens.map((t) => t.tokenRef), requireReferenced: true, }), - ...(duplicatePhraseLinkIds ? [duplicatePhraseLinkIds] : []), ]; } diff --git a/src/utils/concordance.ts b/src/utils/concordance.ts index 6ca9e8fef..9b82bd7c6 100644 --- a/src/utils/concordance.ts +++ b/src/utils/concordance.ts @@ -4,6 +4,7 @@ import type { Collator } from 'platform-bible-utils'; import { isWordToken } from '../types/type-guards'; import { normalizeSurfaceForm } from './analysis-identity'; import { multiStringText } from './multi-string'; +import { prefersLink } from './pick-rules'; /** One place in the source text where a word form appears, carried with the verse it sits in. */ export interface ConcordanceOccurrence { @@ -196,14 +197,18 @@ function approvalKey(tokenRef: string, form: string): string { /** * Gathers the approved links for lookup. A token approved to more than one analysis as the same - * form — a state no write path builds — keeps the first. + * form — a state no write path builds — keeps the one an approval tie resolves to. */ export function approvedAnalysisByToken(links: readonly TokenAnalysisLink[]): ApprovedAnalyses { + const winners = new Map(); const approved = new Map(); links.forEach((link) => { if (link.status !== 'approved') return; const key = approvalKey(link.token.tokenRef, normalizeSurfaceForm(link.token.surfaceText)); - if (!approved.has(key)) approved.set(key, link.analysisId); + const held = winners.get(key); + if (held && !prefersLink(link, held)) return; + winners.set(key, link); + approved.set(key, link.analysisId); }); return approved; } diff --git a/src/utils/pick-rules.ts b/src/utils/pick-rules.ts new file mode 100644 index 000000000..cb703c30e --- /dev/null +++ b/src/utils/pick-rules.ts @@ -0,0 +1,31 @@ +import type { Analysis, AnalysisLink } from 'interlinearizer'; + +/** + * Whether `link` beats `rival` where one link must be picked from several, as an approval tie + * picks: the later `updatedAt`, then the smaller id. + */ +export function prefersLink(link: AnalysisLink, rival: AnalysisLink): boolean { + if (link.updatedAt !== rival.updatedAt) return link.updatedAt > rival.updatedAt; + return link.id < rival.id; +} + +/** The link an approval tie picks from `links`, or `undefined` when there are none. */ +export function preferredLink(links: readonly L[]): L | undefined { + return links.reduce( + (best, link) => (!best || prefersLink(link, best) ? link : best), + undefined, + ); +} + +/** + * The payload a merge folds content-identical ones into: the earliest `createdAt`, then the + * smallest id. `undefined` when `analyses` is empty. + */ +export function preferredPayload(analyses: readonly A[]): A | undefined { + return analyses.reduce((best, analysis) => { + if (!best) return analysis; + if (analysis.createdAt !== best.createdAt) + return analysis.createdAt < best.createdAt ? analysis : best; + return analysis.id < best.id ? analysis : best; + }, undefined); +} From 0daae3955426b923ca39abf1013521999d544bc9 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Thu, 15 Oct 2026 15:51:02 -0600 Subject: [PATCH 2/4] Break concordance approval ties toward links with a payload --- .../components/ConcordanceRowView.test.tsx | 5 +- src/__tests__/services/projectStorage.test.ts | 2 +- src/__tests__/utils/concordance.test.ts | 71 +++++++++++++------ src/components/ConcordancePanel.tsx | 8 +-- src/utils/concordance.ts | 15 +++- 5 files changed, 68 insertions(+), 33 deletions(-) diff --git a/src/__tests__/components/ConcordanceRowView.test.tsx b/src/__tests__/components/ConcordanceRowView.test.tsx index c9a40a1fe..dbe9c9cda 100644 --- a/src/__tests__/components/ConcordanceRowView.test.tsx +++ b/src/__tests__/components/ConcordanceRowView.test.tsx @@ -50,7 +50,8 @@ type RowOptions = Partial<{ function renderRow(options: RowOptions = {}) { const verses = options.verses ?? [{ sid: 'GEN 1:3', text: 'Let there be light' }]; const entries = buildConcordanceEntries([indexBook(makeVerseBook(verses))], new Collator('en')); - const approvedByToken = approvedAnalysisByToken(options.links ?? []); + const analysesById = new Map((options.analyses ?? []).map((a) => [a.id, a])); + const approvedByToken = approvedAnalysisByToken(options.links ?? [], analysesById); const row = deriveConcordanceRows(entries, approvedByToken, options.currentBook ?? 'GEN').find( (r) => r.entry.form === 'light', ); @@ -59,7 +60,7 @@ function renderRow(options: RowOptions = {}) { withTooltipProvider(
    [a.id, a]))} + analysesById={analysesById} analysisLanguage="en" approvedByToken={approvedByToken} isSelected={options.isSelected ?? false} diff --git a/src/__tests__/services/projectStorage.test.ts b/src/__tests__/services/projectStorage.test.ts index c24a546c7..a3c25810f 100644 --- a/src/__tests__/services/projectStorage.test.ts +++ b/src/__tests__/services/projectStorage.test.ts @@ -1552,7 +1552,7 @@ describe('projectStorage', () => { }); ['GEN', 'EXO'].forEach((book) => { draft.analysis.tokenAnalysisLinks.push({ - id: 'tl-3', + id: `tl-${book}`, analysisId: 'shared', ...FIXTURE_STAMPS, status: 'approved', diff --git a/src/__tests__/utils/concordance.test.ts b/src/__tests__/utils/concordance.test.ts index 7e64da917..025b53d35 100644 --- a/src/__tests__/utils/concordance.test.ts +++ b/src/__tests__/utils/concordance.test.ts @@ -171,26 +171,29 @@ describe('approvedAnalysisOf', () => { const [first, second] = entry.occurrences; it('finds the analysis an occurrence is approved to', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark')], new Map()); expect(approvedAnalysisOf(approved, entry, first)).toBe('a1'); expect(approvedAnalysisOf(approved, entry, second)).toBeUndefined(); }); it('skips links of other statuses', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark', 'suggested')]); + const approved = approvedAnalysisByToken( + [link('a1', 'GEN 1:1:0', 'dark', 'suggested')], + new Map(), + ); expect(approvedAnalysisOf(approved, entry, first)).toBeUndefined(); }); it('ignores an approval recorded against a different word at the same place', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')], new Map()); expect(approvedAnalysisOf(approved, entry, first)).toBeUndefined(); }); it('accepts an approval recorded against the word in another case', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'Dark')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'Dark')], new Map()); expect(approvedAnalysisOf(approved, entry, first)).toBe('a1'); }); @@ -205,10 +208,30 @@ describe('approvedAnalysisOf', () => { }; const approved = approvedAnalysisByToken( order.map((id) => (id === 'a1' ? links.a1 : links.a2)), + new Map(), ); expect(approvedAnalysisOf(approved, entry, first)).toBe('a2'); }); + + it.each([ + ['listed first', ['gone', 'valid']], + ['listed last', ['valid', 'gone']], + ])( + 'keeps an approval with a record over a later one naming a missing record when it is %s', + (_, order) => { + const links = { + valid: link('valid', 'GEN 1:1:0', 'dark'), + gone: { ...link('gone', 'GEN 1:1:0', 'dark'), updatedAt: '2026-02-01T00:00:00.000Z' }, + }; + const approved = approvedAnalysisByToken( + order.map((id) => (id === 'valid' ? links.valid : links.gone)), + new Map([['valid', analysis('valid', 'darkness')]]), + ); + + expect(approvedAnalysisOf(approved, entry, first)).toBe('valid'); + }, + ); }); describe('deriveConcordanceRows', () => { @@ -217,11 +240,14 @@ describe('deriveConcordanceRows', () => { const entries = buildConcordanceEntries([genesis, exodus], collator); it('marks a form with every occurrence approved as analyzed', () => { - const approved = approvedAnalysisByToken([ - link('a1', 'GEN 1:1:0', 'light'), - link('a1', 'GEN 1:1:6', 'light'), - link('a1', 'EXO 1:1:0', 'light'), - ]); + const approved = approvedAnalysisByToken( + [ + link('a1', 'GEN 1:1:0', 'light'), + link('a1', 'GEN 1:1:6', 'light'), + link('a1', 'EXO 1:1:0', 'light'), + ], + new Map(), + ); expect(deriveConcordanceRows(entries, approved, 'GEN')[0]).toMatchObject({ analyzedCount: 3, @@ -230,7 +256,7 @@ describe('deriveConcordanceRows', () => { }); it('marks a form with some occurrences approved as partly analyzed', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')], new Map()); expect(deriveConcordanceRows(entries, approved, 'GEN')[0]).toMatchObject({ analyzedCount: 1, @@ -246,7 +272,7 @@ describe('deriveConcordanceRows', () => { }); it('does not count an occurrence whose token was approved as a different word', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark')], new Map()); expect(deriveConcordanceRows(entries, approved, 'GEN')[0]).toMatchObject({ analyzedCount: 0, @@ -268,11 +294,10 @@ describe('tallyAnalyses', () => { const [entry] = buildConcordanceEntries([book], collator); it('counts the occurrences each analysis covers, most-used first', () => { - const approved = approvedAnalysisByToken([ - link('rare', 'GEN 1:1:0'), - link('common', 'GEN 1:1:5'), - link('common', 'GEN 1:1:10'), - ]); + const approved = approvedAnalysisByToken( + [link('rare', 'GEN 1:1:0'), link('common', 'GEN 1:1:5'), link('common', 'GEN 1:1:10')], + new Map(), + ); const records = new Map([ ['rare', analysis('rare', 'utterance')], ['common', analysis('common', 'word')], @@ -285,10 +310,10 @@ describe('tallyAnalyses', () => { }); it('orders equally used analyses by first use', () => { - const approved = approvedAnalysisByToken([ - link('second', 'GEN 1:1:5'), - link('first', 'GEN 1:1:0'), - ]); + const approved = approvedAnalysisByToken( + [link('second', 'GEN 1:1:5'), link('first', 'GEN 1:1:0')], + new Map(), + ); const records = new Map([ ['first', analysis('first', 'one')], ['second', analysis('second', 'two')], @@ -301,20 +326,20 @@ describe('tallyAnalyses', () => { }); it('leaves out an approval recorded against a different word', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')], new Map()); expect(tallyAnalyses(entry, approved, new Map(), 'en')).toEqual([]); }); it('reads an analysis with no gloss in the language as unglossed', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0')]); + const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0')], new Map()); const records = new Map([['a1', analysis('a1', 'word')]]); expect(tallyAnalyses(entry, approved, records, 'fr')[0].gloss).toBe(''); }); it('reads an approval naming a missing record as unglossed', () => { - const approved = approvedAnalysisByToken([link('gone', 'GEN 1:1:0')]); + const approved = approvedAnalysisByToken([link('gone', 'GEN 1:1:0')], new Map()); expect(tallyAnalyses(entry, approved, new Map(), 'en')).toEqual([ { analysisId: 'gone', gloss: '', count: 1 }, diff --git a/src/components/ConcordancePanel.tsx b/src/components/ConcordancePanel.tsx index 6fa012bff..9b3a0d90a 100644 --- a/src/components/ConcordancePanel.tsx +++ b/src/components/ConcordancePanel.tsx @@ -74,14 +74,14 @@ export default function ConcordancePanel({ const analysis = useAnalysis(); const analysisLanguage = useAnalysisLanguage(); - const approvedByToken = useMemo( - () => approvedAnalysisByToken(analysis.tokenAnalysisLinks), - [analysis.tokenAnalysisLinks], - ); const analysesById = useMemo( () => new Map(analysis.tokenAnalyses.map((ta) => [ta.id, ta])), [analysis.tokenAnalyses], ); + const approvedByToken = useMemo( + () => approvedAnalysisByToken(analysis.tokenAnalysisLinks, analysesById), + [analysis.tokenAnalysisLinks, analysesById], + ); // Keyed on the status so a refresh, which passes through loading, starts the window over. const { windowRows: windowEntries, scrollRef, sentinelRef } = useRowWindow(entries, text.status); diff --git a/src/utils/concordance.ts b/src/utils/concordance.ts index 9b82bd7c6..644f9ac40 100644 --- a/src/utils/concordance.ts +++ b/src/utils/concordance.ts @@ -197,16 +197,25 @@ function approvalKey(tokenRef: string, form: string): string { /** * Gathers the approved links for lookup. A token approved to more than one analysis as the same - * form — a state no write path builds — keeps the one an approval tie resolves to. + * form — a state no write path builds — keeps the one an approval tie resolves to, a link whose + * payload is missing from `analysesById` losing to one whose payload is present. */ -export function approvedAnalysisByToken(links: readonly TokenAnalysisLink[]): ApprovedAnalyses { +export function approvedAnalysisByToken( + links: readonly TokenAnalysisLink[], + analysesById: ReadonlyMap, +): ApprovedAnalyses { const winners = new Map(); const approved = new Map(); + const winsTie = (link: TokenAnalysisLink, held: TokenAnalysisLink) => { + const hasPayload = analysesById.has(link.analysisId); + if (hasPayload !== analysesById.has(held.analysisId)) return hasPayload; + return prefersLink(link, held); + }; links.forEach((link) => { if (link.status !== 'approved') return; const key = approvalKey(link.token.tokenRef, normalizeSurfaceForm(link.token.surfaceText)); const held = winners.get(key); - if (held && !prefersLink(link, held)) return; + if (held && !winsTie(link, held)) return; winners.set(key, link); approved.set(key, link.analysisId); }); From fafd323fd28027ea67a96eb972ef61865debd3ae Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 16 Oct 2026 09:26:38 -0600 Subject: [PATCH 3/4] Address review: narrow pick-rule claims and fix link docs --- AGENTS.md | 2 +- perf/dataset/generate.ts | 2 +- src/__tests__/services/projectStorage.test.ts | 4 +-- src/__tests__/utils/pick-rules.test.ts | 2 +- src/store/analysisSlice.ts | 25 +++++++++---------- src/types/interlinearizer.d.ts | 10 +++----- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2a806e73f..126489390 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -112,7 +112,7 @@ The core types are: All character offsets (`charStart`, `charEnd`, `charIndex`) are zero-based UTF-16 code-unit indices — plain JavaScript string indices, not code points and not grapheme clusters — with end offsets exclusive. -Key invariants: `Segment.baselineText.slice(charStart, charEnd) === Token.surfaceText`; at most one linked analysis per token/segment may have `status: 'approved'`. Token and phrase payloads are shared by every occurrence analyzed identically, so a link is identified by its own `id`, never by its `analysisId`. Picking one of several links or payloads goes through [pick-rules.ts](src/utils/pick-rules.ts), never list order. `MultiString` values are keyed by BCP 47 tags. `TokenSnapshot.surfaceText` detects drift when baseline text changes. Every project and draft record carries a `modelVersion`; every write stamps `CURRENT_MODEL_VERSION`, and a record stamped higher than that is refused (`assertSupportedModelVersion`) rather than read or overwritten. +Key invariants: `Segment.baselineText.slice(charStart, charEnd) === Token.surfaceText`; at most one linked analysis per token/segment may have `status: 'approved'`. Token and phrase payloads are shared by every occurrence analyzed identically, so a link is identified by its own `id`, never by its `analysisId`. Picking one of a token's or segment's links, or one of several identical payloads, goes through [pick-rules.ts](src/utils/pick-rules.ts), never list order. `MultiString` values are keyed by BCP 47 tags. `TokenSnapshot.surfaceText` detects drift when baseline text changes. Every project and draft record carries a `modelVersion`; every write stamps `CURRENT_MODEL_VERSION`, and a record stamped higher than that is refused (`assertSupportedModelVersion`) rather than read or overwritten. ### TypeScript path aliases diff --git a/perf/dataset/generate.ts b/perf/dataset/generate.ts index 49b1e8f6b..74a7a19d5 100644 --- a/perf/dataset/generate.ts +++ b/perf/dataset/generate.ts @@ -69,7 +69,7 @@ export function generateAnalysis( const segmentAnalyses: SegmentAnalysis[] = []; const segmentAnalysisLinks: SegmentAnalysisLink[] = []; - // Drawn apart from the shape's stream, so link ids never move a tier's shape. + // Its own stream, so link ids never shift the draws a tier's records come from. const linkIds = createRng(seed, 'link-ids'); let clock = 0; diff --git a/src/__tests__/services/projectStorage.test.ts b/src/__tests__/services/projectStorage.test.ts index a3c25810f..80912b13e 100644 --- a/src/__tests__/services/projectStorage.test.ts +++ b/src/__tests__/services/projectStorage.test.ts @@ -2562,8 +2562,8 @@ describe('projectStorage', () => { writeThroughStore(); jest .spyOn(crypto, 'randomUUID') - .mockReturnValueOnce('00000000-0000-0000-0000-00000000000a') - .mockReturnValueOnce('00000000-0000-0000-0000-00000000000b'); + .mockReturnValueOnce('00000000-0000-0000-0000-000000000001') + .mockReturnValueOnce('00000000-0000-0000-0000-000000000002'); const imported = await savePt9Import( token, 'src-project', diff --git a/src/__tests__/utils/pick-rules.test.ts b/src/__tests__/utils/pick-rules.test.ts index 7235966cb..9e72a3239 100644 --- a/src/__tests__/utils/pick-rules.test.ts +++ b/src/__tests__/utils/pick-rules.test.ts @@ -9,9 +9,9 @@ const LATER = '2026-02-01T00:00:00.000Z'; function link(id: string, updatedAt: string): TokenAnalysisLink { return { + ...FIXTURE_STAMPS, id, analysisId: 'ta-1', - createdAt: EARLIER, updatedAt, status: 'approved', token: { tokenRef: 'GEN 1:1:0', surfaceText: 'word' }, diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 2f3864656..8e3c9e168 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -319,11 +319,11 @@ function reshapePhrase( } /** - * Finds the approved `SegmentAnalysisLink` for `segmentId` — of several, the one the read selectors - * surface — together with the `SegmentAnalysis` it references. Approved links referencing a missing - * analysis (orphaned links from corruption or a migration) are removed from the draft first — so - * the corruption never persists or accumulates duplicate approved links — and the pick falls to the - * rest. Mirrors {@link resolveApprovedAnalysis} for the segment layer. + * Finds the approved `SegmentAnalysisLink` for `segmentId` — of several, the one an approval tie + * picks — together with the `SegmentAnalysis` it references, or `undefined` when no approved link + * with a payload remains. Approved links referencing a missing analysis (orphaned links from + * corruption or a migration) are removed from the draft first — so the corruption never persists or + * accumulates duplicate approved links — and the pick falls to the rest. */ function resolveApprovedSegmentAnalysis( state: AnalysisState, @@ -428,12 +428,11 @@ function approveStaleSegmentAnalysis( } /** - * Finds the approved `TokenAnalysisLink` for `tokenRef` together with the `TokenAnalysis` it - * references. In the data-model-violating case of multiple approved links for one token it picks - * the one the read selectors surface, so a write never lands on a different link than the one they - * read and appears to no-op. Approved links referencing a missing analysis (orphaned links from - * corruption or a migration) are removed from the draft first — so the corruption never persists or - * accumulates duplicate approved links — and the pick falls to the rest. + * Finds the approved `TokenAnalysisLink` for `tokenRef` — of several, the one an approval tie picks + * — together with the `TokenAnalysis` it references, or `undefined` when no approved link with a + * payload remains. Approved links referencing a missing analysis (orphaned links from corruption or + * a migration) are removed from the draft first — so the corruption never persists or accumulates + * duplicate approved links — and the pick falls to the rest. */ function resolveApprovedAnalysis( state: AnalysisState, @@ -484,8 +483,8 @@ function approveHeldLink( /** * Approves the stored payload `analysisId` for a token in place of any approval it held, keeping * one approved link per token, and reclaims the payload that approval named once nothing links it. - * A token holding no link to the payload gets one under `linkId`. A no-op for an id naming no - * stored payload. + * A token with neither an approved link nor one to the payload gets one under `linkId`. A no-op for + * an id naming no stored payload. */ function approveStoredAnalysis( state: AnalysisState, diff --git a/src/types/interlinearizer.d.ts b/src/types/interlinearizer.d.ts index 5904e2691..0b455cdf0 100644 --- a/src/types/interlinearizer.d.ts +++ b/src/types/interlinearizer.d.ts @@ -829,8 +829,7 @@ declare module 'interlinearizer' { * `status: 'approved'`. That linked analysis is the canonical segment-level analysis for * rendering; alternates are available to review workflows via the other statuses. Whatever * writes an approved link owns this invariant; nothing enforces it at runtime, and a breach is - * reported at the storage boundary rather than repaired. Reads and writes resolve a breach to - * the approved link with the latest `updatedAt`, then the smallest `id`. + * reported at the storage boundary rather than repaired. */ segmentAnalyses: SegmentAnalysis[]; @@ -854,8 +853,7 @@ declare module 'interlinearizer' { * available to review workflows via the other statuses (`'suggested'`, `'candidate'`, * `'rejected'`, `'stale'`). Whatever writes an approved link owns this invariant; nothing * enforces it at runtime, and a breach is reported at the storage boundary rather than - * repaired. Reads and writes resolve a breach to the approved link with the latest `updatedAt`, - * then the smallest `id`. + * repaired. */ tokenAnalyses: TokenAnalysis[]; @@ -902,8 +900,8 @@ declare module 'interlinearizer' { */ export interface AnalysisLink { /** - * Unique within the owning `TextAnalysis` — identifies this link, which `analysisId` cannot, - * since targets analyzed identically share one payload. + * Unique within its layer of the owning `TextAnalysis` — identifies this link, which + * `analysisId` cannot, since targets analyzed identically share one payload. */ id: string; From ecef3fe47e7f4bbaddad0a4155612cdd225db4ff Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 16 Oct 2026 13:33:55 -0600 Subject: [PATCH 4/4] Address review: share per-key tie picks and drop orphan approvals --- .../components/ConcordanceRowView.test.tsx | 6 +- src/__tests__/store/analysisSlice.test.ts | 10 +-- src/__tests__/utils/concordance.test.ts | 90 +++++++------------ src/__tests__/utils/pick-rules.test.ts | 37 +++++++- src/store/analysisSlice.ts | 34 +++---- src/utils/concordance.ts | 32 +++---- src/utils/pick-rules.ts | 15 ++++ 7 files changed, 117 insertions(+), 107 deletions(-) diff --git a/src/__tests__/components/ConcordanceRowView.test.tsx b/src/__tests__/components/ConcordanceRowView.test.tsx index dbe9c9cda..587ba9dc7 100644 --- a/src/__tests__/components/ConcordanceRowView.test.tsx +++ b/src/__tests__/components/ConcordanceRowView.test.tsx @@ -134,7 +134,11 @@ describe('ConcordanceRowView', () => { ['partlyAnalyzed', ['GEN 1:3:0'], 'circle-dashed-icon'], ['unanalyzed', [], 'circle-icon'], ])('marks a %s form with its own icon and label', (status, refs, icon) => { - renderRow({ verses: LIGHT_VERSES, links: refs.map((ref) => link('a1', ref)) }); + renderRow({ + verses: LIGHT_VERSES, + links: refs.map((ref) => link('a1', ref)), + analyses: [analysis('a1')], + }); const marker = screen.getByTestId('concordance-row-status'); expect(marker).toHaveAttribute('data-status', status); diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 8d756f4da..9c82e7669 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -6987,7 +6987,7 @@ describe('link ids', () => { .analysis.analysis.tokenAnalysisLinks.filter((l) => l.token.tokenRef === tokenRef); } - it('gives each link a gloss creates an id of its own, though the links share a payload', () => { + it('gives each link created by a gloss its own id, though the links share a payload', () => { const store = createAnalysisStore(); store.dispatch(writeGloss('tok-1', 'word', 'hello')); @@ -7000,7 +7000,7 @@ describe('link ids', () => { expect(first.id).not.toBe(second.id); }); - it('gives the link a breakdown creates an id', () => { + it('gives the link created by a breakdown an id', () => { const store = createAnalysisStore(); store.dispatch(writeMorphemes('tok-1', 'cats', ['cat', '-s'], 'en')); @@ -7008,7 +7008,7 @@ describe('link ids', () => { expect(linksAt(store, 'tok-1')).toEqual([expect.objectContaining({ id: expect.any(String) })]); }); - it('gives the link an approval creates an id', () => { + it('gives the link created by an approval an id', () => { const imported = logos('ta-imported', 'word'); const store = createAnalysisStore(tokenState([imported], [])); @@ -7019,7 +7019,7 @@ describe('link ids', () => { expect(linksAt(store, 'tok-1')).toEqual([expect.objectContaining({ id: expect.any(String) })]); }); - it('gives the link a re-applied stale analysis creates an id', () => { + it('gives the link created by re-applying a stale analysis an id', () => { const stranded = logos('ta-stranded', 'word'); const store = createAnalysisStore( tokenState([stranded], [makeLink(stranded, 'tok-old', 'stale')]), @@ -7039,7 +7039,7 @@ describe('link ids', () => { ]); }); - it('gives the link a free translation creates an id apart from its payload', () => { + it('gives the link created by a free translation an id apart from its payload', () => { const store = createAnalysisStore(); store.dispatch(writeSegmentFreeTranslation('seg-1', 'In the beginning', 'au commencement')); diff --git a/src/__tests__/utils/concordance.test.ts b/src/__tests__/utils/concordance.test.ts index 025b53d35..2566c91b1 100644 --- a/src/__tests__/utils/concordance.test.ts +++ b/src/__tests__/utils/concordance.test.ts @@ -12,6 +12,7 @@ import { deriveConcordanceRows, indexBook, tallyAnalyses, + type ApprovedAnalyses, type ConcordanceOccurrence, } from '../../utils/concordance'; @@ -41,6 +42,13 @@ function analysis(id: string, gloss?: string): TokenAnalysis { return { ...FIXTURE_STAMPS, id, surfaceText: 'word', gloss: gloss ? { en: gloss } : undefined }; } +function approvals(links: readonly TokenAnalysisLink[]): ApprovedAnalyses { + return approvedAnalysisByToken( + links, + new Map(links.map((l) => [l.analysisId, analysis(l.analysisId)])), + ); +} + /** Builds an occurrence of the form at `charStart` in `contextText`. */ function occurrenceIn(contextText: string, form: string): ConcordanceOccurrence { const charStart = contextText.indexOf(form); @@ -171,29 +179,26 @@ describe('approvedAnalysisOf', () => { const [first, second] = entry.occurrences; it('finds the analysis an occurrence is approved to', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0', 'dark')]); expect(approvedAnalysisOf(approved, entry, first)).toBe('a1'); expect(approvedAnalysisOf(approved, entry, second)).toBeUndefined(); }); it('skips links of other statuses', () => { - const approved = approvedAnalysisByToken( - [link('a1', 'GEN 1:1:0', 'dark', 'suggested')], - new Map(), - ); + const approved = approvals([link('a1', 'GEN 1:1:0', 'dark', 'suggested')]); expect(approvedAnalysisOf(approved, entry, first)).toBeUndefined(); }); it('ignores an approval recorded against a different word at the same place', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0', 'light')]); expect(approvedAnalysisOf(approved, entry, first)).toBeUndefined(); }); it('accepts an approval recorded against the word in another case', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'Dark')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0', 'Dark')]); expect(approvedAnalysisOf(approved, entry, first)).toBe('a1'); }); @@ -206,32 +211,16 @@ describe('approvedAnalysisOf', () => { a1: link('a1', 'GEN 1:1:0', 'dark'), a2: { ...link('a2', 'GEN 1:1:0', 'dark'), updatedAt: '2026-02-01T00:00:00.000Z' }, }; - const approved = approvedAnalysisByToken( - order.map((id) => (id === 'a1' ? links.a1 : links.a2)), - new Map(), - ); + const approved = approvals(order.map((id) => (id === 'a1' ? links.a1 : links.a2))); expect(approvedAnalysisOf(approved, entry, first)).toBe('a2'); }); - it.each([ - ['listed first', ['gone', 'valid']], - ['listed last', ['valid', 'gone']], - ])( - 'keeps an approval with a record over a later one naming a missing record when it is %s', - (_, order) => { - const links = { - valid: link('valid', 'GEN 1:1:0', 'dark'), - gone: { ...link('gone', 'GEN 1:1:0', 'dark'), updatedAt: '2026-02-01T00:00:00.000Z' }, - }; - const approved = approvedAnalysisByToken( - order.map((id) => (id === 'valid' ? links.valid : links.gone)), - new Map([['valid', analysis('valid', 'darkness')]]), - ); - - expect(approvedAnalysisOf(approved, entry, first)).toBe('valid'); - }, - ); + it('leaves out an approval naming a missing record', () => { + const approved = approvedAnalysisByToken([link('gone', 'GEN 1:1:0', 'dark')], new Map()); + + expect(approvedAnalysisOf(approved, entry, first)).toBeUndefined(); + }); }); describe('deriveConcordanceRows', () => { @@ -240,14 +229,11 @@ describe('deriveConcordanceRows', () => { const entries = buildConcordanceEntries([genesis, exodus], collator); it('marks a form with every occurrence approved as analyzed', () => { - const approved = approvedAnalysisByToken( - [ - link('a1', 'GEN 1:1:0', 'light'), - link('a1', 'GEN 1:1:6', 'light'), - link('a1', 'EXO 1:1:0', 'light'), - ], - new Map(), - ); + const approved = approvals([ + link('a1', 'GEN 1:1:0', 'light'), + link('a1', 'GEN 1:1:6', 'light'), + link('a1', 'EXO 1:1:0', 'light'), + ]); expect(deriveConcordanceRows(entries, approved, 'GEN')[0]).toMatchObject({ analyzedCount: 3, @@ -256,7 +242,7 @@ describe('deriveConcordanceRows', () => { }); it('marks a form with some occurrences approved as partly analyzed', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0', 'light')]); expect(deriveConcordanceRows(entries, approved, 'GEN')[0]).toMatchObject({ analyzedCount: 1, @@ -272,7 +258,7 @@ describe('deriveConcordanceRows', () => { }); it('does not count an occurrence whose token was approved as a different word', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'dark')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0', 'dark')]); expect(deriveConcordanceRows(entries, approved, 'GEN')[0]).toMatchObject({ analyzedCount: 0, @@ -294,10 +280,11 @@ describe('tallyAnalyses', () => { const [entry] = buildConcordanceEntries([book], collator); it('counts the occurrences each analysis covers, most-used first', () => { - const approved = approvedAnalysisByToken( - [link('rare', 'GEN 1:1:0'), link('common', 'GEN 1:1:5'), link('common', 'GEN 1:1:10')], - new Map(), - ); + const approved = approvals([ + link('rare', 'GEN 1:1:0'), + link('common', 'GEN 1:1:5'), + link('common', 'GEN 1:1:10'), + ]); const records = new Map([ ['rare', analysis('rare', 'utterance')], ['common', analysis('common', 'word')], @@ -310,10 +297,7 @@ describe('tallyAnalyses', () => { }); it('orders equally used analyses by first use', () => { - const approved = approvedAnalysisByToken( - [link('second', 'GEN 1:1:5'), link('first', 'GEN 1:1:0')], - new Map(), - ); + const approved = approvals([link('second', 'GEN 1:1:5'), link('first', 'GEN 1:1:0')]); const records = new Map([ ['first', analysis('first', 'one')], ['second', analysis('second', 'two')], @@ -326,25 +310,17 @@ describe('tallyAnalyses', () => { }); it('leaves out an approval recorded against a different word', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0', 'light')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0', 'light')]); expect(tallyAnalyses(entry, approved, new Map(), 'en')).toEqual([]); }); it('reads an analysis with no gloss in the language as unglossed', () => { - const approved = approvedAnalysisByToken([link('a1', 'GEN 1:1:0')], new Map()); + const approved = approvals([link('a1', 'GEN 1:1:0')]); const records = new Map([['a1', analysis('a1', 'word')]]); expect(tallyAnalyses(entry, approved, records, 'fr')[0].gloss).toBe(''); }); - - it('reads an approval naming a missing record as unglossed', () => { - const approved = approvedAnalysisByToken([link('gone', 'GEN 1:1:0')], new Map()); - - expect(tallyAnalyses(entry, approved, new Map(), 'en')).toEqual([ - { analysisId: 'gone', gloss: '', count: 1 }, - ]); - }); }); describe('contextLine', () => { diff --git a/src/__tests__/utils/pick-rules.test.ts b/src/__tests__/utils/pick-rules.test.ts index 9e72a3239..d0d77109d 100644 --- a/src/__tests__/utils/pick-rules.test.ts +++ b/src/__tests__/utils/pick-rules.test.ts @@ -2,19 +2,24 @@ import type { TokenAnalysis, TokenAnalysisLink } from 'interlinearizer'; import { FIXTURE_STAMPS } from '../test-helpers'; -import { preferredLink, preferredPayload, prefersLink } from '../../utils/pick-rules'; +import { + preferredLink, + preferredLinkByKey, + preferredPayload, + prefersLink, +} from '../../utils/pick-rules'; const EARLIER = '2026-01-01T00:00:00.000Z'; const LATER = '2026-02-01T00:00:00.000Z'; -function link(id: string, updatedAt: string): TokenAnalysisLink { +function link(id: string, updatedAt: string, tokenRef = 'GEN 1:1:0'): TokenAnalysisLink { return { ...FIXTURE_STAMPS, id, analysisId: 'ta-1', updatedAt, status: 'approved', - token: { tokenRef: 'GEN 1:1:0', surfaceText: 'word' }, + token: { tokenRef, surfaceText: 'word' }, }; } @@ -60,6 +65,32 @@ describe('preferredLink', () => { }); }); +describe('preferredLinkByKey', () => { + const byToken = (l: TokenAnalysisLink) => l.token.tokenRef; + + it('picks the preferred link for each key whatever the order', () => { + const first = link('a', LATER, 'GEN 1:1:0'); + const second = link('b', LATER, 'GEN 1:1:5'); + const beaten = [link('c', EARLIER, 'GEN 1:1:0'), link('d', EARLIER, 'GEN 1:1:5')]; + const expected = new Map([ + ['GEN 1:1:0', first], + ['GEN 1:1:5', second], + ]); + + expect(preferredLinkByKey([...beaten, first, second], byToken)).toEqual(expected); + expect(preferredLinkByKey([first, second, ...beaten], byToken)).toEqual(expected); + }); + + it('skips a link given no key', () => { + const kept = link('a', EARLIER); + const skipped = link('b', LATER); + + expect( + preferredLinkByKey([kept, skipped], (l) => (l === skipped ? undefined : byToken(l))), + ).toEqual(new Map([['GEN 1:1:0', kept]])); + }); +}); + describe('preferredPayload', () => { it('picks the earliest-created payload whatever the order', () => { const earliest = payload('z', EARLIER); diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index 8e3c9e168..facc1442f 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -22,7 +22,12 @@ import { import { buildCatalogRows, type HeadingPlacement } from '../utils/analysis-query'; import type { StaleFreeTranslation } from '../utils/stale-free-translations'; import { isEmptyMultiString } from '../utils/multi-string'; -import { preferredLink, preferredPayload, prefersLink } from '../utils/pick-rules'; +import { + preferredLink, + preferredLinkByKey, + preferredPayload, + prefersLink, +} from '../utils/pick-rules'; import { buildPoolIndex, deriveTokenSuggestion, @@ -1988,16 +1993,10 @@ const selectApprovedIdByTokenRef = createSelector( selectTokenAnalysisLinks, selectAnalysisById, (links, byId) => { - const winners = new Map(); - const index = new Map(); - links.forEach((link) => { - if (link.status !== 'approved' || !byId.has(link.analysisId)) return; - const held = winners.get(link.token.tokenRef); - if (held && !prefersLink(link, held)) return; - winners.set(link.token.tokenRef, link); - index.set(link.token.tokenRef, link.analysisId); - }); - return index; + const winners = preferredLinkByKey(links, (link) => + link.status === 'approved' && byId.has(link.analysisId) ? link.token.tokenRef : undefined, + ); + return new Map([...winners].map(([tokenRef, link]) => [tokenRef, link.analysisId])); }, ); @@ -2478,17 +2477,12 @@ const selectApprovedSegmentAnalysisBySegment = createSelector( selectSegmentAnalyses, (links, analyses) => { const byId = new Map(analyses.map((a) => [a.id, a])); - const winners = new Map(); - const bySegment = new Map(); - links.forEach((link) => { + const approved = links.flatMap((link) => { const analysis = byId.get(link.analysisId); - if (link.status !== 'approved' || !analysis) return; - const held = winners.get(link.segmentId); - if (held && !prefersLink(link, held)) return; - winners.set(link.segmentId, link); - bySegment.set(link.segmentId, analysis); + return link.status === 'approved' && analysis ? [{ ...link, analysis }] : []; }); - return bySegment; + const winners = preferredLinkByKey(approved, (link) => link.segmentId); + return new Map([...winners].map(([segmentId, { analysis }]) => [segmentId, analysis])); }, ); diff --git a/src/utils/concordance.ts b/src/utils/concordance.ts index 644f9ac40..9e992e1d7 100644 --- a/src/utils/concordance.ts +++ b/src/utils/concordance.ts @@ -4,7 +4,7 @@ import type { Collator } from 'platform-bible-utils'; import { isWordToken } from '../types/type-guards'; import { normalizeSurfaceForm } from './analysis-identity'; import { multiStringText } from './multi-string'; -import { prefersLink } from './pick-rules'; +import { preferredLinkByKey } from './pick-rules'; /** One place in the source text where a word form appears, carried with the verse it sits in. */ export interface ConcordanceOccurrence { @@ -196,30 +196,20 @@ function approvalKey(tokenRef: string, form: string): string { } /** - * Gathers the approved links for lookup. A token approved to more than one analysis as the same - * form — a state no write path builds — keeps the one an approval tie resolves to, a link whose - * payload is missing from `analysesById` losing to one whose payload is present. + * Gathers the approved links for lookup, leaving out any naming a payload missing from + * `analysesById`. A token approved to more than one analysis as the same form — a state no write + * path builds — keeps the one an approval tie resolves to. */ export function approvedAnalysisByToken( links: readonly TokenAnalysisLink[], analysesById: ReadonlyMap, ): ApprovedAnalyses { - const winners = new Map(); - const approved = new Map(); - const winsTie = (link: TokenAnalysisLink, held: TokenAnalysisLink) => { - const hasPayload = analysesById.has(link.analysisId); - if (hasPayload !== analysesById.has(held.analysisId)) return hasPayload; - return prefersLink(link, held); - }; - links.forEach((link) => { - if (link.status !== 'approved') return; - const key = approvalKey(link.token.tokenRef, normalizeSurfaceForm(link.token.surfaceText)); - const held = winners.get(key); - if (held && !winsTie(link, held)) return; - winners.set(key, link); - approved.set(key, link.analysisId); - }); - return approved; + const winners = preferredLinkByKey(links, (link) => + link.status === 'approved' && analysesById.has(link.analysisId) + ? approvalKey(link.token.tokenRef, normalizeSurfaceForm(link.token.surfaceText)) + : undefined, + ); + return new Map([...winners].map(([key, link]) => [key, link.analysisId])); } /** @@ -261,7 +251,7 @@ export function deriveConcordanceRows( /** * Counts how many of an entry's occurrences each approved analysis covers, glossed in - * `analysisLanguage`. An analysis the approvals name but the records lack reads as unglossed. + * `analysisLanguage`. * * @returns One tally per analysis, the most-used first and ties in order of first use. */ diff --git a/src/utils/pick-rules.ts b/src/utils/pick-rules.ts index cb703c30e..e9002c43f 100644 --- a/src/utils/pick-rules.ts +++ b/src/utils/pick-rules.ts @@ -17,6 +17,21 @@ export function preferredLink(links: readonly L[]): L | ); } +/** The link an approval tie picks for each key `keyOf` gives, skipping links it gives none. */ +export function preferredLinkByKey( + links: readonly L[], + keyOf: (link: L) => string | undefined, +): Map { + const winners = new Map(); + links.forEach((link) => { + const key = keyOf(link); + if (key === undefined) return; + const held = winners.get(key); + if (!held || prefersLink(link, held)) winners.set(key, link); + }); + return winners; +} + /** * The payload a merge folds content-identical ones into: the earliest `createdAt`, then the * smallest id. `undefined` when `analyses` is empty.