fix(server): forward thinking_budget on /v1/completions (#1825) - #1844
Merged
jundot merged 1 commit intoJun 13, 2026
Merged
Conversation
CompletionRequest had no thinking_budget field, so Pydantic dropped the parameter and it was silently ignored on /v1/completions, while the same request worked on /v1/chat/completions (jundot#1825). Add the field and resolve it through _resolve_thinking_budget in both the streaming and non-streaming completion paths, mirroring the chat path. Note: model-level thinking budgets configured in the dashboard now apply to /v1/completions as well, where they were previously ignored.
richgoodson
marked this pull request as ready for review
June 12, 2026 15:00
jundot
pushed a commit
that referenced
this pull request
Jun 13, 2026
#1845) mlx-lm's GenerationBatch.filter only reindexes logits_processors when at least one row slot is non-empty. After a request with no per-request processors finishes (every slot is [], the shape omlx inserts), removal shrinks uids but leaves the stale processor list behind. The next request then extends in behind its own row index: the row reads a leftover empty slot and its real processor (thinking budget, grammar constraint) is silently never applied. Alignment self-heals when the affected request finishes, so the symptom is an intermittently ignored thinking_budget or grammar constraint that depends on request order. Wrap GenerationBatch.filter alongside the existing _step chokepoint patch (#934/#1747): when the original guard would have skipped the reindex, reset the list to one empty slot per surviving row. Found while live-testing #1844. thinking_budget was enforced on a fresh server but ignored when the previous request in the decode batch had no processors. Reproduced on /v1/chat/completions as well, so the bug is independent of the completions forwarding change.
Owner
|
Thanks for narrowing this down. This fixes the actual /v1/completions gap for #1825: the request model now accepts thinking_budget, and both completion paths resolve and pass it to the engine, including model-level settings. I'm going to take this smaller wiring fix here; the streaming prefix and validation polish can be handled separately. This looks good to me, and I'm going to merge it. |
efortin
added a commit
to efortin/omlx
that referenced
this pull request
Jun 13, 2026
jundot#1844 (merged upstream) forwards thinking_budget on /v1/completions via a `**gen_kwargs` dict-unpack instead of an inline `thinking_budget=_resolve_thinking_budget(...)` keyword. After rebasing this branch on top of it, the source-level wiring guard only recognized the inline form and failed. Teach the AST check to also accept the dict-unpack pattern: the handler sets `gen_kwargs["thinking_budget"]` from the resolved value, then unpacks the dict into the engine call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 13, 2026
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
2 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.
Problem
#1825: thinking_budget works on /v1/chat/completions (and budget_tokens on /v1/messages) but is silently ignored on /v1/completions. The cause is what the reporter diagnosed: CompletionRequest has no thinking_budget field, so Pydantic drops the parameter before the handler sees it. The completion handlers also never call _resolve_thinking_budget, so a model-level budget never reached the engine on this endpoint either.
Change
Mirrors the chat path:
Behavior note: model-level thinking budgets configured in the dashboard now apply to /v1/completions too, where they were previously ignored. That matches the chat endpoint, but it is a change for raw-completion users of models with a budget configured.
Two scope notes:
<think>\n) or the model emits one.Testing