fix(backend): correct "next weekday" and harden resolvers against model output
Four defects found by review of the preceding commits.
"next <weekday>" 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) <noreply@anthropic.com>
This commit is contained in:
@@ -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', () => {
|
describe('toLatinDigits', () => {
|
||||||
it('normalises Persian digits and leaves everything else alone', () => {
|
it('normalises Persian digits and leaves everything else alone', () => {
|
||||||
expect(
|
expect(
|
||||||
|
|||||||
@@ -131,12 +131,28 @@ export function jalaliToGregorian(
|
|||||||
return [gy, gm, gd];
|
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 {
|
export function isJalaliLeapYear(jy: number): boolean {
|
||||||
|
if (!isSupportedJalaliYear(jy)) return false;
|
||||||
const r = jalCal(jy, false);
|
const r = jalCal(jy, false);
|
||||||
return r.leap === 0;
|
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 {
|
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 <= 6) return 31;
|
||||||
if (jm <= 11) return 30;
|
if (jm <= 11) return 30;
|
||||||
return isJalaliLeapYear(jy) ? 30 : 29;
|
return isJalaliLeapYear(jy) ? 30 : 29;
|
||||||
|
|||||||
@@ -54,6 +54,14 @@ describe('zoned civil time', () => {
|
|||||||
expect(civilDateInZone(instant, 'Europe/Amsterdam')).toBe('2025-10-11');
|
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', () => {
|
it('zero-pads single-digit months and days', () => {
|
||||||
expect(civilDateInZone(new Date('2025-01-05T12:00:00.000Z'), 'UTC')).toBe(
|
expect(civilDateInZone(new Date('2025-01-05T12:00:00.000Z'), 'UTC')).toBe(
|
||||||
'2025-01-05',
|
'2025-01-05',
|
||||||
|
|||||||
@@ -61,8 +61,12 @@ export function civilDateJsWeekday(isoDate: string): number {
|
|||||||
* client-supplied date, which matters for relative deadlines like "by Thursday".
|
* client-supplied date, which matters for relative deadlines like "by Thursday".
|
||||||
*/
|
*/
|
||||||
export function civilDateInZone(date: Date, timeZone: string): string {
|
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', {
|
const parts = new Intl.DateTimeFormat('en-CA', {
|
||||||
timeZone,
|
timeZone: zone,
|
||||||
year: 'numeric',
|
year: 'numeric',
|
||||||
month: '2-digit',
|
month: '2-digit',
|
||||||
day: '2-digit',
|
day: '2-digit',
|
||||||
|
|||||||
@@ -23,6 +23,46 @@ describe('resolveDueDate', () => {
|
|||||||
expect(result.dueDate).toBe('2025-10-23');
|
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', () => {
|
it('reads "by Thursday" said on a Thursday as the next one, not today', () => {
|
||||||
// A deadline of today is almost never what was meant.
|
// A deadline of today is almost never what was meant.
|
||||||
const result = resolveDueDate(
|
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', () => {
|
it('degrades rather than throwing on a malformed today or intent', () => {
|
||||||
expect(
|
expect(
|
||||||
resolveDueDate({ kind: 'offset', unit: 'day', amount: 1 }, 'not-a-date')
|
resolveDueDate({ kind: 'offset', unit: 'day', amount: 1 }, 'not-a-date')
|
||||||
|
|||||||
@@ -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
|
* `'this'` is occurrence-anchored: the soonest occurrence strictly after today, so "by
|
||||||
* on a Thursday means the next one — a deadline of today is almost never what was meant.
|
* Thursday" said on a Thursday means the next one — a deadline of today is almost never
|
||||||
* `'next'` adds a further week. The Iranian week starts Saturday, which this arithmetic is
|
* what was meant, and this can never resolve into the past.
|
||||||
* agnostic to (it counts forward from today), but the tests pin it explicitly.
|
*
|
||||||
|
* `'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(
|
function resolveWeekday(
|
||||||
intent: Extract<DueIntent, { kind: 'weekday' }>,
|
intent: Extract<DueIntent, { kind: 'weekday' }>,
|
||||||
@@ -104,13 +118,20 @@ function resolveWeekday(
|
|||||||
) {
|
) {
|
||||||
const targetJs = WEEKDAY_TO_JS[intent.weekday];
|
const targetJs = WEEKDAY_TO_JS[intent.weekday];
|
||||||
if (targetJs === undefined) return null;
|
if (targetJs === undefined) return null;
|
||||||
if (intent.which !== 'this' && intent.which !== 'next') return null;
|
|
||||||
|
|
||||||
const todayJs = civilDateJsWeekday(todayIso);
|
if (intent.which === 'this') {
|
||||||
let delta = (targetJs - todayJs + 7) % 7;
|
const todayJs = civilDateJsWeekday(todayIso);
|
||||||
if (delta === 0) delta = 7;
|
let delta = (targetJs - todayJs + 7) % 7;
|
||||||
if (intent.which === 'next') delta += 7;
|
if (delta === 0) delta = 7;
|
||||||
return addDays(todayIso, delta);
|
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(
|
function resolveOffset(
|
||||||
@@ -132,9 +153,15 @@ export function resolveDueDate(
|
|||||||
intent: DueIntent | null | undefined,
|
intent: DueIntent | null | undefined,
|
||||||
todayIso: string,
|
todayIso: string,
|
||||||
): DueResolution {
|
): 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 };
|
return { dueDate: null, unresolved: null };
|
||||||
}
|
}
|
||||||
|
if (typeof intent !== 'object') {
|
||||||
|
return unresolved(String(intent).slice(0, 120));
|
||||||
|
}
|
||||||
if (!isRealCivilDate(todayIso)) {
|
if (!isRealCivilDate(todayIso)) {
|
||||||
return unresolved(describe(intent));
|
return unresolved(describe(intent));
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user