|
1 | | -name: Claude Code Review |
| 1 | +name: PR Review Guidance |
2 | 2 |
|
3 | 3 | on: |
4 | | - pull_request: |
5 | | - types: [opened, reopened, ready_for_review] |
| 4 | + workflow_run: |
| 5 | + workflows: ["CICD NeMo"] |
| 6 | + types: [requested] |
| 7 | + branches: |
| 8 | + - "pull-request/[0-9]+" |
6 | 9 | issue_comment: |
7 | 10 | types: [created] |
8 | 11 |
|
9 | | -permissions: |
10 | | - contents: read |
11 | | - pull-requests: write |
12 | | - issues: write |
13 | | - id-token: write |
| 12 | +permissions: {} |
14 | 13 |
|
15 | 14 | jobs: |
16 | | - pre-flight: |
17 | | - uses: NVIDIA-NeMo/FW-CI-templates/.github/workflows/_cicd_preflight.yml@c1a0837f362a1a696e647238ab1cf916b2a7cf4a # v1.8.6 |
18 | | - with: |
19 | | - default_runner_prefix: ubuntu-latest |
20 | | - sso_users_filename: ${{ vars.SSO_USERS_FILENAME }} |
21 | | - secrets: |
22 | | - NVIDIA_MANAGEMENT_ORG_PAT: ${{ secrets.NVIDIA_MANAGEMENT_ORG_PAT }} |
23 | | - |
24 | | - auto-review: |
25 | | - name: Claude Review (auto) |
26 | | - needs: [pre-flight] |
27 | | - # Auto-review same-repository, non-draft PRs from NVIDIA authors on open / reopen / ready-for-review. |
28 | | - # Fork pull_request workflows cannot access the secrets or OIDC token required by the review action. |
29 | | - # Subsequent commits do not re-trigger; re-request a review by commenting |
30 | | - # "/claude review" (handled by the manual-review job below). |
31 | | - # Skip bot-generated PRs: dependency bumps (`bump-ci-container-*`) and |
32 | | - # auto cherry-picks (`cherry-pick-*`). |
| 15 | + review-hint: |
| 16 | + name: Review command hint |
| 17 | + # Post guidance once for non-draft PRs when copy-pr-bot's |
| 18 | + # pull-request/<number> push requests the CICD NeMo workflow. |
| 19 | + # Skip bot-generated dependency bumps and auto cherry-picks. |
33 | 20 | if: | |
34 | | - github.event_name == 'pull_request' && |
35 | | - github.event.pull_request.head.repo.full_name == github.repository && |
36 | | - needs.pre-flight.outputs.is_member == 'true' && |
37 | | - github.event.pull_request.draft == false && |
38 | | - !startsWith(github.event.pull_request.head.ref, 'bump-ci-container-') && |
39 | | - !startsWith(github.event.pull_request.head.ref, 'cherry-pick-') |
| 21 | + github.event_name == 'workflow_run' && |
| 22 | + github.event.workflow_run.event == 'push' && |
| 23 | + github.event.workflow_run.actor.login == 'copy-pr-bot[bot]' |
40 | 24 | runs-on: ubuntu-latest |
| 25 | + permissions: |
| 26 | + pull-requests: write |
| 27 | + concurrency: |
| 28 | + group: ${{ github.workflow }}-review-hint-${{ github.event.workflow_run.head_branch }} |
| 29 | + cancel-in-progress: false |
41 | 30 | steps: |
42 | | - - name: Checkout repository |
43 | | - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 |
| 31 | + - name: Post review command hint once |
| 32 | + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 |
44 | 33 | with: |
45 | | - fetch-depth: 1 |
46 | | - |
47 | | - - name: Run Claude Code Review |
48 | | - uses: anthropics/claude-code-action@be7b93b1907a4abad570368f3c74b6fe3807510b # v1 |
| 34 | + script: | |
| 35 | + const branch = context.payload.workflow_run.head_branch; |
| 36 | + const number = Number(branch.split("/").pop()); |
| 37 | + const { data: pr } = await github.rest.pulls.get({ |
| 38 | + ...context.repo, |
| 39 | + pull_number: number, |
| 40 | + }); |
| 41 | + const shouldNotify = |
| 42 | + pr.state === "open" && |
| 43 | + pr.head.sha === context.payload.workflow_run.head_sha && |
| 44 | + !pr.draft && |
| 45 | + !pr.head.ref.startsWith("bump-ci-container-") && |
| 46 | + !pr.head.ref.startsWith("cherry-pick-"); |
| 47 | + if (!shouldNotify) return; |
| 48 | +
|
| 49 | + const marker = "<!-- review-command-hint -->"; |
| 50 | + const comments = await github.paginate(github.rest.issues.listComments, { |
| 51 | + ...context.repo, |
| 52 | + issue_number: number, |
| 53 | + per_page: 100, |
| 54 | + }); |
| 55 | + if (comments.some(comment => |
| 56 | + comment.user?.login === "github-actions[bot]" && |
| 57 | + comment.body?.includes(marker) |
| 58 | + )) return; |
| 59 | +
|
| 60 | + await github.rest.issues.createComment({ |
| 61 | + ...context.repo, |
| 62 | + issue_number: number, |
| 63 | + body: [ |
| 64 | + marker, |
| 65 | + "Automatic Claude reviews have been retired. To request a pull-request review, post a comment containing:", |
| 66 | + "", |
| 67 | + "```", |
| 68 | + "/review", |
| 69 | + "```", |
| 70 | + "", |
| 71 | + "Add `model=claude` to use a Claude reviewer (the default is `model=codex`). " + |
| 72 | + "`mode=light|strict` selects the review depth; for example, `/review model=claude mode=strict`. " + |
| 73 | + "Comment `/review help` for all options.", |
| 74 | + ].join("\n"), |
| 75 | + }); |
| 76 | +
|
| 77 | + redirect-to-review: |
| 78 | + name: Redirect to /review |
| 79 | + if: | |
| 80 | + github.event_name == 'issue_comment' && |
| 81 | + github.event.issue.pull_request && |
| 82 | + github.event.comment.user.type != 'Bot' && |
| 83 | + (contains(github.event.comment.body, '/claude review') || |
| 84 | + contains(github.event.comment.body, '/claude strict-review')) |
| 85 | + runs-on: ubuntu-latest |
| 86 | + permissions: |
| 87 | + pull-requests: write |
| 88 | + env: |
| 89 | + GH_TOKEN: ${{ github.token }} |
| 90 | + REPO: ${{ github.repository }} |
| 91 | + PR_NUMBER: ${{ github.event.issue.number }} |
| 92 | + RETIRED_COMMAND: ${{ contains(github.event.comment.body, '/claude strict-review') && '/claude strict-review' || '/claude review' }} |
| 93 | + REVIEW_COMMAND: ${{ contains(github.event.comment.body, '/claude strict-review') && '/review mode=strict' || '/review' }} |
| 94 | + steps: |
| 95 | + - name: Explain the replacement command |
49 | 96 | env: |
50 | | - ANTHROPIC_BASE_URL: ${{ secrets.NVIDIA_INFERENCE_URL }} |
51 | | - CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS: "1" |
52 | | - DISABLE_PROMPT_CACHING: "1" |
53 | | - with: |
54 | | - anthropic_api_key: ${{ secrets.NVIDIA_INFERENCE_KEY }} |
55 | | - claude_args: | |
56 | | - --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr review:*),Read" |
57 | | - --model "${{ vars.CLAUDE_MODEL }}" |
58 | | - prompt: | |
59 | | - REPO: ${{ github.repository }} |
60 | | - PR NUMBER: ${{ github.event.pull_request.number }} |
61 | | -
|
62 | | - Mandatory workflow — never skip or reorder: |
63 | | - 1. Read the PR diff first (gh pr diff). |
64 | | - 2. Based on the changed files and areas, identify relevant skills from skills/<name>/SKILL.md. |
65 | | - Common skill names: linting-and-formatting, testing, cicd, build-and-dependency, |
66 | | - adding-model-support, perf-activation-recompute, perf-memory-tuning, etc. |
67 | | - 3. Read the SKILL.md files for all relevant areas using the Read tool. |
68 | | - 4. Only then perform the review using the skill context. |
69 | | -
|
70 | | - You are doing a light code review. Keep it concise and actionable. |
71 | | -
|
72 | | - Focus ONLY on: |
73 | | - - Critical bugs or logic errors |
74 | | - - Typos in code, comments, or strings |
75 | | - - Missing or insufficient test coverage for changed code |
76 | | - - Outdated or inaccurate documentation affected by the changes |
77 | | -
|
78 | | - Do NOT comment on: |
79 | | - - Style preferences or formatting |
80 | | - - Minor naming suggestions |
81 | | - - Architectural opinions or refactoring ideas |
82 | | - - Performance unless there is a clear, measurable issue |
83 | | -
|
84 | | - Provide feedback using inline comments for specific code suggestions. |
85 | | - Use top-level comments for general observations. |
86 | | -
|
87 | | - Always end the review with a "Suggested test cases" bullet list, derived |
88 | | - from the diff. |
89 | | -
|
90 | | - Map source changes → test-case globs using: |
91 | | - - A function `<model>_<task>_config_<gpu>` in scripts/performance/configs/<family>/... |
92 | | - impacts cases matching `<model>_*gpu_<gpu>_*_perf`. |
93 | | - - A base config `<MODEL>_..._<GPU>_<PRECISION>_V<N>` impacts that precision, |
94 | | - plus any V<N+1> / LARGE_SCALE configs derived from it via `replace(...)`. |
95 | | -
|
96 | | - Each bullet must be an explicit test case name — do NOT use globs |
97 | | - or brace expansion. Enumerate every relevant combination, e.g. |
98 | | - `qwen3_235b_a22b_512gpu_b300_bf16_perf`, |
99 | | - `qwen3_235b_a22b_512gpu_b300_fp8_cs_perf`, |
100 | | - `qwen3_235b_a22b_512gpu_b300_fp8_mx_perf`, |
101 | | - `qwen3_235b_a22b_512gpu_b300_nvfp4_perf`. Discover the exact names |
102 | | - by grepping the diff and the `scripts/performance/configs/` tree |
103 | | - for the affected model/GPU/precision combinations. |
104 | | - If no perf/recipe configs are touched, write "No perf tests impacted." |
105 | | -
|
106 | | - End the review by posting it as a top-level PR comment using |
107 | | - bash ANSI-C quoting so newlines render correctly: |
108 | | - gh pr comment <PR NUMBER> --repo <REPO> --body $'line1\nline2\n...' |
109 | | - Use $'...' (NOT plain "..."). Inside "..." the sequence \n is |
110 | | - passed to gh literally and shows up as the two characters \n in |
111 | | - the rendered comment; inside $'...' bash expands \n to a real |
112 | | - newline before gh sees it. Do NOT use --body-file, output |
113 | | - redirection, or HEREDOCs — the action sandbox blocks all three. |
114 | | - The body must include the "Suggested test cases" list. If there |
115 | | - is nothing to flag, the body is just "LGTM" followed by the |
116 | | - test-case list. Inline comments via |
117 | | - mcp__github_inline_comment__create_inline_comment are optional — |
118 | | - use them only for specific code suggestions. |
119 | | -
|
120 | | - manual-review: |
121 | | - if: github.event_name == 'issue_comment' && github.event.issue.pull_request != null && contains(fromJson('["OWNER", "MEMBER", "COLLABORATOR"]'), github.event.comment.author_association) |
122 | | - uses: NVIDIA-NeMo/FW-CI-templates/.github/workflows/_claude_review.yml@1fc7b2f024bac423b1825a57182f3628c5fcb1f1 # v1.7.0 |
123 | | - with: |
124 | | - model: ${{ vars.CLAUDE_MODEL }} |
125 | | - prompt: | |
126 | | - Mandatory workflow — never skip or reorder: |
127 | | - 1. Read the PR diff first. |
128 | | - 2. Based on the changed files and areas, identify relevant skills from skills/<name>/SKILL.md. |
129 | | - Common skill names: linting-and-formatting, testing, cicd, build-and-dependency, |
130 | | - adding-model-support, perf-activation-recompute, perf-memory-tuning, etc. |
131 | | - 3. Read the SKILL.md files for all relevant areas using the Read tool. |
132 | | - 4. Only then perform the review using the skill context. |
133 | | -
|
134 | | - You are doing a light code review. Keep it concise and actionable. |
135 | | -
|
136 | | - Focus ONLY on: |
137 | | - - Critical bugs or logic errors |
138 | | - - Typos in code, comments, or strings |
139 | | - - Missing or insufficient test coverage for changed code |
140 | | - - Outdated or inaccurate documentation affected by the changes |
141 | | -
|
142 | | - Do NOT comment on: |
143 | | - - Style preferences or formatting |
144 | | - - Minor naming suggestions |
145 | | - - Architectural opinions or refactoring ideas |
146 | | - - Performance unless there is a clear, measurable issue |
147 | | -
|
148 | | - Provide feedback using inline comments for specific code suggestions. |
149 | | - Use top-level comments for general observations. |
150 | | -
|
151 | | - Always end the review with a "Suggested test cases" bullet list, derived |
152 | | - from the diff. |
153 | | -
|
154 | | - Map source changes → test-case globs using: |
155 | | - - A function `<model>_<task>_config_<gpu>` in scripts/performance/configs/<family>/... |
156 | | - impacts cases matching `<model>_*gpu_<gpu>_*_perf`. |
157 | | - - A base config `<MODEL>_..._<GPU>_<PRECISION>_V<N>` impacts that precision, |
158 | | - plus any V<N+1> / LARGE_SCALE configs derived from it via `replace(...)`. |
| 97 | + NOTICE: | |
| 98 | + @${{ github.event.comment.user.login }} `${{ env.RETIRED_COMMAND }}` has been retired. Pull-request reviews now run through `/review`; comment this instead: |
159 | 99 |
|
160 | | - Each bullet must be an explicit test case name — do NOT use globs |
161 | | - or brace expansion. Enumerate every relevant combination, e.g. |
162 | | - `qwen3_235b_a22b_512gpu_b300_bf16_perf`, |
163 | | - `qwen3_235b_a22b_512gpu_b300_fp8_cs_perf`, |
164 | | - `qwen3_235b_a22b_512gpu_b300_fp8_mx_perf`, |
165 | | - `qwen3_235b_a22b_512gpu_b300_nvfp4_perf`. Discover the exact names |
166 | | - by grepping the diff and the `scripts/performance/configs/` tree |
167 | | - for the affected model/GPU/precision combinations. |
168 | | - If no perf/recipe configs are touched, write "No perf tests impacted." |
| 100 | + ``` |
| 101 | + ${{ env.REVIEW_COMMAND }} |
| 102 | + ``` |
169 | 103 |
|
170 | | - End the review by posting it as a top-level PR comment using |
171 | | - bash ANSI-C quoting so newlines render correctly: |
172 | | - gh pr comment <PR NUMBER> --repo <REPO> --body $'line1\nline2\n...' |
173 | | - Use $'...' (NOT plain "..."). Inside "..." the sequence \n is |
174 | | - passed to gh literally and shows up as the two characters \n in |
175 | | - the rendered comment; inside $'...' bash expands \n to a real |
176 | | - newline before gh sees it. Do NOT use --body-file, output |
177 | | - redirection, or HEREDOCs — the action sandbox blocks all three. |
178 | | - The body must include the "Suggested test cases" list. If there |
179 | | - is nothing to flag, the body is just "LGTM" followed by the |
180 | | - test-case list. Inline comments via |
181 | | - mcp__github_inline_comment__create_inline_comment are optional — |
182 | | - use them only for specific code suggestions. |
183 | | - secrets: |
184 | | - NVIDIA_INFERENCE_URL: ${{ secrets.NVIDIA_INFERENCE_URL }} |
185 | | - NVIDIA_INFERENCE_KEY: ${{ secrets.NVIDIA_INFERENCE_KEY }} |
| 104 | + Add `model=claude` to keep a Claude reviewer (the default is `model=codex`). `mode=light|strict` selects the review depth. Comment `/review help` for all options. |
| 105 | + run: gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$NOTICE" |
0 commit comments