fix: stop live polarization fallback

[Asherlc]

Summary

• make activity_polarization_zones the sole source for polarization trend responses
• return the canonical empty result instead of launching a live deduped_sensor scan
• add a real ClickHouse fixture with populated raw sensor data and an empty serving table
• record the production timeout, confirmed root cause, fix, and remaining follow-up

Root cause

An empty serving-model query was treated as a signal to recompute polarization from activity_summary and deduped_sensor, turning a valid no-data response into a long-running request.

Validation

• pnpm vitest run --project unit packages/server/src/repositories/efficiency-repository.test.ts
• pnpm vitest run --project unit packages/server/src/routers/efficiency.test.ts
• TEST_DATABASE_URL="$DATABASE_URL" pnpm vitest run --project integration packages/server/src/repositories/efficiency-repository.integration.test.ts
• TEST_DATABASE_URL="$DATABASE_URL" pnpm vitest run --project integration packages/server/src/routers/efficiency.integration.test.ts
• pnpm lint
• pnpm tsc --noEmit
• cd packages/server && pnpm tsc --noEmit
• cd packages/web && pnpm tsc --noEmit

The coordinating agent is serializing the full Docker-free workspace suite across issue branches.

Fixes #1997

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[sourcery-ai]

Sorry @Asherlc, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[coderabbitai]

Warning

Review limit reached

@Asherlc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8da266d7-a40f-4aa7-ba47-c064f150df17

📥 Commits

Reviewing files that changed from the base of the PR and between e4c429e and cc2dbac.

📒 Files selected for processing (7)

• docs/production-incident-baseline.md
• packages/server/src/repositories/efficiency-repository.integration.test.ts
• packages/server/src/repositories/efficiency-repository.test.ts
• packages/server/src/repositories/efficiency-repository.ts
• packages/server/src/routers/clickhouse-integration-test-helpers.ts
• packages/server/src/routers/router-data.integration.test.ts
• packages/server/src/routers/router.integration.test.ts

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[qodo-code-review]

PR Summary by Qodo

Fix polarization trend: remove live ClickHouse fallback on empty serving model

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

AI Description

• Treat empty activity_polarization_zones results as canonical “no data” (no recompute).
• Remove fallback that scanned deduped_sensor/activity_summary, preventing long-running
 requests.
• Add/adjust unit + ClickHouse integration tests to ensure only the serving model is queried.

Diagram

graph TD
  A["Efficiency router"] --> B["EfficiencyRepository.getPolarizationTrend"] --> C["ActivitySensorStore.query"] --> D[("activity_polarization_zones")]
  C -. "removed fallback" .-> E[("deduped_sensor")]
  C -. "removed join" .-> F[("activity_summary")]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep fallback behind a flag/timeout

• ➕ Provides a “best effort” result even when serving tables lag
• ➕ Reduces user-visible empty results
• ➖ Still risks expensive ClickHouse scans and slow requests
• ➖ Complicates correctness: empty can be valid, not a recompute signal

2. Async backfill instead of request-time compute

• ➕ Preserves responsiveness while still populating serving model
• ➕ Centralizes compute cost in background jobs
• ➖ Requires job orchestration/queueing and monitoring
• ➖ Adds eventual-consistency behavior and operational overhead

Recommendation: The PR’s approach is preferred: activity_polarization_zones should be the sole source of truth for trend responses, and an empty result must remain a fast, canonical response. If backfills are needed, they should be triggered out-of-band (async), not via synchronous request-time scans of deduped_sensor.

Files changed (3) +111 / -117

Bug fix (1) +3 / -83

efficiency-repository.tsRemove polarization live fallback; always query serving model +3/-83

Remove polarization live fallback; always query serving model

• Eliminates the secondary ClickHouse query that recomputed polarization from 'activity_summary' + 'deduped_sensor' when the serving model returned no rows. 'getPolarizationTrend' now performs a single 'activity_polarization_zones' query and returns '{ maxHr: null, weeks: [] }' when empty.

packages/server/src/repositories/efficiency-repository.ts

Tests (2) +108 / -34

efficiency-repository.test.tsUpdate unit tests to assert single-query polarization behavior +9/-34

Update unit tests to assert single-query polarization behavior

• Adjusts the mocked sensor store so polarization queries return provided rows (no forced empty read-model behavior). Updates polarization tests to expect only one ClickHouse query and removes assertions tied to the deleted fallback query/params.

packages/server/src/repositories/efficiency-repository.test.ts

efficiency-repository.integration.test.tsAdd ClickHouse integration test ensuring no raw scan on empty serving table +99/-0

Add ClickHouse integration test ensuring no raw scan on empty serving table

• Introduces an integration test that seeds raw heart-rate samples into 'analytics.deduped_sensor' while leaving 'analytics.activity_polarization_zones' empty. Verifies 'getPolarizationTrend' returns an empty canonical response and issues exactly one query targeting the serving model (not 'deduped_sensor'/'activity_summary').

packages/server/src/repositories/efficiency-repository.integration.test.ts

[github-actions]

Storybook previews for b914d4c5 are ready:

• Web Storybook
• Mobile Storybook

This comment updates automatically on each PR push.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 156 rules

✅ Skills: fix-provider, write-tests, cloudflare

1. No runbook evidence reference 📘 Rule violation ➹ Performance

Description

This PR changes request-path behavior to avoid a potentially expensive ClickHouse scan (no fallback
to raw sensor models), but the change has no nearby reference to the loading-performance runbook or
a checked-in slowdown classification/evidence artifact. This violates the requirement to
classify/record dashboard loading slowdowns (e.g., via Axiom evidence) before changing behavior.

Code

packages/server/src/repositories/efficiency-repository.ts[R541-550]

   * Polarization Index trend per week using Treff 3-zone model.
   * PI = log10((f1 / (f2 * f3)) * 100) where f = fraction of total training time.
   * PI > 2.0 indicates a well-polarized training distribution.
-   * Reads from the pre-computed activity_polarization_zones read model when available.
+   * Reads from the pre-computed activity_polarization_zones read model.
   */
  async getPolarizationTrend(range: ChartRange): Promise<PolarizationTrendResult> {
    const lowerBoundPredicate = range.clickHouseTimestampAfter("started_at");
-    const activitySummaryLowerBoundPredicate = range.clickHouseTimestampAfter("asum.started_at");
    const rangeParams = range.clickHouseParams();

-    // Try pre-computed read model first (avoids expensive deduped_sensor scan)
-    const readModelRows = await this.#sensorStore.query(
+    const rows = await this.#sensorStore.query(

Relevance

⭐⭐ Medium

Repo enforces evidence/citations in incident/runbook docs, but no clear precedent requiring
code-adjacent runbook links for perf changes.

PR-#1394
PR-#1858

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The runbook-driven compliance rule requires recorded slowdown classification/evidence before
implementing behavior changes for slow loading. The updated repository code and the new integration
test demonstrate the behavior change (serving-table-only; no raw sensor scan) but include no
reference to the runbook or an evidence artifact documenting the classification for this change.

Rule 1540813: Classify and record dashboard slowdowns before modifying behavior
packages/server/src/repositories/efficiency-repository.ts[540-550]
packages/server/src/repositories/efficiency-repository.integration.test.ts[73-98]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A performance-motivated behavioral change was made (removing the live fallback scan), but there is no in-repo evidence that the loading-performance runbook was followed and no recorded classification/evidence artifact (e.g., an Axiom query link) tied to this change.

## Issue Context
The runbook requires recording evidence (preferably Axiom) and classifying the slowdown type before changing behavior in response to slow loading / suspected ClickHouse bottlenecks.

## Fix Focus Areas
- packages/server/src/repositories/efficiency-repository.ts[540-550]
- packages/server/src/repositories/efficiency-repository.integration.test.ts[73-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Empty trend cached hour ✗ Dismissed 🐞 Bug ☼ Reliability

Description

With the live fallback removed, getPolarizationTrend returns {maxHr:null, weeks:[]} whenever
analytics.activity_polarization_zones is empty; the efficiency.polarizationTrend endpoint is cached
for 1 hour, so an early empty response can be served for up to an hour even after the serving table
is later populated (absent a separate cache invalidation mechanism).

Code

packages/server/src/repositories/efficiency-repository.ts[R546-553]

  async getPolarizationTrend(range: ChartRange): Promise<PolarizationTrendResult> {
    const lowerBoundPredicate = range.clickHouseTimestampAfter("started_at");
-    const activitySummaryLowerBoundPredicate = range.clickHouseTimestampAfter("asum.started_at");
    const rangeParams = range.clickHouseParams();

-    // Try pre-computed read model first (avoids expensive deduped_sensor scan)
-    const readModelRows = await this.#sensorStore.query(
+    const rows = await this.#sensorStore.query(
      polarizationRowSchema,
      `SELECT
        any(max_hr) AS max_hr,

Relevance

⭐⭐ Medium

Mixed precedent: cache-staleness/invalidation suggestions sometimes accepted, but at least one
similar cache-staleness finding was rejected.

PR-#1961
PR-#749
PR-#900

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The repository now only queries the serving model and returns an empty result when it has no rows;
the integration test demonstrates that raw sensor data can exist while the serving model is empty.
The router caches this endpoint for 1 hour (CacheTTL.LONG), so an empty response can be cached
beyond the time the serving table becomes populated.

packages/server/src/repositories/efficiency-repository.ts[540-591]
packages/server/src/repositories/efficiency-repository.integration.test.ts[73-98]
packages/server/src/routers/efficiency.ts[93-113]
packages/server/src/trpc.ts[227-231]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`EfficiencyRepository.getPolarizationTrend()` now returns an empty canonical response when `analytics.activity_polarization_zones` has no rows. The router caches `efficiency.polarizationTrend` responses for `CacheTTL.LONG` (1 hour), which can cause an empty response to persist for up to an hour even if the serving table becomes populated shortly after.

### Issue Context
This PR intentionally stops the live fallback (no `deduped_sensor` scan), so transient empty serving-table states are now user-visible. The caching layer can amplify that transient state into a longer stale period.

### Fix Focus Areas
- packages/server/src/routers/efficiency.ts[93-113]
- packages/server/src/repositories/efficiency-repository.ts[540-591]

### Suggested fix
Adjust the caching policy for `efficiency.polarizationTrend` to reduce stale-empty duration. Options:
1) Use a shorter TTL (e.g. `CacheTTL.MEDIUM`) for this endpoint.
2) If the cache wrapper supports it, apply a shorter TTL only when the result is empty (`weeks.length === 0`).
3) Alternatively, add/trigger cache invalidation keyed to the serving-model refresh (if such a mechanism exists in this codebase).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[codereviewbot-ai]

🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews.

[Asherlc]

CI follow-up on the latest head:

• Clean integration shards exposed two router fixtures that still expected the removed raw fallback (expected null to be 190). Commit 1e9df3c47 now seeds the canonical activity_polarization_zones serving row through a shared test helper and invalidates only the route cache prefix.
• Focused real-ClickHouse validation passes for both affected suites: router.integration.test.ts 1/1 and router-data.integration.test.ts 1/1. Full repository lint and the server typecheck also pass.
• The unrelated Swift gate had already passed 74 HealthKit tests, eight watch tests, and coverage before artifact upload timed out with ETIMEDOUT. Its unchanged attempt-two rerun passed the same pinned upload step. Commit cc2dbac19 records both operational findings in the incident baseline.