From a165ff9bb6db39076f9484665ac62e85c30cd457 Mon Sep 17 00:00:00 2001 From: Antiheld86 <4201585+Antiheld86@users.noreply.github.com> Date: Wed, 2 Sep 2026 13:38:46 +0000 Subject: [PATCH] Show the HRV tile for an account whose HRV is RMSSD MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HRV lives under two measurement 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 bridge pushing a watch that reports it that way — write nightly RMSSD as `HRV_RMSSD`. Every HRV surface unions the two: `/insights/hrv` through `fallbackMeasurementType`, the Coach through `COACH_SOURCE_MEASUREMENT_TYPES.hrv`, the MCP reads through `withHrvFallback`. `sub-page-metric.ts` even says listing both is what keeps "the tab, pill, and dashboard tile lit whichever source the user has". The dashboard tile did not. It read SDNN alone, so an RMSSD-only account got a Settings → Dashboard toggle that turned on and rendered nothing, with no explanation, while the insights page charted the same readings — which is why it reads as a broken layout rather than a missing series. `pickHrvSummary` picks the series with the same rule `withHrvFallback` uses: the primary wins whenever it has any rows, the fallback only when it has none. Never a merge — the two share `ms` but measure different things, so blending them would invent a number no device reported. The caller gets the winning type back, because the freshness caption has to read the series on show and the label has to name the measure when it is not SDNN (`· RMSSD`, as the insights chart title already does). Second, less visible half: `HRV_RMSSD` is recovery-owned in `METRIC_STATUS_MODULE_OWNERS`, but the dashboard snapshot gates off `SUMMARY_TYPE_MODULE`, a different map, and that one did not list it. The omission was inert while the tile read SDNN alone; adding the fallback without it would hand a recovery-off account a tile built from recovery-owned rows — the exact hazard the owner map documents. The entry goes in beside the four sibling recovery types, so the gate stays server side and the client keeps no competing copy. --- CHANGELOG.md | 22 +++ .../dashboard-hrv-rmssd-fallback.test.ts | 167 ++++++++++++++++++ src/app/page-client.tsx | 28 ++- src/components/dashboard/dashboard-gates.ts | 53 ++++++ src/lib/dashboard/widget-modules.ts | 10 ++ 5 files changed, 277 insertions(+), 3 deletions(-) create mode 100644 src/app/__tests__/dashboard-hrv-rmssd-fallback.test.ts 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