From c456a236a7c9e83adbb1e1236820d4c4a1df1765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 13:07:34 +0200 Subject: [PATCH] fix(analytics): rank Health Connect and the hand-entered sources on the ladders A source a ladder does not name ranks 90, below every source it does. Five had no place anywhere: a weight typed as a reply to a Telegram reminder lost its day to a WHOOP strap's estimate, one an assistant logged over MCP lost to a Fitbit estimate, and a Health Connect day lost to a hand-typed count. Health Connect now sits right after Apple Health on the ladders for what it imports; EXTERNAL, MCP and TELEGRAM sit right after MANUAL, in the alphabetical order they already broke ties in, so MANUAL still outranks the person's own bridge. Relative to each other the sources the ladders already named keep their order, and saved ladders gain the new entries at their end as before. --- .../workouts/__tests__/batch-scope.test.ts | 12 +- .../__tests__/source-priority.test.ts | 45 +++--- src/lib/i18n/source-labels.ts | 5 +- .../__tests__/source-priority.test.ts | 80 ++++++++++- src/lib/validations/source-priority.ts | 69 +++++++++- .../source-ladder-new-sources.test.ts | 128 ++++++++++++++++++ 6 files changed, 307 insertions(+), 32 deletions(-) create mode 100644 tests/integration/source-ladder-new-sources.test.ts diff --git a/src/app/api/workouts/__tests__/batch-scope.test.ts b/src/app/api/workouts/__tests__/batch-scope.test.ts index 61a063093..091b0ad8f 100644 --- a/src/app/api/workouts/__tests__/batch-scope.test.ts +++ b/src/app/api/workouts/__tests__/batch-scope.test.ts @@ -246,8 +246,18 @@ describe("POST /api/workouts/batch — rate limit buckets", () => { describe("POST /api/workouts/batch — what a scoped caller writes", () => { it("attributes its rows to EXTERNAL", async () => { armToken([WORKOUTS_WRITE_SCOPE]); + // Two sessions of the day: EXTERNAL ranks on the workout ladder, so two + // rows of one session would land once. const res = await POST( - makeRequest({ workouts: [workout("s-1"), workout("s-2")] }), + makeRequest({ + workouts: [ + workout("s-1"), + workout("s-2", { + startedAt: "2026-09-20T18:00:00.000Z", + endedAt: "2026-09-20T18:45:00.000Z", + }), + ], + }), ); expect(res.status).toBe(200); const rows = insertedRows(); diff --git a/src/lib/analytics/__tests__/source-priority.test.ts b/src/lib/analytics/__tests__/source-priority.test.ts index 14abaf374..e7d5bb110 100644 --- a/src/lib/analytics/__tests__/source-priority.test.ts +++ b/src/lib/analytics/__tests__/source-priority.test.ts @@ -519,25 +519,23 @@ describe("pickCanonicalSourceRows — v1.4.25 W8c two-axis device-type picker", /** * v1.38.x — what the EXTERNAL source does to a contested day. * - * `EXTERNAL` is on no priority ladder, deliberately: the `steps` and `spo2` - * defaults already sit at `metricSourceLadder`'s `.max(8)`, so a ninth entry - * would come back from `GET /api/auth/me/source-priority`, be PUT back whole - * by the settings page, and 422 — breaking ladder saves for everyone. + * `EXTERNAL` was on no priority ladder at first, because the ladders were + * capped at eight entries and a ninth would have 422'd every settings save. + * The cap is the number of sources now, and since v1.43.11 `EXTERNAL` sits + * right after `MANUAL` on the ladders that rank `MANUAL`. * - * That omission is not inert. It decides a real behaviour, and one that - * CHANGED when ingested rows stopped being labelled `MANUAL`: before, a typed - * reading and a bridged one were the same source and both survived a day; now - * `MANUAL` ranks, `EXTERNAL` does not, and the bridged row leaves the - * canonical set for that day. Seven call sites consume this picker (daily - * totals, the health score, the doctor report, correlations, mood crosstabs, - * PR detection), so the effect is user-visible and is written up in the - * changelog. + * The behaviour this pins CHANGED when ingested rows stopped being labelled + * `MANUAL`: before, a typed reading and a bridged one were the same source + * and both survived a day; now `MANUAL` outranks `EXTERNAL`, and the bridged + * row leaves the canonical set for that day. Seven call sites consume this + * picker (daily totals, the health score, the doctor report, correlations, + * mood crosstabs, PR detection), so the effect is user-visible. * * Pinned here because it is emergent from a rank ladder rather than stated - * anywhere in code: put `EXTERNAL` on a ladder and the first case below goes - * red, which is the review anybody adding it deserves. + * anywhere in code: rank `EXTERNAL` above `MANUAL` and the first case below + * goes red, which is the review anybody doing that deserves. */ -describe("pickCanonicalSourceRows — the unranked EXTERNAL source", () => { +describe("pickCanonicalSourceRows — the EXTERNAL source", () => { const day = "2026-09-02"; function weightRow( @@ -573,10 +571,9 @@ describe("pickCanonicalSourceRows — the unranked EXTERNAL source", () => { expect(out.pickedByDay.get(day)).toBe("MANUAL"); }); - it("keeps a bridged-only day, since nothing on the ladder is present", () => { - // The `!picked` pass-through. Without this arm an account that ONLY ever - // pushes readings would go dark on every surface listed above, which - // would be a far worse regression than the one above is a change. + it("keeps a bridged-only day, picked from the ladder", () => { + // An account that ONLY ever pushes readings keeps every one of them: + // the bridge is the day's one source, and the ladder now names it. const out = pickCanonicalSourceRows( [ weightRow("EXTERNAL", 82.4, "06:30"), @@ -588,8 +585,7 @@ describe("pickCanonicalSourceRows — the unranked EXTERNAL source", () => { ); expect(out.canonicalRows).toHaveLength(2); expect(out.canonicalRows.every((r) => r.source === "EXTERNAL")).toBe(true); - // No ladder source won, so the day names no pick. - expect(out.pickedByDay.has(day)).toBe(false); + expect(out.pickedByDay.get(day)).toBe("EXTERNAL"); }); it("kept both when the bridged row was still labelled MANUAL", () => { @@ -607,10 +603,9 @@ describe("pickCanonicalSourceRows — the unranked EXTERNAL source", () => { expect(out.canonicalRows).toHaveLength(2); }); - it("lets a user who ranks EXTERNAL themselves have it win", () => { - // Not reachable from the settings UI today (the ladder cap keeps it off - // the page), but the picker resolves whatever ladder it is handed, and a - // stored ladder naming it must be honoured rather than ignored. + it("lets a user who ranks EXTERNAL above MANUAL have it win", () => { + // The picker resolves whatever ladder it is handed; a stored ladder that + // puts the bridge first is honoured. const out = pickCanonicalSourceRows( [ weightRow("EXTERNAL", 82.4, "06:30"), diff --git a/src/lib/i18n/source-labels.ts b/src/lib/i18n/source-labels.ts index a8ef4fb2e..57e3bc5f6 100644 --- a/src/lib/i18n/source-labels.ts +++ b/src/lib/i18n/source-labels.ts @@ -40,9 +40,8 @@ export const MEASUREMENT_SOURCE_SETTINGS_LABEL_KEYS: Record< MCP: "settings.sections.sources.sourceLabels.MCP", GOOGLE_HEALTH: "settings.sections.sources.sourceLabels.GOOGLE_HEALTH", STRAVA: "settings.sections.sources.sourceLabels.STRAVA", - // v1.38.x — readings pushed in under a `measurements:write` token. - // Never offered in the ladder (it is on no default priority list), but - // the map is keyed by the enum, so the entry is not optional. + // v1.38.x — readings pushed in under a `measurements:write` token. On the + // default ladders right after MANUAL since v1.43.11. EXTERNAL: "settings.sections.sources.sourceLabels.EXTERNAL", HEALTH_CONNECT: "settings.sections.sources.sourceLabels.HEALTH_CONNECT", }; diff --git a/src/lib/validations/__tests__/source-priority.test.ts b/src/lib/validations/__tests__/source-priority.test.ts index 9db9585f8..aa3b205e4 100644 --- a/src/lib/validations/__tests__/source-priority.test.ts +++ b/src/lib/validations/__tests__/source-priority.test.ts @@ -533,6 +533,84 @@ describe("parseSourcePriority — duplicate tolerance (v1.16.11)", () => { const ladder = resolved.metricPriority.weight; expect(new Set(ladder).size).toBe(ladder.length); expect(ladder.slice(0, 2)).toEqual(["MANUAL", "WITHINGS"]); - expect(ladder.length).toBeLessThanOrEqual(8); + // The schema caps a ladder at one entry per source. + expect(ladder.length).toBeLessThanOrEqual( + measurementSourceEnum.options.length, + ); + }); +}); + +describe("DEFAULT_SOURCE_PRIORITY — the sources that had no place (v1.43.11)", () => { + const HAND_ENTERED = ["EXTERNAL", "MCP", "TELEGRAM"] as const; + // What Health Connect imports, by ladder (`import/health-connect/mapper.ts`). + const HEALTH_CONNECT_LADDERS = [ + "steps", + "activeEnergy", + "walkingRunningDistance", + "bloodPressure", + "bodyFat", + "bodyTemperature", + "hrvRmssd", + "spo2", + "respiratoryRate", + "restingHeartRate", + "vo2Max", + "weight", + ] as const; + + it("ranks the hand-entered sources right after MANUAL wherever MANUAL ranks", () => { + for (const [key, ladder] of Object.entries(DEFAULT_SOURCE_PRIORITY)) { + const i = (ladder as string[]).indexOf("MANUAL"); + if (i === -1) continue; + expect(ladder.slice(i + 1, i + 1 + HAND_ENTERED.length), key).toEqual([ + ...HAND_ENTERED, + ]); + } + }); + + it("ranks Health Connect right after Apple Health on every ladder it imports for", () => { + for (const key of HEALTH_CONNECT_LADDERS) { + const ladder = DEFAULT_SOURCE_PRIORITY[key] as string[]; + expect(ladder[ladder.indexOf("APPLE_HEALTH") + 1], key).toBe( + "HEALTH_CONNECT", + ); + } + }); + + it("leaves the order of the sources the ladders already named alone", () => { + const added = new Set([...HAND_ENTERED, "HEALTH_CONNECT"]); + const without = (key: keyof typeof DEFAULT_SOURCE_PRIORITY) => + DEFAULT_SOURCE_PRIORITY[key].filter((s) => !added.has(s)); + expect(without("weight")).toEqual([ + "WITHINGS", + "APPLE_HEALTH", + "MANUAL", + "WHOOP", + "FITBIT", + "GOOGLE_HEALTH", + ]); + expect(without("bloodPressure")).toEqual([ + "WITHINGS", + "APPLE_HEALTH", + "MANUAL", + ]); + expect(without("spo2")).toEqual([ + "WITHINGS", + "WHOOP", + "FITBIT", + "GOOGLE_HEALTH", + "OURA", + "POLAR", + "APPLE_HEALTH", + "MANUAL", + ]); + }); + + it("fits every default ladder within the schema cap", () => { + for (const [key, ladder] of Object.entries(DEFAULT_SOURCE_PRIORITY)) { + expect(ladder.length, key).toBeLessThanOrEqual( + measurementSourceEnum.options.length, + ); + } }); }); diff --git a/src/lib/validations/source-priority.ts b/src/lib/validations/source-priority.ts index 6ad587dbd..da0bb5a2f 100644 --- a/src/lib/validations/source-priority.ts +++ b/src/lib/validations/source-priority.ts @@ -234,6 +234,16 @@ export type DeviceTypePriority = z.infer; * pulse-ox, Thermo). Apple Health is second-hand (HealthKit * receives the same reading via Withings' Health Mate iOS app). */ +// v1.43.11 — the sources that had no place on any ladder. A source a ladder +// does not name ranks 90, below everything it does, so a typed weight from a +// Telegram reminder lost its day to a WHOOP strap's estimate, and a Health +// Connect day lost to every other source. Health Connect is the Android +// counterpart of the HealthKit relay and sits right after APPLE_HEALTH on +// the ladders for what it imports; the readings a person enters or bridges +// themselves (EXTERNAL: their own hardware's uploader, MCP: an assistant +// writing on their word, TELEGRAM: a reply to a reminder) sit right after +// MANUAL, in the alphabetical order they already broke ties in. Relative to +// each other the existing sources keep their order. export const DEFAULT_SOURCE_PRIORITY: Required = { // v1.12.0 — Fitbit/Pixel rides below Apple Health for cumulative metrics: // HealthKit still aggregates the broadest device set, and a Fitbit-only @@ -253,6 +263,7 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { // still survives. steps: [ "APPLE_HEALTH", + "HEALTH_CONNECT", "WITHINGS", "FITBIT", "GOOGLE_HEALTH", @@ -260,24 +271,35 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "POLAR", "STRAVA", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", ], activeEnergy: [ "APPLE_HEALTH", + "HEALTH_CONNECT", "WITHINGS", "FITBIT", "GOOGLE_HEALTH", "OURA", "POLAR", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", ], walkingRunningDistance: [ "APPLE_HEALTH", + "HEALTH_CONNECT", "WITHINGS", "FITBIT", "GOOGLE_HEALTH", "OURA", "POLAR", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", ], flightsClimbed: [ "APPLE_HEALTH", @@ -285,6 +307,9 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "FITBIT", "GOOGLE_HEALTH", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", ], // v1.11.0 — WHOOP leads the recovery-input ladders (sleep / HRV / RHR): // a worn-all-night strap has higher-resolution overnight sampling than @@ -319,18 +344,31 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "OURA", "POLAR", "APPLE_HEALTH", + "HEALTH_CONNECT", "WITHINGS", ], // A real scale beats a strap's body-measurement estimate for weight. weight: [ "WITHINGS", "APPLE_HEALTH", + "HEALTH_CONNECT", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", "WHOOP", "FITBIT", "GOOGLE_HEALTH", ], - bloodPressure: ["WITHINGS", "APPLE_HEALTH", "MANUAL"], + bloodPressure: [ + "WITHINGS", + "APPLE_HEALTH", + "HEALTH_CONNECT", + "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", + ], // v1.17.0 — continuous optical-HR wearables (Fitbit / Polar / Oura) rank // above a hand-typed MANUAL pulse; the device-source order matches the // rhr / hrv ladders (FITBIT > OURA > POLAR). Withings BPM cuff + the @@ -343,12 +381,29 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "OURA", "POLAR", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", + ], + bodyFat: [ + "WITHINGS", + "APPLE_HEALTH", + "HEALTH_CONNECT", + "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", + "FITBIT", + "GOOGLE_HEALTH", ], - bodyFat: ["WITHINGS", "APPLE_HEALTH", "MANUAL", "FITBIT", "GOOGLE_HEALTH"], bodyTemperature: [ "WITHINGS", "APPLE_HEALTH", + "HEALTH_CONNECT", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", "FITBIT", "GOOGLE_HEALTH", ], @@ -363,15 +418,23 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "OURA", "POLAR", "APPLE_HEALTH", + "HEALTH_CONNECT", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", ], vo2Max: [ "WITHINGS", "APPLE_HEALTH", + "HEALTH_CONNECT", "FITBIT", "GOOGLE_HEALTH", "OURA", "MANUAL", + "EXTERNAL", + "MCP", + "TELEGRAM", ], // v1.11.0 — new WHOOP-overlapping keys. ScanWatch dermal reading is the // primary skin-temperature sensor; WHOOP's strap is second, Fitbit third, @@ -386,6 +449,7 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "OURA", "POLAR", "APPLE_HEALTH", + "HEALTH_CONNECT", "WITHINGS", ], // v1.11.0 — native-vs-derived recovery. WHOOP's device-native Recovery @@ -414,6 +478,7 @@ export const DEFAULT_SOURCE_PRIORITY: Required = { "OURA", "POLAR", "APPLE_HEALTH", + "HEALTH_CONNECT", ], }; diff --git a/tests/integration/source-ladder-new-sources.test.ts b/tests/integration/source-ladder-new-sources.test.ts new file mode 100644 index 000000000..0819ebff8 --- /dev/null +++ b/tests/integration/source-ladder-new-sources.test.ts @@ -0,0 +1,128 @@ +/** + * The sources that had no place on a ladder, against real Postgres through + * the SQL collapse every 90-day aggregate reads (`canonicalMeasurementsFrom`). + * + * A source a ladder does not name ranks 90, below everything it does name: + * a weight typed as a reply to a Telegram reminder lost its day to a WHOOP + * strap's estimate, and a Health Connect day lost to a hand-typed one. The + * hand-entered sources now rank with MANUAL, Health Connect with Apple + * Health, and a day between two sources that were on the ladder before + * resolves exactly as it did. + */ +import { beforeAll, describe, expect, it, vi } from "vitest"; + +import { getPrismaClient, truncateAllTables } from "./setup"; +import type { + MeasurementSource, + MeasurementType, +} from "@/generated/prisma/client"; +import { + buildSourceRankCase, + canonicalMeasurementsFrom, +} from "@/lib/analytics/source-rank-sql"; + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +const prisma = getPrismaClient(); +const USER = "ladder-new-sources"; +const DAY = 86_400_000; +const today = Math.floor(Date.now() / DAY) * DAY; + +/** One reading per (type, source) on day `n` days back, at 08:00 / 09:00 UTC. */ +function reading( + type: MeasurementType, + source: MeasurementSource, + n: number, + value: number, + hour = 8, +) { + return { + userId: USER, + type, + source, + value, + unit: type === "WEIGHT" ? "kg" : "count", + measuredAt: new Date(today - n * DAY + hour * 3_600_000), + }; +} + +async function canonicalSources(): Promise> { + const rank = buildSourceRankCase(null, '"type"', '"source"'); + const rows = await prisma.$queryRawUnsafe< + Array<{ type: string; day: Date; source: string }> + >( + `SELECT DISTINCT m."type"::text AS type, + date_trunc('day', m."measured_at") AS day, + m."source"::text AS source + FROM ${canonicalMeasurementsFrom(rank, "90 days")} + ORDER BY 1, 2, 3`, + USER, + ); + const out = new Map(); + for (const r of rows) { + const key = `${r.type}@${r.day.toISOString().slice(0, 10)}`; + out.set(key, [...(out.get(key) ?? []), r.source]); + } + return out; +} + +const dayKey = (n: number) => + new Date(today - n * DAY).toISOString().slice(0, 10); + +beforeAll(async () => { + await truncateAllTables(prisma); + await prisma.user.create({ + data: { id: USER, username: USER, timezone: "UTC" }, + }); + await prisma.measurement.createMany({ + data: [ + // Day 1: a strap's weight estimate and a typed reply to a reminder. + reading("WEIGHT", "WHOOP", 1, 84), + reading("WEIGHT", "TELEGRAM", 1, 82.5, 9), + // Day 2: an assistant logging on the person's word beside a Fitbit + // scale-less estimate. + reading("WEIGHT", "FITBIT", 2, 85), + reading("WEIGHT", "MCP", 2, 82.4, 9), + // Day 3: a hand-typed weight and the person's own bridged scale. + reading("WEIGHT", "MANUAL", 3, 82.3), + reading("WEIGHT", "EXTERNAL", 3, 82.6, 9), + // Day 4: two sources the ladder always named: unchanged. + reading("WEIGHT", "APPLE_HEALTH", 4, 82.2), + reading("WEIGHT", "WITHINGS", 4, 82.1, 9), + // Day 1: Health Connect's daily steps beside a hand-typed count. + reading("ACTIVITY_STEPS", "HEALTH_CONNECT", 1, 9000), + reading("ACTIVITY_STEPS", "MANUAL", 1, 5000, 9), + // Day 2: Apple Health still outranks Health Connect. + reading("ACTIVITY_STEPS", "APPLE_HEALTH", 2, 9100), + reading("ACTIVITY_STEPS", "HEALTH_CONNECT", 2, 8900, 9), + ], + }); +}, 120_000); + +describe("source ladders for the sources that had no place", () => { + it("ranks a reading the person entered through Telegram or MCP above a device estimate", async () => { + const picked = await canonicalSources(); + expect(picked.get(`WEIGHT@${dayKey(1)}`)).toEqual(["TELEGRAM"]); + expect(picked.get(`WEIGHT@${dayKey(2)}`)).toEqual(["MCP"]); + }); + + it("keeps MANUAL above the person's own bridge", async () => { + const picked = await canonicalSources(); + expect(picked.get(`WEIGHT@${dayKey(3)}`)).toEqual(["MANUAL"]); + }); + + it("ranks Health Connect right after Apple Health", async () => { + const picked = await canonicalSources(); + expect(picked.get(`ACTIVITY_STEPS@${dayKey(1)}`)).toEqual([ + "HEALTH_CONNECT", + ]); + expect(picked.get(`ACTIVITY_STEPS@${dayKey(2)}`)).toEqual(["APPLE_HEALTH"]); + }); + + it("resolves a day between two long-ranked sources exactly as before", async () => { + const picked = await canonicalSources(); + expect(picked.get(`WEIGHT@${dayKey(4)}`)).toEqual(["WITHINGS"]); + }); +});