Pull-Request Workflow with Agents

Published

Last modified: 2026-08-25 13:40:06 (PDT)

The practices below apply whether you are driving an agent’s work or doing the work yourself. They keep parallel sessions from colliding and keep a pull request moving toward a clean, mergeable state.

1 File an Issue Before Starting

When starting a new piece of work, go issue-first: before branching, editing, or opening a PR, make sure a tracking issue exists. Search the tracker first; if no open issue covers the task, file one (gh issue create / glab issue create), then proceed. Never jump straight into a PR without a tracking issue behind it.

The issue is the durable record of intent, scope, and “done” criteria — it gives reviewers context, lets the PR auto-close it via Closes #N, and keeps the work discoverable even if the PR stalls. Skip only when the task is already tracked by an open issue.

This rule settles whether something is tracked, not where it goes. An item whose deliverable is a decision rather than a diff belongs on the discussion board instead, per choose-issue-or-discussion — so read “file one” here as “file one in the right venue”, which for actionable work is the tracker.

When the issue is a bug report, include a minimal reproducible example (a reprex — https://reprex.tidyverse.org/) whenever you can. A reprex is what a maintainer needs to confirm and fix the bug, and it’s what they’ll ask for anyway, so providing it up front saves a round trip. The reprexes skill helps reduce the problem to a minimal, self-contained example.

When filing an issue that contains a list of independent subissues, file each subissue as a child issue linked under the parent (GitHub sub-issues feature: mcp__github__sub_issue_write in remote sessions, or gh api with the sub-issues endpoint in local sessions).

That splitting rule has teeth, and they are worth stating: a PR’s Closes #N closes the whole issue, including every item in it the PR never addressed. Read as tidiness, the rule is easy to skip when the second item feels like a footnote. The actual consequence is that GitHub cannot partially close an issue, so the residual items are not deferred and not reopened — they are silently gone, and nothing in the merge, the PR, or the closed issue reports that anything was dropped.

It is worse than an ordinary lost to-do, because a closed issue is evidence that the work was handled. A later reader searching the tracker finds it closed and reasonably concludes every item in it was dealt with, so the loss is not merely silent but actively misleading.

So before writing Closes #N, re-read #N and confirm the diff covers all of it. When it doesn’t, either split the remainder into its own issue first, or reference the parent with Refs #N, which links without closing.

Do: split at filing time, or at the latest before the closing PR merges.
Do: use Refs #N when a PR advances an issue without completing it.
Don’t: let Closes #N ride on an issue whose scope is wider than the diff.

(Morrison-Lab/ai-config#847, 2026-07-29: an issue was filed carrying a primary bug and a secondary note, and the PR fixing the first said Closes #847. The second item survived only because the maintainer asked about it before the merge, which is not a mechanism; it was split into #852 and shipped as #853, and both PRs merged within the following half hour. The splitting rule directly above already existed and was simply not applied when #847 was filed, which is the argument for stating its consequence rather than only its instruction.)

1.1 Deferring a request out of the current change is allowed, and the tracking issue is what allows it

The rule at the top governs work you are about to start. Its mirror governs work you are declining to start now: a request that arrives while a change is already in flight, and that would grow that change past what it set out to do. Such a request may be deferred, on your own judgment, provided the deferred item is filed as an issue in the same reply. The permission and the condition are one rule rather than two. An untracked deferral is not a deferral, it is dropping the request in the vocabulary of scope discipline.

The requests this covers come from the user, which is what makes it worth stating. A reviewer’s finding already has a Defer disposition, per ardi’s ARD step, and a request the user explicitly defers already routes to defer-issue. Neither reaches the commonest case, where the user asks for something adjacent mid-review and the standing instinct treats any direct request as automatically in scope for whatever happens to be open. A request can be genuinely wanted and genuinely out of scope for the current PR at once, and saying so is a service rather than a refusal.

It is a grant of latitude and not an instruction to defer. The default is unchanged: do what was asked. What the grant removes is the bind a mid-flight request creates, where responsiveness and scope discipline pull opposite ways and doing everything asked is the only move that reads as cooperative.

File the issue so it stands alone, by this fragment’s own standard. The conversation that produced the request will not survive it, so an issue reading “do the thing we discussed” defers nothing and only moves the loss somewhere harder to notice.

Say which parts you deferred and why, in the same reply. This is the near-miss, and it reads as compliance from the inside: three things were asked, two were done, the reply describes the two, and nothing states that a third existed. A silent partial delivery is indistinguishable from having done the whole thing, so the user finds out what was dropped only by rereading their own request. Name the deferred item, give the reason, and link the issue.

1.1.1 The boundary with technical debt

dont-incur-technical-debt says a filed issue records debt rather than paying it, and that a defect you have already diagnosed inside your own diff is yours to fix now. Nothing here softens that, and the two rules read as contradictory until the boundary is drawn.

That fragment supplies the discriminator, so use its question:

Does the diff I am about to push contain the thing I just diagnosed as wrong?

When it does, the request is not out of scope, it is the scope, and no issue number buys it out. This rule covers work adjacent to the diff instead: pre-existing prose the change never authored, a broader sweep the change happens to touch one instance of, a follow-on improvement that would be welcome later.

Do: defer an out-of-scope request on your own judgment, and file the tracking issue in the same reply that declines it.
Do: name each deferred item, its reason, and its issue, so a partial delivery is visible as partial.
Do: ask the technical-debt question first, and fix rather than defer whatever the current diff itself introduced.
Don’t: treat a request as in scope merely because the user made it directly.
Don’t: defer without filing — an untracked deferral is a dropped request, and it reads as scope discipline while being the opposite.
Don’t: read this as a reason to defer; the default is still to do what was asked.

(Directive from the user, 2026-08-09: “cai: it’s ok to defer out-of-scope requests from me; just make sure to track them in issues”. It came mid-review on UCD-SERG/serocalculator#654, a Quarto methodology-vignette formalization, where adjacent requests kept arriving in quick succession — convert propositions to theorems, sweep the chapter for overclaims, reformat multi-equality display equations — several of them touching prose the PR had never authored.)

2 Claim a PR or Issue Before Working on It

Before starting a work session on a GitHub PR or issue — i.e. before fetching the branch, making edits, or invoking an automated review cycle — post a brief comment on the PR/issue so other people and any automated review bots know not to start a conflicting parallel session.

Use:

gh pr comment <N> --body "Working on this --- paws off until I'm done."
gh issue comment <N> --body "Working on this --- paws off until I'm done."

Then proceed with the work. After the session ends (PR merged, issue closed, or work otherwise paused), follow up with a closing comment so the PR/issue is unclaimed for the next person.

Skip the claim step if the most recent comment already says you are working on it and that claim is still live under the expiration rule below. This applies to any task that will push commits to a PR branch or run iterative review loops. It does not apply to read-only inspection (showing a PR, checking status, explaining a diff) — those don’t risk a parallel session.

This includes a PR you opened yourself: in repos with an active @claude agent (claude.yml), the agent can push commits to your branch on PR activity — e.g. merging main in — and collide with your in-flight push, so claim early to flag the branch as actively worked. (See memories/claude-bot-workflows.md, “(claude?) CI action”, for the collision-recovery steps.)

When starting work from an issue, follow the claim comment with an immediate draft PR — see pr-on-claim for the mechanics. An open PR is a stronger “in-flight” signal than a comment alone.

A claim expires 2 hours after the most recent push or comment on the PR/issue — reassert it rather than resuming under a stale one. A claim comment with no expiry binds the thread indefinitely: a crashed or abandoned session leaves its “paws off” standing forever, and a second session has no rule for when the claim stops blocking. So the convention is time-boxed and keyed to observable activity: a claim is live for 2 hours from the most recent push or comment on the PR/issue, and expired past that.

The rule cuts both ways.

As the claimant: resuming work after more than 2 idle hours — no push and no comment in that window — starts with a fresh claim comment, not with an edit. The skip-if-already-claimed shortcut above covers only a live claim; an expired claim of your own no longer covers you, because a parallel session is entitled to treat it as lapsed.
As a would-be second session: another claimant’s claim whose PR/issue shows no push or comment in over 2 hours no longer blocks you. Take over by posting your own claim comment, never by starting silently — the fresh claim is what flips the thread’s state, and it is what tells the stale claimant they were superseded if they return. A claim’s age is evidence about the claim, not proof the branch is quiet, so the mid-task checks below — the “already done” cross-check against the PR’s actual commit list, and the rejected-push tree comparison — still apply before your first push, as does the branch-head re-fetch in the claim-pr skill’s Notes.

Staleness is decidable by one read rather than by judgment, per algorithmatize-checks:

gh pr view <N> --json updatedAt --jq .updatedAt        # VIEW_PR
gh issue view <N> --json updatedAt --jq .updatedAt     # VIEW_ISSUE

updatedAt moves on more events than pushes and comments (labels, reviews, body edits), so it only ever over-approximates freshness: a stale verdict from it is definitive, and a borderline-fresh one defaults to respecting the claim — the safe direction, since over-respecting a dead claim costs a wait while under-respecting a live one costs a collision.

Do: post a fresh claim before resuming work when more than 2 hours have passed since the most recent push or comment on the PR/issue.
Do: treat another session’s claim as expired on the same 2-hour reading, and post your own claim before touching anything.
Don’t: read “the most recent comment already says I’m working on it” as a standing skip — that shortcut covers only a claim under 2 hours old.
Don’t: start work under an expired claim, your own or anyone else’s, without a fresh claim comment — a silent resumption and a silent takeover collide identically.

(Directive from the user, 2026-08-15: “let’s set a convention that pr and issue claims last 2 hours from the most recent push or comment; if it’s been longer than that, reassert your claim.”)

Verify a mid-task “already done” claim against real PR state before trusting or redoing it. A PR you claimed and are actively driving can still gain commits from a second, independently-running session under the same account — a <github-webhook-activity> review-comment-reply event can describe work (“Addressed… Pushed in <sha>”) that this session never did. Don’t assume it’s fabricated or injected, and don’t reflexively redo the same fix: cross-check the PR’s actual commit list (gh pr view --json commits / pull_request_read get_commits) and review threads before either (a) trusting the claim, or (b) starting the same fix yourself. If a commit with that SHA genuinely exists, authored close to when the event arrived, treat it as confirmation a live parallel session owns this PR right now — stop pushing further speculative fixes yourself, and, if genuinely in doubt, ask whether to keep driving or step back, rather than racing the other session’s pushes. This gap is distinct from the initial claim check above: it’s not about claiming a PR before starting, but about re-verifying you’re still the sole active driver once work has been under way for a while — especially when you picked up the PR mid-session (e.g. by answering a diagnostic question about it) rather than through the normal claim-then-branch flow, so no fresh “paws off” check ever ran right before you started pushing. (d-morrison/gha#286, 2026-07-24: a webhook event delivered a review-comment reply attributed to d-morrison reading exactly like a Claude-authored reply, claiming a fix “Addressed… Pushed in 3fb8c5b” that this session hadn’t made; verified real via get_commits before proceeding — a second live session, not injection.)

The git-level variant of that check: a rejected push whose remote commit is byte-for-byte what you were about to push. The section above covers a comment claiming work was done. Here the parallel session makes no claim at all. Your git push is simply rejected because it pushed first, and what it pushed is the same merge you just made. The reflex on a rejected push is to merge again, which would stack a redundant merge commit on top of an identical one.

Four reads settle it before you touch anything:

git rev-parse HEAD^{tree}                 # your merge's tree
git rev-parse origin/<branch>^{tree}      # theirs
git show -s --format=%P HEAD              # your merge's parents
git show -s --format=%P origin/<branch>   # its parents

An identical tree plus identical parents means the two merges are the same merge, so the right action is git reset --hard origin/<branch>.

Do: compare trees and parents before deciding what a rejected push means.
Do: discard your local merge with git reset --hard origin/<branch> once both match.
Don’t: re-merge reflexively on a rejected push — that is what produces the redundant merge commit.
Don’t: force-push over the other session’s commit.

(Morrison-Lab/ai-config#965, 2026-07-31: main moved one commit, a local git merge origin/main was made, and the push was rejected. The remote carried b8d2273, a merge of the same two parents, with tree 1bda1bc, identical to the local merge’s.)

Matching parents with a differing tree means the two sessions resolved the same merge differently — merge the two commits together, don’t reset onto either one. Identical parents but a different tree is not the “same merge” case above: a concurrent session (a human, or an @claude-style bot reacting to PR activity) merged the same main commit into the same branch, but resolved a real conflict (or made an additional fix) differently than you did. git reset --hard origin/<branch> here silently discards whatever your version got right that theirs didn’t — e.g. a version-parity bump their merge didn’t carry, or a merge-conflict resolution theirs got wrong. Instead, fetch and merge the remote branch into your local one (an ordinary three-way merge, since the two commits share both parents as a common ancestor between them); resolve any conflict on its own merits, the same as any other merge, then push the result.

Do: compare parents first, then trees; matching parents with a differing tree calls for a merge of the two commits, not a reset.
Do: merge the remote branch in normally, resolving whatever differs on its own merits.
Don’t: git reset --hard onto a same-parents-different-tree remote commit — that discards your own resolution outright, on the assumption the two were interchangeable.

(UCD-SERG/serocalculator#654, 2026-08-08: main had absorbed a same-version dev bump from an unrelated PR, so this session merged main in and bumped the version past it while separately resolving a real conflict in inst/WORDLIST. The @claude review bot’s own main-sync had pushed a merge with the same two parents in the meantime, but it left the version at parity — still failing version-check — and had never seen the WORDLIST conflict at all, since its sync predated that conflict existing. Merging the two commits, rather than resetting onto either, kept both fixes.)

Handing off mid-task to another agent, on user request (“finish what you’re doing, then relinquish holds; I’ll put another agent on them”): don’t just stop — leave the next agent a clean starting point. On each claimed PR/issue: (1) post a status comment on the PR itself distinguishing what’s done from what’s genuinely not done (the actual point of the issue, not just the side-fixes found along the way) and any blocker still open, so the next agent doesn’t have to re-derive it from the diff; (2) post the closing/unclaim comment on the issue per the pattern above; (3) unsubscribe_pr_activity (or stop babysitting locally) so you don’t keep auto-fixing a PR you no longer own; (4) stop any background watch/poll task tied to that work (e.g. a ScheduleWakeup or a Monitor/background-Bash wait) so it doesn’t fire into a session that’s moved on. A merge-conflict-free git status and a pushed branch are not enough on their own — the status comment is what makes the handoff legible. (ucdavis/bcs gia session, 2026-07-06: handed off PRs #310 and #311 mid-implementation this way, each blocked on the same slow renv::restore().)

3 Keep Your Branch Synced with Main

Whenever main has moved ahead of a PR branch you’re working on, merge main into the PR branch before the next push or review trigger. Don’t wait for a conflict to surface or for someone to ask.

Worked-example case records for the rules below live in sync-with-main.cases.md, moved out of the auto-loaded context.

This fragment covers the single-branch-vs-main case. When orchestrating a multi-agent ultracode session, merges can happen at more points than that — see ultracode-merge-conflicts for the broader check (worktree-isolated agent branches, concurrent parallel() results) and the note on GitHub’s mergeable indicator not evaluating custom .gitattributes merge drivers.

Always check for merge conflicts with main before pushing results to remote. Run this before every push, not just before triggering a review:

git fetch origin main
git log --oneline ..origin/main | head    # any commits? main is ahead --- merge it in
git merge origin/main

If the push is rejected because main has moved (! [rejected] with (fetch first) or (non-fast-forward)), fetch and merge before retrying — don’t force-push.

Always do this before triggering a fresh review too, so the reviewer evaluates the PR against current main rather than a stale snapshot.

Don’t rebase or squash-rewrite a published PR branch unless explicitly asked — a merge commit is the right move because it matches GitHub’s “Update branch” button and preserves the PR history.

If the merge has conflicts, resolve them, run the project’s standard pre-commit checks (render / lint / spell / tests), commit, then push. Don’t push a half-resolved merge.

After merging main, re-check version parity. In R packages with a version-check CI job, the branch’s DESCRIPTION Version: must exceed main’s. A conflict-free merge can silently put them at parity — main advanced (e.g. another PR merged between when you last bumped and now). After every merge of main, compare versions:

git fetch origin main
git show origin/main:DESCRIPTION | grep ^Version
grep ^Version DESCRIPTION

If they match, bump the branch’s Version: by one patch level before pushing.

Re-check main again right before the final push, not just at the start of a merge. Resolving a conflict (rerunning generators, fixing prose, updating a CHANGELOG entry) can take long enough for main to advance a second time. A git fetch origin main immediately before git push — after conflict resolution is done, not only before it started — catches that case; an earlier CI failure on a commit you thought was current is a symptom of skipping this second check.

A conflict-free merge does not mean derived artifacts are in sync. If your branch regenerates a generated tree (e.g. codex-skills/, a lockfile, rendered docs) and main added a new source input the generator consumes (a new skill, a new dependency), git merges both cleanly — but the generator never ran against the new input on your branch, so its output is missing or stale and the sync check fails on main after both land. After merging main, re-run the generator and commit the result whenever main touched the generator’s inputs — don’t trust the absence of conflicts. (Concretely: merge the PR that adds the new skill first, then sync the wrapper-regenerating branch and rerun scripts/sync-codex-skill-wrappers.py before merging it.)

A CI failure on a brand-new PR’s very first commit (e.g. the empty claim-commit from pr-on-claim) is a signal to check main’s position before debugging the failure itself. A local checkout that sat around since before the session started can already be many commits behind main — the failure (a stale generated-tree check, a check main has since added or dropped) often isn’t a real problem with your change at all, just main having moved. git fetch origin main && git log --oneline ..origin/main first; if main is ahead, merge it in and re-run the checks before treating the failure as something to fix in the diff.

The same staleness trap has a silent variant with no CI failure to flag it: a worktree/branch named after a PR’s followup can still be based on a main from before that PR actually merged. A worktree directory or branch name suggesting “after PR #N” (e.g. pr-N-followup-...) is not proof the branch’s actual base commit postdates #N’s merge — it can have been created earlier and simply named for its intended purpose. Trusting that naming, then reasoning from git show <hash> for a commit found via git log --all (which lists every reachable commit across all refs, not just your branch’s ancestry) can make content look present when it isn’t actually in your branch yet. Verify with git log --oneline HEAD..origin/main or by reading the actual blob your branch would produce (git show HEAD:<path>, or the working tree itself before assuming what it contains), not a commit hash pulled from --all. If main has moved, merge it in before building further edits on the assumption the missing content exists.

A real conflict inside a file whose logic is also copied elsewhere (an extracted script, a doc example) needs the copy re-synced too, not just the conflicted file resolved. When a PR extracts inline logic (e.g. a workflow step’s shell block) into a standalone script for testability, and main independently changes that same inline logic while the PR is open, resolving the merge conflict in the workflow file is not enough — the extracted script must be updated to match main’s new logic exactly, or the PR silently reverts main’s fix the moment it merges. Diff the extracted copy against main’s current inline version line-for-line (strip indentation, diff) to confirm an exact match, not just “looks about right.” If the PR carries tests against the extracted copy (fixtures, unit tests), add regression coverage for whatever main’s change fixed — the merge is the natural moment to catch a gap the original PR’s tests didn’t anticipate, and to prove the new fixtures actually catch the regression (temporarily revert the fix, confirm the test fails, then restore).

When main DELETES a file your branch references, resolving the marked conflict is not enough — grep the whole tree for the deleted path. Git only conflicts where both sides edited the same lines, so a merge that brings in a deletion flags the file that used the thing, and nothing else. Any other reference to the deleted path — a docstring citing it as precedent, a comment, a doc cross-reference — merges cleanly and silently becomes a dangling reference, because those files were never in the conflict’s scope. After resolving any merge that removed a file, run grep -rn "<deleted-path>" across the repo and re-point or reword each hit; then distinguish live references (must be fixed) from historical citations of the removal itself (correct as-is, leave them). This is the deletion counterpart to the extracted-copy case above: there the logic moved and a copy went stale, here it vanished and the pointers went dead.

A textual conflict in a skill file can be the symptom of a conceptual duplicate, not just competing edits to the same line. When merging main into a branch that’s authoring a new skill, if the conflict lands in a ## Relationship to other skills section (or main added an entirely new skill in the same territory), that’s a signal to re-run skill-builder’s Step 0 judgment — not just resolve the diff mechanically. Compare the new skill against whatever landed on main: are they the same concern (fold into one, redirect), or genuinely distinct (cross-link both directions so neither reads as an unexplained near-duplicate)? skill-builder’s in-flight-work scan only runs once, at the start; main can grow a colliding skill in the time a PR is open, so the check has to be repeated at merge time too.

The same collision can land before you write a line, and then it produces no conflict at all — just duplicated work nobody flags. The bullet above catches a duplicate at merge time, via a conflict. When main gains the colliding content while your change is still planned rather than written, there is nothing to conflict with: you write the duplicate, push it, and the review has to argue you out of content that was already redundant on arrival. So re-run the dupe check after any fetch that brings in new commits, not only at merge — a plan researched an hour ago was researched against a different main.

The cheap version is to read what actually arrived rather than only the count: git log --oneline <old>..origin/main plus git diff --stat over the same range, then ask whether any of it covers something still on your list. In a session that loads skills or plugins from the repo, a new one appearing in the session’s own skill listing is the same signal arriving for free.

This is a timing gap, and it composes with the scope gap rather than replacing it. check-open-prs-before-duplicating covers work that is still in flight, unmerged, and therefore invisible to any check against main; run that one too, since a duplicate is just as wasted whether the collision has landed yet or not. Both checks share the same weakness — each runs once, at the start, and answers for the moment it ran.

Dropping the planned work is the cheap outcome, so record why in the issue and the PR body rather than deleting it silently — otherwise the next person re-proposes it.

A routine merge from main can create the duplicate inside your own diff. The collision above lands before you write, so the duplicate is redundant on arrival. A later main merge is quieter: both branches were non-duplicative when they were written, and the duplicate appears only when you bring the other branch’s text into yours. Git reports a clean merge because the two copies sit in different files. Diff-scoped added-line checks do not help either, because the duplicated lines already existed on one side or the other. So after merging main into a prose branch, run the duplicate check against the branch’s full current diff and the neighbouring corpus, not only against lines added by the merge commit.

Do: after a main merge, re-run a cross-file duplication check over the merged branch’s whole prose diff.
Do: treat a reviewer finding on such duplication as correct even when each copy was independently right before the merge.
Don’t: assume a conflict-free main merge preserved DRY, or that the duplicate would have appeared in an added-lines-only scan.
Don’t: answer by asking which branch “introduced” the duplication; the merge introduced the state that made both copies coexist.

Two PRs that each append a new terminal numbered subsection to the same file (e.g. ### 5. ... in a CLAUDE.md review-guidelines list) will conflict on merge even when neither side’s content actually disagrees. This isn’t an editorial clash — it’s two authors both writing to “the next number” at the same insertion point. Resolve by keeping both additions and renumbering sequentially from the collision point on, not by dropping either side; then grep the file for any other place that names the old numbering (a cross-reference, an index). This is also a reason fully-clean’s CI-green-and-review-clean verdict is a snapshot, not a mergeability guarantee — main can pick up its own append in the same spot after your last review round, so a PR can go from “reviewed clean” to “needs a merge conflict resolved” with no defect in its own diff. Before reporting a PR ready to merge, re-check with git fetch origin main plus the git merge-tree command from resolve-conflicts, not just a cached mergeable flag or an earlier green CI run.

After merging a PR that extracts an inline block into a reusable unit (a composite action, a shared script/function), check other open PRs that still edit that same inline block — your merge just broke their textual diff, even though their intended change is usually trivial to re-apply to the new location. This is the mirror image of the case above: there, you’re the one resyncing after main moved a copy of your logic; here, you are the one who moved the logic, so the burden of noticing and fixing the resulting conflict falls on you, not on the sibling PR’s author waiting to hit it. Don’t wait for that PR’s own merge/CI to surface the conflict — check every open PR touching the same file right after your extraction merges: git merge-tree "$(git merge-base origin/main origin/<sibling-branch>)" origin/main origin/<sibling-branch> (or gh pr diff <N> against the new main) shows whether it still applies cleanly. Re-apply the sibling PR’s actual semantic change (not a mechanical --theirs) to the new location, verify with a direct diff that the extracted unit now differs from main by exactly that PR’s intended change and nothing else, then push to their branch and flag what you did in a PR comment.

See sync-with-main.cases.md, “Check other open PRs after merging an extraction”.

That “push to their branch” is scoped by standing, not only by cause. gha#201/#202 were CI workflow files in a repo the author drove, where a push saves the sibling’s author a round and risks nothing they were relying on. The same push onto a branch you do not own — a colleague’s active work, and most sharply a release branch carrying an out-of-band process — can disrupt something a comment would not. There, name the extraction, the deletion, or the rename and where the content went in a PR comment, and leave the push to whoever owns the branch. Causing the conflict obliges you to surface it. It does not by itself license editing someone else’s branch. See batch-merge-and-resolve, “A conflict your sweep found is not a conflict your merge caused”, for the attribution step that says which conflicts are yours in the first place.

An add/add conflict on a shared config file usually means two PRs independently fixed the same root cause — reconcile the reasoning, don’t just pick a side. This generalizes the skill-file case above beyond skills: a repo-wide CI/lint/build config fix (a new tool config file, a workflow tweak) is exactly the kind of change multiple sessions or bots are likely to attempt in parallel once a check starts failing on main for everyone. When the conflict is a whole-file add/add (not just competing edits to an existing file), read both sides’ reasoning — code comments, commit messages, the PR discussion — before resolving; usually one side’s explanation is more complete (covers a case the other missed, cites the tool’s actual constraint) and should win outright rather than mechanically merging fragments of both. Re-diff the PR against origin/main after resolving to confirm the PR’s remaining changes are its own original scope, not a reintroduction of what the other, now-merged PR already added.

A dirty mergeable_state on a bot-opened PR can mean a sibling PR already closed the same issue, not just that main drifted. An issue-triggered @claude workflow can fire twice on the same issue in quick succession (a duplicate dispatch, or two people independently routing the same request), producing two independent PRs that both fully resolve it — including adding the identical new file. The second PR’s merge conflict is an add/add on that new file, and it looks like ordinary main-drift, but treating it that way and mechanically resolving in favor of “ours” silently reintroduces a duplicate the other PR’s merge already published. Before resolving, check the PR’s linked issue for other cross-referenced PRs/closing events — if one already merged and closed it, diff the conflicting file against main: if it’s the sibling PR’s already-published version, keep main’s content and keep only this PR’s genuinely distinct remainder (a piece the sibling PR never did), rather than re-adding a second copy.

The same parallel resolution can be a whole-file split, and then files can vanish from your diff with no deletion hunk to read. The add/add and duplicate-issue cases above both say to keep main when a sibling PR already published the same new file. The split case adds a second check, because resolving the one conflict can also make other files disappear from your PR’s diff entirely. Those files look harmlessly gone, and there is no deleted line for ardi’s pre-push deletion sweep to inspect. Two causes are indistinguishable from the final diff alone: main absorbed your cross-reference edit, or the merge dropped your work. So verify each vanished file against the pre-merge head before calling the collapse correct. For each file that left the diff, compare the original head against the merge-base to recover what your branch intended, then confirm current main now carries that same change. Search main’s whole corpus for that change rather than the path it used to live at: a sibling PR that relocated the content — a companion-file split, a rename, a section moved between files — leaves it present but elsewhere, so a path-scoped confirmation reports it missing and invites you to re-add a copy main already has. ardia’s Superseded terminal state carries the measurement. Only after that per-file check is it safe to treat the smaller diff as a successful conflict resolution rather than as lost work.

Do: save or read the original pre-merge head, list the files that left the PR diff after the merge, and verify each one’s intended change is already on main.
Do: keep main’s version for the overlapping split file when the sibling PR has already published the same refactor, then carry forward only this PR’s distinct remainder.
Don’t: infer that a vanished file was safely absorbed merely because the final diff got smaller.
Don’t: rely on the deleted-lines sweep for this case; content that left the diff has no deletion hunk for that sweep to show.

When the whole PR is superseded, not just one file, the conflict is telling you to close it rather than resolve it. The two cases above keep main’s version of a file a sibling PR already published, and carry forward the current PR’s distinct remainder. The remainder can be empty. When a main-merge conflict pits every added line of an idle PR against a better-formatted copy already on main — a sibling PR having landed the same content — resolving toward main leaves nothing, and the PR’s own prior review findings are moot. Confirm by grepping origin/main for the PR’s distinctive added phrases before resolving anything: all present means superseded. The right action is then to recommend closing the PR, since its content is preserved on main, not to push an empty diff to a clean verdict. For an ARDIA sweep this is a terminal state of its own — see ardia’s Superseded, which also gives the up-front check that catches it before rounds are spent.

A merge into a growing numbered list (e.g. gha’s CLAUDE.md “Code review guidelines” section) can produce zero blank lines between two adjacent headings even with no textual conflict — lint catches it, git doesn’t. When a section is a hotspot several PRs independently append items to (each PR adding its own ### N. block at the end), a clean three-way merge can still splice one PR’s closing line directly against the next PR’s heading with no blank line between them — this doesn’t produce a <<<<<<< conflict marker (git resolves it as a straightforward insertion), so it’s easy to push without noticing. markdownlint’s MD022 (blanks-around-headings) is what actually catches it, as a CI failure with no proximate code change to explain it. Re-run the repo’s markdown lint (or at minimum re-read the diff around every ### N. boundary you didn’t personally write) after any merge that touches a shared growing list, not just after a merge with conflicts.

The same splice happens to LIST ITEMS, and there markdownlint most likely does NOT catch it — so nothing turns red at all. The case above is a heading spliced against preceding text, which MD022 decides. The changelog case is a bullet spliced onto the previous item’s continuation line:

  `data-raw/precompute-true-effects-chunk.R` (#429).
* The `docs` workflow's "Build site" step no longer times out intermittently.

That is a valid tight list item, so a list in which every other entry is blank-line separated silently starts mixing tight and loose items and renders inconsistently. markdownlint’s blanks-around-lists rule governs the boundaries of a list, not the gaps between its items, and no default rule enforces consistent looseness within one list — so unlike the heading case, CI stays green and only a human reading the merged section notices.

Check it mechanically instead of by eye; one line decides it:

awk 'prev !~ /^[[:space:]]*$/ && /^[*+\-] / {print FILENAME":"NR": "$0} {prev=$0}' NEWS.md

Use [[:space:]]* rather than a bare /^$/ — a whitespace-only preceding line is not a violation and produces false positives. The pattern [*+\-] covers all three common Markdown unordered-list markers; ^\* alone would miss - and + bullets.

Two consequences. Run this after any merge into a changelog or other growing bulleted list, alongside the heading check above. And note that a merge=union driver on such a file (see configure-gitattributes) increases the rate of this defect, since union resolves an append collision by keeping both sides with no conflict to review — so confirm a detector is wired into CI before enabling one, not after.

Run that check as a whole-file count, and compare it before and after — scoping it to the lines you added cannot see this defect at all. The check above is the right instrument; the natural way to apply it is the wrong one. Having found the file’s spliced bullets, the obvious next question is which of them are yours, and the obvious way to answer is to intersect them with the lines the branch added. That question is unanswerable, because the defect is a deleted blank line before a bullet that was already there. The bullet is context in the diff, never an addition, so the intersection is empty by construction and the check reports a confident zero.

Note how this differs from the scope failures elsewhere in this corpus, where a check’s inputs were too narrow — a glob, a missing flag, a two-dot range. Here the inputs were right and the question was wrong, which no widening fixes. And it fails in the direction that reads as an all-clear, on the one file a reviewer will not re-derive.

The sound form is a count delta over the whole file, which needs no judgment about ownership and no diff at all:

git show origin/main:NEWS.md   | awk '...' | wc -l   # before
awk '...' NEWS.md              | wc -l               # after

A merge must not increase the count. That is an algorithmatize-checks instrument in the strict sense — two integers decide it — and it holds whoever authored the surrounding lines.

Generalize past changelogs, because the property is about the defect rather than the file: when a defect can be introduced by deleting a line, any instrument keyed on added lines is unsound. Ask instead whether a whole-file measurement got worse. The version-parity rule above is the same shape — a conflict-free merge leaves the branch at parity with main, version-check goes red, and there is nothing in the diff to point at — which is why that rule is a direct comparison of two DESCRIPTION versions rather than a diff inspection.

The shared trigger is the practical part: a conflict-free merge is exactly when nothing prompts anyone to look. Both defects arrive through one, both are invisible to diff-scoped checking, and the merge reports success.

Do: measure the whole file before and after a merge, and treat any increase as the merge’s fault regardless of who wrote the lines.
Do: ask, of every diff-scoped check, whether the defect it targets could be caused by a deletion — and replace it with a count if so.
Don’t: intersect a whole-file finding with the branch’s added lines to decide ownership; for a deletion-caused defect that always returns zero.
Don’t: read a conflict-free merge as a merge that changed nothing beyond what the diff shows.

A commit claiming “I’ve pulled main and resolved the merge conflicts” can be lying — verify it actually merged before trusting the claim. A genuine conflict-resolution commit is a merge commit (two parents); a commit that just hand-edits files to look resolved, without running a real git merge, is an ordinary single-parent commit — and it never actually incorporates whatever new state of main prompted the “resolve conflicts” request in the first place. This is easy to miss because GitHub’s own mergeable/mergeStateStatus fields don’t distinguish the two: both look identical from the PR page until you check the commit graph. Verify with git show -s --format="%P" <commit> — one hash means no real merge happened, regardless of what the commit message says. If a branch needed conflict resolution more than once and each attempt claimed success but the PR still shows CONFLICTING, check every “resolved conflicts” commit in its history this way before trying yet another resolution attempt on top of a foundation that was never actually re-merged.

4 Driving a Pull Request to Clean

Whenever you are working a PR/MR, run the full ARDI loop by default, without being asked: Address every flagged item, Rebut findings that are wrong, Defer out-of-scope items to tracked issues, then Iterate with a fresh review — repeating until the latest review is fully clean. Don’t stop at “review-clean, just needs approval” and hand triage back; keep the cycle going until it’s genuinely clean.

Extended rationale — the mechanism, evidence, and argument behind each rule below — lives in ardi.rationale.md, moved out of the auto-loaded context. Each rule here keeps its statement and its Do/Don’t pair; read the companion when the reasoning or the evidence is the question.

Worked-example case records for the rules below live in ardi.cases.md, moved out of the auto-loaded context.

Continuously monitor every PR/MR you are actively working until it reaches that terminal state.

That wait is conditional on a run having been scheduled, and on some repos a push schedules nothing.

Do: read the review workflow’s on: block before the first push to a PR in an unfamiliar repo, and dispatch explicitly after the round’s last push when it carries no push-based trigger.
Do: treat a non-zero check-pr-fully-clean.py on a dispatch-only repo as a prompt to dispatch.
Don’t: read green CI at the current head as evidence a review is in flight — on a dispatch-only repo that is the steady state, not a transient one.
Don’t: let a verdict from an earlier head stand because the repo’s trigger class was already known. Knowing it is not the same as acting on it each round.

Dispatch once, after the round’s LAST push — a per-push rhythm cancels its own reviews.

Dispatch with --ref <PR-branch>, or the resulting failure is invisible on the PR.

Do: finish pushing, then dispatch once, and name in the status report which run you are waiting on.
Do: pass --ref <PR-branch> on every dispatch, so the review’s check runs attach to the PR head.
Do: diagnose a missing verdict by reading the run, since a cancelled dispatched run leaves no trace on the PR at all.
Don’t: dispatch per push — each one cancels the last, and the round spends review time producing nothing.
Don’t: re-dispatch reflexively when a verdict is missing. If one is in flight, the retry cancels it.
Don’t: read a green, nothing-pending PR as reviewed on such a repo — that is also what an invisible cancelled gate looks like.

See ardi.cases.md, “A per-push dispatch cancels its own review, invisibly”.

NEVER use background tasks, async sleep commands, or schedule timers for ARDI status polling. Always execute python3 scripts/check-pr-fully-clean.py <pr> synchronously in the foreground turn. This applies transitively to PR-driving workflows such as gi, gii, and ardia; only monitor PRs the session owns or has explicitly claimed, so the rule does not authorize changing someone else’s work.

The loop’s terminal action is to report the PR ready, not to merge it. Merging is human-gated — it happens only on an explicit human “merge it” (the merge-it skill), never as a step ARDI takes on its own. So when you carry a PR across a ScheduleWakeup or /loop wait, never bake a self-merge directive like “if clean and CI green, merge it” into the wakeup/loop prompt: a scheduled prompt fires back as a user-role turn, so a self-authored “merge it” only looks like human approval (and Claude Code’s auto-mode classifier will rightly deny it as a self-authored merge). Drive to fully clean, report ready, and leave the merge — and any other destructive one-off, e.g. a gh workflow run that force-pushes — for explicit human authorization.

Because the loop ends there, the clean verdict is also where ums runs — don’t hold the pass for the merge, which is on the human’s clock rather than this session’s and may land after a /clear or not at all. See CLAUDE.md’s “Run UMS proactively, as learnings accumulate”; the merge-time pass in post-merge then only has to cover what the merge itself taught.

The one exception: if the human has explicitly granted the mwc (merge-when-confident) session permission, that grant is a live human instruction, not a self-authored one, so baking a self-merge step into a wakeup/loop prompt is fine for the rest of that session. See mwc for the grant’s scope and limits.

In the clear-all family (ardia, gia, gii, gip), “report ready, don’t merge” gates only the merge — it does not pause the sweep. A clean-but-unmerged PR is not a stop; move to the next item, and stack it when it isn’t naturally independent of that PR. See stack-dont-pause.

The same gate does not pause the loop within a single PR either, and that is the harder half to see.

The tell is lexical, and it sits in your own outgoing message: a RECOMMENDATION or question whose proposed action is ordinary ARDI work.

Do: resolve conflicts, sync, push fixes, and re-dispatch reviews on a PR whose merge you are correctly withholding, and report those in the past tense.
Do: name the one action that is gated, so “blocked” stays a claim about a step rather than about the PR.
Don’t: generalize a withheld merge into withholding the rest of the loop as though one authorization covered both.
Don’t: write a recommendation proposing work ARDI already mandates — a request to do the required thing is the error, not a courtesy.

See ardi.cases.md, “A merge gate is not a work gate”.

Self-review against the project’s own stated conventions before every push, not just the first — and don’t just re-read the criteria, actually run the applicable review skills against your own diff and iterate on what they find, the same ARD cycle you’d run against an external reviewer’s findings. Don’t treat the review bot as the mechanism that discovers a project’s documented conventions — self-apply them first.

See ardi.cases.md, “A review round surfacing five findings your own conventions already covered”.

4.0.1 Pre-push checklist

Pause point: after committing, before git push.

The whole test suite ran, not the files you predicted the change touches, and the tests/failed/skipped triple was read — a non-trivial skip count means re-running with the gating flags set (NOT_CRAN=true, and whatever else un-gates a conditional skip).
Generated trees were regenerated if the diff (or a main merge) touched a generator’s inputs, and the PR body states how many changed files are generated.
Added lines were scanned for banned punctuation and multi-sentence lines, run after committing, after every pass that edited the diff (your own reflow included), and with the three-dot range (origin/main...HEAD) — a pre-commit run reports on the wrong tree, a later edit retires the lines an earlier run scanned, and a two-dot range re-attributes whatever main deleted to you.
The changelog entry and EVERY PR description this round touched were re-read against the new behavior, not just the code — none is in the diff, so no reviewer and no grep will catch a stale one. Read “every” literally: a round that corrects a claim appearing in two PRs’ bodies discharges the feeling of having synced bodies as soon as one of them is done, and the one most likely to be skipped is your own, because fixing the other repo’s copy is the part that felt like the work. This fires on a prose diff too: a body that explains the claim the round just walked back is stale in the way that matters most, and “reconciling prose” does not feel like changing what the PR does. Every number in the body was re-derived by command rather than re-read, run at this push rather than carried from the last one, with the command pasted beside it — a wrong count reads exactly as plausible as a right one, so reading is no instrument for it, and a base figure owes its own derivation rather than riding on the delta’s. “At this push” includes a push that only answers a self-review finding, and it includes the figures a “Corrections to this body” entry already refreshed — that entry is a claim about the previous head, so the current push is what expires it. A figure whose deriving command carries a precondition owes one more step, because deriving it freshly discharges “don’t recall it” and says nothing about whether the command was right for this diff: cross-check it against a quantity computed by something else (git diff --shortstat against a hand-run added-lines count), since re-reading a correct-looking pipeline confirms it (fail-fast).
The diff’s deleted lines were read (git diff origin/main...HEAD | grep '^-'), and each one was a decision rather than collateral from an edit’s blast radius — a reviewer reads every deletion as deliberate and will rationalize an accidental one.
main was merged in if it moved, with version parity re-checked afterward, so the round costs one review run rather than two — and any whole-file count a merge can worsen (spliced changelog bullets) compared before against after, since a defect caused by a deleted line is invisible to every added-lines check (sync-with-main).
Killer item: the push landed. git rev-parse HEAD origin/<branch> agree before any reply asserting a fix. This one is marked because its failure is not an omission but a false claim about state, which a reviewer has no reason to doubt: CI reports green because it correctly validated the older head, and the session’s own recollection agrees with the reply.

Review a round’s fixes as one diff, not as N independent fixes: two of them, each correctly addressing its own finding, can compose into a defect neither introduces alone.

Do: re-read the round’s full diff as a unit once every finding is addressed, before running the pre-push checklist.
Do: treat a multi-item commit message as the prompt to check the items against each other.
Don’t: conclude the round is sound because each finding’s fix is sound; that is a claim about the parts.
Don’t: rely on the next review round to catch it — it may, but it is then spending a round on something the previous round created.

See ardi.cases.md, “Two correct fixes composing into a defect neither introduces alone”.

A clean verdict does not certify that your diff contains only what you meant, because a reviewer cannot tell an accident from a decision.

Do: read your diff’s deleted lines before pushing, and confirm each one was a decision rather than a casualty of an edit’s blast radius.
Do: say plainly, in the thread, when a review has blessed something unintended — the reviewer cannot know, and its verdict will otherwise stand as the record.
Don’t: treat a clean verdict as evidence about intent; it is evidence about correctness only.
Don’t: keep an unintended change because the reasoning offered for it turned out to be good.

When the edit is a regex or string patch rather than the Edit tool, two mechanisms turn that displacement into a silent over-deletion, and a self-check can wave both through.

Do: anchor a string or regex patch on text unique to the intended site, and prefer the Edit tool’s exact-string matching over a broad .*? DOTALL span.
Do: make a patch self-check assert that neighbouring structures survive — the sibling function or loop still present, untouched element counts unchanged — not merely that the changed element’s count is as expected.
Don’t: trust a re.sub(..., flags=DOTALL, count=1) whose non-greedy .*? start anchor is non-unique; it binds to the first occurrence.
Don’t: read “the assertions passed” as “the patch is correct”; a count-based check can pass by coincidental balance, and the git diff deletion-review is the real gate.

A clean verdict does not discharge the self-review against project conventions either, and the reviewer’s own “not a finding” is where that shows up.

Do: re-run the project-conventions check against your own diff after a clean verdict, not only before the push.
Do: read a reviewer’s “observations” and “not a finding” items as candidate violations, and grep CLAUDE.md for whatever they discuss.
Don’t: let a reasoned “belt-and-suspenders is fine” settle a question the repo already answered in writing.
Don’t: treat a non-blocking label as deciding whether an item gets checked at all.

Proactively self-correct a technical claim you already told a reviewer, the moment further testing shows it was wrong — don’t wait for the reviewer to catch it. If you stated a rationale (an approach is safe, a risk doesn’t apply, a backstop exists) and then discover through your own follow-up verification that it’s false, post the correction with the actual evidence immediately, rather than leaving the stale claim standing until a review round re-raises it. This keeps the review loop converging instead of churning on a claim you already know is wrong.

See ardi.cases.md, “Self-correcting a rationale before the reviewer re-raises it”.

A fix is not “pushed” until it is on the PR’s head commit — verify with a SHA comparison before telling a reviewer you pushed it. From inside a session, an edited working tree and a pushed commit feel identical, so a round that edits the files, writes the reply, and never runs git push produces a reply asserting a fix that does not exist on the branch. Nothing contradicts it: CI reports green, because it correctly validated the older head; the next review round reviews code without the fix; and the session’s own recollection of having made the change agrees with the reply. That makes it worse than an ordinary wrong claim — it is a false statement about state, which a reviewer has no reason to doubt and no cheap way to check.

A SHA you put in a PR body or a reply must be read, never recalled — and the PR body is where an invented one survives longest.

Knowing the prefix genuinely does not discharge this for the full SHA a link wants.

Do: read every SHA you cite out of git rev-parse or git log, and confirm it resolves before pasting it.
Do: correct a published wrong SHA with a visible note naming the real one.
Don’t: write a short SHA from recollection because it looks like the commit you just made.
Don’t: extend a genuinely-read short prefix into a full SHA by hand — the prefix discharges nothing for the 33 characters it does not contain.
Don’t: expect review to catch it — a reviewer has no reason to suspect a citation, and the body is not in the diff they are reading.

See ardi.cases.md, “A genuinely-read prefix, extended into a fabricated link”.

The same rule governs a merge or squash commit message, which is worse than a PR body on both counts the bullet above names.

The trigger is a PR with no closing issue, which is why “verify identifiers” does not reach it.

An invented number here can close someone else’s live work, because issues and pull requests share one number space.

Do: read any Closes/Fixes/Refs number in a merge message out of the PR body it came from, and confirm the target is what you think it is.
Do: say plainly that a PR closes nothing when it has no tracking issue, rather than leaving the slot empty for a number to fill later.
Do: correct a published wrong reference visibly, in a comment, since the message itself cannot be amended once it is on the default branch.
Don’t: treat a closing keyword as inert against a pull request — it closes one, and the number space is shared with issues.
Don’t: infer from “nothing changed state” that a wrong reference was harmless; check whether the target was open.

See ardi.cases.md, “An invented Closes in a merge commit message”.

A SHA’s provenance is the question its source command answers, not merely that a command produced it.

Do: name the claim, then run the one command that answers it, and paste that command’s output.
Don’t: lift a SHA out of nearby command output because it is genuine and close at hand — git stash list and git reflog answer about their own subjects, not about the tip.

See ardi.cases.md, “A read SHA can answer a different question”.

A verification table you write in the PR body is the same defect one artifact over, and re-reading it cannot catch a wrong number.

It goes stale rather than being wrong on arrival.

It sits in the PR body, which nothing re-reads.

Do: re-derive every count in the PR body with a command at push time, and publish the command next to the count.
Do: treat any round that changes the diff as expiring every figure the body already states, not only the figure that round was about.
Don’t: substitute re-reading for re-deriving — re-reading is the right instrument for a stale description and no instrument at all for a stale number.
Don’t: report a delta without deriving its base; a base carried from recollection is unfalsifiable by any later check of the delta.

See ardi.cases.md, “A verification table in the PR body going stale as rounds change the diff”.

A “Corrections to this body” entry is itself a figure in the body, so the next push expires it too — and it reads as more settled than the figure it corrected.

Do: re-derive every figure a corrections entry vouches for at each push, and record the SHA the new figures were derived at alongside them.
Do: append a further numbered entry when a later push moves the figures again, rather than editing the previous one, so the round that expired them stays visible.
Don’t: read a corrections entry as discharging the figures it names — it is a claim about one commit, and the next push is what falsifies it.
Don’t: treat having written the correction as having done the check; the note is that check’s output, never a substitute for re-running it.

See ardi.cases.md, “A corrections entry expires with the next push”.

The read side of that comparison can lag a push by a few seconds, so test the two local refs against each other before concluding anything failed.

Do: compare HEAD against origin/<branch> first when the PR API disagrees, and re-read rather than re-push when those two agree.
Don’t: amend, force-push, or re-commit on the strength of an API SHA alone.

A brand-new branch can read back at the wrong commit, so the local two-ref comparison above is not sufficient there.

The gap is in the trigger rather than in the remedy.

The likeliest explanation is a local one, and it reproduces offline.

See ardi.cases.md, “A brand-new branch reading back at main’s tip, reproduced offline”.

Do: run git ls-remote origin <branch> after the first push to a new branch, and compare its SHA against git rev-parse HEAD.
Do: run git rev-parse HEAD <branch> first when those two disagree, since a branch ref left behind accounts for the whole signature (and note that --short rejects a second revision, so pass neither).
Do: re-run plain git ls-remote as well, so a ref that self-corrects stays distinguishable from one a re-push repaired.
Do: re-push with git push origin HEAD:refs/heads/<branch> when the mismatch persists, and read the SHA range it prints as the confirmation.
Don’t: treat a git push that exited 0 and printed * [new branch] as evidence the commit reached the remote.
Don’t: assume git push -u origin <branch> sent the commit you just made – it sends the branch ref, which HEAD may have moved past.
Don’t: credit a corrective re-push with having repaired a remote-side fault when neither of those two controls was run.
Don’t: answer a No commits between main and <branch> error by re-checking the base branch argument before checking where the head ref actually points.

The same false claim arrives as incoming state when you pick a PR up mid-flight, and there the SHA comparison usually has nothing to compare.

Do: run gh pr diff <N> --name-only against any inherited “already fixed” claim before deciding a finding is closed.
Do: state plainly in your own summary that the prior claim did not hold, and name the head it was false at.
Don’t: treat green CI as evidence that a claimed fix landed.
Don’t: infer that a finding is stale because a comment says it was addressed.

Run that same command before any readiness claim, not only against an inherited one — a PR whose branch carries no implementation is green on every check.

Do: run gh pr diff <N> --name-only before reporting a PR ready, and read the returned paths against what the PR says it does.
Do: treat an empty return, or a return holding only a main merge’s incidental paths, as the PR carrying no implementation.
Don’t: count the claim commit or a main merge as work — neither is implementation, and both give the branch a plausible history.
Don’t: read all-green CI plus a finding-free review as evidence a PR contains anything; on an empty diff that is the expected result.

When the change affects downstream consumers, validate it against a real consumer repo before reporting the PR ready — a package’s own test fixtures are built to exercise its code, not to resemble the packages that will actually use it.

Input shapes no fixture happens to contain. A real package carries metadata the fixtures never needed — an entry of a different kind, an extra tag, an unusual name — so a branch written for it has never actually run on real input.
Message formatting under real counts. Fixtures usually trip the plural path; a real repo hitting the same code with exactly one item exercises the singular wording, which no test asserted.
The migration/upgrade path, as opposed to the fresh-install path. This is the one fixtures can never reach: a fixture is created new by the test, so it always gets the current templates. An existing consumer has the old config, and whether the feature reaches it at all is a different question from whether it works. Verify the claim in the changelog by running the documented migration step, rather than describing it.

See ardi.cases.md, “Validating against a real consumer repo covers what fixtures cannot”.

Verify a blocker you assert in a PR body or a reply, with the same rigor you apply to a reviewer’s claims — a stated blocker becomes a premise other people build on.

Attempting the base form of a command is not attempting its variants — a refusal describes the invocation you ran, never the flag you did not try.

Do: run the flag variant the docs or the error itself name, before generalizing a refusal into an impossibility.
Do: scope the published claim to the invocation actually run, naming the exact command and what it exited with.
Don’t: read an unconditional-sounding error as covering flags you never passed.
Don’t: count an attempt at the base form as discharging the rule above for a variant of it.

Name the specific gate when you report a blocker, not a category word that happens to be one of several.

Do: quote the clause that distinguishes the failure, and name the gate it belongs to.
Do: re-read a blocker you have restated several times, since a paraphrase repeated across status reports hardens into the record.
Don’t: use one mechanism’s own name as a generic word for its category.
Don’t: treat having verified that something is blocked as having verified why.

When the blocker is a hang, inspect the process rather than re-guessing what it is waiting on.

Do: read ps -o stat=, lsof -d 0, and the process tree before describing what a hung command is waiting for.
Do: say which read produced the answer, so the gate is checkable rather than asserted.
Don’t: substitute one guessed mechanism for another because a probe produced no output.
Don’t: report a timeout signal as evidence about why something blocked; it is evidence only that it had not finished.

A blocker that was true when you published it can stop being true while the PR is open, and withdrawing it is your job, not the reviewer’s.

Do: after every main merge, scan the PR’s touched files for merge-status hedges with whitespace-normalizing search, then re-read each hit against the new base.
Don’t: assume a hedge survived because the file that contained it merged without conflicts, or because literal grep missed a phrase split across semantic lines.

Landing a fix falsifies whatever prose documented the defect, and that prose is never in your diff — so grep for it rather than expecting to be reminded.

Prose staled by the fix. It was accurate when written, so nothing about it reads as a defect, and a workaround it prescribes becomes active misdirection the moment the thing it worked around is gone. Keep the entry where the old behaviour explains something — most of a corpus is written against it — but mark plainly that it is history and name the change that ended it.
Prose asserting conformance to a reference. A docstring saying the code “follows” some reference implementation is a claim about two artifacts, and your own divergence falsifies it. This one is not staleness at all: it was false before you arrived, and it is load-bearing, because a reader checking the code against the reference stops at the sentence saying someone already did.
Do: grep the repository for the defect, the workaround, and the behaviour you changed, before calling a fix complete.
Do: mark a superseded entry as history and name the change that ended it, rather than deleting it, when the old behaviour still explains other text.
Don’t: treat a clean grep over the diff as coverage — the stale prose is outside it by construction.
Don’t: leave a doc asserting conformance to a reference standing when the code diverges; correct the claim in the same change that establishes the divergence.

An instruction’s own suggested code is not exempt from the project-conventions self-review above.

See ardi.cases.md, “An instruction’s own suggested code breaking a project convention”.

When the code path under test has a staging or transform step between input and output, a passing unit suite is not evidence it works — exercise the real path once.

See ardi.cases.md, “A staging step the unit fixtures could not reach”.

When new code branches on a third-party tool’s behavior, read that tool’s own config or docs for the specific behavior — don’t infer it from what the tool broadly does.

A regression test written alongside a fix can lock the bug in rather than catch it — assert the two paths that diverge, not the one you just touched.

A systematic audit done by skimming is worse than the one-at-a-time version it replaces.

Adding an explanation supersedes whatever the file already said about the same thing, so re-read the older passage — your own diff is the likeliest source of a contradiction nobody flags.

The same rule applies within a single diff, and there nothing prompts the check at all.

And when the explanation you add is a mechanism claim, test the class it distinguishes, not just the sample in front of you.

See ardi.cases.md, “A mechanism claim whose population held no true positive”.

A symptom that stops reproducing is a fix having landed, until you have checked otherwise — reaching for nondeterminism is the attractive wrong answer.

Do: look for a merged fix, and date it, before attributing a vanished symptom to anything.
Do: report the before/after with its timestamps, so the negative control is visible rather than asserted.
Don’t: explain a symptom’s disappearance as nondeterminism on the strength of one clean run.
Don’t: carry such a claim into an issue or a decision doc, where it argues against the very fix that produced the silence.

Verify a command, path, or flag you write into a doc, with the same rigor address-every-comment demands for one a reviewer suggests.

Do: confirm every literal you invent against the tool’s own source or help output before it lands in a doc.
Do: cite the file or command you checked, so the claim stays falsifiable.
Don’t: infer a subcommand from a family that has its siblings (gh label list/create/edit does not imply gh label view).
Don’t: treat a literal as exempt because the prose around it is well-sourced — the literal is the part a reader executes.

Run that check over your own fix, too — the remedy for an unverified literal is where the next unverified literal goes.

Do: re-run the rule you are applying against the text of your own fix, before committing it.
Do: say in the thread when a fix’s own draft tripped the same rule, since that is the only place the near-miss is visible.
Don’t: treat the effort of writing a correction as evidence the correction is verified.

The same rule reaches past a literal, to the defect CLASS a code fix just closed.

A consolidation commit is the highest-risk host for it, and the likeliest to be trusted.

Do: ask whether the fix’s own new code instantiates the class it closed, before committing it.
Do: treat a commit that consolidates one duplicated concept as owing a check that it forked none, since its framing argues the other way.
Do: compose an existing shared anchor or helper into a new site rather than hand-rolling an equivalent, so the site inherits later fixes too.
Don’t: read a diff that removes a duplicate as evidence that it added none.
Don’t: treat the next round’s finding at a new address as a fresh gap without first checking whether your own previous fix created that address.

When regenerating a generated tree makes it most of the diff, say so in the PR body — otherwise a reviewer reads it as pollution and blocks.

Do: grep a file for a generated-by header before editing it, and change the source instead.
Do: state in the PR body how many of the changed files are generated, and name the hand-written ones.
Don’t: revert generated output because a reviewer calls it noise — check first whether the sync check requires it.
Don’t: assume a reviewer sees the source files; on a large diff they frequently do not.

See ardi.cases.md, “Editing generated output, then being read as pollution once regenerated”.

Run the whole test suite before pushing, not the files you predict the change touches — and check that the ones you ran were not silently skipped.

Matching the tool’s VERSION is not matching its ENVIRONMENT, and when the tool GENERATES a file you are about to commit, the gap ships.

Read the generator’s own diagnostics first, because a good one says so outright and names the cause.

The file list is the backstop, for a generator that degrades with no diagnostic, or one whose diagnostics scroll past in a long run.

Do: read the generator’s own warnings before its output — roxygen2 names the missing package and the tag that needed it.
Do: compare your generator’s changed-file list against the CI log’s, and treat any extra file as an environment mismatch until explained.
Do: install the optional/dev dependency set as well as the tool, when a generator loads the package to do its work.
Don’t: read “I installed the same version CI installs” as having matched CI — version is one input to the output, and rarely the one that differs.
Don’t: commit generated output whose diff is wider than the job you are trying to satisfy reported.

See ardi.cases.md, “A generator’s environment, not its version, changed the committed artifact”.

Do: run the full suite before pushing, and state the tests/failed/ skipped triple rather than “tests pass”.
Do: set the flags that un-gate conditional skips, and re-run if the skip count is non-trivial.
Don’t: scope a local run to the files you edited — the test asserting the old behaviour is usually somewhere else.
Don’t: read a green subset as a green suite, or a skip as a pass.

Running a script is not running its tests, and an “advisory” check can have a hard-gating twin.

Do: run every check the CI job runs, its test files included, before pushing.
Do: grep the job definition for other steps touching the same property before saying anything about whether it gates.
Don’t: substitute a production script’s exit code for its test file.
Don’t: infer a job’s behaviour from one step’s label — “(advisory)” describes that step, not the job.

A third failure mode of the whole-suite rule above: the suite holds no case that could have failed.

Do: construct the input class the change is supposed to handle and diff its behaviour against the pre-change code, before calling a guard verified.
Do: name which cases could have exercised the defect class, rather than quoting a suite total — the total is a fact about the suite, not the diff.
Don’t: offer a pre-existing suite’s green as verification of a change it holds no case for; those cases predate the defect and cannot speak to it.
Don’t: read the tests/failed/skipped triple above as covering this — it makes the report more precise without making it any more relevant.

A fourth failure mode: the case exists, and which branch it reaches is decided by the host.

Do: name in the test which host-derived value selects which branch, and add a case that pins each branch regardless of that value.
Do: run a host-dependent suite in both environments before believing its coverage, and say which branch each run took.
Don’t: read green in CI as covering a branch whose selection depends on an input CI happens to supply one way.
Don’t: reach for the skip count here — nothing is skipped, so that component is identical on both machines even when the failed counts diverge.

See ardi.cases.md, “A suite whose branch coverage varies by host”.

5 What “Fully Clean” Means

“Fully clean” is the terminal state the ARDI review loop drives toward. A PR/MR is fully clean when both of these hold (and verified via python3 scripts/check-pr-fully-clean.py <pr-number>):

Extended rationale — the mechanism, evidence, and argument behind each rule below — lives in fully-clean.rationale.md, moved out of the auto-loaded context. Each rule here keeps its statement and its Do/Don’t pair; read the companion when the reasoning or the evidence is the question.

Worked-example case records for the rules below live in fully-clean.cases.md, moved out of the auto-loaded context.

All CI workflows and check runs are green AND completed. Every workflow and check run passes — not just the required checks and not just the review job.

status itself can be stale, so never infer a job’s duration from it.
- Do: read elapsed time from log timestamps whenever the length of a run is the thing being judged.
- Don’t: conclude a job is still running, or has passed some duration threshold, from in_progress plus the wall clock.
When you are waiting for a job rather than timing one, poll its step list instead of its status — the steps are not subject to the same lag.
- Do: poll actions/jobs/<id>’s steps[] when waiting on a specific job, and treat its terminal step completing as the signal.
- Do: report which step the job is on, so a stalled job is distinguishable from a slow one.
- Don’t: poll a check run’s status in a loop and read repeated in_progress as evidence the job is still working.
See fully-clean.cases.md, “Poll a job’s step list, not its check-run status”.

A BlobNotFound / HTTP 404 on the job-log fetch means the job has not completed, not that it has hung.
- Do: read a 404 / BlobNotFound on the job-log endpoint as “the job has not finished”, and wait for completion (or read the live UI log) before judging its outcome.
- Do: take a job’s real state from its status/conclusion, since the same 404 covers a still-running job and a completed-with-no-logs one.
- Don’t: read a 404 on the log fetch as positive evidence of a hang or a stall — it is the opposite, evidence the job is still running.
- Don’t: file an issue reporting a review job as hung or “no verdict produced” while its log fetch still 404s and its status is in_progress.
- Don’t: run the rule backwards: a log URL being served is not evidence the job completed. The blob can exist mid-run, so a successful log fetch and a still-running job coexist (measured 2026-08-15 on run 31903219396: the MCP get_job_logs call returned a signed logs_url while the job’s own status still read in_progress and it ran on for several more minutes). Completion comes from status/conclusion alone, in both directions.
gh pr checks is not a complete enumeration of a head’s check runs, so read the commit check-runs endpoint before deciding that everything has finished.

--paginate is load-bearing, not tidiness.

The endpoint covers check runs only, so a repo that still uses legacy commit statuses needs a second query.

Why the two surfaces disagree is unexplained, so do not assert a mechanism for it.
- Do: take the check-run half of criterion 1 from the paginated check-runs endpoint, and add commits/<sha>/status where the repo uses commit statuses, rather than treating either query as sufficient alone.
- Do: report both counts when the endpoint and the rollup disagree, so the gap stays visible to whoever reads the status next.
- Don’t: read 0 pending from gh pr checks as evidence that nothing is still running.
- Don’t: drop --paginate — an unfinished run on page 2 returns the same empty result as a finished head.
- Don’t: offer a reason for the omission — none was established.
Every subsection above explains a check list that is short for a per-PR reason, and a platform outage produces the same shape for a reason none of them can reach.
The latest review is totally clean: no nits, and every item that wasn’t directly Addressed is either Deferred to a tracked follow-up issue, or Rebutted with a rebuttal that actually convinced the reviewer — i.e. the reviewer did not re-raise it on the next round.

Criterion 2’s test is the absence of findings, not the presence of a verdict line saying so.

So when the two disagree inside one comment, the findings win. Read to the end of the comment before calling anything clean, and count the items under every heading, whatever that heading is called — address-every-comment already establishes that “non-blocking”, “nit”, “minor”, and “optional” are prioritization labels rather than a pass, and a reviewer files findings under exactly those words in the section that contradicts its own verdict line.

Both criteria are per-PR, and a stack is where that stops being automatic.

Do: derive a verdict per PR number, and name the PR beside each one.
Do: treat a refusal from one reviewer on one PR as evidence about that reviewer on that PR, and nothing else.
Don’t: report a stack’s review state from a single read — “I read the review” is a per-PR claim, and the stack is what makes it read as a claim about the work.

See fully-clean.cases.md, “Both criteria are per-PR, and a stack is where that stops being automatic”.

The disagreement is measurable, and it is not a wording problem.

A reviewer’s own verification block can be wrong while its verdict is right.

Do: re-derive a posted verification’s groups, not just its total.
Do: fix the wording that invited a wrong reconstruction, even when nothing in the diff was false.
Don’t: let the word “verification” stand in for having verified.
Don’t: read a table that sums as one that partitions correctly.

A clean verdict can ratify an enumeration instead of testing it, and then it reads as independent corroboration of a false scope claim.

Do: derive any enumeration you publish with a command, and publish the command beside it.
Do: treat a reviewer restating your count as that count still being unverified.
Don’t: read a clean verdict as evidence that a scope claim in the diff is complete — a reviewer can only check the members you named.
Don’t: count a reviewer’s agreement as independent when its population came from your own prose.

What “an approving review” means here is not a review state.

Do: read the whole review comment and count findings under every heading before calling a PR clean.
Do: establish approval from the findings and thread lists, since .state is COMMENTED on every review this repo receives.
Don’t: quote a Ready for merge line as the clean signal while the same comment lists findings.
Don’t: wait for a formal APPROVED review, or read COMMENTED as a defect in the reviewer.

Findings hide on several surfaces, and no single check sees all of them — so read the verdict body, any suppressed-comments block, the inline comments, the thread list, and the verdict’s own conclusion every round.

An out-of-diff finding never becomes a thread. A finding about a line the diff did not touch cannot be attached as an inline comment, so it appears only in the body — reviewers say so explicitly (“inline comments were unavailable for out-of-diff lines”). A thread count therefore cannot see it. Zero unresolved threads is not evidence of zero findings.
An empty body hides the mirror case. A review can post a completely empty top-level body and carry its entire finding in one inline comment, so a body-only read finds nothing to act on and concludes there is nothing.
A clean overview can hide a collapsed findings block. Copilot can say it “generated no new comments” and create zero inline comments while placing substantive findings inside a collapsed <details> suppression block in the review body. The heading moves, so match case-insensitively on suppressed inside the <summary> heading, not anywhere in the body: PR #660 emitted Comments suppressed due to low confidence (3), while PRs #1029 and #1031 emitted Suppressed comments (4). A literal grep for either exact phrase can return a false zero. A body-wide match over-corrects the other way and can permanently reject a genuinely clean review, since ordinary overview prose can also contain the word — review 4837572117’s summary table read “suppressed Copilot findings” outside any collapsed block. A body read that stops at the overview is therefore not a body read, and a match against the whole body is not the right instrument either.
“No verdict” is its own state, distinct from “a verdict with no findings”. A review job can fail having posted nothing — not a stub, not an empty comment. Zero findings and zero review are indistinguishable by any count, and they call for opposite responses: one is done, the other needs a self-review and a re-run. Read the job’s step outcomes when a review is missing rather than inferring from the absence of comments.
Do: read all review surfaces before calling a PR clean, every round, including collapsed suppressed-comments blocks.
Do: distinguish “no findings” from “no verdict” explicitly, and treat the latter as unreviewed.
Don’t: report clean on a zero thread count, however many checks are green.
Don’t: treat an empty review body as an all-clear without checking the inline comments.
Don’t: treat a “generated no new comments” overview as an all-clear until every <summary> heading has been checked case-insensitively for suppressed — not until the whole body has, which flags ordinary overview prose that merely mentions suppressed findings.
Don’t: read a reviewer’s silence as a verdict — a job that posted nothing leaves the same zero counts as a job that found nothing.

A comment can be evidence-dense, correct throughout, and state no verdict at all — and its density is what gets read as the conclusion.

A later comment stating no verdict does not supersede an earlier one.

Do: identify the last statement that actually states a verdict, and treat that as the standing one.
Do: scan the whole review history for it, not only items matching HEAD.
Don’t: read a verification section, however rigorous, as an approval — it is evidence, and a verdict is a conclusion about evidence.
Don’t: treat a later comment’s silence on the verdict as superseding an earlier “Needs more work”.

See fully-clean.cases.md, “A later comment stating no verdict does not supersede an earlier one”.

Another surface, and the one that defeats the gate itself: the review check can pass on a blocking verdict.

Do: grep the verdict body for its own conclusion, and treat a require-review pass as orthogonal to whether the PR is clean.
Don’t: let a green review-gate check stand in for reading what the review said.

check-pr-fully-clean.py itself has the mirror false positive: it can report NOT clean over a clean verdict.

Do: read the verdict’s own conclusion when the script reports findings against a review whose prose merely discusses finding vocabulary.
Don’t: treat a contains findings (matched pattern ...) line as a real finding without reading the verdict body it matched.

A verdict comment quotes verdict phrases, so a phrase search identifies nothing — and it misreads in both directions at once.

Do: call check-pr-fully-clean.py for a sweep’s verdict column, exactly as ardi requires for one PR.
Do: anchor on the last ### Verdict heading when parsing by hand, after selecting candidates on the **Claude finished marker.
Don’t: take the first verdict phrase in a body as that body’s verdict — quoting other verdicts is part of what a review comment does.
Don’t: assume such a misread has a safe direction; one sweep produced a false-clean and a false-blocked.

A review comment’s header SHA can be stale, so take the reviewed commit from the run’s own head_sha.

Do: follow the job link in the comment and read that run’s head_sha.
Don’t: treat the SHA in a comment’s heading as the commit reviewed.

That remedy assumes the run checked out the PR head, and a workflow_dispatch-triggered review run does not.

Do: check a workflow_dispatch review’s event field before reaching for head_sha — on that trigger type the field names the dispatch ref, not the reviewed commit.
Do: cross-check a stale-suspected verdict’s specific claims against the file directly, rather than only against run metadata.
Don’t: trust head_sha as “the commit reviewed” on a workflow-dispatch-triggered run — that guarantee only holds for push/pull_request-triggered runs, which check out the PR head by construction.

A third surface names a commit the run never read, and unlike the two above it points the confident direction: the run object’s own pull_requests[].head.sha.

Do: settle which commit a review read from a discriminating claim in its own body, since that is the only surface separating the candidates.
Do: read pull_requests[].head.sha as a fact about the PR’s current head, useful for nothing else.
Don’t: read that field naming your latest commit as evidence the review covered it — it names the current head unconditionally.
Don’t: read an empty pull_requests as evidence about the run; the array empties when the PR closes.

See fully-clean.cases.md, “pull_requests[].head.sha named a commit pushed after the run started”.

check-pr-fully-clean.py uses the same unreliable body-text surface, and whichever SHA that text happens to contain — present, absent, or wrong — is incidental to which head the run actually reviewed.

Do: read a flagging run’s event, head_branch, and head_sha before treating “no review at this HEAD” as a genuine gap.
Do: treat the script’s discharge as the likely reading, since the withholding direction dominates in practice, but not as a certified one.
Don’t: re-dispatch a review, or fall back to self-review, on this signal alone when the flagging run’s own metadata already shows it evaluated the current head.
Don’t: read a body’s SHA, present or absent, as evidence about which head a review covered — it is evidence about what the prose happened to discuss.
Don’t: conclude that reviewers citing their head SHA more consistently would fix this; a body can already cite a SHA and still be citing the wrong one.

A clean CI run and a clean review verdict are a snapshot, not a standing guarantee of mergeability. main can advance after your last check — including gaining its own independent addition that collides with yours (see sync-with-main.md’s “two PRs append the same numbered subsection” case) — so re-verify the branch still merges cleanly against current main before reporting a PR ready, not just trust the last green run.

Re-check version parity in that same sweep, not only conflict-freedom.

Threads: at fully-clean, every inline review thread is resolved, and the only conversation left open is the final all-clear exchange — the reviewer’s all-clear comment and your reply to it. (The all-clear is usually a top-level PR comment, not an inline thread.)

One finding can own two threads, so sweep by thread id rather than by finding.

Deadlock -> escalate to a human. If you and the reviewer(s) can’t reach consensus on an item (a rebuttal was exchanged and neither side is budging), don’t loop forever and don’t unilaterally override the reviewer — request a human reviewer, @-mention them in a comment summarizing the impasse, and surface the open item.

An automated reviewer’s verdict on a disputed factual/technical claim is not stable across independent runs, even with identical evidence available each time. Don’t treat one round’s “settled, no need to keep arguing” as durable: the very same review job, re-triggered later with no new code changes, can re-raise a claim it previously retracted — and then retract it again on a subsequent run — purely from re-deriving the question differently each time, not from anything changing in the PR. This means a rebuttal thread’s outcome (however many rounds of citations and counter-citations) doesn’t itself resolve a genuine deadlock the way a human’s decision does; only escalating per the bullet above actually settles it. The one thing that DOES help going forward: fold the authoritative citation/evidence directly into the code or doc being reviewed (a comment, not just a PR conversation reply) — a fresh reviewer run re-deriving the claim from scratch is more likely to find the citation sitting right next to what it’s evaluating than to dig through prior thread history for it, though even that is not a guarantee against a bot that ignores context already in front of it.

6 Address Every In-Scope Review Comment

When iterating on a PR with a reviewer, address every in-scope flagged item, regardless of severity label. The reviewer’s “Not a blocker”, “minor”, “nit”, “optional”, “consider”, or “if you want” labels are for prioritization, not a free pass for the implementer.

Extended rationale — the mechanism, evidence, and argument behind each rule below — lives in address-every-comment.rationale.md, moved out of the auto-loaded context. Each rule here keeps its statement and its Do/Don’t pair; read the companion when the reasoning or the evidence is the question.

Worked-example case records for the rules below live in address-every-comment.cases.md, moved out of the auto-loaded context.

For each flagged item, do exactly one of:

Fix it in this PR. The default path — most nits are 1–3 line changes.
Defer. Only when the fix expands the PR’s scope (new feature, broader refactor, separate concern), the requester has explicitly said this PR shouldn’t grow, or the flagged content isn’t actually yours to fix here (see the main-sync case below). File a follow-up issue and reference it in a PR comment so the item isn’t lost — except in the main-sync case, where the “follow-up” is fixing it on main directly, not a new issue.

Then trigger another review and repeat until the PR is fully clean — zero flagged items under any heading, no “non-blocking”, “harmless”, “minor observation”, or “could improve” sections. “Looks good” / “no findings” / “approved” with no follow-on bullets is the bar. Resolve every inline review thread along the way, leaving only the final all-clear exchange.

Always resolve an inline thread the moment its comment is successfully addressed — the fix pushed and a reply posted naming it — in the same pass, whatever workflow you’re in: a formal ard/ardi round, a CI-monitor nudge, or a one-off fix outside any loop. Addressing without resolving leaves a thread that reads as outstanding work to every later reviewer, blocks fully-clean’s every-inline-thread-resolved criterion, and drags stale noise into the next review round. The per-disposition settlement rules in ard step 4b still govern the exceptions: a Rebut stays open until the reviewer drops it, and an Address you’re not confident fully settles the concern gets a reply asking for confirmation instead of a resolve. The resolve-pr-threads skill sweeps any stragglers, but it’s a backstop — resolve-on-address is the default, not a cleanup step.

Do not report “ready to merge with one minor nit noted” / “harmless as-is” / “can address if you want” — that hedging just pushes triage back to the requester.

A round count is never a reason to stop, and “the reviewer keeps finding things” is not a finding about the reviewer. There is no threshold after which unaddressed items become acceptable: keep requesting reviews and keep dispositioning findings until a review comes back with none. The only exits are a totally clean review, a genuine per-item deadlock, or the user calling it. Reasoning of the form “we have done N rounds, shall we accept the current state?” is the same hedging this paragraph bans, moved up from one finding to the whole loop — see ardi’s “Stopping conditions” for why it fails and for the case record.

Noise is per-item, not per-round — don’t stop the whole loop over one recurring flag. A long-running PR can have both real findings (worth fixing every round) and one specific item the reviewer re-raises verbatim round after round even though it’s already deferred/tracked (e.g. a file-length guideline already split into a follow-up issue). Keep fixing every new finding as it appears — don’t let the recurring item make you stop processing genuinely new ones. But stop re-litigating that one item every round: reply once pointing at the tracked issue, and hold on it specifically rather than re-deferring it on each pass. Surface the pattern to the user (which item, how many rounds, where it’s tracked) and let them decide whether to resolve it now (e.g. do the split) or leave it as accepted recurring noise — don’t decide unilaterally to either keep re-processing it or silently drop it.

When a finding is a pattern (a formatting/style rule broken in one spot), apply it everywhere it recurs in the same file, not just the flagged line.

That rule’s scope is “the same file”, and a reviewer who enumerates the sites is the reason the scope goes unquestioned.

Do: derive the site list by grepping the whole diff for the flagged phrase, and fix what the grep returns.
Do: report the sweep — the pattern searched and the hit count — rather than the number of sites you fixed.
Don’t: treat a reviewer’s enumeration as the extent of the pattern; it is the extent of that reviewer’s read.
Don’t: write “all N spots you named” into a reply, since quoting the reviewer’s count is the tell that no sweep ran.
Don’t: read a null result as “no further sites”; it means no further hit for that pattern, and a differently-worded instance would not have matched.

The remedy above names a search space — “the whole diff” — and a stack has one of those per branch.

Do: run the derived sweep over every branch in the stack, and report the per-branch counts.
Do: treat a finding that names a convention as scoped to the work rather than to the PR it was filed on.
Don’t: read “the whole diff” as satisfied by the diff of the PR the reviewer commented on — that is one of N.

See address-every-comment.cases.md, “A finding’s site list spans every branch in the stack”.

Deriving the class is necessary and not sufficient, because you can derive the wrong one — and the growth rate across rounds is what says so.

Do: read growth in the list across rounds as evidence about the lever, not as a count of members still to add.
Do: state a reviewer’s diagnosis back in your own words before acting on its examples, so a redirect cannot be worked past in silence.
Do: relax the enumeration and read which cases move — a property shared among them names the axis.
Don’t: treat “I derived the class rather than fixing the reported instances” as discharging the rule above; it is that claim one level up, and it fails the same way.
Don’t: prefer the actionable half of a review to the diagnostic half merely because it is the half you can start on.

A narrower version of the same failure: the class is right, and it is enumerated in more than one place.

Do: paraphrase the last two or three findings into a single sentence, and read a match as evidence that a concept is duplicated rather than incomplete.
Do: derive how many sites encode the concept, then consolidate them into one definition every site consumes.
Don’t: answer a third instance by extending a third list — that is the same round again with a new door.
Don’t: skip the review’s own prose naming a sibling site; it is frequently there, in the paragraph explaining why some other mechanism did not save you.

The mirror case: the enumeration was complete and the fix was not.

Do: count the artifacts a single comment names, and give each one its own disposition before replying.
Do: read the rendered page rather than the diff when confirming that a prose or formula fix landed completely.
Do: grep the whole file for the underlying concept once a second half surfaces — a document stale in two places is usually stale in three.
Don’t: let a visibly-changed flagged line stand in for the finding being closed; the unfixed half appears in the diff as context.
Don’t: reach for the derive-the-site-list remedy above here — that list was complete, and the shortfall was in the delivery.

When a prose fix changes wording that’s also paraphrased elsewhere in the same PR (a CHANGELOG entry, a PR description, a cross-reference), sync that copy too. A CHANGELOG entry written before the review lands often quotes or paraphrases the exact phrase a reviewer later flags; fixing the source prose but leaving the paraphrase stale reintroduces the same wording issue one file over. Grep the diff for the flagged phrase before considering the finding closed.

When syncing copies, search the diff for the claim, not the files or symptom already in front of you.

Do: run whole-diff searches for synchronized figures and phrases, after committing the fix, and report the before/after counts.
Do: when a rationale is retired, search for every wording that states that rationale or criterion, not only for the symptom word that made it fail.
Don’t: substitute grep -rn <term> <files-you-had-open> for grepping the diff.
Don’t: accept a search for the visible contradiction as proof that the retired claim itself is gone.

When the wrong thing is a figure, the unit of repair is the figure — across every artifact carrying the twin, not just the diff.

And a reflow puts its neighbouring sentences into your change, for fact-checking and not only for lint.

Do: grep for the figure’s value across every artifact carrying the twin, before replying that the finding is closed.
Do: fact-check the sentences a reflow pulled into your diff, exactly as you would the ones you wrote.
Don’t: treat the named occurrence as the unit of repair when the same value appears elsewhere.
Don’t: read a clean whole-diff grep as covering a twin the diff never touched.

See address-every-comment.cases.md, “The unit of repair is the figure, across every artifact carrying the twin”.

The PR description is on that list and is the one copy grepping the diff cannot find, so check it separately.

Do: re-read the PR description after any Address that changes what the PR does or why, alongside the changelog check above.
Don’t: treat a clean grep over the diff as evidence every paraphrase is synced — the description was never in it.

Answering a body-staleness finding with a correction comment does not clear it, and this corpus’s own visible-correction convention is what makes that move attractive.

Do: edit the body and record the correction inside it, so nothing is silently overwritten and earlier rounds still resolve.
Don’t: answer a body-staleness finding with a comment — the next reviewer re-reads the body, so the finding survives it.
Don’t: treat the drift risk in rewriting a long body as a reason to leave it; re-deriving every figure is what the round already requires.

See address-every-comment.cases.md, “A body-staleness finding is answered by editing the body”.

A body that reports volatile external state goes stale with no edit of yours, so the trigger above never fires on it.

Do: describe the change, and let CI report CI.
Do: timestamp and scope any status you must state (“red as of <sha>, cause was X”), so it cannot be read as a present claim.
Don’t: put current CI status, mergeability, or a blocker in a PR body undated — the body is the one place nothing re-measures.
Don’t: expect the “after any Address” trigger to catch it; that fires on your edits, and this goes stale without one.

Following that “state it as history” advice is what produces the next block, because an automated reviewer reads the body as a flat statement of intent.

Do: state the current content first, marked as current, before any history.
Do: put the reversal in its own section that opens by saying it is history.
Do: make sure the “what is excluded” section does not name the reversed item at all, in any tense.
Don’t: rely on past tense alone to carry the distinction.
Don’t: revert a maintainer-requested change because a reviewer read the history as current — rebut, and escalate rather than comply.

The same sync is needed when the review fix is to CODE BEHAVIOR rather than to wording — and that case is easier to miss, because nothing about fixing a bug points at the changelog.

Tighter still: a changelog entry can contradict its own commit message, in the same commit, with no review in the loop at all.

One step further back: a figure inherited from the tracking issue is both the copy git keeps and the copy nobody verified.

Do: re-run the check when a figure moves from an issue into a commit message, even having verified it once for the PR body.
Do: read git log -1 --format=%B before pushing, against the same source the body’s claims came from — a commit message is not greppable from the working tree once written.
Don’t: copy a count, version, or path out of the tracking issue on the strength of having written that issue.
Don’t: treat “permanent in history” as settled while the PR is unmerged — git commit --amend still works, and is usually worth a fresh CI round against a wrong figure reaching main.

A corollary for checking any of this in a semantic-line-break corpus: a single-line grep returns false negatives on your own prose.

Inline markup breaks the same search, and that variant aims the false negative at someone else’s work rather than your own.

Do: account for inline markup as well as whitespace before concluding a quoted phrase is absent — see the next block for which side to normalize.
Do: read the single hit when a search for a citation’s target returns only the citation itself.
Don’t: file a dangling-citation issue while the only evidence is a literal grep that found nothing but the citation — that is the search failing, until a normalized one agrees.

Apply whatever normalization you choose to the search term as well as to the text, or the fix produces a third false negative of its own.

Do: normalize the needle with the identical function applied to the text, so the comparison is between two transformed strings.
Do: re-test any earlier absent verdict after extending a normalizer, since the extension can break a term the previous version matched.
Don’t: enumerate which markup to strip and treat that list as the fix.
Don’t: test a raw search term against normalized text, however plain the term looks.

A flagged item that came in via a main-sync merge, not your own diff, is still a Defer — just one where the follow-up is fixing it on main directly, not filing a per-PR issue. This is not the ARD skill’s “Acknowledge” disposition: skills/ard/SKILL.md reserves Acknowledge for praise or a no-ask observation, and explicitly warns against stretching it to dodge a real finding — a redundant config line a reviewer flags is a real finding with an implied fix request, so it needs a real disposition, not a label that means “no change requested.” When a reviewer flags something (a redundant config line, a stale pattern) inside a file your branch only touches because you merged main in to resolve a conflict, check provenance before fixing it: git log/git blame the flagged line, or just compare against origin/main’s current content. If it’s identical to main, “fixing” it on your branch alone doesn’t fix anything — it just makes your branch disagree with main on unrelated content the next person to touch that file will have to reconcile again. Reply agreeing the finding is correct but out of scope for this PR, and leave it for whoever owns that file’s actual content to fix on main directly — no follow-up issue needed, since the fix target is main itself, not this PR’s own change.

This generalizes to a skill’s own inline restatement of a fragment it links to. A SKILL.md that links a backing shared/ fragment for the full detail often also restates the fragment’s approach or word list inline (in its description field, or a short procedure-step summary) so a reader doesn’t have to open the linked file. Fixing a bug in the fragment doesn’t automatically fix these inline restatements — they’re a second, independent copy of the same claim, and a review round after the fragment fix can catch them going stale exactly like a CHANGELOG paraphrase does. Grep the whole PR diff for the fixed phrase/word-list, not just the fragment file, before considering a fragment fix complete.

A bot that re-raises an item as “not addressed” may simply not have seen your reply — check the timestamps before treating it as an impasse. An automated reviewer gathers the PR’s comments once, when its run starts. A rebuttal posted after that snapshot is invisible to it, so the next round reports the item as still open and unaddressed even though a substantive reply is sitting in the thread. The tell is a re-raise that repeats the original finding verbatim and speaks only to whether the code changed, without engaging any argument you made. Before escalating, compare your reply’s timestamp against the review run’s started_at (gh run view <id> --json startedAt, or the started_at field each run carries in get_check_runs when gh is absent): if the reply landed after the run began, it is a stale re-raise, not a genuine disagreement.

Reply-first collides with citing the fix’s SHA, and the way out is to commit between them rather than to pick one.

Commit the round’s fixes. The SHA now exists and is stable.
Reply on each thread, citing that SHA.
Push. The next review’s snapshot already contains the replies.

Do: commit, reply citing the committed SHA, then push — in that order.
Don’t: treat “I need the SHA for the reply” as a reason to push before replying; that is the ordering the bullet above exists to prevent.

A finding can be right while its suggestion block is wrong — verify the suggested literal before applying it.

The same check applies to a fix a reviewer describes in prose rather than in a suggestion block, and the sharpest test is the reviewer’s own example.

A reviewer’s corrected citation is another factual claim, so verify the replacement before adopting it.

Do: verify a proposed replacement citation with the source’s own history before editing the PR to use it.
Do: use git log -S "<exact line>" -- <file> or an equivalent provenance query when the question is which PR introduced text.
Don’t: adopt a reviewer’s corrected issue or PR number because the original was wrong.
Don’t: use word overlap and same-day timing as a substitute for source history.

The highest-yield version of that check: when a comment names an edge case in its own prose and also supplies a fix, run the fix against that edge case.

Do: check a suggested fix against every failure mode the same comment names, before checking anything else about it.
Do: name the reviewer’s own caveat in the reply, so the rebuttal rests on their evidence rather than on your say-so.
Don’t: let a comment’s demonstrated thoroughness transfer to its snippet — they are separate claims.
Don’t: discard a finding because its fix is wrong; the half that named the hazard usually still stands.

A quieter variant: the suggestion introduces no defect at all, it restates the line above it — so applying it deletes coverage while reading as hardening.

Do: evaluate the suggested predicate and its neighbours on real input, and keep the finding while rejecting the snippet when they coincide.
Do: fix the underlying coupling instead, and say in the reply why the suggested form was set aside.
Don’t: accept a suggestion block that restates an adjacent check — passing tests afterward prove nothing, since the survivor passes for both.
Don’t: read a reviewer’s own “the line above already covers this” as support for their replacement.

A finding can be right, and its fix adequate, while the reason it supplies is too weak to ship — and in a corpus of rules, the reason is the deliverable.

Do: read the primary source for the strongest reason before adopting a suggested rationale, even when the suggestion’s conclusion is right.
Do: say in the reply which reason you took and why the offered one was set aside, since deviating from a suggestion block silently reads as having missed it.
Don’t: accept a defensible-sounding mechanism because the conclusion it supports is correct.
Don’t: treat this as grounds to reject the finding — the conclusion usually stands, and only its reason needs strengthening.

And the mirror case: a finding can be wrong on its stated grounds while still pointing at something real.

A third direction, which evades the verification reflex rather than lacking a rule: agreeing with a finding and then escalating it.

Do: verify an escalation against the full scope it claims, which is wider than the scope the finding reported, and which the finding’s own instrument may already cover.
Do: post the correction to the thread that carried the escalation.
Don’t: treat agreeing-and-extending as exempt from the checks a rebuttal gets, since agreement suppresses the reflex that disagreement triggers.
Don’t: report a finding as understated on a measurement you have not shown covers the whole field set.

When a finding cites a source, read the cited source before reproducing anything – it is the cheaper instrument, and it is the one that can show the finding backwards rather than merely unsupported.

When a reviewer hedges a finding because it depends on code it cannot see, check whether you can see it — the hedge is an invitation, not a verdict.

Timestamp the evidence before rebutting a finding with it — during a live incident, a log from twenty minutes ago describes a different system.

A rebuttal’s own evidence is the least-checked claim in a review round, and the commonest way it goes wrong is being measured through a tool that adds a shell layer.

Do: write each command spelling to its own file and run the file when comparing them, so exactly one shell layer applies.
Do: hold your own rebuttal to the standard you would apply to the finding, and say which instrument produced the counter-measurement.
Do: re-run the measurement outside the harness when a reviewer holds their ground, before rebutting a second time.
Don’t: read a rebuttal as self-verifying because disagreeing felt like the rigorous move.
Don’t: cite a named check as settling a question without saying what it ran through; a named check reads as a performed one.
Don’t: compare two command spellings by typing both into the same tool, which is the one measurement guaranteed to make them look alike.

A finding carries a timestamp too, and its precondition can dissolve between the round that raised it and the round that addresses it.

The claim is not thereby fixed either, which is the half that is easy to miss.

Do: re-check what a finding presupposes at the moment you address it, not at the moment it was raised.
Do: replace a claim whose gate has cleared with a derived one — the timestamp and the figures the event produced — rather than hedging it or leaving it.
Don’t: apply a reviewer’s suggested wording without re-checking what that wording presupposes; a hedge is false once the thing it hedges has happened.
Don’t: read a claim that has become true as a claim that has been checked — nothing verified it, and the two read identically.

Four neighbours sit close enough to be mistaken for this, and the boundary is worth drawing because three of them fire on the same PR.

See address-every-comment.cases.md, “A finding’s precondition can dissolve before you address it”.

A finding built on a negative result – “I searched and it isn’t there” – is only as strong as the paths that were searched, and the search scope is the part reviewers state loosest.

Do: ask which paths a negative finding actually searched, and check the obvious location yourself before editing anything.
Do: name the gap when the thing does exist – paths searched versus where it lives – so the same search is not re-run the same way.
Don’t: accept “it isn’t there anywhere” as settled because it is stated more confidently than a positive finding would be.
Don’t: discard the finding once its negative result is disproved – the thing it tripped over is often a real ambiguity.

A note the reviewer declined to raise is still a claim, and so is your refutation of it.

Do: verify a declined, out-of-scope, or passing note against the code before either acting on it or writing it off.
Do: hold the change regardless when the note turns out correct but genuinely optional — verifying decides what is true, not what ships.
Don’t: treat a PR title, commit subject, or changelog line as evidence about what the code does; each states an intent, and a refactor can keep the very thing it says it replaced.
Don’t: let your own refutation past the check you would have applied to the reviewer’s finding — it is a fresh claim, and overturning something feels like having verified it.

7 Reviewing AI-generated work

A reviewer of AI-generated work is not there to confirm that it sounds right. The job is to try to invalidate it. Assume the output is wrong until a check shows otherwise, and search mercilessly for mistakes.

Plausible, fluent prose is the main risk, not a comfort. It conceals errors that a human’s awkward draft would have made obvious. Put on the harshest critic hat, especially for:

citations, DOIs, and URLs that may have been invented
functions, flags, and APIs that may not exist on the version you actually run
numbers, file paths, and “as of” claims that were never measured

Author-side validation (Responsibility for validation) is necessary and not sufficient. A second person (or a later pass by the author wearing a reviewer hat) should still try to break the work.

Automated review is a filter, not a substitute for this stance. Those tools miss domain errors and sometimes invent findings; they do not license a lighter human review.