Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions tests/parser/engine/test_deepseek_v4.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
DSML_TOOL_START_VARIANTS,
DeepSeekV4Parser,
_dsml_arg_converter,
_dsml_arg_value_extender,
_unwrap_wrapper_args,
deepseek_v4_config,
)
Expand Down Expand Up @@ -1302,6 +1303,39 @@ def test_eos_not_leaked_when_reasoning_never_ends(self, chunk_size):
assert output.tool_calls == []


# ── Open string values stream without re-running the converter (#52846) ──


class TestArgValueExtender:
def test_stops_at_split_param_close(self):
extend = _dsml_arg_value_extender(
"<" + _PARAM_OPEN.format(name="k", is_str="true") + "x"
)
assert [extend(d) for d in ['a"\n', "</|DSML|para", "meter>"]] == [
'a\\"\\n',
"</|DSML|para",
None,
]

@pytest.mark.parametrize(
"raw_args",
[
"<" + _PARAM_OPEN.format(name="k", is_str="true") + "x" + _PARAM_CLOSE,
"<" + _PARAM_OPEN.format(name="k", is_str="false") + "1",
"<"
+ _PARAM_OPEN.format(name="k", is_str="true")
+ "x"
+ _PARAM_CLOSE
+ "<"
+ _PARAM_OPEN.format(name="k", is_str="true")
+ "y",
],
ids=["closed_value", "non_string_value", "repeated_key"],
)
def test_no_extender_unless_open_string_value(self, raw_args):
assert _dsml_arg_value_extender(raw_args) is None


# ── Malformed DSML markup must not leak into content (#51914) ─────────

_WRAPPER_VARIANTS = list(DSML_TOOL_START_VARIANTS)
Expand Down
37 changes: 37 additions & 0 deletions tests/parser/engine/test_deepseek_v41.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,40 @@ def test_python_argument_conversion_and_partial_values():
"bare": 7,
"text": " a<b",
}


def test_long_string_argument_streams_before_close():
# Regression for #52846: a string body has no ">" so it used to be
# buffered until the closing parameter tag arrived.
body = "A" * 4096
text = (
'<|DSML| calls>\n<|DSML| invoke name="emit">\n'
f'<|DSML| parameter name="text" string="true">{body}'
"</|DSML| parameter>\n</|DSML| invoke>\n</|DSML| calls>"
)
tools = [
{
"type": "function",
"function": {
"name": "emit",
"parameters": {
"type": "object",
"properties": {"text": {"type": "string"}},
},
},
}
]
tokenizer, tokens = tokenizer_for(text, False)
parser = parser_for(tokenizer, {"thinking": False})
results = replay_streaming(
parser, tokens, chunk_size=32, finished_on_last=True, tools=tools
)
arg_chunks = [
tool_call.function.arguments
for result in results
if result
for tool_call in result.tool_calls or []
if tool_call.function and tool_call.function.arguments
]
assert len(arg_chunks) > len(body) // 64
assert json.loads("".join(arg_chunks)) == {"text": body}
39 changes: 39 additions & 0 deletions tests/tool_parsers/test_deepseekv32_tool_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,45 @@ def test_emits_arguments_before_invoke_completes(self, parser):
assert combined # some partial args emitted
assert combined.startswith('{"k"')

def test_long_string_argument_streams_before_close(self):
"""A string body has no ">" and must not be buffered until
</parameter> arrives (GitHub #52846)."""
tool = ChatCompletionToolsParam(
function=FunctionDefinition(
name="emit",
parameters={
"type": "object",
"properties": {"text": {"type": "string"}},
},
),
)
parser = make_parser(tools=[tool])
body = "A" * 4096
chunks = [
f'{FC_START}\n{INV_START}emit">\n{PARAM_START}text" string="true">',
*(body[i : i + 32] for i in range(0, len(body), 32)),
f"{PARAM_END}\n{INV_END}\n{FC_END}",
]
per_chunk_args = []
prev = ""
for chunk in chunks:
result = parser.extract_tool_calls_streaming(
previous_text=prev,
current_text=prev + chunk,
delta_text=chunk,
previous_token_ids=[],
current_token_ids=[],
delta_token_ids=[1],
request=make_request(),
)
prev += chunk
per_chunk_args.append(
self._reconstruct_args([result]) if result is not None else ""
)

assert sum(1 for args in per_chunk_args[1:-1] if args) > 1
assert json.loads("".join(per_chunk_args)) == {"text": body}

def test_no_marker_leak_chunked(self, parser):
"""Chunked streaming must NOT leak DSML start-marker fragments
as content (GitHub #40801)."""
Expand Down
78 changes: 78 additions & 0 deletions tests/tool_parsers/test_deepseekv4_tool_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,84 @@ def test_streaming_emits_incremental_argument_chunks():
}


def _stream_param_body(properties: dict, param_open: str, body: str) -> list[str]:
"""Stream one parameter body in 32-char chunks, one argument list per chunk."""
tool = ChatCompletionToolsParam(
function=FunctionDefinition(
name="emit",
parameters={"type": "object", "properties": properties},
),
)
parser = make_parser(tools=[tool])
chunks = [
f'{TC_START}\n{INV_START}emit">\n{param_open}',
*(body[i : i + 32] for i in range(0, len(body), 32)),
f"{PARAM_END}\n{INV_END}\n{TC_END}",
]
per_chunk_args = []
previous_text = ""
for delta_text in chunks:
current_text = previous_text + delta_text
delta = parser.extract_tool_calls_streaming(
previous_text=previous_text,
current_text=current_text,
delta_text=delta_text,
previous_token_ids=[],
current_token_ids=[],
delta_token_ids=[1],
request=make_request(),
)
previous_text = current_text
per_chunk_args.append(
"".join(
tool_call.function.arguments
for tool_call in (delta.tool_calls if delta else [])
if tool_call.function and tool_call.function.arguments
)
)
return per_chunk_args


def test_streaming_long_string_argument_is_incremental():
# Regression for #52846: a string body has no ">" so it used to be
# buffered until </parameter> arrived.
body = "A" * 4096
per_chunk_args = _stream_param_body(
{"text": {"type": "string"}},
f'{PARAM_START}text" string="true">',
body,
)

body_chunks = per_chunk_args[1:-1]
assert sum(1 for args in body_chunks if args) > 1
assert json.loads("".join(per_chunk_args)) == {"text": body}


def test_streaming_non_string_argument_is_withheld_until_close():
body = "1" * 128
per_chunk_args = _stream_param_body(
{"count": {"type": "integer"}},
f'{PARAM_START}count" string="false">',
body,
)

assert not any(per_chunk_args[1:-1])
assert json.loads("".join(per_chunk_args)) == {"count": int(body)}


def test_streaming_string_argument_with_markup_is_incremental():
body = '<div class="a">x > y</div>\n' * 64
per_chunk_args = _stream_param_body(
{"html": {"type": "string"}},
f'{PARAM_START}html" string="true">',
body,
)

body_chunks = per_chunk_args[1:-1]
assert sum(1 for args in body_chunks if args) > 1
assert json.loads("".join(per_chunk_args)) == {"html": body}


def _with_strict(
tools: list[ChatCompletionToolsParam],
) -> list[ChatCompletionToolsParam]:
Expand Down
2 changes: 2 additions & 0 deletions vllm/parser/deepseek_v32.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
DSML_PARAM_CLOSE,
DSML_PARAM_START,
_dsml_arg_converter,
_dsml_arg_value_extender,
_unwrap_wrapper_args,
)
from vllm.parser.engine.events import EventType
Expand Down Expand Up @@ -115,6 +116,7 @@ def deepseek_v32_config() -> ParserEngineConfig:
},
arg_converter=_dsml_arg_converter,
arg_structural_chars=frozenset(">"),
arg_value_extender=_dsml_arg_value_extender,
strip_content_whitespace_with_tools=False,
tool_args_json=False,
)
Expand Down
46 changes: 45 additions & 1 deletion vllm/parser/deepseek_v4.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from vllm.parser.engine.events import EventType
from vllm.parser.engine.parser_engine import ParserEngine
from vllm.parser.engine.parser_engine_config import (
ArgValueExtender,
ParserEngineConfig,
ParserState,
Transition,
Expand Down Expand Up @@ -64,12 +65,14 @@ def _param_patterns(

The ``string`` attribute is optional: the model sometimes omits it, and
dropping such a parameter hands the client a tool call with no arguments.
The trailing-partial regex holds back an unfinished next parameter tag
so a streamed string value never includes it.
"""
start, close = re.escape(param_start), re.escape(param_close)
head = rf'{start}\s+name="([^"]+)"(?:\s+string="([^"]*)")?\s*>'
return (
re.compile(rf"{head}(.*?)(?:{close}|(?={start}\s+name=))", re.DOTALL),
re.compile(rf"{head}(.*)$", re.DOTALL),
re.compile(rf"{head}(.*?)(?:{start}[^>]*)?$", re.DOTALL),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this new regex is itself quadratic in time. It tries the optional {start}[^>]* branch at every <|DSML|parameter inside the value, and each attempt scans ahead to the next >. Both the converter and the extender factory run it on every conversion.

The lazy group plus $ also causes a smaller bug: $ matches just before a trailing \n, so for ...string="true">def f():\n the check pm.end(3) != len(raw_args) is true and the extender never arms. Those deltas then go back to waiting for the next >.

My coding assistant suggested a fix like below for this to avoid the quadratic regex and take a test with 2,200 literal markers in one string value from 3.7 seconds for the converter + extender call down to 5 milliseconds on my machine.

Below is one suggested fix and analysis created by my coding assistant, but please review this suggestion and only copy it verbatim if it also looks good to you.


Suggested fix: match greedily and do the holdback in Python. The holdback is the earliest param_start after the last >. That is exactly what the current regex selects, in O(n):

re.compile(rf"{head}(.*)\Z", re.DOTALL)  # trailing-partial regex


def _match_open_param(
    raw_args: str, pos: int, partial_param_re: re.Pattern, param_start: str
) -> tuple[str, str | None, str, bool] | None:
    """Return ``(name, is_str, value, held_back)`` for the trailing open param."""
    pm = partial_param_re.search(raw_args, pos)
    if pm is None:
        return None
    value = pm.group(3)
    cut = value.find(param_start, value.rfind(">") + 1)
    if cut >= 0:
        return pm.group(1), pm.group(2), value[:cut], True
    return pm.group(1), pm.group(2), value, False

The converter and _dsml_arg_value_extender both call this. The extender skips arming when held_back is true, which replaces the pm.end(3) check. _dsml_arg_converter takes a param_start kwarg, and deepseek_v41_config passes DSML_PARAM_START to it. I fuzzed this against the current regex on 200k inputs: results match except for trailing-\n inputs.

Please also add tests for the holdback, which currently has no direct coverage:

  • V4: _dsml_arg_converter(open + 'a > b<|DSML|parameter name="j', True) returns {"k": "a > b"}, and the extender returns None for the same input.
  • V4.1: the same check through deepseek_v41_config().arg_converter with the spaced tags.
  • The extender arms for an open value ending in "\n". This fails on the current head.

)


Expand Down Expand Up @@ -112,6 +115,46 @@ def _dsml_arg_converter(
return json.dumps(params, ensure_ascii=False)


def _dsml_arg_value_extender(
raw_args: str,
*,
param_re: re.Pattern = _PARAM_RE,
partial_param_re: re.Pattern = _PARTIAL_PARAM_RE,
param_start: str = DSML_PARAM_START,
param_close: str = DSML_PARAM_CLOSE,
) -> ArgValueExtender | None:
# ``arg_structural_chars`` skips every delta without ``>``, so a long
# string value would wait for its closing tag. Extend the open trailing
# ``string="true"`` value instead. The lexer can flush a held partial
# tag as text, so keep a tail to catch a tag split across deltas.
names = set()
last_end = 0
for m in param_re.finditer(raw_args):
names.add(m.group(1))
last_end = m.end()
pm = partial_param_re.search(raw_args, last_end)
if (
pm is None
or pm.group(2) != "true"
or pm.group(1) in names
or pm.end(3) != len(raw_args)
):
return None

keep = max(len(param_start), len(param_close)) - 1
tail = raw_args[-keep:]

def extend(delta: str) -> str | None:
nonlocal tail
text = tail + delta
if param_start in text or param_close in text:
return None
tail = text[-keep:]
return json.dumps(delta, ensure_ascii=False)[1:-1]

return extend


def _unwrap_wrapper_args(
args_json: str,
tools: list[Tool] | None,
Expand Down Expand Up @@ -238,6 +281,7 @@ def deepseek_v4_config(thinking: bool = False) -> ParserEngineConfig:
},
arg_converter=_dsml_arg_converter,
arg_structural_chars=frozenset(">"),
arg_value_extender=_dsml_arg_value_extender,
strip_content_whitespace_with_tools=False,
tool_args_json=False,
)
Expand Down
8 changes: 8 additions & 0 deletions vllm/parser/deepseek_v41.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from vllm.parser.deepseek_v4 import (
DeepSeekV4Parser,
_dsml_arg_converter,
_dsml_arg_value_extender,
_param_patterns,
deepseek_v4_config,
)
Expand Down Expand Up @@ -47,6 +48,13 @@ def deepseek_v41_config(thinking: bool = False) -> ParserEngineConfig:
param_re=_PARAM_RE,
partial_param_re=_PARTIAL_PARAM_RE,
),
arg_value_extender=functools.partial(
_dsml_arg_value_extender,
param_re=_PARAM_RE,
partial_param_re=_PARTIAL_PARAM_RE,
param_start=DSML_PARAM_START,
param_close=DSML_PARAM_CLOSE,
),
)


Expand Down
Loading