Skip to content

[Bugfix] Fix step3p5 parser when using mtp#33690

Merged
chaunceyjiang merged 4 commits intovllm-project:mainfrom
mariohong128:fix_step3p5_parser
Feb 5, 2026
Merged

[Bugfix] Fix step3p5 parser when using mtp#33690
chaunceyjiang merged 4 commits intovllm-project:mainfrom
mariohong128:fix_step3p5_parser

Conversation

@mariohong128
Copy link
Contributor

@mariohong128 mariohong128 commented Feb 3, 2026

Purpose

Fix step3.5 parser when using mtp.
If model outputs </tool_call><tool_call>< (using mtp will greatly increase the possibility of this), parser will start a new empty toolcall incorrectly.

{\"error\":{\"message\":\"1 validation error for ValidatorIterator\\\\n1.function.name\\\\n  Field required [type=missing, input_value={\\'arguments\\': \\'{}\\'}, input_type=dict]\\\\n    For further information visit https://errors.pydantic.dev/2.11/v/missing None\",\"type\":\"BadRequestError\",\"param\":null,\"code\":400}}

Test Plan

pytest tests/tool_parsers/test_step3p5_tool_parser.py

Test Result

All tests passed.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: mariohong <mariohong128@gmail.com>
Signed-off-by: mariohong <mariohong128@gmail.com>
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors.

You ask your reviewers to trigger select CI tests on top of fastcheck CI.

Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

🚀

@mergify mergify bot added the bug Something isn't working label Feb 3, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request aims to fix the step3.5 tool parser for scenarios involving multiple tool calls by introducing a fallback_call_id. This new variable is intended to ensure that fallback logic for closing tags applies to the correct tool call, especially in streaming mode. While the overall direction is correct, the implementation for determining fallback_call_id is overly complex and misses a key transition case, which could lead to parsing failures. I've provided a critical review comment with a suggested simplification that makes the logic more robust and corrects the identified issue.

Signed-off-by: mariohong <mariohong128@gmail.com>
@mariohong128 mariohong128 marked this pull request as ready for review February 3, 2026 12:05
@chaunceyjiang chaunceyjiang self-assigned this Feb 3, 2026
@chaunceyjiang chaunceyjiang added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 4, 2026
@chaunceyjiang chaunceyjiang enabled auto-merge (squash) February 5, 2026 13:58
@mariohong128
Copy link
Contributor Author

@chaunceyjiang We have internally verified the correctness of this parser modification. Can we merge it now?

Copy link
Collaborator

@chaunceyjiang chaunceyjiang left a comment

Choose a reason for hiding this comment

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

Thanks~

@chaunceyjiang chaunceyjiang merged commit 82914d2 into vllm-project:main Feb 5, 2026
41 checks passed
ItzDEXX pushed a commit to ItzDEXX/vllm that referenced this pull request Feb 19, 2026
Signed-off-by: mariohong <mariohong128@gmail.com>
tunglinwood pushed a commit to tunglinwood/vllm that referenced this pull request Mar 4, 2026
Signed-off-by: mariohong <mariohong128@gmail.com>
bbrowning added a commit to bbrowning/vllm that referenced this pull request Mar 26, 2026
Audited recent tool parser bug-fix PRs and found that several
landed without corresponding test coverage. Added unit tests
for each fix to prevent regressions.

- Mistral: fast detokenization text detection (PR vllm-project#37209)
- Qwen3Coder: malformed XML crash, anyOf double-encoding,
  speculative decode streaming (PRs vllm-project#36774, vllm-project#36032, vllm-project#35615)
- DeepSeekV32: delimiter preservation with fast detokenization,
  skip_special_tokens adjustment (PR vllm-project#33964)
- GLM-4 MoE: zero-argument tool calls, transformers 5.x delimiter
  handling, Unicode character preservation (PRs vllm-project#32321, vllm-project#31622, vllm-project#30920)
- MiniMax M2: anyOf nullable parameter handling for non-null and
  null values (PR vllm-project#32342)
- Step3p5: MTP-style variable-chunk and multi-token streaming
  (PR vllm-project#33690)
- Kimi K2: native tool call ID extraction and multi-turn ID
  continuity (PR vllm-project#32768)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Ben Browning <bbrownin@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants