fix: correct Claude PR review to use BASE_SHA for accurate diff comparison

[snomiao]

Summary

• Fixes the Claude automated PR review comparing against wrong commits
• Updates the comprehensive-pr-review.md command to use $BASE_SHA instead of origin/$BASE_BRANCH
• Resolves issue where Claude was reviewing unrelated changes from other PRs

Problem

As identified in #5651 (comment #5651 (comment)), the Claude automated review was incorrectly analyzing changes that weren't part of the PR being reviewed. The review was mentioning Turkish language removal, linkRenderer changes, and other modifications that weren't in the actual PR diff.

Root Cause Analysis

The Issue Explained (from Discord discussion)

When Christian Byrne noticed Claude was referencing things from previous reviews on other PRs, we investigated and found:

1. The backport branch was created from origin/main BEFORE Turkish language support was merged

   • Branch state: main.A
   • Backport changes committed: main.A.Backport

2. Turkish language support was then merged into origin/main

   • Main branch updated to: main.A.Turkish

3. Claude review workflow checked out main.A.Backport and ran git diff against origin/main

   • This compared: main.A.Backport <> main.A.Turkish
   • The diff showed: +++Backport changes and ---Turkish removal
   • Because the common parent of both branches was main.A

Why This Happens

When using origin/$BASE_BRANCH, git resolves to the latest commit on that branch. The diff includes:

1. The PR's actual changes (+++Backport)
2. The reverse of all commits merged to main since the PR was created (---Turkish)

This causes Claude to review changes that appear as "removals" of code from other merged PRs, leading to confusing comments about unrelated code.

Solution

Changed the git diff commands to use $BASE_SHA directly, which GitHub Actions provides as the exact commit SHA that represents the merge base. This ensures Claude only reviews the actual changes introduced by the PR.

Before (incorrect):

git diff --name-only "origin/$BASE_BRANCH"  # Compares against latest main
git diff "origin/$BASE_BRANCH"
git diff --name-status "origin/$BASE_BRANCH"

After (correct):

git diff --name-only "$BASE_SHA"  # Compares against merge base
git diff "$BASE_SHA"
git diff --name-status "$BASE_SHA"

Technical Details

GitHub Actions Environment Variables

• BASE_SHA: The commit SHA of the merge base (where PR branched from main)
• BASE_BRANCH: Not provided by GitHub Actions (this was the bug)
• Using origin/$BASE_BRANCH was falling back to comparing against the latest main commit

Alternative Approaches Considered

1. Approach 1: Rebase/update branch before running Claude review
   • Downside: Changes the PR's commits, not always desirable
2. Approach 2: Use BASE_SHA to diff against the merge base ✅
   • This is what GitHub's PR diff view does
   • Shows only the changes introduced by the PR

Testing

The BASE_SHA environment variable is already correctly set in the claude-pr-review.yml workflow (line 88), so this change will work immediately once merged.

Impact

• Claude reviews will now be accurate and only analyze the actual PR changes
• No false positives about "removed" code from other PRs
• More reliable automated PR review process
• Developers won't be confused by comments about code they didn't change

Verification

You can verify this fix by:

1. Creating a PR from an older branch
2. Merging another PR to main
3. Triggering Claude review with the label
4. Claude should only review the PR's changes, not show removals from the newly merged commits

Credits

Thanks to @christian-byrne for reporting the issue and @snomiao for the root cause analysis.

🤖 Generated with Claude Code

[github-actions]

🎭 Playwright Test Results

⚠️ Tests passed with flaky tests

⏰ Completed at: 09/19/2025, 05:04:53 AM UTC

📈 Summary

• Total Tests: 449
• Passed: 419 ✅
• Failed: 0
• Flaky: 1 ⚠️
• Skipped: 29 ⏭️

📊 Test Reports by Browser

• ✅ chromium: View Report • ✅ 412 / ❌ 0 / ⚠️ 1 / ⏭️ 29
• ✅ chromium-2x: View Report • ✅ 2 / ❌ 0 / ⚠️ 0 / ⏭️ 0
• ✅ chromium-0.5x: View Report • ✅ 1 / ❌ 0 / ⚠️ 0 / ⏭️ 0
• ✅ mobile-chrome: View Report • ✅ 4 / ❌ 0 / ⚠️ 0 / ⏭️ 0

---

🎉 Click on the links above to view detailed test results for each browser configuration.

[snomiao]

@christian-byrne yet another branch can only test after merge to main

but at least the git-diff thing works in my local : D

[christian-byrne]

LGTM! Nice catch!