fix(conversation): send tool_choice to anthropic as obj not str- #4531 (#4543)

Author: sicoyle

conversation/anthropic/anthropic.go

@@ -38,6 +38,7 @@ func NewAnthropic(logger logger.Logger) conversation.Conversation {
 		logger: logger,
 		LLM:    langchaingokit.New(logger),
 	}
+	a.SetProvider(langchaingokit.ProviderAnthropic)
 
 	return a
 }

conversation/langchaingokit/model.go

@@ -25,11 +25,18 @@ import (
 	"github.com/dapr/components-contrib/conversation"
 )
 
+// Provider identifies the upstream LLM API so that request options can be translated
+// into the wire format it expects. The zero value means no translation is applied.
+type Provider string
+
+const ProviderAnthropic Provider = "anthropic"
+
 // LLM is a helper struct that wraps a LangChain Go model
 type LLM struct {
 	llms.Model
-	model  string
-	logger logger.Logger
+	model    string
+	provider Provider
+	logger   logger.Logger
 }
 
 func New(logger logger.Logger) LLM {
@@ -38,8 +45,18 @@ func New(logger logger.Logger) LLM {
 	}
 }
 
+// SetProvider records which upstream API this model talks to so that request
+// options needing a provider-specific wire format are translated correctly.
+func (a *LLM) SetProvider(provider Provider) {
+	a.provider = provider
+}
+
+func (a *LLM) GetProvider() Provider {
+	return a.provider
+}
+
 func (a *LLM) Converse(ctx context.Context, r *conversation.Request) (res *conversation.Response, err error) {
-	opts := getOptionsFromRequest(r, a.logger)
+	opts := getOptionsFromRequest(r, a.provider, a.logger)
 
 	var messages []llms.MessageContent
 	if r.Message != nil {
@@ -58,7 +75,7 @@ func (a *LLM) Converse(ctx context.Context, r *conversation.Request) (res *conve
 
 	// If tools were provided but the LLM returned neither content nor tool calls
 	// across any choice, treat it as a retriable error rather than silently succeeding.
-	if r.ToolChoice != nil && *r.ToolChoice == "required" && r.Tools != nil && len(*r.Tools) > 0 {
+	if r.ToolChoice != nil && (*r.ToolChoice == "required" || *r.ToolChoice == "any") && r.Tools != nil && len(*r.Tools) > 0 {
 		hasUsefulResponse := false
 		for _, output := range outputs {
 			for _, choice := range output.Choices {
@@ -134,7 +151,7 @@ func (a *LLM) NormalizeConverseResult(choices []*llms.ContentChoice) ([]conversa
 	return outputs, usage, nil
 }
 
-func getOptionsFromRequest(r *conversation.Request, logger logger.Logger, opts ...llms.CallOption) []llms.CallOption {
+func getOptionsFromRequest(r *conversation.Request, provider Provider, logger logger.Logger, opts ...llms.CallOption) []llms.CallOption {
 	if opts == nil {
 		opts = make([]llms.CallOption, 0)
 	}
@@ -148,7 +165,10 @@ func getOptionsFromRequest(r *conversation.Request, logger logger.Logger, opts .
 	}
 
 	if r.ToolChoice != nil {
-		opts = append(opts, llms.WithToolChoice(r.ToolChoice))
+		hasTools := r.Tools != nil && len(*r.Tools) > 0
+		if toolChoice := translateToolChoice(*r.ToolChoice, provider, hasTools); toolChoice != nil {
+			opts = append(opts, llms.WithToolChoice(toolChoice))
+		}
 	}
 
 	if r.ResponseFormatAsJSONSchema != nil {

conversation/langchaingokit/model_test.go

@@ -164,6 +164,24 @@ func TestConverseEmptyResponseWithTools(t *testing.T) {
 			wantErr:    true,
 			errSubstr:  "LLM returned empty response with no tool calls",
 		},
+		{
+			name: "empty content no tool calls with tools provided and tool_choice=any - returns error",
+			choices: []*llms.ContentChoice{
+				{Content: "", StopReason: "stop"},
+			},
+			tools:      &tools,
+			toolChoice: strPtr("any"),
+			wantErr:    true,
+			errSubstr:  "LLM returned empty response with no tool calls",
+		},
+		{
+			name:       "empty choices slice with tool_choice=any - returns error",
+			choices:    []*llms.ContentChoice{},
+			tools:      &tools,
+			toolChoice: strPtr("any"),
+			wantErr:    true,
+			errSubstr:  "LLM returned empty response with no tool calls",
+		},
 	}
 
 	for _, tt := range tests {

conversation/langchaingokit/translate.go

@@ -36,6 +36,62 @@ const (
 	promptCachedKey       = "PromptCachedTokens"
 )
 
+// Provider-agnostic tool choice values accepted on conversation.Request.ToolChoice.
+const (
+	toolChoiceAuto     = "auto"
+	toolChoiceRequired = "required"
+	toolChoiceAny      = "any"
+	toolChoiceNone     = "none"
+)
+
+// Keys and type values of Anthropic's tool_choice object.
+// ref: https://docs.claude.com/en/api/messages
+const (
+	anthropicToolChoiceTypeKey = "type"
+	anthropicToolChoiceNameKey = "name"
+	anthropicToolChoiceAny     = "any"
+	anthropicToolChoiceNone    = "none"
+	anthropicToolChoiceTool    = "tool"
+)
+
+// translateToolChoice converts the provider-agnostic tool choice into the shape the
+// provider's API expects, where a nil result means the option must be omitted.
+// Anthropic requires an object and rejects the bare string that OpenAI accepts.
+// The result depends only on its arguments: Anthropic invalidates cached message blocks
+// whenever tool_choice changes between turns, so a value must not be omitted on one turn
+// and sent on the next.
+// Caveat: with manual extended thinking enabled, Anthropic accepts only the auto and none
+// forms and 400s on the forced any and tool forms. This package never enables extended
+// thinking, as it does not pass llms.WithReasoning, so that combination is unreachable.
+func translateToolChoice(toolChoice string, provider Provider, hasTools bool) any {
+	if provider != ProviderAnthropic {
+		if toolChoice == toolChoiceAny {
+			return toolChoiceRequired
+		}
+		return toolChoice
+	}
+
+	// Anthropic rejects any tool_choice when the request carries no tools.
+	if !hasTools {
+		return nil
+	}
+
+	switch toolChoice {
+	case toolChoiceAuto:
+		// Anthropic already defaults to {"type": "auto"} when tools are present.
+		return nil
+	case toolChoiceRequired, toolChoiceAny:
+		return map[string]any{anthropicToolChoiceTypeKey: anthropicToolChoiceAny}
+	case toolChoiceNone:
+		return map[string]any{anthropicToolChoiceTypeKey: anthropicToolChoiceNone}
+	default:
+		return map[string]any{
+			anthropicToolChoiceTypeKey: anthropicToolChoiceTool,
+			anthropicToolChoiceNameKey: toolChoice,
+		}
+	}
+}
+
 // extractUint64FromGenInfo extracts a uint64 value from genInfo map to extract usage data from langchaingo's GenerationInfo map in the choices response.
 func extractUint64FromGenInfo(genInfo map[string]any, key string) (uint64, error) {
 	if v, ok := genInfo[key]; ok {

conversation/langchaingokit/translate_test.go

@@ -279,10 +279,103 @@ func TestExtractUsageFromLangchainGenerationInfo(t *testing.T) {
 	}
 }
 
+// resolveCallOptions applies the built options so assertions can inspect the resolved values.
+func resolveCallOptions(opts []llms.CallOption) llms.CallOptions {
+	var resolved llms.CallOptions
+	for _, opt := range opts {
+		opt(&resolved)
+	}
+	return resolved
+}
+
+func TestTranslateToolChoice(t *testing.T) {
+	tests := map[string]struct {
+		toolChoice string
+		provider   Provider
+		hasTools   bool
+		expected   any
+	}{
+		"non-anthropic provider passes the bare string through": {
+			toolChoice: "required",
+			hasTools:   true,
+			expected:   "required",
+		},
+		"non-anthropic provider passes a named tool through": {
+			toolChoice: "get_weather",
+			hasTools:   true,
+			expected:   "get_weather",
+		},
+		"non-anthropic provider passes through without tools": {
+			toolChoice: "required",
+			expected:   "required",
+		},
+		"anthropic auto is omitted": {
+			toolChoice: "auto",
+			provider:   ProviderAnthropic,
+			hasTools:   true,
+			expected:   nil,
+		},
+		"anthropic required maps to any": {
+			toolChoice: "required",
+			provider:   ProviderAnthropic,
+			hasTools:   true,
+			expected:   map[string]any{"type": "any"},
+		},
+		"anthropic any maps to any": {
+			toolChoice: "any",
+			provider:   ProviderAnthropic,
+			hasTools:   true,
+			expected:   map[string]any{"type": "any"},
+		},
+		"anthropic none maps to none": {
+			toolChoice: "none",
+			provider:   ProviderAnthropic,
+			hasTools:   true,
+			expected:   map[string]any{"type": "none"},
+		},
+		"anthropic named tool maps to tool": {
+			toolChoice: "get_weather",
+			provider:   ProviderAnthropic,
+			hasTools:   true,
+			expected:   map[string]any{"type": "tool", "name": "get_weather"},
+		},
+		"anthropic omits required without tools": {
+			toolChoice: "required",
+			provider:   ProviderAnthropic,
+			expected:   nil,
+		},
+		"anthropic omits a named tool without tools": {
+			toolChoice: "get_weather",
+			provider:   ProviderAnthropic,
+			expected:   nil,
+		},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			assert.Equal(t, tt.expected, translateToolChoice(tt.toolChoice, tt.provider, tt.hasTools))
+		})
+	}
+}
+
+// TestTranslateToolChoiceIsDeterministic guards the caching contract: Anthropic invalidates
+// cached message blocks when tool_choice changes, so repeated calls must not vary.
+func TestTranslateToolChoiceIsDeterministic(t *testing.T) {
+	for _, toolChoice := range []string{"auto", "required", "any", "none", "get_weather"} {
+		t.Run(toolChoice, func(t *testing.T) {
+			first := translateToolChoice(toolChoice, ProviderAnthropic, true)
+			for range 3 {
+				assert.Equal(t, first, translateToolChoice(toolChoice, ProviderAnthropic, true))
+			}
+		})
+	}
+}
+
 func TestGetOptionsFromRequest(t *testing.T) {
 	log := logger.NewLogger("test")
 
 	toolChoice := "auto"
+	requiredToolChoice := "required"
 	tools := []llms.Tool{
 		{
 			Type: "function",
@@ -296,6 +389,7 @@ func TestGetOptionsFromRequest(t *testing.T) {
 
 	tests := map[string]struct {
 		request      *conversation.Request
+		provider     Provider
 		existingOpts []llms.CallOption
 		validate     func(t *testing.T, r *conversation.Request, opts []llms.CallOption)
 	}{
@@ -346,6 +440,38 @@ func TestGetOptionsFromRequest(t *testing.T) {
 			},
 			validate: func(t *testing.T, r *conversation.Request, opts []llms.CallOption) {
 				assert.Len(t, opts, 2)
+				assert.Equal(t, "auto", resolveCallOptions(opts).ToolChoice)
+			},
+		},
+		"anthropic auto tool choice omits the option": {
+			request: &conversation.Request{
+				Tools:      &tools,
+				ToolChoice: &toolChoice,
+			},
+			provider: ProviderAnthropic,
+			validate: func(t *testing.T, r *conversation.Request, opts []llms.CallOption) {
+				assert.Len(t, opts, 1)
+				assert.Nil(t, resolveCallOptions(opts).ToolChoice)
+			},
+		},
+		"anthropic required tool choice sets the object form": {
+			request: &conversation.Request{
+				Tools:      &tools,
+				ToolChoice: &requiredToolChoice,
+			},
+			provider: ProviderAnthropic,
+			validate: func(t *testing.T, r *conversation.Request, opts []llms.CallOption) {
+				assert.Len(t, opts, 2)
+				assert.Equal(t, map[string]any{"type": "any"}, resolveCallOptions(opts).ToolChoice)
+			},
+		},
+		"anthropic tool choice without tools omits the option": {
+			request: &conversation.Request{
+				ToolChoice: &requiredToolChoice,
+			},
+			provider: ProviderAnthropic,
+			validate: func(t *testing.T, r *conversation.Request, opts []llms.CallOption) {
+				assert.Empty(t, opts)
 			},
 		},
 		"metadata sets option": {
@@ -382,7 +508,7 @@ func TestGetOptionsFromRequest(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			assert.NotPanics(t, func() {
-				opts := getOptionsFromRequest(tt.request, log, tt.existingOpts...)
+				opts := getOptionsFromRequest(tt.request, tt.provider, log, tt.existingOpts...)
 				tt.validate(t, tt.request, opts)
 			})
 		})

tests/conformance/conversation/conversation.go

@@ -407,6 +407,66 @@ func ConformanceTests(t *testing.T, props map[string]string, conv conversation.C
 			}
 		})
 
+		t.Run("test tool calling with an explicit tool choice", func(t *testing.T) {
+			tools := []llms.Tool{
+				{
+					Type: "function",
+					Function: &llms.FunctionDefinition{
+						Name:        "get_project_name",
+						Description: "Get the name of an open source project",
+						Parameters: map[string]any{
+							"type": "object",
+							"properties": map[string]any{
+								"repo_link": map[string]any{
+									"type":        "string",
+									"description": "The repository link",
+								},
+							},
+							"required": []string{"repo_link"},
+						},
+					},
+				},
+			}
+
+			toolChoices := []string{"auto", "required"}
+			// A named tool must be sent as an object to OpenAI too, which this layer
+			// does not translate yet, so only Anthropic exercises that form for now.
+			if component == "anthropic" {
+				toolChoices = append(toolChoices, "get_project_name")
+			}
+
+			for _, toolChoice := range toolChoices {
+				t.Run(toolChoice, func(t *testing.T) {
+					ctx, cancel := context.WithTimeout(t.Context(), 25*time.Second)
+					defer cancel()
+
+					messages := []llms.MessageContent{
+						{
+							Role: llms.ChatMessageTypeHuman,
+							Parts: []llms.ContentPart{
+								llms.TextContent{Text: "What is this open source project called?"},
+							},
+						},
+					}
+
+					req := &conversation.Request{
+						Message:    &messages,
+						Tools:      &tools,
+						ToolChoice: &toolChoice,
+					}
+					if component == "openai" {
+						req.Temperature = 1
+					}
+
+					// Anthropic rejects a bare string tool_choice with a 400, so the
+					// request succeeding at all is what this guards against.
+					resp, err := conv.Converse(ctx, req)
+					require.NoError(t, err)
+					require.NotEmpty(t, resp.Outputs)
+				})
+			}
+		})
+
 		t.Run("test conversation history with tool calls", func(t *testing.T) {
 			ctx, cancel := context.WithTimeout(t.Context(), 25*time.Second)
 			defer cancel()