Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 30 additions & 89 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: Claude Code Review

on:
pull_request_target:
types: [opened, synchronize, ready_for_review, reopened, labeled]
types: [opened, ready_for_review, reopened, labeled]
issue_comment:
types: [created]

Expand All @@ -15,7 +15,6 @@ jobs:
github.event.action == 'opened' ||
github.event.action == 'ready_for_review' ||
github.event.action == 'reopened' ||
github.event.action == 'synchronize' ||
(
github.event.action == 'labeled' &&
github.event.label.name == 'claude-full-review'
Expand Down Expand Up @@ -47,42 +46,22 @@ jobs:
with:
fetch-depth: 1

- name: Determine PR number and requested review mode
- name: Determine PR number
id: mode
shell: bash
run: |
set -euo pipefail

if [[ "${{ github.event_name }}" == "pull_request_target" ]]; then
PR_NUMBER="${{ github.event.pull_request.number }}"
case "${{ github.event.action }}" in
opened|ready_for_review|reopened)
REQUESTED_MODE="full"
;;
synchronize)
REQUESTED_MODE="incremental"
;;
labeled)
if [[ "${{ github.event.label.name }}" == "claude-full-review" ]]; then
REQUESTED_MODE="full"
else
REQUESTED_MODE="full"
fi
;;
*)
REQUESTED_MODE="full"
;;
esac
elif [[ "${{ github.event_name }}" == "issue_comment" ]]; then
PR_NUMBER="${{ github.event.issue.number }}"
REQUESTED_MODE="full"
else
echo "Unsupported event"
exit 1
fi

echo "pr_number=$PR_NUMBER" >> "$GITHUB_OUTPUT"
echo "requested_mode=$REQUESTED_MODE" >> "$GITHUB_OUTPUT"

- name: Resolve review state
id: state
Expand All @@ -94,7 +73,6 @@ jobs:

PR_NUMBER="${{ steps.mode.outputs.pr_number }}"
REPO="${{ github.repository }}"
REQUESTED_MODE="${{ steps.mode.outputs.requested_mode }}"

mkdir -p .claude-review/context

Expand All @@ -108,23 +86,13 @@ jobs:
)"

EXISTING_COMMENT_ID=""
PREVIOUS_REVIEWED_SHA=""

if [[ -n "${LAST_COMMENT_B64:-}" ]]; then
LAST_COMMENT_JSON="$(printf '%s' "$LAST_COMMENT_B64" | base64 -d)"
EXISTING_COMMENT_ID="$(printf '%s' "$LAST_COMMENT_JSON" | jq -r '.id // empty')"
COMMENT_BODY="$(printf '%s' "$LAST_COMMENT_JSON" | jq -r '.body // empty')"
PREVIOUS_REVIEWED_SHA="$(printf '%s' "$COMMENT_BODY" | sed -n 's/.*reviewed_sha=\([0-9a-fA-F]\{7,\}\).*/\1/p' | tail -n1)"
fi

EFFECTIVE_MODE="$REQUESTED_MODE"
if [[ "$REQUESTED_MODE" == "incremental" && -z "${PREVIOUS_REVIEWED_SHA:-}" ]]; then
EFFECTIVE_MODE="full"
fi

echo "existing_comment_id=${EXISTING_COMMENT_ID:-}" >> "$GITHUB_OUTPUT"
echo "previous_reviewed_sha=${PREVIOUS_REVIEWED_SHA:-}" >> "$GITHUB_OUTPUT"
echo "effective_mode=$EFFECTIVE_MODE" >> "$GITHUB_OUTPUT"

- name: Build review diff and changed-file list
id: review_input
Expand All @@ -136,43 +104,23 @@ jobs:

PR_NUMBER="${{ steps.mode.outputs.pr_number }}"
REPO="${{ github.repository }}"
MODE="${{ steps.state.outputs.effective_mode }}"
CURRENT_SHA="${{ steps.state.outputs.current_head_sha }}"
PREV_SHA="${{ steps.state.outputs.previous_reviewed_sha }}"

mkdir -p .claude-review
: > .claude-review/review.diff
: > .claude-review/changed_files.txt

build_full() {
gh pr diff "$PR_NUMBER" --repo "$REPO" > .claude-review/review.diff
gh pr view "$PR_NUMBER" --repo "$REPO" --json files --jq '.files[].path' > .claude-review/changed_files.txt
echo "actual_mode=full" >> "$GITHUB_OUTPUT"
}
gh pr diff "$PR_NUMBER" --repo "$REPO" > .claude-review/review.diff.raw
gh pr view "$PR_NUMBER" --repo "$REPO" --json files --jq '.files[].path' > .claude-review/changed_files.txt.raw

if [[ "$MODE" == "full" ]]; then
build_full
else
if gh api -H "Accept: application/vnd.github.diff" \
"repos/$REPO/compare/$PREV_SHA...$CURRENT_SHA" > .claude-review/review.diff.tmp 2>/dev/null \
&& gh api "repos/$REPO/compare/$PREV_SHA...$CURRENT_SHA" --jq '.files[].filename' > .claude-review/changed_files.txt.tmp 2>/dev/null; then
mv .claude-review/review.diff.tmp .claude-review/review.diff
mv .claude-review/changed_files.txt.tmp .claude-review/changed_files.txt
echo "actual_mode=incremental" >> "$GITHUB_OUTPUT"
else
echo "Compare diff failed; falling back to full review."
build_full
fi
fi
# Filter out tests/ directory — golden files don't need code review
grep -v '^tests/' .claude-review/changed_files.txt.raw > .claude-review/changed_files.txt || true
# Strip diff hunks for tests/ files
awk '
/^diff --git a\/tests\// { skip=1; next }
/^diff --git / { skip=0 }
!skip { print }
' .claude-review/review.diff.raw > .claude-review/review.diff
Comment on lines +116 to +120
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Rename diff wrongly dropped 🐞 Bug ≡ Correctness

The AWK filter strips any diff whose *old* path starts with tests/ (diff --git a/tests/...),
which will incorrectly drop diffs for renames/moves from tests/ to non-tests/ paths (e.g.,
tests/foo -> src/foo). This can cause the review to miss real (non-test) changes while
changed_files.txt still includes the new non-test path.
Agent Prompt
### Issue description
The AWK rule that strips `tests/` diffs matches `diff --git a/tests/...`, which incorrectly removes diffs for files renamed/moved from `tests/` to non-`tests/` paths.

### Issue Context
`changed_files.txt` is filtered based on the post-change path list (`.files[].path`), so the diff filtering should key off the *new* path (`b/...`) to stay consistent.

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[113-120]

### Suggested change
Update the AWK logic to set `skip` when the `diff --git` header indicates the **new** path is under `tests/` (e.g., match `b/tests/`). For example:
- On `^diff --git ` lines, compute `skip = ($0 ~ /^diff --git a\/[^ ]+ b\/tests\//)`
- Print lines only when `skip==0`
This keeps renames like `tests/foo -> src/foo` in the diff while excluding files whose new location is `tests/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


sed -i '/^[[:space:]]*$/d' .claude-review/changed_files.txt || true

if [[ ! -s .claude-review/review.diff ]] || [[ ! -s .claude-review/changed_files.txt ]]; then
echo "Prepared diff or changed file list is empty; falling back to full review."
build_full
sed -i '/^[[:space:]]*$/d' .claude-review/changed_files.txt || true
fi

echo "review_diff_path=.claude-review/review.diff" >> "$GITHUB_OUTPUT"
echo "changed_files_path=.claude-review/changed_files.txt" >> "$GITHUB_OUTPUT"

Expand All @@ -190,6 +138,10 @@ jobs:

mkdir -p .claude-review/context

# Prioritize src/ files for context (most likely to need review)
sort -t/ -k1,1 -s < "${{ steps.review_input.outputs.changed_files_path }}" | \
awk '/^src\//{print; next} {rest[NR]=$0} END{for(i in rest) print rest[i]}' > .claude-review/sorted_files.txt

while IFS= read -r path; do
[[ -z "$path" ]] && continue
COUNT=$((COUNT + 1))
Expand All @@ -206,10 +158,10 @@ jobs:
[[ -z "$CONTENT" ]] && continue

LINE_COUNT="$(printf '%s' "$CONTENT" | wc -l | tr -d ' ')"
if [[ "$LINE_COUNT" -le 400 ]]; then
if [[ "$LINE_COUNT" -le 1500 ]]; then
printf '%s' "$CONTENT" > ".claude-review/context/$SAFE_NAME"
fi
done < "${{ steps.review_input.outputs.changed_files_path }}"
done < .claude-review/sorted_files.txt

echo "context_dir=.claude-review/context" >> "$GITHUB_OUTPUT"

Expand All @@ -221,6 +173,7 @@ jobs:
: > .claude-review/output.md

- name: Run Claude Code Review
continue-on-error: true
uses: anthropics/claude-code-action@v1
Comment on lines 175 to 177
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Hidden claude step failures 🐞 Bug ✧ Quality

continue-on-error: true on the Claude step prevents the job from failing, so the existing `if:
failure()` fallback summary will never run and Claude outages/quota failures become silent. This can
leave PRs appearing “successfully reviewed” while no review is published.
Agent Prompt
### Issue description
`continue-on-error: true` makes Claude failures non-fatal, but then `if: failure()` never triggers, so failures are not surfaced anywhere.

### Issue Context
You likely want to avoid blocking CI, but still need visibility when the review step fails (quota/auth/outage).

### Fix Focus Areas
- .github/workflows/claude-code-review.yml[175-177]
- .github/workflows/claude-code-review.yml[266-315]

### Suggested change
1) Give the Claude step an `id` (e.g., `id: claude`).
2) Add a follow-up step that runs **always** (`if: always()`) and checks `steps.claude.outcome` / `steps.claude.conclusion`.
3) If it failed, write a clear message to `$GITHUB_STEP_SUMMARY` (and/or post/update the primary comment with an error marker) so failures are visible even though the job stays green.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
Expand All @@ -229,16 +182,13 @@ jobs:
plugins: "code-review@claude-code-plugins"
claude_args: >
--dangerously-skip-permissions
--max-turns 90
--max-turns 40
--allowedTools
"Bash"
prompt: |
You are running in GitHub Actions review automation.

REQUESTED REVIEW MODE: ${{ steps.state.outputs.effective_mode }}
ACTUAL REVIEW MODE: ${{ steps.review_input.outputs.actual_mode }}
PR NUMBER: ${{ steps.mode.outputs.pr_number }}
PREVIOUS REVIEWED SHA: ${{ steps.state.outputs.previous_reviewed_sha }}
CURRENT HEAD SHA: ${{ steps.state.outputs.current_head_sha }}

Review ONLY the prepared inputs for this run.
Expand Down Expand Up @@ -278,23 +228,25 @@ jobs:
8) Do NOT post, update, or create GitHub comments yourself

Review policy:
- In full mode, review the full PR diff.
- In incremental mode, review only the delta diff.
- In incremental mode, report only new issues introduced by that delta.
- Do NOT repeat earlier findings.
- Review the full PR diff.
- Do NOT restate the full PR summary.
- If there are no high-confidence findings, leave .claude-review/output.md empty and STOP.

Review standard:
- Prefer correctness, reliability, cleanup/finalization gaps, inconsistent behavior, edge cases, and meaningful test gaps.
Review standard (in priority order per CLAUDE.md "Code Review Priorities"):
1. Correctness (logic bugs, numerical issues, array bounds)
2. Precision discipline (stp vs wp mixing)
3. Memory management (@:ALLOCATE/@:DEALLOCATE pairing, GPU pointer setup)
4. MPI correctness (halo exchange, buffer sizing, GPU_UPDATE calls)
5. GPU code (GPU_* Fypp macros only, no raw pragmas)
6. Physics consistency (pressure formula matches model_eqns)
7. Compiler portability (4 CI-gated compilers + AMD flang for GPU)
- Avoid style nitpicks.
- A finding is valid only if it is supported by changed lines, with changed-file context used only to confirm it.

Output rules:
- Write markdown only to .claude-review/output.md
- If there are findings, use exactly one of these formats.
- If there are findings, use exactly this format:

For full mode:
Claude Code Review

Head SHA: <sha>
Expand All @@ -308,17 +260,6 @@ jobs:

<!-- claude-review: thread=primary; reviewed_sha=<current_head_sha>; mode=full -->

For incremental mode:
Claude Code Review

Incremental review from: <previous_sha>
Head SHA: <current_sha>

New findings since last Claude review:
- <only high-confidence issues grounded in changed hunks of the delta diff>

<!-- claude-review: thread=primary; reviewed_sha=<current_head_sha>; mode=incremental -->

- Always include the hidden marker exactly once at the end of the file.
- If there are no findings, write nothing.

Expand Down
Loading