[https://nvbugs/6418912][test] Fix KV cache reuse accounting tests

[chienchunhung]

Summary by CodeRabbit

• Bug Fixes

  • Updated reusable cache accounting so released-but-cached blocks continue to count toward reuse calculations.
  • Refined handling for shared-prefix and full-reuse cases, improving consistency in token budgeting and recomputation estimates.

• Tests

  • Adjusted automated coverage to match the updated reuse behavior.

Description

PR #15066 separated two reuse budgets: block-capacity accounting only discounts currently allocated reusable blocks, while estimatedReusableTokens credits every cached prefix block because those tokens do not need recomputation. Three existing tests still conflate those budgets and expect zero reusable tokens after the cached blocks are released.

Update the assertions and comments to expect two recoverable cached blocks, or 32 tokens. Production behavior is unchanged.

Following reviewer feedback, move cpp/test_unit_tests.py::test_unit_tests[batch_manager-80] from A30 post-merge coverage to a dedicated A30-CPP-1 pre-merge stage so active KV-cache development is gated before landing.

This was exposed while validating NVBUG 6418912 / PR #15994, but is independent of the BufferIndexHolder fix.

Affected tests

• KVCacheManagerReuseAccountingTest.CountReusableBlocksPartialMatch
• KVCacheManagerReuseAccountingTest.GetNeededBlocksOneStepWithFullReuse
• KVCacheManagerReuseAccountingTest.MultipleRequestsWithSharedPrefix

Test Coverage

• pre-commit run --files cpp/tests/unit_tests/batch_manager/kvCacheManagerTest.cpp jenkins/L0_Test.groovy tests/integration/test_lists/test-db/l0_a30.yml
• scripts/check_test_list.py --validate: 2,217 unique entries validated
• Stage mapping: batch_manager-80 resolves exclusively to A30-CPP-1
• Combined validation PR [https://nvbugs/6418912][fix] Fix KV-cache lifecycle test failures #16019 included this PR together with [https://nvbugs/6418912][fix] Prevent BufferIndexHolder test deadlock #15994, [https://nvbugs/6418912][test] Fix detached-block eviction tests #16011, and the independent sequence-link fix in [https://nvbugs/6418912][fix] Clear detached KV cache sequence links #16037.
• Targeted A30-CPP-1 pipeline #46619 completed SUCCESS on integration head 1248a186.
• The exact pre-merge wrapper passed: 1 pytest passed, and its batch-manager report completed 525 gtests with 0 failures.
• [https://nvbugs/6418912][fix] Clear detached KV cache sequence links #16037 documents the final end-to-end evidence; its formerly crashing FillKvCacheWithRequestsAndCompleteOneByOne/5 case passed in 1,739 ms.

Merge dependency

Because this PR activates batch_manager-80 in pre-merge, merge it after #15994, #16011, and #16037. Its earlier standalone pipeline #46561 lacked those fixes and is therefore not a valid final merge gate.

[coderabbitai]

📝 Walkthrough

Walkthrough

Test expectations in kvCacheManagerTest.cpp are updated to reflect free-but-cached KV-cache reuse semantics: released blocks with no active references now contribute to estimatedReusableTokens instead of yielding zero, with corresponding comment updates across three test cases.

Changes

KV-Cache Reuse Accounting Test Updates

Layer / File(s)	Summary
Free-but-cached reuse token assertions cpp/tests/unit_tests/batch_manager/kvCacheManagerTest.cpp	Updates estimatedReusableTokens expectations in CountReusableBlocksPartialMatch, GetNeededBlocksOneStepWithFullReuse, and MultipleRequestsWithSharedPrefix to reflect that released-but-cached blocks contribute reusable tokens, with matching comment clarifications.

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

Suggested reviewers: lancelly

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check	✅ Passed	The title is specific and matches the main change: fixing KV cache reuse accounting test expectations.
Description check	✅ Passed	The description includes the issue, solution, affected tests, and test coverage, with only the checklist section not explicitly filled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[SimengLiu-nv]

The test changes looks good to me.
I suggest moving cpp/test_unit_tests.py::test_unit_tests[batch_manager-80]  to pre_merge due to the active dev of kvcache manager and the importance of it.

[chienchunhung]

/bot run

[tensorrt-cicd]

PR_Github #57866 [ run ] triggered by Bot. Commit: 2ae8fa9 Link to invocation

[tensorrt-cicd]

PR_Github #57866 [ run ] completed with state FAILURE. Commit: 2ae8fa9
/LLM/main/L0_MergeRequest_PR pipeline #46561 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

CI Agent Failure Analysis

Link to invocation

[chzblych]

It doesn't always sound good to me if we see a test failure in post-merge, then we just add the test to pre-merge. When everyone does this, the pre-merge pipeline will be bloat eventually.

Perhaps the question is that why existing CPP tests on A10 in pre-merge cannot cover the failure? The fix looks like testing-only changes.

[SimengLiu-nv]

Hi @chzblych , kvcache cpp test is fundamental and important in my opinion. There are quite a lot of active development around kvcache. The whole suite does not take more than 15 mins to execute anyway. I strongly support putting it in the pre_merge stage.

[chienchunhung]

FYI this PR is subsumed by PR#16019, which is a superset of multiple fixes for the NVBUG.

I agree we should clearly define what tests should be included in pre-merge (run more frequently) vs. stay in post-merge (run more comprehensively) to strike the right balance between coverage and overhead. I will defer the decision to kv cache dev folks. cc: @thorjohnsen