From 5dd8be54845a97d3ab62d2dc38815e6d00f350e7 Mon Sep 17 00:00:00 2001 From: hefestocode-max Date: Fri, 14 Aug 2026 10:31:31 -0600 Subject: [PATCH] fix(reasoning): probe Ollama thinking capability for local servers, not just ollama.com MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A profile whose `fallback_providers` points at a local Ollama server running a model without the `thinking` capability fails every request: HTTP 400: "qwen2.5:7b" does not support thinking Two defects combine to produce it. **1. The capability gate never runs for local Ollama.** `AIAgent._supports_reasoning_extra_body()` only probed `/api/show` when the base URL host was `ollama.com`. A local server (`http://localhost:11434/v1`) fell through to `if "openrouter" not in self._base_url_lower: return False`, so the probe was never consulted — even though `hermes_cli.models.ollama_model_supports_thinking()` is documented for Ollama "Cloud or local", already normalises `/v1` to the native base, and is cached per (model, base_url). Port 11434 is Ollama's default across every platform and install method, so it is as reliable a signal as the ollama.com hostname. **2. `CustomProfile` ignored the capability it was handed.** `build_api_kwargs_extras()` absorbed `supports_reasoning` into `**ctx` and never read it, emitting `reasoning_effort` unconditionally. The sibling `ollama-cloud` profile already honours the flag; this brings `custom` in line. Both fixes are required together. Fixing only (2) would be a regression: because the gate always returned False for localhost, a local thinking-capable model such as `deepseek-r1` would stop receiving `reasoning_effort` at all. Only the *enable* branch is gated. Measured against Ollama /v1/chat/completions with qwen2.5:7b: reasoning_effort="medium" -> HTTP 400 (does not support thinking) reasoning_effort="none" -> HTTP 200 think=false -> HTTP 200 The endpoint rejects enabling thinking, not the presence of the field, so the disable branch stays ungated — gating it would silently drop a user's explicit "don't reason" whenever the probe is unavailable, leaving a thinking-capable model reasoning against instructions. Tests: local Ollama is probed for both outcomes and on several host forms; a non-Ollama local server on another port is left untouched; ollama.com keeps its behaviour; the disable branch survives a missing capability; and the transport is pinned to keep forwarding `supports_reasoning`, since the profile's fail-closed default is only safe while it does. --- plugins/model-providers/custom/__init__.py | 33 +++- run_agent.py | 24 ++- .../agent/transports/test_chat_completions.py | 30 ++++ .../test_ollama_local_reasoning_gate.py | 156 ++++++++++++++++++ .../model_providers/test_custom_profile.py | 95 ++++++++++- tests/providers/test_transport_parity.py | 15 ++ 6 files changed, 343 insertions(+), 10 deletions(-) create mode 100644 tests/hermes_cli/test_ollama_local_reasoning_gate.py diff --git a/plugins/model-providers/custom/__init__.py b/plugins/model-providers/custom/__init__.py index de744d50914e6..21a15eccf68ad 100644 --- a/plugins/model-providers/custom/__init__.py +++ b/plugins/model-providers/custom/__init__.py @@ -4,6 +4,12 @@ Ollama instances and OpenAI-compatible reasoning endpoints (GLM-5.2 on Volcengine ARK, vLLM, llama.cpp). Key quirks: - ollama_num_ctx → extra_body.options.num_ctx (local context window) + - Enabling reasoning is gated on ``supports_reasoning`` (the + transport-resolved per-model capability, e.g. Ollama's /api/show + "thinking" flag): a model that doesn't declare thinking support never + receives an effort value, because Ollama's /v1/chat/completions 400s + with ``"" does not support thinking``. Turning reasoning OFF is + ungated — non-thinking models accept it with 200. - reasoning_config disabled → top-level reasoning_effort="none" (Ollama /v1/chat/completions ignores think=False — ollama#14820) + extra_body.think = False for /api/chat and proxies @@ -26,12 +32,13 @@ def build_api_kwargs_extras( *, reasoning_config: dict | None = None, ollama_num_ctx: int | None = None, + supports_reasoning: bool = False, **ctx: Any, ) -> tuple[dict[str, Any], dict[str, Any]]: extra_body: dict[str, Any] = {} top_level: dict[str, Any] = {} - # Ollama context window + # Ollama context window — independent of reasoning capability. if ollama_num_ctx: options = extra_body.get("options", {}) options["num_ctx"] = ollama_num_ctx @@ -40,6 +47,28 @@ def build_api_kwargs_extras( # Reasoning / thinking control for custom OpenAI-compatible endpoints # (GLM-5.2 on Volcengine ARK, vLLM, Ollama, llama.cpp, …). # + # Enabling reasoning is gated on ``supports_reasoning``, which the + # transport resolves per model (for Ollama-compatible routes — local + # servers included since AIAgent._supports_reasoning_extra_body's + # port-11434 fix — from the native /api/show "thinking" capability, + # mirroring the ollama-cloud profile). Without that gate a model which + # does not declare "thinking" still received an effort value and + # Ollama's /v1/chat/completions rejected the whole request with + # ``HTTP 400: "qwen2.5:7b" does not support thinking``. + # + # Only the *enable* branch is gated. Measured against Ollama + # /v1/chat/completions with qwen2.5:7b (a non-thinking model): + # + # reasoning_effort="medium" → HTTP 400 (does not support thinking) + # reasoning_effort="none" → HTTP 200 + # think=false → HTTP 200 + # + # Turning thinking OFF is accepted even by models that cannot think, + # so the disabled branch stays ungated: gating it would silently drop + # a user's explicit "don't reason" for any route whose capability + # probe is unavailable, leaving a thinking-capable model reasoning + # against instructions. + # # - disabled → extra_body.think = False (Ollama's thinking-off flag) # - enabled + effort set → TOP-LEVEL reasoning_effort string, the # format GLM-5.2/ARK and other OpenAI-compatible reasoning APIs @@ -63,7 +92,7 @@ def build_api_kwargs_extras( # ignore them. top_level["reasoning_effort"] = "none" extra_body["think"] = False - elif _effort: + elif _effort and supports_reasoning: top_level["reasoning_effort"] = _effort return extra_body, top_level diff --git a/run_agent.py b/run_agent.py index 12e7da647ebbd..84ff936ffd942 100644 --- a/run_agent.py +++ b/run_agent.py @@ -7250,11 +7250,25 @@ def _supports_reasoning_extra_body(self) -> bool: opts = self._lmstudio_reasoning_options_cached() # "off-only" (or absent) means no real reasoning capability. return any(opt and opt != "off" for opt in opts) - # Ollama Cloud (and any Ollama-compatible server): the native - # /api/show capabilities list is authoritative — emit reasoning_effort - # only for models that declare the "thinking" capability. deepseek-v4 - # has it; gemma3 / qwen3-coder don't. Cached per (model, base_url). - if base_url_host_matches(self._base_url_lower, "ollama.com"): + # Ollama Cloud (and any Ollama-compatible server, local included): the + # native /api/show capabilities list is authoritative — emit + # reasoning_effort only for models that declare the "thinking" + # capability. deepseek-v4 has it; gemma3 / qwen3-coder don't. Cached + # per (model, base_url). + # + # Local Ollama (``http://localhost:11434/v1`` and friends) previously + # fell through this gate to the OpenRouter-only branch below, which + # always returned False — so reasoning_effort was silently never + # emitted even for local thinking-capable models (deepseek-r1), AND + # (via the separate CustomProfile defect) it WAS emitted unguarded + # for non-thinking local models, 400ing with `"" does not + # support thinking`. Port 11434 is Ollama's universal default across + # every platform/install method, so it is as reliable a signal as the + # ollama.com hostname for Ollama Cloud. + if ( + base_url_host_matches(self._base_url_lower, "ollama.com") + or ":11434" in self._base_url_lower + ): return self._ollama_supports_thinking_cached() if "openrouter" not in self._base_url_lower: return False diff --git a/tests/agent/transports/test_chat_completions.py b/tests/agent/transports/test_chat_completions.py index d401d6d958781..8acbc066e290c 100644 --- a/tests/agent/transports/test_chat_completions.py +++ b/tests/agent/transports/test_chat_completions.py @@ -235,6 +235,36 @@ def test_custom_think_false(self, transport): ) assert kw["extra_body"]["think"] is False + def test_custom_disable_survives_missing_capability(self, transport): + """Omitting supports_reasoning (defaults False) must NOT drop the + disable fields: Ollama accepts reasoning_effort="none"/think=false + with HTTP 200 even on non-thinking models, and dropping them would + silently leave a thinking-capable model reasoning against an explicit + request not to.""" + from providers import get_provider_profile + profile = get_provider_profile("custom") + msgs = [{"role": "user", "content": "Hi"}] + kw = transport.build_kwargs( + model="qwen2.5:7b", messages=msgs, + provider_profile=profile, + reasoning_config={"effort": "none"}, + ) + assert kw["extra_body"]["think"] is False + assert kw["reasoning_effort"] == "none" + + def test_custom_supports_reasoning_false_omits_effort(self, transport): + from providers import get_provider_profile + profile = get_provider_profile("custom") + msgs = [{"role": "user", "content": "Hi"}] + kw = transport.build_kwargs( + model="qwen2.5:7b", messages=msgs, + provider_profile=profile, + reasoning_config={"enabled": True, "effort": "medium"}, + supports_reasoning=False, + ) + assert "reasoning_effort" not in kw + assert "think" not in kw.get("extra_body", {}) + def test_gemini_openai_compat_flash_reasoning_maps_to_nested_google_thinking_config(self, transport): diff --git a/tests/hermes_cli/test_ollama_local_reasoning_gate.py b/tests/hermes_cli/test_ollama_local_reasoning_gate.py new file mode 100644 index 0000000000000..c532907dfe51f --- /dev/null +++ b/tests/hermes_cli/test_ollama_local_reasoning_gate.py @@ -0,0 +1,156 @@ +"""Tests for the local-Ollama reasoning capability gate. + +Before this fix, ``AIAgent._supports_reasoning_extra_body()`` only probed +Ollama's ``/api/show`` "thinking" capability for the ``ollama.com`` hostname +(Ollama Cloud). A local Ollama server (``http://localhost:11434/v1`` and +equivalents) fell through that check and hit the final +``if "openrouter" not in self._base_url_lower: return False`` — so the gate +never probed at all and always reported "no reasoning support", regardless of +what the local model actually declared. + +Combined with the separate ``CustomProfile.build_api_kwargs_extras`` defect +(it never consulted ``supports_reasoning`` and emitted ``reasoning_effort`` +unconditionally — covered in +``tests/plugins/model_providers/test_custom_profile.py``), a profile whose +``fallback_providers`` pointed at local Ollama running a non-thinking model +(e.g. ``qwen2.5:7b``) failed with:: + + HTTP 400: "qwen2.5:7b" does not support thinking + +Fixing only the ``CustomProfile`` side would have been a regression on its +own: since the gate always returned False for localhost, a local +thinking-capable model (e.g. ``deepseek-r1``) would stop receiving +``reasoning_effort`` entirely. Both fixes are required together. + +The gate reads exactly one attribute off ``self`` (``_base_url_lower``) plus +two cached probe helpers, so these tests bind the unbound method to a stub +instead of constructing a real ``AIAgent`` — no provider configuration, no +network, no credentials. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +import pytest + + +def _gate(base_url: str, *, probe_result: bool = False, record: list | None = None): + """Invoke ``_supports_reasoning_extra_body`` against a minimal stub. + + ``record`` collects a marker whenever the Ollama probe is consulted, which + is what proves the local branch is reached at all (it previously wasn't). + """ + from run_agent import AIAgent + + def _probe(): + if record is not None: + record.append(base_url) + return probe_result + + stub = SimpleNamespace( + _base_url_lower=base_url.lower(), + provider="custom", + model="test-model", + _ollama_supports_thinking_cached=_probe, + _lmstudio_reasoning_options_cached=lambda: [], + ) + return AIAgent._supports_reasoning_extra_body(stub) + + +LOCAL_URLS = [ + "http://localhost:11434/v1", + "http://127.0.0.1:11434/v1", + "http://localhost:11434", + "http://192.168.1.50:11434/v1", +] + + +class TestLocalOllamaReasoningGate: + """Local Ollama is probed exactly like Ollama Cloud.""" + + @pytest.mark.parametrize("base_url", LOCAL_URLS) + def test_model_without_thinking_suppresses_reasoning(self, base_url): + """qwen2.5:7b-style model: probe says no → gate says no.""" + seen: list = [] + assert _gate(base_url, probe_result=False, record=seen) is False + assert seen == [base_url], "the Ollama probe must actually be consulted" + + @pytest.mark.parametrize("base_url", LOCAL_URLS) + def test_model_with_thinking_allows_reasoning(self, base_url): + """Non-regression: deepseek-r1-style model still gets reasoning. + + This is the half that a CustomProfile-only fix would have broken. + """ + seen: list = [] + assert _gate(base_url, probe_result=True, record=seen) is True + assert seen == [base_url] + + def test_ollama_cloud_still_probed(self): + """The pre-existing ollama.com behaviour is untouched.""" + seen: list = [] + assert _gate("https://ollama.com/v1", probe_result=True, record=seen) is True + assert seen == ["https://ollama.com/v1"] + + def test_non_ollama_local_server_is_not_probed(self): + """The check is Ollama-specific, not "any local endpoint". + + A local vLLM/llama.cpp server on another port must keep falling + through to the existing OpenRouter-only branch, so this fix cannot + change behaviour for endpoints it knows nothing about. + """ + seen: list = [] + assert _gate("http://localhost:8000/v1", probe_result=True, record=seen) is False + assert seen == [], "a non-Ollama port must not trigger the Ollama probe" + + def test_openrouter_branch_unaffected(self): + """Sanity: the gate still refuses unknown non-OpenRouter hosts.""" + assert _gate("https://api.example.com/v1", probe_result=True) is False + + +class TestSupportsReasoningReachesCustomProfile: + """The resolved capability actually reaches the profile. + + ``CustomProfile.build_api_kwargs_extras`` defaults ``supports_reasoning`` + to False (fail closed). That default is only safe because every real call + site passes the transport-resolved value explicitly — this pins it. + """ + + def test_transport_passes_supports_reasoning_to_profile(self): + import inspect + + from agent.transports import chat_completions + + src = inspect.getsource(chat_completions) + assert "supports_reasoning=params.get(" in src, ( + "the transport must forward the resolved capability to the profile; " + "without it CustomProfile's fail-closed default would silently " + "disable reasoning for every custom endpoint" + ) + + def test_non_thinking_model_gets_no_effort(self): + """End-to-end on the profile: capability False → no effort emitted.""" + import model_tools # noqa: F401 (triggers plugin discovery) + import providers + + profile = providers.get_provider_profile("custom") + eb, tl = profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "medium"}, + supports_reasoning=False, + model="qwen2.5:7b", + ) + assert "reasoning_effort" not in tl + assert "think" not in eb + + def test_thinking_model_still_gets_effort(self): + """End-to-end on the profile: capability True → effort emitted.""" + import model_tools # noqa: F401 + import providers + + profile = providers.get_provider_profile("custom") + _, tl = profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "high"}, + supports_reasoning=True, + model="deepseek-r1", + ) + assert tl == {"reasoning_effort": "high"} diff --git a/tests/plugins/model_providers/test_custom_profile.py b/tests/plugins/model_providers/test_custom_profile.py index 2658949ed3e80..0fdf0c8337192 100644 --- a/tests/plugins/model_providers/test_custom_profile.py +++ b/tests/plugins/model_providers/test_custom_profile.py @@ -6,13 +6,22 @@ nothing when reasoning was *enabled*, so a configured ``reasoning_effort`` was silently dropped for every custom endpoint. -These tests pin the wire-shape contract: +These tests pin the wire-shape contract (the enable cases pass +``supports_reasoning=True``, i.e. the target model declares a thinking +capability; the disable cases do not need it): - disabled → extra_body.think = False - enabled + effort → top-level reasoning_effort (native OpenAI-compat format GLM/ARK expect), passed through verbatim including ``max``/``xhigh`` - enabled + no effort → nothing emitted (endpoint's server default applies) - ollama_num_ctx → extra_body.options.num_ctx, orthogonal to reasoning + +``TestCustomReasoningCapabilityGate`` covers the ``supports_reasoning=False`` +side: local Ollama models without the "thinking" capability (e.g. +qwen2.5:7b) must never receive an effort value, since Ollama's +``/v1/chat/completions`` 400s with ``"" does not support thinking``. +Disabling reasoning stays ungated — those same models accept +``reasoning_effort="none"`` and ``think=false`` with HTTP 200. """ from __future__ import annotations @@ -81,7 +90,8 @@ def test_enabled_effort_goes_top_level(self, custom_profile, effort): native deep-reasoning level and must survive. """ eb, tl = custom_profile.build_api_kwargs_extras( - reasoning_config={"enabled": True, "effort": effort}, model="glm-5.2" + reasoning_config={"enabled": True, "effort": effort}, model="glm-5.2", + supports_reasoning=True, ) assert tl == {"reasoning_effort": effort} assert "reasoning_effort" not in eb @@ -92,7 +102,8 @@ def test_does_not_force_think_true_on_enable(self, custom_profile): """We must never send think=True on enable — it's Ollama-only and would 400 on GLM/vLLM endpoints that don't recognize it.""" eb, _ = custom_profile.build_api_kwargs_extras( - reasoning_config={"enabled": True, "effort": "high"}, model="glm-5.2" + reasoning_config={"enabled": True, "effort": "high"}, model="glm-5.2", + supports_reasoning=True, ) assert eb.get("think") is not True @@ -107,3 +118,81 @@ def test_num_ctx_alone(self, custom_profile): assert eb == {"options": {"num_ctx": 8192}} assert tl == {} + def test_num_ctx_survives_capability_gate_suppressing_reasoning(self, custom_profile): + """num_ctx must still be emitted even when supports_reasoning=False + suppresses every reasoning field — the two are wired independently.""" + eb, tl = custom_profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "high"}, + ollama_num_ctx=8192, + supports_reasoning=False, + model="qwen2.5:7b", + ) + assert eb == {"options": {"num_ctx": 8192}} + assert tl == {} + + +class TestCustomReasoningCapabilityGate: + """``supports_reasoning`` gates the ENABLE branch only. + + Reproduces the reported bug: a local Ollama model without the "thinking" + capability (qwen2.5:7b) must never receive an effort value — Ollama's + /v1/chat/completions 400s with ``"qwen2.5:7b" does not support thinking``. + + Measured against that endpoint with qwen2.5:7b: + + reasoning_effort="medium" → HTTP 400 (does not support thinking) + reasoning_effort="none" → HTTP 200 + think=false → HTTP 200 + + So the rejection is about *enabling* thinking, not about the field being + present. The disable branch stays ungated accordingly. + """ + + def test_supports_reasoning_false_suppresses_effort(self, custom_profile): + eb, tl = custom_profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "medium"}, + supports_reasoning=False, + model="qwen2.5:7b", + ) + assert eb == {} + assert tl == {} + + def test_supports_reasoning_false_still_emits_disable_fields(self, custom_profile): + """The explicit-disable branch stays ungated. + + Non-thinking models accept the disable fields with HTTP 200 (see the + class docstring), so gating this branch would buy nothing and would + silently drop a user's explicit "don't reason" on any route whose + capability probe is unavailable — leaving a thinking-capable model + reasoning against instructions. + """ + eb, tl = custom_profile.build_api_kwargs_extras( + reasoning_config={"enabled": False}, + supports_reasoning=False, + model="qwen2.5:7b", + ) + assert eb == {"think": False} + assert tl == {"reasoning_effort": "none"} + + def test_supports_reasoning_true_emits_effort(self, custom_profile): + """Non-regression: a local Ollama model that DOES declare thinking + (e.g. deepseek-r1) still gets reasoning_effort wired through.""" + eb, tl = custom_profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "high"}, + supports_reasoning=True, + model="deepseek-r1", + ) + assert tl == {"reasoning_effort": "high"} + assert "think" not in eb + + def test_default_is_false_when_omitted(self, custom_profile): + """The parameter defaults to False (fail closed) when a caller omits + it entirely — see the transport-level non-regression test for proof + every real call site always passes it explicitly.""" + eb, tl = custom_profile.build_api_kwargs_extras( + reasoning_config={"enabled": True, "effort": "high"}, + model="qwen2.5:7b", + ) + assert eb == {} + assert tl == {} + diff --git a/tests/providers/test_transport_parity.py b/tests/providers/test_transport_parity.py index e6befad371d9c..142803f00c683 100644 --- a/tests/providers/test_transport_parity.py +++ b/tests/providers/test_transport_parity.py @@ -170,3 +170,18 @@ def test_think_false_when_disabled(self, transport): reasoning_config={"enabled": False, "effort": "none"}, ) assert kw["extra_body"]["think"] is False + + def test_no_effort_when_capability_unsupported(self, transport): + """Gate regression check: a model without the thinking capability + (supports_reasoning=False, the local-Ollama default) must never get an + effort value — that is what Ollama 400s on. The disable fields still + go through, since non-thinking models accept them with HTTP 200.""" + kw = transport.build_kwargs( + model="qwen2.5:7b", + messages=_simple_messages(), + tools=None, + provider_profile=get_provider_profile("custom"), + reasoning_config={"enabled": True, "effort": "medium"}, + ) + assert "reasoning_effort" not in kw + assert "think" not in kw.get("extra_body", {})