fix: emit chat_template_kwargs for llama.cpp/vLLM thinking control - #64153
fix: emit chat_template_kwargs for llama.cpp/vLLM thinking control#64153mlaihk wants to merge 1 commit into
Conversation
The custom provider currently emits extra_body.think=False when
reasoning is disabled, which is an Ollama-only flag. llama.cpp and vLLM
honour chat_template_kwargs.enable_thinking instead, so they ignore
the think flag and default to thinking-on — resulting in empty content
responses and retry storms.
Emit chat_template_kwargs={"enable_thinking": false} alongside the
existing think=False so each backend picks up the flag it understands.
|
Nice catch on the Ollama-only While independently building a fix for the same file/same reasoning-control block, I ran into two adjacent problems this PR doesn't cover yet, both on the enable path rather than disable: 1. vLLM rejects Hermes-only effort levels with HTTP 400 vLLM validates This fires whenever a user has 2. Your fix correctly adds Backend detection Both fixes need to know whether the endpoint is vLLM/llama.cpp before touching I have a working patch that:
Happy to either (a) extend this PR with the clamp + enable-path fix so it ships as one coherent change, or (b) open a separate PR scoped to just the clamp/enable-path if you'd rather keep this one minimal and disable-only. Your call — didn't want to just force-push over your branch. Let me know which you'd prefer. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the vLLM/llama.cpp disable-thinking gap. Current main still lacks this automatic field: CustomProfile emits think=False at plugins/model-providers/custom/__init__.py:65, while the vLLM documentation identifies chat_template_kwargs.enable_thinking: false as the applicable shape (website/docs/integrations/providers.md:1216-1229).
Problems
- The new field at
plugins/model-providers/custom/__init__.py:41is emitted for every custom endpoint. That profile also serves GLM/ARK (plugins/model-providers/custom/__init__.py:40-46), and the docs explicitly describe custom request bodies as endpoint-specific (website/docs/integrations/providers.md:1204-1216). Please scope this to compatible backends. - The branch conflicts with current main. Preserve the current
top_level["reasoning_effort"] = "none"behavior introduced by8662254ab; the PR's older return shape would otherwise lose it. - Please add coverage alongside
tests/plugins/model_providers/test_custom_profile.py:51-71for targeted vLLM/llama.cpp emission and unaffected non-target custom endpoints.
Suggested changes
- Resolve the current reasoning-control block first, then gate
chat_template_kwargsusing the existing cached local-server detection inagent/model_metadata.py:683-762.
Automated hermes-sweeper review.
| _enabled = reasoning_config.get("enabled", True) | ||
| if _effort == "none" or _enabled is False: | ||
| extra_body["think"] = False | ||
| extra_body["chat_template_kwargs"] = {"enable_thinking": False} |
There was a problem hiding this comment.
provider=custom also covers GLM/ARK and arbitrary OpenAI-compatible endpoints on current main. Please gate this backend-specific field to compatible vLLM/llama.cpp endpoints rather than changing every custom request's wire shape; the docs describe custom extra_body fields as endpoint-specific.
SummaryTwo open PRs address the same reasoning-disable gap: #12427 adds the llama.cpp/vLLM request field in the former run_agent.py path with tests, while #64153 adds it in the current custom-provider profile but applies it to every custom endpoint. Related pull requests
Duplicates#12427 and #64153 implement substantially the same disable-thinking change; #12427 targets the superseded builder location, while #64153 targets the current custom-provider profile. Suggested consolidationKeep #64153 open with the salvage path recorded in its keep_open review: rebase onto main, scope emission to compatible vLLM/llama.cpp endpoints, preserve the behavior introduced by 8662254, and add targeted and unaffected-endpoint tests. Close #12427 as a duplicate of #64153 despite its automated keep_open verdict, because its diff changes the obsolete run_agent.py path; carry its reasoning-path test cases into #64153 where applicable. 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
subgraph Dup12427 ["PRs duplicating each other"]
P12427["PR #12427 (open)"]
P64153["PR #64153 (open)"]
end
class P12427 open
class P64153 open
class P64153 target
click P12427 "https://github.com/NousResearch/hermes-agent/pull/12427"
click P64153 "https://github.com/NousResearch/hermes-agent/pull/64153"
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 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 5 kB of PR diffs, 5 kB of issue/PR text, 11 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Summary
The custom provider's
build_api_kwargs_extras()emitsextra_body.think=Falsewhen reasoning is disabled. This is an Ollama-only flag — llama.cpp and vLLM ignore it entirely, defaulting to thinking-on, which results in empty content responses and retry storms.Fix
Emit
chat_template_kwargs={enable_thinking: false}alongside the existingthink=Falseso each backend picks up the flag it understands:extra_body.think(unchanged)chat_template_kwargs.enable_thinking(new)Both flags are sent in
extra_body, which the OpenAI Python client serialises into the top-level request body. No conditional provider detection needed.Testing
Verified against llama.cpp server (Qwen3.6-35B-A3B-MTP) running on both Vulkan and ROCm backends:
chat_template_kwargs.enable_thinking: falsereasoning_effort: nonechat_template_kwargsis the only parameter that reliably disables thinking on llama.cpp.Motivation
Without this fix, Hermes configured with the custom provider against llama.cpp/vLLM backends produces empty responses when thinking is disabled (
agent.reasoning_effort: none), because the server still produces reasoning tokens that consume the entire token budget.