mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(mood): ask the ownership table who owns a linked metric, and pick one source
The resolver hand-wrote the map and got it wrong in both directions. It gated steps and active energy on `workouts`, which gates workout SESSIONS and not ambient movement, so the mood sheet hid a figure every other surface shows. It called resting heart rate and heart-rate variability core with no owner, while the table has assigned both to `recovery` since v1.30.22 — including the RMSSD fallback, pinned there so a ring-only account cannot reach through it. An account with recovery switched off was served the domain it had switched off, which made the release note's own promise false. Both directions now resolve through `moduleForMeasurementType()`, the table the MCP wire, the correlations reader and the Coach snapshot already share. Steps, active energy and the two vitals also run through the canonical-source picker now. Without it a phone and a watch reporting the same day summed twice and the sheet showed a walk nobody took; the vitals answered whichever row was latest rather than the picked stream's. A guard pins the three ownership claims, including that the two vitals share one module — the block carries one availability flag for the pair, so the day they diverge that flag becomes a lie and the block has to be split first.
This commit is contained in:
1 parent
88f83104e0
commit
1853a71b18
3 files changed
+268
-49
No files matched your search
+10
-2
@@ -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,
|
||||
|
||||
@@ -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"),
|
||||
);
|
||||
});
|
||||
});
|
||||
+196
-47
@@ -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<LinkedSleep>;
|
||||
activity: LinkedBlock<LinkedActivity>;
|
||||
/**
|
||||
* 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<LinkedVitals>;
|
||||
body: LinkedBlock<LinkedBody>;
|
||||
@@ -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<T extends { measuredAt: Date }>(
|
||||
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<ModuleKey>([
|
||||
...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<ModuleKey, boolean>,
|
||||
): 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<ModuleKey, boolean>(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<LinkedSleep> = 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<LinkedActivity> = 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<LinkedActivity> = 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<LinkedVitals> = {
|
||||
available: true,
|
||||
restingHeartRate: latestOfDay(dayRows, "RESTING_HEART_RATE", "bpm"),
|
||||
heartRateVariability: latestOfDay(dayRows, "HEART_RATE_VARIABILITY", "ms"),
|
||||
};
|
||||
const vitals: LinkedBlock<LinkedVitals> = 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<LinkedBody> = 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.
|
||||
|
||||
Reference in new issue
Block a user