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
This commit is contained in:
parent
6700a0f2a6
commit
6d96bb4ac8
10 changed files with 279 additions and 99 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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, `<?xml version="1.0" encoding="utf-8"?>
|
||||
<html xmlns="http://www.w3.org/1999/xhtml"><head><title>c</title><style>.x{color:red}</style></head>
|
||||
<body>`+c.body+`</body></html>`)
|
||||
|
|
@ -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: `<p>第一章。</p>`},
|
||||
{id: "c1", href: "ch1.xml", mtype: "application/xml", body: `<p>第一章。</p>`},
|
||||
{id: "c2", href: "ch2.xhtml", mtype: "text/xml", body: `<p>第二章。</p>`},
|
||||
{id: "c3", href: "ch3.xhtml", mtype: "application/xhtml+xml; charset=utf-8", body: `<p>第三章。</p>`},
|
||||
}
|
||||
|
|
@ -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: `<p>第一章。</p>`},
|
||||
{id: "c2", href: "ch2.xhtml", body: `<p>第二章。</p>`},
|
||||
{id: "c3", href: "ch3.xhtml", body: `<p>第三章。</p>`},
|
||||
}
|
||||
_, 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 <br> inside <ruby> must not inject a newline between base glyphs (it would
|
||||
// desync base from body and pollute ch.Text — external-review #5).
|
||||
body := `<p><ruby>漢<br/>字<rt>かんじ</rt></ruby>を読む。</p>`
|
||||
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("<br> leaked a newline between base glyphs: %q", doc.Chapters[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestIngestEPUBRubyBaseWhitespaceCollapsed(t *testing.T) {
|
||||
// Pretty-printed jukugo ruby: whitespace BETWEEN <rb> base segments must not
|
||||
// enter the captured base (mis-keys the glossary) nor the body (pollutes ch.Text).
|
||||
|
|
|
|||
|
|
@ -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 <br> inside <ruby> 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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 — `<br>` внутри `<ruby>` (`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`.
|
||||
|
||||
## Полигон
|
||||
(секция параллельной сессии — записи добавлять сюда)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue