From 75231eff2f3873e2bce7cc9558bb5227ea70b808 Mon Sep 17 00:00:00 2001 From: cherry77-cloud <1615405@qq.com> Date: Sun, 9 Aug 2026 00:34:32 +0800 Subject: [PATCH] [Bugfix][Parser] Prevent Inkling block-end leakage with tools (#51391) Signed-off-by: cherry77-cloud <1615405@qq.com> Co-authored-by: Ben Browning <56071+bbrowning@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) --- tests/parser/engine/test_inkling.py | 212 ++++++++++++++++++ tests/parser/engine/test_parser_engine.py | 85 +++++++ vllm/parser/engine/streaming_parser_engine.py | 85 ++++--- 3 files changed, 352 insertions(+), 30 deletions(-) diff --git a/tests/parser/engine/test_inkling.py b/tests/parser/engine/test_inkling.py index 582783fd5d..d0aa68f7e8 100644 --- a/tests/parser/engine/test_inkling.py +++ b/tests/parser/engine/test_inkling.py @@ -19,6 +19,10 @@ from tests.parser.engine.streaming_helpers import ( collect_function_name, collect_tool_arguments, ) +from vllm.entrypoints.openai.chat_completion.protocol import ( + ChatCompletionToolsParam, + FunctionDefinition, +) from vllm.parser.engine.parser_engine_config import ParserState from vllm.parser.inkling import InklingParser, _inkling_arg_converter from vllm.parser.parser_manager import ParserManager @@ -140,6 +144,51 @@ def _collect_reasoning(results) -> str: return "".join(d.reasoning for d, _ in results if d and d.reasoning) +def _function_tool(name: str = "get_weather") -> ChatCompletionToolsParam: + """A real function-tool definition, as a request would carry it.""" + return ChatCompletionToolsParam( + function=FunctionDefinition( + name=name, + parameters={"type": "object", "properties": {}}, + ), + ) + + +def _delegating(mock_tokenizer, tools=None): + """Build the served reasoning+tool DelegatingParser for Inkling.""" + parser_cls = ParserManager.get_parser( + tool_parser_name="inkling", + reasoning_parser_name="inkling", + enable_auto_tools=True, + ) + return parser_cls(mock_tokenizer, tools or []) + + +def _stream_delegating(parser, request, text, chunk_size, prompt_token_ids): + """Stream ``text`` through ``DelegatingParser.parse_delta``, ``chunk_size`` + tokens per delta; return ``(content, reasoning, ordered tool names)``.""" + tokens = _tokenize(text) + content, reasoning, tools = "", "", {} + for start in range(0, len(tokens), chunk_size): + batch = tokens[start : start + chunk_size] + delta = parser.parse_delta( + "".join(t for _, t in batch), + [tid for tid, _ in batch], + request, + prompt_token_ids=prompt_token_ids if start == 0 else None, + finished=(start + chunk_size >= len(tokens)), + ) + if delta and delta.content: + content += delta.content + if delta and delta.reasoning: + reasoning += delta.reasoning + if delta and delta.tool_calls: + for tc in delta.tool_calls: + if tc.function and tc.function.name: + tools[tc.index] = tc.function.name + return content, reasoning, [tools[k] for k in sorted(tools)] + + class TestArgConverter: def test_complete_wrapper(self): raw = '{"name":"get_weather","args":{"city":"SF"}}' @@ -533,3 +582,166 @@ class TestRegisteredAdapters: assert result.tools_called assert result.tool_calls[0].function.name == "f" assert json.loads(result.tool_calls[0].function.arguments) == {"a": 1} + + +class TestDelegatingTwoPass: + """Served reasoning+tool ``DelegatingParser`` path (issue #51387). + + Unlike ``TestStreaming`` above (single engine, ``skip_tool_parsing=False``), + these drive the real two-pass parser, where the reasoning pass runs with + ``skip_tool_parsing=True`` — the path where the trailing-marker leak lived. + + The bug is specific to the *tools-enabled* plain-text/reasoning path, so the + content-only cases (which emit no tool call and would still pass in a + no-tools mode) offer a real function tool on both the request and the + parser, keeping ``tool_choice="auto"``. + """ + + GEN_PROMPT = [_TML_VOCAB[MSG_MODEL]] + + def test_plain_text_non_streaming(self, mock_tokenizer, mock_request): + tools = [_function_tool()] + mock_request.tools = tools + _, content, calls = _delegating(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 END_MESSAGE not in content + assert not calls + + @pytest.mark.parametrize("chunk_size", [1, 3, 7, 64]) + def test_plain_text_streaming(self, mock_tokenizer, mock_request, chunk_size): + tools = [_function_tool()] + mock_request.tools = tools + content, _, calls = _stream_delegating( + _delegating(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 END_MESSAGE not in content + assert not calls + + def test_reasoning_then_text(self, mock_tokenizer, mock_request): + tools = [_function_tool()] + mock_request.tools = tools + reasoning, content, _ = _delegating(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 reasoning == "plan" + assert content == "Let me look." + assert END_MESSAGE not in content + + def test_multi_tool_round_trip(self, mock_tokenizer, mock_request): + _, _, tools = _delegating(mock_tokenizer).parse( + _tool_block("f", '{"a":1}') + MSG_MODEL + _tool_block("g", '{"b":2}'), + mock_request, + enable_auto_tools=True, + ) + assert [t.name for t in tools] == ["f", "g"] + + def test_text_then_tool(self, mock_tokenizer, mock_request): + _, content, tools = _delegating(mock_tokenizer).parse( + f"{TEXT_START}intro{END_MESSAGE}{MSG_MODEL}" + _tool_block("f", "{}"), + mock_request, + enable_auto_tools=True, + ) + assert content == "intro" + assert [t.name for t in tools] == ["f"] + + def test_tool_then_text(self, mock_tokenizer, mock_request): + _, content, tools = _delegating(mock_tokenizer).parse( + _tool_block("f", "{}") + MSG_MODEL + f"{TEXT_START}done{END_MESSAGE}", + mock_request, + enable_auto_tools=True, + ) + assert content == "done" + assert [t.name for t in tools] == ["f"] + + def test_two_tools_then_text(self, mock_tokenizer, mock_request): + _, content, tools = _delegating(mock_tokenizer).parse( + _tool_block("f", "{}") + + MSG_MODEL + + _tool_block("g", "{}") + + MSG_MODEL + + f"{TEXT_START}after{END_MESSAGE}", + mock_request, + enable_auto_tools=True, + ) + assert content == "after" + assert [t.name for t in tools] == ["f", "g"] + + def test_reasoning_then_tool_non_streaming(self, mock_tokenizer, mock_request): + reasoning, _, tools = _delegating(mock_tokenizer).parse( + f"{THINK_START}think{END_MESSAGE}{MSG_MODEL}" + _tool_block("f", '{"x":1}'), + mock_request, + enable_auto_tools=True, + ) + assert reasoning == "think" + assert [t.name for t in tools] == ["f"] + + def test_end_sampling_text_closer_consumed(self, mock_tokenizer, mock_request): + tools = [_function_tool()] + mock_request.tools = tools + _, content, calls = _delegating(mock_tokenizer, tools).parse( + f"{TEXT_START}hi{END_SAMPLING}", + mock_request, + enable_auto_tools=True, + ) + assert content == "hi" + assert END_SAMPLING not in content + assert not calls + + def test_end_sampling_tool_closer_round_trips(self, mock_tokenizer, mock_request): + _, _, tools = _delegating(mock_tokenizer).parse( + f'{TOOL_JSON}{{"name":"f","args":{{}}}}{END_SAMPLING}', + mock_request, + enable_auto_tools=True, + ) + assert [t.name for t in tools] == ["f"] + + def test_incomplete_tool_at_eos(self, mock_tokenizer, mock_request): + _, _, tools = _delegating(mock_tokenizer).parse( + f'{TOOL_JSON}{{"name":"d","args":{{"k":"v"', + mock_request, + enable_auto_tools=True, + ) + assert [t.name for t in tools] == ["d"] + + def test_reset_reuse_after_incomplete_span(self, mock_tokenizer, mock_request): + parser = _delegating(mock_tokenizer) + parser.parse( + f'{TOOL_JSON}{{"name":"d","args":{{"k":"v"', + mock_request, + enable_auto_tools=True, + ) + _, content, _ = parser.parse( + f"{TEXT_START}fresh{END_MESSAGE}", + mock_request, + enable_auto_tools=True, + ) + assert content == "fresh" + assert END_MESSAGE not in content + + @pytest.mark.parametrize("chunk_size", [1, 3, 64]) + def test_reasoning_then_tool_streaming( + self, mock_tokenizer, mock_request, chunk_size + ): + content, _, tools = _stream_delegating( + _delegating(mock_tokenizer), + mock_request, + f"{THINK_START}plan{END_MESSAGE}{MSG_MODEL}" + + _tool_block("get_weather", '{"city":"SF"}'), + chunk_size, + self.GEN_PROMPT, + ) + assert tools == ["get_weather"] + assert TOOL_JSON not in content + assert END_MESSAGE not in content diff --git a/tests/parser/engine/test_parser_engine.py b/tests/parser/engine/test_parser_engine.py index d97aafe365..8616aac381 100644 --- a/tests/parser/engine/test_parser_engine.py +++ b/tests/parser/engine/test_parser_engine.py @@ -1075,6 +1075,91 @@ class TestReasoningOnlyEndTokenLeak: assert "Hi!" in d2.content +class TestSkipToolSpanForwarding: + """Reasoning (skip_tool_parsing) pass: tool syntax is forwarded verbatim + for the tool pass, and the lexical passthrough flag never goes stale. + + ``_combined_config`` defines ```` only from ``TOOL_ARGS``; the + skip pass stays in ``CONTENT``, so the closer arrives with no current-state + transition. It must still be forwarded and must clear the span — otherwise + ``_in_skipped_tool_span`` stays True for the rest of the request. This is the + shape of every current engine grammar whose wrapper closer has no CONTENT + transition (qwen3, deepseek, glm47_moe, nemotron_v3). + """ + + _TOOL_VOCAB = {**_VOCAB, '{"a":1}': 300} + _TOOL_CALL = '{"a":1}' + _TOOL_IDS = [202, 300, 203] + + def _skip_engine(self): + engine = _make_engine( + vocab=self._TOOL_VOCAB, + special_tokens=list(_VOCAB.keys()), + ) + engine._engine.skip_tool_parsing = True + engine._engine.reset(initial_state=ParserState.CONTENT) + return engine + + def test_tool_syntax_forwarded_and_span_cleared(self): + engine = self._skip_engine() + events = engine._engine.feed(self._TOOL_CALL, self._TOOL_IDS) + forwarded = "".join(e.value for e in events if e.type == EventType.TEXT_CHUNK) + assert forwarded == self._TOOL_CALL + assert engine._engine._in_skipped_tool_span is False + + def test_span_not_stale_across_two_tool_calls(self): + engine = self._skip_engine() + engine._engine.feed(self._TOOL_CALL, self._TOOL_IDS) + assert engine._engine._in_skipped_tool_span is False + events = engine._engine.feed(self._TOOL_CALL, self._TOOL_IDS) + forwarded = "".join(e.value for e in events if e.type == EventType.TEXT_CHUNK) + assert forwarded == self._TOOL_CALL + assert engine._engine._in_skipped_tool_span is False + + def test_content_after_transitionless_closer_observable(self): + """Observable lifecycle contract (not just the private flag): after a + tool closer with no CONTENT transition, a later *shared* block-ender + that also closes text blocks must be consumed, not leaked as content. + + The config combines the two real patterns — a qwen3-style tool-only + closer with no CONTENT transition (````) and an inkling-style token + that closes both tool and text blocks (````). A stale span would + route the trailing ```` down the forward path and leak it. + """ + ts, close_a, close_b = "", "", "" + cfg = ParserEngineConfig( + name="shared_closer_test", + terminals={"TOOL_START": ts, "CLOSE_A": close_a, "CLOSE_B": close_b}, + transitions={ + (ParserState.CONTENT, "TOOL_START"): Transition( + ParserState.TOOL_ARGS, (EventType.TOOL_CALL_START,) + ), + (ParserState.TOOL_ARGS, "CLOSE_A"): Transition( + ParserState.CONTENT, (EventType.TOOL_CALL_END,) + ), + (ParserState.TOOL_ARGS, "CLOSE_B"): Transition( + ParserState.CONTENT, (EventType.TOOL_CALL_END,) + ), + (ParserState.CONTENT, "CLOSE_B"): Transition(ParserState.CONTENT, ()), + }, + initial_state=ParserState.CONTENT, + content_events={ + ParserState.CONTENT: EventType.TEXT_CHUNK, + ParserState.TOOL_ARGS: EventType.ARG_VALUE_CHUNK, + }, + ) + engine = _make_engine(config=cfg, vocab={ts: 210, close_a: 211, close_b: 212}) + engine._engine.skip_tool_parsing = True + engine._engine.reset(initial_state=ParserState.CONTENT) + + events = engine._engine.feed(f"{ts}args{close_a}text{close_b}", []) + content = "".join(e.value for e in events if e.type == EventType.TEXT_CHUNK) + + assert content == f"{ts}args{close_a}text" + assert close_b not in content + assert engine._engine._in_skipped_tool_span is False + + # ── TestToolAdapterForwardsKwargs ────────────────────────────────── diff --git a/vllm/parser/engine/streaming_parser_engine.py b/vllm/parser/engine/streaming_parser_engine.py index f5c395f76d..41aa2bfbf6 100644 --- a/vllm/parser/engine/streaming_parser_engine.py +++ b/vllm/parser/engine/streaming_parser_engine.py @@ -156,6 +156,13 @@ class StreamingParserEngine: for (state, terminal), tr in config.transitions.items() if tr.next_state in self._TOOL_STATES or state in self._TOOL_STATES ) + # TOOL_CALL_END may close an inner call rather than its lexical wrapper, + # as in MiniMax, so identify exits from state transitions instead. + self._tool_exit_terminals: frozenset[str] = frozenset( + terminal + for (state, terminal), tr in config.transitions.items() + if state in self._TOOL_STATES and tr.next_state not in self._TOOL_STATES + ) self.skip_tool_parsing = False self.reset(initial_state=initial_state) @@ -186,6 +193,7 @@ class StreamingParserEngine: self._scanner.reset() self._lexer.reset() self._message_header_buffer = "" + self._in_skipped_tool_span = False self._reset_args_state() def feed( @@ -316,39 +324,56 @@ class StreamingParserEngine: if transition is None: if self._has_drops and terminal == DROP_TERMINAL: return [] + # The projected skip state may not define the wrapper closer. + if self.skip_tool_parsing and terminal in self._tool_exit_terminals: + self._in_skipped_tool_span = False return self._emit_for_state(value) if self.skip_tool_parsing and terminal in self._tool_terminals: - if self.state == ParserState.MESSAGE_HEADER: - self.state = ParserState.CONTENT - self._message_header_buffer = "" - return [ - SemanticEvent( - EventType.TEXT_CHUNK, - value=value, - tool_index=self.tool_index, - ) - ] - if EventType.REASONING_END in transition.events: - self.state = ParserState.CONTENT - return [ - SemanticEvent( - EventType.REASONING_END, - value=value, - tool_index=self.tool_index, - ), - SemanticEvent( - EventType.TEXT_CHUNK, - value=value, - tool_index=self.tool_index, - ), - ] - content_type = self.config.content_events.get(self.state) - if content_type is not None: - return [ - SemanticEvent(content_type, value=value, tool_index=self.tool_index) - ] - return [] + # Inkling reuses one terminal for tool, text, and reasoning exits. + # Outside a forwarded tool span, apply its normal transition. + is_opener = transition.next_state in self._TOOL_STATES + is_exit = terminal in self._tool_exit_terminals + used_as_plain_closer = ( + is_exit and not is_opener and not self._in_skipped_tool_span + ) + if not used_as_plain_closer: + if is_opener: + self._in_skipped_tool_span = True + elif is_exit: + self._in_skipped_tool_span = False + if self.state == ParserState.MESSAGE_HEADER: + self.state = ParserState.CONTENT + self._message_header_buffer = "" + return [ + SemanticEvent( + EventType.TEXT_CHUNK, + value=value, + tool_index=self.tool_index, + ) + ] + if EventType.REASONING_END in transition.events: + self.state = ParserState.CONTENT + return [ + SemanticEvent( + EventType.REASONING_END, + value=value, + tool_index=self.tool_index, + ), + SemanticEvent( + EventType.TEXT_CHUNK, + value=value, + tool_index=self.tool_index, + ), + ] + content_type = self.config.content_events.get(self.state) + if content_type is not None: + return [ + SemanticEvent( + content_type, value=value, tool_index=self.tool_index + ) + ] + return [] if transition.skip_in_token_id_mode and self._ever_had_token_ids: return self._emit_for_state(value)