Skip to content

fix(conversation_loop): NameError on rate-limit — _pool_may_recover_from_rate_limit not in scope - #27608

Closed
abeperl wants to merge 1 commit into
NousResearch:mainfrom
abeperl:fix/conversation-loop-pool-recover-import
Closed

fix(conversation_loop): NameError on rate-limit — _pool_may_recover_from_rate_limit not in scope#27608
abeperl wants to merge 1 commit into
NousResearch:mainfrom
abeperl:fix/conversation-loop-pool-recover-import

Conversation

@abeperl

@abeperl abeperl commented May 17, 2026

Copy link
Copy Markdown

What

run_conversation() in agent/conversation_loop.py calls _pool_may_recover_from_rate_limit(), but the symbol is defined in run_agent.py (line 239) and is not imported anywhere in conversation_loop.py. The call therefore raises:

NameError: name '_pool_may_recover_from_rate_limit' is not defined

the moment the rate-limit / billing failover code path is entered (agent/conversation_loop.py:2254 on current main).

Why this isn't caught by the existing tests

The two tests that exercise this helper —

  • tests/agent/test_gemini_fast_fallback.py
  • tests/run_agent/test_provider_fallback.py

— both import the function directly from run_agent (from run_agent import _pool_may_recover_from_rate_limit). They confirm the helper itself behaves correctly but never exercise the call site inside conversation_loop.run_conversation(), so the missing reference is invisible to CI.

Reproduction

Trigger any provider 429 on the primary while a fallback chain is configured. The simplest natural repro is a Gemini RESOURCE_EXHAUSTED:

⚠️  API call failed (attempt 1/3): GeminiAPIError [HTTP 429]
   🔌 Provider: gemini  Model: gemini-3.1-pro-preview
   📝 Error: HTTP 429: Gemini HTTP 429 (RESOURCE_EXHAUSTED)

In a deployed agent (Slack / API server), this surfaces to users as:

Sorry, I encountered an error (NameError).
name '_pool_may_recover_from_rate_limit' is not defined
Try again or use /reset to start a fresh session.

The whole turn dies instead of falling through to either pool rotation or the configured fallback provider — exactly the recovery behavior this code path is meant to gate.

Fix

One-line change: route the call through the existing _ra() lazy module accessor (agent/conversation_loop.py:76) that this file already uses for other run_agent symbols. Avoids the cross-module circular-import issue that motivated _ra() in the first place, and matches the convention noted in its docstring (so test patches on run_agent.* continue to work).

-                    pool_may_recover = _pool_may_recover_from_rate_limit(
+                    pool_may_recover = _ra()._pool_may_recover_from_rate_limit(
                         agent._credential_pool,
                         provider=agent.provider,
                         base_url=getattr(agent, "base_url", None),
                     )

How to test

  • Manual: configure a primary + at least one fallback, force a 429 from the primary, confirm the agent now either rotates credentials or activates the fallback instead of crashing with NameError.
  • A regression test that exercises run_conversation() with a rate-limit-raising fake API client would prevent this class of bug going forward — happy to add one in a follow-up if maintainers prefer.

Platforms tested

Linux (Ubuntu 24.04, Python 3.14.3), Hermes Agent v0.14.0 + this patch, running under systemd as the gateway service.

…r _pool_may_recover_from_rate_limit

run_conversation() in agent/conversation_loop.py calls
_pool_may_recover_from_rate_limit(), but the symbol is defined in
run_agent.py and not imported by this module. The call would raise

    NameError: name '_pool_may_recover_from_rate_limit' is not defined

the moment a rate-limit or billing failover code path is entered.

Triggered in practice on any 429 from a provider — e.g. Gemini
RESOURCE_EXHAUSTED — and surfaces to users as a crashed turn:

    Sorry, I encountered an error (NameError).
    name '_pool_may_recover_from_rate_limit' is not defined

The unit tests in tests/agent/test_gemini_fast_fallback.py and
tests/run_agent/test_provider_fallback.py import the function
directly from run_agent, so they exercise the helper itself but
never hit the missing reference in conversation_loop.

Route the call through the existing _ra() lazy module accessor
(line 76), matching how this file already reaches other run_agent
symbols. Avoids the cross-module circular-import issue that
motivated _ra() in the first place.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint duplicate This issue or pull request already exists labels May 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #27359 — this is the same one-line fix (route _pool_may_recover_from_rate_limit through _ra()) already addressed by #27359 and multiple other PRs (#27374, #27433, #27468, #27532, #27534, #27583). Canonical issue: #27370.

@abeperl

abeperl commented May 17, 2026

Copy link
Copy Markdown
Author

Thanks for the quick triage — closing as duplicate of #27370. Will track the canonical issue. Glad to know fixes are already in flight; the local one-line patch keeps our deployment healthy in the meantime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint duplicate This issue or pull request already exists P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants