Skip to content

feat(email): render the triage list from the scan, not from the model - #2858

Merged
kovtcharov-amd merged 3 commits into
mainfrom
tmi/email-triage-view
Aug 6, 2026
Merged

feat(email): render the triage list from the scan, not from the model#2858
kovtcharov-amd merged 3 commits into
mainfrom
tmi/email-triage-view

Conversation

@itomek

@itomek itomek commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #2789

Ask the email agent to triage your inbox and what came back was a coin flip. Across one evening of live runs against a real mailbox it produced: numbering that pointed archive 3 at a different message than the one labelled 3, items dropped, one message listed twice under a single number, addresses printed twice with a mailto: twin, ten items run together on one line, and — twice — no list at all, just "14 items need attention." Every one of those came from the same place: the chat model was being asked to retype a list the tools had already computed.

It doesn't anymore. The categories are still model judgement — a heuristic, then the specific-ai-triage SLM, then an LLM fallback, all inside pre_scan_inbox. Transcribing that result isn't a judgement, so the breakdown is now rendered from needs_you, carrying the classifier's own reason on each row. The model writes the opening sentence and nothing else.

This also unblocks every other deterministic correction in the agent. ground_final_answer ran on process_query's return value, which the REST/TUI stream never re-reads — so the calendar-conflict, attention-card, invite-claim and fabricated-attendee guards all fired, logged, and reached nobody on the surface users actually drive (#2789). A finalize_answer hook on the base agent moves grounding ahead of the emission point, so one correction now serves the stream, the console, and the return value.

The inbox pre-scan card is gone too: it spent a Gmail scan before the user asked for anything, then reappeared mid-turn as a partial list beside the full answer still being written — two overlapping views of one inbox with no way to tell which to act on.

Evidence

Before — same prompt, three consecutive runs:

Here's your inbox pre-scan — 14 items require attention, with 39 filtered out.
• Phishing Alert: Row 1 — Your account has been suspended…      ← card row 1 is "Partnership intro"
• Critical Topics: Rows 3 & 4 — server room fire alarm…         ← card rows 3 & 4 are something else
4.  Tomasz Testingiewicz tomasz.t@outlook.com mailto:tomasz.t@outlook.com - 30 minutes next week? 5.  Tomasz …

After:

Here's your inbox pre-scan — 14 items need attention, 39 filtered.

### Waiting on your reply

1. Tomasz Testingiewicz — Re: Partnership intro (2d ago · waiting 2d on your reply)
2. Tomasz Testingiewicz — Re: Northgate migration (2d ago · waiting 2d on your reply)

### Needs a response

3. Tomasz Testingiewicz — Your account has been suspended (2d ago · flagged as phishing)

### Meetings to decide

4. Tomasz Testingiewicz — 30 minutes next week? (2d ago)

Numbers are the ref values resolve_needs_you_reference resolves against, so archive 3 now names exactly one message.

Test plan

  • pytest hub/agents/email/python/tests/ -q — 1829 passed, 4 skipped
  • PYTHONPATH=src pytest tests/unit/agents -q — 1381 passed, 25 skipped
  • (cd tui && go test ./...) — all packages green
  • flake8 src/gaia tests — 0
  • Live TUI against a real Gmail mailbox, triage my inbox
Why the list is rendered rather than prompted

Both were tried. The skill was rewritten four times — naming the section order, naming the failing shape outright ("a reply that says '14 items need attention' and stops has failed"), forbidding raw addresses — and each time a live run ignored it. The two fixes that held were the ones moved into code.

Prompt text is also not free: the fuller wording pushed the post-tool envelope to 3511 tokens against a 3482 budget, and test_the_whole_post_tool_turn_fits_the_window_with_skills_loaded failed. Moving the mechanics into the renderer let the skill shrink from 22 lines describing a list to 4 lines saying "write one sentence, don't write the list" — and dropped ~450 tokens per turn that were being spent retyping ten rows.

Deliberately out of scope

itomek added 2 commits August 6, 2026 09:35
Asking the chat model to retype a list the tools had already computed
produced, across a night of live runs: invented numbering that pointed
'archive 3' at the wrong message, dropped items, sections merged so one
message appeared twice under one number, and once no list at all — just
'14 items need attention'.

The categories stay model judgement: a heuristic, then the specific-ai-triage
SLM, then an LLM fallback, all inside pre_scan_inbox. Transcribing that result
is not a judgement, so it is rendered from needs_you, carrying the
classifier's own reason per row.

Grounding also moves ahead of the emission point (#2789). It ran on
process_query's return value, which the REST/TUI stream never re-reads, so
every correction fired, logged, and reached nobody on the surface users
actually drive. A finalize_answer hook on the base agent lets one correction
serve the stream, the console, and the return value.
The card spent a Gmail scan before the user had asked for anything, then
landed again mid-turn as a partial list beside the full triage answer still
being written — two overlapping views of one inbox, with no way to tell which
to act on. The triage reply is now the single view.
@itomek
itomek requested a review from kovtcharov-amd as a code owner August 6, 2026 13:38
@github-actions github-actions Bot added agents agent::email Email agent changes tui Go terminal UI (gaia-tui) labels Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Verdict: Request changes (small, test-shaped — the code is sound).

This PR makes the email triage list deterministic: instead of asking the chat model to retype the worklist (which dropped, renumbered, and merged items across live runs), the numbered list is now rendered directly from the pre_scan_inbox scan. It also adds a base-agent finalize_answer hook so grounding runs before the answer is emitted (reaching the streamed SSE/TUI view, not just process_query's return value), raises the worklist cap 5→10, groups the TUI card by verb, and drops the on-open pre-scan card so the triage reply is the inbox's single view.

The direction is good and the reasoning in the comments is unusually clear. Two things to address before merge:

  • New answer-rewriting logic ships with no unit tests. The ~140 lines that now decide what the user actually reads (list rebuild, sender-label cleanup, lead-paragraph extraction, the finalize_answer grounding path) have no coverage, even though a grounding test file already exists. This is exactly the logic most worth pinning — a silent regression here changes every triage answer.
  • The list rebuild fires on tool-presence, not intent. Any turn that internally calls pre_scan_inbox gets its answer body replaced by the full worklist, even if the user asked something narrower. Worth confirming that's intended for non-"triage my inbox" phrasings.

Real-world evidence

⚠️ The evidence harness failed to run (evidence-bundle.md reports an auth/install/rate-limit error before it exercised anything) — this is not "nothing to test". The changed surface is user-visible on two fronts the rubric cares about: the Email agent in the Agent UI (wants a Playwright before→after screenshot) and the TUI triage card/reply. Neither was exercised here, so the verdict rests on static review alone. Please attach a real gaia chat --ui run (or the TUI equivalent) showing a triage reply rendering one correctly-numbered list, since the whole point is output the unit tests don't gate.

🔍 Technical details

🟡 Missing tests for new user-facing logicanswer_grounding.py:326, :409 (render_needs_you_list), :465 (rewrite_triage_answer), and agent.py:414 (EmailTriageAgent.finalize_answer). hub/agents/email/python/tests/test_answer_grounding.py exists but wasn't touched, and none of the new symbols are referenced by any test. Please add cases for: a scan with items in several kinds → one list, correctly numbered, sections in _TRIAGE_SECTIONS order; an empty needs_yourewrite_triage_answer returns the model answer unchanged; and the finalize_answerprocess_query dedup (grounding not applied twice).

🟡 rewrite_triage_answer keys on tool-presence, not user intent (answer_grounding.py:465). It rewrites the answer whenever last_tool_payload(conversation, "pre_scan_inbox") is non-empty for the turn, appending the full rendered worklist to _lead_paragraph(final_answer). last_tool_payload is turn-scoped (good — no cross-turn bleed), but within a turn any narrow question that provokes a pre_scan_inbox call (e.g. "how many urgent emails do I have?") will have its answer body swapped for the whole list. If pre-scan is only ever reached on a genuine triage request this is fine; worth a sentence in the PR (or an intent gate) confirming it.

🟢 Fragile dedup between finalize_answer and process_query (agent.py:1184). The result.get("result") != self._grounded_answer guard assumes _normalize_plain_text_answer is a perfect no-op on already-grounded text. If normalization ever perturbs the grounded string, equality fails, ground_final_answer re-runs, and the append-style guards (success-claim, etc.) double-fire — the exact bug _grounded_answer exists to prevent. A boolean "grounded-this-turn" flag would be sturdier than string equality. Also, _grounded_answer is never reset per turn, so it carries the prior turn's value into any turn that skips finalize_answer; harmless today but a latent coincidence bug.

🟢 Doubled comment on _ITEM_LINE_EMAIL_RE (answer_grounding.py:314-319) — two overlapping descriptions of the same regex, an editing artifact. Collapse to one:

# Any bare address on an item line, however the model punctuated around it.
# The sender is already named beside it, so a bare address renders twice --
# once as text, once as the mailto: link the markdown renderer expands. An
# explicit mailto: link goes too, for the same reason.

🟢 Stale comment after the cap bump (tui/internal/ui/cards/emailprescan_test.go:534) — NEEDS_YOU_CAP is now 10, but the TestPreScanCapsHitShowsNofM comment still reads "needs_you is capped at 5 server-side". The test uses fixture data so it still passes; just update the number.

Note (not blocking): a future AttentionItemKind added to _NEEDS_YOU_KIND_ORDER must also be added to _TRIAGE_SECTIONS or it silently vanishes from the rendered list — whereas the TUI's sectionLabelForVerb has a default: "NEEDS YOU" fallback. All six current kinds are covered, so no bug today; consider a default section to match the Go side.

Strengths

  • Rendering the list from tool output instead of model prose is the right call, and the "three/five consecutive live runs" rationale in the docstrings makes the determinism argument concrete rather than asserted.
  • The TUI card's budget reservation (countVerbGroups(...)-1)*2) correctly over-reserves against the total group count, so grouping can never overflow maxCardRows — and the narrow-width bound test still guards it.
  • Deleting the on-open pre-scan fetch plus its now-obsolete fix(email): agent is reconstructed every turn, so "reply to 1" can never resolve in the TUI #2829 refresh tests is a clean removal; the two SSE-map tests were updated to pin the intended single-key divergence rather than silently loosening the invariant.

@itomek itomek self-assigned this Aug 6, 2026
…lize_answer

Addresses review: the answer-rewriting logic that now decides what every
triage reply says had no coverage. Adds section ordering, ref numbering,
per-item classifier reasons, sender-address handling, the lead-paragraph
extraction, and the finalize_answer/process_query dedup that stops an
append-style guard firing twice on one turn.

Also fixes the pylint failure (unused finalize_answer default-hook argument),
collapses a doubled comment on _ITEM_LINE_EMAIL_RE, corrects two comments
that still claimed NEEDS_YOU_CAP is 5, and documents why
rewrite_triage_answer keys on tool presence rather than parsing intent.
@itomek
itomek enabled auto-merge August 6, 2026 17:15
@itomek
itomek disabled auto-merge August 6, 2026 17:19
@kovtcharov-amd
kovtcharov-amd added this pull request to the merge queue Aug 6, 2026
Merged via the queue into main with commit 88d130e Aug 6, 2026
78 of 89 checks passed
@kovtcharov-amd
kovtcharov-amd deleted the tmi/email-triage-view branch August 6, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent::email Email agent changes agents tui Go terminal UI (gaia-tui)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Append-style answer_grounding guards never reach the live TUI (correction fires, never renders)

2 participants