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

feat(server): progress locator coherence — percentage-only writes clear stale anchors

Browse Source 
The verify/heal pipeline verified only INBOUND context-bearing writes;
stored-row coherence was never enforced. SaveProgress seeds missing
fields from the existing row, so every percentage-only write (KOReader
low-confidence conversion, KOReader no-pointer payloads, Kobo markup
ReadingSync / analytics / sync-from-server, locator-less queue entries)
silently re-paired its NEW percentage with the PREVIOUS writer's stale
CFI+context — the incoherent row shape that fed the client-side
far-anchor restore floor (and, via the read-side anchor handles, could
even hand a first-party client the OLD position's scroll address).

KOReader's converter already declared the right intent ("storing
percentage only — the row's percentage restores approximately"); the
seeding betrayed it. SaveProgressRequest gains ClearLocator, set by the
percentage-only call sites, and the seeding rule honors it: stale
epubcfi/context/character_offset clear (the upsert assigns columns
directly, so NULL sticks) unless the request carries its own. A
request-carried context survives so the verify/heal pass can still mint
a fresh anchor from it; the HTTP merge path (field-absent PUTs
preserve) and all anchor-bearing writes are untouched. The fixed-layout
branch already cleared locators unconditionally — unchanged.

Consumer audit: percentage — the only thing every sync round-trip
exchanges — is untouched on all paths; KOReader pulls on locator-less
rows already support the nil-CFI shape (builder guards
progress.Epubcfi.Valid); first-party restores go percentage-first
instead of floor-fighting a stale anchor.

Tests: TestProgressWeb_LocatorCoherence pins clear-on-declared,
request-carried-context survival, and merge-path preservation. Full
suite with the change: 218 groups, zero failures.
This commit is contained in:
John O'Keefe committed 2026-10-09 18:22:15 -04:00
1 parent f24a2d6c22
commit 69516ae165
5 files changed
+223 -69
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

cmd/server/tests/progress_service_test.go
+114
View File
@@ -656,3 +656,117 @@ func TestProgressWeb_EdgeCases(t *testing.T) {
assert.InDelta(t, 1.5, progress.ZoomLevel.Float64, 0.01)
})
}
// TestProgressWeb_LocatorCoherence pins the locator coherence rule: a
// percentage-only write that DECLARES no locator (ClearLocator) must clear
// any inherited stale anchor instead of re-pairing the new percentage with
// the previous writer's CFI/context — the incoherent row shape that fed the
// client-side far-anchor restore floor. Request-carried fields always win.
func TestProgressWeb_LocatorCoherence(t *testing.T) {
setup := setupTestServer(t)
ctx := context.Background()
userID := getTestUserID(t, setup.DB)
pgUserID := pgtype.UUID{Bytes: userID, Valid: true}
seedAnchor := func(t *testing.T, mediaItemID string) uuid.UUID {
mediaUUID, _ := uuid.Parse(mediaItemID)
_, err := setup.ProgressService.SaveProgress(ctx, wsync.SaveProgressRequest{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
Source: "web",
Percentage: pFloat64(0.3),
Epubcfi: pStr("epubcfi(/6/4/2:first)"),
ContextText: pStr("some anchor context sentence"),
DeviceType: "web",
Broadcast: false,
})
require.NoError(t, err)
progress, err := setup.DB.GetReadingProgress(ctx, database.GetReadingProgressParams{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
})
require.NoError(t, err)
require.True(t, progress.Epubcfi.Valid, "seed: anchor stored")
return mediaUUID
}
t.Run("percentage-only kobo write clears the inherited anchor", func(t *testing.T) {
mediaItemID := createTestMediaItemID(t, setup)
mediaUUID := seedAnchor(t, mediaItemID)
_, err := setup.ProgressService.SaveProgress(ctx, wsync.SaveProgressRequest{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
Source: "kobo",
Percentage: pFloat64(0.6),
DeviceType: "kobo",
ClearLocator: true,
Broadcast: false,
})
require.NoError(t, err)
progress, err := setup.DB.GetReadingProgress(ctx, database.GetReadingProgressParams{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
})
require.NoError(t, err)
assert.InDelta(t, 0.6, progress.Percentage.Float64, 0.001, "percentage updated")
assert.False(t, progress.Epubcfi.Valid, "stale anchor cleared, not re-paired")
assert.False(t, progress.ContextText.Valid, "stale context cleared")
assert.Equal(t, "kobo", progress.LastSyncSource.String)
})
t.Run("request-carried context survives the clear", func(t *testing.T) {
mediaItemID := createTestMediaItemID(t, setup)
mediaUUID := seedAnchor(t, mediaItemID)
_, err := setup.ProgressService.SaveProgress(ctx, wsync.SaveProgressRequest{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
Source: "koreader",
Percentage: pFloat64(0.7),
ContextText: pStr("a brand new context sentence"),
DeviceType: "koreader",
ClearLocator: true,
Broadcast: false,
})
require.NoError(t, err)
progress, err := setup.DB.GetReadingProgress(ctx, database.GetReadingProgressParams{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
})
require.NoError(t, err)
assert.InDelta(t, 0.7, progress.Percentage.Float64, 0.001)
assert.True(t, progress.ContextText.Valid, "request-carried context kept")
assert.Equal(t, "a brand new context sentence", progress.ContextText.String)
// The verify gate is inert in the test harness (no media path
// resolver wired), so no anchor can be minted here; the rule
// under test is that the STALE one is gone.
assert.False(t, progress.Epubcfi.Valid, "stale anchor cleared")
})
t.Run("HTTP merge semantics untouched: key-absent PUT preserves fields", func(t *testing.T) {
mediaItemID := createTestMediaItemID(t, setup)
mediaUUID := seedAnchor(t, mediaItemID)
// Service-level save WITHOUT ClearLocator mimics the HTTP merge
// path (field absent = inherit): the anchor must survive.
_, err := setup.ProgressService.SaveProgress(ctx, wsync.SaveProgressRequest{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
Source: "queue",
Percentage: pFloat64(0.4),
Broadcast: false,
})
require.NoError(t, err)
progress, err := setup.DB.GetReadingProgress(ctx, database.GetReadingProgressParams{
MediaItemID: pgtype.UUID{Bytes: mediaUUID, Valid: true},
UserID: pgUserID,
})
require.NoError(t, err)
assert.True(t, progress.Epubcfi.Valid, "merge path preserves the anchor")
assert.Equal(t, "epubcfi(/6/4/2:first)", progress.Epubcfi.String)
})
}
internal/handlers/kobo.go
+32 -29
View File
@@ -254,9 +254,9 @@ type KoboInitResponse struct {
}
type KoboSyncStatus struct {
Status string `json:"Status"`
MarkupsSynced int `json:"MarkupsSynced"`
BookmarksSynced int `json:"BookmarksSynced"`
Status string `json:"Status"`
MarkupsSynced int `json:"MarkupsSynced"`
BookmarksSynced int `json:"BookmarksSynced"`
DeletedAnnotations []KoboDeletedAnnotation `json:"DeletedAnnotations,omitempty"`
}
@@ -448,16 +448,17 @@ func (h *KoboHandler) Markup(c *echo.Context) error {
if h.progressSvc != nil {
_, err = h.progressSvc.SaveProgress(c.Request().Context(), wsync.SaveProgressRequest{
MediaItemID: pgMediaUUID,
UserID: pgUserID,
Source: "kobo",
DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true},
Percentage: &percentage,
CurrentPage: currentPage,
TotalPages: totalPages,
DeviceType: "kobo",
DeviceName: device.DeviceName,
Broadcast: true,
MediaItemID: pgMediaUUID,
UserID: pgUserID,
Source: "kobo",
DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true},
Percentage: &percentage,
CurrentPage: currentPage,
TotalPages: totalPages,
DeviceType: "kobo",
DeviceName: device.DeviceName,
Broadcast: true,
ClearLocator: true,
})
} else {
_, err = h.db.UpdateUniversalProgress(c.Request().Context(), database.UpdateUniversalProgressParams{
@@ -798,14 +799,15 @@ func (h *KoboHandler) AnalyticsGettests(c *echo.Context) error {
if h.progressSvc != nil {
_, err = h.progressSvc.SaveProgress(c.Request().Context(), wsync.SaveProgressRequest{
MediaItemID: pgMediaUUID,
UserID: pgUserID,
Source: "kobo",
DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true},
Percentage: &percentage,
DeviceType: "kobo",
DeviceName: device.DeviceName,
Broadcast: true,
MediaItemID: pgMediaUUID,
UserID: pgUserID,
Source: "kobo",
DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true},
Percentage: &percentage,
DeviceType: "kobo",
DeviceName: device.DeviceName,
Broadcast: true,
ClearLocator: true,
})
} else {
_, err = h.db.UpdateUniversalProgress(c.Request().Context(), database.UpdateUniversalProgressParams{
@@ -860,14 +862,15 @@ func (h *KoboHandler) SyncFromServer(c *echo.Context) error {
if h.progressSvc != nil {
_, err = h.progressSvc.SaveProgress(c.Request().Context(), wsync.SaveProgressRequest{
MediaItemID: pgMediaUUID,
UserID: pgUserID,
Source: "bookhoard",
DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true},
Percentage: &percentage,
DeviceType: "kobo",
DeviceName: device.DeviceName,
Broadcast: false,
MediaItemID: pgMediaUUID,
UserID: pgUserID,
Source: "bookhoard",
DeviceID: pgtype.UUID{Bytes: deviceID, Valid: true},
Percentage: &percentage,
DeviceType: "kobo",
DeviceName: device.DeviceName,
Broadcast: false,
ClearLocator: true,
})
} else {
_, err = h.db.UpdateUniversalProgress(c.Request().Context(), database.UpdateUniversalProgressParams{
internal/handlers/koreader.go
+25 -18
View File
@@ -219,24 +219,24 @@ type KOReaderProgressRequest struct {
}
type KOReaderBookProgress struct {
UUID string `json:"uuid,omitempty"`
SHA256 string `json:"sha256,omitempty"`
Title string `json:"title,omitempty"`
Authors []string `json:"authors,omitempty"`
Progress float64 `json:"progress"`
Percentage float64 `json:"percentage"`
LastRead string `json:"last_read,omitempty"`
FilePath string `json:"file_path,omitempty"`
DeviceInfo KOReaderDeviceInfo `json:"device_info,omitempty"`
Bookmarks []KOReaderBookmark `json:"bookmarks,omitempty"`
Highlights []KOReaderHighlight `json:"highlights,omitempty"`
Notes []KOReaderNote `json:"notes,omitempty"`
Chapter *int `json:"chapter,omitempty"`
Character *int64 `json:"character,omitempty"`
Epubcfi *string `json:"epubcfi,omitempty"`
ContextText *string `json:"context_text,omitempty"`
Page *int `json:"page,omitempty"`
TotalPages *int `json:"total_pages,omitempty"`
UUID string `json:"uuid,omitempty"`
SHA256 string `json:"sha256,omitempty"`
Title string `json:"title,omitempty"`
Authors []string `json:"authors,omitempty"`
Progress float64 `json:"progress"`
Percentage float64 `json:"percentage"`
LastRead string `json:"last_read,omitempty"`
FilePath string `json:"file_path,omitempty"`
DeviceInfo KOReaderDeviceInfo `json:"device_info,omitempty"`
Bookmarks []KOReaderBookmark `json:"bookmarks,omitempty"`
Highlights []KOReaderHighlight `json:"highlights,omitempty"`
Notes []KOReaderNote `json:"notes,omitempty"`
Chapter *int `json:"chapter,omitempty"`
Character *int64 `json:"character,omitempty"`
Epubcfi *string `json:"epubcfi,omitempty"`
ContextText *string `json:"context_text,omitempty"`
Page *int `json:"page,omitempty"`
TotalPages *int `json:"total_pages,omitempty"`
// Device-side deletions, reported by dedup key. Keys refer to annotations
// the device previously received from the server (or echoes of its own
// pushes); the device only flags a deletion after observing the key in a
@@ -862,6 +862,11 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
if h.progressSvc != nil {
epubcfi := book.Epubcfi
// No position pointer from the device, or a conversion that
// declined below: the write is percentage-only by intent. The
// service then clears any inherited stale anchor instead of
// re-pairing it with the new percentage.
clearLocator := epubcfi == nil
if epubcfi != nil && wsync.IsCREXPointer(*epubcfi) {
// Same facade every annotation uses: cached converter,
// structural-first resolution, guarded fallbacks. The facade
@@ -897,6 +902,7 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
// 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
clearLocator = true
}
}
}
@@ -916,6 +922,7 @@ func (h *KOReaderHandler) updateProgressForBook(c *echo.Context, deviceID pgtype
DeviceType: "koreader",
DeviceName: deviceModel,
Broadcast: true,
ClearLocator: clearLocator,
}
_, err := h.progressSvc.SaveProgress(ctx, saveReq)
internal/sync/progress.go
+30 -4
View File
@@ -291,10 +291,10 @@ func NewProgressService(db *database.Queries, connManager *ConnectionManager) *P
}
type SaveProgressRequest struct {
MediaItemID pgtype.UUID
UserID pgtype.UUID
Source string
DeviceID pgtype.UUID
MediaItemID pgtype.UUID
UserID pgtype.UUID
Source string
DeviceID pgtype.UUID
Percentage *float64
Epubcfi *string
@@ -315,6 +315,16 @@ type SaveProgressRequest struct {
DeviceType string
DeviceName string
Broadcast bool
// ClearLocator declares that this write INTENDS to carry no position
// locator: percentage-only writers (devices whose converter declined,
// Kobo markup percentage syncs, queue entries without CFI). Without it,
// seeding below would re-pair the new percentage with the PREVIOUS
// writer's stale CFI/context — the incoherent row shape that feeds the
// client-side far-anchor restore floor. Locater-less rows are honest:
// percentage restores are approximate, and the next anchor-bearing
// write re-establishes the pair. Request-supplied fields always win.
ClearLocator bool
}
func (s *ProgressService) SaveProgress(ctx context.Context, req SaveProgressRequest) (database.ReadingProgress, error) {
@@ -407,6 +417,22 @@ func (s *ProgressService) SaveProgress(ctx context.Context, req SaveProgressRequ
params.ReadingMode = pgtype.Text{String: *req.ReadingMode, Valid: true}
}
// Locator coherence: a write that declares no locator must not
// silently re-pair its new percentage with the previous writer's
// stale CFI/context (inherited by the seeding above). Clear them so
// the row is an honest percentage-only position. Request-carried
// fields always win — and a request-carried context survives so the
// verify/heal pass below can mint a fresh anchor from it.
if req.ClearLocator && req.Epubcfi == nil {
params.Epubcfi = pgtype.Text{}
if req.ContextText == nil {
params.ContextText = pgtype.Text{}
}
if req.CharacterOffset == nil {
params.CharacterOffset = pgtype.Int8{}
}
}
params.LastSyncDevice = pgtype.Text{String: req.Source, Valid: true}
params.LastSyncSource = pgtype.Text{String: req.Source, Valid: true}
internal/sync/queue.go
+22 -18
View File
@@ -126,29 +126,29 @@ func (p *SyncQueueProcessor) EnqueueProgress(update *ProgressUpdate) error {
}
type HighlightUpdate struct {
DeviceID pgtype.UUID
MediaItemID pgtype.UUID
UserID pgtype.UUID
SelectionText string
StartPosition string
EndPosition string
Color string
NoteText string
EpubcfiStart string
EpubcfiEnd string
DeviceID pgtype.UUID
MediaItemID pgtype.UUID
UserID pgtype.UUID
SelectionText string
StartPosition string
EndPosition string
Color string
NoteText string
EpubcfiStart string
EpubcfiEnd string
PercentageStart float64
PercentageEnd float64
Source string
DeviceSyncData map[string]interface{}
Source string
DeviceSyncData map[string]interface{}
}
type NoteUpdate struct {
DeviceID pgtype.UUID
MediaItemID pgtype.UUID
UserID pgtype.UUID
Content string
Position string
Source string
DeviceID pgtype.UUID
MediaItemID pgtype.UUID
UserID pgtype.UUID
Content string
Position string
Source string
DeviceSyncData map[string]interface{}
}
@@ -443,6 +443,10 @@ func (p *SyncQueueProcessor) syncProgress(ctx context.Context, userID pgtype.UUI
if v, ok := syncData["epubcfi"].(string); ok {
req.Epubcfi = &v
}
// Entries without a CFI are percentage-only by intent: declare it
// so the service clears any inherited stale anchor instead of
// re-pairing it with this percentage.
req.ClearLocator = req.Epubcfi == nil
if v, ok := syncData["context_text"].(string); ok {
req.ContextText = &v
}