fix: server handling of budget and reasoning params#23673
Open
thiswillbeyourgithub wants to merge 6 commits into
Open
fix: server handling of budget and reasoning params#23673thiswillbeyourgithub wants to merge 6 commits into
thiswillbeyourgithub wants to merge 6 commits into
Conversation
ngxson
requested changes
May 25, 2026
0cc4m
reviewed
May 30, 2026
0cc4m
left a comment
0cc4m
left a comment
Contributor
There was a problem hiding this comment.
Changing the behaviour for server-set thinking budget to be an upper limit is a good change.
Contributor
|
Self note - related to #23434 |
Contributor
|
Please rebase to resolve the conflicts. |
The `common_params` default was `COMMON_REASONING_FORMAT_DEEPSEEK` while the CLI help text, README, and the enum's own "in most cases, use AUTO" comment all said the default was `auto`. Switch the default to `AUTO` so the docs match the behavior, and list `auto` explicitly in the option enumeration in both the CLI help and the server README (it was a valid input but missing from the listed values).
…oint The chat endpoint previously ignored `thinking_budget_tokens` in the request body whenever `--reasoning-budget` had been set, and accepted it without any cap when the CLI was at the default. Both behaviors are wrong: an admin needs the CLI flag to act as an upper bound that clients cannot escape (so `--reasoning-budget 0` reliably forbids thinking), and a client should still be able to lower the budget for an individual request. Treat the CLI value as a permission ceiling: the body field may lower it but never raise it, with -1 meaning unrestricted on both sides. Document the behavior in both the CLI flag entry and the chat-completions body field, and point users at the raw `/completion` field names since that endpoint has no Jinja template to introspect for thinking tags.
…et one The budget exhaustion message is a quality knob (it primes the model with a graceful "wrap it up" phrase before the end-of-thinking tag, leading to better final answers than an abrupt cut) rather than a policy lever, so the ceiling rule used for the numeric budget doesn't translate. Use a simpler "admin-wins-when-set" rule instead: if `--reasoning-budget-message` was supplied at startup, that value is always respected; otherwise the chat endpoint accepts `thinking_budget_message` from the request body and uses it. Document the rule in the CLI flag entry and the body-field paragraph.
Per code review feedback, keep COMMON_REASONING_FORMAT_DEEPSEEK as the default and instead align the CLI help to say `(default: deepseek)`. Also revert all edits inside the auto-generated `<\!-- HELP_START --> ... <\!-- HELP_END -->` block in tools/server/README.md, which is produced by llama-gen-docs and must not be hand-edited; only the hand-written API-parameters prose outside that block stays.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
thiswillbeyourgithub
force-pushed
the
fix-server-reasoning-handling
branch
from
aabc7a9 to
7624f40
Compare
0cc4m
approved these changes
Jun 8, 2026
aldehir
approved these changes
Jun 9, 2026
aldehir
left a comment
aldehir
left a comment
Contributor
There was a problem hiding this comment.
if both values are set: the CLI value is a hard ceiling and the request value is now only able to lower it.
I don't really view reasoning as some sort of resource that necessitates a hard constraint. I suspect users will open issues claiming they can't use the request provided parameter as an escape hatch for their default setting.
That said, I don't care enough.
…recedence Upstream PR ggml-org#24517 (ggerganov) made the per-request thinking_budget_tokens take precedence over the CLI / model.ini --reasoning-budget default, with the CLI value as a fallback. That is the opposite of this branch's earlier hard-ceiling behaviour, so this resolution drops the ceiling clamp and adopts the upstream precedence for the budget value itself. The additive per-request thinking_budget_message fallback from this branch is kept: it does not touch the precedence change. The block comment is updated to describe the new precedence instead of the removed ceiling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
My reasoning was that setting a very high reasoning budget and asking the model to think deeply about whatever sounds a bit DDOS-y. But admins worried about that should just set the max_tokens anyways so it's not really real. I looked at #24517 that went the opposite direction so I resolved the conflict by following ggerganov's need (not capping the budget over the query). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
When bumping into walls for using budget tokens argument in wllama I found out that I had had bugs like this in the past months ago when trying to use Qwen3 or 3.5 models without reasoning. At the time I couldn't make sense of how to invoke the reasoning budget arguments. Turns out that it was a llamacpp issue: IMO the server doc is quite confusing about the distinction between
reasoning-budget andthinking_budget. This PR clarifies it with an admin-wins-when-set rule:--reasoning-budget-messagewas supplied at startup, that value is always used but the chat endpoint still acceptsthinking_budget_messagefrom the request body.This PR also fix the issue where if the user launching the server set a reasoning-budget value then thinking_budget_tokens couldn't be used.
Lastly, thinking_budget_message at request time was not honoured.
While doing this, I also noticed that the doc of the
reasoning-formatmentioned its default as "auto" but was actually "COMMON_REASONING_FORMAT_DEEPSEEK" so I put it back to "COMMON_REASONING_FORMAT_AUTO".Additional information
This is related to this PR on wllama
Possibly related to:
Requirements