fix(conversation_loop): NameError on rate-limit fallback path - #27945
Closed
ta3pks wants to merge 1 commit into
Closed
fix(conversation_loop): NameError on rate-limit fallback path#27945ta3pks wants to merge 1 commit into
ta3pks wants to merge 1 commit into
Conversation
…_rate_limit The eager-fallback path on 429 calls _pool_may_recover_from_rate_limit() as a bare name, but the function lives in run_agent.py and is never imported into conversation_loop.py. When a rate-limit error actually reaches this code path, it crashes with NameError instead of evaluating the pool-recovery check — which means the fallback chain never fires for single-credential providers that can't rotate. Use _ra()._pool_may_recover_from_rate_limit() to access it through the lazy run_agent reference, consistent with every other cross-module call in this file. The bug is latent because the code path only executes when BOTH conditions are met: (1) a 429/rate-limit error occurs AND (2) the fallback chain has available entries. With few fallback entries or rare rate limits, the buggy line is never reached. Introduced in 1fc77f9 (fix(agent): fall back on rate limit when pool has no rotation room).
bbernstein616
approved these changes
May 18, 2026
bbernstein616
left a comment
There was a problem hiding this comment.
Watchdog verification: one-line fix correctly resolves _pool_may_recover_from_rate_limit through the existing _ra() lazy run_agent accessor. Independently tested PR head 72dfb84: scripts/run_tests.sh tests/agent -k 'rate_limit or fallback' -q => 121 passed; ruff check agent/conversation_loop.py => passed. Claude scoped worker also reviewed PR head and ran scripts/run_tests.sh tests/run_agent/test_provider_fallback.py tests/agent/test_gemini_fast_fallback.py => 28 passed. Coverage caveat: existing tests do not drive the exact conversation_loop 429 branch end-to-end, but the static NameError fix is correct and low risk.
Collaborator
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.
Summary
The eager-fallback path for 429 errors calls
_pool_may_recover_from_rate_limit()as a bare name, but the function is defined inrun_agent.pyand is never imported intoagent/conversation_loop.py. When this code path executes, it crashes withNameErrorinstead of evaluating the pool-recovery check — which means the fallback chain never fires for single-credential providers that cannot rotate.The Bug
_pool_may_recover_from_rate_limitis a module-level function inrun_agent.py(introduced in1fc77f995).conversation_loop.pydoes notfrom run_agent importit, nor is it defined locally. Every other cross-module call in this file uses the_ra()lazy accessor pattern (e.g._ra()._set_interrupt,_ra().handle_function_call).The Fix
One-line change:
_pool_may_recover_from_rate_limit(→_ra()._pool_may_recover_from_rate_limit(Why It Wasn't Caught
The code path only executes when both conditions are true:
_fallback_index < len(_fallback_chain))With few fallback entries or rare rate limits, the buggy line is never reached. In production, observed as cron jobs dying silently on 429 instead of falling through to the configured
fallback_providers.Test Plan
tests/run_agent/test_provider_fallback.pyandtests/agent/test_gemini_fast_fallback.pypass_ra()._pool_may_recover_from_rate_limitat runtimeIntroduced in
1fc77f995(fix(agent): fall back on rate limit when pool has no rotation room).