mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-12 06:06:10 +00:00
fix(compliance): expand multi-time legacy schedules through the canonical engine
A medication whose schedule row carried multiple timesOfDay plus a legacy daysOfWeek (and no rrule / rollingIntervalDays) reported 50% compliance for a fully-adherent 2x/day dose. The expected-slot denominator always ran the canonical recurrence engine and counted both daily slots, but the numerator only delegated to the engine for rrule / rolling / one-shot / non-SCHEDULED rows; a plain-legacy row fell to the local cadence walker, which emitted a single windowStart slot per day and ignored timesOfDay. 1 taken / 2 expected read as 50 percent. Route expandScheduleSlots through the engine for every schedule shape whenever a medication context is supplied, so the numerator and denominator read the same source of truth. Harden the context-less legacy walker to iterate timesOfDay as well, mirroring the engine's legacy branch, so the single remaining non-engine path stays correct for multi-time rows. Drop the now-dead expander gate helper. Add regression guards: a 2x/day legacy row reports 2/2 through calculateCompliance, and expectedSlotCountForDay agrees with buildCadenceTimeline on the slot count for an identical schedule.
This commit is contained in:
1 parent
aed83b7747
commit
013ad5d967
3 files changed
+224
-39
No files matched your search
@@ -23,6 +23,7 @@ import {
|
||||
type ComplianceSchedule,
|
||||
} from "../compliance";
|
||||
import type { IntakeTimingClass } from "../compliance";
|
||||
import { buildCadenceTimeline } from "@/lib/medications/scheduling/cadence";
|
||||
import { getUserTodayBounds } from "@/lib/timezone";
|
||||
import { userDayKey } from "@/lib/tz/format";
|
||||
|
||||
@@ -557,6 +558,39 @@ describe("calculateCompliance — engine-routed (medicationContext)", () => {
|
||||
expect(result.rate).toBe(100);
|
||||
expect(result.missed).toBe(0);
|
||||
});
|
||||
|
||||
it("B15: legacy daysOfWeek row with two timesOfDay — 2×/day taken reads 100%, not 50%", () => {
|
||||
// Regression for the compliance divergence: a plain `daysOfWeek`
|
||||
// schedule (no rrule / rolling, SCHEDULED, not one-shot) carrying two
|
||||
// `timesOfDay` must expand its numerator through the same engine as
|
||||
// the denominator. Before the fix the numerator's local legacy walker
|
||||
// emitted one slot/day from `windowStart`, while the denominator's
|
||||
// engine counted both — collapsing a fully-adherent 2×/day med to
|
||||
// 1/2 = 50%.
|
||||
const schedules: ComplianceSchedule[] = [
|
||||
{
|
||||
windowStart: "08:00",
|
||||
windowEnd: "19:30",
|
||||
daysOfWeek: null, // every day
|
||||
timesOfDay: ["08:00", "19:00"],
|
||||
// no rrule, no rollingIntervalDays, scheduleType defaults SCHEDULED
|
||||
},
|
||||
];
|
||||
// Pin a single-day window so exactly two slots are expected. NOW is
|
||||
// late evening UTC so both of today's slots are in the past.
|
||||
vi.setSystemTime(new Date("2025-06-09T23:30:00Z"));
|
||||
const events = [
|
||||
eventAt(new Date("2025-06-09T08:00:00Z"), true),
|
||||
eventAt(new Date("2025-06-09T19:00:00Z"), true),
|
||||
];
|
||||
const result = calculateCompliance(events, schedules, 1, undefined, {
|
||||
medicationContext: ctx({ startsOn: new Date("2025-05-01T00:00:00Z") }),
|
||||
});
|
||||
// Both daily slots expand and both pair with a taken intake → 2/2.
|
||||
expect(result.totalExpected).toBe(2);
|
||||
expect(result.taken).toBe(2);
|
||||
expect(result.rate).toBe(100);
|
||||
});
|
||||
});
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────
|
||||
@@ -645,6 +679,62 @@ describe("expectedSlotCountForDay + lastNonSkippedTakenAt", () => {
|
||||
expect(count).toBe(2);
|
||||
});
|
||||
|
||||
it("B15 invariant: legacy multi-time row — denominator and numerator expand identically", () => {
|
||||
// The B15 bug lived in the numerator's expander, not here — this
|
||||
// helper always ran the engine. Pin the convergence directly: for a
|
||||
// plain `daysOfWeek` row carrying two `timesOfDay` (no rrule), the
|
||||
// per-day denominator and the cadence numerator
|
||||
// (`buildCadenceTimeline`) must report the same slot count.
|
||||
const dayStart = new Date("2025-06-09T00:00:00Z");
|
||||
const dayEnd = new Date("2025-06-10T00:00:00Z");
|
||||
const schedules: ComplianceSchedule[] = [
|
||||
{
|
||||
windowStart: "08:00",
|
||||
windowEnd: "19:30",
|
||||
daysOfWeek: null,
|
||||
timesOfDay: ["08:00", "19:00"],
|
||||
},
|
||||
];
|
||||
const medCtx = ctx({
|
||||
startsOn: new Date("2025-05-01T00:00:00Z"),
|
||||
timeZone: "Europe/Berlin",
|
||||
});
|
||||
|
||||
const denominator = expectedSlotCountForDay(
|
||||
schedules,
|
||||
dayStart,
|
||||
dayEnd,
|
||||
medCtx,
|
||||
);
|
||||
|
||||
const numerator = buildCadenceTimeline(
|
||||
[
|
||||
{
|
||||
windowStart: "08:00",
|
||||
windowEnd: "19:30",
|
||||
daysOfWeek: null,
|
||||
timesOfDay: ["08:00", "19:00"],
|
||||
},
|
||||
],
|
||||
[],
|
||||
new Date("2025-06-09T23:59:00Z"),
|
||||
1,
|
||||
dayStart,
|
||||
medCtx.timeZone,
|
||||
{
|
||||
startsOn: medCtx.startsOn,
|
||||
endsOn: medCtx.endsOn,
|
||||
oneShot: medCtx.oneShot,
|
||||
createdAt: medCtx.createdAt,
|
||||
lastIntakeAt: medCtx.lastIntakeAt,
|
||||
timeZone: medCtx.timeZone,
|
||||
},
|
||||
).length;
|
||||
|
||||
expect(numerator).toBe(denominator);
|
||||
expect(numerator).toBe(2);
|
||||
});
|
||||
|
||||
it("lastNonSkippedTakenAt picks the newest non-skipped takenAt", () => {
|
||||
const events = [
|
||||
{ takenAt: new Date("2025-01-10T08:00:00Z"), skipped: false },
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { complianceChips } from "../compliance";
|
||||
import { buildCadenceTimeline } from "../cadence";
|
||||
import type {
|
||||
CadenceEngineContext,
|
||||
IntakeEventLike,
|
||||
@@ -318,4 +319,41 @@ describe("complianceChips — canonical-engine delegation (v1.7.0 SB-SCHED-2)",
|
||||
// than the equivalent every-day SCHEDULED expansion.
|
||||
expect(engine.missedLast30).toBeLessThan(everyDay.missedLast30);
|
||||
});
|
||||
|
||||
it("B15: a legacy daysOfWeek row with multiple timesOfDay emits one slot per time", () => {
|
||||
// Repro of the compliance divergence: a plain `daysOfWeek` schedule
|
||||
// (no rrule / rolling, SCHEDULED, not one-shot) carrying two
|
||||
// `timesOfDay` must expand to two slots per qualifying day through the
|
||||
// engine. Before the fix the cadence numerator routed such a row to
|
||||
// the local legacy walker, which emitted a single slot/day from
|
||||
// `windowStart` — so a 2×/day medication reported 1/2 = 50%.
|
||||
const twiceDaily: ScheduleLike = {
|
||||
windowStart: "08:00",
|
||||
windowEnd: "19:30",
|
||||
daysOfWeek: null, // every day
|
||||
timesOfDay: ["08:00", "19:00"],
|
||||
// no rrule, no rollingIntervalDays, scheduleType defaults to SCHEDULED
|
||||
};
|
||||
// A single 24-hour window starting at local midnight Berlin.
|
||||
const dayStart = d("2025-06-09T00:00:00Z");
|
||||
const NOW = d("2025-06-09T23:59:00Z");
|
||||
const ctx = engineCtx({
|
||||
startsOn: d("2025-05-01T00:00:00Z"),
|
||||
timeZone: "Europe/Berlin",
|
||||
});
|
||||
|
||||
const timeline = buildCadenceTimeline(
|
||||
[twiceDaily],
|
||||
[],
|
||||
NOW,
|
||||
1,
|
||||
dayStart,
|
||||
ctx.timeZone,
|
||||
ctx,
|
||||
);
|
||||
|
||||
// Both configured times land inside the one-day window → two slots.
|
||||
expect(timeline.length).toBe(2);
|
||||
});
|
||||
|
||||
});
|
||||
@@ -44,16 +44,20 @@ export interface ScheduleLike {
|
||||
/** Encoded recurrence string per `serializeScheduleRecurrence`. */
|
||||
daysOfWeek: string | null;
|
||||
/**
|
||||
* v1.7.0 SB-SCHED-2 — canonical-engine fields. When any of these is
|
||||
* present AND the caller supplies a `CadenceEngineContext`,
|
||||
* v1.7.0 SB-SCHED-2 — canonical-engine fields. As of v1.7.3 (B15),
|
||||
* `expandScheduleSlots` delegates the expected-slot grid to the
|
||||
* canonical recurrence engine (`occurrencesBetween`) instead of the
|
||||
* legacy `daysOfWeek` walker. This routes compliance through the same
|
||||
* engine the projector + reminder worker use, fixing the long-standing
|
||||
* bug where an `rrule = "FREQ=WEEKLY;BYDAY=MO"` schedule expanded to
|
||||
* canonical recurrence engine (`occurrencesBetween`) for EVERY schedule
|
||||
* shape whenever the caller supplies a `CadenceEngineContext` — these
|
||||
* fields carry the recurrence detail the engine reads (rrule / rolling
|
||||
* / cyclic), and a plain legacy `daysOfWeek` row routes through the
|
||||
* engine's `expandLegacy` branch all the same. This keeps compliance on
|
||||
* the same engine the projector + reminder worker use, fixing the bug
|
||||
* where an `rrule = "FREQ=WEEKLY;BYDAY=MO"` schedule expanded to
|
||||
* daily-every-day in compliance (because `daysOfWeek = null` reads as
|
||||
* "every day"). Absent fields keep the legacy weekday path so existing
|
||||
* fixtures / pre-v1.7 callers are byte-stable.
|
||||
* "every day") AND the B15 bug where a multi-`timesOfDay` legacy row
|
||||
* collapsed to one slot/day in the numerator. Only callers that omit a
|
||||
* context fall to the legacy weekday walker (byte-stable for pure-math
|
||||
* / pre-v1.7 fixtures).
|
||||
*/
|
||||
rrule?: string | null;
|
||||
rollingIntervalDays?: number | null;
|
||||
@@ -84,16 +88,6 @@ export interface CadenceEngineContext {
|
||||
timeZone: string;
|
||||
}
|
||||
|
||||
/** A schedule routes through the canonical engine iff it carries a
|
||||
* non-legacy recurrence shape OR a non-SCHEDULED schedule type. */
|
||||
function usesCanonicalEngine(schedule: ScheduleLike): boolean {
|
||||
return (
|
||||
(schedule.rrule ?? null) !== null ||
|
||||
(schedule.rollingIntervalDays ?? null) !== null ||
|
||||
(schedule.scheduleType ?? "SCHEDULED") !== "SCHEDULED"
|
||||
);
|
||||
}
|
||||
|
||||
/** Build a `CanonicalSchedule` from a `ScheduleLike` + its index. */
|
||||
function toCanonical(
|
||||
schedule: ScheduleLike,
|
||||
@@ -230,6 +224,15 @@ function applyTime(day: Date, hhmm: string, tz: string | undefined): Date {
|
||||
return instantInTz(parts.year, parts.month, parts.day, h, m, tz);
|
||||
}
|
||||
|
||||
/** Parse "HH:mm" to minutes-since-midnight, or null when malformed. */
|
||||
function hhmmToMinutes(hhmm: string): number | null {
|
||||
const [hStr, mStr] = hhmm.split(":");
|
||||
const h = Number(hStr);
|
||||
const m = Number(mStr);
|
||||
if (!Number.isFinite(h) || !Number.isFinite(m)) return null;
|
||||
return h * 60 + m;
|
||||
}
|
||||
|
||||
/** Snap a Date down to the user-local midnight. */
|
||||
function startOfLocalDay(d: Date, tz: string | undefined): Date {
|
||||
const parts = wallClockInTz(d, tz);
|
||||
@@ -267,14 +270,25 @@ export function expandScheduleSlots(
|
||||
): ExpectedDose[] {
|
||||
if (to <= from) return [];
|
||||
|
||||
// v1.7.0 SB-SCHED-2 — Option B delegation. When the caller supplies a
|
||||
// medication context AND the schedule carries a canonical recurrence
|
||||
// (rrule / rolling) or a non-SCHEDULED type, expand the expected-slot
|
||||
// grid through the canonical engine so compliance, the cadence chart,
|
||||
// and the projector all read the same source of truth. The legacy
|
||||
// weekday walker below stays the path for plain `daysOfWeek` schedules
|
||||
// and for every caller that doesn't thread a context (byte-stable).
|
||||
if (engineCtx && (engineCtx.oneShot || usesCanonicalEngine(schedule))) {
|
||||
// v1.7.3 B15 — canonical-engine delegation. When the caller supplies a
|
||||
// medication context, expand the expected-slot grid through the
|
||||
// canonical engine for EVERY schedule shape (rrule / rolling / one-shot
|
||||
// / PRN / cyclic AND plain legacy `daysOfWeek`). The engine's
|
||||
// `expandLegacy` branch iterates `effectiveTimesOfDay` correctly, so a
|
||||
// single legacy row carrying multiple `timesOfDay` emits one slot per
|
||||
// time — matching exactly what `expectedSlotCountForDay` already counts.
|
||||
//
|
||||
// The earlier gate (`oneShot || usesCanonicalEngine`) diverged the
|
||||
// compliance numerator from the denominator: a 2×/day legacy schedule
|
||||
// (`daysOfWeek` set, two `timesOfDay`, no rrule) fell through to the
|
||||
// local legacy walker below, which only ever emits ONE slot/day from
|
||||
// `windowStart`, while `expectedSlotCountForDay` always ran the engine
|
||||
// and counted both slots → 1/2 = 50%. Routing both sides through the
|
||||
// single engine converges them for every schedule form.
|
||||
//
|
||||
// The local legacy walker below stays the path only when no context is
|
||||
// threaded (pure-math callers / pre-v1.7 fixtures) — byte-stable.
|
||||
if (engineCtx) {
|
||||
const canonical = toCanonical(schedule, scheduleIndex);
|
||||
const recurrenceCtx = toRecurrenceContext(engineCtx);
|
||||
// v1.7.0 code-correctness M5 — `occurrencesBetween` is inclusive of
|
||||
@@ -333,22 +347,65 @@ export function expandScheduleSlots(
|
||||
}
|
||||
}
|
||||
|
||||
const wStart = applyTime(day, schedule.windowStart, timeZone);
|
||||
let wEnd = applyTime(day, schedule.windowEnd, timeZone);
|
||||
// Overnight window: windowEnd <= windowStart means next day.
|
||||
if (wEnd <= wStart) {
|
||||
wEnd = new Date(wEnd.getTime() + DAY_MS);
|
||||
// v1.7.3 B15 — defence-in-depth for the context-less legacy path. A
|
||||
// schedule carrying multiple `timesOfDay` must emit one slot per time
|
||||
// per qualifying day, mirroring the engine's `expandLegacy`. Pre-fix
|
||||
// this walker only ever emitted a single `windowStart..windowEnd`
|
||||
// window, so a 2×/day legacy row read as one slot/day here — the
|
||||
// numerator half of the B15 divergence. When `timesOfDay` is empty we
|
||||
// keep the historical single-window behaviour (byte-stable for every
|
||||
// pre-v1.7 fixture that relies on it).
|
||||
const times =
|
||||
schedule.timesOfDay && schedule.timesOfDay.length > 0
|
||||
? schedule.timesOfDay
|
||||
: null;
|
||||
|
||||
if (times === null) {
|
||||
const wStart = applyTime(day, schedule.windowStart, timeZone);
|
||||
let wEnd = applyTime(day, schedule.windowEnd, timeZone);
|
||||
// Overnight window: windowEnd <= windowStart means next day.
|
||||
if (wEnd <= wStart) {
|
||||
wEnd = new Date(wEnd.getTime() + DAY_MS);
|
||||
}
|
||||
|
||||
if (wStart >= to) continue;
|
||||
if (wEnd <= from) continue;
|
||||
|
||||
slots.push({
|
||||
day,
|
||||
windowStart: wStart,
|
||||
windowEnd: wEnd,
|
||||
scheduleIndex,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
if (wStart >= to) continue;
|
||||
if (wEnd <= from) continue;
|
||||
for (const time of times) {
|
||||
const wStart = applyTime(day, time, timeZone);
|
||||
// Each time-of-day slot spans the schedule's window length. Reuse
|
||||
// the windowStart..windowEnd span so the pairing radius and the
|
||||
// chart cell keep their existing shape; an overnight window pushes
|
||||
// the end to the next day.
|
||||
const startMin = hhmmToMinutes(schedule.windowStart);
|
||||
const endMin = hhmmToMinutes(schedule.windowEnd);
|
||||
let spanMs = DAY_MS;
|
||||
if (startMin !== null && endMin !== null) {
|
||||
let span = endMin - startMin;
|
||||
if (span <= 0) span += 24 * 60; // overnight window
|
||||
spanMs = span * 60_000;
|
||||
}
|
||||
const wEnd = new Date(wStart.getTime() + spanMs);
|
||||
|
||||
slots.push({
|
||||
day,
|
||||
windowStart: wStart,
|
||||
windowEnd: wEnd,
|
||||
scheduleIndex,
|
||||
});
|
||||
if (wStart >= to) continue;
|
||||
if (wEnd <= from) continue;
|
||||
|
||||
slots.push({
|
||||
day,
|
||||
windowStart: wStart,
|
||||
windowEnd: wEnd,
|
||||
scheduleIndex,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return slots;
|
||||
|
||||
Reference in new issue
Block a user