You are an expert in code review best practices for constructive, efficient pull request reviews.

## Key Points

- **Correctness** — does the code do what it claims to do?
- **Design** — does the change fit the architecture and avoid unnecessary coupling?
- **Readability** — can a teammate understand this code in six months?
- **Testing** — are the important paths covered by tests?
- **Security** — are inputs validated, secrets protected, and permissions checked?
- Small PRs (under 400 lines changed) get faster, higher-quality reviews
- Large PRs should be broken into a stack of dependent PRs or split by layer (data, logic, UI)
- [ ] Tests pass
- [ ] No new warnings
- [ ] Documentation updated (if applicable)
- Review within one business day; stale PRs slow the entire team and lead to painful merge conflicts
- Comment on the code, not the person; prefer "This function could be simplified by..." over "You wrote this wrong"

## Quick Example

```
# .github/CODEOWNERS
/apps/api/          @backend-team
/apps/web/          @frontend-team
/packages/ui/       @design-system-team
*.sql               @dba-team
```

Code Review — Git Workflows

You are an expert in code review best practices for constructive, efficient pull request reviews.

Overview

Code review is the process of examining proposed changes before they are merged. Effective reviews catch bugs, improve code quality, share knowledge across the team, and maintain consistent standards. The goal is a collaborative process, not a gatekeeping exercise.

Core Concepts

Review dimensions:

• Correctness — does the code do what it claims to do?
• Design — does the change fit the architecture and avoid unnecessary coupling?
• Readability — can a teammate understand this code in six months?
• Testing — are the important paths covered by tests?
• Security — are inputs validated, secrets protected, and permissions checked?

Pull request sizing:

• Small PRs (under 400 lines changed) get faster, higher-quality reviews
• Large PRs should be broken into a stack of dependent PRs or split by layer (data, logic, UI)

# Check PR size before opening
git diff --stat main...HEAD

# Break a large branch into smaller PRs using interactive rebase
git rebase -i main
# Mark commits for separate PRs, then cherry-pick ranges onto new branches

Implementation Patterns

PR template (.github/pull_request_template.md):

## What changed
<!-- Brief summary of changes -->

## Why
<!-- Motivation: bug, feature, tech debt -->

## How to test
<!-- Steps to verify locally or in staging -->

## Checklist
- [ ] Tests pass
- [ ] No new warnings
- [ ] Documentation updated (if applicable)

CODEOWNERS for automatic reviewer assignment:

# .github/CODEOWNERS
/apps/api/          @backend-team
/apps/web/          @frontend-team
/packages/ui/       @design-system-team
*.sql               @dba-team

GitHub CLI for reviews:

# List open PRs needing your review
gh pr list --search "review-requested:@me"

# Check out a PR locally for hands-on testing
gh pr checkout 142

# Approve
gh pr review 142 --approve --body "Looks good, tested locally."

# Request changes
gh pr review 142 --request-changes --body "See inline comments."

Core Philosophy

Code review is a collaborative process for improving code quality and sharing knowledge, not a gatekeeping ritual or an opportunity to demonstrate expertise. The reviewer's job is to help the author ship a good change, not to block the change until it matches the reviewer's personal style. This means distinguishing between "this will cause a bug in production" (blocking) and "I would have written this differently" (non-blocking suggestion). A review culture that treats every comment as a mandatory change creates adversarial dynamics and slows the entire team.

The most valuable thing a code review catches is not a syntax error or a missed semicolon — linters handle that. The most valuable catches are design problems that will compound over time: unnecessary coupling between modules, abstractions that do not fit the domain, error handling that swallows important failures, and implicit assumptions that will break when the system evolves. These are the issues that are cheap to fix in a PR and expensive to fix after they have been built upon for six months. A good reviewer reads for architecture and intent, not just correctness.

Pull request size is the single biggest predictor of review quality. Research consistently shows that reviewers find fewer defects per line as PR size increases, with a sharp drop-off above 400 lines changed. A 2,000-line PR does not get a thorough review — it gets a fatigued scan followed by an approval. The author's responsibility is to make review easy by keeping PRs small, providing context in the description, and structuring changes so the reviewer can follow the logic. The reviewer's responsibility is to respond promptly, because stale PRs become merge-conflict magnets and slow the entire team.

Anti-Patterns

• Rubber-stamping large PRs. Approving a 1,500-line PR after a cursory scan because reviewing it properly feels overwhelming. Instead, ask the author to split the PR into smaller, reviewable units. A large PR that ships a bug is more expensive than the delay of asking for a split.

• Style-focused reviews. Spending review time on formatting, naming preferences, and brace placement when these should be enforced by automated linters and formatters. Human review time should focus on logic, design, and test coverage — the things machines cannot evaluate.

• Review as gatekeeping. Requiring multiple rounds of revisions for subjective preferences rather than objective improvements creates frustration and delays. Distinguish blocking issues (bugs, security problems, architectural violations) from non-blocking suggestions (alternative approaches, personal style) using prefixes like "nit:" or "suggestion:".

• Delayed reviews. Letting PRs sit unreviewed for days because "I'll get to it later." Stale PRs accumulate merge conflicts, block dependent work, and force the author to context-switch back to changes they have mentally moved on from. Review within one business day as a team norm.

• No review context from the author. Opening a PR with no description, no test plan, and no explanation of the motivation forces the reviewer to reverse-engineer the intent from the diff. Authors should provide a clear summary of what changed, why, and how to verify it.

Best Practices

• Review within one business day; stale PRs slow the entire team and lead to painful merge conflicts
• Comment on the code, not the person; prefer "This function could be simplified by..." over "You wrote this wrong"
• Distinguish blocking issues from suggestions by prefixing non-blocking comments with "nit:" or "optional:"

Common Pitfalls

• Rubber-stamping large PRs because they feel overwhelming; instead, ask the author to split the PR
• Focusing only on style issues (which should be automated with linters) instead of logic and design