fix(translator/claude): pair tool_use/tool_result to avoid Bedrock 400#3791
fix(translator/claude): pair tool_use/tool_result to avoid Bedrock 400#3791xiaojiou176 wants to merge 2 commits into
Conversation
Re-derived against upstream/main (independent of custom_tool): merges adjacent same-role function_call / function_call_output items into a single Anthropic assistant/user message (parallel-tool-merge), and adds a defense-in-depth reorderClaudeToolUseResultPairs pass that pulls a matching tool_result block to sit immediately after its tool_use block. Prevents AWS Bedrock TOOL_USE_RESULT_MISMATCH (HTTP 400) on interleaved tool sequences.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a defense-in-depth sanitization step to ensure Claude tool_use/tool_result pairing is contiguous (to avoid Bedrock 400s), plus converter tweaks and tests to handle parallel tool merging and reordering edge-cases.
Changes:
- Add
reorderClaudeToolUseResultPairsto reorder/split tool_result blocks so each assistant tool_use batch is immediately followed by its matching user tool_result message(s). - Update request conversion to merge consecutive tool_use (assistant) and tool_result (user) containers instead of emitting same-role consecutive messages.
- Add Go tests covering idempotency and multiple reordering scenarios (including merged tool_result containers across split tool_use batches).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| internal/translator/claude/openai/responses/claude_openai-responses_request_pairing_test.go | Adds test coverage for tool_use/tool_result reordering, split-batch handling, and idempotency. |
| internal/translator/claude/openai/responses/claude_openai-responses_request_pairing.go | Implements block-level tool_use/tool_result pairing sanitization with idempotency guard and byte-preserving fast path. |
| internal/translator/claude/openai/responses/claude_openai-responses_request.go | Merges consecutive tool_use/tool_result containers and runs the sanitization pass on final output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| appendedToolUse := false | ||
| if msgsCount := gjson.GetBytes(out, "messages.#").Int(); msgsCount > 0 { | ||
| lastIdx := msgsCount - 1 | ||
| lastRole := gjson.GetBytes(out, fmt.Sprintf("messages.%d.role", lastIdx)).String() | ||
| lastContent := gjson.GetBytes(out, fmt.Sprintf("messages.%d.content", lastIdx)) | ||
| if lastRole == "assistant" && lastContent.IsArray() { | ||
| out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolUse) | ||
| appendedToolUse = true | ||
| } | ||
| } | ||
| if !appendedToolUse { | ||
| asst := []byte(`{"role":"assistant","content":[]}`) | ||
| for _, partJSON := range pendingReasoningParts { | ||
| asst, _ = sjson.SetRawBytes(asst, "content.-1", []byte(partJSON)) | ||
| } | ||
| pendingReasoningParts = nil | ||
| asst, _ = sjson.SetRawBytes(asst, "content.-1", toolUse) | ||
| out, _ = sjson.SetRawBytes(out, "messages.-1", asst) |
| // FIX (parallel-tool-merge): If the last message in the array is already | ||
| // an assistant message whose content is an array (i.e., a tool_use container), | ||
| // append this tool_use to that message instead of creating a new assistant | ||
| // message. This avoids producing consecutive same-role messages, which violate | ||
| // the Anthropic Messages API requirement that user/assistant strictly alternate | ||
| // and which AWS Bedrock rejects as TOOL_USE_RESULT_MISMATCH (HTTP 400). | ||
| appendedToolUse := false |
| if lastRole == "assistant" && lastContent.IsArray() { | ||
| out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolUse) | ||
| appendedToolUse = true | ||
| } |
| appendedToolResult := false | ||
| if msgsCount := gjson.GetBytes(out, "messages.#").Int(); msgsCount > 0 { | ||
| lastIdx := msgsCount - 1 | ||
| lastRole := gjson.GetBytes(out, fmt.Sprintf("messages.%d.role", lastIdx)).String() | ||
| lastContent := gjson.GetBytes(out, fmt.Sprintf("messages.%d.content", lastIdx)) | ||
| if lastRole == "user" && lastContent.IsArray() { | ||
| out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolResult) | ||
| appendedToolResult = true | ||
| } | ||
| } |
| rebuilt := []byte(`[]`) | ||
| for _, e := range emitted { | ||
| if e.idx >= 0 { | ||
| rebuilt, _ = sjson.SetRawBytes(rebuilt, "-1", []byte(arr[e.idx].Raw)) | ||
| } else { | ||
| rebuilt, _ = sjson.SetRawBytes(rebuilt, "-1", e.raw) | ||
| } | ||
| } | ||
| result, err := sjson.SetRawBytes(out, "messages", rebuilt) | ||
| if err != nil { | ||
| return out | ||
| } | ||
| return result |
There was a problem hiding this comment.
Code Review
This pull request introduces a pairing sanitization pass (reorderClaudeToolUseResultPairs) to ensure that Anthropic tool_use and tool_result blocks are correctly paired and ordered, preventing AWS Bedrock HTTP 400 errors when non-tool messages are interleaved. It also updates the request converter to merge adjacent tool uses and results into existing assistant or user messages where appropriate. The review feedback highlights two critical issues: first, accumulated pendingReasoningParts are ignored when appending tool uses to an existing assistant message, which can lead to role alternation violations later; second, the reordering algorithm itself can produce consecutive same-role messages (e.g., when moving intervening user messages), necessitating a post-processing pass to merge consecutive same-role messages. Corresponding test cases are suggested to verify strict role alternation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| appendedToolUse := false | ||
| if msgsCount := gjson.GetBytes(out, "messages.#").Int(); msgsCount > 0 { | ||
| lastIdx := msgsCount - 1 | ||
| lastRole := gjson.GetBytes(out, fmt.Sprintf("messages.%d.role", lastIdx)).String() | ||
| lastContent := gjson.GetBytes(out, fmt.Sprintf("messages.%d.content", lastIdx)) | ||
| if lastRole == "assistant" && lastContent.IsArray() { | ||
| out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolUse) | ||
| appendedToolUse = true | ||
| } | ||
| } |
There was a problem hiding this comment.
If appendedToolUse is true, any accumulated pendingReasoningParts are currently ignored and left in the slice, which can lead to them being incorrectly flushed later as a separate assistant message (violating role alternation). We should append the reasoning parts to the existing assistant message before appending the tool use, and then clear the slice.
appendedToolUse := false
if msgsCount := gjson.GetBytes(out, "messages.#").Int(); msgsCount > 0 {
lastIdx := msgsCount - 1
lastRole := gjson.GetBytes(out, fmt.Sprintf("messages.%d.role", lastIdx)).String()
lastContent := gjson.GetBytes(out, fmt.Sprintf("messages.%d.content", lastIdx))
if lastRole == "assistant" && lastContent.IsArray() {
for _, partJSON := range pendingReasoningParts {
out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), []byte(partJSON))
}
pendingReasoningParts = nil
out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolUse)
appendedToolUse = true
}
}| func reorderClaudeToolUseResultPairs(out []byte) []byte { | ||
| msgs := gjson.GetBytes(out, "messages") | ||
| if !msgs.IsArray() { | ||
| return out | ||
| } | ||
| arr := msgs.Array() | ||
| n := len(arr) | ||
| if n < 2 { | ||
| return out | ||
| } | ||
|
|
||
| // Pre-parse each message's content blocks (only meaningful for array | ||
| // content; string content yields nil). | ||
| blocks := make([][]contentBlock, n) | ||
| blockTaken := make([][]bool, n) | ||
| for i := range arr { | ||
| blocks[i] = parseContentBlocks(arr[i]) | ||
| if blocks[i] != nil { | ||
| blockTaken[i] = make([]bool, len(blocks[i])) | ||
| } | ||
| } | ||
|
|
||
| consumed := make([]bool, n) | ||
| // An emitted entry is either an original message (idx >= 0) or a freshly | ||
| // constructed message (idx == -1, use raw). | ||
| type entry struct { | ||
| idx int | ||
| raw []byte | ||
| } | ||
| emitted := make([]entry, 0, n) | ||
|
|
||
| for i := 0; i < n; i++ { | ||
| if consumed[i] { | ||
| continue | ||
| } | ||
| consumed[i] = true | ||
|
|
||
| ids := toolUseIDsOrdered(arr[i]) | ||
| if len(ids) == 0 { | ||
| // Not an assistant tool_use batch. If some of this message's | ||
| // tool_result blocks were already pulled out by an earlier batch, | ||
| // emit only the leftover blocks; otherwise emit the message as-is. | ||
| if blocks[i] == nil { | ||
| emitted = append(emitted, entry{idx: i}) | ||
| continue | ||
| } | ||
| anyTaken := false | ||
| for _, t := range blockTaken[i] { | ||
| if t { | ||
| anyTaken = true | ||
| break | ||
| } | ||
| } | ||
| if !anyTaken { | ||
| emitted = append(emitted, entry{idx: i}) | ||
| continue | ||
| } | ||
| remaining := make([]string, 0, len(blocks[i])) | ||
| for bi, b := range blocks[i] { | ||
| if !blockTaken[i][bi] { | ||
| remaining = append(remaining, b.raw) | ||
| } | ||
| } | ||
| if len(remaining) == 0 { | ||
| // Every block was pulled elsewhere; drop the now-empty container. | ||
| continue | ||
| } | ||
| emitted = append(emitted, entry{idx: -1, raw: buildBlocksMessage(arr[i].Get("role").String(), remaining)}) | ||
| continue | ||
| } | ||
|
|
||
| // Assistant tool_use batch: emit it, then pull the matching tool_result | ||
| // blocks (forward only) to sit immediately after it. | ||
| emitted = append(emitted, entry{idx: i}) | ||
|
|
||
| matchedRaw := make([]string, 0, len(ids)) | ||
| srcMsg := -1 | ||
| singleSrc := true | ||
| for _, id := range ids { | ||
| mi, bi, ok := findForwardResultBlock(blocks, blockTaken, consumed, i+1, id) | ||
| if !ok { | ||
| continue | ||
| } | ||
| blockTaken[mi][bi] = true | ||
| matchedRaw = append(matchedRaw, blocks[mi][bi].raw) | ||
| if srcMsg == -1 { | ||
| srcMsg = mi | ||
| } else if srcMsg != mi { | ||
| singleSrc = false | ||
| } | ||
| } | ||
| if len(matchedRaw) == 0 { | ||
| continue | ||
| } | ||
|
|
||
| // Byte-preservation fast path: when every matched result came from one | ||
| // still-unconsumed message that holds ONLY these tool_result blocks | ||
| // (nothing else, nothing extra), keep that message verbatim instead of | ||
| // rebuilding it. This is what keeps an already-canonical sequence | ||
| // byte-identical (idempotency). | ||
| if singleSrc && srcMsg >= 0 && !consumed[srcMsg] && | ||
| len(matchedRaw) == len(blocks[srcMsg]) && allToolResults(blocks[srcMsg]) { | ||
| consumed[srcMsg] = true | ||
| emitted = append(emitted, entry{idx: srcMsg}) | ||
| continue | ||
| } | ||
| emitted = append(emitted, entry{idx: -1, raw: buildBlocksMessage("user", matchedRaw)}) | ||
| } | ||
|
|
||
| // Idempotency guard: if the emitted order is the identity permutation of the | ||
| // original messages (no splits, no moves), return the original bytes. | ||
| identity := len(emitted) == n | ||
| if identity { | ||
| for k, e := range emitted { | ||
| if e.idx != k { | ||
| identity = false | ||
| break | ||
| } | ||
| } | ||
| } | ||
| if identity { | ||
| return out | ||
| } | ||
|
|
||
| rebuilt := []byte(`[]`) | ||
| for _, e := range emitted { | ||
| if e.idx >= 0 { | ||
| rebuilt, _ = sjson.SetRawBytes(rebuilt, "-1", []byte(arr[e.idx].Raw)) | ||
| } else { | ||
| rebuilt, _ = sjson.SetRawBytes(rebuilt, "-1", e.raw) | ||
| } | ||
| } | ||
| result, err := sjson.SetRawBytes(out, "messages", rebuilt) | ||
| if err != nil { | ||
| return out | ||
| } | ||
| return result | ||
| } |
There was a problem hiding this comment.
The current reordering algorithm can produce consecutive same-role messages (e.g., when an intervening user message is moved after a tool_use/tool_result pair, resulting in consecutive user messages). This violates the strict role alternation requirement of Anthropic/Bedrock and will cause HTTP 400 errors. We should add a post-processing pass to merge consecutive same-role messages.
type entry struct {
idx int
role string
raw []byte
}
func reorderClaudeToolUseResultPairs(out []byte) []byte {
msgs := gjson.GetBytes(out, "messages")
if !msgs.IsArray() {
return out
}
arr := msgs.Array()
n := len(arr)
if n < 2 {
return out
}
// Pre-parse each message's content blocks (only meaningful for array
// content; string content yields nil).
blocks := make([][]contentBlock, n)
blockTaken := make([][]bool, n)
for i := range arr {
blocks[i] = parseContentBlocks(arr[i])
if blocks[i] != nil {
blockTaken[i] = make([]bool, len(blocks[i]))
}
}
consumed := make([]bool, n)
// An emitted entry is either an original message (idx >= 0) or a freshly
// constructed message (idx == -1, use raw).
emitted := make([]entry, 0, n)
for i := 0; i < n; i++ {
if consumed[i] {
continue
}
consumed[i] = true
ids := toolUseIDsOrdered(arr[i])
if len(ids) == 0 {
// Not an assistant tool_use batch. If some of this message's
// tool_result blocks were already pulled out by an earlier batch,
emit only the leftover blocks; otherwise emit the message as-is.
role := arr[i].Get("role").String()
if blocks[i] == nil {
emitted = append(emitted, entry{idx: i, role: role})
continue
}
anyTaken := false
for _, t := range blockTaken[i] {
if t {
anyTaken = true
break
}
}
if !anyTaken {
emitted = append(emitted, entry{idx: i, role: role})
continue
}
remaining := make([]string, 0, len(blocks[i]))
for bi, b := range blocks[i] {
if !blockTaken[i][bi] {
remaining = append(remaining, b.raw)
}
}
if len(remaining) == 0 {
// Every block was pulled elsewhere; drop the now-empty container.
continue
}
emitted = append(emitted, entry{idx: -1, role: role, raw: buildBlocksMessage(role, remaining)})
continue
}
// Assistant tool_use batch: emit it, then pull the matching tool_result
// blocks (forward only) to sit immediately after it.
role := arr[i].Get("role").String()
emitted = append(emitted, entry{idx: i, role: role})
matchedRaw := make([]string, 0, len(ids))
srcMsg := -1
singleSrc := true
for _, id := range ids {
mi, bi, ok := findForwardResultBlock(blocks, blockTaken, consumed, i+1, id)
if !ok {
continue
}
blockTaken[mi][bi] = true
matchedRaw = append(matchedRaw, blocks[mi][bi].raw)
if srcMsg == -1 {
srcMsg = mi
} else if srcMsg != mi {
singleSrc = false
}
}
if len(matchedRaw) == 0 {
continue
}
// Byte-preservation fast path: when every matched result came from one
// still-unconsumed message that holds ONLY these tool_result blocks
// (nothing else, nothing extra), keep that message verbatim instead of
// rebuilding it. This is what keeps an already-canonical sequence
// byte-identical (idempotency).
if singleSrc && srcMsg >= 0 && !consumed[srcMsg] &&
len(matchedRaw) == len(blocks[srcMsg]) && allToolResults(blocks[srcMsg]) {
consumed[srcMsg] = true
emitted = append(emitted, entry{idx: srcMsg, role: "user"})
continue
}
emitted = append(emitted, entry{idx: -1, role: "user", raw: buildBlocksMessage("user", matchedRaw)})
}
// Merge consecutive same-role messages to satisfy strict role alternation
// required by Anthropic/Bedrock.
var merged []entry
for _, e := range emitted {
if len(merged) == 0 {
merged = append(merged, e)
continue
}
last := &merged[len(merged)-1]
if last.role == e.role {
blocks1 := getEntryContentBlocks(*last, arr)
blocks2 := getEntryContentBlocks(e, arr)
combined := append(blocks1, blocks2...)
last.idx = -1
last.raw = buildBlocksMessage(last.role, combined)
} else {
merged = append(merged, e)
}
}
emitted = merged
// Idempotency guard: if the emitted order is the identity permutation of the
// original messages (no splits, no moves, no merges), return the original bytes.
identity := len(emitted) == n
if identity {
for k, e := range emitted {
if e.idx != k {
identity = false
break
}
}
}
if identity {
return out
}
rebuilt := []byte(`[]`)
for _, e := range emitted {
if e.idx >= 0 {
rebuilt, _ = sjson.SetRawBytes(rebuilt, "-1", []byte(arr[e.idx].Raw))
} else {
rebuilt, _ = sjson.SetRawBytes(rebuilt, "-1", e.raw)
}
}
result, err := sjson.SetRawBytes(out, "messages", rebuilt)
if err != nil {
return out
}
return result
}
// getEntryContentBlocks extracts the raw content blocks of an entry.
func getEntryContentBlocks(e entry, arr []gjson.Result) []string {
if e.idx >= 0 {
content := arr[e.idx].Get("content")
if content.IsArray() {
var blocks []string
content.ForEach(func(_, b gjson.Result) bool {
blocks = append(blocks, b.Raw)
return true
})
return blocks
}
textBlock := []byte(`{"type":"text","text":""}`)
textBlock, _ = sjson.SetBytes(textBlock, "text", content.String())
return []string{string(textBlock)}
}
content := gjson.ParseBytes(e.raw).Get("content")
var blocks []string
content.ForEach(func(_, b gjson.Result) bool {
blocks = append(blocks, b.Raw)
return true
})
return blocks| if string(second) != string(first) { | ||
| t.Fatalf("reorder not idempotent on a truly-reordered sequence.\nfirst =%s\nsecond=%s", string(first), string(second)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add a test case to verify that the reordering pass maintains strict role alternation and successfully merges any consecutive same-role messages.
}
// TestConvertOpenAIResponses_ReorderWithInterveningMessageAvoidsConsecutiveSameRole verifies
// that when an intervening user message is moved after a tool_use/tool_result pair,
// any resulting consecutive user messages are merged to maintain strict role alternation.
func TestConvertOpenAIResponses_ReorderWithInterveningMessageAvoidsConsecutiveSameRole(t *testing.T) {
body := []byte(`{
"model": "kiro-api/claude-opus-4-7-thinking",
"input": [
{"type":"message","role":"user","content":[{"type":"input_text","text":"hi"}]},
{"type":"function_call","call_id":"toolu_a","name":"exec_command","arguments":"{\"cmd\":\"ls\"}"},
{"type":"message","role":"user","content":[{"type":"input_text","text":"[compact] resuming previous session"}]},
{"type":"function_call_output","call_id":"toolu_a","output":"file1\nfile2"}
],
"tools": []
}`)
out := ConvertOpenAIResponsesRequestToClaude("kiro-api/claude-opus-4-7-thinking", body, false)
msgs := gjson.GetBytes(out, "messages").Array()
// Verify strict role alternation
var lastRole string
for i, m := range msgs {
role := m.Get("role").String()
if role == lastRole {
t.Fatalf("consecutive same-role messages found at index %d: role=%q\nfull=%s", i, role, string(out))
}
lastRole = role
}
}There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c54c1c874
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| lastRole := gjson.GetBytes(out, fmt.Sprintf("messages.%d.role", lastIdx)).String() | ||
| lastContent := gjson.GetBytes(out, fmt.Sprintf("messages.%d.content", lastIdx)) | ||
| if lastRole == "assistant" && lastContent.IsArray() { | ||
| out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolUse) | ||
| appendedToolUse = true | ||
| } | ||
| } |
There was a problem hiding this comment.
Preserve pending reasoning when merging tool calls
When a reasoning item appears after an assistant message whose content is already an array and before the next function_call, this branch appends the new tool_use to the previous assistant message and skips the code that copies pendingReasoningParts and clears it. The pending thinking then gets flushed later (for example just before function_call_output), so the signed thinking block is moved after the tool call/result instead of being attached before the call as the old path did. Please avoid the append path whenever pendingReasoningParts is non-empty, or include those parts in the merged assistant content before appending the tool_use.
Useful? React with 👍 / 👎.
…e pending reasoning, guard tool_result container Address PR router-for-me#3791 review: - reorderClaudeToolUseResultPairs: add mergeConsecutiveSameRoleMessages post-pass. Moving an intervening message after a tool_use/tool_result pair could leave two consecutive same-role messages, which Anthropic/Bedrock reject with HTTP 400. Merge adjacent same-role messages by concatenating their content blocks; the identity/idempotency guards are preserved. - function_call append path: when appending a tool_use into an existing assistant array, first attach pending (signed) reasoning blocks to that message and clear them. Otherwise they linger and get flushed after the tool call/result, moving the signed thinking block out of order. - function_call_output append path: only append a tool_result into the previous user message when it is a genuine tool_result container (isAllToolResultBlocks); a multi-part text/image user array must not absorb a tool_result. - Added tests: strict-role-alternation, reasoning-before-appended-tool_use, tool_result-not-appended-into-non-tool-user-array; updated split-batch test's compact-survival check for the merged shape.
What
When converting OpenAI Responses history to Claude/Anthropic for Bedrock-backed models, parallel
function_call/function_call_outputitems can land as consecutive same-role messages or interleaved tool_use/tool_result pairs. Bedrock rejects these with a 400TOOL_USE_RESULT_MISMATCH(tool_use and its tool_result must stay paired and contiguous, with strict role alternation).This PR adds defensive pairing on the Claude responses-request path:
tool_use/tool_resultblock into the existing last same-role message instead of starting a new consecutive same-role message.reorderClaudeToolUseResultPairs): move any stray non-tool message out from between a tool_use and its matching tool_result so the pair stays contiguous.Test
go build ./...clean;go test ./internal/translator/...green (0 failed); 5 new pairing tests (Reorder / Idempotent / ParallelBatch / SplitBatch / IdempotentAfterRealReorder).Notes
Adds
claude_openai-responses_request_pairing.go+ minimal edits toclaude_openai-responses_request.go; no unrelated changes. Independent of #3790.