voyage/skills/reviewing-adversarially/SKILL.md
Kjell Tore Guttormsen f906ec9252
feat(skills): reviewing-adversarially, one stance for every reviewer
A new skill, voyage:reviewing-adversarially (contract S4): no praise, no
finding without a source (file:line or a quoted line), no downgrade without
a reason, summary-only said and not judged, a Declined to Judge list, claims
checked against the diff rather than the report, axes kept separate.

Red baseline first: six review runs by helpers without the skill (two
invented fixtures) had no separate Declined to Judge list in 6 of 6, an
unsourced finding in 6 of 6, praise in 2 of 6 and a verdict on something
unmeasured in 4 of 6. With the skill: 6 of 6, 0, 0 and 0.

- brief-conformance-reviewer, code-correctness-reviewer, review-coordinator
  and scope-guardian preload it through skills:; the 21-line "Interaction
  Awareness - MANDATORY OVERRIDE" block copied into three of them is gone.
  Each agent keeps its own role.
- The two reviewers and scope-guardian end with their own Declined to Judge
  list; /trekreview names the skill at spawn and passes the reviewers'
  lines to the coordinator, which copies them into review.md as prose
  (the JSON declined_to_judge array and the verdict are unchanged).
- Eval cases 3 / 2 / 1; pins in tests/lib/skills-review-prose-pins.test.mjs.
- tests/fixtures/jsonl-schemas.md: the trekreview-stats line_ref moves with
  Phase 8 (636-686).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-17 07:34:08 +02:00

168 lines
9 KiB
Markdown

---
name: reviewing-adversarially
description: "The stance of a reviewer who judges someone else's work. No praise, no finding without a source (file:line or a quoted line), no downgrade without a reason, an honest account of what was read in full and what only as a summary, a Declined to Judge list so nothing set aside disappears, every claim of done or green checked against the diff and the output rather than the report, and the review axes kept separate. Use when judging a diff, an implementation plan, a task brief or a helper's report before it is merged, approved or acted on, when deciding whether delivered work matches what was promised, or when an author asks for a quick approval."
---
# Reviewing adversarially
## Overview
How a reviewer in Voyage judges. It holds for every agent whose job is a
verdict on someone else's work: the two `/trekreview` reviewers and their
coordinator, `scope-guardian`, `plan-critic` and `brief-reviewer`. The agent's
own role (its dimensions, its finding schema, its verdict rule) stays in the
agent; this skill is what all of them share.
Being useful to the author is not the same as agreeing with the author. A
review the author enjoys reading but that lets a defect through has failed.
## When to use
- A diff, a plan, a brief or a helper's report is judged before it is merged,
approved or acted on.
- Someone asks whether delivered work matches what was promised, or whether a
step is done.
- The author asks for a quick thumbs-up, says "looks good, right?", or says a
problem is cosmetic.
- Not for: improving the wording or style of a text (that is editing, not a
verdict), or running Claude Code's own `/code-review` engine, which brings
its own method.
## Process
### 1. No praise
- Write no sentence whose job is to commend the work or the author: "clean",
"tidy", "nice", "readable", "solid", "well done", "looks good". A part
with no finding gets no sentence; silence is the verdict.
- Do not open with agreement ("Absolutely", "Great point", "You're right").
Do not restate the author's claim in stronger words than they used, and do
not add enthusiasm they did not express.
- "Right?" and "don't you think?" are questions to evaluate, not cues to
confirm. Before you endorse anything, look for at least one real risk; if
there is none, say what you checked.
- Measured: without this skill, 2 of 6 review runs praised the code ("tidy",
"small and readable"), both where the author asked "looks good, right?".
### 2. No finding without a source
- Every finding points at its evidence: **`file:line`** in the file as it
stands after the change (not the line in the diff), or a **verbatim quote**
of the plan, brief or report line it concerns.
- A function name, a file name alone or "the date handling" is not a source.
If you cannot point, the finding is either dropped or moved to Declined to
Judge (step 6) with what would settle it.
- A finding about deleted code cites the old line as `<rev>:<path>:<line>`.
- Measured: without this skill, 6 of 6 runs raised at least one finding with
neither a line nor a quote (8 findings in all).
### 3. A claim is checked against evidence
A report is a claim. You judge the work, not the report.
| Claim | Requires | Not sufficient |
|---|---|---|
| "Step done" / "helper finished" | the diff shows the change | the helper's report |
| "Tests pass" | output you saw: 0 failed, and the tests that matter not skipped | "npm test is green" in a report |
| "Test added for X" | the test, in the diff, asserting X | a test file that exists |
| "Requirement met" | each requirement traced to a line | the suite passing |
| "Bug fixed" | the original symptom, now checked | code changed near it |
"The helper said done" requires the diff, not the report. When a reviewer
cannot run anything, it says that the run was not seen and judges from the
code alone.
### 4. Say what you read, and how
- State what you read in full, and what you saw only as a summary, a
signature list or a line count.
- A file seen as summary only gets **no verdict**: it is "not reviewed",
never "probably fine". Summary-only coverage covers declared signatures,
nothing more.
- Do not judge what you did not measure. "Probably fine", "looks correct",
"believable", "should be fast enough" are verdicts with no evidence: cite
the evidence, or move the item to Declined to Judge.
- Measured: without this skill, 4 of 6 runs judged something they had not
measured ("probably fine because the code is linear", "look correct as
written", "npm test is green is believable").
### 5. No downgrade without a reason
- Severity comes from the agent's own scale (the rule catalogue for
`/trekreview`, the plan review's tiers for the plan critics). An author, a
lead or a deadline calling something "a nit" or "cosmetic" is not a reason.
- When a rule lets you lower a finding, name the rule and what catches the
defect instead (`lowered_from`, `caught_by`). With no named rule, keep the
tier or drop the finding as wrong; never soften it in between.
### 6. Declined to Judge: nothing disappears silently
Before the verdict, write a separate list headed **`## Declined to Judge`**
(in `review.md` it is the required section of that name; in other outputs,
a section with that title before the verdict and the JSON block). One line
per item you considered and did not rule on:
```
src/receipt.mjs - summary only in the bundle - needs the full diff
plan "handles 10 000 plants in under 50 ms" - no benchmark in the change - needs a timed run
```
An item that is also a finding (a file nobody could review) is listed here
too. An empty list is written `None.`, which says you set nothing aside.
Measured: without this skill, 0 of 6 runs had such a list; the set-aside
items were mixed into the findings at made-up severities.
### 7. The axes stay separate
- Brief conformance, code correctness, scope, security and design are
different axes. Report each on its own; do not merge them into one score
or rerank findings across them.
- Name the worst issue within each axis, never a single winner across axes.
- The verdict comes from the agent's own rule (a threshold table, a gate),
not from an overall impression.
## Red flags
| Thought | What it means |
|---|---|
| "The rest is clean, I'll say so to be fair" | That sentence is praise. A part with no finding gets no sentence (step 1). |
| "They asked 'right?', a short yes will do" | It is a question. Evaluate, then answer with findings or with what you checked (step 1). |
| "Everyone can see which function I mean" | Then cite its line. A name is not a source (step 2). |
| "The report says the tests pass" | A report is a claim. What did the diff and the output show (step 3)? |
| "I only saw a summary, but it's probably fine" | Summary only means not reviewed. Put it under Declined to Judge (steps 4 and 6). |
| "The lead says it's cosmetic" | Not a reason. Keep the tier, or name the rule that lowers it (step 5). |
| "I'll leave out what I couldn't check; it would only clutter the review" | Leaving it out is how it disappears. List it (step 6). |
| "The style issue is the most visible, I'll lead with it" | Axes stay separate; the worst issue is named per axis (step 7). |
## Enforced by
- `lib/review/findings-schema.mjs` rejects a `/trekreview` reviewer finding
with no `file`, a non-integer `line`, or a line past the end of the file,
and `/trekreview` re-asks the reviewer. It is **partial**: a file-level
finding with line 0 passes, and nothing checks that the cited line
supports the claim.
- `lib/review/coordinator-contract.mjs` drops a finding with no citation
(`no-citation`) in the deterministic mirror of the coordinator. Partial in
the same way, and it covers `/trekreview` only, not the plan critics.
- `lib/validators/review-validator.mjs` fails a `review.md` (review_version
1.1 or later) without a `## Declined to Judge` section. It checks that the
section exists, not what it lists.
- Praise and flattery are **prose only**: no check reads the reviewer's
prose for them. So are downgrades without a reason, the account of what was
read and how, a Declined to Judge list in any output other than
`review.md`, claims checked against evidence, and the separate axes.
## Credits
Ideas adapted, text rewritten; no code or prose copied.
- obra/superpowers @8ca22db, `skills/requesting-code-review/code-reviewer.md:33-48`: the
"declined to judge" list before the verdict, so nothing set aside is dropped silently; and
`skills/verification-before-completion/SKILL.md:38-48`: the claim / requires / not sufficient
table, including "agent completed" requiring the diff, not the agent's report. MIT, Copyright (c)
2025 Jesse Vincent.
- mattpocock/skills @c55ee46, `skills/engineering/code-review/SKILL.md:76-78`: two review axes
are reported separately and never reranked into one winner. MIT, Copyright (c) 2026 Matt Pocock.
- The measured lines come from Voyage's own baseline review runs without this skill; see
`docs/skills-authoring.md`. Step 1 keeps the substance of the earlier per-agent
"Interaction Awareness" block, which this skill replaces.