Skip to content

[Bugfix][Multimodal] Reject prompts with placeholder tokens outside the recorded placeholders - #57781

Closed
twu3202 wants to merge 1 commit into
vllm-project:mainfrom
twu3202:fix-57740-placeholder-in-user-text
Closed

twu3202 wants to merge 1 commit into
vllm-project:mainfrom
twu3202:fix-57740-placeholder-in-user-text

Conversation

@twu3202

@twu3202 twu3202 commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Prompt updates are matched in prompt order and the first match wins. A placeholder token that the caller wrote themselves is therefore matched before the one the chat template rendered: the item binds to the caller's token, and the rendered placeholder is left holding an unexpanded token. Nothing catches this today, because every existing check is on counts and the item is still bound exactly once. The model answers about the wrong part of the prompt and nobody is told.

Reproduced on the Qwen2.5-VL-3B chat template with one 448×448 image (256 pad tokens), CPU only, no weights loaded:

Message order Where the image placeholder is recorded The rendered <|vision_start|>…<|vision_end|> slot Outcome
[image, text containing a literal <|image_pad|>] offset 15, 256 tokens, inside the wrapper expanded correct
[text containing a literal <|image_pad|>, image], before this change offset 20, 256 tokens, in the middle of the sentence keeps one unexpanded pad (position 291) silently mis-bound, request succeeds
same, after this change — — VLLMValidationError: 257 <|image_pad|> tokens in the prompt, 256 accounted for by the items

_validate_mm_placeholders now also checks that the prompt holds no more occurrences of a replacement's target token than the recorded placeholders account for, raising VLLMValidationError when it does. The new arguments are keyword-only with defaults, so existing callers and any out-of-tree override keep working and simply skip the check. Nothing about what gets matched or recorded changes, so placeholder ranges and M-RoPE are untouched.

This changes behaviour for one case that used to be silently accepted: a prompt that merely quotes a covered model's placeholder token — pasting a log line, quoting the docs, echoing OCR output — now gets a clear validation error instead of an image bound to the quoted text.

It also fails closed on a prompt that is re-submitted already expanded. Today that double-expands silently (#57796 makes the fully-expanded case a no-op; a prompt that mixes expanded and unexpanded items still double-expands, and with this change it raises instead).

Coverage:

Reaches the check Does not, and why
Every model that reaches BaseMultiModalProcessor._maybe_apply_prompt_updates and declares a single-token PromptReplacement target whose replacement re-emits that target — about 55 implementations in tree (Qwen2-VL / 2.5-VL / 3-VL, the LLaVA family, Idefics3 / SmolVLM, Gemma3 / 3n / 4, Mistral3, Kimi, the Qwen Omni thinkers, …) LegacyMultiModalProcessor (the Transformers backend below transformers 5.15)
Raw prompts passed to LLM.generate(prompt_token_ids=...) voxtral_realtime, qwen3_asr_realtime (override _maybe_apply_prompt_updates without validating)
The Transformers backend on the OffsetsMultiModalProcessor path (transformers ≥ 5.15) terratorch (overrides apply)
Qwen Omni with use_audio_in_video (spans are de-duplicated first, see below) Pixtral, Voxtral (target=[])
Multi-token targets: glm4v (<|begin_of_image|><|endoftext|><|end_of_image|>) and moondream3 (<|endoftext|><image>), where the template repeats one of those tokens once per turn
Replacements that swap the target for a different token: phi3v, molmo2, ernie45_vl, isaac, kanana_v — nothing is conserved, so nothing is counted

Two details in the counting. It is over all modalities at once, because one modality's target can sit inside another modality's placeholder: llava_onevision2 and glm4_1v build their video replacements out of image pads. And spans are de-duplicated first, because under use_audio_in_video the Qwen Omni models record an audio and a video placeholder over the same token list, which would otherwise be counted twice.

Also checked and found to balance: gemma3's full_image_sequence (re-emits <start_of_image>, so counts come out at num_crops + 1), every get_dummy_text implementation (one placeholder per item, so profiling is unaffected), Kimi-Audio (one <|im_kimia_text_blank|> per audio on every in-tree path), and the encoder-decoder models whose target is [0] (whisper, cohere_asr, nemotron_parse — the encoder prompt is exactly [0]). list.count over a 100k-token prompt is about 0.3 ms per target, and no model in tree has more than three single-token targets.

#57740 is one instance of this, on Qwen2.5-VL. #57763 takes the other approach for that model specifically, matching the full <|vision_start|><|image_pad|><|vision_end|> wrapper so that quoting the bare token keeps working there; the two are orthogonal, and if #57763 lands then Qwen2.5-VL simply drops out of this check because its target becomes three tokens. This is the generic backstop for every other single-token-target model and for raw prompts. It follows #53999, which turned the same class of overcount into a clear error in the interleaved chat path.

Tests:

Scope Result Failing only with this change
New: tests/models/multimodal/processing/test_placeholder_overcount.py (Qwen2.5-VL, Qwen3-VL, LLaVA-1.5: attack prompt raises; control lands on the placeholder and, where the model has them, between <|vision_start|> and <|vision_end|>, asserted on token ids) 3 passed —
New: 11 model-free cases in tests/multimodal/test_processing.py (exact count, several items, cross-modality counting, the shared span, the overcount, non-target tokens in the replacement, insertions, PromptIndex targets, dropped targets, multi-token targets, and one that feeds the real applier's output back into the check) 11 passed —
tests/models/multimodal/processing/ minus test_common.py and test_tensor_schema.py 548 passed, 208 failed 0 — the 208 are identical to the clean tree (missing open_clip / av, gated repos, a sentencepiece error)
test_common.py -k for the touched families, Kimi-Audio, glm-4v, moondream3, Nemotron-Nano-VL and GLM-4.1V 36 passed —
tests/multimodal/ minus media/test_audio.py 572 passed, 11 failed 0 — same 11 as the clean tree

Developed with AI assistance (Claude); I reviewed every changed line and ran the tests above.

…he recorded placeholders

Prompt updates are matched in prompt order and the first match wins, so a
placeholder token the caller wrote themselves is matched before the one the
chat template rendered. The item binds to the caller's token and the
rendered placeholder is left holding an unexpanded token. Nothing catches
it, because every existing check is on counts and the item is still bound
exactly once, so the model simply answers about the wrong part of the
prompt.

`_validate_mm_placeholders` now also checks that the prompt holds no more
occurrences of a replacement's target token than the recorded placeholders
account for. The new arguments are keyword-only with defaults, so existing
callers and overrides keep working and simply skip the check.

Counting is over all modalities at once, because one modality's target can
sit inside another modality's placeholder: `llava_onevision2` and `glm4_1v`
build their video replacements out of image pads. Spans are de-duplicated
first, since under `use_audio_in_video` the Qwen Omni models record an
audio and a video placeholder over the same tokens.

Three kinds of update are left alone. Insertions keep their target instead
of consuming it. Multi-token targets are not something a caller reproduces
by accident, and the ones in tree share a token with the surrounding
template: `glm4v` targets `<|begin_of_image|><|endoftext|><|end_of_image|>`
and `moondream3` targets `<|endoftext|><image>`. And a target the
replacement does not re-emit is not conserved at all, as in `phi3v` and
`molmo2`.

vllm-project#57740 is one instance of this, on Qwen2.5-VL. Follows
vllm-project#53999, which turned the same class of overcount into a clear error in the
interleaved chat path.

Signed-off-by: Tianyao Wu <rayroy31@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

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

@mergify mergify Bot added multi-modality Related to multi-modality (#4194) qwen Related to Qwen models bug Something isn't working labels Sep 20, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run for upstream CI or /amd-ci run for AMD CI only whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use the corresponding /ci run, /ci retry, and /ci cancel commands, or their /amd-ci variants. New commits do not start upstream CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@twu3202

twu3202 commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

Closing, per the discussion in #57740: the extra count is measurable on long prompts, and the case it guards against, a pad token typed as plain text, isn't worth that cost on every multimodal request.

@twu3202 twu3202 closed this Sep 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working multi-modality Related to multi-modality (#4194) qwen Related to Qwen models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant