mirror of
https://github.com/sipeed/picoclaw.git
synced 2026-07-28 01:27:58 +00:00
Merge pull request #3227 from AayushGupta16/fix/anthropic-tool-use-function-fallback
fix(providers): resolve tool_use name/args from Function on reloaded history
This commit is contained in:
@@ -181,8 +181,15 @@ func buildParams(
|
||||
blocks = append(blocks, anthropic.NewTextBlock(msg.Content))
|
||||
}
|
||||
for _, tc := range msg.ToolCalls {
|
||||
// Resolve tool name: prefer tc.Name, fallback to tc.Function.Name
|
||||
// (tc.Name/tc.Arguments are json:"-" and may be empty when
|
||||
// history is reloaded from the session store)
|
||||
toolName := tc.Name
|
||||
if toolName == "" && tc.Function != nil {
|
||||
toolName = tc.Function.Name
|
||||
}
|
||||
// Skip tool calls with empty names to avoid API errors
|
||||
if tc.Name == "" {
|
||||
if toolName == "" {
|
||||
continue
|
||||
}
|
||||
args := tc.Arguments
|
||||
@@ -194,7 +201,7 @@ func buildParams(
|
||||
if args == nil {
|
||||
args = map[string]any{}
|
||||
}
|
||||
blocks = append(blocks, anthropic.NewToolUseBlock(tc.ID, args, tc.Name))
|
||||
blocks = append(blocks, anthropic.NewToolUseBlock(tc.ID, args, toolName))
|
||||
}
|
||||
anthropicMessages = append(anthropicMessages, anthropic.NewAssistantMessage(blocks...))
|
||||
} else {
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"reflect"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
@@ -77,6 +78,124 @@ func TestBuildParams_ToolCallMessage(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildParams_ToolCallFunctionFallback verifies that tool calls whose
|
||||
// runtime-only fields were lost in a JSON round-trip through the session store
|
||||
// (ToolCall.Name/Arguments are json:"-"; only ToolCall.Function survives) fall
|
||||
// back to Function.Name / Function.Arguments, so the tool_use block is still
|
||||
// emitted and its tool_result pair stays intact. Without the fallback the
|
||||
// tool_use is skipped and the orphaned tool_result 400s at the API
|
||||
// ("unexpected tool_use_id found in tool_result blocks").
|
||||
func TestBuildParams_ToolCallFunctionFallback(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
toolCall ToolCall
|
||||
wantSkipped bool
|
||||
wantToolName string
|
||||
wantInput map[string]any
|
||||
}{
|
||||
{
|
||||
name: "deserialized history shape falls back to Function fields",
|
||||
toolCall: ToolCall{
|
||||
ID: "toolu-fallback-1",
|
||||
Name: "",
|
||||
Arguments: nil,
|
||||
Function: &FunctionCall{Name: "x", Arguments: `{"a":1}`},
|
||||
},
|
||||
wantToolName: "x",
|
||||
wantInput: map[string]any{"a": float64(1)},
|
||||
},
|
||||
{
|
||||
name: "runtime shape with Name set and Function nil still works",
|
||||
toolCall: ToolCall{
|
||||
ID: "toolu-runtime-1",
|
||||
Name: "y",
|
||||
Arguments: map[string]any{"b": 2},
|
||||
},
|
||||
wantToolName: "y",
|
||||
wantInput: map[string]any{"b": 2},
|
||||
},
|
||||
{
|
||||
name: "both Name and Function.Name empty is skipped",
|
||||
toolCall: ToolCall{
|
||||
ID: "toolu-empty-1",
|
||||
Name: "",
|
||||
Function: &FunctionCall{Name: "", Arguments: `{"c":3}`},
|
||||
},
|
||||
wantSkipped: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
messages := []Message{
|
||||
{Role: "user", Content: "run the tool"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []ToolCall{tt.toolCall}},
|
||||
{Role: "tool", ToolCallID: tt.toolCall.ID, Content: "result"},
|
||||
}
|
||||
|
||||
params, err := buildParams(messages, nil, "claude-sonnet-4.6", map[string]any{})
|
||||
if err != nil {
|
||||
t.Fatalf("buildParams() error: %v", err)
|
||||
}
|
||||
if len(params.Messages) != 3 {
|
||||
t.Fatalf("len(Messages) = %d, want 3", len(params.Messages))
|
||||
}
|
||||
|
||||
assistantMsg := params.Messages[1]
|
||||
var toolUses []*anthropic.ToolUseBlockParam
|
||||
for _, block := range assistantMsg.Content {
|
||||
if block.OfToolUse != nil {
|
||||
toolUses = append(toolUses, block.OfToolUse)
|
||||
}
|
||||
}
|
||||
|
||||
// The tool_result in the following user message always carries the
|
||||
// original ID; look it up once for both branches.
|
||||
toolResultMsg := params.Messages[2]
|
||||
if len(toolResultMsg.Content) != 1 || toolResultMsg.Content[0].OfToolResult == nil {
|
||||
t.Fatalf("message after assistant = %+v, want single tool_result block", toolResultMsg.Content)
|
||||
}
|
||||
toolResult := toolResultMsg.Content[0].OfToolResult
|
||||
|
||||
if tt.wantSkipped {
|
||||
if len(toolUses) != 0 {
|
||||
t.Fatalf("tool_use blocks = %d, want 0 (tool call skipped)", len(toolUses))
|
||||
}
|
||||
// Note: matching current behavior, the orphaned tool_result is
|
||||
// still emitted even though its tool_use block was skipped.
|
||||
if toolResult.ToolUseID != tt.toolCall.ID {
|
||||
t.Fatalf("orphaned tool_result ToolUseID = %q, want %q",
|
||||
toolResult.ToolUseID, tt.toolCall.ID)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// (a) tool_use block emitted with resolved name, id, and parsed input.
|
||||
if len(toolUses) != 1 {
|
||||
t.Fatalf("tool_use blocks = %d, want 1", len(toolUses))
|
||||
}
|
||||
toolUse := toolUses[0]
|
||||
if toolUse.Name != tt.wantToolName {
|
||||
t.Errorf("tool_use Name = %q, want %q", toolUse.Name, tt.wantToolName)
|
||||
}
|
||||
if toolUse.ID != tt.toolCall.ID {
|
||||
t.Errorf("tool_use ID = %q, want %q", toolUse.ID, tt.toolCall.ID)
|
||||
}
|
||||
gotInput, ok := toolUse.Input.(map[string]any)
|
||||
if !ok || !reflect.DeepEqual(gotInput, tt.wantInput) {
|
||||
t.Errorf("tool_use Input = %#v, want %#v", toolUse.Input, tt.wantInput)
|
||||
}
|
||||
|
||||
// (b) the following user message's tool_result references the same
|
||||
// id as the tool_use block — the pair is intact.
|
||||
if toolResult.ToolUseID != toolUse.ID {
|
||||
t.Errorf("tool_result ToolUseID = %q, want %q (paired with tool_use)",
|
||||
toolResult.ToolUseID, toolUse.ID)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildParams_WithTools(t *testing.T) {
|
||||
tools := []ToolDefinition{
|
||||
{
|
||||
|
||||
@@ -233,12 +233,26 @@ func buildRequestBody(
|
||||
|
||||
// Add tool_use blocks
|
||||
for _, tc := range msg.ToolCalls {
|
||||
if strings.TrimSpace(tc.Name) == "" {
|
||||
// Resolve tool name: prefer tc.Name, fallback to tc.Function.Name
|
||||
// (tc.Name/tc.Arguments are json:"-" and may be empty when
|
||||
// history is reloaded from the session store)
|
||||
toolName := tc.Name
|
||||
if toolName == "" && tc.Function != nil {
|
||||
toolName = tc.Function.Name
|
||||
}
|
||||
if strings.TrimSpace(toolName) == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
// Handle nil Arguments (GLM-4 may return null input)
|
||||
// Resolve arguments: prefer tc.Arguments, fallback to parsing
|
||||
// tc.Function.Arguments
|
||||
input := tc.Arguments
|
||||
if input == nil && tc.Function != nil && tc.Function.Arguments != "" {
|
||||
if err := json.Unmarshal([]byte(tc.Function.Arguments), &input); err != nil {
|
||||
input = map[string]any{}
|
||||
}
|
||||
}
|
||||
// Handle nil Arguments (GLM-4 may return null input)
|
||||
if input == nil {
|
||||
input = map[string]any{}
|
||||
}
|
||||
@@ -246,7 +260,7 @@ func buildRequestBody(
|
||||
toolUse := map[string]any{
|
||||
"type": "tool_use",
|
||||
"id": tc.ID,
|
||||
"name": tc.Name,
|
||||
"name": toolName,
|
||||
"input": input,
|
||||
}
|
||||
content = append(content, toolUse)
|
||||
|
||||
@@ -614,6 +614,126 @@ func TestBuildRequestBody_UserToolResultsMerged(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestBuildRequestBody_ToolCallFunctionFallback verifies that tool calls whose
|
||||
// runtime-only fields were lost in a JSON round-trip through the session store
|
||||
// (ToolCall.Name/Arguments are json:"-"; only ToolCall.Function survives) fall
|
||||
// back to Function.Name / Function.Arguments, so the tool_use block is still
|
||||
// emitted and its tool_result pair stays intact. Without the fallback the
|
||||
// tool_use is skipped and the orphaned tool_result 400s at the API
|
||||
// ("unexpected tool_use_id found in tool_result blocks").
|
||||
func TestBuildRequestBody_ToolCallFunctionFallback(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
toolCall ToolCall
|
||||
wantSkipped bool
|
||||
wantToolName string
|
||||
wantInput map[string]any
|
||||
}{
|
||||
{
|
||||
name: "deserialized history shape falls back to Function fields",
|
||||
toolCall: ToolCall{
|
||||
ID: "toolu-fallback-1",
|
||||
Name: "",
|
||||
Arguments: nil,
|
||||
Function: &FunctionCall{Name: "x", Arguments: `{"a":1}`},
|
||||
},
|
||||
wantToolName: "x",
|
||||
wantInput: map[string]any{"a": float64(1)},
|
||||
},
|
||||
{
|
||||
name: "runtime shape with Name set and Function nil still works",
|
||||
toolCall: ToolCall{
|
||||
ID: "toolu-runtime-1",
|
||||
Name: "y",
|
||||
Arguments: map[string]any{"b": 2},
|
||||
},
|
||||
wantToolName: "y",
|
||||
wantInput: map[string]any{"b": 2},
|
||||
},
|
||||
{
|
||||
name: "both Name and Function.Name empty is skipped",
|
||||
toolCall: ToolCall{
|
||||
ID: "toolu-empty-1",
|
||||
Name: "",
|
||||
Function: &FunctionCall{Name: "", Arguments: `{"c":3}`},
|
||||
},
|
||||
wantSkipped: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
messages := []Message{
|
||||
{Role: "user", Content: "run the tool"},
|
||||
{Role: "assistant", Content: "", ToolCalls: []ToolCall{tt.toolCall}},
|
||||
{Role: "tool", ToolCallID: tt.toolCall.ID, Content: "result"},
|
||||
}
|
||||
|
||||
got, err := buildRequestBody(messages, nil, "test-model", map[string]any{"max_tokens": 8192})
|
||||
if err != nil {
|
||||
t.Fatalf("buildRequestBody() error: %v", err)
|
||||
}
|
||||
|
||||
apiMessages := got["messages"].([]any)
|
||||
if len(apiMessages) != 3 {
|
||||
t.Fatalf("expected 3 API messages, got %d", len(apiMessages))
|
||||
}
|
||||
|
||||
assistantMsg := apiMessages[1].(map[string]any)
|
||||
content := assistantMsg["content"].([]any)
|
||||
|
||||
if tt.wantSkipped {
|
||||
if len(content) != 0 {
|
||||
t.Fatalf("assistant content = %#v, want empty (tool call skipped)", content)
|
||||
}
|
||||
// Note: matching current behavior, the orphaned tool_result is
|
||||
// still emitted in the following user message even though its
|
||||
// tool_use block was skipped.
|
||||
toolResultMsg := apiMessages[2].(map[string]any)
|
||||
blocks := toolResultMsg["content"].([]map[string]any)
|
||||
if len(blocks) != 1 || blocks[0]["tool_use_id"] != tt.toolCall.ID {
|
||||
t.Fatalf("orphaned tool_result = %#v, want single block with tool_use_id %q",
|
||||
blocks, tt.toolCall.ID)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
// (a) tool_use block emitted with resolved name, id, and parsed input.
|
||||
if len(content) != 1 {
|
||||
t.Fatalf("assistant content length = %d, want 1 tool_use block", len(content))
|
||||
}
|
||||
toolUse := content[0].(map[string]any)
|
||||
if toolUse["type"] != "tool_use" {
|
||||
t.Fatalf("block type = %v, want tool_use", toolUse["type"])
|
||||
}
|
||||
if toolUse["name"] != tt.wantToolName {
|
||||
t.Errorf("tool_use name = %v, want %q", toolUse["name"], tt.wantToolName)
|
||||
}
|
||||
if toolUse["id"] != tt.toolCall.ID {
|
||||
t.Errorf("tool_use id = %v, want %q", toolUse["id"], tt.toolCall.ID)
|
||||
}
|
||||
if !reflect.DeepEqual(toolUse["input"], tt.wantInput) {
|
||||
t.Errorf("tool_use input = %#v, want %#v", toolUse["input"], tt.wantInput)
|
||||
}
|
||||
|
||||
// (b) the following user message's tool_result references the same
|
||||
// id as the tool_use block — the pair is intact.
|
||||
toolResultMsg := apiMessages[2].(map[string]any)
|
||||
if toolResultMsg["role"] != "user" {
|
||||
t.Fatalf("message after assistant role = %v, want user", toolResultMsg["role"])
|
||||
}
|
||||
blocks := toolResultMsg["content"].([]map[string]any)
|
||||
if len(blocks) != 1 {
|
||||
t.Fatalf("tool_result blocks = %d, want 1", len(blocks))
|
||||
}
|
||||
if blocks[0]["tool_use_id"] != toolUse["id"] {
|
||||
t.Errorf("tool_result tool_use_id = %v, want %v (paired with tool_use)",
|
||||
blocks[0]["tool_use_id"], toolUse["id"])
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestParseResponseBodyEdgeCases tests edge cases for parseResponseBody.
|
||||
func TestParseResponseBodyEdgeCases(t *testing.T) {
|
||||
tests := []struct {
|
||||
|
||||
Reference in New Issue
Block a user