pr-review

# Comprehensive PR Review (with Retro Learning)

Comprehensive PR review using specialized agents, with automatic knowledge capture.

This is a local fork of `pr-review-toolkit:review-pr` with a retro learning phase that
records review patterns to the knowledge store.

## Scope

This command accepts either a PR reference or a local branch diff. You do not need an open PR to use it.

| Invocation form | What it reviews |
|----------------|-----------------|
| `/pr-review` (no argument) | Uncommitted and committed local changes against the base branch (`HEAD` vs base) |
| `/pr-review 123` or `/pr-review https://github.com/org/repo/pull/123` | The diff of the specified open PR |

When no argument is provided, the command reads `git diff` to determine changed files. When a PR number or URL is provided, it fetches the PR diff via `gh pr view`. All review phases and retro learning apply in both cases.

## Usage

```
/pr-review              # Full review (all aspects)
/pr-review tests errors # Specific aspects only
/pr-review all parallel # All agents in parallel
```

**Related**: `/pr-review-address-feedback` — Process existing GitHub reviewer comments (validate-then-fix workflow)

## Review Aspects

- **comments** - Analyze code comment accuracy and maintainability
- **tests** - Review test coverage quality and completeness
- **errors** - Check error handling for silent failures
- **types** - Analyze type design and invariants (if new types added)
- **code** - General code review for project guidelines
- **simplify** - Simplify code for clarity and maintainability
- **all** - Run all applicable reviews (default)

## Instructions for Claude

**Review Aspects (optional):** "$ARGUMENTS"

### Phase 1: Determine Review Scope

1. Check git status to identify changed files: `git diff --name-only`
2. Parse arguments to see if user requested specific review aspects
3. Default: Run all applicable reviews
4. Check if PR already exists: `gh pr view 2>/dev/null`

### Phase 2: Identify Applicable Reviews

Based on changes:
- **Always applicable**: code-reviewer (general quality)
- **If test files changed**: pr-test-analyzer
- **If comments/docs added**: comment-analyzer
- **If error handling changed**: silent-failure-hunter
- **If types added/modified**: type-design-analyzer
- **If organization Go project** (see Phase 3 detection): organization-specific reviewer for convention compliance
- **After passing review**: code-simplifier (polish and refine)

### Phase 3: Detect Languages and Domain

Before launching agents, detect the primary language(s) of changed files:
- `.go` files → Go domain
- `.py` files → Python domain
- `.ts`/`.tsx` files → TypeScript domain
- `.md` files → Documentation domain

**Organization detection** (run this when Go files are present):
Check if this is an organization-specific Go project using ANY of:
1. Session context contains an org marker (injected by operator-context-detector hook at session start)
2. `go.mod` contains organization-specific imports
3. Module path matches organization patterns

If any check passes → **org domain**: add organization-specific reviewer to the agent list for Phase 4.

**gopls MCP Integration** (MANDATORY when `.go` files are in the diff):

When Go files are detected in the changed files, the review MUST use gopls MCP tools to provide type-aware context to all downstream review agents:

1. Run `ToolSearch("gopls")` to fetch gopls tool schemas
2. Call `go_workspace` to understand module structure, build configuration, and workspace layout
3. Call `go_file_context` on EACH changed `.go` file to get intra-package dependency context (what other files in the same package this file depends on)
4. Collect all type-aware context (module structure, package dependencies, symbol relationships) and pass it as structured input to every review agent dispatched in Phase 4

This replaces text-only grep analysis with type-aware understanding — catching interface implementations, cross-package symbol usage, and dependency relationships that text search misses.

If gopls MCP is unavailable (ToolSearch returns no results), fall back to Grep-based analysis but note the limitation in the review output.

This determines which retro topics are relevant in Phase 8.

### Phase 3.5: Caller Tracing (MANDATORY for Go signature/semantic changes)

**Trigger**: This phase is MANDATORY for Go projects when the diff modifies any of:
- Function or method signatures (parameters added/removed/retyped)
- Parameter semantics (e.g., a parameter now accepts sentinel values like `"*"`)
- Exported symbols (new exports, changed interfaces)
- Sentinel or special values introduced (constants, magic strings, wildcard patterns)

**Skip condition**: If the diff only modifies function bodies without changing signatures, parameter semantics, or exported symbols, skip to Phase 4.

**Steps**:

1. **Identify changed symbols**: Scan the diff for all functions/methods whose signatures or parameter semantics changed. List each with its file, old signature, and new signature.

2. **Find ALL callers**: For each changed symbol:
   - **Preferred**: Use gopls `go_symbol_references` (type-aware, catches interface implementations)
   - **Fallback**: Grep with receiver syntax (e.g., `.GetEvents(` not just `GetEvents`) across the entire repo, excluding vendor/
   - Document the total caller count per symbol

3. **Verify each call site**: Read each caller and verify it has been updated for the new signature/semantics:
   - Does it pass the correct number/type of arguments?
   - Does it handle new return values or error conditions?
   - For interface methods: do ALL implementations honor the new contract?

4. **Trace security-sensitive parameters**: For parameters that control authorization, filtering, data access, or resource scoping:
   - Classify each value source: **user-controlled** (query params, form values, request body, headers) vs **server-controlled** (auth tokens, UUIDs, constants, config)
   - For use