| name | perfect-code-review |
|---|---|
| description | Structured code review using the PERFECT framework (Purpose, Edge Cases, Reliability, Form, Evidence, Clarity, Taste). Use this skill whenever the user asks to review code, a PR, a diff, a merge request, or says things like "review this", "check my code", "code review", "what do you think of this implementation", "can you CR this", "look over my changes", or pastes code and asks for feedback. Also trigger when the user asks to set up a code review process, review conventions, or a review checklist. |
A structured code review framework based on the PERFECT principles by Daniil Bastrich. Each letter maps to a review dimension, ordered by descending importance. Work through them top-to-bottom; never let lower-priority concerns block a merge when higher-priority ones are satisfied.
- "I don't like it" ≠ "it's wrong." Every comment must state what is wrong, why, and propose an alternative (at least directionally).
- Avoid vague feedback like "this is bad" or "can you clean this up." Be specific and constructive.
- Respect author autonomy on subjective matters unless there's an explicit team convention.
Work through each level sequentially. If a level has blocking issues, flag them before moving to the next — but still complete the full review so the author gets all feedback in one pass.
Does the code solve the stated task?
- Identify the task: read the ticket/PR description, commit messages, or ask the author.
- Mentally outline how you'd approach the solution; compare with the implementation.
- Verify the code actually produces the expected result — not just that it looks reasonable.
- If the code doesn't solve the task, nothing else matters. Flag this as a blocking issue.
Are corner cases handled?
- Look for: unexpected/missing inputs, boundary values, nullability, type limits, empty collections, off-by-one errors, concurrency edge cases.
- Check "impossible cases" — states that seem unreachable now but become reachable after future changes (e.g., unwrapping optionals without guards).
- Consider both business edge cases (rare but valid scenarios) and technical edge cases (overflow, encoding, timezone, locale).
- Tip: For the specific project, maintain a living checklist of common edge cases the team has hit before.
Is the code free from performance and security issues?
- Performance: Check time/memory complexity relative to expected data volume. Look for N+1 queries, unbounded loops, missing pagination, unnecessary allocations, cache invalidation bugs.
- Security: Input/output validation, credential handling, injection vectors (SQL, XSS, command), broken auth on public interfaces, secret leakage in logs.
- Integration: Broken or fragile external dependencies, missing timeouts, missing retries, unhandled error responses.
- Not every PR needs deep perf/security review, but always do a basic sanity pass.
Does the code align with design principles?
- Evaluate cohesion and coupling. Most design discussions reduce to High Cohesion / Low Coupling.
- Check adherence to project-specific conventions and patterns (SOLID, KISS, DRY as they apply).
- This area is more subjective — require clear arguments for change requests, not just "I'd do it differently."
- Look for: god classes/functions, leaky abstractions, inappropriate coupling between modules, duplicated logic that should be shared (or shared logic that should be duplicated for independence).
Do tests and CI pass?
- All existing tests and CI pipelines must pass. If they don't, flag as blocking — fixing broken pipelines can introduce changes that invalidate the rest of the review.
- If team conventions require tests, verify new/changed behavior is covered.
- Review tests with the same rigor as production code: do they test the right thing, are they readable, are they performant?
- Production code should not contain special logic that exists only to support tests. If it does, suggest a design change.
- If tests/pipelines are permanently broken or ignored, recommend removing them — dead tests cause more confusion than benefit.
Does the code clearly communicate its intent?
- Can the code be read "diagonally" without tracing every line?
- Check naming (variables, functions, files), structure, nesting depth, statement order, block length.
- Verify documentation and comments where conventions require them (or where intent is non-obvious).
- If there are no agreed-upon objective criteria and the code isn't explicitly unclear, defer to the author's style.
Personal preferences — note but don't block.
- If a comment is purely stylistic preference without a supporting argument or convention, mark it explicitly as non-blocking (e.g., "nit:", "taste:", "optional:").
- Valuable opinions that recur across reviews should be proposed as team conventions — once agreed, they graduate from "taste" to "form" or "clarity."
- Never block a PR on taste alone.
When reviewing code, structure your response using the PERFECT levels as sections. For each level:
- State whether it passes, has suggestions (non-blocking), or has issues (blocking).
- Be specific: reference line numbers, function names, or code snippets.
- Propose alternatives for any issue raised.
Use this output template:
## Review Summary
| Level | Status | Notes |
|-------|--------|-------|
| Purpose | ✅ Pass | ... |
| Edge Cases | ⚠️ Suggestions | ... |
| Reliability | ❌ Issue | ... |
| Form | ✅ Pass | ... |
| Evidence | ✅ Pass | ... |
| Clarity | ⚠️ Suggestions | ... |
| Taste | 💭 Nits | ... |
## Detailed Findings
### Purpose
...
### Edge Cases
...
(continue for each level with findings)
- ❌ Blocking — Must be fixed before merge. Use for Purpose, Edge Cases, Reliability, Evidence issues.
⚠️ Suggestion — Should be addressed but not a merge blocker. Use for Form, Clarity improvements.- 💭 Nit — Take it or leave it. Use for Taste.
If the user asks to set up a self-review process or review their own code, remind them:
- Use the same PERFECT checklist before requesting peer review.
- Self-review catches the majority of trivial issues and saves everyone's time.
- Walk through each level systematically — don't just re-read the diff casually.
If the user asks to set up review conventions for a team, guide them through:
- Adopt the PERFECT hierarchy as the review framework.
- For each level, define project-specific criteria (e.g., "all public endpoints must validate input" under Reliability).
- Write it down — unwritten conventions become opinion wars.
- Keep conventions actionable and up to date. If a pattern keeps recurring in reviews (comment → discussion → fix), codify it.
- Define process rules: who reviews, how many reviewers, SLA for review turnaround, how to write/resolve comments, approval criteria.
- Ban drive-by "LGTM" approvals — they signal the reviewer didn't engage.
- Automate what you can (linting, formatting, type checks, security scans) so human review focuses on what humans are good at.