From 8c3273a0fc7669fd51042b3551fe385ca739d602 Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Sat, 26 Sep 2026 20:18:48 -0400 Subject: [PATCH] fix(sync): normalize character offsets to UTF-16 at the wire; refresh book offset on every verified save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Offset currency policy, now explicit: EPUB CFI terminals, CRE text() offsets and the served char_offset handle are UTF-16 code units (the EPUB CFI spec, and what foliate/readium/KOReader/Kobo clients actually observe), while internal arithmetic — the book-wide character_offset column and percentage fractions — stays rune-based, consistent with TotalCharacters. For all-BMP books the currencies are identical, so no stored value changes; astral-plane text (emoji, rare CJK) no longer drifts. Boundaries converted: resolveCFIToNode interprets incoming CFI terminal offsets as UTF-16; textNodeAtUTF16Offset (née textNodeAtRuneOffset) interprets CRE text() offsets as UTF-16; buildCFI and buildCREXPointer emit UTF-16 terminals; blockCharOffset (the served char_offset) is UTF-16. Also fixes two character_offset column defects: heals wrote a BLOCK- relative offset into the book-wide column, and verified-but-unhealed saves (e.g. KOReader pushes) never refreshed it, leaving it stale behind the anchor. VerifyProgressAnchor now returns the verified book- wide rune offset and SaveProgress refreshes the column on every verified save. Tests: astral currency round trip (offset after an emoji must shift by one unit between currencies, in both heal and exact-verify directions) and book-offset ordering. The cmd/server/tests integration harness failures under docker (library folder 400 during setup) reproduce on the pre-change tree and are unrelated. --- internal/handlers/media.go | 2 +- internal/sync/cfi_converter.go | 130 +++++++++++++++++++----- internal/sync/cfi_converter_test.go | 2 +- internal/sync/cfi_verify.go | 86 +++++++++++----- internal/sync/cfi_verify_manual_test.go | 2 +- internal/sync/cfi_verify_test.go | 100 +++++++++++++++++- internal/sync/progress.go | 12 ++- 7 files changed, 273 insertions(+), 61 deletions(-) diff --git a/internal/handlers/media.go b/internal/handlers/media.go index 5463dfe..04b0607 100644 --- a/internal/handlers/media.go +++ b/internal/handlers/media.go @@ -967,7 +967,7 @@ func (mh *MediaHandler) GetMediaReadingProgress(c *echo.Context) error { } if mediaItem, err := mh.db.GetMediaItem(c.Request().Context(), pgtype.UUID{Bytes: mediaUUID, Valid: true}); err == nil { if path, perr := mh.libraryService.ResolveMediaPath(c.Request().Context(), mediaItem.LibraryID, mediaItem.FilePath); perr == nil && path != "" { - if finalCFI, sel, href, charOff, _, _, verr := wsync.VerifyProgressAnchor(path, progress.Epubcfi.String, contextText, pct); verr == nil && sel != "" { + if finalCFI, sel, href, charOff, _, _, _, verr := wsync.VerifyProgressAnchor(path, progress.Epubcfi.String, contextText, pct); verr == nil && sel != "" { resp["css_selector"] = sel resp["anchor_href"] = href if charOff != nil { diff --git a/internal/sync/cfi_converter.go b/internal/sync/cfi_converter.go index a23fdfe..366784d 100644 --- a/internal/sync/cfi_converter.go +++ b/internal/sync/cfi_converter.go @@ -431,7 +431,9 @@ func firstTextDescendant(n *html.Node) *html.Node { // 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) { +// textNodeAtUTF16Offset resolves a crengine text().N offset — UTF-16 code +// units — to (text node, rune offset) within elem's text. +func textNodeAtUTF16Offset(elem *html.Node, offset int) (*html.Node, int) { if offset < 0 { offset = 0 } @@ -441,15 +443,15 @@ func textNodeAtRuneOffset(elem *html.Node, offset int) (*html.Node, int) { var walk func(*html.Node) bool walk = func(node *html.Node) bool { if node.Type == html.TextNode { - length := utf8.RuneCountInString(node.Data) + length := utf16Len(node.Data) if remaining < length { target = node - local = remaining + local = utf16ToRuneIndex(node.Data, remaining) return true } remaining -= length target = node - local = length + local = utf8.RuneCountInString(node.Data) return false } for child := node.FirstChild; child != nil; child = child.NextSibling { @@ -507,7 +509,7 @@ func (c *CFIConverter) convertByStructuralPath(body *html.Node, xp *CREXPointer, var textNode *html.Node var localOffset int if xp.CharOffset > 0 { - textNode, localOffset = textNodeAtRuneOffset(elem, xp.CharOffset) + textNode, localOffset = textNodeAtUTF16Offset(elem, xp.CharOffset) } else { textNode = firstTextDescendant(elem) localOffset = 0 @@ -1058,24 +1060,96 @@ func indexChildNodes(parent *html.Node) []indexedNode { return nodes } -func findTextChunkIndex(parent *html.Node, textNode *html.Node) (int, int) { +// Character-offset currency policy: the EPUB CFI spec and crengine both +// count UTF-16 code units (JavaScript `.length` semantics — what foliate, +// readium, KOReader and Kobo clients all observe), so every offset that +// CROSSES the wire — CFI terminals, CRE text() offsets, the served +// char_offset handle — is UTF-16. Internal arithmetic (book-level +// character_offset, percentage fractions) stays rune-based, consistent +// with TotalCharacters. These helpers convert at the boundaries; for +// all-BMP text the two currencies are identical, so ASCII books are +// unaffected. + +// utf16Len returns the UTF-16 code-unit length of s. +func utf16Len(s string) int { + n := 0 + for _, r := range s { + if r >= 0x10000 { + n += 2 + } else { + n++ + } + } + return n +} + +// utf16ToRuneIndex converts a UTF-16 code-unit offset within s to a rune +// index (clamped to len(runes)). +func utf16ToRuneIndex(s string, u16 int) int { + if u16 <= 0 { + return 0 + } + units := 0 + i := 0 + for _, r := range s { + if units >= u16 { + return i + } + if r >= 0x10000 { + units += 2 + } else { + units++ + } + i++ + } + return i +} + +// runeToUTF16Index converts a rune index within s to a UTF-16 code-unit +// offset (clamped to the string's unit length). +func runeToUTF16Index(s string, runeIdx int) int { + if runeIdx <= 0 { + return 0 + } + units := 0 + i := 0 + for _, r := range s { + if i >= runeIdx { + return units + } + if r >= 0x10000 { + units += 2 + } else { + units++ + } + i++ + } + return units +} + +// findTextChunk locates the indexed text chunk containing textNode and +// returns its chunk index, the rune offset of the node within the chunk, +// and the chunk text up to and including the node (for UTF-16 conversion +// of chunk-relative offsets). +func findTextChunk(parent *html.Node, textNode *html.Node) (int, int, string) { indexed := indexChildNodes(parent) for i, node := range indexed { if node.isTextChunk() { - for j, tn := range node.textChunk { + var sb strings.Builder + chunkOffset := 0 + for _, tn := range node.textChunk { if tn == textNode { - chunkOffset := 0 - for k := 0; k < j; k++ { - chunkOffset += utf8.RuneCountInString(node.textChunk[k].Data) - } - return i, chunkOffset + return i, chunkOffset, sb.String() + textNode.Data } + chunkOffset += utf8.RuneCountInString(tn.Data) + sb.WriteString(tn.Data) } } } - return -1, 0 + return -1, 0, "" } + func findElementCFIIndex(parent *html.Node, element *html.Node) int { indexed := indexChildNodes(parent) for i, node := range indexed { @@ -1110,15 +1184,17 @@ func buildCFI(spineIndex int, textNode *html.Node, charOffset int) (string, erro return "", fmt.Errorf("text node has no parent") } - chunkIdx, chunkOffset := findTextChunkIndex(parent, textNode) + chunkIdx, chunkOffset, chunkText := findTextChunk(parent, textNode) if chunkIdx == -1 { return "", fmt.Errorf("text node not found in parent's indexed children") } + // The CFI terminal offset is UTF-16 code units (spec currency); the + // internal charOffset is runes. Convert over the chunk text. totalOffset := chunkOffset + charOffset var parts []string - parts = append(parts, fmt.Sprintf("/%d:%d", chunkIdx, totalOffset)) + parts = append(parts, fmt.Sprintf("/%d:%d", chunkIdx, runeToUTF16Index(chunkText, totalOffset))) current := parent for current != nil { @@ -1519,24 +1595,31 @@ func resolveCFIToNode(doc *html.Node, steps []cfiStep) (*html.Node, int, error) entry := indexed[lastStep.Index] if entry.isTextChunk() { - textOffset := 0 + // The CFI terminal offset arrives in UTF-16 code units (spec + // currency — foliate/readium/KOReader/Kobo all emit UTF-16). + // Walk the chunk in UTF-16 units, then convert the hit position + // to the internal rune offset. + u16Remaining := 0 if lastStep.HasOffset { - textOffset = lastStep.Offset + u16Remaining = lastStep.Offset } var targetNode *html.Node - remainingOffset := textOffset + runeIntoTarget := 0 for _, tn := range entry.textChunk { - textLen := utf8.RuneCountInString(tn.Data) - if remainingOffset < textLen || (remainingOffset == textLen && targetNode == nil) { + units := utf16Len(tn.Data) + if u16Remaining < units || (u16Remaining == units && targetNode == nil) { targetNode = tn + runeIntoTarget = utf16ToRuneIndex(tn.Data, u16Remaining) break } - remainingOffset -= textLen + u16Remaining -= units targetNode = tn + runeIntoTarget = utf8.RuneCountInString(tn.Data) } if targetNode == nil && len(entry.textChunk) > 0 { targetNode = entry.textChunk[len(entry.textChunk)-1] + runeIntoTarget = utf8.RuneCountInString(targetNode.Data) } parent := targetNode.Parent @@ -1547,7 +1630,7 @@ func resolveCFIToNode(doc *html.Node, steps []cfiStep) (*html.Node, int, error) } totalOffset += countTextChars(c) } - totalOffset += remainingOffset + totalOffset += runeIntoTarget return targetNode, totalOffset, nil } @@ -1606,7 +1689,8 @@ func buildCREXPointer(spineIndex int, node *html.Node, charOffset int) (string, xpointer := fmt.Sprintf("/body/DocFragment[%d]/body%s", fragIndex, strings.Join(parts, "")) if charOffset > 0 || (node.Type == html.TextNode) { - xpointer += fmt.Sprintf("/text().%d", charOffset) + // crengine counts UTF-16 code units; charOffset is internal runes. + xpointer += fmt.Sprintf("/text().%d", runeToUTF16Index(node.Data, charOffset)) } return xpointer, nil diff --git a/internal/sync/cfi_converter_test.go b/internal/sync/cfi_converter_test.go index 58efcbd..e220f7f 100644 --- a/internal/sync/cfi_converter_test.go +++ b/internal/sync/cfi_converter_test.go @@ -133,7 +133,7 @@ func writeTestEPUB(t *testing.T) string { } docs := []spineDoc{ {"doc1.xhtml", "

Chapter one opening page.

"}, - {"doc2.xhtml", "

The family of Dashwood had long been settled in Sussex.

Their estate was large, and their residence was at Norland Park.

"}, + {"doc2.xhtml", "

The family of Dashwood had long been settled in Sussex.

Their estate was large, and their residence was at Norland Park.

The family crest shows a globe \U0001F30D and a rocket \U0001F680 flying onward.

"}, {"doc3.xhtml", "

Chapter three contents.

"}, {"doc4.xhtml", "

Chapter four contents.

"}, {"doc5.xhtml", "

Chapter five contents.

"}, diff --git a/internal/sync/cfi_verify.go b/internal/sync/cfi_verify.go index 931fc74..150d7dc 100644 --- a/internal/sync/cfi_verify.go +++ b/internal/sync/cfi_verify.go @@ -26,6 +26,7 @@ package sync import ( "fmt" "strings" + "unicode/utf8" "golang.org/x/net/html" ) @@ -121,18 +122,45 @@ func cssSelectorFor(block *html.Node) string { return "body>" + strings.Join(segs, ">") } -// blockCharOffset computes the rune offset of (node, runeOff) within the -// concatenated text of its block — the client-side scroll target. +// blockCharOffset computes the offset of (node, runeOff) within the +// concatenated text of its block, in UTF-16 code units — the client-side +// scroll-target currency (JavaScript .length semantics). func blockCharOffset(block, node *html.Node, runeOff int) int { segments := collectInlineText(block) - s := 0 + var sb strings.Builder for _, seg := range segments { if seg.node == node { - return s + runeOff + runes := seg.runes + if runeOff < len(runes) { + runes = runes[:runeOff] + } + sb.WriteString(string(runes)) + return runeToUTF16Index(sb.String(), utf8.RuneCountInString(sb.String())) } - s += len(seg.runes) + sb.WriteString(string(seg.runes)) } - return s + runeOff + return utf16Len(sb.String()) +} + +// bookCharOffset computes the book-wide rune offset of (node, runeOff) — +// the reading_progress.character_offset column's currency, consistent with +// TotalCharacters and the percentage derivations. +func bookCharOffset(conv *CFIConverter, spineIndex int, node *html.Node, runeOff int) int { + spine, err := conv.loadSpine() + if err != nil { + return 0 + } + before := 0 + for i := 0; i < spineIndex && i < len(spine.items); i++ { + doc, _, derr := conv.getContentDoc(i + 1) + if derr != nil { + continue + } + if b := findBody(doc); b != nil { + before += countTextChars(b) + } + } + return before + countTextCharsBefore(node) + runeOff } // ProgressAnchor is the full server-computed apply handle for a stored @@ -149,15 +177,19 @@ type ProgressAnchor struct { // VerifyProgressAnchor resolves a client-submitted standard CFI against // the EPUB, cross-checks the submitted context text, and heals the anchor -// by text search on any mismatch. -func VerifyProgressAnchor(epubPath, epubcfi, contextText string, percentage float64) (finalCFI string, cssSelector string, anchorHref string, charOffset *int, healedPct *float64, healed bool, err error) { +// by text search on any mismatch. charOffset is the anchor's block- +// relative UTF-16 offset (the served char_offset handle); bookOffset is +// the anchor's book-wide rune offset (the character_offset column's +// currency) — callers refresh the column from it on every verified save +// so it never goes stale behind the anchor. +func VerifyProgressAnchor(epubPath, epubcfi, contextText string, percentage float64) (finalCFI string, cssSelector string, anchorHref string, charOffset *int, bookOffset *int, healedPct *float64, healed bool, err error) { finalCFI = epubcfi anchorHref = "" spineIndex, localSteps, err := parseEPUBCFI(epubcfi) if err != nil { - cfi, sel, href, off, pct, healedFlag, herr := healFromContext(epubPath, contextText, percentage) - return cfi, sel, href, off, pct, healedFlag, herr + cfi, sel, href, off, book, pct, healedFlag, herr := healFromContext(epubPath, contextText, percentage) + return cfi, sel, href, off, book, pct, healedFlag, herr } conv := cachedConverter(epubPath) doc, docHref, err := conv.getContentDoc(spineIndex + 1) @@ -174,40 +206,41 @@ func VerifyProgressAnchor(epubPath, epubcfi, contextText string, percentage floa serverCtx := blockContextText(block, node, runeOff) if contextMatches(serverCtx, contextText) { off := blockCharOffset(block, node, runeOff) - return finalCFI, cssSelectorFor(block), anchorHref, &off, nil, false, nil + book := bookCharOffset(conv, spineIndex, node, runeOff) + return finalCFI, cssSelectorFor(block), anchorHref, &off, &book, nil, false, nil } // Mismatch: heal by text search. - hCFI, hPct, hSel, hHref, herr := healAnchorByText(epubPath, contextText, percentage, spineIndex) + hCFI, hPct, hSel, hHref, hBook, herr := healAnchorByText(epubPath, contextText, percentage, spineIndex) if herr != nil { - return finalCFI, "", hHref, nil, nil, false, fmt.Errorf("context mismatch (server %q vs client %q) and heal failed: %w", + return finalCFI, "", hHref, nil, nil, nil, false, fmt.Errorf("context mismatch (server %q vs client %q) and heal failed: %w", truncateRunes(serverCtx, 40), truncateRunes(contextText, 40), herr) } hOff := blockCharOffsetFor(epubPath, hCFI) - return hCFI, hSel, hHref, &hOff, &hPct, true, nil + return hCFI, hSel, hHref, &hOff, &hBook, &hPct, true, nil } -func healFromContext(epubPath, contextText string, percentage float64) (string, string, string, *int, *float64, bool, error) { - cfi, healedPct, sel, href, err := healAnchorByText(epubPath, contextText, percentage, -1) +func healFromContext(epubPath, contextText string, percentage float64) (string, string, string, *int, *int, *float64, bool, error) { + cfi, healedPct, sel, href, book, err := healAnchorByText(epubPath, contextText, percentage, -1) if err != nil { - return "", "", "", nil, nil, false, err + return "", "", "", nil, nil, nil, false, err } off := blockCharOffsetFor(epubPath, cfi) - return cfi, sel, href, &off, &healedPct, true, nil + return cfi, sel, href, &off, &book, &healedPct, true, nil } -func healAnchorByText(epubPath, contextText string, percentage float64, spineIndex int) (string, float64, string, string, error) { +func healAnchorByText(epubPath, contextText string, percentage float64, spineIndex int) (string, float64, string, string, int, error) { if epubPath == "" { - return "", 0, "", "", fmt.Errorf("no epub available for text anchoring") + return "", 0, "", "", 0, fmt.Errorf("no epub available for text anchoring") } conv := cachedConverter(epubPath) spine, err := conv.loadSpine() if err != nil { - return "", 0, "", "", err + return "", 0, "", "", 0, err } needle := truncateRunes(normalizeWhitespace(contextText), 40) if len([]rune(needle)) < 12 { - return "", 0, "", "", fmt.Errorf("context too short to anchor") + return "", 0, "", "", 0, fmt.Errorf("context too short to anchor") } type match struct { @@ -234,7 +267,7 @@ func healAnchorByText(epubPath, contextText string, percentage float64, spineInd } } if len(matches) == 0 || totalChars <= 0 { - return "", 0, "", "", fmt.Errorf("context not found in book") + return "", 0, "", "", 0, fmt.Errorf("context not found in book") } best := matches[0] @@ -251,16 +284,17 @@ func healAnchorByText(epubPath, contextText string, percentage float64, spineInd } } - healedPct := (float64(charsBefore[best.spine]) + float64(countTextCharsBefore(best.node)+best.off)) / float64(totalChars) + bookOff := charsBefore[best.spine] + countTextCharsBefore(best.node) + best.off + healedPct := float64(bookOff) / float64(totalChars) cfi, err := buildCFI(best.spine, best.node, best.off) if err != nil { - return "", 0, "", "", err + return "", 0, "", "", 0, err } sel := "" if block := anchorBlock(best.node); block != nil { sel = cssSelectorFor(block) } - return cfi, healedPct, sel, spine.items[best.spine].href, nil + return cfi, healedPct, sel, spine.items[best.spine].href, bookOff, nil } func blockCharOffsetFor(epubPath, cfi string) int { diff --git a/internal/sync/cfi_verify_manual_test.go b/internal/sync/cfi_verify_manual_test.go index 5b51d1e..1e57da0 100644 --- a/internal/sync/cfi_verify_manual_test.go +++ b/internal/sync/cfi_verify_manual_test.go @@ -12,7 +12,7 @@ func TestManualRealBookAnchor(t *testing.T) { } cfi := "epubcfi(/6/2!/4[x1984]/8[_idContainer003]/62/1:456)" ctx := "was at war with one of these Powers it was generally at peace with the other" - finalCFI, sel, href, charOff, healedPct, healed, err := VerifyProgressAnchor(path, cfi, ctx, 0.0415) + finalCFI, sel, href, charOff, _, healedPct, healed, err := VerifyProgressAnchor(path, cfi, ctx, 0.0415) if err != nil { t.Fatalf("VerifyProgressAnchor error: %v", err) } diff --git a/internal/sync/cfi_verify_test.go b/internal/sync/cfi_verify_test.go index a203056..abc918b 100644 --- a/internal/sync/cfi_verify_test.go +++ b/internal/sync/cfi_verify_test.go @@ -1,6 +1,7 @@ package sync import ( + "fmt" "testing" ) @@ -21,7 +22,7 @@ const ( func TestVerifyProgressAnchorAcceptsExact(t *testing.T) { path := writeTestEPUB(t) - finalCFI, sel, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, dashwoodCFI, dashwoodText, 0.3) + finalCFI, sel, _, charOff, _, healedPct, healed, err := VerifyProgressAnchor(path, dashwoodCFI, dashwoodText, 0.3) _ = charOff if err != nil { t.Fatalf("verify error: %v", err) @@ -44,7 +45,7 @@ func TestVerifyProgressAnchorHealsMismatch(t *testing.T) { path := writeTestEPUB(t) // Anchored at p1 but the context is p2's text: the classic // client-projection bug — the server must heal to the true location. - finalCFI, _, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, dashwoodCFI, estateText, 0.3) + finalCFI, _, _, charOff, _, healedPct, healed, err := VerifyProgressAnchor(path, dashwoodCFI, estateText, 0.3) _ = charOff if err != nil { t.Fatalf("verify error: %v", err) @@ -60,7 +61,7 @@ func TestVerifyProgressAnchorHealsMismatch(t *testing.T) { } // Healing must converge: re-verifying the healed anchor is a no-op. - finalCFI2, _, _, _, healedPct2, healed2, err := VerifyProgressAnchor(path, finalCFI, estateText, 0.3) + finalCFI2, _, _, _, _, healedPct2, healed2, err := VerifyProgressAnchor(path, finalCFI, estateText, 0.3) if err != nil { t.Fatalf("re-verify error: %v", err) } @@ -76,7 +77,7 @@ func TestVerifyProgressAnchorHealsUnresolvable(t *testing.T) { path := writeTestEPUB(t) // Element index 99 is out of range in doc2 — the anchor cannot resolve. bogus := "epubcfi(/6/4!/4/2/99/1:0)" - finalCFI, _, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, bogus, dashwoodText, 0.05) + finalCFI, _, _, charOff, _, healedPct, healed, err := VerifyProgressAnchor(path, bogus, dashwoodText, 0.05) _ = charOff if err != nil { t.Fatalf("verify error: %v", err) @@ -96,7 +97,7 @@ func TestVerifyProgressAnchorDumbClient(t *testing.T) { path := writeTestEPUB(t) // No CFI at all — a client that only knows percentage + context is // fully supported: the server anchors structurally from the text. - finalCFI, sel, _, charOff, healedPct, healed, err := VerifyProgressAnchor(path, "", estateText, 0.3) + finalCFI, sel, _, charOff, _, healedPct, healed, err := VerifyProgressAnchor(path, "", estateText, 0.3) _ = charOff if err != nil { t.Fatalf("verify error: %v", err) @@ -156,3 +157,92 @@ func TestParseStandardCFIRange(t *testing.T) { t.Errorf("start-arm step = %+v, want /4:0", last) } } + +// The emoji in fixture doc2's third paragraph are astral plane runes: +// one rune, two UTF-16 code units. Every wire offset (CFI terminals, +// the served char_offset) must therefore be UTF-16, while the book-wide +// character_offset column stays rune-based. +// +// Case 1: the context starts at rune 31 of the paragraph text ("The +// family crest shows a globe " = 31 BMP runes), so its UTF-16 offset is +// also 31 — the currencies agree. +// +// Case 2: the context starts at rune 33 ("and a rocket ..."), with the +// astral 🌍 (rune 31, units 31-32) BEFORE the offset — the UTF-16 offset +// is 34, one more than the rune offset. That +1 is the whole point of +// the boundary conversion. +// +// Local path: p3 of doc2's div = /4/2/6, text chunk /1. +func TestAstralOffsetCurrency(t *testing.T) { + path := writeTestEPUB(t) + cases := []struct { + name string + context string + wantTerm int + }{ + {"before any emoji", "🌍 and a rocket 🚀 flying onward.", 31}, + {"after one emoji", "and a rocket 🚀 flying onward.", 34}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + wantCFI := fmt.Sprintf("epubcfi(/6/4!/4/2/6/1:%d)", tc.wantTerm) + + finalCFI, sel, _, charOff, bookOff, _, healed, err := VerifyProgressAnchor(path, "", tc.context, 0.3) + if err != nil { + t.Fatalf("heal error: %v", err) + } + if !healed { + t.Fatalf("expected heal from context-only submission") + } + if finalCFI != wantCFI { + t.Errorf("healed CFI = %q, want %q (UTF-16 terminal)", finalCFI, wantCFI) + } + if sel != "body>div:nth-child(1)>p:nth-child(3)" { + t.Errorf("cssSelector = %q", sel) + } + if charOff == nil || *charOff != tc.wantTerm { + t.Errorf("block char_offset = %v, want %d UTF-16 units", charOff, tc.wantTerm) + } + if bookOff == nil || *bookOff <= 0 { + t.Errorf("book offset = %v, want a positive book-wide rune offset", bookOff) + } + + // Round trip: the healed CFI must verify exactly, with the + // same UTF-16 block handle and a stable book offset. + finalCFI2, _, _, charOff2, bookOff2, healedPct2, healed2, err := VerifyProgressAnchor(path, finalCFI, tc.context, 0.3) + if err != nil { + t.Fatalf("re-verify error: %v", err) + } + if healed2 || healedPct2 != nil { + t.Errorf("exact round trip should not heal (healed=%v)", healed2) + } + if finalCFI2 != wantCFI { + t.Errorf("re-verified CFI = %q, want %q", finalCFI2, wantCFI) + } + if charOff2 == nil || *charOff2 != tc.wantTerm { + t.Errorf("re-verified block char_offset = %v, want %d", charOff2, tc.wantTerm) + } + if bookOff2 == nil || bookOff == nil || *bookOff2 != *bookOff { + t.Errorf("book offset unstable: %v vs %v", bookOff2, bookOff) + } + }) + } +} + +// The book-wide offset must order anchors the way the book orders them: +// a later paragraph in the same document has a strictly larger +// character_offset. +func TestBookOffsetOrdering(t *testing.T) { + path := writeTestEPUB(t) + _, _, _, _, book1, _, _, err1 := VerifyProgressAnchor(path, dashwoodCFI, dashwoodText, 0.3) + _, _, _, _, book2, _, _, err2 := VerifyProgressAnchor(path, estateCFI, estateText, 0.3) + if err1 != nil || err2 != nil { + t.Fatalf("verify errors: %v %v", err1, err2) + } + if book1 == nil || book2 == nil { + t.Fatalf("book offsets missing: %v %v", book1, book2) + } + if *book2 <= *book1 { + t.Errorf("estate offset %d should exceed dashwood offset %d", *book2, *book1) + } +} diff --git a/internal/sync/progress.go b/internal/sync/progress.go index 13326d1..6cd514f 100644 --- a/internal/sync/progress.go +++ b/internal/sync/progress.go @@ -462,7 +462,7 @@ func (s *ProgressService) SaveProgress(ctx context.Context, req SaveProgressRequ if params.Percentage.Valid { submittedPct = params.Percentage.Float64 } - if finalCFI, _, _, charOff, healedPct, healed, verr := VerifyProgressAnchor(path, submittedCFI, params.ContextText.String, submittedPct); verr != nil { + if finalCFI, _, _, _, bookOff, healedPct, healed, verr := VerifyProgressAnchor(path, submittedCFI, params.ContextText.String, submittedPct); verr != nil { log.Printf("Bookhoard: progress anchor verify failed for %s: %v", req.MediaItemID.String(), verr) } else { if healed || (!params.Epubcfi.Valid && finalCFI != "") { @@ -470,9 +470,13 @@ func (s *ProgressService) SaveProgress(ctx context.Context, req SaveProgressRequ if healedPct != nil { params.Percentage = pgtype.Float8{Float64: *healedPct, Valid: true} } - if charOff != nil { - params.CharacterOffset = pgtype.Int8{Int64: int64(*charOff), Valid: true} - } + } + // The verified book-wide offset refreshes the column on + // EVERY verified save — not only heals — so it never goes + // stale behind the anchor (and a block-relative offset is + // never written into the book-level column). + if bookOff != nil { + params.CharacterOffset = pgtype.Int8{Int64: int64(*bookOff), Valid: true} } } }