fix(cal): refuse phrases the scheduler cannot resolve exactly #428

Merged
KofTwentyTwo merged 1 commits from fix/nl-scheduler-honesty into dev 2026-08-15 02:04:00 +00:00
KofTwentyTwo commented 2026-08-15 01:54:21 +00:00 (Migrated from github.com)

The defect

NaturalLanguageScheduler::parse ignored the requested weekday and time entirely. It always returned reference_timestamp + 86400 — tomorrow, at the same clock time as the reference — and never errored on unrecognised phrasing. Duration was matched by substring against three hardcoded strings (45 min, 30 min, 1 hour) and silently fell back to 30 minutes for everything else.

So "Coffee with Bob next Tuesday at 2pm for 45 minutes" returned an event that was neither Tuesday nor 2pm, with no error. It also fabricated account_id: "default" / calendar_id: "default" on the returned CalendarEvent.

Resolution: narrow it

I chose to narrow the grammar rather than delete or blanket-Unimplemented the parser. The DST-correct core is small, chrono/chrono-tz were already dependencies, and refusing the whole feature would have thrown away work that is genuinely achievable and now genuinely tested.

Supported — and only these

<summary> <day> at <time> for <duration>
Field Accepted
<summary> any non-empty text (separators matched right-to-left, so "Prep for review at HQ tomorrow at 2pm for 45 minutes" works)
<day> today, tomorrow, YYYY-MM-DD
<time> noon, midnight, 3pm, 3:15pm, 9am, 9:05am, 24-hour HH:MM
<duration> <n> minute(s)/min(s), <n> hour(s)/hr(s); non-zero, max 24 hours

Everything else returns an error

  • Weekday phrasings (next Tuesday, this Friday, on Monday, bare Tuesday) → AmbiguousWeekday. Refused deliberately, not merely unimplemented: English speakers do not agree whether "next Tuesday" is the coming Tuesday or the one a week later, and the readings differ by seven days. There is no reading the parser could pick that would not be wrong for a large share of users.
  • Missing time or duration → MissingTime / MissingDuration. Neither is defaulted — a defaulted duration is a guess about intent, and without a time there is no instant to resolve.
  • Bare hours (at 3) → UnsupportedTime; no way to tell 03:00 from 15:00.
  • Unsupported days, malformed minute fields, unparseable or out-of-range durations, and invalid calendar dates each return their own error variant.

Time zones and DST

parse now requires an explicit IANA Tz. A bare Unix timestamp cannot anchor "3pm", and assuming a zone is exactly the bug class this issue is about.

Both local-time anomalies are reported rather than papered over:

  • Time inside a spring-forward gap → NonexistentLocalTime (e.g. 2026-03-08 02:30 America/New_York)
  • Time inside a fall-back overlap → AmbiguousLocalTime (e.g. 2026-11-01 01:30 America/New_York)

Durations are added as elapsed seconds, so an event spanning a transition keeps its real length and its wall-clock end time shifts — which is what a physical meeting does.

Return type

parse now returns ParsedSchedulingIntent (previously a dead struct) instead of a CalendarEvent. A parser cannot know which calendar the user meant, so it no longer invents "default" account and calendar ids. The never-parsed location/attendees fields were dropped from the struct.

Caller impact

None. There is no in-tree consumer outside the crate's own lib.rs re-export — no proto surface, no CLI command, no daemon wiring. This is a POC leftover, as the issue suspected. The signature change and the narrower grammar are therefore not visible to any client today.

Tests

22 tests, all with an injected reference instant — no wall-clock dependency. Every supported phrase asserts an exact hardcoded Unix instant (computed independently via Python zoneinfo, not through the implementation's own code path); every unsupported phrase asserts a specific error variant.

Mutation-verified the timezone/DST behaviour — all four mutants killed:

Mutation Killed by
Resolve in UTC, ignoring the caller's zone 11 tests
Silently pick the earliest instant for an ambiguous fall-back time a_time_inside_the_fall_back_overlap_is_refused
Silently shift a nonexistent spring-forward time forward an hour a_time_inside_the_spring_forward_gap_is_refused
Treat the duration as wall-clock rather than elapsed time 3 tests

Gate

cargo fmt --all -- --check, cargo check-all, and cargo test-all are all green. Three additional RUST_TEST_THREADS=2 cargo test-all runs produced 0 FAILED lines each.

Closes #258

## The defect `NaturalLanguageScheduler::parse` ignored the requested weekday and time entirely. It always returned `reference_timestamp + 86400` — tomorrow, at the same clock time as the reference — and never errored on unrecognised phrasing. Duration was matched by substring against three hardcoded strings (`45 min`, `30 min`, `1 hour`) and silently fell back to 30 minutes for everything else. So `"Coffee with Bob next Tuesday at 2pm for 45 minutes"` returned an event that was neither Tuesday nor 2pm, with no error. It also fabricated `account_id: "default"` / `calendar_id: "default"` on the returned `CalendarEvent`. ## Resolution: narrow it I chose to **narrow the grammar** rather than delete or blanket-`Unimplemented` the parser. The DST-correct core is small, `chrono`/`chrono-tz` were already dependencies, and refusing the whole feature would have thrown away work that is genuinely achievable and now genuinely tested. ### Supported — and only these ``` <summary> <day> at <time> for <duration> ``` | Field | Accepted | |---|---| | `<summary>` | any non-empty text (separators matched right-to-left, so `"Prep for review at HQ tomorrow at 2pm for 45 minutes"` works) | | `<day>` | `today`, `tomorrow`, `YYYY-MM-DD` | | `<time>` | `noon`, `midnight`, `3pm`, `3:15pm`, `9am`, `9:05am`, 24-hour `HH:MM` | | `<duration>` | `<n> minute(s)`/`min(s)`, `<n> hour(s)`/`hr(s)`; non-zero, max 24 hours | ### Everything else returns an error - **Weekday phrasings** (`next Tuesday`, `this Friday`, `on Monday`, bare `Tuesday`) → `AmbiguousWeekday`. Refused deliberately, not merely unimplemented: English speakers do not agree whether "next Tuesday" is the coming Tuesday or the one a week later, and the readings differ by seven days. There is no reading the parser could pick that would not be wrong for a large share of users. - **Missing time or duration** → `MissingTime` / `MissingDuration`. Neither is defaulted — a defaulted duration is a guess about intent, and without a time there is no instant to resolve. - **Bare hours** (`at 3`) → `UnsupportedTime`; no way to tell 03:00 from 15:00. - Unsupported days, malformed minute fields, unparseable or out-of-range durations, and invalid calendar dates each return their own error variant. ## Time zones and DST `parse` now **requires an explicit IANA `Tz`**. A bare Unix timestamp cannot anchor "3pm", and assuming a zone is exactly the bug class this issue is about. Both local-time anomalies are reported rather than papered over: - Time inside a spring-forward gap → `NonexistentLocalTime` (e.g. 2026-03-08 02:30 America/New_York) - Time inside a fall-back overlap → `AmbiguousLocalTime` (e.g. 2026-11-01 01:30 America/New_York) Durations are added as **elapsed** seconds, so an event spanning a transition keeps its real length and its wall-clock end time shifts — which is what a physical meeting does. ## Return type `parse` now returns `ParsedSchedulingIntent` (previously a dead struct) instead of a `CalendarEvent`. A parser cannot know which calendar the user meant, so it no longer invents `"default"` account and calendar ids. The never-parsed `location`/`attendees` fields were dropped from the struct. ## Caller impact **None.** There is no in-tree consumer outside the crate's own `lib.rs` re-export — no proto surface, no CLI command, no daemon wiring. This is a POC leftover, as the issue suspected. The signature change and the narrower grammar are therefore not visible to any client today. ## Tests 22 tests, all with an injected reference instant — no wall-clock dependency. Every supported phrase asserts an **exact hardcoded Unix instant** (computed independently via Python `zoneinfo`, not through the implementation's own code path); every unsupported phrase asserts a specific error variant. Mutation-verified the timezone/DST behaviour — all four mutants killed: | Mutation | Killed by | |---|---| | Resolve in UTC, ignoring the caller's zone | 11 tests | | Silently pick the earliest instant for an ambiguous fall-back time | `a_time_inside_the_fall_back_overlap_is_refused` | | Silently shift a nonexistent spring-forward time forward an hour | `a_time_inside_the_spring_forward_gap_is_refused` | | Treat the duration as wall-clock rather than elapsed time | 3 tests | ## Gate `cargo fmt --all -- --check`, `cargo check-all`, and `cargo test-all` are all green. Three additional `RUST_TEST_THREADS=2 cargo test-all` runs produced 0 `FAILED` lines each. Closes #258
Sign in to join this conversation.