From a1a999a884ce9dd85b9cc4a074bbaa53214d392c Mon Sep 17 00:00:00 2001 From: Amin Mousavi Date: Thu, 20 Aug 2026 17:25:44 +0330 Subject: [PATCH] fix(backend): correct "next weekday" and harden resolvers against model output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four defects found by review of the preceding commits. "next " was occurrence-anchored ("this" plus seven) rather than week- anchored. Said on a Thursday, "Thursday next week" resolved to +14 instead of +7: next week runs Sat 10-18 to Fri 10-24, so its Thursday is 10-23, not 10-30. A lab case a week late. "next" now counts from the start of the following Saturday-start week, which also lets "this" and "next" correctly coincide — said on a Thursday, "the coming Saturday" and "Saturday next week" are the same day. "this" stays occurrence-anchored so it can never resolve into the past. The other three all come from the same root cause: exported functions that are reachable from untrusted model output must degrade, not throw or drop. - a non-object `due` (the model emitting a bare string) was treated as "no deadline spoken" and silently discarded; only null/undefined mean absent now, anything else is flagged so the clinician sees something was heard and lost - isJalaliLeapYear / jalaliDaysInMonth threw for years outside the conversion table, contradicting the module's own "degrade to null" contract; they now return false / 0, which also makes isValidJalaliDate's day check naturally false - civilDateInZone passed a client-supplied zone straight to Intl, which raises RangeError before any fallback; it now validates and backstops to UTC, so a bad zone costs at most a day rather than a 500 Co-Authored-By: Claude Opus 5 (1M context) --- backend/src/common/jalali.spec.ts | 16 ++++++ backend/src/common/jalali.ts | 16 ++++++ backend/src/common/zoned-civil-time.spec.ts | 8 +++ backend/src/common/zoned-civil-time.ts | 6 ++- .../modules/voice/due-date.resolver.spec.ts | 53 +++++++++++++++++++ .../src/modules/voice/due-date.resolver.ts | 49 +++++++++++++---- 6 files changed, 136 insertions(+), 12 deletions(-) diff --git a/backend/src/common/jalali.spec.ts b/backend/src/common/jalali.spec.ts index 8895dc6..aeb7ec9 100644 --- a/backend/src/common/jalali.spec.ts +++ b/backend/src/common/jalali.spec.ts @@ -72,6 +72,22 @@ describe('jalali calendar', () => { }); }); + describe('no export throws on an unsupported year', () => { + // The whole module is reachable from model-supplied values, so it degrades instead of + // raising — jalaliToIsoDate's guard is not the only entry point. + it.each([9999, -9999, 3178, Number.NaN, 1.5])('year %s', (jy) => { + expect(() => isJalaliLeapYear(jy)).not.toThrow(); + expect(() => jalaliDaysInMonth(jy, 1)).not.toThrow(); + expect(isJalaliLeapYear(jy)).toBe(false); + expect(jalaliDaysInMonth(jy, 1)).toBe(0); + }); + + it('returns 0 days for an impossible month', () => { + expect(jalaliDaysInMonth(1404, 0)).toBe(0); + expect(jalaliDaysInMonth(1404, 13)).toBe(0); + }); + }); + describe('toLatinDigits', () => { it('normalises Persian digits and leaves everything else alone', () => { expect( diff --git a/backend/src/common/jalali.ts b/backend/src/common/jalali.ts index 0c72592..4f03a28 100644 --- a/backend/src/common/jalali.ts +++ b/backend/src/common/jalali.ts @@ -131,12 +131,28 @@ export function jalaliToGregorian( return [gy, gm, gd]; } +/** True when the year is inside the conversion table's supported range. */ +export function isSupportedJalaliYear(jy: number): boolean { + return Number.isInteger(jy) && jy >= MIN_JALALI_YEAR && jy < MAX_JALALI_YEAR; +} + +/** False for unsupported years rather than throwing — see the module contract. */ export function isJalaliLeapYear(jy: number): boolean { + if (!isSupportedJalaliYear(jy)) return false; const r = jalCal(jy, false); return r.leap === 0; } +/** + * Days in a Jalali month, or 0 when the year or month is not real. + * + * Zero rather than a throw: every export here is reachable from model-supplied values, so + * the whole module degrades instead of raising. Zero also makes `isValidJalaliDate`'s + * `jd <= jalaliDaysInMonth(...)` naturally false. + */ export function jalaliDaysInMonth(jy: number, jm: number): number { + if (!isSupportedJalaliYear(jy)) return 0; + if (!Number.isInteger(jm) || jm < 1 || jm > 12) return 0; if (jm <= 6) return 31; if (jm <= 11) return 30; return isJalaliLeapYear(jy) ? 30 : 29; diff --git a/backend/src/common/zoned-civil-time.spec.ts b/backend/src/common/zoned-civil-time.spec.ts index 1fafc00..af0863f 100644 --- a/backend/src/common/zoned-civil-time.spec.ts +++ b/backend/src/common/zoned-civil-time.spec.ts @@ -54,6 +54,14 @@ describe('zoned civil time', () => { expect(civilDateInZone(instant, 'Europe/Amsterdam')).toBe('2025-10-11'); }); + it('falls back to UTC on an invalid zone rather than throwing', () => { + // Intl raises RangeError on an unknown zone and this takes a client-supplied string. + const instant = new Date('2025-10-11T21:30:00.000Z'); + expect(() => civilDateInZone(instant, 'Not/AZone')).not.toThrow(); + expect(civilDateInZone(instant, 'Not/AZone')).toBe('2025-10-11'); + expect(civilDateInZone(instant, '')).toBe('2025-10-11'); + }); + it('zero-pads single-digit months and days', () => { expect(civilDateInZone(new Date('2025-01-05T12:00:00.000Z'), 'UTC')).toBe( '2025-01-05', diff --git a/backend/src/common/zoned-civil-time.ts b/backend/src/common/zoned-civil-time.ts index 6a50fb4..269d224 100644 --- a/backend/src/common/zoned-civil-time.ts +++ b/backend/src/common/zoned-civil-time.ts @@ -61,8 +61,12 @@ export function civilDateJsWeekday(isoDate: string): number { * client-supplied date, which matters for relative deadlines like "by Thursday". */ export function civilDateInZone(date: Date, timeZone: string): string { + // Intl throws RangeError on an unknown zone, before any fallback below could help, and + // this receives a client-supplied string. Callers validate first; this is the backstop + // so a bad zone degrades to a date that is at most a day out rather than a 500. + const zone = isValidIanaTimeZone(timeZone) ? timeZone : 'UTC'; const parts = new Intl.DateTimeFormat('en-CA', { - timeZone, + timeZone: zone, year: 'numeric', month: '2-digit', day: '2-digit', diff --git a/backend/src/modules/voice/due-date.resolver.spec.ts b/backend/src/modules/voice/due-date.resolver.spec.ts index db8599a..90f7fd7 100644 --- a/backend/src/modules/voice/due-date.resolver.spec.ts +++ b/backend/src/modules/voice/due-date.resolver.spec.ts @@ -23,6 +23,46 @@ describe('resolveDueDate', () => { expect(result.dueDate).toBe('2025-10-23'); }); + it('anchors "next" to the week, not to "this" plus seven', () => { + // Said on Thursday 2025-10-16: next week runs Sat 10-18 .. Fri 10-24, so its + // Thursday is 10-23. Adding a week to "this Thursday" (already 10-23) would + // overshoot to 10-30 — a lab case a week late. + expect( + resolveDueDate( + { kind: 'weekday', weekday: 'thursday', which: 'next' }, + THURSDAY, + ).dueDate, + ).toBe('2025-10-23'); + }); + + it('lets "this" and "next" coincide when they name the same day', () => { + // On a Thursday, "the coming Saturday" and "Saturday next week" are both 10-18. + expect( + resolveDueDate( + { kind: 'weekday', weekday: 'saturday', which: 'this' }, + THURSDAY, + ).dueDate, + ).toBe('2025-10-18'); + expect( + resolveDueDate( + { kind: 'weekday', weekday: 'saturday', which: 'next' }, + THURSDAY, + ).dueDate, + ).toBe('2025-10-18'); + }); + + it('never resolves a weekday into the past', () => { + // Sunday already passed in the week containing Thursday 10-16. + for (const which of ['this', 'next'] as const) { + const result = resolveDueDate( + { kind: 'weekday', weekday: 'sunday', which }, + THURSDAY, + ); + expect(result.dueDate).not.toBeNull(); + expect(result.dueDate! > THURSDAY).toBe(true); + } + }); + it('reads "by Thursday" said on a Thursday as the next one, not today', () => { // A deadline of today is almost never what was meant. const result = resolveDueDate( @@ -199,6 +239,19 @@ describe('resolveDueDate', () => { }); }); + it('flags a non-object deadline instead of silently dropping it', () => { + // A bare string is a deadline we failed to parse, not an absent one — the clinician + // must see that something was heard and lost. + for (const bad of ['next thursday', 42, true]) { + const result = resolveDueDate(bad as never, SATURDAY); + expect(result.dueDate).toBeNull(); + expect(result.unresolved?.reason).toBe('invalid_date'); + } + expect( + resolveDueDate('next thursday' as never, SATURDAY).unresolved?.spoken, + ).toBe('next thursday'); + }); + it('degrades rather than throwing on a malformed today or intent', () => { expect( resolveDueDate({ kind: 'offset', unit: 'day', amount: 1 }, 'not-a-date') diff --git a/backend/src/modules/voice/due-date.resolver.ts b/backend/src/modules/voice/due-date.resolver.ts index 0584e22..e833d8c 100644 --- a/backend/src/modules/voice/due-date.resolver.ts +++ b/backend/src/modules/voice/due-date.resolver.ts @@ -92,11 +92,25 @@ function describe(intent: DueIntent): string { } } +/** The Iranian week starts Saturday. */ +const WEEK_START_JS = WEEKDAY_TO_JS.saturday; + +/** Most recent Saturday, counting today if today is Saturday. */ +function startOfWeek(iso: string): string { + const back = (civilDateJsWeekday(iso) - WEEK_START_JS + 7) % 7; + return addDays(iso, -back); +} + /** - * `which: 'this'` means the soonest occurrence strictly after today, so "by Thursday" said - * on a Thursday means the next one — a deadline of today is almost never what was meant. - * `'next'` adds a further week. The Iranian week starts Saturday, which this arithmetic is - * agnostic to (it counts forward from today), but the tests pin it explicitly. + * `'this'` is occurrence-anchored: the soonest occurrence strictly after today, so "by + * Thursday" said on a Thursday means the next one — a deadline of today is almost never + * what was meant, and this can never resolve into the past. + * + * `'next'` is *week*-anchored, not "this plus seven". "Thursday next week" means the + * Thursday of the Saturday-start week after this one; adding a week to `'this'` would + * overshoot by seven days whenever `'this'` had already rolled into next week. The two + * can legitimately coincide — said on a Thursday, "the coming Saturday" and "Saturday + * next week" are the same day. */ function resolveWeekday( intent: Extract, @@ -104,13 +118,20 @@ function resolveWeekday( ) { const targetJs = WEEKDAY_TO_JS[intent.weekday]; if (targetJs === undefined) return null; - if (intent.which !== 'this' && intent.which !== 'next') return null; - const todayJs = civilDateJsWeekday(todayIso); - let delta = (targetJs - todayJs + 7) % 7; - if (delta === 0) delta = 7; - if (intent.which === 'next') delta += 7; - return addDays(todayIso, delta); + if (intent.which === 'this') { + const todayJs = civilDateJsWeekday(todayIso); + let delta = (targetJs - todayJs + 7) % 7; + if (delta === 0) delta = 7; + return addDays(todayIso, delta); + } + + if (intent.which === 'next') { + const offsetInWeek = (targetJs - WEEK_START_JS + 7) % 7; + return addDays(startOfWeek(todayIso), 7 + offsetInWeek); + } + + return null; } function resolveOffset( @@ -132,9 +153,15 @@ export function resolveDueDate( intent: DueIntent | null | undefined, todayIso: string, ): DueResolution { - if (!intent || typeof intent !== 'object') { + // Absent is not an error — most utterances carry no deadline. Anything else that is not + // an intent object is a deadline we failed to understand, and must be flagged rather + // than silently dropped. + if (intent === null || intent === undefined) { return { dueDate: null, unresolved: null }; } + if (typeof intent !== 'object') { + return unresolved(String(intent).slice(0, 120)); + } if (!isRealCivilDate(todayIso)) { return unresolved(describe(intent)); }