[Bugfix] Fix 2 precommit issues - (mamba_block_size, kv_cache_config) by tlrmchlsmth · Pull Request #27811 · vllm-project/vllm

tlrmchlsmth · 2025-10-30T13:18:36Z

Fix two precommit issues to get main green

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

tlrmchlsmth · 2025-10-30T13:20:13Z

vllm/model_executor/models/config.py

            return

-        # Save the user input before it gets modified by MambaModelConfig
-        mamba_block_size = vllm_config.cache_config.mamba_block_size


Broken in #27809

@tdoublep do you know why we needed to save the mamba_block_size to begin with?

Yes: https://github.com/vllm-project/vllm/pull/27809/files#r2478085718

#27289 (comment) is the specific thread with the explanation

gemini-code-assist

Code Review

This pull request addresses two pre-commit issues. The removal of the unused mamba_block_size variable in vllm/model_executor/models/config.py is a good cleanup. However, the change in vllm/v1/core/sched/scheduler.py introduces a critical bug. It attempts to fix a potential linting issue by assigning an object of type KVCacheConfig to an attribute that expects a CacheConfig object. This will likely lead to runtime errors. I've left a comment with a suggestion to revert this change.

gemini-code-assist · 2025-10-30T13:20:23Z

vllm/v1/core/sched/scheduler.py


            connector_vllm_config = copy.copy(self.vllm_config)
-            connector_vllm_config.kv_cache_config = copy.copy(kv_cache_config)
+            connector_vllm_config.cache_config = copy.copy(kv_cache_config)


This change appears to introduce a type error. The connector_vllm_config is an instance of VllmConfig, which has a cache_config attribute of type CacheConfig. The kv_cache_config variable is of type KVCacheConfig. These two types are not compatible.

Assigning kv_cache_config to connector_vllm_config.cache_config will likely cause AttributeError exceptions downstream in any code that expects a CacheConfig object, as their attributes are different. For example, CacheConfig has block_size and cache_dtype, while KVCacheConfig has num_blocks and kv_cache_groups.

The previous code connector_vllm_config.kv_cache_config = copy.copy(kv_cache_config) was dynamically adding an attribute, which is allowed but might have been flagged by a linter. If the connector expects a kv_cache_config attribute, the previous implementation was functionally correct. This change seems to fix a linting issue by introducing a runtime bug.

I suggest reverting this change and potentially adding a # type: ignore or # noqa to address the linting warning if that was the original problem.

Suggested change

connector_vllm_config.cache_config = copy.copy(kv_cache_config)

connector_vllm_config.kv_cache_config = copy.copy(kv_cache_config)

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

tlrmchlsmth · 2025-10-30T13:23:34Z

vllm/v1/core/sched/scheduler.py


            connector_vllm_config = copy.copy(self.vllm_config)
-            connector_vllm_config.kv_cache_config = copy.copy(kv_cache_config)
+            connector_vllm_config.cache_config = copy.copy(kv_cache_config)


@KuntaiDu any ideas what's going on here?

Context is here: https://github.com/vllm-project/vllm/pull/25712/files#r2447539735

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

DarkLight1337 · 2025-10-30T13:36:59Z

Some other pre-commit errors are caused by #27108, we can fix them separately

NickLucche · 2025-10-30T13:42:17Z

@GuanLuo had a similar one

GuanLuo · 2025-10-30T13:46:09Z

vllm/v1/core/sched/scheduler.py

Suggested change

return self.connector.request_finished(request, block_ids) # type: ignore[attr-defined]

Should be able to just ignore the type check here, this line will not be hit at the current state (no connector implements HMA interface).

For future reference, I think request_finished_all_groups should be called here as it is defined in SupportHMA interface and has the correct function signature.

Switched to request_finished_all_groups

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

yewentao256

LGTM, thanks for the fix!

vllm/v1/core/sched/scheduler.py

Co-authored-by: Nick Hill <nhill@redhat.com> Signed-off-by: Tyler Michael Smith <tysmith@redhat.com>

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

into tms/fix_precommit Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

simon-mo · 2025-10-30T18:52:23Z

Force merge to unbreak main

…vllm-project#27811) Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com> Signed-off-by: Tyler Michael Smith <tysmith@redhat.com> Co-authored-by: Nick Hill <nhill@redhat.com>

Fix precommit

9ce4949

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

tlrmchlsmth requested review from ApostaC, WoosukKwon, alexm-redhat, comaniac, heheda12345, njhill, robertgshaw2-redhat and ywang96 as code owners October 30, 2025 13:18

mergify bot added the v1 label Oct 30, 2025

tlrmchlsmth commented Oct 30, 2025

View reviewed changes

gemini-code-assist bot reviewed Oct 30, 2025

View reviewed changes

maybe this is the fix

7866fdc

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

tlrmchlsmth changed the title ~~[Bugfix] Fix 2 precommit issues - (unused mamba_block_size, connector_vllm_config.kv_cache_config)~~ [Bugfix] Fix 2 precommit issues - (mamba_block_size, kv_cache_config) Oct 30, 2025

tlrmchlsmth commented Oct 30, 2025

View reviewed changes

tlrmchlsmth added 2 commits October 30, 2025 13:25

update

f9b06dc

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

update

4924449

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

DarkLight1337 approved these changes Oct 30, 2025

View reviewed changes

DarkLight1337 enabled auto-merge (squash) October 30, 2025 13:37

DarkLight1337 disabled auto-merge October 30, 2025 13:37

DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 30, 2025

GuanLuo reviewed Oct 30, 2025

View reviewed changes

tlrmchlsmth added 3 commits October 30, 2025 13:52

bandaid

8436b1b

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

better

016b68d

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

add assert

16eeb2a

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

yewentao256 approved these changes Oct 30, 2025

View reviewed changes

isharif168 mentioned this pull request Oct 30, 2025

Transform HF interleaved weights to halves in vllm #27024

Closed

bbrowning mentioned this pull request Oct 30, 2025

[CI/Build] Add common tool call parser test suite #27599

Open

njhill reviewed Oct 30, 2025

View reviewed changes

vllm/v1/core/sched/scheduler.py Outdated Show resolved Hide resolved

vllm/v1/core/sched/scheduler.py Outdated Show resolved Hide resolved

tlrmchlsmth and others added 3 commits October 30, 2025 13:07

Update vllm/v1/core/sched/scheduler.py

13eb9f9

Co-authored-by: Nick Hill <nhill@redhat.com> Signed-off-by: Tyler Michael Smith <tysmith@redhat.com>

update

111b37b

Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

Merge branch 'tms/fix_precommit' of https://github.com/vllm-project/vllm

6b07a03

into tms/fix_precommit Signed-off-by: Tyler Michael Smith <tlrmchlsmth@gmail.com>

njhill approved these changes Oct 30, 2025

View reviewed changes

njhill enabled auto-merge (squash) October 30, 2025 17:19

simon-mo disabled auto-merge October 30, 2025 18:52

simon-mo merged commit ab98f65 into main Oct 30, 2025
47 of 58 checks passed

simon-mo deleted the tms/fix_precommit branch October 30, 2025 18:52

Kay-Tian mentioned this pull request Oct 31, 2025

vLLM PR #27811 变更核心文件提醒 Kay-Tian/vllm#71

Closed

This was referenced Oct 31, 2025

[KV Connector] Make KVCacheConfig an explicit constructor argument #27887

Merged

Add ORCA endpoint load metrics support #24905

Merged

[Misc] Refactor Attention kv transfer methods into decorator #27816

Merged

	connector_vllm_config.cache_config = copy.copy(kv_cache_config)
	connector_vllm_config.kv_cache_config = copy.copy(kv_cache_config)


	return self.connector.request_finished(request, block_ids) # type: ignore[attr-defined]

Uh oh!

Conversation

tlrmchlsmth commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tlrmchlsmth Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

tdoublep Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

hmellor Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

tlrmchlsmth Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

tlrmchlsmth Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

DarkLight1337 commented Oct 30, 2025

Uh oh!

NickLucche commented Oct 30, 2025

Uh oh!

GuanLuo Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

tlrmchlsmth Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

yewentao256 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

simon-mo commented Oct 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

tlrmchlsmth commented Oct 30, 2025 •

edited

Loading