[None][fix] Install with the venv interpreter, not sys.executable by brnguyen2 · Pull Request #18773 · NVIDIA/TensorRT-LLM

brnguyen2 · 2026-09-06T16:40:06Z

Description

scripts/build_wheel.py --install ran pip install -e .[devel] with sys.executable rather than the venv_python that setup_venv() had just created. A fresh checkout has to start build_wheel.py with the system interpreter, so the package landed in the system site-packages and the newly built venv was left without it.

The -m build invocation a few lines earlier already uses venv_python, so this looks like an oversight rather than a deliberate choice.

The call moves into a small install_editable_package() helper, purely so the interpreter choice can be covered by a unit test without running a build.

Split out of #18752, where it was an unrelated drive-by.

Test Coverage

tests/unittest/others/test_build_wheel_install_interpreter.py (new, 1 case, CPU-only, no subprocesses): patches build_run and asserts the install command is issued with the venv interpreter and not with sys.executable.

$ pytest tests/unittest/others/test_build_wheel_install_interpreter.py -q
1 passed

PR Checklist

Please review the following before submitting your PR:

PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

Dev Engineer Review

install_editable_package(venv_python) now uses the virtual environment interpreter for editable installation.
The --install path no longer installs into the system interpreter environment.
The helper improves unit-test coverage.
No configuration or test-list changes were identified.
The change is focused and consistent with the stated objective.

QA Engineer Review

Added coverage for editable installation with the virtual environment interpreter.
The test verifies that the generated pip command uses venv_python and not sys.executable.
No test-list entry was identified for this unit test.
Verdict: sufficient.

coderabbitai · 2026-09-06T16:48:25Z

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: 1c5b35a9-d70f-4446-b608-28ea02fb5754

📥 Commits

Reviewing files that changed from the base of the PR and between 70feda6 and beb5941.

📒 Files selected for processing (2)

scripts/build_wheel.py
tests/unittest/others/test_build_wheel_install_interpreter.py

Included review availability: Your plan provides up to 12 included reviews per hour; 5 remain after this review.

Walkthrough

The build script now installs the editable package with the configured virtual-environment interpreter. The --install flow uses the new helper, and a test verifies the generated pip command.

Changes

Editable package installation

Layer / File(s)	Summary
Virtual environment installation helper `scripts/build_wheel.py`	Added `install_editable_package(venv_python)` to run the editable package installation with the supplied virtual-environment interpreter.
Install flow integration and validation `scripts/build_wheel.py`, `tests/unittest/others/test_build_wheel_install_interpreter.py`	Updated the `--install` path to call the helper. Added a test that verifies one pip command uses the virtual-environment interpreter and not `sys.executable`.

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

Merge Risk: ⚪ Minimal · up to 45a44

The editable install path now targets the build virtual environment rather than the interpreter that launched the script, with focused coverage for that command selection. No concrete current-head merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title follows the required [None][fix] format and clearly describes the primary change: using the virtual-environment interpreter instead of sys.executable.
Description check	✅ Passed	The description explains the issue, solution, affected behavior, test coverage, and validation result. It includes the required sections and checklist.
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-09-06T17:27:01Z

/bot run

tensorrt-cicd · 2026-09-06T17:34:43Z

PR_Github #71784 [ run ] triggered by Bot. Commit: beb5941 Link to invocation

tensorrt-cicd · 2026-09-06T18:05:53Z

PR_Github #71784 [ run ] completed with state SUCCESS. Commit: beb5941
/LLM/main/L0_MergeRequest_PR pipeline #58863 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-09-06T18:09:04Z

/bot run

tensorrt-cicd · 2026-09-06T18:15:13Z

PR_Github #71790 [ run ] triggered by Bot. Commit: beb5941 Link to invocation

tensorrt-cicd · 2026-09-06T18:44:56Z

PR_Github #71790 [ run ] completed with state SUCCESS. Commit: beb5941
/LLM/main/L0_MergeRequest_PR pipeline #58869 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-09-06T18:50:40Z

/bot run --disable-fail-fast

tensorrt-cicd · 2026-09-06T18:56:17Z

PR_Github #71795 [ run ] triggered by Bot. Commit: beb5941 Link to invocation

tensorrt-cicd · 2026-09-06T20:09:45Z

PR_Github #71795 [ run ] completed with state SUCCESS. Commit: beb5941
/LLM/main/L0_MergeRequest_PR pipeline #58874 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-09-06T20:13:45Z

/bot run

tensorrt-cicd · 2026-09-06T20:21:02Z

PR_Github #71803 [ run ] triggered by Bot. Commit: beb5941 Link to invocation

tensorrt-cicd · 2026-09-06T20:52:14Z

PR_Github #71803 [ run ] completed with state SUCCESS. Commit: beb5941
/LLM/main/L0_MergeRequest_PR pipeline #58882 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-09-06T20:55:19Z

/bot run

tensorrt-cicd · 2026-09-06T21:01:28Z

PR_Github #71808 [ run ] triggered by Bot. Commit: beb5941 Link to invocation

tensorrt-cicd · 2026-09-06T21:33:21Z

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

`build_wheel.py --install` ran `pip install -e .[devel]` with `sys.executable` rather than the `venv_python` that `setup_venv()` had just created. A fresh checkout has to start the script with the system interpreter, so the package landed in the system site-packages and the newly built venv was left without it. The `-m build` invocation a few lines earlier already uses `venv_python`, so this was an oversight rather than a choice. The call moves into a small helper so the interpreter choice can be covered by a unit test without running a build. Signed-off-by: Brian Nguyen <brnguyen@nvidia.com>

brnguyen2 · 2026-09-07T03:00:06Z

/bot run

tensorrt-cicd · 2026-09-07T03:07:08Z

PR_Github #71837 [ run ] triggered by Bot. Commit: 45a4472 Link to invocation

tensorrt-cicd · 2026-09-07T07:53:22Z

PR_Github #71837 [ run ] completed with state SUCCESS. Commit: 45a4472
/LLM/main/L0_MergeRequest_PR pipeline #58914 completed with status: 'UNSTABLE'

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

Link to invocation

brnguyen2 · 2026-09-07T13:44:30Z

/bot skip --comment "The 16 failures are unrelated infrastructure flakes, not caused by this change. This PR only switches the editable-install to the venv interpreter (scripts/build_wheel.py) and adds one unit test; if it were broken the build/install stages would fail and imports would break suite-wide. Instead both x86 and SBSA builds passed and 52074 tests passed with 0 regressions. The failures were confined to two of 208 x86 shards: DGX_H100-PyTorch-1 died on a UCX connection reset (Connection reset by remote peer) mid-test, and DGX_H100-PyTorch-3 died when Jenkins agent node cw-dfw-cs-001-login-02 went offline (ChannelClosedException, slurm job cancelled). Both are infra events on unrelated multimodal/accuracy tests; the PR-added test_build_wheel_install_interpreter.py passed. Skipping."

tensorrt-cicd · 2026-09-07T13:50:25Z

PR_Github #71959 [ skip ] triggered by Bot. Commit: 45a4472 Link to invocation

tensorrt-cicd · 2026-09-07T14:00:15Z

PR_Github #71959 [ skip ] completed with state SUCCESS. Commit: 45a4472
Skipping testing for commit 45a4472

Link to invocation

github-actions Bot assigned brnguyen2 Sep 6, 2026

brnguyen2 marked this pull request as ready for review September 6, 2026 16:42

brnguyen2 requested review from a team as code owners September 6, 2026 16:42

brnguyen2 requested review from BowenFu, QiJune, mlefeb01 and mzweilz September 6, 2026 16:42

BowenFu approved these changes Sep 7, 2026

View reviewed changes

chzblych approved these changes Sep 7, 2026

View reviewed changes

brnguyen2 force-pushed the fix/build-wheel-install-uses-venv-python branch from beb5941 to 45a4472 Compare September 7, 2026 02:59

brnguyen2 enabled auto-merge (squash) September 7, 2026 13:44

brnguyen2 merged commit 99e8195 into NVIDIA:main Sep 7, 2026
8 checks passed

Uh oh!

Conversation

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

Uh oh!

Description

Test Coverage

PR Checklist

GitHub Bot Help

Dev Engineer Review

QA Engineer Review

Uh oh!

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

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

brnguyen2 commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

brnguyen2 commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

brnguyen2 commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

brnguyen2 commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

brnguyen2 commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

tensorrt-cicd commented Sep 6, 2026

Uh oh!

brnguyen2 commented Sep 7, 2026

Uh oh!

tensorrt-cicd commented Sep 7, 2026

Uh oh!

tensorrt-cicd commented Sep 7, 2026

Uh oh!

brnguyen2 commented Sep 7, 2026

Uh oh!

tensorrt-cicd commented Sep 7, 2026

Uh oh!

tensorrt-cicd commented Sep 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

brnguyen2 commented Sep 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 6, 2026 •

edited

Loading