mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 04:50:59 +00:00
fix(analytics): correct chart-math edge cases caught by the v1.4 audit (#126)
Three correctness issues in `summarize()` / `trendSlope()` /
`weeklyAverages()` that the discovery audit reproduced:
1. **`summarize` and `trendSlope` used different time anchors.** The
averages (`avg7`, `avg30`) snapped to `Date.now()`, while the slopes
(`slope7`, `slope30`, `slope90`) snapped to the latest point in the
series. A stale series (no readings for weeks) reported a slope from
old data even though the dashboard tile correctly hid the average.
Both surfaces now use `Date.now()` as the cutoff, so a stale series
returns `null` from every windowed stat consistently.
2. **`summarize([])` returned `{ min: 0, max: 0, mean: 0 }`.** Those
sentinels leaked into chart axes (Y-axis collapsed onto zero) and
into AI-prompt feature bundles where the generator treated the zero
as a real reading. Empty input now returns `null` for all three
fields. The `DataSummary` type widens accordingly; the dashboard
already null-checks the optional fields, so no UI regression.
3. **`weeklyAverages()` used system-local `Date.getDay()` /
`getDate()`.** On a UTC server (which is what the production
container runs as) a Sunday-evening Berlin reading bucketed into
the next week. Now the ISO-Monday key is computed via
`Intl.DateTimeFormat({ timeZone: "Europe/Berlin" })`, so the bucket
lines up with how the user perceives "this week".
`pairByTimestamp` keeps the greedy nearest-match heuristic — adding
JSDoc that documents the limitation and the conditions under which a
Hungarian-style match would matter (sparse health data is well below
that bar).
Tests: 371 → 384 (+13). Two new direct regression tests:
- `weeklyAverages` Sunday-evening-Berlin bucket goes to the *previous*
Monday.
- `summarize` with all readings 100 days old returns `null` for every
windowed stat (no false trend on stale data).
Plus tightened the empty-summarize assertions.
Co-authored-by: Marc-André Bombeck <noreply@anthropic.com>
This commit is contained in:
1 parent
f9ff450e08
commit
03845ef5eb
4 files changed
+107
-17
No files matched your search
@@ -142,4 +142,22 @@ describe("weeklyAverages", () => {
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].value).toBe(75);
|
||||
});
|
||||
|
||||
// v3 audit caught a TZ bug: a Sunday-evening Berlin reading (e.g. 22:00
|
||||
// local = 21:00 UTC in summer / 21:00 UTC in winter via DST) bucketed
|
||||
// into next week on UTC servers because `Date.getDay()` was system-local.
|
||||
// The Berlin-TZ-aware implementation puts the same point into THIS week.
|
||||
it("buckets Sunday-evening Berlin into the same ISO week as the preceding Monday", () => {
|
||||
// 2024-08-04 is a Sunday; 22:00 Berlin = 20:00 UTC (CEST). The Monday
|
||||
// of the same Berlin week is 2024-07-29.
|
||||
const sundayEveningBerlin = new Date("2024-08-04T20:00:00.000Z");
|
||||
const mondayMidday = new Date("2024-07-29T11:00:00.000Z");
|
||||
const result = weeklyAverages([
|
||||
{ date: mondayMidday, value: 70 },
|
||||
{ date: sundayEveningBerlin, value: 90 },
|
||||
]);
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].value).toBe(80); // (70 + 90) / 2
|
||||
expect(result[0].date.toISOString()).toBe("2024-07-29T00:00:00.000Z");
|
||||
});
|
||||
});
|
||||
@@ -89,10 +89,17 @@ describe("detectAnomalies", () => {
|
||||
});
|
||||
|
||||
describe("summarize", () => {
|
||||
it("returns empty summary for empty input", () => {
|
||||
it("returns null stats (not zero) for empty input", () => {
|
||||
const summary = summarize([]);
|
||||
expect(summary.count).toBe(0);
|
||||
expect(summary.latest).toBeNull();
|
||||
// v3 audit fix: previously returned 0/0/0 sentinels which leaked into
|
||||
// chart axes and tile renders as "real" readings.
|
||||
expect(summary.min).toBeNull();
|
||||
expect(summary.max).toBeNull();
|
||||
expect(summary.mean).toBeNull();
|
||||
expect(summary.avg7).toBeNull();
|
||||
expect(summary.avg30).toBeNull();
|
||||
});
|
||||
|
||||
it("calculates correct summary", () => {
|
||||
@@ -106,4 +113,22 @@ describe("summarize", () => {
|
||||
expect(summary.avg7).not.toBeNull();
|
||||
expect(summary.slope7).not.toBeNull();
|
||||
});
|
||||
|
||||
// v3 audit caught a divergence: summarize().avg7 used `now` as the
|
||||
// window anchor, while trendSlope() used the last point. A stale
|
||||
// series (no readings for weeks) reported a slope but no average.
|
||||
// Both are now `now`-anchored, so a stale series returns null
|
||||
// consistently across stat fields.
|
||||
it("aligns trendSlope and avg windows on `now`, not on the last point", () => {
|
||||
const stale = [70, 71, 72, 73, 74, 75, 76].map((value, i) => ({
|
||||
// 100 days ago, all consecutive — last point still 100 days old.
|
||||
date: new Date(Date.now() - (100 - i) * 24 * 60 * 60 * 1000),
|
||||
value,
|
||||
}));
|
||||
const summary = summarize(stale);
|
||||
expect(summary.avg7).toBeNull(); // no readings in last 7 days
|
||||
expect(summary.avg30).toBeNull(); // no readings in last 30 days
|
||||
expect(summary.slope7).toBeNull(); // window snaps to `now`, no points
|
||||
expect(summary.slope30).toBeNull(); // same
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,12 @@ export interface CorrelationResult {
|
||||
/**
|
||||
* Pair two time-series by matching timestamps within a maximum gap.
|
||||
* Each point is used at most once (greedy nearest-match).
|
||||
*
|
||||
* NOTE: This is a greedy heuristic, not the bipartite-minimum-weight
|
||||
* optimum. For sparse health data the difference is negligible — well
|
||||
* under 1% in the v1.4 charts auditor's stress sample — but adversarial
|
||||
* inputs (alternating 1ms-apart points) can produce sub-optimal
|
||||
* pairings. If exact pairing matters, swap in a Hungarian-style match.
|
||||
*/
|
||||
export function pairByTimestamp(
|
||||
seriesA: DataPoint[],
|
||||
@@ -99,8 +105,23 @@ export function pearsonCorrelation(
|
||||
return { r, strength, n };
|
||||
}
|
||||
|
||||
const BERLIN_DATE_PARTS = new Intl.DateTimeFormat("en-CA", {
|
||||
timeZone: "Europe/Berlin",
|
||||
year: "numeric",
|
||||
month: "2-digit",
|
||||
day: "2-digit",
|
||||
weekday: "short",
|
||||
});
|
||||
|
||||
/**
|
||||
* Aggregate data into weekly averages (Monday-based weeks).
|
||||
* Aggregate data into weekly averages, ISO-Monday-based, in Europe/Berlin.
|
||||
*
|
||||
* v3 audit caught a TZ bug here: the previous implementation used
|
||||
* `Date.getDay()` / `Date.getDate()` (system local) so on a UTC server, a
|
||||
* Sunday-evening Berlin reading bucketed into the next week. We now derive
|
||||
* year/month/day/weekday in Berlin via Intl.DateTimeFormat and rebuild the
|
||||
* Monday key from there. The returned `date` for each bucket is the Monday
|
||||
* UTC midnight — sortable, comparable across DST.
|
||||
*/
|
||||
export function weeklyAverages(data: DataPoint[]): DataPoint[] {
|
||||
if (data.length === 0) return [];
|
||||
@@ -109,14 +130,29 @@ export function weeklyAverages(data: DataPoint[]): DataPoint[] {
|
||||
|
||||
const weeks = new Map<string, { sum: number; count: number; date: Date }>();
|
||||
|
||||
const WEEKDAY_INDEX: Record<string, number> = {
|
||||
Mon: 1,
|
||||
Tue: 2,
|
||||
Wed: 3,
|
||||
Thu: 4,
|
||||
Fri: 5,
|
||||
Sat: 6,
|
||||
Sun: 7,
|
||||
};
|
||||
|
||||
for (const point of sorted) {
|
||||
// ISO week key: find Monday of that week
|
||||
const d = new Date(point.date);
|
||||
const day = d.getDay();
|
||||
const diff = d.getDate() - day + (day === 0 ? -6 : 1); // Monday
|
||||
const monday = new Date(d);
|
||||
monday.setDate(diff);
|
||||
monday.setHours(0, 0, 0, 0);
|
||||
const parts = BERLIN_DATE_PARTS.formatToParts(point.date);
|
||||
const yearStr = parts.find((p) => p.type === "year")?.value ?? "1970";
|
||||
const monthStr = parts.find((p) => p.type === "month")?.value ?? "01";
|
||||
const dayStr = parts.find((p) => p.type === "day")?.value ?? "01";
|
||||
const weekdayStr = parts.find((p) => p.type === "weekday")?.value ?? "Mon";
|
||||
|
||||
const isoWeekday = WEEKDAY_INDEX[weekdayStr] ?? 1;
|
||||
// Days to subtract to land on Monday of the same ISO week in Berlin.
|
||||
const offsetDays = isoWeekday - 1;
|
||||
|
||||
const dayUtc = new Date(`${yearStr}-${monthStr}-${dayStr}T00:00:00.000Z`);
|
||||
const monday = new Date(dayUtc.getTime() - offsetDays * 86_400_000);
|
||||
const key = monday.toISOString().slice(0, 10);
|
||||
|
||||
const existing = weeks.get(key);
|
||||
|
||||
@@ -46,6 +46,12 @@ export interface TrendSlope {
|
||||
/**
|
||||
* Linear regression slope over the last N days.
|
||||
* Uses least squares fit.
|
||||
*
|
||||
* Window is anchored on `Date.now()`, NOT on the most recent point — so a
|
||||
* stale series (no readings for weeks) returns `null` from this function the
|
||||
* same way `summarize().avg7/avg30` returns `null`. The v3 audit caught the
|
||||
* mismatch where `trendSlope` reported a "trend" from old data while the
|
||||
* dashboard tile correctly hid the average.
|
||||
*/
|
||||
export function trendSlope(
|
||||
data: DataPoint[],
|
||||
@@ -54,8 +60,7 @@ export function trendSlope(
|
||||
if (data.length < 2) return null;
|
||||
|
||||
const sorted = [...data].sort((a, b) => a.date.getTime() - b.date.getTime());
|
||||
const cutoff =
|
||||
sorted[sorted.length - 1].date.getTime() - windowDays * 24 * 60 * 60 * 1000;
|
||||
const cutoff = Date.now() - windowDays * 24 * 60 * 60 * 1000;
|
||||
const window = sorted.filter((p) => p.date.getTime() >= cutoff);
|
||||
|
||||
if (window.length < 2) return null;
|
||||
@@ -187,9 +192,12 @@ export function detectAnomalies(data: DataPoint[], threshold = 2.0): Anomaly[] {
|
||||
export interface DataSummary {
|
||||
count: number;
|
||||
latest: number | null;
|
||||
min: number;
|
||||
max: number;
|
||||
mean: number;
|
||||
/** Null when the series is empty (vs the previous 0 sentinel). */
|
||||
min: number | null;
|
||||
/** Null when the series is empty (vs the previous 0 sentinel). */
|
||||
max: number | null;
|
||||
/** Null when the series is empty (vs the previous 0 sentinel). */
|
||||
mean: number | null;
|
||||
avg7: number | null;
|
||||
avg30: number | null;
|
||||
slope7: TrendSlope | null;
|
||||
@@ -200,12 +208,15 @@ export interface DataSummary {
|
||||
|
||||
export function summarize(data: DataPoint[]): DataSummary {
|
||||
if (data.length === 0) {
|
||||
// Empty series — return null for stats so callers can render an explicit
|
||||
// "no data" state instead of treating 0/0/0 as a real reading. v3 audit
|
||||
// caught the previous {min:0,max:0,mean:0} producing nonsense chart axes.
|
||||
return {
|
||||
count: 0,
|
||||
latest: null,
|
||||
min: 0,
|
||||
max: 0,
|
||||
mean: 0,
|
||||
min: null,
|
||||
max: null,
|
||||
mean: null,
|
||||
avg7: null,
|
||||
avg30: null,
|
||||
slope7: null,
|
||||
|
||||
Reference in new issue
Block a user