Skip to content

[CI] Fix mypy for vllm/lora#30874

Open
hmellor wants to merge 4 commits intovllm-project:mainfrom
hmellor:mypy-lora
Open

[CI] Fix mypy for vllm/lora#30874
hmellor wants to merge 4 commits intovllm-project:mainfrom
hmellor:mypy-lora

Conversation

@hmellor
Copy link
Copy Markdown
Member

@hmellor hmellor commented Dec 17, 2025

This change brings us closer to not needing a custom mypy check. Part of #26533

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.

Comment thread vllm/config/lora.py Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors LoRA (Low-Rank Adaptation) integration across several modules, primarily focusing on type hinting, validation, and internal logic. Key changes include adjusting mypy configuration for the vllm/lora module, enhancing LoRAConfig validation by ensuring max_cpu_loras is an integer and adding a field_validator for None values during delayed initialization, and improving error messages. For FusedMoE layers, the PR updates type hints (e.g., Mapping, str | None), modifies how MoE configuration parameters are accessed, removes functools.partial usage, and adds type: ignore comments for mypy. It also introduces a validation check for tensorizer_dir when loading LoRA models and refines the parse_fine_tuned_lora_name utility. Significant changes in vllm/lora/model_manager.py involve introducing new abstract base classes (SupportsLoRAModel, SupportsMultiModalLoRAModel) to better define LoRA-compatible models, updating type hints for model parameters and internal caches (MutableMapping), and renaming LoRALRUCache to LoRAModelLRUCache while adding a NoneLRUCache. Finally, the PR removes several assert self.quant_method is not None and assert self.moe_quant_config is not None statements in quantization-related files, relying instead on explicit ValueError checks during initialization to ensure these configurations are present.

Comment thread vllm/lora/model_manager.py Outdated
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Dec 22, 2025

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

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 Dec 22, 2025
Copy link
Copy Markdown
Member

@yewentao256 yewentao256 left a comment

Choose a reason for hiding this comment

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

Please fix the conflicts so that we can get this landed

@hmellor
Copy link
Copy Markdown
Member Author

hmellor commented Dec 22, 2025

Sure, I was waiting to hear from @jeejeelee for what to do about the multimodal type hint.

@jeejeelee
Copy link
Copy Markdown
Collaborator

Sure, I was waiting to hear from @jeejeelee for what to do about the multimodal type hint.

Sorry for the delay response

@mergify mergify Bot removed the needs-rebase label Dec 23, 2025
@hmellor hmellor enabled auto-merge (squash) December 23, 2025 14:45
@hmellor hmellor added the ready ONLY add when PR is ready to merge/full CI is needed label Dec 23, 2025
@hmellor
Copy link
Copy Markdown
Member Author

hmellor commented Dec 23, 2025

Should be good now, I've started CI

Comment thread vllm/lora/model_manager.py Outdated
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Dec 29, 2025

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

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 Dec 29, 2025
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 4, 2026

Hi @hmellor, the pre-commit checks have failed. Please run:

uv pip install pre-commit
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy or markdownlint failing?
mypy and markdownlint are run differently in CI. If the failure is related to either of these checks, please use the following commands to run them locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10
# For markdownlint
pre-commit run --hook-stage manual markdownlint

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Mar 10, 2026

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

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 Mar 10, 2026
Copy link
Copy Markdown
Member

@yewentao256 yewentao256 left a comment

Choose a reason for hiding this comment

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

What's the current status? Could you solve the conflicts so that we can land this?

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@mergify mergify Bot removed the needs-rebase label May 5, 2026
hmellor added 2 commits May 5, 2026 11:54
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 5, 2026

Hi @hmellor, the pre-commit checks have failed. Please run:

uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-files

Then, commit the changes and push to your branch.

For future commits, pre-commit will run automatically on changed files before each commit.

Tip

Is mypy failing?
mypy is run differently in CI. If the failure is related to this check, please use the following command to run it locally:
# For mypy (substitute "3.10" with the failing version if needed)
pre-commit run --hook-stage manual mypy-3.10

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 5, 2026

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

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 May 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase 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