Skip to content

feat(email): create calendar events from email context#1355

Merged
itomek merged 2 commits into
itomek/email-triage-agentfrom
tmi/issue-1274-event-from-email
Jun 3, 2026
Merged

feat(email): create calendar events from email context#1355
itomek merged 2 commits into
itomek/email-triage-agentfrom
tmi/issue-1274-event-from-email

Conversation

@itomek
Copy link
Copy Markdown
Collaborator

@itomek itomek commented Jun 2, 2026

Closes #1274

Before: create_event_from_email would POST {"dateTime": ""} straight to the calendar when an email had no parseable time — silently creating a bogus event. After: extract_event_details pulls title/attendees/time-signal from the email, and creation fails loudly (NoEventDateTimeError, creates nothing) when there's no usable date/time, so the agent asks for a time instead of fabricating one.

Test plan

  • 17 unit tests (test_create_event_from_email.py): title/attendees/time extracted from a clear fixture email → event created with matching fields; no-datetime negative case (blank/missing/whitespace/partial) raises and creates nothing, at both impl and tool-boundary layers
  • full email suite → 96 passed; lint clean

Stacked on #1273 (calendar lane 1272→1273→1274); base tmi/issue-1273-calendar-conflict, retargeted to integration as the chain merges. Deterministic extraction (no eval-trigger).

@github-actions github-actions Bot added tests Test changes agents labels Jun 2, 2026
@itomek itomek self-assigned this Jun 2, 2026
@itomek itomek changed the base branch from tmi/issue-1273-calendar-conflict to itomek/email-triage-agent June 2, 2026 15:21
itomek added 2 commits June 3, 2026 13:39
create_event_from_email already existed but could silently create a bogus
event: when the email carried no parseable date/time, an empty start/end was
POSTed straight to the calendar. Now creation refuses a missing/blank (or
inverted) start/end and surfaces an actionable "no time found" error instead.

Adds extract_event_details — a deterministic extractor that pulls the event
title (subject), attendees (sender, via the existing extract_sender_email),
and a has_datetime time-signal (reusing this module's _TIME_RE) — and tests
proving title/time/attendees are extracted from a fixture email plus the
no-datetime negative case at both the impl and registered-tool layers.
@itomek itomek force-pushed the tmi/issue-1274-event-from-email branch from 5d561fe to da8c9ff Compare June 3, 2026 17:40
@itomek itomek marked this pull request as ready for review June 3, 2026 22:20
@itomek itomek requested a review from kovtcharov-amd as a code owner June 3, 2026 22:20
@itomek itomek merged commit d9564b4 into itomek/email-triage-agent Jun 3, 2026
11 checks passed
@itomek itomek deleted the tmi/issue-1274-event-from-email branch June 3, 2026 22:20
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Review: feat(email): create calendar events from email context (#1355)

Summary

Solid, well-tested fail-loud hardening of calendar event creation — this is exactly the behaviour CLAUDE.md's "No Silent Fallbacks" rule asks for, and the negative-case coverage is thorough. The one thing worth clarifying before this is "done" as described: the new extract_event_details function is fully tested but never called from any production path — the agent-facing create_event_from_email tool takes start_iso/end_iso straight from the LLM and never invokes the extractor. So the value this PR actually ships is the fail-loud guard (NoEventDateTimeError + _has_usable_time) and the window-ordering check, not the extractor the description leads with. Non-blocking, but a reviewer should know the runtime narrative differs from the description.

(Note: I reviewed statically — I couldn't run the suite locally, so I'm trusting the PR's "96 passed; lint clean" CI claim. No prompt-injection content in the diff.)

Issues Found

🟡 extract_event_details is unused in production (calendar_tools.py:675)
The function is defined and gets 7 dedicated tests, but grep finds zero callers outside the test file — the registered create_event_from_email tool never calls it. The PR description frames it as the core of the fix ("extract_event_details pulls title/attendees/time-signal from the email"), yet at runtime the LLM does that extraction and the function is dead code. Two clean resolutions: (a) if it's scaffolding for a later stacked PR, say so in a one-line comment so it doesn't read as wired-in; (b) if it's meant to be live now, call it from the tool to pre-fill/validate before the LLM-supplied args reach create_event_from_email_impl. As-is it risks quiet bit-rot — a tested function nobody invokes.

🟢 has_datetime can false-positive on casual mentions (calendar_tools.py:703)
_TIME_RE matches bare weekday/relative words, so "see you tomorrow" or "talk Monday" flags has_datetime=True with no real slot. Harmless today since has_datetime gates nothing (the impl guard keys off actual start/end), but if you ever wire the extractor in, this becomes a source of bogus "time found" signals. Worth a comment noting has_datetime is a coarse hint, not a commitment.

🟢 Duplicated datetime pull (calendar_tools.py:740, 751)
start.get("dateTime") or start.get("date") is computed in _has_usable_time and again for the ordering check. Trivial; only worth folding if you touch this block again.

Strengths

  • Textbook fail-loud. NoEventDateTimeError(ValueError) is a clean choice — the tool's except Exception boundary turns it into a structured ok=False envelope, while callers that care can match the specific type. Exactly the boundary-translation pattern CLAUDE.md allows.
  • Negative cases are the star. Blank / missing-key / whitespace / partial / inverted / zero-length / all-day are all covered at both the impl and tool-boundary layers — the part most PRs skip.
  • Good reuse, no parallel parser. Leans on the existing _TIME_RE and read_tools.extract_sender_email rather than reinventing extraction, and the tests mirror the established test_calendar_conflicts.py conventions (sys.path bootstrap, _make_email_agent, _TOOL_REGISTRY access).

Verdict

Approve with suggestions. No blocking issues — the fail-loud guard is correct and well-tested. Before closing the loop on #1274, clarify the extract_event_details wiring question (🟡): either land its caller or mark it as future scaffolding so it doesn't drift into dead code.

kovtcharov-amd pushed a commit that referenced this pull request Jun 3, 2026
Closes #1274

Before: `create_event_from_email` would POST `{"dateTime": ""}` straight
to the calendar when an email had no parseable time — silently creating
a bogus event. After: `extract_event_details` pulls
title/attendees/time-signal from the email, and creation fails loudly
(`NoEventDateTimeError`, creates nothing) when there's no usable
date/time, so the agent asks for a time instead of fabricating one.

## Test plan
- [x] 17 unit tests (`test_create_event_from_email.py`):
title/attendees/time extracted from a clear fixture email → event
created with matching fields; **no-datetime negative case**
(blank/missing/whitespace/partial) raises and creates nothing, at both
impl and tool-boundary layers
- [x] full email suite → 96 passed; lint clean

**Stacked on #1273** (calendar lane 1272→1273→1274); base
`tmi/issue-1273-calendar-conflict`, retargeted to integration as the
chain merges. Deterministic extraction (no eval-trigger).
kovtcharov-amd pushed a commit that referenced this pull request Jun 3, 2026
The Code Quality (Lint) job runs pylint over all of src/gaia and was
already failing on main: #1355 left a mutable-default (W0102) in
quality_metrics.py and several Protocol-parity unused-argument signatures
(W0613) in the Outlook/email-MCP backends. Fix the mutable default with a
None sentinel and annotate the interface-parity stubs, so the gate is green
for this PR and every other open PR.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant