Reviewing Code

Structured review methodology producing consistent, severity-based, actionable feedback. This skill defines the review process — how to conduct a review, format findings, and decide; the what to look for lives in perspective reference files the orchestrator loads selectively. Non-code artefacts use assessing-quality (rageatc-core-oss); architecture design review is the critic's job at architecture approval.

Perspective Model

Perspectives are reference files under references/ defining what to examine. The orchestrator tells the reviewer-agent which to load.

Tier defaults (orchestrator may override — e.g., security for a Quick auth fix):

Inputs Required

• Enriched roadmap chunk — acceptance criteria, file set, purpose (the enriched roadmap IS the brief)
• ARCHITECTURE.md — patterns, conventions, component boundaries
• Codebase access and changed files list
• Test results — or the ability to run the suite
• Perspective references — loaded by orchestrator
• For re-reviews: previous review and the developer's changes

Output

# Code Review: [chunk name/ID]

**Brief:** [reference to development brief or roadmap chunk]
**Perspectives:** [which perspectives were applied]
**Decision:** Accept / Revise
**Findings:** [count by severity]

## Findings

[Findings grouped by severity: critical first, then major, minor, notes]

## What Works Well

[Brief acknowledgement of what meets standards]

## Decision

[Accept or Revise with rationale]

## Observations

[Anything relevant to the broader project but outside this chunk's scope]

Review Process

1. Understand intent before reading code. Read the enriched roadmap chunk completely: acceptance criteria (your primary checklist), which files were supposed to change, what the chunk contributes to the system. Do not open code files first.
2. Run tests. Record pass/fail as factual input — no conclusions yet.
3. Review against loaded perspectives, spec-compliance first. Not every concern applies to every chunk — spend time proportional to risk. Traps: reviewing beyond scope (review this chunk; broader issues go in Observations) and style preferences over standards (review against ARCHITECTURE.md patterns, not taste).
4. Structure findings per the Finding Format, with honest severity. Trap: nitpicking on accept — if heading towards acceptance, limit minor findings to the 3-5 most valuable.
5. Step back. Does this code, as a whole, solve the problem the brief describes? A review that catches every style issue but misses that the core logic is wrong has failed.
6. Decide per the criteria below, stating the rationale.

Re-review Protocol

1. Read your previous review first
2. Verify each previously raised finding is addressed
3. Do not re-raise findings that were adequately resolved
4. Check that fixes have not introduced new issues
5. Carry forward unresolved minor findings only if still relevant
6. Apply the same decision criteria — a re-review is not a lower bar

Finding Format

**[SEVERITY]** file:line (or component)

_Finding:_ What the issue is.

_Impact:_ Why it matters.

_Suggestion:_ How to fix it.

Example:

**[MAJOR]** src/validators/email-validator.ts:1-45

_Finding:_ Email validation uses an abstract factory pattern with three
interfaces and a plugin system. The actual validation is 4 lines of code
wrapped in 40 lines of abstraction.

_Impact:_ Maintenance burden disproportionate to the problem. Future
developers must understand the factory pattern to modify a simple check.
Violates simplicity principle.

_Suggestion:_ Replace with a direct validation function. If additional
validators are needed later, extract a pattern then — not before.

Severity Levels

Severity honesty: do not inflate. A naming improvement is not critical; inflation erodes trust and wastes developer cycles. Security findings are never below Major — exploitable vulnerabilities are Critical — even when spec-compliance passes (calibration table in references/security.md).

Decision Criteria

Accept — all of: zero critical findings, zero major findings, all acceptance criteria satisfied, tests pass. Minor findings and notes are included but do not block.

Revise — any of: a critical or major finding, an unsatisfied acceptance criterion, failing tests (unless explained by an intentional, brief-sanctioned change). List the specific issues requiring revision.