From 7fa98d03c7de4e182cb9ec28fc71c92e88350392 Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:03:19 -0700 Subject: [PATCH] Both sections of the advice arrived in the language she is not learning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from the phone: "I have my main language set as Portuguese and I say I'm learning English but yet Petal presents the Ask Petal advice in both sections as Portuguese." Nothing was wrong with targetFor again. A Portuguese document by a Portuguese writer is explained in Portuguese, which is the whole point of Phase 28. The card was right. What was wrong was the tap underneath it: /suggestions/{id} /translate answered "" for exactly that case, on the reasoning that an unasked-for English rendering of an explanation she can already read is not a seed but noise. That reasoning had the writer facing the wrong way. She is learning English. The half she is *practising* is the half worth a tap, and the bubble sits directly beneath the explanation inside the same card, so answering "" left her with Portuguese, the same Portuguese again, and no English anywhere on the card. targetFor's own comment promised the other language stays one tap away in both directions; only one direction had ever been built. So the endpoint keeps the one rule it always claimed: render into whichever half the explanation is not already in. English explanation into her language, as before; her language into the English she is learning, which is new. Both ends of that are now parameters — TranslateMessages took the source language for granted as English because until Phase 28 it always was. The zh prompt is unchanged byte for byte, which its test still pins. The client fallback was the same symptom from a different cause and would have survived the server fix: an empty answer, or an unreachable model, seeded the bubble with the explanation itself — a verbatim repeat of the line two above it, which reads as Petal replying in the language the tap was pressed to escape. With the endpoint always having somewhere to go, empty now means only that the model didn't answer, so the panel opens with no bubble at all and the input where she can ask. AskPetal no longer takes the explanation as a prop; it never needed anything but the id. The test that pinned the refusal now pins the rendering, and carries the report. Claude-Session: https://claude.ai/code/session_01KGACAtTPjvZ2PipDZ5qD99 --- internal/llm/lang_test.go | 4 +- internal/llm/prompts.go | 32 +++++++++------- internal/llm/translate.go | 13 ++++--- internal/suggestions/doclang_test.go | 24 ++++++++---- internal/suggestions/translate.go | 30 +++++++++------ web/src/components/Editor/AskPetal.tsx | 40 ++++++++++++-------- web/src/components/Editor/SuggestionCard.tsx | 2 +- web/src/components/Editor/SuggestionRail.tsx | 2 +- 8 files changed, 88 insertions(+), 59 deletions(-) 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 && (