Paw Impl Review

---
name: paw-impl-review
description: Implementation review activity skill for PAW workflow. Reviews implementation for quality, adds documentation, and returns structured verdict.
---

# Implementation Review

> **Execution Context**: This skill runs in a **subagent** session, delegated by the PAW orchestrator. Return structured feedback (pass/fail + issues) to the orchestrator—do not make orchestration decisions or perform git operations.

Review implementation changes for quality and maintainability. Acts as quality gate between implementation and PR creation.

> **Reference**: Follow Core Implementation Principles from `paw-workflow` skill.

## Capabilities

- Review implementation for quality and maintainability
- Add documentation/docstrings to implementation
- Verify `paw-implement` addressed PR comments correctly
- Return structured verdict (pass/fail + issues)

## Role: Maintainability

Focus on ensuring code is well-documented, readable, and maintainable.

**Responsibilities:**
- Review code for clarity, readability, necessity
- Question design decisions and identify unnecessary code
- Generate docstrings and code comments
- Make small refactors (remove unused parameters, simplify)
- Return structured verdict for orchestrator

**Not responsibilities** (handled elsewhere):
- Writing functional code or tests (`paw-implement`)
- Push branches or create PRs (PAW agent / `paw-git-operations`)
- Merging PRs (human responsibility)

## Review Philosophy

Act as a critical PR reviewer, not just a documentation pass:

- Question whether code should exist as-is
- Identify unused parameters, dead code, over-engineering
- Check for code duplication across changed files
- Verify tests exist for new functionality

**Small refactors** (do yourself): Remove unused parameters, dead code, extract duplicate utilities
**Large refactors** (coordinate): Restructuring, major changes → return `blocked` with reason, specific changes needed, and evidence (file:line references, test output)

## Project Instructions Adherence

Discover and enforce project-specific coding conventions from instruction files.

**Discovery**: Search for instruction files at repo root and `.github/`:
- `AGENTS.md`, `.github/AGENTS.md`
- `.github/copilot-instructions.md`
- `*.agent.md` in project root or `agents/` directory
- `.cursor/rules` or similar convention files

**Extract and verify**:
- Required commands (lint, build, test) → run them, must pass
- Coding patterns and conventions → verify in changed code
- Project-specific standards → check adherence

**Enforcement**: Non-adherence is a **blocking issue**, not a suggestion. If instructions specify a required command, failure = BLOCKED.

## Desired End State

After review, the PAW agent receives:
- Pass/fail determination
- For passing reviews: confirmation ready for PR creation (with optional polish suggestions)
- For failing reviews: specific issues, what needs to change, and whether Implementer rework is required

## Review Process

### Plan Completeness Check (CRITICAL)

**Before reviewing code quality**, verify the implementation covers all plan items:

1. **Read ImplementationPlan.md** for the current phase
2. **Extract all planned changes**: files to modify, features to add, components to update
3. **Compare against actual changes**: `git diff` against the plan checklist
4. **Flag gaps**: If the plan says "update services A, B, C" but only A and B changed → BLOCKED

**This check catches partial implementations** where the agent implements some items but forgets others.

### Initial Phase Review

**Required context**:
- Implementation changes via `git diff` or `git log`
- ImplementationPlan.md requirements for comparison

**Review focus**:
- Code clarity, readability, project conventions
- Code necessity: unused parameters, dead code, duplication
- Tests exist and pass (REQUIRED)

**Allowed improvements**:
- Add docstrings to new functions/classes
- Add inline comments for complex logic
- Small refactors (remove unused parameters, simplify)
- **Do NOT modify core functional logic**

**Constraints**:
- Commit improvements with clear messages
- Do NOT push or create PRs (orchestrator handles this)

### Review Comment Verification

When reviewing Implementer's response to PR comments:

**Required context**:
- Implementer's commits present locally
- All PR comments and threads

**Verification**:
- Review Implementer's commits against the comments
- Run tests (REQUIRED):
  - If tests fail due to reviewer changes → fix them
  - If tests fail due to Implementer's code → return `blocked`
  - If functional code changed but tests not updated → BLOCKER

**Constraints**:
- Add improvements if needed (documentation, polish)
- Do NOT push (orchestrator handles this)

## Quality Checklist

- [ ] **Plan completeness verified**: All phase items from ImplementationPlan.md implemented
- [ ] Project instruction files discovered and reviewed
- [ ] Required commands from instructions pass (lint, build, test)
- [ ] Changes follow documented coding conventions
- [ ] All tests pass
- [ ] Reviewed for code necessity and duplication
- [ ] Docstrings added to public functions/classes
- [ ] No modifications to core functional logic
- [ ] Changes committed locally (not pushed)

## Completion Response

Return structured feedback to PAW agent:

**PASS**: Implementation meets quality criteria, ready for PR
- Confirm tests pass
- Note any non-blocking observations
- Commits made (documentation, polish)

**BLOCKED**: Implementation needs rework
- List blocking issues with file:line references
- Specify what Implementer needs to change
- Note **incomplete plan items** (e.g., "Phase 2 says update services A, B, C but only A implemented")
- Note test failures or missing tests
- Note violations of project instruction conventions

> **After returning PASS**: The PAW orchestrator will handle push/PR creation (via `paw-git-operations`) and then invoke `paw-transition`. This skill does NOT push or create PRs—it only returns the verdict.

### Response Format

```
## Review Result: [PASS|BLOCKED]

### Summary
[One-sentence summary of review outcome]

### Tests
- Status: [PASS|FAIL]
- [Details if relevant]

### Commits Made
- [List of commits added during review, if any]

### Issues Found
[For BLOCKED only: specific issues requiring Implementer attention]

### Notes for Reviewer
[Optional: items flagged for human PR reviewer attention]
```