Skip to content

[Bugfix][Model] Fix mypy safe-super error in EmbeddingGemma2Model - #60280

Merged
ywang96 merged 2 commits into
vllm-project:mainfrom
ywang96:fix-embedding-gemma2-mypy
Oct 6, 2026
Merged

ywang96 merged 2 commits into
vllm-project:mainfrom
ywang96:fix-embedding-gemma2-mypy

Conversation

@ywang96

@ywang96 ywang96 commented Oct 6, 2026

Copy link
Copy Markdown
Member

Overview

Fix the mypy pre-commit failure on main introduced by #60254 (EmbeddingGemma2Model): super().__init__() in a class inheriting VllmModelForPooling (a Protocol) trips [safe-super] + [call-arg].

Claims

  • Fixes the 2 mypy errors at vllm/model_executor/models/embedding_gemma2.py:1023 that break the mypy-3.10–mypy-3.13 pre-commit hooks on main.
  • No runtime behavior change: the MRO already resolved super().__init__() to nn.Module.__init__; this makes the call explicit.
  • Not a duplicate: checked open PRs for embedding_gemma2 / safe-super / [Model] Support EmbeddingGemma2 multimodal pooling architecture #60254 follow-ups — none address this.

Validation

# Fails on main, passes with this change (2 errors -> 0)
mypy --python-version 3.10 vllm/model_executor/models/embedding_gemma2.py
# Also verified: --python-version 3.11, 3.12, 3.13

# The hook as CI runs it, on every file #60254 touched
python tools/pre_commit/mypy.py 3.10 \
  vllm/model_executor/models/embedding_gemma2.py \
  tests/models/multimodal/pooling/test_embedding_gemma2.py \
  vllm/model_executor/models/config.py \
  vllm/model_executor/models/registry.py \
  vllm/multimodal/video.py \
  vllm/v1/attention/ops/triton_prefill_attention.py \
  tests/models/registry.py

ruff check vllm/model_executor/models/embedding_gemma2.py
ruff format --check vllm/model_executor/models/embedding_gemma2.py

All pass. The change is typing-visible only (super().__init__() already resolved to nn.Module.__init__ at runtime), so no model eval was run; happy to run one if reviewers want it.

Details

Root cause: VllmModel/VllmModelForPooling are Protocols whose __init__ requires vllm_config. When a class explicitly inherits such a Protocol, mypy treats the protocol's trivial-body __init__ as the abstract method reachable via super().__init__() and rejects the argument-free call. Fix follows the established codebase pattern (nn.Module.__init__(self) in llama_eagle3.py, cohere_eagle.py, opencua.py, qwen3_dflash.py, openpangu_mtp.py).

AI assistance (Kimi Code) was used to prepare this PR; the submitter has reviewed the change.


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.

PR vllm-project#60254 added EmbeddingGemma2Model with bases (nn.Module,
SupportsMultiModal, VllmModelForPooling). Because VllmModelForPooling is
a Protocol whose __init__ requires vllm_config, mypy resolves the
argument-free super().__init__() against the protocol's abstract
__init__, failing pre-commit (mypy-3.10..3.13) with [safe-super] and
[call-arg] errors on main.

Call nn.Module.__init__(self) explicitly instead, matching the
established pattern in llama_eagle3.py, cohere_eagle.py, opencua.py,
qwen3_dflash.py, and openpangu_mtp.py. No runtime behavior change: the
MRO already resolved super().__init__() to nn.Module.__init__.

Co-authored-by: Kimi Code <noreply@moonshot.cn>
Signed-off-by: Roger Wang <hey@rogerw.io>

@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 Oct 6, 2026
@ywang96 ywang96 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 6, 2026
@ywang96

ywang96 commented Oct 6, 2026

Copy link
Copy Markdown
Member Author

/ci-run

@khluu

khluu commented Oct 6, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #93162 for commit 64787c2bf63c.

@ywang96
ywang96 merged commit bb87d22 into vllm-project:main Oct 6, 2026
10 of 14 checks passed
@dougbtv dougbtv mentioned this pull request Oct 6, 2026
5 of 6 tasks
michael-gpu pushed a commit to practical-inference/vllm-ampere-extended that referenced this pull request Oct 20, 2026
…lm-project#60280)

Signed-off-by: Roger Wang <hey@rogerw.io>
Co-authored-by: Kimi Code <noreply@moonshot.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 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