Skip to content

[Bugfix][Tool Parser] Fix hunyuan_a13b/xlam streaming: argument named "name" spawns a phantom tool call - #49535

Open
muhammadfawaz1 wants to merge 1 commit into
vllm-project:mainfrom
muhammadfawaz1:fix/tool-parser-name-key-phantom-call
Open

muhammadfawaz1 wants to merge 1 commit into
vllm-project:mainfrom
muhammadfawaz1:fix/tool-parser-name-key-phantom-call

Conversation

@muhammadfawaz1

Copy link
Copy Markdown
Contributor

Summary

In streaming mode, the hunyuan_a13b and xlam tool parsers treat any argument key "name" as the start of a new tool call. A tool call whose arguments contain a name field, a common shape, e.g. create_user(name=...), greet(name=...), is streamed as two tool calls: the real call with its arguments dropped, and a phantom tool call named after the argument's value. Non-streaming parsing of the same output is correct.

Root cause

Both parsers count/identify tool calls during streaming with a bare regex:

r'"name"\s*:\s*"([^"]+)"'

which matches the argument key "name" as well as a tool's own name field. This fix anchors the regex on the following "arguments" key so it only matches a tool's own name field:

r'"name"\s*:\s*"([^"]+)"(?=\s*,\s*"arguments"\s*:)'

xlam additionally had this pattern duplicated as inline string literals in two code paths; factored into a single self.tool_name_reg attribute, matching hunyuan_a13b's existing style.

Not a duplicate

Test plan

  • Added regression tests to both test_hunyuan_a13b_tool_parser.py and test_xlam_tool_parser.py covering the "name"-as-argument-key shape, non-streaming and streaming.
  • python -m pytest tests/tool_parsers/test_hunyuan_a13b_tool_parser.py tests/tool_parsers/test_xlam_tool_parser.py -v — all relevant tests pass (1 pre-existing xfail, unrelated nested-JSON limitation, unchanged by this PR).
  • pre-commit run --all-files on changed files: ruff check, ruff format, mypy all pass.

… "name" spawned a phantom tool call

The streaming tool-name regex in both parsers matched any `"name":` key in
the accumulated text, including an argument key also called `name` (e.g.
`create_user(name="Alice", age=30)`). This inflated the detected tool count,
causing a phantom tool call named after the argument's value and dropping
the real call's arguments. Non-streaming parsing was unaffected.

Anchor the regex on the following `"arguments"` key so it only matches a
tool's own name field. Adds regression tests for both parsers covering
non-streaming and streaming.

Co-authored-by: Mahad Rehman <114791389+mahadrehmann@users.noreply.github.com>
Signed-off-by: Mahad Rehman <114791389+mahadrehmann@users.noreply.github.com>
Signed-off-by: muhammadfawaz1 <135441198+muhammadfawaz1@users.noreply.github.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added tool-calling bug Something isn't working labels Jul 23, 2026

@chaunceyjiang chaunceyjiang left a comment

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.

If you discovered these two issues locally, I would suggest migrating these two parsers to the new Parser Engine instead of continuing to modify the old logic.

@muhammadfawaz1

Copy link
Copy Markdown
Contributor Author

@chaunceyjiang Thanks for the pointer , I can see the new Parser Engine exists (gemma4/minimax_m2/nemotron_v3/qwen3 are on it), but I don't see an existing effort tracking migration of hunyuan_a13b/xlam to it. Is that migration something you'd want done as a prerequisite for this fix, or would it be fine to land this minimal correctness fix now and track the engine migration as separate follow-up work? Happy to help with the migration too if that's the preferred path and it's not already being planned by someone else.

@chaunceyjiang

Copy link
Copy Markdown
Contributor

I think we should prioritize the migration. As far as I know, no one is currently working on it.

@muhammadfawaz1

Copy link
Copy Markdown
Contributor Author

@chaunceyjiang Closing this PR, starting the migration instead. Two things I hit that I'd rather confirm than guess:

The 4 engine-based parsers (Qwen3, MiniMax M2, Gemma4, Nemotron V3) all use explicit per-call delimiters (<function=, <invoke name=", etc.) so the state machine knows where one tool call ends and the next begins. Hunyuan_a13b/xlam instead emit one undelimited JSON array ([{...},{...}]) — boundaries are just JSON commas/braces, no terminal token. Does the engine already support that shape, or does it need new engine-level work?

Hunyuan_a13b has a separate hunyuan_a13b_reasoning_parser.py (xlam doesn't). Since ParserEngine unifies reasoning + tool calls in one class, should the reasoning parser get folded in too, or stay separate?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working tool-calling

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants