Repository navigation
fix(notes): stream LLM responses and time out only on a stall - #717
Merged
Merged
Conversation
The notes timeout capped the whole request, so a large chunk whose structured response took over 60s to generate timed out on every retry and came back ungrouped. Every provider now streams its response and aborts only when nothing arrives for the timeout. - ollama: stream NDJSON; an error object mid-stream, or a stream that ends without done, fails retryably - openai, openai-compatible, anthropic: stream through the SDKs; a stream that ends without a finish/stop reason fails retryably - the timeout error chains its abort, so the live test fails on a stall instead of skipping it as an outage - live test: entries carry PR bodies at the 2 KB cap, at production timeouts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Standing release PR: #614 · v0.42.0 queued (4 packages) · open 82d 22h 42m · ✅ ready to merge Release Preview — v0.41.2 → v0.41.3
These changes will be added to the release PR (#614) when merged: Changelogmonorepo 0.41.2 → 0.41.3
After merge — predicted release
Updated automatically by ReleaseKit |
A retryable error with no cause reads as an outage the provider classified, so the live test skipped a stream that closed without `done` instead of failing. The cause names the last line received, which also helps debug a real cut-off. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e provider Each provider repeated the same timer setup, abort-to-timeout mapping and error wrapping. `withIdleTimeout` now does it once and is tested once, in base.spec; each provider keeps a test that a steady stream doesn't time out and one for its missing finish marker. The live test's entries keep their 2 KB PR bodies from a shorter template. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Merged
51 tasks
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.
Description
llm.options.timeoutis an idle timeout: a request fails only when nothing arrives for that long, so a long response that keeps streaming is never cut off. Before, the timeout capped the whole response, so a large chunk with PR context hit it on all three retries (about 6.5 minutes on chore: release v0.42.0 #614's run) and came back ungrouped.done, fails retryably.BaseLLMProvider.withIdleTimeoutruns each request under the idle timer, turns an abort into the timeout error (whatever the SDK then threw), and wraps other errors as<provider> error: …. The OpenAI, Anthropic and openai-compatible prefixes change from… API error:to that form.timeoutdescription in the schema and docs is updated, and the troubleshooting entry is rewritten.Related Issue(s)
None; seen in #614's notes run.
Type of Change
feat— New featurefix— Bug fixdocs— Documentation changerefactor— Code refactoringtest— Adding or updating testschore— Maintenance, dependencies, toolingScope
@releasekit/version@releasekit/notes@releasekit/publish@releasekit/release@releasekit/config@releasekit/coreChecklist
pnpm lint)pnpm test)pnpm typecheck)Testing Notes
base.spec.tstests the timer once: a request that keeps resetting never times out; a quiet one aborts as a retryable timeout with the abort as its cause; an abort wins over whatever the request then throws; other errors are wrapped with their cause; the timer is cleared; and bad timeouts (0, sub-millisecond, NaN, past the timer ceiling) are clamped. Makingreseta no-op, dropping the clamp, or letting anLLMErrorwin over the abort each fails it.reset) and its own missing finish marker. Ollama also covers NDJSON split across reads, including multi-byte characters, and an error object mid-stream.OLLAMA_API_KEY, so it runs in CI's Live Tests job on this PR. It passed on the earlier commits.schema:checkanddocs:checkpass.🤖 Generated with Claude Code


Confidence Score: 5/5
The PR appears safe to merge; no new actionable issues remain.Summary
All four LLM providers now stream responses and use an idle timeout instead of a total request deadline.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Start provider request] --> B[Start idle timer] B --> C{Response activity} C -->|Data arrives| D[Reset idle timer] D --> C C -->|Response completes| E[Return result and clear timer] C -->|Timer expires| F[Abort request] F --> G[Return retryable timeout and clear timer]Reviews (3) · Last reviewed commit: "refactor(notes): share the idle timeout ..." · Reviewed by Greptile