Merge f3bbbece59 into fb8c391a88

src/llm_core.py

@@ -1,6 +1,7 @@
 # src/llm_core.py
 import httpx
 import asyncio
+import copy
 import time
 import json
 import logging
@@ -644,7 +645,7 @@ def _build_ollama_payload(
     if options:
         payload["options"] = options
     if tools:
-        payload["tools"] = tools
+        payload["tools"] = _alias_harmony_tools(tools, model)
     return payload
 
 
@@ -1055,6 +1056,57 @@ def _model_disallows_reasoning_effort_with_chat_tools(model: str) -> bool:
     return bool(re.match(r"^(?:openai/)?gpt-5(?:[.\-]\d+)?(?:[-_:].*)?$", m))
 
 
+# gpt-oss (harmony) ships BUILT-IN tools named `python` and `browser`, invoked
+# with the raw body as the argument (`to=python` + bare source), while custom
+# functions use `to=functions.NAME` + JSON. A tool we expose under a built-in's
+# name therefore gets called with the built-in convention: the model emits raw
+# code, the server tries to parse it as JSON, and the whole request dies
+# ("error parsing tool call: raw='import sys, ...'"). In streaming mode Ollama
+# does not even report it — it truncates the stream, so the turn looks like an
+# empty response. `bash` collides the same way in practice.
+#
+# Measured on gpt-oss:20b via Ollama /v1 with a fixed agentic prompt:
+#   tools named python+bash ............ 2/6 succeeded (4 parse failures)
+#   python renamed ..................... 5/6
+#   python and bash renamed ............ 6/6
+#
+# So rename the colliding tools on the way out and map the names back on the
+# way in. Confined to the transport layer: callers keep using the real names.
+_HARMONY_TOOL_ALIASES = {
+    "python": "run_python_code",
+    "bash": "run_shell_command",
+    "browser": "web_browser_tool",
+}
+_HARMONY_TOOL_ALIASES_REVERSE = {v: k for k, v in _HARMONY_TOOL_ALIASES.items()}
+
+
+def _is_harmony_model(model: str) -> bool:
+    """True for gpt-oss / harmony-format models, which have built-in tool names."""
+    return "gpt-oss" in (model or "").lower()
+
+
+def _alias_harmony_tools(tools: Optional[List[Dict]], model: str) -> Optional[List[Dict]]:
+    """Rename tools that collide with harmony built-ins. Returns a copy."""
+    if not tools or not _is_harmony_model(model):
+        return tools
+    out = []
+    for t in tools:
+        fn = t.get("function") or {}
+        alias = _HARMONY_TOOL_ALIASES.get(fn.get("name"))
+        if alias:
+            t = copy.deepcopy(t)
+            t["function"]["name"] = alias
+        out.append(t)
+    return out
+
+
+def _unalias_harmony_tool_name(name: str, model: str) -> str:
+    """Map an aliased tool name in a model response back to the real name."""
+    if not _is_harmony_model(model):
+        return name
+    return _HARMONY_TOOL_ALIASES_REVERSE.get(name, name)
+
+
 def _scrub_openai_chat_tool_reasoning(payload: Dict, target_url: str, model: str) -> None:
     if not payload.get("tools"):
         return
@@ -2226,7 +2278,7 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
             tok_key = "max_completion_tokens" if _uses_max_completion_tokens(model) else "max_tokens"
             payload[tok_key] = max_tokens
         if tools:
-            payload["tools"] = tools
+            payload["tools"] = _alias_harmony_tools(tools, model)
         elif tool_choice_none:
             payload["tool_choice"] = "none"
         # Mistral thinking-capable models — send reasoning_effort so Mistral
@@ -2360,7 +2412,7 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
                         if fn.get("name"):
                             _ollama_tool_calls.append({
                                 "id": tc.get("id") or f"call_{len(_ollama_tool_calls)}",
-                                "name": fn.get("name") or "",
+                                "name": _unalias_harmony_tool_name(fn.get("name") or "", model),
                                 "arguments": json.dumps(fn.get("arguments") or {}),
                             })
                     if j.get("done"):
@@ -2740,7 +2792,10 @@ async def _stream_llm_inner(url: str, model: str, messages: List[Dict], temperat
                                             if tc.get("extra_content"):
                                                 _tc_acc[idx]["extra_content"] = tc["extra_content"]
                                             if func.get("name"):
-                                                _tc_acc[idx]["name"] = func["name"]
+                                                # Map harmony aliases back to real
+                                                # tool names before anything
+                                                # downstream sees them.
+                                                _tc_acc[idx]["name"] = _unalias_harmony_tool_name(func["name"], model)
                                             if "arguments" in func:
                                                 # Guard against a null arguments delta: `func` can be
                                                 # {"arguments": None} (JSON null), and a raw `+= None`

tests/test_harmony_tool_aliasing.py

@@ -0,0 +1,78 @@
+"""Tools whose names collide with harmony built-ins must be aliased for gpt-oss.
+
+gpt-oss (harmony format) ships BUILT-IN tools named `python` and `browser`,
+invoked with the raw body as the argument (`to=python` + bare source), while
+custom functions use `to=functions.NAME` + JSON. Exposing our own tool under a
+built-in's name makes the model answer with the built-in convention: it emits
+raw code, the server tries to parse it as JSON, and the request dies with
+"error parsing tool call: raw='import sys, ...'". Streaming is worse — Ollama
+truncates the stream instead of reporting it, so the turn looks like an empty
+response and the agent loop reads it as a stall.
+
+Measured on gpt-oss:20b via Ollama /v1 with a fixed agentic prompt:
+python+bash as-is 2/6, python renamed 5/6, both renamed 6/6.
+
+The aliasing is transport-only and gpt-oss-only: every other model's tool
+schemas must pass through untouched, and real tool names must come back out.
+"""
+from src.llm_core import (
+    _alias_harmony_tools,
+    _unalias_harmony_tool_name,
+    _is_harmony_model,
+)
+
+
+def _tools(*names):
+    return [
+        {"type": "function", "function": {"name": n, "parameters": {}}}
+        for n in names
+    ]
+
+
+def _names(tools):
+    return [t["function"]["name"] for t in tools]
+
+
+def test_gpt_oss_colliding_names_are_aliased():
+    out = _alias_harmony_tools(_tools("python", "bash", "web_search"), "gpt-oss:20b")
+    assert _names(out) == ["run_python_code", "run_shell_command", "web_search"]
+
+
+def test_non_harmony_models_are_untouched():
+    tools = _tools("python", "bash", "web_search")
+    for model in ("qwen3-coder:30b", "gemma4:12b", "claude-opus-5", "gpt-4o", "llama-3.3"):
+        out = _alias_harmony_tools(tools, model)
+        assert _names(out) == ["python", "bash", "web_search"], model
+        assert out is tools, f"{model} should get the same list object, not a copy"
+
+
+def test_aliasing_does_not_mutate_the_caller_list():
+    tools = _tools("python")
+    _alias_harmony_tools(tools, "gpt-oss:20b")
+    assert _names(tools) == ["python"], "caller's schema list must not be mutated"
+
+
+def test_response_names_map_back_for_gpt_oss():
+    assert _unalias_harmony_tool_name("run_python_code", "gpt-oss:20b") == "python"
+    assert _unalias_harmony_tool_name("run_shell_command", "gpt-oss:20b") == "bash"
+    # Unrelated names pass through untouched.
+    assert _unalias_harmony_tool_name("web_search", "gpt-oss:20b") == "web_search"
+
+
+def test_response_names_untouched_for_other_models():
+    # A non-harmony model that genuinely has a tool called run_python_code
+    # must not have it rewritten to `python`.
+    assert _unalias_harmony_tool_name("run_python_code", "qwen3-coder:30b") == "run_python_code"
+
+
+def test_harmony_detection():
+    assert _is_harmony_model("gpt-oss:20b") is True
+    assert _is_harmony_model("GPT-OSS:120B") is True
+    assert _is_harmony_model("qwen3-coder:30b") is False
+    assert _is_harmony_model("") is False
+    assert _is_harmony_model(None) is False
+
+
+def test_empty_and_none_tools_are_safe():
+    assert _alias_harmony_tools(None, "gpt-oss:20b") is None
+    assert _alias_harmony_tools([], "gpt-oss:20b") == []