fix(agent-runtime): handle all content_block_start and delta subtypes in normalizeContentBlocks#430
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 4 minutes and 26 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the MessageConverter to explicitly discard content_block_start events that are not tool-use related and catch-all content_block_delta events like thinking_delta and signature_delta. It also includes unit tests to verify that these specific event types are correctly filtered out. I have no feedback to provide.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/agent-runtime/src/MessageConverter.ts (1)
130-156:⚠️ Potential issue | 🟠 MajorNormalize before emitting live stream deltas.
This only fixes the final
mergeContentBlocks()path. Incore/agent-runtime/src/AgentRuntime.ts:343-396,consumeStreamMessages()still sendsMessageConverter.toContentBlocks(msg.message)straight tothread.message.delta, socontent_block_start[thinking|text]andsignature_deltacan still leak to streaming clients even though the final merged message drops them.💡 One low-risk fix is to normalize array payloads inside
toContentBlocks()static toContentBlocks(msg: AgentStreamMessagePayload): MessageContentBlock[] { if (!msg) return []; const content = msg.content; if (typeof content === 'string') { return [{ type: ContentBlockType.Text, text: { value: content, annotations: [] } }]; } if (Array.isArray(content)) { - return content.map(part => { + const blocks = content.map(part => { if (part.type === ContentBlockType.Text) { return { type: ContentBlockType.Text, text: { value: (part as TextInputContentPart).text, annotations: [] }, } as MessageContentBlock; } return part as MessageContentBlock; }); + return MessageConverter.normalizeContentBlocks(blocks); } return []; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-runtime/src/MessageConverter.ts` around lines 130 - 156, The streaming path leaks unwanted content blocks because MessageConverter.toContentBlocks is not normalizing/filtering the array payloads before they are emitted (so AgentRuntime.consumeStreamMessages can pass through content_block_start of non-ToolUse and non-text deltas); update MessageConverter.toContentBlocks to apply the same normalization rules used by mergeContentBlocks: drop content_block_start entries unless content_block.type === ContentBlockType.ToolUse, convert only input_json_delta and text_delta into TextContentBlock (discard other delta types like thinking_delta/signature_delta), and ensure the output array contains only ToolUseContentBlock and TextContentBlock entries so streaming consumers (e.g., the path in AgentRuntime.consumeStreamMessages) never receive disallowed block types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@core/agent-runtime/src/MessageConverter.ts`:
- Around line 130-156: The streaming path leaks unwanted content blocks because
MessageConverter.toContentBlocks is not normalizing/filtering the array payloads
before they are emitted (so AgentRuntime.consumeStreamMessages can pass through
content_block_start of non-ToolUse and non-text deltas); update
MessageConverter.toContentBlocks to apply the same normalization rules used by
mergeContentBlocks: drop content_block_start entries unless content_block.type
=== ContentBlockType.ToolUse, convert only input_json_delta and text_delta into
TextContentBlock (discard other delta types like
thinking_delta/signature_delta), and ensure the output array contains only
ToolUseContentBlock and TextContentBlock entries so streaming consumers (e.g.,
the path in AgentRuntime.consumeStreamMessages) never receive disallowed block
types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2738c51f-085c-438c-8864-98b3c4461ed8
📒 Files selected for processing (2)
core/agent-runtime/src/MessageConverter.tscore/agent-runtime/test/MessageConverter.test.ts
…ing support - Restructure normalizeContentBlocks with two-phase approach: 1. Unwrap streaming protocol events to extract actual content 2. Whitelist filter — only keep known content block types - Whitelist: text, tool_use, tool_result, thinking - content_block_start: only extract tool_use, discard others - content_block_delta: extract text_delta, input_json_delta, thinking_delta; discard others (signature_delta, etc.) - Unknown block types (ping, future events) discarded by whitelist Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
jerryliang64
force-pushed
the
fix-normalize-content-blocks
branch
from
4824417 to
ee2429f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core/agent-runtime/src/MessageConverter.ts (1)
121-127: Consider defining'thinking'as a named constant.The
'thinking'magic string is used both here and at line 168. While the other types come fromContentBlockTypeenum,'thinking'is a string literal. Consider defining it as a constant (e.g.,const THINKING_BLOCK_TYPE = 'thinking') to avoid duplication and reduce maintenance risk.♻️ Suggested refactor
+const THINKING_BLOCK_TYPE = 'thinking'; + /** Content block types allowed in the final assembled message. */ private static readonly ALLOWED_BLOCK_TYPES = new Set([ ContentBlockType.Text, // text ContentBlockType.ToolUse, // tool_use ContentBlockType.ToolResult, // tool_result - 'thinking', // extended thinking + THINKING_BLOCK_TYPE, // extended thinking ]);And at line 168:
- unwrapped.push({ type: 'thinking', thinking } as unknown as MessageContentBlock); + unwrapped.push({ type: THINKING_BLOCK_TYPE, thinking } as unknown as MessageContentBlock);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/agent-runtime/src/MessageConverter.ts` around lines 121 - 127, The 'thinking' magic string is duplicated; define a single named constant (e.g., THINKING_BLOCK_TYPE) and use it wherever the literal appears to avoid drift—add the constant (either top-level or as a static member) and replace the literal in MessageConverter.ALLOWED_BLOCK_TYPES and the other usage referenced in the diff so both refer to THINKING_BLOCK_TYPE instead of the string literal, keeping other ContentBlockType enum uses unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@core/agent-runtime/src/MessageConverter.ts`:
- Around line 121-127: The 'thinking' magic string is duplicated; define a
single named constant (e.g., THINKING_BLOCK_TYPE) and use it wherever the
literal appears to avoid drift—add the constant (either top-level or as a static
member) and replace the literal in MessageConverter.ALLOWED_BLOCK_TYPES and the
other usage referenced in the diff so both refer to THINKING_BLOCK_TYPE instead
of the string literal, keeping other ContentBlockType enum uses unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b410e84-3e8f-4c67-9119-633392577be3
📒 Files selected for processing (2)
core/agent-runtime/src/MessageConverter.tscore/agent-runtime/test/MessageConverter.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- core/agent-runtime/test/MessageConverter.test.ts
…Blocks Previously each thinking_delta from Anthropic streaming became a separate thinking block in the final message. Now consecutive thinking blocks are merged into one, matching the same pattern used for text block merging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
content_block_start: only extracttool_use, discard others (thinking,text, etc.)content_block_delta: only extracttext_deltaandinput_json_delta, discard others (thinking_delta,signature_delta, etc.)Fixes missing cases discovered during integration testing with Claude Agent SDK.
Test plan
content_block_start[thinking]andcontent_block_start[text]signature_delta🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests