Skip to content

Commit 3e7b0bf

Browse files
Aias00nacx
andauthored
fix: use correct slice index for Bedrock tool result coalescing (#2237)
**Description** Fix two bugs in the Anthropic-to-Bedrock translator message merging logic at `internal/translator/anthropic_awsbedrock.go`. **Bug 1: Wrong slice index (original report)** When system messages are present in the request body, `promoteAnthropicSystemMessagesToParam` filters them out, producing a shorter `messages` slice. The coalescing loop was using `body.Messages[i+1]` (the original unfiltered slice) instead of `messages[i+1]` (the filtered slice), causing index misalignment that could read wrong messages or panic with out-of-bounds access. **Bug 2: Coalescing logic was unreachable (root cause)** Tool result messages have `Role: "user"`, so they match `case anthropicschema.MessageRoleUser` and go through `convertUserMessage()` — which handles tool results individually but never coalesces consecutive ones. The coalescing logic in the `default` branch was dead code for tool results since they never reach that branch. **Changes** - Moved the `hasToolResult` check + coalescing loop into the `MessageRoleUser` case branch - Non-tool-result user messages still go through `convertUserMessage()` - Changed `body.Messages[i+1]` → `messages[i+1]` to use the correct filtered slice - Simplified the `default` branch to just return an error for unexpected roles - Updated existing test to expect coalesced output (3 messages instead of 4) - Added regression test with system messages to verify index alignment **Testing** - All existing translator tests pass - New `TestAnthropicToAWSBedrockTranslator_RequestBody_ToolResultMessagesWithSystemMessages` verifies the fix works when system messages are present alongside consecutive tool results --------- Signed-off-by: liuhy <liuhongyu@apache.org> Co-authored-by: Ignasi Barrera <ignasi@tetrate.io>
1 parent 3456009 commit 3e7b0bf

3 files changed

Lines changed: 300 additions & 43 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,4 @@ inference-extension-conformance-test-report.yaml
5252
/aigw
5353
site/.cursorrules
5454
/.cursor
55+
.worktrees/

internal/translator/anthropic_awsbedrock.go

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -78,36 +78,29 @@ func (a *anthropicToAWSBedrockTranslator) RequestBody(_ []byte, body *anthropics
7878
msg := &messages[i]
7979
switch msg.Role {
8080
case anthropicschema.MessageRoleUser:
81-
bedrockMsg, convErr := a.convertUserMessage(msg)
82-
if convErr != nil {
83-
return nil, nil, convErr
84-
}
85-
bedrockReq.Messages = append(bedrockReq.Messages, bedrockMsg)
86-
i++
87-
case anthropicschema.MessageRoleAssistant:
88-
bedrockMsg, convErr := a.convertAssistantMessage(msg)
89-
if convErr != nil {
90-
return nil, nil, convErr
91-
}
92-
bedrockReq.Messages = append(bedrockReq.Messages, bedrockMsg)
93-
i++
94-
default:
95-
// Check for tool_result content blocks (these come as "user" role in Anthropic
96-
// but we handle them specially).
9781
if hasToolResult(msg) {
9882
bedrockMsg := a.convertToolResultMessage(msg)
9983
// Coalesce consecutive tool result messages.
100-
for i+1 < msgLen && hasToolResult(&body.Messages[i+1]) {
101-
nextMsg := &body.Messages[i+1]
84+
for i+1 < msgLen && hasToolResult(&messages[i+1]) {
85+
nextMsg := &messages[i+1]
10286
nextBedrockMsg := a.convertToolResultMessage(nextMsg)
10387
bedrockMsg.Content = append(bedrockMsg.Content, nextBedrockMsg.Content...)
10488
i++
10589
}
10690
bedrockReq.Messages = append(bedrockReq.Messages, bedrockMsg)
10791
} else {
108-
return nil, nil, fmt.Errorf("%w: unexpected role: %s", internalapi.ErrInvalidRequestBody, msg.Role)
92+
bedrockMsg, convErr := a.convertUserMessage(msg)
93+
if convErr != nil {
94+
return nil, nil, convErr
95+
}
96+
bedrockReq.Messages = append(bedrockReq.Messages, bedrockMsg)
10997
}
11098
i++
99+
case anthropicschema.MessageRoleAssistant:
100+
bedrockReq.Messages = append(bedrockReq.Messages, a.convertAssistantMessage(msg))
101+
i++
102+
default:
103+
return nil, nil, fmt.Errorf("%w: unexpected role: %s", internalapi.ErrInvalidRequestBody, msg.Role)
111104
}
112105
}
113106

@@ -236,13 +229,13 @@ func (a *anthropicToAWSBedrockTranslator) convertUserMessage(msg *anthropicschem
236229
return bedrockMsg, nil
237230
}
238231

239-
func (a *anthropicToAWSBedrockTranslator) convertAssistantMessage(msg *anthropicschema.MessageParam) (*awsbedrock.Message, error) {
232+
func (a *anthropicToAWSBedrockTranslator) convertAssistantMessage(msg *anthropicschema.MessageParam) *awsbedrock.Message {
240233
bedrockMsg := &awsbedrock.Message{Role: awsbedrock.ConversationRoleAssistant}
241234
if msg.Content.Text != "" {
242235
bedrockMsg.Content = []*awsbedrock.ContentBlock{
243236
{Text: ptr.To(msg.Content.Text)},
244237
}
245-
return bedrockMsg, nil
238+
return bedrockMsg
246239
}
247240
bedrockMsg.Content = make([]*awsbedrock.ContentBlock, 0, len(msg.Content.Array))
248241
for i := range msg.Content.Array {
@@ -277,7 +270,7 @@ func (a *anthropicToAWSBedrockTranslator) convertAssistantMessage(msg *anthropic
277270
})
278271
}
279272
}
280-
return bedrockMsg, nil
273+
return bedrockMsg
281274
}
282275

283276
func (a *anthropicToAWSBedrockTranslator) convertToolResultMessage(msg *anthropicschema.MessageParam) *awsbedrock.Message {

0 commit comments

Comments
 (0)