fix(llm): add per-turn model request timeout so stalled streams fail fast and retry#802
Merged
Merged
Conversation
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Contributor
Greptile SummaryThis PR adds per-request timeouts so stalled model streams fail and retry. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(llm): use httpx.Timeout read-inactiv..." | Re-trigger Greptile |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Agent-loop model calls had no request timeout, so a stalled model stream (one that stops sending bytes but never errors) would hang the agent indefinitely — no exception, no retry, nothing to recover it. In a multi-agent run this silently removes an agent from the graph and can leave the parent waiting on ghosts until end-of-run cleanup.
The pre-SDK engine did not have this gap:
LLMConfig.timeout(LLM_TIMEOUT, default 300s) wrapped both stream establishment and each streamed chunk inasyncio.wait_for(a per-chunk inactivity watchdog), and a trip was retried. In the v1 SDK path that timeout survived only as a startup warm-up ping — the actual per-turn model settings set no timeout at all.This restores that behavior by forwarding
LLM_TIMEOUTas a per-request read-inactivity timeout on every agent/dedupe model call. A stalled stream tripsread, surfaces as a timeout, and is retried by the existingDEFAULT_MODEL_RETRYpolicy (which already retriesis_timeout/ network errors) instead of hanging. A healthy long stream that keeps emitting tokens never trips it.What changed
New helper (
config/models.py), forwarded viaModelSettings.extra_argsintoresponses.create/chat.completions.create/litellm.acompletion:core/inputs.pymake_model_settings(...)gainsrequest_timeoutand setsextra_args={"timeout": httpx.Timeout(...)}(survives the.resolve()merges for reasoning/tool_choice).core/runner.pypassesrequest_timeout=settings.llm.timeoutfor the agent loop.report/dedupe.pyapplies the same timeout to its one-shot model call.Default is unchanged behavior when
LLM_TIMEOUTis unset/0 (extra_args=None). No change to retry counts/backoff.Why
httpx.Timeoutand not a scalar: a scalar can be interpreted as a total request deadline on some backends, which would kill a healthy long stream.readis a per-chunk inactivity window (httpx has no total-duration timeout), so this only ever fires on a genuinely stalled stream — matching pre-v1's per-chunkwait_for.Why it's retried, not fatal
The SDK normalizes an OpenAI/litellm timeout as
is_timeout=True, andDEFAULT_MODEL_RETRY'snetwork_error()policy already retriesis_network_error or is_timeout. So a stalled turn now: tripsreadatLLM_TIMEOUT→ retried up to 5× with the existing backoff → recovers, instead of wedging.Tests
test_models.py: helper returns anhttpx.Timeoutwithread == value(and cappedconnect) for positive values,NoneforNone/0/negative.test_inputs.py:make_model_settingssets/omits the timeout and it survives the reasoning.resolve()merge.test_model_retry.py:is_timeout/is_network_errorare retried byDEFAULT_MODEL_RETRY.OpenAIResponsesModelthat thehttpx.Timeout(read=300, connect=30) is forwarded intoresponses.create(...).test_runner_root_prompt.py(missingruntime/session_manageron the test's fake settings) that fail identically onmain.Note:
httpxis imported directly but is already a guaranteed, locked transitive dependency of the pinnedopenai/litellm; it's intentionally not added topyproject/uv.lockto avoid an unrelated full-lockfile reformat. The mypy pre-commit hook was skipped for these commits only, due to 2 pre-existingattr-definederrors intest_runner_root_prompt.py(runner.session_manager) that exist onmainand are unrelated to this change.Link to Devin session: https://app.devin.ai/sessions/dad023e379e942f287bcf6822463b7a4
Requested by: @0xallam