Skip to content

fix(openai): require terminal stream events before finishing#246

Draft
ethanndickson wants to merge 1 commit into
charmbracelet:mainfrom
coder:fix/openai-stream-terminal-events
Draft

fix(openai): require terminal stream events before finishing#246
ethanndickson wants to merge 1 commit into
charmbracelet:mainfrom
coder:fix/openai-stream-terminal-events

Conversation

@ethanndickson
Copy link
Copy Markdown
Contributor

@ethanndickson ethanndickson commented May 19, 2026

Context

#232 gated the Chat Completions text Stream and the Responses API text Stream on fantasy.NewIncompleteStreamError(). This PR closes two remaining gaps that those guards don't cover, plus a related misclassification.

What this PR fixes

1. Chat Completions object stream — silent truncation. streamObjectWithJSONMode has no terminal-event check today. A truncated stream where any prior chunk parsed cleanly emits Finish with the partial lastParsedObject — the caller receives a JSON object missing fields, reported as success. Fix: track sawFinishReason; otherwise yield fantasy.NewIncompleteStreamError() (retryable *fantasy.ProviderError with Cause: io.ErrUnexpectedEOF) with ctx.Err() precedence.

2. Responses API object stream — misclassified truncation + silent-finish risk. Post-loop wraps io.EOF from stream.Err() via toProviderErr, which produces a non-retryable error (IsRetryable() matches io.ErrUnexpectedEOF, not bare io.EOF), so transient truncation surfaces as a fatal provider error and retries never engage. And with no terminal-event check beyond the error event, an SDK that returns nil on a clean mid-message close flows straight to Finish with a partial parsed object. Fix: track sawTerminalEvent (set on response.completed / response.incomplete), treat io.EOF from stream.Err() as a fall-through to that check, and emit fantasy.NewIncompleteStreamError() when neither arrived.

3. response.failed misclassified as transport truncation on both Responses streams. Today the event falls through, fails the terminal-event check, and gets retried as if it were a network truncation. Fix: handle response.failed explicitly, yielding a non-retryable *fantasy.Error{Title: "response failed"} with the provider's error message and code. The pre-existing top-level error event is normalized through the same helper as *fantasy.Error{Title: "response error"}, so only synthetic stream truncation is wrapped with io.ErrUnexpectedEOF.

Two smaller refinements ride along: the existing Chat Completions text Stream guard now respects ctx.Err() precedence, and the Responses API text Stream's existing FinishReasonUnknown-sentinel guard is renamed to an explicit sawTerminalEvent for clarity (semantics unchanged).

Tests

Three new table-driven tests cover happy path + each missing terminal signal + (where applicable) response.failed / error events:

  • TestChatCompletionsStreamObject_RequiresFinishReasonBeforeFinish
  • TestResponsesStream_RequiresTerminalEventBeforeFinish
  • TestResponsesStreamObject_RequiresTerminalEventBeforeFinish

The object-stream truncation subtests would emit Finish on main today.

Prior art

OpenAI's own openai/codex (Apache-2.0) implements the same gate for the Responses API: codex-rs/codex-api/src/sse/responses.rs handles response.completed, response.incomplete, and response.failed as distinct terminal events, and emits ApiError::Stream("stream closed before response.completed") on early EOF. Regression test stream_no_completed.rs::retries_on_early_close asserts the retry path fires; an inline error_when_error_event test covers response.failed end-to-end. vercel/ai (Apache-2.0) makes the same isResponseCompletedChunk / isResponseFailedChunk distinction in its OpenAI Responses adapter.

Follow-ups

Similar terminal-event gaps exist in a few of the other providers in this repo; those will land in separate PRs so each can be reviewed in isolation.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants