review-mr

Check CLAUDE.md first for project-specific coding standards and conventions. Apply project rules before general best practices.

• Read the diff first: Never comment on changes without reading the actual diff
• Verify files exist: Check that referenced files are part of this MR
• Trace actual changes: Don't assume what code does - read it
• Cite evidence: Include file:line references for all findings
• Admit uncertainty: If behavior is unclear, say "I need to verify..." and check

• Don't nitpick style in urgent bug fixes
• Don't suggest complete rewrites when small fixes suffice
• Don't make assumptions about code without reading it
• Don't block on minor issues when critical issues exist

1. Identify the branch to review

- If given a branch name, use it

- If given a PR/MR number, use gh pr view to get branch info

- If no argument, review current branch against main/master

1. Gather context

```bash

# Detect the base branch (main, master, develop, etc.)

BASE=$(git symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@^refs/remotes/origin/@@')

# Fallback if remote HEAD not set

if [ -z "$BASE" ]; then

for branch in main master develop; do

git rev-parse --verify $branch &>/dev/null && BASE=$branch && break

done

fi

# Get commit log for the branch

git log $BASE..HEAD --oneline

# Get overall diff stats

git diff $BASE...HEAD --stat

# Get the actual diff

git diff $BASE...HEAD

```

1. Analyze systematically

1. Executive Summary (2-3 sentences)

What does this MR do? What problem does it solve?

2. Changes Overview

• Files changed: X
• Lines added/removed: +X / -X
• Key areas affected: [list main modules/components]

3. Need to Know (Critical for reviewers)

Things that MUST be understood before approving:

• Breaking changes
• New dependencies
• Database migrations
• Environment variables required
• API changes
• Security implications

4. The Good

What's done well:

• Clean code patterns
• Good test coverage
• Proper error handling
• Performance considerations
• Documentation

5. Concerns

Issues that should be addressed:

Critical (must fix before merge):

• Security vulnerabilities
• Data loss risks
• Breaking changes without migration

Important (should fix):

• Missing tests for critical paths
• Error handling gaps
• Performance issues

Minor (nice to have):

• Code style inconsistencies
• Missing documentation
• Refactoring opportunities

6. Questions for the Author

Things that aren't clear from the code:

• Design decisions that need explanation
• Alternative approaches considered
• Testing strategy for edge cases

7. Suggestions

Specific, actionable improvements with code examples where helpful.

• Be specific: Reference files and line numbers
• Be constructive: Suggest solutions, not just problems
• Be proportional: Don't nitpick on style in a bug fix
• Be curious: Ask questions instead of assuming intent
• Be thorough: Check edge cases, error paths, tests

```markdown

# MR Review: [Branch Name]

[2-3 sentence summary of what this MR does]

• Files changed: X
• Additions/Deletions: +X / -X
• Key areas: [list]

• [ ] [Critical item 1]
• [ ] [Critical item 2]

• [Positive point 1]
• [Positive point 2]

Critical

• [file:line] Description of issue

Important

• [file:line] Description of issue

Minor

• [file:line] Description of issue

1. [Question about design/intent]

1. [Area]: [Specific suggestion]

```diff

- old code

+ suggested code

```

[ ] Ready to merge - No critical issues

[ ] Needs changes - Address critical/important issues first

[ ] Needs discussion - Architectural concerns to resolve

```