fix(email): call the LLM to triage urgent emails instead of short-circuiting to the heuristic (#5506)#5530
Open
maxmilian wants to merge 1 commit into
Open
Conversation
…cuiting to the heuristic `action_check_email_urgency` (the hourly "Email Tags" task) computed a keyword heuristic verdict and then `continue`d before the LLM classification block, making the entire ~120-line LLM path — and the only use of the user's `urgent_email_prompt` setting — dead code. Every email was scored by regex; the urgency prompt had no effect. A second defect: even reachable, the classify call capped `max_tokens=220`, so reasoning models spent their budget "thinking" and hit finish_reason=length with an empty completion. - Remove the hardcoded `continue` short-circuit so the LLM classifies each fresh unread email honouring `urgent_email_prompt`. - Keep the heuristic as a graceful fallback for the current scan's aggregation, tagging, and notify — but do NOT cache it: only a successful LLM verdict is persisted, so a transient LLM failure is retried next scan instead of sticking the heuristic at the current TRIAGE_VERSION. - Bump the classify budget 220 -> 2048 so reasoning models can emit their JSON. Regression tests drive the real action end-to-end (IMAP scan -> per-item -> classify) with only the IMAP socket and LLM HTTP faked: the LLM is invoked for a fresh unread email, the budget is large enough, and a transient LLM failure leaves the uid uncached for retry. All three are red on dev, green here. Fixes odysseus-dev#5506 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I'm glad I didn't report a non issue. Thanks for fixing it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The hourly Email Tags task (
action_check_email_urgency) is supposed to LLM-triage new unread mail honouring the user'surgent_email_prompt, but ondevit never calls the LLM at all. The per-item loop computes a keyword heuristic verdict and thencontinues before the LLM block, making the entire ~120-line classification path — the only code that readsurgent_email_prompt— unreachable dead code. Every email is scored by regex and the urgency prompt has no effect. A second, compounding defect: even with thecontinueremoved, the classify call capsmax_tokens=220, so reasoning models (e.g.gemma4:31b) spend their budget "thinking" and hitfinish_reason=lengthwith an empty completion. This PR removes the short-circuit, keeps the heuristic as a graceful (uncached) fallback, and raises the token budget to 2048.Target branch
dev, notmain. All PRs land indev;mainis curated by the maintainer at each release. If your PR is onmainby accident, click "Edit" on this PR and change the base.Linked Issue
Fixes #5506
Type of Change
Checklist
check_email_urgency/ email urgency /urgent_email_prompt.)devdocker compose uporuvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough. — Backend scheduled-task logic, no UI surface. Seeing the fix in-app requires a live IMAP mailbox + LLM endpoint, so I verified two ways instead: (1) booteduvicorn app:appagainst this branch — the app starts cleanly and thebuiltin_actionsmodule loads in the real process (Email built-in MCP registers,/→ 302,/api/tasks→ 401); (2) the regression tests below drive the realaction_check_email_urgencythrough its full IMAP-scan → per-item → LLM-classify path with only the IMAP socket and LLM HTTP faked — i.e. the production code path minus the network.How to Test
The change is in
src/builtin_actions.py(action_check_email_urgency). Three regression tests intests/test_check_email_urgency_llm.pyexercise the real function; to confirm they genuinely catch the bug, run them againstdev(red) and this branch (green):The tests assert, over one fresh unread inbox message: (1) the LLM classifier is actually invoked (red on
dev— thecontinueskips it); (2) the classify call requests ≥1024max_tokens(red ondev— 220); (3) after a transient LLM failure the uid is left uncached, so the next hourly scan re-classifies instead of skipping it as already-triaged.Live sanity check (optional): link an IMAP mailbox, set an
urgent_email_prompt, configure a utility model, run the Email Tags task, and confirmdata/scheduled_emails.db → email_tags.model_usednow records the model (not empty) and thereasonfields reflect the prompt rather than keyword matches.Visual / UI changes — REQUIRED if you touched anything that renders
N/A — backend only. No buttons, icons, CSS, HTML, SVG, or
static/js/changes.