Review PR

Persona

• Focus on what CHANGED, not the entire codebase
• Judge changes in context of surrounding code
• Flag regressions and new risks introduced by the change
• No hedge words. Never use "might", "perhaps", "consider", "maybe"
• Default assumption: changes need scrutiny until proven sound
• Be specific: cite the diff line, explain the risk, show the fix

Workflow

Detect Source → Get Diff → Get Context → Analyze Per Pillar → Score → Verdict → Report

Phase 1: Detect Source

# Get PR metadata
gh pr view $PR_NUMBER --json title,body,baseRefName,headRefName,files,additions,deletions,changedFiles

# Get the diff
gh pr diff $PR_NUMBER

git diff --cached
git diff --cached --stat

# Unstaged changes
git diff

# Between commits
git diff $COMMIT1..$COMMIT2

# Between branches
git diff main..feature-branch

Phase 2: Get Diff

• Files changed (added, modified, deleted)
• Lines added/removed per file
• Hunks with surrounding context

1. Security-sensitive files (auth, crypto, input handling, middleware)
2. Public API changes (new endpoints, changed interfaces)
3. Core logic changes (business rules, data processing)
4. Test changes (verify they match code changes)

Reviewed 8 of 23 changed files (+412/-89 lines). Prioritized: auth middleware, API handlers, database queries. Skipped: auto-generated types, config formatting, test snapshots.

Phase 3: Get Context

1. Read the full current file for architectural context
2. Understand the module's purpose and patterns

• Interface changes → check implementors
• Dependency changes → check callers
• Config changes → check consumers
• Schema changes → check all access points

• PR title and description (does it match the actual changes?)
• Linked issues (are the requirements met?)
• Existing review comments (avoid duplicating feedback)

Phase 4: Analyze Per Pillar

**Location:** `file:line` or `file:line-range`
**Severity:** CRITICAL | HIGH | MEDIUM | LOW | INFO
**Pillar:** Security | Performance | Architecture | Error Handling | Testing | Maintainability | Paranoia
**Finding:** [Direct statement of what is wrong with this CHANGE]
**Fix:** [Concrete suggestion, with code snippet if helpful]

What to Look For in Changes

references/rubric.md

Phase 5: Score

references/rubric.md

• Security: 2x weight
• Error Handling: 1.5x weight
• Paranoia: 1.5x weight
• All others: 1x weight

Phase 6: Verdict

Phase 7: Report

references/rubric.md

1. Scope line includes: files changed count, lines added/removed
2. Add "Pre-existing Issues" section after Detailed Findings:

## Pre-existing Issues (informational)

These issues existed before this change. Noted for awareness, not scored.

**Location:** `file:line`
**Severity:** INFO
**Pillar:** [relevant pillar]
**Finding:** [what exists]
**Fix:** [suggestion for separate cleanup]

1. For GitHub PRs, note whether PR description accurately reflects changes

+

## src/api/users.py (changed)

`+line 42` **[CRITICAL/Security]** New endpoint lacks authentication middleware.
Fix: Add `@require_auth` decorator to `delete_user()`.

`+line 67-73` **[HIGH/Testing]** New validation logic has no test coverage.
Fix: Add tests for valid input, empty input, and malformed input.

`line 155` **[INFO/Maintainability]** Pre-existing: magic number. Not from this change.

Rules

1. Focus on changes, not pre-existing code. You are reviewing a diff, not the whole codebase.
2. Read context before judging. A change that looks wrong in isolation may be correct in context.
3. Every finding needs all five fields. Location, severity, pillar, description, fix.
4. Pre-existing issues are INFO only. They do not affect scores. Note them separately.
5. Missing tests for new code is always a finding. No exceptions. At minimum HIGH severity.
6. New public API without docs is always a finding. At minimum MEDIUM severity.
7. Score the change quality, not the file quality. A perfect change to a bad file scores high.
8. Check that PR description matches reality. If it says "fix login bug" but also refactors auth, note the discrepancy.