fix(reflect): enforce max_tokens on the done-tool completion path (#2756) - #2775
Closed
benfrank241 wants to merge 1 commit into
Closed
fix(reflect): enforce max_tokens on the done-tool completion path (#2756)#2775benfrank241 wants to merge 1 commit into
benfrank241 wants to merge 1 commit into
Conversation
) A mental model's max_tokens is forwarded to the refresh reflect call and the agent capped its forced-final and direct-text short-circuit paths — but the done-tool path, which is how the agent normally finishes, took the answer verbatim from the (intentionally uncapped) tool-call arguments with no token check. So the cap was only enforced on the two paths the agent rarely takes, and mental-model content routinely overshot max_tokens by several times. Extract the short-circuit path's capped-rewrite logic into a shared `_enforce_answer_token_cap` helper and route all three completion paths through it, so the cap is honoured no matter how the agent finishes. The done-tool path now folds the rewrite's token usage into its accounting and adds a `final_rewrite` trace entry, matching the short-circuit path. Tests: direct regression tests over `_process_done_tool` — an over-budget answer triggers exactly one capped rewrite (budget forwarded, usage folded in, traced), an under-budget answer makes no call, and max_tokens=None is a no-op.
Collaborator
|
superseded by #2757 |
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.
Fixes #2756.
Problem
A mental model's
max_tokensis forwarded throughrefresh_mental_model→reflect_async→run_reflect_agent(the #1076 fix), and the agent enforced it on its forced-final path (max_completion_tokens) and its direct-text short-circuit path (rewrite whencount_cl100k_tokens(answer) > max_tokens). But thedone-tool path — how the agent normally finishes — took the answer verbatim from the tool-call arguments (_process_done_tool) with no token check and no rewrite.Those tool-call arguments come from
call_with_tools, which is intentionally uncapped to leave headroom for tool-call JSON. So the cap was only enforced on the two paths the agent rarely takes, and mental-model content routinely overshotmax_tokensby several times.donetool (normal path)Verified against the reporter's repro (
max_tokens=256→ ~1,150-token content;max_tokens=800→ ~1,360 tokens, well-formed, not truncated → the cap was simply never applied).Fix
Extract the short-circuit path's capped-rewrite logic into a shared
_enforce_answer_token_caphelper and route all three completion paths through it, so the cap is honoured no matter how the agent finishes. The done-tool path now:final_rewriteLLM-trace entry (matching the short-circuit path).The rewrite happens before structured-output generation so structured output is derived from the capped answer. No behaviour change when the answer is already within budget (no extra call) or when no cap is configured.
Tests
Deterministic regression tests over
_process_done_tool:max_completion_tokensforwarded,scope="reflect", usage folded in,final_rewritetraced), rewritten text returned;max_tokens=None→ no-op;59 deterministic tests in
test_reflect_agent.pypass (real-LLM judge tests excluded from the local run).Reported by @sphynx79 with a clean root-cause trace and repro.