fix(openai): tolerate empty content in forced tool choice#40148
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the OpenAI serving engine and abstract parser to handle cases where a named tool choice is provided but the content is null. Specifically, it replaces assertions with a default empty string for the content variable, preventing potential runtime errors during forced function calls. Additionally, a new test suite has been added to verify this behavior for both Chat Completion and Responses APIs. I have no feedback to provide.
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>
|
Is this PR still needed? cc @JaredforReal |
…pty content (#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 (#7835) - `f83cb0e6` [Bugfix][Platform] Fix GLM47 tool-call finish backfill (#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>
|
This pull request has merge conflicts that must be resolved before it can be |
|
Hi @QwertyJack Please fix the conflicts. |
2f88d21 to
5dc783e
Compare
|
@chaunceyjiang PTAL |
| ): | ||
| # Named function with standard JSON-based parsing | ||
| assert content is not None | ||
| content = content or "" |
There was a problem hiding this comment.
I think that when content is None, arguments should be "{}"—"" is not a standard OpenAI-defined semantic representation.
|
I feel that making modifications here is not very elegant. Perhaps it would be better to modify the corresponding parser. |
5dc783e to
1184780
Compare
Normalize None content to an empty argument string when forced tool choice runs after reasoning extraction. This avoids AssertionError in both chat-completions and responses parsing paths and adds regression coverage for the named tool-choice case. Fixes vllm-project#40147 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
1554a8b to
1147f21
Compare
|
Correction to my previous note: the latest patch follows your inline suggestion. When forced tool choice sees DeepSeek parser remains unchanged, and the branch is rebased. @chaunceyjiang PTAL. |
) ### 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>
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
chaunceyjiang
left a comment
There was a problem hiding this comment.
LGTM.
@QwertyJack I added an e2e test based on your work.
…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>
…ct#40148) Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: chaunceyjiang <chaunceyjiang@gmail.com> Signed-off-by: Ifta Khairul Alam Adil <ikaadil007@gmail.com>
…ct#40148) Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: chaunceyjiang <chaunceyjiang@gmail.com> Signed-off-by: Libin Tang <libin.tang@intel.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>
…ct#40148) Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: chaunceyjiang <chaunceyjiang@gmail.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>
…ct#40148) Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: chaunceyjiang <chaunceyjiang@gmail.com>
…ct#40148) Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: chaunceyjiang <chaunceyjiang@gmail.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>
…ct#40148) Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: chaunceyjiang <chaunceyjiang@gmail.com> Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Summary
This fixes a forced-tool-choice crash when reasoning extraction returns
content=None.Some reasoning parsers, including the
glm45/DeepSeekV3ReasoningWithThinkingParserpath, may consume the full model output and return(reasoning, None). The forced tool-choice branches in both chat-completions and responses parsing previously asserted thatcontent is not None, which made this a server-side failure instead of a valid function call with empty arguments.Changes
content=Noneto""inOpenAIServing._parse_tool_calls_from_content(...)content=Noneto""inDelegatingParser._parse_tool_calls(...)tool_choicewithcontent=Nonetool_choicewithcontent=NoneFixes #40147.
Testing