From fb5033545955a789a9a379c6ffd714812d9bff0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 23 Jul 2026 09:38:03 +0200 Subject: [PATCH] fix(insights): stop the pulse page from silently swapping to resting heart rate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #584. `/insights/pulse` computed hasRestingHr from RESTING_HEART_RATE presence and used it to swap the ENTIRE primary chart and the Coach read strip to that series whenever any resting row existed, while the title, stat strip, capture action, and "show all values" link kept saying PULSE. A user could see a resting-only chart under a page that claimed to be pulse throughout. The chart now always includes PULSE as its first series; when resting data exists, RESTING_HEART_RATE renders as a second, clearly-labelled series (the same multi-series pattern the blood-pressure page uses for systolic/diastolic) instead of replacing it. The Coach read strip stays pinned to PULSE. The page also drops the resting-calibrated target band it used to paint behind the chart — that band is calibrated to steady-state resting readings and would misreport an expected workout spike in raw PULSE data as "out of target"; the personal target belongs on the dedicated resting-pulse page against the clean resting series. --- .../insights/pulse/__tests__/page.test.tsx | 144 ++++++++++++++++++ src/app/insights/pulse/page.tsx | 91 ++++------- .../import/__tests__/unzip-export-xml.test.ts | 4 +- 3 files changed, 176 insertions(+), 63 deletions(-) create mode 100644 src/app/insights/pulse/__tests__/page.test.tsx diff --git a/src/app/insights/pulse/__tests__/page.test.tsx b/src/app/insights/pulse/__tests__/page.test.tsx new file mode 100644 index 000000000..13e2432ff --- /dev/null +++ b/src/app/insights/pulse/__tests__/page.test.tsx @@ -0,0 +1,144 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; + +/** + * v1.32.1 (issue #584) — `/insights/pulse` must chart the metric its own + * title, capture action, and stat strip claim. `hasRestingHr` used to swap + * the ENTIRE primary chart + Coach read strip to RESTING_HEART_RATE the + * moment any resting-HR row existed, so the page could show a resting-only + * series while every other surface (title, stat strip, capture action, + * "show all values") stayed labelled "Pulse" — a silent full swap. + * RESTING_HEART_RATE may now appear only as a second, clearly-labelled + * series alongside PULSE (the same multi-series pattern the blood-pressure + * page uses for systolic/diastolic) — never a swap. + * + * `SubPageShell` is stubbed to a probe recording its props (including the + * `children` array holding the chart element) so these assert the resolved + * chart/coach-strip props rather than rendered text — responsive classes + * have broken text-based queries in this repo. + */ + +const shellProps = vi.fn(); +vi.mock("@/components/insights/sub-page-shell", () => ({ + SubPageShell: (props: Record) => { + shellProps(props); + return null; + }, +})); + +vi.mock("@/lib/i18n/context", () => ({ + useTranslations: () => ({ + t: (key: string) => key, + }), +})); + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + isAuthenticated: true, + user: { dateOfBirth: null, gender: null, timezone: "UTC" }, + }), +})); + +const analytics = { + current: { summaries: {} as Record }, +}; +vi.mock("@/hooks/use-insights-analytics", () => ({ + useInsightsAnalytics: () => ({ + data: analytics.current, + isLoading: false, + isEmpty: false, + error: null, + refetch: () => {}, + }), +})); + +// `useInsightsLayoutPrefs` is the one real hook left that reaches +// `@tanstack/react-query`; stub `useQuery` so the page renders without a +// `QueryClientProvider` ancestor (mirrors the coach-page-launch-params test). +vi.mock("@tanstack/react-query", () => ({ + useQuery: () => ({ data: undefined, isLoading: false, error: null }), +})); + +import InsightsPulsPage from "../page"; + +interface CapturedShellProps { + captureType?: string; + showAllValuesType?: string; + coachReadStrip?: { props?: Record }; + children?: unknown; +} + +function renderPage( + summaries: Record, +): CapturedShellProps { + analytics.current = { summaries }; + renderToStaticMarkup(); + const calls = shellProps.mock.calls; + return calls[calls.length - 1][0] as CapturedShellProps; +} + +/** Locate the primary `` element among the page's + * children by its distinctive `types` array prop. Never rendered — the + * `SubPageShell` stub does not paint `children` — so this reads the plain + * React element object the page constructed. */ +function findChartElement( + children: unknown, +): { props: { types?: string[] } } | undefined { + const arr = Array.isArray(children) ? children : [children]; + return arr.find( + (el): el is { props: { types?: string[] } } => + Boolean(el) && + typeof el === "object" && + el !== null && + "props" in el && + Array.isArray((el as { props?: { types?: unknown } }).props?.types), + ); +} + +describe("/insights/pulse page — chart identity (issue #584)", () => { + beforeEach(() => { + shellProps.mockClear(); + }); + + it("charts PULSE as the primary (first) series even when resting-HR data exists", () => { + const props = renderPage({ + PULSE: { count: 50 }, + RESTING_HEART_RATE: { count: 30 }, + }); + const chart = findChartElement(props.children); + expect(chart).toBeDefined(); + expect(chart!.props.types).toEqual(["PULSE", "RESTING_HEART_RATE"]); + }); + + it("never swaps the chart to a RESTING_HEART_RATE-only series", () => { + const props = renderPage({ + PULSE: { count: 50 }, + RESTING_HEART_RATE: { count: 30 }, + }); + const chart = findChartElement(props.children); + expect(chart!.props.types).not.toEqual(["RESTING_HEART_RATE"]); + }); + + it("charts PULSE alone when there is no resting-HR data", () => { + const props = renderPage({ PULSE: { count: 50 } }); + const chart = findChartElement(props.children); + expect(chart!.props.types).toEqual(["PULSE"]); + }); + + it("keeps the Coach read strip pinned to PULSE regardless of resting-HR data", () => { + const props = renderPage({ + PULSE: { count: 50 }, + RESTING_HEART_RATE: { count: 30 }, + }); + expect(props.coachReadStrip?.props?.metricType).toBe("PULSE"); + }); + + it("keeps captureType and showAllValuesType pinned to PULSE", () => { + const props = renderPage({ + PULSE: { count: 50 }, + RESTING_HEART_RATE: { count: 30 }, + }); + expect(props.captureType).toBe("PULSE"); + expect(props.showAllValuesType).toBe("PULSE"); + }); +}); diff --git a/src/app/insights/pulse/page.tsx b/src/app/insights/pulse/page.tsx index 0b1b1d313..1f63ab298 100644 --- a/src/app/insights/pulse/page.tsx +++ b/src/app/insights/pulse/page.tsx @@ -35,20 +35,23 @@ const IntradayPulseChart = dynamic( })), { ssr: false, loading: () => }, ); -import { - getAgeFromDateOfBirth, - getPersonalizedPulseTarget, -} from "@/lib/analytics/pulse-targets"; /** * v1.4.25 W4 — `/insights/pulse`. * - * Routed Pulse sub-page. Renders the pulse chart with the personalized - * Karvonen-derived target band plus the per-section AI assessment. - * Note: `chartKey="pulse"` so the chart-cog can override the - * comparison-overlay independently from the dashboard pulse card; the - * MeasurementType filter is `PULSE` (the same field used elsewhere - * in the codebase). + * Routed Pulse sub-page. Renders the raw/workout-inclusive pulse chart + * plus the per-section AI assessment. Note: `chartKey="pulse"` so the + * chart-cog can override the comparison-overlay independently from the + * dashboard pulse card; the MeasurementType filter is `PULSE` (the same + * field used elsewhere in the codebase). + * + * v1.32.1 (issue #584) — this page no longer paints the personalized + * resting-pulse target band (`getPersonalizedPulseTarget()`, CDC/NCHS + * resting-pulse percentiles) behind the chart: that band is calibrated + * to steady-state resting readings and would misreport an expected + * workout spike in the raw PULSE series as "out of target". The + * personal target + its band belong on the dedicated + * `/insights/resting-pulse` page, against the clean resting series. * * Analytics fetch + empty-state branch consume `useInsightsAnalytics()` * + ``. VO₂ max is a cardio-fitness metric, so the page @@ -66,11 +69,16 @@ export default function InsightsPulsPage() { // dedicated cardio-fitness page. const hasVo2 = (analytics?.summaries?.VO2_MAX?.count ?? 0) > 0; const pulseSummary = analytics?.summaries?.PULSE ?? null; - // v1.15.12 A2 — the resting-pulse band is judged against - // RESTING_HEART_RATE (Apple's clean daily resting figure). When the - // user has resting rows the chart shows that series against the band; - // otherwise it charts raw heart rate WITHOUT the resting-band overlay - // (which would flag expected-high workout HR as "outside target"). + // v1.32.1 (issue #584) — this page is titled, captured, and stat-stripped + // as PULSE throughout; the chart must chart PULSE too. `hasRestingHr` used + // to swap the ENTIRE primary chart + Coach read to RESTING_HEART_RATE + // whenever any resting row existed, which silently showed a different + // metric than the page's own title/stat-strip/capture action claimed. + // RESTING_HEART_RATE now only ever appears as a second, clearly-labelled + // series alongside PULSE (same multi-series pattern the blood-pressure + // page uses for systolic/diastolic) — never a full swap. The dedicated + // `/insights/resting-pulse` page owns the resting-only view + its target + // band. const hasRestingHr = (analytics?.summaries?.RESTING_HEART_RATE?.count ?? 0) > 0; @@ -108,44 +116,6 @@ export default function InsightsPulsPage() { ); } - const pulseAge = getAgeFromDateOfBirth(user?.dateOfBirth ?? null); - const pulseTarget = getPersonalizedPulseTarget( - pulseAge, - (user?.gender as "MALE" | "FEMALE" | null | undefined) ?? null, - ); - const pulseBands = [ - { - min: 30, - max: pulseTarget.orangeMin, - color: "var(--destructive)", - opacity: 0.16, - }, - { - min: pulseTarget.orangeMin, - max: pulseTarget.greenMin, - color: "var(--warning)", - opacity: 0.18, - }, - { - min: pulseTarget.greenMin, - max: pulseTarget.greenMax, - color: "var(--success)", - opacity: 0.2, - }, - { - min: pulseTarget.greenMax, - max: pulseTarget.orangeMax, - color: "var(--warning)", - opacity: 0.18, - }, - { - min: pulseTarget.orangeMax, - max: 220, - color: "var(--destructive)", - opacity: 0.16, - }, - ].filter((band) => band.max > band.min); - return ( } coachReadStrip={ - + } diversityNudge={ { Readable.from([Buffer.from("hello"), Buffer.from("world")]), createByteCap(6), async function drain(source) { - for await (const _chunk of source) { - /* drain */ + for await (const chunk of source) { + void chunk; } }, ),