fix(sync): understand cross-block text; never store a guessed locator

Tonight's failures all traced to one blind spot: the converter could
only reason about text within a single block. A position at a chapter
heading sends walk-up context (heading + the paragraphs below, joined by
the plugin's block capture); a selection can span several paragraphs.
Neither shape could be verified (containment compared one block against
a multi-block quote, so the CORRECT structural landing at the heading
was rejected) nor matched by text search (it never crossed block
boundaries). The ladder then fell to the percentage rung — which labeled
its char-count guess Precision "exact" — and that confidently-wrong CFI
was stored: reading positions reopened paragraphs away from the true
spot, and a highlight echo overwrote the row's good web CFIs with a
garbage start anchor that made the highlight unpaintable ("disappeared").

Four changes, all in the forward converter and its consumers:

- Quote verification: after the structural walk lands, read the
  whitespace-normalized document text forward from the landing point
  (crossing block boundaries; inline spans join directly so drop-cap
  splits still read as one word). A usable context must be a prefix of
  that stream — which is exactly what device captures are: the text from
  the position onward, or the selection between two anchors. The old
  single-block containment checks remain as secondary acceptance.
- Cross-block text search: the search rung matches against the whole
  document flattened in reading order, with every rune mapped back to
  its source node and offset. A context spanning blocks now matches, and
  the matched extent yields a true range end (EndEPUBCFI) that
  highlights use as their end anchor, threaded through the facade as
  CanonicalLocator.EndCFI.
- Honest labels: the percentage rung returns Precision "percentage" —
  a char-count estimate must never masquerade as an exact anchor.
- Confident-only storage: progress adopts a converted locator solely at
  structural/exact precision (section hrefs keep their legacy handling;
  anything lower stores percentage only), and highlight conversion
  returns CFIs only at structural/exact precision — a low-confidence
  echo yields empty, which applyLWW coalescing turns into preservation
  of the row's existing web CFIs instead of clobbering them.

Tests: walk-up context at a heading verifies structurally and lands in
the heading; a block-spanning context is found by search with a range
end landing in the following paragraph; the percentage rung is honestly
labeled; all drop-cap guards stay green.
This commit is contained in:
2026-09-09 20:15:12 -04:00
parent ce3ae31ced
commit b6f507b9e5
4 changed files with 293 additions and 43 deletions
+44 -17
View File
@@ -81,28 +81,39 @@ func (h *KOReaderHandler) loadAnnotationEpub(ctx context.Context, mediaItemID pg
// convertHighlightPositions resolves a device annotation's pos0/pos1
// locators to canonical CFIs through the shared facade. contextText is the
// selection's own text — the ideal anchor for the converter's verification
// and text-search rungs. percentage anchors the last-resort fallback so a
// failed conversion degrades to the neighborhood of the true position
// rather than the document start.
// selection's own text — a quote of the document, so the converter can
// verify structural landings against it and, when it must search, anchor a
// range end that spans block boundaries. percentage anchors the
// last-resort fallback. Only structural/exact landings are returned: a
// low-confidence conversion yields "" so an echo preserves the row's
// existing web CFIs (applyLWW coalesces empty) instead of clobbering them
// with a guess, and a new device highlight paints nowhere rather than in
// the wrong place.
func (h *KOReaderHandler) convertHighlightPositions(ec annotationEpub, pos0, pos1, contextText string, percentage float64) (string, string) {
if pos0 == "" || !ec.convertible() {
return "", ""
}
startLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos0, percentage, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
endLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos1, percentage, "", ec.mediaItem.FormatGroup, ec.epubPath, "")
endCFI := endLoc.CFI
// The end conversion carries no context text, so unless it resolved
// exactly it degenerates to a percentage fallback anchored at the
// document start — useless as a range end. When the START resolved
// structurally/exactly, derive the end from it: same node, character
// offset advanced by the selection's UTF-16 length (the CFI offset
// unit).
if endLoc.Precision != "exact" && endLoc.Precision != "structural" &&
(startLoc.Precision == "exact" || startLoc.Precision == "structural") && contextText != "" {
endCFI = extendCFIByLength(startLoc.CFI, contextText)
startCFI := webUsableCFI(startLoc)
endCFI := ""
// A text-search start matched the selection text itself: its extent
// is the selection's true end, even across blocks.
if startCFI != "" && startLoc.EndCFI != "" {
endCFI = startLoc.EndCFI
}
return startLoc.CFI, endCFI
if endCFI == "" && pos1 != "" {
endLoc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, pos1, percentage, "", ec.mediaItem.FormatGroup, ec.epubPath, "")
endCFI = webUsableCFI(endLoc)
}
// The end conversion carries no context text; when neither resolved
// confidently, derive the end from the start advanced by the
// selection's UTF-16 length (the CFI offset unit). Multi-node
// selections produce an out-of-range offset — harmless: resolution
// clamps or fails, and consumers fall back to the start.
if endCFI == "" && startCFI != "" && contextText != "" {
endCFI = extendCFIByLength(startCFI, contextText)
}
return startCFI, endCFI
}
// convertBookmarkPosition resolves a device bookmark's locator to the
@@ -866,10 +877,26 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
contextText = *book.ContextText
}
loc := wsync.ConvertToCanonical(wsync.LocatorSourceKOReader, *epubcfi, pct, contextText, ec.mediaItem.FormatGroup, ec.epubPath, "")
if loc.CFI != "" && loc.CFI != *epubcfi {
switch {
case strings.HasPrefix(loc.CFI, "epubcfi(") &&
(loc.Precision == "structural" || loc.Precision == "exact"):
converted := loc.CFI
epubcfi = &converted
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to %s", loc.Precision, converted)
case loc.CFI != "" && loc.CFI != *epubcfi &&
(loc.Precision == "element" || loc.Precision == "section"):
// Fragment-ID positions resolve to a section href: keep
// serving it (legacy behavior).
converted := loc.CFI
epubcfi = &converted
log.Printf("Bookhoard: CRE→CFI converted progress (%s) to href %s", loc.Precision, converted)
default:
// Low-confidence (percentage/fallback): store no
// canonical locator — the row's percentage restores
// approximately instead of a confidently-wrong CFI,
// and the device keeps its own native position.
log.Printf("Bookhoard: CRE→CFI conversion low-confidence (%s) for %s; storing percentage only", loc.Precision, *epubcfi)
epubcfi = nil
}
}
}
+153 -24
View File
@@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"sync"
"unicode"
"unicode/utf8"
"golang.org/x/net/html"
@@ -245,7 +246,12 @@ func parseElementPart(part string) (string, int) {
}
type ConversionResult struct {
EPUBCFI string
EPUBCFI string
// EndEPUBCFI is set when the conversion matched a context that is a
// quote of the document (text search): the range end anchor of the
// quoted text, valid across block boundaries. Selections use it as
// the highlight end; point positions ignore it.
EndEPUBCFI string
Href string
Percentage float64
Precision string
@@ -510,23 +516,22 @@ func (c *CFIConverter) convertByStructuralPath(body *html.Node, xp *CREXPointer,
return nil
}
// Text is the final verification: when the device sent usable words and
// they disagree with this structural landing, reject it and let text
// search / percentage decide rather than storing a confident-but-wrong CFI.
// Text is the final verification, in quote form: a usable context must
// be a prefix of the document as read forward from this landing point.
// Device captures are exactly that shape — a position's context is the
// text from the position onward (the plugin's walk-up may concatenate
// the heading with the paragraphs below), and a selection is the
// document text between its two anchors. The single-block containment
// checks stay as secondary acceptance for reverse shapes.
if usable, normalized := usableContextText(contextText); usable {
doc := documentTextFrom(body, textNode, localOffset, utf8.RuneCountInString(normalized)+64)
flat := blockFlattenedText(textNode)
if flat != "" && !strings.Contains(flat, normalized) && !strings.Contains(normalized, flat) {
// Compare a prefix too: device sends ~100 chars from the reader
// position while the block may be longer.
prefix := normalized
if utf8.RuneCountInString(prefix) > 40 {
runes := []rune(prefix)
prefix = string(runes[:40])
}
if !strings.Contains(flat, prefix) {
log.Printf("Bookhoard: structural landing disagrees with context in %s (block %q vs ctx %q)", href, truncateForLog(flat, 80), truncateForLog(normalized, 80))
return nil
}
if !(strings.HasPrefix(doc, normalized) ||
strings.Contains(flat, normalized) ||
strings.Contains(normalized, flat)) {
log.Printf("Bookhoard: structural landing disagrees with context in %s (reads %q vs ctx %q)",
href, truncateForLog(doc, 80), truncateForLog(normalized, 80))
return nil
}
}
@@ -592,28 +597,149 @@ func (c *CFIConverter) convertFragmentID(s string, storedPercentage float64) (*C
}, nil
}
// docRunePos records which text node (and rune offset within it) produced
// each rune of the flattened document text.
type docRunePos struct {
node *html.Node
off int
}
// flattenDocumentForSearch renders the document's text as a
// whitespace-normalized rune stream in document order, crossing block
// boundaries: a single space separates blocks, while inline spans join
// directly (drop-cap splits like <span>C</span>onvergence read as one
// word). Every emitted rune carries its source (node, rune offset) so
// matches map back to CFIs.
func flattenDocumentForSearch(body *html.Node) ([]rune, []docRunePos) {
var runes []rune
var poss []docRunePos
pendingSpace := false
var lastBlock *html.Node
appendNode := func(n *html.Node) {
block := findBlockParent(n)
if lastBlock != nil && block != lastBlock {
pendingSpace = true
}
lastBlock = block
off := 0
for _, r := range n.Data {
if unicode.IsSpace(r) {
pendingSpace = true
} else {
if pendingSpace && len(runes) > 0 {
runes = append(runes, ' ')
poss = append(poss, docRunePos{node: n, off: off})
}
pendingSpace = false
runes = append(runes, r)
poss = append(poss, docRunePos{node: n, off: off})
}
off++
}
}
var walk func(n *html.Node)
walk = func(n *html.Node) {
if n.Type == html.TextNode {
if strings.TrimSpace(n.Data) != "" {
appendNode(n)
}
return
}
for c := n.FirstChild; c != nil; c = c.NextSibling {
walk(c)
}
}
walk(body)
return runes, poss
}
// documentTextFrom reads the whitespace-normalized document text starting
// at rune `offset` in `start` (typically a structural landing point),
// crossing block boundaries — the document "as read" from that position.
// Capped at maxRunes.
func documentTextFrom(body *html.Node, start *html.Node, offset, maxRunes int) string {
runes, poss := flattenDocumentForSearch(body)
begin := -1
for i := range poss {
if poss[i].node == start && poss[i].off >= offset {
begin = i
break
}
}
if begin < 0 {
return ""
}
// The separator space before a block's first content rune shares its
// (node, offset) — skip it so the read starts on real text.
if runes[begin] == ' ' && begin+1 < len(runes) {
begin++
}
end := len(runes)
if begin+maxRunes < end {
end = begin + maxRunes
}
return string(runes[begin:end])
}
func (c *CFIConverter) convertByTextSearch(body *html.Node, xp *CREXPointer, spine *spineCache, href string, storedPercentage float64, contextText string) *ConversionResult {
normalizedCtx := normalizeWhitespace(contextText)
if normalizedCtx == "" {
return nil
}
match, matchOffset := findTextInNode(body, normalizedCtx)
if match == nil {
log.Printf("Bookhoard: text search no match for %q in %s", normalizedCtx, href)
// Match against the whole document flattened in reading order — the
// context may span block boundaries (a selection covering several
// paragraphs, a walk-up capture joining a heading with what follows).
runes, poss := flattenDocumentForSearch(body)
text := string(runes)
words := strings.Fields(normalizedCtx)
quoted := make([]string, len(words))
for i, w := range words {
quoted[i] = regexp.QuoteMeta(w)
}
pattern := strings.Join(quoted, `\s+`)
re, err := regexp.Compile(pattern)
if err != nil {
return nil
}
loc := re.FindStringIndex(text)
if loc == nil {
log.Printf("Bookhoard: text search no match for %q in %s", truncateForLog(normalizedCtx, 60), href)
return nil
}
startRune := utf8.RuneCountInString(text[:loc[0]])
endRune := utf8.RuneCountInString(text[:loc[1]]) // exclusive
spineIndex := xp.FragmentIndex - 1
cfi, err := buildCFI(spineIndex, match, matchOffset)
if err != nil || cfi == "" {
startPos := poss[startRune]
startCFI, err := buildCFI(spineIndex, startPos.node, startPos.off)
if err != nil || startCFI == "" {
log.Printf("Bookhoard: text search found match but buildCFI failed: %v", err)
return nil
}
log.Printf("Bookhoard: text search matched %q → %s (precision: exact)", normalizedCtx, cfi)
// The matched context is a quote of the document: its extent gives
// selections a true range end, across block boundaries.
endCFI := ""
if endRune > startRune && endRune <= len(poss) {
endPos := poss[endRune-1]
endOff := endPos.off + 1
if nRunes := utf8.RuneCountInString(endPos.node.Data); endOff > nRunes {
endOff = nRunes
}
if ec, err := buildCFI(spineIndex, endPos.node, endOff); err == nil && ec != "" {
endCFI = ec
}
}
log.Printf("Bookhoard: text search matched %q → %s (precision: exact)", truncateForLog(normalizedCtx, 60), startCFI)
return &ConversionResult{
EPUBCFI: cfi,
EPUBCFI: startCFI,
EndEPUBCFI: endCFI,
Href: href,
Percentage: storedPercentage,
Precision: "exact",
@@ -780,7 +906,10 @@ func (c *CFIConverter) convertByPercentageOffset(body *html.Node, xp *CREXPointe
EPUBCFI: cfi,
Href: href,
Percentage: storedPercentage,
Precision: "exact",
// This is a char-count estimate, not an exact landing: say
// so. Callers gate storage on precision, and a guess must
// never masquerade as an exact anchor.
Precision: "percentage",
}, nil
}
}
+90
View File
@@ -667,6 +667,96 @@ func TestReverseIgnoresSingleCharContext(t *testing.T) {
}
}
// A position at a chapter heading sends walk-up context: the heading text
// concatenated with the paragraphs below (block walk-up in the plugin).
// The structural landing at the heading is correct and must verify — the
// context is a prefix of the document as read from the landing.
func TestWalkUpContextVerifiesAtHeading(t *testing.T) {
c := NewCFIConverter(writeDropCapEPUB(t))
// ch10: h1[1] "Chapter 10", h1[2] "The Three C's of the New Covenant",
// then paragraphs. Position at h1[2]'s text start; the device captured
// the heading plus the following paragraph.
xp := "/body/DocFragment[2]/body/h1[2]/text().0"
ctx := "The Three C's of the New Covenant The Cleansing Life of Christ"
result, err := c.ConvertCREToStandard(xp, 0.52, ctx)
if err != nil {
t.Fatalf("ConvertCREToStandard error: %v", err)
}
t.Logf("walk-up heading → %s (%s)", result.EPUBCFI, result.Precision)
if result.Precision != "structural" {
t.Fatalf("expected structural precision (quote verification), got %s (%s)", result.Precision, result.EPUBCFI)
}
if strings.HasSuffix(result.EPUBCFI, "/4/2/1:0)") {
t.Errorf("collapsed to doc start: %s", result.EPUBCFI)
}
// The landing must be the heading, not a paragraph below it.
reverse, rerr := c.ConvertStandardToCRE(result.EPUBCFI, result.Percentage, "")
if rerr != nil {
t.Fatalf("reverse conversion error: %v", rerr)
}
if !strings.Contains(reverse.XPointer, "h1[2]") {
t.Errorf("expected landing in h1[2], got %s", reverse.XPointer)
}
}
// A selection spanning blocks (heading tail into the next paragraph) must
// be findable by text search across block boundaries, and the matched
// quote's extent gives a true range end.
func TestCrossBlockSearchSpansBlocks(t *testing.T) {
c := NewCFIConverter(writeDropCapEPUB(t))
// No element path → structural rung skipped, text search runs.
xp := "/body/DocFragment[2]/body"
ctx := "New Covenant The Cleansing Life of Christ"
result, err := c.ConvertCREToStandard(xp, 0.52, ctx)
if err != nil {
t.Fatalf("ConvertCREToStandard error: %v", err)
}
t.Logf("cross-block search → %s … %s (%s)", result.EPUBCFI, result.EndEPUBCFI, result.Precision)
if result.Precision != "exact" {
t.Fatalf("expected exact text-search precision, got %s", result.Precision)
}
if result.EPUBCFI == "" || result.EndEPUBCFI == "" {
t.Fatalf("expected range anchors, got %q…%q", result.EPUBCFI, result.EndEPUBCFI)
}
if result.EPUBCFI == result.EndEPUBCFI {
t.Fatalf("range collapsed: %s", result.EPUBCFI)
}
// The start lands in the heading, the end in the following paragraph.
startRev, err1 := c.ConvertStandardToCRE(result.EPUBCFI, result.Percentage, "")
endRev, err2 := c.ConvertStandardToCRE(result.EndEPUBCFI, result.Percentage, "")
if err1 != nil || err2 != nil {
t.Fatalf("reverse conversions failed: %v %v", err1, err2)
}
if !strings.Contains(startRev.XPointer, "h1[2]") {
t.Errorf("expected start in h1[2], got %s", startRev.XPointer)
}
if !strings.Contains(endRev.XPointer, "p[1]") {
t.Errorf("expected end in p[1] (following paragraph), got %s", endRev.XPointer)
}
}
// The percentage rung is a char-count estimate: it must never label its
// landing "exact". Feed it an unmatchable context so the ladder falls all
// the way through.
func TestPercentageFallbackIsHonestlyLabeled(t *testing.T) {
c := NewCFIConverter(writeDropCapEPUB(t))
xp := "/body/DocFragment[2]/body/p[3]/span[1]/text().0"
result, err := c.ConvertCREToStandard(xp, 0.52, "zzz qqq vvv uuu www")
if err != nil {
t.Fatalf("ConvertCREToStandard error: %v", err)
}
t.Logf("unmatchable context → %s (%s)", result.EPUBCFI, result.Precision)
if result.Precision != "percentage" {
t.Errorf("percentage rung must not claim exact, got %s", result.Precision)
}
}
// The bookmark route supplies no context (bookmark text is a display
// label, never book text), so the facade must still resolve the drop-cap
// xpointer structurally instead of collapsing to the document start.
+6 -2
View File
@@ -14,7 +14,11 @@ const (
)
type CanonicalLocator struct {
CFI string
CFI string
// EndCFI carries the matched context's range end (text-search
// conversions of selections) so callers can anchor a true highlight
// range across block boundaries.
EndCFI string
Precision string
Percentage float64
}
@@ -94,7 +98,7 @@ func ConvertToCanonical(
return CanonicalLocator{CFI: devicePos, Precision: "fallback", Percentage: percentage}
}
if result.EPUBCFI != "" {
return CanonicalLocator{CFI: result.EPUBCFI, Precision: result.Precision, Percentage: result.Percentage}
return CanonicalLocator{CFI: result.EPUBCFI, EndCFI: result.EndEPUBCFI, Precision: result.Precision, Percentage: result.Percentage}
}
if result.Href != "" {
return CanonicalLocator{CFI: result.Href, Precision: result.Precision, Percentage: result.Percentage}