[releases/v0.18.0][Platform][BugFix] Guard forced tool choice with empty content#8400
Conversation
Backport the upstream forced-tool-choice fix to the v0.18.0 compatibility layer. Some reasoning parsers may consume the full model output and return content=None; normalize that to an empty argument string before the forced named tool-choice branches delegate back to upstream parsing. References: vllm-project/vllm#40147, vllm-project/vllm#40148 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a compatibility patch for the vLLM platform layer to address an issue where forced named tool choices fail when the reasoning parser returns no content. By normalizing 'None' content to an empty string in specific scenarios, the patch prevents internal assertion errors and ensures robust tool call generation. This change is isolated to the compatibility layer and maintains existing upstream behavior for all other cases. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a monkey-patch to normalize content=None to an empty string when a forced tool choice is active, preventing server-side failures when reasoning parsers consume all model output. The changes include the patch implementation, documentation updates, and new unit tests. Feedback highlights a Python version compatibility issue regarding the use of the | operator in isinstance() and notes that the PR title and summary need to be updated to comply with the repository's style guide.
Replace the PEP 604 union form inside isinstance() with a tuple of types so the compatibility patch remains valid on older supported Python versions. Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
) ### What this PR does / why we need it? Backports the forced tool-choice `content=None` handling from vLLM Ascend PR #8400 for the vLLM 0.19.1 patch layer. Related issue and PR: - vLLM issue: vllm-project/vllm#40147 - vLLM PR: vllm-project/vllm#40148 - vLLM Ascend PR: #8400 Per the latest PR #40148 discussion, this patch does not synthesize an empty function call or empty `{}` arguments. When forced tool choice sees `content=None`, it returns an empty tool-call list and preserves normal forced tool-call behavior when content is present. ### Does this PR introduce _any_ user-facing change? Yes. Forced tool-choice requests no longer assert when reasoning extraction leaves `content=None`; they return no tool calls for that empty-content result instead. ### How was this patch tested? - `ruff format --check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `ruff check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `python -m py_compile vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py` - `PYTHONPATH=../vllm:. pytest -q --confcutdir=tests/ut/patch/platform tests/ut/patch/platform/test_patch_tool_choice_none_content.py` Also ran `bash format.sh ci`; it passed the available hooks but could not complete `shellcheck` because `shellcheck` is not installed in this environment. - vLLM version: v0.19.1 - vLLM main: vllm-project/vllm@d886c26 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
…lm-project#8833) ### What this PR does / why we need it? Backports the forced tool-choice `content=None` handling from vLLM Ascend PR vllm-project#8400 for the vLLM 0.19.1 patch layer. Related issue and PR: - vLLM issue: vllm-project/vllm#40147 - vLLM PR: vllm-project/vllm#40148 - vLLM Ascend PR: vllm-project#8400 Per the latest PR #40148 discussion, this patch does not synthesize an empty function call or empty `{}` arguments. When forced tool choice sees `content=None`, it returns an empty tool-call list and preserves normal forced tool-call behavior when content is present. ### Does this PR introduce _any_ user-facing change? Yes. Forced tool-choice requests no longer assert when reasoning extraction leaves `content=None`; they return no tool calls for that empty-content result instead. ### How was this patch tested? - `ruff format --check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `ruff check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `python -m py_compile vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py` - `PYTHONPATH=../vllm:. pytest -q --confcutdir=tests/ut/patch/platform tests/ut/patch/platform/test_patch_tool_choice_none_content.py` Also ran `bash format.sh ci`; it passed the available hooks but could not complete `shellcheck` because `shellcheck` is not installed in this environment. - vLLM version: v0.19.1 - vLLM main: vllm-project/vllm@d886c26 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
…lm-project#8833) ### What this PR does / why we need it? Backports the forced tool-choice `content=None` handling from vLLM Ascend PR vllm-project#8400 for the vLLM 0.19.1 patch layer. Related issue and PR: - vLLM issue: vllm-project/vllm#40147 - vLLM PR: vllm-project/vllm#40148 - vLLM Ascend PR: vllm-project#8400 Per the latest PR #40148 discussion, this patch does not synthesize an empty function call or empty `{}` arguments. When forced tool choice sees `content=None`, it returns an empty tool-call list and preserves normal forced tool-call behavior when content is present. ### Does this PR introduce _any_ user-facing change? Yes. Forced tool-choice requests no longer assert when reasoning extraction leaves `content=None`; they return no tool calls for that empty-content result instead. ### How was this patch tested? - `ruff format --check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `ruff check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `python -m py_compile vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py` - `PYTHONPATH=../vllm:. pytest -q --confcutdir=tests/ut/patch/platform tests/ut/patch/platform/test_patch_tool_choice_none_content.py` Also ran `bash format.sh ci`; it passed the available hooks but could not complete `shellcheck` because `shellcheck` is not installed in this environment. - vLLM version: v0.19.1 - vLLM main: vllm-project/vllm@d886c26 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: PiratePai <416932041@qq.com>
…lm-project#8833) ### What this PR does / why we need it? Backports the forced tool-choice `content=None` handling from vLLM Ascend PR vllm-project#8400 for the vLLM 0.19.1 patch layer. Related issue and PR: - vLLM issue: vllm-project/vllm#40147 - vLLM PR: vllm-project/vllm#40148 - vLLM Ascend PR: vllm-project#8400 Per the latest PR #40148 discussion, this patch does not synthesize an empty function call or empty `{}` arguments. When forced tool choice sees `content=None`, it returns an empty tool-call list and preserves normal forced tool-call behavior when content is present. ### Does this PR introduce _any_ user-facing change? Yes. Forced tool-choice requests no longer assert when reasoning extraction leaves `content=None`; they return no tool calls for that empty-content result instead. ### How was this patch tested? - `ruff format --check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `ruff check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `python -m py_compile vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py` - `PYTHONPATH=../vllm:. pytest -q --confcutdir=tests/ut/patch/platform tests/ut/patch/platform/test_patch_tool_choice_none_content.py` Also ran `bash format.sh ci`; it passed the available hooks but could not complete `shellcheck` because `shellcheck` is not installed in this environment. - vLLM version: v0.19.1 - vLLM main: vllm-project/vllm@d886c26 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: yangzhe-2026 <yangzhe@isrc.iscas.ac.cn>
…lm-project#8833) ### What this PR does / why we need it? Backports the forced tool-choice `content=None` handling from vLLM Ascend PR vllm-project#8400 for the vLLM 0.19.1 patch layer. Related issue and PR: - vLLM issue: vllm-project/vllm#40147 - vLLM PR: vllm-project/vllm#40148 - vLLM Ascend PR: vllm-project#8400 Per the latest PR #40148 discussion, this patch does not synthesize an empty function call or empty `{}` arguments. When forced tool choice sees `content=None`, it returns an empty tool-call list and preserves normal forced tool-call behavior when content is present. ### Does this PR introduce _any_ user-facing change? Yes. Forced tool-choice requests no longer assert when reasoning extraction leaves `content=None`; they return no tool calls for that empty-content result instead. ### How was this patch tested? - `ruff format --check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `ruff check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `python -m py_compile vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py` - `PYTHONPATH=../vllm:. pytest -q --confcutdir=tests/ut/patch/platform tests/ut/patch/platform/test_patch_tool_choice_none_content.py` Also ran `bash format.sh ci`; it passed the available hooks but could not complete `shellcheck` because `shellcheck` is not installed in this environment. - vLLM version: v0.19.1 - vLLM main: vllm-project/vllm@d886c26 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: ZhuQi-seu <zhuqi12@huawei.com>
…lm-project#8833) ### What this PR does / why we need it? Backports the forced tool-choice `content=None` handling from vLLM Ascend PR vllm-project#8400 for the vLLM 0.19.1 patch layer. Related issue and PR: - vLLM issue: vllm-project/vllm#40147 - vLLM PR: vllm-project/vllm#40148 - vLLM Ascend PR: vllm-project#8400 Per the latest PR #40148 discussion, this patch does not synthesize an empty function call or empty `{}` arguments. When forced tool choice sees `content=None`, it returns an empty tool-call list and preserves normal forced tool-call behavior when content is present. ### Does this PR introduce _any_ user-facing change? Yes. Forced tool-choice requests no longer assert when reasoning extraction leaves `content=None`; they return no tool calls for that empty-content result instead. ### How was this patch tested? - `ruff format --check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `ruff check vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/__init__.py` - `python -m py_compile vllm_ascend/patch/platform/patch_tool_choice_none_content.py tests/ut/patch/platform/test_patch_tool_choice_none_content.py` - `PYTHONPATH=../vllm:. pytest -q --confcutdir=tests/ut/patch/platform tests/ut/patch/platform/test_patch_tool_choice_none_content.py` Also ran `bash format.sh ci`; it passed the available hooks but could not complete `shellcheck` because `shellcheck` is not installed in this environment. - vLLM version: v0.19.1 - vLLM main: vllm-project/vllm@d886c26 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: nanxing <1014662416@qq.com>
…pty content (vllm-project#8400) ### What this PR does / why we need it? This backports the forced-tool-choice `content=None` guard to the `releases/v0.18.0` compatibility layer. Upstream vLLM still has forced named tool-choice branches that assert `content is not None` after reasoning extraction. Some reasoning parsers can legally consume the full output and return `(reasoning, None)`, which makes the assert reachable and can surface as a server-side failure. This PR follows the same compatibility-patch pattern used by: - `7314bbe2` fix(platform): reimplement MiniMax usage accounting patch (vllm-project#7835) - `f83cb0e6` [Bugfix][Platform] Fix GLM47 tool-call finish backfill (vllm-project#7710) The patch is intentionally narrow: - normalize `content=None` to `""` only for forced named tool choice - patch both chat-completions and responses parser entry points - keep the rest of upstream behavior unchanged Upstream tracking: - issue: vllm-project/vllm#40147 - PR: vllm-project/vllm#40148 ### Does this PR introduce _any_ user-facing change? Yes. Forced named tool choice becomes robust when the reasoning parser returns no post-reasoning content, avoiding an internal assertion failure and emitting an empty-argument function call instead. ### How was this patch tested? Unit tests: ```bash pytest -sv tests/ut/patch/platform/test_patch_tool_choice_none_content.py \ tests/ut/patch/platform/test_patch_glm_tool_call_parser.py \ tests/ut/patch/platform/test_patch_minimax_usage_accounting.py ``` Result: 22 passed. --------- Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
What this PR does / why we need it?
This backports the forced-tool-choice
content=Noneguard to thereleases/v0.18.0compatibility layer.Upstream vLLM still has forced named tool-choice branches that assert
content is not Noneafter reasoning extraction. Some reasoning parsers can legally consume the full output and return(reasoning, None), which makes the assert reachable and can surface as a server-side failure.This PR follows the same compatibility-patch pattern used by:
7314bbe2fix(platform): reimplement MiniMax usage accounting patch (fix(platform): reimplement MiniMax usage accounting patch #7835)f83cb0e6[Bugfix][Platform] Fix GLM47 tool-call finish backfill ([Bugfix][Platform] Fix GLM47 tool-call finish backfill #7710)The patch is intentionally narrow:
content=Noneto""only for forced named tool choiceUpstream tracking:
Does this PR introduce any user-facing change?
Yes.
Forced named tool choice becomes robust when the reasoning parser returns no post-reasoning content, avoiding an internal assertion failure and emitting an empty-argument function call instead.
How was this patch tested?
Unit tests:
Result: 22 passed.