[Bugfix][Multimodal] Fix per-video cache option reuse by bvolpato · Pull Request #55203 · vllm-project/vllm · GitHub
Skip to content

[Bugfix][Multimodal] Fix per-video cache option reuse - #55203

Merged
DarkLight1337 merged 3 commits into
vllm-project:mainfrom
bvolpato:bvolpato/fix-per-video-processor-cache
Oct 11, 2026
Merged

DarkLight1337 merged 3 commits into
vllm-project:mainfrom
bvolpato:bvolpato/fix-per-video-processor-cache

Conversation

@bvolpato

@bvolpato bvolpato commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Overview

Preserve each Qwen3-VL video's effective sampling options across repeated items and partial multimodal-cache hits. Reject option lists that are too short before indexing them.

Claims

  • Each video's selected fps and num_frames contribute to its cache identity.
  • Partial cache hits filter option lists together with the remaining videos.
  • Configured defaults, request options, flat/nested scopes, UUID reuse, and encoder profiling preserve their existing behavior.
  • Short option lists raise a clear ValueError with and without a processor cache. Unused trailing entries remain supported.

Validation

Validated on 2026-10-11 at head a0067713d3, rebased onto upstream 01abe975b5.

VLLM_TARGET_DEVICE=cpu CUDA_VISIBLE_DEVICES='' uv run --no-project -- .venv/bin/python -m pytest tests/models/multimodal/processing/test_qwen3_vl.py -q
VLLM_TARGET_DEVICE=cpu CUDA_VISIBLE_DEVICES='' uv run --no-project -- .venv/bin/python -m pytest tests/multimodal/test_cache.py tests/multimodal/test_processing.py -q
uv run --no-project -- .venv/bin/pre-commit run --hook-stage manual --files vllm/multimodal/processing/processor.py vllm/model_executor/models/qwen3_vl.py tests/models/multimodal/processing/test_qwen3_vl.py

The Qwen3 processor, generic processing, and cache suites pass, as do the applicable scoped hooks. The CUDA sleep/wake integration was skipped in the CPU suite. The short-list reproducer failed with IndexError: list index out of range before the guard and raises the expected ValueError afterward. Regressions exercise empty/short lists, warm caches, longer lists through partial hits, and profiling with fewer videos than configured options.

The cache regression checks prompts, placeholders, tensors, hashes, scalar-reference grids, duplicate videos, partial hits, reordering, model/request options, flat/nested scopes, and UUID-only reuse. Input options remain unmodified.

Loaded-model comparison Configuration/result
Model/runtime Real Qwen/Qwen3-VL-2B-Instruct weights, snapshot 89644892e4d85e24eaac8bacfd4f463576704203; RTX 5070 Ti, BF16, MRV2, FlashAttention 2, eager execution
Workload Synthetic video pairs with num_frames=[8,16], duplicate/partial/reordered cache use against uncached references; greedy 8-token generation
Result Prompt IDs, placeholders, tensors, hashes, output token IDs, and complete top-5 logprob maps match; maximum logprob difference 0.0

Validation used the shared development environment, pinned Humming 0.1.16, and existing precompiled CUDA extensions. Optional FlashInfer autotuning and JIT warmup were disabled. This is a focused 2B family comparison, not a 4B answer-quality benchmark.

Details

Request-wide option-list hashes let repeated videos reuse the first video's result despite different sampling settings. Partial hits removed videos without removing the corresponding option entries.

The existing base hooks let Qwen3-VL refine item hashes and select cache-miss options while preserving other processors. The final text/image HF call suppresses configured video lists so merging cannot reintroduce them. The review fix stays in the Qwen3 processor and its existing regression suite.

Merged #56372 handles option scoping and the cache API; this fix uses that behavior. The current overlap search found no competing per-item cache identity/partial-hit fix. #54527 addresses scoped option reads in other models; #60879 bounds MiniMax video decoding.

Affected hashes retain the full option list as well as selected item options, so equivalent reordered requests can incur extra misses. Scalar paths retain their existing hashes.

Upstream pre-commit is blocked at contributor authorization before implementation checks run. Read the Docs reports success from skipping this change because no docs-affecting files changed; MkDocs did not run. AI assistance was used for investigation, implementation, validation, and review.


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: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

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

BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mergify mergify Bot added multi-modality Related to multi-modality (#4194) qwen Related to Qwen models bug Something isn't working labels Sep 3, 2026
@bvolpato
bvolpato force-pushed the bvolpato/fix-per-video-processor-cache branch from 0ea438a to 692e7a5 Compare September 3, 2026 19:57
@github-actions

github-actions Bot commented Sep 3, 2026

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.

🚀

@mergify

mergify Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @bvolpato.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 16, 2026
@bvolpato
bvolpato force-pushed the bvolpato/fix-per-video-processor-cache branch from 692e7a5 to ee9c1a3 Compare September 24, 2026 01:09
@mergify mergify Bot removed the needs-rebase label Sep 24, 2026
@mergify

mergify Bot commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @bvolpato.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 29, 2026
@bvolpato
bvolpato force-pushed the bvolpato/fix-per-video-processor-cache branch from ee9c1a3 to f0f0374 Compare October 5, 2026 03:18
@mergify mergify Bot removed the needs-rebase label Oct 5, 2026
@bvolpato
bvolpato force-pushed the bvolpato/fix-per-video-processor-cache branch from f0f0374 to ff6748b Compare October 8, 2026 23:55
@bvolpato
bvolpato marked this pull request as ready for review October 11, 2026 01:04

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

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-11T01:10:32.944027Z ff6748b Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@DarkLight1337

Copy link
Copy Markdown
Member

@claude review

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread vllm/model_executor/models/qwen3_vl.py
bvolpato and others added 2 commits October 11, 2026 00:14
Include effective per-video sampling options in cache identity and
filter option lists together with uncached video inputs.

Signed-off-by: bvolpato <brunocvcunha@gmail.com>
Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: bvolpato <brunocvcunha@gmail.com>
@bvolpato
bvolpato force-pushed the bvolpato/fix-per-video-processor-cache branch from ff6748b to a006771 Compare October 11, 2026 04:25

@DarkLight1337 DarkLight1337 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, LGTM

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) October 11, 2026 04:57
@DarkLight1337

Copy link
Copy Markdown
Member

/ci run

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

Copy link
Copy Markdown

✅ Triggered Buildkite CI #94050 for commit 7ab17bb3668c.

@DarkLight1337
DarkLight1337 merged commit 61f084b into vllm-project:main Oct 11, 2026
164 checks passed
@danigarciaoca

Copy link
Copy Markdown
Contributor

Hi @bvolpato! Just a couple of questions to make sure I'm understanding this correctly:

What's the rationale for doing this instead of keeping the already resolved values?

# The context merges model defaults again when calling HF.
for key in self._get_per_video_kwargs({}):
non_video_scoped_kwargs[key] = None

Also, I noticed that the return value of this function doesn't seem to be used anywhere:

If I'm reading the code correctly, fps and num_frames are already assigned for each video inside the for item_idx, item in enumerate(videos): loop. Am I missing something here? Could you take a look at this @DarkLight1337

michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
…5203)

Signed-off-by: bvolpato <brunocvcunha@gmail.com>
Co-authored-by: Codex <noreply@openai.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 multi-modality Related to multi-modality (#4194) qwen Related to Qwen 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.

3 participants