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).
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)
- Line ~556:
UpdateDeviceLastSynccalled with zero UUIDpgtype.UUID{Bytes: [16]byte{}, Valid: false}instead of actual device ID. The correct call already exists inSyncProgressat line ~201. Remove the duplicate. - Line ~549:
SourceDevice.IDset touuid.UUID(userID.Bytes).String()(user ID) instead of device ID. Device ID is available from the device context but not passed through toupdateProgressForBook. ChapterProgressalways set tobook.Percentage(overall book progress), not chapter-relative. Fix: only set if KOReader provides it explicitly, otherwise preserve existing value via merge.- Dead code:
conflictsresponse 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)
last-read-place(line ~487):epubcfipassed asValid: trueeven when empty string (BookmarkId doesn't start withepubcfi(). Fix: only setValid: trueif non-empty after stripping.calculateFileSHA256function: Defined but never called. Dead code — remove.parseKoboDeviceHeaderfunction: Defined but never called in kobo.go (may be used by middleware). Verify before removing.GetLibrarybookmark_count: Counts ALL annotations (highlights + notes + bookmarks), not just bookmarks. Known issue, fix separately.
Media handler (internal/handlers/media.go)
UpdateMediaReadingProgress: SetsCharacterOffset,Chapter,ChapterProgress,ViewportX/Y,ZoomLevel,ScrollPositionX/Y,PanelNumber,ReadingModeall toValid: false— nulls them. Fixed by merge approach.
Universal progress handler (internal/handlers/progress.go)
UpdateUniversalProgress: Also setsChapterProgress = percentage(book-wide, not chapter-relative). Same bug as KOReader. Fixed by merge approach.
Sync infrastructure
OfflineDetector(internal/sync/offline.go): Fully implemented but never started incmd/server/main.go. Not part of this migration, but noted.- Queue processor
syncNote/syncHighlight(internal/sync/queue.go): Stub methods, not implemented. Not part of this migration. reading_progress.conflict_detectedcolumn: Never set totrueby any handler. Thesync_conflictstable records conflicts, but the boolean on the progress row stays false. The SQL upsert doesn't include this column in theDO UPDATE SETclause. 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) |
/api/progress/:id |
REMOVED | |||
/api/progress/:id |
REMOVED | |||
/api/progress/:id/history |
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:
- Existing progress has
last_sync_sourcethat differs from current source last_sync_timestampis within 5 minutes- Absolute percentage difference > 0.01 (1%)
- Existing progress has
- On conflict: record in
sync_conflictstable, 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:
- UUID match (confidence 1.0)
- SHA-256 match (confidence 0.9)
- File path match via alias or DB lookup (confidence 0.7)
- Title + Author match (confidence 0.5/0.4)
This logic stays in KOReaderHandler.resolveBookToMediaItem. Only the final progress write goes through ProgressService.
Checkpoint mode
enqueueProgressForBookcreatesProgressUpdatestruct → queue channel- Queue processor's
syncProgresscallsProgressService.SaveProgressinstead of raw upsert ProgressServiceneeds to be injected intoSyncQueueProcessor
Device file aliases
createDeviceFileAliasstays inKOReaderHandler— it's book resolution, not progress writing
Bulk sync
SyncProgressloops over books, resolves each, callsProgressService.SaveProgressper book- If one book fails, others continue (current behavior, must preserve)
- Error from
SaveProgresscauses the book to not be counted inbooksSynced
Execution Order
Each step is independently deployable. If a step breaks, previous steps are safe.
- Add
ProgressServicetointernal/sync/progress.go— additive, nothing breaks - Write tests for
ProgressService.SaveProgress— verify merge, enrichment, conflict detection - Update
cmd/server/main.goandinternal/router/router.go— createProgressService, inject into handlers. Pass as new parameter to constructors. - Update
MediaHandler— acceptProgressService, use it inUpdateMediaReadingProgress. Accept richer request body. - Update
KOReaderHandler— acceptProgressService, use inupdateProgressForBook. Fix bugs. - Update
KoboHandler— acceptProgressService, use in Markup/AnalyticsGettests/SyncFromServer. - Update queue processor —
syncProgresscallsProgressService.SaveProgress - Update
reader.ts— send richer payload - Move routes — add history route to media.go, remove progress.go
- Delete dead code —
UpdateReadingProgressquery,calculateFileSHA256, etc. - Run all tests
- Rebuild frontend (
npm run build:ts) - Rebuild app (
make rebuild-app) - 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
internal/sync/progress_test.go— add tests for:SaveProgresswith no existing data (fresh insert)SaveProgressmerge: KOReader data preserved when web savesSaveProgressmerge: web data preserved when KOReader savesSaveProgressenrichment: character_offset computed from percentageSaveProgressenrichment: current_page computed from percentage + total_pagesSaveProgressconflict detection: triggered when different source within 5 minSaveProgressconflict detection: NOT triggered for same sourceSaveProgressconflict detection: NOT triggered after 5 min window
- Run existing tests —
ConvertProgress,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
Bookmarkhandler — 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
Handlerstruct - 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:
percentageFLOAT (0.0-1.0) — universal progressepubcfiTEXT — exact EPUB positioncharacter_offsetBIGINT — crengine positionchapterINTEGER — chapter numberchapter_progressFLOAT — within-chapter progresscurrent_page/total_pagesINTEGER — page displayviewport_x/y,zoom_level,scroll_position_x/y— fixed-layout statepanel_number— comic panelreading_mode— reading mode identifierlast_sync_device/last_sync_source— sync metadatalast_sync_timestamp— for conflict detectionconflict_detected/conflict_resolved— conflict flags