When she writes in Chinese, say Translate — not Clarity

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
2026-07-28 00:09:42 -07:00
parent 3bcc967f51
commit 25e415daa2
20 changed files with 917 additions and 15 deletions
@@ -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)
+		}
+	}
+}