diff --git a/api/types.go b/api/types.go index 8cc7752ca..953429592 100644 --- a/api/types.go +++ b/api/types.go @@ -3,6 +3,7 @@ package api import ( "encoding/json" "fmt" + "iter" "log/slog" "math" "os" @@ -12,6 +13,7 @@ import ( "time" "github.com/google/uuid" + orderedmap "github.com/wk8/go-ordered-map/v2" "github.com/ollama/ollama/envconfig" "github.com/ollama/ollama/types/model" @@ -193,13 +195,70 @@ type ToolCallFunction struct { Arguments ToolCallFunctionArguments `json:"arguments"` } -type ToolCallFunctionArguments map[string]any +type ToolCallFunctionArguments struct { + om *orderedmap.OrderedMap[string, any] +} + +func NewToolCallFunctionArguments() ToolCallFunctionArguments { + return ToolCallFunctionArguments{ + om: orderedmap.New[string, any](), + } +} + +func (t *ToolCallFunctionArguments) Get(key string) (any, bool) { + if t == nil || t.om == nil { + return nil, false + } + return t.om.Get(key) +} + +func (t *ToolCallFunctionArguments) Set(key string, value any) { + if t == nil { + return + } + if t.om == nil { + t.om = orderedmap.New[string, any]() + } + t.om.Set(key, value) +} + +func (t *ToolCallFunctionArguments) Len() int { + if t == nil || t.om == nil { + return 0 + } + return t.om.Len() +} + +func (t *ToolCallFunctionArguments) All() iter.Seq2[string, any] { + return func(yield func(string, any) bool) { + if t == nil || t.om == nil { + return + } + for pair := t.om.Oldest(); pair != nil; pair = pair.Next() { + if !yield(pair.Key, pair.Value) { + return + } + } + } +} func (t *ToolCallFunctionArguments) String() string { - bts, _ := json.Marshal(t) + if t == nil || t.om == nil { + return "{}" + } + bts, _ := json.Marshal(t.om) return string(bts) } +func (t *ToolCallFunctionArguments) UnmarshalJSON(data []byte) error { + t.om = orderedmap.New[string, any]() + return json.Unmarshal(data, &t.om) +} + +func (t ToolCallFunctionArguments) MarshalJSON() ([]byte, error) { + return json.Marshal(t.om) +} + type Tool struct { Type string `json:"type"` Items any `json:"items,omitempty"` @@ -301,12 +360,114 @@ func mapToTypeScriptType(jsonType string) string { } } +type ToolProperties struct { + om *orderedmap.OrderedMap[string, ToolProperty] +} + +func NewToolProperties() *ToolProperties { + return &ToolProperties{ + om: orderedmap.New[string, ToolProperty](), + } +} + +func (t *ToolProperties) Get(key string) (ToolProperty, bool) { + if t == nil || t.om == nil { + return ToolProperty{}, false + } + return t.om.Get(key) +} + +func (t *ToolProperties) Set(key string, value ToolProperty) { + if t == nil { + return + } + if t.om == nil { + t.om = orderedmap.New[string, ToolProperty]() + } + t.om.Set(key, value) +} + +func (t *ToolProperties) Len() int { + if t == nil || t.om == nil { + return 0 + } + return t.om.Len() +} + +func (t *ToolProperties) All() iter.Seq2[string, ToolProperty] { + return func(yield func(string, ToolProperty) bool) { + if t == nil || t.om == nil { + return + } + for pair := t.om.Oldest(); pair != nil; pair = pair.Next() { + if !yield(pair.Key, pair.Value) { + return + } + } + } +} + +func (t *ToolProperties) MarshalJSON() ([]byte, error) { + if t == nil || t.om == nil { + return []byte("null"), nil + } + return json.Marshal(t.om) +} + +func (t *ToolProperties) UnmarshalJSON(data []byte) error { + t.om = orderedmap.New[string, ToolProperty]() + return json.Unmarshal(data, &t.om) +} + type ToolFunctionParameters struct { - Type string `json:"type"` - Defs any `json:"$defs,omitempty"` - Items any `json:"items,omitempty"` - Required []string `json:"required"` - Properties map[string]ToolProperty `json:"properties"` + Type string `json:"type"` + Defs any `json:"$defs,omitempty"` + Items any `json:"items,omitempty"` + Required []string `json:"required"` + properties *ToolProperties // unexported - accessed via Properties() method +} + +// Properties returns an iterator for template compatibility. +// Templates can range over this directly: {{range $k, $v := .Properties}} +func (t ToolFunctionParameters) Properties() iter.Seq2[string, ToolProperty] { + if t.properties == nil { + return func(yield func(string, ToolProperty) bool) {} + } + return t.properties.All() +} + +// HasProperties returns true if properties exist and are non-empty. +// This is used by templates for conditional checks: {{if .HasProperties}} +func (t ToolFunctionParameters) HasProperties() bool { + return t.properties != nil && t.properties.Len() > 0 +} + +// Len returns the number of properties. +// This is used by templates: {{.Function.Parameters.Len}} +func (t ToolFunctionParameters) Len() int { + if t.properties == nil { + return 0 + } + return t.properties.Len() +} + +// SetProperties sets the properties (used by tests and internal code) +func (t *ToolFunctionParameters) SetProperties(props *ToolProperties) { + t.properties = props +} + +// NewToolFunctionParametersWithProps creates a ToolFunctionParameters with properties (helper for tests) +func NewToolFunctionParametersWithProps(typ string, required []string, props *ToolProperties) ToolFunctionParameters { + return ToolFunctionParameters{ + Type: typ, + Required: required, + properties: props, + } +} + +// GetProperties returns the properties wrapper (used by renderers) +func (t *ToolFunctionParameters) GetProperties() *ToolProperties { + return t.properties } func (t *ToolFunctionParameters) String() string { @@ -314,6 +475,38 @@ func (t *ToolFunctionParameters) String() string { return string(bts) } +func (t *ToolFunctionParameters) MarshalJSON() ([]byte, error) { + type Alias ToolFunctionParameters + return json.Marshal(&struct { + Type string `json:"type"` + Defs any `json:"$defs,omitempty"` + Items any `json:"items,omitempty"` + Required []string `json:"required"` + Properties *ToolProperties `json:"properties"` + }{ + Type: t.Type, + Defs: t.Defs, + Items: t.Items, + Required: t.Required, + Properties: t.properties, + }) +} + +func (t *ToolFunctionParameters) UnmarshalJSON(data []byte) error { + type Alias ToolFunctionParameters + aux := &struct { + Properties *ToolProperties `json:"properties"` + *Alias + }{ + Alias: (*Alias)(t), + } + if err := json.Unmarshal(data, aux); err != nil { + return err + } + t.properties = aux.Properties + return nil +} + type ToolFunction struct { Name string `json:"name"` Description string `json:"description"` diff --git a/api/types_test.go b/api/types_test.go index 5393b4623..3d8edf76d 100644 --- a/api/types_test.go +++ b/api/types_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "math" + "strings" "testing" "time" @@ -450,23 +451,25 @@ func TestToolFunctionParameters_String(t *testing.T) { }{ { name: "simple object with string property", - params: ToolFunctionParameters{ - Type: "object", - Required: []string{"name"}, - Properties: map[string]ToolProperty{ - "name": { + params: NewToolFunctionParametersWithProps( + "object", + []string{"name"}, + func() *ToolProperties { + om := NewToolProperties() + om.Set("name", ToolProperty{ Type: PropertyType{"string"}, Description: "The name of the person", - }, - }, - }, + }) + return om + }(), + ), expected: `{"type":"object","required":["name"],"properties":{"name":{"type":"string","description":"The name of the person"}}}`, }, { name: "marshal failure returns empty string", - params: ToolFunctionParameters{ - Type: "object", - Defs: func() any { + params: func() ToolFunctionParameters { + p := NewToolFunctionParametersWithProps("object", nil, NewToolProperties()) + p.Defs = func() any { // Create a cycle that will cause json.Marshal to fail type selfRef struct { Self *selfRef @@ -474,9 +477,9 @@ func TestToolFunctionParameters_String(t *testing.T) { s := &selfRef{} s.Self = s return s - }(), - Properties: map[string]ToolProperty{}, - }, + }() + return p + }(), expected: "", }, } @@ -488,3 +491,31 @@ func TestToolFunctionParameters_String(t *testing.T) { }) } } + +func TestTemplateRenderingWithArguments(t *testing.T) { + // Test that ToolCallFunctionArguments renders correctly in templates + // This verifies the String() method works for template interpolation + args := NewToolCallFunctionArguments() + args.Set("location", "San Francisco") + args.Set("unit", "fahrenheit") + + // Simulate what a template would do: convert to string + rendered := args.String() + + // Should produce valid JSON + var parsed map[string]any + err := json.Unmarshal([]byte(rendered), &parsed) + require.NoError(t, err, "Arguments should render as valid JSON") + + // Verify the values are present and in order + assert.Equal(t, "San Francisco", parsed["location"]) + assert.Equal(t, "fahrenheit", parsed["unit"]) + + // Verify it maintains insertion order by checking the JSON string directly + // The first Set was "location", so it should appear before "unit" + assert.Contains(t, rendered, `"location":"San Francisco"`) + assert.Contains(t, rendered, `"unit":"fahrenheit"`) + locIndex := strings.Index(rendered, "location") + unitIndex := strings.Index(rendered, "unit") + assert.Less(t, locIndex, unitIndex, "insertion order should be preserved") +} diff --git a/go.mod b/go.mod index 46e7f433f..8c6ab70db 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( github.com/mattn/go-runewidth v0.0.14 github.com/nlpodyssey/gopickle v0.3.0 github.com/pdevine/tensor v0.0.0-20240510204454-f88f4562727c + github.com/wk8/go-ordered-map/v2 v2.1.8 golang.org/x/image v0.22.0 golang.org/x/tools v0.30.0 gonum.org/v1/gonum v0.15.0 @@ -30,6 +31,8 @@ require ( require ( github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40 // indirect + github.com/bahlo/generic-list-go v0.2.0 // indirect + github.com/buger/jsonparser v1.1.1 // indirect github.com/bytedance/sonic/loader v0.1.1 // indirect github.com/chewxy/hm v1.0.0 // indirect github.com/chewxy/math32 v1.11.0 // indirect @@ -39,6 +42,7 @@ require ( github.com/gogo/protobuf v1.3.2 // indirect github.com/google/flatbuffers v24.3.25+incompatible // indirect github.com/kr/text v0.2.0 // indirect + github.com/mailru/easyjson v0.7.7 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.2.0 // indirect diff --git a/go.sum b/go.sum index c0ab53aab..50cfb8ad4 100644 --- a/go.sum +++ b/go.sum @@ -12,7 +12,11 @@ github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40 h1:q4dksr6IC github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40/go.mod h1:Q7yQnSMnLvcXlZ8RV+jwz/6y1rQTqbX6C82SndT52Zs= github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0 h1:jfIu9sQUG6Ig+0+Ap1h4unLjW6YQJpKZVmUzxsD4E/Q= github.com/arbovm/levenshtein v0.0.0-20160628152529-48b4e1c0c4d0/go.mod h1:t2tdKJDJF9BV14lnkjHmOQgcvEKgtqs5a1N3LNdJhGE= +github.com/bahlo/generic-list-go v0.2.0 h1:5sz/EEAK+ls5wF+NeqDpk5+iNdMDXrh3z3nPnH1Wvgk= +github.com/bahlo/generic-list-go v0.2.0/go.mod h1:2KvAjgMlE5NNynlg/5iLrrCCZ2+5xWbdbCW3pNTGyYg= github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= +github.com/buger/jsonparser v1.1.1 h1:2PnMjfWD7wBILjqQbt530v576A/cAbQvEW9gGIpYMUs= +github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0= github.com/bytedance/sonic v1.11.6 h1:oUp34TzMlL+OY1OUWxHqsdkgC/Zfc85zGqw9siXjrc0= github.com/bytedance/sonic v1.11.6/go.mod h1:LysEHSvpvDySVdC2f87zGWf6CIKJcAvqab1ZaiQtds4= github.com/bytedance/sonic/loader v0.1.1 h1:c+e5Pt1k/cy5wMveRDyk2X4B9hF4g7an8N3zCYjJFNM= @@ -121,6 +125,7 @@ github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= @@ -139,6 +144,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= +github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= +github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= @@ -197,6 +204,8 @@ github.com/twitchyliquid64/golang-asm v0.15.1 h1:SU5vSMR7hnwNxj24w34ZyCi/FmDZTkS github.com/twitchyliquid64/golang-asm v0.15.1/go.mod h1:a1lVb/DtPvCB8fslRZhAngC2+aY1QWCk3Cedj/Gdt08= github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE= github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc= +github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/xtgo/set v1.0.0 h1:6BCNBRv3ORNDQ7fyoJXRv+tstJz3m1JVFQErfeZz2pY= diff --git a/integration/api_test.go b/integration/api_test.go index 48572085d..99dca9da0 100644 --- a/integration/api_test.go +++ b/integration/api_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + orderedmap "github.com/wk8/go-ordered-map/v2" + "github.com/ollama/ollama/api" ) @@ -432,12 +434,14 @@ func TestAPIToolCalling(t *testing.T) { Parameters: api.ToolFunctionParameters{ Type: "object", Required: []string{"location"}, - Properties: map[string]api.ToolProperty{ - "location": { + Properties: func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The city and state, e.g. San Francisco, CA", - }, - }, + }) + return props + }(), }, }, }, @@ -497,7 +501,7 @@ func TestAPIToolCalling(t *testing.T) { t.Errorf("unexpected tool called: got %q want %q", lastToolCall.Function.Name, "get_weather") } - if _, ok := lastToolCall.Function.Arguments["location"]; !ok { + if _, ok := lastToolCall.Function.Arguments.Get("location"); !ok { t.Errorf("expected tool arguments to include 'location', got: %s", lastToolCall.Function.Arguments.String()) } case <-ctx.Done(): diff --git a/middleware/openai_test.go b/middleware/openai_test.go index a78ee8b91..cc6e2033b 100644 --- a/middleware/openai_test.go +++ b/middleware/openai_test.go @@ -14,6 +14,7 @@ import ( "github.com/gin-gonic/gin" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/ollama/ollama/api" "github.com/ollama/ollama/openai" @@ -29,6 +30,16 @@ var ( True = true ) +func makeArgs(pairs ...any) api.ToolCallFunctionArguments { + args := api.NewToolCallFunctionArguments() + for i := 0; i < len(pairs); i += 2 { + key := pairs[i].(string) + value := pairs[i+1] + args.Set(key, value) + } + return args +} + func captureRequestMiddleware(capturedRequest any) gin.HandlerFunc { return func(c *gin.Context) { bodyBytes, _ := io.ReadAll(c.Request.Body) @@ -220,10 +231,7 @@ func TestChatMiddleware(t *testing.T) { { Function: api.ToolCallFunction{ Name: "get_current_weather", - Arguments: map[string]any{ - "location": "Paris, France", - "format": "celsius", - }, + Arguments: makeArgs("location", "Paris, France", "format", "celsius"), }, }, }, @@ -259,10 +267,7 @@ func TestChatMiddleware(t *testing.T) { { Function: api.ToolCallFunction{ Name: "get_current_weather", - Arguments: map[string]any{ - "location": "Paris, France", - "format": "celsius", - }, + Arguments: makeArgs("location", "Paris, France", "format", "celsius"), }, }, }, @@ -297,10 +302,7 @@ func TestChatMiddleware(t *testing.T) { { Function: api.ToolCallFunction{ Name: "get_current_weather", - Arguments: map[string]any{ - "location": "Paris, France", - "format": "celsius", - }, + Arguments: makeArgs("location", "Paris, France", "format", "celsius"), }, }, }, @@ -336,10 +338,7 @@ func TestChatMiddleware(t *testing.T) { { Function: api.ToolCallFunction{ Name: "get_current_weather", - Arguments: map[string]any{ - "location": "Paris, France", - "format": "celsius", - }, + Arguments: makeArgs("location", "Paris, France", "format", "celsius"), }, }, }, @@ -375,10 +374,7 @@ func TestChatMiddleware(t *testing.T) { { Function: api.ToolCallFunction{ Name: "get_current_weather", - Arguments: map[string]any{ - "location": "Paris, France", - "format": "celsius", - }, + Arguments: makeArgs("location", "Paris, France", "format", "celsius"), }, }, }, @@ -419,10 +415,7 @@ func TestChatMiddleware(t *testing.T) { { Function: api.ToolCallFunction{ Name: "get_current_weather", - Arguments: map[string]any{ - "location": "Paris, France", - "format": "celsius", - }, + Arguments: makeArgs("location", "Paris, France", "format", "celsius"), }, }, }, @@ -484,26 +477,22 @@ func TestChatMiddleware(t *testing.T) { Function: api.ToolFunction{ Name: "get_weather", Description: "Get the current weather", - Parameters: struct { - Type string `json:"type"` - Defs any `json:"$defs,omitempty"` - Items any `json:"items,omitempty"` - Required []string `json:"required"` - Properties map[string]api.ToolProperty `json:"properties"` - }{ - Type: "object", - Required: []string{"location"}, - Properties: map[string]api.ToolProperty{ - "location": { + Parameters: api.NewToolFunctionParametersWithProps( + "object", + []string{"location"}, + func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The city and state", - }, - "unit": { + }) + props.Set("unit", api.ToolProperty{ Type: api.PropertyType{"string"}, Enum: []any{"celsius", "fahrenheit"}, - }, - }, - }, + }) + return props + }(), + ), }, }, }, @@ -557,7 +546,7 @@ func TestChatMiddleware(t *testing.T) { } return } - if diff := cmp.Diff(&tc.req, capturedRequest); diff != "" { + if diff := cmp.Diff(&tc.req, capturedRequest, cmpopts.IgnoreUnexported(api.ToolCallFunctionArguments{}, api.ToolProperties{}, api.ToolFunctionParameters{})); diff != "" { t.Fatalf("requests did not match: %+v", diff) } if diff := cmp.Diff(tc.err, errResp); diff != "" { diff --git a/model/parsers/qwen3coder.go b/model/parsers/qwen3coder.go index f44d7c8ef..f9ff11898 100644 --- a/model/parsers/qwen3coder.go +++ b/model/parsers/qwen3coder.go @@ -268,17 +268,17 @@ func parseToolCall(raw qwenEventRawToolCall, tools []api.Tool) (api.ToolCall, er } } - toolCall.Function.Arguments = make(api.ToolCallFunctionArguments) + toolCall.Function.Arguments = api.NewToolCallFunctionArguments() for _, parameter := range functionCall.Parameters { // Look up the parameter type if we found the tool var paramType api.PropertyType - if matchedTool != nil && matchedTool.Function.Parameters.Properties != nil { - if prop, ok := matchedTool.Function.Parameters.Properties[parameter.Name]; ok { + if matchedTool != nil && matchedTool.Function.Parameters.GetProperties() != nil { + if prop, ok := matchedTool.Function.Parameters.GetProperties().Get(parameter.Name); ok { paramType = prop.Type } } - toolCall.Function.Arguments[parameter.Name] = parseValue(parameter.Value, paramType) + toolCall.Function.Arguments.Set(parameter.Name, parseValue(parameter.Value, paramType)) } return toolCall, nil diff --git a/model/parsers/qwen3coder_test.go b/model/parsers/qwen3coder_test.go index c77fe2d95..fa8638258 100644 --- a/model/parsers/qwen3coder_test.go +++ b/model/parsers/qwen3coder_test.go @@ -11,10 +11,25 @@ import ( func tool(name string, props map[string]api.ToolProperty) api.Tool { t := api.Tool{Type: "function", Function: api.ToolFunction{Name: name}} t.Function.Parameters.Type = "object" - t.Function.Parameters.Properties = props + p := api.NewToolProperties() + for k, v := range props { + p.Set(k, v) + } + t.Function.Parameters.SetProperties(p) return t } +// Helper function to create ordered arguments for tests +func makeArgs(pairs ...any) api.ToolCallFunctionArguments { + args := api.NewToolCallFunctionArguments() + for i := 0; i < len(pairs); i += 2 { + key := pairs[i].(string) + value := pairs[i+1] + args.Set(key, value) + } + return args +} + func TestQwenParserStreaming(t *testing.T) { type step struct { input string @@ -354,10 +369,7 @@ celsius wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "get_current_temperature", - Arguments: map[string]any{ - "location": "San Francisco", - "unit": "celsius", - }, + Arguments: makeArgs("location", "San Francisco", "unit", "celsius"), }, }, }, @@ -375,10 +387,10 @@ celsius wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "get current temperature", - Arguments: map[string]any{ - "location with spaces": "San Francisco", - "unit with spaces": "celsius", - }, + Arguments: makeArgs( + "location with spaces", "San Francisco", + "unit with spaces", "celsius", + ), }, }, }, @@ -400,10 +412,10 @@ San Francisco wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "\"get current temperature\"", - Arguments: map[string]any{ - "\"location with spaces\"": "San Francisco", - "\"unit with spaces\"": "\"celsius\"", - }, + Arguments: makeArgs( + "\"location with spaces\"", "San Francisco", + "\"unit with spaces\"", "\"celsius\"", + ), }, }, }, @@ -434,12 +446,12 @@ true wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "calculate", - Arguments: map[string]any{ - "x": 3.14, - "y": 42, - "enabled": true, - "items": []any{"a", "b", "c"}, - }, + Arguments: makeArgs( + "x", 3.14, + "y", 42, + "enabled", true, + "items", []any{"a", "b", "c"}, + ), }, }, }, @@ -455,9 +467,7 @@ ls && echo "done" wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "exec", - Arguments: map[string]any{ - "command": "ls && echo \"done\"", - }, + Arguments: makeArgs("command", "ls && echo \"done\""), }, }, }, @@ -472,9 +482,7 @@ ls && echo "a > b and a < b" wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "exec", - Arguments: map[string]any{ - "command": "ls && echo \"a > b and a < b\"", - }, + Arguments: makeArgs("command", "ls && echo \"a > b and a < b\""), }, }, }, @@ -492,10 +500,7 @@ Hello! 你好! 🌟 مرحبا wantToolCall: api.ToolCall{ Function: api.ToolCallFunction{ Name: "获取天气", - Arguments: map[string]any{ - "城市": "北京", - "message": "Hello! 你好! 🌟 مرحبا", - }, + Arguments: makeArgs("城市", "北京", "message", "Hello! 你好! 🌟 مرحبا"), }, }, }, diff --git a/model/renderers/qwen3coder.go b/model/renderers/qwen3coder.go index 32611791b..1d5a264dd 100644 --- a/model/renderers/qwen3coder.go +++ b/model/renderers/qwen3coder.go @@ -94,26 +94,28 @@ func Qwen3CoderRenderer(messages []api.Message, tools []api.Tool, _ *api.ThinkVa } sb.WriteString("\n") - for name, prop := range tool.Function.Parameters.Properties { - sb.WriteString("\n") - sb.WriteString("\n" + name + "") + if tool.Function.Parameters.GetProperties() != nil { + for name, prop := range tool.Function.Parameters.Properties() { + sb.WriteString("\n") + sb.WriteString("\n" + name + "") - if len(prop.Type) > 0 { - sb.WriteString("\n" + formatToolDefinitionType(prop.Type) + "") + if len(prop.Type) > 0 { + sb.WriteString("\n" + formatToolDefinitionType(prop.Type) + "") + } + + if prop.Description != "" { + sb.WriteString("\n" + prop.Description + "") + } + + // Render any additional keys not already handled + handledKeys := map[string]bool{ + "type": true, + "description": true, + } + sb.WriteString(renderAdditionalKeys(prop, handledKeys)) + + sb.WriteString("\n") } - - if prop.Description != "" { - sb.WriteString("\n" + prop.Description + "") - } - - // Render any additional keys not already handled - handledKeys := map[string]bool{ - "type": true, - "description": true, - } - sb.WriteString(renderAdditionalKeys(prop, handledKeys)) - - sb.WriteString("\n") } // Render extra keys for parameters (everything except 'type' and 'properties') @@ -145,7 +147,7 @@ func Qwen3CoderRenderer(messages []api.Message, tools []api.Tool, _ *api.ThinkVa } for _, toolCall := range message.ToolCalls { sb.WriteString("\n\n") - for name, value := range toolCall.Function.Arguments { + for name, value := range toolCall.Function.Arguments.All() { valueStr := formatToolCallArgument(value) sb.WriteString("\n\n" + valueStr + "\n") } diff --git a/model/renderers/qwen3coder_test.go b/model/renderers/qwen3coder_test.go index 6a9e5eccd..c129fcbcd 100644 --- a/model/renderers/qwen3coder_test.go +++ b/model/renderers/qwen3coder_test.go @@ -4,9 +4,32 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/ollama/ollama/api" ) +// Helper function to create ordered arguments for tests +func makeArgs(pairs ...any) api.ToolCallFunctionArguments { + args := api.NewToolCallFunctionArguments() + for i := 0; i < len(pairs); i += 2 { + key := pairs[i].(string) + value := pairs[i+1] + args.Set(key, value) + } + return args +} + +// Helper function to create ordered properties for tests +func makeProps(pairs ...any) *api.ToolProperties { + props := api.NewToolProperties() + for i := 0; i < len(pairs); i += 2 { + key := pairs[i].(string) + value := pairs[i+1].(api.ToolProperty) + props.Set(key, value) + } + return props +} + func TestQwen3CoderRenderer(t *testing.T) { tests := []struct { name string @@ -38,10 +61,8 @@ Hello, how are you?<|im_end|> ToolCalls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Name: "get_weather", - Arguments: map[string]any{ - "unit": "fahrenheit", - }, + Name: "get_weather", + Arguments: makeArgs("unit", "fahrenheit"), }, }, }, @@ -53,18 +74,13 @@ Hello, how are you?<|im_end|> {Function: api.ToolFunction{ Name: "get_weather", Description: "Get the current weather in a given location", - Parameters: api.ToolFunctionParameters{ - Required: []string{"unit"}, - Properties: map[string]api.ToolProperty{ - "unit": {Type: api.PropertyType{"string"}, Enum: []any{"celsius", "fahrenheit"}, Description: "The unit of temperature"}, - // TODO(drifkin): add multiple params back once we have predictable - // order via some sort of ordered map type (see - // ) - /* - "location": {Type: api.PropertyType{"string"}, Description: "The city and state, e.g. San Francisco, CA"}, - */ - }, - }, + Parameters: api.NewToolFunctionParametersWithProps( + "object", + []string{"unit"}, + makeProps( + "unit", api.ToolProperty{Type: api.PropertyType{"string"}, Enum: []any{"celsius", "fahrenheit"}, Description: "The unit of temperature"}, + ), + ), }}, }, expected: `<|im_start|>system @@ -140,19 +156,19 @@ That sounds nice! What about New York?<|im_end|> {Role: "system", Content: "You are a helpful assistant with access to tools."}, {Role: "user", Content: "call double(1) and triple(2)"}, {Role: "assistant", Content: "I'll call double(1) and triple(2) for you.", ToolCalls: []api.ToolCall{ - {Function: api.ToolCallFunction{Name: "double", Arguments: map[string]any{"number": "1"}}}, - {Function: api.ToolCallFunction{Name: "triple", Arguments: map[string]any{"number": "2"}}}, + {Function: api.ToolCallFunction{Name: "double", Arguments: makeArgs("number", "1")}}, + {Function: api.ToolCallFunction{Name: "triple", Arguments: makeArgs("number", "2")}}, }}, {Role: "tool", Content: "{\"number\": 2}", ToolName: "double"}, {Role: "tool", Content: "{\"number\": 6}", ToolName: "triple"}, }, tools: []api.Tool{ - {Function: api.ToolFunction{Name: "double", Description: "Double a number", Parameters: api.ToolFunctionParameters{Properties: map[string]api.ToolProperty{ - "number": {Type: api.PropertyType{"string"}, Description: "The number to double"}, - }}}}, - {Function: api.ToolFunction{Name: "triple", Description: "Triple a number", Parameters: api.ToolFunctionParameters{Properties: map[string]api.ToolProperty{ - "number": {Type: api.PropertyType{"string"}, Description: "The number to triple"}, - }}}}, + {Function: api.ToolFunction{Name: "double", Description: "Double a number", Parameters: api.NewToolFunctionParametersWithProps("object", nil, makeProps( + "number", api.ToolProperty{Type: api.PropertyType{"string"}, Description: "The number to double"}, + ))}}, + {Function: api.ToolFunction{Name: "triple", Description: "Triple a number", Parameters: api.NewToolFunctionParametersWithProps("object", nil, makeProps( + "number", api.ToolProperty{Type: api.PropertyType{"string"}, Description: "The number to triple"}, + ))}}, }, expected: `<|im_start|>system You are a helpful assistant with access to tools. @@ -258,10 +274,8 @@ I'll tell you something interesting about cats`, {Role: "user", Content: "call tool"}, {Role: "assistant", ToolCalls: []api.ToolCall{ {Function: api.ToolCallFunction{ - Name: "echo", - Arguments: map[string]any{ - "payload": map[string]any{"foo": "bar"}, - }, + Name: "echo", + Arguments: makeArgs("payload", map[string]any{"foo": "bar"}), }}, }}, {Role: "tool", Content: "{\"payload\": {\"foo\": \"bar\"}}", ToolName: "echo"}, @@ -368,3 +382,62 @@ func TestQwen3ToolDefinitionTypes(t *testing.T) { }) } } + +func TestMultipleParametersNonDeterministic(t *testing.T) { + // This test demonstrates that tools with multiple parameters are rendered + // non-deterministically due to Go's map iteration order. + // See https://github.com/ollama/ollama/issues/12244 + + tools := []api.Tool{ + {Function: api.ToolFunction{ + Name: "get_weather", + Description: "Get the current weather", + Parameters: api.NewToolFunctionParametersWithProps( + "object", + []string{"location", "unit"}, + makeProps( + "location", api.ToolProperty{Type: api.PropertyType{"string"}, Description: "The city and state"}, + "unit", api.ToolProperty{Type: api.PropertyType{"string"}, Description: "The temperature unit"}, + "format", api.ToolProperty{Type: api.PropertyType{"string"}, Description: "The output format"}, + ), + ), + }}, + } + + msgs := []api.Message{ + {Role: "user", Content: "What's the weather?"}, + {Role: "assistant", ToolCalls: []api.ToolCall{ + {Function: api.ToolCallFunction{ + Name: "get_weather", + Arguments: makeArgs( + "location", "San Francisco, CA", + "unit", "fahrenheit", + "format", "detailed", + ), + }}, + }}, + } + + // Run the renderer multiple times and collect unique outputs + outputs := make(map[string]bool) + for i := 0; i < 15; i++ { + rendered, err := Qwen3CoderRenderer(msgs, tools, nil) + if err != nil { + t.Fatal(err) + } + outputs[rendered] = true + } + + // The renderer should be deterministic - we should only get one unique output + if len(outputs) > 1 { + // Show the first two different outputs for comparison + count := 0 + for output := range outputs { + if count < 2 { + t.Logf("\nOutput variant %d:\n%s", count+1, output) + count++ + } + } + t.Fatalf("Renderer produced %d different outputs across 15 runs (expected deterministic output)", len(outputs)) + } +} diff --git a/server/routes_generate_test.go b/server/routes_generate_test.go index 8385cb17b..d21652a6f 100644 --- a/server/routes_generate_test.go +++ b/server/routes_generate_test.go @@ -13,6 +13,7 @@ import ( "github.com/gin-gonic/gin" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/ollama/ollama/api" "github.com/ollama/ollama/discover" @@ -46,6 +47,16 @@ func (mockRunner) Tokenize(_ context.Context, s string) (tokens []int, err error return } +func makeArgs(pairs ...any) api.ToolCallFunctionArguments { + args := api.NewToolCallFunctionArguments() + for i := 0; i < len(pairs); i += 2 { + key := pairs[i].(string) + value := pairs[i+1] + args.Set(key, value) + } + return args +} + func newMockServer(mock *mockRunner) func(discover.GpuInfoList, string, *ggml.GGML, []string, []string, api.Options, int) (llm.LlamaServer, error) { return func(_ discover.GpuInfoList, _ string, _ *ggml.GGML, _, _ []string, _ api.Options, _ int) (llm.LlamaServer, error) { return mock, nil @@ -389,24 +400,26 @@ func TestGenerateChat(t *testing.T) { Name: "get_weather", Description: "Get the current weather", Parameters: struct { - Type string `json:"type"` - Defs any `json:"$defs,omitempty"` - Items any `json:"items,omitempty"` - Required []string `json:"required"` - Properties map[string]api.ToolProperty `json:"properties"` + Type string `json:"type"` + Defs any `json:"$defs,omitempty"` + Items any `json:"items,omitempty"` + Required []string `json:"required"` + Properties *api.ToolProperties `json:"properties"` }{ Type: "object", Required: []string{"location"}, - Properties: map[string]api.ToolProperty{ - "location": { + Properties: func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The city and state", - }, - "unit": { + }) + props.Set("unit", api.ToolProperty{ Type: api.PropertyType{"string"}, Enum: []any{"celsius", "fahrenheit"}, - }, - }, + }) + return props + }(), }, }, }, @@ -459,15 +472,12 @@ func TestGenerateChat(t *testing.T) { expectedToolCall := api.ToolCall{ Function: api.ToolCallFunction{ - Name: "get_weather", - Arguments: api.ToolCallFunctionArguments{ - "location": "Seattle, WA", - "unit": "celsius", - }, + Name: "get_weather", + Arguments: makeArgs("location", "Seattle, WA", "unit", "celsius"), }, } - if diff := cmp.Diff(resp.Message.ToolCalls[0], expectedToolCall); diff != "" { + if diff := cmp.Diff(resp.Message.ToolCalls[0], expectedToolCall, cmpopts.IgnoreUnexported(api.ToolCallFunctionArguments{}, api.ToolProperties{})); diff != "" { t.Errorf("tool call mismatch (-got +want):\n%s", diff) } }) @@ -480,24 +490,26 @@ func TestGenerateChat(t *testing.T) { Name: "get_weather", Description: "Get the current weather", Parameters: struct { - Type string `json:"type"` - Defs any `json:"$defs,omitempty"` - Items any `json:"items,omitempty"` - Required []string `json:"required"` - Properties map[string]api.ToolProperty `json:"properties"` + Type string `json:"type"` + Defs any `json:"$defs,omitempty"` + Items any `json:"items,omitempty"` + Required []string `json:"required"` + Properties *api.ToolProperties `json:"properties"` }{ Type: "object", Required: []string{"location"}, - Properties: map[string]api.ToolProperty{ - "location": { + Properties: func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The city and state", - }, - "unit": { + }) + props.Set("unit", api.ToolProperty{ Type: api.PropertyType{"string"}, Enum: []any{"celsius", "fahrenheit"}, - }, - }, + }) + return props + }(), }, }, }, @@ -582,15 +594,12 @@ func TestGenerateChat(t *testing.T) { expectedToolCall := api.ToolCall{ Function: api.ToolCallFunction{ - Name: "get_weather", - Arguments: api.ToolCallFunctionArguments{ - "location": "Seattle, WA", - "unit": "celsius", - }, + Name: "get_weather", + Arguments: makeArgs("location", "Seattle, WA", "unit", "celsius"), }, } - if diff := cmp.Diff(finalToolCall, expectedToolCall); diff != "" { + if diff := cmp.Diff(finalToolCall, expectedToolCall, cmpopts.IgnoreUnexported(api.ToolCallFunctionArguments{}, api.ToolProperties{})); diff != "" { t.Errorf("final tool call mismatch (-got +want):\n%s", diff) } }) diff --git a/server/routes_harmony_streaming_test.go b/server/routes_harmony_streaming_test.go index caadcb872..6f138ee4a 100644 --- a/server/routes_harmony_streaming_test.go +++ b/server/routes_harmony_streaming_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/gin-gonic/gin" + "github.com/ollama/ollama/api" "github.com/ollama/ollama/discover" "github.com/ollama/ollama/fs/ggml" @@ -27,20 +28,22 @@ func getTestTools() []api.Tool { Name: "get_weather", Description: "Get the current weather in a given location", Parameters: struct { - Type string `json:"type"` - Defs any `json:"$defs,omitempty"` - Items any `json:"items,omitempty"` - Required []string `json:"required"` - Properties map[string]api.ToolProperty `json:"properties"` + Type string `json:"type"` + Defs any `json:"$defs,omitempty"` + Items any `json:"items,omitempty"` + Required []string `json:"required"` + Properties *api.ToolProperties `json:"properties"` }{ Type: "object", Required: []string{"location"}, - Properties: map[string]api.ToolProperty{ - "location": { + Properties: func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The city and state, e.g. San Francisco, CA", - }, - }, + }) + return props + }(), }, }, }, @@ -50,20 +53,22 @@ func getTestTools() []api.Tool { Name: "calculate", Description: "Calculate a mathematical expression", Parameters: struct { - Type string `json:"type"` - Defs any `json:"$defs,omitempty"` - Items any `json:"items,omitempty"` - Required []string `json:"required"` - Properties map[string]api.ToolProperty `json:"properties"` + Type string `json:"type"` + Defs any `json:"$defs,omitempty"` + Items any `json:"items,omitempty"` + Required []string `json:"required"` + Properties *api.ToolProperties `json:"properties"` }{ Type: "object", Required: []string{"expression"}, - Properties: map[string]api.ToolProperty{ - "expression": { + Properties: func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("expression", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The mathematical expression to calculate", - }, - }, + }) + return props + }(), }, }, }, @@ -196,10 +201,8 @@ func TestChatHarmonyParserStreamingRealtime(t *testing.T) { wantToolCalls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Name: "get_weather", - Arguments: api.ToolCallFunctionArguments{ - "location": "San Francisco", - }, + Name: "get_weather", + Arguments: makeArgs("location", "San Francisco"), }, }, }, @@ -222,10 +225,8 @@ func TestChatHarmonyParserStreamingRealtime(t *testing.T) { wantToolCalls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Name: "calculate", - Arguments: api.ToolCallFunctionArguments{ - "expression": "2+2", - }, + Name: "calculate", + Arguments: makeArgs("expression", "2+2"), }, }, }, diff --git a/template/rewrite.go b/template/rewrite.go new file mode 100644 index 000000000..c70afeed8 --- /dev/null +++ b/template/rewrite.go @@ -0,0 +1,149 @@ +package template + +import ( + "text/template" + "text/template/parse" +) + +// rewritePropertiesCheck walks the template AST and rewrites .Function.Parameters.Properties +// to .Function.Parameters.HasProperties in if/with conditions to fix truthiness checking. +// This maintains backward compatibility with templates that check if Properties exist. +func rewritePropertiesCheck(tmpl *template.Template) { + walk(tmpl.Tree.Root) +} + +func walk(n parse.Node) { + if n == nil { + return + } + + switch node := n.(type) { + case *parse.ListNode: + for _, child := range node.Nodes { + walk(child) + } + case *parse.ActionNode: + // Rewrite len calls in action nodes + rewritePipeProperties(node.Pipe) + case *parse.IfNode: + rewritePipeProperties(node.Pipe) + walk(&node.BranchNode) + case *parse.WithNode: + rewritePipeProperties(node.Pipe) + walk(&node.BranchNode) + case *parse.RangeNode: + // Don't rewrite the pipe for range nodes - they need .Properties for iteration + walk(&node.BranchNode) + case *parse.BranchNode: + if node.List != nil { + walk(node.List) + } + if node.ElseList != nil { + walk(node.ElseList) + } + } +} + +func rewritePipeProperties(pipe *parse.PipeNode) { + if pipe == nil { + return + } + + for _, cmd := range pipe.Cmds { + rewriteCommand(cmd) + } +} + +// rewriteCommand recursively rewrites a command and all its nested command arguments +func rewriteCommand(cmd *parse.CommandNode) { + // Check if this is a "len .Function.Parameters.Properties" call + if isLenPropertiesCall(cmd) { + // Replace entire command with .Function.Parameters.Len field access + replaceLenWithLenMethod(cmd) + return + } + + // Recursively process all arguments + for i, arg := range cmd.Args { + switch argNode := arg.(type) { + case *parse.FieldNode: + // Check for direct .Properties field access + if isPropertiesField(argNode.Ident) { + cmd.Args[i] = replaceWithHasProperties(argNode) + } + case *parse.CommandNode: + // Recursively process nested commands (e.g., inside "and", "gt", etc.) + rewriteCommand(argNode) + case *parse.PipeNode: + // Template function arguments can be wrapped in PipeNodes + rewritePipeProperties(argNode) + } + } +} + +// isLenPropertiesCall checks if a command is "len .Function.Parameters.Properties" +func isLenPropertiesCall(cmd *parse.CommandNode) bool { + if len(cmd.Args) != 2 { + return false + } + + // First arg should be the "len" identifier + if ident, ok := cmd.Args[0].(*parse.IdentifierNode); !ok || ident.Ident != "len" { + return false + } + + // Second arg should be .Function.Parameters.Properties field + if field, ok := cmd.Args[1].(*parse.FieldNode); ok { + return isPropertiesField(field.Ident) + } + + return false +} + +// replaceLenWithLenMethod replaces "len .Function.Parameters.Properties" with ".Function.Parameters.Len" +func replaceLenWithLenMethod(cmd *parse.CommandNode) { + if len(cmd.Args) < 2 { + return + } + + field, ok := cmd.Args[1].(*parse.FieldNode) + if !ok { + return + } + + // Create new field node with .Len instead of .Properties + newIdent := make([]string, len(field.Ident)) + copy(newIdent, field.Ident) + newIdent[len(newIdent)-1] = "Len" + + newField := &parse.FieldNode{ + NodeType: parse.NodeField, + Ident: newIdent, + Pos: field.Pos, + } + + // Replace the command with just the field access (remove "len" function call) + cmd.Args = []parse.Node{newField} +} + +func isPropertiesField(ident []string) bool { + // Match: .Function.Parameters.Properties + // We only rewrite if it ends with Parameters.Properties to avoid false positives + if len(ident) < 3 { + return false + } + return ident[len(ident)-1] == "Properties" && ident[len(ident)-2] == "Parameters" +} + +func replaceWithHasProperties(field *parse.FieldNode) *parse.FieldNode { + // Clone the identifier slice and replace the last element + newIdent := make([]string, len(field.Ident)) + copy(newIdent, field.Ident) + newIdent[len(newIdent)-1] = "HasProperties" + + return &parse.FieldNode{ + NodeType: parse.NodeField, + Ident: newIdent, + Pos: field.Pos, + } +} diff --git a/template/rewrite_test.go b/template/rewrite_test.go new file mode 100644 index 000000000..8c8464223 --- /dev/null +++ b/template/rewrite_test.go @@ -0,0 +1,131 @@ +package template + +import ( + "bytes" + "testing" + "text/template" + + "github.com/ollama/ollama/api" +) + +func TestRewritePropertiesCheck(t *testing.T) { + makeToolWithProps := func(props *api.ToolProperties) api.Tool { + return api.Tool{ + Type: "function", + Function: api.ToolFunction{ + Name: "test", + Description: "test function", + Parameters: api.NewToolFunctionParametersWithProps("object", nil, props), + }, + } + } + + tests := []struct { + name string + template string + data interface{} + expected string + }{ + { + name: "if statement with Properties gets rewritten to HasProperties", + template: `{{if .Function.Parameters.Properties}}Has props{{else}}No props{{end}}`, + data: makeToolWithProps(nil), + expected: "No props", // Should use HasProperties which returns false for empty + }, + { + name: "if statement with Properties and non-empty properties", + template: `{{if .Function.Parameters.Properties}}Has props{{else}}No props{{end}}`, + data: makeToolWithProps(func() *api.ToolProperties { + p := api.NewToolProperties() + p.Set("test", api.ToolProperty{Type: api.PropertyType{"string"}}) + return p + }()), + expected: "Has props", // Should use HasProperties which returns true + }, + { + name: "range over Properties should not be rewritten", + template: `{{range $k, $v := .Function.Parameters.Properties}}{{$k}} {{end}}`, + data: makeToolWithProps(func() *api.ToolProperties { + p := api.NewToolProperties() + p.Set("foo", api.ToolProperty{Type: api.PropertyType{"string"}}) + p.Set("bar", api.ToolProperty{Type: api.PropertyType{"number"}}) + return p + }()), + expected: "foo bar ", // Should still use Properties() for ranging + }, + { + name: "complex template with both if and range", + template: `{{if .Function.Parameters.Properties}}Args: +{{range $k, $v := .Function.Parameters.Properties}} {{$k}} +{{end}}{{else}}No args{{end}}`, + data: makeToolWithProps(func() *api.ToolProperties { + p := api.NewToolProperties() + p.Set("location", api.ToolProperty{Type: api.PropertyType{"string"}}) + return p + }()), + expected: "Args:\n location\n", + }, + { + name: "if with and condition", + template: `{{if and .Function.Parameters.Properties (gt (len .Function.Parameters.Properties) 0)}}yes{{else}}no{{end}}`, + data: makeToolWithProps(nil), + expected: "no", // Empty, so HasProperties returns false + }, + { + name: "len function on Properties gets rewritten to Len method", + template: `{{len .Function.Parameters.Properties}}`, + data: makeToolWithProps(nil), + expected: "0", // Empty properties should have length 0 + }, + { + name: "len function on non-empty Properties", + template: `{{len .Function.Parameters.Properties}}`, + data: makeToolWithProps(func() *api.ToolProperties { + p := api.NewToolProperties() + p.Set("foo", api.ToolProperty{Type: api.PropertyType{"string"}}) + p.Set("bar", api.ToolProperty{Type: api.PropertyType{"number"}}) + return p + }()), + expected: "2", // Two properties + }, + { + name: "nested len in and/gt (gpt-oss pattern)", + template: `{{if and .Function.Parameters.Properties (gt (len .Function.Parameters.Properties) 0)}}has props{{else}}no props{{end}}`, + data: makeToolWithProps(nil), + expected: "no props", // Empty, so both checks should be false + }, + { + name: "nested len in and/gt with properties", + template: `{{if and .Function.Parameters.Properties (gt (len .Function.Parameters.Properties) 0)}}has props{{else}}no props{{end}}`, + data: makeToolWithProps(func() *api.ToolProperties { + p := api.NewToolProperties() + p.Set("test", api.ToolProperty{Type: api.PropertyType{"string"}}) + return p + }()), + expected: "has props", // Has properties, both checks should be true + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Use text/template directly and call rewritePropertiesCheck + tmpl, err := template.New("test").Parse(tt.template) + if err != nil { + t.Fatalf("Failed to parse template: %v", err) + } + + // Apply the rewrite + rewritePropertiesCheck(tmpl) + + var buf bytes.Buffer + err = tmpl.Execute(&buf, tt.data) + if err != nil { + t.Fatalf("Failed to execute template: %v", err) + } + + if buf.String() != tt.expected { + t.Errorf("Expected %q, got %q", tt.expected, buf.String()) + } + }) + } +} diff --git a/template/template.go b/template/template.go index c90190d7a..2d3807fac 100644 --- a/template/template.go +++ b/template/template.go @@ -147,6 +147,10 @@ func Parse(s string) (*Template, error) { return nil, err } + // Rewrite .Function.Parameters.Properties to .Function.Parameters.HasProperties + // in if/with conditions for backward compatibility with templates + rewritePropertiesCheck(tmpl) + t := Template{Template: tmpl, raw: s} vars, err := t.Vars() if err != nil { diff --git a/tools/tools.go b/tools/tools.go index f9a2d3b9b..45780a430 100644 --- a/tools/tools.go +++ b/tools/tools.go @@ -124,16 +124,21 @@ func (p *Parser) parseToolCall() *api.ToolCall { return nil } - var args map[string]any + var argsMap map[string]any if found, i := findArguments(p.buffer); found == nil { return nil } else { - args = found + argsMap = found if i > end { end = i } } + args := api.NewToolCallFunctionArguments() + for k, v := range argsMap { + args.Set(k, v) + } + tc := &api.ToolCall{ Function: api.ToolCallFunction{ Name: tool.Function.Name, diff --git a/tools/tools_test.go b/tools/tools_test.go index 288fa73c5..c3bbfb9d4 100644 --- a/tools/tools_test.go +++ b/tools/tools_test.go @@ -6,9 +6,33 @@ import ( "text/template" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/ollama/ollama/api" ) +func makeArgs(pairs ...any) api.ToolCallFunctionArguments { + args := api.NewToolCallFunctionArguments() + for i := 0; i < len(pairs); i += 2 { + key := pairs[i].(string) + value := pairs[i+1] + args.Set(key, value) + } + return args +} + +// helper to build ToolFunctionParameters with properties +func makeParams(typ string, required []string, propsFn func() *api.ToolProperties) api.ToolFunctionParameters { + params := api.ToolFunctionParameters{ + Type: typ, + Required: required, + } + if propsFn != nil { + params.SetProperties(propsFn()) + } + return params +} + func TestParser(t *testing.T) { qwen, err := template.New("qwen").Parse(`{{if .ToolCalls}}{{range .ToolCalls}}{"name": "{{.Function.Name}}", "arguments": {{.Function.Arguments}}}{{end}}{{end}}`) if err != nil { @@ -41,21 +65,19 @@ func TestParser(t *testing.T) { Function: api.ToolFunction{ Name: "get_temperature", Description: "Retrieve the temperature for a given location", - Parameters: api.ToolFunctionParameters{ - Type: "object", - Required: []string{"city"}, - Properties: map[string]api.ToolProperty{ - "format": { + Parameters: makeParams("object", []string{"city"}, func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("format", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The format to return the temperature in", Enum: []any{"fahrenheit", "celsius"}, - }, - "city": { + }) + props.Set("city", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The city to get the temperature for", - }, - }, - }, + }) + return props + }), }, }, { @@ -63,15 +85,14 @@ func TestParser(t *testing.T) { Function: api.ToolFunction{ Name: "get_conditions", Description: "Retrieve the current weather conditions for a given location", - Parameters: api.ToolFunctionParameters{ - Type: "object", - Properties: map[string]api.ToolProperty{ - "location": { + Parameters: makeParams("object", nil, func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The location to get the weather conditions for", - }, - }, - }, + }) + return props + }), }, }, { @@ -93,15 +114,14 @@ func TestParser(t *testing.T) { Function: api.ToolFunction{ Name: "get_address", Description: "Get the address of a given location", - Parameters: api.ToolFunctionParameters{ - Type: "object", - Properties: map[string]api.ToolProperty{ - "location": { + Parameters: makeParams("object", nil, func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("location", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The location to get the address for", - }, - }, - }, + }) + return props + }), }, }, { @@ -109,19 +129,18 @@ func TestParser(t *testing.T) { Function: api.ToolFunction{ Name: "add", Description: "Add two numbers", - Parameters: api.ToolFunctionParameters{ - Type: "object", - Properties: map[string]api.ToolProperty{ - "a": { + Parameters: makeParams("object", nil, func() *api.ToolProperties { + props := api.NewToolProperties() + props.Set("a", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The first number to add", - }, - "b": { + }) + props.Set("b", api.ToolProperty{ Type: api.PropertyType{"string"}, Description: "The second number to add", - }, - }, - }, + }) + return props + }), }, }, } @@ -155,11 +174,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "San Francisco", - }, + Index: 0, + Name: "get_conditions", + Arguments: makeArgs("location", "San Francisco"), }, }, }, @@ -174,7 +191,7 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, }, @@ -187,11 +204,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "New York", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "New York"), }, }, }, @@ -211,21 +226,16 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "London", - "format": "fahrenheit", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "London", "format", "fahrenheit"), }, }, { Function: api.ToolCallFunction{ - Index: 1, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "Tokyo", - }, + Index: 1, + Name: "get_conditions", + Arguments: makeArgs("location", "Tokyo"), }, }, }, @@ -238,21 +248,16 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "London", - "format": "fahrenheit", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "London", "format", "fahrenheit"), }, }, { Function: api.ToolCallFunction{ - Index: 1, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "Tokyo", - }, + Index: 1, + Name: "get_conditions", + Arguments: makeArgs("location", "Tokyo"), }, }, }, @@ -267,17 +272,14 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "say_hello", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, { Function: api.ToolCallFunction{ - Index: 1, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "London", - "format": "fahrenheit", - }, + Index: 1, + Name: "get_temperature", + Arguments: makeArgs("city", "London", "format", "fahrenheit"), }, }, }, @@ -292,16 +294,14 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, { Function: api.ToolCallFunction{ - Index: 1, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "Tokyo", - }, + Index: 1, + Name: "get_conditions", + Arguments: makeArgs("location", "Tokyo"), }, }, }, @@ -314,11 +314,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "Tokyo", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "Tokyo"), }, }, }, @@ -345,11 +343,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "Tokyo", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "Tokyo"), }, }, }, @@ -369,11 +365,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "Tokyo", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "Tokyo"), }, }, }, @@ -451,20 +445,16 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_temperature", - Arguments: api.ToolCallFunctionArguments{ - "city": "London", - }, + Index: 0, + Name: "get_temperature", + Arguments: makeArgs("city", "London"), }, }, { Function: api.ToolCallFunction{ - Index: 1, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "Tokyo", - }, + Index: 1, + Name: "get_conditions", + Arguments: makeArgs("location", "Tokyo"), }, }, }, @@ -484,11 +474,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "Tokyo", - }, + Index: 0, + Name: "get_conditions", + Arguments: makeArgs("location", "Tokyo"), }, }, }, @@ -526,11 +514,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_conditions", - Arguments: api.ToolCallFunctionArguments{ - "location": "Tokyo", - }, + Index: 0, + Name: "get_conditions", + Arguments: makeArgs("location", "Tokyo"), }, }, }, @@ -563,7 +549,7 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "say_hello_world", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, }, @@ -591,14 +577,14 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "say_hello_world", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, { Function: api.ToolCallFunction{ Index: 1, Name: "say_hello", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, }, @@ -624,14 +610,14 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "say_hello", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, { Function: api.ToolCallFunction{ Index: 1, Name: "say_hello_world", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, }, @@ -648,7 +634,7 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "say_hello", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, }, @@ -665,7 +651,7 @@ func TestParser(t *testing.T) { Function: api.ToolCallFunction{ Index: 0, Name: "say_hello_world", - Arguments: api.ToolCallFunctionArguments{}, + Arguments: makeArgs(), }, }, }, @@ -685,11 +671,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_address", - Arguments: api.ToolCallFunctionArguments{ - "location": "London", - }, + Index: 0, + Name: "get_address", + Arguments: makeArgs("location", "London"), }, }, }, @@ -704,11 +688,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "get_address", - Arguments: api.ToolCallFunctionArguments{ - "location": "London", - }, + Index: 0, + Name: "get_address", + Arguments: makeArgs("location", "London"), }, }, }, @@ -723,12 +705,9 @@ func TestParser(t *testing.T) { calls: []api.ToolCall{ { Function: api.ToolCallFunction{ - Index: 0, - Name: "add", - Arguments: api.ToolCallFunctionArguments{ - "a": "5", - "b": "10", - }, + Index: 0, + Name: "add", + Arguments: makeArgs("a", "5", "b", "10"), }, }, }, @@ -756,7 +735,7 @@ func TestParser(t *testing.T) { } for i, want := range tt.calls { - if diff := cmp.Diff(calls[i], want); diff != "" { + if diff := cmp.Diff(calls[i], want, cmpopts.IgnoreUnexported(api.ToolCallFunctionArguments{})); diff != "" { t.Errorf("Tool call %d mismatch (-got +want):\n%s", i, diff) } }