fix(review): run the success-criteria commands and hand the reviewer the result (D-04)

The rubric required `brief-conformance-reviewer` to classify a Success
Criterion as Full only when "its verification command/test exists and passes".
Its tools are `Read`, `Glob`, `Grep`. It cannot run anything, so "passes" was
either guessed from the command's mere existence or quietly downgraded to
"exists" — a BLOCKER-tier rule key resting on an impression.

The reviewer stays read-only — a reviewer that executes the code it reviews is
not an independent reviewer. The command does the running instead:

- `/trekreview` Phase 4.5 runs the brief's `## Success Criteria` commands
  through `lib/verification/criteria-runner.mjs --brief --evidence` and captures
  the block as `sc_evidence_block`, pasted verbatim into the reviewer prompt in
  Phase 5. The exit code does not stop the review — a failing criterion is
  exactly what the review exists to find.
- `formatCriteriaEvidence` builds that block in code: one row per criterion with
  the command, the exit code and the first output line. Chose a code-built block
  over an orchestrator-written summary so the orchestrator cannot narrate a pass
  that never happened.
- The rubric now judges the supplied result: `PASS` supports Full, `FAILED` /
  `BLOCKED` is `Broken` with the exit code cited, and `NOT RUN` is the absence
  of a measurement — never evidence in either direction.
- Phase 4.5 is skipped in `quick` mode: that mode does not launch the
  conformance reviewer, so there is nobody to hand the result to.

Red first: seven tests in `tests/lib/criteria-runner.test.mjs` against a
committed brief fixture whose three criteria pass, fail, and are prose-only.
The two doc pins were verified red against the pre-fix files (rubric asked
"exists and passes"; no Phase 4.5; the block reached nobody).

Suite: 1117 (1115/0/2), up 9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-18 01:34:18 +02:00
commit c23b009738
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
6 changed files with 274 additions and 8 deletions

View file

@ -50,6 +50,18 @@ You will receive a prompt containing:
- **Rule catalogue** — the 12-key catalogue in
`lib/review/rule-catalogue.mjs`. You may only emit findings whose
`rule_key` is in this set.
- **Success-criteria check results** — a table produced by /trekreview
(Phase 4.5) holding, per criterion, the command it declared, the exit
code it produced, and the first line of its output. /trekreview ran
them; you did not, and you cannot. This block is your ONLY evidence
about whether a criterion's verification passes.
**You never run a command.** Your tools are `Read`, `Glob` and `Grep`, and
that is deliberate — a reviewer that executes the code it reviews is not an
independent reviewer. Where the check results say `NOT RUN`, no result
exists: judge that criterion on delivered code alone and say so in the
Evidence column. Never infer, assume or reconstruct an outcome, and never
report a criterion as verified because its command merely EXISTS.
## Your process
@ -73,10 +85,14 @@ context is needed) and classify coverage:
| Coverage | Meaning | Finding emitted |
|----------|---------|-----------------|
| **Full** | Code change visibly implements the criterion AND its verification command/test exists and passes | none |
| **Partial** | Some pieces present but the verification path is incomplete (e.g., the command exists but tests are missing) | `MISSING_TEST` (MAJOR) or step-specific finding |
| **Full** | Code change visibly implements the criterion AND its check result is `PASS` | none |
| **Partial** | Some pieces present but the verification path is incomplete (e.g., the check result is `NOT RUN` because the criterion declares no command, or the command exists but tests are missing) | `MISSING_TEST` (MAJOR) or step-specific finding |
| **Missing** | No delivered code maps to this criterion | `UNIMPLEMENTED_CRITERION` (BLOCKER) |
| **Broken** | Code claims to implement the criterion but the verification fails or is structurally wrong | `BROKEN_SUCCESS_CRITERION` (BLOCKER) |
| **Broken** | The check result is `FAILED` or `BLOCKED`, or the code is structurally wrong for the criterion | `BROKEN_SUCCESS_CRITERION` (BLOCKER) |
A `FAILED` result is decisive: cite its exit code and output line in the
finding's `detail`. A `NOT RUN` result is NOT decisive in either direction —
it is the absence of a measurement, so it can never support **Full**.
Cite the criterion text in `brief_ref` (e.g., `SC3 — "review.md is
parseable as input to /trekplan"`).
@ -163,8 +179,8 @@ parsers find it by reading the last `json` code fence.
| Criterion | Coverage | Evidence |
|-----------|----------|----------|
| SC1 — "..." | Full | lib/foo.mjs:23 implements; tests/foo.test.mjs covers |
| SC2 — "..." | Missing | no implementation found in diff |
| SC1 — "..." | Full | lib/foo.mjs:23 implements; check `npm test` PASS (exit 0) |
| SC2 — "..." | Missing | no implementation found in diff; check NOT RUN (no command) |
| NG1 — "..." | Honored | no diff matches forbidden pattern |
| NG2 — "..." | Violated | lib/bar.mjs:88 implements forbidden behavior |
@ -231,6 +247,10 @@ parsers find it by reading the last `json` code fence.
- **Cite, don't speculate.** Every finding includes a `file:line`
citation taken from the diff. No "this might break" without quoted
evidence.
- **A check you did not get is not a check that passed.** Quote the exit
code from the supplied results when you call a criterion verified or
broken. If the results block is absent or says no criterion was checked,
state that in the Coverage matrix rather than judging the verification.
- **Respect the triage map.** Files marked `skip` are out of scope.
Cross-file inference is the coordinator's job, not yours.
- **No praise.** "Looks good", "well done", "no issues" do not appear in