M6.5 [P2]: NaturalLanguageScheduler silently returns wrong times - parse genuinely or delete #258

New Issue

2026-07-31T14:42:54Z

KofTwentyTwo commented

2026-07-31 14:42:54 +00:00

(Migrated from github.com)

nuncio-cal/src/nlp.rs:33-75 NaturalLanguageScheduler::parse ignores the requested weekday/time entirely (always returns tomorrow, same time as reference) and never errors on unrecognized phrasing - silently wrong output. Determine whether this is wired anywhere real; if not (likely POC leftover), DELETE it. If it is used or intended, make it genuinely parse the weekday/time or hard-fail on unrecognized input - never silently substitute a wrong time.

Source: full-tree audit 2026-07-30 (scratchpad/audit/AUDIT-SUMMARY.md). Follow docs/STORY-WORKFLOW.md: engine+proto(if surface)+CLI+offline E2E, mock-only tests, no fabricated data, comment convention, PR to dev, PO reviews before close.

nuncio-cal/src/nlp.rs:33-75 NaturalLanguageScheduler::parse ignores the requested weekday/time entirely (always returns tomorrow, same time as reference) and never errors on unrecognized phrasing - silently wrong output. Determine whether this is wired anywhere real; if not (likely POC leftover), DELETE it. If it is used or intended, make it genuinely parse the weekday/time or hard-fail on unrecognized input - never silently substitute a wrong time. Source: full-tree audit 2026-07-30 (scratchpad/audit/AUDIT-SUMMARY.md). Follow docs/STORY-WORKFLOW.md: engine+proto(if surface)+CLI+offline E2E, mock-only tests, no fabricated data, comment convention, PR to dev, PO reviews before close.

KofTwentyTwo commented

2026-08-15 02:04:16 +00:00

(Migrated from github.com)

Resolved in #428 (f5d8c8a), merged to dev. Narrowed rather than refused.

The old parse was worse than the issue title suggests: it ignored the weekday and time outright (always reference + 86400), substring-matched three duration strings and defaulted everything else to 30 minutes, and fabricated "default" account and calendar ids.

New grammar: <summary> today|tomorrow|YYYY-MM-DD at <time> for <duration>. Everything else errors. Time and duration are mandatory and never defaulted. Weekday phrasings (next Tuesday) are refused as genuinely ambiguous — the two ordinary English readings differ by a week, so no guess is defensible.

Timezone is now required as an explicit IANA Tz: a bare epoch cannot anchor "3pm". Spring-forward gaps yield NonexistentLocalTime, fall-back overlaps AmbiguousLocalTime, and durations are added as elapsed seconds so a DST-spanning event keeps its real length. Return type is ParsedSchedulingIntent, dropping the invented calendar identity and the never-parsed location/attendees.

22 tests with injected reference instants; 4/4 mutations killed. No caller is affected — the only reference outside nlp.rs is the lib.rs re-export, confirming this issue's "POC leftover" suspicion. If it ever gets a real surface, the zone must come from user config and at <time> will need an all-day variant the current grammar deliberately rejects.

Resolved in #428 (`f5d8c8a`), merged to `dev`. **Narrowed rather than refused.** The old `parse` was worse than the issue title suggests: it ignored the weekday and time outright (always `reference + 86400`), substring-matched three duration strings and defaulted everything else to 30 minutes, and fabricated `"default"` account and calendar ids. New grammar: `<summary> today|tomorrow|YYYY-MM-DD at <time> for <duration>`. Everything else errors. Time and duration are mandatory and never defaulted. Weekday phrasings (`next Tuesday`) are **refused as genuinely ambiguous** — the two ordinary English readings differ by a week, so no guess is defensible. Timezone is now required as an explicit IANA `Tz`: a bare epoch cannot anchor "3pm". Spring-forward gaps yield `NonexistentLocalTime`, fall-back overlaps `AmbiguousLocalTime`, and durations are added as elapsed seconds so a DST-spanning event keeps its real length. Return type is `ParsedSchedulingIntent`, dropping the invented calendar identity and the never-parsed `location`/`attendees`. 22 tests with injected reference instants; 4/4 mutations killed. No caller is affected — the only reference outside `nlp.rs` is the `lib.rs` re-export, confirming this issue's "POC leftover" suspicion. If it ever gets a real surface, the zone must come from user config and `at <time>` will need an all-day variant the current grammar deliberately rejects.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#258