Skip to content

fix(anthropic): require message_stop before finishing stream#245

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

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

Conversation

@ethanndickson
Copy link
Copy Markdown
Contributor

@ethanndickson ethanndickson commented May 19, 2026

Context

#232 added fantasy.NewIncompleteStreamError() and gated the Anthropic post-loop on acc.StopReason == "". That catches the user-facing-content cases: a stream that closes anywhere from the first content_block_delta up to (but not including) the message_delta event leaves acc.StopReason empty and is correctly reported as a retryable incomplete stream.

What that check still admits is the narrow window between message_delta and message_stop. message_delta carries the stop_reason and sets acc.StopReason non-empty, so the existing guard passes; but the protocol-required message_stop terminator never arrives. The stream then emits StreamPartTypeFinish with a real-looking finish reason, hiding the fact that the transport closed unexpectedly in a window the Anthropic SSE protocol documents as impossible on success.

Fix

Require both terminal signals before yielding Finish:

  • A new sawMessageStop bool is flipped to true inside the chunk-type switch when a message_stop event arrives.
  • After the loop, if sawMessageStop is false or acc.StopReason is empty, the stream yields fantasy.NewIncompleteStreamError() (retryable *fantasy.ProviderError with Cause: io.ErrUnexpectedEOF) instead of Finish. ctx.Err() takes precedence so caller-initiated cancellations stay distinguishable from synthetic transport errors — a small attribution improvement on the existing path as well.

This matches the pattern Anthropic ships in their own newest SDK: MessageAccumulator in anthropic-sdk-java (PR anthropics/anthropic-sdk-java#178) raises IllegalStateException("'message_stop' event not yet received.") on MessageAccumulator.message() and has dedicated unit tests for both messageNotStarted and messageNotStopped.

Tests

A new table-driven test TestStream_RequiresMessageStopBeforeFinish covers:

  • complete happy path (both events present) → Finish
  • message_delta with stop_reason present but message_stop missing → retryable incomplete-stream error (the new window this PR closes)
  • message_stop present but stop_reason missing → retryable
  • a server-sent error SSE event → surfaced verbatim and not retryable

The pre-existing TestStream_TruncatedWithoutStopReason (from #232) covers the simpler case where neither terminal event arrives and is unchanged.

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