review-feedback-quality

Review Feedback Quality — Writing Comments That Get Results

Overview

The quality of a code review is measured by how many issues get fixed with minimum friction. Vague comments are 3x more likely to be ignored or result in incorrect fixes than comments that state what is wrong, why it matters, and how to fix it.

Use this alongside review-cheat-sheet (what to look for) and review-code-quality-process (how to run a review end to end).

---

Comment Templates by Severity

Every comment should answer three questions: What is the problem, Why it matters, and How to fix it. Depth scales with severity.

CRITICAL — Must Fix Before Merge

[CRITICAL] What: <one-line description of the issue>
Why: <the risk or harm — data loss, security breach, crash in production>
Fix: <specific action — code snippet, API to call, pattern to use>
Ref: <link to CVE, docs, or deeper skill if applicable>

Example:

[CRITICAL] What: User-supplied filename passed directly to os.path.join without sanitization.
Why: Path traversal attack — a filename like "../../etc/passwd" lets an attacker read arbitrary files
on the server.
Fix: Validate that the resolved path starts with the expected base directory:
  base = "/var/uploads"
  safe = os.path.realpath(os.path.join(base, user_filename))
  if not safe.startswith(base):
      raise ValueError("Invalid path")
Ref: skills/security-patterns-code-review

HIGH — Strong Confidence the Change Is Wrong

[HIGH] What: <issue description>
Why: <likely defect or maintenance burden this introduces>
Fix: <concrete suggestion>

Example:

[HIGH] What: getUser() is called inside the loop at line 47.
Why: This fires one DB query per iteration. With 500 users in the list, this is 500 round trips
where a single query with IN (...) would suffice.
Fix: Extract the IDs before the loop, call getUsersByIds(ids), then look up results in a map.

MEDIUM — Likely to Cause a Defect or Maintenance Burden

[MEDIUM] What: <issue>
Why: <why this will hurt later>
Fix: <suggested improvement>