[TRTLLM-6822][infra] Add PR-Checklist github action and modify PR template #6029
Conversation
venkywonka
force-pushed
the
pr-checklist
branch
from
26207f5 to
0348797
Compare
venkywonka
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/pull_request_template.md (2)
45-53: Tighten checklist phrasing and fix minor grammar/style nitsPolish for clarity, consistency (sentence casing), and actionability. Suggested edits inline.
-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](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge. -- Test cases are provided for new code paths (see [test instructions](https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work)) -- Any new dependencies have been scanned for license and vulnerabilities -- [CODEOWNERS](https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS) updated if ownership changes -- Documentation updated as needed -- The reviewers assigned automatically/manually are appropriate for the PR. +Please review the following before submitting your PR: +- The PR description clearly explains the problem and the solution. If using CodeRabbit’s summary, please ensure it is accurate and self‑contained. +- The PR follows the [TRT‑LLM coding guidelines](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge. +- Tests are added or updated to cover new or modified code paths (see the [test instructions](https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work)). +- Any new or updated dependencies have been scanned for license compliance and vulnerabilities. +- [CODEOWNERS](https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS) is updated when ownership changes. +- Documentation is updated where applicable. +- Reviewers (auto‑assigned and/or manually assigned) are appropriate for the PR.
55-56: Make the confirmation checkbox explicitSmall wording tweak to make the confirmation intent unambiguous.
-- [ ] Please check this after reviewing the above items as appropriate for this PR. +- [ ] I confirm I have reviewed the items above and checked those that apply to this PR..github/workflows/pr-checklist.yml (2)
7-10: Harden default permissions further and add concurrency to prevent duplicate runs
- Principle of least privilege: current read perms are good; add a concurrency group to auto-cancel superseded runs on rapid pushes.
permissions: contents: read pull-requests: read + +concurrency: + group: pr-checklist-${{ github.event.pull_request.number }} + cancel-in-progress: true
23-27: Consider making the validation step self-contained to avoid code execution driftReferencing a locally checked-out action path couples behavior to whatever was fetched earlier. If you adopt Option A/B above, you remove this coupling. If you keep the local path, add a comment stating the expected version and how to update it to prevent silent drift.
Would you like a follow-up PR that vendors the composite action into .github/actions/pr_checklist and adds a small unit test for the regex it uses to detect the checklist?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/pull_request_template.md(1 hunks).github/workflows/pr-checklist.yml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.github/pull_request_template.md
[style] ~46-~46: Consider using a different verb to strengthen your wording.
Context: .... If using CodeRabbit's summary, please make sure it makes sense. - PR Follows [TRT-LLM C...
(MAKE_SURE_ENSURE)
[grammar] ~47-~47: There might be a mistake here.
Context: ...LINES.md) to the best of your knowledge. - Test cases are provided for new code pat...
(QB_NEW_EN)
venkywonka
force-pushed
the
pr-checklist
branch
from
809517f to
e612af7
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/pull_request_template.md (2)
45-56: Make each checklist item a task-list checkbox and tighten wordingTurning the bullets into checkboxes gives contributors explicit, auditable confirmations and plays nicer with automated validation. Also apply small wording/grammar fixes (ensure vs make sure; licenses plural; consistent tense/case).
-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](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge. -- Test cases are provided for new code paths (see [test instructions](https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work)) -- Any new dependencies have been scanned for license and vulnerabilities -- [CODEOWNERS](https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS) updated if ownership changes -- Documentation updated as needed -- The reviewers assigned automatically/manually are appropriate for the PR. +Before submitting, verify the following (check all that apply): +- [ ] PR description clearly explains what and why. If using CodeRabbit's summary, please ensure it makes sense. +- [ ] PR follows the [TRT-LLM CODING GUIDELINES](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md). +- [ ] Tests are provided for new or changed code paths (see the [test instructions](https://github.com/NVIDIA/TensorRT-LLM/tree/main/tests#1-how-does-the-ci-work)). +- [ ] Any new dependencies have been scanned for license compliance and security vulnerabilities. +- [ ] [CODEOWNERS](https://github.com/NVIDIA/TensorRT-LLM/blob/main/.github/CODEOWNERS) is updated if ownership changes. +- [ ] Documentation is updated as needed. +- [ ] Reviewers assigned automatically/manually are appropriate for the PR. +- [ ] Linked related issues with “Fixes #…”, “Closes #…”, or referenced the tracking ticket where applicable. +- [ ] Noted any breaking changes, migration steps, or backward-compatibility considerations (if applicable). -- [ ] Please check this after reviewing the above items as appropriate for this PR. +<!-- Check all relevant boxes above. -->
46-47: Address minor style/grammar flags from static analysisReplace “make sure” with “ensure” and use lowercase “follows” for consistency with sentence case used elsewhere.
-- PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense. -// ... -- PR Follows [TRT-LLM CODING GUIDELINES](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md) to the best of your knowledge. +- PR description clearly explains what and why. If using CodeRabbit's summary, please ensure it makes sense. +// ... +- PR follows the [TRT-LLM CODING GUIDELINES](https://github.com/NVIDIA/TensorRT-LLM/blob/main/CODING_GUIDELINES.md).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/pull_request_template.md(1 hunks).github/workflows/pr-checklist.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pr-checklist.yml
🧰 Additional context used
🪛 LanguageTool
.github/pull_request_template.md
[style] ~46-~46: Consider using a different verb to strengthen your wording.
Context: .... If using CodeRabbit's summary, please make sure it makes sense. - PR Follows [TRT-LLM C...
(MAKE_SURE_ENSURE)
[grammar] ~47-~47: There might be a mistake here.
Context: ...LINES.md) to the best of your knowledge. - Test cases are provided for new code pat...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (1)
.github/pull_request_template.md (1)
45-56: Confirm PR checklist syntax aligns with the enforced action
- The
pr-checklistworkflow (.github/workflows/pr-checklist.yml) is configured withENFORCE_PR_HAS_CHECKLIST: true, so it will reject PRs without at least one- [ ]item.- Your PR template (
.github/pull_request_template.md) does include a single- [ ]checkbox at line 55.- The source for the
pr_checklistaction (under.github/actions/goggles_action/actions/pr_checklist) wasn’t present in this branch—please manually verify that the action’s parsing rules (section headings, checkbox formats, regex patterns, etc.) match the template’s syntax.Please double-check the action’s expected markers against your template to ensure the “Validate PR Checklist” job will pass.
venkywonka
force-pushed
the
pr-checklist
branch
from
e612af7 to
1aa2df4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/pull_request_template.md(1 hunks).github/workflows/pr-checklist.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pr-checklist.yml
🧰 Additional context used
🪛 LanguageTool
.github/pull_request_template.md
[style] ~46-~46: Consider using a different verb to strengthen your wording.
Context: .... If using CodeRabbit's summary, please make sure it makes sense. - PR Follows [TRT-LLM C...
(MAKE_SURE_ENSURE)
[grammar] ~47-~47: There might be a mistake here.
Context: ...LINES.md) to the best of your knowledge. - Test cases are provided for new code pat...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (1)
.github/pull_request_template.md (1)
55-56: Make final attestation explicit in PR templateReplace the generic post-check reminder with a clear attestation. In
.github/pull_request_template.mdat lines 55–56, apply:- [ ] Please check this after reviewing the above items as appropriate for this PR. + [ ] I have reviewed the checklist above and checked all applicable items.This wording leaves no ambiguity about the contributor’s final sign-off.
Please manually verify that no GitHub Action or workflow in your repository inspects the exact phrasing of this line—only that at least one Markdown checkbox exists.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
scripts/pr_checklist_check.py (3)
60-62: Fix E501: long line in error string.Ruff flagged line length; wrap the string.
- if not final_checkbox_pattern.search(pr_body): - return False, "Missing the required final checkbox '- [ ] Please check this after reviewing the above items as appropriate for this PR.' Please ensure you haven't removed this from the PR template." + if not final_checkbox_pattern.search(pr_body): + return False, ( + "Missing the required final checkbox " + "'- [ ] Please check this after reviewing the above items as appropriate for this PR.' " + "Please ensure you haven't removed this from the PR template." + )
71-89: Reduce false positives by scanning only within the “PR Checklist” section.Today, any “- [ ] …” anywhere in the PR body (including examples or code blocks) will fail the check. Limit parsing to the PR Checklist section to keep friction low (aligned with team learnings).
+def _extract_pr_checklist_section(pr_body: str) -> str: + """Return only the content under '## PR Checklist' up to the next '## ' header.""" + start = re.search(r'^##\s+PR\s+Checklist\b', pr_body, re.IGNORECASE | re.MULTILINE) + if not start: + return pr_body + rest = pr_body[start.end():] + next_hdr = re.search(r'^\s*##\s+', rest, re.MULTILINE) + end = next_hdr.start() if next_hdr else len(rest) + return rest[:end] @@ - all_tasks = find_all_tasks(pr_body) - unresolved = find_unresolved_tasks(pr_body) + checklist_block = _extract_pr_checklist_section(pr_body) + all_tasks = find_all_tasks(checklist_block) + unresolved = find_unresolved_tasks(checklist_block) @@ - print("No checklist items found in PR body.") + print("No checklist items found in PR body.")Note: I intentionally didn’t add code-fence stripping to avoid extra complexity. If needed, we can ignore lines between triple-backticks later. This keeps the UX simple per the team preference to avoid added overhead.
Also applies to: 97-101
57-59: Optional: make final checkbox text check case-insensitive for robustness.Keeps intent while tolerating minor casing edits.
- final_checkbox_pattern = re.compile( - r'^\s*[-*]\s+\[( |x|X)\]\s+Please check this after reviewing the above items', - re.MULTILINE) + final_checkbox_pattern = re.compile( + r'^\s*[-*]\s+\[( |x|X)\]\s+Please check this after reviewing the above items', + re.IGNORECASE | re.MULTILINE).github/workflows/pr-checklist.yml (3)
11-13: Skip on drafts and cancel superseded runs to reduce friction.This avoids failing early on draft PRs and cancels redundant runs on rapid pushes.
name: Validate PR Checklist @@ on: pull_request: types: [opened, edited, synchronize] permissions: contents: read pull-requests: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true @@ jobs: pr-checklist: + if: ${{ github.event.pull_request.draft == false }} runs-on: ubuntu-latest steps:Also applies to: 23-27
15-17: Harden checkout (no writes needed).Use a shallow, read-only checkout; minor security/perf win.
- name: Checkout repository uses: actions/checkout@v4 + with: + persist-credentials: false + fetch-depth: 1
18-22: Optional: verify 3.8 compatibility in CI matrix.The script targets 3.8+, so add 3.8 to the matrix to catch typing regressions.
- - name: Set up Python - uses: actions/setup-python@v5 - with: - python-version: '3.10' + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + strategy: + matrix: + python-version: ['3.8', '3.10']
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/pr-checklist.yml(1 hunks)scripts/pr_checklist_check.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
**/*.py: Code must target Python 3.8+
Indent Python code with 4 spaces; do not use tabs
Preserve module namespaces when importing; import modules/packages and access members via the module (e.g., from package.subpackage import foo; foo.SomeClass())
Python file names should be snake_case
Python class names should be PascalCase
Python functions/methods and local variables should be snake_case; variables beginning with a number should be prefixed with k_ (e.g., k_99th_percentile)
Global variables should be UPPER_SNAKE_CASE prefixed with G_ (e.g., G_MY_GLOBAL); constants should be UPPER_SNAKE_CASE
Avoid shadowing variables from outer scopes; initialize all externally visible members in init
Prefer docstrings for interfaces used outside a file; comments should be reserved for in-function or file-local interfaces
Use Google-style docstrings for classes and functions; attributes and variables may be documented inline with trailing string literals
Avoid reflection when simpler, explicit code suffices (e.g., avoid dict(**locals()) patterns)
In try/except, catch the narrowest exceptions possible
For duck-typing patterns, keep the try body minimal and move logic to else to avoid masking unrelated failures
Files:
scripts/pr_checklist_check.py
**/*.{c,cc,cpp,cxx,h,hh,hpp,hxx,cu,cuh,py}
📄 CodeRabbit inference engine (CODING_GUIDELINES.md)
Prepend the NVIDIA copyright header (current year) to all source files (.cpp, .h, .cu, .py, etc.)
Files:
scripts/pr_checklist_check.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: venkywonka
PR: NVIDIA/TensorRT-LLM#6029
File: .github/pull_request_template.md:45-53
Timestamp: 2025-08-27T17:50:13.243Z
Learning: For PR templates in TensorRT-LLM, avoid suggesting changes that would increase developer overhead, such as converting plain bullets to mandatory checkboxes. The team prefers guidance-style bullets that don't require explicit interaction to reduce friction in the PR creation process.
🪛 Ruff (0.12.2)
scripts/pr_checklist_check.py
61-61: Line too long (206 > 120)
(E501)
🪛 GitHub Actions: Validate PR Checklist
scripts/pr_checklist_check.py
[error] 1-1: PR checklist validation failed due to unresolved items. Command 'python scripts/pr_checklist_check.py' exited with code 1. Unresolved checklist item: 'Please check this after reviewing the above items as appropriate for this PR.'
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.github/workflows/pr-check.yml (3)
57-60: Add least-privilege permissions and cancel in-progress runs.Scope the GITHUB_TOKEN and avoid duplicate runs on rapid pushes.
check-pr-body-checklist: name: Check PR Checklist Resolution runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + concurrency: + group: pr-checklist-${{ github.event.pull_request.number }} + cancel-in-progress: true steps:
69-74: Quote boolean env and guard empty PR bodies.Avoid YAML boolean coercion and null body pitfalls.
- name: Validate PR Checklist env: - PR_BODY: ${{ github.event.pull_request.body }} - ENFORCE_PR_HAS_CHECKLIST: true - run: python scripts/pr_checklist_check.py + PR_BODY: ${{ github.event.pull_request.body || '' }} + ENFORCE_PR_HAS_CHECKLIST: "true" + run: python scripts/pr_checklist_check.py
57-74: Make enforcement non-blocking for drafts and align with team preference.Per past learnings for this repo, avoid increasing PR friction; consider only warning on drafts and enforcing section presence (not checkbox resolution).
check-pr-body-checklist: name: Check PR Checklist Resolution runs-on: ubuntu-latest + if: ${{ !github.event.pull_request.draft }}Context used: retrieved learnings indicate the team prefers guidance-style bullets over mandatory checkboxes. Do you want this job to fail PRs with unresolved items, or only nudge authors?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/pr-check.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: venkywonka
PR: NVIDIA/TensorRT-LLM#6029
File: .github/pull_request_template.md:45-53
Timestamp: 2025-08-27T17:50:13.243Z
Learning: For PR templates in TensorRT-LLM, avoid suggesting changes that would increase developer overhead, such as converting plain bullets to mandatory checkboxes. The team prefers guidance-style bullets that don't require explicit interaction to reduce friction in the PR creation process.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (1)
.github/workflows/pr-check.yml (1)
61-66: Pin GitHub Actions to immutable commit SHAs for supply-chain securityTo ensure reproducibility and mitigate risks from mutable tags, pin third-party actions to their exact commit SHAs.
• Location:
.github/workflows/pr-check.yml, lines 61–66
• Fetched SHAs via:gh api repos/actions/checkout/tags \ | jq -r '.[] | select(.name=="v4") | .commit.sha' # → 08eba0b27e820071cde6df949e0beb9ba4906955 gh api repos/actions/setup-python/tags \ | jq -r '.[] | select(.name=="v5") | .commit.sha' # → a26af69be951a213d495a4c3e4e4022e16d87065Apply the following diff:
- name: Checkout repository - uses: actions/checkout@v4 + uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 - name: Set up Python - uses: actions/setup-python@v5 + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065
|
/bot skip --comment "does not affect L0" |
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky <[email protected]>
- Introduced a new script to validate the presence and status of checklist items in PR descriptions, enforcing checklist requirements. - Now instead of borrowing the action from goggles_action, we maintain it in TRTLLM itself. Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
Signed-off-by: Venky Ganesh <[email protected]>
venkywonka
force-pushed
the
pr-checklist
branch
from
ef1a71b to
dc1f9a3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.github/scripts/pr_checklist_check.py (6)
39-57: Don’t block on the final acknowledgement checkbox; exclude it from “unresolved”.Current logic will fail PRs until authors tick the final “Please check …” box, which adds friction. Skip this specific line when computing unresolved tasks.
Apply this diff within find_unresolved_tasks:
@@ - state, content = match.groups() + state, content = match.groups() + desc = content.strip() if state.lower() == 'x': continue - # Check if the entire content is struck through - if content.strip().startswith('~~') and content.strip().endswith('~~'): + # Ignore the final acknowledgement line to avoid blocking on it + if desc.startswith(FINAL_CHECKBOX_PREFIX): + continue + # Check if the entire content is struck through + if desc.startswith('~~') and desc.endswith('~~'): continue unresolved.append(match.group(0).strip())Add this module-level constant (outside the selected range):
# Place below TASK_PATTERN FINAL_CHECKBOX_PREFIX = "Please check this after reviewing the above items"
72-79: Reuse a shared constant for the final checkbox and keep the regex resilient.Leverage the same prefix constant so the text can evolve without code churn.
- final_checkbox_pattern = re.compile( - r'^\s*[-*]\s+\[( |x|X)\]\s+Please check this after reviewing the above items', - re.MULTILINE) + final_checkbox_pattern = re.compile( + rf'^\s*[-*]\s+\[( |x|X)\]\s+{re.escape(FINAL_CHECKBOX_PREFIX)}', + re.MULTILINE)
97-105: Avoid enforcing “at least one checklist item” to reduce PR friction (team preference).Per team learnings, prefer guidance over mandatory checkboxes. The header + final acknowledgement presence is sufficient.
- # Check if we need to enforce the presence of at least one checklist item - if enforce_checklist and not all_tasks: - print( - "Error: PR body must contain at least one checklist item when ENFORCE_PR_HAS_CHECKLIST is enabled." - ) - print( - "Expected format: - [ ] Task description or * [ ] Task description") - sys.exit(1) + # Do not require at least one item; header + final acknowledgement is enough. + # Intentionally no-op here to minimize PR friction.
106-112: Emit GitHub Actions annotations for easier triage.Use workflow commands so unresolved items surface inline in the PR UI.
- if unresolved: - print("Unresolved checklist items found:") - for item in unresolved: - print(f"{item}") - sys.exit(1) + if unresolved: + print("::error::Unresolved checklist items found:") + for item in unresolved: + print(f"::error::{item}") + sys.exit(1)
73-78: Fix Ruff E501: wrap the long error message.Keeps line length ≤120.
- return False, "Missing the required final checkbox '- [ ] Please check this after reviewing the above items as appropriate for this PR.' Please ensure you haven't removed this from the PR template." + return False, ( + "Missing the required final checkbox " + "'- [ ] Please check this after reviewing the above items as appropriate for this PR.' " + "Please ensure you haven't removed this from the PR template." + )
29-36: Scope parsing to the “PR Checklist” section to avoid false positives (optional).Currently, any checklist in the PR body (e.g., in examples or code blocks) can trip the check. Consider limiting parsing to lines between “## PR Checklist” and the next “## ” header, and ignore fenced code blocks.
Happy to provide a small helper to extract that section and strip fenced blocks if you want it in this PR.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/scripts/pr_checklist_check.py(1 hunks).github/workflows/pr-check.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/pr-check.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: venkywonka
PR: NVIDIA/TensorRT-LLM#6029
File: .github/pull_request_template.md:45-53
Timestamp: 2025-08-27T17:50:13.243Z
Learning: For PR templates in TensorRT-LLM, avoid suggesting changes that would increase developer overhead, such as converting plain bullets to mandatory checkboxes. The team prefers guidance-style bullets that don't require explicit interaction to reduce friction in the PR creation process.
🪛 Ruff (0.12.2)
.github/scripts/pr_checklist_check.py
77-77: Line too long (206 > 120)
(E501)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Pre-commit Check
🔇 Additional comments (1)
.github/scripts/pr_checklist_check.py (1)
20-21: Python 3.9+ is guaranteed in CI → PEP585 generics are safeAll of our GitHub workflows pin Python to versions ≥3.10 (and “3.x”, which today resolves to 3.12), so using built-in
tuple[bool, str]is supported and no changes are needed:
.github/workflows/label_community_pr.ymlusesactions/setup-python@v3withpython-version: '3.x'(latest 3.x).github/workflows/pr-check.ymlusesactions/setup-python@v5withpython-version: '3.10'.github/workflows/precommit-check.ymlusesactions/setup-python@v5withpython-version: '3.12'Since every CI job runs on Python ≥3.9, you can keep the
tuple[bool, str]return annotation.
|
PR_Github #16734 [ skip ] triggered by Bot |
|
PR_Github #16734 [ skip ] completed with state |
Summary by CodeRabbit
Documentation
Chores
Note
Description
Test Coverage
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)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
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
/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.
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)]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. Examples: "A10-PyTorch-1, xxx". 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. 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.--post-merge(OPTIONAL) : Run the L0 post-merge pipeline instead of the ordinary L0 pre-merge pipeline.--extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx"(OPTIONAL) : Run the ordinary L0 pre-merge pipeline and specified test stages. Examples: --extra-stage "H100_PCIe-TensorRT-Post-Merge-1, xxx".--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 thestage-listparameter to access the appropriate container environment. Note: Does NOT update GitHub check status.For guidance on mapping tests to stage names, see
docs/source/reference/ci-overview.mdand the
scripts/test_to_stage_mapping.pyhelper.kill
killKill all running builds associated with pull request.
skip
skip --comment COMMENTSkip 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-pipelineReuse 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.