Type Converse stream events from the :event-type header#16
Conversation
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>
|
🤖 Supernova Code Review — View trace |
There was a problem hiding this comment.
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_chunk → decode_events → nest_event_under_type → build_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.
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>
|
The initial CI run failed on Fixed in 3a6c20d by saving/restoring the original config around each logger example. |
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-typeheader, and the JSON payload is the bare member struct. Captured from a live Bedrock ConverseStream response:(This matches how the official AWS SDK parses event streams —
aws-sdk-core/binary/event_parser.rbroutes on the:event-typeheader.)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 vianormalized_delta,finish_reason, tool-callstart) worked by accident. The paths without fallbacks never matched a real event:track_thinking_delta's index lookup →thinking_statenever populatedcontentBlockStopfinalization →thinking.blocksnever captured from any streamevent.key?('messageStop')safety net → dead codeSo every streamed thinking turn persisted with
blocks: nil, and a turn whose thinking had a signature but no text fell intoformat_single_thinking_block'sredactedContentbranch on replay — sending a signature where the API expects the opaque encrypted blob → 400.Fix
decode_eventsre-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 matchstream_error_event?by their proper name (e.g.throttlingException).format_single_thinking_blockreplays a signature-only thinking turn as an empty-textreasoningTextblock, neverredactedContent. A persisted signature is always a real reasoningText signature (redacted blobs are only ever captured intoblocks), so this is also the correct path for legacy messages persisted before this fix.Verification
us.anthropic.claude-sonnet-5, adaptive thinking + tool call, streaming): before this changethinking.blockswas nil on every turn; after, blocks are captured raw and the tool-loop replay succeeds end-to-end.Encoder,:event-typeheaders, flat payloads) and drive them throughparse_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.🤖 Generated with Claude Code