diff --git a/internal/llm/lang_test.go b/internal/llm/lang_test.go index 55de972..d5056a7 100644 --- a/internal/llm/lang_test.go +++ b/internal/llm/lang_test.go @@ -44,7 +44,7 @@ func TestPromptsNameTheWritersLanguage(t *testing.T) { t.Fatalf("collocation prompt lost its tone guidance:\n%s", collocation) } - translate := TranslateMessages("Try a shorter sentence here.", pt)[0].Content + translate := TranslateMessages("Try a shorter sentence here.", English, pt)[0].Content if !strings.Contains(translate, "European Portuguese") || strings.Contains(translate, "Chinese") { t.Fatalf("translate prompt targets the wrong language:\n%s", translate) } @@ -76,7 +76,7 @@ func TestDefaultPairStillReadsAsBefore(t *testing.T) { if got := CollocationMessages("x", "", EnglishTarget(zh))[0].Content; !strings.Contains(got, "Simplified Chinese (Mandarin) gloss in parentheses") { t.Fatalf("zh collocation gloss changed:\n%s", got) } - if got := TranslateMessages("x", zh)[0].Content; !strings.Contains(got, "natural, friendly Simplified Chinese (Mandarin)") { + if got := TranslateMessages("x", English, zh)[0].Content; !strings.Contains(got, "natural, friendly Simplified Chinese (Mandarin)") { t.Fatalf("zh translate target changed:\n%s", got) } if got := AskPetalSystemPrompt("a", "b", "c", "d", "e", zh); !strings.Contains(got, "为什么") { diff --git a/internal/llm/prompts.go b/internal/llm/prompts.go index d0f8c04..1eba7fa 100644 --- a/internal/llm/prompts.go +++ b/internal/llm/prompts.go @@ -329,23 +329,29 @@ func RewriteMessages(text, style string) []Message { } // translateSystemPrompt drives the explanation translator: it renders a -// suggestion's English explanation into the writer's own language so an ESL -// reader sees the "why" in her first language. Strict about returning ONLY the -// translation (no quotes, no romanisation, no English echo) so it can drop -// straight into the chat bubble. Kept warm and plain — these are short, friendly -// one-liners. -const translateSystemPrompt = `You are Petal, a warm writing assistant. Translate the English text the user ` + - `sends into natural, friendly %[1]s. It is a short explanation of a writing ` + - `suggestion, written for a native %[1]s speaker learning English. +// suggestion's explanation into the half of the pair the explanation is not +// already in, so the "why" is readable from both sides. Strict about returning +// ONLY the translation (no quotes, no romanisation, no echo of the source) so it +// can drop straight into the chat bubble. Kept warm and plain — these are short, +// friendly one-liners. +// +// Both languages are parameters because neither end is a constant. Until Phase +// 28 the source was always English and the destination always hers; a document +// written in her own language is explained in her own language, and then the tap +// runs the other way, into the English she is practising. +const translateSystemPrompt = `You are Petal, a warm writing assistant. Translate the %[1]s text the user ` + + `sends into natural, friendly %[2]s. It is a short explanation of a writing ` + + `suggestion, written for someone who is learning one of %[1]s and %[2]s and reads the other most easily. -Respond with ONLY the %[1]s translation. No quotation marks, no romanisation, no English, no preamble — ` + +Respond with ONLY the %[2]s translation. No quotation marks, no romanisation, no %[1]s, no preamble — ` + `just the translated sentence.` -// TranslateMessages builds the message array for translating one short English -// explanation into the writer's own language. -func TranslateMessages(text string, lang Lang) []Message { +// TranslateMessages builds the message array for rendering one short +// explanation out of the language it arrived in and into the other half of the +// writer's pair. +func TranslateMessages(text string, from, to Lang) []Message { return []Message{ - {Role: "system", Content: fmt.Sprintf(translateSystemPrompt, lang.Name)}, + {Role: "system", Content: fmt.Sprintf(translateSystemPrompt, from.Name, to.Name)}, {Role: "user", Content: text}, } } diff --git a/internal/llm/translate.go b/internal/llm/translate.go index 64f9650..6a35a2a 100644 --- a/internal/llm/translate.go +++ b/internal/llm/translate.go @@ -4,13 +4,14 @@ import ( "context" ) -// RunTranslate renders a short English explanation into the writer's own -// language. It is a one-shot Complete (the result seeds the Ask Petal bubble), -// kept at a low temperature so the translation is faithful rather than creative. Output is -// trimmed of any stray surrounding quotes the model may add. -func RunTranslate(ctx context.Context, client LLMClient, text string, lang Lang) (string, error) { +// RunTranslate renders a short explanation out of the language it was written +// in and into the other half of the writer's pair. It is a one-shot Complete +// (the result seeds the Ask Petal bubble), kept at a low temperature so the +// translation is faithful rather than creative. Output is trimmed of any stray +// surrounding quotes the model may add. +func RunTranslate(ctx context.Context, client LLMClient, text string, from, to Lang) (string, error) { out, err := client.Complete(ctx, CompletionRequest{ - Messages: TranslateMessages(text, lang), + Messages: TranslateMessages(text, from, to), MaxTokens: 512, Temperature: 0.2, TopP: 0.9, diff --git a/internal/suggestions/doclang_test.go b/internal/suggestions/doclang_test.go index 2dc752b..c5dbd0c 100644 --- a/internal/suggestions/doclang_test.go +++ b/internal/suggestions/doclang_test.go @@ -371,10 +371,15 @@ func seedExplanation(t *testing.T, h *Handler, docID, explanation string) string // The tap-through has to read the same decision the card was written under. On a // Portuguese document by a Portuguese writer the explanation already arrived in -// Portuguese, and the old endpoint would have sent it to the model to be -// rendered into Portuguese again. -func TestTranslateSkipsWhenTheExplanationIsAlreadyHers(t *testing.T) { - client := &stubClient{response: "Não devia ser chamado."} +// Portuguese, so the destination is the other half of the pair — the English she +// is practising — and never Portuguese into Portuguese again. +// +// This pins the report that "Petal presents the Ask Petal advice in both +// sections as Portuguese": the endpoint used to answer "" here, which left the +// card Portuguese, the bubble beneath it the same Portuguese, and no English on +// the card at all for a writer whose whole reason for the pair is English. +func TestTranslateRendersHerExplanationIntoTheEnglishSheIsLearning(t *testing.T) { + client := &stubClient{response: "Use this one here."} srv, docID, h := newDirectedServer(t, client, "pt-PT", auth.DirectionLearningEn, ptDocument) setDocLang(t, h, docID, docLangPair) sugID := seedExplanation(t, h, docID, "Aqui usa-se isto.") @@ -387,11 +392,14 @@ func TestTranslateSkipsWhenTheExplanationIsAlreadyHers(t *testing.T) { if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { t.Fatalf("decode: %v", err) } - if out.Translation != "" { - t.Fatalf("translation = %q, want empty: the bubble seeds from the explanation itself", out.Translation) + if out.Translation == "" { + t.Fatal("a Portuguese explanation left the tap with nowhere to go") } - if client.calls != 0 { - t.Fatal("the model was asked to render Portuguese into Portuguese") + if !strings.Contains(client.lastPrompt, "into natural, friendly English") { + t.Fatalf("translate didn't render into English:\n%s", client.lastPrompt) + } + if strings.Contains(client.lastPrompt, "into natural, friendly European Portuguese") { + t.Fatalf("the model was asked to render Portuguese into Portuguese:\n%s", client.lastPrompt) } } diff --git a/internal/suggestions/translate.go b/internal/suggestions/translate.go index a3efca3..e3015d2 100644 --- a/internal/suggestions/translate.go +++ b/internal/suggestions/translate.go @@ -30,11 +30,18 @@ type translateResponse struct { // this endpoint has to read the same decision back, or it round-trips Portuguese // into Portuguese and calls it a translation. // -// So: render into whichever half the explanation is NOT already in, and when the -// explanation already arrived in the language this bubble exists to reach her -// in, skip the model call and answer "". The client seeds the bubble with the -// explanation itself when the translation comes back empty, which is exactly -// right — there is nothing to add. +// So: render into whichever half the explanation is NOT already in — and that +// is the whole rule, in both directions. When it first shipped this endpoint +// answered "" for a Portuguese explanation on the reasoning that an English +// rendering she hadn't asked for was noise. It was reported as the opposite: a +// writer whose pair is Portuguese and English, learning English, met a +// Portuguese card with a Portuguese bubble under it and no English anywhere on +// the card. The tap is the one place the other language was promised, and the +// half she is *practising* is exactly the half worth a tap. +// +// The bubble sits directly beneath the explanation inside the card, so between +// the two the writer always has both languages, whichever way round the document +// put them. func (h *Handler) translate(w http.ResponseWriter, r *http.Request) { sugID := chi.URLParam(r, "id") @@ -69,15 +76,14 @@ func (h *Handler) translate(w http.ResponseWriter, r *http.Request) { // today; the alternative is a language column on every suggestion row, and the // cost of being wrong is one bubble seeded in the language it was already in. target := targetFor(pairLang, direction, docLang) - if target.Explain.Code == target.Pair.Code { - // Already in her language. The other half is English — the language she is - // practising — and an unasked-for English rendering of an explanation she - // can already read is not a seed, it's noise. - httputil.WriteJSON(w, http.StatusOK, translateResponse{Translation: ""}) - return + from, to := target.Explain, target.Pair + if from.Code == to.Code { + // The explanation is already in her language, so the half this tap has to + // reach is the other one: the English she is practising. + to = llm.English } - out, err := llm.RunTranslate(r.Context(), h.Client, explanation, target.Pair) + out, err := llm.RunTranslate(r.Context(), h.Client, explanation, from, to) if err != nil { httputil.UpstreamError(w, "translate", err) return diff --git a/web/src/components/Editor/AskPetal.tsx b/web/src/components/Editor/AskPetal.tsx index 0d5580f..ea58211 100644 --- a/web/src/components/Editor/AskPetal.tsx +++ b/web/src/components/Editor/AskPetal.tsx @@ -5,12 +5,11 @@ import { splitBilingual } from './bilingualReply' import { fromIME } from '../../lib/ime' interface Props { + // The card's explanation is deliberately NOT passed in. Petal's opening + // bubble is that explanation rendered into the other half of the pair, + // fetched on open from this id — and when that can't be had, the panel has + // nothing to say rather than a second copy of the card. suggestionId: string - // The English explanation (shown in the card body). Petal's opening bubble is - // its translation into the pair language, fetched on open — so the panel - // doesn't just repeat the same English text twice. Falls back to this on - // failure. - explanation: string } // CJK fallback stack — Nunito has no Chinese glyphs, and on the zh pair both @@ -44,10 +43,11 @@ const CHAT_MIN_PX = 160 // conversation lives in this component's state — nothing is persisted; closing // the card (unmounting) clears it. Each send streams Petal's reply token-by- // token into the latest assistant bubble. -export function AskPetal({ suggestionId, explanation }: Props) { +export function AskPetal({ suggestionId }: Props) { const t = usePack() - // Opening bubble starts empty (caret-only) and fills with the pair-language - // translation once it lands; `seeding` drives that loading caret. + // Opening bubble starts empty (caret-only) and fills with the other half of + // the pair once it lands; `seeding` drives that loading caret, and the bubble + // goes away entirely if there turns out to be nothing to put in it. const [messages, setMessages] = useState([{ role: 'assistant', content: '' }]) const [seeding, setSeeding] = useState(true) const [input, setInput] = useState('') @@ -82,23 +82,31 @@ export function AskPetal({ suggestionId, explanation }: Props) { inputRef.current?.focus({ preventScroll: true }) }, []) - // Fetch the pair-language translation of the explanation to seed the first - // bubble. + // Fetch the explanation rendered into the other half of her pair to seed the + // first bubble — English under a Portuguese card, her language under an + // English one; the server decides which way round (suggestions/translate.go). // Only replaces the seed bubble if the user hasn't started chatting yet (the - // conversation always opens with this one assistant turn). Falls back to the - // English explanation if the translation can't be fetched. + // conversation opens with this one assistant turn and nothing else). + // + // When it can't be fetched the panel opens empty rather than falling back to + // the explanation, which sits two lines higher in the card: a bubble that + // repeats the card verbatim reads as Petal answering in the language the tap + // was pressed to escape. Nothing to add, nothing shown. useEffect(() => { let cancelled = false + const seed = (text: string) => + setMessages((prev) => + prev.length !== 1 ? prev : text === '' ? [] : [{ role: 'assistant', content: text }], + ) api .translateSuggestion(suggestionId) .then((res) => { if (cancelled) return - const text = res.translation.trim() || explanation - setMessages((prev) => (prev.length === 1 ? [{ role: 'assistant', content: text }] : prev)) + seed(res.translation.trim()) }) .catch(() => { if (cancelled) return - setMessages((prev) => (prev.length === 1 ? [{ role: 'assistant', content: explanation }] : prev)) + seed('') }) .finally(() => { if (!cancelled) setSeeding(false) @@ -106,7 +114,7 @@ export function AskPetal({ suggestionId, explanation }: Props) { return () => { cancelled = true } - }, [suggestionId, explanation]) + }, [suggestionId]) async function send() { const text = input.trim() diff --git a/web/src/components/Editor/SuggestionCard.tsx b/web/src/components/Editor/SuggestionCard.tsx index 4ddb0f4..128c878 100644 --- a/web/src/components/Editor/SuggestionCard.tsx +++ b/web/src/components/Editor/SuggestionCard.tsx @@ -197,7 +197,7 @@ export function SuggestionCard({ {asking ? 'Hide Petal' : 'Ask Petal ✨'} - {asking && } + {asking && }
{hasReplacement && ( diff --git a/web/src/components/Editor/SuggestionRail.tsx b/web/src/components/Editor/SuggestionRail.tsx index cd5bb31..8046a39 100644 --- a/web/src/components/Editor/SuggestionRail.tsx +++ b/web/src/components/Editor/SuggestionRail.tsx @@ -217,7 +217,7 @@ const RailCard = forwardRef(function RailCard( - {expanded && } + {expanded && }
{hasReplacement && (