code-review-checklist

Code Review Standards

---

Mandatory Pre-Flight Context Inspection

Before performing code reviews or writing PR comments, you MUST inspect:

1. Label Convention Standardization (Section 27) → Categorize all feedback into BLOCKER:, CONCERN:, SUGGESTION:, or NOTE:
2. Context Window Discipline (Section 120) → Quote concise 1-3 line snippets with line numbers; ban pasting massive blocks of unchanged code
3. Anti-Nitpicking Rule (Section 130) → Delegate syntax/formatting formatting checks to linters (eslint/Prettier); focus reviews on logic and security

Code Review Standards

---

Review Mindset

Reviews are collaborative. The goal is better code — not proof that the reviewer is smarter.

Before commenting:

• Understand what the code is trying to do before judging how it does it
• Distinguish between personal preference and objective problems
• Label your findings so the author understands the expected action

Comment label convention:

• BLOCKER: — must be fixed before merge (bug, security issue, broken behavior)
• CONCERN: — likely problem that needs discussion before proceeding
• SUGGESTION: — would improve the code but is not required
• NOTE: — observation or question, no action needed

---

What to Check

Correctness

• Does the code do what it claims to do?
• Are edge cases handled? (empty input, null, max value, concurrent execution)
• Does error handling cover realistic failure modes?
• Are there off-by-one errors? Integer overflow risks?

Security

• Is user input validated before it's used?
• Are SQL queries parameterized — never string-concatenated?
• Are secrets in environment variables — not in code?
• Are auth checks happening before business logic executes?
• Is the OWASP API Top 10 considered for any API routes?

Readability

• Can you understand the intent in under 30 seconds per function?
• Are names self-documenting at the right level of abstraction?
• Are complex sections commented with why, not what?
• Is nesting kept to a manageable depth (≤3 levels)?

Design

• Is this code easy to change? Or would changing one thing break five others?
• Are there clear boundaries between concerns?
• Is logic duplicated anywhere that should be shared?
• Is the new code consistent with how the rest of the codebase does similar things?

Tests

• Are tests testing behavior or implementation details?
• Do tests cover the happy path, edge cases, and known failure modes?
• Do test names describe the expected behavior in plain language?
• Would these tests catch a regression if someone broke this code?

Performance

• Are there database queries inside loops?
• Are large datasets loaded into memory when they could be streamed?
• Are expensive operations (network, file I/O) done unnecessarily?

---

Review Process

1. Read the PR description first — understand intent before reading code
2. Read tests first — they tell you what the code is supposed to do
3. Read the implementation — verify it matches what the tests describe
4. Run it locally for significant changes — static reading misses runtime behavior

---

Giving Feedback

Effective feedback is:

• Specific — references the exact line and the exact concern
• Actionable — tells the author what to change, not just that something is wrong
• Explanatory — gives the reasoning, not just the verdict

# ❌ Unhelpful
This function is too long.

# ✅ Helpful
SUGGESTION: This function handles both data fetching and data transformation.
Splitting into `fetchUserData()` and `transformUserData()` would make each
half easier to test independently and reuse elsewhere.

---

Receiving Feedback

• "We disagree" is not the same as "they're wrong"
• If a comment is unclear, ask for clarification before defending
• BLOCKER and CONCERN comments need resolution, not just a response
• SUGGESTION and NOTE are optional — you can explain why you're not acting on them

---

🛑 Context Window Discipline

When an AI acts as a reviewer, context bloat ruins reasoning:

1. Never quote massive blocks of code back to the user. Use line numbers or tiny 1-3 line snippets.
2. Never attach the entire project context to a single file review.
3. Keep reviews scoped. Do not suggest a full architecture rewrite if the PR is fixing a typo in a CSS class.

---

🤖 LLM-Specific Review Traps

AI reviewers frequently fail by focusing on the wrong things. Avoid these strict anti-patterns:

1. Syntax Nitpicking: Commenting on formatting, semicolons, or line length. Let eslint or Prettier handle this. Only comment if logic is affected.
2. "Clean Code" Hallucinations: Telling the author to extract a perfectly readable 10-line function into 3 separate abstract classes.
3. Invented Methods: Suggesting the author use .toSortedMap() when that method literally does not exist in the language or framework used.
4. False Bottlenecks: Claiming an O(n^2) loop is a performance critical error when n is a configuration array guaranteed to be < 10 items.
5. The Compliment Sandwich: You do not need to soften every critique with "Great job on the rest of the code!" Be direct, professional, and concise.

---

Output Format

When this skill completes a task, structure your output as:

━━━ Code Review Checklist Output ━━━━━━━━━━━━━━━━━━━━━━━━
Task:        [what was performed]
Result:      [outcome summary — one line]
─────────────────────────────────────────────────
Checks:      ✅ [N passed] · ⚠️  [N warnings] · ❌ [N blocked]
VBC status:  PENDING → VERIFIED
Evidence:    [link to terminal output, test result, or file diff]

---

---

AI coding assistants often fall into specific bad habits when dealing with this domain. These are strictly forbidden:

1. Over-engineering: Proposing complex abstractions or distributed systems when a simpler approach suffices.
2. Hallucinated Libraries/Methods: Using non-existent methods or packages. Always // VERIFY or check package.json / requirements.txt.
3. Skipping Edge Cases: Writing the "happy path" and ignoring error handling, timeouts, or data validation.
4. Context Amnesia: Forgetting the user's constraints and offering generic advice instead of tailored solutions.
5. Silent Degradation: Catching and suppressing errors without logging or re-raising.

---

Slash command: /review or /tribunal-full Active reviewers: logic-reviewer · security-auditor

❌ Forbidden AI Tropes

1. Blind Assumptions: Never make an assumption without documenting it clearly with // VERIFY: [reason].
2. Silent Degradation: Catching and suppressing errors without logging or handling.
3. Context Amnesia: Forgetting the user's constraints and offering generic advice instead of tailored solutions.

Review these questions before confirming output:

✅ Did I rely ONLY on real, verified tools and methods?
✅ Is this solution appropriately scoped to the user's constraints?
✅ Did I handle potential failure modes and edge cases?
✅ Have I avoided generic boilerplate that doesn't add value?

🛑 Verification-Before-Completion (VBC) Protocol

CRITICAL: You must follow a strict "evidence-based closeout" state machine.

• ❌ Forbidden: Declaring a task complete because the output "looks correct."
• ✅ Required: You are explicitly forbidden from finalizing any task without providing concrete evidence (terminal output, passing tests, compile success, or equivalent proof) that your output works as intended.

Pre-Flight Checklist

• Have I reviewed the user's specific constraints and requests?
• Have I checked the environment for relevant existing implementations?

VBC Protocol (Verification-Before-Completion)

You MUST verify existing code signatures and variables before attempting to modify or call them. No hallucination is permitted.

---

🤖 LLM-Specific Traps

AI coding assistants often fall into specific bad habits when dealing with this domain. These are strictly forbidden:

1. Over-engineering: Proposing complex abstractions or distributed systems when a simpler approach suffices.
2. Hallucinated Libraries/Methods: Using non-existent methods or packages. Always // VERIFY or check package.json / requirements.txt.
3. Skipping Edge Cases: Writing the "happy path" and ignoring error handling, timeouts, or data validation.
4. Context Amnesia: Forgetting the user's constraints and offering generic advice instead of tailored solutions.
5. Silent Degradation: Catching and suppressing errors without logging or re-raising.

---

🏛️ Tribunal Integration (Anti-Hallucination)

Slash command: /review or /tribunal-full Active reviewers: logic-reviewer · security-auditor

❌ Forbidden AI Tropes

1. Blind Assumptions: Never make an assumption without documenting it clearly with // VERIFY: [reason].
2. Silent Degradation: Catching and suppressing errors without logging or handling.
3. Context Amnesia: Forgetting the user's constraints and offering generic advice instead of tailored solutions.

✅ Pre-Flight Self-Audit

Review these questions before confirming output:

✅ Did I rely ONLY on real, verified tools and methods?
✅ Is this solution appropriately scoped to the user's constraints?
✅ Did I handle potential failure modes and edge cases?
✅ Have I avoided generic boilerplate that doesn't add value?

🛑 Verification-Before-Completion (VBC) Protocol

CRITICAL: You must follow a strict "evidence-based closeout" state machine.

• ❌ Forbidden: Declaring a task complete because the output "looks correct."
• ✅ Required: You are explicitly forbidden from finalizing any task without providing concrete evidence (terminal output, passing tests, compile success, or equivalent proof) that your output works as intended.