Skip to content

Commit d015f22

Browse files
maxmilianclaude
andcommitted
fix(email): call the LLM to triage urgent emails instead of short-circuiting 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 #5506 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c80462e commit d015f22

2 files changed

Lines changed: 188 additions & 6 deletions

File tree

src/builtin_actions.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,11 +2071,12 @@ def _scan_one(account=acc, cache_uids=cache.get("uids", {})):
20712071
# Skip uids we couldn't fetch (no subject/from/body).
20722072
if not item.get("subject") and not item.get("from"):
20732073
continue
2074-
verdict = _heuristic_email_verdict(item)
2075-
cache.setdefault("uids", {})[item["uid"]] = verdict
2076-
per_uid_scores[key] = verdict
2077-
saved_classifications += 1
2078-
continue
2074+
# Heuristic verdict as a fallback for THIS scan's aggregation,
2075+
# tagging, and notify. Deliberately NOT written to `cache`: only
2076+
# a successful LLM classification is persisted below, so a
2077+
# transient LLM failure is retried on the next scan instead of
2078+
# sticking the heuristic verdict at the current TRIAGE_VERSION.
2079+
per_uid_scores[key] = _heuristic_email_verdict(item)
20792080
# ── LLM-classify. JSON-only response; bullet-proof parse.
20802081
llm_attempts += 1
20812082
prompt = (
@@ -2100,7 +2101,7 @@ def _scan_one(account=acc, cache_uids=cache.get("uids", {})):
21002101
raw = await llm_call_async_with_fallback(
21012102
candidates,
21022103
[{"role": "user", "content": prompt}],
2103-
temperature=0.1, max_tokens=220, timeout=30,
2104+
temperature=0.1, max_tokens=2048, timeout=30,
21042105
)
21052106
# Tolerant JSON-parse: strip code fences if present.
21062107
txt = (raw or "").strip()
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
"""Regression tests for issue #5506: the Email Tags task
2+
(`check_email_urgency`) must actually call the configured LLM to triage
3+
fresh unread mail — honouring `urgent_email_prompt` — instead of silently
4+
short-circuiting to the keyword heuristic, and it must give reasoning models
5+
enough token budget to emit their JSON verdict.
6+
"""
7+
8+
from types import SimpleNamespace
9+
10+
import pytest
11+
12+
13+
class _Column:
14+
def __eq__(self, _other):
15+
return True
16+
17+
def __ne__(self, _other):
18+
return True
19+
20+
21+
class _Query:
22+
def __init__(self, rows):
23+
self._rows = rows
24+
25+
def filter(self, *_args, **_kwargs):
26+
return self
27+
28+
def all(self):
29+
return list(self._rows)
30+
31+
32+
class _Db:
33+
def __init__(self, rows_by_model):
34+
self._rows_by_model = rows_by_model
35+
self.closed = False
36+
37+
def query(self, model):
38+
return _Query(self._rows_by_model.get(model, []))
39+
40+
def close(self):
41+
self.closed = True
42+
43+
44+
class _FakeImap:
45+
"""Minimal IMAP stub yielding one fresh, unread inbox message."""
46+
47+
def __init__(self):
48+
self.logged_out = False
49+
50+
def select(self, *_args, **_kwargs):
51+
return "OK", [b"1"]
52+
53+
def uid(self, command, *args):
54+
if command == "SEARCH":
55+
return "OK", [b"5"]
56+
if command == "FETCH":
57+
raw = (
58+
b"From: Alice Manager <alice@example.com>\r\n"
59+
b"Subject: Need your sign-off on the Q3 budget today\r\n"
60+
b"Message-ID: <m5@example.com>\r\n"
61+
b"\r\n"
62+
b"Please approve the Q3 budget by 5pm; the release is blocked until you do."
63+
)
64+
# FLAGS () => no \\Seen => unread.
65+
return "OK", [(b"5 (UID 5 FLAGS () RFC822.HEADER", raw)]
66+
return "OK", []
67+
68+
def logout(self):
69+
self.logged_out = True
70+
71+
72+
async def _run_urgency_capturing_llm(monkeypatch, tmp_path, llm_raises=False):
73+
"""Drive action_check_email_urgency over one fresh unread email and return
74+
the list of max_tokens values the LLM classifier was invoked with. When
75+
llm_raises is True the fake LLM raises, exercising the heuristic fallback."""
76+
from core import database
77+
from routes import email_helpers
78+
from src import builtin_actions, llm_core, settings, task_endpoint
79+
80+
class FakeEmailAccount:
81+
enabled = _Column()
82+
owner = _Column()
83+
imap_user = _Column()
84+
from_address = _Column()
85+
86+
account = SimpleNamespace(
87+
id="acc1",
88+
enabled=True,
89+
owner="alice",
90+
imap_user="alice",
91+
from_address="alice",
92+
)
93+
db = _Db({FakeEmailAccount: [account]})
94+
95+
monkeypatch.setattr(database, "EmailAccount", FakeEmailAccount)
96+
monkeypatch.setattr(database, "SessionLocal", lambda: db)
97+
monkeypatch.setattr(
98+
settings,
99+
"load_settings",
100+
lambda: {"urgent_email_prompt": "Flag anything my manager sends."},
101+
)
102+
monkeypatch.setattr(
103+
task_endpoint,
104+
"resolve_task_candidates",
105+
lambda *args, **kwargs: [("http://llm", "alice-model", {})],
106+
)
107+
monkeypatch.setattr(
108+
email_helpers, "_imap_connect", lambda *_a, **_k: _FakeImap()
109+
)
110+
111+
# Keep all state file writes inside the tmp dir.
112+
monkeypatch.setattr(builtin_actions, "DATA_DIR", str(tmp_path))
113+
monkeypatch.setattr(
114+
builtin_actions, "EMAIL_URGENCY_CACHE_DIR", str(tmp_path / "urgency_cache")
115+
)
116+
monkeypatch.setattr(email_helpers, "SCHEDULED_DB", tmp_path / "scheduled_emails.db")
117+
email_helpers._init_scheduled_db()
118+
119+
max_tokens_seen = []
120+
121+
async def fake_llm(_candidates, _messages, **kwargs):
122+
max_tokens_seen.append(kwargs.get("max_tokens"))
123+
if llm_raises:
124+
raise RuntimeError("endpoint unreachable")
125+
# Score < 2 so the notify path stays silent (no SMTP in the test).
126+
return '{"score":1,"tags":["action-needed"],"spam":false,"reason":"manager asked for sign-off"}'
127+
128+
monkeypatch.setattr(llm_core, "llm_call_async_with_fallback", fake_llm)
129+
130+
from src.builtin_actions import action_check_email_urgency
131+
132+
await action_check_email_urgency("alice")
133+
return max_tokens_seen
134+
135+
136+
@pytest.mark.asyncio
137+
async def test_urgency_calls_llm_for_fresh_unread_email(monkeypatch, tmp_path):
138+
"""The LLM classifier must be invoked for a fresh unread email; on `dev`
139+
the hardcoded `continue` makes the LLM block unreachable, so this is red
140+
until the short-circuit is removed."""
141+
max_tokens_seen = await _run_urgency_capturing_llm(monkeypatch, tmp_path)
142+
assert max_tokens_seen, (
143+
"LLM classifier was never called — check_email_urgency short-circuited "
144+
"to the keyword heuristic and ignored urgent_email_prompt (#5506)"
145+
)
146+
147+
148+
@pytest.mark.asyncio
149+
async def test_urgency_gives_reasoning_models_enough_tokens(monkeypatch, tmp_path):
150+
"""Reasoning models spend tokens thinking before emitting JSON; a 220-token
151+
cap yields an empty completion. The classify call must request a budget
152+
large enough to return the verdict (#5506, second defect)."""
153+
max_tokens_seen = await _run_urgency_capturing_llm(monkeypatch, tmp_path)
154+
assert max_tokens_seen, "LLM classifier was never called (#5506)"
155+
assert max_tokens_seen[0] is not None and max_tokens_seen[0] >= 1024, (
156+
f"classify call requested only {max_tokens_seen[0]} max_tokens; reasoning "
157+
"models need room to emit their JSON verdict (#5506)"
158+
)
159+
160+
161+
@pytest.mark.asyncio
162+
async def test_urgency_retries_llm_next_scan_after_transient_failure(monkeypatch, tmp_path):
163+
"""A transient LLM failure must not stick the heuristic fallback in the
164+
cache: the LLM is attempted, and the uid is left uncached so the next
165+
hourly scan re-classifies it instead of skipping it as already-triaged."""
166+
import json
167+
168+
max_tokens_seen = await _run_urgency_capturing_llm(
169+
monkeypatch, tmp_path, llm_raises=True
170+
)
171+
assert max_tokens_seen, "LLM classifier was never even attempted (#5506)"
172+
173+
cache_file = tmp_path / "urgency_cache" / "acc1.json"
174+
cached_uids = {}
175+
if cache_file.exists():
176+
cached_uids = json.loads(cache_file.read_text()).get("uids", {})
177+
assert "5" not in cached_uids, (
178+
"heuristic fallback was cached at the current triage version after an "
179+
"LLM failure, so the next scan would skip re-classification instead of "
180+
"retrying the LLM"
181+
)

0 commit comments

Comments
 (0)