From e7f56ef3d8ac70280b05ec66989dfe0845f8f114 Mon Sep 17 00:00:00 2001 From: Devon Rifkin Date: Thu, 18 Sep 2025 14:55:59 -0700 Subject: [PATCH] harmony: remove special casing in routes.go Now that we have a built-in parser abstraction, which was introduced in , we can modify our harmony parser to match this and then get rid of nearly all of the harmony-specific logic in routes.go. We do have a small amount of code that turns the parser on by default if the architecture matches and no other built-in parser was provided. The built-in parser interface was modified in order to handle harmony's prefill and tool name translation requirements. --- .gitignore | 1 + harmony/harmonyparser.go | 77 +++++++++++++++++++++ model/parsers/parsers.go | 16 ++++- model/parsers/qwen3coder.go | 14 ++-- server/routes.go | 132 ++++++++++++------------------------ 5 files changed, 144 insertions(+), 96 deletions(-) diff --git a/.gitignore b/.gitignore index 3a2af0bd1..eabf94c28 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ dist build .cache +.gocache *.exe .idea test_data diff --git a/harmony/harmonyparser.go b/harmony/harmonyparser.go index a51819dda..b365b763d 100644 --- a/harmony/harmonyparser.go +++ b/harmony/harmonyparser.go @@ -1,6 +1,7 @@ package harmony import ( + "encoding/json" "fmt" "log/slog" "strings" @@ -265,6 +266,8 @@ type HarmonyMessageHandler struct { state harmonyMessageState HarmonyParser *HarmonyParser FunctionNameMap *FunctionNameMap + toolAccumulator *HarmonyToolCallAccumulator + convertedTools map[string]struct{} } // NewHarmonyMessageHandler creates a new message handler @@ -277,6 +280,7 @@ func NewHarmonyMessageHandler() *HarmonyMessageHandler { HeaderEndTag: "<|message|>", }, FunctionNameMap: NewFunctionNameMap(), + convertedTools: make(map[string]struct{}), } } @@ -384,6 +388,79 @@ func NewFunctionNameMap() *FunctionNameMap { } } +// Init initializes the handler with tools and optional last message +// Implements the Parser interface +func (h *HarmonyMessageHandler) Init(tools []api.Tool, lastMessage *api.Message) []api.Tool { + // Initialize the harmony parser + if h.HarmonyParser == nil { + h.HarmonyParser = &HarmonyParser{ + MessageStartTag: "<|start|>", + MessageEndTag: "<|end|>", + HeaderEndTag: "<|message|>", + } + } + + // Handle prefill for chat mode + if lastMessage != nil { + h.HarmonyParser.AddImplicitStartOrPrefill(lastMessage) + } else { + h.HarmonyParser.AddImplicitStart() + } + + // Initialize tool accumulator + h.toolAccumulator = h.CreateToolParser() + + // Process tools and return renamed versions + if len(tools) == 0 { + return tools + } + + processedTools := make([]api.Tool, len(tools)) + copy(processedTools, tools) + for i, tool := range processedTools { + if tool.Function.Name != "" { + processedTools[i].Function.Name = h.FunctionNameMap.ConvertAndAdd(tool.Function.Name) + h.convertedTools[tool.Function.Name] = struct{}{} + } + } + return processedTools +} + +// Add implements the Parser interface - processes streamed content and extracts content, thinking, and tool calls +func (h *HarmonyMessageHandler) Add(s string, done bool) (content string, thinking string, calls []api.ToolCall, err error) { + content, thinking, toolContent := h.AddContent(s, h.toolAccumulator) + if toolContent != "" { + h.toolAccumulator.Add(toolContent) + } + + // tool calls always happen one at a time, and always at the end of a message, + // so for simplicity we defer parsing them until we know we're done + if done { + toolName, raw := h.toolAccumulator.Drain() + if toolName != nil { + name := strings.TrimPrefix(*toolName, "functions.") + name = h.FunctionNameMap.OriginalFromConverted(name) + var args api.ToolCallFunctionArguments + if err := json.Unmarshal([]byte(raw), &args); err != nil { + return "", "", nil, fmt.Errorf("error parsing tool call: raw='%s', err=%w", raw, err) + } + calls = append(calls, api.ToolCall{Function: api.ToolCallFunction{Name: name, Arguments: args}}) + } + } + + return content, thinking, calls, nil +} + +// HasToolSupport implements the Parser interface +func (h *HarmonyMessageHandler) HasToolSupport() bool { + return true +} + +// HasThinkingSupport implements the Parser interface +func (h *HarmonyMessageHandler) HasThinkingSupport() bool { + return true +} + func (m *FunctionNameMap) ConvertAndAdd(userFunctionName string) string { harmonyFunctionName := m.deriveName(userFunctionName) m.userToHarmony[userFunctionName] = harmonyFunctionName diff --git a/model/parsers/parsers.go b/model/parsers/parsers.go index e6dbd1f4f..a1d4e8127 100644 --- a/model/parsers/parsers.go +++ b/model/parsers/parsers.go @@ -2,10 +2,16 @@ package parsers import ( "github.com/ollama/ollama/api" + "github.com/ollama/ollama/harmony" ) type Parser interface { - Add(s string, tools []api.Tool) (content string, thinking string, calls []api.ToolCall, err error) + // Init initializes the parser with tools and optional last message for chat prefill + // Returns processed tools if the parser needs to modify them (e.g., harmony renames them) + Init(tools []api.Tool, lastMessage *api.Message) []api.Tool + // Add processes streamed content and returns parsed content, thinking, and tool calls + // The done flag indicates if this is the last chunk (used for draining accumulators) + Add(s string, done bool) (content string, thinking string, calls []api.ToolCall, err error) HasToolSupport() bool HasThinkingSupport() bool } @@ -17,6 +23,8 @@ func ParserForName(name string) Parser { return parser case "passthrough": return &PassthroughParser{} + case "harmony": + return harmony.NewHarmonyMessageHandler() default: return nil } @@ -24,7 +32,11 @@ func ParserForName(name string) Parser { type PassthroughParser struct{} -func (p *PassthroughParser) Add(s string, tools []api.Tool) (content string, thinking string, calls []api.ToolCall, err error) { +func (p *PassthroughParser) Init(tools []api.Tool, lastMessage *api.Message) []api.Tool { + return tools // passthrough doesn't modify tools +} + +func (p *PassthroughParser) Add(s string, done bool) (content string, thinking string, calls []api.ToolCall, err error) { return s, "", nil, nil } diff --git a/model/parsers/qwen3coder.go b/model/parsers/qwen3coder.go index b0e8ec48c..b3629a5cc 100644 --- a/model/parsers/qwen3coder.go +++ b/model/parsers/qwen3coder.go @@ -31,6 +31,7 @@ const ( type Qwen3CoderParser struct { state qwenParserState acc strings.Builder + tools []api.Tool } func (p *Qwen3CoderParser) HasToolSupport() bool { @@ -41,7 +42,12 @@ func (p *Qwen3CoderParser) HasThinkingSupport() bool { return false } -func (p *Qwen3CoderParser) Add(s string, tools []api.Tool) (content string, thinking string, calls []api.ToolCall, err error) { +func (p *Qwen3CoderParser) Init(tools []api.Tool, lastMessage *api.Message) []api.Tool { + p.tools = tools + return tools // Qwen doesn't modify tools +} + +func (p *Qwen3CoderParser) Add(s string, done bool) (content string, thinking string, calls []api.ToolCall, err error) { p.acc.WriteString(s) events := p.parseEvents() @@ -51,7 +57,7 @@ func (p *Qwen3CoderParser) Add(s string, tools []api.Tool) (content string, thin for _, event := range events { switch event := event.(type) { case qwenEventRawToolCall: - toolCall, err := parseToolCall(event, tools) + toolCall, err := parseToolCall(event, p.tools) if err != nil { slog.Warn("qwen tool call parsing failed", "error", err) return "", "", nil, err @@ -359,7 +365,7 @@ func parseValue(raw string, paramType api.PropertyType) any { // Try array if typeSet["array"] { - var arr []interface{} + var arr []any if err := json.Unmarshal([]byte(raw), &arr); err == nil { return arr } @@ -371,7 +377,7 @@ func parseValue(raw string, paramType api.PropertyType) any { // Try object if typeSet["object"] { - var obj map[string]interface{} + var obj map[string]any if err := json.Unmarshal([]byte(raw), &obj); err == nil { return obj } diff --git a/server/routes.go b/server/routes.go index c02045318..3e9407025 100644 --- a/server/routes.go +++ b/server/routes.go @@ -34,7 +34,6 @@ import ( "github.com/ollama/ollama/envconfig" "github.com/ollama/ollama/format" "github.com/ollama/ollama/fs/ggml" - "github.com/ollama/ollama/harmony" "github.com/ollama/ollama/llm" "github.com/ollama/ollama/logutil" "github.com/ollama/ollama/model/parsers" @@ -288,17 +287,21 @@ func (s *Server) GenerateHandler(c *gin.Context) { return } - useHarmony := shouldUseHarmony(m) && !req.Raw - var harmonyMessageHandler *harmony.HarmonyMessageHandler - var harmonyToolParser *harmony.HarmonyToolCallAccumulator - if useHarmony { - harmonyMessageHandler = harmony.NewHarmonyMessageHandler() - harmonyMessageHandler.HarmonyParser.AddImplicitStart() - harmonyToolParser = harmonyMessageHandler.CreateToolParser() + var builtinParser parsers.Parser + if shouldUseHarmony(m) && m.Config.Parser == "" { + m.Config.Parser = "harmony" } - // Validate Think value: string values currently only allowed for gptoss models - if req.Think != nil && req.Think.IsString() && !useHarmony { + if !req.Raw && m.Config.Parser != "" { + builtinParser = parsers.ParserForName(m.Config.Parser) + if builtinParser != nil { + // no tools or last message for generate endpoint + builtinParser.Init(nil, nil) + } + } + + // Validate Think value: string values currently only allowed for harmony/gptoss models + if req.Think != nil && req.Think.IsString() && m.Config.Parser != "harmony" { c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("think value %q is not supported for this model", req.Think.String())}) return } @@ -422,7 +425,7 @@ func (s *Server) GenerateHandler(c *gin.Context) { } var thinkingState *thinking.Parser - if !useHarmony { + if builtinParser == nil { openingTag, closingTag := thinking.InferTags(m.Template.Template) if req.Think != nil && req.Think.Bool() && openingTag != "" && closingTag != "" { thinkingState = &thinking.Parser{ @@ -459,11 +462,17 @@ func (s *Server) GenerateHandler(c *gin.Context) { }, } - if useHarmony { - content, thinking, toolContent := harmonyMessageHandler.AddContent(cr.Content, harmonyToolParser) + if builtinParser != nil { + content, thinking, toolCalls, err := builtinParser.Add(cr.Content, cr.Done) + if err != nil { + ch <- gin.H{"error": err.Error()} + return + } res.Response = content res.Thinking = thinking - harmonyToolParser.Add(toolContent) + if cr.Done && len(toolCalls) > 0 { + res.ToolCalls = toolCalls + } } else if thinkingState != nil { thinking, content := thinkingState.AddContent(cr.Content) res.Thinking = thinking @@ -475,26 +484,6 @@ func (s *Server) GenerateHandler(c *gin.Context) { } if cr.Done { - if useHarmony { - toolName, toolContent := harmonyToolParser.Drain() - if toolName != nil { - *toolName = strings.TrimPrefix(*toolName, "functions.") - var args api.ToolCallFunctionArguments - if err := json.Unmarshal([]byte(toolContent), &args); err != nil { - errStr := fmt.Sprintf("error parsing tool call: raw='%s', err=%s", toolContent, err.Error()) - ch <- gin.H{"error": errStr} - return - } - - res.ToolCalls = append(res.ToolCalls, api.ToolCall{ - Function: api.ToolCallFunction{ - Name: *toolName, - Arguments: args, - }, - }) - } - } - res.DoneReason = cr.DoneReason.String() res.TotalDuration = time.Since(checkpointStart) res.LoadDuration = checkpointLoaded.Sub(checkpointStart) @@ -509,7 +498,7 @@ func (s *Server) GenerateHandler(c *gin.Context) { } } - if useHarmony { + if builtinParser != nil { // only send messages with meaningful content (empty messages confuse clients) if res.Response != "" || res.Thinking != "" || res.Done || len(res.ToolCalls) > 0 { ch <- res @@ -1853,32 +1842,23 @@ func (s *Server) ChatHandler(c *gin.Context) { } msgs = filterThinkTags(msgs, m) - var builtinParser parsers.Parser - if m.Config.Parser != "" { - builtinParser = parsers.ParserForName(m.Config.Parser) + if shouldUseHarmony(m) && m.Config.Parser == "" { + m.Config.Parser = "harmony" } - var harmonyMessageHandler *harmony.HarmonyMessageHandler - var harmonyToolParser *harmony.HarmonyToolCallAccumulator - - useHarmony := shouldUseHarmony(m) || m.Config.Parser == "harmony" - + var builtinParser parsers.Parser processedTools := req.Tools - if useHarmony { - harmonyMessageHandler = harmony.NewHarmonyMessageHandler() - var lastMessage *api.Message - if len(msgs) > 0 { - lastMessage = &msgs[len(msgs)-1] - } - harmonyMessageHandler.HarmonyParser.AddImplicitStartOrPrefill(lastMessage) - harmonyToolParser = harmonyMessageHandler.CreateToolParser() - // make a copy of tools to pass to the chat prompt. Function names may be - // renamed to be valid Harmony function names. - processedTools = make([]api.Tool, len(req.Tools)) - copy(processedTools, req.Tools) - for i, tool := range processedTools { - processedTools[i].Function.Name = harmonyMessageHandler.FunctionNameMap.ConvertAndAdd(tool.Function.Name) + if m.Config.Parser != "" { + builtinParser = parsers.ParserForName(m.Config.Parser) + if builtinParser != nil { + // Determine last message for chat prefill + var lastMessage *api.Message + if len(msgs) > 0 { + lastMessage = &msgs[len(msgs)-1] + } + // Initialize parser and get processed tools + processedTools = builtinParser.Init(req.Tools, lastMessage) } } @@ -1902,8 +1882,8 @@ func (s *Server) ChatHandler(c *gin.Context) { return } - // Validate Think value: string values currently only allowed for gptoss models - if req.Think != nil && req.Think.IsString() && !useHarmony { + // Validate Think value: string values currently only allowed for harmony/gptoss models + if req.Think != nil && req.Think.IsString() && m.Config.Parser != "harmony" { c.JSON(http.StatusBadRequest, gin.H{"error": fmt.Sprintf("think value %q is not supported for this model", req.Think.String())}) return } @@ -1922,7 +1902,7 @@ func (s *Server) ChatHandler(c *gin.Context) { } var toolParser *tools.Parser - if len(req.Tools) > 0 && !useHarmony { + if len(req.Tools) > 0 && (builtinParser == nil || !builtinParser.HasToolSupport()) { toolParser = tools.NewParser(m.Template.Template, req.Tools) } @@ -1954,38 +1934,10 @@ func (s *Server) ChatHandler(c *gin.Context) { res.LoadDuration = checkpointLoaded.Sub(checkpointStart) } - // TODO(drifkin): fold this as much as possibleinto the generic m.Config.Parser logic - if useHarmony { - content, thinking, toolContent := harmonyMessageHandler.AddContent(r.Content, harmonyToolParser) - res.Message.Content = content - res.Message.Thinking = thinking - harmonyToolParser.Add(toolContent) - - if r.Done { - toolName, toolContent := harmonyToolParser.Drain() - if toolName != nil { - *toolName = strings.TrimPrefix(*toolName, "functions.") - *toolName = harmonyMessageHandler.FunctionNameMap.OriginalFromConverted(*toolName) - var args api.ToolCallFunctionArguments - if err := json.Unmarshal([]byte(toolContent), &args); err != nil { - errStr := fmt.Sprintf("error parsing tool call: raw='%s', err=%s", toolContent, err.Error()) - ch <- gin.H{"error": errStr} - return - } - res.Message.ToolCalls = []api.ToolCall{{Function: api.ToolCallFunction{Name: *toolName, Arguments: args}}} - } - } - - // only send messages with meaningful content (empty messages confuse clients) - if res.Message.Content != "" || res.Message.Thinking != "" || len(res.Message.ToolCalls) > 0 || res.Done { - ch <- res - } - - return - } else if builtinParser != nil { + if builtinParser != nil { slog.Log(context.TODO(), logutil.LevelTrace, "builtin parser input", "parser", m.Config.Parser, "content", r.Content) - content, thinking, toolCalls, err := builtinParser.Add(r.Content, req.Tools) + content, thinking, toolCalls, err := builtinParser.Add(r.Content, r.Done) if err != nil { ch <- gin.H{"error": err.Error()} return