mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 14:40:52 +00:00
fix(reminders): resolve each dose slot's own window duration, not the whole schedule's span
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.
This commit is contained in:
1 parent
592705550f
commit
08e535a0d9
3 files changed
+305
-7
No files matched your search
@@ -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<ReminderCheckPayload>[]) {
|
||||
// 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<ReminderCheckPayload>[]) {
|
||||
// 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;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user