Bookhoard/bookhoard
0
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 29 Wiki Activity

fix(sync): normalize character offsets to UTF-16 at the wire; refresh book offset on every verified save

Browse Source 
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.
This commit is contained in:
John O'Keefe
2026-09-26 20:18:48 -04:00
parent aec226af1a
commit 8c3273a0fc
7 changed files with 273 additions and 61 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/sync/cfi_verify_test.go
+95 -5
View File
@@ -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)
}
}