model/parsers: finalize incomplete GLM tool calls (#17250)

The GLM parser buffered tool calls until it observed </tool_call>, but ignored the terminal done signal. If the model omitted or partially emitted the outer closing tag, Ollama returned a successful empty response instead of a tool call or an actionable error, leaving coding agents unable to continue.

On end-of-stream, finalize only structurally complete calls for declared tools with all required arguments. Complete calls missing only the outer delimiter now proceed through the existing parser, while genuinely truncated calls return an explicit error rather than being silently dropped.

Fixes #16497
This commit is contained in:
Daniel Hiltgen
2026-07-22 13:53:47 -07:00
committed by GitHub
parent 479664e7aa
commit b517b9bd01
2 changed files with 203 additions and 0 deletions

View File

@@ -96,6 +96,14 @@ func (p *GLM46Parser) Add(s string, done bool) (content string, thinking string,
p.buffer.WriteString(s) p.buffer.WriteString(s)
events := p.parseEvents() events := p.parseEvents()
if done && (p.state == glm46ParserState_ToolStartedEatingWhitespace || p.state == glm46ParserState_CollectingToolContent) {
event, err := p.finalizeToolCall()
if err != nil {
return "", "", nil, fmt.Errorf("incomplete GLM tool call: %v", err)
}
events = append(events, event)
}
var toolCalls []api.ToolCall var toolCalls []api.ToolCall
var contentSb strings.Builder var contentSb strings.Builder
var thinkingSb strings.Builder var thinkingSb strings.Builder
@@ -123,6 +131,66 @@ func (p *GLM46Parser) Add(s string, done bool) (content string, thinking string,
return contentSb.String(), thinkingSb.String(), toolCalls, nil return contentSb.String(), thinkingSb.String(), toolCalls, nil
} }
func (p *GLM46Parser) finalizeToolCall() (glm46EventRawToolCall, error) {
raw := p.buffer.String()
if overlapLen := overlap(raw, glm46ToolCloseTag); overlapLen > 0 {
raw = strings.TrimRightFunc(raw[:len(raw)-overlapLen], unicode.IsSpace)
}
escaped := escapeGLM46Content(raw)
var parsed GLMToolCallXML
if err := xml.Unmarshal([]byte("<tool_call>"+escaped+"</tool_call>"), &parsed); err != nil {
return glm46EventRawToolCall{}, err
}
if err := validateFinalGLM46ToolCall(parsed, p.tools); err != nil {
return glm46EventRawToolCall{}, err
}
p.buffer.Reset()
p.state = glm46ParserState_CollectingContent
return glm46EventRawToolCall{raw: raw}, nil
}
// validateFinalGLM46ToolCall is intentionally stricter than normal GLM parsing.
// At end-of-stream only the outer closing tag may be missing; repairing a
// truncated argument could turn partial model output into a mutating tool call.
func validateFinalGLM46ToolCall(parsed GLMToolCallXML, tools []api.Tool) error {
functionName := strings.TrimSpace(parsed.Content)
if functionName == "" {
return fmt.Errorf("empty function name")
}
if len(parsed.Keys) != len(parsed.Values) {
return fmt.Errorf("mismatched arg_key and arg_value counts: %d keys, %d values", len(parsed.Keys), len(parsed.Values))
}
var declaredTool *api.Tool
for i := range tools {
if tools[i].Function.Name == functionName {
declaredTool = &tools[i]
break
}
}
if declaredTool == nil {
return fmt.Errorf("tool %q is not declared", functionName)
}
seen := make(map[string]struct{}, len(parsed.Keys))
for _, rawKey := range parsed.Keys {
key := strings.TrimSpace(rawKey)
if key == "" {
return fmt.Errorf("empty argument name")
}
seen[key] = struct{}{}
}
for _, required := range declaredTool.Function.Parameters.Required {
if _, ok := seen[required]; !ok {
return fmt.Errorf("required argument %q is missing for tool %q", required, functionName)
}
}
return nil
}
func (p *GLM46Parser) parseEvents() []glm46Event { func (p *GLM46Parser) parseEvents() []glm46Event {
var all []glm46Event var all []glm46Event

View File

@@ -3,6 +3,7 @@ package parsers
import ( import (
"encoding/xml" "encoding/xml"
"reflect" "reflect"
"strings"
"testing" "testing"
"github.com/ollama/ollama/api" "github.com/ollama/ollama/api"
@@ -445,6 +446,140 @@ func TestGLM46ParserStreaming(t *testing.T) {
} }
} }
func TestGLM46ParserFinalizesCompleteToolCallOnDone(t *testing.T) {
type chunk struct {
content string
done bool
}
toolBody := `grep
<arg_key>pattern</arg_key>
<arg_value>needle</arg_value>
<arg_key>path</arg_key>
<arg_value>.</arg_value>`
tests := []struct {
name string
chunks []chunk
}{
{
name: "empty final chunk",
chunks: []chunk{
{content: "<tool_call>" + toolBody},
{done: true},
},
},
{
name: "tool body in final chunk",
chunks: []chunk{
{content: "<tool_call>" + toolBody, done: true},
},
},
{
name: "partial outer close in final chunk",
chunks: []chunk{
{content: "<tool_call>" + toolBody + "</tool_", done: true},
},
},
}
tools := glm46FinalizationTestTools()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
parser := GLM46Parser{}
parser.Init(tools, nil, nil)
var calls []api.ToolCall
for _, chunk := range tt.chunks {
content, thinking, got, err := parser.Add(chunk.content, chunk.done)
if err != nil {
t.Fatal(err)
}
if content != "" || thinking != "" {
t.Fatalf("content=%q thinking=%q, want empty", content, thinking)
}
calls = append(calls, got...)
}
if len(calls) != 1 {
t.Fatalf("got %d tool calls, want 1", len(calls))
}
if calls[0].Function.Name != "grep" {
t.Fatalf("tool name=%q, want grep", calls[0].Function.Name)
}
if pattern, ok := calls[0].Function.Arguments.Get("pattern"); !ok || pattern != "needle" {
t.Fatalf("pattern=%#v, %v; want needle", pattern, ok)
}
if path, ok := calls[0].Function.Arguments.Get("path"); !ok || path != "." {
t.Fatalf("path=%#v, %v; want .", path, ok)
}
})
}
}
func TestGLM46ParserRejectsIncompleteToolCallOnDone(t *testing.T) {
tests := []struct {
name string
input string
}{
{name: "empty body", input: "<tool_call>"},
{name: "partial tool name", input: "<tool_call>gr"},
{
name: "incomplete argument value",
input: `<tool_call>write
<arg_key>path</arg_key>
<arg_value>out.txt</arg_value>
<arg_key>content</arg_key>
<arg_value>partial`,
},
{
name: "undeclared tool",
input: `<tool_call>shell
<arg_key>command</arg_key>
<arg_value>echo unsafe</arg_value>`,
},
{
name: "missing required argument",
input: `<tool_call>write
<arg_key>path</arg_key>
<arg_value>out.txt</arg_value>`,
},
}
tools := glm46FinalizationTestTools()
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
parser := GLM46Parser{}
parser.Init(tools, nil, nil)
content, thinking, calls, err := parser.Add(tt.input, true)
if err == nil || !strings.Contains(err.Error(), "incomplete GLM tool call") {
t.Fatalf("error=%v, want incomplete GLM tool call", err)
}
if content != "" || thinking != "" || len(calls) != 0 {
t.Fatalf("content=%q thinking=%q calls=%v, want no output", content, thinking, calls)
}
})
}
}
func glm46FinalizationTestTools() []api.Tool {
grep := tool("grep", map[string]api.ToolProperty{
"pattern": {Type: api.PropertyType{"string"}},
"path": {Type: api.PropertyType{"string"}},
})
grep.Function.Parameters.Required = []string{"pattern", "path"}
write := tool("write", map[string]api.ToolProperty{
"path": {Type: api.PropertyType{"string"}},
"content": {Type: api.PropertyType{"string"}},
})
write.Function.Parameters.Required = []string{"path", "content"}
return []api.Tool{grep, write}
}
// TestGLMToolCallXMLOrderPreservation verifies that xml.Unmarshal preserves // TestGLMToolCallXMLOrderPreservation verifies that xml.Unmarshal preserves
// document order when collecting multiple elements with the same tag name into slices. // document order when collecting multiple elements with the same tag name into slices.
// This is a critical assumption for the GLM-4.6 parser's struct-based approach. // This is a critical assumption for the GLM-4.6 parser's struct-based approach.