---
name: radin-review
description: |
  Review a commit, PR, directory or range on two axes, Standards (repo
  rubrics) and Spec (the originating backlog entry), triage findings with
  the user, and log the kept ones as backlog entries. Use for /radin-review,
  "review and log to backlog".
---
# Review to Backlog

Review a caller-specified scope on two axes, and persist every finding the
user keeps as a backlog entry instead of terminal output. That leaves a
durable backlog `radin-execute` (or a human) works through later.

- **Standards** — does the scope follow the rubrics this repo documents?
- **Spec** — does the scope do what the originating backlog entry or its plan
  asked for?

The axes stay separate end to end, for the reason
`/mattpocock-skills:code-review` states under *Why two axes*. radin supplies
the spec that skill has to hunt for: the backlog entry, or the plan
`radin-plan` wrote from it.

**Sub-agent caller** — `radin-execute`'s Phase 6 dispatches this skill that
way, and a sub-agent reaches neither the user nor a spawned agent's result:
run every step inline, Standards axis
first, skip each `AskUserQuestion`, log every in-scope finding, and say in
Step 6's report that no triage happened.

## Step 1: Resolve the scope

```bash
RADIN_CLI scope [<arg>]
```

It prints `type`/`scope`/`command`/`passes` lines, and Step 3 invokes exactly
the skills the `passes` line names. Route on its exit code:

- **1**: not a commit, PR, directory, range, or `since <date>`. Report it and
  stop.
- **2**: ambiguous, candidates on stderr (e.g. both a PR number and a
  directory). Ask which one; a sub-agent caller reports both readings and
  stops.

State the resolved scope in one line — `Scope: commit a1b2c3d`,
`Scope: directory src/auth/` — then capture both inputs the axes need, here
and once:

- **The diff command.** The `command` line is the scope's one diff command.
  Record it verbatim and paste that same string into every later Bash call and
  both briefs.
- **The commit list.** `RADIN_CLI scope --commits [<the same scope arg>]`
  prints one `<hash> <subject>` line per commit. A `dir` scope prints nothing:
  it has no commit list, and both briefs say so.

Run the `command` before either axis starts. No output — for a `dir` scope, no
file under the path — means report "nothing to review in `<scope>`" and stop.
Write no backlog entry to prove the skill ran.

## Step 2: Resolve the spec

The Spec axis reviews against the originating backlog entry or its plan. Work
these rungs in order and stop at the first that yields a spec:

1. **The invoking prompt named one** — a backlog id, a title, or a path. For an
   id or title:
   `RADIN_CLI backlog field "<it>" TASK_FILE` and
   `RADIN_CLI backlog field "<it>" PLAN_PATHS` (its exit 1 means no plan; the
   task file alone is then the spec). For a path: that file is the spec.
2. **The backlog holds it.** `radin backlog add` stamps each entry with its
   creation instant and branch, and `task-done` copies the creation instant,
   commit and branch into `completed.json`. One call joins them to the scope:

   ```bash
   RADIN_CLI scope --specs [<the same scope arg as Step 1>]
   ```

   It prints one `id`, `created`, `branch`, `commit`, `source` line per
   candidate, oldest first. `source` says how the entry joined: `commit` (the
   task's commit is in scope), `reflog` (a squash or rebase rewrote that
   commit, and the review branch's reflog still lists it) or `pending` (an
   open entry recorded on the review branch). Route on the result:

   - **Exit 4**: a recorded commit no longer exists and HEAD sits on no branch.
     Report the stderr line and stop the review.
   - **No output**: fall to rung 3.
   - **Lines**: one `AskUserQuestion` shows them in the printed order and asks
     the user to confirm the order or drop entries. A sub-agent caller skips
     the question and takes the `commit` and `reflog` lines only: a `pending`
     entry is unfinished work, not what the scope implemented.

   For each confirmed `id`, a `pending` one resolves as in rung 1. For the
   others, `RADIN_CLI state completed-show "<id>"` names the plan the task ran
   against: its `plan` line carries the recorded paths, comma-separated, and
   those files are the spec. Several ids: every one of their plan files is the
   spec, and the axis covers all of them. Also run `RADIN_CLI state trace "<id>"`
   for each completed id: when its `facts` line names a file, the Spec axis
   reads that file with the plans, because it records where the implementation
   departed from the plan on purpose and why.
3. **Ask.** One `AskUserQuestion` offering the top entries from
   `RADIN_CLI backlog list` as options, with the tool's free-text field
   carrying a path the user types instead. "There isn't one" falls to rung 4.
4. **Skip.** No spec: the Spec axis does not run. Say so once here, and report
   `no spec available` for that axis in Steps 3 and 6. A sub-agent caller
   arrives here straight from rung 2.

## Step 3: Run both axes

Dispatch both axes in one message as two parallel sub-agents, so neither
pollutes the other's context.

Each brief is self-contained: paste in full the resolved scope line, Step 1's
`command` string and commit list verbatim, and the citation rule below. The
spec goes in as an absolute path with "read it in full before reviewing"; the
sub-agent's `Read` is the access that paste-in-full rule exists to provide.

> Every finding cites one `path:line` the scope introduced: for a diff scope
> the lines it adds or changes, for a `dir` scope the files under that path.
> Read surrounding code for context, never to find findings. A problem that
> predates the scope qualifies only when a changed line is what makes it wrong,
> and that changed line is the citation. Skip anything a formatter or linter
> already enforces.
>
> Give each finding a severity (`critical`, `warning` or `nit`) and its
> evidence: the rung it reached — `cited` (you pointed at the line), `traced`
> (you followed a caller or data path and showed the bad case happens) or
> `ran` (you executed it and saw it fail) — then the reasoning or output.
> Trace callers and data paths through `codebase-memory-mcp`'s MCP tools
> before Grep: a graph hit is a pointer, so read the file before you cite it,
> and never conclude a caller is absent from an empty result.

**Standards brief.** Invoke `/thermo-nuclear` against the scope, then every
skill Step 1's `passes` line names. Report — per file/hunk where relevant —
(a) every place the scope breaks a rubric this repo documents: cite the
rubric, file and rule; and (b) every structural finding the passes raise: name
it and quote the hunk. A documented repo rubric overrides a pass's generic
judgement, and a pass finding stays a labelled judgement call
("possible <name>") — the label reaches the backlog entry an agent later acts
on without the review in front of it. Report findings only, each as its claim, citation, severity and evidence.

**Spec brief.** Skipped entirely when Step 2 reached rung 4. Report: (a)
requirements the spec asked for that are missing or only partly implemented;
(b) behaviour in the scope that the spec never asked for (scope creep); (c)
requirements that look implemented but whose implementation looks wrong. Quote
the spec line for each finding, and cite the `path:line` it lands on. Report findings only, each as its claim, citation, severity and evidence.

Relay both reports under `## Standards` and `## Spec` headings before any
triage, each axis in its own order and neither reranked against the other.

## Step 4: Filter, classify, triage

Nothing reaches the backlog until the user agrees to it. Both axes' citations
go through one filter, which drops the out-of-scope findings:

```bash
printf '%s\n' "<path:line per finding, one per line>" |
  RADIN_CLI scope --in-scope [<the same scope arg as Step 1>]
```

Keep the findings on the `in` lines, drop the `out` ones, and carry the
`dropped` count into Step 6. Cite one line per finding; for a range, its first
line. A Spec finding about missing behaviour cites the in-scope line the
requirement should have landed on — the hunk with the partial behaviour, or the
one nearest where it belongs. A finding with genuinely no in-scope line stays
in Step 3's relay, and Step 6 names it as unlogged.

Then test each `cited` finding that depends on an input or a state ("what if
this is null"): it survives only when you find a call site or data path that
produces that input. Find the callers through `codebase-memory-mcp`'s MCP
tools, read them to settle it, and dismiss the finding when none does — an
empty graph result is not proof, so confirm with Grep before dismissing. A `traced` or `ran` finding already carries that proof.

Then classify each survivor. This is a rule, not a judgment: **fix** for
incorrect behavior — on the Spec axis, a missing, partial or wrongly
implemented requirement — and **refactor** for everything else, scope creep
included.

Then attribute each survivor's origin, so the review shows whether defects
trace to plans:

```bash
printf '%s\n' "<path:line per survivor>" |
  RADIN_CLI scope --attribute [<the same scope arg as Step 1>]
```

Each line carries the citation, its task id and its plan paths (comma-joined);
`none` in either field, a `dir` scope or an untraced commit means class
`none`. Otherwise settle the class in this order:

1. `RADIN_CLI state plan-covers "<plan csv>" "<cited path>"` prints `out`:
   class `executor`, the code touched a file outside the plan's Changes.
2. Otherwise read the plan's `## Decisions`: class `plan` when a Decisions line
   prescribed the flaw or left the choice open for the executor to guess,
   `executor` when the plan was right and the code deviated. Quote that
   Decisions line, or write `no Decision covers it`, so the label can be
   audited. For class `plan`, also name the plan-template section whose entry
   would have caught the flaw (`outcome`, `decisions`, `changes`,
   `reach-and-reuse`, `rubrics-triggered`, `modes-and-states`, `order`,
   `testing`, `out-of-scope`), or leave it empty when no section would have.

A sub-agent caller does this inline.

Print the survivors as a numbered list — number, axis, category, severity,
location, the finding in a clause — every Standards finding first, then every Spec finding,
each in its own axis's order. It is the triage handle for what Step 3 already
printed, so it carries no further detail. Mark the ones you recommend
tackling, on severity and effort rather than count; recommending all of them
is a valid answer.

Then gate on `AskUserQuestion` (single select):

1. **Recommended only** — log the ones you marked.
2. **All** — log every in-scope finding.

A free-text answer names the numbers you printed; read it as that subset and
nothing more, and restate the subset in one line before continuing.

## Step 5: Log the agreed findings to backlog

The body carries what a sub-agent with no review context needs to act on the
finding:

```bash
RADIN_CLI backlog add <fix|refactor> "<short title>" <<'EOF'
**Scope:** <what was reviewed, from Step 1>
**Finding:**
<the problem as the review stated it: direct, specific. A Spec-axis finding
opens with the spec line the brief quoted.>
**Severity:** <critical|warning|nit>
**Origin:** task <id|none> · plan <path|none> · class <plan|executor|none> — <the quoted ## Decisions line, or the reason>
**Evidence:** <rung: cited|traced|ran> — <the reasoning or output the brief gave>
**Preferred remedy:**
<the concrete restructuring suggested>
EOF
RADIN_CLI backlog set-meta <id> location "<the cited path:line>"
```

For class `plan` and `executor`, end the `**Origin:**` line with
`Read <plan path> ## Decisions before changing code.`, so the fix's sub-agent
reads the original plan's decisions.

Log one entry per agreed finding, in the order presented. Log nothing the
user discarded.

After the last `add`, journal the logged findings in one call (skip it when
nothing was logged) and keep its output for Step 6. Task files are deleted at
`task-done`; the journal event keeps the finding-to-plan link. A class `plan`
finding carries its Step 4 section as a fourth column, empty when it has none;
other classes carry no fourth column:

```bash
printf '<task id or empty>\t<class>\t<plan path or none>[\t<section>]\n...' |
  RADIN_CLI state review-findings
```

## Step 6: Report back

- The resolved scope reviewed, and the spec path or `no spec available`.
- The entries logged, one line each — these are the `add` calls you just made.
- The origin rollup: `review-findings`' output verbatim (for example
  `12 findings: 9 plan (3 tasks), 2 executor, 1 none`, then one line per task).
- Findings the user discarded, findings dropped as out of scope, and Spec
  findings dropped for having no in-scope line: three counts, no detail.
- Findings dismissed by Step 4's call-site test: one line each, the finding
  and the reason, so the user can overrule a dismissal.
- The backlog index path.
- Zero findings: say the review passed both axes. Every finding discarded:
  report that. Either way, write no entry to prove the skill ran.

End with one line: total findings per axis, and the worst issue within each
axis. Give each axis its own worst, never one winner across the two — that is
the reranking the separation exists to prevent.