From 08e535a0d96a155d8c0cc8deb1e5c1d81f055814 Mon Sep 17 00:00:00 2001 From: Mathew Sims <25371401+mathewcsims@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:55:34 +0100 Subject: [PATCH] fix(reminders): resolve each dose slot's own window duration, not the whole schedule's span MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 15-minute reminder tick computed windowDuration ONCE per schedule as windowEnd - windowStart, then applied that SAME span to every timeOfDay slot in the schedule. For a single-dose schedule that span genuinely is the one slot's window, but for a multi-dose schedule (e.g. windowStart=08:00/windowEnd=18:00 with timesOfDay=[08:00,18:00]) it silently gave the 08:00 slot a 600-minute grace before even entering ORANGE phase — ten hours nobody configured — while an identically-intended single-dose schedule with a narrow legacy span (08:00-09:00) correctly escalated on schedule but then had only a 30-minute radius for a logged-but-late dose to suppress its reminder. Same misapplied field, opposite-looking symptoms: one medication silently never reminded, the other kept escalating despite being logged as taken. resolveSlotWindowDurationMinutes resolves each slot's duration independently, checked in priority order: an explicit per-dose doseWindows entry for that exact timeOfDay (the actual per-dose rule set in the dose-window editor) > the schedule's reminderGraceMinutes (documented for exactly this purpose but never actually read by the reminder tick until now — the write path already reads it via graceToleranceMs) > the same default an unconfigured "point" dose gets everywhere else in the app, bounded by the gap to the nearest sibling slot so two doses can never share a window. A single-dose schedule with neither override keeps the exact legacy windowEnd - windowStart behaviour, unchanged, for backward compatibility with schedules that predate the per-dose doseWindows system. The core property this restores: a dose's resolved window never depends on how many sibling doses share its schedule. Two identically-configured doses now resolve identically regardless of whether one of them happens to have a second dose elsewhere in the day. Full suite (18k+ tests) passes with no regressions; 13 new tests cover both real-world cases that surfaced this (a single 08:00-09:00 schedule and a twice-daily 08:00/18:00 one) plus the sibling- independence property directly. --- .../reminder/medication-reminder-check.ts | 23 ++- .../__tests__/worker-helpers.test.ts | 147 ++++++++++++++++++ .../medications/scheduling/worker-helpers.ts | 142 ++++++++++++++++- 3 files changed, 305 insertions(+), 7 deletions(-) diff --git a/src/lib/jobs/reminder/medication-reminder-check.ts b/src/lib/jobs/reminder/medication-reminder-check.ts index ff64ab72d..f230c5cd7 100644 --- a/src/lib/jobs/reminder/medication-reminder-check.ts +++ b/src/lib/jobs/reminder/medication-reminder-check.ts @@ -14,6 +14,7 @@ import { medicationDoseTag } from "@/lib/notifications/dose-tag"; import { buildCanonicalSchedule, buildRecurrenceContext, + resolveSlotWindowDurationMinutes, scheduleEmitsInWindow, shouldMintMissedDoseRow, } from "@/lib/medications/scheduling/worker-helpers"; @@ -290,17 +291,15 @@ export async function handleReminderCheck(jobs: Job[]) { // slots per day; the pre-v1.7 worker keyed phase + dedup on the // single `windowStart`, so the evening dose never reminded. // Iterate every first-class time-of-day, each with its own - // window (anchored at the time, spanning the legacy - // `windowEnd - windowStart` duration), phase, dedup key - // (now including the time-of-day), and RED-mint instant. + // window (anchored at the time, spanning its own resolved + // per-slot duration — see `resolveSlotWindowDurationMinutes`), + // phase, dedup key (now including the time-of-day), and + // RED-mint instant. // // The legacy single-window contract is preserved: a schedule // with no first-class `timesOfDay` emits exactly one slot at // `windowStart` with `timeOfDay = ""`, which dedupes against // pre-v1.7 rows (backfilled to "") byte-for-byte. - const baseStartMins = parseTimeToMinutes(schedule.windowStart); - const baseEndMins = parseTimeToMinutes(schedule.windowEnd); - const windowDuration = baseEndMins - baseStartMins; const currentMins = parseTimeToMinutes(currentTime); const hasFirstClassTimes = @@ -321,6 +320,18 @@ export async function handleReminderCheck(jobs: Job[]) { // explicit HH:mm. const dedupTimeOfDay = hasFirstClassTimes ? slotTime : ""; const slotStartMins = parseTimeToMinutes(slotTime); + // Per-slot, not per-schedule — see resolveSlotWindowDurationMinutes. + // A multi-time-of-day schedule's whole-span windowEnd-windowStart + // is not a safe per-slot duration; this resolves each slot's own + // explicit dose-window / reminderGraceMinutes override (or a + // shared unconfigured-dose default, bounded by the gap to the + // adjacent slot) so a dose's resolved window never depends on + // how many sibling doses its schedule has. + const windowDuration = resolveSlotWindowDurationMinutes( + schedule, + slotTime, + canonicalSchedule.doseWindows ?? null, + ); const slotEndMins = slotStartMins + windowDuration; const minutesToEnd = slotEndMins - currentMins; const minutesFromStart = currentMins - slotStartMins; diff --git a/src/lib/medications/scheduling/__tests__/worker-helpers.test.ts b/src/lib/medications/scheduling/__tests__/worker-helpers.test.ts index 7a9e5bb10..1179e9610 100644 --- a/src/lib/medications/scheduling/__tests__/worker-helpers.test.ts +++ b/src/lib/medications/scheduling/__tests__/worker-helpers.test.ts @@ -8,8 +8,10 @@ import { describe, expect, it } from "vitest"; import { buildCanonicalSchedule, normaliseDoseWindows, + resolveSlotWindowDurationMinutes, type WorkerScheduleRow, } from "../worker-helpers"; +import { DOSE_WINDOW_DEFAULTS } from "../dose-window-defaults"; describe("normaliseDoseWindows", () => { it("returns null for null / non-array input", () => { @@ -78,3 +80,148 @@ describe("normaliseDoseWindows", () => { expect(buildCanonicalSchedule(row).doseWindows).toBeNull(); }); }); + +/** + * `resolveSlotWindowDurationMinutes` — closes the bug where a + * multi-time-of-day schedule's reminder tick reused the WHOLE + * schedule's `windowEnd - windowStart` span for every individual + * slot, silently giving each dose a grace period equal to the gap + * between the schedule's earliest and latest dose instead of its own + * configured window. Reproduces the real-world case that surfaced it: + * two identically-*intended* once/twice-daily medications + * (Lansoprazole: one 08:00 dose; Metoprolol: 08:00 + 18:00 doses) + * resolving to wildly different per-slot durations (60min vs 600min) + * purely because one schedule had a sibling dose. + */ +describe("resolveSlotWindowDurationMinutes", () => { + it("single-time-of-day schedule: unchanged legacy windowEnd - windowStart", () => { + const schedule: WorkerScheduleRow = { + id: "s1", + windowStart: "08:00", + windowEnd: "09:00", + daysOfWeek: null, + timesOfDay: ["08:00"], + reminderGraceMinutes: null, + rrule: null, + rollingIntervalDays: null, + }; + expect(resolveSlotWindowDurationMinutes(schedule, "08:00", null)).toBe(60); + }); + + it("REGRESSION (Lansoprazole real data): single 08:00-09:00 slot resolves to 60min, not inflated", () => { + const schedule: WorkerScheduleRow = { + id: "lansoprazole-schedule", + windowStart: "08:00", + windowEnd: "09:00", + daysOfWeek: null, + timesOfDay: ["08:00"], + reminderGraceMinutes: null, + rrule: "FREQ=DAILY", + rollingIntervalDays: null, + }; + expect(resolveSlotWindowDurationMinutes(schedule, "08:00", null)).toBe(60); + }); + + it("REGRESSION (Metoprolol real data): a 08:00/18:00 schedule no longer gives the 08:00 slot a 600min window", () => { + const schedule: WorkerScheduleRow = { + id: "metoprolol-schedule", + windowStart: "08:00", + windowEnd: "18:00", + daysOfWeek: null, + timesOfDay: ["08:00", "18:00"], + reminderGraceMinutes: null, + rrule: "FREQ=DAILY", + rollingIntervalDays: null, + }; + const morning = resolveSlotWindowDurationMinutes(schedule, "08:00", null); + const evening = resolveSlotWindowDurationMinutes(schedule, "18:00", null); + expect(morning).toBeLessThan(600); + expect(evening).toBeLessThan(600); + expect(morning).toBe(DOSE_WINDOW_DEFAULTS.dailyOnTimeMinutes * 2); + expect(evening).toBe(DOSE_WINDOW_DEFAULTS.dailyOnTimeMinutes * 2); + }); + + it("core requirement: an unconfigured dose resolves the SAME whether or not it has a sibling slot, once both use an explicit override", () => { + const single: WorkerScheduleRow = { + id: "single", + windowStart: "08:00", + windowEnd: "09:00", + daysOfWeek: null, + timesOfDay: ["08:00"], + reminderGraceMinutes: 180, + rrule: null, + rollingIntervalDays: null, + }; + const multi: WorkerScheduleRow = { + id: "multi", + windowStart: "08:00", + windowEnd: "18:00", + daysOfWeek: null, + timesOfDay: ["08:00", "18:00"], + reminderGraceMinutes: 180, + rrule: null, + rollingIntervalDays: null, + }; + // Same explicit reminderGraceMinutes, same slotTime — the presence + // of Metoprolol-style sibling slot must not change the 08:00 + // dose's resolved duration at all. + expect(resolveSlotWindowDurationMinutes(single, "08:00", null)).toBe( + resolveSlotWindowDurationMinutes(multi, "08:00", null), + ); + expect(resolveSlotWindowDurationMinutes(multi, "08:00", null)).toBe(180); + }); + + it("an explicit per-dose doseWindows entry wins over everything else, keyed by timeOfDay", () => { + const schedule: WorkerScheduleRow = { + id: "s1", + windowStart: "08:00", + windowEnd: "18:00", + daysOfWeek: null, + timesOfDay: ["08:00", "18:00"], + reminderGraceMinutes: 180, // present but should be overridden for 08:00 + rrule: null, + rollingIntervalDays: null, + }; + const doseWindows = [ + { timeOfDay: "08:00", start: "07:30", end: "09:30" }, // 120min + ]; + expect( + resolveSlotWindowDurationMinutes(schedule, "08:00", doseWindows), + ).toBe(120); + // The 18:00 slot has no matching doseWindows entry, so it still + // falls through to reminderGraceMinutes. + expect( + resolveSlotWindowDurationMinutes(schedule, "18:00", doseWindows), + ).toBe(180); + }); + + it("caps the unconfigured-dose default at the minimum inter-slot gap so two close doses never overlap", () => { + const schedule: WorkerScheduleRow = { + id: "s1", + windowStart: "08:00", + windowEnd: "09:30", // legacy span irrelevant here — multi-slot ignores it + daysOfWeek: null, + timesOfDay: ["08:00", "09:30"], // only 90min apart — narrower than the 120min default + reminderGraceMinutes: null, + rrule: null, + rollingIntervalDays: null, + }; + const duration = resolveSlotWindowDurationMinutes(schedule, "08:00", null); + expect(duration).toBe(90); + expect(duration).toBeLessThan(DOSE_WINDOW_DEFAULTS.dailyOnTimeMinutes * 2); + }); + + it("caps by the midnight-wrap gap too, not just forward gaps", () => { + const schedule: WorkerScheduleRow = { + id: "s1", + windowStart: "23:00", + windowEnd: "23:30", + daysOfWeek: null, + timesOfDay: ["23:00", "23:30"], // 30min apart, wrapping close to midnight + reminderGraceMinutes: null, + rrule: null, + rollingIntervalDays: null, + }; + expect(resolveSlotWindowDurationMinutes(schedule, "23:00", null)).toBe(30); + }); +}); diff --git a/src/lib/medications/scheduling/worker-helpers.ts b/src/lib/medications/scheduling/worker-helpers.ts index 92e18ffa9..8d3f843f4 100644 --- a/src/lib/medications/scheduling/worker-helpers.ts +++ b/src/lib/medications/scheduling/worker-helpers.ts @@ -23,7 +23,11 @@ import { type ScheduleType, occurrencesBetween, } from "@/lib/medications/scheduling/recurrence"; -import { hhmmToMinutes } from "@/lib/medications/scheduling/hhmm"; +import { + hhmmToMinutes, + hhmmToMinutesOrNull, +} from "@/lib/medications/scheduling/hhmm"; +import { DOSE_WINDOW_DEFAULTS } from "@/lib/medications/scheduling/dose-window-defaults"; /** * Minimal Prisma-shape projection used by the worker. Mirrors the @@ -170,6 +174,142 @@ export function scheduleEmitsInWindow( return occurrencesBetween(schedule, windowStart, windowEnd, ctx).length > 0; } +/** + * Per-slot phase-window duration, in minutes: how long after a dose + * slot's own start time it stays "on-time" before the reminder tick + * starts escalating through ORANGE/RED, and (halved) the radius a + * logged dose must land within to suppress that slot's reminder. + * + * Bug this closes: the reminder tick used to compute this ONCE per + * schedule as the whole schedule's `windowEnd - windowStart` span and + * apply that SAME span to every `timeOfDay` slot in the schedule. That + * is correct for a single-time-of-day schedule (the span already IS + * that one slot's window), but for a multi-time-of-day schedule it + * silently inflates every slot's grace period to the full gap between + * the schedule's earliest and latest dose. A twice-daily schedule with + * `windowStart=08:00`/`windowEnd=18:00` (representing "somewhere + * between the morning and evening dose", not a per-dose window) gave + * its 08:00 slot a 600-minute (10h) grace before it even entered + * ORANGE — silently far too lenient — while a schedule with a single + * `08:00` dose and the same-looking `windowEnd=09:00` correctly used a + * 60-minute span, and thus a 30-minute suppression radius that a dose + * logged more than 30 minutes late could never satisfy — far too + * strict. Same misapplied field, opposite-looking symptoms. + * + * A multi-time-of-day schedule must NOT get a different answer than a + * single-time-of-day one for a dose configured the same way — the + * point of this function is that the count of sibling slots on the + * schedule never changes one slot's own resolved duration except + * through the explicit, bounded inter-slot-overlap guard in tier 4 + * below (which only ever narrows, and only when slots truly are close + * enough together to otherwise overlap). + * + * Four-tier resolution, checked in priority order — the first + * applicable tier wins, each strictly a "the user configured this + * exact thing" tier before falling back to a shared default: + * + * 1. An explicit per-dose `doseWindows` entry for THIS `slotTime` + * (`{ timeOfDay, start, end }`, persisted by the dose-window + * editor — v1.15.18) is the actual rule the user set for this + * specific dose in the GUI. Its `end - start` (in minutes) is + * used directly. Independent of every other dose on the + * schedule, by construction — this is a per-`timeOfDay` lookup. + * 2. `reminderGraceMinutes`, when set, is used directly. The field + * already exists and is documented ("Replaces the implicit + * windowEnd - windowStart span for late-classification") for + * exactly this purpose, and the write-path already reads it via + * `graceToleranceMs` — but the reminder tick itself never read + * it, so setting it had no effect on when/whether a reminder + * fired. This is a schedule-wide override, applied identically + * to every slot on the schedule (the field itself is per-schedule, + * not per-dose, so uniform application here is what the value + * represents — not a slot-count-dependent choice). + * 3. With NEITHER of the above set, this dose has no explicit + * configuration at all, and gets the same default an unconfigured + * "point" dose gets everywhere else in the app (`defaultBandForTime` + * in `dose-window.ts`, `±DOSE_WINDOW_DEFAULTS.dailyOnTimeMinutes`). + * Deliberately NOT the legacy per-schedule `windowEnd - windowStart` + * span here: for a multi-time-of-day schedule that span is not a + * per-dose window at all — it was derived as the earliest and + * latest `timeOfDay` (see the bug writeup above), so an + * unconfigured dose on a two-dose schedule must not silently + * inherit a multi-hour gap purely because it has a sibling. + * 4. Bounded by the minimum gap between adjacent sorted `timesOfDay` + * (the midnight wrap counted as one of those gaps) whenever the + * schedule has more than one slot, so two distinct same-day doses + * can never share a window — this can only ever narrow tiers 1-3, + * never widen them, and is a no-op for a single-time-of-day + * schedule (nothing to overlap with). + * + * The one legacy exception, preserved for backward compatibility: a + * single-time-of-day schedule with none of tiers 1-2 set keeps using + * the literal `windowEnd - windowStart` span rather than tier 3's + * default — for a schedule that predates the per-dose `doseWindows` + * system (v1.15.18), that legacy span IS the one and only window the + * user configured for that dose, and reinterpreting it as "unconfigured" + * would silently narrow an intentionally wide window on every such + * pre-existing schedule. + */ +export function resolveSlotWindowDurationMinutes( + schedule: Pick< + WorkerScheduleRow, + "windowStart" | "windowEnd" | "timesOfDay" | "reminderGraceMinutes" + >, + slotTime: string, + doseWindows: DoseWindowEntry[] | null, +): number { + const explicit = doseWindows?.find((w) => w.timeOfDay === slotTime); + if (explicit) { + const startMin = hhmmToMinutesOrNull(explicit.start); + const endMin = hhmmToMinutesOrNull(explicit.end); + if (startMin !== null && endMin !== null) { + let span = endMin - startMin; + if (span < 0) span += 24 * 60; // overnight window + return span; + } + } + + if ( + typeof schedule.reminderGraceMinutes === "number" && + Number.isFinite(schedule.reminderGraceMinutes) && + schedule.reminderGraceMinutes > 0 + ) { + return schedule.reminderGraceMinutes; + } + + const slots = ( + schedule.timesOfDay && schedule.timesOfDay.length > 0 + ? schedule.timesOfDay + : [schedule.windowStart] + ) + .map(hhmmToMinutesOrNull) + .filter((m): m is number => m !== null) + .sort((a, b) => a - b); + + let span: number; + if (slots.length > 1) { + span = DOSE_WINDOW_DEFAULTS.dailyOnTimeMinutes * 2; + } else { + const startMin = hhmmToMinutesOrNull(schedule.windowStart); + const endMin = hhmmToMinutesOrNull(schedule.windowEnd); + span = startMin === null || endMin === null ? 0 : endMin - startMin; + if (span < 0) span += 24 * 60; // overnight window + return span; // single-slot legacy path — no sibling to cap against + } + + let minGap = Infinity; + for (let i = 1; i < slots.length; i++) { + minGap = Math.min(minGap, slots[i] - slots[i - 1]); + } + const wrapGap = slots[0] + 24 * 60 - slots[slots.length - 1]; + minGap = Math.min(minGap, wrapGap); + if (Number.isFinite(minGap) && minGap > 0) { + span = Math.min(span, minGap); + } + + return span; +} + /** * Minimal Prisma surface the missed-dose guard needs — just a `count` * over `MedicationIntakeEvent`. Keeps the helper unit-testable with a