diff --git a/src/llm_core.py b/src/llm_core.py index 3e84c1060..620460f19 100644 --- a/src/llm_core.py +++ b/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` diff --git a/tests/test_harmony_tool_aliasing.py b/tests/test_harmony_tool_aliasing.py new file mode 100644 index 000000000..dded8d19c --- /dev/null +++ b/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") == []