fix(conversation_loop): route _pool_may_recover_from_rate_limit through _ra() - #4
Merged
Merged
Conversation
…gh _ra()
The eager rate-limit fallback path in agent/conversation_loop.py calls
_pool_may_recover_from_rate_limit() directly, but that helper is defined
in run_agent.py and never imported into this module. Any 429/quota error
that triggers the eager-fallback branch raises:
NameError: name '_pool_may_recover_from_rate_limit' is not defined
masking the real provider failure and aborting the turn.
The file already uses the lazy _ra() accessor (line 76) for exactly this
kind of cross-module reach — both to avoid an import cycle with
run_agent.py and to keep monkey-patched run_agent symbols reachable in
tests. Route the call through _ra() so the production code path matches
what the test suite already validates (tests import the helper directly
from run_agent, which is why this regressed silently when the
conversation loop was extracted out of run_agent.py).
Repro: any rate-limit (FailoverReason.rate_limit / billing) with a
configured fallback chain — the branch fires and NameErrors instead of
switching providers.
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-reference |
1 |
First entries
agent/conversation_loop.py:2335: [unresolved-reference] unresolved-reference: Name `_pool_may_recover_from_rate_limit` used when not defined
Unchanged: 4650 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
pai-scaffolde
pushed a commit
that referenced
this pull request
Jun 12, 2026
…eSessionPage (NousResearch#43487) When auto-compression rotates the session tip (old #4 → new #5), the incoming page carries the new tip but the previous list still holds the old one. The old tip's id differs from the new tip's id, so the existing id-only dedup in mergeSessionPage() preserves both as separate sidebar rows. Add lineage-level dedup: build a set of incoming lineage keys (`_lineage_root_id ?? id`) and filter survivors whose lineage key matches any incoming row. This mirrors the existing sessionPinId() logic used for pin stability. Fixes NousResearch#43483
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
2 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.
Bug
The eager rate-limit fallback path in
agent/conversation_loop.py(line 2335) calls_pool_may_recover_from_rate_limit(...)directly, but the helper is defined inrun_agent.pyand is never imported intoconversation_loop. Any 429 / quota error that classifies asFailoverReason.rate_limitorFailoverReason.billing— with a configured fallback model — raises:…masking the real provider failure and aborting the turn instead of failing over.
Why it slipped through
The test suite imports the helper directly from
run_agent(tests/run_agent/test_provider_fallback.py,tests/agent/test_gemini_fast_fallback.py), so the unit tests for the function pass — but the production call site in the extracted loop module has no path to the symbol. Likely regression from when the conversation loop was carved out ofrun_agent.pywithout porting the call through the lazy_ra()accessor that the rest of this file already uses.Fix
Route the call through the existing
_ra()helper (defined at line 76 of the same file), matching the established pattern for reachingrun_agentsymbols without an import cycle and while keeping monkey-patched symbols reachable in tests.One-line change. Lint clean. No test changes needed — the existing tests still exercise the helper directly via
run_agent.Repro
Trigger any rate-limit/billing error on a provider that has a fallback configured. Pre-fix:
NameErroraborts the turn. Post-fix: eager fallback branch evaluates correctly and either rotates the pool or activates the fallback model as designed (NousResearch#11314, NousResearch#13636 behavior preserved).Verified
~/.hermes/hermes-agent/agent/conversation_loop.py); no more NameError on rate-limit branch.