[lori-ren]

Dev Engineer Review

model_engine.py adopts standard Ruff formatting and Python 3.10 annotation syntax. The supplied change summary reports no intended behavior changes. Preserve eager annotation evaluation, call signatures, import side effects, and execution and cleanup order.

The configuration changes remove model_engine.py from legacy formatting and file-selection patterns. The author reports that full pre-commit passed and that AST comparison and repeated formatting checks confirmed the intended mechanical changes. The author also reports rerunning the listed tests on H100 after rebasing. These results are author-reported.

QA Engineer Review

No test changes.

Per-File QA Perspective

• .pre-commit-config.yaml: Removes model_engine.py from two hook file patterns. Verify standard hooks select the file as intended.
• legacy-files.txt: Removes model_engine.py from the legacy-file list. Verify legacy handling no longer applies to this file.
• pyproject.toml: Removes the Ruff formatting exclusion for model_engine.py. Verify the configured formatter accepts the file.
• ruff-legacy.toml: Removes model_engine.py from the legacy Ruff include list. Verify formatter scope matches the other configuration changes.
• tensorrt_llm/_torch/pyexecutor/model_engine.py: Updates annotations and formatting. Verify runtime behavior and API compatibility remain unchanged.

Description

model_engine.py still uses the legacy formatting and lint toolchain and older typing aliases. Move it to standard Ruff and Python 3.10 annotation syntax while preserving eager annotation evaluation, call signatures, import side effects, and execution and cleanup order.

Test Coverage

Reran test_pytorch_model_engine.py, test_pytorch_model_engine_warmup.py, engine/test_runners.py, and test_eager_workspace_engine.py on H100 after rebasing onto main at 0b10da8224 (PR head 3c09e8dc36).

Full pre-commit passed inside the container on this rebased head. AST comparison and repeated formatting checks confirmed the intended mechanical changes.

PR Checklist

• Reviewed the applicable coding, test coverage, API, dependency, documentation, and ownership requirements.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/TensorRT-LLM
• Branch: main
• File: .coderabbit.yaml
• Commit: 5be3334
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 73dbd5c6-f770-4f0d-8882-29016263bdea

Commits

Reviewing files that changed from the base of the PR and between 3c09e8d and ec4edef.

Files selected for processing: 1

• tensorrt_llm/_torch/pyexecutor/model_engine.py

Files skipped from review as they are similar to previous changes: 1

• tensorrt_llm/_torch/pyexecutor/model_engine.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

ModelEngine now uses modern Python type annotations and reformatted expressions. The file is no longer selected by the listed legacy and formatting exclusions. The supplied summaries report no changes to its execution behavior.

Changes

ModelEngine modernization

Layer / File(s)	Summary
Engine setup and runner configuration .pre-commit-config.yaml, legacy-files.txt, pyproject.toml, ruff-legacy.toml, tensorrt_llm/_torch/pyexecutor/model_engine.py	The file was removed from legacy and formatting-exclusion lists. Its initialization, compilation setup, and runner configuration use updated annotations and formatting.
Runtime methods and multimodal handling tensorrt_llm/_torch/pyexecutor/model_engine.py	Metadata access, callable registration, warmup, encoder forwarding, and multimodal handling use updated annotations and formatting. Existing checks and outcomes remain unchanged.
Compilation, refit, and execution lifecycle tensorrt_llm/_torch/pyexecutor/model_engine.py	Sequence-length inference, graph release, compilation, refit, forward execution, and userbuffer initialization are reformatted. Their existing conditions and behavior remain unchanged.

Priority: Low

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

Change: Refactor

Suggested reviewers: juney-nvidia, chzblych

Merge Risk: Minimal · up to ec4ed

The typing and formatting changes have no identified merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description check		The description explains the change, states the preservation requirements, lists relevant tests and validation steps, and includes a completed checklist. It is sufficiently complete for this refactori…
Title check		The title identifies the ticket, refactor type, removal from legacy formatting, and modernization of type annotations. It clearly summarizes the main changes.
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: Description check

Explanation

The description explains the change, states the preservation requirements, lists relevant tests and validation steps, and includes a completed checklist. It is sufficiently complete for this refactoring pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. 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 38 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

[lori-ren]

/bot run

[github-actions]

👎 Promotion blocked, new vulnerability found

Vulnerability report

Component	Vulnerability	Description	Severity
py-filelock	CVE-2026-112906	filelock is a platform-independent file lock for Python. From 4.1.0 until 4.2.1, SoftFileLock versions from 4.1.0 can reuse a stale observation when an attacker pre-dates and removes its own marker, at least two lock-user processes are waiting, coarse or matching modification times preserve the observation, and an honest contender creates a fresh marker on the reused inode, causing that live marker to be removed as stale. This issue is fixed in version 4.2.1.	MEDIUM
py-filelock	CVE-2026-112940	filelock is a platform-independent file lock for Python. From 3.20.4 until 4.2.1, ReadWriteLock let SQLite reopen the database and side files by name; on POSIX systems an attacker with rename access in a non-sticky parent can swap those names to cause overwrite or unlink operations, while a writable adjacent hot journal is additionally required for attacker-chosen content overwrites. This issue is fixed in version 4.2.1.	HIGH

[brnguyen2]

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

Checked the mechanical claim independently: the module AST matches the pre-PR file once annotations and the typing/collections.abc imports are stripped (the one remaining diff is the docstring wrap in get_max_num_sequences), ruff 0.9.4 passes check and format --check, there is no from __future__ import annotations so evaluation stays eager, and scripts/legacy_utils.py check-configs plus ruff-legacy-baseline.json have no leftover reference to the file.

One description fix: the "stacked on #21066, rebase and repeat validation after it merges" paragraph is stale. #21066 is already on the base of this branch and the diff is just these five files. Please drop that paragraph and state whether the H100 test runs listed under Test Coverage were done on the current base.

[lori-ren]

/bot run

[tensorrt-cicd]

PR_Github #81299 [ run ] triggered by Bot. Commit: ec4edef Link to invocation

[github-actions]

Automatically added "ci: full pre-merge approved" because this PR has satisfied the required GitHub review approvals. Unresolved review conversations and other required checks remain independent merge requirements.

[tensorrt-cicd]

PR_Github #81299 [ run ] completed with state FAILURE. Commit: ec4edef
/LLM/main/L0_MergeRequest_PR pipeline #66991 completed with status: 'FAILURE'

CI Report

⚠️ Resource Maintenance — Test Coverage Reduced:

Resource maintenance in LLM/main/L0_Test-x86_64-Single-GPU #9631: L40S-PyTorch-2 is temporarily paused due to Kubernetes agent provisioning and node issues.
Affected stages:
L40S-PyTorch-2

⚠️ 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

[lori-ren]

/bot help

[github-actions]

GitHub Bot Help

/bot [-h] ['run', 'kill', 'skip', 'reuse-pipeline'] ...

Provide a user friendly way for developers to interact with a Jenkins server.

Run /bot [-h|--help] to print this help message.

See details below for each supported subcommand.

Details

run [--reuse-test (optional)pipeline-id --disable-fail-fast --skip-test --stage-list "A10-PyTorch-1, xxx" --gpu-type "A30, H100_PCIe" --test-backend "pytorch, cpp" --add-multi-gpu-test --only-multi-gpu-test --disable-multi-gpu-test --post-merge --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" --detailed-log --debug(experimental) --high-priority]

Launch build/test pipelines. All previously running jobs will be killed.

--reuse-test (optional)pipeline-id (OPTIONAL) : Allow the new pipeline to reuse build artifacts and skip successful test stages from a specified pipeline or the last pipeline if no pipeline-id is indicated. If the Git commit ID has changed, this option will be always ignored. The DEFAULT behavior of the bot is to reuse build artifacts and successful test results from the last pipeline.

--disable-reuse-test (OPTIONAL) : Explicitly prevent the pipeline from reusing build artifacts and skipping successful test stages from a previous pipeline. Ensure that all builds and tests are run regardless of previous successes.

--disable-fail-fast (OPTIONAL) : Disable fail fast on build/tests/infra failures.

--skip-test (OPTIONAL) : Skip all test stages, but still run build stages, package stages and sanity check stages. Note: Does NOT update GitHub check status.

--stage-list "A10-PyTorch-1, xxx" (OPTIONAL) : Only run the specified test stages. Supports wildcard * for pattern matching (e.g., "*PerfSanity*" matches all stages containing PerfSanity). Examples: "A10-PyTorch-1, xxx", "PerfSanity". The patterns "*", "*Post-Merge*", and "*PerfSanity*", including equivalent escaped or repeated-star forms and their use in comma-separated lists, require the ci: post-merge approved PR label. Note: Does NOT update GitHub check status.

--gpu-type "A30, H100_PCIe" (OPTIONAL) : Only run the test stages on the specified GPU types. Examples: "A30, H100_PCIe". Note: Does NOT update GitHub check status.

--test-backend "pytorch, cpp" (OPTIONAL) : Skip test stages which don't match the specified backends. Only support [pytorch, cpp, tensorrt, triton]. Examples: "pytorch, cpp" (does not run test stages with tensorrt or triton backend). Note: Does NOT update GitHub pipeline status.

--only-multi-gpu-test (OPTIONAL) : Only run the multi-GPU tests. Requires the ci: full pre-merge approved label on the PR (ask a member of NVIDIA/trt-llm-ci-approvers). Note: Does NOT update GitHub check status.

--disable-multi-gpu-test (OPTIONAL) : Disable the multi-GPU tests. Note: Does NOT update GitHub check status.

--add-multi-gpu-test (OPTIONAL) : Force run the multi-GPU tests in addition to running L0 pre-merge pipeline. Requires the ci: full pre-merge approved label on the PR (ask a member of NVIDIA/trt-llm-ci-approvers).

--post-merge (OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline. Requires the ci: post-merge approved PR label applied by an active member of NVIDIA/trt-llm-ci-approvers. The approval label remains in place when new commits are pushed.

--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx" (OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Supports wildcard * for pattern matching. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx", --extra-stage "Post-Merge". The patterns "*", "*Post-Merge*", and "*PerfSanity*", including equivalent escaped or repeated-star forms and their use in comma-separated lists, require the ci: post-merge approved PR label.

--detailed-log (OPTIONAL) : Enable flushing out all logs to the Jenkins console. This will significantly increase the log volume and may slow down the job.

--debug (OPTIONAL) : Experimental feature. Enable access to the CI container for debugging purpose. Note: Specify exactly one stage in the stage-list parameter to access the appropriate container environment. Note: Does NOT update GitHub check status.

--high-priority (OPTIONAL) : Run the pipeline with high priority. This option is restricted to authorized users only and will route the job to a high-priority queue.

kill

kill

Kill all running builds associated with pull request.

skip

skip --comment COMMENT

Skip testing for latest commit on pull request. --comment "Reason for skipping build/test" is required. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

reuse-pipeline

reuse-pipeline

Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break.

[lori-ren]

/bot run

[tensorrt-cicd]

PR_Github #81394 [ run ] triggered by Bot. Commit: ec4edef Link to invocation

[tensorrt-cicd]

PR_Github #81394 [ run ] completed with state SUCCESS. Commit: ec4edef
/LLM/main/L0_MergeRequest_PR pipeline #67069 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

[lori-ren]

/bot run

[tensorrt-cicd]

PR_Github #81426 [ run ] triggered by Bot. Commit: ec4edef Link to invocation

[tensorrt-cicd]

PR_Github #81426 [ run ] completed with state SUCCESS. Commit: ec4edef
/LLM/main/L0_MergeRequest_PR pipeline #67102 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

[lori-ren]

/bot run

[tensorrt-cicd]

PR_Github #81453 [ run ] triggered by Bot. Commit: ec4edef Link to invocation

[tensorrt-cicd]

PR_Github #81453 [ run ] completed with state SUCCESS. Commit: ec4edef
/LLM/main/L0_MergeRequest_PR pipeline #67127 completed with status: 'SUCCESS'

CI Report

Link to invocation