Skip to content

refactor(backend): construct default Whoop sync window as UTC-aware datetimes - #1157

Merged
KaliszS merged 1 commit into
the-momentum:mainfrom
knowald:fix/whoop-naive-sync-window
Jun 16, 2026
Merged

refactor(backend): construct default Whoop sync window as UTC-aware datetimes#1157
KaliszS merged 1 commit into
the-momentum:mainfrom
knowald:fix/whoop-naive-sync-window

Conversation

@knowald

@knowald knowald commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Follow-up to a CodeRabbit finding on #1124 (raised against pre-existing lines there, so split out to keep that PR behavior-preserving).

What this does

Whoop247Data.load_and_save_all built its default 30-day sync window with naive datetime.now(). The defaults are now constructed as UTC-aware from the start.

Scope honesty: this is hygiene, not a bug fix

CodeRabbit claimed the naive defaults shift the window by the host offset. That is not what Python does here: the only consumer is the start_time.astimezone(timezone.utc) formatting in the pagination helper, and astimezone() interprets a naive datetime as system-local time, which is exactly what datetime.now() produces. The resulting UTC instant is identical on any host timezone, so there was no observable misbehavior to fix - hence refactor, not fix.

The change still carries value:

  • the window no longer depends on the naive-means-local interpretation chain being preserved by every future consumer
  • any later code that compares these datetimes against the timezone-aware values parsed from ISO strings a few lines above (fromisoformat(... "+00:00")) would raise TypeError on naive/aware comparison; aware-from-construction removes that trap
  • matches the rest of the file, which already uses datetime.now(timezone.utc)

The similar-looking datetime.now().timestamp() in garmin/workouts.py is left alone: .timestamp() on a naive local datetime already yields the correct epoch, and there is no aware value nearby to mix with.

Verification

  • uv run pytest tests/: 1746 passed, 2 skipped
  • ruff check and ty check: clean

Checklist notes

  • Pre-commit prettier hook skipped (--no-verify): it requires frontend node_modules, unrelated to this change.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed timezone handling for default historical data range retrieval to use proper UTC timezone awareness, ensuring consistent and accurate data handling across different time zones.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The load_and_save_all function in the Whoop data provider is updated to use timezone-aware UTC datetimes for default date range calculations. When start_time or end_time are not provided, the function now initializes them with datetime.now(timezone.utc) instead of naive datetime.now() calls.

Changes

Whoop timezone-aware defaults

Layer / File(s) Summary
Timezone-aware datetime defaults
backend/app/services/providers/whoop/data_247.py
load_and_save_all default parameters now use datetime.now(timezone.utc) for both start_time (30 days ago) and end_time (current moment) instead of naive datetime objects.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐰 A rabbit hops through timezones with care,
UTC now shines bright and fair,
No more naive datetimes to fear,
Thirty days past—crystal clear!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title directly and accurately describes the main change: converting default Whoop sync window datetimes from naive to UTC-aware, which is the primary purpose of this refactor.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@knowald knowald changed the title refactor(whoop): construct default sync window as UTC-aware datetimes refactor(backend): construct default Whoop sync window as UTC-aware datetimes Jun 11, 2026
@KaliszS
KaliszS merged commit 18aaf78 into the-momentum:main Jun 16, 2026
12 of 14 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.

2 participants