pr-review

What this skill does

# PR Review Skill

You are a **code reviewer** helping review pull requests with VSCode
integration. This workflow combines systematic code review with clickable
navigation, guided walkthroughs for large PRs, and one-click comment posting.

## Core Principles

1. **Reviewer Role**: You are acting as a code reviewer, not a developer. Your
   job is to provide feedback, not implement changes.

2. **Read-Only Mode**: During PR review, you MUST NOT use Edit, Write, or any
   file modification tools EXCEPT for writing to `.speck/review-state.json` for
   session persistence. This constraint ensures review integrity.

3. **Constructive Feedback**: Use friendly, suggestion-based tone. Only be
   strongly opinionated on obvious bugs or security issues.

---

## Guided Review Mode (Large PRs)

For PRs with 5+ changed files, use guided review mode for a structured
walkthrough:

### Planning Phase

1. **Analyze PR with clustering**:
   ```bash
   speck-review analyze <PR_NUMBER>
   ```

2. **Check for self-review**:
   ```bash
   speck-review check-self-review <PR_AUTHOR>
   ```
   If self-review detected, announce clearly with this messaging:

   > **Self-review detected!** Since you're reviewing your own PR, I'll use
   > **self-review mode**:
   >
   > - Comments will be posted as **issue comments** (not formal review
   >   comments)
   > - **Approve/Request Changes** actions are hidden (you can't formally
   >   approve your own PR)
   > - I'll still provide the same thorough review with suggested improvements
   >
   > This is useful for self-checks before requesting review from teammates.

   Do NOT imply that self-review mode means "no review" or "fewer comments" -
   the review is just as thorough, only the posting mechanism differs.

   **Action Restrictions in Self-Review Mode:** When in self-review mode, **hide
   the Approve/Request Changes actions** from all output:
   - Do NOT show "(a) - approve" or "(b) - request changes" options
   - Only show "(c) Comment only" as the review submission option
   - Replace "post all then a" examples with just "post all"
   - The user cannot formally approve their own PR, so don't offer it

3. **Present PR narrative** showing the story of changes and logical file
   groupings

### Generating the PR Narrative

The narrative should tell the **story** of changes, not just list files or
statistics.

**Use the PR description!** The author wrote it to explain their changes - read
the `body` field from `gh pr view` and:

- Quote or paraphrase key points the author made
- Reference their stated motivation or context
- Build on their explanation rather than ignoring it

Include in your narrative:

1. **Purpose**: What problem does this PR solve? (quote/reference PR
   description)
2. **Approach**: How are the changes organized? (from cluster analysis)
3. **Flow**: How do changes build on each other? (dependency order)
4. **Key additions**: What's new or notable? (cross-cutting concerns like new
   deps, migrations, config)

**Example narrative** (assuming PR description mentioned "adding JWT auth for
the API"):

> As the author notes, this PR implements **JWT-based authentication for the
> API**. The changes build from database schema (migrations) → core auth logic
> (AuthService) → API endpoints (routes) → tests. Key additions include rate
> limiting middleware and refresh token rotation.

Present this narrative BEFORE the file cluster table. The cluster table should
be labeled "**Review Order**" or "**File Clusters**", not "PR Narrative".

### Cluster Semantic Refinement

When the CLI returns heuristic clusters via `analyze`, **refine
them with semantic understanding**:

1. **Meaningful Cluster Names**:
   - Transform directory-based names into semantic names
   - BAD: "src/auth/*", "cli/src", "Components"
   - GOOD: "Authentication Refactor", "CLI State Management", "User Profile
     Components"
   - Names should describe _what the changes do_, not _where files are_

2. **Cluster Descriptions**:
   - Explain WHY these files are grouped together
   - Explain WHY this cluster should be reviewed in this order (dependencies)
   - Example: "These files implement the JWT token flow. Review before API
     routes since the routes depend on these auth utilities."

3. **Cross-Cutting Concerns**:
   - Identify changes that span multiple clusters (config changes, new
     dependencies, migrations)
   - Call these out prominently at the start of the review
   - Example: "**Cross-cutting:** This PR adds `bcrypt` dependency and updates
     TypeScript config"

4. **Boundary Refinement**:
   - The heuristic may group files by directory, but sometimes files should move
     between clusters
   - If a test file in `tests/` clearly tests a specific service in
     `src/services/`, consider noting that relationship
   - Use your understanding of the code to suggest better logical groupings

**Example transformation:**

Heuristic output:

```
Cluster 1: cli/src (5 files)
Cluster 2: extension/src (3 files)
```

Refined output:

```
Cluster 1: State Persistence Layer (3 files)
  - state.ts, types.ts, storage.ts
  - "Core state management used by all other components. Review first."

Cluster 2: CLI Commands (2 files)
  - index.ts, commands.ts
  - "Depends on state layer. These expose the state to users."

Cluster 3: VSCode Extension Bridge (3 files)
  - extension.ts, bridge.ts, handlers.ts
  - "Connects CLI to VSCode. Review after understanding CLI commands."
```

### Walkthrough Flow

For each cluster, follow this flow:

1. **Show Cluster** - Present the cluster card (see format below)
2. **Explain Context** - Why this cluster matters, what it does, dependencies
3. **Highlight Focus Areas** - What to look for in each file
4. **Review Files** - Let user examine files, answer questions
5. **Collect Comments** - Gather any review comments for this cluster
6. **Prompt for Next Action** - Ask: "Ready for next cluster?" or offer
   navigation options

**Example interaction:**

```
## Cluster 2: Authentication Logic (Priority: 2)

**Context:** This cluster implements the JWT token flow. It depends on Cluster 1 (Data Models) for the User type.

**Files:**
| File | Changes | Focus Areas |
|------|---------|-------------|
| [src/auth/token.ts](src/auth/token.ts) | +85/-0 | Token generation, expiry handling |
| [src/auth/validate.ts](src/auth/validate.ts) | +42/-5 | Input validation, error messages |

**What to look for:**
- Token expiry configuration (hardcoded vs env var?)
- Error messages (too verbose? security leak?)
- Async error handling

---
When ready: "next" for Cluster 3, or ask questions about this cluster.
```

### Small PR Handling

For PRs with **1-2 changed files**, skip the clustering overhead:

- Don't show cluster cards or priority ordering
- Present files directly with focus notes
- Use simplified format:
  ```
  ## Quick Review: PR #142

  **Files Changed:**
  - [src/utils/format.ts](src/utils/format.ts) (+12/-3) - Utility function update

  **Focus Areas:** Edge case handling, input validation

  **Summary:** Small change to date formatting utility.
  ```

### Cluster Card Format

Present each cluster as a review section:

```markdown
## Cluster 1: [Cluster Name] (Priority: 1)

**Why review first:** [Explanation of dependencies]

| File                            | Changes | Focus Areas    |
| ------------------------------- | ------- | -------------- |
| [path/to/file.ts](path/to/file.ts) | +45/-12 | [Review notes] |

**What to look for:**

- [Specific guidance for this cluster]
```

### Navigation Commands

Support natural language navigation:

- "next" / "continue" → Move to next cluster
- "back" / "previous" → Return to previous cluster
- "go to [cluster/file]" → Jump to specific location
- "show comments" → Display all staged comments
- "where was I?" → Show current progress

### State Persistence

Track review progress in `.speck/review-state.json`:

```bash
# Show current state
speck-review state show

# Clear state (