[https://nvbugs/6535767][fix] Added an anonymous-namespace childTokensPerBlock(NodeBase const&) deriving…

[trtllm-agent]

Summary

• Root cause: addOrGetExistingBlock sized a new block via prev->tokensPerBlock(), a grandparent hop that derefs prev->prev — null once eviction detached prev while a live KvCache still held it — segfaulting in KvCache::_commitBlock.
• Fix: Added an anonymous-namespace childTokensPerBlock(NodeBase const&) deriving the value from prev alone (mirroring Python _block_radix_tree.py:390-393), used it at the call site and in Block::tokensPerBlock(), and removed the now-passing test's waiver.
• Automated fix generated by repair-bot

Test plan

• Verify fix on the same GPU type as the original failure
• Check for regressions in related tests

Links

• Bug: https://nvbugs/6535767

Dev Engineer Review

• Added childTokensPerBlock(NodeBase const&) to derive block size without accessing a detached predecessor.
• Updated Block::tokensPerBlock() and addOrGetExistingBlock() to use the helper.
• Delayed block-size calculation until after exact-child lookup.
• Removed the waiver for TestQwen3NextInstruct::test_nvfp4[tp1_block_reuse-cutlass].
• No public API or configuration changes were made.
• The change addresses detached parent state after eviction. The reported segmentation fault may also require a separate fix for reset_prefix_cache synchronization.

QA Engineer Review

• Modified tests/integration/test_lists/waives.txt.
• Removed TestQwen3NextInstruct::test_nvfp4[tp1_block_reuse-cutlass].
• No test-db/ or qa/ files were modified.
• CBTS coverage data is unavailable.

Verdict: needs follow-up

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

The block radix tree derives child block sizes directly from the parent and delays sizing until exact-child lookup. The skipped integration-test waiver entry was removed.

Changes

Block sizing and lookup

Layer / File(s)	Summary
Child block-size derivation and lookup cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/blockRadixTree.cpp, tests/integration/test_lists/waives.txt	childTokensPerBlock derives child sizes from the tree configuration or parent block. Block::tokensPerBlock() and addOrGetExistingBlock() use the helper. The waiver entry for TestQwen3NextInstruct::test_nvfp4[tp1_block_reuse-cutlass] was removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• NVIDIA/TensorRT-LLM#14047: Modifies related block-sizing logic in blockRadixTree.cpp.

Suggested reviewers: bowenfu, tburt-nv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the fix and references the relevant NVBugs issue.
Description check	✅ Passed	The description explains the root cause, fix, affected test, and test plan, but it does not include the full checklist template.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/blockRadixTree.cpp (1)

481-495: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard the full-block check for orphan parents.

When prev is an orphan Block, isFull() calls tokensPerBlock() with prev->prev == nullptr. With debug checks enabled, this fails before the exact-child lookup. Skip the check for orphan parents or retain block capacity independently of prev.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/blockRadixTree.cpp` around
lines 481 - 495, The full-block check in the block-creation path must not call
tokensPerBlock() for an orphan prev block whose prev pointer is null. Update the
logic around childTokensPerBlock and the exact-child lookup to skip or safely
handle that check for orphan parents, while preserving normal capacity
validation for attached parents.

Source: MCP tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/blockRadixTree.cpp`:
- Around line 481-495: The full-block check in the block-creation path must not
call tokensPerBlock() for an orphan prev block whose prev pointer is null.
Update the logic around childTokensPerBlock and the exact-child lookup to skip
or safely handle that check for orphan parents, while preserving normal capacity
validation for attached parents.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ddf6c527-9781-4468-b3d7-9b1fff5d2643

📥 Commits

Reviewing files that changed from the base of the PR and between 3304709 and b4b953b.

📒 Files selected for processing (2)

• cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/blockRadixTree.cpp
• tests/integration/test_lists/waives.txt

💤 Files with no reviewable changes (1)

• tests/integration/test_lists/waives.txt

[lowsfer]

The original bug symptom does not seems to be seg fault. It may actually be a bug I already fixed as part of #14047
See https://github.com/NVIDIA/TensorRT-LLM/pull/14047/changes#diff-0413424e359b84d8e2d7b2f6b85d795f9f680928dd6f38ea19faf733643bbe07 for mamba_cache_manager.py

[lowsfer]

The real bug for the segfault is because reset_prefix_cache did not wait until all _KVCache instances are closed. Some are still open and committing more blocks later when the radix tree is gone. I'll fix it in another PR.