Surface WHOOP Bluetooth on web and differentiate cloud vs BLE - #1359
Conversation
Push-only WHOOP BLE now appears in sync.providers with last-received timestamps, and the web settings/detail UI treats it as a mobile-ingested source instead of a pull sync provider. Co-authored-by: Cursor <cursoragent@cursor.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughAdds WHOOP BLE as a mobile push-only provider: new ChangesWHOOP BLE push provider
CI: Infisical CLI installation pinning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds first-class support for a push-only (mobile/BLE) WHOOP provider and updates labeling/logo handling to distinguish cloud vs Bluetooth variants across web, server, and shared provider metadata.
Changes:
- Introduce
PUSH_PROVIDERSmetadata (e.g.,whoop_ble) and surfacepushOnlyproviders viasyncRouter.providers(). - Update web UI to treat push-only providers differently (status, no manual sync controls, different messaging).
- Add logo aliasing (
whoop_ble→whoop) and rename WHOOP labels to “WHOOP (Cloud)” / “WHOOP (Bluetooth)”.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/providers/whoop/provider.ts | Renames WHOOP cloud provider display name. |
| packages/web/src/pages/ProviderDetailPage.tsx | Adds push-only UI path (mobile sync panel, hides sync controls/history). |
| packages/web/src/components/SyncProviderCard.tsx | Adds push-only rendering mode and messaging. |
| packages/web/src/components/ProviderLogo.tsx | Uses logo ID indirection for aliased providers. |
| packages/web/src/components/DataSourcesPanel.tsx | Excludes push-only providers from “sync all” and auth/sync flows; passes pushOnly into cards. |
| packages/server/src/routers/sync.ts | Appends push providers to providers list; computes push “last received”. |
| packages/server/src/routers/sync.test.ts | Updates expectations for new push provider in providers list. |
| packages/server/src/routers/heart-rate.test.ts | Updates mocked label for whoop_ble. |
| packages/server/src/repositories/whoop-ble-sync-repository.ts | Centralizes push provider ID/label usage for inserts and external IDs. |
| packages/server/src/repositories/sync-repository.ts | Adds ClickHouse query to compute push provider last-received timestamps. |
| packages/server/src/repositories/sync-repository.test.ts | Adds test coverage for getPushProviderLastReceived(). |
| packages/providers-meta/src/push-providers.ts | New shared push-provider registry and helper. |
| packages/providers-meta/src/push-providers.test.ts | Tests push-provider registry contents and lookup. |
| packages/providers-meta/src/providers.ts | Updates labels; adds logo aliasing + providerLogoId(). |
| packages/providers-meta/src/providers.test.ts | Updates label/logo tests for WHOOP changes. |
| packages/providers-meta/package.json | Exports new ./push-providers entrypoint. |
| packages/mobile/components/ProviderLogo.tsx | Uses providerLogoId() for logo URL resolution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Storybook previews for This comment updates automatically on each PR push. |
Mobile PreviewScan to open on device:
To test on device:
|
Resolve typecheck, lint, and test issues from pushOnly typing, biome formatting, banned type assertions, and updated WHOOP label expectations. Co-authored-by: Cursor <cursoragent@cursor.com>
Catch push-provider last-received query errors and use a noop sensor store in the server integration test so sync.providers no longer returns 500 when ClickHouse is unavailable. Co-authored-by: Cursor <cursoragent@cursor.com>
Expose push provider descriptions from sync.providers, log ClickHouse lookup failures, drop FINAL from last-received query, and dedupe relative-time formatting in the detail page. Co-authored-by: Cursor <cursoragent@cursor.com>
Add description to all sync.providers entries so web typecheck passes, and install Infisical from GitHub releases to avoid apt-get update 403s on Microsoft repos. Co-authored-by: Cursor <cursoragent@cursor.com>
Cover push provider authorization, ClickHouse error handling, and repository fallbacks so new WHOOP BLE logic meets the mutation score threshold. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep push-only sync history hiding while adopting the provider-scoped SyncHistory key from main. Co-authored-by: Cursor <cursoragent@cursor.com>
Apply formatter output for chained mocks and long expect assertions. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/providers/whoop-journal.test.ts (1)
134-139: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAssert the new WHOOP lookback contract here too.
This test was updated for the renamed provider, but it still leaves
scheduledSyncLookbackDays = 30unverified even though that is part of the changed public behavior.As per coding guidelines, "When a PR touches code that lacks tests, add tests for the changed behavior — never dismiss missing coverage as pre-existing."
🤖 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 `@src/providers/whoop-journal.test.ts` around lines 134 - 139, The WHOOP provider rename test in whoop-journal.test.ts only checks id and name, but it does not verify the changed public behavior for scheduledSyncLookbackDays. Update the WhoopProvider test block that imports WhoopProvider and instantiates it to also assert the new lookback contract, specifically that scheduledSyncLookbackDays is 30, so the renamed provider test covers all changed behavior.Source: Coding guidelines
packages/server/src/routers/sync.ts (1)
155-233: 🗄️ Data Integrity & Integration | 🟠 MajorAdd an output schema to
providers()
packages/server/src/routers/sync.tsreturns a mixed array of configured-provider rows and push-provider rows without.output(...); add a Zod output schema that covers both variants.🤖 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 `@packages/server/src/routers/sync.ts` around lines 155 - 233, The providers() query currently returns a mixed array of registered-provider and push-provider objects without an explicit output contract. Add a Zod .output(...) schema on cachedProtectedQuery(CacheTTL.SHORT).query for providers() that matches the returned shape, using a discriminated union or shared object schema to cover both variants. Reference the provider mapping logic in providers(), including ProviderModel and PUSH_PROVIDERS, and ensure the schema includes all fields returned by the final array.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.
Inline comments:
In @.github/actions/export-infisical-dotenv/action.yml:
- Around line 34-40: The Infisical CLI version is still pinned to an outdated
release in the action setup, so update the version variable used by the
download/install steps in the export-infisical-dotenv action and mirror the same
pin in the load-infisical-secrets action. Make sure the version string
referenced by the curl tarball URL and install flow is bumped consistently to
the latest stable CLI release so both actions stay in sync.
In @.github/actions/load-infisical-secrets/action.yml:
- Around line 32-38: The Infisical CLI version pin is stale in the Linux install
flow, so update the version used in the action’s install script to v0.43.96 and
keep it consistent with the matching Infisical action that uses the same pinned
release. Make the change in the load-infisical-secrets action by updating the
version variable and ensure any shared install logic or sibling action that
references the same CLI download URL is bumped in lockstep.
In `@packages/providers-meta/src/providers.ts`:
- Around line 167-170: Add direct test coverage for the new providerLogoId()
alias path in providers.ts, since providerLogoType("whoop_ble") alone does not
verify the actual logo ID used by web/mobile asset lookup. Update the existing
provider logo tests to call providerLogoId() for WHOOP Bluetooth and assert it
resolves through resolveLogoId() to the expected asset stem, ensuring this path
stays covered if the alias changes.
In `@packages/server/src/repositories/sync-repository.ts`:
- Around line 198-210: The query in sync-repository.ts using
this.#providerStatsStore.query against ingest.metric_stream needs to read the
deduplicated view of the versioned table. Add FINAL to the SELECT source so
max(recorded_at) is computed from the latest row state, and keep the existing
provider_id/user_id filters and is_deleted = 0 condition in place.
In `@packages/server/src/routers/sync.test.ts`:
- Around line 615-627: The test setup for `does not query ClickHouse when sensor
store is not configured` is incomplete because the `query` spy is never passed
into `createCaller()`, so the assertion cannot detect regressions. Update this
test in `sync.test.ts` to inject the `query` mock through the same path used by
the `providers` router (via `createCaller` and any related ClickHouse/client
setup), then assert against that injected spy so the test actually observes
whether `providers()` triggers a ClickHouse query.
In `@packages/server/src/routers/sync.ts`:
- Around line 161-175: The degraded ClickHouse error handlers in sync.ts are
swallowing unexpected failures after only logging a warning. In the Promise.all
branches for repo.getProviderStats() and repo.getPushProviderLastReceived(),
update the catch callbacks to call captureException(error) before returning an
empty array, keeping the existing logProvidersQueryFailure context and ensuring
the fallback still returns the typed [] result.
In `@packages/web/src/components/ProviderLogo.test.tsx`:
- Line 24: Add a test for the new whoop_ble alias behavior in
ProviderLogo.test.tsx. The current WHOOP label assertion only covers
providerLabel("whoop"); add a case that exercises providerLogoId(provider)
through rendering whoop_ble and verifies it resolves to the same shared WHOOP
asset path as whoop. Keep the existing WHOOP label test, but extend coverage so
the new alias mapping in ProviderLogo cannot regress unnoticed.
In `@packages/web/src/components/SyncProviderCard.tsx`:
- Around line 65-68: The conditional labels in SyncProviderCard should not use
provider.authorized as a proxy for live BLE activity because it can be true for
stale data; update the pushOnly copy to neutral wording and keep freshness
information in the explicit Last received timestamp. Make the same copy change
in both affected render paths in SyncProviderCard so the UI no longer says
“Receiving data” or “Live BLE push” based only on authorized.
---
Outside diff comments:
In `@packages/server/src/routers/sync.ts`:
- Around line 155-233: The providers() query currently returns a mixed array of
registered-provider and push-provider objects without an explicit output
contract. Add a Zod .output(...) schema on
cachedProtectedQuery(CacheTTL.SHORT).query for providers() that matches the
returned shape, using a discriminated union or shared object schema to cover
both variants. Reference the provider mapping logic in providers(), including
ProviderModel and PUSH_PROVIDERS, and ensure the schema includes all fields
returned by the final array.
In `@src/providers/whoop-journal.test.ts`:
- Around line 134-139: The WHOOP provider rename test in whoop-journal.test.ts
only checks id and name, but it does not verify the changed public behavior for
scheduledSyncLookbackDays. Update the WhoopProvider test block that imports
WhoopProvider and instantiates it to also assert the new lookback contract,
specifically that scheduledSyncLookbackDays is 30, so the renamed provider test
covers all changed behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c49c209d-6c9b-4320-a5db-8068d4d5e733
📒 Files selected for processing (22)
.github/actions/export-infisical-dotenv/action.yml.github/actions/load-infisical-secrets/action.ymlpackages/mobile/components/ProviderLogo.tsxpackages/providers-meta/package.jsonpackages/providers-meta/src/providers.test.tspackages/providers-meta/src/providers.tspackages/providers-meta/src/push-providers.test.tspackages/providers-meta/src/push-providers.tspackages/server/src/index.integration.test.tspackages/server/src/repositories/sync-repository.test.tspackages/server/src/repositories/sync-repository.tspackages/server/src/repositories/whoop-ble-sync-repository.tspackages/server/src/routers/heart-rate.test.tspackages/server/src/routers/sync.test.tspackages/server/src/routers/sync.tspackages/web/src/components/DataSourcesPanel.tsxpackages/web/src/components/ProviderLogo.test.tsxpackages/web/src/components/ProviderLogo.tsxpackages/web/src/components/SyncProviderCard.tsxpackages/web/src/pages/ProviderDetailPage.tsxsrc/providers/whoop-journal.test.tssrc/providers/whoop/provider.ts
Report degraded ClickHouse lookups to Sentry, tighten providers output typing, use neutral push-provider copy, and add missing test coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Share SyncProviderSummary typing across web components, use server-provided push descriptions, document FINAL for ReplacingMergeTree correctness, and tighten sync router test coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Add the required description property to provider mocks after SyncProviderSummary typing was tightened. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
sync.providers, including last-received timestamps from ClickHouse metric stream data.Test plan
npm test -- --run packages/server/src/routers/sync.test.ts packages/providers-meta/src/push-providers.test.tsMade with Cursor
Summary by cubic
Surface WHOOP Bluetooth on web as a mobile, push-only provider with last-received timestamps, and clearly separate it from WHOOP cloud data. Tightened server typing and error handling so ClickHouse issues don’t break
sync.providers.New Features
@dofek/providers/push-providers;sync.providersreturns a standardized summary withpushOnly,authorized,description, andlastSyncedAt.SyncRepository.getPushProviderLastReceived()fromingest.metric_stream; push providers are authorized when data or a last-received timestamp exists.SyncProviderSummarytyping across components and use server-provided push descriptions.providerLogoIdsowhoop_blereuses WHOOP assets on web and mobile.Bug Fixes
sync.providersoutput typing and neutral push-provider copy; expanded tests to cover push authorization and error handling. UpdatedSyncProviderCardstories/tests to includedescriptionin mocks to matchSyncProviderSummary.Written for commit e0c2864. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes