[https://nvbugs/6435097][fix] Remove the single stale nvbugs/6427411 waiver line for…

[trtllm-agent]

Summary

• Root cause: PR [None][perf] Move greedy stop checks to host #15920 added a use_host_stop_criteria fast path that under-populated per-step/seq_slot/beam token buffers on non-last PP ranks, causing IndexError; PR [TRTLLM-14155][fix] Revert host-side greedy stop checks from #15920 #16163 (commit 9a8ec05) reverted PR [None][perf] Move greedy stop checks to host #15920 and is present at HEAD, so the code is already fixed. The waiver line for this test was left behind by cleanup PRs [https://nvbugs/6427411][chore] Waive failed tests in Pre-merge #16103/[https://nvbugs/6427411][chore] Waive a failed test in Pre-merge #16105/[https://nvbugs/6427411][chore] Waive remaining PP sampler regressions #16127.
• Fix: Remove the single stale nvbugs/6427411 waiver line for test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2] in waives.txt; do not touch other test entries or other bug IDs.
• 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/6435097

Dev Engineer Review

• Removed the stale nvbugs/6427411 waiver and SKIP marker for TestLlama3_1_8BInstruct::test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2].
• The change is limited to one waives.txt entry.
• The test-list format remains consistent.
• No other tests, bug references, or public entities changed.

QA Engineer Review

• No test-db/ or qa/ files were modified.
• Removed one entry from tests/integration/test_lists/waives.txt.
• The affected test will run without the obsolete waiver.
• Verdict: needs follow-up because CBTS coverage data is unavailable.

[coderabbitai]

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: f03ee1b4-3c6f-4391-97ad-521fdd0091f6

📥 Commits

Reviewing files that changed from the base of the PR and between 8e588da and f0380c6.

📒 Files selected for processing (1)

• tests/integration/test_lists/waives.txt

💤 Files with no reviewable changes (1)

• tests/integration/test_lists/waives.txt

---

Walkthrough

The integration waiver list removes the SKIP entry for the Llama 3.1 8B Instruct asymmetric context/parallelism test configuration.

Changes

Cohort / File(s)	Summary
Waiver list update tests/integration/test_lists/waives.txt	Removes the waiver and nvbugs reference for test_ctx_pp_gen_tp_asymmetric with GSM8K-gen_tp=1-ctx_pp=2.

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

Possibly related PRs

• NVIDIA/TensorRT-LLM#15585: Modifies the same integration waiver list.
• NVIDIA/TensorRT-LLM#17219: Removes an obsolete waiver entry from the same list.
• NVIDIA/TensorRT-LLM#17309: Removes a specific skipped-test waiver from the same list.

Suggested labels: ci: full pre-merge approved

Suggested reviewers: crazydemo, brnguyen2, mlefeb01

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description explains the root cause, the stale waiver removal, scope limits, test plan, and bug reference.
Title check	✅ Passed	The title identifies the bug, fix type, and removal of the stale waiver, but it ends with an ellipsis and an incomplete test reference.
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.}

[BowenFu]

LGTM — removes a stale skip; the under-populated-buffer regression it guarded was already reverted (#16163). No product code.

[mzweilz]

LGTM if the unwaived test passes.

[fredricz-20070104]

Review summary - Approve

Reviewed the full diff; no blocking or major issues found.

Minor, non-blocking notes:

• tests/integration/test_lists/waives.txt: Sibling waiver for the same bug id 6427411 left in place

Automated review by NVCortex Lite, run by @fredricz-20070104.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[brnguyen2]

Approving — the comments below are optional touch-ups, not blockers.

The waiver being removed is one of 29 lines in waives.txt citing nvbugs/6427411, including sibling parametrizations of the same test (gen_tp=2-ctx_pp=2, MMLU-gen_tp=1-ctx_pp=2, test_tp_pp_symmetric[GSM8K-tp1pp2]). If the revert in #16163 fixed the root cause, those are stale too, and the bug can't be closed while they remain; if only this one parametrization was verified, say so in the description so the rest aren't assumed handled.

Since this un-skips a post-merge test, trigger the owning stage explicitly (/bot run --extra-stage "...") rather than relying on the default pipeline.

[crazydemo]

Review summary - CONCERNS

Verdict: The change itself is correct and low-risk (removes one stale SKIP for a test whose root cause was reverted in #16163), but it cannot merge as-is: mergeable_state is dirty, so the branch needs a rebase, and the un-waived test is not shown to pass.

Issues

• [MINOR] tests/integration/test_lists/waives.txt:3 - sibling waiver for same bug id 6427411 left in place

QA view

• Test coverage: partial - only a SKIP entry is removed; nothing in the diff demonstrates the un-waived test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=1-ctx_pp=2] now passes against the #16163 revert.
• SM coverage: architecture-independent - test-list waiver edit only, no arch-guarded code.
• Test code: waiver hygiene is inconsistent - other 6427411 waivers remain in the same file.
• Test time: small - re-enables one disaggregated-serving accuracy parametrization in the post-merge suite.
• Needs /qa-verify: yes - unwaives a post-merge test with no in-diff proof it passes; trigger the owning stage explicitly and confirm green.

Does this actually fix nvbugs/6435097 / 6427411?

Partial. The real fix was the revert in #16163, present at HEAD; this PR only drops the SKIP so the test runs again, which is correct for this one parametrization. But bug 6427411 also guards sibling parametrizations still waived here (e.g. line 3 gen_tp=2-ctx_pp=2). If the revert fixed the shared root cause, those are stale too and 6427411 can't be closed while they remain; if only this parametrization was verified, the rest are uncovered. Please state the intended scope in the description.

Possible new issues

• Branch is in a merge-conflict (dirty) state and must be rebased before it can land.
• Re-enabling the test on main will fail CI if the revert did not fully cover the gen_tp=1-ctx_pp=2 path.

What I could not verify

Whether the un-waived test actually passes now, and whether the sibling 6427411 waivers share the same (now-fixed) root cause - neither is determinable from the one-line diff.

Automated review by NVCortex Lite, run by @crazydemo.

[crazydemo]

[MINOR] Sibling waiver for same bug id 6427411 left in place

This line still waives test_ctx_pp_gen_tp_asymmetric[GSM8K-gen_tp=2-ctx_pp=2] under the same nvbugs/6427411 that the removed line cited. If the revert in #16163 fixed the shared root cause (under-populated token buffers on non-last PP ranks), this and the other 6427411 waivers are equally stale and 6427411 cannot be closed while they remain. If instead only the gen_tp=1-ctx_pp=2 parametrization was actually verified, state that in the PR description so the remaining ones are not assumed handled. No change required to merge, but the scope should be made explicit.

[crazydemo]

Review summary - Approve (non-blocking)

Approving so this is not blocked on me. The points raised in my review comment above are non-blocking — please read them and address what you agree with before merging.

Worth doing before this is relied on: This unwaives a post-merge test without any in-diff proof it passes; QA should trigger the owning stage explicitly and confirm green before this is trusted, and confirm whether the sibling 6427411 waivers can also be removed.

Automated review by NVCortex Lite, run by @crazydemo.