[v0.25.1rc][BugFix][Frontend] Align DeepSeek V4 system tool rendering - #14035
Conversation
Attach request-level tools to the first existing system message and only synthesize a system message when one is absent. Preserve caller-owned message dictionaries. 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 addresses a rendering mismatch in the DeepSeek V4 Python wrapper for vLLM v0.25.1. By aligning the tool attachment logic with the Rust renderer and official checkpoint references, the patch ensures consistent prompt formatting when system messages and tools are both provided. The changes improve reliability for tool-calling workflows while maintaining the integrity of user-provided message structures. Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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
|
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. Tip 💡 Consider Linking a Related Issue or RFCYour PR title contains the [BugFix] tag, indicating a bug fix or new feature. Linking a related issue or RFC in the PR description is strongly encouraged — it gives reviewers helpful context and speeds up the review. You can use any of these keywords:
🙏 Thanks for helping us keep the project well-organized! |
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][BugFix] Fix DeepSeek V4 tokenizer tool attachment to existing system messageSuggested PR Summary:
### What this PR does / why we need it?
This PR fixes an issue in the DeepSeek V4 tokenizer patch where tools were always inserted as a new system message at the beginning of the message list, even if a system message already existed. The updated logic now searches for an existing system message and attaches the tools to it if found; otherwise, it inserts a new system message at the beginning.
### Does this PR introduce _any_ user-facing change?
Yes, it ensures that tools are correctly attached to the existing system message instead of creating a duplicate system message, which improves compatibility with chat templates that expect a single system message.
### How was this patch tested?
Added unit tests in `tests/ut/patch/platform/test_deepseek_v4_thinking.py` to verify both cases (when a system message is already present and when it is missing).I have no further feedback to provide as the implementation is correct and well-tested.
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: lijiaqi139 <lijiaqi139@huawei.com>
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: lijiaqi139 <lijiaqi139@huawei.com>
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: lijiaqi139 <lijiaqi139@huawei.com>
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: lijiaqi139 <lijiaqi139@huawei.com> Signed-off-by: jiaqi-lee <15316070896@163.com>
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: lijiaqi139 <lijiaqi139@huawei.com> Signed-off-by: jiaqi-lee <15316070896@163.com>
…vllm-project#14035) ### What this PR does / why we need it? This is a follow-up to vllm-project#13519. The v0.25.1 release monkey patch inherited an upstream Python renderer mismatch reported in vllm-project/vllm#51829. For DeepSeek V4 requests containing both an existing system message and top-level tools, the Python wrapper always inserted a synthetic system message and rendered the tools before the caller's system content. This differs from both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference. This patch aligns the release Python path with those references: - attach request-level tools to a shallow copy of the first existing system message; - insert a synthetic system message only when no system message exists; and - leave caller-owned message dictionaries unchanged. The reasoning-effort normalization introduced by vllm-project#13519 is unchanged. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests with both a system message and top-level tools now render the system content and tool schemas in the same order as the Rust and checkpoint reference renderers. Requests without a system message keep the existing synthetic-system behavior. ### How was this patch tested? - Tested with vLLM `v0.25.1` at commit `752a3a504485790a2e8491cacbb35c137339ad34`. - `VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py` - Result: `26 passed`. - Coverage includes existing-system tools, missing-system tools, caller input immutability, and default/high/low effort behavior. - Tokenizer integration comparison against the checkpoint encoder for `system/no-system x low/high`: - all four rendered prompts matched exactly; - all four input-ID sequences matched exactly; and - caller messages remained unchanged in all four cases. - `ruff check` and `ruff format --check` on both changed files passed. - `git diff --check origin/releases/v0.25.1rc...HEAD` passed. vllm-project/vllm@752a3a5 - vLLM version: v0.25.1 - vLLM main: vllm-project/vllm@fe784ff Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Signed-off-by: lijiaqi139 <lijiaqi139@huawei.com> Signed-off-by: jiaqi-lee <15316070896@163.com>
…4624) ### What this PR does / why we need it? Ports the remaining DeepSeek V4 frontend fixes from the v0.25 release branch to `releases/v0.26.0rc`: - #14035: attach tools to an existing system message instead of inserting a second system message; - #14521: stream long schema-typed string tool arguments incrementally before the closing parameter tag. The v0.26 reasoning-effort and thinking-default changes are already covered by #13993 and #14074, so this PR does not duplicate them. For long string arguments, the patch remains release-scoped until the supported vLLM contains vllm-project/vllm#52865. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests preserve their original system prompt when tools are present, and long string tool arguments are emitted incrementally instead of being buffered until the parameter closes. ### How was this patch tested? Tested after rebasing onto the latest `releases/v0.26.0rc` with its paired vLLM source: ```bash PYTHONPATH=$VLLM_SOURCE pytest -q \ tests/ut/patch/platform/test_deepseek_v4_thinking.py \ tests/ut/patch/platform/test_deepseek_v4_tool_streaming.py # 45 passed ruff check tests/ut/patch/platform/test_deepseek_v4_thinking.py tests/ut/patch/platform/test_deepseek_v4_tool_streaming.py vllm_ascend/patch/__init__.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/platform/patch_deepseek_v4_thinking.py vllm_ascend/patch/platform/patch_deepseek_v4_tool_streaming.py # All checks passed! ruff format --check tests/ut/patch/platform/test_deepseek_v4_thinking.py tests/ut/patch/platform/test_deepseek_v4_tool_streaming.py vllm_ascend/patch/__init__.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/platform/patch_deepseek_v4_thinking.py vllm_ascend/patch/platform/patch_deepseek_v4_tool_streaming.py # 6 files already formatted git diff --check upstream/releases/v0.26.0rc...HEAD ``` - vLLM version: v0.26.0 - vLLM main: vllm-project/vllm@d02df74 Signed-off-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com> Co-authored-by: QwertyJack <7554089+QwertyJack@users.noreply.github.com>
- Fix version number: v0.23.0rc1 -> v0.25.1rc1 - Add missing space after comma - Fix PR link: vllm-project#14035 now points to correct PR Signed-off-by: alex7092 <15105576+alex7092@users.noreply.github.com> Signed-off-by: 刘星 <liuxing35@huawei.com>
…lm-project#14624) ### What this PR does / why we need it? Ports the remaining DeepSeek V4 frontend fixes from the v0.25 release branch to `releases/v0.26.0rc`: - vllm-project#14035: attach tools to an existing system message instead of inserting a second system message; - vllm-project#14521: stream long schema-typed string tool arguments incrementally before the closing parameter tag. The v0.26 reasoning-effort and thinking-default changes are already covered by vllm-project#13993 and vllm-project#14074, so this PR does not duplicate them. For long string arguments, the patch remains release-scoped until the supported vLLM contains vllm-project/vllm#52865. ### Does this PR introduce _any_ user-facing change? Yes. DeepSeek V4 requests preserve their original system prompt when tools are present, and long string tool arguments are emitted incrementally instead of being buffered until the parameter closes. ### How was this patch tested? Tested after rebasing onto the latest `releases/v0.26.0rc` with its paired vLLM source: ```bash PYTHONPATH=$VLLM_SOURCE pytest -q \ tests/ut/patch/platform/test_deepseek_v4_thinking.py \ tests/ut/patch/platform/test_deepseek_v4_tool_streaming.py # 45 passed ruff check tests/ut/patch/platform/test_deepseek_v4_thinking.py tests/ut/patch/platform/test_deepseek_v4_tool_streaming.py vllm_ascend/patch/__init__.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/platform/patch_deepseek_v4_thinking.py vllm_ascend/patch/platform/patch_deepseek_v4_tool_streaming.py # All checks passed! ruff format --check tests/ut/patch/platform/test_deepseek_v4_thinking.py tests/ut/patch/platform/test_deepseek_v4_tool_streaming.py vllm_ascend/patch/__init__.py vllm_ascend/patch/platform/__init__.py vllm_ascend/patch/platform/patch_deepseek_v4_thinking.py vllm_ascend/patch/platform/patch_deepseek_v4_tool_streaming.py # 6 files already formatted git diff --check upstream/releases/v0.26.0rc...HEAD ``` - vLLM version: v0.26.0 - vLLM main: vllm-project/vllm@d02df74 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 is a follow-up to #13519. The v0.25.1 release monkey patch inherited an
upstream Python renderer mismatch reported in
vllm-project/vllm#51829.
For DeepSeek V4 requests containing both an existing system message and
top-level tools, the Python wrapper always inserted a synthetic system message
and rendered the tools before the caller's system content. This differs from
both vLLM's Rust renderer and the DeepSeek-V4-Flash-0731 checkpoint reference.
This patch aligns the release Python path with those references:
message;
The reasoning-effort normalization introduced by #13519 is unchanged.
Does this PR introduce any user-facing change?
Yes. DeepSeek V4 requests with both a system message and top-level tools now
render the system content and tool schemas in the same order as the Rust and
checkpoint reference renderers. Requests without a system message keep the
existing synthetic-system behavior.
How was this patch tested?
v0.25.1at commit752a3a504485790a2e8491cacbb35c137339ad34.VLLM_VERSION=0.25.1 python -m pytest -q tests/ut/patch/platform/test_deepseek_v4_thinking.py26 passed.immutability, and default/high/low effort behavior.
system/no-system x low/high:ruff checkandruff format --checkon both changed files passed.git diff --check origin/releases/v0.25.1rc...HEADpassed.vllm-project/vllm@752a3a5