Skip to content

[https://nvbugs/6811958][chore] Unwaive test_build_capture_manifest_matches_committed_golden - #20057

Merged
farazkh80 merged 1 commit into
NVIDIA:mainfrom
farazkh80:am/nvbug-6811958-manifest
Oct 16, 2026
Merged

farazkh80 merged 1 commit into
NVIDIA:mainfrom
farazkh80:am/nvbug-6811958-manifest

Conversation

@farazkh80

@farazkh80 farazkh80 commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Unwaive unittest/usage/test_llmapi_config_telemetry_docs.py::test_build_capture_manifest_matches_committed_golden (https://nvbugs/6811958) and regenerate tensorrt_llm/usage/llm_args_golden_manifest.json.

The original stale manifest (#19024 added sparse_attention_config.uses_spcompress) was regenerated in #19493, but the manifest has since drifted again: #19811 changed the capture-policy derivation for nested model-list fields (list[...] wrapper for cuda_graph_config.extra_model_inputs.shape / encoder_cuda_graph_config.extra_model_inputs.shape) without regenerating, and the waiver hid it. Unwaiving without regenerating would re-break the SBSA CPU stage, so both changes are in one commit.

Test Coverage

CPU-only, on ToT:

  • python3 scripts/generate_llm_args_golden_manifest.py → diff empty after this change
  • pytest tests/unittest/usage/test_llmapi_config_telemetry_docs.py → 19 passed (fails on the committed manifest with the "manifest is stale" message before this change)
  • pytest tests/unittest/usage/test_llmapi_config_capture.py tests/unittest/usage/test_llmapi_model_sequences.py → 74 passed

Covered in CI by l0_cpu.yml (unittest/usage).

PR Checklist

  • PR title follows the required format
  • PR description explains what and why
  • Test coverage listed
  • Related documentation N/A
  • Confirmed CODING_GUIDELINES.md and no large files / secrets

Dev Engineer Review

  • The waiver removal enables the committed-manifest check. The manifest summary reports a new boolean entry for TorchLlmArgs.sparse_attention_config.gvr_deterministic_topk.
  • The PR description says the manifest was regenerated to address capture-policy changes and a later field addition. The supplied CI comments report subsequent failures in pipelines #64214 and #64247, but do not identify the failing tests. Their cause and resolution remain unverified.
  • The reported CPU test results and manifest-generation result are author-reported. The shell inspection found no uncommitted diff, so it does not independently confirm those results or the change contents.

QA Engineer Review

  • Modified test-list file: tests/integration/test_lists/waives.txt. It removes the waiver for unittest/usage/test_llmapi_config_telemetry_docs.py::test_build_capture_manifest_matches_committed_golden.
  • The test checks the committed capture manifest. tests/integration/test_lists/test-db/l0_cpu.yml lists unittest/usage.
  • The PR description reports 19 telemetry-doc tests and 74 related capture and model-sequence tests passing. It also reports that manifest generation produced no diff after the change. These results are author-reported. Later merge-request pipeline failures remain unexplained.
  • Coverage verdict: needs follow-up. Confirm the manifest is current on the target branch and investigate the reported pipeline failures.

Per-File QA Perspective

  • tests/integration/test_lists/waives.txt: Removing the waiver enables the golden-manifest check to run. The test is under unittest/usage, which l0_cpu.yml lists.
  • tensorrt_llm/usage/llm_args_golden_manifest.json: The change summary reports the addition of the boolean sparse_attention_config.gvr_deterministic_topk entry. Verify the committed manifest matches the generator on the target branch.

@farazkh80
farazkh80 requested a review from a team as a code owner October 9, 2026 15:18
@farazkh80
farazkh80 requested a review from arysef October 9, 2026 15:18
@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/TensorRT-LLM/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 38af9e57-ef47-4d78-a2a8-92e49c78f867
📥 Commits

Reviewing files that changed from the base of the PR and between 67cff13 and 824306a.

📒 Files selected for processing: 1
  • tensorrt_llm/usage/llm_args_golden_manifest.json

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

The committed TorchLlmArgs usage manifest now records sparse_attention_config.gvr_deterministic_topk as a boolean. The integration-test waiver list no longer skips the capture-manifest golden test.

Changes

Capture Manifest Golden Test

Layer / File(s) Summary
Update manifest and remove test waiver
tensorrt_llm/usage/llm_args_golden_manifest.json, tests/integration/test_lists/waives.txt
The manifest adds the boolean value for sparse_attention_config.gvr_deterministic_topk. The waiver list removes the skip entry for the capture-manifest golden test.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: juney-nvidia

Merge Risk: ⚪ Minimal · up to 82430

The manifest records the sparse-attention boolean, and the enabled golden test checks the complete generated manifest. No material merge risk is evident.

Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Title check Passed The title clearly identifies the NVBugs issue, change type, and primary change: removing the waiver for the manifest golden test.
Description check Passed The description explains the change and its cause, lists relevant test coverage, and completes the applicable checklist items. It provides the required information despite using a Summary heading inst…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 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.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #76978 [ run ] triggered by Bot. Commit: 269cdb6 Link to invocation

@farazkh80 farazkh80 added the api-compatible Accepted LLM API contract change that is backwards-compatible label Oct 9, 2026
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #76978 [ run ] completed with state SUCCESS. Commit: 269cdb6
/LLM/main/L0_MergeRequest_PR pipeline #63462 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

@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@longcheng-nv

Copy link
Copy Markdown
Collaborator

Heads-up: #20100 adds a new auto-enrolling llm-args field (sparse_attention_config.gvr_deterministic_topk), so if it lands before this PR, please regenerate the golden manifest once more before merging — thanks!

@farazkh80
farazkh80 force-pushed the am/nvbug-6811958-manifest branch from 269cdb6 to a532821 Compare October 13, 2026 14:06
@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77537 [ run ] triggered by Bot. Commit: a532821 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77537 [ run ] completed with state SUCCESS. Commit: a532821
/LLM/main/L0_MergeRequest_PR pipeline #63919 completed with status: 'SUCCESS'

CI Report

Link to invocation

@longcheng-nv

Copy link
Copy Markdown
Collaborator

Follow-up with confirmation: this branch is still based on a pre-#20100 main (CI checks out the PR head directly, so pipeline #63919 is green only because llm_args.py here doesn't have sparse_attention_config.gvr_deterministic_topk yet). On current main that bool auto-enrolls into the capture manifest while the committed golden doesn't contain it, so merging this unwaive as-is would break test_build_capture_manifest_matches_committed_golden in post-merge CI. Please rebase onto latest main and run python3 scripts/generate_llm_args_golden_manifest.py once more before merging — thanks!

@farazkh80
farazkh80 force-pushed the am/nvbug-6811958-manifest branch from a532821 to 67cff13 Compare October 14, 2026 19:26
@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77902 [ run ] triggered by Bot. Commit: 67cff13 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77902 [ run ] completed with state SUCCESS. Commit: 67cff13
/LLM/main/L0_MergeRequest_PR pipeline #64214 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

@QiJune

QiJune commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

/bot run

@QiJune

QiJune commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

@farazkh80 Please cherry pick this fix to release/1.3 as well.

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77939 [ run ] triggered by Bot. Commit: 67cff13 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #77939 [ run ] completed with state FAILURE. Commit: 67cff13
/LLM/main/L0_MergeRequest_PR pipeline #64247 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

@farazkh80

Copy link
Copy Markdown
Collaborator Author

@QiJune opened #20419 for release/1.3 (waiver removal only — the 1.3 manifest is already current, the extra rows here are main-only). The latest CI failure on this PR is the unwaived test doing its job: #20100 added sparse_attention_config.gvr_deterministic_topk without regenerating the manifest; folding the regeneration into this PR.

@farazkh80
farazkh80 force-pushed the am/nvbug-6811958-manifest branch from 67cff13 to 824306a Compare October 15, 2026 16:45
@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78188 [ run ] triggered by Bot. Commit: 824306a Link to invocation

…atches_committed_golden

Re-enable the premerge privacy gate and regenerate the golden manifest,
which drifted again while the waiver hid the test: NVIDIA#20100 added
sparse_attention_config.gvr_deterministic_topk to llm_args.py without
regenerating.

Signed-off-by: Faraz Khoubsirat <58580514+farazkh80@users.noreply.github.com>
@farazkh80
farazkh80 force-pushed the am/nvbug-6811958-manifest branch from 824306a to ab41e80 Compare October 15, 2026 20:15
@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

farazkh80 added a commit to farazkh80/TensorRT-LLM that referenced this pull request Oct 15, 2026
…atches_committed_golden

The golden manifest on release/1.3 is current (regenerated in NVIDIA#19493 via
129655f), so re-enable the premerge privacy gate. Branch-specific
counterpart of NVIDIA#20077/NVIDIA#20057 on main.

Signed-off-by: Faraz Khoubsirat <58580514+farazkh80@users.noreply.github.com>
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78222 [ run ] triggered by Bot. Commit: ab41e80 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78188 [ run ] completed with state ABORTED. Commit: 824306a

Link to invocation

@farazkh80
farazkh80 enabled auto-merge (squash) October 15, 2026 20:55
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78222 [ run ] completed with state FAILURE. Commit: ab41e80
/LLM/main/L0_MergeRequest_PR pipeline #64475 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

@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78489 [ run ] triggered by Bot. Commit: ab41e80 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #78489 [ run ] completed with state SUCCESS. Commit: ab41e80
/LLM/main/L0_MergeRequest_PR pipeline #64692 completed with status: 'SUCCESS'

CI Report

Link to invocation

@farazkh80
farazkh80 merged commit a857768 into NVIDIA:main Oct 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-compatible Accepted LLM API contract change that is backwards-compatible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants