review-map

Imported from wyeworks/accountable-review (skills/review-map/SKILL.md). Install upstream with npx skills add wyeworks/accountable-review --skill review-map. Copyright stays with the author.

name: review-map description: Builds an HTML review map of a pull request — what changed, the judgments the reviewer has to make with the exact lines that settle each one, the order to read the code in, and what the change reaches in code it did not touch — so a reviewer can explain the change before judging it. Targets Rails and Elixir/Phoenix — a Phoenix LiveView app or a Rails or Phoenix JSON API, with or without a separate client such as Next.js. Use this whenever someone needs to understand a change rather than grade it: asks what a PR or branch does, where to start on a large diff, which files actually matter, what the change might break, whether the frontend and backend still agree, or needs to bring a reviewer up to speed on someone else's work — even if they never say "review map" or "walkthrough". Invoke with /accountable-review:review-map in Claude Code or $review-map in Codex, optionally passing a PR number, URL, branch, or diff range. There is one page shape and no flag chooses it. An effort level is separate: --effort high is the default and tries to falsify the run's own analysis before the page is written, --effort low skips that pass. --mentor is for a reviewer new to the stack rather than to the change: it adds a framework primer inside the checkpoints that earn one and changes nothing else about the page. Passing --output makes the run non-interactive: the page is written to /index.html as portable static HTML instead of being published, which is how CI generates one. Not for posting review comments or approval verdicts.

Review Map

Turns a diff into one HTML page a reviewer can work from: what the change is for, which judgments it asks of them and where to look to make each one, the order to read the code in, and what the change reaches outside the lines it touched.

The page is a review agenda, and the deep analysis is how it is built — not what it prints. A run traces consumers across the whole diff, reads the tests, follows a value across the boundary and attacks its own conclusions. What reaches the reader is the part they have to act on. Reducing the reading obligation is the goal; reducing the analysis is the failure.

Do not determine whether the PR is correct. Help a competent reviewer determine whether it is. That is the whole product. A page good enough to approve from without reading code is a failure — the reviewer ends up holding a verdict instead of a mental model, and the debt lands on whoever touches the code next.

So the page is a navigation aid, never a replacement for reading the code. The question it has to pass is: after following this, can the reviewer explain what changed, why it works, and where the important decisions live? Not: can a model summarize this diff?

You are not a code reviewer here. Do not grade the change, do not recommend approval, do not post comments on the PR. If the project has a review command, say so at the end and let it do that job.

Host and invocation

Use /accountable-review:review-map in Claude Code or $review-map in Codex. Both accept a PR number, URL, branch, or diff range; --effort high (default) or --effort low; --mentor with an optional stack; and --output <dir>. --review is not implemented.

Before step 1, read only your host's reference: Codex or Claude Code. It owns delegation and delivery mechanics; the ten steps below own the review. Resolve bundled paths relative to this SKILL.md, not the repository being reviewed. User instructions take precedence over skill guidance.

What is bundled

The procedure below relies on ten bundled files. Read each at the step that needs it rather than up front — the procedure itself is the only part that has to be in context the whole way through.

Three of the ten come in pairs, and you read one of each pair, never both. Step 2's stack detection picks the lens file and the catalogue: a Rails run reads the Rails pair, a Phoenix run the Phoenix pair, and reading the other stack's file costs context and teaches the wrong searches. The host reference is the same rule one level up — yours, not the other host's.

Paths are relative to this skill's base directory; do not rely on a plugin-root environment variable such as $CLAUDE_PLUGIN_ROOT, which is not set in the shell.

One bundled file is not one you read: references/claim-falsifier.md, the independent reader's mandate. At --effort high, which is the default, a reader is sent at each analysis note this run wrote at the end of step 6, and its challenges are folded in at step 8. You hand that reader the mandate's absolute path; you never load it yourself — its instructions are its own, which is the point of putting them in a separate context. The host reference owns how the reader is launched.

1. Resolve the target and the effort

• Argument may be a PR number, a PR URL, a branch, or a diff range. With no argument, use the current branch against its base.

• There is one page shape, and no flag chooses it. No argument selects a length, a depth or a second document, and there is nothing to map one onto if someone invents one. An argument starting with -- that is not one of --effort, --mentor, --output, --update, --repository, --base-sha or --head-sha with its value is reported, not guessed at. A misread flag silently produces the wrong run, and the reader has no way to tell.

• The page has a word budget, stated as guidance, and you write to it rather than trimming to it. references/report-format.md § The agenda budget owns the numbers — 80 to 160 words for What changed, 50 to 140 a checkpoint, and a page total that is those parts summed — about 700 to 1,500 visible words at four checkpoints on a small or medium PR. Read it now, with the rest of that file, rather than discovering the caps while drafting. It caps prose and nothing else: no checkpoint, no citation, no evidence tier and no figure comes out for the budget's sake, and the number it never touches is the checkpoint count.

• Read the effort off the invocation too, and hold it the same way. One of --effort high, --effort low, in any position; no flag means high. It decides how hard the run works to be right, and at --effort high that buys exactly one thing today: the falsification pass — spawned at the end of step 6 against the analysis notes, folded in at step 8. --effort low opts out of it, and is the flag to reach for when the diff is small enough that a second reader has nothing to find.

  --effort normal is accepted and means low. It was the name of the opt-out back when it was the default, and a name that says normal for the path fewer runs take is the wrong way round. Take it silently rather than reporting it: an invocation someone had in a script is not a typo, and this is the one --effort value that is not guessing.

  high is the default because it changes what the page finds, and it costs time it does not cost tokens. Measured on one 28-file PR: the falsifiers cost 23 seconds of blocked parent, 0.9% of a 2607-second run, because they run while the next stage is drafted rather than instead of it. The same target at low missed five findings the falsified run carried, including the two the reviewer most needed.

  That 0.9% is blocked wall clock and is not what the pass costs. Each falsifier reads in its own context, and those tokens are the run's tokens: measured across three real runs, the pass was 17-23% of every cache-read token the run spent, on 145-216 requests. Both numbers are true and they answer different questions — the first is why spawning them does not slow the run down, the second is what they add to the bill. evals/profile.sh prints them side by side, and Claude's falsifier runs on its own model (agents/claim-falsifier.md) so the second number can be bought down without touching the first. Codex inherits its configured model, and both measurements are from Claude runs. Effort is what decides whether the page is right, and nothing makes a run faster by making the page shorter — the time goes into tracing consumers at step 5, not into writing sections. Three rules:

  • It produces no section, no marker, no chip and no sentence. Two pages of the same target at the two efforts differ in their claims, never in their shape, and a reader cannot tell which produced the one in front of them.
  • An --effort value that is none of the three is reported, not guessed at — work that silently differs, with nothing in the output to tell the reader which they got.

  Do not announce the effort — nothing about the pass reaches the page, and step 8 says why.

• --mentor is the one flag that puts anything on the page, and it is off by default. It says the reviewer is new to the stack rather than to the change, so a judgment that turns on a framework rule they may not know gets that rule stated rather than linked. What it admits is one component and nothing else: the primer callout, aside.primer, inside the checkpoints that earn one. references/report-format.md § Mentor mode owns every rule about it, and step 7g is where a checkpoint earns one.

  It may carry a stack name — --mentor rails, --mentor elixir, --mentor phoenix — and the name is checked, never used to choose. Step 2 detects the stack from the repository; a name on the command line that disagrees with what is in front of you is reported, not obeyed. Taking it as an override would reintroduce the worst regression this skill has: a Rails lens over a service that is not Rails, inventing findings confidently. A bare --mentor is the ordinary form and the one to prefer, and a value that is none of those three names is reported like any other unknown flag.

  Hold it, and hold the subtraction rule with it. Everything else about the page is the page a run without the flag writes — same sections, the same checkpoints in the same ranked order, same reading path, same impact panel, same evidence foot, same prose budget on every other part. Delete the primers and you have that page back. A run that also lengthened its explanations, added a checkpoint or reordered anything "because the reader is new" has turned a flag into a second document.

  There is no mentor marker, chip or banner. The flag's effect is visible by being on the page, which is exactly why it needs nothing announcing it — and a count of primers would be the page grading its own thoroughness. Say which flags you took in chat, not in the artifact.

• --output <dir> makes the run non-interactive. It is the only flag that changes where the page goes rather than what is on it: the page is written to <dir>/index.html and nothing is published — no Artifact call, at any stage. Everything else is identical, and has to be. Same sections, same depth rules, same excerpt budget, same gate. A CI run and a person's run produce the same page from the same procedure; the moment this flag starts meaning a cheaper page, there are two products and only one of them is developed against.

  Three flags travel with it, carrying what gh would otherwise be asked for:

  --repository <owner/repo>   --base-sha <sha>   --head-sha <sha>
  

  Prefer them over anything you derive. --base-sha is BASE and --head-sha is HEAD for every diff in this run; do not compute a merge-base over them and do not call gh for what they already say. The caller knows which revision it asked about, and a run that recomputed a base from a branch that has since moved would describe a revision nobody requested. With --repository and a PR number the run also has what link rung 1 needs without gh at all — check reachability as always, but do not re-derive the identity of the change. The flags are legal on their own; they simply have no other reason to appear.

  <dir> must be outside the repository under review, for the reason the hard rules give: the page must never become part of the diff it describes.

  $W is unchanged by this, and that matters. --output moves the finished page, not the run's scratch: $W stays the derived work directory and excerpt fragments keep landing there. Write the page itself straight to <dir>/index.html — a crash then leaves a useful page where the caller is going to look for one — and leave <dir> holding nothing else, because whatever is in it is what gets delivered.

  ci/generate-review-map.sh, at the plugin root, uses this path for Claude CI. It supplies all four flags, checks afterwards that the page names its revision and no longer says it is being written, and refuses to deliver one that does.

• --update re-reads only the commits since the last map. It takes no value, and it says: a page for this target already exists where this run would write, so inspect the delta since the revision that page names and edit it in place rather than rebuilding it. § Re-running over new commits below owns every rule about it; read that section before acting on the flag.

  Two things about it belong here, beside the flags it sits with. It is not a level and not an effort. It never lowers a cap, never skips the gate, never skips step 8 for anything it writes, and never admits or removes a component; the moment it means "fewer excerpts" or "skip section 04" it has become a second document reached by a flag, which is the thing this page has no shapes for. And with no previous page, or when the plan below refuses, it falls back to a full run and says so in chat. That is not an error: a full run is always the better page, so every way this flag can fail leads to one.

• Find the base ref: the PR's base if there is one, else the default branch (git symbolic-ref refs/remotes/origin/HEAD, falling back to main, then master). This gives you a ref, not a merge point — do not compute a merge-base yourself. The three-dot diff below already resolves it, and hand-rolling one is how you end up diffing against a moved base.

• Use three-dot diffs throughout (git diff BASE...HEAD) so you see the branch's own work and not unrelated commits from the base.

• Check the working tree with git status --porcelain. Three-dot diffs ignore it entirely, so uncommitted edits mean the page can describe code that differs from what the reviewer has checked out. Say so in the page and cover the committed state only. If the uncommitted changes are substantial enough that the page would be misleading, stop and say why instead.

• If there is a GitHub PR, capture owner, repo, number, head SHA, title, author, body via gh pr view <target> --json number,title,author,body,headRefOid,headRefName,baseRefName,url. The number and the head SHA are what the links are built from.

• Record the SHA the diff's left side comes from as well — git merge-base <BASE> HEAD — with or without a PR. Deep links need both ends: head for a line that still exists, the left side for a line the change removed or for behaviour cited as it was. This is the one legitimate merge-base here, a permalink needs a commit rather than a range; keep using BASE...HEAD for every diff.

• If gh is missing or there is no PR, continue anyway with the local branch. This is a normal case, not an error.

• Fix the work directory now, and derive it rather than choosing it. Everything this run writes goes in one place, named after the target so a re-run recomputes it instead of remembering it:

  W="${TMPDIR:-/tmp}/review-map/<repo>-pr-<N>"      # a PR
  W="${TMPDIR:-/tmp}/review-map/<repo>-<branch>"    # no PR; / in the branch becomes -
  mkdir -p "$W"                                     # the page is $W/page.html

  Derived, not invented, because step 9 republishes to the same file path and step 10 reuses it on a re-run — one PR, one URL, across pushes as well as stages — and a session-scoped scratch directory cannot satisfy that: the next session gets a different one. Export W once and use it in every later command. A run that instead picked a path per command spent nine calls and seventy seconds re-establishing it while splicing excerpt files it had first written somewhere else and then had to move.

• Fix the deep-link mode now, not at render time. Check whether the head SHA is even reachable on a remote — git branch -r --contains <HEAD_SHA>, where exit 0 and empty output means it was never pushed and every permalink to it would 404. Unpushed branches and worktrees are among the most common targets for this skill, so expect this. A non-zero exit is a state of its own and neither of the other two: git was asked and could not answer, so nothing is known about reachability. Do not read it as unpushed — say in the masthead that reachability could not be determined and render citations as plain text, rather than asserting a branch is unpushed on an answer git never gave. Pick one rung from the ladder in references/report-format.md and hold it for every citation. The rung decides whether anything is clickable; it does not decide the form — inside a rung, a line in the diff links to the diff page and a line outside it links to a blob, with one exception that is settled per file in step 3 and not per citation. At rung 2 there is no PR page, so check the base SHA for reachability too: without it there is no compare view to anchor into.

2. Discover the project

Assume nothing about layout or conventions — this skill travels between repos.

Detect the stack first, because it decides which two of the bundled files the rest of the run reads:

• config/application.rb, or a Gemfile, → Rails: the lens file is references/rails-nextjs.md, the catalogue references/rails-docs.md.

• mix.exs → Elixir/Phoenix: the lens file is references/phoenix-liveview.md, the catalogue references/elixir-docs.md.

• More than one root, or one of each — a monorepo with an api/ and a services/, engines, an umbrella — ask which to cover rather than picking. Same rule as several Rails roots, and for the same reason: covering the wrong half produces a page that is confidently about code the reviewer is not reading.

  Look at the diff before asking, and answer it from there when you can. If every changed application path sits under one root, that root is detected rather than chosen and there is no question to put. Ask only when the diff genuinely spans more than one.

  Non-interactively (--output) there is nobody to ask, and picking is still forbidden. So when the diff does not settle it, stop and say which roots were found and that the run needs one named — a missing map is reported by the caller and a confidently wrong one is not. This is the case CI meets most often, which is why it is spelled out rather than left to the general rule.

• Neither — say so plainly, cover the diff with the stack-independent material (the five sections, the checkpoints, the tiers, the impact chains, the evidence foot), and emit no documentation link and no probe. Do not default to Rails: a Rails lens applied to a Go service invents findings, and a catalogue that does not describe this application is the failure both catalogues fail closed to avoid.

If --mentor named a stack, check it here and nowhere else. The detection above is the answer; the name on the command line is a claim to test against it. Agreement is silent. A disagreement — --mentor rails in a repository whose only root is mix.exs — stops the run, names both readings and says to re-run with a bare --mentor. It is never correct input, which is why it is handled like any other flag value that cannot apply rather than resolved in the run's favour: the likeliest cause is the wrong checkout, and a page confidently about the wrong repository is the failure a one-line stop is cheap against. What it must never do is let the name pick the lens.

Detecting the stack is not reading its files. This step decides which pair the rest of the run uses. The lens is read at step 5, where its search recipes are the work; the catalogue at step 7, when a claim first asks for a URL. Nothing between here and there needs either, and both are large — a run that opens them now carries them through the whole of steps 3 to 6, which is where the consumer tracing happens and where the context is already largest. The versions this step records come out of Gemfile.lock or mix.lock, not out of the catalogue.

Backend, Rails. Locate the Rails root by finding config/application.rb. It may be at the repo root, under a subdirectory such as api/, or there may be several (engines, monorepo). Detect, don't assume: RSpec vs Minitest; API-only (config.api_only) vs server-rendered; the authorization library, if any; the serializer library; the background job adapter; whether strong_migrations is present.

Backend, Phoenix. Locate the Mix project by finding mix.exs, and read the OTP app name from it — that is what lib/<app> (the domain) and lib/<app>_web (the web layer) are named after, and the split is where the context boundary lives. Detect, don't assume: the Repo module and its adapter; whether this is a LiveView app, a JSON API, or both (live "…" in the router versus json responses, @derive {Jason.Encoder, …} or a …JSON render module); how authentication is attached (on_mount inside a live_session, or a plug in a pipe_through pipeline) — the two are different mechanisms and a route can miss either; whether Oban, Broadway or bare Task does background work; whether assets/ holds JS hooks; and whether a separate frontend application exists at all.

Record the versions the documentation links are pinned to. This is not bookkeeping: every documentation link on the page is pinned with them, so a run that skipped this step cannot emit a doc link at all — recording the version is this step's job, and knowing which URL to pin is step 7's. What to record differs by stack, and so does its shape:

• Rails — the Rails version and its series from the rails (x.y.z) line in Gemfile.lock (rails (8.0.2) gives 8.0), plus the exact locked versions of the gems above, which pin to a tag. One series covers the whole framework.
• Elixir — the exact locked version of each package from mix.lock (phoenix, phoenix_live_view, ecto, ecto_sql, plug, oban), plus the Elixir version from .tool-versions or mix.exs. There is no series: hexdocs serves exact versions, each package pins independently, and a page carrying several different version segments is correct rather than broken. The catalogue carries a few packages beyond that list; if step 7 asks for one of them, grep its line out of mix.lock then — opening the catalogue here to find out which names to look for is the read this step is trying not to do.

Each catalogue's § Pinning owns the emitted forms. The same versions also decide what the page may claim, because two marks in each catalogue turn a version-sensitive behaviour into a probe rather than a sentence.

At --mentor, a closed catalogue also decides whether the flag can do anything. A primer is what a doc link escalates into and is gated on carrying one, so while elixir-docs.md § Version withholds every link a Phoenix run at --mentor writes no primers at all and produces the ordinary page. Say that to the user, in chat, when the run starts — a flag that silently did nothing is worse than one that says why it could not. Do not say it on the page: a line explaining the absence of primers is the mentor badge § Mentor mode refuses, with an apology attached.

Frontend. If the stack is Phoenix LiveView with no separate client application, the frontend is the .heex templates and whatever sits in assets/ — there is no second application and no generated type to reconcile, so skip to Conventions and let § LiveView in the lens file carry the seam. A LiveView app's boundary is the phx-* attribute and the callback that answers it, not a JSON contract. Otherwise locate the client the same way — package.json, next.config.*, app/ versus pages/. Then find the seam between the two sides, because that is what a contract judgment is built from:

• the API client or fetch wrapper, and where base URLs and error handling live;
• whether types crossing the boundary are generated from the backend or hand-written. Generated types drift loudly, at build time. Hand-written ones drift silently, which is the case worth hunting;
• runtime validation at the boundary (Zod or similar) and whether it is applied to every response or only some;
• the data-fetching layer: React Query, SWR, server components, route handlers.

Conventions. Look for the project's own in CLAUDE.md, AGENTS.md, docs/, README, CONTRIBUTING.md. If found, check the PR against them. If not, infer the house style from adjacent unchanged code of the same kind — often more accurate than a stale document. Note which top-level directories under app/ (or lib/<app>/) exist and what each holds: that inventory is the evidence step 7b needs to ask whether a coding decision in this diff departs from one, and a departure it cannot cite is a question it may not ask.

3. Inventory the diff

• git diff --name-status BASE...HEAD is the source of truth for what changed.

• Bucket every path: migrations and schema, models, routes and controllers and serializers, services and jobs and mailers, config and dependencies, backend tests, frontend source, frontend types and API client, frontend tests, agent and developer tooling, generated files.

• Keep the full file list. Every path must appear in the finished page. This is a hard invariant, checked in step 10.

• Ask which of those files GitHub will actually render, at link rungs 1 and 2, and hold the answer for the whole run:

  <skill base directory>/scripts/diff-render.sh BASE HEAD --collapsed-only

  Every path it prints is one whose diff sits behind Load diff — a generated file, a lockfile, a binary, a diff past 400 lines or 20 KB — so a citation into one of those takes the blob form even though the line is inside the diff, because a diff anchor there lands on a stub with the cited line nowhere in the page. Nothing about the page says which files these were; the only thing that changes is the href, and references/report-format.md § When the diff will not render owns the rule, the asymmetry and what the excerpt beside such a claim has to be. Read its trailing summary too: past 300 files or 1 MB of diff, GitHub withholds files that are individually small, and that is a limit to state in prose rather than to guess at per link. At rungs 3 and 4 there are no hrefs, so skip this.

• Notice a uniform transform before you start opening its instances. A rename, a codemod, a regenerated lockfile or a vendored bump shows itself in the cheap signals you already have — a run of R status letters, eighty files each moving three lines with the same add-to-delete ratio in git diff --numstat, the paths --collapsed-only just printed. That bulk is one judgment — is the transform uniform, and what did it miss? — plus an inventory, so open the exceptions rather than the instances: the files where the transform did something different, and the places that should have been transformed and were not. Reading the eighty spends the run's scarcest resource on the part of the diff carrying the least to decide.

• If the whole diff is trivial (a few files, no migration, no new behaviour), say so and offer to stop rather than generate ceremony. A page nobody needs is worse than no page. Non-interactively (--output), make the page. There is nobody to offer to, and the offer has already been answered upstream: ci/application-code.sh measures exactly this and skips the job before the skill is reached, so a run that got here is one that gate judged worth a map. Say the diff is slight in What changed, as a stated limit, and carry on.

4. Derive what changed

Before any layer, establish what the change is for. The reviewer cannot judge a mechanism without knowing the behaviour it is meant to produce.

Answer, concretely: what problem does this solve; what were the user or system use cases; what was possible before; what becomes possible or changes after; which actors are involved; what are the primary execution paths.

Rank your sources by how much they can be trusted:

Write each behaviour down for yourself as an actor plus a behaviour plus a path, not as a feature name. This is the notation step 6 clusters on and step 7 builds chains from:

A workspace admin archives a project — new time entries are prevented, historical ones preserved.
ProjectSettings → PATCH /api/projects/:id → ProjectsController#update
  → Projects::Archive → Project → projects.archived_at

That is analysis, and it is not the page. What changed gets one paragraph and at most a handful of bullets — references/report-format.md § Section 1 — and the paths feed step 6's clustering and step 7's chains rather than being published as a list. A page that opened with five use cases and their execution paths spent its first screen on material the checkpoints then said again.

Say which parts you inferred. Where the intended behaviour is not pinned by a test or spelled out in code, name the gap instead of smoothing it over: "It is unclear whether existing time entries stay editable after archival. No test covers it." An honest gap is more useful to a reviewer than a confident guess, and it is the kind of thing they can resolve in one question to the author.

5. Trace the flows and find affected-but-unchanged code

This is the step that makes the page worth generating. A diff already shows changed lines well. What no diff shows is the unchanged code those lines just changed the meaning of — and that is where the expensive bugs are. Spend more turns here than on prose.

Work outward from each changed thing to its consumers:

The lens file the stack selected in step 2 — references/rails-nextjs.md or references/phoenix-liveview.md — carries the concrete search patterns per artifact kind. Use them; do not improvise a grep and call the area clear.

Record what you searched, not just what you found — but the finding goes in the open prose and the search goes in the collapsed record. "Nothing else calls Project#archive" is the finding, and it is a sentence a reader meets without opening anything; rg 'archive[!?]?\b' app lib is how you know, and it belongs in details.searched, shut, inside the evidence foot. Unrecorded entirely, absence and omission look identical and the reviewer redoes the work; left only inside the toggle, the finding is hidden rather than disclosed. One row per search: the command, then what it returned in a clause — the form and the budget are references/report-format.md § What was searched, which owns them.

Then draw the primary flow end to end, from user action to persistence and back, and list the secondary effects hanging off it. That flow is the run's backbone: step 6 splits it into flows for analysis, Impact outside the diff shows the crossings whole, and the checkpoints are the judgments it turns on. Nothing publishes after this step: the crossings found here reach the page at milestone 3, after 7c has had its chance to merge two of them into one judgment and after there is a checkpoint for each card to point at (step 9).

6. Cluster into flows, write the analysis notes, spawn the falsifiers

Group by behaviour, never by directory. Services / Models / Hooks / Components is the repository's structure, not the change's, and a reviewer who reads it still has to assemble the behaviour themselves. Group into vertical slices instead — one per use case, each cutting through controller, service, model, client and tests:

Flow A — Archiving a project
Flow B — Preventing time entries against archived projects
Flow C — Showing archived projects in historical reports

Say why you split it that way, to yourself. The split is the insight, and it is what keeps a checkpoint from turning out to be a directory.

The flow is the unit of ANALYSIS and no longer a unit of the page. Nothing below writes a flow section: a flow becomes a note, the note is what gets attacked, and step 7 turns notes into the ranked handful of judgments the reader actually gets.

And no sentence on the page ever names one. These letters are yours, for the notes in $W/analysis/ and for your own ranking; the page's designators are Checkpoint A and Impact path A, which are the reader's. A checkpoint reading "a leader-opened thread (Flow A) has no connection_id" has pointed at a section nobody wrote. It is easy to write and hard to see, because the note you are holding, the checkpoint you are writing and the impact path beside it are all lettered — references/report-format.md § One canonical home owns the rule.

Then label each flow and each leftover file, for your own ranking in step 7 — the label reaches the page only as what What changed leads with and what the agenda leaves off:

• Primary — directly implements the stated use cases.
• Supporting — refactors or infrastructure the primary behaviour needed.
• Secondary — independently reviewable, outside the primary mental model.

Stay neutral about secondary work. The point is to tell the reviewer which changes they can hold separately, not to criticize the author for bundling. "Appears unrelated to archival; no correctness concern identified; review independently" is the whole register.

6b. Write one analysis note per flow

Before anything is synthesised, write $W/analysis/flow-<x>.md for each flow — plain markdown, no page markup. It is the record the falsifier attacks and the record step 7 ranks from, so it is written for both readers at once: complete, cited, and dull.

Each note carries, under these headings:

• Behaviour — the actor, the behaviour and the path, from step 4.
• Trace — every hop with a file:line, in execution order, and the hunk each changed hop turns on.
• Affected, not changed — one entry per finding: path:line, the clause on why the change reaches it, and its evidence tier written out (diff, from unchanged code, inferred from tests, inferred, uncertain). Write the tier on every entry here, the diff ones included. On the page silence means the first tier; in a note silence is a claim nobody labelled, and the falsifier is told to treat it as one.
• Searched — one line per search, the command then the result clause, in the form details.searched will take.
• Tests — which behaviour each pins, which branch it leaves open.
• Open — what you could not establish, and what would settle it.

Why a file and not a slice of the page. The falsifier used to be handed a published <section id="flow-x">, which meant a flow was wrong in public for as long as a challenge took to arrive, and meant the agent read markup, excerpts and a rail to find the claims. A note has neither cost. It also survives the run: on a diff that strains the context, $W/analysis/ is what says which region got a trace and which got skimmed.

6c. At --effort high: send the falsifiers at the notes

Everything so far is this run checking its own work. That is the weakest kind of check — the context that wrote a claim is the one least able to see what it assumed. At --effort high the notes get a second reader whose only job is to break them.

When. Here, as soon as the notes are written, and before any of it reaches the page. That is the whole reason this moved: the pass used to run after the flows were published, so a corrected claim had been public for as long as the challenge took to arrive.

How. One independent reader per note, launched the way your host reference says — Claude spawns the registered accountable-review:claim-falsifier agent, Codex spawns a session subagent — up to the host’s available concurrency. This is the one exception to the rule against subagents in § Hard rules.

Then keep working — do not wait on them. They come back as notifications, not as a blocked turn, and the parent's job in the meantime is step 7. Measured on a real run: five falsifiers launched over 23 seconds, each returning a launch receipt in about 2 seconds, with the first challenges arriving 365 seconds later while the parent drafted. Blocked time was 0.9% of the run. A run that spawns them and then idles has converted the cheapest thing in this procedure into the most expensive.

Cheap in wall clock is not cheap in tokens, and the cap is what holds the second one down. Each falsifier reads in its own context, and across three real runs the pass came to 17-23% of every cache-read token the run spent over 145-216 requests — the most expensive thing in the procedure after step 5, and invisible in the parent's transcript. That is the cap's real job: six agents is the point past which a second reader stops paying for itself.

Launch the selected readers without waiting between launches, up to the host’s concurrency limit. Queue any remaining notes and continue drafting. A run that reached for a single blocking Explore agent paid 997 seconds, 41% of its wall clock, for one sequential spawn.

Give each agent four things and no more: the absolute path of references/claim-falsifier.md, which it is asked to read first; the repository path; BASE and HEAD; and the path of that one note. Not the whole set — a falsifier holding every note is one long blocked turn again, and it has no way to tell which claims are its to attack.

Cap it at six. Past that, take the notes carrying the most affected, not changed entries and the most inferred and uncertain tiers. Those are the claims a reader cannot check cheaply, which is the only reason to spend an agent on them.

7. Synthesise the agenda

The notes are the analysis. This step turns them into the page's argument, and it is a step of its own because the failure it prevents is structural: a run that goes from notes straight to prose writes one section per flow, which is the page this one replaced.

Nothing here opens a file the notes have not already cited. The work is deciding, not tracing. Do it in this order, and write the outcome of each part into $W/analysis/agenda.md before moving on — the falsifiers' challenges are arriving while you do this, and a written agenda is what lets step 8 fold one in without re-deriving the ranking.

7a. Name the semantic delta. One sentence: what is true of the system after this change that was not true before, in behavioural terms. Not what files moved — what a user, an operator or a downstream caller now experiences differently. If the notes support two unrelated sentences, the PR bundles two changes and What changed says so, neutrally. This sentence is the spine of section 01, and almost every checkpoint has to be a judgment about it.

Almost, because there are two kinds of checkpoint and only the first answers to this sentence. A behavioural checkpoint is a judgment about the delta — what the system now does. A coding-decision checkpoint is a judgment about how the change was built: where a class was put, what kind of object it is, which existing abstraction it went around. That second kind is capped at one and ranked last, and 7b to 7e say why and how.

7b. Identify the human judgments. For each note, ask what a competent reviewer has to decide rather than learn: whether a scope is still the one that was intended, whether a nil default is safe for every consumer, whether a guard's new order admits a class it used to refuse, whether a migration is safe to run against the rows that already exist. Write each as a question.

A judgment is something the reviewer could get wrong, with consequences. A fact is something they read once. Facts feed the explanations; only judgments become checkpoints.

Then ask the coding-decision question once, and it has a bar in front of it: name the departure. The form is the PR chose X; this codebase already does Y for the same job; is X deliberate? — a value object under app/models where app/services/ already holds four of its kind, a query built in a controller where app/queries/ exists, a hand-rolled guard where a policy class was waiting, a *Manager among a dozen *Services. Step 2 collected the conventions; this is where they are spent.

The bar is a citation, in this repository. The question exists only if you can point at the existing answer it departs from — a sibling file, a populated directory, a line in a convention doc — and the citation goes on the page beside the question. No in-repo counter-example, no question. That is what separates this from a style guide: a reviewer who knows the codebase can ask it, a linter cannot, and the difference is the sibling you had to go and find. The lens file's search recipes are how; report-format.md § Coding decisions owns the rest of the rule.

Ask it; never answer it. No should have been, no unidiomatic, no the Rails way. The reviewer knows why the codebase is shaped as it is and you do not — step 6's neutrality rule is the register, and a decision you cannot resolve goes on the Open question line, not into a verdict.

Some judgments hide outside any one flow, and a flow-by-flow reading is exactly what misses them. references/report-format.md § The review checkpoint carries the list to ask against — migration safety, an application invariant with no database counterpart, the authorization model, deploy ordering, test infrastructure, agentic tooling, jobs and flags and environment variables and transaction boundaries, and the one coding decision that departs from something this repository already does. Each is a checkpoint only if it is a judgment for this diff; the list is a prompt, not a form.

7c. Merge related observations. A constructor change, the nil default it introduces and the two consumers that do not handle nil are one judgment — is nil safe here? — not four observations. Merge until each candidate is a single question whose answer settles everything under it.

This is where the agenda gets short honestly. Three observations that share a question are one checkpoint, and a run that lists them separately has written the same judgment three times.

7d. Rank. Order the candidates by four things, in this priority: the consequence if the reviewer misunderstands it; how uncertain the notes are about it, because the reviewer's attention is worth most where the page's own evidence is weakest; how far it sits from the obvious reading of the diff, since a consequence visible in the hunk needs less help than one three files away; and how important the affected unchanged code is.

A coding-decision checkpoint is ranked last, whatever those four say about it. Not because it does not matter — a class in the wrong place outlives the PR — but because misreading it costs the reviewer a conversation and misreading a behavioural one costs them production. The reader meets the consequences first and the shape afterwards.

The order is the order to think about them, and it is not a scale. No number that reads as severity, no high or low, no blocking, no watch. The rail and the reading path refer to a checkpoint by its question.

7e. Select the smallest useful agenda. Three to five checkpoints per independent semantic delta 7a named, and seven on the page at the outside. Fewer for a PR small enough that three would be padding — a run that reached this step on a diff step 3 nearly stopped for may have one. Ask: if the reviewer understood and investigated these, would they have the right mental model of this change? If yes, stop.

The count follows the delta, never the file count. One delta is three to five whatever the diff weighs: eighty files of one rename is one judgment, and one behaviour reaching across four layers is also one — its reach becomes Look at entries, a chain and a card in section 04, not more checkpoints. Two delta bullets in § 01 is what earns a sixth. Since those bullets are on the page, the reader has already been told why the agenda is longer.

A sixth candidate under one delta is a signal to merge again, not to add a sixth; if it will not merge, the checkpoint nearest to it names it in one clause. Past seven on any PR, stop adding: write 7a's bundling sentence in What changed and, if you skimmed, the statement of which region. An eighth checkpoint is a list the reviewer will triage instead of an agenda they can hold.

Two conditions on going past the fifth, and check both before you do:

• Every checkpoint is routable from section 03 — its own stop, or a stop whose span.why names it. If routing them all would consume the whole reading order and leave no room for conceptual sequence, the agenda is wider than the page carries. report-format.md § Section 3 owns the rule.
• It rests on a note you traced, not one you glanced at. $W/analysis/ records which is which. A checkpoint built on a glance is the page's weakest claim sitting under a heading that promises the exact lines that settle it — and on a strained run the glanced notes are exactly the ones a wide agenda reaches for.

A short agenda on a large diff is two different things, and you have to say which. Two checkpoints over sixty files is either a merge that went well or a trace that stopped early. Decide it here: if 7c genuinely collapsed the diff to two questions, that is the right page and nothing is owed. If you ran out of room, the region you skimmed is stated in What changed under § How big should the page be?, which is the rule that already covers it.

A finding that does not become a checkpoint is not lost — affected code keeps its entry in Impact outside the diff or in the evidence foot, which is what makes leaving it off the agenda honest rather than hidden. A judgment is the exception: an entry in the foot the reviewer has to decide about is a checkpoint that was mis-filed, per the hard rule below, which is why the agenda has room to grow rather than a ceiling that would push one down there. What is never done is dropping a checkpoint to hit a word count: § The agenda budget is guidance on how a checkpoint is written, never on how many there are.

The coding decision gets at most one slot, and it takes the last one or none. Three caps, and the middle one is the product:

• One per page — not one per delta. This is the one count above that does not scale with the number of independent changes: a PR shipping three deltas still asks at most one question about how the code was built. A second is the page becoming a style review, and it will read as thoroughness. The departure that does not get the slot is not written down anywhere — an observation about shape with nowhere to go is noise, and the page is not obliged to report everything it noticed.
• It never displaces a behavioural judgment, and it never justifies going past seven. If the behavioural agenda already fills the page, there is no slot, and that is the correct outcome rather than a loss. A page that dropped is nil safe for every consumer? to ask where a class lives has traded the product for a preference — and a page that reached an eighth checkpoint to fit one in has bought a preference with the reviewer's attention.
• Unless the diff is a refactor, in which case the coding decision may be the only judgment there is, and then it is the page. Say that plainly in What changed rather than manufacturing a behavioural checkpoint to sit in front of it.

7f. Choose each checkpoint's representation. Every checkpoint has a question and an explanation of two to four sentences. Then decide whether it also earns a chain, and how many Look at entries it needs. Each entry is three things in order — a title, a clause, then the citation. The title names what is at that location in two to six words, so four entries can be scanned rather than read; the file name is not a title, because the citation says that already. report-format.md § The review checkpoint owns the rest.

The chain is earned when the judgment turns on a mechanism the reader cannot hold from prose — a guard order, a value derived across three or more hops, a request path with a branch in it. Ask it affirmatively rather than looking for an excuse: if the explanation would have to name three hops in sequence, draw them. It shows that mechanism inside the change, so if the chain you want to draw crosses into unchanged code, it is an impact path: it belongs in section 04, drawn once, and the checkpoint says so in a clause.

Diagram and prose have different jobs. The chain says how the value gets there; the explanation says what the reader has to judge about it. An explanation that walks the chain node by node is the defect — it doubles the length and teaches nothing twice, and it is what a run reaches for when it is unsure the figure landed.

7g. Select the evidence. For each checkpoint, take only what is needed to investigate it:

• Which citations are load-bearing for the judgment. Those get a collapsed excerpt beside the Look at entry they confirm, generated in step 9.

• Which claims carry an evidence tier.

• Whether a probe would settle the question and would answer where the reviewer runs it — a pre.probe, proposed and never run, inside the checkpoint after its explanation, at most one per checkpoint. There is no separate validations section to send it to. Class-level reflection answers in any checkout; a probe that reads rows answers only where those rows exist, so its setup is a line of the same block, and a probe against behaviour this change introduces has no rows to read and asks the classes instead. Label it in three parts — the instrument, the safety, and what the output would settle: a probe whose label you cannot complete is a probe that was not earned. § Framework anchors owns all of it.

• Whether a doc link explains why the framework consequence follows. At most one a.doc per checkpoint, and never on a claim with no file:line beside it.

• At --mentor only: whether that link should be a primer instead. Ask it of the judgment, not of the flag. Without the flag the test is could the reviewer decide this without knowing the framework's rule?; with it, would they decide it better knowing the rule than knowing where to read it? That is a lower bar and it is still a bar — a behaviour every developer in the stack already knows earns nothing at either setting. One per checkpoint, three per page, and the primer holds that checkpoint's one doc link rather than adding a second.

  A run that earns none writes none. Three primers is a ceiling, never a quota, and the flag is not an instruction to find lessons. Manufacturing one is how the page becomes a framework manual with a diff attached, and it arrives looking generous. A ‡ probe row is never a primer's subject: those behaviours changed inside the supported range, so no paragraph about one is true of every app. references/report-format.md § Mentor mode owns the rest.

The catalogue opens here — references/rails-docs.md or references/elixir-docs.md, whichever step 2 named, and not before. It is a lookup table: you go to it with a concept a claim already needs, never read it to find concepts. Read its § Version first; a closed catalogue returns no link for every concept, and a page with no doc link is narrower rather than wrong.

Tests and author questions are facets, not sections. A test appears inside a checkpoint when it changes how the reviewer judges it — "the spec at :88 pins the admin branch and leaves the invitee branch open" — and a list of the specs that touch the file does not appear at all. An author question goes on the checkpoint's p.open line.

7h. Build the reading path. Three to seven stops, in the order that builds understanding — schema before the code that trusts it, the smallest complete example before the bulk, irreversible code last. Each stop is a path, one sentence on why here, and a link to the checkpoint it belongs to. It is built after the agenda because it is a route through the agenda. Ask, per stop: what does the reviewer have to understand first for the next file to make sense?

7i. Identify the external impact. One to three paths from the notes' affected, not changed entries: each starts in changed code, passes through at least one unchanged consumer, and ends at something a user or an operator would see. Choosing the third-best over the fourth is the work; the fourth keeps its entry below the figure or in the foot. If nothing crosses into unchanged code, section 04 is omitted and the rail loses its entry — an empty section saying nothing reaches unchanged code reads as a clean bill of health.

For each path you keep, settle two things here rather than while typing markup. What the path is about — one or two sentences on what goes wrong and to whom, plus which checkpoint judges it; that is the card's p.ip-why, and deciding it now is what stops it becoming a retelling of the hops. And where each node's code is: every changed and every affected node needs a path:line for its locator, and a node you cannot address is a node you have not actually traced.

7j. Deduplicate. What changed, a checkpoint's explanation, a chain, the reading path and the impact section can all mention the same scope. Each occurrence must serve a different purpose — intent, judgment, mechanism, route, consequence — and be one clause wherever it is not the canonical home. Read references/report-format.md § One canonical home for the routing table, then read your agenda once looking only for the second time anything is explained.

Two rules keep this from becoming ceremony:

• A checkpoint is a judgment, never a category. Controller changes is a directory; Does the new filter preserve the intended scope? is a question a reviewer can get wrong. If a candidate cannot be phrased as a question the reviewer answers by looking at the code, it is an explanation and belongs in a sentence somewhere else.
• Validation steps must be real. The actual rake or mix task, the actual route, the actual factory in this repo — a command a reviewer can paste. Invented steps are worse than none, because they burn the reader's trust in the whole page on the first paste that fails.

Milestone 2 opens after this step (step 9), with one pending stub per checkpoint. Milestone 3 follows it once 7i's cards have checkpoints to point at.

8. Verify before asserting — at every publish boundary

This is not a phase that happens once, before writing. The page ships in stages (step 9), s

Truncated - read the full file at https://github.com/wyeworks/accountable-review/blob/3f5753d6b1dfcd5c59b3b4bc78ce1834d4e0daca/skills/review-map/SKILL.md.