Code Review
Principles for high-signal code review. For tool-specific mechanics (diff-size routing, suggestion-block format, GitHub workflow), see the reviewer agent.
Four-Axis Review Lens
Every review covers four axes:
- Correctness — off-by-ones, null paths, concurrency races, lost errors, unhandled edge cases.
- Security — injection, auth/authz gaps, secrets in code, unsafe deserialization, SSRF, missing input validation at trust boundaries.
- Design integrity — SOLID violations, leaky abstractions, tight coupling, circular deps, domain logic bleeding into infrastructure.
For complexity / duplication / length, prefer Quality-stage output over subjective comments — see
workflow-development.
- Tests — missing coverage on new branches, brittle tests, tests that mirror implementation rather than behavior.
What's Not a Finding
Do not surface these:
- Formatting, import order, quote style — owned by the linter, not the reviewer.
- Stylistic preferences with no behavioral impact.
- Speculative "could be" comments without a concrete failure mode.
These erode review signal. If your only comment is a style preference, stay silent.
Confidence-Based Filtering
Before surfacing a finding, apply this filter:
- Name the failure scenario. What breaks, under what inputs, in what deployment context? If you cannot articulate it, the finding is speculative — drop it.
- One strong comment over five weak ones. Ten medium-confidence findings force the author to triage; two high-confidence findings close the loop.
- Missing tests are a finding, not an afterthought. Untested new branches are incomplete work.
Confidence floor by severity:
| Severity |
Minimum confidence |
Action if below floor |
| Critical |
0.9 |
Must name an exploit or data-loss path |
| High |
0.75 |
Must name a realistic failure scenario |
| Medium |
0.5 |
Can surface; mark as "likely" |
| Low / Nit |
Any |
Drop unless the fix is a one-liner |
Tone
Review the code, not the author.
- Observation language: "This path returns nil without checking the error" — not "You forgot to check the error."
- Acknowledge good work briefly. Silence is not approval; a short note reduces defensive reading.
- Frame as options for Medium/Low. "One option: …" not "You should …".
- Reserve mandates for Critical/High. Direct language is appropriate when the stakes are real.
Red Flags
| Pattern |
What it signals |
| Finding with no named failure scenario |
Speculation — refine or drop |
| Five+ findings, none above Medium |
Signal-to-noise failure — prioritize |
| Only linter-owned comments (style, formatting) |
Scope creep — let the linter own these |
| Zero positive observations in a long review |
Adversarial culture risk |
| Nitpick marked Critical |
Severity inflation — recalibrate |
When Code Review Hurts
- Throwaway prototypes — review cost exceeds value if you're about to discard the code.
- Hotfix rollbacks — reverting to a known-good state introduces no new logic to review.
- Trivially covered one-liners — if the tests already encode the contract, a comment adds ceremony without safety.
Source: lugassawan/swe-workbench — distributed by TomeVault.
1---2name: principle-code-review3description: Code review heuristics — four-axis review lens (correctness, security, design integrity, test coverage); confidence-based filtering (no finding without a concrete failure scenario); review comment tone (observation over accusation); nitpick filtering; what counts as a real finding vs linter noise. Auto-load when writing or framing a review comment, deciding whether a PR finding is worth surfacing, reviewing a diff for correctness, or filtering review nitpicks. Use when this capability is needed.4---56# Code Review78Principles for high-signal code review. For tool-specific mechanics (diff-size routing, suggestion-block format, GitHub workflow), see the `reviewer` agent.910## Four-Axis Review Lens1112Every review covers four axes:1314- **Correctness** — off-by-ones, null paths, concurrency races, lost errors, unhandled edge cases.15- **Security** — injection, auth/authz gaps, secrets in code, unsafe deserialization, SSRF, missing input validation at trust boundaries.16- **Design integrity** — SOLID violations, leaky abstractions, tight coupling, circular deps, domain logic bleeding into infrastructure.17 *For complexity / duplication / length, prefer Quality-stage output over subjective comments — see `workflow-development`.*18- **Tests** — missing coverage on new branches, brittle tests, tests that mirror implementation rather than behavior.1920## What's Not a Finding2122Do not surface these:2324- Formatting, import order, quote style — owned by the linter, not the reviewer.25- Stylistic preferences with no behavioral impact.26- Speculative "could be" comments without a concrete failure mode.2728These erode review signal. If your only comment is a style preference, stay silent.2930## Confidence-Based Filtering3132Before surfacing a finding, apply this filter:33341. **Name the failure scenario.** What breaks, under what inputs, in what deployment context? If you cannot articulate it, the finding is speculative — drop it.352. **One strong comment over five weak ones.** Ten medium-confidence findings force the author to triage; two high-confidence findings close the loop.363. **Missing tests are a finding, not an afterthought.** Untested new branches are incomplete work.3738Confidence floor by severity:3940| Severity | Minimum confidence | Action if below floor |41|---|---|---|42| Critical | 0.9 | Must name an exploit or data-loss path |43| High | 0.75 | Must name a realistic failure scenario |44| Medium | 0.5 | Can surface; mark as "likely" |45| Low / Nit | Any | Drop unless the fix is a one-liner |4647## Tone4849Review the code, not the author.5051- **Observation language:** "This path returns nil without checking the error" — not "You forgot to check the error."52- **Acknowledge good work briefly.** Silence is not approval; a short note reduces defensive reading.53- **Frame as options for Medium/Low.** "One option: …" not "You should …".54- **Reserve mandates for Critical/High.** Direct language is appropriate when the stakes are real.5556## Red Flags5758| Pattern | What it signals |59|---|---|60| Finding with no named failure scenario | Speculation — refine or drop |61| Five+ findings, none above Medium | Signal-to-noise failure — prioritize |62| Only linter-owned comments (style, formatting) | Scope creep — let the linter own these |63| Zero positive observations in a long review | Adversarial culture risk |64| Nitpick marked Critical | Severity inflation — recalibrate |6566## When Code Review Hurts6768- **Throwaway prototypes** — review cost exceeds value if you're about to discard the code.69- **Hotfix rollbacks** — reverting to a known-good state introduces no new logic to review.70- **Trivially covered one-liners** — if the tests already encode the contract, a comment adds ceremony without safety.7172---73> Source: [lugassawan/swe-workbench](https://github.com/lugassawan/swe-workbench) — distributed by [TomeVault](https://tomevault.io).74<!-- tomevault:4.0:skill_md:2026-05-22 -->
tomevault-io