Switch sleep dashboard reads to ClickHouse - #1151
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates runtime sleep-night reads from Postgres ChangesSleep Data Migration to ClickHouse
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes
|
…ep-stale # Conflicts: # docs/production-incident-baseline.md
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
Contributor
|
Review app is ready: This environment runs on a dedicated Hetzner server for PR #1151 and updates on each push. |
The setupTestDatabase loop that re-runs migrations 0008/0017/0019/0025 was missing the filter that the primary migration loop uses. Migration 0019 creates clickhouse.v_sleep referencing fitness.v_sleep, but 0025 drops fitness.v_sleep, so the proxy creation fails on the secondary replay. Apply the same filter so the CREATE statement is skipped consistently in both loops.
The previous query aliased formatDateTime(started_at) AS started_at in the same projection that the row_number() window function ran over. ClickHouse resolved the inner toTimeZone(started_at, ...) call to the aliased String column, causing 'Illegal type String of argument of function toTimezone' on initial Stryker test runs. Push the formatting up into the outer SELECT so the inner subquery operates on the raw DateTime64 columns. The outer shape (and Zod schema) is unchanged.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/server/src/repositories/anomaly-detection-repository.ts (1)
35-49: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueSchema retains unused sleep columns.
anomalyCheckRowSchemastill definesduration_minutes,sleep_mean,sleep_sd,sleep_count(lines 45-48), but these are now always NULL/0 from the SQL. Consider removing them to keep the schema honest about what the query actually returns.🤖 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/repositories/anomaly-detection-repository.ts` around lines 35 - 49, The schema anomalyCheckRowSchema includes unused sleep-related fields (duration_minutes, sleep_mean, sleep_sd, sleep_count) that the SQL now always returns as NULL/0; remove these four keys from the z.object so the schema matches the actual query result and avoid misleading nullable fields—update anomalyCheckRowSchema definition to only include date, resting_hr, rhr_mean, rhr_sd, rhr_count, hrv, hrv_mean, hrv_sd, and hrv_count.src/personalization/refit.ts (1)
371-454:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing Zod validation for Postgres sleep rows path.
Lines 443-450 access
row.duration_minutesandrow.datedirectly on the result ofdb.execute(Postgres path, lines 373-385) without schema validation. The ClickHouse path at lines 386-410 correctly uses a Zod schema viasensorStore.query, but the Postgres path bypasses this. Per AGENTS.md: "Use Zod to parse data crossing runtime boundaries (DB results)."The
hrvRowsprocessing at lines 435-442 validates with Zod inline. Apply the same pattern tosleepRows:🛡️ Proposed fix to add Zod validation for Postgres sleepRows
+const sleepRowDbSchema = z.object({ + date: z.string(), + duration_minutes: z.coerce.number().nullable(), +}); + const [sleepRows, hrvRows] = await Promise.all([ typeof sensorStoreOrUserId === "string" - ? db.execute( + ? db.execute( sql`WITH nightly AS ( ... ) SELECT date, duration_minutes FROM nightly ORDER BY date ASC`, - ) + ).then((rows) => rows.map((row) => sleepRowDbSchema.parse(row))) : sensorStoreOrUserId.query( z.object({ date: z.string(), duration_minutes: z.coerce.number().nullable(), }), ... ),Alternatively, use
executeWithSchemafromtyped-sql.tsfor the Postgres query.🤖 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/personalization/refit.ts` around lines 371 - 454, The Postgres branch that populates sleepRows (db.execute call when sensorStoreOrUserId is a string) returns raw rows but is used later assuming shape (row.date and row.duration_minutes); add Zod validation like the ClickHouse path to parse each Postgres row before using it (or replace the db.execute with executeWithSchema from typed-sql.ts). Specifically, validate the result of the db.execute used to produce sleepRows with a schema z.object({ date: z.string(), duration_minutes: z.coerce.number().nullable() }) and parse/transform to the same shape the sensorStore.query branch produces so the later rows.flatMap logic (and parseSleepRows) can safely access row.date and row.duration_minutes.packages/server/src/routers/mobile-dashboard.test.ts (1)
63-91: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueMock sequence is brittle and undocumented.
The
querymock relies on a specific call order that must match production code exactly. If the production code reorders queries or adds new ones, this test will silently return wrong data instead of failing clearly.Consider adding inline comments documenting which call each
mockResolvedValueOncecorresponds to, or use a more explicit matching approach.Suggested documentation
function makeSensorStore( dailyLoads: Array<{ metric_date: string; daily_load: number }> = [], yesterdayLoad = 0, currentPhysiologyRows: Array<{ physiological_load: number | null }> = [], baselineSleepRows: SleepTestRow[] = [], lastNightSleepRows: SleepTestRow[] = baselineSleepRows, ): SensorStore { const query = vi.fn(); - query.mockResolvedValueOnce(dailyLoads); - query.mockResolvedValueOnce([{ load: yesterdayLoad }]); - query.mockResolvedValueOnce([]); - query.mockResolvedValueOnce(sleepRowsForClickHouse(baselineSleepRows)); - query.mockResolvedValueOnce(sleepRowsForClickHouse(lastNightSleepRows)); - query.mockResolvedValueOnce(sleepRowsForClickHouse(baselineSleepRows)); - query.mockResolvedValueOnce(currentPhysiologyRows); - query.mockResolvedValue([]); + query.mockResolvedValueOnce(dailyLoads); // 1: daily load (60d) + query.mockResolvedValueOnce([{ load: yesterdayLoad }]); // 2: yesterday load + query.mockResolvedValueOnce([]); // 3: resting heart rate + query.mockResolvedValueOnce(sleepRowsForClickHouse(baselineSleepRows)); // 4: readiness sleep (60d) + query.mockResolvedValueOnce(sleepRowsForClickHouse(lastNightSleepRows)); // 5: sleep analytics (14d) + query.mockResolvedValueOnce(sleepRowsForClickHouse(baselineSleepRows)); // 6: sleep baseline (90d) + query.mockResolvedValueOnce(currentPhysiologyRows); // 7: current physiology + query.mockResolvedValue([]); // fallback for any additional calls return {🤖 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/mobile-dashboard.test.ts` around lines 63 - 91, The test helper makeSensorStore builds a brittle query mock by using a fixed sequence of query.mockResolvedValueOnce calls; replace this with an explicit mapping or document each call: either (A) change query to query.mockImplementation((sql, params) => { if (matches expected SQL/params for dailyLoads) return Promise.resolve(dailyLoads); if (matches yesterday load query) return Promise.resolve([{ load: yesterdayLoad }]); if (matches baseline sleep) return Promise.resolve(sleepRowsForClickHouse(baselineSleepRows)); ... else return Promise.resolve([]); }) using the actual query text/params used in production, or (B) at minimum add inline comments next to each query.mockResolvedValueOnce that state which production query it corresponds to (e.g., "dailyLoads query", "yesterday load query", "baseline sleep rows", "last night sleep rows", "current physiology rows"), referencing makeSensorStore, query, and sleepRowsForClickHouse to locate the code.
🤖 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-05-20-clickhouse-sleep-dashboard.md`:
- Line 13: The heading "Task 1: Add Red Tests For ClickHouse Sleep Reads"
currently uses an H3 and causes a heading-level jump; change it to H2 (use "##
Task 1: Add Red Tests For ClickHouse Sleep Reads") or insert an intermediate H2
above it so the document flows H1 → H2 → H3 and resolves the markdown lint
failure.
- Line 3: Replace the agent-only directive "**For agentic workers: REQUIRED
SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or
superpowers:executing-plans to implement this plan task-by-task. Steps use
checkbox (`- [ ]`) syntax for tracking." with human-actionable instructions that
do not reference agent sub-skills (e.g., "Follow the step-by-step checklist
below and mark tasks complete using `- [ ]` syntax"). Update any phrasing that
assumes an agent (search for the exact agent-only phrase) so the plan reads as
standalone human-executable guidance and maintain the checklist syntax and task
structure.
In `@packages/server/src/repositories/anomaly-detection-repository.ts`:
- Around line 298-306: The fallback using row.duration_minutes / row.sleep_mean
/ row.sleep_sd / row.sleep_count is dead because the SQL now returns NULL/0 for
those columns; update the logic in anomaly-detection-repository.ts so sleepStats
is only derived from fetchSleepNights results: replace the current ternary so
sleepStats = sleepRows.length > 0 ? sleepStatsForDate(sleepRows, date) : null
(or a well-defined empty default), remove references to
row.duration_minutes/row.sleep_* there, and update any callers of sleepStats to
handle the null/empty-default case accordingly; key symbols: sleepStats,
sleepRows, sleepStatsForDate, and the surrounding block where row is used.
In `@packages/server/src/repositories/clickhouse-sleep-repository.ts`:
- Around line 41-46: The accessWindowClause uses raw started_at bounds which
mismatch the query's canonical sleep-day expression; update accessWindowClause
to apply the same timezone-shifted canonical-sleep-day transformation used in
selection (the toTimeZone(... ) - INTERVAL 6 HOUR expression) and compare that
transformed value against parseDateTimeBestEffort({accessStartDate:String}) and
parseDateTimeBestEffort({accessEndDateExclusive:String}) (or their toDate
equivalents) instead of started_at, and make the identical replacement at the
other occurrences referenced (the similar started_at comparisons around the
blocks at the other locations).
In `@packages/server/src/repositories/insights-repository.ts`:
- Around line 68-88: The mapping of fetchSleepNights results hardcodes is_nap:
false inside the sleepRowSchema.parse call, which will mislabel nap sessions;
change the mapping in the promise handler so it reads the nap flag from the
fetched row (e.g., use row.is_nap or row.isNap with a safe fallback like false)
instead of the literal false, i.e. pass the actual nap property through to
sleepRowSchema.parse for each row returned by fetchSleepNights.
In `@packages/server/src/repositories/predictions-repository.ts`:
- Around line 387-395: The endDate is being computed in UTC with new
Date().toISOString().slice(0,10) which misaligns timezone-local daily metrics;
change the endDate calculation used before calling fetchSleepNights to produce
the current date in this.#timezone (timezone-local) instead of UTC so
fetchSleepNights({ ..., endDate, ... }) receives the local date string; update
the code that sets endDate (the variable referenced and the call site to
fetchSleepNights) to derive the YYYY-MM-DD for this.#timezone (e.g., using a
timezone-aware formatter or library) so sleep windows align with the user's
timezone.
In `@packages/server/src/repositories/sleep-repository.test.ts`:
- Around line 136-162: The test for getLatestStages is missing an assertion that
the Postgres path is invoked after ClickHouse returns data; update the test in
sleep-repository.test.ts to assert that the repo's this.query/execute mock was
called (e.g., expect(execute).toHaveBeenCalled() or toHaveBeenCalledTimes(1))
after calling repo.getLatestStages(), so that when clickHouseRows is non-empty
the code still calls the Postgres query path; locate the test block that
constructs makeRepository and repo.getLatestStages() and add the execute/mock
verification immediately after the existing result assertions.
In `@packages/server/src/routers/healthspan-query.ts`:
- Around line 203-220: fetchSleepNights is called without the accessWindow
filter so sleepRows may include data outside the user's billing entitlements;
update the fetchSleepNights invocation in healthspan-query.ts to pass
ctx.accessWindow (e.g., add accessWindow: ctx.accessWindow to the argument
object) so sleepRows, and derived values like sleepDurations, bedtimes,
avgSleepMin and bedtimeStddevMin, are computed only for data within the user's
allowed window.
In
`@packages/server/src/routers/recovery-settings-sleep-need-sport-settings.test.ts`:
- Around line 82-85: The test is using a hardcoded today for every synthesized
sleep schedule row so all rows share the same date; change the generator to use
each fixture row's date (e.g., row.date or fixture.date) when filling the date
field and when calling hourTimestamp for started_at and ended_at (instead of
today) so each schedule entry reflects its corresponding fixture row's date;
update the created object where date, started_at, ended_at, and any
duration/minute computations derive from that per-row date.
In `@packages/server/src/routers/recovery.ts`:
- Around line 709-714: The call in strainTarget to fetchLatestSleepNight
currently retrieves the globally latest sleep night and can return data after
the requested endDate; modify strainTarget to constrain that lookup by passing
input.endDate to fetchLatestSleepNight (or extend fetchLatestSleepNight to
accept an endDate/ cutoff parameter), update the fetchLatestSleepNight signature
and all its call sites accordingly to enforce historical determinism, and ensure
strainTarget uses the returned sleep night filtered by that endDate when
computing readiness/target.
In `@packages/server/src/routers/router-logic.integration.test.ts`:
- Line 84: Delete the dead no-op function refreshViews() from the test file and
remove all calls to refreshViews() (it is invoked in five places) since
fitness.v_sleep no longer exists; ensure any necessary behavior is handled by
the existing syncClickHouseTestActivitySensorStore() calls already present (do
not replace those calls with refreshViews(), just delete the refreshViews()
calls and the function definition).
In `@packages/server/src/routers/sleep-need.ts`:
- Around line 247-252: The route is calling fetchLatestSleepNight without
scoping to the requested analysis date, so lastSleep can be newer than
input.endDate; update the call in the performance route to pass the requested
end date (input.endDate) and, if fetchLatestSleepNight lacks that parameter, add
an endDate (or asOf) argument to fetchLatestSleepNight and make it filter/search
for the latest sleep whose end/time <= endDate. Ensure the symbol names
fetchLatestSleepNight and lastSleep are updated accordingly and that all
historical endpoints that rely on fetchLatestSleepNight are adjusted to use the
new endDate/asOf parameter.
- Around line 129-141: The HRV query omits an upper bound and proper
access-window gating, allowing out-of-range dates to affect median/baseline;
update the SQL in the executeWithSchema call that queries
fitness.v_daily_metrics to constrain date to the requested window by adding both
a lower and upper bound (e.g., change the WHERE clause to use date >=
dateWindowStart(input.endDate, 90) AND date <= ${input.endDate} or a BETWEEN
equivalent) and ensure you keep the user filter (ctx.userId) — this will prevent
future or out-of-range HRV rows from influencing medianHrv/baseline
calculations.
In `@src/db/test-helpers.ts`:
- Around line 83-86: Extract the duplicated string predicate into a single
constant and reuse it in both filter calls: define a constant (e.g.,
SKIP_SLEEP_VIEW = "CREATE OR REPLACE VIEW clickhouse.v_sleep AS") near the top
of the module and replace the inline .filter(...) checks that call
.includes("CREATE OR REPLACE VIEW clickhouse.v_sleep AS") with
.includes(SKIP_SLEEP_VIEW) so both occurrences (the filter at the earlier
.filter(...) and the later similar .filter(...)) reference the same symbol.
---
Outside diff comments:
In `@packages/server/src/repositories/anomaly-detection-repository.ts`:
- Around line 35-49: The schema anomalyCheckRowSchema includes unused
sleep-related fields (duration_minutes, sleep_mean, sleep_sd, sleep_count) that
the SQL now always returns as NULL/0; remove these four keys from the z.object
so the schema matches the actual query result and avoid misleading nullable
fields—update anomalyCheckRowSchema definition to only include date, resting_hr,
rhr_mean, rhr_sd, rhr_count, hrv, hrv_mean, hrv_sd, and hrv_count.
In `@packages/server/src/routers/mobile-dashboard.test.ts`:
- Around line 63-91: The test helper makeSensorStore builds a brittle query mock
by using a fixed sequence of query.mockResolvedValueOnce calls; replace this
with an explicit mapping or document each call: either (A) change query to
query.mockImplementation((sql, params) => { if (matches expected SQL/params for
dailyLoads) return Promise.resolve(dailyLoads); if (matches yesterday load
query) return Promise.resolve([{ load: yesterdayLoad }]); if (matches baseline
sleep) return Promise.resolve(sleepRowsForClickHouse(baselineSleepRows)); ...
else return Promise.resolve([]); }) using the actual query text/params used in
production, or (B) at minimum add inline comments next to each
query.mockResolvedValueOnce that state which production query it corresponds to
(e.g., "dailyLoads query", "yesterday load query", "baseline sleep rows", "last
night sleep rows", "current physiology rows"), referencing makeSensorStore,
query, and sleepRowsForClickHouse to locate the code.
In `@src/personalization/refit.ts`:
- Around line 371-454: The Postgres branch that populates sleepRows (db.execute
call when sensorStoreOrUserId is a string) returns raw rows but is used later
assuming shape (row.date and row.duration_minutes); add Zod validation like the
ClickHouse path to parse each Postgres row before using it (or replace the
db.execute with executeWithSchema from typed-sql.ts). Specifically, validate the
result of the db.execute used to produce sleepRows with a schema z.object({
date: z.string(), duration_minutes: z.coerce.number().nullable() }) and
parse/transform to the same shape the sensorStore.query branch produces so the
later rows.flatMap logic (and parseSleepRows) can safely access row.date and
row.duration_minutes.
🪄 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: 3ef61f49-6da2-4c58-8249-d730e1fc56eb
📒 Files selected for processing (54)
docs/production-incident-baseline.mddocs/superpowers/plans/2026-05-20-clickhouse-sleep-dashboard.mddrizzle/0025_drop_v_sleep.sqldrizzle/_views/02_v_sleep.sqlpackages/server/src/billing/entitlement.tspackages/server/src/insights/schemas.tspackages/server/src/lib/sql-fragments.test.tspackages/server/src/lib/sql-fragments.tspackages/server/src/repositories/anomaly-detection-repository.test.tspackages/server/src/repositories/anomaly-detection-repository.tspackages/server/src/repositories/clickhouse-sleep-repository.tspackages/server/src/repositories/correlation-repository.test.tspackages/server/src/repositories/correlation-repository.tspackages/server/src/repositories/derived-cardio-repository.integration.test.tspackages/server/src/repositories/insights-repository.test.tspackages/server/src/repositories/insights-repository.tspackages/server/src/repositories/life-events-repository.test.tspackages/server/src/repositories/life-events-repository.tspackages/server/src/repositories/predictions-repository.test.tspackages/server/src/repositories/predictions-repository.tspackages/server/src/repositories/sleep-repository.test.tspackages/server/src/repositories/sleep-repository.tspackages/server/src/repositories/stress-repository.tspackages/server/src/repositories/training-repository.test.tspackages/server/src/repositories/training-repository.tspackages/server/src/routers/anomaly-detection.test.tspackages/server/src/routers/body-calendar-sleep-nutrition.test.tspackages/server/src/routers/healthspan-query.tspackages/server/src/routers/healthspan.integration.test.tspackages/server/src/routers/hiking-insights-life-events.test.tspackages/server/src/routers/life-events.test.tspackages/server/src/routers/mobile-dashboard.integration.test.tspackages/server/src/routers/mobile-dashboard.test.tspackages/server/src/routers/mobile-dashboard.tspackages/server/src/routers/predictions.integration.test.tspackages/server/src/routers/predictions.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/router-data.integration.test.tspackages/server/src/routers/router-logic.integration.test.tspackages/server/src/routers/router.integration.test.tspackages/server/src/routers/sleep-consistency-across-endpoints.integration.test.tspackages/server/src/routers/sleep-need.integration.test.tspackages/server/src/routers/sleep-need.test.tspackages/server/src/routers/sleep-need.tspackages/server/src/routers/sleep.integration.test.tspackages/server/src/routers/sleep.test.tspackages/server/src/routers/sleep.tspackages/server/src/routers/stress.tssrc/db/test-helpers.tssrc/personalization/refit.integration.test.tssrc/personalization/refit.test.tssrc/personalization/refit.ts
💤 Files with no reviewable changes (13)
- packages/server/src/lib/sql-fragments.test.ts
- packages/server/src/routers/sleep-consistency-across-endpoints.integration.test.ts
- packages/server/src/lib/sql-fragments.ts
- packages/server/src/routers/mobile-dashboard.integration.test.ts
- drizzle/_views/02_v_sleep.sql
- packages/server/src/billing/entitlement.ts
- packages/server/src/routers/sleep-need.integration.test.ts
- packages/server/src/routers/predictions.integration.test.ts
- packages/server/src/repositories/derived-cardio-repository.integration.test.ts
- packages/server/src/routers/router-data.integration.test.ts
- packages/server/src/routers/sleep.integration.test.ts
- packages/server/src/routers/router.integration.test.ts
- packages/server/src/routers/healthspan.integration.test.ts
The previous fix moved formatting to the outer SELECT, but the outer SELECT still aliased formatDateTime(started_at, ...) AS started_at alongside toString(toDate(toTimeZone(started_at, ...))) AS date. ClickHouse resolves identifiers in the SELECT list to other aliases in the same clause, so toTimeZone(started_at) hit the String alias again. Project the raw DateTime64 columns out of the subquery under distinct names (started_at_dt, ended_at_dt) and compute the partition-key date inside the subquery. The outer SELECT then only formats the distinctly-named columns. Same wire shape and Zod schema.
…ep-stale # Conflicts: # packages/server/src/repositories/training-repository.test.ts # packages/server/src/repositories/training-repository.ts
- test-helpers.ts: extract shared filter predicate for v_sleep skip - router-logic.integration.test.ts: delete dead refreshViews() no-op and its 5 call sites - sleep-repository.test.ts: assert execute is called when ClickHouse returns sleep windows in getLatestStages - recovery-settings-sleep-need-sport-settings.test.ts: use each fixture row's date when synthesizing schedule rows so cutoff assertions remain meaningful - clickhouse-sleep-repository.ts: align accessWindowClause with the timezone-shifted sleep-day predicate so entitlement windows agree with the rest of the query - predictions-repository.ts: compute endDate in user timezone so sleep windows align with daily metrics - healthspan-query.ts: pass ctx.accessWindow into fetchSleepNights - sleep-need.ts: bound the HRV query to <= endDate and apply dateAccessPredicate so out-of-window HRV cannot influence baselines - docs/superpowers/plans/2026-05-20-clickhouse-sleep-dashboard.md: replace agent-only directive with human-executable note and bump task headings from H3 to H2 to satisfy markdownlint
Add an optional endDate parameter that caps the search to the timezone-shifted sleep-day <= endDate. Routes parameterized by an end date (recovery.strainTarget, sleepNeed.performance) now pass input.endDate so historical readiness/performance calls cannot pull sleep data newer than the requested date. Sleep-repository call sites that genuinely want the global latest are left unchanged (the new parameter is optional).
Three issues surfaced in CI: 1) sleepMinutes regression in recovery.sleepAnalytics — the migration started computing sleepMinutes from stage sums for every provider, but on main only Apple Health gets that treatment because other providers already exclude awake time from duration_minutes. Restore provider-aware logic by surfacing provider_id through fetchSleepNights / ClickHouseSleepNight and only deriving from stages when provider_id === 'apple_health'. Also use the same logic for the 7-day rolling average. 2) days=N window off-by-one — fetchSleepNights used a strict '>' lower bound on the timezone-shifted sleep day, dropping the night exactly N days before endDate. Tests that seed N nights expect all N to be returned. Switch the lower bound to '>='. 3) sleep.integration.test.ts and the AH branch of router.integration.test.ts insert sleep_session rows into Postgres and then query the ClickHouse-backed analytics view. The old test refreshed fitness.v_sleep, but that matview no longer exists; mirror the rows into ClickHouse via createClickHouseTestActivitySensorStore (initial sync) and syncClickHouseTestActivitySensorStore (post- insert sync) so the analytics view sees them.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/server/src/repositories/clickhouse-sleep-repository.ts (1)
110-111: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueRemove redundant Zod parsing after
sensorStore.query.
ActivitySensorStore.queryis documented to “parse rows with the supplied Zod schema”, andClickHouseActivitySensorStore.queryalready doesreturn rows.map((row) => schema.parse(row)).clickhouse-sleep-repository.tsthen parses again (clickHouseSleepNightSchema.parse(row)), making the extra.parse()redundant.Suggested fix
- return rows.map((row) => clickHouseSleepNightSchema.parse(row)); + return rows;Apply the same adjustment to the
fetchLatestSleepNightpath (theparsedRows = rows.map(...parse...)section).🤖 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/repositories/clickhouse-sleep-repository.ts` around lines 110 - 111, The rows returned by ActivitySensorStore.query are already parsed by ClickHouseActivitySensorStore.query using the provided Zod schema, so remove the redundant clickHouseSleepNightSchema.parse calls: in the function that currently does return rows.map((row) => clickHouseSleepNightSchema.parse(row)) simply return rows (or rows as the expected type), and in fetchLatestSleepNight remove the parsedRows = rows.map(...parse...) step and use the already-parsed rows directly; keep references to clickHouseSleepNightSchema only where the schema is passed into ActivitySensorStore.query (not for re-parsing).
🤖 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 `@packages/server/src/routers/activity-dedup.integration.test.ts`:
- Around line 24-28: The dateDaysAgo function recomputes new Date() on each call
which can cross UTC midnight and cause flakiness; fix by capturing a single base
timestamp once (e.g., const baseDate = new Date() or baseIso = new
Date().toISOString()) at the top of the test or before generating fixtures and
then change dateDaysAgo to derive dates from that base (either by accepting
baseDate/baseIso as a parameter or closing over the captured baseDate) so all
calls use the same reference moment when computing date strings.
In `@packages/server/src/routers/sleep.integration.test.ts`:
- Around line 91-99: The UTC ISO regex used in the "sleep.list returns
started_at in UTC ISO 8601 format" test (UTC_ISO_REGEX) is too strict and
rejects valid fractional-second timestamps like "...:ss.SSSZ"; update
UTC_ISO_REGEX to allow an optional fractional-second component (e.g. \.\d+ )
before the trailing Z so strings like "2023-01-01T12:00:00.123Z" match, keeping
the existing Date parsing check (new Date(row.started_at).getTime()) in the test
unchanged.
---
Outside diff comments:
In `@packages/server/src/repositories/clickhouse-sleep-repository.ts`:
- Around line 110-111: The rows returned by ActivitySensorStore.query are
already parsed by ClickHouseActivitySensorStore.query using the provided Zod
schema, so remove the redundant clickHouseSleepNightSchema.parse calls: in the
function that currently does return rows.map((row) =>
clickHouseSleepNightSchema.parse(row)) simply return rows (or rows as the
expected type), and in fetchLatestSleepNight remove the parsedRows =
rows.map(...parse...) step and use the already-parsed rows directly; keep
references to clickHouseSleepNightSchema only where the schema is passed into
ActivitySensorStore.query (not for re-parsing).
🪄 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: 0f90b04d-496e-477a-a0a3-e9cd3d5e8120
📒 Files selected for processing (22)
docs/production-incident-baseline.mddocs/superpowers/plans/2026-05-20-clickhouse-sleep-dashboard.mdpackages/server/src/repositories/anomaly-detection-repository.test.tspackages/server/src/repositories/anomaly-detection-repository.tspackages/server/src/repositories/clickhouse-sleep-repository.tspackages/server/src/repositories/predictions-repository.tspackages/server/src/repositories/sleep-repository.test.tspackages/server/src/routers/activity-dedup.integration.test.tspackages/server/src/routers/anomaly-detection.test.tspackages/server/src/routers/body-calendar-sleep-nutrition.test.tspackages/server/src/routers/healthspan-query.tspackages/server/src/routers/mobile-dashboard.integration.test.tspackages/server/src/routers/mobile-dashboard.test.tspackages/server/src/routers/mobile-dashboard.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/router-logic.integration.test.tspackages/server/src/routers/router.integration.test.tspackages/server/src/routers/sleep-need.tspackages/server/src/routers/sleep.integration.test.tssrc/db/test-helpers.ts
💤 Files with no reviewable changes (3)
- packages/server/src/routers/mobile-dashboard.test.ts
- packages/server/src/routers/mobile-dashboard.integration.test.ts
- packages/server/src/routers/router-logic.integration.test.ts
This was referenced Jul 26, 2026
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves runtime sleep reads from stale Postgres
fitness.v_sleepto ClickHouseanalytics.v_sleepacross sleep, dashboard, recovery, stress, prediction, healthspan, and personalization paths.Adds a shared ClickHouse sleep helper and a migration dropping
clickhouse.v_sleep/fitness.v_sleep, removes the canonical Postgres view artifact, and records the production incident baseline.Validated with
pnpm lint,pnpm tsc --noEmit,cd packages/server && pnpm tsc --noEmit,pnpm migrate, and targeted sleep tests.Known:
pnpm test:changedstill fails because existing unit/integration tests need conversion from mocked/refreshed Postgresv_sleepsetup to ClickHouse test-store sync.Summary by CodeRabbit
Bug Fixes
Chores / Migrations
Documentation
Tests