fix: aws text content blocks must be non-empty - #3077
Merged
Conversation
Contributor
WalkthroughThis PR enhances Claude relay message handling to safely process tool calls with empty or malformed arguments. It updates assistant message construction to use null content when appropriate, implements safer JSON parsing for tool-call arguments with empty map defaults, and expands test coverage with new scenarios for tool-call translation and streaming paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
relay/channel/claude/relay-claude.go (1)
251-265:⚠️ Potential issue | 🟠 MajorUse non-empty tool-call checks instead of non-nil checks.
At Line 251 and Line 264,
message.ToolCalls != niltreats an empty slice as a real tool-call message. That can bypass the"..."fallback and produce empty assistant content blocks, which can still trigger upstream validation failures.💡 Suggested fix
for i, message := range textRequest.Messages { if message.Role == "" { textRequest.Messages[i].Role = "user" } + hasAssistantToolCalls := message.Role == "assistant" && len(message.ParseToolCalls()) > 0 + fmtMessage := dto.Message{ Role: message.Role, Content: message.Content, } @@ - if message.Role == "assistant" && message.ToolCalls != nil { + if hasAssistantToolCalls { fmtMessage.ToolCalls = message.ToolCalls if message.IsStringContent() && message.StringContent() == "" { fmtMessage.SetNullContent() } } @@ - if fmtMessage.Content == nil && !(message.Role == "assistant" && message.ToolCalls != nil) { + if fmtMessage.Content == nil && !hasAssistantToolCalls { fmtMessage.SetStringContent("...") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/claude/relay-claude.go` around lines 251 - 265, The code currently checks message.ToolCalls != nil which treats empty slices as valid tool-call messages and can bypass the "..." fallback; update both occurrences to use a non-empty check (len(message.ToolCalls) > 0) instead of != nil — i.e., change the condition in the block that assigns fmtMessage.ToolCalls (the if with message.Role == "assistant" && message.ToolCalls != nil) and the final fallback guard (the if checking fmtMessage.Content == nil && !(message.Role == "assistant" && message.ToolCalls != nil)) so they read message.Role == "assistant" && len(message.ToolCalls) > 0 and !(message.Role == "assistant" && len(message.ToolCalls) > 0) respectively; keep the rest of the logic (fmtMessage.ToolCalls assignment, SetNullContent/SetStringContent) unchanged.
🧹 Nitpick comments (1)
relay/channel/claude/relay_claude_test.go (1)
166-172: Tighten theTextassertion for tool_use blocks.Line 170-172 currently allows non-nil
Textas long as it isn’t empty. If the intended behavior is “no placeholder text for assistant tool calls,” make this strict withassert.Nil.🧪 Suggested test tightening
- if contentBlocks[0].Text != nil { - assert.NotEqual(t, "", *contentBlocks[0].Text) - } + assert.Nil(t, contentBlocks[0].Text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@relay/channel/claude/relay_claude_test.go` around lines 166 - 172, The test currently only checks that contentBlocks[0].Text is non-empty when non-nil; tighten this for tool_use blocks by asserting it is nil for tool calls: locate the test in relay_claude_test.go where contentBlocks is inspected (the block that checks Input and Text) and replace the loose non-empty check with a strict nil assertion for tool_use blocks (e.g., check contentBlocks[0].Type == "tool_use" and assert.Nil(t, contentBlocks[0].Text)); leave the existing Input map type assertion intact.
🤖 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 `@relay/channel/claude/relay-claude.go`:
- Around line 251-265: The code currently checks message.ToolCalls != nil which
treats empty slices as valid tool-call messages and can bypass the "..."
fallback; update both occurrences to use a non-empty check
(len(message.ToolCalls) > 0) instead of != nil — i.e., change the condition in
the block that assigns fmtMessage.ToolCalls (the if with message.Role ==
"assistant" && message.ToolCalls != nil) and the final fallback guard (the if
checking fmtMessage.Content == nil && !(message.Role == "assistant" &&
message.ToolCalls != nil)) so they read message.Role == "assistant" &&
len(message.ToolCalls) > 0 and !(message.Role == "assistant" &&
len(message.ToolCalls) > 0) respectively; keep the rest of the logic
(fmtMessage.ToolCalls assignment, SetNullContent/SetStringContent) unchanged.
---
Nitpick comments:
In `@relay/channel/claude/relay_claude_test.go`:
- Around line 166-172: The test currently only checks that contentBlocks[0].Text
is non-empty when non-nil; tighten this for tool_use blocks by asserting it is
nil for tool calls: locate the test in relay_claude_test.go where contentBlocks
is inspected (the block that checks Input and Text) and replace the loose
non-empty check with a strict nil assertion for tool_use blocks (e.g., check
contentBlocks[0].Type == "tool_use" and assert.Nil(t, contentBlocks[0].Text));
leave the existing Input map type assertion intact.
Calcium-Ion
approved these changes
Mar 2, 2026
Merged
ennnnny
pushed a commit
to ennnnny/new-api
that referenced
this pull request
Mar 17, 2026
fix: aws text content blocks must be non-empty
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #1854