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}