[Bugfix] Fix forced tool_choice crash when reasoning_parser consumes all content#40529
Open
FutureSkyFly wants to merge 1 commit into
Open
[Bugfix] Fix forced tool_choice crash when reasoning_parser consumes all content#40529FutureSkyFly wants to merge 1 commit into
FutureSkyFly wants to merge 1 commit into
Conversation
2 tasks
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces several assert statements with default value assignments in the OpenAI chat completion serving, engine serving, and abstract parser modules. Specifically, it ensures that tool_calls defaults to an empty list and content defaults to an empty string when they are None, preventing potential assertion errors and improving robustness. I have no feedback to provide.
…all content When using reasoning parsers (e.g. glm45, deepseek_r1) with forced tool_choice, the parser may consume the entire model output into <think>...</think> tags, leaving content as None. This caused AssertionError crashes in forced tool_choice paths. Fix: normalize None content to empty string (consistent with the existing "required" branch behavior) and replace the downstream assert with a defensive fallback. Changes: - engine/serving.py: replace 2x `assert content is not None` with `content = content or ""` in ToolChoiceFunction and ChatCompletionNamedToolChoiceParam branches - abstract_parser.py: same fix for the Responses API parsing path - chat_completion/serving.py: replace `assert tool_calls is not None` with `tool_calls = tool_calls or []` for defensive downstream handling Fixes vllm-project#40528 Signed-off-by: liuchenbing <liuchenbing2026@outlook.com>
0c51175 to
e516b5e
Compare
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When using
--reasoning-parser(e.g.,glm45,deepseek_r1) with forcedtool_choice, the reasoning parser may consume the entire model output into<think>...</think>tags, leavingcontentasNone. This causesAssertionErrorcrashes in the forced tool_choice code paths.Fix: Normalize
Nonecontent to empty string""(consistent with the existing"required"branch which already doescontent = content or ""), and replace the downstreamassert tool_callswith a defensive fallback.Changes
vllm/entrypoints/openai/engine/serving.pyassert content is not None→content = content or ""vllm/parser/abstract_parser.pyassert content is not None→content = content or ""vllm/entrypoints/openai/chat_completion/serving.pyassert tool_calls is not None and len(tool_calls) > 0→tool_calls = tool_calls or []Total: 5 lines changed across 3 files.
How to reproduce
Why this is not a duplicate
PR #40148 fixes
engine/serving.pyandabstract_parser.pybut does not fix the downstreamassertinchat_completion/serving.py(line 1417). This PR covers all 5 crash points across all 3 files.Fixes #40528
Related: #40147, #40148
Test
The fix is consistent with the existing
"required"branch pattern which already usescontent = content or ""andtool_calls = tool_calls or [].AI Disclosure
AI assistance was used in identifying and fixing this bug.
Signed-off-by: liuchenbing liuchenbing2026@outlook.com