[None][fix] Initialize KV cache fixtures and unwaive nine cases on all platforms by yizhang-nv · Pull Request #19382 · NVIDIA/TensorRT-LLM

yizhang-nv · 2026-09-18T02:17:04Z

Dev Engineer Review

The fixtures now initialize kv_connector_manager=None and is_estimating_kv_cache=False. Nine global waivers were removed, restoring two cached-token attribution cases and seven connector-prefix cases. No production or public API changes are included. Material review findings are unavailable.

QA Engineer Review

Two unit-test fixture files and tests/integration/test_lists/waives.txt changed. The waiver file removes nine KV-cache entries. The affected KV-cache manager suite is registered in test-db/l0_b200.yml, l0_cpu.yml, and l0_h100.yml; related connector-prefix coverage is registered in test-db/l0_a10.yml. Test results were not supplied. Coverage verdict: needs follow-up.

Per-File QA Perspective

tests/unittest/_torch/executor/kv_cache/test_kv_cache_manager_v2.py: Initializes kv_connector_manager for cached-token attribution tests. The suite is listed in the test-db CI files; verify the restored cases.
tests/unittest/_torch/executor/test_kv_connector_v2_prefix.py: Initializes is_estimating_kv_cache for connector-prefix tests. Related connector-prefix coverage is listed in test-db/l0_a10.yml; verify the restored cases across applicable platforms.
tests/integration/test_lists/waives.txt: Removes nine KV-cache waivers. Verify that all restored cases pass on applicable platforms.

Description

After #18583 and #18762 landed together, nine KV cache unit tests fail with AttributeError because their fixtures bypass KVCacheManagerV2.__init__. Initialize is_estimating_kv_cache=False in the connector fixture and kv_connector_manager=None in the cached-token attribution fixture so the tests reach their existing assertions.

The fixture fix adds two initialization lines in test setup. Failure example: main pre-merge #60944.

After #19384 merged, this branch was rebased onto origin/main including its merge commit ffddc5abd188f308adb5a2906112c2b1e7192d7d. This PR explicitly removes all nine global SKIP entries introduced by #19384: two cached-token attribution cases and seven connector-prefix cases (AskTiming, ReEntryAfterAServe, and SwaScratchReuse). These deletions restore the cases on all platforms where their existing CI lists select them, including H100 and B300 pre-merge. Jenkins applies explicit PR waiver deletions when combining the current main waiver list. The final diff is three files, +2/-9.

Test Coverage

Existing connector-prefix and cached-token attribution tests in tests/unittest/_torch/executor/test_kv_connector_v2_prefix.py and tests/unittest/_torch/executor/kv_cache/test_kv_cache_manager_v2.py cover both fixtures.
Normal commit hooks and DCO sign-off are retained for both commits.
Static inspection confirms the nine cases remain selected by existing GPU CI lists and are not disabled by broader waivers or case-level skip conditions. Fresh CI on the final rebased head is the requested runtime validation; earlier-head CI does not cover this unwaive.
Attempted both test files on umbriel-b200-034 on the earlier fixture-only head. Collection was blocked while loading tests/unittest/conftest.py: the existing libth_common.so could not resolve the PyTorch torch::Library::_def symbol. No tests executed in this local attempt; runtime validation remains pending CI.

PR Checklist

Please review the following before submitting your PR:

PR description clearly explains what and why.
PR follows the TRT-LLM coding guidelines.
Existing tests cover the changed fixture initialization.
No API, dependency, ownership, documentation, or architecture changes.
Reviewers are assigned according to CODEOWNERS.
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.

coderabbitai · 2026-09-18T02:19:31Z

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: a8f54342-29d6-41ef-9f88-b30f4b5129c7

📥 Commits

Reviewing files that changed from the base of the PR and between 8f499c4 and 743b06b.

📒 Files selected for processing (1)

tests/integration/test_lists/waives.txt

💤 Files with no reviewable changes (1)

tests/integration/test_lists/waives.txt

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.

Walkthrough

Two KV-cache test fixtures now initialize previously unset manager state. One sets kv_connector_manager to None. The other sets is_estimating_kv_cache to False. Nine related integration-test skip entries are removed.

Changes

KV-cache test fixture defaults

Layer / File(s)	Summary
Explicit KV-cache fixture defaults `tests/unittest/_torch/executor/kv_cache/test_kv_cache_manager_v2.py`, `tests/unittest/_torch/executor/test_kv_connector_v2_prefix.py`, `tests/integration/test_lists/waives.txt`	The admission manager fixture initializes `kv_connector_manager` to `None`. The prefix test manager fixture disables KV-cache estimation mode by default. Nine related test skip entries are removed.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Bug fix

Suggested reviewers: brnguyen2

Merge Risk: ⚪ Minimal · up to 743b0

The restored tests now receive the state required for their intended paths, with no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows the required [None][fix] format and clearly summarizes both fixture initialization and removal of the nine skipped cases.
Description check	✅ Passed	The description explains the issue, solution, affected tests, unwaive scope, test coverage, validation limits, and checklist status. It is sufficiently complete for this change.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

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

yizhang-nv · 2026-09-18T02:22:24Z

/bot run

tensorrt-cicd · 2026-09-18T02:30:32Z

PR_Github #74264 [ run ] triggered by Bot. Commit: 4292b39 Link to invocation

zhaoyangwang-nvidia

Approve with nit.

github-actions · 2026-09-18T02:34:57Z

Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements.

yizhang-nv · 2026-09-18T02:47:49Z

/bot kill

tensorrt-cicd · 2026-09-18T02:54:33Z

PR_Github #74271 [ kill ] triggered by Bot. Commit: 4292b39 Link to invocation

yizhang-nv · 2026-09-18T02:56:30Z

/bot run --disable-fail-fast --add-multi-gpu-test

tensorrt-cicd · 2026-09-18T02:59:27Z

PR_Github #74264 [ run ] completed with state ABORTED. Commit: 4292b39

Link to invocation

tensorrt-cicd · 2026-09-18T02:59:46Z

PR_Github #74271 [ kill ] completed with state SUCCESS. Commit: 4292b39
Successfully killed previous jobs for commit 4292b39

Link to invocation

tensorrt-cicd · 2026-09-18T03:03:35Z

PR_Github #74280 [ run ] triggered by Bot. Commit: 8f499c4 Link to invocation

yizhang-nv · 2026-09-18T03:05:01Z

/bot kill

tensorrt-cicd · 2026-09-18T03:12:20Z

PR_Github #74288 [ kill ] triggered by Bot. Commit: 8f499c4 Link to invocation

tensorrt-cicd · 2026-09-18T03:17:15Z

PR_Github #74280 [ run ] completed with state ABORTED. Commit: 8f499c4

Link to invocation

tensorrt-cicd · 2026-09-18T03:17:23Z

PR_Github #74288 [ kill ] completed with state SUCCESS. Commit: 8f499c4
Successfully killed previous jobs for commit 8f499c4

Link to invocation

yizhang-nv · 2026-09-18T03:22:29Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-18T03:30:08Z

PR_Github #74299 [ run ] triggered by Bot. Commit: 8f499c4 Link to invocation

Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>

Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com> # Conflicts: # tests/integration/test_lists/waives.txt

yizhang-nv · 2026-09-18T03:41:07Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-18T03:50:09Z

PR_Github #74308 [ run ] triggered by Bot. Commit: 743b06b Link to invocation

tensorrt-cicd · 2026-09-18T03:55:11Z

PR_Github #74299 [ run ] completed with state ABORTED. Commit: 8f499c4

Link to invocation

tensorrt-cicd · 2026-09-18T04:58:06Z

PR_Github #74308 [ run ] completed with state SUCCESS. Commit: 743b06b
/LLM/main/L0_MergeRequest_PR pipeline #61126 completed with status: 'SUCCESS'

CI Report

Link to invocation

yizhang-nv · 2026-09-18T05:02:50Z

/bot run --disable-fail-fast --add-multi-gpu-test

tensorrt-cicd · 2026-09-18T05:08:40Z

PR_Github #74321 [ run ] triggered by Bot. Commit: 743b06b Link to invocation

yizhang-nv · 2026-09-18T05:28:21Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-18T05:34:05Z

PR_Github #74328 [ run ] triggered by Bot. Commit: 743b06b Link to invocation

tensorrt-cicd · 2026-09-18T05:36:46Z

PR_Github #74321 [ run ] completed with state ABORTED. Commit: 743b06b

Link to invocation

tensorrt-cicd · 2026-09-18T06:00:25Z

PR_Github #74328 [ run ] completed with state SUCCESS. Commit: 743b06b
/LLM/main/L0_MergeRequest_PR pipeline #61145 completed with status: 'SUCCESS'

CI Report

Link to invocation

…st waives The directory-level waive added in NVIDIA#19381 for https://nvbugs/6800103 was left in place after NVIDIA#19382 fixed that bug, so every test under unittest/_torch/executor has been skipped in CI since 2026-09-18. Running the tree on main with the line removed gives 2436 passed and 54 failed. Replace the blanket entry with waives for exactly the failing tests, so the rest of the tree runs again: - test_kv_cache_budget_split.py: class-level waives for TestSplitGpuBudgetForDraft and TestExternalDrafterKvDtype, tracked under the existing https://nvbugs/6800223 (same stale-fixture cause). - test_mamba_cache_manager.py: 16 test functions whose fixture lacks the `mapping` attribute production now reads. - test_kv_cache_v2_vocab_size.py: 11 parametrizations whose fixture lacks `_max_cuda_graph_batch_size`. - test_pytorch_model_engine.py: test_prepare_tp_inputs_with_helix_parallelism, base update_helix_param signature lacks helix_owned_new_tokens. The last three are tracked under https://nvbugs/6818710. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

yizhang-nv requested review from a team as code owners September 18, 2026 02:17

yizhang-nv requested review from YihuiLu512, nvpohanh, thorjohnsen and zhaoyuanh-nvidia September 18, 2026 02:17

github-actions Bot assigned yizhang-nv Sep 18, 2026

yizhang-nv mentioned this pull request Sep 18, 2026

[None][test] Temporarily waive nine KV cache regressions on all platforms #19384

Merged

1 task

eopXD approved these changes Sep 18, 2026

View reviewed changes

zhaoyangwang-nvidia approved these changes Sep 18, 2026

View reviewed changes

Comment thread tests/unittest/_torch/executor/kv_cache/test_kv_cache_manager_v2.py

trtllm-agent added the ci: full pre-merge approved label Sep 18, 2026

yizhang-nv changed the title ~~[None][fix] Initialize KV cache test fixture state~~ [None][fix] Initialize KV cache fixtures and unwaive nine cases on all platforms Sep 18, 2026

yizhang-nv force-pushed the codex/fix-kv-cache-test-fixtures branch from 4292b39 to 8f499c4 Compare September 18, 2026 02:55

[None][fix] Initialize KV cache test fixture state

e5a8fc2

Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>

[None][test] Unwaive nine KV cache fixture cases on all platforms

743b06b

Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com> # Conflicts: # tests/integration/test_lists/waives.txt

yizhang-nv force-pushed the codex/fix-kv-cache-test-fixtures branch from 8f499c4 to 743b06b Compare September 18, 2026 03:39

This was referenced Sep 18, 2026

[None][fix] Preserve KV transfer outcomes independently of polling #19377

Draft

[None][fix] Retire KV transfer ownership after late backend completion #19378

Draft

yizhang-nv enabled auto-merge (squash) September 18, 2026 05:28

yizhang-nv merged commit 9f26cc3 into NVIDIA:main Sep 18, 2026
11 checks passed

brnguyen2 mentioned this pull request Sep 22, 2026

[None][ci] Replace blanket unittest/_torch/executor waive with per-test waives #19533

Merged

3 tasks

Conversation

yizhang-nv commented Sep 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dev Engineer Review

QA Engineer Review

Per-File QA Perspective

Description

Test Coverage

PR Checklist

GitHub Bot Help

Uh oh!

coderabbitai Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

zhaoyangwang-nvidia left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

yizhang-nv commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

tensorrt-cicd commented Sep 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yizhang-nv commented Sep 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 18, 2026 •

edited

Loading