[PR #11910] [MERGED] harmony: convert fn names to be valid ts identifiers #24194

Closed
opened 2026-04-19 17:26:23 -05:00 by GiteaMirror · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/ollama/ollama/pull/11910
Author: @drifkin
Created: 8/15/2025
Status: Merged
Merged: 8/18/2025
Merged by: @drifkin

Base: mainHead: drifkin/harmony-fn-names


📝 Commits (1)

  • 048bd44 harmony: convert fn names to be valid ts identifiers

📊 Changes

3 files changed (+193 additions, -18 deletions)

View changed files

📝 server/harmonyparser.go (+99 -2)
📝 server/harmonyparser_test.go (+68 -0)
📝 server/routes.go (+26 -16)

📄 Description

In https://github.com/ollama/ollama/issues/11704#issuecomment-3177380197 I noticed that hyphens in function names could possibly cause the model to become confused. Later in that issue I found other explanations, but at a minimum tool names with spaces in them are confusing to the model because of the prompt format.

In this change I create a mapper that converts arbitrary tool names into valid typescript identifiers. It's a little overly strict in that it doesn't allow all unicode characters that might be valid in ts identifiers, but it's still very permissive. Since mappings aren't reversible, we must temporarily store this mapping in order to unmap it if the model comes back with a call. We also handle the case where multiple mappings collide into the same mapping and append a counter to the end to make them unique


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/ollama/ollama/pull/11910 **Author:** [@drifkin](https://github.com/drifkin) **Created:** 8/15/2025 **Status:** ✅ Merged **Merged:** 8/18/2025 **Merged by:** [@drifkin](https://github.com/drifkin) **Base:** `main` ← **Head:** `drifkin/harmony-fn-names` --- ### 📝 Commits (1) - [`048bd44`](https://github.com/ollama/ollama/commit/048bd4472adf2bdb41c16010ab286e072252bfd2) harmony: convert fn names to be valid ts identifiers ### 📊 Changes **3 files changed** (+193 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `server/harmonyparser.go` (+99 -2) 📝 `server/harmonyparser_test.go` (+68 -0) 📝 `server/routes.go` (+26 -16) </details> ### 📄 Description In <https://github.com/ollama/ollama/issues/11704#issuecomment-3177380197> I noticed that hyphens in function names could possibly cause the model to become confused. Later in that issue I found other explanations, but at a minimum tool names with spaces in them are confusing to the model because of the prompt format. In this change I create a mapper that converts arbitrary tool names into valid typescript identifiers. It's a little overly strict in that it doesn't allow all unicode characters that might be valid in ts identifiers, but it's still very permissive. Since mappings aren't reversible, we must temporarily store this mapping in order to unmap it if the model comes back with a call. We also handle the case where multiple mappings collide into the same mapping and append a counter to the end to make them unique --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-04-19 17:26:23 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/ollama#24194