From 00c282d311906a93c0d1c658b05c6c9de64facf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 30 Jul 2026 11:54:58 +0200 Subject: [PATCH 1/3] fix(workouts): collapse a re-sent session on every surface, not just the list The canonical picker keyed a session on sport, start, end and duration. Only the workout list selected the end timestamp; the Coach narration, the per-sport baseline and the deep-link cluster passed rows without it, so those rows carried no session identity at all and the duplicate survived. One record counted once in the list and two or three times everywhere else, including in the averages a person is compared against. Drop the end timestamp from the identity. On a workout with pauses it measures wall-clock time while the duration counts only moving time, so two re-sends of one session can differ in that field alone and the stricter key rejected a genuine match. It also bought nothing in exchange: nobody starts a second workout of the same sport in the same instant with the same duration. The deep-link cluster now selects the duration it needs. A structural guard freezes the call-site set and records which file owns each caller's select, so a new surface cannot join the set without fetching the identity, and the identity cannot grow a field only some callers fetch. --- .../workout-canonical-identity-guard.test.ts | 120 ++++++++++++++++++ src/app/api/workouts/[id]/route.ts | 5 + .../pick-canonical-workout-rows.test.ts | 62 +++++++++ .../pick-canonical-workout-rows.ts | 41 ++++-- 4 files changed, 216 insertions(+), 12 deletions(-) create mode 100644 src/__tests__/workout-canonical-identity-guard.test.ts diff --git a/src/__tests__/workout-canonical-identity-guard.test.ts b/src/__tests__/workout-canonical-identity-guard.test.ts new file mode 100644 index 000000000..7c976d1ec --- /dev/null +++ b/src/__tests__/workout-canonical-identity-guard.test.ts @@ -0,0 +1,120 @@ +/** + * Structural guard on the canonical-workout session identity. + * + * `pickCanonicalWorkoutRows()` collapses a HealthKit session that arrived more + * than once under different external UUIDs. It can only do that when the row it + * is handed carries the identity fields, and the row shape comes from each + * caller's own Prisma `select`. Nothing in the type system connects the two: + * `durationSec` is optional on `WorkoutPickerRow`, so a caller that omits it + * gets silent passthrough rather than a compile error. + * + * That is exactly what happened. When the collapse first shipped, the key was + * `(sportType, startedAt, endedAt, durationSec)` and only the workout list + * selected `endedAt`. The list showed one workout while the Coach narrated the + * duplicate, the per-sport baseline averaged it twice and the deep-link cluster + * kept both rows. One record, four surfaces, two answers. + * + * These are tripwires, not proofs. They cannot show the collapse is correct — + * that is what the unit tests do. What they can do is refuse to let a new call + * site join the set without someone editing this file, and refuse to let the + * identity quietly grow a field that only some callers fetch. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { globSync } from "node:fs"; +import { join, sep } from "node:path"; + +const SRC = join(process.cwd(), "src"); + +/** The picker module itself — declares the identity, does not consume it. */ +const PICKER = "lib/measurements/pick-canonical-workout-rows.ts"; + +/** + * Every production call site of the read-time picker, frozen. + * + * Adding a surface that collapses workouts is a deliberate act: it must fetch + * the identity fields or it silently disagrees with every other surface. Adding + * the file here is the acknowledgement. + */ +const IDENTITY_OWNER: Record = { + "app/api/workouts/[id]/route.ts": "app/api/workouts/[id]/route.ts", + "lib/jobs/workout-insight-generate.ts": + "lib/jobs/workout-insight-generate.ts", + "lib/workouts/list-read.ts": "lib/workouts/list-read.ts", + "lib/workouts/sport-context.ts": "lib/workouts/sport-context.ts", + // The only indirection in the set. The Coach block is handed rows it does not + // read itself; the snapshot builder fetches them in parallel with every other + // block's data. Recording the owner here rather than assuming the call site + // owns its own select is the whole point of this map. + "lib/ai/coach/snapshot-blocks/workouts-block.ts": "lib/ai/coach/snapshot.ts", +}; + +const EXPECTED_CALL_SITES = Object.keys(IDENTITY_OWNER).sort(); + +function sourceFiles(): string[] { + return globSync("**/*.{ts,tsx}", { cwd: SRC }) + .map((p) => p.split(sep).join("/")) + .filter((p) => !p.startsWith("generated/")) + .filter((p) => !p.includes("__tests__")) + .filter((p) => !p.endsWith(".test.ts") && !p.endsWith(".test.tsx")) + .sort(); +} + +function read(rel: string): string { + return readFileSync(join(SRC, rel), "utf8"); +} + +/** + * Lines that actually invoke the picker. Comment lines are excluded because the + * write-time sibling (`lib/workouts/canonical-rows.ts`) names the read-time + * picker in its docblock to explain how the two layer, and that mention is + * documentation rather than a call. + */ +function invokesPicker(source: string): boolean { + return source + .split("\n") + .some( + (line) => + line.includes("pickCanonicalWorkoutRows(") && + !/^\s*(\*|\/\/|\/\*)/.test(line), + ); +} + +describe("the picker's call-site set is frozen", () => { + it("has exactly the call sites this file acknowledges", () => { + const found = sourceFiles() + .filter((rel) => rel !== PICKER) + .filter((rel) => invokesPicker(read(rel))); + + expect(found).toEqual(EXPECTED_CALL_SITES); + }); +}); + +describe("every call site fetches the session identity", () => { + it.each(EXPECTED_CALL_SITES)("%s gets rows carrying durationSec", (rel) => { + // File-level rather than per-select: a caller that fetches workout rows + // without their duration cannot collapse a re-send, whichever select the + // rows came from. Coarse in the safe direction — it can only fail to catch + // a second unrelated select in the same file, never pass a caller that + // fetches no duration at all. + expect(read(IDENTITY_OWNER[rel])).toContain("durationSec: true"); + }); +}); + +describe("the identity cannot quietly grow a field callers do not fetch", () => { + it("keys the session on sportType, startedAt and durationSec only", () => { + const picker = read(PICKER); + const body = picker.slice( + picker.indexOf("function exactSessionKey"), + picker.indexOf("function rowRichness"), + ); + + expect(body).toContain("row.sportType"); + expect(body).toContain("row.startedAt"); + expect(body).toContain("row.durationSec"); + // `endedAt` is the specific field that split the callers. On a workout with + // pauses it disagrees with `durationSec` by design, so it rejected genuine + // re-sends while forcing every caller to fetch a fourth column. + expect(body).not.toContain("endedAt"); + }); +}); diff --git a/src/app/api/workouts/[id]/route.ts b/src/app/api/workouts/[id]/route.ts index 97fd208d5..439dbe5f2 100644 --- a/src/app/api/workouts/[id]/route.ts +++ b/src/app/api/workouts/[id]/route.ts @@ -114,6 +114,11 @@ export const GET = apiHandler( source: true, startedAt: true, sportType: true, + // Part of the canonical session identity consulted by + // `pickCanonicalWorkoutRows` below — without it a HealthKit re-send of + // this very workout stays in the cluster and the deep link can resolve + // to the duplicate instead of the row the list shows. + durationSec: true, avgHeartRate: true, maxHeartRate: true, metadata: true, diff --git a/src/lib/measurements/__tests__/pick-canonical-workout-rows.test.ts b/src/lib/measurements/__tests__/pick-canonical-workout-rows.test.ts index be2f4f13d..29e43e414 100644 --- a/src/lib/measurements/__tests__/pick-canonical-workout-rows.test.ts +++ b/src/lib/measurements/__tests__/pick-canonical-workout-rows.test.ts @@ -277,6 +277,68 @@ describe("pickCanonicalWorkoutRows", () => { expect(result[0].id).toBe("duplicate-b"); }); + it("collapses a re-send whose endedAt disagrees with the first arrival", () => { + // A workout with pauses has a wall-clock end that is longer than the + // counted duration, and the two sources of that number do not always agree + // across re-sends. `endedAt` was part of the session key when the collapse + // first shipped, so this exact pair survived as two rows. + const base = { + startedAt: new Date("2026-07-29T12:54:00Z"), + durationSec: 480, + sportType: "cycling", + source: "APPLE_HEALTH" as const, + }; + const result = pickCanonicalWorkoutRows([ + { + ...base, + id: "first-arrival", + endedAt: new Date("2026-07-29T13:02:00Z"), + }, + { + ...base, + id: "resend", + endedAt: new Date("2026-07-29T13:09:00Z"), + avgHeartRate: 132, + }, + ]); + + expect(result).toHaveLength(1); + expect(result[0].id).toBe("resend"); + }); + + it("keeps two same-source sessions whose durations differ", () => { + // The counterpart guarantee: the looser key must not merge genuinely + // distinct sessions that happen to share a bucket slot and a sport. + const base = { + startedAt: new Date("2026-07-29T12:54:00Z"), + sportType: "strength", + source: "APPLE_HEALTH" as const, + }; + const result = pickCanonicalWorkoutRows([ + { ...base, id: "short", durationSec: 480 }, + { ...base, id: "long", durationSec: 1800 }, + ]); + + expect(result.map((row) => row.id).sort()).toEqual(["long", "short"]); + }); + + it("passes rows without a duration through rather than guessing", () => { + // Fixture-only path: a row with no duration carries no session identity, so + // it cannot be collapsed. Every production caller selects the field and + // `workout-canonical-identity-guard.test.ts` keeps that true. + const base = { + startedAt: new Date("2026-07-29T12:54:00Z"), + sportType: "yoga", + source: "APPLE_HEALTH" as const, + }; + const result = pickCanonicalWorkoutRows([ + { ...base, id: "no-duration-a" }, + { ...base, id: "no-duration-b" }, + ]); + + expect(result).toHaveLength(2); + }); + it("does not collapse or enrich rows belonging to different users", () => { const result = pickCanonicalWorkoutRows([ { diff --git a/src/lib/measurements/pick-canonical-workout-rows.ts b/src/lib/measurements/pick-canonical-workout-rows.ts index 31fe6dce2..cb97fd0ad 100644 --- a/src/lib/measurements/pick-canonical-workout-rows.ts +++ b/src/lib/measurements/pick-canonical-workout-rows.ts @@ -46,8 +46,12 @@ import { export interface WorkoutPickerRow { userId?: string; startedAt: Date; - /** Optional exact-session fields used to collapse repeated same-source sync rows. */ - endedAt?: Date; + /** + * Session length. Together with `sportType` + `startedAt` this is the + * exact-session identity used to collapse repeated same-source sync rows. + * Optional only so fixtures can omit it; every production call site MUST + * select it, and `workout-canonical-identity-guard.test.ts` freezes that set. + */ durationSec?: number; sportType: string; source: MeasurementSource; @@ -60,18 +64,31 @@ export interface WorkoutPickerRow { * HealthKit can resend the same session with a new external UUID. The source * ladder correctly chooses Apple over WHOOP, but previously kept every Apple * row in that winning bucket, which made the web list show the same workout - * two or three times. Only collapse rows when the exact session identity is - * available; fixtures and legacy callers without end/duration remain - * governed by the original bucket contract. + * two or three times. + * + * The identity is `(sportType, startedAt, durationSec)`. `endedAt` was part of + * this key when the collapse first shipped and has been dropped deliberately: + * + * - It is not a free extra check. On a workout with pauses, `endedAt` is the + * wall-clock end while `durationSec` counts only moving time, so the two + * legitimately disagree, and a stored row can carry either convention. Two + * re-sends of one session can therefore differ in `endedAt` alone, and the + * stricter key let that duplicate through. + * - It bought nothing in exchange. Nobody starts a second workout of the + * same sport in the same instant with the same duration, so start plus + * duration already identifies the session uniquely. + * - Requiring it split the callers. Four of the five call sites select + * `durationSec` but not `endedAt`, so the collapse ran on the workout list + * and silently did nothing for the Coach narration, the per-sport baseline + * and the deep-link cluster. One list, one number, two answers. + * + * A row without `durationSec` cannot be identified and passes through + * uncollapsed. That is a fixture-only path: every production call site selects + * the field and `workout-canonical-identity-guard.test.ts` keeps it that way. */ function exactSessionKey(row: WorkoutPickerRow): string | null { - if (row.endedAt == null || row.durationSec == null) return null; - return [ - row.sportType, - row.startedAt.getTime(), - row.endedAt.getTime(), - row.durationSec, - ].join(":"); + if (row.durationSec == null) return null; + return [row.sportType, row.startedAt.getTime(), row.durationSec].join(":"); } function rowRichness(row: WorkoutPickerRow): number { From 17ea7bea2d56f8edc652748d7bf1e192903da5d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 30 Jul 2026 11:55:16 +0200 Subject: [PATCH 2/3] fix(sw): re-anchor the fallback cache version and gate it against the release The service worker derives its cache names from the version injected by `sw-version.js`, and falls back to a literal compiled into `sw.js` when that import fails. The prebuild step rewrites both from `package.json`, so both were correct as long as a release rebuilt. One did not, and the literal trailed the release by a version while every test stayed green. Nothing checked it. The generated file is compared against `package.json`, and the behavioural suite deliberately reads the literal out of the source so its cache-name assertions cannot drift from the literal they exercise. Reasonable apart, and together they left the literal itself unverified. The effect is confined to the degraded path and is still worth closing: a client that cannot load `sw-version.js` computes the previous release's cache names, so it keeps serving the old shell and its cleanup treats the current caches as the stale ones. Re-anchors the literal and adds the equality as a gate, since it is a question with an exact answer. A version bump committed without a rebuild now fails locally instead of shipping. --- public/sw.js | 2 +- .../service-worker-version-anchor.test.ts | 61 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 src/__tests__/service-worker-version-anchor.test.ts diff --git a/public/sw.js b/public/sw.js index 499698631..4095c17dd 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.34.0"; + /* @sw-version-fallback */ "v1.34.1"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*` diff --git a/src/__tests__/service-worker-version-anchor.test.ts b/src/__tests__/service-worker-version-anchor.test.ts new file mode 100644 index 000000000..201204c29 --- /dev/null +++ b/src/__tests__/service-worker-version-anchor.test.ts @@ -0,0 +1,61 @@ +/** + * The service worker's fallback version must equal the release version. + * + * `public/sw.js` derives its cache names from `self.__APP_VERSION__`, injected by + * `public/sw-version.js`, which `scripts/generate-sw-version.mjs` writes from + * `package.json` on every prebuild. When `importScripts('/sw-version.js')` + * fails, the SW falls back to a literal compiled into `sw.js` behind the + * `@sw-version-fallback` anchor, and the same prebuild step rewrites that + * literal too. + * + * Two places therefore declare one version, and until this file existed nothing + * made them agree. `scripts/__tests__/generate-sw-version.test.ts` checks the + * GENERATED file against `package.json`; the behavioural suite in + * `service-worker.test.ts` reads the fallback literal out of the source so its + * cache-name assertions "never drift from the literal it is exercising". Both + * are reasonable in isolation and together they leave the literal unchecked: a + * release whose version was bumped without a rebuild ships a fallback pointing + * at the PREVIOUS release, and every test stays green. + * + * The consequence is small but real, and it only bites on the degraded path. A + * client that cannot load `/sw-version.js` computes last release's cache names, + * so it keeps serving the previous shell and its cleanup step treats the current + * caches as the stale ones. + * + * This is a one-line equality, which makes it a gate rather than a review aid. + * Failing it means the tree needs a rebuild before it is committed, which is + * exactly the step whose absence caused the drift. + */ +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const ROOT = process.cwd(); + +function swFallbackVersion(): string { + const source = readFileSync(join(ROOT, "public/sw.js"), "utf8"); + const match = /\/\* @sw-version-fallback \*\/\s*"(v[^"]*)"/.exec(source); + if (!match) { + throw new Error( + "no @sw-version-fallback anchor in public/sw.js — the prebuild generator keys on it", + ); + } + return match[1]; +} + +function packageVersion(): string { + const pkg = JSON.parse( + readFileSync(join(ROOT, "package.json"), "utf8"), + ) as Record; + const version = pkg.version; + if (typeof version !== "string") { + throw new Error("package.json carries no string version"); + } + return version; +} + +describe("the service-worker fallback version is anchored to the release", () => { + it("matches package.json", () => { + expect(swFallbackVersion()).toBe(`v${packageVersion()}`); + }); +}); From b8c9a519c115757981b298f42d3b51f09d7d1455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 30 Jul 2026 11:58:20 +0200 Subject: [PATCH 3/3] =?UTF-8?q?chore(release):=20v1.34.2=20=E2=80=94=20cou?= =?UTF-8?q?nt=20a=20re-sent=20workout=20once=20on=20every=20surface;=20re-?= =?UTF-8?q?anchor=20the=20service-worker=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries the four fixes that landed after the v1.34.1 tag plus the two found while reviewing them. The workout collapse that shipped in that batch only worked on the workout list, because it keyed a session on a field three of the five callers never fetched. Those three kept averaging one session into a person's baseline more than once. The service-worker fallback version had drifted a release behind with nothing checking it; both halves now have a gate. --- CHANGELOG.md | 29 +++++++++++++++++++++++++++++ docs/api/openapi.yaml | 2 +- package.json | 2 +- public/sw.js | 2 +- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bcb5e8857..e99571f23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,35 @@ ## [Unreleased] +## [1.34.2] — 2026-07-30 + +Point release. One workout that arrived more than once, and several surfaces +that were reading the same record differently from one another. + +### Fixed + +- Badminton recorded through Google Health keeps its sport instead of arriving + as "Other". Reported by @tarantila. +- A workout the same source sent twice is now counted once everywhere. The + workout list already collapsed the repeat, and the Coach narration, the + per-sport comparison and the workout deep link did not, so one session could + be folded two or three times into the averages a person is measured against. + The three that missed it were fetching the workout without its duration, and + the identity check needed that field. +- Insights prose renders at full contrast, and the two assessment cards share + one internal rhythm again after one of them drifted away from it. +- The dashboard headline survives for anyone who has switched the score lead + off. +- The service worker's fallback cache version follows the release again. It had + slipped a version behind, which only shows up in a browser that cannot load + the version file, and that browser then kept serving the previous release's + cached pages. + +### Credit + +Reported by @tarantila, directly rather than on the tracker: the badminton +mapping, following on from the workout-type loss in v1.34.0. + ## [1.34.1] — 2026-07-29 Security and reliability point release. diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index d6817840d..5938e2f0b 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: HealthLog API - version: 1.34.1 + version: 1.34.2 description: >- Self-hosted personal-health-tracking PWA — public API surface for the iOS native client and external ingest. diff --git a/package.json b/package.json index b35f9000a..7e94e47ee 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "healthlog", - "version": "1.34.1", + "version": "1.34.2", "description": "Self-hosted personal-health-tracking PWA with Withings integration, AI insights, and doctor-report PDF export.", "license": "PolyForm-Noncommercial-1.0.0", "homepage": "https://healthlog.dev", diff --git a/public/sw.js b/public/sw.js index 4095c17dd..c247424d4 100644 --- a/public/sw.js +++ b/public/sw.js @@ -36,7 +36,7 @@ try { // v1.4.38.4 → v1.4.42. Do not hand-edit; bump `package.json` and rebuild. const CACHE_VERSION = (typeof self !== "undefined" && self.__APP_VERSION__) || - /* @sw-version-fallback */ "v1.34.1"; + /* @sw-version-fallback */ "v1.34.2"; const STATIC_CACHE = `healthlog-static-${CACHE_VERSION}`; const PAGE_CACHE = `healthlog-pages-${CACHE_VERSION}`; // v1.18.6 — read-only data cache for a curated allowlist of safe GET `/api/*`