bdd173b5159f30b340373a0f3ef62d955092a78f

Message

Update code review skill

Diff

This diff is truncated to protect this page.

 1diff --git a/dot_agents/skills/code-review/SKILL.md b/dot_agents/skills/code-review/SKILL.md
 2index e390c4e064df792d82321e7dc831e33a4ea1137c..a26a51942b46c9407baf568979a6010d04fdc4be 100644
 3--- a/dot_agents/skills/code-review/SKILL.md
 4+++ b/dot_agents/skills/code-review/SKILL.md
 5@@ -20,7 +20,7 @@ Seven principles, priority order. Higher-priority violations are more likely blo
 6 3. **Simplicity** -- Simplest working solution? Anything removable?
 7 4. **Types & naming** -- Names reveal intent? Types prevent invalid states?
 8 5. **Semantics** -- Logic correct? Edge cases handled?
 9-6. **Proof** -- Changes tested? Conditional paths covered?
10+6. **Proof** -- Changes tested?
11 7. **PR hygiene** -- Diff minimal? Commits logical? Description adequate?
12 
13 ## Coding Principles
14@@ -70,7 +70,6 @@ Never silently discard errors. `_ = doSomething()` is almost always wrong.
15 
16 ### Tests
17 
18-- Every conditional path introduced by the change needs a test.
19 - Test behavior, not implementation. Tests that break on refactoring are fragile.
20 - Test names read as behavior specs: "returns error when input is empty".
21 
22@@ -81,15 +80,36 @@ Never silently discard errors. `_ = doSomething()` is almost always wrong.
23 - **nit** -- Optional. Style, naming, minor simplifications. Always prefix with `nit:`.
24 - **question** -- Genuine uncertainty needing author clarification.
25 
26+A blocker requires a concrete bad outcome you can name: wrong data, a crash, a broken contract, a security hole. The following are NOT blockers, however much analysis went into finding them:
27+
28+- The PR description doesn't match what the code does. That is a `suggestion` to fix the description.
29+- Existing behavior is preserved where the author expected change, or vice versa, with no harmful result. That is a `question` asking which they want.
30+- A deliberate mechanism in untouched code behaves in a way the author may not have anticipated. That is a `question`. Machinery someone built on purpose is not a defect because this PR interacts with it.
31+- It took you significant work to uncover. Effort spent is not severity.
32+
33+If your own analysis concludes the data and runtime outcome is fine, you do not have a blocker. Notice when you are about to contradict yourself.
34+
35 ## Verdicts
36 
37 - **approved** -- No blockers, no suggestions. Rare and earned.
38 - **approved, with suggestions** -- No blockers. Author can merge and address suggestions later.
39 - **changes requested** -- One or more blockers. Must fix before merge.
40