| name | review-rubric |
| description | Standard code review checklist for all reviewers. Load this when performing a peer review at Stage 5. Covers spec compliance, correctness, security, test coverage, readability, and performance. Defines what constitutes APPROVED vs CHANGES_REQUESTED and how to write review findings. |
Review Rubric
Use this skill when performing a code review. It defines the standard
review checklist every reviewer must work through.
Mandatory Checks (every review)
1. Spec Compliance
- Does the code match
pipeline/design-spec.md?
- Are all API contracts implemented as specified?
- Are there undocumented deviations? (BLOCKER if unexplained)
2. Correctness
- Are all edge cases from the spec handled?
- Are error paths handled and tested?
- Are there off-by-one errors, null dereferences, or type mismatches?
3. Security
- No secrets in code
- Input validation present
- Auth checks in place where spec requires them
- No SQL string concatenation
4. Test Coverage
- Does new behaviour have corresponding tests?
- Do tests actually test the behaviour (not just call the function)?
5. Readability
- Can a new team member understand this code in 5 minutes?
- Are complex sections commented?
Comment Classification
Every review comment must be one of:
BLOCKER — Must be fixed before this PR can merge.
Use for: security issues, missing tests, spec violations, broken behaviour.
SUGGESTION — Would improve the code. Author's call.
Use for: style preferences, minor optimisations, naming improvements.
QUESTION — Need clarification. Not blocking unless answered badly.
Use for: unclear intent, possible oversight, design question.
Verdict
REVIEW: APPROVED — No blockers. May have suggestions.
REVIEW: CHANGES REQUESTED — One or more blockers. List them.
Scope
This rubric is for pipeline Stage 5 peer reviews — when dev agents review
each other's PRs during a /pipeline run. For standalone pre-merge review
of changes made outside the pipeline (direct edits, implement skill work,
/hotfix runs), use the pre-pr-review skill instead.
Gotchas (add failures here over time)
- Do not approve a PR just because it "looks fine". Work through the checklist.
- Do not raise the same point another reviewer already raised (read their review first).
- Do not BLOCKER a style preference. That's what linters are for.
Platform Reviewer Focus (Stage 5, when reviewing as dev-platform)
Apply the coding-principles rubric explicitly — BLOCKER for unstated
assumptions (§1), overcomplication (§2), drive-by edits (§3), or a
missing/weak Plan with unverifiable steps (§4).
Focus on: infrastructure impact, deploy risk, CI coverage, observability
(metrics, logs, traces named in the design-spec).
Classify as BLOCKER / SUGGESTION / QUESTION inside each section.
Use PATTERN: to call out something done especially well.
QA Reviewer Focus (Stage 5, when reviewing as dev-qa)
Apply the coding-principles rubric. BLOCKER on unstated assumptions (§1),
overcomplication (§2), drive-by edits (§3), or missing/weak plan (§4).
Focus on: testability. Does the change actually admit tests for the
acceptance criteria? Are state transitions observable? Is the tested surface
stable? Flag hidden coupling (singletons, global clocks, module-level state)
as a BLOCKER — it obstructs tests.
Use PATTERN: to call out testing patterns the team should adopt as default.
