Skip to content

[None][fix] Allow fail-early when reuse block and legacy mamba cache - #15540

Closed
Wanli-Jiang wants to merge 1 commit into
NVIDIA:mainfrom
Wanli-Jiang:user/williamj/fail-fast-for-wrong-comb
Closed

[None][fix] Allow fail-early when reuse block and legacy mamba cache #15540
Wanli-Jiang wants to merge 1 commit into
NVIDIA:mainfrom
Wanli-Jiang:user/williamj/fail-fast-for-wrong-comb

Conversation

@Wanli-Jiang

@Wanli-Jiang Wanli-Jiang commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Enhanced error handling for KV cache configuration: now provides clear guidance when incompatible block reuse settings are detected, directing users to the supported cache manager option.

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

@Wanli-Jiang
Wanli-Jiang requested a review from a team as a code owner June 23, 2026 09:46
@Wanli-Jiang
Wanli-Jiang requested a review from lancelly June 23, 2026 09:46
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e0602af6-f0f6-40c0-ab09-5b59088d398b

📥 Commits

Reviewing files that changed from the base of the PR and between 31d4301 and 35a1a7d.

📒 Files selected for processing (1)
  • tensorrt_llm/_torch/pyexecutor/_util.py

📝 Walkthrough

Walkthrough

In KVCacheCreator._get_model_kv_cache_manager_cls, the previous warn-only log for enable_block_reuse=True on hybrid linear models routed to the legacy path is replaced with a ValueError that fails fast when the resolved cache manager class is MixedMambaHybridCacheManager, directing users to CppMambaHybridCacheManager.

Changes

Block reuse guard for legacy hybrid cache manager

Layer / File(s) Summary
ValueError guard replacing warn-only path
tensorrt_llm/_torch/pyexecutor/_util.py
Removes the legacy warning log emitted when enable_block_reuse=True is combined with the disagg/env/speculative-decoding routing conditions, and replaces it with a ValueError raised whenever the resolved KV cache manager class is MixedMambaHybridCacheManager, with an error message instructing use of CppMambaHybridCacheManager.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description was not filled in; it contains only the template with no explanation of the issue, solution, test coverage, or implementation details. Provide a description explaining the issue being fixed, the solution approach, and the test coverage for this change.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding fail-early detection for the unsupported combination of block reuse and legacy mamba cache.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

Signed-off-by: Wanli Jiang <35160485+Wanli-Jiang@users.noreply.github.com>
@Wanli-Jiang
Wanli-Jiang force-pushed the user/williamj/fail-fast-for-wrong-comb branch from 35a1a7d to 20458ff Compare June 23, 2026 09:50
@Wanli-Jiang

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #55222 [ run ] triggered by Bot. Commit: 20458ff Link to invocation

"Block reuse does not work with MTP for hybrid linear models "
"when using the legacy MambaCacheManager (TRTLLM_USE_CPP_MAMBA=1)"
)
and kv_cache_config.enable_block_reuse \

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.

Let's await this #15533 ci pipeline

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #55222 [ run ] completed with state SUCCESS. Commit: 20458ff
/LLM/main/L0_MergeRequest_PR pipeline #44181 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

@Wanli-Jiang

Copy link
Copy Markdown
Collaborator Author

the change will be merged into #16597

"KVCacheManagerV2 is not supported with %s. "
"Falling back to KVCacheManager.", incompat_str)
cls = KVCacheManager
# The V1-route hybrid mamba managers (disagg, TRTLLM_USE_CPP_MAMBA,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

actually we can drop this part since no longer used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants