[yuxianq]

Dev Engineer Review

The test fixtures now use BatchState and SampleStateTorch for previous-batch state, and set capturable_guided_decoder to None in the executor fixture. This aligns the fixtures with fields read during previous-batch processing. The diff contains no production code changes.

QA Engineer Review

The patch modifies one test file and tests/integration/test_lists/waives.txt. It removes six waivers for cancellation-boundary, deferred LoRA generation, and overlap-marker tests. The test file does not appear in the searched test-db/ CI or qa/ manual-QA lists. Test results for the current head are not available. Coverage verdict: sufficient for the fixture repair and restored tests.

Per-File QA Perspective

• tests/unittest/_torch/disaggregation/test_disagg_idle_cancellation.py: Updates previous-batch fixtures used by cancellation and token-harvest tests. The file does not appear in the searched test-db/ CI or qa/ manual-QA lists.
• tests/integration/test_lists/waives.txt: Removes six NVBug 7071872 skip entries for this test file, restoring those cases from waiver status without changing their assertions.

Description

After #20903, previous-batch processing reads BatchState.guided_failures, but the disaggregation cancellation fixtures construct partial SimpleNamespace batch states. Six CPU tests fail with AttributeError before reaching their cancellation and token-harvest assertions.

Use the real BatchState and SampleStateTorch defaults and explicitly set capturable_guided_decoder = None in the executor fixture. Remove the six waivers added by #21963 for NVBug 7071872, restoring these cases to the existing CPU CI selection.

This is an independent fixture repair based directly on main at 11131ecfbe; it addresses the shared failures observed while validating #19396.

Test Coverage

Run with normal native runtime initialization on x86, with GPUs hidden:

CUDA_VISIBLE_DEVICES='' python3 -m pytest -m cpu_only tests/unittest/_torch/disaggregation/test_disagg_idle_cancellation.py -q --tb=short

• Before the fix, at fc41ebd016: 6 failed, 79 passed; all six failures reproduce the waived guided_failures error.
• With this patch on 11131ecfbe: 85 passed, no skips.
• Pre-commit and git diff --check passed.
• Test consolidation: 85 cases before and after, with no new cases, parameter expansion, or weakened assertions. The six restored cases cover context/generation cancellation, already-completed requests, deferred LoRA token harvesting, and the empty-batch overlap marker. The suite remains under the existing l0_cpu.yml directory selection. Local pytest reported 10.10 seconds for the fixed suite; this is not a CI timing measurement.

PR Checklist

• Description explains the failure and repair.
• Changes follow the coding guidelines and carry DCO sign-off.
• Existing regression coverage passes and the six addressed waivers are removed.
• Please check this after reviewing the above items as appropriate for this PR.

[yuxianq]

/bot run --disable-fail-fast

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

This change updates disaggregation idle-cancellation tests to build previous_batch with BatchState, adds a fixture attribute, and removes six related integration waivers.

Changes

Disaggregation idle-cancellation tests

Layer / File(s)	Summary
BatchState test setup and waiver removal tests/unittest/_torch/disaggregation/test_disagg_idle_cancellation.py, tests/integration/test_lists/waives.txt	Three tests now build previous_batch with BatchState instead of SimpleNamespace. The executor fixture sets capturable_guided_decoder to None. Six waiver entries for disaggregated idle-cancellation tests were removed.

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

Change: Other

Suggested reviewers: chienchunhung

Merge Risk: Minimal · up to e4897

This change only repairs test fixtures and re-enables six previously waived tests. It does not alter production behavior, and no merge-blocking risk remains.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title follows the required NVBugs ID and lowercase type format. It clearly describes the fixture alignment change.
Description check		The description explains the failure and repair, lists relevant test coverage and results, and completes the PR checklist.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 1 files.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/TensorRT-LLM
• Branch: main
• File: .coderabbit.yaml
• Commit: a1c22fc
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: e353c290-14d4-47b2-8363-c19fdd2ee721

Commits

Reviewing files that changed from the base of the PR and between bf410f4 and e4897ba.

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: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

[tensorrt-cicd]

PR_Github #83773 [ run ] triggered by Bot. Commit: bf410f4 Link to invocation

[sunnyqgg]

I checked the fixture repair against the production path it unblocks. After #20903, _process_previous_batch reads BatchState.guided_failures / guided_batch_id and PyExecutor.capturable_guided_decoder (py_executor.py:7613-7615, 11917-11924). With the real BatchState defaults plus capturable_guided_decoder = None, _take_capturable_guided_failures returns [], so each of the six restored cases now reaches its own assertions. The six removed lines are exactly the #21963 waives for nvbugs/7071872, and the pre-merge CPU stage still collects the file (l0_cpu.yml:114). Counts: 0 blockers, 0 possibly blocking, 2 required, 0 nice-to-have. The L0 run for bf410f4 (PR_Github #83773) was still queued when I wrote this, so please merge only after the CPU stage has run the six restored cases green.

• [Required] PR description: the auto-generated CodeRabbit block says "the tests are not found in the test-db/ CI or qa/ manual-QA lists" and "The file is not found in the test-db/ CI or qa/ manual-QA lists". That is wrong. tests/integration/test_lists/test-db/l0_cpu.yml:114 (- unittest/_torch/disaggregation, the pre-merge CPU stage) collects this cpu_only file. The six cases failed there in L0_MergeRequest_PR #69106 (stage CPU-Generic-x86-2), which is why they were waived, and your own Test Coverage section says the suite stays under the l0_cpu.yml directory selection. Please delete or correct those CodeRabbit lines, so the PR does not claim that the restored cases never run in CI.

[sunnyqgg]

[Required] This is about the six test_disagg_idle_cancellation.py waivers removed just above. The auto-waive PR #21958 for the same bug (nvbugs/7071872) is still open, and it adds these same six cases again as full:CPU/unittest/_torch/disaggregation/test_disagg_idle_cancellation.py::... SKIP (https://nvbugs/7071872) lines.

Its hunk sits in the full: section of this file (around lines 87-93, next to full:CPU/unittest/scripts/test_subprocess_timeouts.py), which this PR does not touch. The prefixed lines are also different text from the unprefixed ones removed here, so #21958 still applies cleanly after this PR and the duplicate-line check does not catch it. If it is merged later, the six restored cases are skipped again on the CPU stage, the only stage that collects this file (l0_cpu.yml:114). Nothing in CI would flag it, and the bug would look fixed.

Please close #21958 as superseded by #21963 plus this PR, or link it here and ask the bot owner to close it.

[tensorrt-cicd]

PR_Github #83773 [ run ] completed with state SUCCESS. Commit: bf410f4
/LLM/main/L0_MergeRequest_PR pipeline #69270 completed with status: 'SUCCESS'

CI Report

Link to invocation

[yuxianq]

/bot run --disable-fail-fast

[tensorrt-cicd]

PR_Github #83840 [ run ] triggered by Bot. Commit: e4897ba Link to invocation