Skip to content

[Bugfix][MM] Fix MiniCPM-V placeholder replacement and image processor loading on Transformers v5 - #48413

Merged
vllm-bot merged 5 commits into
vllm-project:mainfrom
YunzhuLu:fix-minicpmv-placeholder-token-mismatch
Aug 6, 2026
Merged

vllm-bot merged 5 commits into
vllm-project:mainfrom
YunzhuLu:fix-minicpmv-placeholder-token-mismatch

Conversation

@YunzhuLu

@YunzhuLu YunzhuLu commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Purpose

This PR fixes two independent but related issues that block MiniCPM-V models (2.5 / 2.6 / 4.0 / 4.5) from working with Transformers v5 in vLLM:

  1. Placeholder mismatch: Multi-modal placeholder replacement fails at runtime with found 0 prompt placeholders.
  2. Wrong Image processor reuse: When multiple MiniCPM-V checkpoints are loaded in the same process (e.g. serial pytest runs), later models incorrectly reuse an earlier model's MiniCPMVImageProcessor class.

Also removes the max_transformers_version="4.57" cap in tests/models/registry.py so MiniCPM-V processing tests can run under Transformers v5, since #44282 already vendors the MiniCPMV processor and aliases MiniCPMVBatchFeature to BatchFeature.

Problem 1: Placeholder mismatch

After upgrading to Transformers v5, starting MiniCPM-V4 with vllm serve fails immediately during initialization

vllm serve /root/autodl-tmp/huggingface/hub/MiniCPM-V-4/OpenBMB/MiniCPM-V-4 \
  --trust-remote-code \
  --served-model-name MiniCPM-V-4 \
  --gpu-memory-utilization 0.75 \
  --max-model-len 4096 \
  --max-num-batched-tokens 4096 \
  --limit-mm-per-prompt '{"video": 1, "image": 1}'
RuntimeError: Expected there to be 1 prompt placeholders corresponding to 1 image items, but instead found 0 prompt placeholders! Make sure the implementation of `_call_hf_processor` and `_get_mm_fields_config` are consistent with each other.

Root cause

MiniCPM-V placeholders are ordinary text, not tokenizer special tokens. Under BPE, the same string tokenizes differently when encoded standalone vs. in a full prompt context.

Solution

Override _apply_prompt_updates in MiniCPMVMultiModalProcessor:

  1. Try token matching first
  2. On failure, fall back to locating placeholders in the decoded prompt text, then encode the prefix, replacement, and suffix separately and concatenate the resulting token IDs directly. This avoids re-encoding the full prompt string in one pass, which can change BPE boundaries and break placeholder replacement.

Test Plan

function test

Serve MiniCPM-V-4 with Transformers v5 and send a single-image request
server

vllm serve /root/autodl-tmp/huggingface/hub/MiniCPM-V-4/OpenBMB/MiniCPM-V-4 \
  --trust-remote-code \
  --served-model-name MiniCPM-V-4 \
  --gpu-memory-utilization 0.75 \
  --max-model-len 4096 \
  --max-num-batched-tokens 4096 \
  --limit-mm-per-prompt '{"video": 1, "image": 1}'

client

payload = {
    "model": "MiniCPM-V-4",
    "messages": [
        {
            "role": "user",
            "content": [
                {
                    "type": "image_url",
                    "image_url": {"url": f"data:image/jpeg;base64,{base64_image}"}
                },
                {"type": "text", "text": "What is in this image?"}
            ]
        }
    ],
    "max_tokens": 128,
    "temperature": 0
}

Test Result

server

(APIServer pid=18436) INFO:     Started server process [18436]
(APIServer pid=18436) INFO:     Waiting for application startup.
(APIServer pid=18436) INFO:     Application startup complete.

client

The image depicts a person standing indoors. The individual is wearing a green short-sleeved top and denim shorts. In the background, there is a television mounted on a stand displaying an image of an aircraft carrier with some text in Chinese. The room has light blue walls, a window, and some modern furniture including chairs and a small table. The overall setting appears to be a casual indoor environment, possibly an office or a meeting room.

Problem 2: Wrong image processor loaded when switching MiniCPM-V models

While validating the placeholder fix, running test_processing_correctness for multiple MiniCPM-V models in the same pytest process exposed a second issue.

Reproduction

Reproduction: Run 2.5 and V-4 serially in one process

pytest \
  "tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.3-openbmb/MiniCPM-Llama3-V-2_5]" \
  "tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.3-openbmb/MiniCPM-V-4]" \
  -v
  • MiniCPM-Llama3-V-2_5 passes
  • MiniCPM-V-4 fails with
TypeError: MiniCPMVImageProcessor.get_sliced_grid() got an unexpected keyword argument 'max_slice_nums'

However, running each model in isolation passes; the failure only appears when models are loaded serially in the same process.

Root cause

MiniCPM-V repos (openbmb/MiniCPM-Llama3-V-2_5, openbmb/MiniCPM-V-2_6, openbmb/MiniCPM-V-4, openbmb/MiniCPM-V-4_5, etc.) declare the same remote class name in preprocessor_config.json. The class name is identical across repos, but implementations differ. After loading 2.5, a subsequent V-4 load reuse 2.5's dynamically loaded processor instead of loading V-4's own.

Solution

Override MiniCPMVProcessingInfo.get_hf_processor to bypass AutoImageProcessor's shared class resolution:

  1. Load the image processor class per repo via get_class_from_dynamic_module
  2. Instantiate via cached_get_image_processor(..., processor_cls_overrides=processor_cls) so processor_cls.from_pretrained is called directly on the repo-specific class.

Test Plan

MiniCPM-V-2_6 is skipped because it is a gated repo and the test environment does not have access.

pytest tests/models/multimodal/processing/test_common.py -k "minicpm and not 2_6" -v

Test Result

tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.3-openbmb/MiniCPM-V-4_6] PASSED                                                                                                                            [  8%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.3-openbmb/MiniCPM-Llama3-V-2_5] PASSED                                                                                                                     [ 16%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.3-openbmb/MiniCPM-V-4] PASSED                                                                                                                              [ 25%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.3-openbmb/MiniCPM-V-4_5] PASSED                                                                                                                            [ 33%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.5-openbmb/MiniCPM-V-4_6] PASSED                                                                                                                            [ 41%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.5-openbmb/MiniCPM-Llama3-V-2_5] PASSED                                                                                                                     [ 50%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.5-openbmb/MiniCPM-V-4] PASSED                                                                                                                              [ 58%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-0.5-openbmb/MiniCPM-V-4_5] PASSED                                                                                                                            [ 66%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-1.0-openbmb/MiniCPM-V-4_6] PASSED                                                                                                                            [ 75%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-1.0-openbmb/MiniCPM-Llama3-V-2_5] PASSED                                                                                                                     [ 83%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-1.0-openbmb/MiniCPM-V-4] PASSED                                                                                                                              [ 91%]
tests/models/multimodal/processing/test_common.py::test_processing_correctness[1.0-32-1.0-openbmb/MiniCPM-V-4_5] PASSED                                                                                                                            [100%]

============================================================================================== 12 passed, 438 deselected, 16 warnings in 647.89s (0:10:47) ===============================================================================================

Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

Signed-off-by: YunzhuLu <lucia.yunzhu@gmail.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 the bug Something isn't working label Jul 12, 2026
@YunzhuLu

Copy link
Copy Markdown
Contributor Author

cc @DarkLight1337, thanks for taking a look!

@DarkLight1337

Copy link
Copy Markdown
Member

cc @tc-mb

@tc-mb

tc-mb commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

cc @tc-mb

ok

@tc-mb

tc-mb commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@YunzhuLu
The new _minicpmv_hf_processor cache appears to ignore per-call mm_processor_kwargs. After the first call, get_hf_processor(**kwargs) returns the same processor instance without considering subsequent kwargs.

I verified this with MiniCPM-V-4: calling it first with max_slice_nums=1 and then with max_slice_nums=2 returned the same processor, and the second call still had max_slice_nums=1.

Since cached_get_image_processor already caches by its normalized arguments, could we remove this additional cache or key it by the normalized kwargs? A regression test covering consecutive calls with different kwargs would also help prevent stale processor configuration.

Signed-off-by: YunzhuLu <lucia.yunzhu@gmail.com>
@mergify mergify Bot added the multi-modality Related to multi-modality (#4194) label Jul 15, 2026
@YunzhuLu

Copy link
Copy Markdown
Contributor Author

@tc-mb Thanks for the review. I've updated the cache to be keyed by normalized kwargs from _merge_mm_kwargs, so per-call mm_processor_kwargs are applied correctly (e.g. different max_slice_nums now yield different processor instances).

I kept the outer cache (instead of removing it) because dropping it caused MiniCPM-V processing tests to spend much more time repeatedly resolving the dynamic processor class and constructing processors. To avoid repeating get_class_from_dynamic_module on every call, the image processor class is loaded once per model via @cached_property.

Added unit tests test_get_hf_processor_for_different_kwargs and test_get_hf_processor_for_same_kwargs in tests/models/multimodal/processing/test_minicpmv.py; both pass locally. The existing MiniCPM-V processing tests also still pass (12 passed, 438 deselected, 16 warnings in 356.32s).

@YunzhuLu

Copy link
Copy Markdown
Contributor Author

Hi @tc-mb, just a gentle ping. I've addressed all the review comments and updated the PR. Could you please take another look when you have time? Thanks!

@tc-mb

tc-mb commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Hi @tc-mb, just a gentle ping. I've addressed all the review comments and updated the PR. Could you please take another look when you have time? Thanks

Okay, sorry, I was on WAIC a few days ago and didn't keep up. I'll continue today.

@tc-mb

tc-mb commented Jul 20, 2026

Copy link
Copy Markdown
Contributor
  • The newly added _minicpmv_hf_processor_cache is unbounded. Per-request processor kwargs may continuously create new entries in a long-running server.
  • Dynamic class resolution is already cached by cached_property, while cached_get_image_processor already provides a bounded LRU cache. The additional cache and object-identity test appear unnecessary.
  • The tests should reproduce the original failures:
    • load MiniCPM-V 2.5 followed by V-4 and verify that their processor classes/modules are different;
    • cover a prompt whose BPE boundaries cause standalone and in-context placeholder tokenization to differ.
  • The placeholder workaround duplicates substantial private logic from the generic multimodal processor and imports several private helpers. A reusable helper in the common processor layer would reduce maintenance risk.
  • The image processor class reference should preferably come from auto_map.AutoImageProcessor rather than being hard-coded.
  • get_class_from_dynamic_module executes repository code directly, so the path should explicitly enforce trust_remote_code.

Signed-off-by: YunzhuLu <lucia.yunzhu@gmail.com>
@YunzhuLu

Copy link
Copy Markdown
Contributor Author

@tc-mb , hope WAIC went well! Thanks for the thorough review, below are what changed:

  • Removed the unbounded _minicpmv_hf_processor_cache; class resolution (which costs most time) is memoized via @cached_property.
  • Replaced the identity-based unit tests with test_image_processor_for_dif_model and test_prompt_has_dif_BPE_boundaries_in_context, which reproduce the original 2.5/V-4 mismatch and the BPE boundary issue directly. Both fail on main, pass on this branch.
  • Removed _apply_prompt_updates_by_text_locate and its private imports. The logic is now apply_text_matches_as_segmented_tokens() in processor.py.
  • Image processor class now comes from config file via ImageProcessingMixin.get_image_processor_dict(). I first tried reading it from model_config.hf_image_processor_config. But it hits a latent bug in transformers' get_image_processor_config() which drops auto_map when processor_config.json exists without a nested "image_processor" key. I plan to file a fix upstream in transformers, will link it here once it's up.
  • trust_remote_code is now explicitly enforced.
pytest tests/models/multimodal/processing/test_minicpmv.py -v
pytest tests/models/multimodal/processing/test_common.py -k "minicpm and not 2_6" -v

Both pass locally; confirmed test_image_processor_for_dif_model and test_prompt_has_dif_BPE_boundaries_in_context fail on main and pass with this branch.

@YunzhuLu

Copy link
Copy Markdown
Contributor Author

Quick update on the Transformers issue mentioned above: I've opened huggingface/transformers#47628 to fix the config fallback behavior that drops auto_map.
This vLLM PR does not depend on the upstream change; I'm linking it here for tracking.
@tc-mb, could you please take another look at the latest changes when you have a chance? Thanks!

@tc-mb

tc-mb commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Quick update on the Transformers issue mentioned above: I've opened huggingface/transformers#47628 to fix the config fallback behavior that drops auto_map. This vLLM PR does not depend on the upstream change; I'm linking it here for tracking. @tc-mb, could you please take another look at the latest changes when you have a chance? Thanks!

OK, @YunzhuLu thank you for your update, I feel like PR can be merged.
I did the verification of v25/v26/v40/v45/v46.
cc: @DarkLight1337

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) August 3, 2026 10:30
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 3, 2026
@mergify

mergify Bot commented Aug 6, 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, @YunzhuLu.

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 Aug 6, 2026
Signed-off-by: Yunzhu Lu <lucia.yunzhu@gmail.com>
auto-merge was automatically disabled August 6, 2026 00:46

Head branch was pushed to by a user without write access

@YunzhuLu

YunzhuLu commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Hi @DarkLight1337 , the previous CI failures seemed to be caused by a possible architecture-name mismatch between ExaoneMoeForCausalLM and ExaoneMoEForCausalLM, which appeared unrelated to this PR.
I resolved the import conflicts with the latest main branch changes, but the required buildkite/ci/pr check has not been triggered yet. Could you please help check the CI status? Thanks!

@mergify mergify Bot removed the needs-rebase label Aug 6, 2026
@vllm-bot
vllm-bot merged commit f84df12 into vllm-project:main Aug 6, 2026
6 checks passed
@DarkLight1337

Copy link
Copy Markdown
Member

@tc-mb there seems to be some problems with the remote module: https://buildkite.com/vllm/ci/builds/82629/list?sid=019fd5a9-db3a-46d7-b41b-a497da37000c&tab=output

@DarkLight1337

Copy link
Copy Markdown
Member

Could you fix them?

hmellor added a commit to hmellor/vllm that referenced this pull request Aug 11, 2026
vllm-project#48413 fixed the vLLM-side `MiniCPMVBatchFeature` incompatibility and
dropped the `max_transformers_version` cap on `MiniCPMV` entirely. That
also un-gated the tests that build an HF reference model, exposing a
separate HF-side break: MiniCPMV's remote code never calls
`self.post_init()`, so `all_tied_weights_keys` is never set and
Transformers v5 raises in `_move_missing_keys_from_meta_to_device`.

Restore the cap with an `hf`-scoped reason so HF-runner comparisons skip
while vLLM-only coverage keeps running.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.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) 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.

4 participants