From e7cf4730d6c441723faec060b99cf597b701f285 Mon Sep 17 00:00:00 2001 From: Suvrakamal Das Date: Wed, 2 Sep 2026 03:11:36 +0900 Subject: [PATCH] [Bugfix] Drop incomplete tool-call markup in non-streaming to match streaming (#47562) Signed-off-by: Suvrakamal Das Co-authored-by: Claude Co-authored-by: Ben Browning <56071+bbrowning@users.noreply.github.com> --- tests/parser/engine/test_inkling.py | 110 ++++++++++ tests/parser/engine/test_parser_engine.py | 197 ++++++++++++++++++ vllm/parser/abstract_parser.py | 19 ++ vllm/parser/engine/adapters.py | 17 ++ vllm/parser/engine/parser_engine.py | 18 +- vllm/parser/engine/streaming_parser_engine.py | 39 ++++ 6 files changed, 399 insertions(+), 1 deletion(-) diff --git a/tests/parser/engine/test_inkling.py b/tests/parser/engine/test_inkling.py index da88476a72..9026d548df 100644 --- a/tests/parser/engine/test_inkling.py +++ b/tests/parser/engine/test_inkling.py @@ -869,3 +869,113 @@ def test_content_tool_start_emits_reasoning_end_in_reasoning_pass(): EventType.TEXT_CHUNK, ] assert events[1].value == TOOL_JSON + + +def test_skip_reasoning_parsing_inert_for_shared_markers(): + """``skip_reasoning_parsing`` may only bypass reasoning-exclusive + markers. Inkling has none — ``<|end_message|>`` is labelled THINK_END + yet also closes text, header, and tool blocks — so the flag must be + inert: a tool block still closes through its transition and following + text returns to CONTENT instead of leaking into the argument stream.""" + engine = StreamingParserEngine(inkling_config(), tokenizer=None) + engine.skip_reasoning_parsing = True + engine.reset(initial_state=ParserState.CONTENT) + events = engine.parse_complete( + f'{TOOL_JSON}{{"name":"f","args":{{}}}}{END_MESSAGE}after' + ) + types = [e.type for e in events] + end_idx = types.index(EventType.TOOL_CALL_END) + text_after = [e.value for e in events[end_idx:] if e.type == EventType.TEXT_CHUNK] + assert text_after == ["after"] + + +class TestToolParserWithoutReasoningParser: + """Inkling served with only the tool parser, no ``--reasoning-parser``. + + That configuration turns on ``skip_reasoning_parsing``, and Inkling + labels ``<|end_message|>`` as THINK_END; a bypass keyed on the label + would neutralize the closer of every block kind, leaking markers into + content and leaving tool calls unterminated. Structure must keep + parsing exactly as with the reasoning parser attached.""" + + GEN_PROMPT = [_TML_VOCAB[MSG_MODEL]] + + def _tool_only(self, mock_tokenizer, tools=None): + parser_cls = ParserManager.get_parser( + tool_parser_name="inkling", + enable_auto_tools=True, + ) + return parser_cls(mock_tokenizer, tools or []) + + def test_plain_text_non_streaming(self, mock_tokenizer, mock_request): + tools = [_function_tool()] + mock_request.tools = tools + _, content, calls = self._tool_only(mock_tokenizer, tools).parse( + f"{TEXT_START}The answer is 42.{END_MESSAGE}", + mock_request, + enable_auto_tools=True, + ) + assert content == "The answer is 42." + assert not calls + + @pytest.mark.parametrize("chunk_size", [1, 3, 64]) + def test_plain_text_streaming(self, mock_tokenizer, mock_request, chunk_size): + tools = [_function_tool()] + mock_request.tools = tools + content, _, names, _ = _stream_delegating( + self._tool_only(mock_tokenizer, tools), + mock_request, + f"{TEXT_START}The answer is 42.{END_MESSAGE}", + chunk_size, + self.GEN_PROMPT, + ) + assert content == "The answer is 42." + assert not names + + def test_tool_block_non_streaming(self, mock_tokenizer, mock_request): + _, content, calls = self._tool_only(mock_tokenizer).parse( + _tool_block("get_weather", '{"city":"SF"}'), + mock_request, + enable_auto_tools=True, + ) + assert [c.name for c in calls] == ["get_weather"] + assert not content + + @pytest.mark.parametrize("chunk_size", [1, 3, 64]) + def test_tool_block_streaming(self, mock_tokenizer, mock_request, chunk_size): + tools = [_function_tool()] + mock_request.tools = tools + content, _, names, args = _stream_delegating( + self._tool_only(mock_tokenizer, tools), + mock_request, + _tool_block("get_weather", '{"city":"SF"}'), + chunk_size, + self.GEN_PROMPT, + ) + assert names == ["get_weather"] + assert json.loads(args[0]) == {"city": "SF"} + assert content == "" + + def test_thinking_then_text_keeps_structure(self, mock_tokenizer, mock_request): + tools = [_function_tool()] + mock_request.tools = tools + _, content, calls = self._tool_only(mock_tokenizer, tools).parse( + f"{THINK_START}plan{END_MESSAGE}" + f"{MSG_MODEL}{TEXT_START}Let me look.{END_MESSAGE}", + mock_request, + enable_auto_tools=True, + ) + assert content == "Let me look." + assert END_MESSAGE not in content + assert THINK_START not in content + assert not calls + + def test_thinking_then_tool_still_promotes(self, mock_tokenizer, mock_request): + _, content, calls = self._tool_only(mock_tokenizer).parse( + f"{THINK_START}think{END_MESSAGE}{MSG_MODEL}" + + _tool_block("get_weather", '{"x":1}'), + mock_request, + enable_auto_tools=True, + ) + assert [c.name for c in calls] == ["get_weather"] + assert not content diff --git a/tests/parser/engine/test_parser_engine.py b/tests/parser/engine/test_parser_engine.py index 330461042b..e759853cbc 100644 --- a/tests/parser/engine/test_parser_engine.py +++ b/tests/parser/engine/test_parser_engine.py @@ -1909,3 +1909,200 @@ class TestDropSpecialTokens: e.value for e in events if e.type == EventType.REASONING_CHUNK ) assert "" not in reasoning_text + + +# ── TestTruncatedToolOpenerStreamParity ────────────────────────────── + + +class TestTruncatedToolOpenerStreamParity: + """Regression tests for #47137: when generation terminates inside a + ```` opener that has not been promoted to a tool call + (via ``max_tokens`` or a ``stop`` string), the non-streaming path + must drop the incomplete markup, matching the streaming path.""" + + _QWEN3_VOCAB = { + "": 100, + "": 101, + } + + def _make_parser(self): + from vllm.parser.parser_manager import ParserManager + + parser_cls = ParserManager.get_parser( + tool_parser_name="qwen3_coder", + enable_auto_tools=True, + ) + tokenizer = make_mock_tokenizer(self._QWEN3_VOCAB) + return parser_cls(tokenizer, []) + + def _stream_content(self, request, chunks: list[str]) -> str: + parser = self._make_parser() + content = "" + for i, chunk in enumerate(chunks): + delta_token_ids = [ + tid for text, tid in self._QWEN3_VOCAB.items() if text in chunk + ] + delta = parser.parse_delta( + chunk, + delta_token_ids, + request, + prompt_token_ids=[1], + finished=(i == len(chunks) - 1), + ) + if delta and delta.content: + content += delta.content + return content + + @pytest.mark.parametrize( + "chunks", + [ + [""], + ["", "\n"], + ["", "\n", "<"], + ["", "\n", "", "\n", "", "garbage", "", " B"] + text = "".join(chunks) + + _, content, tool_calls = self._make_parser().parse( + text, mock_request, enable_auto_tools=True + ) + streamed = self._stream_content(mock_request, chunks) + + assert not tool_calls + assert content == streamed == "A B" + + def test_complete_tool_call_still_promoted(self, mock_request): + """Sanity check: a complete tool call still parses in the + non-streaming path after the truncation fix.""" + text = ( + "\n" + "\n" + "Tokyo\n" + "\n" + "" + ) + _, content, tool_calls = self._make_parser().parse( + text, mock_request, enable_auto_tools=True + ) + assert tool_calls is not None and len(tool_calls) == 1 + assert tool_calls[0].name == "get_weather" + assert not content + + +class TestThinkMarkupWithoutReasoningParser: + """With no reasoning parser configured, reasoning markup is plain + content: engine-backed tool parsers must return it verbatim in both + the non-streaming and streaming paths, not consume the markers or + reclassify the block as reasoning and drop it.""" + + _VOCAB = { + "": 90, + "": 91, + "": 100, + "": 101, + } + + # Two distinct failure modes are covered: qwen3_coder consumed a + # stray marker in CONTENT state (unbalanced markup), + # deepseek_v4 re-entered REASONING on and dropped the block + # from non-streaming content entirely. + _PARSERS = ["qwen3_coder", "deepseek_v4"] + + def _make_parser(self, tool_parser_name): + from vllm.parser.parser_manager import ParserManager + + parser_cls = ParserManager.get_parser( + tool_parser_name=tool_parser_name, + enable_auto_tools=True, + ) + tokenizer = make_mock_tokenizer(self._VOCAB) + return parser_cls(tokenizer, []) + + def _stream(self, tool_parser_name, request, chunks): + parser = self._make_parser(tool_parser_name) + content, reasoning = "", "" + for i, chunk in enumerate(chunks): + delta_token_ids = [ + tid for text, tid in self._VOCAB.items() if text in chunk + ] + delta = parser.parse_delta( + chunk, + delta_token_ids, + request, + prompt_token_ids=[1], + finished=(i == len(chunks) - 1), + ) + if delta and delta.content: + content += delta.content + if delta and delta.reasoning: + reasoning += delta.reasoning + return content, reasoning + + @pytest.mark.parametrize("tool_parser_name", _PARSERS) + def test_think_block_passes_through_both_paths( + self, mock_request, tool_parser_name + ): + chunks = ["", "reasoning here", "", "the answer"] + text = "".join(chunks) + + reasoning, content, tool_calls = self._make_parser(tool_parser_name).parse( + text, mock_request, enable_auto_tools=True + ) + streamed_content, streamed_reasoning = self._stream( + tool_parser_name, mock_request, chunks + ) + + assert not tool_calls + assert reasoning is None + assert streamed_reasoning == "" + assert content == streamed_content == text + + def test_think_block_preserved_alongside_tool_call(self, mock_request): + """The tools-called branch also returns the tool parser's content; + a think block preceding a promoted tool call must survive in it.""" + think = "plan" + text = think + ( + "\n" + "\n" + "Tokyo\n" + "\n" + "" + ) + _, content, tool_calls = self._make_parser("qwen3_coder").parse( + text, mock_request, enable_auto_tools=True + ) + assert tool_calls is not None and len(tool_calls) == 1 + assert tool_calls[0].name == "get_weather" + assert content == think diff --git a/vllm/parser/abstract_parser.py b/vllm/parser/abstract_parser.py index 7aba387d8a..c9b7796e45 100644 --- a/vllm/parser/abstract_parser.py +++ b/vllm/parser/abstract_parser.py @@ -131,6 +131,17 @@ class Parser: self._reasoning_parser is None or self._reasoning_parser.engine_based_streaming ) and (self._tool_parser is None or self._tool_parser.engine_based_streaming) + if ( + self._reasoning_parser is None + and self._tool_parser is not None + and hasattr(self._tool_parser, "skip_reasoning_parsing") + ): + # With no reasoning parser configured, reasoning markup is + # plain content: an engine-based tool parser should pass it + # through verbatim where its grammar allows, not consume or + # reclassify it. The engine ignores the flag for markers + # shared with non-reasoning structure. + self._tool_parser.skip_reasoning_parsing = True self._stream_state = StreamState( tool_call_id_type=( get_tool_call_id_type(model_config) @@ -508,6 +519,14 @@ class DelegatingParser(Parser): or (isinstance(content, str) and not content.strip()) ): return [], None + # No complete tool calls: for engine-based parsers, return + # the tool parser's content, which drops incomplete + # tool-call markup (e.g. a opener truncated by + # max_tokens or a stop string), so the non-streaming path + # matches streaming. Legacy parsers keep their existing + # behavior of returning the raw content. + if self._engine_based and tool_call_info is not None: + return None, tool_call_info.content or None return None, content return tool_calls, content diff --git a/vllm/parser/engine/adapters.py b/vllm/parser/engine/adapters.py index 9730b82845..cd723f5f8b 100644 --- a/vllm/parser/engine/adapters.py +++ b/vllm/parser/engine/adapters.py @@ -150,6 +150,15 @@ class ParserEngineToolAdapter(ToolParser): state so it can parse reasoning-stripped content (i.e. the output of :meth:`ReasoningParser.extract_reasoning`). + When no reasoning parser is configured at the serving layer, the + owning :class:`~vllm.parser.abstract_parser.Parser` sets + :attr:`skip_reasoning_parsing` so reasoning markup — plain content + in that configuration — passes through verbatim instead of being + consumed or reclassified by the engine. The engine honors the flag + only for markers that are reasoning-exclusive in its config; a + grammar like Inkling, whose block closer is shared with text and + tool blocks, keeps parsing unchanged. + Subclasses set :attr:`_parser_engine_cls` to the concrete :class:`ParserEngine` class. """ @@ -166,6 +175,14 @@ class ParserEngineToolAdapter(ToolParser): super().__init__(tokenizer, tools) self._parser_engine = self._parser_engine_cls(tokenizer, tools, **kwargs) # type: ignore[call-arg] + @property + def skip_reasoning_parsing(self) -> bool: + return self._parser_engine.skip_reasoning_parsing + + @skip_reasoning_parsing.setter + def skip_reasoning_parsing(self, value: bool) -> None: + self._parser_engine.skip_reasoning_parsing = value + def adjust_request( self, request: ChatCompletionRequest | ResponsesRequest, diff --git a/vllm/parser/engine/parser_engine.py b/vllm/parser/engine/parser_engine.py index 8f6f6ac17b..2e6bd92344 100644 --- a/vllm/parser/engine/parser_engine.py +++ b/vllm/parser/engine/parser_engine.py @@ -175,6 +175,14 @@ class ParserEngine(Parser): def skip_tool_parsing(self, value: bool) -> None: self._engine.skip_tool_parsing = value + @property + def skip_reasoning_parsing(self) -> bool: + return self._engine.skip_reasoning_parsing + + @skip_reasoning_parsing.setter + def skip_reasoning_parsing(self, value: bool) -> None: + self._engine.skip_reasoning_parsing = value + @property def reasoning_ended(self) -> bool: return self._reasoning_ended @@ -714,6 +722,7 @@ class ParserEngine(Parser): content_parts: list[str] = [] reasoning_parts: list[str] = [] + carried_deferred = self._deferred_content seen_tool_event = False suppress = self._suppress_tool_calls for event in events: @@ -750,7 +759,14 @@ class ParserEngine(Parser): tool_call_deltas = self._coalesce_tool_call_deltas(tool_call_deltas) if self._deferred_content and (not seen_tool_event or not tool_call_deltas): - content_parts.insert(0, self._deferred_content) + # Deferred content carried in from a previous delta precedes this + # delta's content; content deferred during this call (text after + # an unpromoted tool event) follows it. + deferred_now = self._deferred_content[len(carried_deferred) :] + if carried_deferred: + content_parts.insert(0, carried_deferred) + if deferred_now: + content_parts.append(deferred_now) self._deferred_content = "" content_str = "".join(content_parts) diff --git a/vllm/parser/engine/streaming_parser_engine.py b/vllm/parser/engine/streaming_parser_engine.py index ec21b05f4d..46dfb79c23 100644 --- a/vllm/parser/engine/streaming_parser_engine.py +++ b/vllm/parser/engine/streaming_parser_engine.py @@ -164,7 +164,12 @@ class StreamingParserEngine: if state in self._TOOL_STATES and tr.next_state not in self._TOOL_STATES ) + self._reasoning_markup_terminals: frozenset[str] = ( + self._compute_reasoning_markup_terminals() + ) + self.skip_tool_parsing = False + self.skip_reasoning_parsing = False self.reset(initial_state=initial_state) @property @@ -358,6 +363,37 @@ class StreamingParserEngine: } ) + _PLAIN_STATES = frozenset({ParserState.CONTENT, ParserState.REASONING}) + + _REASONING_EVENTS = frozenset({EventType.REASONING_START, EventType.REASONING_END}) + + def _compute_reasoning_markup_terminals(self) -> frozenset[str]: + """Terminals the ``skip_reasoning_parsing`` bypass may neutralize. + + Only reasoning-exclusive markers qualify: every transition they + participate in stays within CONTENT/REASONING and emits nothing + but reasoning events. Inkling's ``<|end_message|>`` is labelled + THINK_END yet also closes text, header, and tool blocks; + bypassing a shared marker would eat that structure, so one impure + marker disables the bypass for the whole config. + """ + markers = frozenset( + terminal + for (state, terminal), tr in self.config.transitions.items() + if ParserState.REASONING in (state, tr.next_state) + and tr.next_state not in self._TOOL_STATES + ) + for (state, terminal), tr in self.config.transitions.items(): + if terminal not in markers: + continue + if ( + state not in self._PLAIN_STATES + or tr.next_state not in self._PLAIN_STATES + or not self._REASONING_EVENTS.issuperset(tr.events) + ): + return frozenset() + return markers + def _on_terminal( self, terminal: str, value: str, token_count: int = 0 ) -> list[SemanticEvent]: @@ -372,6 +408,9 @@ class StreamingParserEngine: self._in_skipped_tool_span = False return self._emit_for_state(value, token_count) + if self.skip_reasoning_parsing and terminal in self._reasoning_markup_terminals: + return self._emit_for_state(value, token_count) + if self.skip_tool_parsing and terminal in self._tool_terminals: # Inkling reuses one terminal for tool, text, and reasoning exits. # Outside a forwarded tool span, apply its normal transition.