fix(error): treat 402 'can only afford N tokens' as recoverable rate-limit - #49785
tcconnally wants to merge 6 commits into
Conversation
dd7821f to
e94c813
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the 402 classification gap; current main still routes every non-transient affordable-token 402 through agent/error_classifier.py:1072-1098 as terminal billing.
Problems
agent/conversation_loop.py:3051resetsretry_countbeforecontinue. The current recovery loop is bounded bywhile retry_count < max_retriesatagent/conversation_loop.py:1105; repeated affordable-token errors therefore never reach its exhaustion path. Keep the retry bounded (for example, one clamp retry or only a strictly lower cap).- The auxiliary edit does not change a real 402:
_is_payment_error()returns immediately forstatus == 402at PRagent/auxiliary_client.py:2620. It also changes 429 behavior while the unchanged test attests/agent/test_auxiliary_client.py:1829-1832expects"can only afford"not to be a rate limit.
Suggested changes
- Add an end-to-end recovery test covering: 402 → capped outgoing request → success, plus a repeated 402 termination case.
- Re-scope or complete the auxiliary path with an actual cap reduction and matching tests.
Automated hermes-sweeper review.
| f"retrying with max_tokens={safe_out:,} " | ||
| f"(affordable={affordable_tokens:,})" | ||
| ) | ||
| retry_count = 0 |
There was a problem hiding this comment.
retry_count is the bound for the enclosing while retry_count < max_retries loop. Resetting it for every repeated affordable-token 402 makes that loop unbounded; keep this recovery to one attempt or only retry when the cap strictly decreases.
| @@ -2627,7 +2627,7 @@ def _is_payment_error(exc: Exception) -> bool: | |||
| if status in {402, 403, 404, 429, None}: | |||
| if any(kw in err_lower for kw in ( | |||
| "credits", "insufficient funds", | |||
| "can only afford", "billing", | |||
| "billing", | |||
There was a problem hiding this comment.
This does not affect the stated HTTP 402 case because the preceding if status == 402: return True remains. It changes only non-402 message handling and needs either a matching auxiliary cap-recovery implementation or removal from this PR.
| @@ -2685,7 +2685,7 @@ def _is_rate_limit_error(exc: Exception) -> bool: | |||
| # Generic 429 without billing keywords = likely a rate limit | |||
| if not any(kw in err_lower for kw in ( | |||
| "credits", "insufficient funds", "billing", | |||
| "payment required", "can only afford", | |||
| "payment required", | |||
There was a problem hiding this comment.
Removing this exclusion makes a 429 containing can only afford classify as a rate limit, but the unchanged tests/agent/test_auxiliary_client.py assertion for that exact message expects False.
|
Please hold off on this one — reviewing my own change, it has two real problems I need to fix before it should be considered:
The underlying bug (affordable-token 402s being treated as terminal) is real, but I'll rework this with a bounded cap, drop/rescope the auxiliary change, add an end-to-end test (402 → cap → success, plus repeated-402 termination), and correct the description, which referenced a function name and line numbers that don't match the diff. Apologies for the premature PR — will update or replace it. |
|
Rework complete: added bounded clamp to the conversation loop to prevent infinite spinning, reverted unnecessary |
|
Addressed the review feedback in |
|
Follow-up: CI exposed an attribution-only failure for |
317f545 to
7b5fc9e
Compare
|
Hi — just checking in. This PR has been quiet for a while. Is there anything still needed from my side to move it forward? |
…limit When a provider returns HTTP 402 with 'can only afford N tokens' (common with OpenRouter), the account HAS usable credit — just not enough for the requested max_tokens. Previously classified as terminal billing exhaustion, causing the request to be dropped when it would succeed with a lower output cap. Changes: - error_classifier: _AFFORDABLE_TOKENS_PATTERNS + regex to detect 'can only afford N' in 402 bodies; new branch in _classify_402 before billing exhaustion: extract token count, classify as rate_limit (retryable) with affordable_max_tokens in error_context. - conversation_loop: intercept affordable-tokens before eager fallback; clamp _ephemeral_max_output_tokens and retry instead. - auxiliary_client: remove 'can only afford' from credit exhaustion keywords (402 status-code check unchanged — safe). - tests: 2 new tests for affordable-tokens classification. Closes #49769
Reviewer feedback on #49785: the auxiliary_client.py edits could not affect the stated HTTP 402 case (status==402 returns True before the keyword lists are consulted) and only changed non-402 handling. The remaining diff was a pure reordering of the keyword lists. Revert agent/auxiliary_client.py to the upstream state so the PR contains no auxiliary changes at all; the 402 'can only afford N' recovery lives entirely in error_classifier.py (classification + affordable_max_tokens hint) and conversation_loop.py (bounded one-shot clamp). Guarded by existing tests: - tests/agent/test_auxiliary_client.py::TestIsRateLimitError:: test_429_with_billing_message_is_not_rate_limit (429 'can only afford' is NOT a rate limit) - tests/agent/test_auxiliary_client.py::TestIsPaymentError:: test_402_with_credits_message (402 'can only afford' IS payment)
7b5fc9e to
01e593d
Compare
Summary
When a provider returns HTTP 402 with the message "can only afford N tokens" (common with OpenRouter and other credit-routing platforms), the account HAS usable credit — just not enough for the requested
max_tokens. Previously this was classified as terminal billing exhaustion (FailoverReason.billing,retryable=False), causing the request to be dropped when it would succeed with a lower output cap.Root Cause
_classify_402inerror_classifier.pyhad no detection for the "can only afford" pattern — it fell through to terminal billing exhaustion.Fix (2 coordinated changes)
1.
error_classifier.py— detect affordable-tokens 402s_classify_402: when "can only afford N" is detected, extract the token count, storeaffordable_max_tokensinClassifiedError.error_context, and classify asrate_limit(retryable) with no credential rotation or fallback.2.
conversation_loop.py— bounded clamp max_tokenserror_contextcontainsaffordable_max_tokens, we do a strictly lowering clamp on_ephemeral_max_output_tokenstoaffordable - 64and retry.Testing
auxiliary_client.pychanges (429 exclusion lists remain unaffected).test_error_classifier.pyandtest_auxiliary_client.pytests pass.Closes #49769