feat(recovery): expose baseline-relative metrics - #2287
Conversation
Additive ClickHouse columns keep the incremental recovery models compatible while exposing prior-calendar-day statistics. Refs #2248
Serve canonical recovery baselines through API, MCP, web, and mobile. Reuse the same z-scores for readiness, stress, and health status so every consumer compares against one baseline definition.
Load the existing web test setup through a repository-relative path. Bare setup resolution escaped Conductor worktrees into the parent workspace and prevented canonical Vitest runs.
|
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. |
|
🤖 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. |
There was a problem hiding this comment.
Sorry @Asherlc, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughRecovery baseline statistics are computed in ClickHouse and exposed through server contracts, tRPC, MCP, web, and mobile surfaces. The change adds baseline means, standard deviations, z-scores, coverage, 7-day versus prior-28-day comparisons, and related rendering and test coverage. ChangesBaseline computation and storage
Server contract and consumers
Client rendering
Documentation and validation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant TrendsAPI
participant RecoveryBaselineRepository
participant ClickHouse
participant HealthStatus
Client->>TrendsAPI: request recovery trends
TrendsAPI->>RecoveryBaselineRepository: load baseline rows
RecoveryBaselineRepository->>ClickHouse: query daily_recovery
ClickHouse-->>RecoveryBaselineRepository: baseline statistics and z-scores
RecoveryBaselineRepository-->>TrendsAPI: baselineRelative metrics
TrendsAPI->>HealthStatus: build health statuses
TrendsAPI-->>Client: trends, baselineRelative, healthStatus
Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
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 |
Reviewer's GuideThis PR standardizes recovery metric baselines by pushing canonical baseline-relative calculations into ClickHouse read models, exposing them via new server contracts and repositories, and wiring them through tRPC/MCP/web/mobile so HRV, resting HR, respiratory rate, and sleep efficiency share a consistent 30‑day baseline + 7‑vs‑28‑day comparison context and z-scores for readiness/stress/health surfaces. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Mobile PreviewScan to open on device:
To test on device:
|
PR Summary by QodoExpose baseline-relative recovery metrics via ClickHouse read models and shared contract
AI Description
Diagram
High-Level Assessment
Files changed (47)
|
|
Storybook previews for This comment updates automatically on each PR push. |
|
🤖 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. |
Code Review by Qodo
Context used✅ Compliance rules (platform):
218 rules✅ Skills:
fix-provider, write-tests, cloudflare 1.
|
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
analytics/models/read_models/daily_recovery_inputs.sql (1)
122-223: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNullify empty-window means to preserve explicit-gap semantics.
Following the analytics-rule requirement to preserve bounded/dirty-key behavior, empty windows here currently emit
NaNviaavg(...) OVER (...); the SD guards handlebaseline_sample_countandIS NOT NULLchecks, but mean-only gaps are not caught and incrementally comparedNaNs make the dirty-key comparison fragile. Convert empty-window means toNULLconsistently, includinghrv_mean_7d/hrv_mean_previous_28dand all RHR/RR/efficiency counterparts, usingavgOrNull(...)or an equivalent sample-count guard.🤖 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 `@analytics/models/read_models/daily_recovery_inputs.sql` around lines 122 - 223, Update the windowed mean expressions in the window_statistics CTE to use avgOrNull (or an equivalent count-based NULL guard) so empty windows return NULL instead of NaN. Apply this consistently to the 30-day, 7-day, previous-28-day, and 60-day means for HRV, resting heart rate, respiratory rate, and efficiency_pct, while preserving existing window ranges and standard-deviation logic.analytics/models/read_models/daily_recovery.sql (1)
71-98: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTreat empty rolling windows as missing baselines before scoring.
hrv_mean_30d/rhr_mean_30d/rr_mean_30d/efficiency_mean_30dare produced by plainavg(...) OVER (...)indaily_recovery_inputs.sql; when the 30-day preceding window is empty, ClickHouse returnsNaN, andNaN IS NOT NULLmakes the z-score guards pass. The result is NaN persisted forhrv_z_score/resting_hr_z_score/respiratory_rate_z_score, and the downstreamIS NULLfallbacks in this file lose their intended 62 sentinel values.Fix this at the source in
daily_recovery_inputs.sql’swindow_statisticsCTE by returningNULLwhen the relevant baseline sample count is zero, then keep the existing z-score/null checks.🤖 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 `@analytics/models/read_models/daily_recovery.sql` around lines 71 - 98, The window_statistics CTE in daily_recovery_inputs.sql must convert empty 30-day baseline averages to NULL by checking each relevant baseline sample count and returning NULL when it is zero. Update the HRV, resting heart rate, respiratory rate, and efficiency baseline expressions at the source, while leaving the existing z-score guards and sleep_score logic in daily_recovery.sql unchanged.packages/server/src/contracts/mobile-dashboard-contracts.ts (1)
105-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate metric-key literals — derive from
baselineRelativeMetricKeySchemainstead.
healthMetricKeySchema's first four values (hrv,resting_heart_rate,respiratory_rate,sleep_efficiency) duplicatebaselineRelativeMetricKeySchemainbaseline-relative-metrics.ts. Adding a new baseline metric later requires remembering to update both enums in sync.♻️ Proposed fix to derive from the canonical schema
+import { baselineRelativeMetricKeySchema } from "./baseline-relative-metrics.ts"; + export const healthMetricKeySchema = z.enum([ - "hrv", - "resting_heart_rate", - "respiratory_rate", - "sleep_efficiency", + ...baselineRelativeMetricKeySchema.options, "spo2", "steps", "skin_temperature", "trend_weight", "body_fat_percentage", ]);🤖 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/contracts/mobile-dashboard-contracts.ts` around lines 105 - 115, Update healthMetricKeySchema to derive the shared baseline metric keys from baselineRelativeMetricKeySchema instead of duplicating its first four literals, while retaining the remaining health-specific keys and preserving the resulting schema behavior.packages/server/src/services/mobile-recovery-tab.test.ts (1)
62-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOne 30+ column
analytics.daily_recoveryfixture, copy-pasted per test file. Identical bodies also exist inpackages/server/src/mcp/route.test.tsandpackages/server/src/routers/daily-metrics.test.ts, so every new baseline column costs four synchronized edits and any missed copy fails as a Zod parse error rather than an obvious diff.
packages/server/src/services/mobile-recovery-tab.test.ts#L62-L99: moverecoveryRowinto a colocatedtest-helpers.tsunderpackages/server/src/services/(or a shared server-test helper module) and export it.packages/server/src/routers/mobile-dashboard.test.ts#L822-L844: replace the inline row literal with a call to that shared helper plus the overrides this test actually cares about.As per coding guidelines: "Shared test helpers and fixtures used by multiple tests belong in a local test-helpers.ts file, not in production source files or another test file."
🤖 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/services/mobile-recovery-tab.test.ts` around lines 62 - 99, Move the duplicated recoveryRow fixture from packages/server/src/services/mobile-recovery-tab.test.ts:62-99 into an exported local test-helpers.ts, then update that test and the inline fixture at packages/server/src/routers/mobile-dashboard.test.ts:822-844 to import and call the shared recoveryRow helper with only the needed overrides; preserve all existing fixture defaults and test-specific values.Source: Coding guidelines
packages/server/src/routers/recovery.ts (1)
433-433: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThe 30-day warmup fetch is now dead weight. Baselines and z-scores are precomputed in
analytics.daily_recovery, so nothing in this handler needs rows before the output window — they are fetched, parsed, and then dropped by themetrics.date <= cutoffDateguard on Line 488. Query the requested window directly.♻️ Proposed change
- const queryRange = range.withWarmupDays(30); + const queryRange = range;Also applies to: 455-469
🤖 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/recovery.ts` at line 433, Remove the withWarmupDays(30) expansion from the recovery handler’s query range construction, including the analogous range used around the rows referenced at lines 455-469. Query only the requested output window while preserving the existing date cutoff and downstream metric processing.
🤖 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 `@docs/superpowers/plans/2026-07-29-baseline-relative-recovery-metrics.md`:
- Line 28: Update the story requirements in the plan to explicitly include
loading and empty/no-data states alongside default/complete, sparse,
unavailable, and directional variants. Apply the same requirement to the
corresponding story requirement at the other referenced location, preserving the
existing meaningful variant coverage.
In `@packages/mobile/app/`(tabs)/recovery.tsx:
- Around line 71-96: Extract the shared baseline-relative metric formatting into
`@dofek/format`, preserving the complete behavior of formatBaselineContext and
supporting the optional formatter/unit parameters needed by the web caller. In
packages/mobile/app/(tabs)/recovery.tsx lines 71-96, remove the local
formatBaselineContext implementation and import the shared helper. In
packages/web/src/components/HealthStatusBar.tsx lines 27-55, remove the
duplicate formatting logic, including formatContextValue, and call the shared
helper with the existing formatter and unit values.
- Around line 397-400: Move the shared baseline-context formatting logic used by
formatBaselineContext in recovery.tsx and HealthStatusBar.tsx into
`@dofek/format`, preserving the required baseline, z-score, and comparison
semantics for each platform. Export the shared formatter/helper from
`@dofek/format`, then update both platform components to import and use it instead
of maintaining separate client-side implementations.
In `@packages/server/src/mcp/tools.ts`:
- Around line 323-339: Update the get_health_trends handler to fail explicitly
when context.sensorStore is absent, using the same precondition-check pattern
and error message style as get_body_metrics. After validating the required
ClickHouse analytics store, remove the baselineRows fallback to [] and preserve
the existing repository query path.
In `@packages/server/src/routers/recovery.test.ts`:
- Around line 47-68: Remove the TypeScript z-score synthesis from
packages/server/src/routers/recovery.test.ts#L47-L68 by deleting baselineZScore
and canonicalizeRecoveryRows, then provide all three *_z_score values directly
in makeSensorStore fixtures. In
packages/server/src/repositories/stress-repository.test.ts#L27-L52, restore the
query mock to return rows directly, add both z-score fields to makeRow, and
remove mean/sd fields. In packages/server/src/routers/stress.test.ts#L53-L83,
remove zScore and define z-score values in makeRow defaults or overrides. In
packages/server/src/routers/strength-stress.test.ts#L184-L203, remove zScore and
recoveryRows mapping so the analytics.daily_recovery branch returns
caller-supplied rows; leave statistical semantics to the ClickHouse integration
test.
In `@packages/server/src/routers/recovery.ts`:
- Around line 491-506: Create a shared readiness-components helper in
`@dofek/scoring` beside zScoreToRecoveryScore, with a named 62 fallback constant,
centralized sign inversions, and sleep-efficiency clamping. In
packages/server/src/routers/recovery.ts#L491-L506, replace the inline
calculations with the helper using the parsed row’s four *_z_score fields and
efficiency_pct; in packages/server/src/services/mobile-recovery-tab.ts#L60-L98,
have computeReadinessRows resolve the four baseline metrics and delegate to the
same helper so both surfaces use one canonical computation.
In `@packages/server/src/services/daily-recovery-read-model.integration.test.ts`:
- Around line 52-75: Add a regression test alongside the existing daily recovery
integration fixture that represents a new user or metric with zero antecedent
baseline samples. Seed only the required current-day data, run
materializeRecoveryInputs and materializeRecovery, then assert
hrv_baseline_sample_count is 0 and the resulting mean/z-score are NULL while the
recovery score uses the documented default of 62; ensure the assertions reject
NaN.
In `@packages/server/src/services/health-status.test.ts`:
- Around line 38-80: Expand the test around buildDailyMetricHealthStatuses to
table-drive all four recoveryMetricIntent metrics: hrv, resting_heart_rate,
respiratory_rate, and sleep_efficiency. Provide each case with its corresponding
baseline-relative metric and assert the expected intent-derived status, ensuring
respiratory_rate validates the intended non-neutral branch rather than the
current fallthrough.
In `@packages/server/src/services/health-status.ts`:
- Around line 220-223: Make baselineRelative a required parameter of
buildDailyMetricHealthStatuses by removing its = [] default, preserving the
existing production caller in daily-metrics.ts. Update the health-status test
that omits this argument to explicitly pass an empty array.
- Around line 201-205: Update recoveryMetricIntent so the respiratory_rate
metric returns "lower", aligning its intent with the inverted z-score behavior
in computeReadinessRows and the readinessScore handler. Keep the existing hrv,
sleep_efficiency, resting_heart_rate, and neutral fallbacks unchanged.
In `@packages/server/src/services/mobile-recovery-tab.ts`:
- Around line 56-58: Update findRecoveryMetric so its metric parameter uses the
BaselineRelativeMetric["metric"] literal union instead of string, importing
BaselineRelativeMetric as a type from the baseline-relative-metrics contract.
Keep the existing lookup behavior unchanged while ensuring invalid metric keys,
including mismatched spellings, fail at compile time.
---
Outside diff comments:
In `@analytics/models/read_models/daily_recovery_inputs.sql`:
- Around line 122-223: Update the windowed mean expressions in the
window_statistics CTE to use avgOrNull (or an equivalent count-based NULL guard)
so empty windows return NULL instead of NaN. Apply this consistently to the
30-day, 7-day, previous-28-day, and 60-day means for HRV, resting heart rate,
respiratory rate, and efficiency_pct, while preserving existing window ranges
and standard-deviation logic.
In `@analytics/models/read_models/daily_recovery.sql`:
- Around line 71-98: The window_statistics CTE in daily_recovery_inputs.sql must
convert empty 30-day baseline averages to NULL by checking each relevant
baseline sample count and returning NULL when it is zero. Update the HRV,
resting heart rate, respiratory rate, and efficiency baseline expressions at the
source, while leaving the existing z-score guards and sleep_score logic in
daily_recovery.sql unchanged.
In `@packages/server/src/contracts/mobile-dashboard-contracts.ts`:
- Around line 105-115: Update healthMetricKeySchema to derive the shared
baseline metric keys from baselineRelativeMetricKeySchema instead of duplicating
its first four literals, while retaining the remaining health-specific keys and
preserving the resulting schema behavior.
In `@packages/server/src/routers/recovery.ts`:
- Line 433: Remove the withWarmupDays(30) expansion from the recovery handler’s
query range construction, including the analogous range used around the rows
referenced at lines 455-469. Query only the requested output window while
preserving the existing date cutoff and downstream metric processing.
In `@packages/server/src/services/mobile-recovery-tab.test.ts`:
- Around line 62-99: Move the duplicated recoveryRow fixture from
packages/server/src/services/mobile-recovery-tab.test.ts:62-99 into an exported
local test-helpers.ts, then update that test and the inline fixture at
packages/server/src/routers/mobile-dashboard.test.ts:822-844 to import and call
the shared recoveryRow helper with only the needed overrides; preserve all
existing fixture defaults and test-specific values.
🪄 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: 2ed9aebc-eb1d-4b94-aca6-0750a78a42d6
📒 Files selected for processing (47)
analytics/models/read_models/daily_recovery.sqlanalytics/models/read_models/daily_recovery_inputs.sqlanalytics/models/read_models/read_model_microbatch.sql.test.tsdocs/mcp.mddocs/superpowers/plans/2026-07-29-baseline-relative-recovery-metrics.mdpackages/mobile/app/(tabs)/recovery.stories.tsxpackages/mobile/app/(tabs)/recovery.test.tsxpackages/mobile/app/(tabs)/recovery.tsxpackages/mobile/components/HealthStatusCards.tsxpackages/server/package.jsonpackages/server/src/contracts/baseline-relative-metrics.test.tspackages/server/src/contracts/baseline-relative-metrics.tspackages/server/src/contracts/mobile-dashboard-contracts.test.tspackages/server/src/contracts/mobile-dashboard-contracts.tspackages/server/src/mcp/route.test.tspackages/server/src/mcp/tools.tspackages/server/src/repositories/recovery-baseline-repository.test.tspackages/server/src/repositories/recovery-baseline-repository.tspackages/server/src/repositories/stress-repository.test.tspackages/server/src/repositories/stress-repository.tspackages/server/src/routers/clickhouse-integration-test-models.tspackages/server/src/routers/daily-metrics.test.tspackages/server/src/routers/daily-metrics.tspackages/server/src/routers/mobile-dashboard.test.tspackages/server/src/routers/recovery-settings-sleep-need-sport-settings.test.tspackages/server/src/routers/recovery.test.tspackages/server/src/routers/recovery.tspackages/server/src/routers/strength-stress.test.tspackages/server/src/routers/stress.test.tspackages/server/src/services/daily-recovery-read-model.integration.test.tspackages/server/src/services/health-status.test.tspackages/server/src/services/health-status.tspackages/server/src/services/mobile-recovery-tab.test.tspackages/server/src/services/mobile-recovery-tab.tspackages/server/src/types.tspackages/web/src/components/HealthStatusBar.stories.tsxpackages/web/src/components/HealthStatusBar.test.tsxpackages/web/src/components/HealthStatusBar.tsxpackages/web/src/components/TimeRangeSelector.consumers.test.tsxpackages/web/src/lib/healthStatus.tspackages/web/src/pages/Dashboard.test.tsxpackages/web/src/pages/Dashboard.tsxsrc/db/clickhouse-migrations/0062_daily_recovery_baseline_context.test.tssrc/db/clickhouse-migrations/0062_daily_recovery_baseline_context.tssrc/db/clickhouse-migrations/registry.test.tssrc/db/clickhouse-migrations/registry.tsvitest.config.ts
|
🤖 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. |
|
🤖 Review failed: an error occurred during analysis. |
|
🤖 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. |
|
🤖 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. |
Summary
Semantics
Validation
pnpm test:changed: 39 files, 732 tests passedpnpm typecheckpnpm lint:sandboxgit diff --check origin/main...HEADLocal Docker Desktop's daemon API stopped responding after the focused ClickHouse integration passed, so the final analytics SQL/dbt rerun is delegated to the required CI check rather than bypassed.
Fixes #2248
Summary by Sourcery
Add canonical baseline-relative recovery metrics to analytics and server contracts and surface them consistently across tRPC, MCP, web, and mobile recovery experiences.
New Features:
Enhancements:
Build:
Deployment:
Documentation:
get_health_trendsbehavior, including window definitions and z-score availability rules.Tests:
Summary by cubic
Adds baseline‑relative recovery metrics (HRV, resting HR, respiratory rate, sleep efficiency) with 30‑day baselines, z‑scores, and 7‑vs‑28‑day comparisons across API, MCP, web, and mobile. Unifies readiness and stress scoring on canonical z‑scores and adds
@dofek/format/baseline-contextfor consistent display. Implements #2248.New Features
baseline-relative-metricscontract andRecoveryBaselineRepository;daily-metrics,recovery, and mobile dashboard return abaselineRelativearray; export contract/types; latest baseline query bounded to the selected/access window.get_health_trendsnow includes baseline‑relative recovery context.HealthStatusBarand mobile recovery tab render baseline context; addrespiratory_rateandsleep_efficiency; use@dofek/format/baseline-contextfor formatting../packages/web/test-setup.ts.Migration
0062_daily_recovery_baseline_context(additive columns; incremental models remain compatible).baselineRelativearray.Written for commit 81acbe7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation