Skip to content

fix(sse): report a stream that completes without any content - #8732

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
backryun:fix/8649-combo-empty-stream
Jul 27, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
backryun:fix/8649-combo-empty-stream

Conversation

@backryun

Copy link
Copy Markdown
Contributor

Closes #8649.

The gap

The non-streaming path already refuses a contentless completion: isEmptyContentResponse rewrites a 200-with-no-content into a 502 "Provider returned empty content", which the combo layer classifies as a model-level transient and fails over on (#5085).

The streaming path had no equivalent. Two guards exist and neither covers the reported case:

guard what it actually checks why it misses this
ensureStreamReadiness liveness — its own failure text is "Stream ended before producing a non-ping SSE event" any structured non-ping frame satisfies it. hasStreamReadinessSignal returns true for a bare delta:{"role":"assistant"} — I checked by running it against the reported payload
createDisconnectAwareStream (#7699) a missing terminal marker, scoped to the Claude client format the reported stream is OpenAI-format and terminates correctly, with finish_reason:"stop" and [DONE]

So the reported stream — OpenAI format, terminates properly, contains nothing — trips neither, and the agent receives a clean empty assistant turn with no error to stop on. It then retries to its cap, which is the harm the issue describes.

The fix

hasUsefulStreamContent already existed in streamReadiness.ts: exported, correct, and wired to nothing. It already treats tool-call-only and reasoning-only output as real content, which is the #2520 lesson baked in. This PR gives it a caller.

createStreamContentWatcher() wraps it to handle two things a bare predicate cannot:

  • frames split across network chunks — buffers to blank-line boundaries so a delta arriving in two TCP reads is still scanned as one payload (bounded; overflow is scanned in pieces, which can only err toward "saw content", never toward a false empty)
  • legitimately empty terminal states — length, tool_calls, content_filter, max_tokens, tool_use, kept in step with errorClassifier.ts's LEGIT_EMPTY_OPENAI_FINISH / LEGIT_EMPTY_CLAUDE_STOP

createDisconnectAwareStream's if (done) branch then reports "Provider returned empty content" — deliberately the same wording the non-streaming path uses, so the two halves are diagnostically identical.

This guard is format-agnostic, unlike #7699's. That is intentional and the asymmetry is justified: a missing terminal marker is genuinely ambiguous across formats (many have no [DONE] equivalent), which is exactly why #7699 scoped itself to Claude. Completed normally but emitted zero content is not ambiguous anywhere.

Over-firing, and the regression it caught

Two conditions keep it narrow: bytes must have been forwarded, and the body must have looked like SSE.

The second one is not theoretical — without it, four existing tests failed. Not every body reaching this wrapper is an event stream; a plain JSON completion travels the same path, has no data: frames, and would have been read as "no content seen". sawSseFrame() gates that, and tests/unit/empty-stream-no-content-8649.test.ts pins the case.

Scope: surface, not fail over

The issue asks for "surface the upstream error OR fail over". This does the first.

Failing over would mean holding every stream until its first content token, because by the time the emptiness is known the combo has already returned leg 1's response — a much larger and riskier change touching every streaming request's latency. Surfacing the error stops the retry loop, which is the reported harm, and leaves the combo path untouched.

Test plan

tests/unit/empty-stream-no-content-8649.test.ts — 8 tests, one per arm:

Five of those pass vacuously before the fix, since nothing fired at all; after it they are the guards against over-firing, which is where the real risk in this change sits.

Verification

  • Full unit suite: 26,611 tests, 6 failures — 5 of them reproduce identically on the pristine base (machineId hostname mocking ×2, groupByDay timezone ×2, getCliRuntimeStatus PATH/symlink ×1). The sixth, syncAllProviderLimits … spacingMs=0, is a wall-clock test (Date.now() gap assertions) that passes 3/3 in isolation and only fails under full-suite load; it has no relationship to SSE streaming.
  • Neighbour suites — silent-sse-close-7699, stream-handler, stream-utilities, stream-handler-deadline, stream-readiness, combo-empty-content-failover-5085: 84/84.
  • typecheck:core clean; ESLint clean on all touched files.
  • check:complexity-ratchets 2187 against a baseline of 2188 — extracting the if (done) reasoning into resolveSilentCloseReason() dropped pull back under the function-length ceiling and cleared one pre-existing violation with it.
  • check:file-size, check:known-symbols, check:any-budget:t11, check:error-helper — OK.

Note on what is not covered

This addresses the silent-empty symptom generally. The issue's specific trigger — an auto/* combo whose first step resolves to an uncredentialed auggie backend (account: "noauth", apiKeyId: null) — is a separate question about whether the auto-combo candidate builder should be offering an uncredentialed provider as a step at all. autoComboCandidates.ts already filters on connectionId !== "noauth", so that path is worth a look on its own; it is not in scope here.

@backryun
backryun requested a review from diegosouzapw as a code owner July 27, 2026 00:07
@backryun
backryun force-pushed the fix/8649-combo-empty-stream branch from 363001c to 5a65445 Compare July 27, 2026 15:18
An `auto/*` combo whose first step lands on an uncredentialed backend returns
HTTP 200 with `finish_reason: "stop"`, `content: null` and `error: null`. The
agent sees a clean empty assistant turn, has no error to stop on, and retries to
its cap.

The non-streaming path already refuses this: `isEmptyContentResponse` rewrites a
200-with-no-content into a 502 "Provider returned empty content", which the combo
layer classifies as a model-level transient and fails over on (cheaper-inference#5085). The
streaming path had no equivalent, and neither existing guard covers it:

- `ensureStreamReadiness` is a LIVENESS probe, not a content one. Its failure
  message says so — "Stream ended before producing a non-ping SSE event" — and
  `hasStreamReadinessSignal` returns true for a bare `delta:{"role":"assistant"}`.
- `createDisconnectAwareStream`'s cheaper-inference#7699 branch fires on a MISSING terminal marker
  and is scoped to the Claude client format, because for other formats a
  marker-less close is genuinely ambiguous.

The reported stream trips neither: OpenAI format, terminates with
`finish_reason: "stop"` and `[DONE]`, contains nothing.

"Completed normally but emitted zero content" is not ambiguous the way a missing
marker is, so this guard is format-agnostic. It reuses `hasUsefulStreamContent`,
which already existed in streamReadiness.ts — exported, correct, and wired to
nothing — and which already counts tool-call-only and reasoning-only output as
real (cheaper-inference#2520). A watcher wraps it to handle frames split across network chunks and
to spot the terminal states where emptiness is legitimate, kept in step with
errorClassifier.ts's `LEGIT_EMPTY_OPENAI_FINISH` / `LEGIT_EMPTY_CLAUDE_STOP`:
length, tool_calls, content_filter, max_tokens, tool_use.

Two guards keep it from over-firing, one of which caught a real regression while
building this: the check applies only when bytes were forwarded, and only when
the body actually looked like SSE. A plain JSON completion travels through the
same wrapper and has no `data:` frames, so "no content seen" says nothing about
it — without the SSE gate, four existing stream tests failed.

The `if (done)` branch's reasoning moved into `resolveSilentCloseReason()`, which
also drops `pull` back under the function-length ceiling; cyclomatic lands at
2187 against a baseline of 2188.

This surfaces the error rather than failing over. Failing over would mean holding
every stream until its first content token, since the combo has already returned
leg 1's response by then — a much larger change. Surfacing the error satisfies
the issue's stated expectation ("surface the upstream error OR fail over") and
stops the retry loop, which is the reported harm.

Closes cheaper-inference#8649
@backryun
backryun force-pushed the fix/8649-combo-empty-stream branch from 5a65445 to 9a08105 Compare July 27, 2026 15:35
@diegosouzapw
diegosouzapw merged commit eedade9 into cheaper-inference:release/v3.8.49 Jul 27, 2026
15 checks passed
@backryun
backryun deleted the fix/8649-combo-empty-stream branch July 27, 2026 22:21
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…-inference#8732)

An `auto/*` combo whose first step lands on an uncredentialed backend returns
HTTP 200 with `finish_reason: "stop"`, `content: null` and `error: null`. The
agent sees a clean empty assistant turn, has no error to stop on, and retries to
its cap.

The non-streaming path already refuses this: `isEmptyContentResponse` rewrites a
200-with-no-content into a 502 "Provider returned empty content", which the combo
layer classifies as a model-level transient and fails over on (cheaper-inference#5085). The
streaming path had no equivalent, and neither existing guard covers it:

- `ensureStreamReadiness` is a LIVENESS probe, not a content one. Its failure
  message says so — "Stream ended before producing a non-ping SSE event" — and
  `hasStreamReadinessSignal` returns true for a bare `delta:{"role":"assistant"}`.
- `createDisconnectAwareStream`'s cheaper-inference#7699 branch fires on a MISSING terminal marker
  and is scoped to the Claude client format, because for other formats a
  marker-less close is genuinely ambiguous.

The reported stream trips neither: OpenAI format, terminates with
`finish_reason: "stop"` and `[DONE]`, contains nothing.

"Completed normally but emitted zero content" is not ambiguous the way a missing
marker is, so this guard is format-agnostic. It reuses `hasUsefulStreamContent`,
which already existed in streamReadiness.ts — exported, correct, and wired to
nothing — and which already counts tool-call-only and reasoning-only output as
real (cheaper-inference#2520). A watcher wraps it to handle frames split across network chunks and
to spot the terminal states where emptiness is legitimate, kept in step with
errorClassifier.ts's `LEGIT_EMPTY_OPENAI_FINISH` / `LEGIT_EMPTY_CLAUDE_STOP`:
length, tool_calls, content_filter, max_tokens, tool_use.

Two guards keep it from over-firing, one of which caught a real regression while
building this: the check applies only when bytes were forwarded, and only when
the body actually looked like SSE. A plain JSON completion travels through the
same wrapper and has no `data:` frames, so "no content seen" says nothing about
it — without the SSE gate, four existing stream tests failed.

The `if (done)` branch's reasoning moved into `resolveSilentCloseReason()`, which
also drops `pull` back under the function-length ceiling; cyclomatic lands at
2187 against a baseline of 2188.

This surfaces the error rather than failing over. Failing over would mean holding
every stream until its first content token, since the combo has already returned
leg 1's response by then — a much larger change. Surfacing the error satisfies
the issue's stated expectation ("surface the upstream error OR fail over") and
stops the retry loop, which is the reported harm.

Closes cheaper-inference#8649
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…-inference#8732)

An `auto/*` combo whose first step lands on an uncredentialed backend returns
HTTP 200 with `finish_reason: "stop"`, `content: null` and `error: null`. The
agent sees a clean empty assistant turn, has no error to stop on, and retries to
its cap.

The non-streaming path already refuses this: `isEmptyContentResponse` rewrites a
200-with-no-content into a 502 "Provider returned empty content", which the combo
layer classifies as a model-level transient and fails over on (cheaper-inference#5085). The
streaming path had no equivalent, and neither existing guard covers it:

- `ensureStreamReadiness` is a LIVENESS probe, not a content one. Its failure
  message says so — "Stream ended before producing a non-ping SSE event" — and
  `hasStreamReadinessSignal` returns true for a bare `delta:{"role":"assistant"}`.
- `createDisconnectAwareStream`'s cheaper-inference#7699 branch fires on a MISSING terminal marker
  and is scoped to the Claude client format, because for other formats a
  marker-less close is genuinely ambiguous.

The reported stream trips neither: OpenAI format, terminates with
`finish_reason: "stop"` and `[DONE]`, contains nothing.

"Completed normally but emitted zero content" is not ambiguous the way a missing
marker is, so this guard is format-agnostic. It reuses `hasUsefulStreamContent`,
which already existed in streamReadiness.ts — exported, correct, and wired to
nothing — and which already counts tool-call-only and reasoning-only output as
real (cheaper-inference#2520). A watcher wraps it to handle frames split across network chunks and
to spot the terminal states where emptiness is legitimate, kept in step with
errorClassifier.ts's `LEGIT_EMPTY_OPENAI_FINISH` / `LEGIT_EMPTY_CLAUDE_STOP`:
length, tool_calls, content_filter, max_tokens, tool_use.

Two guards keep it from over-firing, one of which caught a real regression while
building this: the check applies only when bytes were forwarded, and only when
the body actually looked like SSE. A plain JSON completion travels through the
same wrapper and has no `data:` frames, so "no content seen" says nothing about
it — without the SSE gate, four existing stream tests failed.

The `if (done)` branch's reasoning moved into `resolveSilentCloseReason()`, which
also drops `pull` back under the function-length ceiling; cyclomatic lands at
2187 against a baseline of 2188.

This surfaces the error rather than failing over. Failing over would mean holding
every stream until its first content token, since the combo has already returned
leg 1's response by then — a much larger change. Surfacing the error satisfies
the issue's stated expectation ("surface the upstream error OR fail over") and
stops the retry loop, which is the reported harm.

Closes cheaper-inference#8649
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.

fix(resilience): auto/* combos swallow upstream auth failure — empty finish:stop with no error when tools attached

2 participants