Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 150 additions & 0 deletions core/providers/bedrock/bedrock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,156 @@ func TestBifrostToBedrockRequestConversion(t *testing.T) {
AdditionalModelResponseFieldPaths: []string{"field1", "field2"},
},
},
{
name: "ParallelToolCalls",
input: &schemas.BifrostChatRequest{
Model: "claude-3-sonnet",
Input: []schemas.ChatMessage{
{
Role: schemas.ChatMessageRoleUser,
Content: &schemas.ChatMessageContent{
ContentStr: schemas.Ptr("Invoke all tools in parallel that are available to you"),
},
},
{
Role: schemas.ChatMessageRoleAssistant,
Content: &schemas.ChatMessageContent{
ContentStr: schemas.Ptr("I'll invoke both available tools in parallel for you."),
},
ChatAssistantMessage: &schemas.ChatAssistantMessage{
ToolCalls: []schemas.ChatAssistantMessageToolCall{
{
Index: 0,
Type: schemas.Ptr("function"),
ID: schemas.Ptr("tooluse_Yl388l8ES0G_3TQtDcKq_g"),
Function: schemas.ChatAssistantMessageToolCallFunction{
Name: schemas.Ptr("hello"),
Arguments: "{}",
},
},
{
Index: 1,
Type: schemas.Ptr("function"),
ID: schemas.Ptr("tooluse_eARDw2iqRXak8uyRC2KxXw"),
Function: schemas.ChatAssistantMessageToolCallFunction{
Name: schemas.Ptr("world"),
Arguments: "{}",
},
},
},
},
},
{
Role: schemas.ChatMessageRoleTool,
Content: &schemas.ChatMessageContent{
ContentStr: schemas.Ptr("Hello"),
},
ChatToolMessage: &schemas.ChatToolMessage{
ToolCallID: schemas.Ptr("tooluse_Yl388l8ES0G_3TQtDcKq_g"),
},
},
{
Role: schemas.ChatMessageRoleTool,
Content: &schemas.ChatMessageContent{
ContentStr: schemas.Ptr("World"),
},
ChatToolMessage: &schemas.ChatToolMessage{
ToolCallID: schemas.Ptr("tooluse_eARDw2iqRXak8uyRC2KxXw"),
},
},
},
},
expected: &bedrock.BedrockConverseRequest{
ModelID: "claude-3-sonnet",
Messages: []bedrock.BedrockMessage{
{
Role: bedrock.BedrockMessageRoleUser,
Content: []bedrock.BedrockContentBlock{
{
Text: schemas.Ptr("Invoke all tools in parallel that are available to you"),
},
},
},
{
Role: bedrock.BedrockMessageRoleAssistant,
Content: []bedrock.BedrockContentBlock{
{
Text: schemas.Ptr("I'll invoke both available tools in parallel for you."),
},
{
ToolUse: &bedrock.BedrockToolUse{
ToolUseID: "tooluse_Yl388l8ES0G_3TQtDcKq_g",
Name: "hello",
Input: map[string]interface{}{},
},
},
{
ToolUse: &bedrock.BedrockToolUse{
ToolUseID: "tooluse_eARDw2iqRXak8uyRC2KxXw",
Name: "world",
Input: map[string]interface{}{},
},
},
},
},
{
Role: bedrock.BedrockMessageRoleUser,
Content: []bedrock.BedrockContentBlock{
{
ToolResult: &bedrock.BedrockToolResult{
ToolUseID: "tooluse_Yl388l8ES0G_3TQtDcKq_g",
Content: []bedrock.BedrockContentBlock{
{
Text: schemas.Ptr("Hello"),
},
},
Status: schemas.Ptr("success"),
},
},
{
ToolResult: &bedrock.BedrockToolResult{
ToolUseID: "tooluse_eARDw2iqRXak8uyRC2KxXw",
Content: []bedrock.BedrockContentBlock{
{
Text: schemas.Ptr("World"),
},
},
Status: schemas.Ptr("success"),
},
},
},
},
},
ToolConfig: &bedrock.BedrockToolConfig{
Tools: []bedrock.BedrockTool{
{
ToolSpec: &bedrock.BedrockToolSpec{
Name: "hello",
Description: schemas.Ptr("Tool extracted from conversation history"),
InputSchema: bedrock.BedrockToolInputSchema{
JSON: map[string]interface{}{
"type": "object",
"properties": map[string]interface{}{},
},
},
},
},
{
ToolSpec: &bedrock.BedrockToolSpec{
Name: "world",
Description: schemas.Ptr("Tool extracted from conversation history"),
InputSchema: bedrock.BedrockToolInputSchema{
JSON: map[string]interface{}{
"type": "object",
"properties": map[string]interface{}{},
},
},
},
},
},
},
},
},
{
name: "NilRequest",
input: nil,
Expand Down
145 changes: 80 additions & 65 deletions core/providers/bedrock/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ func convertMessages(bifrostMessages []schemas.ChatMessage) ([]BedrockMessage, [
var messages []BedrockMessage
var systemMessages []BedrockSystemMessage

for _, msg := range bifrostMessages {
for i := 0; i < len(bifrostMessages); i++ {
msg := bifrostMessages[i]
switch msg.Role {
case schemas.ChatMessageRoleSystem:
// Convert system message
Expand All @@ -290,10 +291,20 @@ func convertMessages(bifrostMessages []schemas.ChatMessage) ([]BedrockMessage, [
messages = append(messages, bedrockMsg)

case schemas.ChatMessageRoleTool:
// Convert tool message - this should be part of the conversation
bedrockMsg, err := convertToolMessage(msg)
// Collect all consecutive tool messages and group them into a single user message
var toolMessages []schemas.ChatMessage
toolMessages = append(toolMessages, msg)

// Look ahead for more consecutive tool messages
for j := i + 1; j < len(bifrostMessages) && bifrostMessages[j].Role == schemas.ChatMessageRoleTool; j++ {
toolMessages = append(toolMessages, bifrostMessages[j])
i = j
}

// Convert all collected tool messages into a single Bedrock message
bedrockMsg, err := convertToolMessages(toolMessages)
if err != nil {
return nil, nil, fmt.Errorf("failed to convert tool message: %w", err)
return nil, nil, fmt.Errorf("failed to convert tool messages: %w", err)
}
messages = append(messages, bedrockMsg)

Expand Down Expand Up @@ -362,83 +373,87 @@ func convertMessage(msg schemas.ChatMessage) (BedrockMessage, error) {
return bedrockMsg, nil
}

// convertToolMessage converts a Bifrost tool message to Bedrock format
func convertToolMessage(msg schemas.ChatMessage) (BedrockMessage, error) {
bedrockMsg := BedrockMessage{
Role: "user", // Tool messages are typically treated as user messages in Bedrock
// convertToolMessages converts multiple consecutive Bifrost tool messages to a single Bedrock message
func convertToolMessages(msgs []schemas.ChatMessage) (BedrockMessage, error) {
if len(msgs) == 0 {
return BedrockMessage{}, fmt.Errorf("no tool messages provided")
}

// Tool messages should have a tool_call_id
if msg.ChatToolMessage == nil || msg.ChatToolMessage.ToolCallID == nil {
return BedrockMessage{}, fmt.Errorf("tool message missing tool_call_id")
bedrockMsg := BedrockMessage{
Role: "user",
}

// Convert content to tool result
var toolResultContent []BedrockContentBlock
if msg.Content.ContentStr != nil {
// Bedrock expects JSON to be a parsed object, not a string
// Try to unmarshal the string content as JSON
var parsedOutput interface{}
if err := json.Unmarshal([]byte(*msg.Content.ContentStr), &parsedOutput); err != nil {
// If it's not valid JSON, wrap it as a text block instead
toolResultContent = append(toolResultContent, BedrockContentBlock{
Text: msg.Content.ContentStr,
})
} else {
// Use the parsed JSON object
toolResultContent = append(toolResultContent, BedrockContentBlock{
JSON: parsedOutput,
})
}
} else if msg.Content.ContentBlocks != nil {
for _, block := range msg.Content.ContentBlocks {
switch block.Type {
case schemas.ChatContentBlockTypeText:
if block.Text != nil {
toolResultContent = append(toolResultContent, BedrockContentBlock{
Text: block.Text,
})
// Cache point must be in a separate block
if block.CacheControl != nil {
var contentBlocks []BedrockContentBlock

for _, msg := range msgs {
var toolResultContent []BedrockContentBlock
if msg.Content.ContentStr != nil {
// Bedrock expects JSON to be a parsed object, not a string
// Try to unmarshal the string content as JSON
var parsedOutput interface{}
if err := json.Unmarshal([]byte(*msg.Content.ContentStr), &parsedOutput); err != nil {
// If it's not valid JSON, wrap it as a text block instead
toolResultContent = append(toolResultContent, BedrockContentBlock{
Text: msg.Content.ContentStr,
})
} else {
// Use the parsed JSON object
toolResultContent = append(toolResultContent, BedrockContentBlock{
JSON: parsedOutput,
})
}
} else if msg.Content.ContentBlocks != nil {
for _, block := range msg.Content.ContentBlocks {
switch block.Type {
case schemas.ChatContentBlockTypeText:
if block.Text != nil {
toolResultContent = append(toolResultContent, BedrockContentBlock{
CachePoint: &BedrockCachePoint{
Type: BedrockCachePointTypeDefault,
},
Text: block.Text,
})
// Cache point must be in a separate block
if block.CacheControl != nil {
toolResultContent = append(toolResultContent, BedrockContentBlock{
CachePoint: &BedrockCachePoint{
Type: BedrockCachePointTypeDefault,
},
})
}
}
}
case schemas.ChatContentBlockTypeImage:
if block.ImageURLStruct != nil {
imageSource, err := convertImageToBedrockSource(block.ImageURLStruct.URL)
if err != nil {
return BedrockMessage{}, fmt.Errorf("failed to convert image in tool result: %w", err)
}
toolResultContent = append(toolResultContent, BedrockContentBlock{
Image: imageSource,
})
// Cache point must be in a separate block
if block.CacheControl != nil {
case schemas.ChatContentBlockTypeImage:
if block.ImageURLStruct != nil {
imageSource, err := convertImageToBedrockSource(block.ImageURLStruct.URL)
if err != nil {
return BedrockMessage{}, fmt.Errorf("failed to convert image in tool result: %w", err)
}
toolResultContent = append(toolResultContent, BedrockContentBlock{
CachePoint: &BedrockCachePoint{
Type: BedrockCachePointTypeDefault,
},
Image: imageSource,
})
// Cache point must be in a separate block
if block.CacheControl != nil {
toolResultContent = append(toolResultContent, BedrockContentBlock{
CachePoint: &BedrockCachePoint{
Type: BedrockCachePointTypeDefault,
},
})
}
}
}
}
}
}

// Create tool result content block
toolResultBlock := BedrockContentBlock{
ToolResult: &BedrockToolResult{
ToolUseID: *msg.ChatToolMessage.ToolCallID,
Content: toolResultContent,
Status: schemas.Ptr("success"), // Default to success
},
// Create tool result content block for this tool message
toolResultBlock := BedrockContentBlock{
ToolResult: &BedrockToolResult{
ToolUseID: *msg.ChatToolMessage.ToolCallID,
Content: toolResultContent,
Status: schemas.Ptr("success"), // Default to success
},
}

contentBlocks = append(contentBlocks, toolResultBlock)
Comment on lines +445 to +453
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add nil check for ToolCallID to prevent potential panic.

Line 447 dereferences msg.ChatToolMessage.ToolCallID without checking if it's nil. While the schema design suggests this should always be present for tool messages, defensive programming is recommended to prevent runtime panics on malformed input.

🔎 Proposed fix
+		// Validate required field
+		if msg.ChatToolMessage.ToolCallID == nil {
+			return BedrockMessage{}, fmt.Errorf("tool message missing required ToolCallID")
+		}
+
 		// Create tool result content block for this tool message
 		toolResultBlock := BedrockContentBlock{
 			ToolResult: &BedrockToolResult{
 				ToolUseID: *msg.ChatToolMessage.ToolCallID,
 				Content:   toolResultContent,
 				Status:    schemas.Ptr("success"), // Default to success
 			},
 		}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In core/providers/bedrock/utils.go around lines 445 to 453, the code
dereferences msg.ChatToolMessage.ToolCallID without checking for nil; add a nil
check before using *msg.ChatToolMessage.ToolCallID and handle the missing ID
defensively — e.g., if ToolCallID is nil, log a warning (using the existing
logger in scope or fmt.Printf if none) and skip creating/appending the
toolResultBlock (or create it with a safe fallback ID and an error status),
otherwise proceed to dereference and append as before.

}

bedrockMsg.Content = []BedrockContentBlock{toolResultBlock}
bedrockMsg.Content = contentBlocks
return bedrockMsg, nil
}

Expand Down