Skip to content

fix(progress sync): take into consideration timezone offsets - #225

Merged
OGKevin merged 4 commits into
OGKevin:mainfrom
rlkandela:sync-time-offset
Aug 4, 2026
Merged

fix(progress sync): take into consideration timezone offsets#225
OGKevin merged 4 commits into
OGKevin:mainfrom
rlkandela:sync-time-offset

Conversation

@rlkandela

Copy link
Copy Markdown
Contributor

Closes #204

Summary

The differences between KOReader local time and Kobo UTC time wasn't taken into account.
This PR fixes it by switching to local time always before converting a time string into a timestamp.

Also fixes the Kobo time not being synchronised to KOReader ReadHistory.

Test Plan

  • Added unit tests in spec/reading_state_sync_spec.lua asserting that:
    • ReadHistory is correctly modified after PULL synchronisation.
  • Added unit tests in spec/kobo_state_reader_spec.lua asserting that:
    • readTimestamp returns the expected output.
  • Added unit tests in spec/kobo_state_writer_spec.lua asserting that:
    • modified entry.time after write is in the expected format.
  • Verified on device (Kobo, KOReader v2026.03, kobo.koplugin 0.4.1):
    • Kobo database contains the correct date after sync.
    • ReadHistory is written and books are ordered correctly after sync.
    • Chaining synchronisations no longer syncs the same books multiple times.

@OGKevin OGKevin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks generally good, thanks.

Some minor ✨ change requests.

Comment thread src/lib/kobo_state_reader.lua Outdated
Comment thread src/lib/kobo_state_reader.lua Outdated
Comment thread src/lib/kobo_state_reader.lua Outdated
Comment thread src/lib/kobo_state_reader.lua Outdated
Comment thread src/reading_state_sync.lua Outdated
@rlkandela
rlkandela requested a review from OGKevin August 2, 2026 15:16
@rlkandela

Copy link
Copy Markdown
Contributor Author

Hi, I have amended some changes that (imo) resolve some of the comments. I marked the comments as resolved even though I have replied to all of them.

Also I have explained the two questions related to os.date and isdst but left the comments open in case that's not enough.

Feel free to reply to my comments, request new changes, ask me to add documentation or anything you consider necessary.

Thank you.

- Convert UTC to local time before obtaining timestamps.
- Use existing timestamps for synchronization.
From now on, in Koreader, bookmarks will be written when you pull from
Kobo.
adds test for KoboStateReader and KoboStateWriter to assert that the
timezones are managed correctly when working with timestamps.
refactors ReadHistory mock to be able to call addItem function.
@rlkandela

Copy link
Copy Markdown
Contributor Author

Hi, I have updated the localUtcOffset's comment and linked to the reference manual section of interest.

OGKevin commented Aug 4, 2026

Copy link
Copy Markdown
Owner

@cursor review

@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot couldn't run — GitHub account mismatch

The GitHub account linked to your Cursor account does not match the PR author.

Please ensure you're using the correct GitHub account, or run Bugbot from a team that covers this repository.

@OGKevin

OGKevin commented Aug 4, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b2df202-e0cd-415a-b3d5-d460ccc28569

📥 Commits

Reviewing files that changed from the base of the PR and between 2c462ca and e43e9f4.

📒 Files selected for processing (7)
  • spec/helper.lua
  • spec/kobo_state_reader_spec.lua
  • spec/kobo_state_writer_spec.lua
  • spec/reading_state_sync_spec.lua
  • src/lib/kobo_state_reader.lua
  • src/lib/kobo_state_writer.lua
  • src/reading_state_sync.lua
📜 Recent review details
🔇 Additional comments (9)
spec/helper.lua (1)

1005-1024: LGTM!

Also applies to: 1250-1263, 1351-1387, 2238-2238

src/lib/kobo_state_writer.lua (1)

20-20: LGTM!

spec/kobo_state_reader_spec.lua (1)

1-47: LGTM!

spec/kobo_state_writer_spec.lua (1)

1-62: LGTM!

src/reading_state_sync.lua (2)

557-557: LGTM!


429-438: 🗄️ Data Integrity & Integration

No change needed for ReadHistory:addItem.

KOReader’s production ReadHistory:addItem(file, ts, no_flush) accepts an optional timestamp and keeps matching entries at the correct history position, so the calls preserve the Kobo/PULL timestamp.

spec/reading_state_sync_spec.lua (2)

4-4: LGTM!

Also applies to: 26-26, 244-248, 281-285, 570-572, 594-596, 618-620, 898-900, 947-949, 1023-1025, 1087-1087, 1132-1132, 1184-1186, 1245-1245, 1303-1303, 1367-1367, 1415-1415


1453-1648: LGTM!

src/lib/kobo_state_reader.lua (1)

79-88: 🎯 Functional Correctness

Check offset-qualified timestamp conversion before relying on it during DST gaps.

parseKoboTimestamp converts the ISO fields with local os.time before applying the Z or signed offset. On a DST-observing host, an offset such as 2025-03-09T02:30:00Z may need os.time to resolve a local civil time that does not exist, and the resulting local offset can skew the final UTC-to-local conversion. The fixed timestamp is still stored as kobo_state.timestamp and drives PULL/PUSH in ReadingStateSync:syncBidirectional, so a target LJK Lua DST regression should use actual runtime behavior before merging.


Walkthrough

The change adds timezone-aware Kobo timestamp parsing and UTC timestamp serialization. Reading-state PUSH now uses the KOReader timestamp selected for synchronization. PULL updates matching ReadHistory entries by exact path or virtual book ID. Shared ReadHistory and SQLite mock helpers support the new tests. The test suite covers timestamp formats, offset conversion, duplicate removal, virtual paths, and missing document paths.

Sequence Diagram(s)

sequenceDiagram
  participant KoboStateReader
  participant ReadingStateSync
  participant ReadHistory
  participant KoboStateWriter
  KoboStateReader->>ReadingStateSync: provide parsed Kobo timestamp
  ReadingStateSync->>ReadHistory: persist timestamp during PULL
  ReadHistory-->>ReadingStateSync: update matching entry
  ReadingStateSync->>KoboStateWriter: serialize selected timestamp during PUSH
Loading

Poem

A rabbit checks the clock at night,
Converts each zone to local light.
ReadHistory hops to the front,
Old duplicate trails are gone.
Kobo and KOReader agree,
Timestamps now sync neatly.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the Conventional Commits format and clearly describes the timezone-offset synchronization fix.
Linked Issues check ✅ Passed The changes address issue #204 by correcting timezone handling and synchronizing Kobo timestamps with KOReader ReadHistory.
Out of Scope Changes check ✅ Passed The implementation and tests remain within the stated timezone-handling and progress-synchronization objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@OGKevin
OGKevin merged commit 3b14de1 into OGKevin:main Aug 4, 2026
13 checks 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.

Kobo Virtual Library Sync Time Offset issue

2 participants