Skip to content

Commit 4d0ec0b

Browse files
committed
[Bugfix][Structured Output] Unify reasoning-boundary grammar advance
Use the common trim-and-advance path for all structured output backends. Remove the redundant deferred grammar drain and update the regression tests to cover advancing only the verified post-reasoning suffix. Signed-off-by: Allen.Yu <yuyue0225sc@163.com>
1 parent a126ab3 commit 4d0ec0b

2 files changed

Lines changed: 9 additions & 206 deletions

File tree

tests/v1/structured_output/test_reasoning_structured_output.py

Lines changed: 7 additions & 158 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
"""Unit tests for reasoning-aware structured output functionality (PR #25515)."""
55

6-
from unittest.mock import Mock, patch
6+
from unittest.mock import Mock
77

88
import pytest
99

@@ -209,37 +209,11 @@ def test_should_advance_reasoning_just_ended(
209209
mock_request_with_structured_output
210210
)
211211

212-
# Should set reasoning_ended to True but return False for this step
212+
# The scheduler trims the reasoning prefix before advancing the grammar.
213213
assert (
214214
mock_request_with_structured_output.structured_output_request.reasoning_ended
215215
is True
216216
)
217-
assert result is False
218-
219-
def test_should_advance_reasoning_just_ended_with_spec_decode_structural_tag(
220-
self,
221-
manager_with_reasoner,
222-
mock_request_with_structured_output,
223-
):
224-
"""When reasoning ends this step, advance immediately for structural
225-
tags with speculative decoding."""
226-
structured_req = mock_request_with_structured_output.structured_output_request
227-
structured_req.reasoning_ended = False
228-
structured_req.structured_output_key = (
229-
StructuredOutputOptions.STRUCTURAL_TAG,
230-
"{}",
231-
)
232-
reasoner = MockReasoner(tokenizer=Mock())
233-
reasoner.is_reasoning_end_streaming.return_value = True
234-
structured_req.reasoner = reasoner
235-
236-
manager_with_reasoner.vllm_config.speculative_config = Mock()
237-
238-
result = manager_with_reasoner.should_advance(
239-
mock_request_with_structured_output
240-
)
241-
242-
assert structured_req.reasoning_ended is True
243217
assert result is True
244218

245219
def test_should_advance_reasoning_already_ended(
@@ -305,7 +279,7 @@ def test_should_advance_uses_new_token_ids_when_provided(
305279
assert list(called_delta) == new_token_ids
306280

307281
assert structured_req.reasoning_ended is True
308-
assert result is False
282+
assert result is True
309283

310284
def test_should_advance_without_new_token_ids_falls_back(
311285
self,
@@ -334,16 +308,12 @@ def test_should_advance_without_new_token_ids_falls_back(
334308
assert list(called_delta) == [4, 5]
335309
assert result is False
336310

337-
def test_should_advance_drains_post_marker_into_grammar(
311+
def test_should_advance_trims_reasoning_prefix_for_json(
338312
self,
339313
manager_with_reasoner,
340314
mock_request_with_structured_output,
341315
):
342-
"""On the step that ends reasoning, post-marker content tokens are
343-
fed to the grammar so the next step's bitmask reflects the post-
344-
marker FSM state. Without this, the model can emit a duplicate
345-
opening token (e.g. "{{" for json_object).
346-
"""
316+
"""JSON uses the common trim-then-advance path at the boundary."""
347317
structured_req = mock_request_with_structured_output.structured_output_request
348318
structured_req.reasoning_ended = False
349319
structured_req.structured_output_key = (
@@ -372,131 +342,10 @@ def is_reasoning_end_streaming(self, input_ids, delta_ids):
372342
new_token_ids=new_token_ids,
373343
)
374344

375-
# grammar.accept_tokens was called exactly once with the post-marker
376-
# portion of new_token_ids, excluding the reasoning prefix and the
377-
# marker itself.
378-
accept_calls = structured_req.grammar.accept_tokens.call_args_list
379-
assert len(accept_calls) == 1
380-
_, fed_tokens = accept_calls[0].args
381-
assert fed_tokens == [271, 5005]
382-
383-
assert structured_req.reasoning_ended is True
384-
# Deferred backend: still return False so the scheduler does not
385-
# also feed full new_token_ids (which would include reasoning).
386-
assert result is False
387-
388-
def test_should_advance_no_postmarker_skips_grammar_accept(
389-
self,
390-
manager_with_reasoner,
391-
mock_request_with_structured_output,
392-
):
393-
"""When the marker is the last token in new_token_ids, there is no
394-
post-marker tail to drain, so grammar.accept_tokens is not called.
395-
"""
396-
structured_req = mock_request_with_structured_output.structured_output_request
397-
structured_req.reasoning_ended = False
398-
structured_req.structured_output_key = (
399-
StructuredOutputOptions.JSON_OBJECT,
400-
"{}",
401-
)
402-
403-
marker = 248069
404-
405-
class MarkerReasoner:
406-
def __init__(self, *_, **__):
407-
pass
408-
409-
def is_reasoning_end_streaming(self, input_ids, delta_ids):
410-
return marker in list(delta_ids)
411-
412-
structured_req.reasoner = MarkerReasoner()
413-
414-
new_token_ids = [9, 198, marker]
415-
mock_request_with_structured_output.all_token_ids = (
416-
[1, 2, 3] + new_token_ids
417-
)
418-
419-
result = manager_with_reasoner.should_advance(
420-
mock_request_with_structured_output,
421-
new_token_ids=new_token_ids,
422-
)
423-
424345
structured_req.grammar.accept_tokens.assert_not_called()
425346
assert structured_req.reasoning_ended is True
426-
assert result is False
427-
428-
def test_should_advance_structural_tag_with_new_token_ids(
429-
self,
430-
manager_with_reasoner,
431-
mock_request_with_structured_output,
432-
):
433-
"""The structural-tag path uses the exact current-step window."""
434-
structured_req = mock_request_with_structured_output.structured_output_request
435-
structured_req.reasoning_ended = False
436-
structured_req.structured_output_key = (
437-
StructuredOutputOptions.STRUCTURAL_TAG,
438-
"{}",
439-
)
440-
441-
marker = 248069
442-
reasoner = MockReasoner(tokenizer=Mock())
443-
reasoner.is_reasoning_end_streaming = Mock(
444-
side_effect=lambda input_ids, delta_ids: marker in list(delta_ids)
445-
)
446-
structured_req.reasoner = reasoner
447-
manager_with_reasoner.vllm_config.speculative_config = Mock()
448-
449-
new_token_ids = [9, marker, 271]
450-
mock_request_with_structured_output.all_token_ids = (
451-
[1, 2, 3] + new_token_ids
452-
)
453-
454-
result = manager_with_reasoner.should_advance(
455-
mock_request_with_structured_output,
456-
new_token_ids=new_token_ids,
457-
)
458-
459347
assert result is True
460-
assert structured_req.reasoning_end_token_index == 4
348+
assert structured_req.reasoning_end_token_index == 5
461349
assert manager_with_reasoner.trim_reasoning_for_advance(
462350
mock_request_with_structured_output, new_token_ids
463-
) == [271]
464-
465-
def test_should_advance_tolerates_rejected_post_marker_tokens(
466-
self,
467-
manager_with_reasoner,
468-
mock_request_with_structured_output,
469-
):
470-
"""A rejected speculative tail must not fail the request."""
471-
structured_req = mock_request_with_structured_output.structured_output_request
472-
structured_req.reasoning_ended = False
473-
structured_req.structured_output_key = (
474-
StructuredOutputOptions.JSON_OBJECT,
475-
"{}",
476-
)
477-
structured_req.grammar.accept_tokens.return_value = False
478-
479-
marker = 248069
480-
reasoner = MockReasoner(tokenizer=Mock())
481-
reasoner.is_reasoning_end_streaming = Mock(
482-
side_effect=lambda input_ids, delta_ids: marker in list(delta_ids)
483-
)
484-
structured_req.reasoner = reasoner
485-
486-
new_token_ids = [9, marker, 271]
487-
mock_request_with_structured_output.all_token_ids = (
488-
[1, 2, 3] + new_token_ids
489-
)
490-
491-
with patch("vllm.v1.structured_output.logger.warning") as warning:
492-
result = manager_with_reasoner.should_advance(
493-
mock_request_with_structured_output,
494-
new_token_ids=new_token_ids,
495-
)
496-
497-
assert result is False
498-
assert structured_req.reasoning_ended is True
499-
structured_req.grammar.accept_tokens.assert_called_once_with(
500-
"mock_req", [271]
501-
)
502-
warning.assert_called_once()
351+
) == [271, 5005]

vllm/v1/structured_output/__init__.py

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,13 @@
77
from typing import TYPE_CHECKING
88

99
from vllm.config import VllmConfig
10-
from vllm.logger import init_logger
1110
from vllm.reasoning import ReasoningParserManager
1211
from vllm.tokenizers import cached_tokenizer_from_config
1312
from vllm.utils.import_utils import LazyLoader
1413
from vllm.v1.structured_output.backend_guidance import GuidanceBackend
1514
from vllm.v1.structured_output.backend_types import (
1615
StructuredOutputBackend,
1716
StructuredOutputGrammar,
18-
StructuredOutputOptions,
1917
)
2018
from vllm.v1.structured_output.backend_xgrammar import XgrammarBackend
2119

@@ -30,9 +28,6 @@
3028
torch = LazyLoader("torch", globals(), "torch")
3129

3230

33-
logger = init_logger(__name__)
34-
35-
3631
class StructuredOutputManager:
3732
"""Engine-level manager for structured output requests."""
3833

@@ -422,54 +417,13 @@ def should_advance(
422417
if reasoner.is_reasoning_end_streaming(all_token_ids, delta_ids):
423418
structured_req.reasoning_ended = True
424419

425-
# Locate the reasoning-end marker within the step once; both
426-
# branches below rely on it. Everything up to and including the
427-
# marker is reasoning content and must never reach the grammar.
420+
# Record the boundary so the scheduler can exclude reasoning tokens.
428421
end_index = self._find_reasoning_end_index(
429422
reasoner, all_token_ids, start
430423
)
431424

432-
# Reasoning just ended this step. Defer FSM advance until the next
433-
# pass (see reasoning_ended check above) for JSON/regex/choice/grammar:
434-
# advancing on the closing boundary token can accept tokens that still
435-
# belong to the reasoning stream. Structural tags are the only safe
436-
# same-step exception: they model phased output (e.g. thinking tag ->
437-
# answer tag), and speculative decoding must run grammar.validate_tokens
438-
# on draft tokens produced immediately after that transition.
439-
if (
440-
self.vllm_config.speculative_config is not None
441-
and structured_req.structured_output_key[0]
442-
== StructuredOutputOptions.STRUCTURAL_TAG
443-
):
444-
# The scheduler will advance the grammar with this step's
445-
# tokens right away, but the step still contains reasoning
446-
# content up to and including the end marker. Record where
447-
# it ends so trim_reasoning_for_advance() can drop it.
448-
structured_req.reasoning_end_token_index = end_index
449-
return True
450-
451425
structured_req.reasoning_end_token_index = end_index
452-
453-
# Deferred backends still need the post-marker tail of this step's
454-
# new_token_ids to be drained into the FSM, otherwise the next
455-
# step's bitmask preparation sees grammar at its initial state and
456-
# the model can emit a duplicate opening token (e.g. "{{") when
457-
# reasoning ended inside a spec-decode window.
458-
if new_token_ids:
459-
end_offset = end_index - start
460-
post_marker = list(new_token_ids[end_offset + 1 :])
461-
grammar = structured_req.grammar
462-
if post_marker and grammar is not None:
463-
if not grammar.accept_tokens(request.request_id, post_marker):
464-
# These tokens were sampled before the grammar became
465-
# active and are not guaranteed valid; tolerate the
466-
# rejection like grammar_bitmask does post-marker.
467-
logger.warning(
468-
"Grammar rejected post-reasoning tokens %s for "
469-
"request %s; continuing without the advance.",
470-
post_marker,
471-
request.request_id,
472-
)
426+
return True
473427

474428
return False
475429

0 commit comments

Comments
 (0)