Skip to content

fix(agent): add jittered backoff to empty-response retries - #35296

Closed
arimu1 wants to merge 1 commit into
NousResearch:mainfrom
arimu1:fix/empty-response-retry-backoff
Closed

fix(agent): add jittered backoff to empty-response retries#35296
arimu1 wants to merge 1 commit into
NousResearch:mainfrom
arimu1:fix/empty-response-retry-backoff

Conversation

@arimu1

@arimu1 arimu1 commented May 30, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Empty-response retries fired back-to-back with no delay. When a model returns an empty response, the agent retried up to 3 times immediately — all within <1 second — then switched to the fallback provider and did the same. Worst case: 6 rapid-fire API calls with no wait.

This applies the same jittered_backoff() already used for rate-limit and API-error retries, so the agent waits before each empty-response retry. The status buffer message now shows the countdown so the user knows what's happening.

Related Issue

Fixes #35230

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/conversation_loop.py (~line 4112): added jittered_backoff(retry_count, base_delay=5.0, max_delay=60.0) and time.sleep(wait_time) before continue in the empty-response retry path. Updated log and status messages to include the wait time.

How to Test

  1. Use a model that occasionally returns empty responses (e.g. a local model with a low token budget, or GLM-4.5-Air under load)
  2. Trigger an empty response
  3. Before: retries 1/3, 2/3, 3/3 appear and vanish instantly in the status bar
  4. After: each retry shows ⚠️ Empty response from model — retrying (1/3) in 5s and waits before the next attempt

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(agent):)
  • I searched for existing PRs — none found for this specific path
  • My PR contains only changes related to this fix
  • I've run pytest tests/ -q — no test suite available in my local env
  • I've added tests — existing retry logic has no unit tests to pattern against
  • Tested on: macOS 15.5

Documentation & Housekeeping

  • N/A — no config keys, docs, or architecture changes
  • Cross-platform: time.sleep() and jittered_backoff() are platform-agnostic

Screenshots / Logs

Before — all 3 retries fire within 1 second:

⚠️ Empty response from model — retrying (1/3)
⚠️ Empty response from model — retrying (2/3)
⚠️ Empty response from model — retrying (3/3)

After — each retry waits with jitter (5s base, 60s max):

⚠️ Empty response from model — retrying (1/3) in 5s
⚠️ Empty response from model — retrying (2/3) in 10s
⚠️ Empty response from model — retrying (3/3) in 21s

@alt-glitch alt-glitch added type/perf Performance improvement or optimization comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have labels May 30, 2026
@arimu1

arimu1 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Gentle bump — this has been sitting quiet for ~11 days. It's a small, isolated diff (1 file, 9 lines: jittered backoff + time.sleep() before the empty-response retry continue) fixing the back-to-back retry burst described in #35230. Still merges cleanly against current main. Happy to rebase or adjust the backoff parameters if the maintainers prefer different defaults.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating a real retry burst. Remote main still takes the empty-response branch at agent/conversation_loop.py:4997-5008 with an immediate continue, so the underlying problem remains.

Problems

  • agent/conversation_loop.py:4128 adds a one-shot time.sleep(wait_time). Current main's other retry waits deliberately poll agent._interrupt_requested every 200 ms and touch activity while waiting at agent/conversation_loop.py:4173-4198; this sleep would defer that handling for the full empty-response wait.
  • The existing empty-response test checks retry count only (tests/run_agent/test_run_agent.py:4289-4308), and the shared fixture forces jittered_backoff() to 0.0 (tests/run_agent/conftest.py:34-44). The new delay/status path is therefore untested.

Suggested changes

  • Apply the change at the current empty-response branch and use the existing interrupt-aware incremental wait pattern instead of a single blocking sleep.
  • Add a deterministic regression test for the backoff/status and interruption behavior.

Automated hermes-sweeper review.

Comment thread agent/conversation_loop.py Outdated
)
time.sleep(wait_time)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please avoid a one-shot sleep here. Current retry backoffs poll _interrupt_requested every 200 ms and touch activity (agent/conversation_loop.py:4173-4198 on main); use that interrupt-aware wait pattern so /stop is not deferred for the full empty-response delay.

@arimu1

arimu1 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

Gentle bump — this has been quiet since the June 10 ping. Still a small, isolated change (jittered backoff + time.sleep() before the empty-response retry continue) fixing back-to-back empty-response retries that burn API quota.

Rebased/mergeable against current main as of the recent status check. Happy to adjust if you'd prefer a different backoff schedule.

@arimu1
arimu1 force-pushed the fix/empty-response-retry-backoff branch from 7399f41 to a8e6a3f Compare July 18, 2026 23:05
@arimu1

arimu1 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @teknium1 — addressed in the latest push.

The empty-response retry wait is now interrupt-aware: it polls agent._interrupt_requested in small increments (same pattern as the other retry backoffs) so /stop is not deferred for the full delay, and added regression coverage for backoff + early abort on interrupt.

@arimu1
arimu1 force-pushed the fix/empty-response-retry-backoff branch 2 times, most recently from bad692c to 567b75a Compare July 23, 2026 05:43
Empty content retries previously fired back-to-back with no delay,
wasting up to 3 rapid API calls, and could not be cancelled mid-wait.
Apply the same jittered_backoff() already used for rate-limit and
API-error retries, sleeping in small increments so a user interrupt
aborts the wait instead of blocking until it elapses.

Fixes NousResearch#35230
@arimu1
arimu1 force-pushed the fix/empty-response-retry-backoff branch from 567b75a to b58d08e Compare July 24, 2026 07:17
@arimu1

arimu1 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Rebuilt this branch from scratch on current main (was 359 commits behind and conflicting). The old second commit accidentally ran black over the whole file alongside the functional change, which is why it was so large and why it conflicted — that's squashed out here. This now contains only the intended fix: jittered backoff before each empty-response retry (matching the existing jittered_backoff() pattern used for rate-limit/API-error retries), with the wait made interrupt-aware so a user cancel aborts it instead of blocking. Diff is +109/-3, no reformatting. Tests added in tests/run_agent/test_run_agent.py (backoff status message + interrupt-during-wait); full tests/run_agent/test_run_agent.py suite (446 tests) passes via scripts/run_tests.sh.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Thanks @arimu1 — clean work: reuses the shared jittered_backoff util, deterministic tests, interrupt path preserved. Salvaged with your authorship in #77608; the only fold is test-only (a fake clock — the backoff-status test was busy-spinning 7.5 wall-clock seconds because the loop gates on real time.time()). Closing in favor of the salvage.

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 P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/perf Performance improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Enhancement] Empty response retries have no backoff delay — immediate retry wastes API calls

4 participants