Pr Feedback

PR Feedback Triage

Triage a pull request: sort every unresolved review item and failing check by urgency, then hand the user's picks to pr-respond. Read-only — never edit files, post replies, or re-run CI.

--auto <policy> lets a caller (a loop, another skill) supply the selection up front, so §4 reports without asking and §5 hands off directly. --auto confirmed selects every failing check, every Nit, and every item whose verdict is ✅ confirmed. A check carries no verdict and is not a Nit — naming it here is what keeps it from falling out of the selection and being held, which is the one thing a red check must never be. It leaves three kinds unselected and flagged for the user: ❓ unclear, ❌ refuted-but-blocking, and anything needing a merge decision. Without --auto, §4 asks as usual.

Two limits hold under every --auto policy, because the caller is unattended:

• Only bots and checks are selectable. An item authored by a human is reported and held, never acted on — no reply, no reaction, no resolve, no code change. A human wrote to the author, and an automated pass answering in their place is the one thing no policy authorises.
• A confirmed item that §2 turned into a reply for want of a decision is posted as a reply, its thread is left open, and it is counted as held and named in §4's held line. What qualifies is narrow: the reply ends in a question the author must answer before the code can be right. A claim confirmed as real but deliberate — a test or a doc names the intent, the scope was chosen — is answered, not held: it is replied to and resolved like any settled item. Resolving a held item would file the one thing the author still has to act on under the threads GitHub hides: the report carries a gist, the open thread carries the question, and the author needs both. Our reply being the last word is also what marks the thread held on the next fetch, so it is reported and never re-triaged, re-answered or spawned on again.

1. Fetch

The script ships with the plugin; resolve its path the same way gate-wf resolves its own (plugin-root env → newest plugin cache → global-skills fallback), then run it:

for c in "${CLAUDE_PLUGIN_ROOT:+$CLAUDE_PLUGIN_ROOT/skills/pr-feedback/scripts/fetch-pr.py}" \
         $(ls -1 "$HOME"/.claude/plugins/cache/*/ai-skills/*/skills/pr-feedback/scripts/fetch-pr.py 2>/dev/null | sort -V | tail -1) \
         "$HOME/.claude/skills/pr-feedback/scripts/fetch-pr.py"; do
  [ -n "$c" ] && [ -f "$c" ] && FETCH="$c" && break
done
python3 "$FETCH" [pr-number-or-url]

One call, one JSON blob on stdout: the PR (number, url, owner, repo, author, head, base, state, draft, mergeable), every live inline review thread (paginated, with id, comment_id, path, line, awaiting_reviewer, held, rounds, and each comment's author / is_bot / body), settled_threads — the resolved and outdated ones, same shape in 300-char excerpts — the reviews, the conversation comments, and each entry in failing_checks with a 50-line log_tail. A bot's superseded review passes are collapsed to its latest one.

Then ask whether the PR sits in a stack — one call, and it changes what the code in the diff means:

gh pr list --state open --json number,headRefName,baseRefName

The parent is the open PR whose headRefName equals this PR's base; the children are those whose baseRefName equals this PR's head. Note the numbers and stop there — no diff, no threads, nothing else is fetched until a claim actually needs them (§2).

Omit the argument to detect the PR from the current branch. A source that failed leaves a line in errors[] instead of taking the run down: triage the rest and say in the report which source is missing.

The working set is threads + reviews + comments + failing_checks + the branch state (mergeable, draft). A branch that cannot merge blocks the PR as hard as a red check and shows up in no thread and no check run, so mergeable: "CONFLICTING" is an item like any other — see §3. settled_threads is not part of it: someone closed those, so they earn no verdict, no priority and no row. They ship for one job — a PR-level summary carries no path:line of its own and recaps findings that each got a thread, so the settled excerpts are the only evidence that separates a live claim from one already answered. Read them for that (§3), then leave them closed: never re-adjudicate one against current code, and never re-fetch their full bodies.

Done when: the resolved #<n> — <url> is stated for the user and the JSON is in hand.

2. Adjudicate each claim

A review comment is a claim, not a fact. Before an item gets a priority it gets a verdict, and the verdict is earned by reading the code — never by reading the comment more carefully. Default to refuted when the evidence does not arrive: a bot's fluent prose is not evidence, the file is.

For every item asserting a defect — bug, race, security hole, missing test, broken behavior, dead code, wrong type:

1. Read the cited path:line, plus enough of its callers to see the claimed path actually run.
2. Try to refute it. Name the input that triggers the defect, or the reason it cannot occur. "No caller passes null here" refutes; "looks fine" does not.
3. Record the verdict with its evidence — confirmed (file:line + the triggering case) | refuted (file:line + why it cannot happen) | unclear (the one thing that would settle it).

Items asserting taste — naming, formatting, phrasing, an optional refactor — assert no fact, so they carry no verdict. Send them straight to §3.

A reviewer asking why

A question asserts nothing, so it carries no verdict — but it still gets its own read, and that read goes somewhere the defect pass never goes. The diff shows what the code does, and the reviewer has already read the diff: handing it back explains nothing. What they are missing is the intent — the product rule, the screen the behavior feeds, the bug that forced it. Find that, and make it the answer material you carry into the handoff.

Answer material naming only the mechanism — the type that was widened, the field now omitted, the call that no longer runs — produces a reply that walks someone through a diff they are looking at. The test on a draft answer is whether it would survive the follow-up yes, but why do we want that? If it would not, the read is not finished.

Is the shape it fears actually there?

A claim about a data shape — a field that may be absent, a link that may be stale, an id that may point at the wrong row, an enum value nothing writes any more — is not settled by reading code. Code proves the shape is reachable; only the data says whether it occurs, and the disposition turns on that.

So for a confirmed claim of that family, run one query against real data before it becomes a fix, and carry the count into the verdict: "confirmed — 1 row in 250 (local clone)".

• Occurs at scale → fix.
• Reachable, zero or near-zero rows → reply carrying the count, naming where the invariant is actually held: a migration, a constraint, a write path.
• No data reachable → unclear, capped at P2. A guard does not get written on a maybe.

A bot cannot query the database. That asymmetry is the whole value this pass adds over the bot that raised the claim.

Does the repo already do this?

Before a fix adds a helper, a constant, a guard or an error class, grep for it. A bot names what is missing at the cited line; it does not know the same file solves it seventy lines up, or that a util already ships the predicate. A fix that duplicates something the repo has is not a fix, it is a second implementation.

Search the file, then the module, then the package. Found it → the disposition stays fix, but the change is to call the existing thing, and the reply says which. Found nothing → proceed.

Was it written that way on purpose?

Reading the code proves a defect exists. It cannot prove the defect is an accident — that lives in the history, not in the file. So for an item that came out confirmed and would become a fix, and only for those, spend one more look: git log/git blame on the cited lines, the ADRs, CLAUDE.md, and the project memory under ~/.claude/projects/<slug>/memory/.

Find a deliberate decision that the claim contradicts, or an ambiguity you cannot settle from those sources, and the verdict stays confirmed while the disposition becomes reply — the remedy is a choice, and choices belong to the author. Say which source settled it. Find nothing, or find a source that backs the claim, and fix stands.

Does the rest of the stack answer it?

History is not the only place a decision hides. A stacked PR ships foundations on purpose: the caller, the second implementation, the test that exercises the new path all land one PR up. So a claim from the family unused / never called / dead code / only one implementation / premature abstraction / parameter never read is not adjudicated against this diff alone — read the child's diff (gh pr diff <child>) before ruling.

Find the consumer there and the claim is refuted, evidence #<child> plus the consuming file:line, disposition reply. Find nothing in any child and it stands: a stack is a plan, not an alibi, and code that no PR in the stack ever uses is dead today.

The other direction matters as much. A confirmed defect on lines this PR did not introduce — they belong to the parent's commits, outside base..HEAD — is not this PR's to fix: folding it here rewrites the parent's history under it, and fixup refuses that by design. Verdict stays confirmed, disposition becomes reply naming the parent PR as where the fix belongs.

The smallest fix that holds

A fix disposition is not a licence to write code. What travels to pr-respond is the smallest change that makes the claim false — and a bot's own suggested patch is the least likely candidate: it was written without the repo in view, so it adds a guard where a type would do, a helper where a call exists, a config where a constant is enough.

So before a fix leaves §2, invoke /ponytail:ponytail and take its ladder on the intended change: does it need to exist at all, does the stdlib or the platform cover it, does an already-installed dependency, can it be one line. Stop at the first rung that holds and carry that shape into the handoff, not the bot's.

A fix that grows an abstraction, a flag, an interface or a new file to answer one review comment has failed this pass. Write down the shape in one clause ("widen the param type", "call assertUser", "drop the branch") — that clause is what pr-respond implements, and anything larger is the author's call, so the disposition becomes reply instead.

Registry of past refutations

A bot re-raises what another bot already lost. gate-wf keeps the store for this — see its references/dismissals.md for the file path and the content-anchor helper; the same registry is shared, so a claim refuted here stops being re-flagged there.

Before adjudicating, compute each item's anchor and drop the ones already in the registry, noting them in one line ("2 claims already refuted on this branch"). After adjudicating, upsert every refuted item with rule_id: "bot:<login>", source: "pr-thread", and confidence: "resolved" once its thread is closed ("rebutted" while it is still open). An item with no file:line — a PR-level recap comment — has no anchor and stays out.

Done when: every defect-asserting item carries a verdict naming a file:line read in this run, every confirmed fix has had its history checked and carries the smallest-fix clause, and the refutations are in the registry. A verdict resting only on the comment's own wording is not a verdict — go read the file.

3. Classify

The threads marked awaiting_reviewer are awaiting reviewer: the ball is with the reviewer, so they take no priority and no disposition, and they land at the end of the report. Two exceptions pull a thread back out. The author's reply only promised a change, and that change is not in the code — read the cited file to tell the two apart. Or the reviewer is a bot: awaiting_reviewer only means the last comment is the author's, and no bot ever takes that ball back.

A bot thread we answered last carries held: true, and that is a deliberately open thread — an earlier pass adjudicated the claim and left the decision to the author. It is not an item: no priority, no disposition, no reply, and above all no resolve. Name it on the held line of §4 and move on. pr-respond resolves in the same batch it replies, so a bot thread still open with our answer on top is one that was held on purpose; only threads predating that rule are stragglers, and closing those is the author's call, not an automated pass's.

A thread with rounds >= 1 and a bot speaking last is the bot answering us. The item is that reply, never the finding the thread opened with — re-adjudicating the head posts a second answer to a claim already settled and reads, to anyone on the thread, as if the exchange never happened. Read the tail first and classify it on its own merits:

• It acknowledges, agrees, or restates ("justification accepted", "resolved") — nothing is owed. Not an item, no reply, no resolve, no priority. If an earlier pass left a question with the author, that question is still the state of the thread: leave it exactly as it is.
• It disputes, narrows, or raises something new — that is a real item, adjudicated like any other, but the claim under verdict is the reply's, and your own earlier answer is evidence already on the record. Do not repeat it.

Everything else takes one priority:

• P1 — blocking: mergeable: "CONFLICTING" (the branch conflicts with base), a failing required check, a confirmed correctness or security bug, an item carrying CHANGES_REQUESTED, new behavior shipped without a test. A defect claim reaches P1 on its verdict, not on its wording — an unadjudicated claim is not P1 material.
• P2 — important, not blocking: design concerns, maintainability, perf tradeoffs, ambiguous behavior, a reviewer question that needs an answer before merge.
• Nit: naming, formatting, phrasing, optional refactors, taste. Signalled by nit:, optional:, consider, suggestion:.

Doubt about severity rounds up — take the higher priority and note it was close. Doubt about truth rounds down: an unclear verdict caps at P2, and a refuted one at Nit unless a reviewer blocked the PR on it. A refuted claim still earns an answer, just not a code change.

An item with no Resolve button — a PR-level conversation comment (a bot's AI Review summary, a CI recap), or the body of an APPROVED / CHANGES_REQUESTED review submission — gets one check before anything else: match each of its claims against settled_threads. Such a summary recaps findings that each got their own thread, and it is frozen at the SHA that produced it — answering the findings does not rewrite it. A claim that already sits in a settled thread is answered; it produces no row. Say it in one line under the table ("the AI Review body recaps 3 threads, all resolved") and move on. Only a claim with no settled counterpart survives as an item.

What survives is triaged like any other, reply included: an answer is often exactly what it deserves. What changes is who posts it. There is no thread to answer there, so a reply becomes a new top-level comment everyone gets mailed about, and that is the author's call. Mark it reply (hand back) — the answer is written into §4's report, ready to paste, and nothing posts it: not pr-respond, not you. gh pr comment and gh pr review are outside this skill and its handoff, whatever the disposition says.

And one disposition — fix (change the code) | reply (answer, no change) | decline (won't fix) | defer (track for later). pr-respond acts on the disposition, not on the priority, so mark an item fix only when code must actually change. A refuted claim is reply carrying the refutation, or decline — never fix.

A conflicting branch is the one fix that never travels: resolving it is a rebase against base, which pr-respond does not do. Mark it fix (hand back), name the base it conflicts with, and leave it to the user — under --auto it is held, like anything needing a merge decision.

When that base is another open PR, say so in the row: a stacked branch goes conflicting on its own the moment its parent is rewritten under it, so the remedy is a restack (gh-stack), not a judgment call on whose side of a conflict wins. Same hand-back, different sentence — and under babysit-prs the PR's own agent performs it.

Weigh the fix set as a whole

Every rule above judges one claim. Nothing judges their sum, and a review loop only ever adds: a bot names what is missing, never what is surplus. Twelve individually defensible fix items produce a diff nobody would have written on purpose.

So before the report, measure the fix set as one thing:

• Sum the lines those items would add, against git diff --shortstat <base>..HEAD. Past roughly a quarter of the PR's own size, say so in the report: the fix set has outgrown the fix.
• Re-read the fix list as a list. Findings restating one worry from several angles — the same field, the same link, the same call — collapse into one fix, not one per thread. Name the collapse in the report ("4 threads, one guard").

A PR that has already been through a bot pass is the dangerous case: the new findings are about the code the last pass added. When a fix targets a line this PR wrote in answer to an earlier finding, that is the signal the earlier finding was over-applied — reconsider that one instead of layering on top of it.

Done when: every item in the working set carries a priority and a disposition consistent with its verdict, or is marked awaiting-reviewer. A bot's item is adjudicated like anyone else's — its author decides neither the verdict nor the priority.

4. Report

Empty working set — no table. mergeable is not CONFLICTING, no live thread, no failing check, and every PR-level claim matched a settled thread: say that in two or three lines (the PR state, what the settled stock already covers, what is actually left — usually an approval), and stop. No rows, no dispositions, no apply all prompt, no handoff. A row is a commitment to act, and a settled PR has nothing to commit to; a † footnote on a row that says "already answered" is that row admitting it should not exist. Under --auto that is a report of zero selected, zero held — the ordinary state of a green PR whose threads are all settled.

Otherwise, one markdown table — pipes and header included, one row per item, rows ordered P1 → P2 → Nit. Never a per-item list or a paragraph: the point is scanning a column, and a cell that will not fit goes to a footnote.

• # — P1 | P2 | Nit. Append † when the priority was rounded up, and footnote the reason in one line under the table.
• Where — path/to/file.ts:42, the check name, or merge for the branch state.
• Who — 👤 <login> for a human, 🤖 <login> when author.__typename is Bot, the login ends in [bot], or it is a known bot account (naboo-ai-reviews, cursor / Bugbot), ⚙️ check for a failing check.
• Claim — the item in one line, paraphrased. Quote only when the exact wording is the point.
• Verdict — ✅ confirmed | ❌ refuted | ❓ unclear, then the file:line that settled it. — for taste items and checks, which carry no verdict.
• Do — fix | reply | decline | defer.

Keep every cell to one line so rows stay scannable; the evidence that will not fit goes in a footnote under the table.

Then the awaiting-reviewer threads in their own table — Where | Who | Waiting on — one row each, nothing to act on.

Close with one line of files touched by the fix rows, then 1–3 sentences on overall health, counting the refutations alongside the rest ("2 P1 CI failures + 1 confirmed P1 review comment, 2 bot claims refuted, 3 P2s, 5 nits, 2 awaiting reviewer"). Never call a PR clean while it conflicts with its base, and say so when it is still a draft. When §3's weighing found the fix set oversized or several threads collapsing into one, that goes here too, in its own sentence. Then:

Tell me which items to act on — "all P1" / "all" works. Nothing is edited, pushed or posted on that answer: it picks the items, then pr-respond drafts everything and shows you one batch to approve.

Under --auto, skip that prompt. Mark each row selected or held, then list the held rows in one line ("2 held for you: #3 unclear, #7 needs a merge call").

Done when: every triaged item is a row — count the rows against step 3's working set — and the user has been asked to pick, or the --auto policy has selected for them. Zero items is a valid outcome, reported in prose with no prompt and no handoff.

5. Hand off

• A selection is not an authorization. "apply all" names items; it authorizes no edit, no commit, no push, no publication. Consent is a separate act, asked on pr-respond's batch preview, never on the selection. Under --auto, the caller gave that consent up front: pr-respond shows the batch and proceeds without waiting. The gate protects an attended run; it is not a stall for an unattended one.
• Nothing leaves this skill directly. pr-feedback never runs gh pr comment, gh pr review, gh api …/comments, a gh api graphql mutation, git commit, git rebase or git push. The picks leave by one door: pr-respond, whose dispositions are the whole outward surface — a defer is a note in the report, not a Linear issue; a reply (hand back) is a draft, not a posted comment. Anything beyond that, the user asks for by name.
• Do not reimplement pr-respond. Doing its work inline — editing, folding, pushing, replying — bypasses humanizer, the batch preview and the confirmation, which exist only there. If pr-respond cannot be invoked, stop and say so — under --auto, that means reporting the item as blocked, not abandoning it quietly. Never carry on by hand.

Invoke pr-respond with the user's picks. It reads this triage from the conversation and does not re-fetch, so carry each picked item's priority, disposition, verdict with its evidence (the reply to a refuted claim is written from it), the smallest-fix clause for every fix, thread id, first-comment databaseId, owner, repo and PR number into the handoff.

Under --auto, pass the policy through so pr-respond skips its confirmation too.

Done when: pr-respond is invoked, or the user picks nothing (or there was nothing to pick) and the triage is left as the deliverable — and no write command ran inside this skill. A file edited, a commit, a push or a comment posted before pr-respond earned its confirmation is a failed step, not a shortcut.