deep-review: output was designed for a terminal reader who wrote the code — it fails as a PR comment. Rethink delivery (and read the story)

This is a design discussion, not a spec

Nothing here is decided. The goal of this issue is to agree on what deep-review is for now, pick a delivery model, and then split the work into implementation issues. Please argue with the framing before arguing with the ideas.

Progress across the whole effort is tracked in #223.

---

1. What it was built for

/deep-review was written for one channel and one reader:

• Channel: a terminal. Output scrolls; the reader's eye lands at the bottom, and they scroll up for detail.
• Reader: the person who wrote the code, reviewing their own branch before pushing. They have the diff in their head, they can act on all 30 findings immediately, and nobody is waiting on them.

Every design choice follows from that, and they are all deliberate:

• commands/deep-review.md:14 — "Do not self-limit, do not summarize… Length is not a concern; thoroughness is."
• commands/deep-review.md:22 — "There is no length limit on the Findings section. A review with 30 findings is better than a review with 10 findings that misses 20."
• commands/deep-review.md:275 — ⚡ Bottom Line is the last section, which in a terminal is the first thing you read.

For self-review before a push, that design is right and should not change.

2. What it's actually being used for now

Two workflows it was never designed for:

1. Posting the review onto someone else's PR. The reader is now the author, not the reviewer. They read top-down, they didn't write the review's mental model, and they receive 30 findings as a wall of text authored by a machine.
2. Sweeping many PRs at once (4–10 in a batch). The reviewer wants a triage table — PR, current state, suggested vote, what's blocking, mergeable-now yes/no — and drills into detail only for the ones that are rejected or need changes. There is a working ad-hoc paradigm for this, but it is not codified anywhere in this repo. The README documents parallel worktree agents (one per branch), and each agent returns its own full five-section review with no aggregation layer.

3. Why the current output fails in those channels

• The synthesis is at the bottom. ⚖️ Verdict is technically first, but the actionable synthesis — finding counts and "the single most important thing" — is ⚡ Bottom Line, at the end, after an unbounded flat findings list. On a PR page you scroll top-down, so the reader hits the wall before the point.
• Exhaustiveness is a feature that becomes an insult in this channel. 30 findings including style, naming, and logging nits is a great self-review and a demoralizing PR comment. The volume also buries the 2 findings that actually block merge.
• Everything is graded for one audience. 💡 suggestions and 🔴 blockers arrive in the same undifferentiated list, so the author has to do the triage the reviewer should have done.
• It's one comment, not a review. GitHub already has a mechanism for distributing volume — inline comments anchored to changed lines, plus a short review summary. We're not using it.
• Nothing is machine-addressable. When the author's agent picks the review back up, it re-parses prose that was formatted for a human.

Key reframe: the fix is probably not "make the review shorter." A short review is a worse review. The fix is to stop conflating the review artifact (complete, exhaustive, unchanged) with the delivery (compact, audience-specific, per-channel). One analysis, several renderings.

Three distinct audiences, currently served by one blob:

Audience	Wants	Right medium
Reviewer running the sweep	triage across N PRs, drill-down on demand	table / artifact
PR author	what blocks merge, where, why — nothing else	short PR comment + inline anchors
Agent (author's or ours, later)	full structured findings, stable identifiers	machine-readable payload

4. First principles: what are we actually optimizing?

Worth settling before picking a format, because it changes the answer.

A PR with 7 review rounds is normal, and each round has real latency: review posted → author reads it (10 minutes, or 2 days) → author pushes → back to the reviewer. A PR can take a week or more to land, and almost all of that is wall-clock waiting, not working. Under that model:

• The cost of a review isn't tokens, it's round trips. A finding that adds a round trip has to be worth a day.
• Exhaustiveness in round 1 is good — it front-loads everything into a single trip.
• Exhaustiveness in round 5 is actively harmful — a new 💡 naming suggestion on round 5 costs another day and buys a better variable name.
• So severity may need to be a function of round number, not just of the code. Proposal to debate: after round 1, only regressions and blockers can be raised; anything new and non-blocking is logged, not posted.

Open question: what is the goal? "Every defect found before merge" and "PRs land in 2 days" are different objectives and the current skill only optimizes the first. My take: the goal is merge confidence per round trip. If we agree on that, most of the ideas below sort themselves out.

5. Ideas on the table

A. Post only the bottom line

Post ⚖️ Verdict + ⚡ Bottom Line to the PR; keep everything else out of the thread.

• ✅ Trivially implementable; instantly fixes the wall-of-text.
• ❌ Throws away the work. The author can't act on "3 critical issues" without the list.
• Probably right as the shape of the top of the comment, wrong as the whole comment.

B. Move the bottom line to the top

Reorder the template for PR delivery: synthesis first, then findings.

• ✅ Cheap, obviously correct for a top-down reader.
• ❌ Doesn't reduce volume — just puts the point above the wall.
• Note the terminal case: the current tail-first ordering is correct there. So this likely means a per-channel template, not a reordering of the one template.

C. Human summary visible, full detail encoded for agents

Human sees the bottom line; the complete findings ride along in a form only an agent reads.

Mechanisms, best to worst:

1. <details> blocks — native GitHub, collapsed by default, human-expandable, fully readable via the API. This is almost certainly the right primary mechanism.
2. HTML comments (<!-- … -->) carrying a JSON payload — invisible in the rendered view, present in the raw body for any agent that fetches it. Good for structured data (finding IDs, severities, file:line, fingerprints) that shouldn't be prose. Caveats: it bloats the comment body, and it's visible to anyone who clicks edit/raw — so it must not be a place to hide anything embarrassing.
3. "Very small text" — GitHub sanitizes CSS, so arbitrary font sizing isn't available; <sub> is the only real lever and it just makes text annoying rather than hidden. Not worth pursuing.

Real question underneath: do we want a stable finding identity (a fingerprint per finding) so later runs can say "finding #4 is now fixed" instead of re-describing it? That unlocks idea F.

D. Inline anchored comments instead of one blob

🔴/🟡 findings become inline review comments on the changed lines they concern; the top-level comment shrinks to verdict + counts + the one thing that matters most.

• ✅ This is what the platform is designed for; volume distributes to where it's relevant. Threads can be resolved individually, which gives round-to-round state for free.
• ❌ More API surface; anchoring must obey the rule already in standards/CLAUDE.md (anchor on a changed line, never on the unchanged symptom site — GitHub rejects off-diff anchors, ADO silently buries them in Overview).
• ❌ 30 inline comments is still 30 notifications. Needs severity gating: inline for 🔴/🟡 only, 💡 stays in the artifact.

E. Artifacts for the multi-PR sweep

Every batch run publishes an HTML artifact that opens in the browser: overview table on page one (PR · state · suggested vote · blockers · mergeable now?), tabs per PR for the full review.

• ✅ Matches the drill-down behavior exactly — the reviewer scans the table, dives into the 2 PRs that need changes.
• ✅ The reviewer gets an aesthetically readable cockpit without spending any of the PR author's attention.
• ❌ Artifacts are for the reviewer, not the author — the author shouldn't need a claude.ai link to learn what blocks their PR. Artifact and PR comment are complementary, not alternatives.
• ❌ Batch review has no codified prompt/format today; the table shape needs to be pinned down before it can be generated consistently.

F. Delta re-review (the churn answer)

On re-review of a PR already reviewed, compare against the previously reviewed SHA and report only: prior findings fixed, prior findings still open, and new findings introduced by the new commits.

• ✅ Directly attacks the 7-round problem — round N's comment is short because it's a delta, not because we suppressed anything.
• ✅ Gives the author a visible ratchet: the list shrinks each round.
• ❌ Requires remembering the last reviewed SHA and finding identity across runs (see C). State could live in the review comment itself (the HTML-comment payload) — no external store needed.
• Pairs with the round-based severity ratchet from §4.

G. Read the story (this one is a gap, not an idea)

/deep-review never fetches the intent behind the work on its own. It accepts an issue or work-item reference as context, but only when the user explicitly passes one, and resolve-pr.sh resolves at most one reference by precedence — so /deep-review pr 4506 plus an issue URL resolves the PR and silently drops the issue (commands/deep-review.md:45). The most common invocation is exactly the one where the acceptance criteria get dropped.

Nothing auto-discovers the linked issue from the PR body (Closes #N), the branch name (branches/<id>-<slug> — we have the ID right there), or the ADO AB#<id> link. So Step 4 "Feature Fitness" is judged against the branch name and commit messages — i.e. against what the author said they did, not against what was asked for.

Consequence: the review can't answer the questions that decide a vote — does this satisfy the acceptance criteria? Is anything in scope missing? Is anything here out of scope? A review that grades style but can't grade fitness-for-purpose is optimizing the wrong thing, and this is arguably the highest-value item in this issue.

Proposal: auto-resolve the work item from (in order) an explicit argument → Closes/Fixes #N in the PR body → the numeric prefix of the branch name → AB#<id> in commits. Fetch description + acceptance criteria, and add an explicit acceptance-criteria checklist to the output — met / not met / can't tell from the diff.

6. Cross-cutting decisions to make

1. One skill or two? Does /deep-review grow channel-aware rendering, or do we keep /deep-review as the terminal/self-review tool and add a separate skill that consumes its output and delivers it to a PR? (Leaning: keep the analysis in one place, add renderers. But a separate /post-review composes better with #163 and #164.)
2. What's the canonical intermediate form? If there are multiple renderings, the analysis should produce structured findings once (severity, file, line, description, fingerprint) and render from that. Every other idea here gets easier if this exists.
3. What does the PR comment contain by default? Verdict + bottom line + blockers inline, 💡 suppressed? Or everything with 💡 collapsed in a <details>?
4. Who decides the vote? The batch table has a "suggested vote" column — is the skill ever allowed to cast it, or only ever suggest it to the human running the sweep?
5. Does exhaustiveness survive? I think yes, unchanged, in the artifact — and we gate only what gets delivered to the author. Confirm before anyone starts cutting review depth.

7. What "done" looks like for this issue

Not code. This issue is done when we have:

• An agreed statement of the goal (§4) and of the three audiences (§3).

• A decision on the delivery model (§6.1, §6.3).

• Follow-up issues filed for the pieces we're actually building. Filed under the deep-review: delivery rework milestone, tracked in #223:

  • #218 — structured findings as the canonical intermediate form (with stable fingerprints)
  • #219 — PR-comment renderer: verdict-first summary plus inline anchored blockers
  • #220 — auto-resolve the work item behind a PR and grade against acceptance criteria
  • #221 — multi-PR sweep: triage table plus drill-down artifact
  • #222 — delta re-review: fixed / still-open / new against the last reviewed SHA

  Dependency order (stated in each issue's Depends on section — GitHub's Blocked-by relationships are not reachable from this session): #220 can start immediately, it's a capability gap rather than a design question. #218 is the foundation for #219, #221 and #222. #222 additionally needs #219, since the round-to-round state lives in the comment that renderer owns. Everything except #220 waits on the decisions in §6.

8. Related

• #91 /review-audit — compares deep-review output against real PR comments to find blind spots; same "is the review actually useful" question from the measurement side.
• #163 respond-to-pr-feedback and #164 verify-pr-resolution — the other half of the round trip; whatever format we post has to be the format those consume.
• #211 refute-pass before assigning severity — changes what lands in each severity bucket, which changes what a gated PR comment shows.
• #79 (closed) — findings collapsing into one paragraph in markdown renderers; the first symptom of exactly this terminal-vs-rendered mismatch.
• standards/CLAUDE.md → PR Review Comment Anchoring — binding constraint on idea D.

Start with commands/deep-review.md, especially the cited output rules and resolve-pr.sh reference handling, then read standards/CLAUDE.md for comment anchoring constraints. Compare the terminal, PR, and batch-review needs described here; this issue is done when the delivery model and canonical output approach are agreed and implementation work is split into follow-up issues.