fix(agent): grow output-cap retry margin exponentially to compensate for per-retry input-token drift (#61761) - #61846
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the fixed-margin convergence failure. Current main still has the exact available_out - 64 calculation at agent/conversation_loop.py:3451, so the proposed reserve addresses a real current-HEAD defect.
Problems
- The PR adds no regression test. The existing tests at
tests/test_ctx_halving_fix.py:315-327and:340-350manually repeat the old 64-token calculation; they do not execute the recovery branch or cover the reported +65-token drift. - Output-cap recovery still consumes
compression_attemptsand eventually reports compression exhaustion (agent/conversation_loop.py:3458-3477). A drift larger than the bounded reserve still receives a misleading compression result. The cross-referenced #62197 is relevant because it addresses this coupling separately.
Suggested changes
- Add a loop-level regression that simulates parseable output-cap failures with +65 input tokens between attempts and verifies recovery without context compression.
- Decide whether to retain this as a narrow margin fix or fold it into an output-cap-specific retry budget/terminal error path.
Automated hermes-sweeper review.
| @@ -3448,7 +3448,8 @@ def _perform_api_call(next_api_kwargs): | |||
| # Error is purely about the output cap being too large. | |||
| # Cap output to the available space and retry without | |||
| # touching context_length or triggering compression. | |||
| safe_out = max(1, available_out - 64) # small safety margin | |||
| safety_margin = 256 * (2 ** compression_attempts) # exponential to outpace per-retry input drift (~65 tok/retry) | |||
There was a problem hiding this comment.
Please add a regression test that drives this recovery path through successive provider errors with a +65-token input increase. The current tests manually reimplement the retired 64-token calculation, so they would not fail if this branch still used a non-converging reserve.
…for per-retry input-token drift (NousResearch#61761)
f5c7b1c to
180fec4
Compare
|
Independent confirmation of the bug this fixes, from a different model and context size — I hit it before finding your PR and opened a duplicate (#72826, which I'll close in favour of this one). Hermes 0.18.2, Same 65-token per-retry drift, same constant overshoot of exactly 1, on a 262k window rather than #61761's 200k. Your diagnosis holds across both. Worth stating explicitly because it shows raising Two things I ran into while implementing the same fix, offered as review rather than as a competing opinion: 1. This also removes the 2. On the base value: I tested 64 / 256 / 1024 against the observed 65-token drift. All converge, but with
256 is comfortably fine for the observed drift; 1024 just buys more headroom if a provider drifts more than measured. Your call — the escalation is the part that matters. I have 9 tests that pin the escalation property rather than any specific constant, so they'd pass against this PR's |
|
Closing as superseded, with thanks for the analysis on #61761. Main took a two-part path instead of margin growth: #55546 made the output-cap retry compress alongside the clamp, and #90563 (auto-merge armed) removes the drift's root cause — vLLM's "at least N input tokens" figure is back-computed from the constraint (window + 1 − requested), so the parser now detects that degenerate shape and halves the cap instead of chasing it. With the drift gone at the source, exponential margin growth no longer has a failure mode to compensate for. |
Summary
When a request fails because
input_tokens + max_tokens > context_length, the recovery path shrinksmax_tokensand retries. A fixed 64-token margin was erased by ~65-token per-retry input drift, causing the loop to never converge and burn through all compression attempts.Trace
Change
Replaced the fixed 64-token margin with an exponentially growing margin:
256 * (2 ** compression_attempts), producing margins of 256, 512, 1024, ... that outpace the ~65-token drift per retry.Verification
25 context/compression/retry tests pass.