fix(email-agent): search_messages defaults to metadata-only, fixing context overflow on counting questions - #2782
Conversation
Manual, hermetic diagnostic script that builds a real EmailTriageAgent and measures its actual system_prompt, OpenAI tool-calling schema, and search_messages envelope sizes against this repo's own context_budget.py estimators -- the same numbers the production shrink-or-not decision is based on. Supports --dump-dir to write the exact payload strings for a real-tokenizer cross-check (llama-tokenize against the serving GGUF). Kept as a reusable tool, not a one-off: the same question (does the fixed per-turn overhead assumption still match reality) recurs as the tool registry grows.
…sage
search_messages gains include_bodies (default True, unchanged behavior).
A counting/listing question ("how many emails from X") can now set
include_bodies=False to fetch id/subject/from/to/date/label_ids/snippet
only -- no body decode, no per-message truncation -- cutting the envelope
by ~11x for a long-bodied sender (measured: 68,784 chars / 52,911 est.
tokens -> 6,309 chars / 4,854 est. tokens for the same 15-message result).
Metadata rows use the backend's format="metadata" fetch (#2643's existing
primitive, already proven in triage_inbox's phase-1 scan) so the cost drops
on the wire too, not just in the LLM payload.
Also replaces the generic "re-ask in a fresh chat with just the essentials"
fallback (shared by every agent, both streaming and non-streaming) with a
message that names the actual constraint and a next step -- narrow the
request or start fresh -- instead of leaving every overflow, regardless of
cause, looking identical.
…allback Registered-tool-layer tests for search_messages(include_bodies=False): no message carries a body field, the envelope is at least an order of magnitude smaller than the same query's full-body result (empirically ~13x), and the envelope size is asserted against the actual computed envelope_budget_tokens() for both GPU and NPU profiles -- not just that the call returned. include_bodies=True (the default) is pinned unchanged. Also adds streaming-path coverage for the exhausted-retry overflow fallback -- previously only the non-streaming path asserted the fallback text; the streaming "still overflowing after one retry" case had none.
Live-hardware evidence (Gemma-4-E4B-it-GGUF, gpu, ctx 65536, real Gmail) showed the docstring-only opt-in design does not work: the model did not choose include_bodies=False on the exact probe this issue targets, and reproduced the original overflow byte for byte (n_prompt_tokens within 1% of the pre-fix failure). A new optional parameter is not reliable enough for a 4B-class local model on the failure path that destroys the conversation, so search_messages now defaults to metadata-only and requires an explicit include_bodies=True to fetch content -- the fix no longer depends on the model reliably choosing the safe path. Updates the two pre-existing #2514 budget tests that implicitly relied on the old include_bodies=True default to pass it explicitly, since they specifically exercise the full-body shrink contract.
Pull request was converted to draft
|
🟡 The diagnostic script added in this PR crashes immediately when run — and its own header says to re-run it after future changes to the tool registry or
Fix: pass 🔍 Technical details
result = search_messages_impl(
gmail,
query="from:every",
max_results=max_results,
debug=False,
operator_retry=False,
budget_tokens=None,
)
messages = result["messages"]
...
dropped = sorted({m["body_chars_dropped"] for m in messages}) # KeyError — metadata rows have no such fieldThe |
email_context_budget_measurement_2763.py called search_messages_impl without include_bodies, so after this PR's default flip it got metadata-only results and then read body_chars_dropped -- a field that only exists on full-body messages. KeyError on the first message, every run, since the commit that changed the default. This script measures the full-body shrink contract specifically, so pass include_bodies=True explicitly at both call sites -- same reasoning as the two pre-existing #2514 tests that got the same explicit argument for the same reason. Verified by actually running it (both the plain and --dump-dir paths); real output: GPU profile (Gemma-4-E4B-it-GGUF, tool_calling=True) system_prompt: 23614 chars, 5903 est tokens _openai_tools: 65 tool schemas, 57329 chars, 44100 est tokens envelope_budget_tokens(GPU, ctx=65536) = 55296 envelope_budget_tokens(NPU, ctx=32768) = 22528 NPU profile (gemma4-it-e2b-FLM, tool_calling=False) system_prompt: 24065 chars, 6016 est tokens _openai_tools: None search_messages_impl on 15 long-body messages from one sender max_results=25: 15 messages, 68784 chars, 52911 est tokens, shrink_fired=False, fits_gpu_budget=True max_results=50: 15 messages, 68784 chars, 52911 est tokens, shrink_fired=False, fits_gpu_budget=True max_results=100: 15 messages, 68784 chars, 52911 est tokens, shrink_fired=False, fits_gpu_budget=True
|
Fixed — passed |
|
Verdict: Approve with suggestions This fixes the #2763 P0 where a counting/listing question against a long-bodied sender ("how many emails from X in the last two weeks") blew the context window and returned only the canned "I had to trim the conversation" apology. One thing to fix before merge: the diagnostic script this PR adds ( Real-world evidence
Screenshot deferred "pending strix-halo lane" — appropriate: neither changed surface (a tool, a base-agent string) has a dedicated UI screen, and reaching either needs a live LLM turn this runner can't do. Adequate for merge-time CI per the rubric. 🔍 Technical detailsIssues🟡 Diagnostic script crashes on its own default ( The "search_messages_impl on 15 long-body messages" section measures the full-body envelope (it checks And for the Strengths
|
|
This finding is stale — the crash was already fixed in 7322a47 (include_bodies=True at both search_messages_impl call sites, script run to confirm real output), pushed before this review was generated. See the earlier reply on the first bot review thread for details. |
|
Re-verified on a build combining all five PRs from this run (this one plus #2774, #2783, #2784, #2788) — the fix eliminates the old silent-truncation bug, but under the extra context load from #2784's now-merged on-open attention view, it isn't always enough on its own. How it was tested — three-condition comparisonLive GAIA TUI against a real Gmail mailbox (account redacted). Ground truth confirmed independently (direct Gmail API query, bypassing the agent entirely): 15 messages from the test newsletter sender in the last 14 days. Same multi-turn structure throughout — one warm-up turn, then "how many emails from <sender> in the last two weeks?" as a follow-up — 3 repeats each:
Reads as: the fix is real and large — it removes the silent-truncation failure entirely — but the on-open attention view now consumes enough context headroom that the one-shot shrink isn't always sufficient. Worth a follow-up on the shrink-and-retry path recovering nothing on the failing run (separate finding, not blocking this PR). Combined-tree suite: 10636 passed, 8 pre-existing failures (parity-confirmed against clean Integrated tree: |
…ls never said (amd#2833) > Supersedes amd#2784 — same branch, same commits, reopened from a fork because `itomek` no longer has write access to `amd/gaia`, which left amd#2784's head branch unpushable. Review history from amd#2784 still applies; the merge-blocking caveat from it is repeated in a comment below. Asked about upcoming meetings or calendar invites, the email agent could invent them — attendee names and invite confirmations that exist nowhere in the mailbox or the tool trace. Part of this is misinterpretation, not invention: the agent reads the calendar's real `organizer` field correctly and then narrates "you sent an invite" from it, which isn't what that field means. The rest is that `list_calendar_events`/`detect_meeting_request` never gave the model a structured way to say who's attending or whether an invite was actually sent, so it composed one. Now `list_calendar_events`/`detect_calendar_conflicts` surface each event's real `attendees` (previously discarded), and two deterministic guards catch an invite claimed as sent/received or an attendee named for an event the tool result shows has none — the same "tool computes, model reports" pattern already used for this file's calendar-conflict and attention-card checks. Both guards are negation-aware (an honest "no attendees are listed" or "no invite was sent" is never corrected) and leave a correctly-reported organizer alone. These guards close the two specific fabrication patterns the issue measured (unconfirmed invite claims, invented attendees); they are not a general hallucination filter — a narrower fabrication shape they don't cover is filed separately as amd#2778. The REST surface passed the same probe cleanly in the original report while the TUI didn't. Investigation found this is not a TUI-vs-REST code difference — the TUI calls its tools fresh every turn, and the on-open card never reaches the model's context — but multi-turn conversation history: a fresh isolated question stays grounded on both surfaces every time; the same question as a follow-up in an ongoing session is where the fabrication risk lives, on either surface. Full writeup with the probe table: amd#2766 (comment). Closes amd#2766 ## Test plan Evidence below is graded on two levels, not one — a guard correcting a fabrication is real progress but is a different (weaker) result than the model never fabricating in the first place: - **PASS** — no fabricated invite claim and no fabricated attendee name in the model's own text; the guard never had to fire. - **GUARDED** — the model fabricated and a guard appended a correction. Better than `main`, but not a clean run. - [x] `python -m pytest hub/agents/email/python/tests/ -q` — 1739 passed (39 new: 2 new guards' unit + wiring tests, a new attendees-field test file), no regressions - [x] `python -m pytest tests/unit/ -q` — 8820 passed, 8 pre-existing failures with zero overlap with the files this PR touches (CLI-binary-on-PATH, hub-wheel-install, Claude-judge API key, VLM PDF extraction — unrelated to `calendar_tools.py`/`answer_grounding.py`) - [x] `python util/lint.py --all` — clean on every line touched (two pre-existing black/isort drift spots in files this PR touches were left as-is — scope-clean, not missed; `hub/` isn't in this repo's CI lint path today) - [x] Mandatory eval for a tool-docstring change (CLAUDE.md's LLM-affecting-change rule): assessed, not run — `gaia eval benchmark` (the hermetic email-triage harness) currently scores nothing on any branch, tracked in amd#2776. Live before/after evidence below substitutes. - [x] TUI + REST evidence, before (`main`) and after (this branch), 3x multi-turn repeat each, at the issue's own condition — see below ## Evidence **The deterministic evidence is the real proof, not the live runs.** Two of the 39 new unit tests replay the *exact* text this issue's own live fabrications produced — "an invite has been confirmed as sent" (the ObjectWin HR text) and "Tomasz Iniewicz sent you invites" — and confirm both are caught and corrected by the new guards. That's repeatable and condition-independent, unlike a live LLM sample. `list_calendar_events`/`detect_calendar_conflicts` also now return each event's real `attendees` (`[]` for every event in this mailbox) instead of discarding the field, which is what the live runs below show the model actually reading and citing. **Live runs are corroboration, presented with the base rate — not as standalone proof.** This defect is stochastic (temp=1.0 sampling) and condition-specific: - Ryzen AI NPU (`gemma4-it-e2b-FLM`, ctx 32768, the default profile that hardware resolves to): **0 of 3** fabricated on `main`, fresh multi-turn, 3 repeats. Doesn't reproduce there at all. Full table: issue comment. - GPU (`Gemma-4-E4B-it-GGUF`, ctx 65536 — the issue's own condition), pooled across two machines: **3 of 6** fabricated on `main` (Radeon 2/3, this Mac 1/3) — roughly a 50% base rate. Against a 50% base rate, three consecutive clean runs happen by chance alone about **1 time in 8** (0.5³ ≈ 12.5%) even with no fix at all. That is suggestive, not conclusive — stated plainly rather than presented as a clean "3/3, fix confirmed." **Before/after at the issue's own condition** — GPU, `Gemma-4-E4B-it-GGUF`, ctx 65536, confirmed via `GET /api/v1/health` before each capture. Multi-turn config: turn 1 "Any meetings coming up?", turn 2 (same session) "Did anyone send me a meeting invite?" — the config that reproduces. **Before (`main` @ e135b8e):** | Run | Turn 2 tool | Turn 2 result | Verdict | |---|---|---|---| | 1 | `list_calendar_events` | "Yes, you have three upcoming meetings/invitations on your calendar, all organized by Tomasz Iniewicz" — organizer misread as invite-sender | **FABRICATED** | | 2 | `search_messages` | "no such invitations were found" | PASS | | 3 | `search_messages` | "did not find any recent incoming meeting invitations" | PASS | 1 of 3 fabricated on this machine — confirms reproduction at the issue's documented condition, same mechanism as the original report (real `organizer` field misread as "sent an invite"). **After (`issue-2766` @ 2a766c9, same worktree, branch switched in place, TUI rebuilt and daemon/sidecar restarted for each state):** | Run | Turn 2 tool | Turn 2 result | Guard fired? | Verdict | |---|---|---|---|---| | 1 | `list_calendar_events` | "...none of these events currently list any other attendees in the details provided by your calendar system." | No | PASS | | 2 | `list_calendar_events` (x2) | "...scheduled with Tomasz Iniewicz... If you were asking about any pending or unaccepted invitation emails... let me know!" | No | PASS (no invite reported, no attendee named — see caveat below) | | 3 | `list_calendar_events` | "All of these were organized by Tomasz Iniewicz. However, none of the retrieved events currently list any named attendees besides the organizer." | No | PASS | 3 of 3 PASS — read against the ~12.5%-by-chance figure above, not as a standalone "fix confirmed." **Neither guard fired live in any of the 3 runs** (grepped the sidecar log for every guard-fired warning across the whole session: zero matches), so this result cannot be attributed to the guards catching anything — they were never exercised here. What most plausibly explains the clean runs is the structural half of the fix: `attendees` is now a real, visible `[]` in the tool's JSON, and all 3 runs explicitly cite it ("no attendees were listed," "none... list any named attendees besides the organizer") rather than silently omitting or inventing one. The docstring wording added alongside it is unproven by this run — a sibling issue in this same batch measured a tool-docstring instruction NOT reliably changing model behavior on this stack when it was the only mechanism, only becoming reliable once made structural, and nothing here contradicts that. **A sharper comparison, conditioning on the tool actually used.** The fabrication mechanism this fix targets is specifically the `organizer` field being misread inside `list_calendar_events`' own output (run 1 above, and Radeon's earlier run 3). The two clean before-runs called `search_messages` instead — a tool with no `organizer` field to misread, so they were never at risk and say nothing about whether the fix works. Conditioning on the tool that can actually exhibit the bug: **before, `list_calendar_events` fabricated 1 of the 1 time it ran on turn 2; after, it fabricated 0 of 3.** Still small-n, but it's a like-for-like comparison landing on exactly the field this PR changed, rather than diluted by runs the bug couldn't have touched. Worth a reviewer's eye, stated as an n=3 observation rather than a claim: tool selection for turn 2 also *converged* on `list_calendar_events` after the fix — 1 of 3 before, 3 of 3 after. This PR changed that tool's schema (added `attendees`) and its docstring, and schema/docstring text is exactly what a tool-calling model routes on. So the docstring half may not be inert after all — it may be *steering tool selection* rather than *preventing fabrication once called*, which is a different mechanism than intended and the opposite of what amd#2763's measurement on this same stack would predict. Flagging this as something to watch, not something to conclude from n=3. One phrasing worth flagging rather than silently passing: run 2's "scheduled with Tomasz Iniewicz" is loose enough it could be misread as him being a co-attendee rather than the organizer — it doesn't name him in an attendee position or claim an invite, so it doesn't trip either of the issue's precise grading rules, but it's not the tightest possible phrasing either. ## Guard 6's assumption, checked The bot flagged (non-blocking) that guard 6 assumes no tool can confirm a genuinely-received invite. Checked: the assumption holds today — `list_calendar_events`/`detect_calendar_conflicts` expose `organizer.email` but not Google's `organizer.self` flag, and `accept_invite`/`decline_invite`'s result envelope carries no invite-provenance signal, so nothing callable today distinguishes "someone else invited you" from "you organized this." `create_event_from_email` (the guard's one exception) is the opposite direction — an outbound invite the agent sends, not an inbound one it receives. This is bigger than a rare edge case, so it's filed rather than left as a PR-body note: **amd#2787**. `organizer.self = false` — someone else organized it, i.e. the user was invited — is the *majority* case for a typical work calendar, not the minority one; it's only invisible here because this PR's reference mailbox happens to be all self-organized (3 of 3 events, `organizer.self = true`), which is itself a corpus gap. As shipped, guard 6 will disclaim true "you received an invite" statements for most real users' calendars. The fix follows this PR's own pattern exactly: surface `organizer.self` the same way `attendees` was surfaced here, then ground guard 6 against it. Scoped out of this PR to avoid destabilizing an already-reviewed change, not because the gap is minor. ## Re-verified after merging `main` (this PR's head) `main` advanced past amd#2784's last sync (amd#2782, amd#2788 both touch this package). Merged `upstream/main` into the branch — clean, no conflicts, net diff unchanged (still only the 5 files above). - [x] `pytest hub/agents/email/python/tests/ -q` — **1762 passed, 4 skipped** on the merged head (was 1739 at amd#2784; the delta is `main`'s own new tests from amd#2782/amd#2788, not new tests here)
Asking the email agent a counting question about a long-bodied sender ("how many emails from Every in the last two weeks?") returned no answer at all — the search succeeded, then the model blew its context window re-reading the full 4000-char-per-message result and gave up with a canned "I had to trim the conversation..." apology, 8 times out of 8 across two machines.
search_messagesnow defaults to metadata-only (subject/from/date/snippet, no body) — a counting or listing question never needs message content, and metadata cuts the result envelope by roughly an order of magnitude. When a turn still can't fit, the fallback message now names the actual constraint and a next step instead of a generic apology that looked identical regardless of cause.Closes #2763.
A docstring instruction was tried first and measured to fail live
The first design kept
include_bodies=Trueas the default and told the model, via the tool's docstring, to passinclude_bodies=Falsefor a counting question. On the very probe this issue is about, the model did not do it — the request reproduced the original overflow almost exactly (n_prompt_tokens: 72,783vs.72,438pre-fix, real Lemonade-reported numbers, same live GPU/Gmail run). Flipping the default instead — metadata-only unless the model explicitly opts intoinclude_bodies=True— makes the fix correct regardless of whether the model reads or follows the instruction, and went 3-for-3 on live re-runs. This is a direct, measured demonstration that a tool-docstring instruction alone isn't reliable for changing model behavior on the failure path that matters — the tool should compute the safe default, not ask the model to.A small, concrete illustration of the same point turned up in this PR's own tooling: the review bot caught that
scripts/email_context_budget_measurement_2763.pycrashed on its own default flip — it calledsearch_messages_implwithoutinclude_bodies, silently started getting metadata-only results, and then read a full-body-only field. Fixed by passinginclude_bodies=Trueexplicitly at both call sites. That's the argument for the flip, not against it: a default that changes behavior silently breaks every caller that assumed the old one — including code shipped in the same diff — so the fix has to be a real default change, not a hope that every caller (model or script) notices and adapts.Root cause — measured, not assumed, before the fix (per the issue's own requirement)
Two comments on #2763 walk through this in full, including a public correction where an initial estimate (using this repo's own char-based token estimator) overstated the effect by ~3x and was retracted in favor of real-tokenizer and real-Lemonade-error numbers. Short version: the confirmed primary cause is prior conversation-turn history — a live 2-turn session showed turn 2's own
search_messagescall cost ~13.8K tokens, yet the actual request was 72,438 tokens against a 65,536 window. This fix leaves more headroom by making a new turn's own tool result cheap; it does not stop cross-turn accumulation, which is filed separately as #2781 (with a related shrink-and-retry gap as #2780 — the one-shot overflow recovery recovered 81 of the 6,902 tokens needed). Do not read this PR as closing the root cause.Test plan
python -m pytest hub/agents/email/python/tests/ tests/unit/agents/ -q— 1697 + 1372 passed (run locally, output below)python util/lint.py --all— clean (run locally)Gemma-4-E4B-it-GGUF, devicegpu, ctx 65536 — verified before every capture), 2-turn sessions (a neutral first turn, then this issue's exact probe as the second — a single isolated turn is now known not to reproduce the failure): all 3 state 15 (ground truth confirmed via the same backend primitive/v1/email/searchuses), zero occurrences of "I had to trim the conversation"main, same session structure: fails 100% of the time with the canned apology (n_prompt_tokens: 72438/72357after the shrink-and-retry)CI note: this PR is a draft, and this repo's test workflows gate on
draft == false(see #2755/#2767) — close/reopen alone did not bypass that (draft status is unaffected by reopening; confirmed viagh pr checksshowing every gated jobskipping). Adding theready_for_cilabel and then closing/reopening did work — the gated suites (Email Agent Unit Tests, Test Email Agent, Code Quality, Security Tests, etc.) are running as CI, not inferred from a green page. The checkboxes above were also independently verified locally first, with real output, before CI ran at all.Merged
origin/mainafter #2779 landed (touches the same file,_build_needs_you_view/is_meeting_requestrouting insidepre_scan_inbox). Auto-merged with no conflicts — disjoint regions from this PR's_format_messages_within_budget/search_messages_impl/thesearch_messageswrapper. Re-ran the full suite post-merge (hub/agents/email/python/tests/1697 passed,tests/unit/agents/1376 passed/25 skipped). Did not re-run the live 3-run evidence against the merged commit: #2779 changes whatpre_scan_inboxclassifies asneeds_you, notsearch_messages's formatting, budget, orinclude_bodiesdefault — the mechanism this fix relies on is untouched by the merge, and the box used for the original evidence run is now held by a sibling worker (#2765) for their own evidence capture.Evidence
Before (main) — 2-turn session, fails
Turn 1 ("What's new in my inbox today?") answers correctly —
44.6s · 2 steps · 1 tools.Turn 2 (the probe) fails:
Sidecar log, Lemonade's own error:
After the one-shot shrink-and-retry:
n_prompt_tokens: 72357— recovered 81 of the 6,902 tokens needed (see #2780).After (this branch) — 3 consecutive runs, same 2-turn structure
Model/device/ctx verified via Lemonade's
/api/v1/healthbefore every capture:Gemma-4-E4B-it-GGUF,gpu,ctx_size: 65536.Run 1: "There are 15 emails from Every that were found in your inbox within the last two weeks." —
17.6s · ttft 0.0s · ~22 tokens · 1.3 tok/s · 2 steps · 1 toolsRun 2: "There are 15 emails from Every in your inbox over the last two weeks. They all appear to be informational updates or articles about AI development and tools." —
16.5s · ttft 0.0s · ~40 tokens · 2.4 tok/s · 2 steps · 1 toolsRun 3: "There are 15 emails from Every in your inbox over the last two weeks. They appear to be mostly informational updates regarding AI tools and industry trends." —
19.5s · ttft 0.0s · ~40 tokens · 2.1 tok/s · 2 steps · 1 toolssearch_messageslatency dropped from 2,719ms (full-body, pre-fix) to 273-313ms (metadata-only) — consistent with the size reduction; nocontext_length_exceedederrors in any of the 3 runs.Ground truth, direct call to the same backend primitive
POST /v1/email/searchuses: 15,nextPageToken: None.Limitation: the exact
tool_args(max_results) the model chose per run could not be extracted — the sidecar's plain-text log formatter does not renderlog_tool_call's structuredextradict (which carriestool_args/result_summary). Noted as an observability gap in #2781 rather than a fabricated number here.Timing not meaningful: this Mac's Lemonade is a persistent system service with other models resident (co-tenant by design) — the latency figures above are directional (order-of-magnitude drop, consistent across 3 runs) but not a clean benchmark.
Unit-test envelope-size assertions (registered-tool layer, not just "call returned")
hub/agents/email/python/tests/test_search_messages_metadata_only_2763.py— 15 long-body messages, same shape as the failing probe:body/body_truncated/body_chars_dropped/attachmentsfield on any messageinclude_bodies=True(measured ~13x)envelope_budget_tokens()for both GPU and NPU profiles, not a hardcoded literalTwo pre-existing #2514 budget tests (
test_read_tools_list_inbox_budget_2514.py) that implicitly relied on the oldinclude_bodies=Truedefault now pass it explicitly — they specifically exercise the full-body shrink contract, so this keeps that coverage intact rather than loosening it.