fix(conversation_loop): use _ra() to access _pool_may_recover_from_rate_limit#28189
Closed
medardoacevallos wants to merge 1 commit into
Closed
Conversation
…te_limit The bare-name reference at agent/conversation_loop.py:2320 raises NameError because _pool_may_recover_from_rate_limit is defined in run_agent.py and is never imported into agent/conversation_loop.py. This module accesses run_agent symbols via the _ra() lazy-import helper everywhere else (see the helper at lines 76-82, and call sites at 553, 555, 3765, 3814); this single call site was missed. Triggered only when a provider returns rate-limit / quota / billing and a fallback_chain is configured — i.e. the rate-limit eager-fallback decision path. Easy to miss in routine testing. Refs NousResearch#11314, NousResearch#13636 (the issues the existing comment block points to).
Collaborator
outsourc-e
approved these changes
May 18, 2026
outsourc-e
left a comment
Contributor
There was a problem hiding this comment.
Validated locally on clean upstream/main worktree. Targeted fallback tests passed (28 passed). Narrow one-line call-site fix; no security concerns.
Contributor
|
Closing as duplicate of #28159 by @AceWattGit, which has been merged via salvage PR #28345 (commit 25e0f4d). All three of your PRs converged on the same fix (routing the call through `_ra()`), and AceWattGit's version also ships an inspect-based regression test guarding the call shape. Thanks for spotting the NameError — community signal helped triage it fast. |
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
4 participants
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
One-line fix for a latent
NameErroron the rate-limit eager-fallback path.agent/conversation_loop.py:2320calls_pool_may_recover_from_rate_limit(...)as a bare name, but that symbol is defined inrun_agent.py(line 239) and is never imported intoagent/conversation_loop.py. Everywhere else in this file,run_agentsymbols are reached through the_ra()lazy-import helper defined on lines 76–82 (also used on 553, 555, 3765, 3814). This single site was missed.Reproduction
Triggered when a provider returns a rate-limit / quota / billing error and a
fallback_chainis configured. Observed in production on ahomercrmprofile gateway after the primary OpenAI Codex provider returned HTTP 429; the conversation crashed instead of switching to the fallback provider:The existing tests in
tests/run_agent/test_provider_fallback.pyandtests/agent/test_gemini_fast_fallback.pyexercise the function directly viafrom run_agent import _pool_may_recover_from_rate_limit, so they don't catch this call-site bug — they bypass the conversation-loop import scope.Test plan
inspect.getsource(agent.conversation_loop)contains_ra()._pool_may_recover_from_rate_limitRefs #11314, #13636 — the issues the existing comment block points to.