diff --git a/cmd/server/tests/progress_service_test.go b/cmd/server/tests/progress_service_test.go index 21276489..11a49f71 100644 --- a/cmd/server/tests/progress_service_test.go +++ b/cmd/server/tests/progress_service_test.go @@ -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) + }) +} diff --git a/internal/handlers/kobo.go b/internal/handlers/kobo.go index bc093a39..155e11f3 100644 --- a/internal/handlers/kobo.go +++ b/internal/handlers/kobo.go @@ -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{ diff --git a/internal/handlers/koreader.go b/internal/handlers/koreader.go index 1b5fd125..1e786ecc 100644 --- a/internal/handlers/koreader.go +++ b/internal/handlers/koreader.go @@ -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) diff --git a/internal/sync/progress.go b/internal/sync/progress.go index 6cd514f6..e01dff8a 100644 --- a/internal/sync/progress.go +++ b/internal/sync/progress.go @@ -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} diff --git a/internal/sync/queue.go b/internal/sync/queue.go index 4a579b30..6f7bf152 100644 --- a/internal/sync/queue.go +++ b/internal/sync/queue.go @@ -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 }