Repository navigation
[Bugfix] Apply structured output on /v1/completions when the prompt never opens reasoning - #57754
RichardoMrMu wants to merge 5 commits into
Conversation
ec3b7e9 to
97cc77d
Compare
`is_reasoning_end()` returns False both for a prompt sitting inside an open reasoning block and for one that never entered reasoning at all. A `/v1/completions` prompt renders no chat template, so it contains neither a reasoning-start nor a reasoning-end token, and no reasoning-end token will ever arrive -- the grammar bitmask is never applied and the completion comes back unconstrained with a 200 and no warning. Distinguish the two cases with the parser's reasoning-start marker, which the `ReasoningParser` interface already exposes and the structured-output manager never consulted. Parsers without a start marker keep the previous behaviour. Assisted-by: Doubao Signed-off-by: RichardoMu <tianbowen.richardo@bytedance.com>
…reasoning Assisted-by: Doubao Signed-off-by: RichardoMu <tianbowen.richardo@bytedance.com>
97cc77d to
f75abb0
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
77932f0 to
38c942b
Compare
…gate Resolve vllm/v1/structured_output/__init__.py: keep the prompt-skips-reasoning gate alongside main's failed_index bitmask refactor. (vllm-project#57754) Signed-off-by: RichardoMu <44485717+RichardoMrMu@users.noreply.github.com>
38c942b to
ae251a6
Compare
|
This pull request has merge conflicts that must be resolved before it can be |
…oning A /v1/completions prompt renders no chat template, so it contains neither a reasoning-start nor a reasoning-end token. `is_reasoning_end()` returns False for it, but no reasoning-end token will ever arrive: the previous logic left the request unconstrained for its whole completion. Detect prompts that provably never open a reasoning block via the parser's reasoning-start marker, and start the constraint immediately instead of waiting for a reasoning-end token that can never come. Fixes vllm-project#57726 Signed-off-by: RichardoMu <tianbowen.richardo@bytedance.com>
arpera
left a comment
There was a problem hiding this comment.
First of all thank you for your PR!
I have some concerns about approach that you have chosen to resolve initial issue #57726.
There is a problem in case user sent request with token to /v1/completions. In that case we would falsely detect that reasoning has already started and we would skip constraint for structured output -- the same problem we are trying to resolve.
Would it be a better idea simply to put warning in case we have /v1/completions + --reasoning-parser + structured output? I think this should not be hot path as well as /v1/completions API is now considered as superseded by /v1/chat/completions? In that case patch would be simple and small enough. What do you think about this?
|
This pull request has merge conflicts that must be resolved before it can be |
|
Thanks for the review. On the A completions prompt that contains The case you are pointing at is real, but narrower than a skip: a raw completions prompt that contains On the warning alternative: it is smaller and touches no chat path, and I am fine with it. The tradeoff is that the request stays unconstrained, so the warning makes #57726 visible rather than fixed. If you prefer warning-only, I will rework it that way, or I can add the warning on top of this predicate so both cases are covered. Tests covering the two behaviours are |
# Conflicts: # tests/v1/structured_output/test_structured_output_manager.py # vllm/v1/structured_output/__init__.py
Purpose
Fixes #57726.
With a reasoning parser configured, a
/v1/completionsrequest carrying astructured-output constraint comes back unconstrained, with a 200 and no
warning. The same request on the same model without
--reasoning-parserisconstrained.
/v1/chat/completionsis unaffected.Root cause
StructuredOutputManager._get_constraint_start()infers the initial reasoningstate from the prompt with a single question:
BaseThinkingReasoningParser.is_reasoning_end()scans backwards and returnsFalseon the start token,Trueon the end token, and falls through toreturn Falsewhen it finds neither. That final line conflates two oppositestates:
is_reasoning_end<think>False</think>— correct<think></think>TrueFalseA raw completions prompt renders no chat template, so it is the third row. No
reasoning-end token will ever arrive, so the bitmask is never applied and the
model is unconstrained for the whole completion.
The same holds for the engine parser family:
ParserEngineReasoningAdapterinherits
ReasoningParserand proxies the same properties, so both familiesreach the gate through this one inference.
The change
Ask the second question at the same place: does the prompt contain a reasoning
start marker at all?
ReasoningParser.reasoning_start_stralready existsfor this and the manager never consulted it.
A parser that exposes no start marker, or whose marker is absent from the
vocabulary, keeps today's behaviour — the helper returns
Falsefor "cannottell", so only a provable never-entered-reasoning prompt changes.
Verification
Behaviour of the gate before and after, driven through the repo's own
is_reasoning_endsource:<think>)<think></think>)Exactly the two raw-completions rows change; the five chat rows are untouched.
Parsers without a start marker are identical on all seven.
Tests added to
tests/v1/structured_output/test_structured_output_manager.py,parametrized over both backends:
test_constraint_applies_when_prompt_never_opens_reasoning— a prompt thatopens reasoning still waits, one that closes it is constrained, and one with
no reasoning tokens is now constrained rather than free.
test_parser_without_start_marker_keeps_waiting— the fallback stays on theprevious behaviour.
Both fail on
mainfor the no-reasoning-tokens case and pass with this change.