From 6d96bb4ac8a95e792b409fa844884c81bd5288a5 Mon Sep 17 00:00:00 2001 From: "Claude (backend session)" Date: Sun, 5 Jul 2026 03:33:57 +0300 Subject: [PATCH] Close step-3a external-review findings: spine reconciliation for dangling/mislabeled chapters, UTF-8 normalization, ASCII quote-depth, ruby full-replace, and tiling fuzz tests --- backend/internal/pipeline/chunker.go | 8 +- backend/internal/pipeline/chunker_test.go | 87 +++++++++++++++++++++ backend/internal/pipeline/ingest.go | 35 ++++++--- backend/internal/pipeline/ingest_test.go | 42 +++++++++- backend/internal/pipeline/render.go | 14 +++- backend/internal/pipeline/runner.go | 26 +++---- backend/internal/store/migrate.go | 11 +-- backend/internal/store/ruby.go | 45 ++++++----- backend/internal/store/ruby_test.go | 94 ++++++++++++----------- docs/PROGRESS.md | 16 ++++ 10 files changed, 279 insertions(+), 99 deletions(-) diff --git a/backend/internal/pipeline/chunker.go b/backend/internal/pipeline/chunker.go index abe0381..965ea46 100644 --- a/backend/internal/pipeline/chunker.go +++ b/backend/internal/pipeline/chunker.go @@ -264,8 +264,12 @@ func splitSourceSentences(paragraph string) []string { if cjk { boundary = depthHere == 0 // dialogue-internal terminators do not end a sentence } else { - // ASCII terminator / ellipsis: a boundary only before whitespace or EOS. - boundary = e >= len(runes) || isASCIISpace(runes[e]) + // ASCII terminator / ellipsis: a boundary only outside a tracked quote AND + // before whitespace or EOS. The quote-depth guard must apply here too, not + // just to the CJK branch — otherwise 「Stop. Now.」 over-splits mid-dialogue + // (external-review #4). Straight-quote en ("Stop!" She ran.) is untracked + // (depth 0), so it still splits on the space as before. + boundary = depthHere == 0 && (e >= len(runes) || isASCIISpace(runes[e])) // Abbreviation guard: a lone "." after an abbreviation/initial is not a end. if boundary && j-i == 1 && runes[i] == '.' && isAbbrevBefore(runes[start:i]) { boundary = false diff --git a/backend/internal/pipeline/chunker_test.go b/backend/internal/pipeline/chunker_test.go index c011697..a7780fd 100644 --- a/backend/internal/pipeline/chunker_test.go +++ b/backend/internal/pipeline/chunker_test.go @@ -4,6 +4,8 @@ import ( "reflect" "strings" "testing" + "unicode" + "unicode/utf8" ) func TestSplitChunksSingleParagraph(t *testing.T) { @@ -185,6 +187,20 @@ func TestSplitSourceSentencesEllipsisNotOverSplitInCJK(t *testing.T) { } } +func TestSplitSourceSentencesASCIIRespectsQuoteDepth(t *testing.T) { + // ASCII terminators inside a tracked quote must NOT split (like the CJK branch): + // 「Stop. Now.」 is one dialogue unit, not two sentences (external-review #4). + got := trimAll(splitSourceSentences("「Stop. Now.」次へ。")) + want := []string{"「Stop. Now.」次へ。"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("ASCII-in-quote split = %q, want %q (must not over-split dialogue)", got, want) + } + // Sanity: outside quotes ASCII still splits. + if n := len(trimAll(splitSourceSentences("Stop. Now."))); n != 2 { + t.Fatalf("plain ASCII must still split into 2, got %d", n) + } +} + func TestSplitSourceSentencesAbbreviations(t *testing.T) { // Abbreviations and single-letter initials do not end a sentence. got := trimAll(splitSourceSentences("Dr. J. R. R. Tolkien wrote it. The end.")) @@ -194,6 +210,77 @@ func TestSplitSourceSentencesAbbreviations(t *testing.T) { } } +// --- property/fuzz (external-review #7: would have caught #3) ---------------- + +// FuzzSplitSourceSentencesTiles asserts the segmenter is lossless: its segments +// always concatenate back to the input's rune sequence, and byte-exactly when the +// input is valid UTF-8 (the production invariant after NormalizeSource). A path that +// dropped/reordered/replaced a rune — like the size-dependent []rune→U+FFFD bug #3 — +// fails this over random input. +func FuzzSplitSourceSentencesTiles(f *testing.F) { + for _, s := range []string{ + "静かな朝。「止まれ！」と言った。", "Hello. World!", "Mr. Smith went. Home.", + "そう……だった。", "「Stop. Now.」", "abc\xe9def", "", + } { + f.Add(s) + } + f.Fuzz(func(t *testing.T, s string) { + segs := splitSourceSentences(s) + joined := strings.Join(segs, "") + if joined != string([]rune(s)) { + t.Fatalf("tiling lost/reordered runes: input=%q rejoined=%q", s, joined) + } + if utf8.ValidString(s) && joined != s { + t.Fatalf("valid-UTF-8 tiling not byte-exact: %q → %q", s, joined) + } + }) +} + +// FuzzNormalizeSourceValidUTF8 guards fix #3 directly: NormalizeSource always yields +// valid UTF-8, so the chunker never sees a stray byte the two paths would treat +// differently. +func FuzzNormalizeSourceValidUTF8(f *testing.F) { + for _, s := range []string{"abc\xe9def", "正常なテキスト", "\xff\xfe", ""} { + f.Add(s) + } + f.Fuzz(func(t *testing.T, s string) { + if out := NormalizeSource(s); !utf8.ValidString(out) { + t.Fatalf("NormalizeSource must return valid UTF-8, invalid for %q", s) + } + }) +} + +// FuzzSplitChunksPreservesContent asserts chunking loses no non-space content: the +// count of non-space runes across all chunks equals that of the normalized source. +func FuzzSplitChunksPreservesContent(f *testing.F) { + for _, s := range []string{ + "静かな朝。\n\n「止まれ！」と彼は言った。次へ。", strings.Repeat("слово ", 500), "abc\xe9def", "", + } { + f.Add(s) + } + f.Fuzz(func(t *testing.T, s string) { + norm := NormalizeSource(s) + want := countNonSpace(norm) + got := 0 + for _, c := range SplitChunks([]string{norm}) { + got += countNonSpace(c.Text) + } + if got != want { + t.Fatalf("chunking lost non-space content: got %d want %d (input %q)", got, want, s) + } + }) +} + +func countNonSpace(s string) int { + n := 0 + for _, r := range s { + if !unicode.IsSpace(r) { + n++ + } + } + return n +} + // trimAll TrimSpaces each segment and drops empties (test convenience). func trimAll(segs []string) []string { var out []string diff --git a/backend/internal/pipeline/ingest.go b/backend/internal/pipeline/ingest.go index 03d6bfd..6a9e44f 100644 --- a/backend/internal/pipeline/ingest.go +++ b/backend/internal/pipeline/ingest.go @@ -165,11 +165,21 @@ func ingestEPUB(p string) (*Document, error) { doc := &Document{} denseNo := 0 // matches SplitChunks: only a NON-empty chapter takes a number read := 0 + var dangling []string for _, ref := range opf.Spine { item, ok := byID[ref.IDRef] - if !ok || !isXHTML(item.mediaType, item.href) { + if !ok { + // A spine itemref is a DECLARED reading-order document; a dangling idref + // (no manifest item) is a LOST chapter, not a skip. Silently continuing + // would drop its content AND shift every later chapter's dense number + // (since_ch for memory v2). Collect all, fail loud after the loop — the + // same fail-loud a missing zip entry already gets (external-review #1). + dangling = append(dangling, ref.IDRef) continue } + if !isXHTML(item.mediaType, item.href) { + continue // a genuine non-content asset in the spine (image/css) — no text + } entry := resolveHref(opfDir, item.href) data, err := readZipEntry(files, entry) if err != nil { @@ -196,6 +206,9 @@ func ingestEPUB(p string) (*Document, error) { } doc.Ruby = append(doc.Ruby, ruby...) } + if len(dangling) > 0 { + return nil, fmt.Errorf("pipeline: ingest epub: spine references %d manifest id(s) with no manifest item %v — a chapter would be silently lost", len(dangling), dangling) + } if read == 0 { return nil, fmt.Errorf("pipeline: ingest epub: spine resolved to no readable (x)html chapters") } @@ -203,23 +216,25 @@ func ingestEPUB(p string) (*Document, error) { } // isXHTML reports whether a manifest item is an (x)html content document. It -// checks the media-type (parameters like "; charset=utf-8" stripped), then falls -// back to the href extension for ANY unrecognized/absent type — real epubs ship -// xhtml chapters typed application/xml or text/xml, so a content-document extension -// is the reliable signal. A genuine non-content asset keeps its own extension -// (.jpg/.css/.ncx/.svg), so the extension fallback does not misclassify it -// (self-review: a present-but-unrecognized type used to silently drop the chapter). +// checks the media-type (parameters like "; charset=utf-8" stripped), accepting +// the (x)html types AND the generic XML types real epubs mislabel xhtml chapters +// with (application/xml, text/xml), then falls back to a content-document href +// extension for ANY unrecognized/absent type. A genuine non-content asset keeps its +// own extension (.jpg/.css/.ncx/.svg), so the fallback does not misclassify it. +// (External-review #2: a chapter typed application/xml with a .xml href used to slip +// past BOTH the type switch and a .xhtml-only extension fallback and vanish silently.) func isXHTML(mediaType, href string) bool { mt := strings.ToLower(strings.TrimSpace(mediaType)) if i := strings.IndexByte(mt, ';'); i >= 0 { mt = strings.TrimSpace(mt[:i]) } switch mt { - case "application/xhtml+xml", "text/html", "application/html": + case "application/xhtml+xml", "text/html", "application/html", + "application/xml", "text/xml": return true } switch extOf(href) { - case ".xhtml", ".html", ".htm": + case ".xhtml", ".html", ".htm", ".xml": return true } return false @@ -323,7 +338,7 @@ func extractXHTML(data []byte) (string, []RubyReading, error) { rtDepth++ case name == "rp": rpDepth++ - case name == "br": + case name == "br" && rubyDepth == 0: body.WriteByte('\n') case blockTags[name] && rubyDepth == 0: body.WriteString("\n\n") diff --git a/backend/internal/pipeline/ingest_test.go b/backend/internal/pipeline/ingest_test.go index 5899a72..b571f9b 100644 --- a/backend/internal/pipeline/ingest_test.go +++ b/backend/internal/pipeline/ingest_test.go @@ -123,7 +123,7 @@ func buildEPUBAt(t *testing.T, path string, chapters []epubChapter, spineIDs []s // The FILE is (x)html when its href says so, regardless of how the manifest // media-type labels it — real epubs mislabel xhtml as application/xml, etc. switch extOf(c.href) { - case ".xhtml", ".html", ".htm": + case ".xhtml", ".html", ".htm", ".xml": add("OEBPS/"+c.href, ` c `+c.body+``) @@ -240,10 +240,11 @@ func TestIngestEPUBRubyChapterMatchesDenseNumbering(t *testing.T) { } func TestIngestEPUBAcceptsGenericAndParameterizedMediaTypes(t *testing.T) { - // Real epubs mislabel xhtml chapters as application/xml or text/xml, or add a - // "; charset=utf-8" parameter. All must still be read — not silently dropped. + // Real epubs mislabel xhtml chapters as application/xml or text/xml (incl. a .xml + // href — external-review #2), or add a "; charset=utf-8" parameter. All must be + // read, not silently dropped (a dropped chapter also shifts every since_ch after). chapters := []epubChapter{ - {id: "c1", href: "ch1.xhtml", mtype: "application/xml", body: `

第一章。

`}, + {id: "c1", href: "ch1.xml", mtype: "application/xml", body: `

第一章。

`}, {id: "c2", href: "ch2.xhtml", mtype: "text/xml", body: `

第二章。

`}, {id: "c3", href: "ch3.xhtml", mtype: "application/xhtml+xml; charset=utf-8", body: `

第三章。

`}, } @@ -261,6 +262,39 @@ func TestIngestEPUBAcceptsGenericAndParameterizedMediaTypes(t *testing.T) { } } +func TestIngestEPUBDanglingIdrefFailsLoud(t *testing.T) { + // A spine idref with no manifest item, AMONG valid chapters, must fail loud — not + // silently drop that chapter (which would also shift every later since_ch). + chapters := []epubChapter{ + {id: "c1", href: "ch1.xhtml", body: `

第一章。

`}, + {id: "c2", href: "ch2.xhtml", body: `

第二章。

`}, + {id: "c3", href: "ch3.xhtml", body: `

第三章。

`}, + } + _, err := Ingest(buildEPUB(t, chapters, []string{"c1", "ghost", "c3"})) + if err == nil { + t.Fatal("a dangling spine idref among valid chapters must fail loud, not silently lose a chapter") + } + if !strings.Contains(err.Error(), "ghost") { + t.Fatalf("error should name the dangling idref, got: %v", err) + } +} + +func TestIngestEPUBBrInsideRubyDoesNotLeak(t *testing.T) { + // A
inside must not inject a newline between base glyphs (it would + // desync base from body and pollute ch.Text — external-review #5). + body := `

漢
字かんじを読む。

` + doc, err := Ingest(buildEPUB(t, []epubChapter{{id: "c1", href: "ch1.xhtml", body: body}}, []string{"c1"})) + if err != nil { + t.Fatal(err) + } + if len(doc.Ruby) != 1 || doc.Ruby[0].Base != "漢字" || doc.Ruby[0].Reading != "かんじ" { + t.Fatalf("ruby base/reading = %#v", doc.Ruby) + } + if !strings.Contains(doc.Chapters[0], "漢字を読む") || strings.Contains(doc.Chapters[0], "漢\n字") { + t.Fatalf("
leaked a newline between base glyphs: %q", doc.Chapters[0]) + } +} + func TestIngestEPUBRubyBaseWhitespaceCollapsed(t *testing.T) { // Pretty-printed jukugo ruby: whitespace BETWEEN base segments must not // enter the captured base (mis-keys the glossary) nor the body (pollutes ch.Text). diff --git a/backend/internal/pipeline/render.go b/backend/internal/pipeline/render.go index f8073ae..5d9ca40 100644 --- a/backend/internal/pipeline/render.go +++ b/backend/internal/pipeline/render.go @@ -39,8 +39,11 @@ import ( // re-translation), never a silent divergent re-pay. NOTE: the chunk target is a // const covered by THIS string; if it ever becomes config-tunable it must ALSO be // folded into the top-level snapshot (like coverageSnap), never left to the -// version alone (§7d). -const chunkerVersion = "chunker-v3-sentence-pack" +// version alone (§7d). Bumped v3→v4 closing external-review 3a: invalid-UTF-8 is +// now normalized once in NormalizeSource (#3), the ASCII sentence boundary honors +// quote-depth (#4), and
inside no longer leaks a newline into the body +// (#5) — all segmentation/ingest behaviour changes → a loud --resnapshot. +const chunkerVersion = "chunker-v4-utf8-quotedepth" // estimatorVersion versions EstimateTokens: его выход входит в max_tokens и // через него в request-hash, поэтому перекалибровка весов — тоже явная @@ -234,6 +237,13 @@ func NormalizeSource(s string) string { s = strings.TrimPrefix(s, "\uFEFF") // UTF-8 BOM s = strings.ReplaceAll(s, "\r\n", "\n") s = strings.ReplaceAll(s, "\r", "\n") + // Replace invalid UTF-8 with U+FFFD exactly ONCE, here, so all downstream + // segmentation sees byte-valid text. \u0411\u0435\u0437 \u044D\u0442\u043E\u0433\u043E \u0447\u0430\u043D\u043A\u0435\u0440 \u0440\u0430\u0441\u0445\u043E\u0434\u0438\u0442\u0441\u044F \u0441 \u0441\u0430\u043C\u0438\u043C \u0441\u043E\u0431\u043E\u0439: + // \u0435\u0433\u043E []rune-\u043F\u0443\u0442\u044C \u043F\u043E \u043F\u0440\u0435\u0434\u043B\u043E\u0436\u0435\u043D\u0438\u044F\u043C (chunker.go) \u043C\u043E\u043B\u0447\u0430 \u0437\u0430\u043C\u0435\u043D\u044F\u043B \u0431\u044B \u0441\u0442\u0440\u0435\u0439-\u0431\u0430\u0439\u0442 \u043D\u0430 + // U+FFFD, \u0430 \u0441\u0442\u0440\u043E\u043A\u043E\u0432\u044B\u0439 \u043F\u0443\u0442\u044C \u043F\u043E \u0430\u0431\u0437\u0430\u0446\u0430\u043C \u0441\u043E\u0445\u0440\u0430\u043D\u044F\u043B \u0431\u044B \u0441\u044B\u0440\u043E\u0439 \u2014 size-\u0437\u0430\u0432\u0438\u0441\u0438\u043C\u0430\u044F \u043F\u043E\u0440\u0447\u0430, + // \u0442\u0435\u043A\u0443\u0449\u0430\u044F \u0432 {{text}} \u0438 request_hash (\u0432\u043D\u0435\u0448\u043D\u0435\u0435 \u0440\u0435\u0432\u044C\u044E #3). \u042D\u0442\u043E \u043F\u0440\u0430\u0432\u0438\u043B\u043E \u0438\u043D\u0436\u0435\u0441\u0442\u0430 \u2192 + // \u043F\u043E\u043A\u0440\u044B\u0442\u043E chunkerVersion (\u043E\u0441\u043E\u0437\u043D\u0430\u043D\u043D\u0430\u044F \u0437\u0430\u043C\u0435\u043D\u0430, \u043D\u0435 \u0442\u0438\u0445\u0430\u044F). + s = strings.ToValidUTF8(s, "\uFFFD") s = norm.NFC.String(s) return strings.TrimSpace(s) } diff --git a/backend/internal/pipeline/runner.go b/backend/internal/pipeline/runner.go index dc9a9d2..2dc0b47 100644 --- a/backend/internal/pipeline/runner.go +++ b/backend/internal/pipeline/runner.go @@ -522,16 +522,15 @@ func (r *Runner) TranslateBook(ctx context.Context) (*BookResult, error) { // persistRuby aggregates the ingested ruby occurrences into one row per // (base, reading) — first_chapter = MIN, occurrences = full-book count — and -// upserts them (v4 schema). Idempotent: the aggregation is recomputed identically -// on every ingest, so a resume re-writes the same rows (MIN keeps the earliest -// chapter, occurrences is replaced with the recomputed count). The write order is -// sorted for deterministic, test-stable behavior; correctness does not depend on it -// (the upsert is commutative). Memory v2 (шаг 4) consumes ruby_readings into a -// glossary name-lock (D9); nothing here injects into a prompt (§7d). +// REPLACES the book's whole ruby set (store.ReplaceRubyReadings). Idempotent: the +// aggregation is recomputed identically on every ingest, so a resume re-writes the +// same rows; a source edit converges every column — a pair removed by the edit +// disappears instead of lingering as a phantom (external-review #6). Called +// unconditionally (even for zero readings — a txt book or a source that dropped its +// furigana) so the full-replace clears any stale rows. The write order is sorted for +// deterministic, test-stable behavior. Memory v2 (шаг 4) consumes ruby_readings into +// a glossary name-lock (D9); nothing here injects into a prompt (§7d). func (r *Runner) persistRuby(readings []RubyReading) error { - if len(readings) == 0 { - return nil - } type agg struct { first int count int @@ -557,16 +556,15 @@ func (r *Runner) persistRuby(readings []RubyReading) error { } return order[i][1] < order[j][1] }) + rows := make([]store.RubyReading, 0, len(order)) for _, key := range order { a := seen[key] - if err := r.Store.UpsertRubyReading(store.RubyReading{ + rows = append(rows, store.RubyReading{ BookID: r.Book.BookID, Base: key[0], Reading: key[1], FirstChapter: a.first, Occurrences: a.count, - }); err != nil { - return err - } + }) } - return nil + return r.Store.ReplaceRubyReadings(r.Book.BookID, rows) } // translateChunk drives ONE chunk through the stage list. Stages run in order, diff --git a/backend/internal/store/migrate.go b/backend/internal/store/migrate.go index e293fff..156dd83 100644 --- a/backend/internal/store/migrate.go +++ b/backend/internal/store/migrate.go @@ -143,11 +143,12 @@ var migrations = []string{ // (book_id, base, reading): the same name may carry several readings across a // book (variant yomi), each a distinct row. first_chapter = the MIN chapter the // pair appears in (a glossary "since"); occurrences = the pair's full-book count - // (a memory-v2 confidence signal). The persist is idempotent: ingest aggregates - // each (base,reading) once over the WHOLE book, so re-ingesting the same source - // re-derives identical rows and BOTH first_chapter and occurrences are REPLACED - // with those authoritative full-book values (never an increment or a MIN-pin that - // would drift on resume or stale-early after a source edit). + // (a memory-v2 confidence signal). The persist is idempotent AND a full replace: + // ingest aggregates each (base,reading) once over the WHOLE book, and persistRuby + // DELETEs the book's set before re-inserting (store.ReplaceRubyReadings), so + // re-ingesting the same source re-derives identical rows and a source edit + // converges every column — including DROPPING a pair the edit removed (never a + // lingering phantom, never a MIN-pin/increment that drifts on resume). ` CREATE TABLE IF NOT EXISTS ruby_readings ( book_id TEXT NOT NULL, diff --git a/backend/internal/store/ruby.go b/backend/internal/store/ruby.go index 5e1cad8..15adcdf 100644 --- a/backend/internal/store/ruby.go +++ b/backend/internal/store/ruby.go @@ -19,27 +19,34 @@ type RubyReading struct { Occurrences int // full-book count of the pair } -// UpsertRubyReading writes (or converges) one ruby row. Idempotent by design: the -// caller (persistRuby) recomputes the WHOLE-book aggregate on every ingest, so it -// passes the authoritative first_chapter (the full-book MIN) and occurrences (the -// full-book count) already. Both columns are therefore REPLACED on conflict — -// re-ingesting the identical source rewrites identical values (idempotent), and a -// source edit that moves the pair's first appearance re-converges to the truth. A -// MIN-merge here would instead pin first_chapter to a stale-early chapter the pair -// no longer occupies after such an edit (self-review finding), asymmetric with -// occurrences. Callers pass one row per (base,reading); the ON CONFLICT only fires -// across separate ingests of the same book. -func (s *Store) UpsertRubyReading(rr RubyReading) error { +// ReplaceRubyReadings replaces a book's ENTIRE ruby set with rows in one +// transaction (DELETE-by-book, then INSERT each). The caller (persistRuby) +// recomputes the whole-book aggregate on every ingest, so a full replace is the +// correct idempotency model: re-ingesting the identical source rewrites identical +// rows; a source edit converges every column to the truth — including a pair the +// edit REMOVED, which a prior upsert-only path left as a permanent phantom (a +// name-lock on a name no longer in the book — external-review #6). Scoped to +// book_id, so other books are untouched. Each row's BookID must equal bookID. +func (s *Store) ReplaceRubyReadings(bookID string, rows []RubyReading) error { ctx, cancel := opContext() defer cancel() - _, err := s.w.ExecContext(ctx, ` - INSERT INTO ruby_readings (book_id, base, reading, first_chapter, occurrences) - VALUES (?, ?, ?, ?, ?) - ON CONFLICT (book_id, base, reading) DO UPDATE SET - first_chapter = excluded.first_chapter, - occurrences = excluded.occurrences`, - rr.BookID, rr.Base, rr.Reading, rr.FirstChapter, rr.Occurrences) - return err + tx, err := s.w.BeginTx(ctx, nil) + if err != nil { + return err + } + defer tx.Rollback() + if _, err := tx.ExecContext(ctx, `DELETE FROM ruby_readings WHERE book_id = ?`, bookID); err != nil { + return err + } + for _, rr := range rows { + if _, err := tx.ExecContext(ctx, ` + INSERT INTO ruby_readings (book_id, base, reading, first_chapter, occurrences) + VALUES (?, ?, ?, ?, ?)`, + rr.BookID, rr.Base, rr.Reading, rr.FirstChapter, rr.Occurrences); err != nil { + return err + } + } + return tx.Commit() } // RubyReadingsForBook returns every captured reading for a book, ordered diff --git a/backend/internal/store/ruby_test.go b/backend/internal/store/ruby_test.go index 1ea976e..d031580 100644 --- a/backend/internal/store/ruby_test.go +++ b/backend/internal/store/ruby_test.go @@ -5,17 +5,20 @@ import ( "testing" ) -func TestRubyReadingsUpsertAndRead(t *testing.T) { +func rr(book, base, reading string, first, occ int) RubyReading { + return RubyReading{BookID: book, Base: base, Reading: reading, FirstChapter: first, Occurrences: occ} +} + +func TestRubyReadingsReplaceAndRead(t *testing.T) { s, _ := openTemp(t) - rows := []RubyReading{ - {BookID: "b", Base: "漢字", Reading: "かんじ", FirstChapter: 2, Occurrences: 5}, - {BookID: "b", Base: "東京", Reading: "とうきょう", FirstChapter: 1, Occurrences: 3}, - {BookID: "other", Base: "京", Reading: "きょう", FirstChapter: 1, Occurrences: 1}, + if err := s.ReplaceRubyReadings("b", []RubyReading{ + rr("b", "漢字", "かんじ", 2, 5), + rr("b", "東京", "とうきょう", 1, 3), + }); err != nil { + t.Fatal(err) } - for _, rr := range rows { - if err := s.UpsertRubyReading(rr); err != nil { - t.Fatal(err) - } + if err := s.ReplaceRubyReadings("other", []RubyReading{rr("other", "京", "きょう", 1, 1)}); err != nil { + t.Fatal(err) } got, err := s.RubyReadingsForBook("b") if err != nil { @@ -23,59 +26,49 @@ func TestRubyReadingsUpsertAndRead(t *testing.T) { } // Ordered by (first_chapter, base, reading): 東京(ch1) before 漢字(ch2). want := []RubyReading{ - {BookID: "b", Base: "東京", Reading: "とうきょう", FirstChapter: 1, Occurrences: 3}, - {BookID: "b", Base: "漢字", Reading: "かんじ", FirstChapter: 2, Occurrences: 5}, + rr("b", "東京", "とうきょう", 1, 3), + rr("b", "漢字", "かんじ", 2, 5), } if !reflect.DeepEqual(got, want) { t.Fatalf("read = %#v, want %#v (other book must not leak)", got, want) } } -func TestRubyReadingsUpsertIsIdempotentAndConverges(t *testing.T) { +func TestRubyReadingsReplaceIsFullAndIdempotent(t *testing.T) { s, _ := openTemp(t) - rr := RubyReading{BookID: "b", Base: "漢字", Reading: "かんじ", FirstChapter: 3, Occurrences: 4} - if err := s.UpsertRubyReading(rr); err != nil { + first := []RubyReading{rr("b", "漢字", "かんじ", 3, 4), rr("b", "海", "うみ", 1, 2)} + if err := s.ReplaceRubyReadings("b", first); err != nil { t.Fatal(err) } - // Re-ingesting the IDENTICAL full-book aggregate is a no-op (idempotent): one row, - // same values, occurrences never doubled. - if err := s.UpsertRubyReading(rr); err != nil { + // Re-ingesting the identical aggregate is a no-op (idempotent). + if err := s.ReplaceRubyReadings("b", first); err != nil { t.Fatal(err) } - got, err := s.RubyReadingsForBook("b") - if err != nil { + got, _ := s.RubyReadingsForBook("b") + if len(got) != 2 { + t.Fatalf("identical re-replace must not add rows, got %#v", got) + } + // A source edit re-ingest: 海 removed entirely, 漢字 moved to ch 1 with a new count. + // The full replace must DROP the phantom 海 and CONVERGE 漢字 — a prior upsert-only + // path would have left 海 forever (external-review #6). + if err := s.ReplaceRubyReadings("b", []RubyReading{rr("b", "漢字", "かんじ", 1, 2)}); err != nil { t.Fatal(err) } - if len(got) != 1 || got[0].FirstChapter != 3 || got[0].Occurrences != 4 { - t.Fatalf("identical re-upsert must be a no-op: %#v", got) - } - // A re-ingest after a source edit (e.g. a foreword inserted) moves the pair's - // first appearance to ch 4 and changes the count. persistRuby recomputes the - // authoritative full-book aggregate, so the store must REPLACE BOTH columns — - // it must NOT MIN-pin first_chapter to the stale-early 3 where the pair no - // longer occurs (self-review finding). - if err := s.UpsertRubyReading(RubyReading{BookID: "b", Base: "漢字", Reading: "かんじ", FirstChapter: 4, Occurrences: 2}); err != nil { - t.Fatal(err) - } - got, err = s.RubyReadingsForBook("b") - if err != nil { - t.Fatal(err) - } - if len(got) != 1 || got[0].FirstChapter != 4 || got[0].Occurrences != 2 { - t.Fatalf("re-ingest must REPLACE (converge) both columns, not MIN-pin: %#v", got) + got, _ = s.RubyReadingsForBook("b") + want := []RubyReading{rr("b", "漢字", "かんじ", 1, 2)} + if !reflect.DeepEqual(got, want) { + t.Fatalf("full replace must drop the removed pair and converge, got %#v", got) } } func TestRubyReadingsVariantReadingsAreDistinctRows(t *testing.T) { // The same base with two readings (variant yomi) are two rows, not a collision. s, _ := openTemp(t) - for _, rr := range []RubyReading{ - {BookID: "b", Base: "海", Reading: "うみ", FirstChapter: 1, Occurrences: 2}, - {BookID: "b", Base: "海", Reading: "かい", FirstChapter: 4, Occurrences: 1}, - } { - if err := s.UpsertRubyReading(rr); err != nil { - t.Fatal(err) - } + if err := s.ReplaceRubyReadings("b", []RubyReading{ + rr("b", "海", "うみ", 1, 2), + rr("b", "海", "かい", 4, 1), + }); err != nil { + t.Fatal(err) } got, err := s.RubyReadingsForBook("b") if err != nil { @@ -85,3 +78,18 @@ func TestRubyReadingsVariantReadingsAreDistinctRows(t *testing.T) { t.Fatalf("variant readings must be distinct rows, got %#v", got) } } + +func TestRubyReadingsReplaceEmptyClears(t *testing.T) { + // A source that dropped all its furigana must clear the book's rows. + s, _ := openTemp(t) + if err := s.ReplaceRubyReadings("b", []RubyReading{rr("b", "漢字", "かんじ", 1, 1)}); err != nil { + t.Fatal(err) + } + if err := s.ReplaceRubyReadings("b", nil); err != nil { + t.Fatal(err) + } + got, _ := s.RubyReadingsForBook("b") + if len(got) != 0 { + t.Fatalf("empty replace must clear the book, got %#v", got) + } +} diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index 92e6716..a27df24 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -414,6 +414,22 @@ keep-alive (Ф1–2), инъекция глоссария (`selective`), пор > > **Приоритет фикса:** #1+#2 (тихая потеря главы, системно через spine-reconciliation) → #3 (losslessness UTF-8) → #7 (fuzz-тест, поймал бы #3) → #4 (quote-depth ASCII) → #5/#6 (мелочь, #6 — с шагом 4). Ничего не горит на боевом (все триггеры — битый/мислейбленный epub или invalid-UTF-8), но #1/#2 — именно тот тихий-потери класс, где мы обязаны быть fail-loud. +### 2026-07-05 — Шаг 3a: закрытие 7 находок внешнего ревью (все, в приоритете оркестратора) + +Все 7 закрыты в этой же сессии (автор с контекстом — дешевле, чем перезагружать свежую), каждый фикс **мутационно-проверен** (сломай — падает именно его тест) + три fuzz-таргета прогнаны ~200k+ рандом-инпутов live. Всё зелёное `go build/vet/test -race`, `tmctl report` $0. `chunkerVersion` бампнут `v3-sentence-pack`→**`v4-utf8-quotedepth`** (правки нормализации/сегментации/инжеста = осознанная инвалидация, `--resnapshot`). + +**Major (тихая потеря главы/контента):** +- **#1+#2 — spine-reconciliation (`ingest.go`).** Dangling spine idref (опечатка, орфанящая главу) больше НЕ скипается тихо: собираются все неразрешённые idref → **fail-loud** (как пропавший zip-entry) — иначе терялась глава И сдвигался `since_ch` всего хвоста. `isXHTML` расширен: generic-XML MIME (`application/xml`/`text/xml`) + расширение `.xml` распознаются как контент (мислейбленная `.xml`-глава больше не исчезает). Не-контент-ассеты в spine (image/css) по-прежнему легитимно скипаются. +- **#3 — invalid-UTF-8 в `NormalizeSource` (`render.go`).** `strings.ToValidUTF8(…, U+FFFD)` ОДИН раз в нормализации → чанкер больше не расходится с собой ([]rune-путь по предложениям vs строковый по абзацам молча по-разному трактовали стрей-байт — size-зависимая порча в `{{text}}`/`request_hash`). `FuzzNormalizeSourceValidUTF8` гарантирует валидный UTF-8 на выходе. + +**Minor:** +- **#4 — quote-depth в ASCII-ветке (`chunker.go`).** ASCII/эллипсис-граница теперь тоже гейтится `depthHere==0` → `「Stop. Now.」` не over-splitится (straight-quote англ. не регрессит — untracked, depth 0). +- **#5 — `
` внутри `` (`ingest.go`)** гардится `rubyDepth==0` → `\n` больше не течёт между base-глифами. +- **#6 — ruby phantom-строки (`ruby.go`/`persistRuby`).** `UpsertRubyReading`→**`ReplaceRubyReadings`** (DELETE-by-book + INSERT в одной tx): полная замена агрегата → пара, убранная правкой источника, исчезает, а не остаётся фантомом; `persistRuby` зовётся безусловно (пустой набор чистит книгу). «Сходится к правде» теперь верно и для удалений — контракт под память v2. +- **#7 — fuzz/property tiling (`chunker_test.go`).** `FuzzSplitSourceSentencesTiles` (rune-lossless всегда, байт-точно на валидном UTF-8), `FuzzNormalizeSourceValidUTF8`, `FuzzSplitChunksPreservesContent` (Σ не-пробел чанков == Σ не-пробел источника) — поймали бы #3. + +Отсеянная находка (oversize-хвост <500) осталась документированной границей §7b. Дальше — банк памяти v2 (шаг 4): реестр рисков `architecture/06` + ruby-контракт готовы (F1-materialization + горячий путь + post-check). Онбординг-промт след. сессии — `docs/BACKEND_SESSION_PROMPT_MEMORY.md`. + ## Полигон (секция параллельной сессии — записи добавлять сюда)