[Bugfix] Reject partially bound diffusion LoRA adapters by provoke210 · Pull Request #8008 · vllm-project/vllm-omni

provoke210 · 2026-09-22T09:38:09Z

Purpose

A diffusion LoRA adapter can currently become active when only some of its supplied modules bind. For example, a matching attn.to_out module can bind while an unmatched attn.to_out.0 module is silently ignored. The generic validation only rejects adapters that bind no layers.

Reject activation when any supplied module remains unbound, reporting the adapter ID, binding count, unbound names, and expected engine targets. Fused output shape mismatches now raise immediately with the actual lora_b dimension and expected output_slices total. Replace the existing zero-binding guard with this unified check, preserving rejection of empty adapters. Run model-specific validation afterward and reuse the existing activation-error cleanup to reset uploaded weights and invalidate active/suspended state. Sparse packed targets, such as Q or Q/V without K, remain valid when all supplied modules bind.

Addresses the partial-binding validation portion of #8001. The guide explains that in-memory tensor adapters with renamed keys and PEFT checkpoints with extra keys that pass loading can now fail at activation. Checkpoint-to-engine mapping remains outside this PR.

Test Plan

Regression coverage includes unmatched module names and incompatible fused output shapes, each while switching from an active or suspended adapter. It checks rollback and successful reactivation of the previous adapter, plus rejection of completely unmatched and empty adapters. Packed-target coverage exercises Q, Q/V, and Q/K/V across three supported name forms.

With the project test dependencies and vLLM available, run from the repository root:

# Minimal regression: fails against the original manager with the new tests applied.
VLLM_TARGET_DEVICE=cpu python -m pytest \
  tests/diffusion/lora/test_lora_manager.py::test_lora_manager_rejects_partial_binding_and_rolls_back -q

# Related CPU regression suite.
VLLM_TARGET_DEVICE=cpu python -m pytest \
  tests/diffusion/lora/test_lora_manager.py \
  tests/diffusion/lora/test_base_linear.py \
  tests/diffusion/lora/test_loader.py \
  -m 'core_model and cpu' --run-level core_model -q

pre-commit run --files \
  vllm_omni/diffusion/lora/manager.py \
  tests/diffusion/lora/test_lora_manager.py \
  docs/user_guide/diffusion/lora.md

vLLM Version: 0.29.0 (98dff2a81d747d1dba01a47f939f48c3526d4206 Python sources loaded via PYTHONPATH; native extensions were not built locally).

vLLM-Omni Commit: 75390448f10322d93b28173fc44ad421ecca32fd, based on 9ebef4b.

Local environment: macOS / Apple M5, Python 3.12.14, PyTorch 2.13.0, Transformers 5.14.1, Diffusers 0.40.0. Tests use the existing CPU dummy-layer fixtures and real LoRA weight objects.

Test Result

The four partial-binding regression cases fail against the original manager (f7e2834) because the expected ValueErrors are not raised.
The related CPU suite passed 65 tests (16 warnings, no skips), covering the unified guard and empty-adapter regression.
All applicable pre-commit hooks and git diff --check passed. Full precheck-pr self-check found no blocking code issues.
GPU integration tests and real-model generation were not run locally.

Assisted by OpenAI Codex for the implementation, regression tests, documentation, code review, local test execution, and PR description.

Raise when any supplied adapter module remains unbound, preserving the existing model-specific validator and activation rollback. Cover partial binding failures, adapter reuse after failure, and sparse packed targets. Related to vllm-project#8001. Assisted-by: OpenAI Codex Signed-off-by: Haozhe Jiang <162801044+provoke210@users.noreply.github.com>

vllm-omni-review-bot · 2026-09-22T09:42:37Z

This PR appears to belong to: docs/design/module/diffusion/diffusion_model_integration.md, docs/design/module/diffusion/index.md.

Module owners: @RuixiangMa @wtomin @david6666666

Routing: @RuixiangMa via module of the changed files, module named in the PR description, semantic router; @wtomin via module of the changed files, module named in the PR description, semantic router; @david6666666 via module of the changed files, module named in the PR description

@provoke210, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

vllm-omni-review-bot · 2026-09-22T09:46:19Z

Omni ReviewBot triage note

Automated triage of commit 1edcf844e2c0 produced:

Priority: high. Prompt maintainer attention is suggested.

These are automated triage suggestions only — the final decision belongs to the maintainers.

provoke210 · 2026-09-22T09:47:10Z

Self-review (assisted by OpenAI Codex): checked partial-binding errors, rollback and adapter reactivation, and valid sparse Q/V targets. All 64 related CPU tests and applicable pre-commit checks passed. GPU integration tests were not run locally.

NickCao

cc @Hiro208 from #7349

NickCao · 2026-09-22T12:25:58Z

                bound_lora_names=frozenset(bound_lora_names),
            )

+        unbound_lora_names = sorted(set(lora_model.loras) - bound_lora_names)


This should replace the if not bound_lora_names: guard above.

Replace the zero-binding guard with the generic incomplete-binding check before model-specific validation. Keep rejecting empty adapters and cover that boundary alongside the updated zero-binding error assertions. Assisted-by: OpenAI Codex Signed-off-by: Haozhe Jiang <162801044+provoke210@users.noreply.github.com>

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved review issues remain.

Review effort: Lite
Findings: None

What changed in this PR

Rejects diffusion LoRA adapters with partially bound modules while preserving valid sparse targets and rollback behavior.

Changes:

Adds complete-binding validation and cleanup.
Expands regression coverage.
Documents binding requirements.

File	Summary
`vllm_omni/diffusion/lora/manager.py`	Validates complete bindings and preserves cleanup.
`tests/diffusion/lora/test_lora_manager.py`	Adds validation and rollback coverage.
`docs/user_guide/diffusion/lora.md`	Documents adapter binding behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

SamitHuang

The unified incomplete-binding check is the right fail-loud replacement for the zero-bind guard, and the CPU tests cover rollback plus sparse Q/V. Two issues still undermine the point of this PR: the new error no longer reports expected engine modules (the signal needed for to_out.0 vs to_out), and the fused shape-mismatch path still logs “Skipping” as if activation continued. The user guide also needs to say that previously silent partial binds (Qwen-Image / Boogu / Wan tensor adapters with renamed keys) will now hard-fail until #8001 name mapping lands.

SamitHuang · 2026-09-23T07:39:45Z

-                "Activating it would leave the base model unchanged."
+                f"LoRA adapter {lora_model.id} binding is incomplete: "
+                f"bound={len(bound_lora_names)}/{len(lora_model.loras)}, "
+                f"unbound modules={unbound_lora_names}"


This message drops expected_lora_modules and the received key list that the zero-bind guard used to print. For the #8001 case (attn.to_out.0 unbound while the engine module is attn.to_out) the unbound name alone does not tell the caller which engine target it failed to match. Keep bound= / unbound modules= and also include sorted(self._expected_lora_modules) so activation failures stay diagnosable. The fused shape-mismatch branch a few lines above still logger.warning("Skipping LoRA...") and continues without _record_bound; this guard then raises with no lora_b / output_slices numbers, so logs still look like a skip. Raise from that branch or attach the mismatch to the unbound entry.

SamitHuang · 2026-09-23T07:39:45Z

+unbound modules and resets the LoRA layers to avoid leaving the adapter partly
+active. An adapter may still target only some projections of a fused layer, such
+as Q and V without K, as long as all supplied modules bind.
+


This documents the new contract but not the break. Tensor-path adapters that previously activated with dropped modules (Qwen-Image / Boogu-Image / Wan to_out.0, and any PEFT dump with extra unbound keys) will now raise at _activate_adapter. State that explicitly, and point at #8001 / the follow-up mapper work so users do not treat the new ValueError as a loader regression.

Thanks for the review. I’ve updated the error messages and the guide as suggested. Could you take another look when you have a chance?

Report expected engine targets for unmatched adapter modules, raise with actual and expected dimensions for fused shape mismatches, and document the behavior change for tensor adapters with renamed keys. Assisted-by: OpenAI Codex Signed-off-by: Haozhe Jiang <162801044+provoke210@users.noreply.github.com>

Document that PEFT checkpoints with extra keys can fail at activation when those keys pass load-time validation but do not bind to engine modules. Distinguish this from unsupported suffixes already rejected during loading. Assisted-by: OpenAI Codex Signed-off-by: Haozhe Jiang <162801044+provoke210@users.noreply.github.com>

vllm-omni-review-bot · 2026-09-23T10:57:52Z

Omni ReviewBot: superseded

The CI failure noted on 75390448f103 refers to an earlier head; the pull request now points at 1edcf844e2c0.

Limit the synthetic manager test to the one LoRA layer it exposes, so complete-binding validation still exercises the packed activation path. Assisted-by: OpenAI Codex Signed-off-by: Haozhe Jiang <162801044+provoke210@users.noreply.github.com>

Preserve HunyuanImage-3 fused-QKV de-interleaving while reporting invalid layouts as binding errors, and update its rejection assertion. Assisted-by: OpenAI Codex Signed-off-by: Haozhe Jiang <162801044+provoke210@users.noreply.github.com>

SamitHuang

Re-review of 1edcf844 against the earlier change requests.

The incomplete-binding guard now prints bound= / unbound modules= and sorted(self._expected_lora_modules), so a dropped to_out.0 can be matched against engine to_out. Fused lora_b vs output_slices and HunyuanImage-3 de-interleave failures raise with the actual/expected row counts instead of logging Skipping LoRA and continuing. The guide states that Qwen-Image / Boogu-Image / Wan tensor to_out.0 keys and extra PEFT keys that pass load now fail at activation, and points at #8001 for mapping.

CPU coverage hits partial bind from an active and a suspended adapter, empty adapters, sparse packed Q / Q+V / QKV, Hunyuan unexpected rows, and the MiniMax packed fixture (restricted to the one injected module). Buildkite on this SHA is still pending.

SamitHuang · 2026-09-24T03:58:28Z

+                f"LoRA adapter {lora_model.id} binding is incomplete: "
+                f"bound={len(bound_lora_names)}/{len(lora_model.loras)}, "
+                f"unbound modules={unbound_lora_names}; "
+                f"expected target modules in {sorted(self._expected_lora_modules)}"


The unified guard now includes expected target modules in {sorted(self._expected_lora_modules)} next to bound= / unbound modules=, which is the signal needed for to_out.0 vs to_out. Confirming this addresses the earlier request; empty adapters still fail via not bound_lora_names (bound=0/0).

SamitHuang · 2026-09-24T03:58:28Z

-                            full_module_name,
-                            lora_weights.lora_b.shape[0],
-                            total,
+                        raise ValueError(


This fused mismatch now raises with lora_b.shape[0], sum(output_slices), and the slice tuple instead of logger.warning("Skipping LoRA") + continue. Combined with _activate_adapter's except: _reset_lora_layers(), a shape-incompatible extra key cannot leave slot 0 half-updated. The Hunyuan de-interleave branch does the same with expected output_sizes.

SamitHuang · 2026-09-24T03:58:28Z

+validation; unsupported module suffixes may already fail during loading.
+Checkpoint-to-engine name mapping is tracked in [#8001](https://github.com/vllm-project/vllm-omni/issues/8001);
+until it is available, adapter keys must resolve to supported engine modules.
+


This now names the break: in-memory Qwen-Image / Boogu-Image / Wan tensor keys such as to_out.0, and PEFT dumps with extra keys that pass the loader, fail at activation until #8001 mapping exists. That is the diagnostic users need so they do not treat the new ValueError as a loader regression.

SamitHuang

LGTM

provoke210 requested review from Gaohan123, NickCao, RuixiangMa, david6666666, hsliuustc0106, wtomin and yenuo26 as code owners September 22, 2026 09:38

NickCao reviewed Sep 22, 2026

View reviewed changes

hsliuustc0106 added high priority high priority issue, needs to be done asap bug Something isn't working labels Sep 23, 2026

SamitHuang requested review from SamitHuang and a lite review from Copilot September 23, 2026 06:55

Copilot started reviewing on behalf of SamitHuang September 23, 2026 07:00

View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

SamitHuang requested changes Sep 23, 2026

View reviewed changes

provoke210 added 2 commits September 23, 2026 15:49

SamitHuang added ready label to trigger buildkite CI cuda-test Used to trigger vllm-omni cuda CI separately. labels Sep 23, 2026

provoke210 added 2 commits September 24, 2026 01:22

SamitHuang added ready label to trigger buildkite CI and removed ready label to trigger buildkite CI cuda-test Used to trigger vllm-omni cuda CI separately. labels Sep 24, 2026

SamitHuang added the cuda-test Used to trigger vllm-omni cuda CI separately. label Sep 24, 2026

SamitHuang reviewed Sep 24, 2026

View reviewed changes

SamitHuang approved these changes Sep 24, 2026

View reviewed changes

SamitHuang merged commit dbd6a35 into vllm-project:main Sep 24, 2026
6 checks passed

rufushsu9987 mentioned this pull request Sep 24, 2026

[Bug]: Diffusion LoRA loading bypasses model checkpoint→engine name renames and silently no-ops on partial binds #8001

Open

zhtmike mentioned this pull request Oct 2, 2026

[RFC][tracking] Upstream patch debt: audited ledger of temporary compatibility patches verl-project/verl-omni#445

Open

28 tasks

Conversation

provoke210 commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Test Plan

Test Result

Uh oh!

vllm-omni-review-bot commented Sep 22, 2026

Uh oh!

vllm-omni-review-bot commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Omni ReviewBot triage note

Uh oh!

provoke210 commented Sep 22, 2026

Uh oh!

NickCao left a comment

Choose a reason for hiding this comment

Uh oh!

NickCao Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

SamitHuang left a comment

Choose a reason for hiding this comment

Uh oh!

SamitHuang Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

SamitHuang Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

provoke210 Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

vllm-omni-review-bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Omni ReviewBot: superseded

Uh oh!

SamitHuang left a comment

Choose a reason for hiding this comment

Uh oh!

SamitHuang Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

SamitHuang Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

SamitHuang Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

SamitHuang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

provoke210 commented Sep 22, 2026 •

edited

Loading

vllm-omni-review-bot commented Sep 22, 2026 •

edited

Loading

vllm-omni-review-bot commented Sep 23, 2026 •

edited

Loading