Skip to content

fix(qwen-image): drop the shadowed dead _get_qwen_prompt_embeds duplicate - #7461

Merged
NickCao merged 1 commit into
vllm-project:mainfrom
Anai-Guo:fix-qwen-image-edit-dupdef
Sep 14, 2026
Merged

NickCao merged 1 commit into
vllm-project:mainfrom
Anai-Guo:fix-qwen-image-edit-dupdef

Conversation

@Anai-Guo

Copy link
Copy Markdown
Contributor

Problem

QwenImageEditPipeline._get_qwen_prompt_embeds is defined twice in
vllm_omni/diffusion/models/qwen_image/pipeline_qwen_image_edit.py. In Python
the second def silently shadows the first, so the first 44-line copy is dead
code that can never run.

The two definitions are not interchangeable:

  • The first (dead) copy has signature (self, prompt, image, dtype) and no
    sequence-length validation.

  • The second (live) copy adds max_sequence_length / prompt_name, calls
    validate_prompt_sequence_lengths(...), and is what encode_prompt actually
    invokes:

    prompt_embeds, prompt_embeds_mask = self._get_qwen_prompt_embeds(
        prompt, image,
        max_sequence_length=max_sequence_length,
        prompt_name=prompt_name,
    )

    Those keyword arguments only exist on the second definition — the first would
    raise TypeError if it were ever bound.

Fix

Remove the unreachable first definition. This is a pure no-op at runtime (the
live method is untouched) and drops a latent footgun.

+0 / -45, no behavior change.

🤖 Generated with Claude Code

QwenImageEditPipeline defines _get_qwen_prompt_embeds twice, so Python binds
only the second one. The live call site in encode_prompt passes
max_sequence_length= and prompt_name=, which only the second definition
accepts -- the first (44-line) copy would raise TypeError if it were ever
reached. Remove the unreachable duplicate; no behavior change.

Signed-off-by: Anai-Guo <antai12232931@outlook.com>
@Anai-Guo
Anai-Guo requested a review from wtomin as a code owner September 12, 2026 16:04
@vllm-omni-review-bot

Copy link
Copy Markdown

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

Module owners: @wtomin @david6666666 @Isotr0py

Routing: @wtomin via module of the changed files, module named in the PR description, CODEOWNERS; @david6666666 via module of the changed files, module named in the PR description; @Isotr0py via module of the changed files, module named in the PR description

@Anai-Guo, 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.

@hsliuustc0106 hsliuustc0106 added refactor refactoring for better code scalability and quality diffusion codes related to diffusion models labels Sep 12, 2026

@NickCao NickCao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, the shadowed first copy is never reachable.

@NickCao NickCao added the ready label to trigger buildkite CI label Sep 14, 2026
@NickCao
NickCao enabled auto-merge (squash) September 14, 2026 13:23
@NickCao
NickCao merged commit 1b6cd28 into vllm-project:main Sep 14, 2026
6 of 9 checks passed
mlaneuville pushed a commit to mlaneuville/vllm-omni that referenced this pull request Sep 22, 2026
…cate (vllm-project#7461)

Signed-off-by: Anai-Guo <antai12232931@outlook.com>
Signed-off-by: Matthieu Laneuville <matthieu.laneuville@surf.nl>
khairulkabir1661 pushed a commit to khairulkabir1661/vllm-omni that referenced this pull request Sep 25, 2026
…cate (vllm-project#7461)

Signed-off-by: Anai-Guo <antai12232931@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diffusion codes related to diffusion models ready label to trigger buildkite CI refactor refactoring for better code scalability and quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants