fix(agent): add exponential backoff and chat_completion retry to stream API calls (#60029) - #60299
fix(agent): add exponential backoff and chat_completion retry to stream API calls (#60029)#60299webtecnica wants to merge 1 commit into
Conversation
…am API calls _interruptible_streaming_api_call had no backoff (immediate retry), only handled streaming endpoint, and leaked state across retries. Added non-streaming chat_completion handler, exponential backoff (1s/2s/4s/8s/16s capped at 30s), and per-attempt state cleanup. Closes NousResearch#60029
Related: competes with #60031 for the same issue #60029. This PR (#60299) is a clean single-file |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the retry/backoff work.
Problems
- The new non-streaming branch at
agent/chat_completion_helpers.py:2555is not the normal non-streaming route on current main:agent/conversation_loop.py:1358-1362sends non-streaming calls to_interruptible_api_call, whose shared dispatch is_dispatch_nonstreaming_api_requestatagent/chat_completion_helpers.py:240-293. time.sleep()atagent/chat_completion_helpers.py:2550cannot observe an interrupt that arrives during the wait. Current regression coverage requires retries to stop immediately (tests/run_agent/test_stream_interrupt_retry.py:45-77).- #60029 also requires chat-completions parse-error classification and Anthropic stale-close retry handling. This diff does not modify the gate that currently excludes chat-completions at
run_agent.py:1130-1137.
Suggested changes
- Rework the backoff around the current retry continuations with a cancellation-aware wait, remove the duplicate non-streaming path, and add deterministic retry/cancellation tests. Either include the remaining #60029 recovery cases or narrow the issue linkage.
Automated hermes-sweeper review.
| if _stream_attempt > 0: | ||
| _backoff = min(2 ** (_stream_attempt - 1), 30) | ||
| import time as _time | ||
| _time.sleep(_backoff) |
There was a problem hiding this comment.
time.sleep() cannot observe /stop after the pre-attempt check at line 2540. Use a cancellation-aware wait so an interrupt during backoff cannot leave the retry worker sleeping until the full delay expires; current tests require retry interruption to be immediate.
| if agent.api_mode == "anthropic_messages": | ||
| agent._try_refresh_anthropic_client_credentials() | ||
| result["response"] = _call_anthropic() | ||
| elif agent.api_mode == "chat_completions" and not api_kwargs.get("stream", True): |
There was a problem hiding this comment.
Current non-streaming calls do not normally enter this helper: conversation_loop dispatches them to _interruptible_api_call, which uses _dispatch_nonstreaming_api_request. Please remove this parallel handler and place any needed non-streaming behavior on that established path.
Fixes three defects in _interruptible_streaming_api_call: (1) adds exponential backoff (1s/2s/4s/8s/16s capped at 30s), (2) adds non-streaming chat_completion handler, (3) adds per-attempt state cleanup. Closes #60029