Skip to content

feat(core): retry primary on transient 429 before falling back - #51677

Open
zdjackzdg wants to merge 2 commits into
NousResearch:mainfrom
zdjackzdg:feature/retry-primary-on-transient-429
Open

zdjackzdg wants to merge 2 commits into
NousResearch:mainfrom
zdjackzdg:feature/retry-primary-on-transient-429

Conversation

@zdjackzdg

Copy link
Copy Markdown

Problem

When the primary provider returns a 429 (rate limit) and no credential pool is configured, the agent immediately activates the fallback provider — even when the 429 is a transient cluster-level throttle ("high demand") rather than genuine quota exhaustion. This causes unnecessary provider switches on recoverable errors.

Changes

agent/agent_runtime_helpers.py — recover_with_credential_pool()

When pool is None and the error is a 429, parse the error body for quota-exhaustion keywords (quota, month, billing, insufficient_quota). If none found, set agent._transient_rate_limit = True to signal the retry loop rather than falling back.

agent/conversation_loop.py — eager-fallback section

Before activating fallback for a rate-limit error, check agent._transient_rate_limit. When set, consume the flag and retry the primary with exponential backoff (2, 4, 8... capped at 30s) instead of falling back. Quota-exhaustion 429s fall back immediately as before.

agent/chat_completion_helpers.py — try_activate_fallback()

Make the _rate_limited_until cooldown configurable via agent._rate_limit_cooldown_seconds (default 60). Users who want faster recovery on rate limits may set this lower.

Testing

  • Transient 429: logged as "Rate limited (transient) - retrying primary in Ns...", primary retried with backoff
  • Quota-exhaustion 429: logged as "Rate limited - switching to fallback provider...", fallback activates immediately
  • No credential pool: works without one (the common single-key setup)
  • Existing pool-based recovery: unchanged

The changes are additive — no existing behavior is modified unless a transient 429 is detected.

Previously, any 429 from the primary provider immediately triggered
fallback activation when no credential pool was configured. This caused
unnecessary provider switches on transient rate limits (e.g. "high
demand" / cluster-level throttling) while providing no benefit for
genuine quota exhaustion.

Changes in agent/agent_runtime_helpers.py:
  - In recover_with_credential_pool(), when pool is None and a 429 has
    no quota-exhaustion keywords in its body, set agent._transient_rate_limit
    instead of returning immediately — signals the loop to retry primary
    with backoff rather than falling back.

Changes in agent/conversation_loop.py:
  - In the eager-fallback section (rate-limit path), check the
    _transient_rate_limit flag. When set, consume it and retry the
    primary with exponential backoff (2, 4, 8... capped at 30s) instead
    of activating fallback. Quota-exhaustion 429s still fall back
    immediately.

Changes in agent/chat_completion_helpers.py:
  - Make the _rate_limited_until cooldown configurable via
    agent._rate_limit_cooldown_seconds (default 60). Users who want
    faster primary recovery on rate limits may set this lower; those
    who prefer a longer backoff may set it higher.

Closes #...
@alt-glitch alt-glitch added type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 24, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #46912 — both add "retry primary on transient 429 before falling back" with the same exponential-backoff mechanism in agent/conversation_loop.py. #46912 is the earlier open PR and gates the behavior behind an opt-in knob. Also related to the broader failover cluster: #50228 (transport-error fallback), #41590 (usage-limit routing). Maintainer to pick the canonical one.

…exhausted

When transient rate-limit retries consumed all api_max_retries, the
while loop exited without ever attempting fallback, producing 'no reply'
even though a fallback provider was configured.

Fix: replace unconditional 'continue' with a budget guard. Only retry
the primary while retry_count < max_retries; when exhausted, fall through
to the fallback activation path below.
@zdjackzdg
zdjackzdg requested a review from a team June 25, 2026 01:46

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the single-key 429 fallback path; the current main flow still switches immediately when the pool cannot recover (agent/conversation_loop.py:3162-3234).

Problems

  • The cooldown knob is not wired: the diff reads agent._rate_limit_cooldown_seconds, but current initialization only loads agent.api_max_retries (agent/agent_init.py:1519-1527), and no configured source initializes the new attribute.
  • The new four-keyword quota check duplicates and narrows the canonical classification. Existing billing patterns include "insufficient credits", "credit balance", and "credits exhausted" (agent/error_classifier.py:103-124), which this branch would treat as transient.
  • Direct time.sleep() bypasses the normal rate-limit retry path's Retry-After handling and interrupt polling (agent/conversation_loop.py:4121-4199).
  • Please remove the unrelated package-lock.json version change and tinker-atropos gitlink, and add regression coverage for both classifications and fallback after retry-budget exhaustion.

Suggested changes

  • Rework this around the existing classifier and retry scheduler instead of a per-agent flag and second keyword classifier.

Automated hermes-sweeper review.

# Cooldown before the next primary restore attempt.
# Default 60s; users may tune via fallback.rate_limit_cooldown_seconds
# in config.yaml to favour faster recovery or longer backoff.
_cooldown = 60

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This attribute is never initialized from config in this PR. Current initialization only loads agent.api_max_retries (agent/agent_init.py:1519-1527), so fallback.rate_limit_cooldown_seconds cannot affect this value; wire the documented config through initialization or remove the claimed knob.

f"retrying primary in {_delay}s..."
)
agent._flush_status_buffer()
time.sleep(_delay)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not sleep directly here. The normal retry path honors Retry-After and polls _interrupt_requested every 200ms (agent/conversation_loop.py:4121-4199); this bypass would make a transient-rate-limit wait uninterruptible and discard provider retry guidance.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026

This branch has not been deployed

No deployments
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 P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants