Skip to content

[Bugfix][Multimodal] Do not expand prompt updates that are already applied - #70

Open
aoshen02 wants to merge 624 commits into
mainfrom
mm-idempotent-prompt-updates
Open

aoshen02 wants to merge 624 commits into
mainfrom
mm-idempotent-prompt-updates

Conversation

@aoshen02

@aoshen02 aoshen02 commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Purpose

BaseMultiModalProcessor._apply_prompt_updates assumes the prompt it receives is in the pre-update form (one placeholder token per item) and never checks that assumption. When the prompt is already expanded, the compact target still matches the first token of the expanded run, so a second copy of the replacement is spliced in — silently.

Reproduced on Qwen3-VL-2B-Instruct with one 224×224 image, calling the processor directly (no server, no GPU):

compact prompt      : 11 tokens,  <|image_pad|> run = [1]
after apply #1      : 74 tokens,  run = [64]   placeholders = offset=4, length=64   ← correct
feed the 74 back in :
after apply #2      : 137 tokens, run = [127]  placeholders = offset=4, length=64   ← wrong

The prompt grows by 63 tokens and the placeholder range is not updated to match. The 64 image features are written over the first 64 positions of a 127-token run; the remaining 63 positions keep the <|image_pad|> row of the embedding table, which is never trained. The request returns 200 with no warning.

This is reachable from every entry point that pairs a token sequence with media, not just one endpoint:

For context, I checked how other engines behave when handed an already-expanded prompt plus media: TensorRT-LLM raises ValueError, LMDeploy raises assert/IndexError, SGLang errors out or falls back with a warning. vLLM is the one that corrupts the prompt and returns success.

Changes

A new _find_applied_updates, next to the _find_mm_placeholders it delegates to, returns the placeholders only when every item's post-update content is already present, and None otherwise. _apply_prompt_updates returns the prompt untouched on a hit and is unchanged on a miss:

applied = self._find_applied_updates(token_ids, mm_prompt_updates)
if applied is not None:
    return token_ids, applied

Two properties worth checking, because they are what makes this safe to put in a function shared by all models:

1. The change is strictly a skip. Content matching tells you what the prompt contains, not whether it has been processed: with a replacement of [1] → [1, 1], the prompt [1, 1] is both "two untouched targets" and "one applied content", and no amount of local inspection separates them. So an incomplete match is treated as no evidence at all and falls through to the normal path. Nothing that is updated today stops being updated, and nothing that succeeds today starts failing. test_apply_prompt_updates_expands_targets_aliasing_one_content pins exactly that [1, 1] case.

2. Detection does not assume a content is a run of one repeated token. A content like [1, 2] expands a prompt [1] to [1, 2], and the check has to recognise that on the way back in. Pinned by test_apply_prompt_updates_is_idempotent_for_mixed_content. Relatedly, a content of length 1 is identical before and after the update, so it is excluded from the evidence entirely (test_apply_prompt_updates_still_expands_single_token_items).

Known limitation, unchanged by this PR: a prompt that mixes expanded and unexpanded items is still mis-expanded, because it lands in the ambiguous case above. Fixing it needs either run-boundary-aware matching or an explicit caller-side contract, and both are out of scope here. vllm-project#57781 is the complementary backstop: with both applied, a compact prompt expands, a fully expanded one is a no-op, and a mixed one raises. I verified that combination end to end (results below). Callers that need mixed input normalise on their side today (OpenRLHF's dedup_media_tokens, verl's qwen2_5_vl_dedup_image_tokens, TRL vllm-project#6718/vllm-project#6719, NeMo-RL vllm-project#4182 all collapse expanded runs back to compact before calling vLLM).

Cost

The scan is preceded by a cheap necessary condition: a content cannot be a slice of the prompt unless the prompt holds its most repeated token at least as many times as the content does. This is a lower bound for any content shape, so it can never reject a prompt that really is expanded — at worst it fails to skip and the scan runs anyway. It costs one list.count() per distinct probe, which keeps the common (unexpanded) path off the scan entirely:

prompt len with the guard scan unconditionally saved
203 12.0 µs 34.5 µs 3×
8003 98.5 µs 742.0 µs 8×
32003 218.9 µs 3378.8 µs 15×

Per _apply_prompt_updates call on Qwen3-VL-2B-Instruct, against the rest of the work:

images prompt len form added code rest of apply full apply()
1 223 compact 3.9 14.8 5.0 ms
1 8803 compact 82.6 50.3 4.1 ms
1 33003 compact 276.0 166.9 4.7 ms
8 8824 compact 79.5 123.2 88.3 ms
1 8998 expanded 79.3 skipped 4.1 ms
8 10384 expanded 124.9 skipped 88.3 ms

Against the full multimodal apply() (dominated by image preprocessing) that is ~2% on a typical 8.8k prompt. Items of the same modality share a probe token, so the guard does not grow with item count. In the already-expanded case the apply is skipped entirely.

I surveyed the 83 in-tree _get_prompt_updates implementations for contents where the guard degenerates — i.e. where the most repeated token is an ordinary token with a low count, so the condition always holds and the scan always runs. All of them are video/audio paths where frame separators or timestamp text dominate: qwen3_vl.py get_video_repl under EVS pruning, nano_nemotron_vl's per-frame prose separators, llava_onevision2's codec video runs, and moss_audio's time markers. In each the scan is a few ms against an apply() dominated by media decoding, and correctness is unaffected either way.

Why this is not a duplicate

Searched open PRs/issues for _apply_prompt_updates, find_mm_placeholders, prompt_token_ids_are_expanded, already expanded placeholder, and PRs referencing vllm-project#51472. Nothing overlaps. Adjacent but distinct: vllm-project#57781 is the converse direction (a placeholder token the caller wrote themselves stealing an item's position) and composes with this — see above. vllm-project#53169 (WIP) hits the same hazard on the features path and works around it by running the processor over a synthetic placeholder-only prompt — its workaround becomes unnecessary once this lands, but it does not fix the processor. vllm-project#57740 / vllm-project#57763 are instances of the vllm-project#57781 direction on Qwen2.5-VL.

Test Plan

Test Result

.venv/bin/python -m pytest tests/multimodal/test_processing.py -q
# 90 passed, 21 failed

The 21 failures are all llava-hf/llava-v1.6-mistral-7b-hf config downloads hitting a PermissionError in this machine's HF cache; they are unrelated to the change and reproduce on a clean tree. The full tests/multimodal/ failure set is identical to the clean-tree baseline.

Real-processor check, two images expanding to 64 and 196 tokens:

1. compact -> expanded   : 12 -> 270 tokens, ranges [offset=4 len=64, offset=72 len=196]
2. expanded -> unchanged : PASS (270 tokens, identical ranges)
3. mixed (1st expanded)  : 75 -> 333 tokens, no error   (unchanged from main)
4. mixed (2nd expanded)  : 207 -> 465 tokens, no error  (unchanged from main)

With vllm-project#57781 applied on top (it patches cleanly, no conflicts; both PRs' unit tests pass together, 37 passed):

1. compact -> expanded   : 12 -> 270 tokens
2. expanded -> unchanged : PASS (270 tokens)
3. mixed (1st expanded)  : VLLMValidationError - 323 <|image_pad|> tokens, 260 accounted for
4. mixed (2nd expanded)  : VLLMValidationError - 455 <|image_pad|> tokens, 260 accounted for

Model evaluation is not applicable to the fixed path: today it produces a corrupted prompt, so there is no meaningful "before" quality number. Every other path is byte-identical.

Developed with AI assistance (Claude Code). The human submitter has reviewed every changed line.

🤖 Generated with Claude Code

@aoshen02
aoshen02 force-pushed the mm-idempotent-prompt-updates branch 7 times, most recently from da66492 to 7698993 Compare September 21, 2026 02:27
@aoshen02
aoshen02 force-pushed the mm-idempotent-prompt-updates branch 2 times, most recently from 67cd971 to 899c47b Compare October 6, 2026 02:05
jperezdealgaba and others added 21 commits October 13, 2026 10:17
…ct#61226)

Signed-off-by: juanpas97 <juan.perezalgaba@gmail.com>
Signed-off-by: Juan Pérez de Algaba <jperezde@redhat.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
… in backend validation (vllm-project#58862)

Signed-off-by: Samay <samay.piduru@gmail.com>
Co-authored-by: Artem Perevedentsev <aperevedents@nvidia.com>
…tches from Rust frontend (vllm-project#60979)

Signed-off-by: zh98 <workallday.zhh@gmail.com>
Co-authored-by: Isotr0py <Isotr0py@outlook.com>
…amba2 n_groups (vllm-project#60704)

Signed-off-by: Vadim Gimpelson <vadim.gimpelson@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
…rammars (vllm-project#60749)

Signed-off-by: Vadim Gimpelson <vadim.gimpelson@gmail.com>
Signed-off-by: Artem Perevedentsev <aperevedents@nvidia.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Artem Perevedentsev <aperevedents@nvidia.com>
…60468)

Signed-off-by: Simon Veitner <sveitner@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Roberto L. Castro <38211239+LopezCastroRoberto@users.noreply.github.com>
…roject#61401)

Signed-off-by: Shaun Kotek <skotek@nvidia.com>
Co-authored-by: Shaun Kotek - Nvidia <skotek@nvidia.com>
…5866)

Signed-off-by: Simon Veitner <sveitner@redhat.com>
Co-authored-by: Lucas Wilkinson <lwilkins@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Lucas Wilkinson <LucasWilkinson@users.noreply.github.com>
Co-authored-by: Matthew Bonanni <mbonanni@redhat.com>
…dividing per element (vllm-project#61343)

Signed-off-by: Thien Tran <gau.nernst@yahoo.com.sg>
Signed-off-by: Dakai An <dakaian108@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: zjy0516 <riverclouds.zhu@qq.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Chris Leonard <chleonar@redhat.com>
Signed-off-by: Shengqi Chen <harry-chen@outlook.com>
Co-authored-by: Shengqi Chen <harry-chen@outlook.com>
…instead of ROCM_ATTN and fixed minor bug in Gemma3 Multimodal code (vllm-project#46754)

Signed-off-by: GodRishUniverse <rishabh.agarwal@amd.com>
Signed-off-by: Rishabh Agarwal <risagarw@amd.com>
Signed-off-by: GodRishUniverse <risagarw@amd.com>
…actly on (vllm-project#52412)

Signed-off-by: Taksh <takshkothari09@gmail.com>
Signed-off-by: Nick Hill <nickhill123@gmail.com>
…-project#61323)

Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
vllm-project#59604)

Signed-off-by: Andrii Skliar <askliar@nvidia.com>
Signed-off-by: Luke Alonso <lalonso@gmail.com>
Co-authored-by: Andrii Skliar <askliar@nvidia.com>
Co-authored-by: Luke Alonso <lalonso@gmail.com>
shernshiou and others added 29 commits October 15, 2026 22:06
…vllm-project#59554)

Signed-off-by: Shern Shiou Tan <shernshiou@gmail.com>
Signed-off-by: Simon Veitner <sveitner@redhat.com>
Signed-off-by: Raphael Rialland <raphael.rialland@mistral.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Simon Veitner <sveitner@redhat.com>
Co-authored-by: Raphael Rialland <raphael.rialland@mistral.ai>
Co-authored-by: Lucas Wilkinson <LucasWilkinson@users.noreply.github.com>
…he target head (vllm-project#59973)

Signed-off-by: Stefano Castagnetta <scastagnetta@nvidia.com>
Co-authored-by: Kevin H. Luu <khluu000@gmail.com>
…#61767)

Signed-off-by: Nick Hill <nickhill123@gmail.com>
Co-authored-by: hjjq <50634613+hjjq@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… decode (vllm-project#57392)

Signed-off-by: omuhamma <omar.muhammad@amd.com>
Signed-off-by: Nicholas Susanto <nicholas.susanto@amd.com>
Co-authored-by: Nicholas Susanto <nicholas.susanto@amd.com>
Co-authored-by: Simon Danielsson <70206058+simondanielsson@users.noreply.github.com>
…cutor (vllm-project#61713)

Signed-off-by: Woosuk Kwon <woosuk@inferact.ai>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…project#61724)

Signed-off-by: Juntian Liu <Juntianl777@gmail.com>
Signed-off-by: Nick Hill <nickhill123@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…vllm-project#61913)

Signed-off-by: Anerudhan Gopal <agopal@nvidia.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
…+ KV Cache runtime fusion for Qwen3-30B-A3B on ROCM_ATTN (vllm-project#42755)

Signed-off-by: Jack Hu <Jack.Hu@amd.com>
Signed-off-by: Jack Hu <jingxujaster@hotmail.com>
Signed-off-by: Jack Hu <jack.hu@amd.com>
Co-authored-by: Douglas Lehr <91553416+dllehr-amd@users.noreply.github.com>
…-project#61388)

Signed-off-by: Ronald1995 <ronaldautomobile@163.com>
Signed-off-by: Ao Shen <281812371+aoshen02@users.noreply.github.com>
Signed-off-by: Ao Shen <aoshen@inferact.ai>
Co-authored-by: Ao Shen <281812371+aoshen02@users.noreply.github.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
Co-authored-by: Ao Shen <aoshen@inferact.ai>
…t#61924)

Signed-off-by: Tessa Pham <tepham@redhat.com>
Co-authored-by: Benjamin Chislett <bchislett@nvidia.com>
Signed-off-by: Jee Jee Li <jeejeelee@inferact.ai>
… parameters (vllm-project#61930)

Signed-off-by: khluu <khluu000@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…m-project#61471)

Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Co-authored-by: Claude <noreply@anthropic.com>
…oject#61716)

Signed-off-by: Monishver Chandrasekaran <monishverchandrasekaran@gmail.com>
Co-authored-by: Kevin H. Luu <khluu000@gmail.com>
…roject#43202)

Signed-off-by: lqh-hh <liuqihang6@huawei.com>
Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Co-authored-by: lqh-hh <liuqihang6@huawei.com>
Co-authored-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: 2017403603 <705328646@qq.com>
Co-authored-by: Julien Mancuso <jmancuso@nvidia.com>
Co-authored-by: Itay Alroy <75032521+itayalroy@users.noreply.github.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Signed-off-by: Sandeep Maddipatla <sandeep.maddipatla@intel.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Kevin H. Luu <khluu000@gmail.com>
…llm-project#61867)

Signed-off-by: Benjamin Chislett <bchislett@nvidia.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: vllm-agent <claw@inferact.ai>
…ject#61895)

Signed-off-by: Andreas Karatzas <akaratza@amd.com>
Co-authored-by: Claude <noreply@anthropic.com>
…project#61736)

Signed-off-by: maithilijoshi20 <maithilij2003@gmail.com>
Co-authored-by: Cyrus Leung <tlleungac@connect.ust.hk>
…nds (vllm-project#61574)

Signed-off-by: Maroon Ayoub <mayoub@redhat.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Robert Shaw <114415538+robertgshaw2-redhat@users.noreply.github.com>
Signed-off-by: Wei Zhao <51183510+wzhao18@users.noreply.github.com>
…llm-project#61932)

Signed-off-by: Chaojun Zhang <chaojun.zhang@intel.com>
Co-authored-by: vllm-agent <claw@inferact.ai>
Signed-off-by: wangyicong <wangyicong@bytedance.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
Co-authored-by: Lucas Wilkinson <lwilkins@redhat.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kevin H. Luu <khluu000@gmail.com>
… /generate

Multi-turn multimodal RL sends earlier images already expanded and new
images as raw media. The two inputs were mutually exclusive, so callers
had to collapse every earlier placeholder run before sending, and a
missed collapse expanded the run a second time without any error.

Allow both in one request: features carry the earlier items at their
expanded ranges, and only the prompt after the last features range is
rendered with content_parts. Its placeholders are shifted past that
prefix and appended to the features items. The processor is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Describe content_parts and features, how they combine, and a two-turn
example that sends earlier images as features and new ones as
content_parts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
An unmatched item hit an assert in the processor and surfaced as a 500.
Raise VLLMValidationError instead, and test the case of a content_parts
placeholder placed before the last features range.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
- Name the features ordering rule in the 400 when content_parts come
  before the last features range.
- Copy mm_hashes instead of extending request.features in place.
- Test features that carry only mm_hashes, as in the docs example.
- Note in the docs that models inserting media at the prompt start
  should not combine the two fields.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
@aoshen02
aoshen02 force-pushed the mm-idempotent-prompt-updates branch from 579f59e to 86ed29b Compare October 16, 2026 08:21
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.