Skip to content

UN-4223 [FIX] Cap a streamed LLM completion at a total time limit - #2314

Open
johnyrahul wants to merge 2 commits into
mainfrom
UN-4223-llm-stream-deadline
Open

johnyrahul wants to merge 2 commits into
mainfrom
UN-4223-llm-stream-deadline

Conversation

@johnyrahul

Copy link
Copy Markdown
Contributor

What

  • LLM.complete() now stops a streamed completion once it has run for LLM_STREAM_MAX_SECONDS in total (default 1800 s, retries included) and raises LLMError naming the adapter.
  • collect_with_retry gains an optional max_seconds that raises DeadlineExceededError once it passes. The stream is closed and not replayed.

Why

UN-4223: on 2026-10-01, 10-02 and 10-03, single vertex_ai/gemini-3.1-flash-lite calls in unstract-worker-pg-executor ran for 2+ hours. Pod liveness eventually took whole executor pods offline, which paged a P0 on the US Checkly check.

Prod logs for the two hung processes on 10-02 (5hgn2/p24, rm772/p12) show what was happening:

  • The last line each logged was the start of the Gemini call. Nothing followed until the container restarted about 4 h later: no Retry N/M, no timeout, no error. Those lines are logged at WARNING/ERROR and do appear for other failures in the same pods during that window.
  • Even the 6000 s read timeout LiteLLM actually applied on this path (see UN-4223 [FIX] Make streamed Gemini calls honour the adapter timeout #2313) never fired. So bytes kept arriving.
  • The Incident Agent reported pod memory climbing to around 9–10 GB. That matches collect_with_retry holding an ever-growing list of chunks.
  • Just before the hang, each call logged LiteLLM's own warning: "Setting temperature < 1.0 for Gemini 3 models … can cause infinite loops".

A read timeout resets on every chunk, so no timeout setting can stop a model that keeps generating. A total wall-clock limit can, and it works for every provider, not just Gemini.

How

  • collect_with_retry(..., max_seconds=...):
    • The deadline is taken once, before the first attempt, so it covers retries too.
    • It is checked as each chunk arrives. If it has passed, the stream is closed and DeadlineExceededError is raised.
    • That error is never retried, whether or not content had started, because a second attempt would hit the same wall.
    • A failure before content that happens after the deadline has passed is raised as-is, with no new attempt.
  • LLM._complete_via_stream passes max_seconds=_stream_max_seconds(), which reads LLM_STREAM_MAX_SECONDS:
    • unset or empty → 1800
    • a number → that value
    • 0 or negative → disabled
    • not a number → 1800, with a one-time warning
  • Why 1800 s:
    • It clears the longest legitimate generations we've seen. The streaming comment in llm.py cites Anthropic replies of around 16 minutes.
    • It stays well under the executor's 1 h EXECUTOR_RESULT_TIMEOUT. The caller gets a real error instead of a timeout on a job that keeps running.
  • What this doesn't bound: the check runs per chunk, so a stream that goes silent is still bounded by the read timeout, not this deadline. That is 900 s on Anthropic, and the adapter's timeout on Gemini once UN-4223 [FIX] Make streamed Gemini calls honour the adapter timeout #2313 lands.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • Behaviour change, intended: a streamed completion still running after 30 minutes now fails instead of continuing. That is longer than any legitimate generation we've seen. If one ever needs more, raise LLM_STREAM_MAX_SECONDS, or set it to 0 to turn the limit off.
  • Adapters opted out of streaming (enable_streaming=false), acomplete() and stream_complete() are unchanged. stream_complete() yields chunks to its caller as they arrive, so the caller owns its pacing; it has no production callers today.
  • collect_with_retry callers that don't pass max_seconds behave exactly as before. The existing tests are unchanged and pass.

Database Migrations

  • None.

Env Config

  • New, optional: LLM_STREAM_MAX_SECONDS (default 1800, 0 disables). Documented in workers/sample.env next to EXECUTOR_RESULT_TIMEOUT. Nothing needs to be set to get the default.

Relevant Docs

  • workers/sample.env

Related Issues or PRs

  • UN-4223 (prod outages), UN-4179 (same mechanism on staging, Anthropic).
  • UN-4223 [FIX] Make streamed Gemini calls honour the adapter timeout #2313: makes streamed Gemini calls honour the adapter's timeout, which bounds the silent stream case this PR doesn't cover.
  • Still open on UN-4223 (separate work): the PG supervisor kills only a stuck child instead of the pod's health following its oldest child; and the chart's executor terminationGracePeriodSeconds.

Dependencies Versions

  • None.

Notes on Testing

  • collect_with_retry (3 new cases):
    • An endless stream is stopped at the deadline, both with and without content. The generator is closed and there is no retry.
    • No new attempt is made once the deadline has passed.
    • Normal retries still happen within the deadline.
  • LLM.complete() (new tests/test_llm_stream_deadline.py):
    • An endless stream of real LiteLLM chunks fails with LLMError at the limit, after exactly one call.
    • The env var is parsed correctly for each case: unset, value, 0, negative, invalid with warning.
  • Mutation-verified: with max_seconds not passed from LLM, the endless-stream test hangs (killed by timeout after 20 s), which is the production behaviour. With it, the test fails fast at the limit.
  • Full sdk1 suite: 732 passed. ruff 0.3.4 (the pre-commit pin) is clean.

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

🤖 Generated with Claude Code

A read timeout resets on every chunk, so it never stops a model that
keeps generating. In production single Gemini calls streamed for 2+
hours with no timeout, error or retry logged, and memory grew to ~10 GB
until pod liveness took whole executor pods offline.

collect_with_retry takes an optional max_seconds that caps the whole
call, retries included, and raises DeadlineExceededError when it passes;
the stream is closed and not replayed. LLM.complete() applies it to
every streamed completion: LLM_STREAM_MAX_SECONDS, default 1800 s, 0
disables it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

via Greptile

RetriggerConfidence Score: 5/5

[Medium risk] Adds a total time limit to streamed LLM completions.

The latest changes appear safe to merge, though a stream that finishes after the deadline can still return successfully.

Fix All in Claude CodeFindings

  1. P2 Clean finish skips deadline ▶
Fix with agent prompt
### Issue 1
unstract/sdk1/src/unstract/sdk1/utils/retry_utils.py:undefined-328
If the last chunk arrives within the limit but the stream finishes after it, this return path never checks the deadline. `LLM.complete()` can then return a successful response despite exceeding the stated total limit.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary

The PR adds a configurable total-time limit to streamed LLM.complete() calls, including retries, and adds coverage for runaway streams and invalid configuration.

  • The latest changes reject non-finite limit values and avoid a retry when its planned backoff would pass the deadline.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[LLM.complete] --> B[Read stream limit]
  B --> C[Collect provider chunks]
  C --> D{Deadline passed on chunk?}
  D -->|Yes| E[Close stream and raise]
  D -->|No| F{Stream finished?}
  F -->|Yes| G[Build response]
  F -->|No| C
Loading

Reviews (2) · Last reviewed commit: "UN-4223 [FIX] Don't retry past the strea..."

f"stream exceeded its {max_seconds:g}s total time "
f"limit after {len(items)} chunk(s)"
)
return items

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.

P2 Clean finish skips deadline

If the last chunk arrives within the limit but the stream finishes after it, this return path never checks the deadline. LLM.complete() can then return a successful response despite exceeding the stated total limit.

Knowledge Base Used: SDK LLM runtime

Prompt To Fix With AI
This is a comment left during a code review.
Path: unstract/sdk1/src/unstract/sdk1/utils/retry_utils.py
Line: 328

Comment:
**Clean finish skips deadline**

If the last chunk arrives within the limit but the stream finishes after it, this return path never checks the deadline. `LLM.complete()` can then return a successful response despite exceeding the stated total limit.

**Knowledge Base Used:** [SDK LLM runtime](https://app.greptile.com/zipstack/-/custom-context/knowledge-base/zipstack/unstract/-/docs/sdk1-llm-runtime.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

Comment thread unstract/sdk1/src/unstract/sdk1/llm.py
@greptile-apps

This comment has been minimized.

… limits

A retryable failure just before the limit could sleep through the
backoff and then send a new request after the deadline. Raise instead
when the backoff would end past it.

LLM_STREAM_MAX_SECONDS values "inf" and "nan" parse as floats and would
quietly switch the limit off; treat them like any other invalid value
and fall back to the default with a warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Oct 5, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
✅ e2e-api-deployment e2e 3 0 0 0 6.8
✅ e2e-coowners e2e 1 0 0 0 1.3
✅ e2e-etl e2e 1 0 0 0 14.3
✅ e2e-login e2e 2 0 0 0 1.2
✅ e2e-prompt-studio e2e 1 0 0 0 7.6
✅ e2e-smoke e2e 2 0 0 0 1.1
✅ e2e-workflow e2e 1 0 0 0 14.4
✅ frontend unit 620 0 0 0 19.1
✅ integration-backend integration 603 0 0 26 58.7
✅ integration-connectors integration 1 0 0 7 8.2
✅ integration-workers integration 164 0 0 1 39.4
❌ ui e2e 0 1 0 0 0.0
✅ unit-backend unit 1422 0 0 1 46.9
✅ unit-connectors unit 72 0 0 0 9.8
✅ unit-core unit 276 0 0 0 3.2
✅ unit-platform-service unit 15 0 0 0 2.6
✅ unit-rig unit 120 0 0 0 4.5
✅ unit-runner unit 10 0 0 0 2.9
✅ unit-sdk1 unit 734 0 0 0 29.4
✅ unit-workers unit 1383 0 0 1 117.6
TOTAL 5431 1 0 36 389.1

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • platform-key-whoami — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant