From dd21111c0de94431268c42d8f7bf1c0859402e25 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 14 Sep 2026 18:40:18 +0000 Subject: [PATCH 1/4] Drop the reviewed/auto tier; surface the translator's per-slide notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stored `status: 'reviewed' | 'auto'` never earned its keep. Viewers were told "unreviewed" about text nobody was going to fix mid-service — noise they can't act on — and in practice what decides whether a slide gets a human's attention is a *note*, not a tier. Deleted: - `SlideStatus`, `status` and `reviewedAt` from `SlideTranslationEntry`, the library record, and `PerSlideTranslation`. `provenance` stays; nothing branches on it. - the "unreviewed" badge in the slide viewer. - `resolveSlideTranslation`'s two-pass quality walk — it now returns the first entry along the fallback chain. Behavior change: a Creole viewer with no Creole text now sees the French one whatever produced it, where before an unreviewed French text was withheld and the slide read "(not translated)". - the Python work-ahead translator's protected-entry check. It now writes only keys that are *missing*, so it can't clobber anyone. Keys are content-addressed, so an existing entry already translates this exact text. The per-cell chip in the review grid now says "Saved", which is what it always meant: the draft matches what is in the library. Surfaced instead: `set_translations` has always let the model attach a per-segment `note` — "a genuine ambiguity or judgement call worth flagging" — and we dropped it on the floor. Notes now ride on the conversation (`SlideConversation.notes`, content-addressed by language + source text, seeded into each agent run and replaced wholesale by its result), and the review screen is organized around them: the service-item list is a row of chips badged with each item's note count, flagged rows are tinted and marked, and a flagged cell leads with its note above the textarea. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RuD8vmbn9ku8wbVQYMH5BH --- docs/slide-translations-plan.md | 24 +++++ nlp.test.ts | 77 ++++++++++++++ nlp.ts | 61 ++++++++++- server.ts | 11 +- sessionExport.test.ts | 5 +- slideConversationStore.ts | 7 ++ slideLibrary.test.ts | 4 +- slideLibrary.ts | 32 +++--- slide_translator.py | 18 ++-- src/SlideReview.test.tsx | 22 +++- src/SlideReview.tsx | 44 ++++++-- src/SlideReviewContainer.tsx | 155 +++++++++++++++++++--------- src/SlideTranslationViewer.test.tsx | 28 ++--- src/SlideTranslationViewer.tsx | 30 ++---- src/slideItemTranslation.test.ts | 34 ++---- src/slideItemTranslation.ts | 46 ++++----- src/slideTranslation.test.ts | 40 ++----- src/slideTranslation.ts | 46 ++++----- src/slideTranslationApi.test.ts | 10 +- src/slideTranslationApi.ts | 15 ++- src/strings.ts | 32 ++++-- tests/test_slide_translator.py | 23 ++--- 22 files changed, 484 insertions(+), 280 deletions(-) diff --git a/docs/slide-translations-plan.md b/docs/slide-translations-plan.md index 2157a1a..28c4240 100644 --- a/docs/slide-translations-plan.md +++ b/docs/slide-translations-plan.md @@ -384,3 +384,27 @@ review screen writes `reviewed` edits straight into `slideTranslations` (and sti POSTs to the library for persistence), and the Python service's seed now **protects existing `reviewed` entries** rather than overwriting them. See "Two tiers" and Phase C above. + +**The `reviewed`/`auto` tier, removed (post-build).** Everything above that turns on a +stored `status: 'reviewed' | 'auto'` (and its `reviewedAt` stamp) is history: the field is +gone. It never earned its keep. Viewers were told "unreviewed" about text nobody was going +to fix mid-service, which is noise they can't act on; and in practice the thing that decides +whether a slide gets a human's attention is a **note**, not a tier. What replaced it: + +- `SlideTranslationEntry` is `{ text, provenance }`. `provenance` still records where a + translation came from; nothing branches on it. +- `resolveSlideTranslation` returns the first entry along the fallback chain. A Creole + viewer with no Creole text now sees the French one whatever produced it — previously an + unreviewed French text was withheld and the slide read "(not translated)". +- The library is still the durable tier, and still only holds what a human saved. The review + screen's per-cell chip says **Saved**, not Reviewed: it is about persistence, not quality. +- The Python work-ahead translator no longer needs to recognize a protected entry — it + writes only keys that are **missing** (`_store_translations`), so it can't clobber anyone. + +**Notes, surfaced (same change).** `set_translations` always let the model attach a +per-segment `note` — "a genuine ambiguity or judgement call worth flagging" — and we dropped +it on the floor. Notes are now carried on the conversation (`SlideConversation.notes`, +content-addressed by language + source text, replaced wholesale by each agent run) and are +what the review screen is organized around: the service-item list badges each item with its +note count, flagged rows are tinted and marked, and a flagged cell leads with its note above +the textarea. An item with no notes is an item nobody needs to open. diff --git a/nlp.test.ts b/nlp.test.ts index 6271e99..d4b976a 100644 --- a/nlp.test.ts +++ b/nlp.test.ts @@ -526,3 +526,80 @@ describe('buildSeedConversationPrompt', () => { expect(prompt).not.toContain('\\n'); }); }); + +describe('runSlideTranslationAgent — per-slide review notes', () => { + const slides = ['Praise the Lord', 'Forever']; + + it('carries a note attached in set_translations, keyed by source text', async () => { + const { provider } = fakeAgentProvider({ + languages: [ + { + language: 'French', + segments: [ + { segmentId: 0, translation: 'Louez le Seigneur' }, + { segmentId: 1, translation: 'Pour toujours', note: '"Forever" could be liturgical here.' }, + ], + }, + ], + }); + + const result = await runSlideTranslationAgent(provider, { + sourceSlides: slides, + messages: [{ role: 'user', parts: [{ text: 'prompt' }] }], + bibleLanguages: [], + }); + + expect(result.notes).toEqual([ + { language: 'French', sourceText: 'Forever', text: '"Forever" could be liturgical here.' }, + ]); + }); + + it('withdraws a note when the slide is rewritten without one', async () => { + const { provider } = fakeAgentProvider({ + languages: [ + { language: 'French', segments: [{ segmentId: 1, translation: "Pour l'éternité" }] }, + ], + }); + + const result = await runSlideTranslationAgent(provider, { + sourceSlides: slides, + messages: [{ role: 'user', parts: [{ text: 'prompt' }] }], + bibleLanguages: [], + currentNotes: [{ language: 'French', sourceText: 'Forever', text: 'Old caveat.' }], + }); + + expect(result.notes).toEqual([]); + }); + + it('keeps notes on the slides a follow-up never touched', async () => { + const provider = fakeScriptedProvider([ + [{ name: 'revise_translation', args: { language: 'French', segmentId: 0, find: 'Louez', replace: 'Louons' } }], + ]); + + const result = await runSlideTranslationAgent(provider, { + sourceSlides: slides, + messages: [{ role: 'user', parts: [{ text: 'prompt' }] }], + bibleLanguages: [], + currentTranslations: { French: ['Louez le Seigneur', 'Pour toujours'] }, + currentNotes: [{ language: 'French', sourceText: 'Forever', text: 'Old caveat.' }], + }); + + // The run reports the item's whole note set, so the server can replace rather than merge. + expect(result.notes).toEqual([ + { language: 'French', sourceText: 'Forever', text: 'Old caveat.' }, + ]); + }); + + it('drops a seeded note whose source text is no longer in the item', async () => { + const { provider } = fakeAgentProvider({ languages: [] }); + + const result = await runSlideTranslationAgent(provider, { + sourceSlides: slides, + messages: [{ role: 'user', parts: [{ text: 'prompt' }] }], + bibleLanguages: [], + currentNotes: [{ language: 'French', sourceText: 'A slide that was edited away', text: 'Stale.' }], + }); + + expect(result.notes).toEqual([]); + }); +}); diff --git a/nlp.ts b/nlp.ts index 4843ad3..2b9c7bd 100644 --- a/nlp.ts +++ b/nlp.ts @@ -2,7 +2,8 @@ import { FunctionCallingConfigMode, Type, type Content, type FunctionDeclaration import { Gemini as GoogleGenAI } from '@posthog/ai/gemini'; import { PostHog } from 'posthog-node'; import { BIBLE_TRANSLATIONS, lookupBiblePassage, type BibleLookupArgs, type BibleToolCall } from './bible.ts'; -import { unescapeLiteralEscapes } from './src/slideTranslation.ts'; +import { normalizeSlideText, unescapeLiteralEscapes, type SlideReviewNote } from './src/slideTranslation.ts'; +export type { SlideReviewNote }; export class GeminiProvider { apiClient: GoogleGenAI; @@ -344,8 +345,12 @@ const SET_TRANSLATIONS_TOOL: FunctionDeclaration = { note: { type: Type.STRING, description: - 'Optional caveat for the reviewer. Omit unless there is ' + - 'a genuine ambiguity or judgement call worth flagging.', + 'Optional caveat for the reviewer. It is shown beside this ' + + "slide's translation on the review screen and marks the whole " + + 'item as wanting a look, so omit it unless there is a genuine ' + + 'ambiguity or judgement call worth flagging — a note on every ' + + 'slide is worth no more than none. Rewriting a slide without a ' + + 'note withdraws the one it had.', }, }, }, @@ -425,12 +430,29 @@ const workingFor = (working: WorkingTranslations, language: string): Map>; + +/** Flatten the working notes back to the stored, content-keyed shape. */ +const collectNotes = (notes: WorkingNotes, sourceSlides: string[]): SlideReviewNote[] => { + const out: SlideReviewNote[] = []; + for (const [language, perSlide] of notes) { + for (const segmentId of [...perSlide.keys()].sort((a, b) => a - b)) { + const text = perSlide.get(segmentId) ?? ''; + if (text === '' || sourceSlides[segmentId] === undefined) continue; + out.push({ language, sourceText: sourceSlides[segmentId], text }); + } + } + return out; +}; + /** Apply a `set_translations` call to the working copy; returns the ids it touched. */ const applySetTranslations = ( args: Record | undefined, sourceSlides: string[], working: WorkingTranslations, changed: Map>, + notes: WorkingNotes, ): void => { const languageResults = ((args?.languages as unknown[]) ?? []) as Array<{ language: string; @@ -445,6 +467,11 @@ const applySetTranslations = ( workingFor(working, language).set(segmentId, unescapeLiteralEscapes(segment.translation ?? '')); if (!changed.has(language)) changed.set(language, new Set()); changed.get(language)?.add(segmentId); + // Rewriting a slide restates its caveat: a note given here replaces whatever + // was there, and a rewrite with no note means the model no longer has one. + const note = (segment.note ?? '').trim(); + if (note) workingFor(notes, language).set(segmentId, note); + else notes.get(language)?.delete(segmentId); } } }; @@ -530,6 +557,11 @@ export type SlideAgentRunResult = { * run never touched are absent, so a targeted edit reports only what it edited. */ translations: Record; + /** + * Every caveat currently attached to a slide, for the whole item — the seeded notes + * carried forward plus whatever this run set or cleared. Replaces the stored set. + */ + notes: SlideReviewNote[]; /** The full updated conversation (raw Gemini Content, replay-safe — keep verbatim). */ messages: Content[]; /** Whether the model called `set_translations` during this run. */ @@ -567,12 +599,17 @@ export const runSlideTranslationAgent = async ( * missing are treated as "not yet translated". */ currentTranslations?: Record; + /** + * Caveats already attached to these slides, carried across rounds so a follow-up + * that touches one slide doesn't silently drop the notes on all the others. + */ + currentNotes?: SlideReviewNote[]; onToolCall?: (call: BibleToolCall) => void; /** PostHog trace/distinct-id tags so every round groups under one conversation. */ observability?: AgentObservability; }, ): Promise => { - const { sourceSlides, messages, bibleLanguages, currentTranslations, onToolCall, observability } = params; + const { sourceSlides, messages, bibleLanguages, currentTranslations, currentNotes, onToolCall, observability } = params; const model = params.model ?? provider.defaultModel; const functionDeclarations: FunctionDeclaration[] = [SET_TRANSLATIONS_TOOL, REVISE_TRANSLATION_TOOL]; @@ -590,6 +627,15 @@ export const runSlideTranslationAgent = async ( } }); } + // Notes are seeded the same way, by matching the stored source text back to a slide id. + // A `revise_translation` deliberately leaves them alone: only a rewrite of the whole + // slide restates (or withdraws) the caveat. + const notes: WorkingNotes = new Map(); + const slideIdByText = new Map(sourceSlides.map((slide, i) => [normalizeSlideText(slide), i])); + for (const note of currentNotes ?? []) { + const segmentId = slideIdByText.get(normalizeSlideText(note.sourceText)); + if (segmentId !== undefined && note.text) workingFor(notes, note.language).set(segmentId, note.text); + } let setTranslationsCalled = false; let usage = emptyUsage(); @@ -622,6 +668,7 @@ export const runSlideTranslationAgent = async ( sourceSlides, working, changed, + notes, ); setTranslationsCalled = true; responseParts.push({ @@ -673,6 +720,7 @@ export const runSlideTranslationAgent = async ( return { translations: collectChanged(working, changed, sourceSlides), + notes: collectNotes(notes, sourceSlides), messages, setTranslationsCalled, usage, @@ -864,13 +912,15 @@ export const draftItemTranslations = async ( onToolCall?: (call: BibleToolCall) => void; /** Receives the full agent conversation (raw Gemini Content) once drafting completes. */ onConversation?: (messages: Content[]) => void; + /** Receives the per-slide caveats the model raised for the reviewer. */ + onNotes?: (notes: SlideReviewNote[]) => void; /** Receives the run's token usage (incl. cache hits) once drafting completes. */ onUsage?: (usage: TokenUsage) => void; /** PostHog trace/distinct-id tags so the draft's generations group by conversation. */ observability?: AgentObservability; }, ): Promise> => { - const { sourceSlides, targets, referenceText, existingTranslation, generalContext, onToolCall, onConversation, onUsage, observability } = params; + const { sourceSlides, targets, referenceText, existingTranslation, generalContext, onToolCall, onConversation, onNotes, onUsage, observability } = params; const itemTitle = params.itemTitle?.trim(); const model = params.model ?? provider.defaultModel; // Languages we can actually fetch canonical Scripture for. @@ -898,6 +948,7 @@ export const draftItemTranslations = async ( observability, }); onConversation?.(result.messages); + onNotes?.(result.notes); onUsage?.(result.usage); return result.translations; }; diff --git a/server.ts b/server.ts index abdffb4..0a87574 100644 --- a/server.ts +++ b/server.ts @@ -13,7 +13,7 @@ import { ElevenLabs, ElevenLabsClient } from '@elevenlabs/elevenlabs-js'; import { PostHog, setupExpressErrorHandler, setupExpressRequestContext } from 'posthog-node'; import { translateBlock, draftItemTranslations, runSlideTranslationAgent, buildSeedConversationPrompt, GeminiProvider, emptyUsage, mergeUsage } from './nlp.ts'; -import type { TranslationTodo, TokenUsage, AgentObservability } from './nlp.ts'; +import type { TranslationTodo, TokenUsage, AgentObservability, SlideReviewNote } from './nlp.ts'; import { BIBLE_TRANSLATIONS, type BibleToolCall } from './bible.ts'; import { SlideLibrary } from './slideLibrary.ts'; import { translateItem } from './src/slideItemTranslation.ts'; @@ -663,6 +663,8 @@ app.post('/api/translateItem', requireWriteKey('/api/translateItem'), async (req const bibleLookups: BibleToolCall[] = []; // The raw agent history, captured so we can persist it for review + follow-ups. let conversationMessages: Content[] = []; + // Per-slide caveats the model raised — the review screen's "look at this one" signal. + let slideNotes: SlideReviewNote[] = []; // Token usage across the draft's model calls (surfaced so cache hits/cost are visible). let usage: TokenUsage = emptyUsage(); const translations = await translateItem({ @@ -686,6 +688,9 @@ app.post('/api/translateItem', requireWriteKey('/api/translateItem'), async (req onConversation: (messages) => { conversationMessages = messages; }, + onNotes: (notes) => { + slideNotes = notes; + }, onUsage: (runUsage) => { usage = mergeUsage(usage, runUsage); }, @@ -717,6 +722,7 @@ app.post('/api/translateItem', requireWriteKey('/api/translateItem'), async (req slidesHash: slidesHash(slides), languages: requestedLanguages, messages: seededMessages, + notes: slideNotes, status: 'idle', usage, }); @@ -768,6 +774,7 @@ app.post('/api/slideConversation/message', requireWriteKey('/api/slideConversati model: STRONG_MODEL, bibleLanguages, currentTranslations, + currentNotes: conversation.notes, observability, onToolCall: (call) => { bibleLookups.push(call); @@ -777,6 +784,8 @@ app.post('/api/slideConversation/message', requireWriteKey('/api/slideConversati }, }); conversation.status = 'idle'; + // The run reports the item's whole note set (seeded from this one), so replace, not merge. + conversation.notes = result.notes; // Fold this run's tokens into the conversation's running total (initial draft + follow-ups). conversation.usage = mergeUsage(conversation.usage ?? emptyUsage(), result.usage); writeConversation(conversationsMap, conversation); diff --git a/sessionExport.test.ts b/sessionExport.test.ts index e4517b0..09b578f 100644 --- a/sessionExport.test.ts +++ b/sessionExport.test.ts @@ -69,7 +69,6 @@ describe('buildSessionExport', () => { doc.getMap('proclaimPresentations').set('i1', { title: 'Reading', slides: ['A verse'] }); doc.getMap('slideTranslations').set(slideTranslationKey('French', 'A verse'), { text: 'Un verset', - status: 'reviewed', provenance: 'human', }); // Audio in German. @@ -103,12 +102,10 @@ describe('buildSessionExport', () => { const translations = doc.getMap('slideTranslations'); translations.set(slideTranslationKey('French', 'Line one'), { text: 'Ligne un', - status: 'reviewed', provenance: 'human', }); translations.set(slideTranslationKey('French', 'A verse'), { text: 'Un verset', - status: 'auto', provenance: 'llm', }); @@ -119,7 +116,7 @@ describe('buildSessionExport', () => { // Service order wins: item-b before item-a. expect(data.presentations.map((p) => p.title)).toEqual(['Reading', 'Hymn']); expect(data.presentations[0].slides[0].translations.French.entry.text).toBe('Un verset'); - expect(data.presentations[0].slides[0].translations.French.entry.status).toBe('auto'); + expect(data.presentations[0].slides[0].translations.French.entry.provenance).toBe('llm'); expect(data.presentations[1].slides[0].translations.French.entry.text).toBe('Ligne un'); // Untranslated slide has no entry. expect(data.presentations[1].slides[1].translations.French).toBeUndefined(); diff --git a/slideConversationStore.ts b/slideConversationStore.ts index da37728..be88199 100644 --- a/slideConversationStore.ts +++ b/slideConversationStore.ts @@ -18,6 +18,7 @@ import * as Y from 'yjs'; import type { DocumentManager } from '@y-sweet/sdk'; import type { Content } from '@google/genai'; import type { TokenUsage } from './nlp.ts'; +import type { SlideReviewNote } from './src/slideTranslation.ts'; import { connectServerDoc, type ServerDoc } from './serverDoc.ts'; export type SlideConversationStatus = 'running' | 'idle' | 'error'; @@ -32,6 +33,12 @@ export interface SlideConversation { languages: string[]; /** Raw Gemini history, stored verbatim so a resume replays the agent faithfully. */ messages: Content[]; + /** + * Per-slide caveats the model raised for the reviewer, keyed by language + source text. + * This is the item's whole current set (each run returns it in full), so it is replaced + * rather than appended to. Absent on conversations created before this existed. + */ + notes?: SlideReviewNote[]; status: SlideConversationStatus; /** * Token usage summed across every agent run for this conversation (the initial draft plus diff --git a/slideLibrary.test.ts b/slideLibrary.test.ts index f42b7b6..7f98719 100644 --- a/slideLibrary.test.ts +++ b/slideLibrary.test.ts @@ -25,16 +25,14 @@ describe('SlideLibrary', () => { expect(lib.lookup('French', 'anything')).toBeUndefined(); }); - it('upserts a reviewed entry and looks it up by normalized text', async () => { + it('upserts an entry and looks it up by normalized text', async () => { const lib = new SlideLibrary(filePath); await lib.load(); await lib.upsert({ language: 'French', sourceText: 'Praise the Lord', text: 'Louez le Seigneur' }); const entry = lib.lookup('French', ' Praise the Lord '); expect(entry?.text).toBe('Louez le Seigneur'); - expect(entry?.status).toBe('reviewed'); expect(entry?.provenance).toBe('human'); - expect(entry?.reviewedAt).toBeTypeOf('number'); }); it('persists across reloads from a fresh instance', async () => { diff --git a/slideLibrary.ts b/slideLibrary.ts index a8dcb19..a74a06e 100644 --- a/slideLibrary.ts +++ b/slideLibrary.ts @@ -1,10 +1,10 @@ /** * Persistent, file-backed slide-translation library (server-side). * - * This is the durable "reviewed" tier from the plan: it outlives the per-day Yjs - * doc and is reused across services (canonical Bible/creed texts, past human - * reviews). Auto/machine fallbacks are NOT stored here — those live in the per-day - * doc. The library therefore only ever holds `reviewed` entries. + * This is the durable tier: it outlives the per-day Yjs doc and is reused across + * services (canonical Bible/creed texts, past human edits). Machine drafts are NOT + * stored here — those live in the per-day doc. An entry is in the library because a + * human saved it. * * Storage is a single JSON file written atomically (temp file + rename). Writes are * serialized through a promise chain so concurrent upserts can't interleave. @@ -59,20 +59,22 @@ export class SlideLibrary { const parsed = JSON.parse(raw) as LibraryFile; this.records.clear(); for (const record of parsed.entries ?? []) { - this.records.set(slideTranslationKey(record.language, record.sourceText), record); + // Rebuild rather than store verbatim, so fields dropped from the shape (the old + // status/reviewedAt pair) don't ride along into the next write. + this.records.set(slideTranslationKey(record.language, record.sourceText), { + language: record.language, + sourceText: record.sourceText, + text: record.text, + provenance: record.provenance, + }); } } private toEntry(record: SlideLibraryRecord): SlideTranslationEntry { - return { - text: record.text, - status: record.status, - provenance: record.provenance, - reviewedAt: record.reviewedAt, - }; + return { text: record.text, provenance: record.provenance }; } - /** Look up the reviewed entry for a concrete language + slide text, if any. */ + /** Look up the stored entry for a concrete language + slide text, if any. */ lookup(language: string, slideText: string): SlideTranslationEntry | undefined { const record = this.records.get(slideTranslationKey(language, slideText)); return record ? this.toEntry(record) : undefined; @@ -83,7 +85,7 @@ export class SlideLibrary { return (language, slideText) => this.lookup(language, slideText); } - /** All stored records (reviewed entries), sorted by language then source text. */ + /** All stored records, sorted by language then source text. */ list(): SlideLibraryRecord[] { return [...this.records.values()].sort( (a, b) => @@ -92,7 +94,7 @@ export class SlideLibrary { } /** - * Create or replace a reviewed translation, then persist. The source text is + * Create or replace a stored translation, then persist. The source text is * normalized so keys are stable regardless of incidental whitespace. */ async upsert(input: UpsertInput): Promise { @@ -101,9 +103,7 @@ export class SlideLibrary { language: input.language, sourceText, text: input.text, - status: 'reviewed', provenance: input.provenance ?? 'human', - reviewedAt: Date.now(), }; this.records.set(slideTranslationKey(input.language, sourceText), record); await this.persist(); diff --git a/slide_translator.py b/slide_translator.py index 2a0e3c6..f1e70e7 100644 --- a/slide_translator.py +++ b/slide_translator.py @@ -28,7 +28,7 @@ logger = logging.getLogger(__name__) # (slides, item_title, item_id, existing_translation, doc_id) -# -> {language: [{text, status, provenance}]} +# -> {language: [{text, provenance}]} # doc_id names the doc this session is bound to, so the server can write the agent # conversation into the same per-day doc. It arrives via bind() rather than being closed over # at construction, because a date rollover swaps the doc underneath a long-lived translator. @@ -118,8 +118,8 @@ def _scan_order(self, snap: FeedSnapshot) -> List[str]: def _has_missing_translation(self, item: FeedItem) -> bool: """True if any non-empty slide lacks a translation in any target language. - Reads the live map (not the snapshot) so reviewed entries written by the frontend - count as present. + Reads the live map (not the snapshot) so entries written by the frontend count as + present. """ for language in self.languages: for slide in item.slides: @@ -130,7 +130,13 @@ def _has_missing_translation(self, item: FeedItem) -> bool: return False def _store_translations(self, slides: List[str], translations: Dict[str, Any]) -> None: - """Seed per-slide results into slideTranslations, never clobbering reviewed entries.""" + """Seed per-slide results into slideTranslations, filling gaps only. + + Keys are content-addressed, so an existing entry is already a translation of this + exact text — whoever wrote it (a human saving in the review screen, an earlier + draft) knows at least as much as this run does. We only ever fill in what is + missing; nothing here overwrites. + """ assert self.ydoc is not None, "_store_translations before bind()" with self.ydoc.transaction(): for language, per_slide in translations.items(): @@ -138,11 +144,9 @@ def _store_translations(self, slides: List[str], translations: Dict[str, Any]) - if not slide.strip() or not entry: continue key = slide_translation_key(language, slide) - existing = self.translations_map[key] if key in self.translations_map else None - if existing is not None and existing.get('status') == 'reviewed': + if key in self.translations_map: continue self.translations_map[key] = { 'text': entry.get('text', ''), - 'status': entry.get('status', 'auto'), 'provenance': entry.get('provenance', 'llm'), } diff --git a/src/SlideReview.test.tsx b/src/SlideReview.test.tsx index 4406703..6ad0dc1 100644 --- a/src/SlideReview.test.tsx +++ b/src/SlideReview.test.tsx @@ -5,6 +5,7 @@ import { SlideReview } from './SlideReview'; const baseProps = { slides: ['Praise the Lord', 'Forever'], languages: ['French'] as const, + notes: { French: [null, null] as (string | null)[] }, editable: true, busy: false, }; @@ -17,6 +18,7 @@ describe('SlideReview', () => { slides={[]} drafts={{ French: [] }} savedTexts={{ French: [] }} + notes={{ French: [] }} onDraftChange={vi.fn()} onSaveCell={vi.fn()} />, @@ -71,7 +73,7 @@ describe('SlideReview', () => { expect(onSaveCell).toHaveBeenCalledWith('French', 0); }); - it('marks a cell reviewed and disables Save when the draft matches the saved text', () => { + it('marks a cell saved and disables Save when the draft matches the saved text', () => { render( { onSaveCell={vi.fn()} />, ); - expect(screen.getByText(/Reviewed/)).toBeInTheDocument(); + expect(screen.getByText(/Saved/)).toBeInTheDocument(); expect(screen.getAllByRole('button', { name: 'Save' })[0]).toBeDisabled(); }); + it('shows the translator note above the cell it belongs to, and marks the row', () => { + render( + , + ); + expect(screen.getByText(/Ambiguous: "forever"/)).toBeInTheDocument(); + // Exactly one row is flagged — the one the note belongs to. + expect(screen.getAllByLabelText('Translator notes')).toHaveLength(1); + }); + it('disables editing when not editable', () => { render( ; - /** The translation currently saved as reviewed in the library (null if none). */ + /** The translation currently saved in the library (null if none). */ savedTexts: Record; + /** + * The translator's caveat for a cell, keyed by language then slide index (null = none). + * These are the reason to be on this screen at all, so they read before the text. + */ + notes: Record; editable: boolean; /** True while a network action (suggest/save) is in flight. */ busy: boolean; @@ -15,20 +20,21 @@ export interface SlideReviewProps { onSaveCell: (language: string, slideIndex: number) => void; } -type CellState = 'reviewed' | 'unsaved' | 'empty'; +type CellState = 'saved' | 'unsaved' | 'empty'; function cellState(draft: string, savedText: string | null): CellState { if (draft.trim() === '') return 'empty'; if (savedText !== null && normalizeSlideText(savedText) === normalizeSlideText(draft)) { - return 'reviewed'; + return 'saved'; } return 'unsaved'; } /** * Pure grid for reviewing/editing slide translations: one row per slide, one column - * per target language, plus the source text. Each cell shows the draft translation, - * a saved/unsaved status chip, and a per-cell Save button. All data and persistence + * per target language, plus the source text. A cell whose translation the translator + * flagged leads with that note; the row is marked too, so a long item can be skimmed + * for the handful of slides that actually want attention. All data and persistence * are owned by the container. */ export function SlideReview({ @@ -36,6 +42,7 @@ export function SlideReview({ languages, drafts, savedTexts, + notes, editable, busy, onDraftChange, @@ -47,6 +54,10 @@ export function SlideReview({ return
{s.noSlidesToReview}
; } + const rowHasNote = slides.map((_, slideIndex) => + languages.some((language) => notes[language]?.[slideIndex]), + ); + return (
@@ -63,15 +74,30 @@ export function SlideReview({ - + {languages.map((language) => { const draft = drafts[language]?.[slideIndex] ?? ''; const savedText = savedTexts[language]?.[slideIndex] ?? null; + const note = notes[language]?.[slideIndex] ?? null; const state = cellState(draft, savedText); return (
{slide} + {rowHasNote[slideIndex] && ( + + 📝 + + )} + {slide} + + {note && ( +

+ {note} +

+ )}