Skip to content

fix(streaming): keep accumulated stop_* fields across message_delta events - #1204

Open
feiiiiii5 wants to merge 1 commit into
anthropics:mainfrom
feiiiiii5:fix-streaming-stop-fields
Open

feiiiiii5 wants to merge 1 commit into
anthropics:mainfrom
feiiiiii5:fix-streaming-stop-fields

Conversation

@feiiiiii5

Copy link
Copy Markdown

The bug

MessageStream / BetaMessageStream accumulate a running Message snapshot from the stream events. In the message_delta branch, three of the accumulated fields are assigned unconditionally:

snapshot.stop_reason = event.delta.stop_reason;
snapshot.stop_sequence = event.delta.stop_sequence;
snapshot.stop_details = event.delta.stop_details;

RawMessageDeltaEvent.Delta types all three as nullable (src/resources/messages/messages.ts:2763-2778, and the Beta equivalent), so a message_delta whose payload does not carry them arrives as null — and a null then erases whatever an earlier delta in the same stream already established. finalMessage() reports stop_reason: null for a message that did stop, and stop_details (the refusal classification) disappears.

The two fields directly below in the very same block are already guarded against exactly this:

if (event.delta.container != null) { ... }
if (event.delta.context_management != null) { ... }

and the repo's own test keeps accumulated container and context_management when a later message_delta omits them (tests/api-resources/BetaMessageStream.test.ts) drives a two-message_delta stream to prove it. So the "a later delta may omit what an earlier one set" shape is already an accepted case here — stop_* was just left out of it. usage.output_tokens stays unconditional, which is correct: it is typed number, non-null.

The change

Guard the three assignments with the same != null check used by their siblings, so a delta only contributes fields it actually carries.

Tests

Six new cases (three per stream class) in the existing tests/api-resources/*MessageStream.test.ts, appended without editing existing tests:

  • a usage-only message_delta following a refusal delta keeps stop_reason + stop_details
  • a later delta that omits stop_sequence keeps it while stop_reason updates
  • a later delta that sends them explicitly as null keeps them — same convention the existing container test asserts with an explicit container: null

Regression evidence, same command on both trees (node@24, vitest run tests/api-resources/MessageStream.test.ts tests/api-resources/BetaMessageStream.test.ts):

tree result
base 3c5d9c0, new tests, unmodified src/lib `Tests 6 failed
this branch Tests 36 passed (36)

Full suite after ./scripts/mock --daemon: 5 failed | 1791 passed | 57 skipped, versus 5 failed | 1785 passed on a stashed clean tree — the same 5 pre-existing failures (tests/tools/agent-toolset.test.ts ×4, tests/skills.test.ts ×1), none in streaming, so this change adds no regression. ./scripts/lint passes (prettier, eslint, build, tsc, attw, publint: 0 errors). Sub-package suites pass: vertex 17, bedrock 62, foundry 12, aws 84, google-cloud 63.

Scope notes

The generator never writes to src/lib/, so this is a hand-written fix in a hand-written file. I did not touch the compaction-delta null handling in BetaMessageStream#1174 covers that, and it is a different root cause from these three fields.

…vents

A message_delta that carries null stop_reason/stop_sequence/stop_details overwrote whatever an earlier delta in the same stream had already accumulated, even though the block guards container and context_management against exactly that.
@feiiiiii5
feiiiiii5 requested a review from a team as a code owner September 19, 2026 16:16

This branch has not been deployed

No deployments
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.

1 participant