From c7e839f97a97d96b7fefb378c7c02482a79b5934 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 28 May 2026 10:05:41 +0200 Subject: [PATCH] fix(meds): close the legacy-tier startsOn gap in the recurrence engine The legacy fallback now honours medication.startsOn the way every other dispatch tier does, so a legacy-shape schedule with a future startsOn no longer emits historical slots. Adds a MAX_CHUNKS cap on the forward walk for defence-in-depth, surfaces RRULE parse errors through annotate, and stops the engine from compounding the user's UNTIL/COUNT with its own UNTIL suffix. The header doc-comment is rewritten to reflect that the read-flip for the legacy walkers is the v1.5.1 release, not this one. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../scheduling/__tests__/recurrence.test.ts | 142 ++++++++++++++++++ src/lib/medications/scheduling/recurrence.ts | 71 +++++---- 2 files changed, 185 insertions(+), 28 deletions(-) diff --git a/src/lib/medications/scheduling/__tests__/recurrence.test.ts b/src/lib/medications/scheduling/__tests__/recurrence.test.ts index fe7915475..2891b6b78 100644 --- a/src/lib/medications/scheduling/__tests__/recurrence.test.ts +++ b/src/lib/medications/scheduling/__tests__/recurrence.test.ts @@ -595,6 +595,148 @@ describe("endsOn cap", () => { // 01, 02, 03 (endsOn day inclusive) → 3 slots. expect(slots).toHaveLength(3); }); + + it("expandLegacy honours medication.startsOn floor", () => { + // Schedule with daysOfWeek = null → daily; startsOn in the future + // means the legacy walker should emit nothing for the pre-startsOn + // window, mirroring the endsOn cap. Pre-fix the legacy fallback + // ignored startsOn and emitted historical slots. + const schedule = makeSchedule({ + daysOfWeek: null, + windowStart: "08:00", + timesOfDay: [], + }); + const ctx = makeCtx({ + medication: { startsOn: d("2026-06-10T00:00:00Z") }, + }); + // Window straddles startsOn: 06-05..06-12 (8 days), 4 of which + // are before startsOn (06-05..06-08) and 3 on/after (06-10..06-12; + // 06-09 is also before startsOn). + const slots = occurrencesBetween( + schedule, + d("2026-06-05T00:00:00Z"), + d("2026-06-12T23:59:59Z"), + ctx, + ); + expect(slots).toHaveLength(3); + const days = slots.map((s) => + new Intl.DateTimeFormat("en-CA", { + timeZone: "Europe/Berlin", + year: "numeric", + month: "2-digit", + day: "2-digit", + }).format(s.at), + ); + expect(days).toEqual(["2026-06-10", "2026-06-11", "2026-06-12"]); + }); +}); + +// ──────────────────────────────────────────────────────────────────── +// RRULE COUNT / UNTIL collision guard +// ──────────────────────────────────────────────────────────────────── + +describe("expandRrule respects user-supplied COUNT / UNTIL", () => { + it("skips engine-side UNTIL when the user RRULE has COUNT", () => { + // FREQ=DAILY;COUNT=3 → exactly 3 daily occurrences from DTSTART. + // The engine used to append `;UNTIL=` unconditionally, + // producing an invalid two-bound RRULE that rrule.fromString threw + // on, collapsing the schedule to zero slots. + const schedule = makeSchedule({ + rrule: "FREQ=DAILY;COUNT=3", + timesOfDay: ["08:00"], + }); + const ctx = makeCtx({ + medication: { + startsOn: d("2026-06-01T00:00:00Z"), + endsOn: d("2026-06-30T00:00:00Z"), + }, + }); + const slots = occurrencesBetween( + schedule, + d("2026-06-01T00:00:00Z"), + d("2026-06-30T23:59:59Z"), + ctx, + ); + // COUNT=3 → June 01, 02, 03 (one slot per day at 08:00 local). + expect(slots).toHaveLength(3); + }); + + it("skips engine-side UNTIL when the user RRULE has UNTIL", () => { + const schedule = makeSchedule({ + rrule: "FREQ=DAILY;UNTIL=20260603T235959Z", + timesOfDay: ["08:00"], + }); + const ctx = makeCtx({ + medication: { + startsOn: d("2026-06-01T00:00:00Z"), + endsOn: d("2026-06-30T00:00:00Z"), + }, + }); + const slots = occurrencesBetween( + schedule, + d("2026-06-01T00:00:00Z"), + d("2026-06-30T23:59:59Z"), + ctx, + ); + expect(slots).toHaveLength(3); + }); + + it("returns [] without throwing on a syntactically malformed rrule", () => { + const schedule = makeSchedule({ + rrule: "FREQ=DAILY", + timesOfDay: ["08:00"], + }); + // Patch with a malformed RRULE that bypasses the Zod regex (the + // engine accepts whatever the caller wired). Verifies the catch + // surfaces zero slots instead of bubbling a ParseError. + const broken = { + ...schedule, + rrule: "FREQ=DAILY;TWICE_DAILY", + }; + const ctx = makeCtx({ + medication: { startsOn: d("2026-06-01T00:00:00Z") }, + }); + expect(() => + occurrencesBetween( + broken, + d("2026-06-01T00:00:00Z"), + d("2026-06-02T23:59:59Z"), + ctx, + ), + ).not.toThrow(); + const slots = occurrencesBetween( + broken, + d("2026-06-01T00:00:00Z"), + d("2026-06-02T23:59:59Z"), + ctx, + ); + expect(slots).toEqual([]); + }); +}); + +// ──────────────────────────────────────────────────────────────────── +// nextOccurrenceAfter MAX_CHUNKS cap +// ──────────────────────────────────────────────────────────────────── + +describe("nextOccurrenceAfter chunk cap", () => { + it("aborts after MAX_CHUNKS for a pathologically-rare schedule with a future startsOn", () => { + // A schedule with a startsOn 50 years in the future will never + // emit a slot within the 10-year hardCap. The chunk-walk would + // otherwise iterate ~40+ 90-day chunks before terminating; with + // MAX_CHUNKS the walk returns null in bounded time. + const schedule = makeSchedule({ + rrule: "FREQ=DAILY", + timesOfDay: ["08:00"], + }); + const ctx = makeCtx({ + medication: { startsOn: d("2080-01-01T00:00:00Z") }, + }); + const start = Date.now(); + const next = nextOccurrenceAfter(schedule, d("2026-06-01T00:00:00Z"), ctx); + const elapsed = Date.now() - start; + expect(next).toBeNull(); + expect(elapsed).toBeLessThan(1000); + }); }); // ──────────────────────────────────────────────────────────────────── diff --git a/src/lib/medications/scheduling/recurrence.ts b/src/lib/medications/scheduling/recurrence.ts index f83f032cc..f20bb2882 100644 --- a/src/lib/medications/scheduling/recurrence.ts +++ b/src/lib/medications/scheduling/recurrence.ts @@ -2,17 +2,13 @@ * v1.5.0 — canonical recurrence engine for medication scheduling. * * Single source of truth for "what dose slots does this schedule emit - * between A and B?" Consumed by the reminder worker, the today - * projector, the cadence chart, the medication card, and the - * medication form. Replaces three duplicate walks (R-3 finding 3): - * - * - `expandScheduleSlots` in `cadence.ts` - * - `getNextOccurrenceTimestamp` in `medication-card.tsx` - * - `getNextRecurrenceDate` in `medication-form.tsx` - * - * Those engines will be retired in a follow-up wire-up step; this - * module ships standalone so the schema migration + the wizard can - * land first. + * between A and B?" The canonical engine is introduced in this release; + * the v1.5.0 cut wires only the reminder worker (via + * `worker-helpers.ts`) through it. The today-projector + * (`expandTodayIntakes`), the cadence chart (`expandScheduleSlots`), + * the medication card (`getNextOccurrenceTimestamp`), and the + * form-level helpers continue on the legacy walker through v1.5.x and + * migrate in v1.5.1 per the read-flip plan. * * Dispatch tiers (first matching tier wins): * @@ -44,6 +40,7 @@ import { RRule } from "rrule"; +import { annotate } from "@/lib/logging/context"; import { parseScheduleRecurrence } from "@/lib/medication-schedule"; import { wallClockInTz } from "@/lib/tz/wall-clock"; @@ -187,9 +184,21 @@ export function nextOccurrenceAfter( } // Calendar cadences — walk in 90-day chunks. + // + // Defence-in-depth alongside the 10-year `hardCap`: every chunk + // costs one `rrule.between(...)` invocation (the rrule lib's + // internal MAXYEAR=9999 bounds each call, but the chunk count + // itself is unbounded). Cap the walk at MAX_CHUNKS so a pathological + // RRULE that emits very rarely (e.g. `FREQ=YEARLY;BYMONTHDAY=29; + // BYMONTH=2`) cannot spend an open-ended amount of time iterating + // the engine. Returns null on cap-hit, matching the "schedule has + // terminated" contract. + const MAX_CHUNKS = 80; const chunkMs = 90 * DAY_MS; let cursor = new Date(after.getTime() + 1); + let chunks = 0; while (cursor.getTime() <= limit.getTime()) { + if (chunks++ >= MAX_CHUNKS) return null; const chunkEnd = new Date( Math.min(cursor.getTime() + chunkMs, limit.getTime()), ); @@ -271,20 +280,6 @@ function expandRolling( if (n === null || n <= 0) return []; const anchor = ctx.lastIntakeAt ?? ctx.medication.startsOn ?? ctx.medication.createdAt; - // No last intake AND no startsOn → terminate. createdAt is the - // schema-level fallback but the design synthesis says rolling - // schedules require a starting reference; we honour that by - // refusing to emit when the caller passes a medication with - // neither lastIntakeAt nor startsOn. (Pre-v1.5 rows always have - // createdAt populated, so this guard only fires on hand-crafted - // test inputs.) - if ( - ctx.lastIntakeAt === null && - ctx.medication.startsOn === null && - !ctx.medication.createdAt - ) { - return []; - } const nextDue = new Date(anchor.getTime() + n * DAY_MS); @@ -318,15 +313,25 @@ function expandRrule( const dtstart = ctx.medication.startsOn ?? ctx.medication.createdAt; const dtstartLine = `DTSTART:${formatUtcBasic(startOfUtcDay(dtstart))}`; - const untilSuffix = ctx.medication.endsOn - ? `;UNTIL=${formatUtcBasic(endOfUtcDay(ctx.medication.endsOn))}` - : ""; + // Skip the engine-side UNTIL suffix when the user's RRULE already + // bounds the recurrence with COUNT or UNTIL — RFC 5545 forbids both + // (and any two-UNTIL collision), and RRule.fromString throws on the + // duplicate, silently collapsing the schedule to zero slots. + const userBoundsRecurrence = /(?:^|;)(?:COUNT|UNTIL)=/.test(rruleStr); + const untilSuffix = + ctx.medication.endsOn && !userBoundsRecurrence + ? `;UNTIL=${formatUtcBasic(endOfUtcDay(ctx.medication.endsOn))}` + : ""; const full = `${dtstartLine}\nRRULE:${rruleStr}${untilSuffix}`; let rule: RRule; try { rule = RRule.fromString(full); } catch { + annotate({ + action: { name: "medication.recurrence.parse_error" }, + meta: { rrule: rruleStr }, + }); return []; } @@ -374,11 +379,21 @@ function expandLegacy( // that might land in [from, to] after applying the local-tz HH:mm. const start = startOfUtcDay(new Date(from.getTime() - DAY_MS)); const end = startOfUtcDay(new Date(to.getTime() + DAY_MS)); + const startsOnFloor = ctx.medication.startsOn + ? startOfUtcDay(ctx.medication.startsOn).getTime() + : null; for ( let day = start; day.getTime() <= end.getTime(); day = new Date(day.getTime() + DAY_MS) ) { + // startsOn floor — every other dispatch tier honours it; the + // legacy walker used to iterate from creation regardless, so a + // legacy-shape schedule with a future startsOn emitted historical + // slots. Mirror the endsOn cap below. + if (startsOnFloor !== null && day.getTime() < startsOnFloor) { + continue; + } // Day-of-week filter (empty = every day). Use the user's // timezone weekday so the legacy "Mon = 1" encoding aligns with // the user's local-Mon, not UTC-Mon.