Files
bookhoard/PROGRESS_MIGRATION.md
T
john-okeefe 2736409a79 docs: add PROGRESS_MIGRATION.md with full plan, bug list, and execution order
Documents the ProgressService migration including: data loss bug analysis,
handler-by-handler migration plan, route changes, test strategy, and
known issues for future work (conflict_detected column never set to true,
offline detector not started, server-side CFI generation needs Go EPUB
parser).
2026-04-25 21:17:19 -04:00

24 KiB

Universal Progress Service Migration Plan

Goal

Consolidate all progress-saving handlers into a single ProgressService that:

  • Merges new data with existing progress (preventing data loss across client switches)
  • Enriches progress with computed fields (e.g., character_offset from percentage)
  • Detects conflicts between different sync sources
  • Broadcasts updates via WebSocket
  • Is called by all clients: web reader, KOReader, Kobo

Architecture

Client Request → HTTP Handler (thin) → ProgressService.SaveProgress()
                                              ↓
                                         1. Read existing progress from DB
                                         2. Merge new data over existing (keep unset fields)
                                         3. Enrich (compute missing fields)
                                         4. Conflict detection
                                         5. Upsert enriched progress to DB
                                         6. WebSocket broadcast

Current State: Three Separate Handlers Writing Progress

Route Handler Auth What it does
PUT /api/media-items/:id/progress MediaHandler.UpdateMediaReadingProgress JWT Raw upsert, 4 fields only (percentage, current_page, total_pages, epubcfi). ALL other fields set to NULL.
POST /api/progress/:id Handler.UpdateUniversalProgress (ScannerHandler) JWT Page→percentage conversion, WebSocket broadcast. Still nulls unset fields.
POST /api/sync/koreader/progress KOReaderHandler.SyncProgress Device token Book resolution (UUID→hash→path→title), conflict detection, checkpoint mode, WebSocket broadcast. Sets chapter/character_offset but nulls viewport/zoom/panel.
POST /api/sync/kobo/:token/markup KoboHandler.Markup Device token ContentId mapping, ReadingSync + BookmarkSync. last-read-place only sets epubcfi/chapter, NULLs everything else.
POST /api/sync/kobo/:token/v1/analytics/gettests KoboHandler.AnalyticsGettests Device token Same as ReadingSync in Markup.
POST /api/sync/kobo/:token/sync-from-server KoboHandler.SyncFromServer Device token Same pattern, last_sync_source = "bookhoard".

Critical Bug in Current Code (Data Loss)

UpdateUniversalProgress SQL uses ON CONFLICT DO UPDATE SET ... = EXCLUDED.* — it replaces ALL fields. Any field passed as Valid: false (NULL) overwrites whatever was previously stored.

This means every cross-client save loses data. Examples:

  • KOReader saves character_offset → web reader saves → character_offset becomes NULL
  • Kobo saves percentage → Kobo sends last-read-place → percentage becomes NULL
  • KOReader saves chapter → web reader saves → chapter becomes NULL

The merge approach in this migration fixes this.

Read Routes

Route Handler Returns
GET /api/media-items/:id/progress MediaHandler.GetMediaReadingProgress Raw ReadingProgress struct
GET /api/progress/:id Handler.GetUniversalProgress Enriched with format_group, total_characters, chapter_count from media_items JOIN
GET /api/progress/:id/history Handler.GetProgressHistory Reading history array
GET /progress (frontend) Inline in frontend.go Progress overview page, calls GetAllProgressData

Existing Bugs to Fix During Migration

KOReader handler (internal/handlers/koreader.go)

  1. Line ~556: UpdateDeviceLastSync called with zero UUID pgtype.UUID{Bytes: [16]byte{}, Valid: false} instead of actual device ID. The correct call already exists in SyncProgress at line ~201. Remove the duplicate.
  2. Line ~549: SourceDevice.ID set to uuid.UUID(userID.Bytes).String() (user ID) instead of device ID. Device ID is available from the device context but not passed through to updateProgressForBook.
  3. ChapterProgress always set to book.Percentage (overall book progress), not chapter-relative. Fix: only set if KOReader provides it explicitly, otherwise preserve existing value via merge.
  4. Dead code: conflicts response field is initialized but never populated. This is intentional — conflicts are only shown in web UI, not returned to devices. No change needed.

Kobo handler (internal/handlers/kobo.go)

  1. last-read-place (line ~487): epubcfi passed as Valid: true even when empty string (BookmarkId doesn't start with epubcfi(). Fix: only set Valid: true if non-empty after stripping.
  2. calculateFileSHA256 function: Defined but never called. Dead code — remove.
  3. parseKoboDeviceHeader function: Defined but never called in kobo.go (may be used by middleware). Verify before removing.
  4. GetLibrary bookmark_count: Counts ALL annotations (highlights + notes + bookmarks), not just bookmarks. Known issue, fix separately.

Media handler (internal/handlers/media.go)

  1. UpdateMediaReadingProgress: Sets CharacterOffset, Chapter, ChapterProgress, ViewportX/Y, ZoomLevel, ScrollPositionX/Y, PanelNumber, ReadingMode all to Valid: false — nulls them. Fixed by merge approach.

Universal progress handler (internal/handlers/progress.go)

  1. UpdateUniversalProgress: Also sets ChapterProgress = percentage (book-wide, not chapter-relative). Same bug as KOReader. Fixed by merge approach.

Sync infrastructure

  1. OfflineDetector (internal/sync/offline.go): Fully implemented but never started in cmd/server/main.go. Not part of this migration, but noted.
  2. Queue processor syncNote/syncHighlight (internal/sync/queue.go): Stub methods, not implemented. Not part of this migration.
  3. reading_progress.conflict_detected column: Never set to true by any handler. The sync_conflicts table records conflicts, but the boolean on the progress row stays false. The SQL upsert doesn't include this column in the DO UPDATE SET clause. Schema fix needed separately.

New Code: ProgressService

Location: internal/sync/progress.go (add to existing file)

Struct

type ProgressService struct {
    db          *database.Queries
    connManager *ConnectionManager
}

func NewProgressService(db *database.Queries, connManager *ConnectionManager) *ProgressService

Input Struct

type SaveProgressRequest struct {
    MediaItemID     pgtype.UUID
    UserID          pgtype.UUID
    Source          string       // "web", "koreader", "kobo", "bookhoard"
    DeviceID        pgtype.UUID  // for conflict detection context

    // All pointer fields — nil means "don't change existing value"
    Percentage      *float64
    Epubcfi         *string
    CharacterOffset *int64
    Chapter         *int
    ChapterProgress *float64
    CurrentPage     *int
    TotalPages      *int
    ViewportX       *float64
    ViewportY       *float64
    ZoomLevel       *float64
    ScrollX         *float64
    ScrollY         *float64
    PanelNumber     *int
    ReadingMode     *string

    // For broadcast and conflict detection
    DeviceType      string
    DeviceName      string
}

SaveProgress Logic (pseudocode)

func SaveProgress(ctx, req) (ReadingProgress, error):

    // 1. Get media item metadata (for enrichment)
    mediaItem = db.GetMediaItem(ctx, req.MediaItemID)

    // 2. Read existing progress
    existing, err = db.GetReadingProgress(ctx, {MediaItemID, UserID})
    if err == pgx.ErrNoRows:
        existing = empty defaults
    else if err != nil:
        return err

    // 3. Merge: build UpdateUniversalProgressParams by starting with
    //    existing values, then overwriting with any non-nil fields from req
    params = buildParamsFromExisting(existing)
    params = mergeRequestOverParams(params, req)

    // 4. Enrich missing fields
    if params.Percentage.Valid && !params.CharacterOffset.Valid && mediaItem.TotalCharacters.Valid && mediaItem.TotalCharacters.Int64 > 0:
        charOffset = PercentageToCharacter(params.Percentage.Float64, mediaItem.TotalCharacters.Int64)
        params.CharacterOffset = {Int64: charOffset, Valid: true}

    if params.Percentage.Valid && !params.CurrentPage.Valid && params.TotalPages.Valid:
        page = PercentageToPage(params.Percentage.Float64, int(params.TotalPages.Int32))
        params.CurrentPage = {Int32: int32(page), Valid: true}

    // (Future: generate CFI from character_offset using EPUB parser)

    // 5. Set sync metadata
    params.MediaItemID = req.MediaItemID
    params.UserID = req.UserID
    params.LastSyncDevice = {String: req.DeviceType, Valid: true}
    params.LastSyncSource = {String: req.Source, Valid: true}

    // 6. Conflict detection
    if existing exists AND existing.LastSyncSource.Valid:
        if existing.LastSyncSource.String != req.Source AND existing.LastSyncTimestamp.Valid:
            if time.Since(existing.LastSyncTimestamp.Time) < 5*time.Minute:
                pctDiff = abs(params.Percentage.Float64 - existing.Percentage.Float64)
                if pctDiff > 0.01:
                    // Record conflict
                    conflictData = buildConflictData(existing, req)
                    db.CreateSyncConflict(ctx, {MediaItemID, UserID, "progress", conflictData})
                    connManager.BroadcastConflictNotification(mediaItemID, "detection", "")

    // 7. Upsert
    result, err = db.UpdateUniversalProgress(ctx, params)
    if err != nil:
        return err

    // 8. Broadcast
    deviceName = req.DeviceName
    if deviceName == "": deviceName = req.DeviceType + " Device"
    connManager.BroadcastProgressUpdate(
        mediaItemID,
        params.Percentage.Float64,
        SourceDevice{ID: req.DeviceID, Name: deviceName, Type: req.Source},
    )

    return result, nil

Merge Logic Detail

The buildParamsFromExisting function reads every field from the existing ReadingProgress row into UpdateUniversalProgressParams.

The mergeRequestOverParams function only overwrites a field if the corresponding pointer in SaveProgressRequest is non-nil.

This ensures:

  • Web reader sends percentage + epubcfi + current_page + total_pages → character_offset from KOReader's last save is preserved
  • KOReader sends percentage + character_offset + chapter → epubcfi from web reader's last save is preserved
  • Kobo sends only percentage → everything else preserved
  • Kobo sends epubcfi + chapter (last-read-place) → percentage from previous ReadingSync preserved

File Changes

Modified Files

File Change
internal/sync/progress.go Add ProgressService struct, NewProgressService, SaveProgress, merge/enrich helpers
internal/handlers/media.go UpdateMediaReadingProgress: parse richer request, call ProgressService.SaveProgress. GetMediaReadingProgress: use GetUniversalProgress query for enriched response. MediaHandler struct: add progressService field. Constructor: accept ProgressService.
internal/handlers/koreader.go updateProgressForBook: replace raw upsert with ProgressService.SaveProgress call. Fix SourceDevice.ID bug (use device ID). Remove duplicate UpdateDeviceLastSync with zero UUID. enqueueProgressForBook: update ProgressUpdate struct if needed. KOReaderHandler struct: add progressService field. Constructor: accept ProgressService.
internal/handlers/kobo.go Markup ReadingSync: call ProgressService.SaveProgress. Markup last-read-place: call ProgressService.SaveProgress (merge preserves percentage). AnalyticsGettests: same. SyncFromServer: same. KoboHandler struct: add progressService field. Constructor: accept ProgressService.
internal/handlers/progress.go Remove UpdateUniversalProgress method. Keep GetUniversalProgress, GetAllProgressData, GetProgressHistory.
internal/sync/queue.go syncProgress method: call ProgressService.SaveProgress instead of raw db.UpdateUniversalProgress. SyncQueueProcessor struct: add progressService field.
internal/router/media.go Add GET /media-items/:id/progress/history route. Remove "Legacy" comment from progress routes.
internal/router/progress.go DELETE THIS FILE — routes moved to media.go or removed.
internal/router/router.go Remove registerProgressRoutes call. Create ProgressService and inject into MediaHandler, KOReaderHandler, KoboHandler, SyncQueueProcessor.
cmd/server/main.go Create ProgressService after connManager and queueProcessor creation. Pass to handler constructors.
web/src/reader/reader.ts saveProgress: send richer payload (add chapter, chapter_progress, reading_mode, zoom_level, etc.)

Deleted Files

File Why
internal/router/progress.go All routes moved to media.go or removed

Dead Code to Remove

What Where
UpdateReadingProgress query queries/queries.sql (source) + queries.sql.go + querier.go (generated)
registerProgressRoutes function router/progress.go (file deleted)
calculateFileSHA256 function handlers/kobo.go — never called
parseKoboDeviceHeader function handlers/kobo.go — verify it's not used by middleware before removing

Route Changes

Before

Method Route Handler Auth
GET /api/media-items/:id/progress MediaHandler.GetMediaReadingProgress JWT
PUT /api/media-items/:id/progress MediaHandler.UpdateMediaReadingProgress JWT
DELETE /api/media-items/:id/progress MediaHandler.DeleteMediaReadingProgress JWT
GET /api/progress/:id Handler.GetUniversalProgress JWT
POST /api/progress/:id Handler.UpdateUniversalProgress JWT
GET /api/progress/:id/history Handler.GetProgressHistory JWT
POST /api/sync/koreader/progress KOReaderHandler.SyncProgress Device
GET /api/sync/koreader/metadata/:uuid KOReaderHandler.GetMetadata Device
GET /api/sync/koreader/library KOReaderHandler.GetLibrary Device
POST /api/sync/koreader/bookmarks KOReaderHandler.SyncBookmarks Device
POST /api/sync/kobo/:token/markup KoboHandler.Markup Device
POST /api/sync/kobo/:token/bookmark KoboHandler.Bookmark Device
POST /api/sync/kobo/:token/v1/analytics/gettests KoboHandler.AnalyticsGettests Device
GET /api/sync/kobo/:token/v1/initialization KoboHandler.Initialization Device
POST /api/sync/kobo/:token/sync-from-server KoboHandler.SyncFromServer Device

After

Method Route Handler Auth Change
GET /api/media-items/:id/progress MediaHandler.GetMediaReadingProgress JWT Enhanced response (adds format_group, total_characters)
PUT /api/media-items/:id/progress MediaHandler.UpdateMediaReadingProgress JWT Now calls ProgressService, richer request
DELETE /api/media-items/:id/progress MediaHandler.DeleteMediaReadingProgress JWT No change
GET /api/media-items/:id/progress/history MediaHandler.GetProgressHistory JWT NEW (moved from /progress/:id/history)
GET /api/progress/:id removed REMOVED
POST /api/progress/:id removed REMOVED
GET /api/progress/:id/history removed MOVED to media-items
POST /api/sync/koreader/progress KOReaderHandler.SyncProgress Device Internally uses ProgressService
GET /api/sync/koreader/metadata/:uuid KOReaderHandler.GetMetadata Device No change
GET /api/sync/koreader/library KOReaderHandler.GetLibrary Device No change
POST /api/sync/koreader/bookmarks KOReaderHandler.SyncBookmarks Device No change
POST /api/sync/kobo/:token/markup KoboHandler.Markup Device Internally uses ProgressService
POST /api/sync/kobo/:token/bookmark KoboHandler.Bookmark Device No change (bookmark-only, no progress)
POST /api/sync/kobo/:token/v1/analytics/gettests KoboHandler.AnalyticsGettests Device Internally uses ProgressService
GET /api/sync/kobo/:token/v1/initialization KoboHandler.Initialization Device No change
POST /api/sync/kobo/:token/sync-from-server KoboHandler.SyncFromServer Device Internally uses ProgressService

All device-facing URLs are unchanged. KOReader and Kobo firmware expect exact paths.

Conflict Rules (Preserved From Current Behavior)

  • Conflict detected ONLY when:
    1. Existing progress has last_sync_source that differs from current source
    2. last_sync_timestamp is within 5 minutes
    3. Absolute percentage difference > 0.01 (1%)
  • On conflict: record in sync_conflicts table, broadcast WebSocket notification
  • Current client's data ALWAYS wins (overwrite, don't merge with conflicting data)
  • Conflict details NOT returned to device caller (only shown in web UI)
  • Same-source rapid syncs never trigger conflicts (built-in debouncing for web, same-source check in handler)

Enrichment Rules

After merge, compute missing fields:

Condition Enrichment
Has percentage, missing character_offset, media has total_characters character_offset = PercentageToCharacter(pct, totalChars)
Has percentage, missing current_page, has total_pages current_page = PercentageToPage(pct, totalPages)
Has current_page + total_pages, missing percentage percentage = PageToPercentage(page, totalPages)
Has character_offset + total_characters, missing percentage percentage = CharacterToPercentage(char, totalChars)
Missing chapter_progress Preserve existing value (never compute from book-wide percentage)

All enrichment is only computed when source data is valid and non-zero. Silently skip if insufficient data.

Kobo Special Cases

last-read-place (in Markup BookmarkSync)

  • Only provides: epubcfi (parsed from BookmarkId), chapter, chapter_progress = 0.5
  • Does NOT provide: percentage, current_page, total_pages, character_offset
  • Before migration: These fields get NULLed (data loss bug)
  • After migration: Merge preserves existing values, only overwrites epubcfi/chapter/chapter_progress

ReadingSync (in Markup)

  • Only provides: percentage (from PercentRead/100)
  • Does NOT provide: epubcfi, chapter, character_offset, etc.
  • Before migration: These fields get NULLed (data loss bug)
  • After migration: Merge preserves existing values, enrichment may compute character_offset from percentage

SyncFromServer

  • last_sync_source = "bookhoard" (NOT "kobo") — this must be preserved
  • No WebSocket broadcast — this must be preserved

KOReader Special Cases

Book resolution (stays in handler, NOT in ProgressService)

The 4-priority resolution chain is KOReader-specific:

  1. UUID match (confidence 1.0)
  2. SHA-256 match (confidence 0.9)
  3. File path match via alias or DB lookup (confidence 0.7)
  4. Title + Author match (confidence 0.5/0.4)

This logic stays in KOReaderHandler.resolveBookToMediaItem. Only the final progress write goes through ProgressService.

Checkpoint mode

  • enqueueProgressForBook creates ProgressUpdate struct → queue channel
  • Queue processor's syncProgress calls ProgressService.SaveProgress instead of raw upsert
  • ProgressService needs to be injected into SyncQueueProcessor

Device file aliases

  • createDeviceFileAlias stays in KOReaderHandler — it's book resolution, not progress writing

Bulk sync

  • SyncProgress loops over books, resolves each, calls ProgressService.SaveProgress per book
  • If one book fails, others continue (current behavior, must preserve)
  • Error from SaveProgress causes the book to not be counted in booksSynced

Execution Order

Each step is independently deployable. If a step breaks, previous steps are safe.

  1. Add ProgressService to internal/sync/progress.go — additive, nothing breaks
  2. Write tests for ProgressService.SaveProgress — verify merge, enrichment, conflict detection
  3. Update cmd/server/main.go and internal/router/router.go — create ProgressService, inject into handlers. Pass as new parameter to constructors.
  4. Update MediaHandler — accept ProgressService, use it in UpdateMediaReadingProgress. Accept richer request body.
  5. Update KOReaderHandler — accept ProgressService, use in updateProgressForBook. Fix bugs.
  6. Update KoboHandler — accept ProgressService, use in Markup/AnalyticsGettests/SyncFromServer.
  7. Update queue processorsyncProgress calls ProgressService.SaveProgress
  8. Update reader.ts — send richer payload
  9. Move routes — add history route to media.go, remove progress.go
  10. Delete dead codeUpdateReadingProgress query, calculateFileSHA256, etc.
  11. Run all tests
  12. Rebuild frontend (npm run build:ts)
  13. Rebuild app (make rebuild-app)
  14. Manual test: web reader → KOReader sync → Kobo sync → back to web reader

Dependency Injection Changes

Current (cmd/server/main.go)

connManager = sync.NewConnectionManager()
queueProcessor = sync.NewSyncQueueProcessor(queries)
mediaHandler = handlers.NewMediaHandler(queries, libraryService, worker)
koreaderHandler = handlers.NewKOReaderHandler(queries, connManager, queueProcessor)
koboHandler = handlers.NewKoboHandler(queries, connManager)

After

connManager = sync.NewConnectionManager()
progressService = sync.NewProgressService(queries, connManager)
queueProcessor = sync.NewSyncQueueProcessor(queries, progressService)
mediaHandler = handlers.NewMediaHandler(queries, libraryService, worker, progressService)
koreaderHandler = handlers.NewKOReaderHandler(queries, connManager, queueProcessor, progressService)
koboHandler = handlers.NewKoboHandler(queries, connManager, progressService)

Tests to Write/Update

  1. internal/sync/progress_test.go — add tests for:
    • SaveProgress with no existing data (fresh insert)
    • SaveProgress merge: KOReader data preserved when web saves
    • SaveProgress merge: web data preserved when KOReader saves
    • SaveProgress enrichment: character_offset computed from percentage
    • SaveProgress enrichment: current_page computed from percentage + total_pages
    • SaveProgress conflict detection: triggered when different source within 5 min
    • SaveProgress conflict detection: NOT triggered for same source
    • SaveProgress conflict detection: NOT triggered after 5 min window
  2. Run existing testsConvertProgress, MergeProgress, PageToPercentage, etc. must still pass

Functionality That Must Not Be Touched

  • KOReader bookmark/highlight/note sync (SyncBookmarks) — annotation creation, not progress
  • Kobo bookmark creation in Bookmark handler — annotation creation
  • Kobo library initialization — read-only
  • Kobo ContentId mapping helpers — book resolution
  • KOReader book resolution logic — book resolution
  • KOReader metadata/library retrieval — read-only
  • WebSocket connection management — infrastructure
  • Offline detection — infrastructure (not started anyway)
  • Scanner/worker functionality on Handler struct
  • Frontend progress overview page
  • Frontend book detail page progress display
  • All annotation-related queries and handlers

Context: The Bigger Picture

This migration is the foundation for the "universal sync engine" — the core purpose of the Bookhoard project. The goal is seamless reading progress sync across all devices:

  • Web reader (foliate-js based)
  • KOReader (crengine based, running on Kindle/Kobo/Android/desktop)
  • Kobo (stock firmware)

The ProgressService is designed to eventually support:

  • Server-side CFI generation from crengine data (EPUB parser needed)
  • Server-side crengine XPointer generation from CFI (EPUB parser needed)
  • Bidirectional exact position sync between any two clients

Current percentage is the universal fallback. CFI is exact for EPUB. Character offset bridges the gap for crengine. The ProgressService enrichment step is where future CFI generation will be added.

Database Schema (unchanged)

The reading_progress table already has all needed fields. No schema changes required.

Key columns:

  • percentage FLOAT (0.0-1.0) — universal progress
  • epubcfi TEXT — exact EPUB position
  • character_offset BIGINT — crengine position
  • chapter INTEGER — chapter number
  • chapter_progress FLOAT — within-chapter progress
  • current_page / total_pages INTEGER — page display
  • viewport_x/y, zoom_level, scroll_position_x/y — fixed-layout state
  • panel_number — comic panel
  • reading_mode — reading mode identifier
  • last_sync_device / last_sync_source — sync metadata
  • last_sync_timestamp — for conflict detection
  • conflict_detected / conflict_resolved — conflict flags