From 74836ce65f9a433c8ffaa2133240cb85685eb005 Mon Sep 17 00:00:00 2001 From: FenjuFu Date: Wed, 30 Sep 2026 20:38:19 +0800 Subject: [PATCH 1/3] fix(llm): forward tool definitions to Ollama OllamaProvider built the /api/chat payload without the tools from LLMInput, so Ollama models were never told any tools existed and a ReActAgent backed by Ollama never ran one. The response side already parsed message.tool_calls. Send the tools in the OpenAI function format that /api/chat accepts, matching the other providers. Fixes #3274 Signed-off-by: FenjuFu --- src/llm/providers/ollama.py | 2 ++ tests/test_provider_tools.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/llm/providers/ollama.py b/src/llm/providers/ollama.py index 58505ac7b..c36304c93 100644 --- a/src/llm/providers/ollama.py +++ b/src/llm/providers/ollama.py @@ -78,6 +78,8 @@ class OllamaProvider(LLMProvider): options["num_predict"] = input.max_tokens if options: payload["options"] = options + if input.tools: + payload["tools"] = [tool.to_openai_tool() for tool in input.tools] data = json.dumps(payload).encode("utf-8") req = urllib.request.Request(url, data=data, headers={"Content-Type": "application/json"}) diff --git a/tests/test_provider_tools.py b/tests/test_provider_tools.py index ba4c09415..f73f5fdd6 100644 --- a/tests/test_provider_tools.py +++ b/tests/test_provider_tools.py @@ -168,6 +168,37 @@ def test_ollama_provider_serializes_generation_options( assert output.stop_reason == "stop" +def test_ollama_provider_serializes_tools_and_parses_tool_calls(monkeypatch): + requests = [] + + def fake_urlopen(request, timeout): + requests.append(request) + return BytesIO( + b'{"message": {"content": "", "tool_calls": [{"function": ' + b'{"name": "search", "arguments": {"query": "ollama"}}}]}, "done_reason": "stop"}' + ) + + monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) + provider = OllamaProvider(base_url="http://localhost:11434", default_model="qwen3") + + output = provider.generate(LLMInput(messages=[Message(role=Role.USER, content="hi")], tools=[_tool()])) + + assert json.loads(requests[0].data)["tools"] == [ + { + "type": "function", + "function": { + "name": "search", + "description": "Search", + "parameters": {"type": "object", "properties": {"query": {"type": "string"}}}, + "strict": True, + }, + } + ] + assert output.has_tool_calls + assert output.tool_calls[0].name == "search" + assert output.tool_calls[0].arguments == {"query": "ollama"} + + def test_claude_provider_serializes_tools_for_messages_api(): provider = ClaudeProvider(api_key="test") client = _AnthropicClient() From 558c06c3f88cdc95507f87b64ab7964396cac438 Mon Sep 17 00:00:00 2001 From: FenjuFu <92919259+FenjuFu@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:15:58 +0800 Subject: [PATCH 2/3] fix(llm): send Ollama tools only to tool-capable models and keep call identity Tools are now forwarded only when the selected model declares tool support: catalogued models answer from ModelInfo.supports_tools (llama3.2 and mistral are tool models on ollama.com, codellama is not), and any other model is looked up once through /api/show, whose capabilities list is what the installed model declares. A failed lookup sends no tools. Native Ollama tool calls often have no id, so each one gets a unique id, and tool results are sent with tool_name resolved from the call they answer, so several results stay attributable. Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com> --- src/llm/providers/ollama.py | 73 +++++++++++++++-- tests/test_provider_tools.py | 151 ++++++++++++++++++++++++++++++++--- 2 files changed, 205 insertions(+), 19 deletions(-) diff --git a/src/llm/providers/ollama.py b/src/llm/providers/ollama.py index c36304c93..65256b179 100644 --- a/src/llm/providers/ollama.py +++ b/src/llm/providers/ollama.py @@ -2,7 +2,11 @@ from __future__ import annotations +import json +import logging import os +import urllib.request +import uuid from typing import Any from llm.core.interface import ( @@ -15,11 +19,35 @@ from llm.core.interface import ( from llm.core.types import ( LLMInput, LLMOutput, + Message, ModelInfo, ProviderType, + Role, ToolCall, ) +logger = logging.getLogger(__name__) + + +def _messages_payload(messages: list[Message]) -> list[dict[str, Any]]: + """Serialize messages, naming the tool each tool result answers. + + Ollama correlates a tool result with its call through ``tool_name``; ids + are optional on its side, so the name is what keeps several results apart. + """ + names: dict[str, str] = {} + payload = [] + for msg in messages: + item = msg.to_dict() + for tc in msg.tool_calls or []: + names[tc.id] = tc.name + if msg.role == Role.TOOL: + tool_name = msg.name or names.get(msg.tool_call_id or "") + if tool_name: + item["tool_name"] = tool_name + payload.append(item) + return payload + class OllamaProvider(LLMProvider): provider_type = ProviderType.OLLAMA @@ -35,7 +63,7 @@ class OllamaProvider(LLMProvider): ModelInfo( name="llama3.2", provider=ProviderType.OLLAMA, - supports_tools=False, + supports_tools=True, supports_vision=False, max_tokens=4096, context_window=128000, @@ -43,7 +71,7 @@ class OllamaProvider(LLMProvider): ModelInfo( name="mistral", provider=ProviderType.OLLAMA, - supports_tools=False, + supports_tools=True, supports_vision=False, max_tokens=4096, context_window=8192, @@ -57,18 +85,42 @@ class OllamaProvider(LLMProvider): context_window=16384, ), ] + self._tool_support: dict[str, bool] = {} + + def model_supports_tools(self, model: str) -> bool: + """Whether ``model`` declares tool support. + + Catalogued models answer from :attr:`ModelInfo.supports_tools`. Any other + model is looked up once through ``/api/show``, whose ``capabilities`` + list is what the installed model itself declares. A failed lookup counts + as no tool support. + """ + for info in self._models: + if info.name == model: + return info.supports_tools + if model not in self._tool_support: + try: + req = urllib.request.Request( + f"{self.base_url}/api/show", + data=json.dumps({"model": model}).encode("utf-8"), + headers={"Content-Type": "application/json"}, + ) + with urllib.request.urlopen(req, timeout=10) as response: + capabilities = json.loads(response.read().decode("utf-8")).get("capabilities") or [] + self._tool_support[model] = "tools" in capabilities + except Exception as e: + logger.warning("Could not read capabilities for Ollama model '%s': %s", model, type(e).__name__) + self._tool_support[model] = False + return self._tool_support[model] def generate(self, input: LLMInput) -> LLMOutput: - import json - import urllib.request - try: url = f"{self.base_url}/api/chat" model = input.model or self.default_model payload: dict[str, Any] = { "model": model, - "messages": [msg.to_dict() for msg in input.messages], + "messages": _messages_payload(input.messages), "stream": False, } options: dict[str, Any] = {} @@ -79,7 +131,10 @@ class OllamaProvider(LLMProvider): if options: payload["options"] = options if input.tools: - payload["tools"] = [tool.to_openai_tool() for tool in input.tools] + if self.model_supports_tools(model): + payload["tools"] = [tool.to_openai_tool() for tool in input.tools] + else: + logger.warning("Ollama model '%s' does not declare tool support; sending no tools", model) data = json.dumps(payload).encode("utf-8") req = urllib.request.Request(url, data=data, headers={"Content-Type": "application/json"}) @@ -93,7 +148,9 @@ class OllamaProvider(LLMProvider): if result.get("message", {}).get("tool_calls"): tool_calls = [ ToolCall( - id=tc.get("id", ""), + # Native Ollama calls often carry no id; give each one a + # unique id so its result can be matched to it. + id=tc.get("id") or f"call_{uuid.uuid4().hex[:24]}", name=tc.get("function", {}).get("name", ""), arguments=tc.get("function", {}).get("arguments", {}), ) diff --git a/tests/test_provider_tools.py b/tests/test_provider_tools.py index f73f5fdd6..caa0290c4 100644 --- a/tests/test_provider_tools.py +++ b/tests/test_provider_tools.py @@ -1,3 +1,4 @@ +import asyncio import json import urllib.request from io import BytesIO @@ -10,6 +11,7 @@ from llm.providers.claude import ClaudeProvider from llm.providers.constants import EMPTY_FILTERED_RESPONSE_ERROR from llm.providers.ollama import OllamaProvider from llm.providers.openai import OpenAIProvider +from llm.tools import ReActAgent, ToolExecutor, ToolRegistry def _tool() -> ToolDefinition: @@ -168,22 +170,59 @@ def test_ollama_provider_serializes_generation_options( assert output.stop_reason == "stop" +class _FakeOllama: + """Serves /api/show and replays queued /api/chat replies, recording requests.""" + + def __init__(self, monkeypatch, replies, capabilities=None): + self.replies = list(replies) + self.capabilities = capabilities + self.chats = [] + self.shows = [] + monkeypatch.setattr(urllib.request, "urlopen", self.urlopen) + + def urlopen(self, request, timeout): + body = json.loads(request.data) + if request.full_url.endswith("/api/show"): + self.shows.append(body["model"]) + if self.capabilities is None: + raise OSError("connection refused") + return BytesIO(json.dumps({"capabilities": self.capabilities}).encode()) + self.chats.append(body) + return BytesIO(json.dumps(self.replies.pop(0)).encode()) + + +def _ollama_tool_calls(*calls): + return { + "message": { + "content": "", + "tool_calls": [{"function": {"name": name, "arguments": args}} for name, args in calls], + }, + "done_reason": "stop", + } + + +def _ollama_answer(text): + return {"message": {"content": text}, "done_reason": "stop"} + + +def _lookup_tool() -> ToolDefinition: + return ToolDefinition( + name="lookup", + description="Look up a city", + parameters={"type": "object", "properties": {"city": {"type": "string"}}}, + ) + + def test_ollama_provider_serializes_tools_and_parses_tool_calls(monkeypatch): - requests = [] - - def fake_urlopen(request, timeout): - requests.append(request) - return BytesIO( - b'{"message": {"content": "", "tool_calls": [{"function": ' - b'{"name": "search", "arguments": {"query": "ollama"}}}]}, "done_reason": "stop"}' - ) - - monkeypatch.setattr(urllib.request, "urlopen", fake_urlopen) + server = _FakeOllama( + monkeypatch, [_ollama_tool_calls(("search", {"query": "ollama"}))], capabilities=["completion", "tools"] + ) provider = OllamaProvider(base_url="http://localhost:11434", default_model="qwen3") output = provider.generate(LLMInput(messages=[Message(role=Role.USER, content="hi")], tools=[_tool()])) - assert json.loads(requests[0].data)["tools"] == [ + assert server.shows == ["qwen3"] + assert server.chats[0]["tools"] == [ { "type": "function", "function": { @@ -197,6 +236,96 @@ def test_ollama_provider_serializes_tools_and_parses_tool_calls(monkeypatch): assert output.has_tool_calls assert output.tool_calls[0].name == "search" assert output.tool_calls[0].arguments == {"query": "ollama"} + assert output.tool_calls[0].id + + +@pytest.mark.parametrize( + ("model", "capabilities", "expect_tools", "expect_shows"), + [ + ("llama3.2", None, True, []), + ("codellama", None, False, []), + ("qwen3", ["completion", "tools"], True, ["qwen3"]), + ("gemma3", ["completion", "vision"], False, ["gemma3"]), + ("unknown", None, False, ["unknown"]), + ], +) +def test_ollama_provider_sends_tools_only_to_models_that_declare_them( + monkeypatch, model, capabilities, expect_tools, expect_shows +): + server = _FakeOllama(monkeypatch, [_ollama_answer("ok"), _ollama_answer("ok")], capabilities=capabilities) + provider = OllamaProvider(default_model=model) + request = LLMInput(messages=[Message(role=Role.USER, content="hi")], tools=[_tool()]) + + provider.generate(request) + provider.generate(request) + + assert all(("tools" in chat) is expect_tools for chat in server.chats) + # The capability lookup is cached per model. + assert server.shows == expect_shows + + +def test_ollama_react_agent_correlates_multiple_calls_without_ids(monkeypatch): + server = _FakeOllama( + monkeypatch, + [ + _ollama_tool_calls(("lookup", {"city": "Hefei"}), ("search", {"query": "weather"})), + _ollama_answer("Hefei is sunny."), + ], + capabilities=["completion", "tools"], + ) + registry = ToolRegistry() + registry.register(_lookup_tool(), lambda city: f"city={city}") + registry.register(_tool(), lambda query: f"query={query}") + agent = ReActAgent(OllamaProvider(default_model="qwen3"), ToolExecutor(registry)) + + output = asyncio.run( + agent.run( + LLMInput( + messages=[Message(role=Role.USER, content="Weather in Hefei?")], + tools=[_lookup_tool(), _tool()], + ) + ) + ) + + assert output.content == "Hefei is sunny." + follow_up = server.chats[1] + assert "tools" in follow_up + assistant, first, second = follow_up["messages"][1:] + ids = [call["id"] for call in assistant["tool_calls"]] + assert all(ids) and len(set(ids)) == 2 + assert [call["function"]["name"] for call in assistant["tool_calls"]] == ["lookup", "search"] + assert first == {"role": "tool", "content": "city=Hefei", "tool_call_id": ids[0], "tool_name": "lookup"} + assert second == {"role": "tool", "content": "query=weather", "tool_call_id": ids[1], "tool_name": "search"} + + +def test_ollama_react_agent_reports_unknown_tool_under_its_own_name(monkeypatch): + server = _FakeOllama( + monkeypatch, + [_ollama_tool_calls(("missing", {}), ("search", {"query": "x"})), _ollama_answer("done")], + capabilities=["completion", "tools"], + ) + registry = ToolRegistry() + registry.register(_tool(), lambda query: f"query={query}") + agent = ReActAgent(OllamaProvider(default_model="qwen3"), ToolExecutor(registry)) + + asyncio.run(agent.run(LLMInput(messages=[Message(role=Role.USER, content="go")], tools=[_tool()]))) + + results = server.chats[1]["messages"][2:] + assert [(r["tool_name"], r["content"]) for r in results] == [ + ("missing", "Error: Tool 'missing' not found"), + ("search", "query=x"), + ] + + +def test_ollama_react_agent_without_tool_support_answers_directly(monkeypatch): + server = _FakeOllama(monkeypatch, [_ollama_answer("I cannot call tools.")]) + agent = ReActAgent(OllamaProvider(default_model="codellama"), ToolExecutor()) + + output = asyncio.run(agent.run(LLMInput(messages=[Message(role=Role.USER, content="hi")], tools=[_tool()]))) + + assert output.content == "I cannot call tools." + assert "tools" not in server.chats[0] + assert server.shows == [] def test_claude_provider_serializes_tools_for_messages_api(): From 7556360412b1c2e7f8e6ddc777ef38f4dffca4c6 Mon Sep 17 00:00:00 2001 From: FenjuFu Date: Sat, 3 Oct 2026 01:33:53 +0800 Subject: [PATCH 3/3] fix(llm): retry Ollama capability lookups that failed Only an answered /api/show lookup is cached now, and the cache is replaced rather than mutated. A failed lookup sends no tools for that request and is retried on the next one, so a transient outage no longer disables tools for the provider's lifetime. Also annotate the test helpers and tests this PR adds. Signed-off-by: FenjuFu --- src/llm/providers/ollama.py | 35 +++++++++++++----------- tests/test_provider_tools.py | 52 ++++++++++++++++++++++++++---------- 2 files changed, 57 insertions(+), 30 deletions(-) diff --git a/src/llm/providers/ollama.py b/src/llm/providers/ollama.py index 65256b179..47f5f1cc6 100644 --- a/src/llm/providers/ollama.py +++ b/src/llm/providers/ollama.py @@ -92,26 +92,29 @@ class OllamaProvider(LLMProvider): Catalogued models answer from :attr:`ModelInfo.supports_tools`. Any other model is looked up once through ``/api/show``, whose ``capabilities`` - list is what the installed model itself declares. A failed lookup counts - as no tool support. + list is what the installed model itself declares. Only an answered lookup + is cached: a failed one counts as no tool support for that request and + is retried on the next. """ for info in self._models: if info.name == model: return info.supports_tools - if model not in self._tool_support: - try: - req = urllib.request.Request( - f"{self.base_url}/api/show", - data=json.dumps({"model": model}).encode("utf-8"), - headers={"Content-Type": "application/json"}, - ) - with urllib.request.urlopen(req, timeout=10) as response: - capabilities = json.loads(response.read().decode("utf-8")).get("capabilities") or [] - self._tool_support[model] = "tools" in capabilities - except Exception as e: - logger.warning("Could not read capabilities for Ollama model '%s': %s", model, type(e).__name__) - self._tool_support[model] = False - return self._tool_support[model] + if model in self._tool_support: + return self._tool_support[model] + try: + req = urllib.request.Request( + f"{self.base_url}/api/show", + data=json.dumps({"model": model}).encode("utf-8"), + headers={"Content-Type": "application/json"}, + ) + with urllib.request.urlopen(req, timeout=10) as response: + capabilities = json.loads(response.read().decode("utf-8")).get("capabilities") or [] + except Exception as e: + logger.warning("Could not read capabilities for Ollama model '%s': %s", model, type(e).__name__) + return False + supports_tools = "tools" in capabilities + self._tool_support = {**self._tool_support, model: supports_tools} + return supports_tools def generate(self, input: LLMInput) -> LLMOutput: try: diff --git a/tests/test_provider_tools.py b/tests/test_provider_tools.py index caa0290c4..accdef762 100644 --- a/tests/test_provider_tools.py +++ b/tests/test_provider_tools.py @@ -3,6 +3,7 @@ import json import urllib.request from io import BytesIO from types import SimpleNamespace +from typing import Any import pytest @@ -173,14 +174,19 @@ def test_ollama_provider_serializes_generation_options( class _FakeOllama: """Serves /api/show and replays queued /api/chat replies, recording requests.""" - def __init__(self, monkeypatch, replies, capabilities=None): + def __init__( + self, + monkeypatch: pytest.MonkeyPatch, + replies: list[dict[str, Any]], + capabilities: list[str] | None = None, + ) -> None: self.replies = list(replies) self.capabilities = capabilities - self.chats = [] - self.shows = [] + self.chats: list[dict[str, Any]] = [] + self.shows: list[str] = [] monkeypatch.setattr(urllib.request, "urlopen", self.urlopen) - def urlopen(self, request, timeout): + def urlopen(self, request: urllib.request.Request, timeout: float) -> BytesIO: body = json.loads(request.data) if request.full_url.endswith("/api/show"): self.shows.append(body["model"]) @@ -191,7 +197,7 @@ class _FakeOllama: return BytesIO(json.dumps(self.replies.pop(0)).encode()) -def _ollama_tool_calls(*calls): +def _ollama_tool_calls(*calls: tuple[str, dict[str, Any]]) -> dict[str, Any]: return { "message": { "content": "", @@ -201,7 +207,7 @@ def _ollama_tool_calls(*calls): } -def _ollama_answer(text): +def _ollama_answer(text: str) -> dict[str, Any]: return {"message": {"content": text}, "done_reason": "stop"} @@ -213,7 +219,7 @@ def _lookup_tool() -> ToolDefinition: ) -def test_ollama_provider_serializes_tools_and_parses_tool_calls(monkeypatch): +def test_ollama_provider_serializes_tools_and_parses_tool_calls(monkeypatch: pytest.MonkeyPatch) -> None: server = _FakeOllama( monkeypatch, [_ollama_tool_calls(("search", {"query": "ollama"}))], capabilities=["completion", "tools"] ) @@ -246,12 +252,16 @@ def test_ollama_provider_serializes_tools_and_parses_tool_calls(monkeypatch): ("codellama", None, False, []), ("qwen3", ["completion", "tools"], True, ["qwen3"]), ("gemma3", ["completion", "vision"], False, ["gemma3"]), - ("unknown", None, False, ["unknown"]), + ("unknown", None, False, ["unknown", "unknown"]), ], ) def test_ollama_provider_sends_tools_only_to_models_that_declare_them( - monkeypatch, model, capabilities, expect_tools, expect_shows -): + monkeypatch: pytest.MonkeyPatch, + model: str, + capabilities: list[str] | None, + expect_tools: bool, + expect_shows: list[str], +) -> None: server = _FakeOllama(monkeypatch, [_ollama_answer("ok"), _ollama_answer("ok")], capabilities=capabilities) provider = OllamaProvider(default_model=model) request = LLMInput(messages=[Message(role=Role.USER, content="hi")], tools=[_tool()]) @@ -260,11 +270,25 @@ def test_ollama_provider_sends_tools_only_to_models_that_declare_them( provider.generate(request) assert all(("tools" in chat) is expect_tools for chat in server.chats) - # The capability lookup is cached per model. + # An answered capability lookup is cached per model; a failed one is retried. assert server.shows == expect_shows -def test_ollama_react_agent_correlates_multiple_calls_without_ids(monkeypatch): +def test_ollama_provider_retries_capability_lookup_after_a_failure(monkeypatch: pytest.MonkeyPatch) -> None: + server = _FakeOllama(monkeypatch, [_ollama_answer("ok"), _ollama_answer("ok"), _ollama_answer("ok")]) + provider = OllamaProvider(default_model="qwen3") + request = LLMInput(messages=[Message(role=Role.USER, content="hi")], tools=[_tool()]) + + provider.generate(request) + server.capabilities = ["completion", "tools"] + provider.generate(request) + provider.generate(request) + + assert ["tools" in chat for chat in server.chats] == [False, True, True] + assert server.shows == ["qwen3", "qwen3"] + + +def test_ollama_react_agent_correlates_multiple_calls_without_ids(monkeypatch: pytest.MonkeyPatch) -> None: server = _FakeOllama( monkeypatch, [ @@ -298,7 +322,7 @@ def test_ollama_react_agent_correlates_multiple_calls_without_ids(monkeypatch): assert second == {"role": "tool", "content": "query=weather", "tool_call_id": ids[1], "tool_name": "search"} -def test_ollama_react_agent_reports_unknown_tool_under_its_own_name(monkeypatch): +def test_ollama_react_agent_reports_unknown_tool_under_its_own_name(monkeypatch: pytest.MonkeyPatch) -> None: server = _FakeOllama( monkeypatch, [_ollama_tool_calls(("missing", {}), ("search", {"query": "x"})), _ollama_answer("done")], @@ -317,7 +341,7 @@ def test_ollama_react_agent_reports_unknown_tool_under_its_own_name(monkeypatch) ] -def test_ollama_react_agent_without_tool_support_answers_directly(monkeypatch): +def test_ollama_react_agent_without_tool_support_answers_directly(monkeypatch: pytest.MonkeyPatch) -> None: server = _FakeOllama(monkeypatch, [_ollama_answer("I cannot call tools.")]) agent = ReActAgent(OllamaProvider(default_model="codellama"), ToolExecutor())