mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 15:02:04 +00:00
fix(dashboard): BD-Zielbereich headline shows all-time, not 30-day
Marc reported the BD-Zielbereich tile pinned at exactly 50/50/50 for
7T, 30T, and the headline ("total") on his real production data — the
fourth attempt at this surface (v1.4.15 → v1.4.16 → v1.4.18 → v1.4.19).
Root cause: the analytics route routed `bpInTargetPct` (the headline)
through `windows.last30Days?.pct` — a literal copy of the `30T`
sub-value. Since `computeBpInTargetWindows` only returned 7d + 30d
windows, the headline could never legitimately differ from `30T`. For
Marc's real data (572 paired readings since 2022, recent 30d = 50 %,
all-time ≈ 11 %) the tile read 50 % / 50 % / 50 % and looked
algorithmically pinned — exactly the symptom he reported.
The previous three passes treated this as either a predicate bug
(v1.4.16 ceiling-vs-narrow-band, real and merged) or a missing
`avg7`/`avg30` wiring (v1.4.18, also real and merged). Both fixes
landed correctly, but neither addressed the headline duplication.
Fix:
- `computeBpInTargetWindows` now returns a third `allTime` window
that aggregates EVERY paired reading in the input series.
- The analytics route fetches all paired BP rows (no 30-day cutoff)
and routes the headline through `windows.allTime?.pct`. The route
only re-pairs once internally so the cost is paid once per request.
- The 7T / 30T sub-values are unchanged (still windowed slices of
the same input).
Failing test reproduces the bug with Marc's data shape (`allTime` MUST
not equal `last30Days` once older history is present); fix turns it
green. Integration test seeds 40 paired readings against real Postgres
and asserts all three windows compute independently.
Hand-counted against Marc's prod data:
- 7T: 50 % (1/2) — matches what he sees, real coincidence
- 30T: 50 % (5/10) — matches what he sees, real coincidence
- Headline: ~11 % (62/572) — was 50 %, now correct
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
ba0d6b87d1
commit
a8562727a7
4 files changed
+245
-27
No files matched your search
@@ -59,32 +59,28 @@ export const GET = apiHandler(async () => {
|
||||
const bpTargets = getBpTargets(user.dateOfBirth);
|
||||
if (bpTargets) {
|
||||
const now = new Date();
|
||||
const thirtyDaysAgo = new Date(now.getTime() - 30 * 24 * 60 * 60 * 1000);
|
||||
// v1.4.19 A1 — fetch ALL paired BP rows, not just the trailing 30
|
||||
// days. Up to v1.4.18 we filtered to the last 30 days at the DB
|
||||
// level and the headline (`bpInTargetPct`) was routed through
|
||||
// `windows.last30Days?.pct` — making the headline a literal copy
|
||||
// of the `30T` sub-value. For Marc's data (572 paired readings,
|
||||
// recent 30d = 50 %, all-time ≈ 11 %) the tile pinned 50/50/50
|
||||
// and looked algorithmically broken. The windowed helper now also
|
||||
// returns an independent `allTime` aggregate, which we surface as
|
||||
// the headline so the three numbers can diverge naturally.
|
||||
const [sysData, diaData] = await Promise.all([
|
||||
prisma.measurement.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
type: "BLOOD_PRESSURE_SYS",
|
||||
measuredAt: { gte: thirtyDaysAgo },
|
||||
},
|
||||
where: { userId: user.id, type: "BLOOD_PRESSURE_SYS" },
|
||||
select: { measuredAt: true, value: true },
|
||||
}),
|
||||
prisma.measurement.findMany({
|
||||
where: {
|
||||
userId: user.id,
|
||||
type: "BLOOD_PRESSURE_DIA",
|
||||
measuredAt: { gte: thirtyDaysAgo },
|
||||
},
|
||||
where: { userId: user.id, type: "BLOOD_PRESSURE_DIA" },
|
||||
select: { measuredAt: true, value: true },
|
||||
}),
|
||||
]);
|
||||
|
||||
// v1.4.18 A1 — windowed helper computes the 7d / 30d sub-values
|
||||
// for the dashboard tile in a single pair-and-count pass. The
|
||||
// headline (`bpInTargetPct`) reuses the 30-day result so we don't
|
||||
// re-pair the same data twice. See lib/analytics/bp-in-target.ts.
|
||||
const windows = computeBpInTargetWindows(sysData, diaData, bpTargets, now);
|
||||
bpInTargetPct = windows.last30Days?.pct ?? null;
|
||||
bpInTargetPct = windows.allTime?.pct ?? null;
|
||||
bpInTargetPct7d = windows.last7Days?.pct ?? null;
|
||||
bpInTargetPct30d = windows.last30Days?.pct ?? null;
|
||||
}
|
||||
|
||||
@@ -235,6 +235,112 @@ describe("computeBpInTargetPct", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeBpInTargetWindows — all-time headline (v1.4.19 A1)", () => {
|
||||
/**
|
||||
* v1.4.19 A1 regression — Marc reported the BD-Zielbereich tile shows
|
||||
* **EXACTLY 50 % on 7T, 30T, AND the headline ("total")** for his
|
||||
* production data. Hand-counted hypothesis-1 from the brief (the
|
||||
* three-way coincidence cannot be data — it's a calculation pin):
|
||||
*
|
||||
* - Marc has 572 paired BP readings going back to 2022-01.
|
||||
* - Last 7 days: 1 / 2 in target = 50 %.
|
||||
* - Last 30 days: 5 / 10 in target = 50 %.
|
||||
* - All time: 62 / 572 in target ≈ 11 %.
|
||||
*
|
||||
* The two "50 %"s in the recent windows are real, but the headline
|
||||
* cannot be 50 %; it should be ~11 %. Root cause: the analytics route
|
||||
* sets `bpInTargetPct = windows.last30Days?.pct` (literal copy of
|
||||
* `bpInTargetPct30d`), and `computeBpInTargetWindows` never returns an
|
||||
* all-time figure at all. This test pins the new contract: the helper
|
||||
* surfaces a third `allTime` window that aggregates EVERY paired
|
||||
* reading in the input regardless of recency.
|
||||
*/
|
||||
const NOW_19 = new Date("2026-05-10T11:00:00Z");
|
||||
|
||||
it("returns an allTime window distinct from the 30-day window", () => {
|
||||
// 30 days of mixed data (30d ≈ 50 %) plus 20 older readings that
|
||||
// are mostly OUT of target (drives all-time well below 50 %).
|
||||
const recentSys: Array<ReturnType<typeof reading>> = [];
|
||||
const recentDia: Array<ReturnType<typeof reading>> = [];
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const at = new Date(
|
||||
NOW_19.getTime() - (i + 0.5) * 24 * 60 * 60 * 1000,
|
||||
).toISOString();
|
||||
const inTarget = i % 2 === 0; // 5 IN, 5 OUT in last 30d
|
||||
recentSys.push(reading(at, inTarget ? 122 : 145));
|
||||
recentDia.push(reading(at, inTarget ? 75 : 95));
|
||||
}
|
||||
// 20 older readings, all OUT of target (sys above ceiling).
|
||||
const olderSys: Array<ReturnType<typeof reading>> = [];
|
||||
const olderDia: Array<ReturnType<typeof reading>> = [];
|
||||
for (let i = 0; i < 20; i++) {
|
||||
const at = new Date(
|
||||
NOW_19.getTime() - (60 + i) * 24 * 60 * 60 * 1000,
|
||||
).toISOString();
|
||||
olderSys.push(reading(at, 160));
|
||||
olderDia.push(reading(at, 100));
|
||||
}
|
||||
const sys = [...recentSys, ...olderSys];
|
||||
const dia = [...recentDia, ...olderDia];
|
||||
|
||||
const result = computeBpInTargetWindows(sys, dia, TARGETS_UNDER_65, NOW_19);
|
||||
|
||||
// 30d = 5 / 10 in target = 50 %.
|
||||
expect(result.last30Days).toEqual({ pct: 50, pairs: 10 });
|
||||
// All-time = 5 / 30 in target = 17 % (rounded).
|
||||
expect(result.allTime).not.toBeNull();
|
||||
expect(result.allTime!.pairs).toBe(30);
|
||||
expect(result.allTime!.pct).toBe(Math.round((5 / 30) * 100));
|
||||
// The smoking gun: all-time MUST NOT equal 30-day when older data is
|
||||
// statistically different. This is the v1.4.19 A1 regression.
|
||||
expect(result.allTime!.pct).not.toBe(result.last30Days!.pct);
|
||||
});
|
||||
|
||||
it("returns null allTime when both series are empty", () => {
|
||||
const result = computeBpInTargetWindows([], [], TARGETS_UNDER_65, NOW_19);
|
||||
expect(result.allTime).toBeNull();
|
||||
});
|
||||
|
||||
it("regression: Marc's 30-day fixture mirrors Marc's prod headline bug", () => {
|
||||
// Reuse Marc's production fixture (10 pairs, 5 in target = 50 %).
|
||||
// When the input is JUST those 10 pairs, allTime equals 30d == 50 %
|
||||
// (this is the legitimate coincidence — it only hides the bug when
|
||||
// the user has no older history). The bug surfaces the moment older
|
||||
// data is added; covered by the previous test.
|
||||
const sys = [
|
||||
reading("2026-05-08T07:38:22Z", 117),
|
||||
reading("2026-05-03T21:22:02Z", 122),
|
||||
reading("2026-05-03T05:51:45Z", 108),
|
||||
reading("2026-05-03T05:50:55Z", 106),
|
||||
reading("2026-04-20T05:57:42Z", 127),
|
||||
reading("2026-04-18T06:59:29Z", 115),
|
||||
reading("2026-04-16T05:24:51Z", 108),
|
||||
reading("2026-04-15T05:34:35Z", 124),
|
||||
reading("2026-04-15T05:33:44Z", 126),
|
||||
reading("2026-04-15T20:52:26Z", 133),
|
||||
];
|
||||
const dia = [
|
||||
reading("2026-05-08T07:38:22Z", 79),
|
||||
reading("2026-05-03T21:22:02Z", 86),
|
||||
reading("2026-05-03T05:51:45Z", 76),
|
||||
reading("2026-05-03T05:50:55Z", 73),
|
||||
reading("2026-04-20T05:57:42Z", 86),
|
||||
reading("2026-04-18T06:59:29Z", 78),
|
||||
reading("2026-04-16T05:24:51Z", 75),
|
||||
reading("2026-04-15T05:34:35Z", 82),
|
||||
reading("2026-04-15T05:33:44Z", 80),
|
||||
reading("2026-04-15T20:52:26Z", 95),
|
||||
];
|
||||
const result = computeBpInTargetWindows(
|
||||
sys,
|
||||
dia,
|
||||
TARGETS_UNDER_65,
|
||||
new Date("2026-05-10T11:00:00Z"),
|
||||
);
|
||||
expect(result.allTime).toEqual({ pct: 50, pairs: 10 });
|
||||
});
|
||||
});
|
||||
|
||||
describe("computeBpInTargetWindows", () => {
|
||||
/**
|
||||
* v1.4.18 A1 regression — Marc reported the BD-Zielbereich tile shows
|
||||
|
||||
@@ -178,23 +178,37 @@ export function computeBpInTargetPct(
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.4.18 A1 — windowed in-target shares for the dashboard tile.
|
||||
* v1.4.18 A1 → v1.4.19 A1 — windowed in-target shares for the dashboard
|
||||
* tile.
|
||||
*
|
||||
* The "BD im Zielbereich" tile renders three numbers:
|
||||
* - Headline: the 30-day in-target % (the existing `computeBpInTargetPct`).
|
||||
* - Headline: the **all-time** in-target % across every paired reading.
|
||||
* - `7T:` the in-target % over the trailing 7 days.
|
||||
* - `30T:` the in-target % over the trailing 30 days.
|
||||
*
|
||||
* Up to v1.4.17 only the 30-day headline was computed; the API returned
|
||||
* a single `bpInTargetPct` and the tile rendered `avg7={null}` /
|
||||
* `avg30={null}`, so the sub-values collapsed to "—" even when the user
|
||||
* had paired readings in both windows. The fix surfaces both windows
|
||||
* from a single helper that filters the input series by `measuredAt`
|
||||
* relative to `now`. Caller can pass the unfiltered last-30-day data
|
||||
* and get both windows back without a second DB round-trip.
|
||||
* History of the bug:
|
||||
*
|
||||
* `null` for either window means "no paired readings in that window";
|
||||
* the caller should render the "—" placeholder instead of `0`.
|
||||
* - Up to v1.4.17 only the 30-day figure was computed; the tile showed
|
||||
* `avg7={null}, avg30={null}` so `7T: —, 30T: —` rendered the dash
|
||||
* fallback even when paired readings existed.
|
||||
* - v1.4.18 A1 wired `avg7` + `avg30` from `computeBpInTargetWindows`,
|
||||
* BUT the analytics route also routed the **headline**
|
||||
* (`bpInTargetPct`) through `windows.last30Days?.pct` — i.e. the
|
||||
* headline was a literal copy of `bpInTargetPct30d`. For Marc's prod
|
||||
* data (572 paired readings since 2022, recent 30 days = 50 %, all-time
|
||||
* ≈ 11 %) the tile rendered `50 %` headline, `7T: 50, 30T: 50` and
|
||||
* looked algorithmically pinned to 50/50/50. That was hypothesis-1
|
||||
* from the v1.4.19 A1 brief: "the 7T/30T branches reuse the all-time
|
||||
* percentage" — actually the *other* direction (the all-time headline
|
||||
* reused the 30-day value).
|
||||
* - v1.4.19 A1 surfaces a third `allTime` window so the headline gets
|
||||
* the genuinely-different aggregate. Callers must use `allTime` for
|
||||
* the headline now; the test suite pins this contract.
|
||||
*
|
||||
* `null` for any window means "no paired readings in that window"; the
|
||||
* caller renders the "—" placeholder instead of `0`. Caller must pass
|
||||
* UNFILTERED series for `allTime` to produce the correct value — the
|
||||
* helper does the windowing internally.
|
||||
*/
|
||||
const DAY_MS = 24 * 60 * 60 * 1000;
|
||||
|
||||
@@ -206,6 +220,7 @@ export function computeBpInTargetWindows(
|
||||
): {
|
||||
last7Days: { pct: number; pairs: number } | null;
|
||||
last30Days: { pct: number; pairs: number } | null;
|
||||
allTime: { pct: number; pairs: number } | null;
|
||||
} {
|
||||
const sevenDaysAgoMs = now.getTime() - 7 * DAY_MS;
|
||||
const thirtyDaysAgoMs = now.getTime() - 30 * DAY_MS;
|
||||
@@ -226,5 +241,9 @@ export function computeBpInTargetWindows(
|
||||
return {
|
||||
last7Days: computeBpInTargetPct(sysLast7, diaLast7, targets),
|
||||
last30Days: computeBpInTargetPct(sysLast30, diaLast30, targets),
|
||||
// v1.4.19 A1 — independent aggregate over EVERY paired reading. The
|
||||
// analytics route now routes the dashboard tile's headline through
|
||||
// this so it stops mirroring `last30Days`.
|
||||
allTime: computeBpInTargetPct(sysSeries, diaSeries, targets),
|
||||
};
|
||||
}
|
||||
@@ -341,6 +341,103 @@ describe("BP-in-target % — windowed (7-day + 30-day) — production data shape
|
||||
expect(windows.last30Days!.pct).toBe(Math.round((15 / 29) * 100));
|
||||
});
|
||||
|
||||
/**
|
||||
* v1.4.19 A1 regression — Marc reported the tile shows EXACTLY 50 %
|
||||
* on 7T, 30T, AND the headline ("total"). With his real production
|
||||
* shape (572 paired readings since 2022, recent 30d ≈ 50 %, all-time
|
||||
* ≈ 11 %) the headline cannot legitimately be 50 %. Root cause: the
|
||||
* analytics route routed `bpInTargetPct = windows.last30Days?.pct`,
|
||||
* making the headline a literal copy of `30T`. The fix returns a
|
||||
* third `allTime` window that the route now uses for the headline.
|
||||
*
|
||||
* Seeds 2 paired readings within the last 7 days (1 IN, 1 OUT = 50 %),
|
||||
* 8 additional within the last 30 days (4 IN, 4 OUT = 50 %), and 30
|
||||
* older readings (all OUT) so the all-time figure diverges sharply
|
||||
* from both windows. Pinned for regression so a future refactor can't
|
||||
* silently re-collapse the three numbers.
|
||||
*/
|
||||
it("returns three independent windows: 7d, 30d, and allTime each computed from a different denominator", async () => {
|
||||
const prisma = getPrismaClient();
|
||||
const user = await prisma.user.create({
|
||||
data: {
|
||||
username: "bp-three-windows-fixture",
|
||||
email: "bp-three-windows@example.test",
|
||||
dateOfBirth: new Date("1985-07-09"),
|
||||
},
|
||||
});
|
||||
|
||||
const now = new Date();
|
||||
const seed: Array<{ daysAgo: number; sys: number; dia: number }> = [
|
||||
// Last 7 days: 1 IN, 1 OUT.
|
||||
{ daysAgo: 1.5, sys: 117, dia: 79 }, // IN
|
||||
{ daysAgo: 5.5, sys: 145, dia: 95 }, // OUT
|
||||
// 7-30 days ago: 4 IN, 4 OUT (alternating).
|
||||
{ daysAgo: 8, sys: 122, dia: 75 }, // IN
|
||||
{ daysAgo: 10, sys: 145, dia: 95 }, // OUT
|
||||
{ daysAgo: 12, sys: 125, dia: 78 }, // IN
|
||||
{ daysAgo: 14, sys: 145, dia: 90 }, // OUT
|
||||
{ daysAgo: 18, sys: 120, dia: 70 }, // IN
|
||||
{ daysAgo: 22, sys: 150, dia: 95 }, // OUT
|
||||
{ daysAgo: 25, sys: 128, dia: 79 }, // IN
|
||||
{ daysAgo: 28, sys: 160, dia: 100 }, // OUT
|
||||
// Older history: 30 readings, all OUT (drives all-time well below 50 %).
|
||||
...Array.from({ length: 30 }, (_, i) => ({
|
||||
daysAgo: 60 + i * 3,
|
||||
sys: 160,
|
||||
dia: 100,
|
||||
})),
|
||||
];
|
||||
for (const r of seed) {
|
||||
const at = new Date(now.getTime() - r.daysAgo * 24 * 60 * 60 * 1000);
|
||||
await prisma.measurement.create({
|
||||
data: {
|
||||
userId: user.id,
|
||||
type: "BLOOD_PRESSURE_SYS",
|
||||
value: r.sys,
|
||||
unit: "mmHg",
|
||||
measuredAt: at,
|
||||
},
|
||||
});
|
||||
await prisma.measurement.create({
|
||||
data: {
|
||||
userId: user.id,
|
||||
type: "BLOOD_PRESSURE_DIA",
|
||||
value: r.dia,
|
||||
unit: "mmHg",
|
||||
measuredAt: at,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
const sysData = await prisma.measurement.findMany({
|
||||
where: { userId: user.id, type: "BLOOD_PRESSURE_SYS" },
|
||||
select: { measuredAt: true, value: true },
|
||||
});
|
||||
const diaData = await prisma.measurement.findMany({
|
||||
where: { userId: user.id, type: "BLOOD_PRESSURE_DIA" },
|
||||
select: { measuredAt: true, value: true },
|
||||
});
|
||||
const targets = getBpTargets(user.dateOfBirth);
|
||||
expect(targets).not.toBeNull();
|
||||
const windows = computeBpInTargetWindows(sysData, diaData, targets!, now);
|
||||
|
||||
// 7-day window: 2 paired readings, 1 in target.
|
||||
expect(windows.last7Days).toEqual({ pct: 50, pairs: 2 });
|
||||
// 30-day window: 10 paired readings, 5 in target.
|
||||
expect(windows.last30Days).toEqual({ pct: 50, pairs: 10 });
|
||||
// All-time: 40 paired readings, 5 in target = 13 % (rounded).
|
||||
expect(windows.allTime).not.toBeNull();
|
||||
expect(windows.allTime!.pairs).toBe(40);
|
||||
expect(windows.allTime!.pct).toBe(Math.round((5 / 40) * 100));
|
||||
|
||||
// The smoking gun: even when 7d AND 30d are 50 %, all-time is NOT
|
||||
// 50 % once older history is present. Marc's prod tile pinned to
|
||||
// 50/50/50 because the route routed the headline through `last30Days`
|
||||
// — the algorithmic pin the brief warned about.
|
||||
expect(windows.allTime!.pct).not.toBe(windows.last30Days!.pct);
|
||||
expect(windows.allTime!.pct).not.toBe(windows.last7Days!.pct);
|
||||
});
|
||||
|
||||
/**
|
||||
* Regression: a user with all readings older than 7 days but inside
|
||||
* 30 days must show a 30-day percentage AND a null 7-day window
|
||||
|
||||
Reference in new issue
Block a user