fix(agent): make empty-response retries configurable with jittered backoff (Closes #74916) - #74927
fix(agent): make empty-response retries configurable with jittered backoff (Closes #74916)#74927Synxneuos wants to merge 1 commit into
Conversation
0831591 to
a71d42b
Compare
Related to #74917, not a duplicate: both add configurable empty-response retries and backoff, but they choose different default retry policy and implementation details. Please consolidate on the intended behavior. |
|
Thanks for the triage check @alt-glitch! We have consolidated PR #74927 on the intended behavior matching #74916:
|
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing a live recovery gap. Current main still hard-codes the empty-response retry limit and immediately continues at agent/conversation_loop.py:6498-6509, so the premise remains valid.
Problems
- The proposed interrupt branch at
agent/conversation_loop.py:3626callsclear_interrupt()and returns. Current retry backoffs preserve pending redirects withclear_interrupt(preserve_redirect=True)and rebuild the turn (agent/conversation_loop.py:2564-2599); this needs the same handling or a user correction arriving during the wait is lost. - The selected default is inconsistent with the issue discussion: #74916 and the follow-up comment say
5, while the submitted value is3athermes_cli/config.py:537. - GitHub reports this PR as conflicting, and the current branch now has buffered status/failover synchronization around the target block (
agent/conversation_loop.py:6498-6541).
Suggested changes
- Salvage the delay and setting into the current block while preserving its status, failover, and redirect semantics.
- Resolve the default policy, align docs/config/tests, and add a redirect-during-backoff regression test.
Automated hermes-sweeper review.
| force=True, | ||
| ) | ||
| agent._persist_session(messages, conversation_history) | ||
| agent.clear_interrupt() |
There was a problem hiding this comment.
Current main preserves a pending redirect during retry backoff via clear_interrupt(preserve_redirect=True) and restarts from the redirected messages (agent/conversation_loop.py:2564-2599). Clearing and returning here would drop a user correction that arrives during this wait; port that redirect-preserving pattern.
There was a problem hiding this comment.
We ported this exact pattern to the empty-response retry backoff sleep loop in agent/conversation_loop.py. Now, if a user steering redirect lands during the backoff (getattr(agent, '_pending_steer', None) is not None), the wait loop refunds the iteration budget (agent.iteration_budget.refund()), calls agent.clear_interrupt(preserve_redirect=True), and breaks out to re-enter the loop so the user correction is preserved and applied on the next iteration.
We also added a regression test (test_empty_response_retries_preserves_redirect_during_backoff) in tests/run_agent/test_empty_response_retries.py to cover this behavior.
| @@ -532,6 +532,9 @@ def _ensure_hermes_home_managed(home: Path): | |||
| # on flaky primaries; raise it if you prefer to tolerate longer | |||
| # provider hiccups on a single provider. | |||
| "api_max_retries": 3, | |||
| # Max retries when a model returns an empty response (no content/reasoning) | |||
| # before switching to fallback provider or finishing. 0 = no retries. | |||
| "empty_response_retries": 3, | |||
There was a problem hiding this comment.
Please resolve the selected default before salvage: #74916 proposes 5 retries and the follow-up says this PR was changed to 5, but this submitted value remains 3. Align the final decision across config, docs, and tests.
There was a problem hiding this comment.
Resolved and aligned! The default has been updated to 5 (agent.empty_response_retries: 5) consistently across all layers:
DEFAULT_CONFIGinhermes_cli/config.pyagent/agent_init.pydefault (_empty_retries = int(_agent_section.get("empty_response_retries", 5)))cli-config.yaml.examplewebsite/docs/user-guide/configuration.md- Test assertions in
tests/run_agent/test_empty_response_retries.py
…ckoff and redirect preservation - Add agent.empty_response_retries configuration setting to config.yaml and DEFAULT_CONFIG (default 5), allowing users to configure or disable (0) empty-response retries before fallback activation. - Implement jittered backoff (base_delay=3.0, max_delay=15.0) with interruptible sleep loop during empty-response retries in run_conversation(). - Preserve pending user steering redirects (clear_interrupt(preserve_redirect=True)) during retry backoff wait so user corrections land on the next iteration. - Add test suite in tests/run_agent/test_empty_response_retries.py including redirect preservation coverage. Closes NousResearch#74916
a71d42b to
2c03560
Compare
|
Thanks @teknium1 for the thorough review and guidance! We have fully addressed all three points in the updated commit (
All unit tests passing cleanly! Ready for final review and merge. |
SummarySeven PRs address or reference this issue complex. The diffs cover two related causes: immediate empty-response retries and the hardcoded retry ceiling; #74596 addresses only adjacent transcript persistence, not retry timing or configurability. Related pull requests
Duplicates#38002 is a closed duplicate of #35296. #74950 duplicates the backoff portion of #35296, #74917, and #74927; #58706 duplicates the configurable-retry portion of the combined fixes. #74917 and #74927 are competing implementations of the same complete #74916 fix. Suggested consolidationauthor action: rebase #74927 onto main, or split out the part that can merge, specifically resolving the [contributor:7624 commits] keep_open review on the redirect-preservation lifecycle and supplying a non-zero-delay regression test; do not merge over that blocking review. Keep #74917 open as the competing complete implementation, retain #58706 with a salvage path for its focused configurability tests, keep closed #35296 closed in favor of #77608, and close #38002 and #74950 as duplicates; leave #74596 closed under the maintainer-bot's implemented-on-main verdict. Complex graphflowchart TD
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I35230(["issue #35230 (closed)"])
I58670(["issue #58670 (open)"])
I74916(["issue #74916 (open)"])
subgraph Dup35296 ["PRs duplicating each other"]
P35296["PR #35296 (closed)"]
P38002["PR #38002 (closed)"]
P74917["PR #74917 (open)"]
P74927["PR #74927 (open)"]
P74950["PR #74950 (open)"]
end
P74927 -->|fixes| I35230
P74927 -->|fixes| I58670
P74927 -->|best fix| I74916
class I35230 closed
class I58670 open
class I74916 open
class P35296 closed
class P38002 closed
class P74917 open
class P74927 open
class P74950 open
class P35296 best
class P74917 best
class P74927 best
class P74927 target
click I35230 "https://github.com/NousResearch/hermes-agent/issues/35230"
click I58670 "https://github.com/NousResearch/hermes-agent/issues/58670"
click I74916 "https://github.com/NousResearch/hermes-agent/issues/74916"
click P35296 "https://github.com/NousResearch/hermes-agent/pull/35296"
click P38002 "https://github.com/NousResearch/hermes-agent/pull/38002"
click P74917 "https://github.com/NousResearch/hermes-agent/pull/74917"
click P74927 "https://github.com/NousResearch/hermes-agent/pull/74927"
click P74950 "https://github.com/NousResearch/hermes-agent/pull/74950"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 7 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 56 kB of PR diffs, 18 kB of issue/PR text, 22 kB of discussion (37 comments), 25 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
Resolves #74916.
When an LLM returns an empty response (no content or reasoning),
run_conversationpreviously retried up to a hardcoded 3 times immediately with zero backoff before switching to the fallback provider. For transiently-empty model API responses, three immediate retries hit the same stall state instantly, triggering unnecessary failover to weaker models.Changes Made
agent.empty_response_retriesconfiguration setting inconfig.yaml(default3, overridable,0= skip retries and go straight to fallback attempt).base_delay=3.0,max_delay=15.0) with an interruptible sleep loop before re-firing the API call on empty responses.tests/run_agent/test_empty_response_retries.py(5/5 tests passing).Verification
All 10 tests passing cleanly.