diff --git a/docs/slide-translations-plan.md b/docs/slide-translations-plan.md index 2157a1a..f413486 100644 --- a/docs/slide-translations-plan.md +++ b/docs/slide-translations-plan.md @@ -384,3 +384,31 @@ 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. `'imported'` went with the tier — it existed + only for the `firstDraftBySlide` path the alignment rethink removed (Phase C-import above), + so it had no writer left. `'bible'`/`'creed'` are still unwritten but kept: they are the + vocabulary for the agent *claiming* a slide is the published wording, which is worth having + because a harness can check the claim against the looked-up source. +- `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..5468df3 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; @@ -237,7 +238,7 @@ export type DraftItemTarget = { language: string; /** Per source slide: does this slide still need translating into this language? */ isTranslationNeeded: boolean[]; - /** Already-reviewed translations in this language, joined for style/terminology context. */ + /** Translations already in the library for this language, joined for style/terminology context. */ context: string; }; @@ -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, @@ -681,7 +729,7 @@ export const runSlideTranslationAgent = async ( /** * Build the opening user prompt for the slide-translation agent: the numbered source slides, - * the target languages (each with which slides still need translating and any reviewed + * the target languages (each with which slides still need translating and any library * translations as style/terminology context), an optional multilingual `referenceText` dump, * Bible-grounding instructions when any target language has a canonical translation, and the * instruction to record results via the `set_translations` tool. @@ -832,7 +880,7 @@ useful to say.`; * Translate a whole item into several languages in one agent loop. * * The model receives the numbered source slides, the target languages (each with which - * slides still need translating and any already-reviewed translations as context), and an + * slides still need translating and any library translations as context), and an * optional free-text `referenceText` dump that may hold prior translations in one or more of * the target languages. It may call `lookup_bible_passage` to ground Scripture, then records * results via `set_translations`. `onToolCall` reports each Bible lookup; `onConversation` @@ -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/proclaim_service.py b/proclaim_service.py index d30e9b7..0926efb 100644 --- a/proclaim_service.py +++ b/proclaim_service.py @@ -71,7 +71,7 @@ TRANSLATION_SCAN_INTERVAL = float(os.getenv('PROCLAIM_TRANSLATION_SCAN_INTERVAL', '1.0')) # seconds # Target languages to pre-translate slides into (must match the frontend's configured # languages). The translator asks the server to translate the active item into these and -# writes the reviewed-or-auto results into the per-day slideTranslations map. +# fills any missing keys in the per-day slideTranslations map with the results. SLIDE_TRANSLATION_LANGUAGES = [ lang.strip() for lang in os.getenv('SLIDE_TRANSLATION_LANGUAGES', 'French,Haitian Creole,Spanish').split(',') @@ -209,7 +209,7 @@ def announce(doc: Doc, doc_id: str) -> None: def make_translate_fn(ysweet_url: str, languages: List[str], write_key: Optional[str] = None): """Build the translation call the SlideTranslator injects: POST /api/translateItem. - Returns the ``{language: [{text, status, provenance}, ...]}`` map, or None on failure + Returns the ``{language: [{text, provenance}, ...]}`` map, or None on failure (translation is best-effort; a failure must not drop the session). """ async def translate( diff --git a/server.ts b/server.ts index abdffb4..b5a5884 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,11 +784,13 @@ 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); - // Flatten revised translations for the browser to apply (auto / llm-agent provenance). + // Flatten revised translations for the browser to apply. const updatedTranslations: Array<{ language: string; sourceText: string; text: string }> = []; for (const [language, blocks] of Object.entries(result.translations)) { for (const block of blocks) { 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..fa10509 100644 --- a/slide_translator.py +++ b/slide_translator.py @@ -7,9 +7,9 @@ worker read state back out of the maps it was feeding. It does, however, read *and* write the live ``slideTranslations`` map: cache-miss detection -must see ``reviewed`` entries that the frontend review screen writes there, and its own -seeding must never clobber them. Each content version is attempted once (tracked by slides -hash) so a failed/partial translation doesn't spin. +must see entries the frontend review screen writes there, and its own seeding only ever fills +keys that are missing, so it can't clobber them. Each content version is attempted once +(tracked by slides hash) so a failed/partial translation doesn't spin. The actual translation call is injected (``translate_fn``) so the source of translations (the server's ``/api/translateItem`` in production, a fake in tests) is swappable. @@ -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} +

+ )}