From b24227274b66a651e6b7e0c0e1e4413deed52ad1 Mon Sep 17 00:00:00 2001 From: pctablet505 <55033230+pctablet505@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:36:19 +0530 Subject: [PATCH 1/2] [Bugfix][Parser] GLM-4.7: stray arg tags must not leak into tool-call argument keys `_glm47_arg_converter` matched the argument key with an unbounded `.*?` before ``. Because the group is non-greedy but unbounded, the regex engine backtracks *past* the first `` whenever that is what it takes to reach the following `` -- so a repeated or stray tag in the model stream is absorbed into the key rather than ignored. Observed in production against a GLM server run with `--tool-call-parser glm47`: an argument object came back as `{"path": "/Users/user/Desktop/project"}`. The client rejected the call ("must have required property 'path'"), and the model then retried with an invented path. Match runs of non-`<` text instead, so the key group cannot cross its own delimiter, and accept a run of closing tags so the intended key survives. Well-formed input is unaffected. The unrolled possessive form also avoids the backtracking a tempered dot would add on the streaming path, which re-runs the converter over the whole accumulated buffer on every chunk: on an unterminated 20 KB buffer it is 1251 us -> 10 us. Covers both `_ARG_RE` and `_PARTIAL_ARG_RE`, with a streaming test that fails if only one of the two is fixed. Signed-off-by: pctablet505 <55033230+pctablet505@users.noreply.github.com> --- .../test_glm47_moe_tool_parser.py | 94 +++++++++++++++++++ vllm/parser/glm47_moe.py | 11 ++- 2 files changed, 101 insertions(+), 4 deletions(-) diff --git a/tests/tool_parsers/test_glm47_moe_tool_parser.py b/tests/tool_parsers/test_glm47_moe_tool_parser.py index dc4b726a5383..9665852f00f6 100644 --- a/tests/tool_parsers/test_glm47_moe_tool_parser.py +++ b/tests/tool_parsers/test_glm47_moe_tool_parser.py @@ -17,6 +17,7 @@ ) from vllm.entrypoints.openai.responses.protocol import ResponsesRequest from vllm.entrypoints.openai.responses.utils import build_response_output_items +from vllm.parser.glm47_moe import _glm47_arg_converter from vllm.tokenizers import get_tokenizer from vllm.tool_parsers.glm47_moe_tool_parser import Glm47MoeModelToolParser @@ -266,3 +267,96 @@ def test_with_args(self, glm47_tool_parser, mock_request): ] args = json.loads("".join(arguments)) assert args["city"] == "Beijing" + + def test_stray_arg_key_tag(self, glm47_tool_parser, mock_request): + """A duplicated `` in the stream must not reach the key. + + This is the end-to-end path: the engine accumulates the stray tag into + `slot.args`, so only a streaming test proves the converter sees it. + """ + _reset(glm47_tool_parser) + chunks = [ + "", + "get_weather\n", + "city", + "", + "", + "Beijing", + "", + "", + ] + current_text = "" + deltas = [] + for chunk in chunks: + current_text += chunk + delta = glm47_tool_parser.extract_tool_calls_streaming( + previous_text="", + current_text=current_text, + delta_text=chunk, + previous_token_ids=[], + current_token_ids=[], + delta_token_ids=[], + request=mock_request, + ) + if delta: + deltas.append(delta) + arguments = [ + tool_call.function.arguments + for delta in deltas + for tool_call in (delta.tool_calls or []) + if tool_call.function and tool_call.function.arguments + ] + joined = "".join(arguments) + # Checked before json.loads: a partial fix truncates the stream, and + # the JSONDecodeError would hide which half regressed. + assert "" not in joined + assert json.loads(joined) == {"city": "Beijing"} + + +@pytest.mark.parametrize( + "raw_args", + [ + pytest.param( + "path/tmp/x", + id="repeated-closing-tag", + ), + pytest.param( + "path/tmp/x", + id="repeated-opening-tag", + ), + pytest.param( + "path/tmp/x", + id="well-formed", + ), + ], +) +def test_arg_converter_ignores_stray_arg_tags(raw_args: str): + """Stray `arg_key` tags must not leak into the argument key.""" + assert json.loads(_glm47_arg_converter(raw_args, False)) == {"path": "/tmp/x"} + + +@pytest.mark.parametrize( + "raw_args", + [ + pytest.param( + "path/tmp/x", + id="repeated-closing-tag", + ), + pytest.param( + "path/tmp/x", + id="repeated-opening-tag", + ), + pytest.param( + "path/tmp/x", + id="well-formed", + ), + ], +) +def test_partial_arg_converter_ignores_stray_arg_tags(raw_args: str): + """The streaming path uses a separate pattern; it needs the same guarantee. + + These inputs have no closing ``, which is what an in-flight + stream actually feeds, so they reach `_PARTIAL_ARG_RE` rather than + `_ARG_RE`. + """ + assert json.loads(_glm47_arg_converter(raw_args, True)) == {"path": "/tmp/x"} diff --git a/vllm/parser/glm47_moe.py b/vllm/parser/glm47_moe.py index 00e517f2ea68..c27663890252 100644 --- a/vllm/parser/glm47_moe.py +++ b/vllm/parser/glm47_moe.py @@ -48,14 +48,17 @@ ("<|system|>", "<|user|>", "<|assistant|>", "<|observation|>") ) +# A plain ``.*?`` here backtracks past the first ```` to reach the +# following ````, absorbing a stray tag into the key. Match runs of +# non-``<`` text instead, so the group cannot cross its own delimiter. +_ARG_KEY = r"(?P[^<]*+(?:<(?!/?arg_key>)[^<]*+)*+)(?:\s*)+" + _ARG_RE = re.compile( - r"(?P.*?)\s*" - r"(?P.*?)", + _ARG_KEY + r"(?P.*?)", re.DOTALL, ) _PARTIAL_ARG_RE = re.compile( - r"(?P.*?)\s*" - r"(?P.*)$", + _ARG_KEY + r"(?P.*)$", re.DOTALL, ) From d2a6926566605796637c3ba54c28ef7ba69c67a4 Mon Sep 17 00:00:00 2001 From: pctablet505 <55033230+pctablet505@users.noreply.github.com> Date: Sat, 10 Oct 2026 19:01:52 +0530 Subject: [PATCH 2/2] [Bugfix][Parser] GLM-4.7: trim comments and test docstrings Keep the one comment a reader needs; the bug narrative lives in the PR description. Co-Authored-By: Claude Sonnet 5.5 Signed-off-by: pctablet505 <55033230+pctablet505@users.noreply.github.com> --- tests/tool_parsers/test_glm47_moe_tool_parser.py | 13 ++----------- vllm/parser/glm47_moe.py | 4 +--- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/tool_parsers/test_glm47_moe_tool_parser.py b/tests/tool_parsers/test_glm47_moe_tool_parser.py index 9665852f00f6..2ab633d0ac23 100644 --- a/tests/tool_parsers/test_glm47_moe_tool_parser.py +++ b/tests/tool_parsers/test_glm47_moe_tool_parser.py @@ -269,11 +269,7 @@ def test_with_args(self, glm47_tool_parser, mock_request): assert args["city"] == "Beijing" def test_stray_arg_key_tag(self, glm47_tool_parser, mock_request): - """A duplicated `` in the stream must not reach the key. - - This is the end-to-end path: the engine accumulates the stray tag into - `slot.args`, so only a streaming test proves the converter sees it. - """ + """A duplicated `` in the stream must not reach the key.""" _reset(glm47_tool_parser) chunks = [ "", @@ -353,10 +349,5 @@ def test_arg_converter_ignores_stray_arg_tags(raw_args: str): ], ) def test_partial_arg_converter_ignores_stray_arg_tags(raw_args: str): - """The streaming path uses a separate pattern; it needs the same guarantee. - - These inputs have no closing ``, which is what an in-flight - stream actually feeds, so they reach `_PARTIAL_ARG_RE` rather than - `_ARG_RE`. - """ + """Without `` the input reaches `_PARTIAL_ARG_RE`, not `_ARG_RE`.""" assert json.loads(_glm47_arg_converter(raw_args, True)) == {"path": "/tmp/x"} diff --git a/vllm/parser/glm47_moe.py b/vllm/parser/glm47_moe.py index a6f988e13462..c2c7d1611ea0 100644 --- a/vllm/parser/glm47_moe.py +++ b/vllm/parser/glm47_moe.py @@ -51,9 +51,7 @@ ("<|system|>", "<|user|>", "<|assistant|>", "<|observation|>") ) -# A plain ``.*?`` here backtracks past the first ```` to reach the -# following ````, absorbing a stray tag into the key. Match runs of -# non-``<`` text instead, so the group cannot cross its own delimiter. +# The key group cannot cross ````, so a stray tag never leaks into it. _ARG_KEY = r"(?P[^<]*+(?:<(?!/?arg_key>)[^<]*+)*+)(?:\s*)+" _ARG_RE = re.compile(