From 03a6040360048186289b05fc1d79efa8474af470 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Mon, 12 Oct 2026 20:46:08 -0400 Subject: [PATCH] test(sync): add manual repro harness for CRE-to-CFI highlight conversion TestManualCleanRepro replays a real device highlight export through the same conversion path the sync handler uses, diffs the output against the CFIs stored at ingest time, and computes a ground-truth oracle by locating each selection in the flattened document itself. Classifies every row as healthy / diff-base / backwards for both the converter and the stored data. Skips automatically when fixture files are not present on the machine (manual test, like cfi_verify_manual_test.go). REPRO_VERBOSE=1 prints per-row detail; REPRO_EMIT_SQL= emits idempotent UPDATE statements that heal database rows written by the old converter from the preserved raw device positions - used to backfill dev and validated against production data. --- internal/sync/cfi_repro_clean_test.go | 379 ++++++++++++++++++++++++++ 1 file changed, 379 insertions(+) create mode 100644 internal/sync/cfi_repro_clean_test.go diff --git a/internal/sync/cfi_repro_clean_test.go b/internal/sync/cfi_repro_clean_test.go new file mode 100644 index 00000000..f5810291 --- /dev/null +++ b/internal/sync/cfi_repro_clean_test.go @@ -0,0 +1,379 @@ +package sync + +// Manual repro harness for the CRE→CFI highlight conversion defects found in +// "Clean" (Rosser K Powitzky): 28 device highlights whose stored web CFIs are +// structurally inconsistent (mismatched start/end base paths, backwards +// ranges). This test mirrors handlers.convertHighlightPositions exactly — +// pos0 converted WITH the selection text as context, pos1 WITHOUT any — then +// reports the result alongside the CFIs stored at ingest time and a +// ground-truth oracle computed by locating the selection text in the document +// itself. Skips silently when the book/export are not on this machine, like +// cfi_verify_manual_test.go. + +import ( + "fmt" + "encoding/json" + "os" + "strings" + "testing" + + "golang.org/x/net/html" +) + +const ( + reproExport = "/home/nymusicman/Code/bookhoard/export/media_highlights.json" + reproEpub = "/home/nymusicman/Code/bookhoard/uploads/Ebooks/Rosser K Powitzky/Clean_ How the Jewish Roots of Atone (77)/Clean_ How the Jewish Roots of - Rosser K Powitzky.epub" +) + +type reproRow struct { + ID string `json:"id"` + StartPosition string `json:"start_position"` + EndPosition string `json:"end_position"` + StoredStart string `json:"epubcfi_start"` + StoredEnd string `json:"epubcfi_end"` + SelectionText string `json:"selection_text"` + PctStart float64 `json:"percentage_start"` +} + +// webUsable mirrors handlers.webUsableCFI (unexported there; cannot import +// the handlers package from here without a cycle). +func webUsable(precision, cfi string) string { + if precision != "structural" && precision != "exact" { + return "" + } + if !strings.HasPrefix(cfi, "epubcfi(") || !strings.HasSuffix(cfi, ")") { + return "" + } + return cfi +} + +// reproExtendCFIByLength mirrors handlers.extendCFIByLength: advance the +// trailing character offset of a point CFI by the UTF-16 length of text. +func reproExtendCFIByLength(cfi, text string) string { + if cfi == "" || text == "" { + return "" + } + if !strings.HasSuffix(cfi, ")") { + return "" + } + inner := cfi[:len(cfi)-1] + idx := strings.LastIndex(inner, ":") + if idx < 0 { + return cfi + } + var off int + if _, err := fmtSscanInt(inner[idx+1:], &off); err != nil { + return cfi + } + return inner[:idx+1] + itoa(off+utf16Len(text)) + ")" +} + +// cfiBase splits "epubcfi(...)" into (base, offset): base is everything +// before the final ":N" character-offset terminator. +func cfiBase(cfi string) (string, int, bool) { + if !strings.HasSuffix(cfi, ")") { + return "", 0, false + } + inner := cfi[:len(cfi)-1] + idx := strings.LastIndex(inner, ":") + if idx < 0 { + return inner, 0, true + } + off := 0 + if _, err := fmtSscanInt(inner[idx+1:], &off); err != nil { + return inner, 0, true + } + return inner[:idx], off, true +} + +func fmtSscanInt(s string, out *int) (int, error) { + n := 0 + ok := false + for _, r := range s { + if r < '0' || r > '9' { + break + } + n = n*10 + int(r-'0') + ok = true + } + if !ok { + return 0, errReproNoInt + } + *out = n + return len(s), nil +} + +var errReproNoInt = errReproNoIntErr{} + +type errReproNoIntErr struct{} + +func (errReproNoIntErr) Error() string { return "no integer" } + +func itoa(n int) string { + if n == 0 { + return "0" + } + neg := n < 0 + if neg { + n = -n + } + var b [20]byte + i := len(b) + for n > 0 { + i-- + b[i] = byte('0' + n%10) + n /= 10 + } + if neg { + i-- + b[i] = '-' + } + return string(b[i:]) +} + +// oracleCFIs locates the selection text in the document named by the start +// xpointer's fragment and returns ground-truth (startCFI, endCFI, spineIndex). +// Whitespace runs are collapsed on both sides before matching, so device +// text with reflowed newlines still lands. +func oracleCFIs(t *testing.T, conv *CFIConverter, row reproRow) (string, string, string) { + xp, err := ParseCREXPointer(row.StartPosition) + if err != nil { + return "", "", "unparseable start xpointer" + } + doc, href, err := conv.getContentDoc(xp.FragmentIndex) + if err != nil { + return "", "", "content doc missing: " + err.Error() + } + body := findBody(doc) + if body == nil { + return "", "", "no body" + } + + // Flatten every text node in document order. + var segs []textSegment + var walk func(*html.Node) + walk = func(n *html.Node) { + if n.Type == html.TextNode { + segs = append(segs, textSegment{node: n, runes: []rune(n.Data)}) + } + for c := n.FirstChild; c != nil; c = c.NextSibling { + walk(c) + } + } + walk(body) + + var flat []rune + for _, s := range segs { + flat = append(flat, s.runes...) + } + + needle := []rune(normalizeWhitespace(row.SelectionText)) + if len(needle) == 0 { + return "", "", "empty selection text" + } + // Collapse whitespace in `flat` while mapping each kept rune back to a + // flat index. + var norm []rune + var normToFlat []int + lastWS := false + for i, r := range flat { + isWS := r == ' ' || r == '\t' || r == '\n' || r == '\r' || r == ' ' || r == '​' + if isWS { + if lastWS { + continue + } + norm = append(norm, ' ') + normToFlat = append(normToFlat, i) + lastWS = true + continue + } + norm = append(norm, r) + normToFlat = append(normToFlat, i) + lastWS = false + } + + // Trim wrapped-quote artifacts KOReader adds to partial-paragraph text. + trimmed := needle + for len(trimmed) > 0 && (trimmed[0] == '{' || trimmed[0] == '}') { + trimmed = trimmed[1:] + } + for len(trimmed) > 0 && (trimmed[len(trimmed)-1] == '{' || trimmed[len(trimmed)-1] == '}') { + trimmed = trimmed[:len(trimmed)-1] + } + + pos := indexRunes(norm, trimmed) + if pos < 0 { + return "", "", "selection text not found in doc " + href + } + + flatStart := normToFlat[pos] + flatEnd := normToFlat[pos+len(trimmed)-1] + 1 // end is exclusive + + nodeAt := func(flatIdx int) (*html.Node, int) { + acc := 0 + for _, s := range segs { + n := len(s.runes) + if flatIdx < acc+n { + return s.node, flatIdx - acc + } + acc += n + } + return nil, 0 + } + + startNode, startOff := nodeAt(flatStart) + endNode, endOff := nodeAt(flatEnd) + if startNode == nil || endNode == nil { + return "", "", "flat index out of range" + } + + spineIndex := xp.FragmentIndex - 1 + startCFI, err := buildCFI(spineIndex, startNode, startOff) + if err != nil { + return "", "", "buildCFI start: " + err.Error() + } + endCFI, err := buildCFI(spineIndex, endNode, endOff) + if err != nil { + return "", "", "buildCFI end: " + err.Error() + } + return startCFI, endCFI, "" +} + +func indexRunes(hay, needle []rune) int { + if len(needle) == 0 || len(needle) > len(hay) { + return -1 + } + for i := 0; i+len(needle) <= len(hay); i++ { + match := true + for j := range needle { + if hay[i+j] != needle[j] { + match = false + break + } + } + if match { + return i + } + } + return -1 +} + +func TestManualCleanRepro(t *testing.T) { + raw, err := os.ReadFile(reproExport) + if err != nil { + t.Skipf("export json not present: %v", err) + } + if _, err := os.Stat(reproEpub); err != nil { + t.Skipf("book epub not present: %v", err) + } + var rows []reproRow + if err := json.Unmarshal(raw, &rows); err != nil { + t.Fatalf("unmarshal export: %v", err) + } + + conv := cachedConverter(reproEpub) + + stats := map[string]int{} + for _, row := range rows { + // Mirror handlers.convertHighlightPositions (text-anchored primary + // path, legacy per-endpoint fallback). + startLoc := ConvertHighlightRangeToCanonical(reproEpub, "reflowable", row.StartPosition, row.EndPosition, row.SelectionText, row.PctStart) + startCFI := webUsable(startLoc.Precision, startLoc.CFI) + endCFI := "" + if startCFI != "" && startLoc.EndCFI != "" { + endCFI = webUsable(startLoc.Precision, startLoc.EndCFI) + } + if startCFI == "" { + startLoc = ConvertToCanonical(LocatorSourceKOReader, row.StartPosition, row.PctStart, row.SelectionText, "reflowable", reproEpub, "") + startCFI = webUsable(startLoc.Precision, startLoc.CFI) + endCFI = "" + if startCFI != "" && startLoc.EndCFI != "" { + endCFI = startLoc.EndCFI + } + if endCFI == "" && row.EndPosition != "" { + endLoc := ConvertToCanonical(LocatorSourceKOReader, row.EndPosition, row.PctStart, "", "reflowable", reproEpub, "") + endCFI = webUsable(endLoc.Precision, endLoc.CFI) + } + if endCFI == "" && startCFI != "" && row.SelectionText != "" { + endCFI = reproExtendCFIByLength(startCFI, row.SelectionText) + } + } + + oracleStart, oracleEnd, oracleNote := oracleCFIs(t, conv, row) + stats["oracleMiss:"+statusOr(oracleNote)]++ + + cls := func(a, b string) string { + ba, oa, okA := cfiBase(a) + bb, ob, okB := cfiBase(b) + switch { + case !okA || !okB || a == "" || b == "": + return "missing" + case ba != bb: + return "diff-base" + case ob <= oa: + return "backwards" + default: + return "healthy" + } + } + cur := cls(startCFI, endCFI) + sto := cls(row.StoredStart, row.StoredEnd) + ora := cls(oracleStart, oracleEnd) + stats["current:"+cur]++ + stats["stored:"+sto]++ + stats["oracle:"+ora]++ + + if os.Getenv("REPRO_VERBOSE") != "" || cur != "healthy" || sto != "healthy" { + t.Logf("row %s", row.ID[:8]) + t.Logf(" raw: %s -> %s", row.StartPosition, row.EndPosition) + t.Logf(" ctx: %q", truncateRunes(row.SelectionText, 60)) + t.Logf(" stored: %-38s %-38s [%s]", row.StoredStart, row.StoredEnd, sto) + t.Logf(" current: %-38s %-38s [%s]", startCFI, endCFI, cur) + t.Logf(" oracle: %-38s %-38s [%s] %s", oracleStart, oracleEnd, ora, oracleNote) + } + } + + t.Logf("summary: %v", stats) + + // REPRO_EMIT_SQL=: emit UPDATE statements carrying the NEW + // converter output into the database rows this export came from + // (one-off healing of rows written by the broken converter). + if out := os.Getenv("REPRO_EMIT_SQL"); out != "" { + var b strings.Builder + b.WriteString("BEGIN;\n") + for _, row := range rows { + startLoc := ConvertHighlightRangeToCanonical(reproEpub, "reflowable", row.StartPosition, row.EndPosition, row.SelectionText, row.PctStart) + startCFI := webUsable(startLoc.Precision, startLoc.CFI) + endCFI := "" + if startCFI != "" && startLoc.EndCFI != "" { + endCFI = webUsable(startLoc.Precision, startLoc.EndCFI) + } + if startCFI == "" { + continue // leave untouched; legacy fallback rows keep old CFIs + } + b.WriteString(fmt.Sprintf( + "UPDATE media_highlights SET epubcfi_start=%s, epubcfi_end=%s WHERE id='%s';\n", + pgQuote(startCFI), pgQuote(endCFI), row.ID, + )) + } + b.WriteString("COMMIT;\n") + if err := os.WriteFile(out, []byte(b.String()), 0o600); err != nil { + t.Fatalf("write sql: %v", err) + } + t.Logf("wrote %s", out) + } +} + +func pgQuote(s string) string { + if s == "" { + return "''" + } + return "'" + strings.ReplaceAll(s, "'", "''") + "'" +} + +func statusOr(s string) string { + if s == "" { + return "found" + } + return s +}