Просмотр исходного кода

Reviewer judges the spec as a vision document; plans list the five implied cases most likely to bite

code-reviewer.md: behavior the spec is silent on is graded by what a
reasonable person using the software expects, and a 'Declined to judge'
list makes every scoping decision visible. writing-plans: a Review Focus
section names the five implied input classes or failure modes most
likely to bite, each pinned by a test in the owning task. executing-plans
hands the section to the final reviewer and rules on every declined line.
Jesse Vincent 14 часов назад
Родитель
Сommit
7474980bb9

+ 10 - 3
skills/executing-plans/SKILL.md

@@ -241,8 +241,11 @@ Run `../subagent-driven-development/scripts/review-package PLAN_FILE MERGE_BASE
 available model — the whole-branch review is a judgment task — using
 superpowers:requesting-code-review's
 [code-reviewer.md](../requesting-code-review/code-reviewer.md), with the
-package path, the plan and spec paths, and a pointer to the ledger's
-`Ruling:` lines so it can weigh the calls you made. Specify the model
+package path, the plan and spec paths, the plan's Review Focus section
+verbatim if it has one (the input classes and failure modes the plan's
+tests do not exercise — the reviewer checks each deliberately), and a
+pointer to the ledger's `Ruling:` lines so it can weigh the calls you
+made. Specify the model
 explicitly; an omitted model inherits the session's, which may not be the
 most capable. This is the one fresh context the whole run buys. Do not
 skip it, and do not replace it with your own read of the diff.
@@ -255,7 +258,11 @@ weaker than a fresh reviewer, and your human partner decides whether that
 is enough before merge.
 
 Sort the findings before you act on any of them. The reviewer's severity
-labels are advice; the gate is yours. Re-grade first, by effect: the
+labels are advice; the gate is yours. Its "Declined to judge" list is
+yours too: every line there is a ruling you make and ledger, exactly like
+a plan conflict — `Final: Ruling: <behavior the reviewer set aside> —
+<what a reasonable person using this software gets, and why that stands
+or why it is now a finding> — <cost if wrong>`. Re-grade first, by effect: the
 spec is a vision document, and a finding's grade is what a reasonable
 person using this software gets if it ships, not whether the spec names
 the input that triggers it — a reviewer who set a finding at Minor

+ 17 - 0
skills/requesting-code-review/code-reviewer.md

@@ -30,6 +30,23 @@ Subagent (general-purpose):
     git diff [BASE_SHA]..[HEAD_SHA]
     ```
 
+    ## The spec is a vision document
+
+    The spec says what the software must do. It does not enumerate every
+    input, environment, or condition the software will meet. For behavior
+    the spec is silent on, judge by what a reasonable person using this
+    software would expect: a reasonable person's expectation is a
+    requirement, and a spec's silence is not permission. Grade such
+    findings by their effect on that person, not by whether the spec
+    mentions the trigger.
+
+    ## Declined to judge
+
+    Before your verdict, list every behavior you considered and set aside
+    as outside the plan or spec, one line each, with the reason. The
+    executor rules on each line; nothing you set aside is dropped
+    silently. An empty list means you set nothing aside.
+
     ## Read-Only Review
 
     Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.

+ 14 - 0
skills/writing-plans/SKILL.md

@@ -76,6 +76,18 @@ naming and copy rules, platform requirements — one line each, with exact
 values copied verbatim from the spec. Every task's requirements implicitly
 include this section.]
 
+## Review Focus
+
+[The five input classes or failure modes the spec implies but no task's
+tests exercise that are most likely to bite a person using this software
+— one line each, naming the input or condition and the behavior a
+reasonable person would expect, most likely first. The spec is a vision
+document: it says what the software must do, not everything it will
+meet, and its silence on an input is not permission for that input to
+break the program. Write the list here, once, with the spec in front of
+you. Then, for each line, add the test that pins it to the task that
+owns the code, in that task's own step style.]
+
 ---
 ```
 
@@ -148,6 +160,8 @@ After writing the complete plan, look at the spec with fresh eyes and check the
 
 **3. Type consistency:** Do the types, method signatures, and property names you used in later tasks match what you defined in earlier tasks? A function called `clearLayers()` in Task 3 but `clearFullLayers()` in Task 7 is a bug.
 
+**4. Review Focus:** For each input class or failure mode the spec implies, is there a task whose tests exercise it? The five uncovered ones most likely to bite a person go in the Review Focus section, and each line there gets its test added to the owning task. An empty section means you checked and found none, not that you skipped the check.
+
 If you find issues, fix them inline. No need to re-review — just fix and move on. If you find a spec requirement with no task, add the task.
 
 ## Execution Handoff