diff --git a/CHANGELOG.md b/CHANGELOG.md index 953226978..fa7fb1af2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,8 +22,16 @@ the corrected figure the next time you open it, because nothing was copied. A figure nobody recorded reads as not recorded rather than as zero, and a module you have switched off leaves its block out entirely instead of - showing empty rows. Body symptoms stay in the illness journal, linked rather - than duplicated. + showing empty rows: sleep answers to the sleep module, resting heart rate + and heart-rate variability to recovery, the body line to the illness + journal. Steps and active energy have no module of their own and are always + shown, which is how the rest of the app already treats them. Body symptoms + stay in the illness journal, linked rather than duplicated. + + Where two devices reported the same day, the figures follow the source order + you set rather than adding both together. A phone and a watch both counting + your steps is the ordinary case, and summing the pair would have shown a day + you did not walk. - Where the app compares the day's context to the day's mood, it names how many days each side of the comparison rests on, right beside the sentence, diff --git a/src/lib/mood/__tests__/linked-context-ownership.test.ts b/src/lib/mood/__tests__/linked-context-ownership.test.ts new file mode 100644 index 000000000..fcc89320c --- /dev/null +++ b/src/lib/mood/__tests__/linked-context-ownership.test.ts @@ -0,0 +1,62 @@ +/** + * The linked block's gating, checked against the one ownership table. + * + * This file exists because the first draft of `linked-context.ts` hand-wrote + * that map and got it wrong in both directions at once: it gated ambient steps + * and active energy on `workouts`, which hides movement behind a toggle the + * user never associated with it, and it declared resting heart rate and HRV + * "core, no owner", which served the `recovery` domain to accounts that had + * switched it off. Neither was catchable by any test that existed, because + * both ends of the claim lived in the same file. + * + * So the claims are pinned to `moduleForMeasurementType()` — the table the MCP + * wire, the correlations reader and the Coach snapshot all resolve through. + * The resolver reads it at runtime, and these assertions say what it must keep + * answering for the block shapes to stay honest. + * + * Mutation check: assign `ACTIVITY_STEPS` an owner in + * `measurement-scope.ts` and the ambient row goes red; move + * `HEART_RATE_VARIABILITY` to a different module from `RESTING_HEART_RATE` and + * the shared-owner row goes red naming both. + */ +import { describe, expect, it } from "vitest"; + +import { moduleForMeasurementType } from "@/lib/modules/measurement-scope"; + +describe("linked day context — module ownership", () => { + it("keeps sleep owned by the sleep module", () => { + expect(moduleForMeasurementType("SLEEP_DURATION")).toBe("sleep"); + }); + + it("leaves ambient movement unowned, so the block is never gated", () => { + // `workouts` gates workout SESSIONS. Steps and active energy are on the + // ownership table's reviewed-unscoped list, and gating them here made the + // mood sheet stricter than every other surface in the product. + expect(moduleForMeasurementType("ACTIVITY_STEPS")).toBeNull(); + expect(moduleForMeasurementType("ACTIVE_ENERGY_BURNED")).toBeNull(); + }); + + it("keeps both vitals owned by recovery, and by the SAME module", () => { + const rhr = moduleForMeasurementType("RESTING_HEART_RATE"); + const hrv = moduleForMeasurementType("HEART_RATE_VARIABILITY"); + expect(rhr).toBe("recovery"); + expect(hrv).toBe("recovery"); + // The block carries one `available` flag for the pair, so the pair has to + // answer to one module. The day they diverge, that single flag becomes a + // lie about whichever of the two it does not describe, and the block has + // to be split before the table is changed. + expect( + hrv, + "the vitals block gates both figures on one module; these two no longer share an owner", + ).toBe(rhr); + }); + + it("carries the HRV fallback in the same module as the primary type", () => { + // A ring-only account resolves HRV to its RMSSD fallback. Left unowned, + // the fallback would serve exactly the recovery data the primary type + // refuses. + expect(moduleForMeasurementType("HRV_RMSSD")).toBe( + moduleForMeasurementType("HEART_RATE_VARIABILITY"), + ); + }); +}); diff --git a/src/lib/mood/linked-context.ts b/src/lib/mood/linked-context.ts index 59632f33f..f7c858c41 100644 --- a/src/lib/mood/linked-context.ts +++ b/src/lib/mood/linked-context.ts @@ -2,15 +2,13 @@ * What a mood entry's day looked like in the modules that already own it. * * The rule this file exists to serve is the one the whole context feature - * hangs off: one fact, one home. Sleep belongs to the sleep module, steps and - * active energy to the activity data, resting heart rate and HRV to the - * measurement engine, body symptoms to the illness module. A mood entry shows - * them and never asks for them a second time, and **nothing here is copied - * onto the mood row** — every figure is resolved at read time from the rows - * that own it, so correcting a sleep session corrects it here too, in the same - * moment, with nothing to re-sync. + * hangs off: one fact, one home. A mood entry shows what the other modules + * already know and never asks for it a second time, and **nothing here is + * copied onto the mood row** — every figure is resolved at read time from the + * rows that own it, so correcting a sleep session corrects it here too, in the + * same moment, with nothing to re-sync. * - * Two consequences that are easy to get wrong and are therefore stated: + * Three consequences that are easy to get wrong and are therefore stated: * * * **Absence is `{ present: false }`, never `0`.** A night nobody recorded * and a night of no sleep are different facts, and a surface that renders @@ -20,20 +18,43 @@ * block is simply not there, the same way `snapshot.ts` blanks rather than * filters. Turning a module off is a statement about what the person wants * to see, not a filter over what they wanted to see. + * * **Which module owns which metric is asked, never assumed.** The first + * draft of this file hand-wrote the ownership: it called steps and active + * energy `workouts`, and it called resting heart rate and HRV core with no + * owner at all. Both were wrong in the two directions that matter — the + * first hid ambient movement behind a toggle the user never associated + * with it, the second served the `recovery` domain to an account that had + * switched it off, which made the release note's own promise false. The + * answer comes from `moduleForMeasurementType()` now, which is the one + * table the MCP wire, the correlations reader and the Coach snapshot all + * resolve through. A second copy of an ownership map is a copy that drifts. + * + * Cross-source de-dup runs on every measurement figure, through the same + * `pickCanonicalSourceRows` ladder the crosstab and the doctor report use. + * Without it, an account syncing a phone and a watch reads its steps doubled + * on the mood sheet — the cumulative channels sum, and two sources reporting + * the same day sum twice. * * The reads are bounded: one local day per entry, resolved through the day-key * helper that already carries the legacy-Berlin fallback for `tz IS NULL` * rows. There is no unbounded `findMany` here, and adding one would turn a * detail view into a table scan on an account with years of history. */ -import type { MeasurementType } from "@/generated/prisma/enums"; +import type { + MeasurementSource, + MeasurementType, +} from "@/generated/prisma/enums"; import { prisma } from "@/lib/db"; import { isModuleEnabled } from "@/lib/modules/gate"; +import { moduleForMeasurementType } from "@/lib/modules/measurement-scope"; +import type { ModuleKey } from "@/lib/modules/registry"; import { DEFAULT_TIMEZONE, moodDateKey } from "@/lib/mood/date-key"; import { reconstructSleepNights, type SleepStageRow, } from "@/lib/analytics/sleep-night"; +import { pickCanonicalSourceRows } from "@/lib/analytics/source-priority"; +import { metricKeyForType } from "@/lib/measurements/cumulative-day-sum"; import { loadUserSourcePriority } from "@/lib/rollups/measurement-read"; /** A figure that either exists or honestly does not. */ @@ -51,6 +72,12 @@ export interface LinkedSleep { inBed: LinkedFigure; } +/** + * Ambient movement. Never gated: `workouts` gates workout SESSIONS, and steps + * and active energy are on the ownership table's reviewed-unscoped list for + * exactly that reason. The block carries an `available` flag anyway so every + * block in the payload has one shape. + */ export interface LinkedActivity { steps: LinkedFigure; activeEnergy: LinkedFigure; @@ -78,9 +105,10 @@ export interface LinkedDayContext { sleep: LinkedBlock; activity: LinkedBlock; /** - * Resting heart rate and HRV. Not gated: the measurement engine is core and - * has no off switch, so this block is always available and answers absence - * per figure instead. + * Resting heart rate and HRV. Gated on `recovery`, which is the module the + * ownership table has assigned both types since v1.30.22 — including the + * RMSSD fallback, pinned there so a ring-only account cannot reach through + * the fallback for what the primary type refuses. */ vitals: LinkedBlock; body: LinkedBlock; @@ -97,13 +125,64 @@ function figure(value: number | null | undefined, unit: string): LinkedFigure { : { present: true, value, unit }; } -/** Sum a day's rows for a cumulative metric; no rows means absent, not zero. */ +/** A measurement row as this resolver reads it. */ +interface LinkedRow { + type: MeasurementType; + value: number; + measuredAt: Date; + source: MeasurementSource; + deviceType: string | null; +} + +/** + * One day's rows for one metric, collapsed to a single source. + * + * The picker walks the user's source ladder and then the device-type ladder, + * so a day reported by both a phone and a watch keeps one stream. Everything + * downstream — the sum, the latest — then describes one device rather than an + * accidental union of two. A metric with no ladder key returns its rows + * untouched, which is the picker's own pass-through and is correct for a type + * no two sources compete over. + */ +function canonicalRowsOfDay( + rows: readonly LinkedRow[], + type: MeasurementType, + day: string, + tz: string, + priorityJson: unknown, +): LinkedRow[] { + const matching = rows.filter((r) => r.type === type); + if (matching.length === 0) return []; + const metricKey = metricKeyForType(type); + if (metricKey === null) return matching; + // The rows are already narrowed to one local day, so the picker's day key + // is constant here; it still goes through the real helper rather than a + // constant, because the ladder resolution is what is wanted and the bucket + // shape is the picker's business. + const { canonicalRows } = pickCanonicalSourceRows( + matching, + metricKey, + priorityJson, + (d) => moodDateKey(d, tz), + ); + return canonicalRows.length > 0 ? canonicalRows : matching; +} + +/** + * A cumulative metric's day total, over one source. + * + * No rows means absent, not zero: a day nobody's phone reported and a day + * spent motionless are different facts. + */ function sumOfDay( - rows: ReadonlyArray<{ type: MeasurementType; value: number }>, + rows: readonly LinkedRow[], type: MeasurementType, unit: string, + day: string, + tz: string, + priorityJson: unknown, ): LinkedFigure { - const matching = rows.filter((r) => r.type === type); + const matching = canonicalRowsOfDay(rows, type, day, tz, priorityJson); if (matching.length === 0) return ABSENT; return { present: true, @@ -112,19 +191,18 @@ function sumOfDay( }; } -/** Latest reading of the day for a point-in-time metric. */ +/** Latest reading of the day for a point-in-time metric, over one source. */ function latestOfDay( - rows: ReadonlyArray<{ - type: MeasurementType; - value: number; - measuredAt: Date; - }>, + rows: readonly LinkedRow[], type: MeasurementType, unit: string, + day: string, + tz: string, + priorityJson: unknown, ): LinkedFigure { - const matching = rows - .filter((r) => r.type === type) - .sort((a, b) => a.measuredAt.getTime() - b.measuredAt.getTime()); + const matching = canonicalRowsOfDay(rows, type, day, tz, priorityJson).sort( + (a, b) => a.measuredAt.getTime() - b.measuredAt.getTime(), + ); if (matching.length === 0) return ABSENT; return { present: true, value: matching[matching.length - 1].value, unit }; } @@ -158,14 +236,44 @@ function onDay( return rows.filter((r) => moodDateKey(r.measuredAt, tz) === day); } -/** The measurement types the linked block reads. One query covers all four. */ -const LINKED_MEASUREMENT_TYPES: MeasurementType[] = [ +/** The measurement types the linked block reads. One query covers all five. */ +const LINKED_MEASUREMENT_TYPES = [ "SLEEP_DURATION", "ACTIVITY_STEPS", "ACTIVE_ENERGY_BURNED", "RESTING_HEART_RATE", "HEART_RATE_VARIABILITY", -]; +] as const satisfies readonly MeasurementType[]; + +/** + * The modules that own anything this resolver reads, asked of the one + * ownership table rather than restated here. + * + * `illness` is added by hand because it owns a table rather than a measurement + * type, so the measurement-keyed table has nothing to say about it. Everything + * else is derived, which is what stops this file from growing a second opinion + * about who owns resting heart rate. + */ +const LINKED_MODULE_KEYS: readonly ModuleKey[] = Array.from( + new Set([ + ...LINKED_MEASUREMENT_TYPES.map(moduleForMeasurementType).filter( + (m): m is ModuleKey => m !== null, + ), + "illness", + ]), +); + +/** Whether the module owning `type` is on, for a resolved gate map. */ +function typeAvailable( + type: MeasurementType, + enabled: ReadonlyMap, +): boolean { + const owner = moduleForMeasurementType(type); + // `null` means the ownership table has reviewed this type and assigned it no + // module — ambient movement is the case here. Ungated is the answer, and it + // is the same answer the Coach snapshot and the MCP wire give. + return owner === null ? true : (enabled.get(owner) ?? true); +} /** * Resolve the linked figures for one entry's local day. @@ -183,17 +291,21 @@ export async function resolveLinkedDayContext( const day = entry.date; const { from, to } = localDayWindow(day); - const [sleepOn, activityOn, illnessOn] = await Promise.all([ - isModuleEnabled(userId, "sleep"), - isModuleEnabled(userId, "workouts"), - isModuleEnabled(userId, "illness"), - ]); + // Every gate this resolver needs, resolved once against the modules the + // ownership table names. Nothing here decides who owns what. + const gateStates = await Promise.all( + LINKED_MODULE_KEYS.map( + async (key) => [key, await isModuleEnabled(userId, key)] as const, + ), + ); + const enabled = new Map(gateStates); + const priority = await loadUserSourcePriority(userId); - const measurements = await prisma.measurement.findMany({ + const measurements: LinkedRow[] = await prisma.measurement.findMany({ where: { userId, deletedAt: null, - type: { in: LINKED_MEASUREMENT_TYPES }, + type: { in: [...LINKED_MEASUREMENT_TYPES] }, measuredAt: { gte: from, lte: to }, }, select: { @@ -204,22 +316,27 @@ export async function resolveLinkedDayContext( source: true, deviceType: true, }, - orderBy: { measuredAt: "asc" }, + // The canonical-source picker's determinism precondition: it keeps + // insertion order inside a bucket, so a stable read order is what makes + // its device-type tie-break reproducible rather than whatever the planner + // returned this time. + orderBy: [{ measuredAt: "asc" }, { id: "asc" }], }); const dayRows = onDay(measurements, day, tz); let sleep: LinkedBlock = MODULE_OFF; - if (sleepOn) { + if (typeAvailable("SLEEP_DURATION", enabled)) { // The stage rows of the whole window, not just the day's, and then the // night whose WAKE day is this one. A night starts the evening before, so // filtering the raw rows by day first would cut it in half — the - // reconstruction is the thing that knows where a night begins. + // reconstruction is the thing that knows where a night begins. It runs its + // own per-night source de-dup, which is why these rows do not go through + // the day picker first. const stageRows = measurements.filter((r) => r.type === "SLEEP_DURATION"); if (stageRows.length === 0) { sleep = { available: true, asleep: ABSENT, inBed: ABSENT }; } else { - const priority = await loadUserSourcePriority(userId); const night = reconstructSleepNights( stageRows as unknown as SleepStageRow[], tz, @@ -233,22 +350,54 @@ export async function resolveLinkedDayContext( } } - const activity: LinkedBlock = activityOn + // Ambient movement, ungated by the ownership table's own verdict. The + // `typeAvailable` call is not decoration: it is what makes a future change + // to that verdict reach this surface without an edit here. + const activity: LinkedBlock = typeAvailable( + "ACTIVITY_STEPS", + enabled, + ) ? { available: true, - steps: sumOfDay(dayRows, "ACTIVITY_STEPS", "steps"), - activeEnergy: sumOfDay(dayRows, "ACTIVE_ENERGY_BURNED", "kcal"), + steps: sumOfDay(dayRows, "ACTIVITY_STEPS", "steps", day, tz, priority), + activeEnergy: sumOfDay( + dayRows, + "ACTIVE_ENERGY_BURNED", + "kcal", + day, + tz, + priority, + ), } : MODULE_OFF; - const vitals: LinkedBlock = { - available: true, - restingHeartRate: latestOfDay(dayRows, "RESTING_HEART_RATE", "bpm"), - heartRateVariability: latestOfDay(dayRows, "HEART_RATE_VARIABILITY", "ms"), - }; + const vitals: LinkedBlock = typeAvailable( + "RESTING_HEART_RATE", + enabled, + ) + ? { + available: true, + restingHeartRate: latestOfDay( + dayRows, + "RESTING_HEART_RATE", + "bpm", + day, + tz, + priority, + ), + heartRateVariability: latestOfDay( + dayRows, + "HEART_RATE_VARIABILITY", + "ms", + day, + tz, + priority, + ), + } + : MODULE_OFF; let body: LinkedBlock = MODULE_OFF; - if (illnessOn) { + if (enabled.get("illness") ?? true) { // Read only. The illness module owns symptom capture and its severity // scale; the mood surface links into it and captures nothing, which is // what keeps the two from disagreeing about the same day.