[Bugfix] Drop incomplete tool-call markup in non-streaming to match streaming (#47562)
Signed-off-by: Suvrakamal Das <jyb2084.usa@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Ben Browning <56071+bbrowning@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude
Ben Browning
parent
2f01039666
commit
e7cf4730d6
@@ -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
|
||||
|
||||
@@ -1909,3 +1909,200 @@ class TestDropSpecialTokens:
|
||||
e.value for e in events if e.type == EventType.REASONING_CHUNK
|
||||
)
|
||||
assert "<bos>" not in reasoning_text
|
||||
|
||||
|
||||
# ── TestTruncatedToolOpenerStreamParity ──────────────────────────────
|
||||
|
||||
|
||||
class TestTruncatedToolOpenerStreamParity:
|
||||
"""Regression tests for #47137: when generation terminates inside a
|
||||
``<tool_call>`` 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 = {
|
||||
"<tool_call>": 100,
|
||||
"</tool_call>": 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",
|
||||
[
|
||||
["<tool_call>"],
|
||||
["<tool_call>", "\n"],
|
||||
["<tool_call>", "\n", "<"],
|
||||
["<tool_call>", "\n", "<function"],
|
||||
],
|
||||
)
|
||||
def test_truncated_opener_dropped_in_both_paths(self, mock_request, chunks):
|
||||
"""A cutoff inside the opener yields no content and no tool calls
|
||||
in both the non-streaming and streaming paths."""
|
||||
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 or "") == streamed == ""
|
||||
|
||||
def test_content_before_truncated_opener_preserved(self, mock_request):
|
||||
"""Only the incomplete markup is dropped; content generated before
|
||||
the opener is returned identically by both paths."""
|
||||
chunks = ["Checking the weather. ", "<tool_call>", "\n", "<function"]
|
||||
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 == "Checking the weather. "
|
||||
|
||||
def test_content_around_unpromoted_tool_block_stays_ordered(self, mock_request):
|
||||
"""Text surrounding a complete-but-unparsable tool block keeps its
|
||||
original order in the non-streaming path, matching streaming."""
|
||||
chunks = ["A ", "<tool_call>", "garbage", "</tool_call>", " 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 = (
|
||||
"<tool_call>\n"
|
||||
"<function=get_weather>\n"
|
||||
"<parameter=city>Tokyo</parameter>\n"
|
||||
"</function>\n"
|
||||
"</tool_call>"
|
||||
)
|
||||
_, 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 = {
|
||||
"<think>": 90,
|
||||
"</think>": 91,
|
||||
"<tool_call>": 100,
|
||||
"</tool_call>": 101,
|
||||
}
|
||||
|
||||
# Two distinct failure modes are covered: qwen3_coder consumed a
|
||||
# stray </think> marker in CONTENT state (unbalanced markup),
|
||||
# deepseek_v4 re-entered REASONING on <think> 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 = ["<think>", "reasoning here", "</think>", "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 = "<think>plan</think>"
|
||||
text = think + (
|
||||
"<tool_call>\n"
|
||||
"<function=get_weather>\n"
|
||||
"<parameter=city>Tokyo</parameter>\n"
|
||||
"</function>\n"
|
||||
"</tool_call>"
|
||||
)
|
||||
_, 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
|
||||
|
||||
@@ -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 <tool_call> 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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user