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
36 changes: 36 additions & 0 deletions providers/bedrock/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,42 @@ func TestRequestMapper_ToolDefinitions(t *testing.T) {
assert.True(t, ok)
}

func TestRequestMapper_EmptyToolDescription(t *testing.T) {
t.Parallel()

cfg := &Config{
ModelName: "claude-sonnet-4-6",
setOptions: make(map[string]bool),
}

mapper := NewRequestMapper(cfg)

schema := json.RawMessage(`{"type":"object"}`)

req := &llm.Request{
Messages: []llm.Message{
llm.NewMessage(llm.RoleUser, llm.NewTextPart("hi")),
},
Tools: []llm.ToolDefinition{
{Name: "search", Description: "Search the web", Parameters: schema},
{Name: "subagent_a", Description: "", Parameters: schema},
{Name: "summarize", Description: "Summarize text", Parameters: schema},
{Name: "subagent_b", Description: "", Parameters: schema},
},
ToolChoice: &llm.ToolChoice{Type: llm.ToolChoiceAuto},
}

_, err := mapper.ToConverseInput(req)
require.Error(t, err)
require.ErrorIs(t, err, llm.ErrRequestMapping)
// All offenders surfaced in a single error so callers can fix in one pass.
assert.Contains(t, err.Error(), "subagent_a")
assert.Contains(t, err.Error(), "subagent_b")
// Tools with valid descriptions are not flagged.
assert.NotContains(t, err.Error(), "missing on: search")
assert.NotContains(t, err.Error(), "summarize,")
}

func TestRequestMapper_ToolChoiceSpecific(t *testing.T) {
t.Parallel()

Expand Down
17 changes: 17 additions & 0 deletions providers/bedrock/request_mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"

"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/service/bedrockruntime"
Expand Down Expand Up @@ -314,6 +315,22 @@ func (rm *RequestMapper) mapToolResultBlock(resp *llm.ToolResponse) types.ToolRe

// mapToolConfig converts tool definitions and choice to Bedrock ToolConfiguration.
func (rm *RequestMapper) mapToolConfig(tools []llm.ToolDefinition, choice *llm.ToolChoice) (*types.ToolConfiguration, error) {
// Bedrock Converse rejects any tool whose description is the empty
// string with an opaque ValidationException ("Member must have length
// greater than or equal to 1"). Validate upfront and surface every
// offender at once so callers can fix all of them in one pass.
var missingDesc []string

for _, tool := range tools {
if tool.Description == "" {
missingDesc = append(missingDesc, tool.Name)
}
}

if len(missingDesc) > 0 {
return nil, fmt.Errorf("bedrock: every tool must have a non-empty description; missing on: %s", strings.Join(missingDesc, ", "))
}
Comment on lines +324 to +332
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.

@simon0191 I was bothered by this solution and did some research. I noticed:

  1. Bedrock / AWS itself claims the tool description is NOT required, but when provided it can't be empty, see: https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_ToolSpecification.html
  2. This leads me to believe that this is the problem - we should omit it instead of setting an empty aws.String()

Have you tried this and see if that works?

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.

good catch!


apiTools := make([]types.Tool, 0, len(tools))

for _, tool := range tools {
Expand Down
Loading