fix(sync): resolve CRE positions structurally with text as verification

Drop-cap markup like <p><span>C</span>onvergence of Heaven and Earth</p> made getTextFromXPointer return just 'C'. The text search then matched the first 'C' in the chapter and stored doc-start (/4/2/1:0) with 'precision: exact', so the web reader reopened at the chapter start while the percentage looked mid-chapter. - Add convertByStructuralPath: walk the parsed CRE ElementPath against the raw XHTML (same-tag 1-based indexing, mirroring buildCREXPointer), map CharOffset into the target element's text, and build the CFI. Usable device text verifies the landing; disagreement falls through instead of storing a confident-but-wrong CFI. - Add usableContextText guard (>=8 runes, >=2 words): single chars can never claim an exact text-search hit in either direction (ConvertCREToStandard and reverseByTextSearch). - Add drop-cap regression fixtures plus usable-context unit tests. - Verified against the real book: DocFragment[26]/p[12]/span header now converts to epubcfi(/6/52!/4/28/2/1:0) structural both with 'C' and the full header, and round-trips back to DocFragment[26].
2026-09-08 22:32:51 -04:00
parent b1eda696f1
commit f7c4dfe2e8
2 changed files with 372 additions and 5 deletions
@@ -324,16 +324,233 @@ func (c *CFIConverter) ConvertCREToStandard(xpointer string, storedPercentage fl
 		}, nil
 	}

-	if contextText != "" {
-		result := c.convertByTextSearch(body, xp, spine, href, storedPercentage, contextText)
-		if result != nil {
+	// Primary: structural walk of the CRE element path. This is exact when
+	// the live CRE DOM still lines up with the raw XHTML (same-tag indices,
+	// see buildCREXPointer for the counting rule). Text is the final
+	// verification, not a fallback: a usable context that disagrees with
+	// the structural landing rejects it so text search gets a chance.
+	if len(xp.ElementPath) > 0 {
+		if result := c.convertByStructuralPath(body, xp, spine, href, storedPercentage, contextText); result != nil {
 			return result, nil
 		}
 	}

+	if usable, normalized := usableContextText(contextText); usable {
+		result := c.convertByTextSearch(body, xp, spine, href, storedPercentage, normalized)
+		if result != nil {
+			return result, nil
+		}
+	} else if normalizeWhitespace(contextText) != "" {
+		log.Printf("Bookhoard: ignoring too-short context %q for text search in %s (structural missed)", contextText, href)
+	}
+
 	return c.convertByPercentageOffset(body, xp, spine, href, storedPercentage)
 }

+// usableContextText reports whether a device context string is distinctive
+// enough to claim an "exact" text-search hit. Single characters from split
+// inline markup (e.g. drop-cap <span>C</span>onvergence) must never qualify:
+// findTextInNode would match the first "C" in the chapter (doc start).
+// Callers treat unusable as empty and fall through to the next strategy.
+func usableContextText(s string) (bool, string) {
+	normalized := normalizeWhitespace(s)
+	if normalized == "" {
+		return false, ""
+	}
+	if utf8.RuneCountInString(normalized) < 8 {
+		return false, ""
+	}
+	if len(strings.Fields(normalized)) < 2 {
+		return false, ""
+	}
+	return true, normalized
+}
+
+// resolveCREElementPath walks a parsed CRE element path against the raw
+// XHTML body. Indexing mirrors buildCREXPointer: 1-based nth element with
+// the same tag among element siblings (case-insensitive). Returns nil when
+// the live CRE DOM has drifted too far from disk (splits, injected wrappers)
+// so callers can fall through instead of landing on the wrong node.
+func resolveCREElementPath(body *html.Node, path []pathStep) *html.Node {
+	current := body
+	for _, step := range path {
+		if step.tag == "" {
+			continue
+		}
+		if strings.EqualFold(step.tag, "body") {
+			continue
+		}
+		var matches []*html.Node
+		for child := current.FirstChild; child != nil; child = child.NextSibling {
+			if child.Type == html.ElementNode && strings.EqualFold(child.Data, step.tag) {
+				matches = append(matches, child)
+			}
+		}
+		if len(matches) == 0 {
+			return nil
+		}
+		idx := step.index - 1
+		if idx < 0 || idx >= len(matches) {
+			return nil
+		}
+		current = matches[idx]
+	}
+	if current == body {
+		return nil
+	}
+	return current
+}
+
+// firstTextDescendant returns the first text node with non-whitespace content
+// under n (document order), for structural landings on elements.
+func firstTextDescendant(n *html.Node) *html.Node {
+	var found *html.Node
+	var walk func(*html.Node) bool
+	walk = func(node *html.Node) bool {
+		if node.Type == html.TextNode && strings.TrimSpace(node.Data) != "" {
+			found = node
+			return true
+		}
+		for child := node.FirstChild; child != nil; child = child.NextSibling {
+			if walk(child) {
+				return true
+			}
+		}
+		return false
+	}
+	walk(n)
+	return found
+}
+
+// textNodeAtRuneOffset walks text nodes under elem in document order and
+// returns the node containing the rune offset plus the local offset within
+// that node. Offsets beyond the end clamp to the last node.
+func textNodeAtRuneOffset(elem *html.Node, offset int) (*html.Node, int) {
+	if offset < 0 {
+		offset = 0
+	}
+	var target *html.Node
+	local := 0
+	remaining := offset
+	var walk func(*html.Node) bool
+	walk = func(node *html.Node) bool {
+		if node.Type == html.TextNode {
+			length := utf8.RuneCountInString(node.Data)
+			if remaining < length {
+				target = node
+				local = remaining
+				return true
+			}
+			remaining -= length
+			target = node
+			local = length
+			return false
+		}
+		for child := node.FirstChild; child != nil; child = child.NextSibling {
+			if walk(child) {
+				return true
+			}
+		}
+		return false
+	}
+	walk(elem)
+	return target, local
+}
+
+// blockFlattenedText returns the flattened visible text of the nearest
+// non-inline block ancestor (span/a/em/etc. are transparent), so drop-cap
+// splits like <span>C</span>onvergence verify against the full words.
+func blockFlattenedText(textNode *html.Node) string {
+	block := findBlockParent(textNode)
+	if block == nil {
+		if textNode.Parent != nil {
+			block = textNode.Parent
+		} else {
+			return strings.TrimSpace(textNode.Data)
+		}
+	}
+	segments := collectInlineText(block)
+	var runes []rune
+	for _, seg := range segments {
+		runes = append(runes, seg.runes...)
+	}
+	// Include leading direct text children the inline walk skips.
+	var direct []rune
+	for child := block.FirstChild; child != nil; child = child.NextSibling {
+		if child.Type == html.TextNode {
+			direct = append(direct, []rune(child.Data)...)
+		}
+	}
+	combined := string(direct)
+	if flattened := string(runes); flattened != "" {
+		if combined != "" {
+			combined += " " + flattened
+		} else {
+			combined = flattened
+		}
+	}
+	return normalizeWhitespace(combined)
+}
+
+func (c *CFIConverter) convertByStructuralPath(body *html.Node, xp *CREXPointer, spine *spineCache, href string, storedPercentage float64, contextText string) *ConversionResult {
+	elem := resolveCREElementPath(body, xp.ElementPath)
+	if elem == nil {
+		return nil
+	}
+
+	var textNode *html.Node
+	var localOffset int
+	if xp.CharOffset > 0 {
+		textNode, localOffset = textNodeAtRuneOffset(elem, xp.CharOffset)
+	} else {
+		textNode = firstTextDescendant(elem)
+		localOffset = 0
+	}
+	if textNode == nil {
+		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.
+	if usable, normalized := usableContextText(contextText); usable {
+		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
+			}
+		}
+	}
+
+	spineIndex := xp.FragmentIndex - 1
+	cfi, err := buildCFI(spineIndex, textNode, localOffset)
+	if err != nil || cfi == "" {
+		return nil
+	}
+	log.Printf("Bookhoard: structural match → %s (precision: structural)", cfi)
+	return &ConversionResult{
+		EPUBCFI:    cfi,
+		Href:       href,
+		Percentage: storedPercentage,
+		Precision:  "structural",
+	}
+}
+
+func truncateForLog(s string, n int) string {
+	if utf8.RuneCountInString(s) <= n {
+		return s
+	}
+	return string([]rune(s)[:n]) + "…"
+}
+
 func (c *CFIConverter) convertFragmentID(s string, storedPercentage float64) (*ConversionResult, error) {
 	frag, err := ParseCREFragmentID(s)
 	if err != nil {
@@ -1306,8 +1523,11 @@ func (c *CFIConverter) ConvertStandardToCRE(epubcfi string, storedPercentage flo
 }

 func (c *CFIConverter) reverseByTextSearch(epubcfi string, storedPercentage float64, contextText string) (*ReverseConversionResult, error) {
-	normalizedCtx := normalizeWhitespace(contextText)
-	if normalizedCtx == "" {
+	usable, normalizedCtx := usableContextText(contextText)
+	if !usable {
+		if normalizeWhitespace(contextText) != "" {
+			log.Printf("Bookhoard: CFI→CRE ignoring too-short context %q", contextText)
+		}
 		return &ReverseConversionResult{
 			Precision:  "percentage",
 			Percentage: storedPercentage,