fix(kobo): ignore no-progress KOReader sessions - #25
Conversation
Require automatic KOReader sessions to advance reading progress by at least 0.1 percent. Exclude older sessions below the threshold from reading-log rows and aggregate totals.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62c0ffc988
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| or( | ||
| isNull(readingSessions.source), | ||
| ne(readingSessions.source, 'koreader'), | ||
| gte(readingSessions.progressDelta, MIN_LOGGED_READING_PROGRESS_DELTA), | ||
| )!, |
There was a problem hiding this comment.
For users with KOReader sessions recorded before this change, this predicate hides those sessions only from the per-book log. The same rows remain in reading_sessions and in the already-populated user_reading_daily_stats, while user statistics, dashboards, achievements, and shared insights continue querying those sources without this predicate, such as UserStatisticsRepository.getDailyReadingStats() and getPeakReadingHours(). Consequently, a book log can report zero reading while daily totals, heatmaps, and other analytics still count the ignored session indefinitely unless a later sync happens to reconcile its cluster. Apply the exclusion consistently to raw-session aggregates and rebuild or migrate the persisted daily aggregates.
Useful? React with 👍 / 👎.
Apply the shared KOReader progress threshold to dashboards, achievements, statistics, lifecycle dates, and shared insights. Rebuild affected persisted daily summaries at startup without deleting historical session rows.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb207059d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return or( | ||
| isNull(readingSessions.source), | ||
| ne(readingSessions.source, 'koreader'), | ||
| gte(readingSessions.progressDelta, MIN_LOGGED_READING_PROGRESS_DELTA), |
There was a problem hiding this comment.
When a later KOReader upload recomputes a day containing a historical no-progress row outside the upload's affected cluster, KoreaderPluginRepository.recomputeDailyStats() at lines 322-338 selects that row without loggedReadingSessionFilter() and writes it back into user_reading_daily_stats; MigrationImportRepository.recomputeReadingDailyStats() at lines 862-878 does the same during imports. The bootstrap cleanup is therefore not durable, so dashboards can start counting ignored reading time again. Fresh evidence after the earlier review is that these two persisted-stat writers still omit the newly added predicate.
Useful? React with 👍 / 👎.
| constructor(private readonly userStatisticsService: UserStatisticsService) {} | ||
|
|
||
| async onApplicationBootstrap() { | ||
| await this.rebuildNoProgressKoreaderDailyStats(); |
There was a problem hiding this comment.
On every process restart, this awaited hook starts again at afterId = 0 and scans every retained no-progress KOReader session, then performs sequential day recomputations before the application can become ready. Because the historical rows are deliberately retained and there is no completion marker, large installations repeatedly pay the full backfill cost and can remain unavailable for an extended period after every deploy or crash; run this once through a persisted migration/checkpoint or move it to resumable background work.
AGENTS.md reference: AGENTS.md:L17-L21
Useful? React with 👍 / 👎.
Apply the reading-session threshold to KOReader and migration daily-stat recomputations. Process historical repairs in checkpointed background batches without blocking application startup.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd45f335dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return or( | ||
| isNull(readingSessions.source), | ||
| ne(readingSessions.source, 'koreader'), | ||
| gte(readingSessions.progressDelta, MIN_LOGGED_READING_PROGRESS_DELTA), |
There was a problem hiding this comment.
For historical KOReader rows already attached to a reading attempt, ReadingAttemptRepository.list() still counts every joined readingSessions row in totalSessions and totalSeconds. The reading-log table now hides these rows, but ReadingAttemptHistory.vue continues displaying totals that include them, so the same book tab reports contradictory session counts. Apply this predicate to the attempt aggregation join as well.
Useful? React with 👍 / 👎.
| } catch (error) { | ||
| const errorClass = error instanceof Error ? error.name : 'UnknownError'; | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| this.logger.warn( | ||
| `[reading_session.rebuild_no_progress_stats] [fail] durationMs=${Date.now() - startedAt} errorClass=${errorClass} error="${sanitizeLogValue(message)}" - historical stats rebuild failed`, | ||
| ); |
There was a problem hiding this comment.
If any batch rejects, this catch logs the error and permanently ends the only rebuild invocation for the lifetime of the process. The hourly cron only calls recomputeRecent(), so older polluted aggregates remain incorrect until the application happens to restart; this can also follow a transient database error or deadlock while the concurrently started recent recomputation is running. Reschedule the checkpointed rebuild with bounded backoff until it completes.
AGENTS.md reference: AGENTS.md:L21-L21
Useful? React with 👍 / 👎.
| const timeZone = resolveTimeZone((row.settings as { timezone?: unknown } | undefined)?.timezone, 'UTC'); | ||
| for (const day of getReadingSessionDayKeys(row, timeZone)) { |
There was a problem hiding this comment.
For a user who changed timezone after these historical sessions were aggregated, deriving affected days solely with the current setting can target a different date from the persisted contaminated row. For example, a session at 00:30 UTC previously stored on the UTC date moves to the preceding date in an American timezone, so the rebuild recomputes that preceding date while leaving the old UTC-date total untouched and then marks the checkpoint complete. Rebuild all daily rows for each affected user and library, or otherwise include day keys produced under the prior aggregation timezone.
Useful? React with 👍 / 👎.
Exclude ignored sessions from reading-attempt totals. Retry historical repairs with capped backoff and clear daily keys that may have been written under a previous timezone.
1 participant
Follow-up to #24.
62c0ffc9What changed
Automatic KOReader sessions are now recorded only when forward reading progress increases by at least 0.1 percent. Existing KOReader sessions below that threshold are excluded from per-book reading-log rows and aggregate totals, while manual and audiobook sessions remain unchanged.
This prevents briefly opening a book or looking at a page without reading from creating a 0.0 percent reading session.
Verification
Commands run.
Manual testing. Not run against a live KOReader device or populated production database. Unit coverage verifies the 0.0, sub-0.1, negative, and exactly-0.1 percent boundaries and verifies that every per-book reading-log aggregate applies the KOReader-only filter.
Anything you could not test: Live KOReader sync behavior and visual confirmation against the user's existing reading log.
Evidence
No UI changes.
Authorship and review
Contributor checklist
pnpm verifypasses against the final diff. Targeted verification is documented above.