From a6b11c4c1667ea8024dd1bd126a656a4d82573f8 Mon Sep 17 00:00:00 2001 From: lazymio Date: Wed, 2 Sep 2026 01:48:07 +0000 Subject: [PATCH 1/2] [Bugfix][Parser] Derive the structural-tag reasoning flag from the request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _apply_structural_tag hardcodes reasoning=False. The two grammars xgrammar builds are not interchangeable -- for deepseek_v4 with tools and tool_choice="auto": reasoning=False TriggeredTagsFormat(triggers=['<｜DSML｜tool_calls>'], tags=[...], excludes=['', '']) reasoning=True SequenceFormat(elements=[ TagFormat(begin='', content=AnyTextFormat(excludes=[]), end=''), TriggeredTagsFormat(...)]) reasoning=False puts in the free-text excludes, so a model whose prompt ends inside an open thinking block can never close it. reasoning=True opens with an any-text span terminated by , so a non-thinking request waits for a marker its prompt already consumed and EOS stays masked. Verified with an xgrammar GrammarMatcher on a DeepSeek-V4-Flash-0731 tokenizer: reasoning=False reasoning=True "...answer" rejected accepted "..." + call rejected accepted runaway invoke name rejected rejected This was latent while ParserManager collapsed matching parsers into a bare ParserEngine, since _apply_structural_tag never ran for them. #52830 removed that shortcut, so the path is now live for every engine-backed parser (deepseek_v3_2/v4, qwen3, glm47_moe, kimi_k2, minimax_m2, ...) whenever a structural tag is applied -- today that is tool_choice=required/named, or auto with a strict tool. Add ReasoningParser.emits_reasoning_span, defaulting to False so existing parsers keep their current behaviour, and override it in ParserEngine from parser_engine_config.initial_state, which is derived per request from chat_template_kwargs. The same root cause was reported in #46149 (Qwen3.6-27B, tool_choice="auto" with per-tool strict=true) and closed without a fix. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: lazymio --- tests/tool_parsers/test_structural_tag_registry.py | 12 +++++++++--- vllm/parser/abstract_parser.py | 11 ++++++++++- vllm/parser/engine/adapters.py | 4 ++++ vllm/parser/engine/parser_engine.py | 6 ++++++ vllm/reasoning/abs_reasoning_parsers.py | 13 +++++++++++++ 5 files changed, 42 insertions(+), 4 deletions(-) diff --git a/tests/tool_parsers/test_structural_tag_registry.py b/tests/tool_parsers/test_structural_tag_registry.py index b77b4efaffe6..17b9dcee98c7 100644 --- a/tests/tool_parsers/test_structural_tag_registry.py +++ b/tests/tool_parsers/test_structural_tag_registry.py @@ -524,10 +524,13 @@ def fake_get_model_structural_tag(*, reasoning: bool, **kwargs): assert captured == [False] -def test_unified_parser_get_structural_tag_disables_reasoning( +@pytest.mark.parametrize("emits_reasoning_span", [False, True]) +def test_unified_parser_get_structural_tag_reasoning_follows_reasoning_parser( monkeypatch: pytest.MonkeyPatch, sample_tools_strict: list[ChatCompletionToolsParam], + emits_reasoning_span: bool, ): + """The tag's reasoning span must match the request's thinking mode.""" captured: list[bool] = [] def fake_get_model_structural_tag(*, reasoning: bool, **kwargs): @@ -549,11 +552,14 @@ class TestParser(DelegatingParser): tool_choice="auto", ) parser = TestParser(MagicMock(), tools=sample_tools_strict) - parser.reasoning_parser = MagicMock(adjust_request=lambda request: request) + parser.reasoning_parser = MagicMock( + adjust_request=lambda request: request, + emits_reasoning_span=emits_reasoning_span, + ) parser.adjust_request(request) - assert captured == [False] + assert captured == [emits_reasoning_span] def test_xgrammar_function_parameters_are_preserved( diff --git a/vllm/parser/abstract_parser.py b/vllm/parser/abstract_parser.py index cf83bb996e63..dfef2911338e 100644 --- a/vllm/parser/abstract_parser.py +++ b/vllm/parser/abstract_parser.py @@ -578,9 +578,18 @@ def _apply_structural_tag( if not need_tool_calling: return request + # The two grammars are not interchangeable: with reasoning=False the + # free-text span carries excludes=['', ''], so a model + # whose prompt ends inside an open thinking block can never close it; + # with reasoning=True the grammar opens with a span terminated by + # '', which a non-thinking request never emits and EOS stays + # masked. Ask the reasoning parser which shape this request needs. structure_tag = self._tool_parser.get_structural_tag( request, - reasoning=False, + reasoning=( + self._reasoning_parser is not None + and self._reasoning_parser.emits_reasoning_span + ), ) if structure_tag is None: return request diff --git a/vllm/parser/engine/adapters.py b/vllm/parser/engine/adapters.py index 4f4aa9f5a1e0..f896135bf8f6 100644 --- a/vllm/parser/engine/adapters.py +++ b/vllm/parser/engine/adapters.py @@ -62,6 +62,10 @@ def _skip_tool_parsing(self) -> Iterator[None]: finally: self._parser_engine.skip_tool_parsing = saved + @property + def emits_reasoning_span(self) -> bool: + return self._parser_engine.emits_reasoning_span + def is_reasoning_end(self, input_ids: Sequence[int]) -> bool: return self._parser_engine.is_reasoning_end(list(input_ids)) diff --git a/vllm/parser/engine/parser_engine.py b/vllm/parser/engine/parser_engine.py index 4817dedb4663..75311b7f3f12 100644 --- a/vllm/parser/engine/parser_engine.py +++ b/vllm/parser/engine/parser_engine.py @@ -647,6 +647,12 @@ def find_reasoning_end_offset(self, token_ids: Sequence[int]) -> int | None: return offset return None + @property + def emits_reasoning_span(self) -> bool: + # initial_state is derived per request from chat_template_kwargs, so + # this tracks the thinking/no-thinking distinction of *this* request. + return self.parser_engine_config.initial_state == ParserState.REASONING + def is_reasoning_end(self, input_ids: list[int]) -> bool: end_id = self._reasoning_end_token_id start_id = self._reasoning_start_token_id diff --git a/vllm/reasoning/abs_reasoning_parsers.py b/vllm/reasoning/abs_reasoning_parsers.py index 6fb694545a4a..f42661c59887 100644 --- a/vllm/reasoning/abs_reasoning_parsers.py +++ b/vllm/reasoning/abs_reasoning_parsers.py @@ -69,6 +69,19 @@ def has_engine_confirmed_reasoning_end(self) -> bool: """ return False + @property + def emits_reasoning_span(self) -> bool: + """Whether this request's output opens with a reasoning span. + + Consulted when building a structural tag: the grammars xgrammar + derives for the two cases are not interchangeable. A reasoning grammar + expects the output to close a thinking block, while a non-reasoning one + excludes the thinking markers outright. Parsers whose thinking mode is + request-dependent should override this; the default preserves the + historical assumption that no reasoning span is emitted. + """ + return False + @abstractmethod def is_reasoning_end(self, input_ids: Sequence[int]) -> bool: """ From 52a91318477955a0e915a3e100c285042e75173a Mon Sep 17 00:00:00 2001 From: lazymio Date: Thu, 3 Sep 2026 12:24:48 +0000 Subject: [PATCH 2/2] [Bugfix][Parser] Stop the turn when a tool-call block closes The parser treats the closing tool-call tag as a state transition back to CONTENT and keeps consuming, but nothing tells the sampler to stop. A model that has just closed a tool-call block is free to open another one, and in agentic workloads (long transcripts, many tools) it does -- repeatedly, until it hits max_tokens. Observed in production on DeepSeek-V4-Flash: 32000 output tokens and ~600s spent re-emitting the same block shape, with `finish_reason` still reported as `tool_use`. A tool call is a turn boundary: the client has to run the tool and send the result back, so nothing generated after the closer can be used. Hand the closer to the sampler as a stop string and the turn ends where it logically ends. The hook goes on ParserEngine.adjust_request rather than on the tool adapter, because a model whose parser is a unified Parser never builds that adapter -- deepseek_v4 sets `tool_parser_cls = None` and is used as the Parser directly. The tests target the unified parser for the same reason. Verified by replaying a captured 32k production request: 32000 tokens / 606s -> 514-601 tokens / 6-21s over four runs, with `stop_reason` reporting the closer. Parallel calls inside a single block are unaffected (3/3 preserved). Co-authored-by: Claude Opus 5 Signed-off-by: lazymio --- tests/parser/engine/test_deepseek_v4.py | 60 +++++++++++++++++++++++++ vllm/parser/engine/parser_engine.py | 50 +++++++++++++++++++++ 2 files changed, 110 insertions(+) diff --git a/tests/parser/engine/test_deepseek_v4.py b/tests/parser/engine/test_deepseek_v4.py index 1c1029b51a01..43387addc99c 100644 --- a/tests/parser/engine/test_deepseek_v4.py +++ b/tests/parser/engine/test_deepseek_v4.py @@ -3,6 +3,7 @@ """Tests for DeepSeek V4-specific parser engine semantics.""" import json +from unittest.mock import MagicMock import pytest @@ -1391,3 +1392,62 @@ def test_delegating_parser_corrupted_wrapper(self, wrapper, chunk_size): assert output.tool_calls[0]["name"] == "get_weather" assert json.loads(output.tool_calls[0]["arguments"]) == {"city": "Seoul"} assert "DSML" not in (output.content or "") + + +# ── Tool-call closer as a sampling stop ─────────────────────────────── + + +class TestToolCallStop: + """The closing tool_calls tag must end the turn. + + Without it the sampler keeps going after a complete tool-call block and + the model opens another one, repeatedly, until it hits max_tokens. + """ + + def _engine(self, mock_tokenizer): + return DeepSeekV4Parser(mock_tokenizer) + + def _request(self, stop=None, tools=True): + req = MagicMock() + req.tools = [{"type": "function"}] if tools else None + req.stop = stop + req.include_stop_str_in_output = False + return req + + def test_exposes_the_block_closer(self, mock_tokenizer): + assert self._engine(mock_tokenizer).tool_call_stop == DSML_TOOL_END + + def test_adjust_request_sets_the_stop(self, mock_tokenizer): + """The unified parser is what serves deepseek_v4. + + It is used directly as the Parser -- ``tool_parser_cls`` is None and + no ParserEngineToolAdapter is ever built -- so a hook that lives only + on the adapter never runs for this model. + """ + req = self._engine(mock_tokenizer).adjust_request(self._request([])) + assert req.stop == [DSML_TOOL_END] + assert req.include_stop_str_in_output is True + + def test_closer_is_added_and_kept_in_output(self, mock_tokenizer): + req = self._engine(mock_tokenizer)._add_tool_call_stop(self._request([])) + assert req.stop == [DSML_TOOL_END] + # the parser still has to see the closer to finish the call + assert req.include_stop_str_in_output is True + + def test_preserves_caller_supplied_stops(self, mock_tokenizer): + e = self._engine(mock_tokenizer) + assert e._add_tool_call_stop(self._request(["A"])).stop == ["A", DSML_TOOL_END] + assert e._add_tool_call_stop(self._request("A")).stop == ["A", DSML_TOOL_END] + + def test_idempotent(self, mock_tokenizer): + req = self._engine(mock_tokenizer)._add_tool_call_stop( + self._request([DSML_TOOL_END]) + ) + assert req.stop == [DSML_TOOL_END] + + def test_untouched_without_tools(self, mock_tokenizer): + req = self._engine(mock_tokenizer)._add_tool_call_stop( + self._request([], tools=False) + ) + assert req.stop == [] + assert req.include_stop_str_in_output is False diff --git a/vllm/parser/engine/parser_engine.py b/vllm/parser/engine/parser_engine.py index 75311b7f3f12..c153fb175420 100644 --- a/vllm/parser/engine/parser_engine.py +++ b/vllm/parser/engine/parser_engine.py @@ -217,10 +217,60 @@ def _reset(self, initial_state: ParserState | None = None) -> None: self._content_has_nonws = False self._prompt_streaming_prepared = False + @property + def tool_call_stop(self) -> str | None: + """The literal that closes a tool-call block, if the format has one. + + Formats whose tool payload is delimited (DeepSeek's + ````, Qwen's ````, ...) expose it + here so the request path can hand it to the sampler as a stop + string. Formats without a distinct closer return ``None``. + """ + return self.parser_engine_config.terminal_literal("TOOL_END") + def adjust_request( self, request: ChatCompletionRequest | ResponsesRequest ) -> ChatCompletionRequest | ResponsesRequest: request.skip_special_tokens = False + return self._add_tool_call_stop(request) + + def _add_tool_call_stop( + self, request: ChatCompletionRequest | ResponsesRequest + ) -> ChatCompletionRequest | ResponsesRequest: + """Stop generating once a tool-call block is closed. + + The parser treats the closing tag as a state transition back to + CONTENT and keeps consuming, but nothing tells the *sampler* to + stop. A model that has just closed a tool-call block is free to + open another one, and in agentic workloads (long transcripts, many + tools) it does -- repeatedly, until it hits ``max_tokens``. + Observed in production on DeepSeek-V4-Flash: 32k output tokens and + ~400s spent re-emitting the same block shape, with + ``finish_reason`` still reported as ``tool_use``. + + A tool call is a turn boundary: the client has to run the tool and + send the result back, so nothing generated after the closer can be + used. Handing the closer to the sampler ends the turn where it + logically ends. + """ + closer = self.tool_call_stop + if not closer or not getattr(request, "tools", None): + return request + stop = getattr(request, "stop", None) + if stop is None: + request.stop = [closer] + elif isinstance(stop, str): + if stop == closer: + return request + request.stop = [stop, closer] + elif closer in stop: + return request + else: + stop.append(closer) + # The parser needs to *see* the closer to finish the tool call, and + # stop strings are stripped from the output by default. + if hasattr(request, "include_stop_str_in_output"): + request.include_stop_str_in_output = True return request def _preprocess_feed(