template: add tool result compatibility #11294
Conversation
api/types.go
Outdated
| @@ -159,6 +165,7 @@ func (m *Message) UnmarshalJSON(b []byte) error { | |||
|
|
|||
| type ToolCall struct { | |||
| Function ToolCallFunction `json:"function"` | |||
| ID string `json:"id"` | |||
There was a problem hiding this comment.
This ID is a pass through from user to us - not really required yet but added for server-side management of tools
server/routes.go
Outdated
| // Create a hash of the function arguments | ||
| argHash := sha256.Sum256([]byte(toolCalls[i].Function.Arguments.String())) | ||
| // Create a meaningful ID that includes function name and first 8 chars of arg hash | ||
| toolCalls[i].ID = fmt.Sprintf("%s_%x", toolCalls[i].Function.Name, argHash[:4]) |
There was a problem hiding this comment.
hash is not unique- based on args and name only. can add a time component here too
| if len(collated) > 0 && collated[len(collated)-1].Role == msg.Role { | ||
| collated[len(collated)-1].Content += "\n\n" + msg.Content | ||
| // merges consecutive messages of the same role into a single message (except for tool messages) | ||
| if len(collated) > 0 && collated[len(collated)-1].Role == msgs[i].Role && msgs[i].Role != "tool" { |
There was a problem hiding this comment.
we should not collate any messages in general but currently leaving most logic for it in place
There was a problem hiding this comment.
That's a good change for now! Agreed we should remove this entirely at some point
| @@ -163,10 +163,12 @@ func TestParse(t *testing.T) { | |||
| {"{{ .System }} {{ .Prompt }} {{ .Response }}", []string{"prompt", "response", "system"}}, | |||
| {"{{ with .Tools }}{{ . }}{{ end }} {{ .System }} {{ .Prompt }}", []string{"prompt", "response", "system", "tools"}}, | |||
| {"{{ range .Messages }}{{ .Role }} {{ .Content }}{{ end }}", []string{"content", "messages", "role"}}, | |||
| {"{{ range .Messages }}{{ if eq .Role \"tool\" }}Tool Result: {{ .ToolName }} {{ .Content }}{{ end }}{{ end }}", []string{"content", "messages", "role", "toolname"}}, | |||
There was a problem hiding this comment.
We'd be able to add ID similarly if needed. possible that a model creates one which should be easy to check and pipe through
api/types.go
Outdated
|
|
||
| type ToolResult struct { | ||
| ToolName string `json:"tool_name,omitempty"` | ||
| ID string `json:"tool_id,omitempty"` |
There was a problem hiding this comment.
okay to leave internal field as ID or should I match it?
api/types.go
Outdated
| @@ -143,6 +143,12 @@ type Message struct { | |||
| Thinking string `json:"thinking,omitempty"` | |||
| Images []ImageData `json:"images,omitempty"` | |||
| ToolCalls []ToolCall `json:"tool_calls,omitempty"` | |||
| ToolResult | |||
There was a problem hiding this comment.
This seems like a great design inline with others:
- A model can output one or more tool calls
- For each tool call the API consumer then needs to create a message per tool result, setting these fields
Would it be possible to update the api.md docs?
| @@ -163,10 +163,12 @@ func TestParse(t *testing.T) { | |||
| {"{{ .System }} {{ .Prompt }} {{ .Response }}", []string{"prompt", "response", "system"}}, | |||
| {"{{ with .Tools }}{{ . }}{{ end }} {{ .System }} {{ .Prompt }}", []string{"prompt", "response", "system", "tools"}}, | |||
| {"{{ range .Messages }}{{ .Role }} {{ .Content }}{{ end }}", []string{"content", "messages", "role"}}, | |||
| {"{{ range .Messages }}{{ if eq .Role \"tool\" }}Tool Result: {{ .ToolName }} {{ .Content }}{{ end }}{{ end }}", []string{"content", "messages", "role", "toolname"}}, | |||
There was a problem hiding this comment.
Templates that adopt this variable will break on old versions of Ollama since only top level variables can be non-existent (we saw a similar thing when @drifkin added `.Thinking).
I think this is OK, however we'll have to update the minimum version of the models that adopt this new variable, or find a way to make it backwards compatible (I don't see an easy way)
There was a problem hiding this comment.
Yeah – new templates will break on older versions of the engine we can update the minimum value. Had tried to avoid this but templates are a bit limiting and not worth the time sink
e69700a to
96fe109
Compare
96fe109 to
e0865db
Compare
* origin/main: ggml: Report ordinal IDs for AMD GPUs on Windows doc: add MacOS docs (ollama#11334) Reduce default parallelism to 1 (ollama#11330) API/CLI context enhancements (ollama#11331) add `tool_name` to api.md (ollama#11326) template: add tool result compatibility (ollama#11294) ci: modularization (ollama#11324) Revert "ggml: Temporarily disable reporting UUIDs" readme: update Ollama icon size int: add performance integration tests (ollama#11173) doc: add NVIDIA blackwell to supported list (ollama#11307) Update base image to Ubuntu 24.04 LTS (ollama#9681) doc: Update link for mac install (ollama#11288) mimic logs for layers on new engine (ollama#11278) readme: add NativeMind to community integrations (ollama#11242) tools: fix parsing tool calls with empty arguments, missing required fields (ollama#11233) readme: add ollama-bash-toolshed to community integrations (ollama#11224)
To allow for better templating with tool use models - template changes incoming