feat(backend): add Whoop cycle endpoint for daily strain scores - #1124
feat(backend): add Whoop cycle endpoint for daily strain scores#1124knowald wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThis PR implements Whoop cycle endpoint support for daily strain score synchronization. A shared pagination helper is introduced to eliminate duplication across sleep/recovery endpoints, the new cycle data flow fetches and normalizes cycles into health scores, and the orchestrator is extended to integrate cycle syncing with transaction safety. ChangesCycle Data Synchronization Implementation
Sequence DiagramsequenceDiagram
participant load_and_save_all as load_and_save_all()
participant load_and_save_cycles as load_and_save_cycles()
participant get_cycle_data as get_cycle_data()
participant _get_paginated_records as _get_paginated_records()
participant normalize_cycle as normalize_cycle()
participant db as Database
load_and_save_all->>load_and_save_cycles: call with user_id, start_time, end_time
load_and_save_cycles->>get_cycle_data: request cycles for range
get_cycle_data->>_get_paginated_records: paginate /v2/cycle endpoint with params
_get_paginated_records-->>get_cycle_data: return aggregated raw pages
get_cycle_data-->>load_and_save_cycles: return raw cycle list
loop per raw cycle
load_and_save_cycles->>normalize_cycle: convert raw cycle -> HealthScoreCreate?
normalize_cycle-->>load_and_save_cycles: HealthScoreCreate or None
end
load_and_save_cycles->>db: bulk_create HealthScore rows
load_and_save_cycles->>db: commit transaction
load_and_save_cycles-->>load_and_save_all: return created_count
load_and_save_all->>load_and_save_all: set results["cycle_scores_synced"]
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/providers/whoop/data_247.py (1)
35-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd return type annotation to
__init__.
__init__is missing-> None, which violates the repository’s required function typing rules.Suggested patch
def __init__( self, provider_name: str, api_base_url: str, oauth: BaseOAuthTemplate, - ): + ) -> None: super().__init__(provider_name, api_base_url, oauth) self.event_record_repo = EventRecordRepository(EventRecord) self.data_source_repo = DataSourceRepository(DataSource) self.connection_repo = UserConnectionRepository()As per coding guidelines,
**/*.py: "All Python functions must have type annotations."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/services/providers/whoop/data_247.py` around lines 35 - 40, The __init__ method in the class defined in data_247.py is missing a return type annotation; update the constructor signature for the __init__ method (the one taking provider_name, api_base_url, oauth / BaseOAuthTemplate) to include the explicit return type -> None so it conforms to the repository’s function typing rules.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/app/services/providers/whoop/data_247.py`:
- Around line 35-40: The __init__ method in the class defined in data_247.py is
missing a return type annotation; update the constructor signature for the
__init__ method (the one taking provider_name, api_base_url, oauth /
BaseOAuthTemplate) to include the explicit return type -> None so it conforms to
the repository’s function typing rules.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 68e170a4-c845-4c58-85be-1563fe0b34a0
📒 Files selected for processing (3)
backend/app/services/providers/whoop/data_247.pybackend/tests/providers/whoop/__init__.pybackend/tests/providers/whoop/test_whoop_247_data.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/services/providers/whoop/data_247.py (2)
467-470:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the default sync window UTC-aware.
datetime.now()creates naive timestamps here._get_paginated_records()later calls.astimezone(timezone.utc), so on any worker that is not already running in UTC the default 30-day window shifts by the host offset and can miss or re-fetch boundary records. Usedatetime.now(timezone.utc)for both defaults.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/services/providers/whoop/data_247.py` around lines 467 - 470, The default 30-day window uses naive datetimes; change the defaults for start_time and end_time to timezone-aware UTC datetimes (use datetime.now(timezone.utc)) so they match the expectations of _get_paginated_records() which calls .astimezone(timezone.utc); update the start_time and end_time assignments accordingly and ensure timezone is imported/available from datetime in this module.
985-1011:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't report deduped cycle rows as
*_synced.On a replayed window,
bulk_createcan legitimately no-op because theHealthScorerow already exists, but this method still returnslen(health_scores). That makes the newcycle_scores_syncedfield overstate actual writes and turns an idempotent resync into a misleading success count. Either return the inserted-row count from the persistence layer or rename this metric toprocessed/normalizedso downstream sync reporting stays accurate.Based on learnings: external-provider
HealthScorededupe is keyed by(user_id, provider, category, recorded_at), so repeated windows can no-op at the DB even when this method returns the full normalized count.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/services/providers/whoop/data_247.py` around lines 985 - 1011, The method currently returns len(health_scores) which counts normalized items even if bulk_create dedupes existing rows; update load_and_save_cycles to use the actual inserted-row count from the persistence layer instead of the length of the normalized list: change the call to health_score_service.bulk_create(db, health_scores) to return an int (e.g. inserted_count) from the service (or have the service return that from the underlying repository upsert/insert_many), assign and use that returned value (commit after successful insert) and return inserted_count; reference the load_and_save_cycles function and health_score_service.bulk_create/underlying repository insert/upsert methods when making this change so metrics reflect actual DB writes rather than processed/normalized items.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@backend/app/services/providers/whoop/data_247.py`:
- Around line 467-470: The default 30-day window uses naive datetimes; change
the defaults for start_time and end_time to timezone-aware UTC datetimes (use
datetime.now(timezone.utc)) so they match the expectations of
_get_paginated_records() which calls .astimezone(timezone.utc); update the
start_time and end_time assignments accordingly and ensure timezone is
imported/available from datetime in this module.
- Around line 985-1011: The method currently returns len(health_scores) which
counts normalized items even if bulk_create dedupes existing rows; update
load_and_save_cycles to use the actual inserted-row count from the persistence
layer instead of the length of the normalized list: change the call to
health_score_service.bulk_create(db, health_scores) to return an int (e.g.
inserted_count) from the service (or have the service return that from the
underlying repository upsert/insert_many), assign and use that returned value
(commit after successful insert) and return inserted_count; reference the
load_and_save_cycles function and health_score_service.bulk_create/underlying
repository insert/upsert methods when making this change so metrics reflect
actual DB writes rather than processed/normalized items.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3024946-936c-4c96-b2c1-0c854055a67f
📒 Files selected for processing (4)
backend/app/schemas/model_crud/activities/health_score.pybackend/app/services/providers/whoop/data_247.pydocs/providers/coverage.mdxdocs/providers/whoop-api-integration.mdx
✅ Files skipped from review due to trivial changes (2)
- docs/providers/coverage.mdx
- backend/app/schemas/model_crud/activities/health_score.py
|
Addressing the three CodeRabbit findings from the review body:
|
Description
Adds
/v2/cyclesupport to the Whoop provider, as requested in #1008. Whoop cycles carry the daily strain score (present even on days without workouts), which was previously never ingested - only per-workout strain was stored.get_cycle_datafetches cycles with pagination from/v2/cyclenormalize_cyclemaps a scored cycle to aHealthScoreCreatewithcategory=STRAIN,qualifier="daily", the cycle'stimezone_offsetaszone_offset, and kilojoule / average_heart_rate / max_heart_rate as score components. The qualifier separates day-level cycle strain from the per-workout strain scoresworkouts.pyalready emits under the same(provider, category)- without it, consumers could not tell the two apart.load_and_save_cyclesis wired intoload_and_save_all(newcycle_scores_syncedresult key)get_sleep_dataandget_recovery_datawere extracted into_get_paginated_records, which the new cycle fetch also usesdocs/providers/coverage.mdx: updated the stale "Strain scores available but not yet implemented" line in the Whoop accordionIn-progress cycles (no
endyet) are skipped: their strain is provisional, and since health score inserts dedupe viaon_conflict_do_nothing, saving a partial value would lock it in permanently. The completed cycle is picked up on the next sync.No scope changes needed -
read:cyclesis already inwhoop_default_scope.Resolves #1008
Checklist
General
docs/(or no docs update needed)Backend Changes
You have to be in
backenddirectory to make it work:uv run pre-commit run --all-filespasses (ruff + ty pass; prettier hook needs frontendnode_modules, no frontend files touched)Testing Instructions
Steps to test:
cd backend && uv run pytest tests/providers/whoop/strainentries (qualifierdaily) should appear for each completed cycle, alongsiderecovery.Expected behavior:
One strain health score per completed, scored cycle, with kilojoule and heart rate values as components and the cycle's timezone offset preserved. Unscored or in-progress cycles are skipped. Re-syncing the same window does not duplicate scores (covered by a test).
Additional Notes
db.commit()/db.rollback()in this file to the savepoint/flush pattern. Whichever of the two merges second needs a small rebase; if refactor: move transaction boundaries from services/repos to callers #1092 lands first I will adapt the cycle block tobegin_nested()/flush().PENDING_SCOREwhen a sync window passes over it can be missed if it scores only after the next window starts. Padding the cycle fetch window behindlast_synced_at(dedupe makes re-fetching idempotent) would close that gap; happy to do it here or in a follow-up if desired.on_conflict_do_nothing. Fixing that means switching the dedupe toon_conflict_do_update, which changes sharedHealthScoreRepository.bulk_createbehavior for all providers - deliberately not done in this PR. The consumer-facing semantics ofdailystrain scores are documented in thequalifierfield description andcoverage.mdx.Summary by CodeRabbit
New Features
Tests
Documentation