Skip to content

fix(translator/claude): pair tool_use/tool_result to avoid Bedrock 400#3791

Open
xiaojiou176 wants to merge 2 commits into
router-for-me:devfrom
xiaojiou176:pr-cpa-pairing
Open

fix(translator/claude): pair tool_use/tool_result to avoid Bedrock 400#3791
xiaojiou176 wants to merge 2 commits into
router-for-me:devfrom
xiaojiou176:pr-cpa-pairing

Conversation

@xiaojiou176

Copy link
Copy Markdown

What

When converting OpenAI Responses history to Claude/Anthropic for Bedrock-backed models, parallel function_call / function_call_output items can land as consecutive same-role messages or interleaved tool_use/tool_result pairs. Bedrock rejects these with a 400 TOOL_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:

  • Parallel-merge: append a tool_use / tool_result block into the existing last same-role message instead of starting a new consecutive same-role message.
  • Reorder pass (reorderClaudeToolUseResultPairs): move any stray non-tool message out from between a tool_use and its matching tool_result so the pair stays contiguous.
  • Idempotent: re-running the reorder on already-correct input is a no-op (covered by tests).

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 to claude_openai-responses_request.go; no unrelated changes. Independent of #3790.

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.
Copilot AI review requested due to automatic review settings June 9, 2026 22:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 reorderClaudeToolUseResultPairs to 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.

Comment on lines +363 to +380
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)
Comment on lines +357 to +363
// 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
Comment on lines +368 to +371
if lastRole == "assistant" && lastContent.IsArray() {
out, _ = sjson.SetRawBytes(out, fmt.Sprintf("messages.%d.content.-1", lastIdx), toolUse)
appendedToolUse = true
}
Comment on lines +396 to +405
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
}
}
Comment on lines +166 to +178
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

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +363 to +372
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
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

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
					}
				}

Comment on lines +42 to +179
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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

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))
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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
	}
}

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment on lines +366 to +372
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
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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.
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.

2 participants