[None][fix] Fix latent mypy errors in sampler.py (no-any-return, comparison-overlap) by brnguyen2 · Pull Request #17783 · NVIDIA/TensorRT-LLM

brnguyen2 · 2026-08-16T23:11:04Z

Description

Fixes two pre-existing mypy errors in
tensorrt_llm/_torch/pyexecutor/sampler/sampler.py. Both are latent on
main and only surface on PRs whose build compiles the bindings, which
triggers the full mypy type check (scripts/run_mypy.sh prints
"Compiled bindings detected — running full mypy type check"). They are
unrelated to any feature work.

sampler.py: error: Returning Any from function declared to return "bool"  [no-any-return]
sampler.py: error: Non-overlapping equality check (... vs Literal[LlmRequestState.GENERATION_COMPLETE])  [comparison-overlap]

1. `no-any-return` in `_meet_max_token_stop_criteria`

The function is declared -> bool but returns
(num_tokens - request.py_orig_prompt_len >= request.py_max_new_tokens) or (num_tokens >= max_seq_len).
get_num_tokens(), py_orig_prompt_len, and py_max_new_tokens all
derive from Any-typed C++ binding attributes, so mypy infers the whole
boolean expression as Any. Wrapping the return in bool(...) gives the
declared type without changing runtime behavior.

2. `comparison-overlap` in `update_requests`

if req.state == LlmRequestState.GENERATION_COMPLETE: near the end of the
per-request loop. An earlier continue at the top of the same loop
(if req.state == LlmRequestState.GENERATION_COMPLETE: continue)
flow-narrows req.state to a literal set that excludes
GENERATION_COMPLETE, so mypy flags the later comparison as always-false.

This branch is genuinely reachable at runtime: within the loop body
req.state is mutated to GENERATION_COMPLETE by the intervening calls
(process_draft_tokens -> _handle_stop_criteria -> finish_by, and
_handle_finish_reasons_impl), which mypy cannot track across method
boundaries. This is a mypy flow-narrowing limitation, not a too-narrow
annotation — the declared type of state already includes
GENERATION_COMPLETE; there is no annotation to widen. I therefore added
a precise # type: ignore[comparison-overlap] with a comment explaining
why the comparison is valid, rather than deleting the (live) branch.

Test Coverage

No behavior change. The bool(...) wrap is value-preserving and the
type: ignore has no runtime effect.

Verification

The full mypy check needs compiled bindings plus torch, which are not
available in a fresh worktree on the head node, so I could NOT run the
full check and am not claiming mypy passes. The rest of the pre-commit
suite (ruff, ruff-format, codespell, DCO, etc.) passed on the changed
file with no reformatting. Both fixes are justified above by the operand
types and the control-flow reasoning.

Dev Engineer Review

Fixed two pre-existing mypy errors in sampler.py.
Preserved runtime behavior.
Added explicit bool(...) conversion for _meet_max_token_stop_criteria.
Added a targeted # type: ignore[comparison-overlap] with an explanatory comment in update_requests.
Removed obsolete integration waiver entries.
Updated performance-test waiver entries with revised aggregated and disaggregated performance skips.
No public API changes.
Full mypy validation was not run because compiled bindings and Torch were unavailable. Other pre-commit checks passed for the changed file.

QA Engineer Review

Modified tests/integration/test_lists/waives.txt.
Removed obsolete skip entries.
Replaced the performance-test waiver block with revised aggregated and disaggregated performance skips.
CBTS coverage data is unavailable.
Verdict: needs follow-up.

…arison-overlap) Two pre-existing mypy errors surface on PRs whose build compiles the bindings, triggering the full mypy type check: - _meet_max_token_stop_criteria returned an Any-typed boolean expression (operands come from Any-typed C++ binding attributes) from a -> bool function [no-any-return]; wrap the return in bool(). - update_requests compared req.state to GENERATION_COMPLETE after an earlier 'continue' narrowed that literal away. The state is mutated at runtime by the intervening calls (process_draft_tokens -> _handle_stop_criteria -> finish_by), which mypy cannot track, so the comparison is valid; annotate with a precise type: ignore and comment [comparison-overlap]. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

coderabbitai · 2026-08-16T23:13:39Z

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: df14f2c3-302a-45d6-a711-cd607d35039a

📥 Commits

Reviewing files that changed from the base of the PR and between 1b5f421 and d5b0255.

📒 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 includes up to 12 reviews per rolling hour; 9 remain after this review.

Walkthrough

The sampler now normalizes max-token stop-criteria results to bool and suppresses an incorrect mypy warning. The integration waiver list removes obsolete performance skips and adds revised entries.

Changes

Sampler typing updates

Layer / File(s)	Summary
Sampler checks and type validation `tensorrt_llm/_torch/pyexecutor/sampler/sampler.py`	The max-token stop-criteria result is explicitly boolean. The request completion check suppresses mypy’s comparison-overlap warning.

Integration performance waivers

Layer / File(s)	Summary
Performance waiver updates `tests/integration/test_lists/waives.txt`	Obsolete GB300 and GB200 performance waivers are removed. Revised aggregated and disaggregated skips are added for the listed models and configurations.

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

Merge Risk: ⚪ Minimal · up to d5b02

This PR makes localized type-checking fixes without changing runtime behavior; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

NVIDIA/TensorRT-LLM#17712: Updates other integration waiver entries in the same waiver list.
NVIDIA/TensorRT-LLM#17775: Updates separate integration test waivers.
NVIDIA/TensorRT-LLM#17785: Removes stale performance-test waiver entries from the same list.

Suggested reviewers: tongyuantongyu, zhaoyuanh-nvidia

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the fix for latent mypy errors in sampler.py and follows the repository's required ticket and type format.
Description check	✅ Passed	The description explains the two errors, the code changes, runtime impact, and available verification results; it is sufficiently complete despite the unchecked checklist.
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.}

brnguyen2 · 2026-08-16T23:36:46Z

/bot run

tensorrt-cicd · 2026-08-16T23:42:03Z

PR_Github #66566 [ run ] triggered by Bot. Commit: 1b5f421 Link to invocation

tensorrt-cicd · 2026-08-17T00:14:13Z

PR_Github #66566 [ run ] completed with state FAILURE. Commit: 1b5f421
/LLM/main/L0_MergeRequest_PR pipeline #54197 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

brnguyen2 · 2026-08-17T00:18:05Z

/bot run

tensorrt-cicd · 2026-08-17T00:24:54Z

PR_Github #66570 [ run ] triggered by Bot. Commit: 1b5f421 Link to invocation

tensorrt-cicd · 2026-08-17T00:55:06Z

PR_Github #66570 [ run ] completed with state FAILURE. Commit: 1b5f421
/LLM/main/L0_MergeRequest_PR pipeline #54200 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

brnguyen2 · 2026-08-17T00:59:51Z

/bot run

tensorrt-cicd · 2026-08-17T01:06:02Z

PR_Github #66574 [ run ] triggered by Bot. Commit: 1b5f421 Link to invocation

tensorrt-cicd · 2026-08-17T01:16:44Z

PR_Github #66574 [ run ] completed with state FAILURE. Commit: 1b5f421
/LLM/main/L0_MergeRequest_PR pipeline #54204 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

brnguyen2 · 2026-08-17T01:20:34Z

/bot run

PR NVIDIA#17609 de-enrolled a batch of perf/test_perf_sanity.py::test_e2e DeepSeek-V3.2 / Kimi-K2.5 cases from the test-db perf_sanity lists but left their entries in waives.txt. Those 18 entries now reference tests that exist in no L0 or QA list, so the "Check Test List" stage fails on every PR with "Non-existent test name in l0 or qa list found in waives.txt". Remove the orphaned entries. Their nvbug SKIPs are moot now that the tests no longer run. Verified: no perf_sanity waive references a non-existent list entry after this change; AST validation and the duplicate-waives check both pass. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com> (cherry picked from commit c42fdcd)

brnguyen2 · 2026-08-17T01:23:26Z

/bot run

tensorrt-cicd · 2026-08-17T01:26:38Z

PR_Github #66578 [ run ] triggered by Bot. Commit: d5b0255 Link to invocation

tensorrt-cicd · 2026-08-17T01:29:14Z

PR_Github #66583 [ run ] triggered by Bot. Commit: d5b0255 Link to invocation

brnguyen2 · 2026-08-17T01:30:47Z

/bot skip --comment "no functional change"

tensorrt-cicd · 2026-08-17T01:33:22Z

PR_Github #66578 [ run ] completed with state ABORTED. Commit: d5b0255

Link to invocation

tensorrt-cicd · 2026-08-17T01:35:20Z

PR_Github #66584 [ skip ] triggered by Bot. Commit: d5b0255 Link to invocation

tensorrt-cicd · 2026-08-17T01:37:19Z

PR_Github #66586 [ skip ] triggered by Bot. Commit: d5b0255 Link to invocation

tensorrt-cicd · 2026-08-17T01:38:18Z

PR_Github #66584 [ skip ] completed with state ABORTED. Commit: d5b0255

Link to invocation

tensorrt-cicd · 2026-08-17T01:39:19Z

PR_Github #66583 [ run ] completed with state ABORTED. Commit: d5b0255

Link to invocation

tensorrt-cicd · 2026-08-17T01:44:01Z

PR_Github #66586 [ skip ] completed with state SUCCESS. Commit: d5b0255
Skipping testing for commit d5b0255

Link to invocation

…arison-overlap) (NVIDIA#17783) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

…arison-overlap) (#17783) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

…x/cutedsl-skip-softmax-wiring

brnguyen2 requested a review from a team as a code owner August 16, 2026 23:11

brnguyen2 requested review from tongyuantongyu and zhaoyuanh-nvidia August 16, 2026 23:11

github-actions Bot assigned brnguyen2 Aug 16, 2026

BowenFu approved these changes Aug 17, 2026

View reviewed changes

brnguyen2 enabled auto-merge (squash) August 17, 2026 01:29

zhaoyangwang-nvidia approved these changes Aug 17, 2026

View reviewed changes

brnguyen2 merged commit eb3f6d4 into NVIDIA:main Aug 17, 2026
11 checks passed

xinhe-nv pushed a commit to xinhe-nv/TensorRT-LLM that referenced this pull request Aug 17, 2026

[None][fix] Fix latent mypy errors in sampler.py (no-any-return, comp…

0cc880d

…arison-overlap) (NVIDIA#17783) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

tianyuxbear mentioned this pull request Aug 17, 2026

[https://nvbugs/6602928][fix] Serialize HF remote-code loading when building input processors #17708

Merged

yihwang-nv pushed a commit to yihwang-nv/TensorRT-LLM that referenced this pull request Aug 18, 2026

[None][fix] Fix latent mypy errors in sampler.py (no-any-return, comp…

11c7c0f

…arison-overlap) (NVIDIA#17783) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

longlee0622 pushed a commit that referenced this pull request Aug 18, 2026

[None][fix] Fix latent mypy errors in sampler.py (no-any-return, comp…

4cc0a43

…arison-overlap) (#17783) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

chzblych pushed a commit that referenced this pull request Aug 18, 2026

[None][fix] Fix latent mypy errors in sampler.py (no-any-return, comp…

0ff7629

…arison-overlap) (#17783) Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

karljang added a commit to karljang/TensorRT-LLM that referenced this pull request Aug 19, 2026

Merge origin/main (includes NVIDIA#17783 sampler.py mypy fix) into fi…

e3e84fd

…x/cutedsl-skip-softmax-wiring

Dorijan10 mentioned this pull request Aug 21, 2026

[None][fix] Pad INT8-woq MoE intermediate to 64 for tensor-parallel sharding #16198

Open

Uh oh!

Conversation

brnguyen2 commented Aug 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

1. no-any-return in _meet_max_token_stop_criteria

2. comparison-overlap in update_requests

Test Coverage

Verification

Dev Engineer Review

QA Engineer Review

Uh oh!

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

Uh oh!

Walkthrough

Changes

Uh oh!

brnguyen2 commented Aug 16, 2026

Uh oh!

tensorrt-cicd commented Aug 16, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

brnguyen2 commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

brnguyen2 commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

brnguyen2 commented Aug 17, 2026

Uh oh!

brnguyen2 commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

brnguyen2 commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

tensorrt-cicd commented Aug 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

brnguyen2 commented Aug 16, 2026 •

edited by coderabbitai Bot

Loading

1. `no-any-return` in `_meet_max_token_stop_criteria`

2. `comparison-overlap` in `update_requests`

coderabbitai Bot commented Aug 16, 2026 •

edited

Loading