diff --git a/CHANGELOG.md b/CHANGELOG.md index da9672701..62f88c460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## [Unreleased] + +### Fixed + +- **The heart-rate-variability tile never appeared on the dashboard for + people whose HRV comes from a ring or a strap.** Turning the widget on + under Settings → Dashboard did nothing and said nothing, while the same + readings charted perfectly on the HRV page — which made it look as though + the layout itself was broken. HRV is stored two ways: Apple Health, Fitbit + and Google Health record SDNN, whereas Oura, Polar and WHOOP record nightly + RMSSD, as does anything pushed in through a bridge from a watch that + reports it that way. Every other place HRV appears already accepted both; + the dashboard tile looked only for SDNN, so an account that had only RMSSD + had nothing for the tile to show. It now uses whichever series you actually + have, and says "RMSSD" beside the name when that is the one on screen, so a + reading of 40 is not mistaken for a collapsed SDNN figure. An account with + both keeps seeing SDNN, and the two are never blended — they are different + measures that happen to share a unit. Turning the Recovery module off still + takes the RMSSD reading off the dashboard, which it should have done all + along: that series belongs to Recovery everywhere else in the app, and the + dashboard was the one place not checking. + ## [1.38.6] — 2026-09-03 Blood glucose can be shown in mmol/L, which it never could before, and the diff --git a/src/app/__tests__/dashboard-hrv-rmssd-fallback.test.ts b/src/app/__tests__/dashboard-hrv-rmssd-fallback.test.ts new file mode 100644 index 000000000..f9be40f77 --- /dev/null +++ b/src/app/__tests__/dashboard-hrv-rmssd-fallback.test.ts @@ -0,0 +1,167 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { pickHrvSummary } from "@/components/dashboard/dashboard-gates"; +import { + SUMMARY_TYPE_MODULE, + disabledSummaryTypes, +} from "@/lib/dashboard/widget-modules"; +import { moduleForMeasurementType } from "@/lib/modules/measurement-scope"; +import type { DataSummary } from "@/lib/analytics/trends"; + +/** + * The dashboard HRV tile must accept either HRV type. + * + * HRV is stored under two measurement types that are not the same measure: + * SDNN (`HEART_RATE_VARIABILITY`, Apple / Fitbit / Google Health) and nightly + * RMSSD (`HRV_RMSSD`, Oura / Polar / WHOOP, and a bridge pushing a Garmin-style + * feed). Every other HRV surface already unions them — `/insights/hrv` via + * `fallbackMeasurementType`, the Coach via `COACH_SOURCE_MEASUREMENT_TYPES.hrv`, + * the MCP reads via `withHrvFallback` — and `sub-page-metric.ts` states that + * listing both is what keeps "the tab, pill, and dashboard tile lit whichever + * source the user has". + * + * The tile did not. It read `summaries.HEART_RATE_VARIABILITY` alone, so an + * RMSSD-only account got a Settings → Dashboard toggle that turned on and + * rendered nothing, with no message, while `/insights/hrv` charted the same + * readings. That asymmetry is what this file pins. + * + * Mutation check: return the fallback unconditionally and the "SDNN wins" + * cases go red; drop the fallback arm and the RMSSD cases go red; return the + * fallback's TYPE while returning the primary's summary and the freshness + * case goes red. + */ +function summary(count: number, latest: number): DataSummary { + return { + count, + latest, + min: latest, + max: latest, + mean: latest, + median: latest, + avg7: latest, + avg30: latest, + slope30: 0, + } as unknown as DataSummary; +} + +describe("pickHrvSummary", () => { + it("takes SDNN when the account has it", () => { + const picked = pickHrvSummary({ + HEART_RATE_VARIABILITY: summary(12, 48), + }); + expect(picked.type).toBe("HEART_RATE_VARIABILITY"); + expect(picked.summary?.latest).toBe(48); + }); + + it("falls back to RMSSD for a ring / strap account", () => { + // The regression itself: pre-fix this returned undefined, `hasHrv` was + // false, and the tile never rendered however the widget was toggled. + const picked = pickHrvSummary({ HRV_RMSSD: summary(30, 42) }); + expect(picked.type).toBe("HRV_RMSSD"); + expect(picked.summary?.latest).toBe(42); + }); + + it("prefers SDNN when the account carries both series", () => { + // The case a naive `sdnn ?? rmssd` written the wrong way round breaks. + // Both are stored in ms but they measure different things, so the tile + // shows one series, never a blend, and SDNN is the primary. + const picked = pickHrvSummary({ + HEART_RATE_VARIABILITY: summary(9, 55), + HRV_RMSSD: summary(30, 42), + }); + expect(picked.type).toBe("HEART_RATE_VARIABILITY"); + expect(picked.summary?.latest).toBe(55); + }); + + it("treats a present-but-empty SDNN summary as absent", () => { + // The slice can carry a zero-count summary for a type the account has no + // rows for. Keying off presence rather than `count` would pin the tile to + // an empty SDNN series and re-break the ring user. + const picked = pickHrvSummary({ + HEART_RATE_VARIABILITY: summary(0, 0), + HRV_RMSSD: summary(30, 42), + }); + expect(picked.type).toBe("HRV_RMSSD"); + expect(picked.summary?.count).toBe(30); + }); + + it("reports no series, on the primary type, when the account has neither", () => { + // The tile is hidden either way; reporting the fallback type here would + // make an account with no HRV at all look like a ring user to the label + // and the freshness lookup. + for (const input of [undefined, {}, { HRV_RMSSD: summary(0, 0) }]) { + const picked = pickHrvSummary(input); + expect((picked.summary?.count ?? 0) > 0).toBe(false); + expect(picked.type).toBe("HEART_RATE_VARIABILITY"); + } + }); + + it("does not gate on the recovery module itself", () => { + // The gate belongs server-side (see the sibling test below), so a second + // one here would be a competing source of truth. Scoped to the function + // body: the docblock above it legitimately explains where gating lives. + const src = readFileSync( + join(process.cwd(), "src/components/dashboard/dashboard-gates.ts"), + "utf8", + ); + const body = src.slice(src.indexOf("export function pickHrvSummary(")); + expect(body).not.toMatch(/modules|recovery|isModuleEnabled/i); + }); +}); + +describe("the RMSSD fallback cannot leak recovery-module data", () => { + it("drops HRV_RMSSD from the snapshot summaries when recovery is off", () => { + // The half of this fix that is easy to miss. `HRV_RMSSD` is recovery-owned + // in `METRIC_STATUS_MODULE_OWNERS`, which governs the metric-status and + // MCP reads — but the dashboard snapshot gates off `SUMMARY_TYPE_MODULE`, + // a DIFFERENT map, and that one did not list it. Inert while the tile read + // SDNN alone; the moment the tile falls back to RMSSD it would show a + // recovery-off account a tile built from recovery-owned rows. + expect(disabledSummaryTypes({ recovery: false })).toContain("HRV_RMSSD"); + // SDNN is a plain vital and stays ungated — turning recovery off must not + // take the ordinary HRV tile away from an Apple / Fitbit account. + expect(disabledSummaryTypes({ recovery: false })).not.toContain( + "HEART_RATE_VARIABILITY", + ); + // And nothing is dropped when the module is on. + expect(disabledSummaryTypes({ recovery: true }).size).toBe(0); + }); + + it("keeps the fallback aligned with the metric-status owner map", () => { + // The two maps answer different questions and are allowed to differ, but + // not for THIS type: a fallback that serves data the primary path refuses + // is exactly what `METRIC_STATUS_MODULE_OWNERS` documents as the hazard. + expect(moduleForMeasurementType("HRV_RMSSD")).toBe("recovery"); + expect(SUMMARY_TYPE_MODULE.HRV_RMSSD).toBe("recovery"); + }); +}); + +describe("the tile wiring follows the chosen series", () => { + const PAGE = readFileSync( + join(process.cwd(), "src/app/page-client.tsx"), + "utf8", + ); + + it("sources the HRV summary from the helper, not a bare SDNN lookup", () => { + expect(PAGE).toContain("pickHrvSummary("); + // The exact line the bug lived on. If it comes back, so does the bug. + expect(PAGE).not.toContain( + "const hrvSummary = data?.summaries?.HEART_RATE_VARIABILITY", + ); + }); + + it("reads freshness off the type actually shown", () => { + // A hardcoded SDNN lookup here captions a live RMSSD tile as stale. + expect(PAGE).toContain("staleDays={tileStaleDays(hrvType)}"); + expect(PAGE).not.toContain('tileStaleDays("HEART_RATE_VARIABILITY")'); + }); + + it("names the measure when the RMSSD series is the one on show", () => { + // Mirrors how `/insights/hrv` appends `· RMSSD` to its chart title, so a + // 20-60 ms RMSSD reading is not mistaken for a collapsed SDNN one. + expect(PAGE).toContain('hrvType === "HRV_RMSSD"'); + expect(PAGE).toContain("· RMSSD"); + }); +}); diff --git a/src/app/page-client.tsx b/src/app/page-client.tsx index 408330c7e..72aecb9d6 100644 --- a/src/app/page-client.tsx +++ b/src/app/page-client.tsx @@ -173,6 +173,7 @@ import { resolveDashboardFirstPaintGate, resolveConfiguredTileCount, resolveChartRowPlaceholderCount, + pickHrvSummary, } from "@/components/dashboard/dashboard-gates"; /** @@ -490,7 +491,16 @@ export default function DashboardPageClient({ // the same summaries slice (`computeSummariesSlice` iterates the full // measurement-type enum), so no backend change was needed; the strip // tiles below self-gate on `count > 0`. - const hrvSummary = data?.summaries?.HEART_RATE_VARIABILITY; + // HRV is stored under two types and the tile has to accept either: SDNN + // (`HEART_RATE_VARIABILITY`, Apple / Fitbit / Google Health) or nightly + // RMSSD (`HRV_RMSSD`, Oura / Polar / WHOOP and bridge pushes). Reading + // SDNN alone left a ring / strap account with a Settings toggle that + // rendered nothing while `/insights/hrv` charted the same data. The + // helper keeps the union in one place and hands back which type won, so + // the freshness caption and the label below follow the series shown. + const { summary: hrvSummary, type: hrvType } = pickHrvSummary( + data?.summaries, + ); const spo2Summary = data?.summaries?.OXYGEN_SATURATION; const respRateSummary = data?.summaries?.RESPIRATORY_RATE; const wristTempSummary = data?.summaries?.WRIST_TEMPERATURE; @@ -1309,7 +1319,16 @@ export default function DashboardPageClient({ node: ( ), }); diff --git a/src/components/dashboard/dashboard-gates.ts b/src/components/dashboard/dashboard-gates.ts index af861d41d..32633ebbd 100644 --- a/src/components/dashboard/dashboard-gates.ts +++ b/src/components/dashboard/dashboard-gates.ts @@ -6,6 +6,7 @@ * live in one cohesive module. `page.tsx` re-exports them so the existing * `../page` test imports stay valid. */ +import type { DataSummary } from "@/lib/analytics/trends"; import type { DashboardLayout } from "@/lib/dashboard-layout"; /** @@ -129,3 +130,55 @@ export function resolveChartRowPlaceholderCount( } return count; } + +/** The two measurement types the HRV surface reads, primary first. */ +export type HrvSummaryType = "HEART_RATE_VARIABILITY" | "HRV_RMSSD"; + +/** + * Which HRV series backs the dashboard tile. + * + * HRV is stored under two types and they are not the same measure: Apple + * Health / Fitbit / Google Health write SDNN as `HEART_RATE_VARIABILITY`, + * while Oura / Polar / WHOOP — and a Garmin-style feed pushed in through a + * bridge — write nightly RMSSD as `HRV_RMSSD`. Every other HRV surface + * already unions the two: `/insights/hrv` passes `fallbackMeasurementType` + * (`app/insights/hrv/page.tsx`), the Coach resolves both + * (`COACH_SOURCE_MEASUREMENT_TYPES.hrv`), and the MCP reads swap through + * `withHrvFallback` (`lib/mcp/rich-reads.ts`). The catalogue entry in + * `lib/insights/sub-page-metric.ts` even says listing both is what "keeps the + * tab, pill, and dashboard tile lit whichever source the user has" — but the + * tile read SDNN alone, so an RMSSD-only account got a Settings toggle that + * silently rendered nothing while the insights page charted the data. + * + * Same rule as `withHrvFallback`: the primary wins whenever it has ANY rows, + * and the fallback is taken only when it has none. Deliberately not a merge — + * SDNN and RMSSD share `ms` but measure different things, so blending them + * would invent a number no device reported. That is why the caller is handed + * the winning `type` back: the freshness caption has to read the series being + * shown, and the label has to name the measure when it is not SDNN. + * + * Module gating belongs upstream and must NOT be repeated here. `HRV_RMSSD` + * is a `recovery`-owned type, so `SUMMARY_TYPE_MODULE` carries it and + * `gateSummariesByModules()` strips it out of `tiles.summaries` before the + * client sees anything — a `recovery`-off account has no key to fall back to. + * That entry was added WITH this helper: while the tile read SDNN alone the + * omission was inert, and a client-side gate here would be a second source of + * truth that drifts from the server's. + */ +export function pickHrvSummary( + summaries: Record | undefined, +): { summary: DataSummary | null; type: HrvSummaryType } { + const primary = summaries?.HEART_RATE_VARIABILITY; + if ((primary?.count ?? 0) > 0) { + return { summary: primary ?? null, type: "HEART_RATE_VARIABILITY" }; + } + const fallback = summaries?.HRV_RMSSD; + if ((fallback?.count ?? 0) > 0) { + return { summary: fallback ?? null, type: "HRV_RMSSD" }; + } + // Neither has rows. Report the primary type so the caller's stale-days + // lookup and label stay on SDNN for the empty case — the tile is hidden + // either way, and reporting the fallback here would make an account with + // no HRV at all look like a ring user. + return { summary: primary ?? null, type: "HEART_RATE_VARIABILITY" }; +} diff --git a/src/lib/dashboard/widget-modules.ts b/src/lib/dashboard/widget-modules.ts index 793d33c91..375d28db5 100644 --- a/src/lib/dashboard/widget-modules.ts +++ b/src/lib/dashboard/widget-modules.ts @@ -68,6 +68,16 @@ export const SUMMARY_TYPE_MODULE: Partial> = { SIX_MINUTE_WALK_DISTANCE: "recovery", STAIR_ASCENT_SPEED: "recovery", STAIR_DESCENT_SPEED: "recovery", + // The HRV fallback series. `METRIC_STATUS_MODULE_OWNERS` already owns this + // type for the metric-status / MCP reads, for the reason stated there: a + // fallback path must not serve the recovery data the primary type refuses. + // The dashboard snapshot gates off THIS map instead, and the entry was + // missing — harmless only while the dashboard HRV tile read SDNN alone. + // Now that the tile falls back to RMSSD (`pickHrvSummary`), the omission + // would hand a recovery-off account a tile built from recovery-owned rows. + // `HEART_RATE_VARIABILITY` stays deliberately ungated: SDNN is a plain + // vital, and it is what the tile shows whenever the account has it. + HRV_RMSSD: "recovery", // Per-night breathing-disturbance index is a sleep-page signal. BREATHING_DISTURBANCES: "sleep", // v1.29 — fluid-intake dashboard tile summary, derived server-side from