From 1c0ccb66179c959e335fdac7ba6a9036363e2f2e Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 17:22:56 +0000 Subject: [PATCH] Surface the translator's per-slide notes on the entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit set_translations always accepted a per-segment note ("a genuine ambiguity or judgement call worth flagging") and we threw it away. It now rides on the slide translation entry as `note?`, next to provenance. A note is a caveat about one text, so whatever replaces the text replaces the note: set_translations brings its own or none, revise_translation drops it, and a reviewer's Save drops it. The model's wording stays in the stored conversation. Because the note lives on the entry, nothing has to seed notes into a follow-up run or replace an item-wide set: a slide the run doesn't write keeps its note in the doc. Review screen: a note shows above its cell, and the item picker appends "· 📝 n" for items whose doc entries carry notes. The library never stores a note. The tool description now tells the model the note is displayed and that rewriting withdraws it. Second half of #152, stacked on the tier removal. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01F2MPHXNLbLbVZuEDZToVvz --- docs/slide-translations-plan.md | 10 +++++ nlp.test.ts | 42 ++++++++++++++++++ nlp.ts | 32 ++++++++++++-- server.ts | 4 +- slideSync.test.ts | 10 +++++ slideSync.ts | 6 ++- src/SlideReview.test.tsx | 16 +++++++ src/SlideReview.tsx | 13 ++++-- src/SlideReviewContainer.tsx | 73 ++++++++++++++++++++++++-------- src/slideItemTranslation.test.ts | 16 +++++++ src/slideItemTranslation.ts | 16 ++++--- src/slideTranslation.test.ts | 13 ++++++ src/slideTranslation.ts | 24 +++++++++++ src/slideTranslationApi.ts | 2 + 14 files changed, 245 insertions(+), 32 deletions(-) diff --git a/docs/slide-translations-plan.md b/docs/slide-translations-plan.md index 41bce9a..6449e81 100644 --- a/docs/slide-translations-plan.md +++ b/docs/slide-translations-plan.md @@ -405,3 +405,13 @@ fix mid-service, which is noise they can't act on. What replaced it: only**. Keys are content-addressed, so an existing entry already translates this exact text, and it may be a reviewer's save or an agent follow-up's revision that a fresh draft must not undo. "Protect reviewed" was a special case of that rule. + +**Notes, surfaced (post-build).** `set_translations` always let the model attach a +per-segment `note`, and we dropped it. It now rides on the entry, `{ text, provenance, +note? }`, because a note is a caveat about *that text*: whatever replaces the text replaces +the note. A rewrite through `set_translations` brings its own note or none, a +`revise_translation` drops it, and a reviewer's Save drops it — the human has dealt with it. +The model's original wording stays in the stored conversation either way. Notes are not +seeded into a follow-up run: a slide the run doesn't write keeps its note where it is stored. +The library never stores one. The review screen shows a note above its cell and counts an +item's notes in the picker. diff --git a/nlp.test.ts b/nlp.test.ts index 6271e99..3381f85 100644 --- a/nlp.test.ts +++ b/nlp.test.ts @@ -418,6 +418,48 @@ describe('revise_translation (targeted edits)', () => { }); }); +describe('per-slide notes', () => { + const slides = ['Praise the Lord', 'Forever']; + const set = (segments: object[]) => ({ + name: 'set_translations', + args: { languages: [{ language: 'French', segments }] }, + }); + const run = (rounds: Array>) => + runSlideTranslationAgent(fakeScriptedProvider(rounds), { + sourceSlides: slides, + messages: [{ role: 'user', parts: [{ text: 'prompt' }] }], + bibleLanguages: [], + }); + + it('attaches a note to the slide it was given with, trimmed, and to no other', async () => { + const result = await run([ + [set([ + { segmentId: 0, translation: 'Louez le Seigneur', note: ' Seigneur or Éternel? ' }, + { segmentId: 1, translation: 'Pour toujours', note: ' ' }, + ])], + ]); + expect(result.translations.French.map((r) => r.note)).toEqual(['Seigneur or Éternel?', undefined]); + }); + + it('a rewrite without a note withdraws the one the slide had', async () => { + const result = await run([ + [set([{ segmentId: 0, translation: 'Louez le Seigneur', note: 'Seigneur or Éternel?' }])], + [set([{ segmentId: 0, translation: "Louez l'Éternel" }])], + ]); + expect(result.translations.French[0]).not.toHaveProperty('note'); + }); + + it('a revision withdraws the note, which stays in the conversation', async () => { + const result = await run([ + [set([{ segmentId: 0, translation: 'Louez le Seigneur', note: 'Seigneur or Éternel?' }])], + [{ name: 'revise_translation', args: { language: 'French', segmentId: 0, find: 'le Seigneur', replace: "l'Éternel" } }], + ]); + expect(result.translations.French[0]).toMatchObject({ translatedText: "Louez l'Éternel" }); + expect(result.translations.French[0]).not.toHaveProperty('note'); + expect(JSON.stringify(result.messages)).toContain('Seigneur or Éternel?'); + }); +}); + describe('token usage accounting', () => { it('sums usageMetadata across rounds, tolerating missing fields', () => { let usage = emptyUsage(); diff --git a/nlp.ts b/nlp.ts index 2306638..ac7a460 100644 --- a/nlp.ts +++ b/nlp.ts @@ -30,6 +30,8 @@ export type TranslationTodo = { export type TranslationBlockResult = { sourceText: string; translatedText: string; + /** The model's caveat for the reviewer on this slide, if it raised one (slide agent only). */ + note?: string; } /** @@ -344,8 +346,11 @@ 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, shown beside this slide on ' + + 'the review screen and counted on its item. Omit unless there ' + + 'is a genuine ambiguity or judgement call worth flagging. It ' + + 'belongs to this text: rewriting or revising the slide ' + + 'withdraws it unless you restate it here.', }, }, }, @@ -425,11 +430,19 @@ const workingFor = (working: WorkingTranslations, language: string): Map | undefined, sourceSlides: string[], working: WorkingTranslations, + notes: WorkingNotes, changed: Map>, ): void => { const languageResults = ((args?.languages as unknown[]) ?? []) as Array<{ @@ -443,6 +456,9 @@ const applySetTranslations = ( const segmentId = segment.segmentId; if (!(segmentId >= 0 && segmentId < sourceSlides.length)) continue; workingFor(working, language).set(segmentId, unescapeLiteralEscapes(segment.translation ?? '')); + const note = segment.note?.trim(); + if (note) workingFor(notes, language).set(segmentId, note); + else notes.get(language)?.delete(segmentId); if (!changed.has(language)) changed.set(language, new Set()); changed.get(language)?.add(segmentId); } @@ -460,6 +476,7 @@ const applyReviseTranslation = ( args: Record | undefined, sourceSlides: string[], working: WorkingTranslations, + notes: WorkingNotes, changed: Map>, ): Record => { const language = typeof args?.language === 'string' ? args.language : ''; @@ -498,6 +515,7 @@ const applyReviseTranslation = ( const updated = current.slice(0, first) + replace + current.slice(first + find.length); workingFor(working, language).set(segmentId, updated); + notes.get(language)?.delete(segmentId); if (!changed.has(language)) changed.set(language, new Set()); changed.get(language)?.add(segmentId); return { ok: true, language, segmentId, text: updated }; @@ -506,17 +524,20 @@ const applyReviseTranslation = ( /** Collect the slides the run changed, per language, from the working copy. */ const collectChanged = ( working: WorkingTranslations, + notes: WorkingNotes, changed: Map>, sourceSlides: string[], ): Record => { const out: Record = {}; for (const [language, ids] of changed) { const perSlide = working.get(language); + const notesFor = notes.get(language); out[language] = [...ids] .sort((a, b) => a - b) .map((segmentId) => ({ sourceText: sourceSlides[segmentId], translatedText: perSlide?.get(segmentId) ?? '', + ...(notesFor?.has(segmentId) && { note: notesFor.get(segmentId) }), language, })); } @@ -582,6 +603,9 @@ export const runSlideTranslationAgent = async ( // Seeded with what already exists, so an edit-only run has something to edit. `changed` // stays empty until the model actually writes, so seeds are never reported as updates. const working: WorkingTranslations = new Map(); + // Notes are not seeded: a slide this run doesn't write keeps whatever note it has where + // it is stored, and a slide it does write gets a fresh one (or none). + const notes: WorkingNotes = new Map(); const changed = new Map>(); for (const [language, perSlide] of Object.entries(currentTranslations ?? {})) { perSlide.forEach((text, segmentId) => { @@ -621,6 +645,7 @@ export const runSlideTranslationAgent = async ( call.args as Record | undefined, sourceSlides, working, + notes, changed, ); setTranslationsCalled = true; @@ -635,6 +660,7 @@ export const runSlideTranslationAgent = async ( call.args as Record | undefined, sourceSlides, working, + notes, changed, ), }, @@ -672,7 +698,7 @@ export const runSlideTranslationAgent = async ( } return { - translations: collectChanged(working, changed, sourceSlides), + translations: collectChanged(working, notes, changed, sourceSlides), messages, setTranslationsCalled, usage, diff --git a/server.ts b/server.ts index 785b2ab..f4fd29c 100644 --- a/server.ts +++ b/server.ts @@ -901,10 +901,10 @@ app.post('/api/slideConversation/message', requireWriteKey('/api/slideConversati writeConversation(conversationsMap, conversation); // Flatten revised translations for the browser to apply. - const updatedTranslations: Array<{ language: string; sourceText: string; text: string }> = []; + const updatedTranslations: Array<{ language: string; sourceText: string; text: string; note?: string }> = []; for (const [language, blocks] of Object.entries(result.translations)) { for (const block of blocks) { - updatedTranslations.push({ language, sourceText: block.sourceText, text: block.translatedText }); + updatedTranslations.push({ language, sourceText: block.sourceText, text: block.translatedText, note: block.note }); } } return res.json({ ok: true, conversation, updatedTranslations }); diff --git a/slideSync.test.ts b/slideSync.test.ts index 8f3c9c2..d91f971 100644 --- a/slideSync.test.ts +++ b/slideSync.test.ts @@ -133,6 +133,16 @@ describe('storeTranslations', () => { expect(map.size).toBe(1); }); + it("keeps the model's note with the text it was written for", () => { + const doc = new Y.Doc(); + storeTranslations(doc, ['Hello'], { French: [{ text: 'Bonjour', provenance: 'llm', note: 'Salut?' }] }); + expect(doc.getMap('slideTranslations').get(slideTranslationKey('French', 'Hello'))).toEqual({ + text: 'Bonjour', + provenance: 'llm', + note: 'Salut?', + }); + }); + it('never overwrites an existing entry', () => { // A reviewer's save and an agent follow-up's revision look alike here; a fresh draft // of the same source text must not undo either. diff --git a/slideSync.ts b/slideSync.ts index 1dd771f..58978d2 100644 --- a/slideSync.ts +++ b/slideSync.ts @@ -312,7 +312,11 @@ export function storeTranslations( if (!slide.trim() || !entry) return; const key = slideTranslationKey(language, slide); if (map.has(key)) return; - map.set(key, { text: entry.text ?? '', provenance: entry.provenance ?? 'llm' }); + map.set(key, { + text: entry.text ?? '', + provenance: entry.provenance ?? 'llm', + ...(entry.note && { note: entry.note }), + }); }); } }); diff --git a/src/SlideReview.test.tsx b/src/SlideReview.test.tsx index 39b12f7..b3fc150 100644 --- a/src/SlideReview.test.tsx +++ b/src/SlideReview.test.tsx @@ -7,6 +7,7 @@ const baseProps = { languages: ['French'] as const, editable: true, busy: false, + notes: {}, }; describe('SlideReview', () => { @@ -85,6 +86,21 @@ describe('SlideReview', () => { expect(screen.getAllByRole('button', { name: 'Save' })[0]).toBeDisabled(); }); + it("shows the model's note on its cell only", () => { + render( + , + ); + expect(screen.getAllByText(/📝/)).toHaveLength(1); + expect(screen.getByText(/could also be "pour toujours"/)).toBeInTheDocument(); + }); + it('disables editing when not editable', () => { render( ; /** The translation currently saved in the library (null if none). */ savedTexts: Record; + /** The model's caveat on each draft (null if none), keyed like `drafts`. */ + notes: Record; editable: boolean; /** True while a network action (suggest/save) is in flight. */ busy: boolean; @@ -27,15 +29,16 @@ function cellState(draft: string, savedText: string | null): CellState { /** * 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 - * are owned by the container. + * per target language, plus the source text. Each cell shows the model's note (if any), + * the draft translation, a saved/unsaved status chip, and a per-cell Save button. All data + * and persistence are owned by the container. */ export function SlideReview({ slides, languages, drafts, savedTexts, + notes, editable, busy, onDraftChange, @@ -70,8 +73,12 @@ export function SlideReview({ const draft = drafts[language]?.[slideIndex] ?? ''; const savedText = savedTexts[language]?.[slideIndex] ?? null; const state = cellState(draft, savedText); + const note = notes[language]?.[slideIndex]; return ( + {note && ( +

📝 {note}

+ )}