From bf0a637bab86b116ba091d37a42a82b6cb50d58e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Fri, 31 Jul 2026 19:05:20 +0200 Subject: [PATCH] Keep the score's own coverage honest about what it counts The composite's coverage counts distinct areas of health, not pillars. The learning line called them pillars, so an account recording blood pressure, glycaemia and cholesterol read that it had one eligible pillar while three sat in front of it. The count was right and stays; the noun was wrong. The meter beneath the score carried the same confusion further. Its fraction is the three-area minimum the score requires, and a score only exists once that floor is met, so it rendered as full coverage for every account that has one. The arithmetic is untouched. Below the floor the fraction is a real number and is shown as one; at or above it, the floor is stated. The dots and the percentage were always honest, they track how much history is behind the number, and they stay. Both are held down by an AST guard rather than a text search, with a counter-test for the neighbouring lines that are legitimately about pillars and legitimately about a real fraction. --- e2e/settings-score.spec.ts | 293 ++++++++++++++++++ .../health-score-coverage-axis-guard.test.tsx | 40 +++ .../__tests__/score-change-notice.test.tsx | 54 +++- .../__tests__/score-pillar-row.test.tsx | 31 +- .../score-refusal-consumer-guard.test.ts | 138 +++++++++ src/components/settings/score-section.tsx | 16 +- 6 files changed, 555 insertions(+), 17 deletions(-) create mode 100644 e2e/settings-score.spec.ts create mode 100644 src/components/settings/__tests__/score-refusal-consumer-guard.test.ts diff --git a/e2e/settings-score.spec.ts b/e2e/settings-score.spec.ts new file mode 100644 index 000000000..2f8f3a24a --- /dev/null +++ b/e2e/settings-score.spec.ts @@ -0,0 +1,293 @@ +import { expect, test } from "./setup/test"; + +import { STORAGE_STATE_PATH } from "./setup/global-setup"; +import { settleBeforeMeasure } from "./utils/settle"; + +/** + * Settings → Health Score: the surface that decides which pillars count. + * + * Everything asserted here goes through stable `data-slot` / `data-*` + * attributes rather than viewport-dependent text, so the spec keeps working + * at any width and in any locale (only the copy assertions read English, and + * the suite pins the English locale through the storage state). + * + * The refusal case is driven through the REAL write. Deselecting down to + * activity and wellbeing genuinely fails the server's breadth rule, and a + * refused write persists nothing, so the case leaves the shared account + * exactly as it found it. Mocking the 422 would have proved that the page + * can render a message, not that the rule is enforced or that the page reads + * the reason the server actually sends. + * + * The analytics read IS mocked, because it is the page's secondary read and + * the states it drives — waiting for data, safety signposting — depend on + * what the seeded account happens to hold. Fixing it makes those two rows + * deterministic without touching the write path under test. + */ + +/** A slim Health Score report: only the fields the settings page reads. */ +function scoreReport() { + const coverage = { + requiredInputs: 3, + presentInputs: 2, + historyDays: 14, + missing: [], + }; + const provenance = { + inputs: [], + source: "live", + windowDays: 90, + computedAt: new Date().toISOString(), + }; + const insufficient = (reason: string) => ({ + status: "insufficient", + reason, + coverage, + provenance, + }); + return { + composite: insufficient("three_domains_required"), + pillars: [ + { + id: "LIPIDS", + domain: "cardiometabolic", + result: insufficient("not_tracked"), + }, + { + id: "WELLBEING", + domain: "wellbeing", + result: insufficient("crisis_signposting"), + }, + { + id: "BLOOD_PRESSURE", + domain: "cardiometabolic", + result: insufficient("read_failed"), + }, + ], + delta: null, + deltaReason: "no_current_score", + scoreVersion: 2, + weightGoal: insufficient("no_goal"), + // Raised and never dismissed, which is the state every account is in: + // nothing has ever rendered this notice. + algorithmNotice: { itemKey: "health_score_algorithm:2", dismissed: false }, + }; +} + +async function mockAnalytics(page: import("@playwright/test").Page) { + await page.route(/\/api\/analytics(\?|$)/, async (route) => { + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { summaries: {}, healthScore: scoreReport() }, + error: null, + }), + }); + }); +} + +test.describe("Settings → Health Score", () => { + test.use({ storageState: STORAGE_STATE_PATH }); + + test.beforeEach(async ({ page }) => { + await mockAnalytics(page); + }); + + test("groups the pillars by area and makes the three axes legible", async ({ + page, + }) => { + await page.goto("/settings/score", { waitUntil: "domcontentloaded" }); + const card = page.locator('[data-slot="score-config-card"]'); + await settleBeforeMeasure(page, card); + + // The heading + subheading pair the settings IA requires, owned by the + // shell and not re-declared inside the card. + await expect(page.locator("#settings-section-score-title")).toBeVisible(); + await expect( + page.locator("#settings-section-score-title + p"), + ).toBeVisible(); + + // One concept, one card. A second card beside it would be the + // top/bottom split of one decision the rules forbid; the notice is a + // different concept and is allowed its own. + await expect(page.locator('[data-slot="score-config-card"]')).toHaveCount( + 1, + ); + + // The cardiometabolic triple, visibly under one heading. + const cardio = page.locator( + '[data-slot="score-config-group"][data-domain="cardiometabolic"]', + ); + await expect(cardio).toHaveCount(1); + await expect(cardio.locator('[data-slot="score-pillar-row"]')).toHaveCount( + 3, + ); + await expect( + cardio.locator('[data-slot="score-config-group-note"]'), + ).toBeVisible(); + + // Every visible row names recording and showing beside its switch, and + // the card says in words that this page leaves those two alone. + const rows = page.locator('[data-slot="score-pillar-row"]'); + const rowCount = await rows.count(); + expect(rowCount).toBeGreaterThan(0); + await expect( + page.locator('[data-slot="score-pillar-axis-recorded"]'), + ).toHaveCount(rowCount); + await expect( + page.locator('[data-slot="score-pillar-axis-shown"]'), + ).toHaveCount(rowCount); + await expect( + page.locator('[data-slot="score-config-three-axes"]'), + ).toContainText("keeps being recorded"); + }); + + test("shows the pillar it cannot score as present, disabled and honest", async ({ + page, + }) => { + await page.goto("/settings/score", { waitUntil: "domcontentloaded" }); + const fitness = page.locator( + '[data-slot="score-pillar-row"][data-pillar="FITNESS"]', + ); + await settleBeforeMeasure(page, fitness); + + await expect(fitness).toHaveAttribute("data-selectable", "false"); + await expect(fitness).toHaveAttribute("data-eligibility", "unavailable"); + // No switch at all, rather than one that no-ops behind `disabled`. + await expect( + fitness.locator('[data-slot="score-pillar-switch"]'), + ).toHaveCount(0); + await expect( + fitness.locator('[data-slot="score-pillar-unavailable"]'), + ).toHaveAttribute("aria-disabled", "true"); + }); + + test("tells waiting for data apart from safety guidance and a failed read", async ({ + page, + }) => { + await page.goto("/settings/score", { waitUntil: "domcontentloaded" }); + const card = page.locator('[data-slot="score-config-card"]'); + await settleBeforeMeasure(page, card); + + const state = (pillar: string) => + page.locator( + `[data-slot="score-pillar-row"][data-pillar="${pillar}"] [data-slot="score-pillar-state"]`, + ); + + await expect(state("LIPIDS")).toHaveAttribute("data-state", "waiting"); + // Safety signposting is its own state and never a configuration error. + await expect(state("WELLBEING")).toHaveAttribute("data-state", "crisis"); + // A failed read is a failure, not absence. + await expect(state("BLOOD_PRESSURE")).toHaveAttribute( + "data-state", + "read_failed", + ); + }); + + test("renders the score notice and lets it be dismissed", async ({ + page, + }) => { + await page.goto("/settings/score", { waitUntil: "domcontentloaded" }); + const notice = page.locator('[data-slot="score-change-notice"]'); + await settleBeforeMeasure(page, notice); + + await expect(notice).toBeVisible(); + + let dismissed: string | null = null; + await page.route(/\/api\/daily\/digest\/dismiss$/, async (route) => { + dismissed = String(route.request().postDataJSON()?.itemKey ?? ""); + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ data: { dismissed: true }, error: null }), + }); + }); + // The refetch after the dismissal has to come back already dismissed, + // or the notice would simply be raised again and the test would be + // asserting a race rather than the behaviour. + await page.route(/\/api\/analytics(\?|$)/, async (route) => { + const report = scoreReport(); + report.algorithmNotice.dismissed = true; + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ + data: { summaries: {}, healthScore: report }, + error: null, + }), + }); + }); + + await notice.locator('[data-slot="score-change-notice-dismiss"]').click(); + + await expect(notice).toHaveCount(0); + expect(dismissed).toBe("health_score_algorithm:2"); + }); + + test("refuses a selection too narrow to produce a score, in plain language", async ({ + page, + }) => { + await page.goto("/settings/score", { waitUntil: "domcontentloaded" }); + const card = page.locator('[data-slot="score-config-card"]'); + await settleBeforeMeasure(page, card); + + // Start from everything, then take out every pillar except activity and + // wellbeing: two areas of health and no physical measurement, which the + // server refuses. + await page.locator('[data-slot="score-config-preset-all"]').click(); + const keep = new Set(["ACTIVITY", "WELLBEING"]); + const rows = page.locator( + '[data-slot="score-pillar-row"][data-selectable="true"]', + ); + for (let index = 0; index < (await rows.count()); index += 1) { + const row = rows.nth(index); + const pillar = await row.getAttribute("data-pillar"); + if (!pillar || keep.has(pillar)) continue; + if ((await row.getAttribute("data-counts")) === "false") continue; + await row.locator('[data-slot="score-pillar-switch"]').click(); + } + + await page.locator('[data-slot="score-config-save"]').click(); + + const refusal = page.locator('[data-slot="score-config-refusal"]'); + await expect(refusal).toBeVisible(); + await expect(refusal).toHaveAttribute("role", "alert"); + // The server's machine reason, rendered as a sentence a person can act + // on, not as an error code and not as the server's English fallback. + await expect(refusal).toContainText("physical measurement"); + await expect(refusal).not.toContainText("_required"); + }); + + test("keeps the content column the same width as its neighbours", async ({ + page, + }, testInfo) => { + test.skip( + testInfo.project.name !== "chromium-desktop", + "viewport-driven assertion; desktop project only", + ); + await page.setViewportSize({ width: 1440, height: 900 }); + + async function columnWidth(route: string, sentinel: string) { + await page.goto(route, { waitUntil: "domcontentloaded" }); + await settleBeforeMeasure(page, page.locator(sentinel)); + return page.evaluate(() => { + const column = document.querySelector("section[aria-labelledby]"); + const grid = column?.closest("div.grid") ?? null; + const cell = grid?.lastElementChild ?? null; + return cell ? Math.round(cell.getBoundingClientRect().width) : -1; + }); + } + + const modules = await columnWidth( + "/settings/modules", + '[data-slot="card"]', + ); + const score = await columnWidth( + "/settings/score", + '[data-slot="score-config-card"]', + ); + + expect(modules).toBeGreaterThan(0); + expect(Math.abs(score - modules)).toBeLessThanOrEqual(1); + }); +}); diff --git a/src/components/insights/__tests__/health-score-coverage-axis-guard.test.tsx b/src/components/insights/__tests__/health-score-coverage-axis-guard.test.tsx index 0107d54b7..085688c61 100644 --- a/src/components/insights/__tests__/health-score-coverage-axis-guard.test.tsx +++ b/src/components/insights/__tests__/health-score-coverage-axis-guard.test.tsx @@ -29,8 +29,11 @@ import { readFileSync } from "node:fs"; import { join } from "node:path"; import ts from "typescript"; +import { renderToStaticMarkup } from "react-dom/server"; import { describe, expect, it } from "vitest"; +import { I18nProvider } from "@/lib/i18n/context"; +import { CoverageMeter } from "../derived/coverage-meter"; import en from "../../../../messages/en.json"; const CARD_FILE = join( @@ -231,3 +234,40 @@ describe("the composite's coverage meter names its own axis", () => { ); }); }); + +/** + * The attribute guard above proves the override is written down. This + * proves it does something: a prop that is declared and then ignored is + * exactly the shape of a check that cannot fail. + */ +describe("the axis override reaches the markup", () => { + const coverage = { + requiredInputs: 3, + presentInputs: 3, + historyDays: 20, + missing: [], + }; + + function render(axisLabel?: string): string { + return renderToStaticMarkup( + + + , + ); + } + + it("replaces the fraction in the label and the accessible name", () => { + const html = render("Covers the three areas the score needs"); + + expect(html).toContain("Covers the three areas the score needs"); + expect(html).not.toContain(">3/3<"); + expect(html).toMatch(/aria-label="Covers the three areas the score needs/); + }); + + it("keeps the fraction for every caller that does not override it", () => { + const html = render(); + + expect(html).toContain("3/3"); + expect(html).toContain("3 of 3 inputs"); + }); +}); diff --git a/src/components/settings/__tests__/score-change-notice.test.tsx b/src/components/settings/__tests__/score-change-notice.test.tsx index f78415f70..4c50ba780 100644 --- a/src/components/settings/__tests__/score-change-notice.test.tsx +++ b/src/components/settings/__tests__/score-change-notice.test.tsx @@ -97,15 +97,51 @@ describe("dismissal", () => { }); describe("localisation", () => { - it("renders both shapes in each shipped locale", () => { - for (const locale of ["de", "fr", "es", "it", "pl"] as Locale[]) { - for (const version of [0, 2]) { - const html = render( - { configVersion: version, changedAt: "2026-07-31T09:00:00.000Z" }, - locale, - ); - expect(html).not.toContain("settings.sections.score"); - } + // Not an absence check: `t()` falls back to English, so a missing key + // renders English rather than a key path. These are each locale's own + // titles, so an English fallback, a copied value or a mis-wired bundle + // all fail here. Key parity across bundles is a repo-wide guard already. + const TITLES: Record = { + de: { + upgrade: "Was zählt, entscheidest jetzt du", + changed: "Du hast geändert, was zählt", + }, + fr: { + upgrade: "Ce qui compte, c'est vous qui le décidez", + changed: "Vous avez changé ce qui compte", + }, + es: { + upgrade: "Ahora decides tú qué cuenta", + changed: "Has cambiado qué cuenta", + }, + it: { + upgrade: "Ora sei tu a decidere cosa conta", + changed: "Hai cambiato cosa conta", + }, + pl: { + upgrade: "Teraz to Ty decydujesz, co się liczy", + changed: "Zmieniłeś to, co się liczy", + }, + }; + + it("renders both shapes in each locale's own words", () => { + for (const [locale, titles] of Object.entries(TITLES)) { + const upgrade = render( + { configVersion: 0, changedAt: null }, + locale as Locale, + ); + expect(upgrade, `${locale} upgrade note`).toContain( + // The apostrophe in the French title is escaped in the markup. + titles.upgrade.replace(/'/g, "'"), + ); + expect(upgrade).not.toContain("What counts is now yours to choose"); + + const changed = render( + { configVersion: 2, changedAt: "2026-07-31T09:00:00.000Z" }, + locale as Locale, + ); + expect(changed, `${locale} changed note`).toContain(titles.changed); + expect(changed).not.toContain("You changed what counts"); } }); }); diff --git a/src/components/settings/__tests__/score-pillar-row.test.tsx b/src/components/settings/__tests__/score-pillar-row.test.tsx index 2fb6846bf..85101c7ee 100644 --- a/src/components/settings/__tests__/score-pillar-row.test.tsx +++ b/src/components/settings/__tests__/score-pillar-row.test.tsx @@ -139,12 +139,31 @@ describe("a pillar this build cannot score", () => { }); describe("localisation", () => { - it("renders every state in each shipped locale", () => { - for (const locale of ["de", "fr", "es", "it", "pl"] as Locale[]) { - const html = render({ ...base, eligibility: "waiting" }, locale); - // A missing key falls through to the key path; seeing one here means - // a locale bundle is short. - expect(html).not.toContain("settings.sections.score"); + // Asserting the absence of a key path would prove nothing: `t()` falls + // back to the English bundle, so a missing locale key renders English + // rather than the key. Locale PARITY is already a repo-wide guard + // (`i18n-locale-integrity`); what belongs here is that the row actually + // reaches each locale's own words, which fails on a missing key, on an + // English value copied into a bundle, and on a locale wired to the + // wrong strings. + const WAITING: Record = { + de: "Ausgewählt, wartet auf Daten", + fr: "Sélectionné, en attente de données", + es: "Seleccionado, esperando datos", + it: "Selezionato, in attesa di dati", + pl: "Wybrany, czeka na dane", + }; + + it("renders each locale's own words, not the English fallback", () => { + for (const [locale, expected] of Object.entries(WAITING)) { + const html = render( + { ...base, eligibility: "waiting" }, + locale as Locale, + ); + expect(html, `${locale} row`).toContain(expected); + expect(html, `${locale} row fell back to English`).not.toContain( + "Selected, waiting for data", + ); } }); }); diff --git a/src/components/settings/__tests__/score-refusal-consumer-guard.test.ts b/src/components/settings/__tests__/score-refusal-consumer-guard.test.ts new file mode 100644 index 000000000..2a864de77 --- /dev/null +++ b/src/components/settings/__tests__/score-refusal-consumer-guard.test.ts @@ -0,0 +1,138 @@ +/** + * The write-time refusal has two ends, and this holds them together. + * + * The server owns the breadth rule and answers a too-narrow selection + * with a machine reason in `meta.reason`. The settings surface turns that + * reason into a sentence in the reader's own language. Nothing in the + * type system connects the two: rename a reason on the route, and the + * client silently falls through to "couldn't save, try again" while the + * person is left with no idea what is wrong with their selection. That is + * a refusal that stops refusing anything in particular, and it looks + * perfectly healthy in every unit test on either side. + * + * So: the reasons the rule can produce, the reasons the route has copy + * for, and the reasons the client has copy for must be the same set. The + * rule's own list is read out of `breadth.ts` rather than restated here, + * because a restated list is a fourth place to forget. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import ts from "typescript"; +import { describe, expect, it } from "vitest"; + +import en from "../../../../messages/en.json"; +import { REFUSAL_KEYS } from "../score-section"; + +const ROOT = process.cwd(); +const RULE_FILE = join(ROOT, "src/lib/analytics/score/breadth.ts"); +const ROUTE_FILE = join( + ROOT, + "src/app/api/auth/me/health-score-config/route.ts", +); + +function parse(path: string): ts.SourceFile { + return ts.createSourceFile( + path, + readFileSync(path, "utf8"), + ts.ScriptTarget.Latest, + true, + ts.ScriptKind.TS, + ); +} + +function walk(node: ts.Node, visit: (node: ts.Node) => void): void { + visit(node); + node.forEachChild((child) => walk(child, visit)); +} + +/** The string members of an exported union type alias. */ +function unionMembers(file: ts.SourceFile, name: string): string[] { + const members: string[] = []; + walk(file, (node) => { + if (!ts.isTypeAliasDeclaration(node)) return; + if (node.name.text !== name) return; + const type = node.type; + const parts = ts.isUnionTypeNode(type) ? type.types : [type]; + for (const part of parts) { + if ( + ts.isLiteralTypeNode(part) && + ts.isStringLiteral(part.literal) && + part.literal.text.length > 0 + ) { + members.push(part.literal.text); + } + } + }); + return members; +} + +/** The property names of a top-level object-literal constant. */ +function objectKeys(file: ts.SourceFile, name: string): string[] { + const keys: string[] = []; + walk(file, (node) => { + if (!ts.isVariableDeclaration(node)) return; + if (!ts.isIdentifier(node.name) || node.name.text !== name) return; + let initializer = node.initializer; + while ( + initializer && + (ts.isAsExpression(initializer) || + ts.isSatisfiesExpression(initializer) || + ts.isParenthesizedExpression(initializer)) + ) { + initializer = initializer.expression; + } + if (!initializer || !ts.isObjectLiteralExpression(initializer)) return; + for (const property of initializer.properties) { + if (!ts.isPropertyAssignment(property)) continue; + const propertyName = property.name; + if (ts.isIdentifier(propertyName)) keys.push(propertyName.text); + else if (ts.isStringLiteral(propertyName)) keys.push(propertyName.text); + } + }); + return keys; +} + +function message(key: string): string | undefined { + let cursor: unknown = en; + for (const segment of key.split(".")) { + if (cursor === null || typeof cursor !== "object") return undefined; + cursor = (cursor as Record)[segment]; + } + return typeof cursor === "string" ? cursor : undefined; +} + +const ruleReasons = unionMembers(parse(RULE_FILE), "ScoreBreadthFailure"); +const routeReasons = objectKeys(parse(ROUTE_FILE), "BREADTH_REFUSAL"); + +describe("every refusal the rule can give reaches the reader", () => { + it("reads the rule's own list rather than a copy of it", () => { + // A matcher that finds nothing agrees with everything. + expect(ruleReasons.length).toBeGreaterThan(0); + expect(ruleReasons).toContain("three_domains_required"); + }); + + it("gives the route a sentence for each of them", () => { + expect([...routeReasons].sort()).toEqual([...ruleReasons].sort()); + }); + + it("gives the settings surface localised copy for each of them", () => { + expect(Object.keys(REFUSAL_KEYS).sort()).toEqual([...ruleReasons].sort()); + }); + + it("resolves every one of those keys to real copy", () => { + for (const [reason, key] of Object.entries(REFUSAL_KEYS)) { + const copy = message(key); + expect(copy, `${reason} → ${key}`).toBeTypeOf("string"); + expect(copy?.length ?? 0).toBeGreaterThan(20); + } + }); + + it("stays quiet on a constant that is not the refusal map", () => { + // The matcher is name-anchored, not shape-anchored: it must not pick + // up the neighbouring object literals on either file and quietly + // widen what the comparison above is comparing. + expect(objectKeys(parse(ROUTE_FILE), "NOT_A_REAL_CONSTANT")).toEqual([]); + expect(unionMembers(parse(RULE_FILE), "NotARealType")).toEqual([]); + }); +}); diff --git a/src/components/settings/score-section.tsx b/src/components/settings/score-section.tsx index 29768ea0d..f89c37ea2 100644 --- a/src/components/settings/score-section.tsx +++ b/src/components/settings/score-section.tsx @@ -79,6 +79,7 @@ import { } from "@/lib/analytics/score/types"; import type { HealthScoreReport } from "@/lib/analytics/score/types"; import type { ScoreModuleKey } from "@/lib/analytics/score/modules"; +import type { ScoreBreadthFailure } from "@/lib/analytics/score/breadth"; /** The resolved configuration `GET /api/auth/me/health-score-config` returns. */ interface HealthScoreConfigPayload { @@ -99,8 +100,14 @@ interface HealthScoreConfigPayload { * exists for callers without a locale, and rendering it would leave five * of the six locales reading English at exactly the moment the person * needs to understand something. + * + * Typed as a total map over `ScoreBreadthFailure` rather than over + * `string`, so a third refusal reason added to the rule cannot ship with + * this surface quietly falling through to a generic "couldn't save". The + * TYPE is imported, never the rule: the breadth question is answered + * server-side and answered once. */ -const REFUSAL_KEYS: Record = { +export const REFUSAL_KEYS: Record = { measured_physiological_domain_required: "settings.sections.score.refusal.physiological", three_domains_required: "settings.sections.score.refusal.domains", @@ -213,8 +220,13 @@ export function ScoreSection() { // only translates the reason it gave. if (err instanceof ApiError && err.status === 422) { const reason = err.meta?.reason; + // The wire carries an unknown string until it is checked against + // the map; an unrecognised one falls through to the generic + // failure rather than rendering an empty alert. const key = - typeof reason === "string" ? REFUSAL_KEYS[reason] : undefined; + typeof reason === "string" && reason in REFUSAL_KEYS + ? REFUSAL_KEYS[reason as ScoreBreadthFailure] + : undefined; if (key) { setRefusal(key); return;