fix(agent): size the step budget for research turns and fail usefully - #27
fix(agent): size the step budget for research turns and fail usefully#27jerelvelarde wants to merge 1 commit into
Conversation
cc40eaf to
7c7b25d
Compare
There was a problem hiding this comment.
We seemed to have created a custom impl of the ag-ui agent. Doesn't seem like the smallest possible way to do this. I'd prefer that we answer why we're recursing so much instead.
A turn died in production with "Recursion limit of 25 reached without
hitting a stop condition". The reflex is to read that as the agent
over-recursing. It was not: 25 super-steps is four tool calls.
The limit counts super-steps, and the middleware chain makes one tool
call cost four of them (model -> TodoListMiddleware.after_model ->
CopilotKitMiddleware.after_model -> tools) on top of a six-step baseline.
Measured against the real graph rather than reasoned about, driving it
with a scripted model that makes a known number of tool calls:
tool calls 0 1 2 5
super-steps 6 10 14 26
recursion_limit=25 -> completes at most 4 tool calls
recursion_limit=60 -> completes at most 13
Four was the right size when the limit was lowered from 100 to 25 in
4ac1a7a for a triage-only bot. The agent now loads 85 tools and gets
asked research questions: "find how many PRs shipped in CPK related to
channels, categorize them, chart it" answered from 212 PRs and must have
sat right at the cap, and the next comparable question went over. A
search, a page of results, and a chart is already four calls.
Raise the default to 60 and read it from AGENT_RECURSION_LIMIT so it can
be tuned without a deploy. Reject a value too low to finish a single tool
call, since that breaks every turn rather than tightening anything.
The tests pin the conversion to the graph, so if a middleware is added or
removed the arithmetic fails rather than silently misreporting -- and
what startup prints is checked against what the graph actually completes
at four different limits.
7c7b25d to
fd9dc65
Compare
|
Both fair. I dropped the custom agent subclass, and you're right that I should have answered the "why" first — so I measured it instead of reasoning about it. Why we're recursing so much: we aren'tThe limit counts super-steps, not tool calls, and the middleware chain makes one tool call cost four of them: Driving the real graph with a scripted model that makes a known number of tool calls:
25 was a four-tool-call budget. A GitHub search, one page of results, and a chart is already four. So the failing turns weren't necessarily thrashing — the ceiling was below the floor for anything but a lookup. That was the right size when it went 100 → 25 in Both numbers are now tests, not comments — The custom agent impl is gone
That does mean a blown budget still dies as an ASGI error and the user still gets the Channel's generic "I hit an error". I think that's worth fixing separately, but it's a different concern from sizing the budget and I'd rather not smuggle it in here. Happy to leave it, or open it standalone — your call. What I still can't tell youWhether any specific dead turn was also thrashing. Nothing records the trajectory: the agent logs only 81 tests pass. Rebased onto |
The error
It escaped as an ASGI error, killing the SSE stream mid-answer.
25 was never 25 tool calls
The limit counts super-steps, and the middleware chain makes one model-call-plus-tool round trip cost four of them:
(25 − 3) / 4≈ 5–6 tool calls per turn. The budget also resets on resume (stop = step + limit + 1,_loop.py:1701), so this was a single turn wanting a sixth call — not a long conversation accumulating.Why it started now
The value was deliberately lowered 100 → 25 in
4ac1a7a"make OpenTag triage-first", which was right for a triage-only bot. Since then the agent loads 85 tools — GitHub search landed in #21 — and people ask it research questions. From the same log, two minutes earlier:That ran 55s, answered "Found 212 merged PRs", and must have landed right at the cap. The next comparable question went over.
Changes
Default 60 (≈14 tool calls), read from
AGENT_RECURSION_LIMITso it can be tuned without a deploy. Values too low to finish one tool call are rejected as misconfiguration rather than accepted as tuning. Startup logs the limit and the tool-call budget it implies.Failing usefully. Raising the ceiling makes a runaway turn more expensive, so the failure is no longer lost.
GraphRecursionErrornow ends the run the way the protocol expects — text message, thenRUN_FINISHED— with:instead of a truncated stream and the Channel's generic "I hit an error", 40 seconds in.
Diagnosability. The tool trail is logged at the failure point. Reconstructing this one required reading Slack, because the log showed only
POST / 200lines and a traceback.Testing
88 passed (18 new): budget arithmetic, env parsing and rejection, pass-through on success, the closing event sequence and its run identity, non-recursion errors still propagating, and the tool trail reaching the logs.
test_wheel_includes_every_runtime_modulecaught the new module missing frompy-modules— fixed.Trade-off
60 steps is a longer worst case: a runaway turn can now run ~2 minutes and spend proportionally more before stopping. That is the argument for the graceful failure and the trail logging landing with it, not after. If that ceiling feels high for a Slack bot,
AGENT_RECURSION_LIMITmoves it without a deploy.Separate from #23, which fixes the approval-gate issues found in the same session.