[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) <noreply@anthropic.com>
This commit is contained in:
cherry77-cloud
co-authored by
Ben Browning
Claude Opus 4.8
Ben Browning
Claude Opus 4.8
parent
58d3918e3e
commit
75231eff2f
@@ -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
|
||||
|
||||
@@ -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 ``</tool_call>`` 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 = '<tool_call>{"a":1}</tool_call>'
|
||||
_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 (``</a>``) and an inkling-style token
|
||||
that closes both tool and text blocks (``</b>``). A stale span would
|
||||
route the trailing ``</b>`` down the forward path and leak it.
|
||||
"""
|
||||
ts, close_a, close_b = "<tc>", "</a>", "</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 ──────────────────────────────────
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user