From 25e415daa24a77c4c8a80b11029abeb76f584a45 Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:09:42 -0700 Subject: [PATCH] =?UTF-8?q?When=20she=20writes=20in=20Chinese,=20say=20Tra?= =?UTF-8?q?nslate=20=E2=80=94=20not=20Clarity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit She reaches for her own language mid-sentence when English won't come, and Petal already handled it: it found the span and rendered it into English. It just filed the result as a Clarity fix, so the pair model's flagship moment read as tidying up her Chinese. The type is now derived from the span rather than asked of the model. A type is structural, and a model that re-reasons every pass would drift between labels for a sentence nobody had touched — the instability the last session spent itself removing. The label the model volunteers is still ignored. Only the grammar checkpoint can be promoted. A pass with a forced type owns its family: voice reads paragraphs for tone and its rows carry no replacement, so a "translation" there would be a card offering nothing to accept. zh is a different script and counting Han runes is close to certain. The Latin pairs share an alphabet with English and get none of that, so they fall back to function words and need two before Petal claims anything — with every word that is also English left out, even the common ones. The heuristic is justified by how cheap being wrong is: it changes a coloured pill, and nothing else. The pill is the one bilingual type name in the rail. Every other type stays English because those are the terms she is learning; this card's whole subject is her own language. And it stops truncating its two lines — elsewhere the diff is a word and the explanation is what she reads, but here the two sentences are the card. Two things only the running page could report. The inline underline was invisible: the decoration carries a per-type class and the base rule is a transparent border, so a type with no colour rule gets no mark at all. And at 1517×810 with the document list open there is no rail — the margin is 258 where railEnabled wants 348 — so what she gets is the inline hover card. Item 7 is written the other way round. Migration 0015 rebuilds the suggestions table for the CHECK, which makes it the first one here that could quietly drop her rows; there is a test that carries every column, both timestamps and both indexes across it. Claude-Session: https://claude.ai/code/session_016y6gyuHkQXPiEuW8RGQyua --- UX_REVIEW_2026-07-27.md | 107 +++++++++++ internal/db/db.go | 38 ++++ internal/db/db_test.go | 137 ++++++++++++++ internal/db/models.go | 9 +- internal/suggestions/handlers.go | 11 +- internal/suggestions/language.go | 174 +++++++++++++++++ internal/suggestions/language_test.go | 176 ++++++++++++++++++ internal/suggestions/reconcile.go | 12 +- internal/suggestions/translate_type_test.go | 107 +++++++++++ web/src/api/client.ts | 13 +- web/src/components/Editor/SuggestionCard.tsx | 9 +- web/src/components/Editor/SuggestionRail.tsx | 24 ++- .../components/Editor/suggestionMeta.test.ts | 54 ++++++ web/src/components/Editor/suggestionMeta.ts | 16 ++ web/src/i18n/i18n.test.ts | 29 ++- web/src/i18n/packs/fr.ts | 1 + web/src/i18n/packs/pt-PT.ts | 1 + web/src/i18n/packs/zh.ts | 1 + web/src/i18n/types.ts | 6 + web/src/index.css | 7 + 20 files changed, 917 insertions(+), 15 deletions(-) create mode 100644 internal/suggestions/language.go create mode 100644 internal/suggestions/language_test.go create mode 100644 internal/suggestions/translate_type_test.go create mode 100644 web/src/components/Editor/suggestionMeta.test.ts diff --git a/UX_REVIEW_2026-07-27.md b/UX_REVIEW_2026-07-27.md index 614888e..8c478e7 100644 --- a/UX_REVIEW_2026-07-27.md +++ b/UX_REVIEW_2026-07-27.md @@ -399,6 +399,106 @@ card's strikethrough line so she can still see what she wrote). card within one check cycle; accepting swaps in the English; an English-only doc and a Chinese-only doc are unaffected. +### 5 — DONE (sixth session). The label was the whole gap, and it isn't the model's to give. + +The re-scoped premise held: Petal already found the span and already rendered +it into English. Only the type was wrong. So the work was to decide that type +*structurally* rather than ask for it — a model that re-reasons every pass +would drift between labels for a sentence that hadn't changed, which is exactly +the instability item 2 just spent a session removing. + +**Implemented:** + +- `language.go` — `isTranslation(original, replacement, pairLang)`. Both halves + must hold: the span reads as her language *and* what Petal offers back reads + as English. The second half is not decoration — a Chinese span rewritten into + different Chinese is something else, and Petal has no business calling it a + translation. +- `reconcile.go` — the promotion sits at the single point where a type is + stamped, and only on the open-typed grammar checkpoint. A pass with a + `forceType` owns its family outright: voice reads paragraphs for tone and its + rows carry no replacement, so a "translation" there would be a card offering + nothing to accept. There is a test for exactly that. +- `normalizeType` still refuses `translate` from the model, deliberately. A + model that volunteers the label lands on grammar and is then promoted — or + not — on the evidence. +- Migration `0015` rebuilds the suggestions table for the extended `type` CHECK + (SQLite can't ALTER one), as `0005` and `0008` did before it. +- Client: `--color-jade`, the `translate` entry in `TYPE_META`, and + `.petal-suggestion-translate` for the inline underline. +- The pill is the one bilingual type name in the rail — `翻译 · Translate`, + `Tradução · Translate`, `Traduction · Translate`, from the pack. Every other + type stays English on purpose: those are the terms she is learning, and she is + learning them in English. This card's whole subject is her own language. +- The rail card stops truncating its two lines for a translation. Elsewhere the + original and replacement differ by a word and the explanation below is what + she reads; here the two lines are a whole sentence in each language and *are* + the card. + +**The pair families need different tests, and pretending otherwise was the +trap.** The item's plan says to reuse the CJK detection from spellcheck — which +works, for zh, because it is a different script. It gives nothing for pt-PT, fr +or es, where no such signal exists. Those fall back to function words, and need +two distinct markers before Petal will claim anything; the lists deliberately +omit every word that is *also* English (`do`, `con`, `ya`, `todo`, and the +pan-Romance shorts), even where that costs a very common one. A single marker is +never enough, so a one-word Portuguese span won't trip it — a single word is a +vocabulary question, not a translation. The whole heuristic is justified by how +cheap its failure is: a wrong answer changes a coloured pill and nothing else, +because the replacement, the explanation and the Accept button are identical +either way. + +**Two things only the browser could have told me.** + +- **The inline underline was invisible.** The decoration plugin emits a + per-type class, and `.petal-suggestion` sets `border-bottom: 2px solid + transparent` — so a type with no colour rule renders with no underline at + all. Every test passed; the flagship span simply had no mark under it on + screen. Found by looking at the page, and it is the reason this doc keeps + insisting on that. +- **At her viewport there is no rail.** `railEnabled` needs 348 px beside the + editor, and at 1517×810 with the document list open the margin is 258 — so + the card she actually gets is the inline hover panel, not a rail card. Worth + knowing before item 7 is scoped: that item assumes the rail is what she sees + and treats the anchored popover as the missing half. On this screen it is the + other way round. (The fifth session measured rail cards at the same width, so + some state does reach it; not chased here.) + +**Verified against the running binary**, with a stand-in model server so no VPN +or GPU was involved. The stub types the Chinese finding `"clarity"` — exactly +what the live build did — so the label on screen can only have come from Petal's +own detection. Through the real `/check`: `grammar` for the English sentence, +`translate` for the Chinese one. In the browser: the jade underline distinct +from grammar's mint, and the card showing the `翻译 · Translate` pill, the +Chinese struck through and wrapped over two lines rather than clipped, the +English rendering, the bilingual explanation, and Accept. + +**Not verified in a browser:** the *rail* card's version of the same thing. The +rail is unreachable on this 1517 px display (above), and forcing it by hand kept +being overridden by React's own layout. Its label goes through the same +`typeLabel` call the hover card just proved, and its diff differs only by a +class toggle — but that is a reading, not a measurement, and it is written down +as one. + +Coverage: `language_test.go` (the flagship sentence; a lone Han rune; one +Chinese word inside English prose; Chinese→Chinese; a Chinese span on the wrong +pair; all three Latin pairs; an English sentence stuffed with pan-Romance +lookalikes; French elision; unknown and absent pairs), `translate_type_test.go` +(the type through the real `/check`, an English correction keeping its own type, +and voice unable to mint one), `suggestionMeta.test.ts` (every type has a colour +and a name, translate's colour is its own, the pill is bilingual per pair and +every other pill isn't), and `db_test.go`'s +`TestTranslateTypeMigrationPreservesRows` — 0015 rebuilds the table, so it is +the first migration here that could silently *drop her rows*; every column, +both timestamps and both indexes are asserted across it. + +One test changed contract: the pt-PT pack's Brazilian-forms grep searched +`JSON.stringify(pack)`, which includes field *names* — and duly failed on +`translateLabel`, since it lowercases to "transla·tela·bel" and so "contains" +the pt-BR *tela*. It now searches the pack's copy only, with two canaries, since +every assertion in it is a negative and a haystack that quietly went empty would +make the whole test pass by having nothing to search. + ## 6. Ask Petal answers: bilingual, and room to read The card's *explanation* is bilingual, but the Ask Petal *answer* came back @@ -504,6 +604,13 @@ vitest tests. So the three sessions of work she hasn't seen, she can now see. Untouched: 6, 7, 8, item 3's incremental half, item 5's re-scoped Translate card type.)* +*(Sixth session, second half: item 5 done — see the subsection under it. It sits +on `feat/translate-card`, **not deployed**; `main` is at the stack merge. Two +findings there are worth reading before picking the next item: the inline +underline needs a per-type CSS rule or it renders invisibly, and at her actual +viewport the rail is disabled — the inline hover card is what she sees, which +inverts item 7's premise. Untouched: 6, 7, 8, item 3's incremental half.)* + **Suggested next (sixth session onward):** items 6, 7, 8 are all untouched and all small; item 5's re-scoped Translate card type is the cheapest *visible* win (see its Status note — the span is already detected and diff --git a/internal/db/db.go b/internal/db/db.go index b5ea7e1..872c398 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -523,6 +523,44 @@ CREATE TABLE checked_chunks ( hash TEXT NOT NULL, PRIMARY KEY (doc_id, family, hash) ); +`, + }, + { + // A sentence she wrote in her own language gets its own type. Petal already + // detected such spans and already rendered them into English — it just + // filed the result under 'clarity', so the pair model's flagship moment + // read as tidying up her Chinese. As with 0005 and 0008, the `type` CHECK + // can't be ALTERed in place, so rebuild the table with the extended + // constraint, copy every row across, and recreate both indexes. + // + // Existing rows are left on whatever type they have. A card she is already + // reading keeps the label she has already read (the same rule reconcile.go + // follows for a re-proposed edit); new findings get the new label. + name: "0015_translate_suggestion_type", + stmt: ` +CREATE TABLE suggestions_new ( + id TEXT PRIMARY KEY DEFAULT (lower(hex(randomblob(16)))), + doc_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + from_pos INTEGER NOT NULL, + to_pos INTEGER NOT NULL, + original TEXT NOT NULL, + replacement TEXT NOT NULL, + explanation TEXT NOT NULL, + type TEXT NOT NULL CHECK(type IN ('grammar','phrasing','idiom','clarity','translate','voice','collocation','mechanics')), + status TEXT NOT NULL DEFAULT 'pending' CHECK(status IN ('pending','accepted','rejected')), + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + resolved_at DATETIME, + source TEXT NOT NULL DEFAULT 'llm', + chunk_hash TEXT NOT NULL DEFAULT '' +); + +INSERT INTO suggestions_new (id, doc_id, from_pos, to_pos, original, replacement, explanation, type, status, created_at, resolved_at, source, chunk_hash) +SELECT id, doc_id, from_pos, to_pos, original, replacement, explanation, type, status, created_at, resolved_at, source, chunk_hash FROM suggestions; + +DROP TABLE suggestions; +ALTER TABLE suggestions_new RENAME TO suggestions; +CREATE INDEX idx_suggestions_doc_id ON suggestions(doc_id); +CREATE INDEX idx_suggestions_resolved ON suggestions(status, resolved_at); `, }, } diff --git a/internal/db/db_test.go b/internal/db/db_test.go index e705c0e..3caa927 100644 --- a/internal/db/db_test.go +++ b/internal/db/db_test.go @@ -3,6 +3,7 @@ package db import ( "path/filepath" "testing" + "time" ) func TestOpenMigratesAndSeeds(t *testing.T) { @@ -227,3 +228,139 @@ func TestSuggestionSourceBackfill(t *testing.T) { t.Errorf("default source = %q, want %q", fresh, SuggestionSourceLLM) } } + +// TestTranslateTypeMigrationPreservesRows runs migration 0015 against a database +// that predates it. Unlike the two backfills above, 0015 *rebuilds the table* — +// SQLite can't ALTER a CHECK constraint — so it copies every row across by hand, +// and a column left out of that copy list silently loses her data. Every test +// elsewhere starts from a fresh database and would never notice; the live box has +// years of rows in it. +func TestTranslateTypeMigrationPreservesRows(t *testing.T) { + path := filepath.Join(t.TempDir(), "old.db") + d, err := Open(path) + if err != nil { + t.Fatalf("open: %v", err) + } + + // Rewind to the pre-0015 table: the same shape, minus 'translate' in the CHECK. + if _, err := d.Exec(` +CREATE TABLE suggestions_old ( + id TEXT PRIMARY KEY DEFAULT (lower(hex(randomblob(16)))), + doc_id TEXT NOT NULL REFERENCES documents(id) ON DELETE CASCADE, + from_pos INTEGER NOT NULL, + to_pos INTEGER NOT NULL, + original TEXT NOT NULL, + replacement TEXT NOT NULL, + explanation TEXT NOT NULL, + type TEXT NOT NULL CHECK(type IN ('grammar','phrasing','idiom','clarity','voice','collocation','mechanics')), + status TEXT NOT NULL DEFAULT 'pending' CHECK(status IN ('pending','accepted','rejected')), + created_at DATETIME DEFAULT CURRENT_TIMESTAMP, + resolved_at DATETIME, + source TEXT NOT NULL DEFAULT 'llm', + chunk_hash TEXT NOT NULL DEFAULT '' +); +DROP TABLE suggestions; +ALTER TABLE suggestions_old RENAME TO suggestions; +CREATE INDEX idx_suggestions_doc_id ON suggestions(doc_id); +CREATE INDEX idx_suggestions_resolved ON suggestions(status, resolved_at); +DELETE FROM schema_migrations WHERE name = '0015_translate_suggestion_type'; +`); err != nil { + t.Fatalf("rewind schema: %v", err) + } + + if _, err := d.Exec(`INSERT INTO documents (id, user_id) VALUES ('d1', ?)`, LocalUserID); err != nil { + t.Fatalf("insert document: %v", err) + } + // One row with every column carrying a distinguishable value, so a dropped + // column shows up as a changed value rather than as a passing test. + if _, err := d.Exec( + `INSERT INTO suggestions (id, doc_id, from_pos, to_pos, original, replacement, explanation, type, status, created_at, resolved_at, source, chunk_hash) + VALUES ('s-1', 'd1', 7, 11, 'by foots', 'on foot', 'idiom advice she has read', 'idiom', 'accepted', '2026-01-02 03:04:05', '2026-01-02 03:05:00', 'local', 'abc123')`, + ); err != nil { + t.Fatalf("seed row: %v", err) + } + d.Close() + + d2, err := Open(path) + if err != nil { + t.Fatalf("reopen (migrate): %v", err) + } + defer d2.Close() + + var ( + docID, original, replacement, explanation string + typ, status, source, chunkHash string + from, to int + // Scanned as instants, not strings: the driver renders a DATETIME column in + // its own format, so the claim is "the same moment", not the same text. + createdAt, resolvedAt time.Time + ) + if err := d2.QueryRow( + `SELECT doc_id, from_pos, to_pos, original, replacement, explanation, type, status, created_at, resolved_at, source, chunk_hash + FROM suggestions WHERE id = 's-1'`, + ).Scan(&docID, &from, &to, &original, &replacement, &explanation, + &typ, &status, &createdAt, &resolvedAt, &source, &chunkHash); err != nil { + t.Fatalf("read migrated row: %v", err) + } + for _, c := range []struct{ name, got, want string }{ + {"doc_id", docID, "d1"}, + {"original", original, "by foots"}, + {"replacement", replacement, "on foot"}, + {"explanation", explanation, "idiom advice she has read"}, + {"type", typ, SuggestionTypeIdiom}, + {"status", status, SuggestionStatusAccepted}, + {"source", source, SuggestionSourceLocal}, + {"chunk_hash", chunkHash, "abc123"}, + } { + if c.got != c.want { + t.Errorf("%s = %q, want %q", c.name, c.got, c.want) + } + } + if from != 7 || to != 11 { + t.Errorf("offsets = (%d, %d), want (7, 11)", from, to) + } + // created_at and resolved_at must survive: the rail's arrival chime keys on + // created_at, and the growth journal counts by resolved_at. A rebuild that + // reset either would re-chime her whole document and rewrite her history. + for _, c := range []struct { + name string + got time.Time + want string + }{ + {"created_at", createdAt, "2026-01-02 03:04:05"}, + {"resolved_at", resolvedAt, "2026-01-02 03:05:00"}, + } { + want, err := time.Parse("2006-01-02 15:04:05", c.want) + if err != nil { + t.Fatalf("parse want: %v", err) + } + if !c.got.Equal(want) { + t.Errorf("%s = %v, want the original instant %v", c.name, c.got, want) + } + } + + // The point of the rebuild: the new type is now insertable, and a bogus one + // still isn't. + if _, err := d2.Exec( + `INSERT INTO suggestions (id, doc_id, from_pos, to_pos, original, replacement, explanation, type) + VALUES ('s-2', 'd1', 0, 3, '苹果', 'apple', 'x', ?)`, SuggestionTypeTranslate, + ); err != nil { + t.Fatalf("insert translate row: %v", err) + } + if _, err := d2.Exec( + `INSERT INTO suggestions (id, doc_id, from_pos, to_pos, original, replacement, explanation, type) + VALUES ('s-3', 'd1', 0, 3, 'x', 'y', 'x', 'nonsense')`, + ); err == nil { + t.Error("CHECK constraint accepted an unknown type after the rebuild") + } + + // Both indexes must come back, or every document load starts table-scanning. + for _, idx := range []string{"idx_suggestions_doc_id", "idx_suggestions_resolved"} { + var name string + if err := d2.QueryRow( + `SELECT name FROM sqlite_master WHERE type = 'index' AND name = ?`, idx, + ).Scan(&name); err != nil { + t.Errorf("index %s missing after rebuild: %v", idx, err) + } + } +} diff --git a/internal/db/models.go b/internal/db/models.go index 1c9e073..6bbe9dd 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -104,7 +104,7 @@ type Suggestion struct { Original string `json:"original"` Replacement string `json:"replacement"` Explanation string `json:"explanation"` - Type string `json:"type"` // grammar | phrasing | idiom | clarity | voice | collocation + Type string `json:"type"` // grammar | phrasing | idiom | clarity | translate | voice | collocation Status string `json:"status"` // pending | accepted | rejected // Source names the engine that proposed the edit, not its family: an offline // rule and the model can both propose a collocation, and the writer is never @@ -119,6 +119,13 @@ const ( SuggestionTypePhrasing = "phrasing" SuggestionTypeIdiom = "idiom" SuggestionTypeClarity = "clarity" + // A span she wrote in her own language, rendered into English. Not a + // correction — nothing was wrong with it — which is why it is its own type + // rather than a clarity fix: the card is the pair model's flagship moment + // (SUGGESTIONS §1), and labelling it "Clarity" reads as a tidy-up of her + // first language. The model isn't asked for this label; it is derived from the + // span itself (see suggestions/language.go), so it can't drift. + SuggestionTypeTranslate = "translate" SuggestionTypeVoice = "voice" SuggestionTypeCollocation = "collocation" SuggestionTypeMechanics = "mechanics" // deterministic rule-based pass (no LLM) diff --git a/internal/suggestions/handlers.go b/internal/suggestions/handlers.go index fb9a1cc..57b1a89 100644 --- a/internal/suggestions/handlers.go +++ b/internal/suggestions/handlers.go @@ -284,7 +284,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R // Nothing to analyze on an empty document — skip the LLM round-trip. The // family's rows go with the text they were about. if strings.TrimSpace(contentText) == "" { - if err := h.reconcilePending(docID, contentText, nil, scope, nil, nil, false); err != nil { + if err := h.reconcilePending(docID, contentText, pairLang, nil, scope, nil, nil, false); err != nil { httputil.ServerError(w, err) return } @@ -320,7 +320,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R if len(changed) == 0 { // Every sentence has already been read. Drop the rows whose sentence is // gone, keep the rest exactly as they are, and answer immediately. - if err := h.reconcilePending(docID, contentText, nil, scope, chunks, nil, false); err != nil { + if err := h.reconcilePending(docID, contentText, pairLang, nil, scope, chunks, nil, false); err != nil { httputil.ServerError(w, err) return } @@ -365,7 +365,7 @@ func (h *Handler) runPass(w http.ResponseWriter, r *http.Request, limiter *llm.R // A whole-document pass re-read everything, so every one of its rows is up for // re-proposal; a chunked pass only puts the sentences it asked about in play. - if err := h.reconcilePending(docID, contentText, raw, scope, chunks, fresh, !scope.chunked); err != nil { + if err := h.reconcilePending(docID, contentText, pairLang, raw, scope, chunks, fresh, !scope.chunked); err != nil { httputil.ServerError(w, err) return } @@ -756,6 +756,11 @@ func locate(contentText, original string) (int, int) { // normalizeType maps the model's type string onto a valid suggestion type, // defaulting unknown values to grammar so a stray label never trips the CHECK. +// +// 'translate' is absent on purpose, and stays absent even though the type now +// exists: it is decided from the span (see language.go), never taken from the +// model. A model that volunteers the label anyway lands on grammar here and is +// then promoted — or not — on the evidence. func normalizeType(t string) string { switch strings.ToLower(strings.TrimSpace(t)) { case db.SuggestionTypeGrammar, db.SuggestionTypePhrasing, db.SuggestionTypeIdiom, db.SuggestionTypeClarity, db.SuggestionTypeCollocation: diff --git a/internal/suggestions/language.go b/internal/suggestions/language.go new file mode 100644 index 0000000..6ae0e60 --- /dev/null +++ b/internal/suggestions/language.go @@ -0,0 +1,174 @@ +package suggestions + +import ( + "strings" + "unicode" +) + +// Telling her language from English, well enough to label a card. +// +// When the checkpoint quotes a span she wrote in her own language and hands back +// an English rendering, that is not a correction — nothing was wrong with what +// she wrote — and it should not be filed under 'clarity'. The label is derived +// here rather than asked of the model: a type is structural, and a model that +// re-reasons every pass would drift between labels for the same sentence. +// +// The failure mode is deliberately cheap. Getting this wrong changes a card's +// coloured pill and nothing else — the replacement, the explanation and the +// Accept button are identical either way — so a heuristic is the right tool. It +// is written to under-claim: a span it isn't sure about stays whatever the model +// called it. +// +// The two pair families need genuinely different tests, and pretending otherwise +// would be the bug: +// +// - zh is a different script. Counting Han runes is close to certain. +// - pt-PT, fr and es share the Latin alphabet with English, where no such +// signal exists. Those fall back to function words — the short, extremely +// common words a sentence in that language can hardly avoid and an English +// sentence has no reason to contain. + +// isTranslation reports whether this edit is her own language rendered into +// English, rather than a correction to her English. Both halves must hold: the +// quoted span reads as the pair language, and what Petal offers back reads as +// English. The second half matters — a Chinese span rewritten into different +// Chinese is something else entirely, and Petal has no business calling it a +// translation. +func isTranslation(original, replacement, pairLang string) bool { + if strings.TrimSpace(original) == "" || strings.TrimSpace(replacement) == "" { + return false + } + return readsAsPairLang(original, pairLang) && readsAsEnglish(replacement) +} + +// readsAsPairLang reports whether s is predominantly in the writer's language. +func readsAsPairLang(s, pairLang string) bool { + switch normalizePairLang(pairLang) { + case "zh": + han, latin := scriptCounts(s) + // Predominantly, not merely partly: one Chinese word inside an English + // sentence is a vocabulary question, and the sentence around it is still + // English prose with its own grammar to correct. Two runes is the floor + // because a single Han character is as likely to be a stray keystroke. + return han >= 2 && han > latin + case "pt-PT", "fr", "es": + return distinctMarkers(s, latinMarkers[normalizePairLang(pairLang)]) >= 2 + } + // A pair Petal has no test for. Say no: an unlabelled card is a card that + // reads as it did yesterday, and a wrongly-labelled one is a new defect. + return false +} + +// readsAsEnglish reports whether s is English prose rather than more of her own +// language. It is not a language identifier — it only has to separate "English" +// from "the pair language", and it is only ever asked about text Petal itself +// generated, so the bar is low on purpose: Latin letters present, and not +// swamped by another script. +func readsAsEnglish(s string) bool { + han, latin := scriptCounts(s) + return latin > 0 && latin > han +} + +// normalizePairLang folds the stored `users.pair_lang` into the codes below. +// Empty (a document whose owner has no pair recorded) falls through to no test. +func normalizePairLang(pairLang string) string { + switch p := strings.ToLower(strings.TrimSpace(pairLang)); p { + case "zh", "zh-cn", "zh-hans": + return "zh" + case "pt", "pt-pt": + return "pt-PT" + case "fr", "fr-fr": + return "fr" + case "es", "es-es": + return "es" + default: + return p + } +} + +// scriptCounts counts Han runes and ASCII letters. Everything else — digits, +// punctuation, spaces, emoji — is ignored, so trailing 。or a stray comma +// changes nothing. +func scriptCounts(s string) (han, latin int) { + for _, r := range s { + switch { + case unicode.Is(unicode.Han, r): + han++ + case r < unicode.MaxASCII && unicode.IsLetter(r): + latin++ + } + } + return han, latin +} + +// distinctMarkers counts how many *different* marker words appear in s. Distinct +// rather than total: "que ... que" is one writer's habit, while "eu quero" is two +// independent pieces of evidence. +func distinctMarkers(s string, markers map[string]bool) int { + if len(markers) == 0 { + return 0 + } + seen := map[string]bool{} + for _, w := range strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { + // Split on anything that isn't a letter, so punctuation and digits are + // separators. Apostrophes included: French elision (j'ai, n'est) should + // yield its parts. + return !unicode.IsLetter(r) + }) { + if markers[w] { + seen[w] = true + } + } + return len(seen) +} + +// Function words that a sentence in each Latin pair can hardly avoid. +// +// Curated against English, not for coverage: every entry here is a word an +// English sentence has essentially no reason to contain, which is why the lists +// omit plenty of far more common words. Deliberately absent — each of them a +// false positive waiting to happen — is anything that is *also* an English word: +// the pan-Romance shorts (a, o, e, as, no, on, en, de, se, na, mi, son, era, +// plus, pour, si, ma, ce, ne), Portuguese "do", Spanish "con", "ya" and "todo". +// Dropping "con" costs the Spanish list one of its commonest words, and that is +// the right trade — a marker that fires on English corroborates the wrong +// answer, which is worse than a sentence Petal declines to label. +// +// A single marker is not enough (see readsAsPairLang), so these lists are read +// as evidence to be corroborated rather than as a decision. +var latinMarkers = map[string]map[string]bool{ + "fr": words( + "je", "tu", "il", "elle", "ils", "elles", "nous", "vous", "est", "sont", + "était", "étais", "une", "des", "les", "du", "dans", "avec", "que", "qui", + "mais", "très", "être", "avoir", "pas", "cette", "cet", "ces", "mon", + "mes", "notre", "votre", "leur", "aussi", "alors", "parce", "comme", + "beaucoup", "toujours", "jamais", "quand", "bien", "chose", "temps", + "moi", "toi", "lui", "peux", "veux", "sais", "faire", "dit", "aujourd", + "hui", "quelque", "chez", "tout", "tous", "rien", "déjà", "encore", + ), + "pt-PT": words( + "eu", "você", "ele", "ela", "eles", "elas", "nós", "são", "uma", "os", + "da", "dos", "das", "com", "que", "mas", "muito", "não", "meu", + "minha", "seu", "sua", "isso", "este", "esta", "está", "estou", "quero", + "também", "quando", "porque", "coisa", "tempo", "fazer", "sempre", + "nunca", "bem", "obrigado", "obrigada", "gosto", "tenho", "tem", "foi", + "ser", "ter", "mais", "já", "ainda", "aqui", "ali", "nada", "tudo", + "todos", "para", "pela", "pelo", "sobre", "assim", + ), + "es": words( + "yo", "él", "ella", "ellos", "ellas", "nosotros", "una", "los", "las", + "del", "que", "pero", "muy", "esto", "esta", "este", "está", + "estoy", "quiero", "también", "cuando", "porque", "cosa", "tiempo", + "hacer", "siempre", "nunca", "bien", "gracias", "tengo", "tiene", "fue", + "ser", "tener", "más", "aquí", "allí", "nada", "todos", + "para", "sobre", "así", "hola", "señor", "usted", "muchas", + ), +} + +func words(list ...string) map[string]bool { + out := make(map[string]bool, len(list)) + for _, w := range list { + out[w] = true + } + return out +} diff --git a/internal/suggestions/language_test.go b/internal/suggestions/language_test.go new file mode 100644 index 0000000..efe0b61 --- /dev/null +++ b/internal/suggestions/language_test.go @@ -0,0 +1,176 @@ +package suggestions + +import "testing" + +// The flagship case, and the ones next to it that must NOT become translations. +func TestIsTranslation(t *testing.T) { + cases := []struct { + name string + original string + replacement string + pairLang string + want bool + }{ + { + // The sentence from the UX review, verbatim. + name: "whole Chinese sentence rendered into English", + original: "我想说这句话但是不知道用英语怎么说。", + replacement: "I want to say this but I don't know how to say it in English.", + pairLang: "zh", + want: true, + }, + { + name: "ordinary English correction is not a translation", + original: "She goes to market yesterday", + replacement: "She went to the market yesterday", + pairLang: "zh", + want: false, + }, + { + // One Chinese word inside English prose. The sentence around it is + // still English with its own grammar to fix, and calling the card a + // translation would mislabel a grammar fix. + name: "single Chinese word inside an English sentence", + original: "I bought a 苹果 at the store", + replacement: "I bought an apple at the store", + pairLang: "zh", + want: false, + }, + { + name: "a lone stray Han rune is not a sentence", + original: "的", + replacement: "of", + pairLang: "zh", + want: false, + }, + { + // Chinese in, Chinese out: whatever this is, Petal is not translating. + name: "Chinese rewritten as Chinese", + original: "我想说这句话", + replacement: "我要说这句话", + pairLang: "zh", + want: false, + }, + { + // The same Chinese span, but the writer is on the French pair. Petal + // has no business offering to translate a language she never claimed. + name: "Chinese span on a non-zh pair", + original: "我想说这句话但是不知道用英语怎么说。", + replacement: "I want to say this in English.", + pairLang: "fr", + want: false, + }, + { + name: "French sentence rendered into English", + original: "Je ne sais pas comment le dire en anglais.", + replacement: "I don't know how to say it in English.", + pairLang: "fr", + want: true, + }, + { + name: "Portuguese sentence rendered into English", + original: "Eu quero dizer isso mas não sei como.", + replacement: "I want to say this but I don't know how.", + pairLang: "pt-PT", + want: true, + }, + { + name: "Spanish sentence rendered into English", + original: "Yo quiero decir esto pero no sé cómo.", + replacement: "I want to say this but I don't know how.", + pairLang: "es", + want: true, + }, + { + // A single marker is not evidence. "Que" appears in English writing + // about other languages, in names, in quoted phrases. + name: "one Latin marker is not enough", + original: "The word que confused me", + replacement: "The word que confuses me", + pairLang: "pt-PT", + want: false, + }, + { + // The words most likely to sink this heuristic: English function words + // that are also Romance function words. They are kept out of the lists + // precisely so this sentence stays a grammar fix. + name: "English full of pan-Romance lookalikes", + original: "I do not know if a con man on the plus side as no era", + replacement: "I do not know whether a con man, on the plus side, is no era", + pairLang: "es", + want: false, + }, + { + name: "English with a borrowed French phrase stays English", + original: "It was a pas de deux, more or less", + replacement: "It was a pas de deux, more or less.", + pairLang: "fr", + want: false, + }, + { + name: "empty replacement (an awareness-only finding)", + original: "我想说这句话但是不知道用英语怎么说。", + replacement: "", + pairLang: "zh", + want: false, + }, + { + // A document whose owner has no pair recorded. No test, no label. + name: "no pair language", + original: "我想说这句话但是不知道用英语怎么说。", + replacement: "I want to say this in English.", + pairLang: "", + want: false, + }, + { + // An unshipped pair. Same rule: decline rather than guess. + name: "unknown pair language", + original: "Ich weiß nicht wie man das sagt.", + replacement: "I don't know how to say that.", + pairLang: "de", + want: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := isTranslation(c.original, c.replacement, c.pairLang); got != c.want { + t.Errorf("isTranslation(%q, %q, %q) = %v, want %v", + c.original, c.replacement, c.pairLang, got, c.want) + } + }) + } +} + +// pair_lang is stored as the pack code, but a stored value has drifted before +// (see the picker's history), so the fold is tested rather than assumed. +func TestNormalizePairLang(t *testing.T) { + for in, want := range map[string]string{ + "zh": "zh", "zh-CN": "zh", "ZH": "zh", + "pt": "pt-PT", "pt-PT": "pt-PT", "pt-pt": "pt-PT", + "fr": "fr", "fr-FR": "fr", + "es": "es", "es-ES": "es", + " zh ": "zh", + "": "", + "de": "de", + } { + if got := normalizePairLang(in); got != want { + t.Errorf("normalizePairLang(%q) = %q, want %q", in, got, want) + } + } +} + +// French elision must yield its parts, or "j'ai" and "n'est" — two of the +// commonest shapes in the language — count for nothing. +func TestElisionYieldsMarkers(t *testing.T) { + if n := distinctMarkers("Je n'est pas", latinMarkers["fr"]); n < 3 { + t.Errorf("elided French: got %d markers, want >= 3 (je, est, pas)", n) + } +} + +// Distinct, not total: one word repeated is one piece of evidence. +func TestRepeatedMarkerCountsOnce(t *testing.T) { + if n := distinctMarkers("que que que", latinMarkers["pt-PT"]); n != 1 { + t.Errorf("repeated marker: got %d, want 1", n) + } +} diff --git a/internal/suggestions/reconcile.go b/internal/suggestions/reconcile.go index 6048bd8..b2c499b 100644 --- a/internal/suggestions/reconcile.go +++ b/internal/suggestions/reconcile.go @@ -130,8 +130,11 @@ func reposition(tx *sql.Tx, row pendingRow, from, to int, chunkHash string) erro // called at all). inPlayAll marks the whole-document passes — voice and the // collocation coach — where every row is up for re-proposal because the model // just re-read everything. +// +// `pairLang` is the writer's own language, needed only to type a finding that +// turns out to be her language rendered into English (see language.go). func (h *Handler) reconcilePending( - docID, contentText string, + docID, contentText, pairLang string, raw []llm.RawSuggestion, scope pendingScope, chunks, fresh []chunk, @@ -225,9 +228,16 @@ func (h *Handler) reconcilePending( } continue } + // A pass with a forced type owns its family outright and is never asked + // about translation: voice reads whole paragraphs for tone, and the + // collocation coach is about English word pairings. Only the open-typed + // grammar checkpoint can turn out to have been handed her own language. typ := scope.forceType if typ == "" { typ = normalizeType(s.Type) + if isTranslation(s.Original, s.Replacement, pairLang) { + typ = db.SuggestionTypeTranslate + } } if _, err := tx.Exec( `INSERT INTO suggestions (doc_id, from_pos, to_pos, original, replacement, explanation, type, source, chunk_hash) diff --git a/internal/suggestions/translate_type_test.go b/internal/suggestions/translate_type_test.go new file mode 100644 index 0000000..75b1d0d --- /dev/null +++ b/internal/suggestions/translate_type_test.go @@ -0,0 +1,107 @@ +package suggestions + +import ( + "encoding/json" + "net/http" + "testing" + + "gitea.parodia.dev/drwily/petal/internal/db" +) + +// The pair model's flagship moment, end to end: she reaches for a sentence in +// her own language mid-document, and the card that comes back is labelled as a +// translation rather than as a tidy-up of her Chinese. +// +// The label is asserted through the real /check path rather than against +// isTranslation directly, because the point of the item was never the detector — +// Petal already found these spans and already rendered them into English. What +// was wrong was the type that reached the rail. +func TestChineseSpanBecomesATranslateCard(t *testing.T) { + // Note the model calls it "clarity", as the live build did. The type it + // volunteers is not consulted. + client := &stubClient{response: `{"suggestions":[ + {"original":"我想说这句话但是不知道用英语怎么说。","replacement":"I want to say this but I don't know how to say it in English.","explanation":"这是英文说法 · Here is how to say it in English","type":"clarity"} + ]}`} + srv, docID, database := newPairServer(t, client, "zh") + setDocTextDB(t, database, docID, "My weekend was good. 我想说这句话但是不知道用英语怎么说。") + + var out []db.Suggestion + rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "") + if rec.Code != http.StatusOK { + t.Fatalf("check: code=%d body=%s", rec.Code, rec.Body) + } + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if len(out) != 1 { + t.Fatalf("want 1 card, got %d: %+v", len(out), out) + } + if out[0].Type != db.SuggestionTypeTranslate { + t.Fatalf("card type = %q, want %q", out[0].Type, db.SuggestionTypeTranslate) + } + // The rendering and the reasoning are the model's, untouched — only the label + // is Petal's. + if out[0].Replacement != "I want to say this but I don't know how to say it in English." { + t.Fatalf("replacement was rewritten: %q", out[0].Replacement) + } +} + +// The other half of the same claim: an ordinary English correction on the same +// writer's document keeps the type the model gave it. A relabel that fired on +// everything would be no better than the label it replaced. +func TestEnglishCorrectionKeepsItsType(t *testing.T) { + client := &stubClient{response: `{"suggestions":[ + {"original":"My weekend was very good","replacement":"My weekend was wonderful","explanation":"stronger wording","type":"phrasing"} + ]}`} + srv, docID, database := newPairServer(t, client, "zh") + setDocTextDB(t, database, docID, "My weekend was very good.") + + var out []db.Suggestion + rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/check", "") + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + if len(out) != 1 { + t.Fatalf("want 1 card, got %d: %+v", len(out), out) + } + if out[0].Type != db.SuggestionTypePhrasing { + t.Fatalf("card type = %q, want %q", out[0].Type, db.SuggestionTypePhrasing) + } +} + +// The voice pass reads whole paragraphs for tone and stamps its own family. A +// Chinese paragraph must not be able to smuggle a translate row into it — voice +// rows carry no replacement to accept, so a "translation" there would be a card +// offering nothing. +func TestVoicePassCannotProduceATranslateCard(t *testing.T) { + client := &stubClient{response: `{"suggestions":[ + {"original":"我想说这句话但是不知道用英语怎么说。","replacement":"I want to say this in English.","explanation":"tone","type":"clarity"} + ]}`} + srv, docID, database := newPairServer(t, client, "zh") + setDocTextDB(t, database, docID, "A first paragraph.\n\n我想说这句话但是不知道用英语怎么说。") + + var out []db.Suggestion + rec := do(t, srv, http.MethodPost, "/docs/"+docID+"/voice", "") + if rec.Code != http.StatusOK { + t.Fatalf("voice: code=%d body=%s", rec.Code, rec.Body) + } + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatalf("decode: %v", err) + } + for _, s := range out { + if s.Type == db.SuggestionTypeTranslate { + t.Fatalf("voice pass produced a translate card: %+v", s) + } + } +} + +// setDocTextDB is setDocText for the pair harness, which hands back the DB +// rather than the Handler. +func setDocTextDB(t *testing.T, database *db.DB, docID, text string) { + t.Helper() + if _, err := database.Exec( + `UPDATE documents SET content_text = ? WHERE id = ?`, text, docID, + ); err != nil { + t.Fatalf("update doc text: %v", err) + } +} diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 6eaf908..4b963a8 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -101,7 +101,18 @@ export interface Gloss { reverse?: string } -export type SuggestionType = 'grammar' | 'phrasing' | 'idiom' | 'clarity' | 'voice' | 'collocation' | 'mechanics' +// 'translate' is a span she wrote in her own language, rendered into English — +// not a correction. The server decides the label from the span itself, never from +// the model, so the client can trust it (see suggestions/language.go). +export type SuggestionType = + | 'grammar' + | 'phrasing' + | 'idiom' + | 'clarity' + | 'translate' + | 'voice' + | 'collocation' + | 'mechanics' // One word in the vocabulary garden: a looked-up word with its gloss/phonetic, // the sentence it was met in, and its spaced-repetition state. `reps` drives how diff --git a/web/src/components/Editor/SuggestionCard.tsx b/web/src/components/Editor/SuggestionCard.tsx index 7c016e9..8651ab0 100644 --- a/web/src/components/Editor/SuggestionCard.tsx +++ b/web/src/components/Editor/SuggestionCard.tsx @@ -1,7 +1,8 @@ import { useState } from 'react' import type { Suggestion } from '../../api/client' +import { usePack } from '../../i18n' import { AskPetal } from './AskPetal' -import { TYPE_META } from './suggestionMeta' +import { TYPE_META, typeLabel } from './suggestionMeta' interface Props { suggestion: Suggestion @@ -28,7 +29,9 @@ export function SuggestionCard({ onPointerLeave, onExpandChange, }: Props) { + const pack = usePack() const meta = TYPE_META[suggestion.type] + const label = typeLabel(suggestion.type, pack) const hasReplacement = suggestion.replacement.trim() !== '' const [asking, setAsking] = useState(false) @@ -43,7 +46,7 @@ export function SuggestionCard({ return (
- {meta.label} + {label} {hasReplacement && ( diff --git a/web/src/components/Editor/SuggestionRail.tsx b/web/src/components/Editor/SuggestionRail.tsx index 1b970a5..242921c 100644 --- a/web/src/components/Editor/SuggestionRail.tsx +++ b/web/src/components/Editor/SuggestionRail.tsx @@ -1,7 +1,8 @@ import { forwardRef, useLayoutEffect, useRef, useState } from 'react' import type { Suggestion } from '../../api/client' +import { usePack } from '../../i18n' import { AskPetal } from './AskPetal' -import { TYPE_META } from './suggestionMeta' +import { TYPE_META, typeLabel } from './suggestionMeta' // Vertical breathing room kept between stacked cards when their natural anchors // would otherwise collide. @@ -138,14 +139,21 @@ const RailCard = forwardRef(function RailCard( { suggestion, top, active, expanded, onAccept, onDismiss, onHover, onActivate, onToggleExpand }, ref, ) { + const pack = usePack() const meta = TYPE_META[suggestion.type] + const label = typeLabel(suggestion.type, pack) const hasReplacement = suggestion.replacement.trim() !== '' + // Every other card truncates its diff to one line each: the original and the + // replacement differ by a word or two, and the explanation below is the part + // she reads. A translation is the reverse — the two lines are a whole sentence + // in each language, and they are the entire point of the card — so it wraps. + const clampDiff = suggestion.type !== 'translate' return (
onHover(suggestion.id)} onMouseLeave={() => onHover(null)} className={`petal-rail-card${active ? ' petal-rail-card-active' : ''}`} @@ -156,7 +164,7 @@ const RailCard = forwardRef(function RailCard( className="inline-flex items-center rounded-full px-2 py-0.5 text-[0.68rem] font-bold" style={{ background: meta.color, color: 'var(--color-plum)' }} > - {meta.label} + {label}