Skip to content

[Bugfix][Frontend] Align DeepSeek V4.1 images with rendered content - #60792

Merged
chaunceyjiang merged 6 commits into
vllm-project:mainfrom
xijiaat:codex/dsv41-media-order
Oct 19, 2026
Merged

chaunceyjiang merged 6 commits into
vllm-project:mainfrom
xijiaat:codex/dsv41-media-order

Conversation

@xijiaat

@xijiaat xijiaat commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Fix Python DeepSeek V4.1 rendering when tool results reorder image placeholders or thinking mode drops historical developer content. Carry source image indices through the native encoder and apply the resulting order to both image data and UUIDs.

This follows the source-order approach of #56366, which fixed the Rust frontend only.

Claims

  • Out-of-order tool results keep their own images after sorting into tool-call order.
  • Images belonging to dropped developer content no longer reach multimodal prompt expansion without a placeholder.

Validation

The following results are from the tests run on 2026-10-09, not a new rerun. The four production files tested then are unchanged at the current PR head.

Two regression cases, each exercised through sync and async rendering; the existing V4/V4.1 tokenizer suites are also included.

CPU tests Result
Baseline source checkout + regression tests 4 failed, 75 passed
Patched implementation 79 passed

Run in a CPU-only container against the source checkout, with model/processor metadata stubbed and real message parsing, image tracking and native encoding:

.venv/bin/python -m pytest --confcutdir=/work/checks/tests/tokenizers_ -o addopts= \
  /work/checks/tests/tokenizers_/test_deepseek_v41.py \
  /work/checks/tests/tokenizers_/test_deepseek_v4.py -v
Actual CPU output: baseline failures and patched result

Verbatim excerpts from regression-baseline.log:

FAILED ../checks/tests/tokenizers_/test_deepseek_v41.py::test_tool_images_follow_rendered_order[sync]
FAILED ../checks/tests/tokenizers_/test_deepseek_v41.py::test_tool_images_follow_rendered_order[async]
FAILED ../checks/tests/tokenizers_/test_deepseek_v41.py::test_dropped_developer_only_image_leaves_text_prompt[sync]
FAILED ../checks/tests/tokenizers_/test_deepseek_v41.py::test_dropped_developer_only_image_leaves_text_prompt[async]
================== 4 failed, 75 passed, 15 warnings in 15.77s ==================

From regression-patched.log:

======================= 79 passed, 15 warnings in 15.66s =======================

Full pre-commit checks passed on all five changed files, including mypy and test CI tethering.

GPU validation passed on the real DeepSeek-V4.1-Flash checkpoint (DeepseekV41ForCausalLM), using 8x H20-3e, TP=8, eager execution and temperature=0. The GPU runtime was vLLM 0.31.0 / PyTorch 2.13.0+cu130 with this PR's four production files mounted unchanged; their SHA-256 hashes were verified against commit f5622b6. This was not a full build of main.

The same loaded model compared the original media handling (the base renderer without an image-order accumulator) and the patched V4.1 renderer, using generated 560x560 red and blue images:

Full-model case Observed result
Ordered tool results, baseline A=red, B=blue
Reversed tool results, baseline A=blue, B=red (wrong image assignment)
Reversed tool results, patched A=red, B=blue; output token IDs match the ordered baseline
Dropped historical developer image, baseline AssertionError: Failed to apply prompt replacement for mm_items['image'][0]
Dropped historical developer image, patched Successful generation, final answer OK
Actual full-model GPU output, including generated text and token IDs

The in-container validation command was .venv/bin/python /work/gpu_validate.py. These are verbatim result lines from gpu.log, not reconstructed from the summary table:

torch 2.13.0+cu130 vllm 0.31.0 gpus 8
GPU_RESULT {"case": "ordered_baseline", "text": "A=red, B=blue", "token_ids": [35, 31, 1157, 14, 406, 31, 27602, 1]}
GPU_RESULT {"case": "reversed_baseline", "text": "A=blue, B=red", "token_ids": [35, 31, 27602, 14, 406, 31, 1157, 1]}
GPU_RESULT {"case": "reversed_patched", "text": "A=red, B=blue", "token_ids": [35, 31, 1157, 14, 406, 31, 27602, 1]}
GPU_RESULT {"case": "developer_baseline", "error_type": "AssertionError", "error": "Failed to apply prompt replacement for mm_items['image'][0]"}
GPU_RESULT {"case": "developer_patched", "text": "We need answer user says \"Reply OK.\" So just reply OK. Need comply. final maybe \"OK\".</think>OK", "token_ids": [2581, 1309, 3287, 3967, 4230, 582, 19905, 20370, 2148, 3016, 1438, 20430, 20370, 16, 21017, 27233, 16, 4087, 10796, 582, 11932, 3305, 128822, 11932, 1]}
GPU_VALIDATION_PASSED

The log subsequently contains shutdown warnings about force-killing the remaining EngineCore and one leaked shared-memory object. The pass marker covers the inference assertions above, not a warning-free shutdown.

These checks verify the two rendering regressions, not performance or general model accuracy.

Details

The encoder already decides which content survives and in what order it renders. Image indices now follow those same merge/sort/drop operations; the renderer does not duplicate those policies or scan text for image markers. The V4.1 renderer applies the order to media and UUIDs together. Existing prompt fixtures and rendering parity checks retain the encoded text.

Duplicate checks covered open PRs, the related Rust PR's changed files and discussion, and V4.1 frontend searches. #56366 changes Rust files only; #60522 only moves Python imports and does not fix this behavior. No matching Python fix was found.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).
  • AI assistance was used during the creation of this PR.
  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.
  • Testing and Validation: CPU reproduction, 79 regression tests, and full-model GPU before/after checks passed.
  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.
  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: ahmed xijiaat <52128022+xijiaat@users.noreply.github.com>
@mergify mergify Bot added deepseek Related to DeepSeek models DSv4.1 Related to DeepSeek-V4.1 models bug Something isn't working labels Oct 9, 2026
@xijiaat
xijiaat marked this pull request as ready for review October 9, 2026 07:36

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@DarkLight1337
DarkLight1337 requested review from Isotr0py and chaunceyjiang and a balanced review from Copilot October 9, 2026 07:45
@chaunceyjiang chaunceyjiang self-assigned this Oct 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open findings
What changed in this PR

Fixes DeepSeek V4.1 multimodal rendering so image attachments remain aligned with the final encoded prompt after tool-result reordering and thinking/developer-content dropping.

Changes:

  • Track per-image source indices during message normalization and propagate them through V4.1 encoding/merging.
  • Accumulate the final rendered image order during encoding and apply it to both multi_modal_data["image"] and multi_modal_uuids["image"].
  • Register a dedicated DeepseekV41Renderer and add regression tests for tool-result reorder + dropped developer-only images.
File Description
vllm/​tokenizers/​deepseek_v41_encoding.py Propagates and collects _image_indices during preprocessing/rendering to compute a final image_order.
vllm/​tokenizers/​deepseek_v41.py Assigns source image indices during normalization and threads image_order through apply_chat_template.
vllm/​renderers/​registry.py Routes deepseek_v41 to the new DeepseekV41Renderer.
vllm/​renderers/​deepseek_v4.py Adds prompt-building step that reorders/prunes image mm payloads based on image_order; introduces DeepseekV41Renderer.
tests/​tokenizers_/​test_deepseek_v41.py Adds sync+async regression coverage for tool image reordering and dropped developer-only images.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread vllm/renderers/deepseek_v4.py Resolved
Comment thread vllm/renderers/deepseek_v4.py Resolved
Comment thread vllm/renderers/deepseek_v4.py Resolved
@xijiaat

xijiaat commented Oct 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi @chaunceyjiang, when you have a chance, would you mind taking a look at this fix? The PR description includes the reproduction and CPU/H20 validation results, and I've replied to the automated review comments with the relevant source references.

Please let me know if any additional changes or tests would be helpful. I understand reviews take time and appreciate your help. Thank you!

@lesj0610 lesj0610 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Checked the image ordering and async rendering paths. Image data and UUIDs follow the rendered order, and the async wrapper uses the overridden _apply_chat_template.

@xijiaat

xijiaat commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

LGTM. The sync and async tests pass for reordered tool images and dropped developer images. Image data and UUIDs follow the same rendered order; V4 keeps its existing ordering.

thanks for the reveiw!!!

@chaunceyjiang chaunceyjiang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you paste some actual test results into the PR description?

@xijiaat

xijiaat commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @chaunceyjiang. I've added the actual CPU and full-model H20 output to the PR description: the four baseline regression failures, the patched 79 passed result, and all five GPU case results with generated text and token IDs.

The reversed-image case changes from A=blue, B=red to the correct A=red, B=blue; the dropped-developer-image case changes from an assertion failure to a successful OK response. These are the October 9 results, with the tested runtime and scope stated explicitly; the four production files are unchanged at the current PR head. Thank you for reviewing!

robertgshaw2-redhat and others added 2 commits October 17, 2026 18:36
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Robert Shaw <robertgshaw2@gmail.com>
@robertgshaw2-redhat

Copy link
Copy Markdown
Collaborator

Merged main and fixed a test import.

Before: on current main, tests/tokenizers_/test_deepseek_v41.py failed to load (ModuleNotFoundError: vllm.entrypoints.chat_utils), because chat_utils moved to vllm/renderers/. That took down every V4.1 tokenizer test, not just the new ones.

After: the import is now from vllm.renderers.chat_utils import BaseMultiModalItemTracker. pytest tests/tokenizers_/test_deepseek_v41.py tests/tokenizers_/test_deepseek_v4.py: 79 passed.

No other changes.

@robertgshaw2-redhat robertgshaw2-redhat added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 17, 2026
@github-actions

Copy link
Copy Markdown

✅ @xijiaat, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@robertgshaw2-redhat

Copy link
Copy Markdown
Collaborator

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #95629 for commit d8fd027c7f27.

@robertgshaw2-redhat

Copy link
Copy Markdown
Collaborator

What this fixes: the assistant calls two tools, red then blue, and the results come back in the other order, each with an image.

[{"role": "assistant", "tool_calls": [call("red"), call("blue")]},
 {"role": "tool", "tool_call_id": "blue", "content": [text("blue"), image("blue")]},
 {"role": "tool", "tool_call_id": "red",  "content": [text("red"),  image("red")]}]

The encoder sorts tool results into call order, so the prompt reads <tool_result>red <image> then <tool_result>blue <image>.

Broken (main): images are passed in message order, [blue, red], so each result gets the other result's image.

Fixed (this PR): images follow the prompt order, [red, blue].

Verified with test_tool_images_follow_rendered_order: it fails on main (['blue', 'red'] != ['red', 'blue']) and passes here.

@robertgshaw2-redhat

Copy link
Copy Markdown
Collaborator

The official DeepSeek encoder had this bug too and fixed it on 2026-10-16 (314113f, also in the 1017 release). It now sorts tool results before collecting images, so on the case above it returns ['red.png', 'blue.png'], the same as this PR.

The approach differs: the reference encoder collects the images itself and returns them in prompt order, while vLLM collects them earlier, so this PR records each image's index and reorders afterwards.

Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: ahmed xijiaat <52128022+xijiaat@users.noreply.github.com>
@xijiaat

xijiaat commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #95647 for commit 64b90c287529.

@xijiaat

xijiaat commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @robertgshaw2-redhat for merging main, fixing the test import, validating the regression, and kicking off CI. The explanation and comparison with the official encoder were really helpful. I appreciate your time and help getting this fix reviewed!

@chaunceyjiang chaunceyjiang left a comment

Copy link
Copy Markdown
Contributor

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 3c8245b into vllm-project:main Oct 19, 2026
123 checks passed
bantianyue pushed a commit to bantianyue/vllm that referenced this pull request Oct 28, 2026
…llm-project#60792)

Signed-off-by: ahmed xijiaat <52128022+xijiaat@users.noreply.github.com>
Signed-off-by: Robert Shaw <robertgshaw2@gmail.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Robert Shaw <robertgshaw2@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.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 deepseek Related to DeepSeek models DSv4.1 Related to DeepSeek-V4.1 models 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.

5 participants