fix: gate Ollama think/reasoning_effort on model's declared capabilities - #63315
Tim-ImpendingTech wants to merge 1 commit into
Conversation
The custom provider profile (provider=custom, covers Ollama, vLLM, GLM/ARK, llama.cpp) unconditionally emitted think/reasoning_effort whenever agent.reasoning_effort was configured. Ollama models that don't declare thinking support (e.g. llama3.3:70b — capabilities: [completion, tools]) reject both fields with HTTP 400: "<model>" does not support thinking, which broke every turn for any non-thinking model as soon as the user set a non-none reasoning_effort. Ollama publishes per-model capabilities via POST /api/show. This adds a small cached probe (_ollama_model_capabilities, permanent cache on success, short TTL on failure) and gates the think/reasoning_effort emission on "thinking" being present in that list — only for base URLs that look like Ollama (port 11434 or "ollama" in the host). Non-Ollama custom backends (vLLM/GLM/ARK/llama.cpp) keep the original unconditional behavior since they don't expose an equivalent capability endpoint. Added tests for the new gate (thinking vs non-thinking models, disabled path, non-Ollama/no-base_url bypass) plus unit tests for the probe helper itself (fetch/parse, /v1 suffix stripping, failure caching, missing args) and the _is_ollama_base_url matcher. All 13 pre-existing tests in test_custom_profile.py continue to pass unchanged.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing this to the live CustomProfile path. Current main still emits the incompatible fields at plugins/model-providers/custom/__init__.py:52-58, and the live transport forwards base_url and model to this hook (agent/transports/chat_completions.py:582-590).
Problems
plugins/model-providers/custom/__init__.py:70-74maps every probe failure to[], and line 124 treats that exactly like a successful non-thinking declaration. A transient/api/showfailure therefore suppresses a configuredreasoning_effortfor the five-minute negative-cache TTL, instead of preserving current behavior.
Suggested changes
- Return a distinct unknown/failure value from the probe and preserve existing emission when capability discovery is unavailable; suppress fields only after a successful capability response lacks
thinking. - Add a failed-probe wire-shape regression test alongside the existing success-path tests.
Automated hermes-sweeper review.
| _base_url = ctx.get("base_url") | ||
| _emit = True | ||
| if _is_ollama_base_url(_base_url): | ||
| _emit = "thinking" in _ollama_model_capabilities(ctx.get("model"), _base_url) |
There was a problem hiding this comment.
_ollama_model_capabilities() returns [] both for a confirmed non-thinking model and for any exception. This makes a transient /api/show failure suppress configured reasoning for the negative-cache TTL. Please distinguish unknown from a successful empty declaration and retain the existing emission behavior when the probe is unavailable.
SummaryThirty-seven PRs address or reference this broad Ollama/provider complex, spanning Cloud provider/catalog behavior, response recovery, Windows support, and reasoning-control wire compatibility. For the target failure—non-thinking Ollama models rejecting Related pull requests
Duplicates#3197 is incorporated into #10782; #6038 and #10740 were salvaged into #10782; #11296, #11362, #11395, #11520, #12488, and #12914 are obsolete-path variants of the #11237 fix now represented on the live path by #64286; #16192 and #16196 are superseded by #19881; #25866 and #29820 overlap, with #29820's provider half salvaged into #64608; #55280, #55428, and #57601 are superseded by #58156; #59678 is an alternative to target #63315; #52613 overlaps #59819 but lacks its endpoint-specific model shaping. Suggested consolidationFor #63315, author action: distinguish Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I59660(["issue #59660 (open)"])
subgraph Dup59678 ["PRs duplicating each other"]
P59678["PR #59678 (open)"]
P63315["PR #63315 (open)"]
end
P63315 -->|best fix| I59660
class I59660 open
class P59678 open
class P63315 open
class P63315 best
class P63315 target
click I59660 "https://github.com/NousResearch/hermes-agent/issues/59660"
click P59678 "https://github.com/NousResearch/hermes-agent/pull/59678"
click P63315 "https://github.com/NousResearch/hermes-agent/pull/63315"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 37 pull requests and 16 issues in this complex. Each diff was read against this issue; Assessment working set: 331 kB of PR diffs, 187 kB of issue/PR text, 71 kB of discussion (93 comments), 74 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Closing — the The |
Problem
The
customprovider profile (provider=custom, covers local Ollama, vLLM, GLM/ARK, llama.cpp) unconditionally sendsthink/reasoning_efforton every request onceagent.reasoning_effortis set to anything other thannone.Ollama models that don't declare thinking support — e.g.
llama3.3:70b, whose/api/tagsand/api/showcapabilities are["completion", "tools"]with no"thinking"entry — reject both fields outright:This breaks every single turn for any non-thinking model as soon as the user configures a non-
nonereasoning_effort, which is the default for most setups. The only workaround today is disabling reasoning globally, even though the model works fine otherwise (including tool calling).Fix
Ollama already publishes per-model capabilities via
POST /api/show. This adds a small cached probe (_ollama_model_capabilities, permanent cache on success, 5-minute TTL on failure/empty) and gatesthink/reasoning_effortemission on"thinking"being present in that list.The gate is scoped tightly:
base_urllooks like Ollama (port11434or"ollama"in the host) — mirrors the existing_is_ollama_glm_backenddetection pattern already used elsewhere inrun_agent.py.base_urlis passed at all (e.g. delegation/summary paths that don't thread it through), behavior is unchanged from before this patch.Testing
Added 12 new tests in
tests/plugins/model_providers/test_custom_profile.py:TestOllamaThinkingCapabilityGate— thinking vs non-thinking models, disabled path also gated, non-Ollama base_url bypasses the probe, no base_url bypasses the probe.TestOllamaCapabilityProbe— fetch/parse/api/show,/v1suffix stripping so the probe hits the Ollama root not the OpenAI-compat path, failure caching, missing model/base_url short-circuits.TestIsOllamaBaseUrl— port11434matching, hostname matching, negative cases.All 13 pre-existing tests in that file continue to pass unchanged (they don't pass
base_url, so the new gate is a no-op for them — confirms the change is additive, not a behavior change for the documented GLM/ARK contract).Also manually verified end-to-end against a real Ollama instance serving
llama3.3:70b: confirmed/api/showreports["completion", "tools"](no thinking), confirmed the pre-patch 400 reproduces withagent.reasoning_effort: medium, confirmed post-patchhermes chatsucceeds with reasoning enabled including a tool-calling round-trip.Related
Builds on the reasoning wiring introduced for the custom provider in #57601 — see the docstring reference in the test file.