review

name	review
description	Review code for quality, root cause analysis, and fix confidence. Supports PR review and local review of uncommitted/branch changes. Default mode is local (reviews current branch changes). Triggers on: review pr, review this pr, /review <pr_url>, /review local, /review, check pr quality.
allowed-tools	Bash(gh issue list:*), Bash(gh issue view:*), Bash(gh pr list:*), Bash(gh pr view:*), Bash(gh pr diff:*), Bash(git diff:*), Bash(git log:*), Bash(git status:*), Bash(git merge-base:*), Bash(git branch:*), Bash(git rev-parse:*), Read, Edit, Grep, Glob, Agent

Code Review Skill

Perform a comprehensive review of code changes in any repository. Supports two modes:

• Local mode (default): Reviews uncommitted changes + current branch's diff from the main branch
• PR mode: Reviews a specific pull request by URL or number

---

The Job

Determine Review Mode

Parse the arguments to determine the mode:

• /review or /review local -> Local mode
• /review <pr_url> or /review <pr_number> -> PR mode

If no argument is provided or the argument is local, use local mode.

---

Paths

This skill runs from the root of the current repository.

• Auto-detect the repo root via git rev-parse --show-toplevel
• Auto-detect the remote repo identifier (e.g., owner/repo) via gh repo view --json nameWithOwner --jq '.nameWithOwner'

---

Local Mode

When reviewing local changes, gather the diff from two sources:

Step L1: Determine the Base Branch

The base branch is what this branch's changes should be compared against. Do NOT assume main or master -- the branch may depend on another feature branch.

Detect the base branch in this order:

1. Check for an existing PR: If a PR exists for this branch, use its base branch:

   CURRENT_BRANCH=$(git branch --show-current)
   REPO=$(gh repo view --json nameWithOwner --jq '.nameWithOwner')
   PR_BASE=$(gh pr view "$CURRENT_BRANCH" --repo "$REPO" \
     --json baseRefName --jq '.baseRefName' 2>/dev/null) || true
   

2. Check the upstream tracking branch: If no PR exists, check what the branch tracks:

   TRACKING=$(git rev-parse --abbrev-ref \
     "$CURRENT_BRANCH@{upstream}" 2>/dev/null) || true
   # Strip the remote prefix (e.g., "origin/branch-A" -> "branch-A")
   

3. Fall back to default branch: If neither method yields a result, detect the default branch:

   DEFAULT_BRANCH=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null \
     | sed 's|refs/remotes/origin/||') || DEFAULT_BRANCH="main"
   

Step L2: Gather Local Changes

# 1. Get the merge base with the detected base branch
MERGE_BASE=$(git merge-base HEAD $BASE_BRANCH)

# 2. Get all committed changes on this branch since diverging from base
git diff $MERGE_BASE..HEAD

# 3. Get uncommitted changes (staged + unstaged)
git diff HEAD

# 4. Get list of changed files for context
git diff --name-only $MERGE_BASE..HEAD
git diff --name-only HEAD

Combine these diffs to form the complete set of changes to review. The combined diff represents what would be in a PR against $BASE_BRANCH if one were created right now.

Report the base branch at the start of the review so it's clear what the changes are compared against (e.g., "Reviewing against base branch: branch-A").

Step L3: Gather Context

1. Check the branch name for hints about what the change does:

   git branch --show-current
   

2. Check recent commit messages on this branch for context:

   git log $MERGE_BASE..HEAD --oneline
   

3. Read the modified files in full to understand the surrounding code context

Then proceed to the Common Analysis Steps below (Step 3 onward), using the gathered diff instead of a PR diff.

Note: For local reviews, skip Steps 1-2 (PR-specific steps) and the best practices discovery step (Step 6.1) can proceed without GitHub data.

---

PR Mode

When reviewing a PR, follow Steps 1-3 below, then continue with the Common Analysis Steps.

Step 1: Parse PR URL and Gather Context

Extract PR information from the provided URL or number:

# Example: https://github.com/owner/repo/pull/12345
PR_REPO="owner/repo"  # extract from URL, or auto-detect from current repo
PR_NUMBER="12345"      # extract from URL or argument

If only a number is given, auto-detect the repo:

PR_REPO=$(gh repo view --json nameWithOwner --jq '.nameWithOwner')

Get PR details:

gh pr view $PR_NUMBER --repo $PR_REPO --json title,body,state,headRefName,author,files

---

Step 2: Research Previous Fix Attempts and Prove Differentiation (PR Mode Only)

CRITICAL: Before evaluating the current fix, understand what has been tried before. If previous attempts exist, the current fix MUST prove it is materially different or the review is an AUTOMATIC FAIL.

Where to search: Previous fix attempts live as pull requests in the target repository. Search by issue number AND by test name/keywords, since not all PRs reference the issue directly:

# Extract issue number from PR body
ISSUE_NUMBER="<extracted from PR body>"

# Search PRs in the target repo by issue number and test name
gh api search/issues --method GET \
  -f q="repo:$PR_REPO is:pr $ISSUE_NUMBER OR <test-name>" \
  --jq '.items[] | {number, title, state, html_url, user: .user.login}'

For each previous attempt found:

# Get the diff to understand what was tried
gh pr diff <pr-number> --repo $PR_REPO

# Get review comments to understand why it failed/was rejected
gh pr view <pr-number> --repo $PR_REPO --json reviews,comments

Document findings:

• What approaches were tried before?
• Why did they fail or get rejected?
• Are there patterns in the failures?

Differentiation Requirement (AUTOMATIC FAIL if not met)

When previous fix attempts exist, you MUST compare the current PR's diff against each previous attempt's diff and answer:

1. Is the approach materially different? Compare the actual code changes, not just the PR description. Look at:

   • Are the same files being modified?
   • Are the same lines/functions being changed?
   • Is the same strategy being applied (e.g., both add a wait, both add a null check, both reorder operations)?

2. If the approach IS different, explain HOW:

   • "Previous PR #1234 added a RunUntilIdle() call. This PR instead uses TestFuture to synchronize on the specific callback."
   • "Previous PR #1234 disabled the test. This PR fixes the underlying race condition by adding an observer."

3. If the approach is the same or substantially similar -> AUTOMATIC FAIL:

   • Same files modified with same type of change
   • Same strategy (e.g., both add timing delays, both add the same kind of guard)
   • Same root cause explanation with no new evidence
   • Cosmetically different but functionally identical (e.g., different wait duration, different variable name for the same fix)

The burden of proof is on the current fix. If you cannot clearly articulate why this fix is different from previous failed attempts, the review MUST FAIL with the reason: "Fix is not materially different from previous attempt(s) #XXXX."

---

Common Analysis Steps

The following steps apply to both local and PR mode reviews.

---

Step 3: Fetch Diff and Classify Changed Files

For PR mode, fetch the full diff once and save it for subagent use:

PR_DIFF=$(gh pr diff $PR_NUMBER --repo $PR_REPO)

For local mode, the diff was already gathered in Step L2. Combine the committed + uncommitted diffs into PR_DIFF.

Extract the file list from the diff:

echo "$PR_DIFF" | grep '^diff --git' | sed 's|.*b/||'

File classification is handled automatically in Step 6.1 -- changed files are categorized by extension and path to match relevant documentation.

---

Step 4: Fetch GitHub Data (PR Mode Only)

For the associated issue (if any):

# Detect the issues repo -- may be same as PR repo or a separate tracker
gh issue view $ISSUE_NUMBER --repo $PR_REPO --json title,body,comments

For PR reviews and comments:

gh api repos/$PR_REPO/pulls/$PR_NUMBER/reviews --paginate
gh api repos/$PR_REPO/pulls/$PR_NUMBER/comments --paginate
gh api repos/$PR_REPO/issues/$PR_NUMBER/comments --paginate

---

Step 5: Analyze the Proposed Changes

Analyze the code in context:

1. Read the modified files from the repo:

   # For each changed file, read the full file to understand context
   

2. Read related files to understand the module:

   • Header files (.h/.hpp) for modified implementation files
   • Interface/type definition files for the modified code
   • Other files in the same directory
   • Test files that exercise the modified code

Questions to answer:

1. What files are changed?
2. What is the nature of the change?
   • Is it a code fix, test fix, or both?
   • Is it adding a filter/disable (potential workaround)?
3. Does the change match the problem description?
4. Is the change minimal and focused?
5. Does the fix make sense given the surrounding code?

---

Step 6: Check Against Best Practices (Chunked Subagent Review)

IMPORTANT: The main context does NOT load best practices docs directly. Each review is performed by subagents -- one per chunk of ~3 rules -- running in parallel. Large best-practice documents are split into evenly-sized chunks, so each subagent handles a focused set of rules. This ensures every rule is systematically checked rather than relying on a single pass to hold many rules in mind.

ZERO-TOLERANCE RULE: You MUST launch a subagent for EVERY chunk from EVERY discovered document. No exceptions. No filtering. No "focusing on key areas." No commentary about the number of chunks. Just launch them all.

CRITICAL -- NO SHORTCUTS FOR LARGE DIFFS: Regardless of diff size (even 100KB+), you MUST pass the complete, untruncated diff to every subagent and review ALL changed files. Do NOT skip files, truncate the diff, selectively review "key chunks", or take any other shortcut based on diff size. The chunked subagent architecture is specifically designed to handle large diffs -- each subagent only checks ~3 rules, so the diff size is not a constraint. If the diff is large, that means MORE subagents are needed, not fewer.

CRITICAL -- LAUNCH ALL DISCOVERED DOCS: You MUST launch subagents for ALL documents returned by the discovery step -- not just the ones you consider "most relevant." If a document is found, it MUST get a subagent. Do NOT second-guess the discovery logic or selectively skip documents. The only filtering happens during discovery; you do not apply additional filtering.

CRITICAL -- NO COMMENTARY ABOUT CHUNK COUNT: Do NOT output any text commenting on the number of chunks, expressing concern about the volume of work, or announcing that you will "focus on key areas" or "launch focused subagents for the most relevant areas." Just launch ALL subagents silently. The number of chunks is irrelevant -- launch them all in parallel without editorializing. Any message like "Given the large number of chunks (N total), I'll focus on..." is WRONG and violates this skill's requirements. You must launch every single chunk subagent regardless of how many there are.

Step 6.1: Discover Applicable Docs and Build Chunks

Search the repository for best practices, standards, and contributing documentation. Look in these locations (in priority order):

REPO_ROOT=$(git rev-parse --show-toplevel)

# Search for best practices / standards docs in common locations
DOC_PATHS=""
for dir in \
  "$REPO_ROOT/docs/best-practices" \
  "$REPO_ROOT/docs/standards" \
  "$REPO_ROOT/docs/guidelines" \
  "$REPO_ROOT/docs/contributing" \
  "$REPO_ROOT/.github"; do
  if [ -d "$dir" ]; then
    DOC_PATHS="$DOC_PATHS $dir"
  fi
done

# Also check for standalone contributing/standards files
for f in \
  "$REPO_ROOT/CONTRIBUTING.md" \
  "$REPO_ROOT/.github/CONTRIBUTING.md" \
  "$REPO_ROOT/docs/CONTRIBUTING.md" \
  "$REPO_ROOT/CODING_STANDARDS.md" \
  "$REPO_ROOT/STYLE_GUIDE.md"; do
  if [ -f "$f" ]; then
    DOC_PATHS="$DOC_PATHS $f"
  fi
done

If no project-specific docs are found, fall back to general software engineering best practices. In this case, construct a single synthetic chunk covering:

• Code correctness and logic errors
• Error handling completeness
• Resource management (memory leaks, unclosed handles, dangling references)
• Concurrency safety (race conditions, deadlocks, missing synchronization)
• Security basics (input validation, injection, secrets exposure)
• API misuse and contract violations
• Test quality (assertions, coverage of edge cases, flaky patterns)
• Naming clarity and code readability

Chunking logic: For each discovered document, split it into chunks of ~3 ## headings each. Small documents (<=5 headings) stay as one chunk. For each chunk, record:

• doc: the source document filename
• chunk_index / total_chunks: position within the document
• headings: list of rule heading texts (for the audit trail)
• content: the full text to pass to the subagent (includes the doc header + the chunk's rules)

Use Bash + simple text processing to split the documents, or use the Read tool to load each doc and split by ## headings in your reasoning.

File type matching: Filter documents by relevance to the changed files:

• Testing docs apply when test files are changed
• Language-specific docs apply when files of that language are changed
• General docs (architecture, contributing, documentation) always apply
• Match by naming convention: a doc named testing-python.md applies when .py test files are changed; android.md applies when .java/.kt files are changed

Step 6.2: Launch Subagents

For each chunk, launch a subagent using the Agent tool. Use multiple Agent tool calls in a single message so they run in parallel.

Each subagent prompt MUST include:

1. The chunk content -- embed the content field directly in the prompt. The subagent does NOT read any files -- all rules are provided inline:

   Here are the best practice rules to check:
   ```markdown
   <chunk content>
   ```
   

2. The diff content -- include the complete, untruncated diff text directly in the prompt. Never omit, summarize, or truncate any portion of the diff regardless of its size. The subagent MUST NOT call gh pr diff or git diff -- the diff is already provided:

   Here is the diff to review:
   ```diff
   <PR_DIFF content>
   ```
   

3. The review rules (copied into the subagent prompt):
   • Only flag violations in ADDED lines (+ lines), not existing code
   • Also flag bugs introduced by the change (e.g., missing string separators, duplicate entries, code inside wrong guard)
   • Check surrounding context before making claims. When a violation involves dependencies, includes, or patterns, read the full file context to verify your claim is accurate. Do NOT claim a PR "adds a dependency" or "introduces a pattern" if it already existed before the PR.
   • Only comment on things the author introduced. If a dependency, pattern, or architectural issue already existed before this PR, do not flag it -- even if it violates a best practice. The author is not responsible for pre-existing issues. Focus exclusively on what the changes add or modify.
   • Do not suggest renaming imported symbols defined outside the change. When a + line imports or calls a function/class/variable from another module, and that symbol's definition is NOT in a file changed by the diff, do not comment on the symbol's naming. Only flag naming issues on symbols that are defined or renamed within the changed files.
   • Security-sensitive areas (authentication, crypto, credentials, payments) deserve extra scrutiny -- type mismatches, truncation, and correctness issues should use stronger language
   • Do NOT flag: existing code not being changed, template functions defined in headers, simple inline getters in headers, style preferences not in the documented best practices, include/import ordering (this is handled by formatting tools and linters)
   • Every claim must be verified in the best practices source document. Do NOT make claims based on general knowledge or assumptions about what "should" be a best practice. If the best practices docs do not contain a rule about something, do NOT flag it as a violation -- even if you believe it to be true. Hallucinated rules erode trust and waste developer time. When in doubt, do not comment. (Exception: when using the general engineering fallback, flag only clear correctness/security/logic bugs.)
   • Comment style: short (1-3 sentences), targeted, acknowledge context. Use "nit:" for genuinely minor/stylistic issues. Substantive issues (test reliability, correctness, banned APIs) should be direct without "nit:" prefix
4. Best practice link requirement -- if the repo's docs use stable ID anchors (e.g., <a id="CS-001"></a>) on the line before the heading, for each violation the subagent MUST include a direct link using that ID. The link format is:

   https://github.com/$PR_REPO/tree/$DEFAULT_BRANCH/docs/best-practices/<doc>.md#<ID>
   

   CRITICAL: The rule_link fragment MUST be an exact <a id="..."> value from the rules provided in the chunk. Do NOT invent IDs, guess ID numbers, or construct anchors from heading text. If no <a id> tag exists for the rule, omit the rule_link field entirely.
5. The systematic audit requirement (Step 6.3 below)
6. Required output format (Step 6.4 below)

Step 6.3: Systematic Audit Requirement

CRITICAL -- this is what prevents the subagent from stopping after finding a few violations.

The subagent MUST work through its chunk heading by heading, checking every ## rule against the diff. It must output an audit trail listing EVERY ## heading in the chunk with a verdict:

AUDIT:
PASS: Always Include What You Use (IWYU)
PASS: Use Positive Form for Booleans and Methods
N/A: Consistent Naming Across Layers
FAIL: Don't Use rapidjson
PASS: Use CHECK for Impossible Conditions
... (one entry per ## heading in the chunk)

Verdicts:

• PASS: Checked the diff -- no violation found
• N/A: Rule doesn't apply to the types of changes in this diff
• FAIL: Violation found -- must have a corresponding entry in VIOLATIONS

This forces the model to explicitly consider every rule rather than satisficing after a few findings.

Step 6.4: Required Subagent Output Format

Each subagent MUST return this structured format:

DOCUMENT: <document name> (chunk <chunk_index+1>/<total_chunks>)

AUDIT:
PASS: <rule heading>
N/A: <rule heading>
FAIL: <rule heading>
... (one line per ## heading in the chunk)

VIOLATIONS:
- file: <path>, line: <line_number>, severity: <"high"|"medium"|"low">, rule: "<rule heading>", rule_link: <full URL to the rule heading>, issue: <brief description>, draft_comment: <1-3 sentence comment>
- ...
NO_VIOLATIONS (if none found)

Severity guide:
- **high**: Correctness bugs, use-after-free, security issues, banned APIs, test reliability problems
- **medium**: Substantive best practice violations (wrong container type, missing error handling, architectural issues)
- **low**: Nits, style preferences, missing docs, naming suggestions, minor cleanup

Step 6.5: Aggregate and Validate Results

After ALL chunk subagents return:

1. Aggregate violations from all chunk subagents into a single list. Sort by severity: high -> medium -> low.

2. Validate rule links -- for each violation with a rule_link, verify the anchor ID exists in the source document by searching for the <a id="..." string in the doc file. If the ID is invalid, strip the link from the comment text. Violations missing rule_link that are not genuine bug/correctness/security findings should be dropped.

3. Deep-dive validation -- before including in the report, validate every remaining violation by reading the actual source code:

   • Read the actual source file at and around the flagged line using the Read tool (not the diff) to see the full file context
   • Verify the claim is true. If the violation says "this should use X instead of Y", confirm X is actually available, appropriate, and consistent with the rest of the file/module
   • Deprecation claims require verification. Read the actual header or source file to confirm deprecation. Do NOT rely on training data
   • Check surrounding context for justification. Look for comments, TODOs, or patterns that explain the code
   • Drop false positives. If reading the source reveals the violation is incorrect, drop it

4. Present violations one at a time and offer to fix each. After validation, walk through the remaining violations sequentially. For each violation, present it to the user and ask whether they want it fixed:

   **Violation 1/N** (severity: high)
   **File**: path/to/file.cc:42
   **Rule**: <rule heading>
   **Issue**: <description>
   **Suggested fix**: <what the fix would look like>
   
   Fix this violation? (yes/no/skip)
   

   • yes: Apply the fix immediately using the Edit tool. Read the file first if not already loaded, make the targeted change, then confirm what was changed before moving to the next violation.
   • no or skip: Leave the code as-is and move to the next violation.
   • If the user says "fix all" or "yes to all", apply all remaining violations without further prompting.
   • If the user says "stop" or "no to all", skip all remaining violations and proceed to the report.

   Fix guidelines:

   • Fixes must be minimal and targeted -- only change what the violation requires
   • Do NOT refactor surrounding code or make "while you're here" improvements
   • If a fix is ambiguous or could be done multiple ways, explain the options and ask the user which approach they prefer before editing
   • If a violation cannot be auto-fixed (e.g., requires architectural redesign or new tests), say so and move on

5. Include all violations (fixed and unfixed) in the review report under the Best Practices section. Mark each as (fixed) or (unfixed) so the user knows what remains.

---

Step 7: Validate Root Cause Analysis

Read the PR body and any issue analysis carefully.

Check for RED FLAGS indicating insufficient root cause analysis:

Vague/Uncertain Language (FAIL if unexplained)

• "should" - e.g., "This should fix the issue"
• "might" - e.g., "This might be causing the problem"
• "possibly" - e.g., "This is possibly a race condition"
• "probably" - e.g., "The test probably fails because..."
• "seems" - e.g., "It seems like the timing is off"
• "appears" - e.g., "The issue appears to be..."
• "could be" - e.g., "This could be the root cause"
• "may" - e.g., "The callback may not be completing"

These words are acceptable ONLY if followed by concrete investigation:

• BAD: "This should fix the race condition"
• GOOD: "The race condition occurs because X happens before Y. Adding a wait for signal Z ensures proper ordering."

Questions to Ask

1. Can you explain WHY the failure occurs? (Not just symptoms, but cause)
2. Can you explain HOW the fix addresses the root cause? (Mechanism, not hope)
3. Is there a clear causal chain? (A causes B, fix C breaks the chain)

AI Slop Detection

Watch for generic explanations that could apply to any bug:

• "Improved error handling"
• "Fixed timing issues"
• "Better synchronization"
• "Enhanced stability"
• "Optimized performance"
• "Improved robustness"

Demand specifics:

• WHAT timing issue? Between which operations?
• WHAT synchronization was missing? What signal is now used?
• WHERE was the race condition? What two things were racing?
• WHAT error handling was missing? What error was unhandled?

---

Step 8: Additional Best Practices Checks (Non-Chunked)

These checks are performed directly by the main context (not subagents) because they require PR-level reasoning rather than per-rule checking:

Timing-Based "Fixes" (AUTOMATIC FAIL)

If the fix works by altering execution timing rather than adding proper synchronization:

BANNED patterns:

• Adding sleep/delay calls
• Adding logging that changes timing
• Reordering code without synchronization explanation
• Adding arbitrary waits without condition checks

ACCEPTABLE patterns:

• Condition-based polling (wait until condition is true)
• Future/promise-based callback synchronization
• Observer patterns with explicit quit conditions
• Event-driven waiting (MutationObserver, event listeners)

Test Disables

If the fix is disabling a test:

• Is there thorough documentation of why?
• Were other approaches tried first?
• Is the test an upstream test or a project-specific test?
• Is the disable scoped as narrowly as possible (platform-specific, build-type specific)?

Red flags (overly broad disables):

• Disabling across all platforms when failure is only reported on one
• Disabling for all build types when failure is only on sanitizer builds
• No investigation of which CI configurations actually fail

Intermittent/Flaky Test Analysis

For flaky tests, the root cause analysis must explain why the failure is intermittent -- not just why it fails, but why it doesn't fail every time:

Questions to answer:

• What variable condition causes the test to sometimes pass and sometimes fail?
• Is it timing-dependent? (e.g., race between two async operations)
• Is it resource-dependent? (e.g., system load, memory pressure)
• Is it order-dependent? (e.g., test isolation issues, shared state)
• Is it platform-specific? (e.g., only flaky on certain OS/architecture)

Red flags (incomplete analysis):

• "The test is flaky" (without explaining the variable condition)
• "Sometimes passes, sometimes fails" (just restating the symptom)
• "Timing-dependent" (without explaining what timing varies)

---

Step 9: Assess Fix Confidence

Rate confidence level:

HIGH Confidence (likely to work)

• Clear root cause identified and explained
• Fix directly addresses the root cause
• Change is minimal and focused
• Similar patterns exist in codebase
• Tests verify the fix

MEDIUM Confidence (may work, needs verification)

• Root cause identified but explanation has minor gaps
• Fix seems reasonable but relies on assumptions
• Could benefit from additional tests

LOW Confidence (likely to fail or regress)

• Root cause not clearly identified
• Fix is a workaround, not a solution
• Uses timing-based approaches
• Overly complex for the problem
• Changes unrelated code
• Fix is not materially different from a previous failed attempt

---

Step 10: Generate Review Report

CRITICAL: Avoid Redundancy

• Each piece of information should appear ONCE in the report
• Do NOT repeat the same issue in multiple sections
• The verdict reasoning should be a brief reference, not a restatement of everything above

CRITICAL: Fill Informational Gaps Yourself

• If the PR is missing context that you CAN research (e.g., "why does this fail in this environment?"), DO THE RESEARCH and provide the answer in your analysis
• Only list something as an "issue requiring iteration" if it requires action from the PR author that you cannot provide
• The confidence level should reflect the state AFTER you've provided any missing context -- if you filled the gaps, confidence should be higher

CRITICAL: No Vague Language in YOUR Analysis

• The same vague language rules (Step 7) apply to YOUR review output, not just the PR's analysis
• If you write "appears to", "seems to", "might be", etc. in your analysis, you have NOT completed the review
• You must either:
  1. Investigate further until you can make a definitive statement, OR
  2. Flag it as requiring investigation in the "Issues Requiring Author Action" section
• Example of what NOT to do: "The detection appears to return STABLE" -- this is incomplete
• Example of what TO do: Either trace the exact code path to confirm what value is returned, OR list "Determine exact return value in CI environment" as an issue requiring investigation

PR Mode Report Format

# PR Review: #<number> - <title>

## Summary

<2-3 sentences: what this PR does, the root cause, and whether the fix is
appropriate>

## Context

- **Issue**: #<number or "N/A">
- **Previous attempts**: <Brief list or "None found">
- **Differentiation**: <How this fix differs from previous attempts, or "N/A -
  no previous attempts">

## Analysis

### Root Cause

<Summarize the PR's explanation. If incomplete, research and provide the missing
context yourself rather than flagging it as an issue.>

### Fix Evaluation

<Does the fix address the root cause? Any best practices violations?>

### Best Practices (Chunked Subagent Results)

<Summarize findings from the chunked best practices review. List any validated
violations with file, line, severity, and the specific rule violated. Include
rule links where available.>

## Issues Requiring Author Action

<ONLY list issues that genuinely require the PR author to take action. Do NOT
include:

- Informational gaps you filled in the Analysis section
- Context you researched and provided above
- Minor suggestions>

If no issues: "None - PR is ready for review."

## Verdict: PASS / FAIL (assessed AFTER accounting for any context you provided above)

**Confidence**: HIGH / MEDIUM / LOW

<1-2 sentence reasoning>

Local Mode Report Format

# Local Review: <branch-name>

## Summary

<2-3 sentences: what these changes do and whether the approach is sound>

## Changes Overview

- **Branch**: <branch-name>
- **Base branch**: <base-branch> (how it was detected: PR / tracking / default)
- **Files changed**: <count>
- **Commits on branch**: <count> (+ uncommitted changes if any)

## Analysis

### Change Evaluation

<What do the changes accomplish? Is the approach correct? Any logic errors?>

### Best Practices (Chunked Subagent Results)

<Summarize findings from the chunked best practices review. List any validated
violations with file, line, severity, and the specific rule violated. Include
rule links where available.>

## Issues Found

<List significant issues that should be addressed before creating a PR. Do NOT
include:

- Style preferences
- Minor naming suggestions
- Optional refactoring ideas>

If no issues: "None - changes look ready for PR."

## Verdict: PASS / FAIL

**Confidence**: HIGH / MEDIUM / LOW

<1-2 sentence reasoning>

---

Important Guidelines

Only Report Significant Issues

DO report:

• Logic errors or bugs in the fix
• Missing synchronization or race conditions
• Violations of documented best practices
• Incomplete root cause analysis
• High-risk changes without adequate testing
• Potential regressions

DO NOT report:

• Style preferences
• Minor naming suggestions
• Optional refactoring ideas
• "While you're here..." improvements
• Anything that doesn't warrant a round-trip iteration

Be Specific and Actionable

• BAD: "The root cause analysis is weak"
• GOOD: "The PR says 'This should fix the timing issue' but doesn't explain what timing issue exists or why this change fixes it. Specifically, what two operations are racing and how does the new wait prevent that race?"

Read the Source Code

• Always read the actual files from the repo to understand context
• Don't just look at the diff in isolation
• Check related files, headers, and tests
• Before each review comment, verify your claims by reading the relevant source code. Do not make assertions about APIs, patterns, deprecations, or behavior without first confirming them in the actual codebase. Look at how the API/pattern is used elsewhere, check header files for documentation. Every comment you make should be grounded in what the code actually says, not assumptions.

Local by Default

• DO NOT post comments, approve, or request changes on GitHub unless the user explicitly asks
• DO NOT merge or close the PR
• This is an analysis tool for the reviewer's eyes only

Posting to GitHub

If the user asks you to post the review as a comment on GitHub, always prefix the review body with:

This is an automated review for informational purposes only.

This disclaimer must appear at the very beginning of the review body (as plain text, not as a blockquote).

Post as inline code comments when possible. When the review identifies specific issues tied to files and lines, post them as inline review comments on the actual code rather than as a single general comment. Use the GitHub review API to submit a single review with:

• Review body: The summary, verdict, and any general observations (with the disclaimer prefix above)
• Inline comments: Each specific issue placed on its file and line

gh api repos/$PR_REPO/pulls/$PR_NUMBER/reviews \
  --method POST \
  --input - <<'EOF'
{
  "event": "COMMENT",
  "body": "This is an automated review for informational purposes only.\n\n## Summary\n...\n\n## Verdict: PASS/FAIL\n...",
  "comments": [
    {
      "path": "path/to/file.cc",
      "line": 42,
      "side": "RIGHT",
      "body": "specific issue description for this line"
    }
  ]
}
EOF

Key details:

• side: "RIGHT" targets the new version of the file (changed lines)
• line is the line number in the new file
• All inline comments are batched into one review (one notification to the author)
• If an issue can't be tied to a specific line in the diff, include it in the review body instead
• If the inline API call fails for a comment (line outside diff range), fall back to including that issue in the review body

---

Example Usage

Review local changes (default -- no argument needed):

/review
/review local

Review a specific PR by URL:

/review https://github.com/owner/repo/pull/12345

Review a PR by number (auto-detects current repo):

/review 12345

---

Checklist Before Completing Review

Both Modes

• Auto-detected repo root and remote identifier
• Analyzed the diff (local changes or PR diff)
• Searched for best practices / standards / contributing docs in the repo
• Built chunks from each discovered document (~3 headings per chunk)
• Launched parallel subagents for all chunks
• Each subagent produced a full AUDIT trail (one verdict per ## heading)
• Aggregated and validated all violations from subagents
• Deep-dive validated each violation by reading actual source files
• Read the actual source files to understand context
• Validated root cause analysis quality (or assessed code quality for local)
• Checked timing-based fixes and test disables (Step 8)
• Assessed fix confidence level
• Only reported important issues
• Provided clear pass/fail verdict with reasoning

PR Mode Only

• Parsed PR URL and gathered context
• Extracted associated GitHub issue (if any)
• Researched previous fix attempts
• If previous attempts exist: proved current fix is materially different (or FAILED the review)
• Only posted to GitHub if user explicitly requested (with disclaimer prefix)

Local Mode Only

• Detected correct base branch (PR base > tracking branch > default branch)
• Gathered uncommitted changes (staged + unstaged)
• Gathered branch changes since diverging from base branch
• Checked branch name and commit messages for context