Code Review

Rigorous code review focused on quality, maintainability, and architectural soundness.

When to Use

• After implementing a feature or fix

• Before committing changes

• When explicitly asked to review code

• Before creating a PR

Input

Default: Use current staged/committed changes.

If argument provided:

• GitHub issue number/URL: Fetch context with scripts/gh_issue_phase.sh get-issue $ARG to understand the original task requirements and decisions from prior phases.

Method

Start by inspecting the changes. Use the deterministic script to collect the review context:

scripts/collect_review_context.sh

If on the main branch, review the staged git diff. If on a different branch, review committed and uncommitted changes compared to main.

Dispatch two subagents to carefully review the code changes. Tell them they're competing with another agent - whoever finds more legitimate issues wins honour and glory. Make sure they examine both architecture AND implementation, and check every criterion below.

Review Criteria

1. Code Quality

Check Look For

DRY Duplicated logic, copy-pasted code, repeated patterns that should be abstracted

Code Bloat Unnecessary code, over-engineering, premature abstractions, dead code

Bugs Logic errors, edge cases, off-by-one errors, null/undefined handling

2. Code Slop & Technical Debt

Symptom Description

Magic values Hardcoded strings/numbers without constants

Inconsistent naming Mixed conventions, unclear names

Missing error handling Unhandled exceptions, silent failures

TODO/FIXME comments Deferred work that should be tracked

Commented-out code Delete it or explain why it exists

Dependency bloat New deps when stdlib/existing deps suffice

3. Architecture (in context of broader system)

Principle Review Questions

Modularity Are changes properly bounded? Do they respect module boundaries?

Cohesion Does each unit have a single, clear responsibility?

Separation of Concerns Is business logic mixed with presentation/data access?

Information Hiding Are implementation details properly encapsulated?

Coupling Does this create tight coupling? Are dependencies appropriate?

4. Devil's Advocate

Challenge the implementation:

• Is this the simplest solution? Could it be simpler?

• What happens under load/scale?

• What are the failure modes?

• What assumptions might be wrong?

• Is there a more fundamentally correct approach, even if harder?

5. Test Effectiveness

Check Criteria

Coverage Are the important paths tested?

Meaningful assertions Do tests verify behavior, not implementation?

Edge cases Are boundaries and error conditions tested?

Readability Can you understand what's tested from test names?

Fragility Will tests break on valid refactors?

Output Format

Report findings organized by severity:

Code Review Findings

Critical (must fix)

• [Issue]: [Location] - [Why it matters]

Important (should fix)

• [Issue]: [Location] - [Recommendation]

Minor (consider fixing)

• [Issue]: [Location] - [Suggestion]

Positive Observations

• [What was done well]

GitHub Issue Tracking

If a GitHub issue was provided or is available from prior phases:

Post review findings as a phase comment and set the label:

echo "$REVIEW_SUMMARY" | scripts/gh_issue_phase.sh post-phase $ISSUE review scripts/gh_issue_phase.sh set-label $ISSUE phase:review

Pass the issue number to the next skill (e.g., /commit #42 ).

Common Mistakes

Mistake Correction

Surface-level review Dig into logic, trace data flow

Ignoring context Review changes in relation to the system

Only finding negatives Note what's done well

Vague feedback Be specific: file, line, concrete suggestion

Bikeshedding Focus on impact, not style preferences

Red Flags - STOP and Investigate

• New dependencies added without clear justification

• Changes that bypass existing patterns without explanation

• Test coverage decreased

• Complex logic without tests

• Security-sensitive code modified