diff --git a/messages/de.json b/messages/de.json index d7655e6fd..951aa164c 100644 --- a/messages/de.json +++ b/messages/de.json @@ -2860,6 +2860,13 @@ "scoresBack": "Zurück zu Insights", "ecgDescription": "Einkanal-EKG-Aufzeichnungen deines Geräts, jeweils mit dem Ergebnis des Geräts. HealthLog zeigt die Kurve und die Aufzeichnung – die Kurve wird nicht interpretiert." }, + "units": { + "steps": "Schritte", + "flights": "Etagen", + "falls": "Stürze", + "years": "Jahre", + "breathingEvents": "Ereignisse" + }, "sleep": { "title": "Schlaf", "description": "Phasen-Aufschlüsselung und nächtliche Dauer – gezogen aus Apple Health.", diff --git a/messages/en.json b/messages/en.json index 3d7a6aea7..ead59de68 100644 --- a/messages/en.json +++ b/messages/en.json @@ -2860,6 +2860,13 @@ "scoresBack": "Back to insights", "ecgDescription": "Single-lead ECG recordings from your device, each with the device's own result. HealthLog shows the trace and the recording — it does not interpret the waveform." }, + "units": { + "steps": "steps", + "flights": "flights", + "falls": "falls", + "years": "years", + "breathingEvents": "events" + }, "sleep": { "title": "Sleep", "description": "Stage breakdown and nightly duration, drawn from Apple Health.", diff --git a/messages/es.json b/messages/es.json index cd4c91f63..088243721 100644 --- a/messages/es.json +++ b/messages/es.json @@ -2860,6 +2860,13 @@ "scoresBack": "Volver a Insights", "ecgDescription": "Registros de ECG de una derivación de tu dispositivo, cada uno con el resultado del propio dispositivo. HealthLog muestra el trazado y el registro, pero no interpreta la onda." }, + "units": { + "steps": "pasos", + "flights": "pisos", + "falls": "caídas", + "years": "años", + "breathingEvents": "eventos" + }, "sleep": { "title": "Sueño", "description": "Desglose por fases y duración nocturna, tomados de Apple Health.", diff --git a/messages/fr.json b/messages/fr.json index edcb3446b..ac626a5e9 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -2860,6 +2860,13 @@ "scoresBack": "Retour aux Insights", "ecgDescription": "Enregistrements ECG à dérivation unique de votre appareil, chacun accompagné du résultat de l'appareil. HealthLog affiche le tracé et l'enregistrement, mais n'interprète pas la courbe." }, + "units": { + "steps": "pas", + "flights": "étages", + "falls": "chutes", + "years": "ans", + "breathingEvents": "événements" + }, "sleep": { "title": "Sommeil", "description": "Répartition des phases et durée nocturne, issues d’Apple Health.", diff --git a/messages/it.json b/messages/it.json index 87f9dfc30..882ca79f3 100644 --- a/messages/it.json +++ b/messages/it.json @@ -2860,6 +2860,13 @@ "scoresBack": "Torna a Insights", "ecgDescription": "Registrazioni ECG a singola derivazione dal tuo dispositivo, ciascuna con il risultato del dispositivo. HealthLog mostra il tracciato e la registrazione, ma non interpreta l'onda." }, + "units": { + "steps": "passi", + "flights": "piani", + "falls": "cadute", + "years": "anni", + "breathingEvents": "eventi" + }, "sleep": { "title": "Sonno", "description": "Suddivisione per fasi e durata notturna, da Apple Health.", diff --git a/messages/pl.json b/messages/pl.json index 5c64b542d..1e1f83bcc 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -2860,6 +2860,13 @@ "scoresBack": "Powrót do Insights", "ecgDescription": "Zapisy EKG jednoodprowadzeniowe z Twojego urządzenia, każdy z wynikiem samego urządzenia. HealthLog pokazuje przebieg i zapis — nie interpretuje krzywej." }, + "units": { + "steps": "kroki", + "flights": "piętra", + "falls": "upadki", + "years": "lata", + "breathingEvents": "zdarzenia" + }, "sleep": { "title": "Sen", "description": "Podział na fazy i nocna długość snu, dane z Apple Health.", diff --git a/src/__tests__/insights-unit-literal-guard.test.ts b/src/__tests__/insights-unit-literal-guard.test.ts new file mode 100644 index 000000000..d65de1b44 --- /dev/null +++ b/src/__tests__/insights-unit-literal-guard.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect } from "vitest"; +import { readFileSync, readdirSync, statSync } from "node:fs"; +import { join } from "node:path"; + +/** + * GUARD — no insights page passes an English word as a unit. + * + * A German reader was shown "13,387.6 steps" because `/insights/steps` + * handed the scaffold the literal string `steps`. Five of the eighteen unit + * literals under `src/app/insights` were English words: steps, years, + * flights, falls, and a "count" that was a poor label in any language. The + * other thirteen are language-neutral symbols — bpm, %, mmHg, m/s and their + * kin read the same in every locale, and "translating" one would be the + * opposite mistake. + * + * So the rule is not "route every unit through i18n", it is: a unit literal + * on an insights page must be a SYMBOL. A word travels as an i18n key + * (`unitKey`) and is resolved by the scaffold — and that key must resolve in + * every locale, which the `t()`-call-site coverage sweep cannot see because + * the key is a prop, not a call argument. Both halves are asserted here. + */ + +const LOCALES = ["en", "de", "es", "fr", "it", "pl"] as const; +const INSIGHTS_PAGES_DIR = join(process.cwd(), "src/app/insights"); + +/** + * Unit strings that are language-neutral and therefore legitimately appear + * as literals. Every entry is a symbol, an SI abbreviation, or a composed + * one — nothing here changes when the reader's locale does. The empty string + * is the "this metric has no unit" case (pain score, waist-to-height ratio). + */ +const LANGUAGE_NEUTRAL_UNITS = new Set([ + "", + "%", + "/10", + "bpm", + "breaths/min", + "dBA", + "kcal", + "kg/m²", + "m", + "m/s", + "mL/(kg·min)", + "min", + "mmHg", + "ms", +]); + +/** Every `page.tsx` beneath `src/app/insights`, recursively. */ +function insightsPageFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const full = join(dir, entry); + if (statSync(full).isDirectory()) { + out.push(...insightsPageFiles(full)); + } else if (entry === "page.tsx") { + out.push(full); + } + } + return out; +} + +/** + * Every `unit="…"` / `yAxisUnit="…"` STRING literal in a source file. + * Expression props (`unit={glucoseUnit}`) resolve at runtime from a unit + * preference or an i18n call and are out of scope by construction. + */ +function unitLiterals(source: string): string[] { + return [...source.matchAll(/\b(?:yAxisUnit|unit)="([^"]*)"/g)].map( + (match) => match[1], + ); +} + +/** Every `unitKey="…"` literal in a source file. */ +function unitKeys(source: string): string[] { + return [...source.matchAll(/\bunitKey="([^"]*)"/g)].map((match) => match[1]); +} + +/** Resolve a dotted key against a loaded message bundle. */ +function resolve(bundle: unknown, key: string): unknown { + let node = bundle; + for (const segment of key.split(".")) { + if (typeof node !== "object" || node === null) return undefined; + node = (node as Record)[segment]; + } + return node; +} + +describe("insights unit-literal guard", () => { + const files = insightsPageFiles(INSIGHTS_PAGES_DIR); + const sources = files.map((file) => ({ + path: file.slice(file.indexOf("src/")), + source: readFileSync(file, "utf8"), + })); + + it("finds the insights pages to scan", () => { + // A broken walk would make every assertion below vacuously true. + expect(files.length).toBeGreaterThan(30); + }); + + it("scans real content — the known symbols are actually present", () => { + const seen = new Set(sources.flatMap((f) => unitLiterals(f.source))); + // If the matcher silently stopped matching, this fails rather than the + // offender list below going quietly empty. + for (const symbol of ["bpm", "mmHg", "%", "m/s"]) { + expect(seen.has(symbol), `expected to see unit="${symbol}"`).toBe(true); + } + }); + + it("passes only language-neutral unit symbols as literals", () => { + const offenders: string[] = []; + for (const { path, source } of sources) { + for (const literal of unitLiterals(source)) { + if (!LANGUAGE_NEUTRAL_UNITS.has(literal)) { + // A word-shaped unit belongs in `messages/*.json` behind `unitKey`. + offenders.push(`${path}: unit="${literal}"`); + } + } + } + expect(offenders).toEqual([]); + }); + + it("resolves every unitKey in all six locales", () => { + const keys = [...new Set(sources.flatMap((f) => unitKeys(f.source)))]; + // The five words the pages actually carry. + expect(keys.length).toBeGreaterThanOrEqual(5); + + const missing: string[] = []; + for (const locale of LOCALES) { + const bundle = JSON.parse( + readFileSync(join(process.cwd(), `messages/${locale}.json`), "utf8"), + ); + for (const key of keys) { + const value = resolve(bundle, key); + if (typeof value !== "string" || value.trim() === "") { + missing.push(`${locale}: ${key}`); + } + } + } + expect(missing).toEqual([]); + }); +}); diff --git a/src/app/insights/breathing-disturbances/page.tsx b/src/app/insights/breathing-disturbances/page.tsx index 51324fe2a..c9d62cb30 100644 --- a/src/app/insights/breathing-disturbances/page.tsx +++ b/src/app/insights/breathing-disturbances/page.tsx @@ -21,8 +21,7 @@ export default function InsightsBreathingDisturbancesPage() { i18nPrefix="insights.breathingDisturbances" explainerMetric="breathingDisturbances" color="var(--info)" - unit="count" - yAxisUnit="count" + unitKey="insights.units.breathingEvents" emptyStateIcon={} emptyStateCtaType={null} coachPrefill="I haven't logged any sleep-breathing-disturbance data yet — what does this metric tell me about my health, and how do I improve it?" diff --git a/src/app/insights/falls/page.tsx b/src/app/insights/falls/page.tsx index 9cd6ff193..157383c52 100644 --- a/src/app/insights/falls/page.tsx +++ b/src/app/insights/falls/page.tsx @@ -20,8 +20,7 @@ export default function InsightsFallsPage() { i18nPrefix="insights.falls" explainerMetric="falls" color="var(--destructive)" - unit="falls" - yAxisUnit="falls" + unitKey="insights.units.falls" emptyStateIcon={} emptyStateCtaType={null} coachPrefill="I haven't logged any fall data yet — what does this metric tell me about my health, and how do I lower my fall risk?" diff --git a/src/app/insights/flights-climbed/page.tsx b/src/app/insights/flights-climbed/page.tsx index 8486cfbf3..c76a259f0 100644 --- a/src/app/insights/flights-climbed/page.tsx +++ b/src/app/insights/flights-climbed/page.tsx @@ -22,8 +22,7 @@ export default function InsightsStockwerkePage() { i18nPrefix="insights.flightsClimbed" explainerMetric="flightsClimbed" color="var(--success)" - unit="flights" - yAxisUnit="flights" + unitKey="insights.units.flights" emptyStateIcon={} emptyStateCtaType={null} coachPrefill="I haven't logged any flights climbed yet — what does this metric tell me about my health, and how do I improve it?" diff --git a/src/app/insights/steps/page.tsx b/src/app/insights/steps/page.tsx index 134694d32..cbd8015d3 100644 --- a/src/app/insights/steps/page.tsx +++ b/src/app/insights/steps/page.tsx @@ -27,8 +27,7 @@ export default function InsightsStepsPage() { explainerMetric="steps" statIcon={Footprints} color="var(--success)" - unit="steps" - yAxisUnit="steps" + unitKey="insights.units.steps" emptyStateIcon={} emptyStateCtaType={null} captureType="ACTIVITY_STEPS" diff --git a/src/app/insights/vascular-age/page.tsx b/src/app/insights/vascular-age/page.tsx index dfc04b043..82edd06fc 100644 --- a/src/app/insights/vascular-age/page.tsx +++ b/src/app/insights/vascular-age/page.tsx @@ -22,8 +22,7 @@ export default function InsightsGefaessalterPage() { i18nPrefix="insights.vascularAge" explainerMetric="vascularAge" color="var(--chart-3)" - unit="years" - yAxisUnit="years" + unitKey="insights.units.years" emptyStateIcon={} emptyStateCtaType={null} coachPrefill="I haven't logged any vascular age yet — what does this metric tell me about my health, and how do I improve it?" diff --git a/src/components/charts/health-chart.tsx b/src/components/charts/health-chart.tsx index 26958e593..ea6df35be 100644 --- a/src/components/charts/health-chart.tsx +++ b/src/components/charts/health-chart.tsx @@ -1337,7 +1337,7 @@ export function HealthChart({ .filter((value): value is number => typeof value === "number") .filter((value) => Number.isFinite(value)); - return computePaddedYDomain(values); + return computePaddedYDomain(values, types); }, [ chartDataWithCompare, effectiveCompareBaseline, diff --git a/src/components/insights/__tests__/integer-metric-precision-guard.test.tsx b/src/components/insights/__tests__/integer-metric-precision-guard.test.tsx new file mode 100644 index 000000000..a6da4062a --- /dev/null +++ b/src/components/insights/__tests__/integer-metric-precision-guard.test.tsx @@ -0,0 +1,174 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderToStaticMarkup } from "react-dom/server"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { I18nProvider } from "@/lib/i18n/context"; +import { + INTEGER_VALUED_MEASUREMENT_TYPES, + metricFractionDigits, + DEFAULT_METRIC_FRACTION_DIGITS, +} from "@/lib/measurements/value-domain"; +import type { CoachReadStripData } from "@/lib/insights/derived/coach-read-shape"; + +/** + * GUARD — an integer-valued metric never renders a fractional part. + * + * The step sub-page read "MIN 104.0 steps" and "your usual range is + * …–13,387.6 steps". Precision was a per-page prop defaulting to one + * decimal: the pulse page remembered to pass 0, the step page did not, and + * a tenth of a step was rendered. Every page remembering this individually + * IS the defect — so the assertions below pin that the METRIC answers, and + * that a page which passes nothing still renders a count as a count. + * + * Both surfaces on the reported card are covered: the Min/Max stat strip and + * the coach-read strip underneath it. + */ + +vi.mock("next/dynamic", () => ({ + default: () => { + const Stub = () =>
; + Stub.displayName = "HealthChartStub"; + return Stub; + }, +})); + +vi.mock("next/navigation", () => ({ + usePathname: () => "/insights/steps", +})); + +vi.mock("@/hooks/use-auth", () => ({ + useAuth: () => ({ + user: { timezone: "UTC", dateOfBirth: null, gender: null }, + isAuthenticated: true, + }), +})); + +vi.mock("@/hooks/use-insights-layout-prefs", () => ({ + useInsightsLayoutPrefs: () => ({ layout: null, compareBaseline: false }), +})); + +// The coach-read strip paints nothing until it is client-mounted (React +// #418). Force the mounted branch so the band sentence reaches the markup. +vi.mock("@/hooks/use-mounted", () => ({ useMounted: () => true })); + +const analyticsMock = vi.fn(); +vi.mock("@/hooks/use-insights-analytics", () => ({ + useInsightsAnalytics: () => analyticsMock(), +})); + +const coachReadMock = vi.fn(); +vi.mock("@tanstack/react-query", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useQuery: () => coachReadMock() }; +}); + +import { CoachReadStrip } from "../derived/coach-read-strip"; +import InsightsStepsPage from "@/app/insights/steps/page"; + +function render(node: React.ReactNode, locale: "en" | "de" = "de") { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, staleTime: Infinity } }, + }); + return renderToStaticMarkup( + + {node} + , + ); +} + +/** The band and the reading the reporter saw, to the digit. */ +const REPORTED_BAND: CoachReadStripData = { + learning: false, + baseline: { + low: 104, + high: 13387.6, + latest: 8213.4, + placement: "within", + }, + driver: null, +} as unknown as CoachReadStripData; + +/** + * A German-locale decimal on a count: "104,0", "13.387,6". The Latin-1 + * comma-decimal shape is what the reporter photographed. + */ +const FRACTIONAL_GERMAN = /\d,\d/; + +beforeEach(() => { + analyticsMock.mockReset(); + coachReadMock.mockReset(); +}); + +describe("integer-metric precision guard", () => { + it("resolves zero decimals for every discrete metric", () => { + expect(INTEGER_VALUED_MEASUREMENT_TYPES.size).toBeGreaterThan(10); + for (const type of INTEGER_VALUED_MEASUREMENT_TYPES) { + expect(metricFractionDigits(type), type).toBe(0); + } + // A continuous metric keeps the shared default; an unknown one too. + expect(metricFractionDigits("PULSE")).toBe(DEFAULT_METRIC_FRACTION_DIGITS); + expect(metricFractionDigits("WEIGHT")).toBe(DEFAULT_METRIC_FRACTION_DIGITS); + expect(metricFractionDigits(null)).toBe(DEFAULT_METRIC_FRACTION_DIGITS); + }); + + it("renders the coach-read band without a fractional step", () => { + coachReadMock.mockReturnValue({ data: REPORTED_BAND }); + + const html = render( + , + ); + + expect(html).toContain('data-slot="coach-read-baseline"'); + expect(html).not.toMatch(FRACTIONAL_GERMAN); + // The band edges are still there, rounded to whole steps. + expect(html).toContain("13.388"); + expect(html).toContain("104"); + }); + + it("keeps the decimal for a metric that is not a count", () => { + coachReadMock.mockReturnValue({ + data: { + learning: false, + baseline: { low: 71.4, high: 84.2, latest: 78.6, placement: "within" }, + driver: null, + } as unknown as CoachReadStripData, + }); + + const html = render(); + expect(html).toMatch(FRACTIONAL_GERMAN); + }); + + it("renders the step page stat strip without a fractional step", () => { + // The page passes no precision prop at all — that is the point. + analyticsMock.mockReturnValue({ + data: { + summaries: { + ACTIVITY_STEPS: { + count: 214, + latest: 8213, + min: 104, + max: 11762, + mean: 6431.72, + median: 6802.5, + }, + }, + }, + isEmpty: false, + isLoading: false, + }); + coachReadMock.mockReturnValue({ data: undefined }); + + const html = render(); + + expect(html).toContain('data-slot="metric-stat-strip"'); + expect(html).not.toMatch(FRACTIONAL_GERMAN); + // The numbers themselves survive, thousands-grouped and whole. + expect(html).toContain("11.762"); + expect(html).toContain("104"); + // …and the unit reads in German, not as the English word. (The route + // slug `/insights/steps` legitimately appears in the header links, so + // match the unit in its rendered position: right after a number.) + expect(html).toContain("104 Schritte"); + expect(html).not.toMatch(/\d\s*steps\b/); + }); +}); diff --git a/src/components/insights/derived/coach-read-strip.tsx b/src/components/insights/derived/coach-read-strip.tsx index 10c5bf57e..8dfad52fe 100644 --- a/src/components/insights/derived/coach-read-strip.tsx +++ b/src/components/insights/derived/coach-read-strip.tsx @@ -8,6 +8,7 @@ import { useUnitDisplay } from "@/hooks/use-unit-display"; import { useMounted } from "@/hooks/use-mounted"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; +import { metricFractionDigits } from "@/lib/measurements/value-domain"; import { apiGet } from "@/lib/api/api-fetch"; import { Card, CardContent, CardHeader } from "@/components/ui/card"; import { TileHeader } from "@/components/insights/tile-header"; @@ -38,9 +39,12 @@ export interface CoachReadStripProps { /** Unit suffix rendered next to the band edges + today's value. */ unit: string; /** - * Decimal precision for the formatted numbers. Defaults to 1 — enough for - * weight (78.4) / HRV (41.5); integer metrics drop the trailing zero via - * `Intl.NumberFormat`. Pages pass 0 for integer-only metrics (BP, steps). + * Decimal precision for the formatted numbers. Optional: when omitted the + * precision follows the METRIC (`metricFractionDigits`) — a discrete metric + * resolves 0, everything else the shared default of 1. It used to default + * to a flat 1 and rely on each page remembering to pass 0, which is how a + * step page came to print "104.0 steps". Pass it only to override the + * metric's own answer. */ fractionDigits?: number; /** @@ -55,9 +59,10 @@ export interface CoachReadStripProps { export function CoachReadStrip({ metricType, unit, - fractionDigits = 1, + fractionDigits, valueScale = 1, }: CoachReadStripProps) { + const digits = fractionDigits ?? metricFractionDigits(metricType); const { isAuthenticated } = useAuth(); const { t, locale } = useTranslations(); const mounted = useMounted(); @@ -89,7 +94,7 @@ export function CoachReadStrip({ const fmt = (value: number): string => new Intl.NumberFormat(locale, { minimumFractionDigits: 0, - maximumFractionDigits: fractionDigits, + maximumFractionDigits: digits, }).format( transformed ? unitDisplay.toDisplay(metricType, value) diff --git a/src/components/insights/healthkit-metric-page.tsx b/src/components/insights/healthkit-metric-page.tsx index e4c0519b4..c8ccf4cad 100644 --- a/src/components/insights/healthkit-metric-page.tsx +++ b/src/components/insights/healthkit-metric-page.tsx @@ -14,6 +14,7 @@ import { useTranslations } from "@/lib/i18n/context"; import type { InsightMetric } from "@/lib/insights/metric-availability"; import type { MetricStatusMetricId } from "@/lib/insights/metric-status-registry"; import type { ChartOverlayKey } from "@/lib/dashboard-layout"; +import { metricFractionDigits } from "@/lib/measurements/value-domain"; import { Button } from "@/components/ui/button"; import { QueryErrorRow } from "@/components/ui/query-error-row"; import { Card, CardContent } from "@/components/ui/card"; @@ -98,6 +99,17 @@ export interface HealthKitMetricPageProps { * transformed types omit these props; everything else still passes `unit`. */ unit?: string; + /** + * i18n key for a unit that is a WORD rather than a symbol — `steps`, + * `flights`, `falls`, `years`, `events`. Resolved through `t()` so it + * renders in the reader's language; a German user was shown "13.387,6 + * steps". Symbols (`bpm`, `mmHg`, `%`, `m/s`, `kg/m²`, …) are language- + * neutral and stay literals in `unit` — translating a symbol would be the + * opposite mistake. Wins over `unit` / `yAxisUnit` for both the chart and + * the axis label; a type with a registered display transform still + * overrides everything. + */ + unitKey?: string; /** Optional y-axis label shown above the unit. */ yAxisUnit?: string; /** Optional value bands (Apple-Health-style target zone shading). */ @@ -206,6 +218,7 @@ export function HealthKitMetricPage({ i18nPrefix, color, unit, + unitKey, yAxisUnit, valueBands, emptyStateCtaType, @@ -264,13 +277,24 @@ export function HealthKitMetricPage({ const transform = unitDisplay.isTransformed(effectiveType) ? unitDisplay.transformFor(effectiveType) : null; - const resolvedUnit = transform ? transform.displayUnit : unit; - const resolvedYAxisUnit = transform ? transform.displayUnit : yAxisUnit; + // A word-shaped unit arrives as an i18n key and is resolved here, once, for + // the chart line, the axis label, the stat strip and the coach-read strip. + const localisedUnit = unitKey ? t(unitKey) : undefined; + const resolvedUnit = transform + ? transform.displayUnit + : (localisedUnit ?? unit); + const resolvedYAxisUnit = transform + ? transform.displayUnit + : (localisedUnit ?? yAxisUnit); const resolvedScale = transform ? transform.factor : (valueScale ?? 1); const resolvedOffset = transform ? (transform.offset ?? 0) : 0; + // Precision follows the METRIC, not the page. A discrete metric renders no + // fractional part even when the page says nothing — the steps page said + // nothing and its Min read "104.0 steps". An explicit `statFractionDigits` + // still wins for the metrics that genuinely need one (mmol/L, skin temp). const resolvedFractionDigits = transform ? transform.decimals - : statFractionDigits; + : (statFractionDigits ?? metricFractionDigits(effectiveType)); // Value bands are absolute bounds → affine-convert (factor + offset) so the // shaded zone tracks the converted line. Metric users get the identity. const resolvedBands = diff --git a/src/lib/ai/coach/snapshot-blocks/core-metrics-block.ts b/src/lib/ai/coach/snapshot-blocks/core-metrics-block.ts index c4a5f377d..6049a5986 100644 --- a/src/lib/ai/coach/snapshot-blocks/core-metrics-block.ts +++ b/src/lib/ai/coach/snapshot-blocks/core-metrics-block.ts @@ -59,8 +59,8 @@ export function buildCoreMetricsBlocks( ); const olderSys = sysRows.filter((row) => row.measuredAt < ctx.recentCutoff); const olderDia = diaRows.filter((row) => row.measuredAt < ctx.recentCutoff); - const sysBand = bpBandFromRows(sysRows); - const diaBand = bpBandFromRows(diaRows); + const sysBand = bpBandFromRows(sysRows, "BLOOD_PRESSURE_SYS"); + const diaBand = bpBandFromRows(diaRows, "BLOOD_PRESSURE_DIA"); const usualRange = sysBand || diaBand ? { diff --git a/src/lib/ai/coach/snapshot-series.ts b/src/lib/ai/coach/snapshot-series.ts index 636b9f5bc..9357b0716 100644 --- a/src/lib/ai/coach/snapshot-series.ts +++ b/src/lib/ai/coach/snapshot-series.ts @@ -293,6 +293,7 @@ export function buildDailyBpRows( */ export function bpBandFromRows( rows: ReadonlyArray<{ measuredAt: Date; value: number }>, + type: "BLOOD_PRESSURE_SYS" | "BLOOD_PRESSURE_DIA", ): { low: number; high: number } | null { const byDay = new Map(); for (const r of rows) { @@ -304,7 +305,7 @@ export function bpBandFromRows( } const dayMeans = [...byDay.values()].map((a) => a.sum / a.count); if (dayMeans.length < 7) return null; - const band = buildBaselineBand(dayMeans); + const band = buildBaselineBand(dayMeans, type); if (!band) return null; return { low: Math.round(band.low), high: Math.round(band.high) }; } diff --git a/src/lib/illness/correlation.ts b/src/lib/illness/correlation.ts index e9edbe04c..7dd52abcc 100644 --- a/src/lib/illness/correlation.ts +++ b/src/lib/illness/correlation.ts @@ -425,7 +425,10 @@ function bandFor(series: VitalSeries): { spread: number; } | null { if (series.baselineDays.length < MIN_BASELINE_DAYS) return null; - const band = buildBaselineBand(series.baselineDays.map((p) => p.mean)); + const band = buildBaselineBand( + series.baselineDays.map((p) => p.mean), + series.type, + ); if (!band || band.spread <= 0) return null; return { center: band.center, spread: band.spread }; } diff --git a/src/lib/insights/chart-y-domain.ts b/src/lib/insights/chart-y-domain.ts index 4b278992b..53b2ee018 100644 --- a/src/lib/insights/chart-y-domain.ts +++ b/src/lib/insights/chart-y-domain.ts @@ -8,14 +8,25 @@ * (at least 1 unit), with a little extra headroom on top. * - Otherwise: pad the bottom by 8% of the span (min 0.5) and the top by * 16% of the span (min 1). - * - Zero clamp: when the series contains no negative values the lower - * bound never dips below 0 — padding only extends upward or down to 0. - * Counts, steps, weights etc. must not render a negative axis. Series - * with genuine negative values (deltas, differences) keep the full - * downward padding. + * - Zero clamp: the lower bound never dips below 0 when the charted metrics + * cannot be negative — padding only extends upward or down to 0. Counts, + * steps, weights etc. must not render a negative axis. Series with genuine + * negative values (deltas, differences) keep the full downward padding. + * + * "Cannot be negative" is answered by `areAllNonNegativeMetrics` off the + * metric's own declared plausibility domain — the SAME fact the personal + * baseline band floors against, so the axis and the sentence printed under + * it can no longer disagree. It also covers the derived overlays: a trend + * line extrapolating a step count below zero now clips at the axis floor + * instead of dragging the whole chart into negative territory. The observed + * minimum stays a second, weaker trigger, so a caller that names no type + * (a mini chart over an anonymous series) keeps the data-derived behaviour. */ +import { areAllNonNegativeMetrics } from "@/lib/measurements/value-domain"; + export function computePaddedYDomain( values: readonly number[], + types: readonly (string | null | undefined)[] = [], ): [number, number] | undefined { const finite = values.filter( (value): value is number => @@ -26,7 +37,9 @@ export function computePaddedYDomain( const min = Math.min(...finite); const max = Math.max(...finite); // Non-negative series never get a negative lower bound. - const clampLower = (lower: number) => (min >= 0 ? Math.max(0, lower) : lower); + const nonNegative = min >= 0 || areAllNonNegativeMetrics(types); + const clampLower = (lower: number) => + nonNegative ? Math.max(0, lower) : lower; if (min === max) { const delta = Math.max(Math.abs(min) * 0.05, 1); diff --git a/src/lib/insights/derived/__tests__/baseline.test.ts b/src/lib/insights/derived/__tests__/baseline.test.ts index 2c13ab12b..a4742a89a 100644 --- a/src/lib/insights/derived/__tests__/baseline.test.ts +++ b/src/lib/insights/derived/__tests__/baseline.test.ts @@ -53,7 +53,7 @@ describe("pure statistics", () => { }); it("buildBaselineBand returns a symmetric MAD band", () => { - const band = buildBaselineBand([50, 52, 54, 56, 58], 3); + const band = buildBaselineBand([50, 52, 54, 56, 58], "PULSE", 3); expect(band).not.toBeNull(); expect(band!.center).toBe(54); expect(band!.low).toBeLessThan(band!.center); @@ -65,8 +65,8 @@ describe("pure statistics", () => { it("parity: a composed-DAY-bucket series and the raw-DAY series yield the same band", () => { // Same per-day means whether they arrive as raw rows or DAY rollups. const perDayMeans = [60, 61, 62, 59, 63, 58, 64]; - const fromRaw = buildBaselineBand(perDayMeans, 3); - const fromRollup = buildBaselineBand([...perDayMeans], 3); + const fromRaw = buildBaselineBand(perDayMeans, "PULSE", 3); + const fromRollup = buildBaselineBand([...perDayMeans], "PULSE", 3); expect(fromRollup).toEqual(fromRaw); }); }); diff --git a/src/lib/insights/derived/baseline.ts b/src/lib/insights/derived/baseline.ts index 83c090e8f..8ae06c775 100644 --- a/src/lib/insights/derived/baseline.ts +++ b/src/lib/insights/derived/baseline.ts @@ -32,6 +32,7 @@ import type { MeasurementType, PrismaClient } from "@/generated/prisma/client"; import { prisma } from "@/lib/db"; import { getAgeFromDateOfBirth } from "@/lib/analytics/pulse-targets"; import { toProfileSex } from "@/lib/profile/sex"; +import { clampDerivedLowerBound } from "@/lib/measurements/value-domain"; import type { ProfileSex } from "@/lib/profile/sex"; import { probeRollupCoverage, @@ -63,7 +64,11 @@ export interface VitalsBaselineValue { type: MeasurementType; /** Robust center (median of the per-day means). */ center: number; - /** Band lower edge (center − k·MAD·scale). */ + /** + * Band lower edge (center − k·MAD·scale), floored at 0 for a metric whose + * plausibility domain forbids negative values. `spread` below stays the + * unclamped dispersion, so a deviation-in-σ calculation is unaffected. + */ low: number; /** Band upper edge (center + k·MAD·scale). */ high: number; @@ -173,9 +178,20 @@ export function medianAbsoluteDeviation(values: number[]): number { * caller supplies the already-resolved DAY-native series so this can be * unit-tested against both a raw-DAY series and a rollup-composed series * (they must agree: DAY rollup `mean` equals the per-day raw mean). + * + * `type` names the metric the series belongs to and is REQUIRED, not + * optional: `center − spread` is arithmetic, and arithmetic does not know + * that a step, a flight, or a kilocalorie cannot be owed. On a metric with a + * wide relative spread the lower edge crosses zero and the strip underneath + * the chart reads "your usual range is −5,595.6–13,387.6 steps". The floor + * comes from `clampDerivedLowerBound`, which reads the metric's declared + * plausibility domain — the same fact the chart's y-axis clamps against, now + * answered in one place instead of two. Pass `null` only for a series with no + * metric identity; the band then keeps the raw arithmetic edge. */ export function buildBaselineBand( dayMeans: number[], + type: MeasurementType | null, k: number = DEFAULT_MAD_K, ): Omit | null { if (dayMeans.length === 0) return null; @@ -185,7 +201,7 @@ export function buildBaselineBand( const spread = k * mad * MAD_SIGMA_SCALE; return { center, - low: center - spread, + low: clampDerivedLowerBound(type, center - spread), high: center + spread, spread, sampleDays: dayMeans.length, @@ -334,6 +350,7 @@ export async function computeVitalsBaseline( const band = buildBaselineBand( points.map((p) => p.mean), + type, k, ); if (!band) { diff --git a/src/lib/insights/narrative/period-narrative.ts b/src/lib/insights/narrative/period-narrative.ts index 1740c81dc..66ddcd8f9 100644 --- a/src/lib/insights/narrative/period-narrative.ts +++ b/src/lib/insights/narrative/period-narrative.ts @@ -363,7 +363,7 @@ export function assemblePeriodNarrativeContext( const { current, prior } = splitByPeriod(points, currentFrom, priorFrom); if (prior.length < MIN_BASELINE_DAYS) continue; if (current.length < MIN_COVERED_DAYS_PER_METRIC) continue; - const band = buildBaselineBand(prior); + const band = buildBaselineBand(prior, type); if (!band) continue; const center = median(current); const above = center > band.high; @@ -454,7 +454,7 @@ function computeCoincidentFlags( } } if (prior.length < MIN_BASELINE_DAYS) continue; - const band = buildBaselineBand(prior); + const band = buildBaselineBand(prior, type); if (band) bands.set(type, { low: band.low, high: band.high }); } diff --git a/src/lib/insights/streak-detector.ts b/src/lib/insights/streak-detector.ts index 9484b3a02..bb21c1ffb 100644 --- a/src/lib/insights/streak-detector.ts +++ b/src/lib/insights/streak-detector.ts @@ -28,6 +28,7 @@ * * Pure: no DB, no clock, no network. Fully unit-testable over injected series. */ +import type { MeasurementType } from "@/generated/prisma/client"; import { buildBaselineBand } from "@/lib/insights/derived/baseline"; /** One DAY-bucket point: a calendar day key (YYYY-MM-DD) + that day's mean. */ @@ -103,10 +104,15 @@ function placeValue(value: number, band: StreakBand): BandPlacement { * `band` may be supplied (e.g. a prior-period band) — when omitted it is built * from the whole series via `buildBaselineBand`. A gap day (a calendar day with * no point between two present days) BREAKS any streak that would span it. + * + * `metricType` names the metric the series belongs to so a self-built band + * floors at zero on a metric that cannot go negative. Omit it only when the + * series genuinely has no metric identity, or when the caller supplies `band`. */ export function detectStreak( series: StreakPoint[], band?: StreakBand | null, + metricType?: string | null, ): StreakResult { const empty: StreakResult = { latestPlacement: null, @@ -127,7 +133,12 @@ export function detectStreak( // Resolve the personal band: caller-supplied, else median ± k·MAD over the // whole series. Null (too little history) → omit rather than guess. - const resolvedBand = band ?? buildBandFromSeries(points.map((p) => p.value)); + const resolvedBand = + band ?? + buildBandFromSeries( + points.map((p) => p.value), + metricType, + ); if (!resolvedBand) return empty; // Per-day placement, ascending. A gap day inserts an explicit break marker so @@ -194,8 +205,14 @@ export function detectStreak( } /** Build the personal band from a value series; null when too little data. */ -function buildBandFromSeries(values: number[]): StreakBand | null { - const band = buildBaselineBand(values); +function buildBandFromSeries( + values: number[], + metricType: string | null | undefined, +): StreakBand | null { + const band = buildBaselineBand( + values, + (metricType ?? null) as MeasurementType | null, + ); if (!band) return null; if (!Number.isFinite(band.low) || !Number.isFinite(band.high)) return null; return { low: band.low, high: band.high }; diff --git a/src/lib/measurements/__tests__/non-negative-metric-guard.test.ts b/src/lib/measurements/__tests__/non-negative-metric-guard.test.ts new file mode 100644 index 000000000..98a6e1e4a --- /dev/null +++ b/src/lib/measurements/__tests__/non-negative-metric-guard.test.ts @@ -0,0 +1,116 @@ +import { describe, it, expect } from "vitest"; + +import { + VALUE_RANGES, + measurementTypeEnum, +} from "@/lib/validations/measurement"; +import { + areAllNonNegativeMetrics, + isNonNegativeMetric, +} from "@/lib/measurements/value-domain"; +import { buildBaselineBand } from "@/lib/insights/derived/baseline"; +import { computePaddedYDomain } from "@/lib/insights/chart-y-domain"; + +/** + * GUARD — a non-negative metric's baseline `low` is never below zero. + * + * The step sub-page printed "your usual range is −5,595.6–13,387.6 steps" + * because the band's lower edge is `median − k·MAD·scale` and nothing said a + * step cannot be owed. The chart above it already refused a negative axis, so + * the app held both answers at once. + * + * These assertions pin the fixed shape end to end: the fact is declared once + * (the metric's plausibility floor), the band reads it, and the axis reads the + * same one. A series engineered so the spread exceeds the median is the exact + * condition that produced the reported sentence. + */ + +/** + * Median 10, MAD 10 → spread = 3 · 10 · 1.4826 ≈ 44.5, i.e. well past the + * centre. The raw arithmetic edge is ≈ −34.5. + */ +const SPREAD_EXCEEDS_CENTRE = [0, 0, 0, 10, 20, 30, 40]; + +/** The two types whose declared floor is genuinely signed. */ +const SIGNED_TYPES = ["ANS_CHARGE", "BODY_TEMPERATURE_DEVIATION"] as const; + +describe("non-negative metric guard", () => { + it("every MeasurementType declares a plausibility range", () => { + // The range map is the home of the "can this be negative" fact. A type + // missing from it answers "unknown" and silently loses the floor — and, + // separately, passes any number at all through ingest validation. + const missing = measurementTypeEnum.options.filter( + (type) => VALUE_RANGES[type] === undefined, + ); + expect(missing).toEqual([]); + }); + + it("classifies every type from its own declared floor", () => { + for (const type of measurementTypeEnum.options) { + expect(isNonNegativeMetric(type)).toBe(VALUE_RANGES[type].min >= 0); + } + for (const type of SIGNED_TYPES) { + expect(isNonNegativeMetric(type)).toBe(false); + } + // An unnamed or unknown metric is "unknown", never "non-negative". + expect(isNonNegativeMetric(null)).toBe(false); + expect(isNonNegativeMetric("NOT_A_METRIC")).toBe(false); + expect(areAllNonNegativeMetrics([])).toBe(false); + expect(areAllNonNegativeMetrics(["ACTIVITY_STEPS", "ANS_CHARGE"])).toBe( + false, + ); + }); + + it("never floors the baseline band below zero for a non-negative metric", () => { + const nonNegative = measurementTypeEnum.options.filter((type) => + isNonNegativeMetric(type), + ); + // Sanity: the sweep must actually cover the bulk of the catalogue. + expect(nonNegative.length).toBeGreaterThan(60); + + for (const type of nonNegative) { + const band = buildBaselineBand(SPREAD_EXCEEDS_CENTRE, type); + expect(band, type).not.toBeNull(); + expect(band!.low, `${type} band low`).toBeGreaterThanOrEqual(0); + // The dispersion itself stays unclamped — a deviation-in-σ calculation + // reads `spread`, and flooring it would understate every excursion. + expect(band!.spread, `${type} spread`).toBeGreaterThan(band!.center); + } + }); + + it("reproduces the reported step sentence with a non-negative lower edge", () => { + // Roughly the reported shape: a median around 4,000 steps with days near + // zero, which is what pushed the MAD band past the origin. + const dayMeans = [0, 120, 900, 4000, 6200, 9800, 11762]; + const band = buildBaselineBand(dayMeans, "ACTIVITY_STEPS"); + expect(band!.low).toBe(0); + expect(band!.high).toBeGreaterThan(band!.center); + }); + + it("keeps the real lower edge for a metric that can genuinely be negative", () => { + for (const type of SIGNED_TYPES) { + const band = buildBaselineBand(SPREAD_EXCEEDS_CENTRE, type); + expect(band!.low, type).toBeLessThan(0); + } + // No metric identity → no invented floor either. + expect(buildBaselineBand(SPREAD_EXCEEDS_CENTRE, null)!.low).toBeLessThan(0); + }); + + it("gives the chart axis the same answer as the band", () => { + // A derived overlay (trend line, moving average) can extrapolate a step + // count below zero even though no observation ever was. The axis floors + // it because the METRIC cannot be negative, not because the data was. + const domain = computePaddedYDomain([-320, 1200, 8400], ["ACTIVITY_STEPS"]); + expect(domain![0]).toBe(0); + + // A signed metric keeps its downward padding. + const signed = computePaddedYDomain([-40, 5, 60], ["ANS_CHARGE"]); + expect(signed![0]).toBeLessThan(0); + + // An anonymous series keeps the older data-derived behaviour: padding + // that would dip under an all-positive series stops at 0, a series with + // real negative values keeps the full downward padding. + expect(computePaddedYDomain([0.2, 0.5])![0]).toBe(0); + expect(computePaddedYDomain([-5, 3, 10])![0]).toBeLessThan(0); + }); +}); diff --git a/src/lib/measurements/value-domain.ts b/src/lib/measurements/value-domain.ts new file mode 100644 index 000000000..33e913f27 --- /dev/null +++ b/src/lib/measurements/value-domain.ts @@ -0,0 +1,115 @@ +/** + * Per-metric numeric-domain facts — the ONE home for the two questions the + * derivation and render layers kept answering independently, and therefore + * kept answering differently: + * + * 1. **Can this metric be negative?** The chart's y-axis already refused a + * negative lower bound (`chart-y-domain.ts`) while the personal-baseline + * band underneath the same chart happily printed one, so a step page + * could read "your usual range is −5,595.6–13,387.6 steps". Same app, + * same question, two answers. The fact is a property of the METRIC, not + * of the surface that renders it, so it belongs beside the metric — and + * it is already declared there: `VALUE_RANGES` in + * `@/lib/validations/measurement` is the app-wide plausibility domain + * every ingest path validates against, and its `min` IS the metric's + * floor. Nothing new to maintain; this module only reads it. Two types + * declare a genuinely signed floor (`ANS_CHARGE`, + * `BODY_TEMPERATURE_DEVIATION`) and are correctly excluded. + * + * 2. **Is this metric integer-valued?** A step count has no fractional + * part, so "104.0 steps" is not a rounding preference, it is wrong. This + * one had no home at all — every sub-page passed (or forgot) its own + * `fractionDigits` prop, and the pages that forgot rendered a fraction + * of a step. The set below is the metrics whose values are discrete by + * construction: tallies of events, category flags pinned to 1, closed + * integer instrument scores, and integer ordinal levels. + * + * Deliberately NOT in the integer set: rates and continuous quantities that + * merely tend to be logged as whole numbers (bpm, mmHg, kcal, minutes, + * metres). A mean of 68.4 bpm is a real quantity; a mean of 104.3 steps is a + * real quantity too, but a step is indivisible, so the metric is read as a + * count and rendered as one. + */ +import { VALUE_RANGES } from "@/lib/validations/measurement"; + +/** + * Decimal places used for a metric with no discreteness constraint — the + * long-standing default of the stat strip and the coach-read strip. + */ +export const DEFAULT_METRIC_FRACTION_DIGITS = 1; + +/** + * Metrics whose stored values are whole numbers by construction. Keys are + * `MeasurementType` values; typed as strings so client components can ask + * without importing the Prisma client into the browser bundle. + */ +export const INTEGER_VALUED_MEASUREMENT_TYPES: ReadonlySet = new Set([ + // Tallies — a fraction of one of these does not exist. + "ACTIVITY_STEPS", + "FLIGHTS_CLIMBED", + "FALL_COUNT", + "BREATHING_DISTURBANCES", + "SLEEP_DISTURBANCE_COUNT", + // Category events — the row carries a fixed `value = 1` (one fired event). + "AUDIO_EXPOSURE_EVENT", + "BREATHING_DISTURBANCE_EVENT", + "HIGH_HEART_RATE_EVENT", + "IRREGULAR_RHYTHM_NOTIFICATION", + "LOW_HEART_RATE_EVENT", + "WALKING_STEADINESS_EVENT", + // Closed integer instrument scores — the questionnaire sums whole items. + "PAIN_NRS", + "PHQ9_SCORE", + "GAD7_SCORE", + "WHO5_SCORE", + "SCI_SCORE", + // Integer ordinal / rating scales. + "RESILIENCE", + "VISCERAL_FAT", + // Whole years, as the device reports it. + "VASCULAR_AGE", +]); + +/** + * True when the metric's own plausibility floor forbids a negative value. + * Unknown types answer `false` — an absent range is an absent fact, never a + * guessed one. + */ +export function isNonNegativeMetric(type: string | null | undefined): boolean { + if (!type) return false; + const range = VALUE_RANGES[type]; + return range !== undefined && range.min >= 0; +} + +/** + * True when EVERY named metric forbids negative values (and at least one was + * named). Used by the chart, which can paint several series in one axis: a + * single signed series keeps the full downward padding for all of them. + */ +export function areAllNonNegativeMetrics( + types: readonly (string | null | undefined)[], +): boolean { + return types.length > 0 && types.every((type) => isNonNegativeMetric(type)); +} + +/** + * Clamp a DERIVED lower edge (a baseline band edge, a padded axis bound) to + * the metric's zero floor. Derived edges are arithmetic — median − k·MAD, a + * padded minimum — and arithmetic does not know that a step cannot be owed. + */ +export function clampDerivedLowerBound( + type: string | null | undefined, + lower: number, +): number { + return isNonNegativeMetric(type) ? Math.max(0, lower) : lower; +} + +/** + * Decimal places a metric's values are read at. Zero for a discrete metric, + * the shared default otherwise. Surfaces call this instead of hard-coding a + * default, so a page that says nothing still renders the metric correctly. + */ +export function metricFractionDigits(type: string | null | undefined): number { + if (type && INTEGER_VALUED_MEASUREMENT_TYPES.has(type)) return 0; + return DEFAULT_METRIC_FRACTION_DIGITS; +} diff --git a/src/lib/validations/measurement.ts b/src/lib/validations/measurement.ts index 32dbc045f..3cd8ac1bd 100644 --- a/src/lib/validations/measurement.ts +++ b/src/lib/validations/measurement.ts @@ -495,6 +495,11 @@ export const VALUE_RANGES: Record = { // the ×100 server-side scaling the canonical band is 0..100. WALKING_ASYMMETRY: { min: 0, max: 100 }, WALKING_DOUBLE_SUPPORT: { min: 0, max: 100 }, + // Same ×100-scaled 0..100 percent as its two gait siblings above. The + // entry was simply missing; the map is the app's plausibility domain AND + // (via `value-domain.ts`) the answer to "can this metric be negative", + // so an omission reads as "unknown" on both questions. + WALKING_STEADINESS: { min: 0, max: 100 }, // ── v1.5.5 iOS-coord follow-up — raw-SI gait pair ── // Step length (metres) — adult walking sits around 0.5–0.8 m; the // 0.1 floor captures shuffling gait and the 2.0 ceiling covers @@ -541,6 +546,14 @@ export const VALUE_RANGES: Record = { LOW_HEART_RATE_EVENT: { min: 1, max: 1 }, WALKING_STEADINESS_EVENT: { min: 1, max: 1 }, BREATHING_DISTURBANCE_EVENT: { min: 1, max: 1 }, + // Loud-listening event flag (env + headphone). One fired event per row, so + // the floor is 1 — a non-event is not stored. The entry was missing from + // the map entirely, which let ANY number through ingest, including a + // negative one. The ceiling stays generous rather than pinned at 1 like the + // five events above: this type is written only by the iOS client and + // nothing in this repo asserts the exact value, so the floor is the part + // that is certain. + AUDIO_EXPOSURE_EVENT: { min: 1, max: 1000 }, // ── v1.10.0 — computed scores (WX-C) ── // Server-derived 0–100 scores. The plausibility band pins them to the // score range so a malformed store (a stray negative or > 100) is rejected.