Skip to content

Type Converse stream events from the :event-type header#16

Merged
mattwebbio merged 2 commits into
mainfrom
fix-converse-stream-event-type-header
Jul 14, 2026
Merged

Type Converse stream events from the :event-type header#16
mattwebbio merged 2 commits into
mainfrom
fix-converse-stream-event-type-header

Conversation

@mattwebbio

Copy link
Copy Markdown

Problem

Follow-up to #14, which did not resolve the live failure it targeted: with extended/adaptive thinking enabled, a streamed Converse tool-call turn whose thinking block carries a signature but no text is rejected on the next round-trip of the same tool loop with Invalid \data` in `redacted_thinking` block`.

The reason #14 never engaged: real ConverseStream wire messages carry the event type in the eventstream :event-type header, and the JSON payload is the bare member struct. Captured from a live Bedrock ConverseStream response:

headers={":event-type"=>"contentBlockDelta", ...} payload={"contentBlockIndex":0,"delta":{"reasoningContent":{"text":"..."}},"p":"..."}
headers={":event-type"=>"messageStop", ...}       payload={"stopReason":"tool_use","p":"..."}

(This matches how the official AWS SDK parses event streams — aws-sdk-core/binary/event_parser.rb routes on the :event-type header.)

The streaming extractors instead dig by a nested event name ({'contentBlockDelta' => {...}}) — a shape only this repo's specs ever fabricated. Extractors with flat fallbacks (text/signature deltas via normalized_delta, finish_reason, tool-call start) worked by accident. The paths without fallbacks never matched a real event:

So every streamed thinking turn persisted with blocks: nil, and a turn whose thinking had a signature but no text fell into format_single_thinking_block's redactedContent branch on replay — sending a signature where the API expects the opaque encrypted blob → 400.

Fix

  1. decode_events re-nests each payload under its :event-type (or :exception-type) header, so the nested-shape paths engage on real traffic. Already-nested payloads (the old spec shape) pass through unchanged. Exception events now also match stream_error_event? by their proper name (e.g. throttlingException).
  2. format_single_thinking_block replays a signature-only thinking turn as an empty-text reasoningText block, never redactedContent. A persisted signature is always a real reasoningText signature (redacted blobs are only ever captured into blocks), so this is also the correct path for legacy messages persisted before this fix.

Verification

  • Live repro against Bedrock (us.anthropic.claude-sonnet-5, adaptive thinking + tool call, streaming): before this change thinking.blocks was nil on every turn; after, blocks are captured raw and the tool-loop replay succeeds end-to-end.
  • New specs frame events exactly as the wire does (aws-eventstream Encoder, :event-type headers, flat payloads) and drive them through parse_stream_chunk — the previous specs bypassed decoding entirely, which is how Finalize open thinking blocks on messageStop for Bedrock Converse streaming #14 passed while real streams stayed broken.
  • Full suite: 1641 examples, 0 failures. RuboCop clean on changed files.

🤖 Generated with Claude Code

ConverseStream wire messages carry the event type in the eventstream
:event-type header; the JSON payload is the bare member struct
({"contentBlockIndex":0,"delta":{...}}, {"stopReason":"tool_use"}) —
verified against a live Bedrock ConverseStream response. The streaming
extractors instead dig by a nested event name ({'contentBlockDelta' =>
{...}}), a shape that only this repo's specs ever produced. Extractors
with flat fallbacks (text/signature deltas, finish_reason, tool calls)
worked by accident; the ones without — thinking_state index tracking,
contentBlockStop finalization, and the messageStop safety net from #14 —
never matched a real event, so thinking.blocks was never populated from
a stream and every text-less thinking turn fell back to
format_single_thinking_block's lossy reconstruction on replay.

decode_events now re-nests each payload under its :event-type (or
:exception-type) header, so the nested-shape paths engage on real
traffic; already-nested payloads pass through unchanged for
compatibility.

Also fix that lossy fallback: a persisted signature is always a real
reasoningText signature (redacted blobs are only captured into blocks),
so replay it as an empty-text reasoningText block. Wrapping it in
redactedContent made Anthropic reject the next request with
"Invalid `data` in `redacted_thinking` block" — the failure that broke
every Supernova MCP tool-call turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mattwebbio mattwebbio requested a review from a team as a code owner July 14, 2026 03:26
@staging-supernova-dx-appf-io

Copy link
Copy Markdown

🤖 Supernova Code Review — View trace

@staging-supernova-dx-appf-io staging-supernova-dx-appf-io Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff and both changed files at the PR head. No actionable findings.

decode_events / nest_event_under_type — the re-nesting logic is correct. The idempotency guard (event.key?(type)) correctly passes through the old nested-shape events used by pre-existing unit tests that call build_chunk directly, while real wire events (flat payloads + :event-type header) get wrapped as the extractors expect. The rescue StandardError => nil in event_type_header is intentionally broad to stay safe in the face of malformed eventstream objects; the worst outcome is a silently-dropped un-nested event, not a crash.

format_single_thinking_block — the reasoning in the comment is sound: anything persisted into thinking.signature came from a reasoningContent signature delta, so it is always a real reasoningText signature. Using redactedContent there was always wrong; the new always-reasoningText path is both correct and backward-compatible for legacy-persisted messages.

finalize_thinking_block — correctly handles the signature-only case: text.empty? && signature.nil? is false when a signature is present, so an empty-string text field is preserved in the output block and compact leaves it intact. The spec at line 234 and the wire-framing spec at line 332 both cover this path.

New wire-framing specs — the wire_chunk / stream_to_message helpers go through the real aws-eventstream encoder and the full parse_stream_chunkdecode_eventsnest_event_under_typebuild_chunk path, which is exactly what was missing before. These tests are a meaningful improvement over the previous approach of calling build_chunk with hand-crafted nested hashes.

Nothing to request changes on. The fix is clean and well-tested.


Re-run this review

The logger specs left @config nil for the rest of the worker process. The
next lazy RubyLLM.config builds a bare Configuration whose
model_registry_source is nil — ActsAs installs its ActiveRecordSource only
once, when the module is included at load time — so acts_as_model's registry
integration specs fail with a fallback to the JSON registry whenever
test-queue happens to schedule ruby_llm_spec.rb earlier in the same worker.
Reproducible with:

  rspec spec/ruby_llm_spec.rb spec/ruby_llm/active_record/acts_as_model_spec.rb --order defined

Save and restore the original config/logger around each example instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mattwebbio

Copy link
Copy Markdown
Author

The initial CI run failed on acts_as_model_spec.rb's registry-integration examples across most matrix jobs — a pre-existing order-dependent flake, not a regression from this change: the logger specs in ruby_llm_spec.rb nil out RubyLLM.@config and leave it nil, destroying the model_registry_source that ActsAs.included installs once at load time. Whenever test-queue schedules ruby_llm_spec.rb before acts_as_model_spec.rb in the same worker, those two examples fail (this PR's added specs merely reshuffled the scheduling). Deterministic repro:

rspec spec/ruby_llm_spec.rb spec/ruby_llm/active_record/acts_as_model_spec.rb --order defined

Fixed in 3a6c20d by saving/restoring the original config around each logger example.

@mattwebbio mattwebbio merged commit ceab242 into main Jul 14, 2026
21 checks passed
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