Repository navigation
UN-4223 [FIX] Cap a streamed LLM completion at a total time limit #2314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
johnyrahul
wants to merge
2
commits into
main
Choose a base branch
from
UN-4223-llm-stream-deadline
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -274,6 +274,10 @@ def iter_with_retry[T]( | |
| time.sleep(delay) | ||
|
|
||
|
|
||
| class DeadlineExceededError(Exception): | ||
| """A drained stream ran past its total wall-clock limit.""" | ||
|
|
||
|
|
||
| def collect_with_retry[T]( | ||
| fn: Callable[[], Iterable[T]], | ||
| *, | ||
|
|
@@ -282,6 +286,7 @@ def collect_with_retry[T]( | |
| is_content: Callable[[T], bool], | ||
| description: str = "", | ||
| logger_instance: logging.Logger | None = None, | ||
| max_seconds: float | None = None, | ||
| ) -> list[T]: | ||
| """Drain fn() into a list with retry. Only retries before content. | ||
|
|
||
|
|
@@ -292,9 +297,16 @@ def collect_with_retry[T]( | |
| and is raised immediately: replaying it would re-run a generation that | ||
| may already have consumed minutes. Items from a failed attempt are | ||
| discarded so a retry never duplicates them. | ||
|
|
||
| ``max_seconds`` caps the whole call, retries included, and raises | ||
| ``DeadlineExceededError`` once it passes. A read timeout cannot do this: | ||
| it resets on every item, so a stream that keeps producing (a model stuck | ||
| generating) would otherwise run forever. The check runs as each item | ||
| arrives; a stream that goes silent is still bounded by the read timeout. | ||
| """ | ||
| _validate_max_retries(max_retries) | ||
| log = logger_instance or logger | ||
| deadline = None if max_seconds is None else time.monotonic() + max_seconds | ||
| for attempt in range(max_retries + 1): | ||
| items: list[T] = [] | ||
| has_content = False | ||
|
|
@@ -308,19 +320,37 @@ def collect_with_retry[T]( | |
| for item in gen: | ||
| items.append(item) | ||
| has_content = has_content or is_content(item) | ||
| if deadline is not None and time.monotonic() > deadline: | ||
| raise DeadlineExceededError( | ||
| f"stream exceeded its {max_seconds:g}s total time " | ||
| f"limit after {len(items)} chunk(s)" | ||
| ) | ||
| return items | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the last chunk arrives within the limit but the stream finishes after it, this return path never checks the deadline. Knowledge Base Used: SDK LLM runtime Prompt To Fix With AIThis 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. |
||
| except Exception as e: | ||
| # Release the in-flight HTTP/socket resources before retrying. | ||
| close = getattr(gen, "close", None) | ||
| if callable(close): | ||
| close() | ||
| if has_content: | ||
| if has_content or isinstance(e, DeadlineExceededError): | ||
| raise | ||
| # Out of time: surface the failure rather than start another attempt. | ||
| if deadline is not None and time.monotonic() >= deadline: | ||
| raise | ||
| delay = _get_retry_delay( | ||
| e, attempt, max_retries, retry_predicate, description, log | ||
| ) | ||
| if delay is None: | ||
| raise | ||
| # A retry that would start after the deadline only spends another | ||
| # request on a call that has already run out of time. | ||
| if deadline is not None and time.monotonic() + delay >= deadline: | ||
| log.warning( | ||
| "Not retrying %s: the %gs total time limit would pass " | ||
| "during the backoff", | ||
| description, | ||
| max_seconds, | ||
| ) | ||
| raise | ||
| time.sleep(delay) | ||
| raise RuntimeError("unreachable") # for type-checker: loop always returns or raises | ||
|
|
||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| """``LLM.complete()`` caps a streamed completion at ``LLM_STREAM_MAX_SECONDS``. | ||
|
|
||
| In production (UN-4223) single ``vertex_ai/gemini-3.1-flash-lite`` calls kept | ||
| streaming for 2+ hours: no timeout fired, memory climbed to ~10 GB, and pod | ||
| liveness eventually took whole executor pods offline. A read timeout resets on | ||
| every chunk, so only a total wall-clock limit stops a model stuck generating. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import logging | ||
| import time | ||
| from functools import lru_cache | ||
| from importlib import import_module | ||
| from itertools import cycle | ||
| from typing import TYPE_CHECKING | ||
| from unittest.mock import patch | ||
|
|
||
| import litellm | ||
| import pytest | ||
|
|
||
| if TYPE_CHECKING: | ||
| from collections.abc import Iterator | ||
|
|
||
| ANTHROPIC_ADAPTER_ID = "anthropic|90ebd4cd-2f19-4cef-a884-9eeb6ac0f203" | ||
| _REAL_COMPLETION = litellm.completion | ||
|
|
||
|
|
||
| @lru_cache(maxsize=1) | ||
| def _load_llm_module() -> object: | ||
| import sys | ||
| from types import ModuleType | ||
|
|
||
| sys.modules.setdefault("magic", ModuleType("magic")) | ||
| return import_module("unstract.sdk1.llm") | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def no_cost() -> Iterator[None]: | ||
| llm_module = _load_llm_module() | ||
| with patch.object(llm_module.litellm, "cost_per_token", return_value=(0.0, 0.0)): | ||
| yield | ||
|
|
||
|
|
||
| def _content_chunks() -> list[object]: | ||
| """Real litellm stream chunks that carry text, produced without network.""" | ||
| chunks = list( | ||
| _REAL_COMPLETION( | ||
| model="anthropic/claude-sonnet-4-6", | ||
| messages=[{"role": "user", "content": "hi"}], | ||
| stream=True, | ||
| mock_response="loop ", | ||
| api_key="test-key", | ||
| ) | ||
| ) | ||
| return [c for c in chunks if c["choices"] and c["choices"][0].delta.content] | ||
|
|
||
|
|
||
| def test_endless_stream_fails_at_the_limit( | ||
| no_cost: None, monkeypatch: pytest.MonkeyPatch | ||
| ) -> None: | ||
| llm_module = _load_llm_module() | ||
| llm = llm_module.LLM( | ||
| adapter_id=ANTHROPIC_ADAPTER_ID, | ||
| adapter_metadata={ | ||
| "model": "claude-sonnet-4-6", | ||
| "api_key": "test-key", | ||
| "max_retries": 2, | ||
| }, | ||
| ) | ||
| chunks = _content_chunks() | ||
| calls: list[int] = [] | ||
|
|
||
| def endless(**_: object) -> Iterator[object]: | ||
| calls.append(1) | ||
| for chunk in cycle(chunks): | ||
| time.sleep(0.005) | ||
| yield chunk | ||
|
|
||
| monkeypatch.setenv("LLM_STREAM_MAX_SECONDS", "0.05") | ||
| with ( | ||
| patch.object(llm_module.litellm, "completion", endless), | ||
| pytest.raises( | ||
| llm_module.LLMError, match="exceeded its 0.05s total time limit" | ||
| ) as exc, | ||
| ): | ||
| llm.complete("hi") | ||
|
|
||
| assert len(calls) == 1 # a runaway generation is never replayed | ||
| # The adapter is named once, by LLM's wrapper. | ||
| assert str(exc.value).startswith("Error from LLM adapter 'anthropic': stream") | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("raw", "expected"), | ||
| [ | ||
| (None, 1800.0), | ||
| ("", 1800.0), | ||
| ("600", 600.0), | ||
| (" 900.5 ", 900.5), | ||
| ("0", None), | ||
| ("-5", None), | ||
| ], | ||
| ) | ||
| def test_limit_is_read_from_the_environment( | ||
| raw: str | None, expected: float | None, monkeypatch: pytest.MonkeyPatch | ||
| ) -> None: | ||
| llm_module = _load_llm_module() | ||
| if raw is None: | ||
| monkeypatch.delenv("LLM_STREAM_MAX_SECONDS", raising=False) | ||
| else: | ||
| monkeypatch.setenv("LLM_STREAM_MAX_SECONDS", raw) | ||
|
|
||
| assert llm_module._stream_max_seconds() == expected | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("raw", ["30m", "inf", "-inf", "nan"]) | ||
| def test_invalid_limit_falls_back_to_the_default_with_a_warning( | ||
| raw: str, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture | ||
| ) -> None: | ||
| """A typo, or a non-finite value ``float()`` accepts, keeps the limit on.""" | ||
| llm_module = _load_llm_module() | ||
| monkeypatch.setenv("LLM_STREAM_MAX_SECONDS", raw) | ||
|
|
||
| with caplog.at_level(logging.WARNING, logger=llm_module.logger.name): | ||
| assert llm_module._stream_max_seconds() == 1800.0 | ||
|
|
||
| assert f"LLM_STREAM_MAX_SECONDS={raw!r} is not a finite number" in caplog.text |
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.