/refactor: reviewers should receive the diff by path — inlining it routes a byte-exact artifact through the model's output channel

[Danglebary]

Summary

/refactor step 4 requires every reviewer brief to carry the diff inlined as content, and forbids handing a reviewer the .pirr/diff.patch path:

• The diff itself, inlined as content — not .pirr/diff.patch's path. Reviewers hold no path to it by contract: exactly two agent bodies name that artifact and DIFF-MATERIALIZATION.md's two-agent contract is published, so handing a reviewer the path both widens that contract and hands a Read-only agent a file it was never promised.

— skills/engineering/refactor/SKILL.md:68, reinforced as an anti-pattern at :164.

At real diff sizes this is unexecutable, and the way it fails is silent.

Where it bites

A single-ticket diff of ~129 KB (2532 lines, 8 files) against a Reviewers manifest of 8 lenses. The orchestrator must emit ~43k tokens of diff per lens — ~344k tokens of generated output, and more than a single message's output budget can carry even once.

The workarounds available to the orchestrator are all defects:

1. Abbreviate hunks to fit. This is what happens in practice, and it is invisible downstream — the reviewer returns confident findings against a diff that differs from the real one. Observed directly: on a first dispatch the orchestrator silently collapsed two doc comments to ... and compressed several context hunks.
2. Reduce the diff per lens. Produces reviewers reading different material, which makes the convergence count /refactor step 5 asks for uninterpretable — the skill's own ranking input is destroyed by the accommodation.
3. Hand-transcribe faithfully across many messages. No mechanism verifies the transcription, so it trades a visible departure for an invisible one.

The underlying defect

The diff is a relay, not a composition. Routing a byte-exact artifact through the model's output channel asks the one component in the system that cannot guarantee fidelity to act as a copy mechanism.

The rest of the brief — ticket contract, Glossary vocabulary, settled-ADR list, deferrals, the planning-artifact hunk label — is genuine synthesis, and inlining is correct for all of it, because the orchestrator is the only thing that can produce it.

The seam: inline what you composed; reference what you copied.

The stated rationale is self-referential

The justification is a count: "exactly two agent bodies name that artifact." But the two-agent contract in skills/_shared/DIFF-MATERIALIZATION.md:68 says two because only two were written — and then that tally becomes the reason a third consumer may not join. It is a constraint that rots on first growth.

This is the same failure mode ADR 0100's Population rule already fixes for briefs: membership is declared, never inferred from a tally.

The plugin already contradicts itself

Three diff-consuming dispatch sites, two rules:

Site	What reviewers get
skills/engineering/done/SKILL.md:58	"The diff artifact path (.pirr/diff.patch)"
skills/engineering/retro/SKILL.md:51	"The diff artifact path (.pirr/diff.patch)"
skills/engineering/refactor/SKILL.md:68	path forbidden, inlining mandated

/done states the reason plainly at :28 — "the close-out agents have no git access; this artifact is their only view of the diff." That reasoning applies verbatim to reviewer agents, which hold the same Read/Grep/Glob toolset and the same absence of git access.

A downstream fork of this plugin already resolved it the other way: its DIFF-MATERIALIZATION.md names "the diff-operating reviewer agents parse it" as a first-class consumer of the artifact, and its /refactor hands reviewers "the diff (or scope)" with no inline mandate. Nothing broke.

This is not fixed by keeping diffs small

Worth stating because it is the tempting read. The wall is O(1) in the design rather than O(n) in the repo:

• Ticket diffs are already scoped — the size comes from the files one ticket touched, not from repo size.
• At 8 lenses, even a 30 KB diff costs ~240 KB of generated output and still carries transcription risk.
• Shrinking diffs lowers how often the wall is hit. It does not remove it.

Proposed change

1. skills/_shared/DIFF-MATERIALIZATION.md ("The artifact") — replace the enumerated two-agent contract with a named consumer population: the close-out pair plus every reviewer /refactor dispatches. Keep "never move or rename it" and the ephemeral-scratch status unchanged.

2. skills/engineering/refactor/SKILL.md step 4 — reviewers receive the artifact path; the composed planning context stays inlined. The ad-hoc files-or-tree arm keeps inlining its scope description, which is small by construction.

3. skills/engineering/refactor/SKILL.md anti-patterns — invert the bullet into the general rule it was reaching for: don't route through the model an artifact the model would have to reproduce. The genuine prohibition — never hand-roll a git diff — already lives at step 3 and is untouched.

4. (Optional, and the one that makes it a contract rather than a convention) — give the shipped reviewer bodies the artifact line the two close-out bodies already carry, so the consumer states its own input.

Note on what does not change

The anti-substitution and pinned-subagent_type rules are unaffected, as is the exit-code discipline at step 3. This changes how the diff reaches a reviewer, not which reviewers run or how their absence is recorded.

Version

Observed on pirr 1.4.0.

---

🤖 Filed with Claude Code

[Danglebary]

Adjacent overhaul: the other half of the same seam

The relay diagnosis holds, and inline what you composed; reference what you copied is the right cut. This comment is about the half that stays inlined — the material the orchestrator genuinely composes — plus what happens to the returns after the lenses answer.

Filing it here rather than separately for one practical reason: proposal 4 opens all thirteen shipped reviewer bodies. Several changes below want that same sweep, and splitting them means touching thirteen files twice for one contract.

Everything below is argued from the skill's own text. Where a change contradicts a settled ADR, I say so rather than routing around it.

1. A finding carries no checkable object

The candidate contract in the reviewer bodies is Lens, Problem, Direction, ADR conflicts — plus Dependency category on the architecture lens. The two enumerated fields classify; the two carrying the actual claim are free prose.

That is what makes step 5's merge-layer duty — "adversarially verify reviewer factual claims against source before relaying them" — cost what it costs. There is no named field to check, so verifying means re-reading the cited range and reconstructing what the reviewer meant. Cost scales with findings, and the check is the last thing standing between a false premise and the user.

It also puts the orchestrator in the position of grading prose. A well-written Problem and a correct one are different properties, and a merge layer reading paragraphs is partly ranking the former.

The fix is to make the checkable thing a named field the finding cannot omit, and to have the framework name the field. The deepening vocabulary already implies most of the set:

Witness	Fields	What it makes checkable
caller-census	symbol, callerCount, anchors[]	the deletion test — "complexity reappears across N callers" becomes a number rather than an impression
adapter-count	seam, adapters[]	one adapter = hypothetical, two = real — currently asserted every time and counted none
test-reach	behavior, testFile, reachesThrough	the interface is the test surface
trace	input, expected, actual	a concrete input through the cited lines, never a class of inputs
quoted-authority	rule, source, precedenceCheck	a rule quoted from CLAUDE.md, an ADR, or the Glossary — with the precedence check, so a slice-omitted override cannot produce a confidently wrong candidate

A candidate carrying none of these is inadmissible.

Note what this replaces. The bodies currently suppress noise with prose — "don't pad", "don't list every shallow-looking module", "apply the deletion test first", "don't flag style or naming". Those are pleas competing with the rest of a system prompt. A required field is a shape, and the shape does the work: a reviewer that cannot fill callerCount has not run the deletion test, and now that is visible instead of inferred.

This is also the change that makes the deletion test honest. It is the framework's central instrument and it is currently a guess.

2. Verification runs in the wrong agent, after the wrong step

Step 5 has the merge layer verify, dedupe, count convergence, and then present. The same agent therefore knows which lens said what, has already read every Problem paragraph, and is the one that will argue the surviving candidates to the user. That is three jobs with different biases in one context.

Split it. After the returns land, the orchestrator dedupes and hands a stripped record — claim, witness, anchor, nothing naming the lens or its area — to a separate pass that proposed none of them. LLM evaluators show measurable preference for output they recognize, and an instruction not to look is the wrong shape of fix; removing the field to look at is the right one.

Four verdicts, each with a destination that already exists or should:

• admit — a deepening candidate, into step 6's numbered list
• demote — real but below the seam, into the minor-cleanups group step 6 already separates
• lead — plausible, witness could not be checked
• reject — affirmatively refuted, with one line saying why

Two of those are new, and both are holes today. A candidate whose witness nobody could confirm is currently either presented as real or dropped in silence. A refuted candidate leaves no trace at all, so the same one returns next pass — which is the exact evaporation rationale step 7 already gives for banking deferrals.

One set-level rule makes the pass auditable: every candidate is ruled exactly once. A candidate nobody ruled on has vanished between dispatch and presentation, and that is the one way a finding is lost with no section admitting it. This is the Dispatch record's own completeness property, one layer down.

An admitted trace witness should carry the ruling pass's own re-derivation from the cited lines. A field that cannot be left empty is what makes that a step rather than a claim.

3. The cap sits in the prompt, where it manufactures findings

Every reviewer body ends with "if you find more than ~5 candidates, surface only the highest-leverage ones." The anti-patterns then say "don't propose more than 3-5 candidates per ticket."

Two problems, one of them backwards. An agent told how many slots it has tends to fill them — a cap stated to a producer reads as a target. And the narrowing is being done by the lens, which is the one participant that cannot compare its candidate against the other twelve lenses' candidates.

Move it. Reviewer bodies keep a runaway guard only — a bound high enough that no honest return touches it. The orchestrator seats the final set after ruling, where the whole field is visible. This is a deletion from thirteen bodies and one addition to step 6, and it fits the sweep proposal 4 already opens.

4. Convergence is carrying more weight than it can hold

Step 5 makes convergence a ranking input and asks for it explicitly — "4 of 5 reviewers flagged this."

This issue already found one way that breaks: reducing the diff per lens makes the count uninterpretable. There is a second, present even when every lens reads identical material. The lenses are topical, not disjoint, and they all read the whole diff — so three lenses landing on one function may be reporting that the function is long, not three independent confirmations. Correlated observers agreeing is weaker evidence than the number suggests.

Keep the signal, weaken the claim: dedupe before stripping provenance and carry convergence: <n> as an integer on the merged candidate. A count tells the ruling pass how many lenses landed there without telling it which — recognition keys on identity and style, not on a number, so the blinding survives.

This narrows step 5's convergence rule as written. Worth naming explicitly in whatever change lands.

5. Thirteen lenses independently re-derive the same repo facts

Every reviewer holds Read, Grep, Glob and gets the whole diff. Any candidate about reach — callers, adapters, test coverage of a seam — requires a search. With eight lenses on the manifest, that is up to eight independent sweeps for the same symbols, each unreliable on its own: EVIDENCE-PRINCIPLE.md's Empty read corollary asks every one of them to run a positive control before treating an empty result as absence, and a lens spending its budget on grep hygiene is not spending it on its lens.

Hoist the sweep. Before dispatch, the orchestrator runs it once and composes a census: per module the diff touches, its callers with anchors, its adapter count, its test references, and the positive control that proves the search resolved. Every brief carries the same table.

Three things this buys, in descending order of value. The deletion test and the seam test get a shared number instead of thirteen guesses. The Empty-read control runs once, correctly, in the layer that can attest to it. And the per-lens search cost collapses.

This is composed content, so it inlines — the seam this issue draws puts it on the correct side.

6. Nothing is confirmed before the lenses spend

The pass reads the manifest, materializes, and dispatches. The user's first look at anything is step 6, after every lens has answered.

By then the orchestrator has assembled a brief out of the ticket's Goal, Acceptance criteria and Deviations, the Glossary, the settled-ADR set, the spec's open-ticket list, and prior deferrals — plus the diff. A misread in any of them is inherited by every lens simultaneously, and the failure is silent in exactly the way this issue describes for abbreviated hunks.

Add a blocking restatement before dispatch: render what the pass believes it is reviewing — the resolved range and hunk inventory, the extracted contract, the seams touched, the census, the ADR decisions being marked do-not-relitigate, the deferrals — and stop. The user confirms or corrects, and corrections fold into the briefs before anything is dispatched.

Two notes on fit. The shape is established rather than new — the pass already gates at step 3's post-merge arm ("a confirm gate, not a notification") and twice more in the close-out — so this is another instance of it, though where a pre-dispatch gate sits relative to ADR 0004's doctrine is worth settling rather than assuming.

The second note is the payoff. This is the honest route to selective dispatch: if the gate also renders which lenses will run and which will not, a skipped lens is confirmed by the user rather than silently filtered by the orchestrator. That matters because ADR 0006 splits coverage claims by who can attest to them, and "don't run reviewer dispatch silently if the Reviewers manifest lists agents that don't exist" is the same instinct pointed at a different failure. Routing without a gate would need a fifth Dispatch-record state to stay honest — returned, degraded, refused, unresolved, and now not-applicable. Routing behind a gate needs only the record to state what the user already approved.

7. References are unconstrained, so a wrong one renders as a plausible one

The brief hands over Glossary terms, ADR decisions, open tickets, deferrals, and hunks as prose. A candidate cites them however it likes, which means a candidate can name a module that does not exist, or re-litigate a settled ADR by misciting it, and nothing catches either.

Number them in the brief — hunks, glossary terms, ADR decisions, open tickets, deferrals, census rows — and require every candidate to carry a Reference naming one. The orchestrator holds the list, so a reference that does not resolve is caught by lookup, with no source re-read. That converts the cheap half of step 5's verification duty into a mechanical check and leaves the expensive half — the witness — to the ruling pass.

It has a second use: a numbered restatement is a correctable restatement. The user can rewrite an identifier at the gate in item 6 instead of describing which part is wrong in prose.

Relation to this issue, and sequencing

Independent of the relay fix. None of it changes how the diff reaches a reviewer, which lenses run, or the exit-code discipline at step 3.

Two land naturally inside proposal 4's sweep, since it opens every reviewer body anyway:

1. Witness kinds on the candidate contract (item 1), with the shared text homed in one doc and copied into the bodies as a declared sync-set — the shape EVIDENCE-PRINCIPLE.md already uses for the Partial verdict register.
2. Cap removal from the bodies (item 3).

The rest is orchestrator-side and can follow: the ruling pass and its four verdicts (items 2 and 4), the census (item 5), the gate and the identifier space (items 6 and 7).

Worth stating plainly: items 2, 4 and 6 contradict the skill as written — step 5's convergence rule, step 5's in-line verification, and the absence of any gate before dispatch. They are proposals to change those, not readings of them.

---

🤖 Analysis with Claude Code