fix(insights): stop the pulse page from silently swapping to resting heart rate

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.
This commit is contained in:
Marc-André Bombeck committed 2026-07-23 09:38:03 +02:00
1 parent 79843f8770
commit fb50335459
3 files changed
+176 -63

No files matched your search

@@ -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<string, unknown>) => {
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<string, { count: number }> },
};
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<string, unknown> };
children?: unknown;
}
function renderPage(
summaries: Record<string, { count: number }>,
): CapturedShellProps {
analytics.current = { summaries };
renderToStaticMarkup(<InsightsPulsPage />);
const calls = shellProps.mock.calls;
return calls[calls.length - 1][0] as CapturedShellProps;
}
/** Locate the primary `<HealthChartDynamic>` 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");
});
});
+30 -61
View File
@@ -35,20 +35,23 @@ const IntradayPulseChart = dynamic(
})),
{ ssr: false, loading: () => <ChartSkeleton /> },
);
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()`
* + `<MetricEmptyState>`. 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 (
<SubPageShell
title={t("insights.pulseSectionTitle")}
@@ -161,11 +131,7 @@ export default function InsightsPulsPage() {
/>
}
coachReadStrip={
<CoachReadStrip
metricType={hasRestingHr ? "RESTING_HEART_RATE" : "PULSE"}
unit="bpm"
fractionDigits={0}
/>
<CoachReadStrip metricType="PULSE" unit="bpm" fractionDigits={0} />
}
diversityNudge={
<MeasurementDiversityNudge
@@ -180,12 +146,15 @@ export default function InsightsPulsPage() {
>
<HealthChartDynamic
chartKey="pulse"
types={hasRestingHr ? ["RESTING_HEART_RATE"] : ["PULSE"]}
types={hasRestingHr ? ["PULSE", "RESTING_HEART_RATE"] : ["PULSE"]}
title={t("charts.pulse")}
titleIcon={Heart}
colors={["var(--success)"]}
colors={
hasRestingHr
? ["var(--success)", "var(--destructive)"]
: ["var(--success)"]
}
unit="bpm"
valueBands={hasRestingHr ? pulseBands : undefined}
compareBaseline={compareBaseline}
userTimezone={user?.timezone}
onVisibleStats={onVisibleStats}
@@ -177,8 +177,8 @@ describe("createByteCap", () => {
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;
}
},
),