Skip to content

Fix mixed-image Online DPO server batches - #6968

Open
DaoyuanLi2816 wants to merge 2 commits into
huggingface:mainfrom
DaoyuanLi2816:fix/online-dpo-mixed-image-batch
Open

Fix mixed-image Online DPO server batches#6968
DaoyuanLi2816 wants to merge 2 commits into
huggingface:mainfrom
DaoyuanLi2816:fix/online-dpo-mixed-image-batch

Conversation

@DaoyuanLi2816

@DaoyuanLi2816 DaoyuanLi2816 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #6967.

Online DPO's vLLM server path sent mixed VLM batches through chat, but a text-only row still carried the unfilled image placeholder inserted for the batch. Passing no image for that row then raised a placeholder-count ValueError before generation.

This change removes only unfilled image placeholders from rows whose parallel image value is None. Rows with images are still populated normally, and image blocks that already contain an embedded image are preserved.

The regression test exercises _generate_vllm_server with one image row and one text-only row, verifies the first image is inlined, and verifies the second message contains only its text block.

This also follows up on the current, unresolved review finding in #6765: #6765 (comment)

Validation:

python -m pytest tests/experimental/test_online_dpo_trainer.py::test_generate_vllm_server_with_mixed_image_batch -q --no-cov
1 passed

pre-commit ruff-check, ruff-format, and doc-builder-style
passed

git diff --check
passed

Before submitting

AI writing disclosure

  • No AI usage: the PR was written entirely by a human.
  • AI-assisted: some parts were suggested or improved by AI, but the PR was written and reviewed by a human.
  • AI-generated: the PR was mostly or fully generated by an AI tool.

Who can review?

Anyone in the community is free to review the PR once the tests have passed.


Note

Low Risk
Narrow regression fix in the vLLM server multimodal path, covered by a new unit test; no auth, data, or API surface changes.

Overview
Fixes Online DPO vLLM server generation when a batch mixes vision and text-only rows. Text-only samples still had {"type": "image"} placeholders from batch formatting, which caused a placeholder-count ValueError before chat could run.

_generate_vllm_server now preprocesses prompts when images are present: for rows with image is None, it drops image content parts that lack an embedded "image" key, while rows with real images are unchanged and still get inlined via prepare_multimodal_messages.

Adds test_generate_vllm_server_with_mixed_image_batch, a unit test with mocks that checks one row keeps an inlined image and the other sends text only.

Reviewed by Cursor Bugbot for commit 70f0231. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor 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.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4066f7f. Configure here.

Comment thread trl/experimental/online_dpo/online_dpo_trainer.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Online DPO vLLM server fails on mixed image and text-only batches

1 participant