Skip to content

Harden sync#99

Merged
GeorgeSG merged 14 commits into
GeorgeSG:masterfrom
tku137:harden-sync
Mar 2, 2026
Merged

Harden sync#99
GeorgeSG merged 14 commits into
GeorgeSG:masterfrom
tku137:harden-sync

Conversation

@tku137
Copy link
Copy Markdown
Collaborator

@tku137 tku137 commented Feb 25, 2026

Fix last_open corruption, data loss, and edge case crashes

Problems

Main bugs

Several bugs were causing incorrect or missing data after syncing:

  • "56 years ago" last open date: the annotation sync path read summary.modified (an ISO date string like "2024-11-30") as the last_open timestamp. SQLite coerced it to 0, making every book appear last opened at Unix epoch (1970)
  • Last open date on bulk sync: after fixing the above, os.time() was used as a fallback — but the sidecar contains no reliable last-open timestamp at all. This caused every bulk-synced book to be stamped with the sync time instead of the actual last open date
  • Silent data loss on manual sync: KoReader buffers reading session stats in memory and only flushes to disk every ~50 page turns. Triggering a sync mid-session meant the current session's data was never captured. Sync-on-suspend was unaffected because the suspend event chain flushed to statistics.sqlite3 beforehand

Captured along the way

  • In valid page stats reaching the DB: no server-side validation on duration or total_pages fields, allowing null/NaN/zero values to be inserted
  • Server error on annotation-only sync: the plugin sends stats = {} (empty Lua table → JSON object {}) on the annotation-only path (simplest option is sending one data strucutre server-side). Calling .filter() on a plain object threw TypeError, returning HTTP 500
  • "NaN" avg. daily reading time: possible division by zero on the book detail page when a book had no recorded reading days yet.
  • Infinity started-reading date: getStartedReading() returned Infinity for books with no page stats, which could propagate to the client

Solutions

  • Plugin: Set last_open = 0 in the annotation sync path. The sidecar file contains no reliable last-open timestamp, and the statistics sync path already sets this correctly. The server-side guard skips merging zero values, so this is safe.
  • Plugin: call ReaderStatistics:insertDB() before reading statistics.sqlite3, ensuring the current in-memory session is flushed first.
  • Server: normalize the stats payload to an empty array if it is not an array, guarding against the plugin sending {} on the annotation-only path
  • Server: 0nly merge last_open into the DB if it is a valid positive finite number
  • Server: filter out page stat rows with invalid/zero duration or total_pages before inserting
  • Server: return 0 from getStartedReading() when the stats array is empty instead of Infinity
  • Frontend: guard avgPerDay calculation against division by zero when no reading days exist

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens the KOReader sync pipeline across plugin, server, and web UI to prevent last_open corruption, invalid page-stat ingestion, and edge-case crashes (annotation-only sync payload shape, empty stats, and division-by-zero UI issues).

Changes:

  • Plugin now flushes in-memory reading stats to statistics.sqlite3 before reading/syncing, and stops attempting to derive last_open from sidecar metadata.
  • Server normalizes/validates incoming stats payloads, conditionally merges last_open, and filters invalid page-stat rows before insertion.
  • Web UI avoids division-by-zero when computing average per day.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
plugins/koinsight.koplugin/db_reader.lua Flushes KOReader statistics to DB before reading page stats for sync.
plugins/koinsight.koplugin/annotation_reader.lua Sets last_open to 0 for sidecar-derived metadata since it’s not reliably available.
apps/web/src/pages/book-page/book-page.tsx Prevents avgPerDay from becoming NaN when there are zero reading days.
apps/server/src/upload/upload-service.ts Normalizes stats payload shape, validates last_open, and filters invalid page stat rows before inserting.
apps/server/src/books/books-service.ts Returns 0 for started-reading when no stats exist (avoids Infinity).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/server/src/books/books-service.ts
Comment thread apps/server/src/upload/upload-service.ts Outdated
Comment thread apps/server/src/upload/upload-service.ts Outdated
Comment thread apps/server/src/upload/upload-service.ts Outdated
Comment thread apps/server/src/upload/upload-service.ts Outdated
Comment thread plugins/koinsight.koplugin/db_reader.lua Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented Feb 28, 2026

Coverage Status

coverage: 56.065% (+2.3%) from 53.749%
when pulling 6d4a17d on tku137:harden-sync
into 86e740d on GeorgeSG:master.

Comment thread apps/server/src/upload/upload-service.ts Outdated
Comment thread apps/server/src/upload/upload-service.ts
Comment thread apps/web/src/pages/book-page/book-page.tsx Outdated
Comment thread apps/server/src/upload/upload-service.ts Outdated
@GeorgeSG GeorgeSG merged commit d5592a3 into GeorgeSG:master Mar 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants