Code Quality Review

Code Quality Review

Perform an evidence-based review across three dimensions: production code, comments and documentation, and tests. Prioritize defects and maintainability risks that can change behavior or make future changes unsafe. Do not turn personal style preferences into findings.

Establish the Review Contract

1. Determine the target from the user's request: explicit files, working-tree changes, a commit range, a branch, a pull request, or the whole repository.
2. If the target is ambiguous, choose the narrowest reasonable scope available from the current context and state the assumption. Ask only when different scopes would materially change the result.
3. Read repository instructions and the testing guide before judging the change. Treat local conventions and documented requirements as authoritative.
4. Infer the intended behavior from the issue, request, surrounding code, public API, and tests. Separate confirmed requirements from assumptions.
5. Keep review and implementation separate. Inspect and report by default; modify files only when the user explicitly requests fixes.

For a diff review, inspect both the diff and enough surrounding code to understand callers, invariants, error paths, and existing tests. Do not review changed lines in isolation.

Build an Evidence Base

• Enumerate changed files and classify production code, tests, generated files, configuration, and documentation.
• Search for call sites, analogous implementations, shared helpers, and tests that exercise the affected behavior.
• Run the smallest relevant formatter, compiler, linter, and test commands that the repository supports. Expand only when risk or failures justify it.
• Treat command success as supporting evidence, not proof of quality. Review behavior and assertions directly.
• Distinguish introduced problems from pre-existing problems. Report pre-existing issues only when the change makes them newly reachable, more severe, or directly relevant to the request.
• Record uncertainty. Do not present an unverified suspicion as a finding.

Review Code Quality

Check correctness before aesthetics.

Behavior and safety

• Trace normal, boundary, error, cancellation, retry, and cleanup paths.
• Check validation at trust boundaries and assumptions about ordering, uniqueness, nullability, encoding, time, concurrency, and platform behavior.
• Check whether errors are preserved, classified, handled at the right layer, and observable without exposing sensitive data.
• Check resource ownership, lifecycle, atomicity, idempotency, and partial-failure behavior where relevant.
• Check security and privacy implications when input, permissions, secrets, serialization, filesystem access, or external commands are involved.

Design and maintainability

• Prefer the simplest design that preserves required behavior.
• Flag duplication only when it creates a realistic divergence or maintenance risk; do not demand abstraction for superficial similarity.
• Check names, module boundaries, dependency direction, cohesion, coupling, and API contracts.
• Identify hidden side effects, surprising control flow, boolean blindness, temporal coupling, and invalid states that the design permits unnecessarily.
• Check consistency with established repository patterns unless those patterns conflict with an explicit requirement.
• Treat formatter and linter output as authoritative for mechanical style. Avoid repeating automated diagnostics unless they explain a larger problem.

Review Comment and Documentation Quality

Evaluate comments for truthfulness and decision value, not quantity.

• Verify that comments and API documentation match current behavior, parameters, errors, side effects, units, ownership, and concurrency guarantees.
• Prefer explanations of intent, constraints, invariants, tradeoffs, or non-obvious reasons. Flag comments that merely narrate clear syntax when they add noise or can become stale.
• Require documentation for public contracts and safety-critical or surprising behavior when repository conventions call for it.
• Check that workarounds explain the external constraint and, when appropriate, include a traceable issue or removal condition.
• Check TODO, FIXME, and SAFETY comments for specificity, ownership context, and validity.
• Flag commented-out code and obsolete explanations when version control is the better record.
• Do not demand comments to compensate for confusing code when a small code improvement would express the idea more reliably.

Review Test Quality

Map tests to behavioral risks before considering line coverage.