[trtllm-agent]

Dev Engineer Review

The GLM-5.2 test now scopes rank environment overrides while constructing LLM and passes the yielded mapping as env_overrides. This avoids leaving those overrides in the caller’s process environment after the test. The waiver for test_modeling_v2_env_scope.py was removed. No unrelated files changed.

QA Engineer Review

The accuracy test changed its engine setup; its test ID did not change. The test-list change removes the waiver for unittest/_torch/modeling_v2/test_modeling_v2_env_scope.py. The GLM-5.2 test is listed in tests/integration/test_lists/test-db/l0_gb300_multi_nodes_node2_gpu8.yml. Coverage verdict: sufficient for the environment-scope correction. The full accuracy test was not rerun; reported stubbed checks do not establish end-to-end coverage.

Per-File QA Perspective

• tests/integration/defs/accuracy/modeling_v2/glm_moe_dsa/glm_5_2_nvfp4__sm_103__tp8/test_accuracy.py: The GSM8K TP8 test checks GLM-5.2 engine behavior with scoped environment overrides. It is listed in the test-db CI list.
• tests/integration/test_lists/waives.txt: Removes the environment-scope test waiver, allowing that test to run without this skip entry. This list change does not alter test functions or selectors.

Description

LLM writes env_overrides into the calling process's os.environ (BaseLLM._process_env_overrides) and does not restore them. The modeling_v2 accuracy tests therefore open each engine inside scoped_env_overrides (tests/integration/defs/accuracy/modeling_v2/_env.py), and tests/unittest/_torch/modeling_v2/test_modeling_v2_env_scope.py checks that every LLM(...) call under accuracy/modeling_v2/ is built that way.

TestModelingV2Glm52Nvfp4Sm103Tp8, added in #20524 and moved to glm_moe_dsa/glm_5_2_nvfp4__sm_103__tp8/test_accuracy.py in #21603, passes env_overrides to an engine built outside such a scope. TRTLLM_MODELING_V2_VALIDATE and TRTLLM_DSA_INDEXER_BF16 therefore stay set in the pytest process after the test, and on main the check fails:

AssertionError: glm_moe_dsa/glm_5_2_nvfp4__sm_103__tp8/test_accuracy.py constructs LLM outside an environment scope at lines [73]

CI does not report it because the whole check file is waived under https://nvbugs/7024242. That waiver was added when the check failed on test_minimax_m3.py, whose engine #20623 has since scoped.

This PR opens the GLM-5.2 sm_103 tp8 engine inside scoped_env_overrides and passes it the yielded mapping, as _env.py documents, and removes the waiver.

Fixes https://nvbugs/7024242

Test Coverage

• unittest/_torch/modeling_v2/test_modeling_v2_env_scope.py, selected by tests/integration/test_lists/test-db/l0_cpu.yml for the CPU-Generic stages, covers this change. It was run on main and on this branch with the same waives.txt (main's without the line this PR removes), in the CI image with a wheel built from main using the jenkins/Build.groovy x86_64 flags, and the CPU stage's pytest command line:
  • main: test_accuracy_engines_are_inside_environment_scopes[glm_moe_dsa/glm_5_2_nvfp4__sm_103__tp8/test_accuracy.py] fails, the other 31 cases pass;
  • this PR: all 32 cases pass.
• The l0_cpu shard that holds the check (CPU-Generic-x86-2, 184 entries, one pytest process): main 1 failed (this check), 182 passed, 1 skipped; this PR 183 passed, 1 skipped (unittest/_torch/modeling, skipped by waives.txt in both runs).
• TestModelingV2Glm52Nvfp4Sm103Tp8::test_gsm8k_tp8 runs on 2x4 GB300 (l0_gb300_multi_nodes_node2_gpu8.yml) and was not rerun. With this change its module collects, and with the engine and the GSM8K task stubbed, _evaluate passes the engine the same arguments as on main; the two variables are set while the engine is built, evaluated and closed, and are unset in the test process afterwards (on main they stay set).

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.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The GLM-5.2 accuracy test now scopes rank environment overrides during LLM construction. The skip-list entry for test_modeling_v2_env_scope.py is removed.

Changes

GLM-5.2 environment scoping

Layer / File(s)	Summary
Scope environment overrides during LLM construction tests/integration/defs/accuracy/modeling_v2/glm_moe_dsa/glm_5_2_nvfp4__sm_103__tp8/test_accuracy.py, tests/integration/test_lists/waives.txt	The test applies _SM103_RANK_ENV through scoped_env_overrides when constructing LLM, rather than returning the overrides from _llm_kwargs. The waiver for test_modeling_v2_env_scope.py is removed.

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

Possibly related PRs

• NVIDIA/TensorRT-LLM#21345: Adds scoped_env_overrides and updates Modeling V2 accuracy tests to use it.
• NVIDIA/TensorRT-LLM#21812: Uses TRTLLM_DSA_INDEXER_BF16 to select BF16 projection behavior in the GLM indexer.
• NVIDIA/TensorRT-LLM#21603: Expands test_modeling_v2_env_scope.py to check accuracy-test LLM() calls for use of scoped_env_overrides.

Merge Risk: Minimal · up to d1a2d

The environment overrides are scoped around the engine lifecycle. No merge-blocking issue is established.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title follows the repository format and clearly describes the main change: scoping the GLM-5.2 sm_103 tp8 accuracy engine’s environment overrides.
Description check		The description explains the issue and solution, lists relevant test coverage and results, and includes the completed PR checklist. It also states that the GPU test was not rerun.
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. 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

No eligible functions found in the changed files. Skipping docstring coverage check. 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 0 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: cf3b00b
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 33f98dcc-81f2-4886-a075-96c79c1b4adc

Commits

Reviewing files that changed from the base of the PR and between cf3b00b and d1a2d07.

Files selected for processing: 2

• tests/integration/defs/accuracy/modeling_v2/glm_moe_dsa/glm_5_2_nvfp4__sm_103__tp8/test_accuracy.py
• 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.