Skip to content

docs(ardi): a fix-round commit's file set must match its findings' files - #2401

Merged
d-morrison merged 2 commits into
mainfrom
docs/ardi-fix-round-file-set
Aug 27, 2026
Merged

docs(ardi): a fix-round commit's file set must match its findings' files#2401
d-morrison merged 2 commits into
mainfrom
docs/ardi-fix-round-file-set

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Closes #2400

Adds an authoring-side check to shared/workflow/ardi.md, beside its existing incoming and push-side "claimed fix landed" rules: before pushing a fix round, derive the union of files the round's findings name and compare it member-by-member against the fix commit's own changed-file list (git show --name-only). A matching count is not a matching set.

Measured 2026-08-27 on #2229: a seven-finding fix commit changed seven files and the match read as confirmation, but the findings named eight distinct files (two findings each spanned two files, one file was named by two findings), the commit covered seven of the eight, and the one unfixed finding hid behind the equal counts — the next adversarial round re-raised it.

Two commits: the entry, then fixes for its own pre-push adversarial review's three findings (--name-only over ellipsizable --stat, resolved pronouns, reconstructable arithmetic). The follow-up review at head 668aeb7f returned Ready for merge (verified the arithmetic against the real #2229 artifacts); both review rounds will be published as comments.

Checks: new-line-breaks gate vs origin/main clean, check-links.py clean, non-ASCII scan of added lines clean.


Generated by Claude Code

claude added 2 commits August 27, 2026 06:39
A commit message enumerating a review round's findings asserts its diff
touches every file those findings name, and nothing checks that by
default. Measured on ai-config#2229 (2026-08-27): a seven-finding fix
commit changed seven files and the count match read as coverage, while
one finding's file was untouched; the next round re-raised it.

Adds the authoring-side check beside ardi.md's existing incoming and
push-side "claimed fix landed" rules: derive the union of files the
findings name and compare member-by-member against the fix commit's
changed-file list before pushing.

Closes #2400.
Addresses the three findings from the pre-push adversarial review at
528a63c:

1. The mandated instrument is now git show --name-only, since --stat
   can ellipsize long paths into indistinguishable members -- the
   exact conflation the entry exists to prevent.
2. The re-raise clause names its referents: "re-raised the finding
   against a commit message that claimed to have fixed it."
3. The case narration now reconstructs: the seven findings named
   eight distinct files (two findings each spanned two files, one
   file was named by two findings), the commit covered seven of the
   eight, and the unfixed finding hid behind the equal counts.

Copy link
Copy Markdown
Collaborator Author

Adversarial pre-push review (round 1 of 2) — dispatched adversarial-reviewer subagent (Claude), read-only, against the PR delta at 528a63cd. Report published verbatim below; round 2 follows.


Summary of Changes

One commit (528a63cd) adds a single ~22-line block to shared/workflow/ardi.md (after line 602), stating an authoring-side pre-push check: a fix-round commit's changed-file list must match, member by member, the union of files the round's findings name, with a measured 2026-08-27 case on ai-config#2229 and a labelled Do/Don't pair. Commit message carries Closes #2400.

Findings

  1. [Defect] shared/workflow/ardi.md (added block, "compare it against the fix commit's own changed-file list (git show --stat)"): git show --stat is the wrong instrument for the check the entry mandates. Verified in this worktree: --stat output truncates paths to fit the stat-graph width (at constrained width, codex-skills/delegate-to-opencode/SKILL.md and skills/delegate-to-opencode/SKILL.md both render as .../SKILL.md — indistinguishable members), and in a non-tty script context the width defaults low enough that this repo's long shared/workflow/*.cases.md paths are at risk. A member-by-member set comparison built on truncated, ellipsized paths can silently conflate two distinct files — the exact failure class (a coincidence masking a set mismatch) the entry exists to prevent. The immediately preceding sibling block uses gh pr diff <N> --name-only; the parallel here is git show --name-only (or git diff-tree --no-commit-id --name-only -r), which emits full, one-per-line paths.

  2. [Convention] shared/workflow/ardi.md (added block): "so the next review round re-raised it against a commit message that claimed it fixed." Two problems in one clause, per shared/writing/ambiguous-reference.md: the first it (intended referent: the finding) has "the one file a remaining finding named" as its nearest antecedent, and "claimed it fixed" is ungrammatical as written — it parses neither as "claimed to have fixed it" nor "claimed it was fixed", and the second it has no clear antecedent at all. The wording mirrors issue UMS: a fix-round commit's file set must match the files its findings name #2400's body verbatim, but the corpus prose rules govern the rule entry, not the issue that requested it. Replace the pronouns with nouns: "...re-raised the finding against a commit message that claimed to have fixed it."

  3. [Edge Case] shared/workflow/ardi.md (added block): the case narration does not reconstruct arithmetically. Seven findings, seven changed files, "two findings shared a file, another file carried two findings' fixes, and the one file a remaining finding named was untouched" — under any reading (the two sharing-clauses as one instance or two), the named-and-touched union comes to 4–5 files, so the count coincidence requires 2–3 changed files no finding named, which the prose never says. The two sharing-clauses also read as the same relation stated twice, leaving the reader unable to tell whether they describe one file or two. One added clause (e.g. "the remaining changed files were ones no finding named") makes the coincidence mechanism checkable instead of asserted.

Verified clean: semantic line breaks (gha-check-new-line-breaks.py, exit 0); ASCII punctuation on all added lines (zero non-ASCII matches); relative links (check-links.py, 2605 links / 627 files, no breaks); no contradiction of or redundancy with the neighbouring "claimed fix landed" blocks (ardi.md's existing rules cover the incoming variant via gh pr diff --name-only and the readiness claim; a corpus grep across shared/, memories/, skills/, CLAUDE.md found no existing authoring-side set-vs-count rule — the nearest hit, ardi.cases.md:713, is a different lesson about roxygen environments); Do/Don't pair present, both sides concrete; Closes #2400 matches the issue's scope exactly (the issue, open, filed 2026-08-27 by d-morrison, asks for precisely this ardi.md entry); PR #2229 exists and matches the cited context.

Verdict: Needs more work

Reviewed-Commit: 528a63c

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Adversarial pre-push review (round 2 of 2) — dispatched adversarial-reviewer subagent (Claude), read-only, scoped to fix commit 668aeb7f plus resolution of round 1's three findings. Report published verbatim below. All three round-1 findings were Addressed in 668aeb7f; this is the clean verdict the push shipped under.


All checks complete. All three prior findings verify as genuinely resolved, and — critically — the rewritten arithmetic was checked against the real PR #2229 artifacts, not just for internal consistency.

Summary of Changes

Reviewed git diff origin/main...HEAD at 668aeb7 (2 commits, 1 file: shared/workflow/ardi.md, one ~24-line block plus an +8/−6 fix atop it). The block adds a rule that a fix-round commit's changed-file set must be compared member-by-member against the union of files its round's findings name, with PR #2229 as the measured case. The fix commit swaps git show --stat for git show --name-only with a rationale parenthetical, resolves the previously ambiguous pronouns, and rewrites the case arithmetic.

Findings

No actionable findings identified.

Verification performed:

  1. Arithmetic reconstructs and matches reality. From a83e29a8's own commit message and GitHub file list: 7 findings; findings 1 (README.md + setup.qmd) and 6 (session-start.sh + README.md) each span two files; README.md is named by two findings. Namings = 7 + 2 − 0 = 9; distinct = 9 − 1 = 8. The commit changed exactly 7 files — every named file except shared/workflow/keep-checkouts-fresh.md (finding 4). So "seven-finding fix commit changed seven files", "eight distinct files", "covered seven of the eight", and "the one unfixed finding hid behind the equal counts" are all correct, and the equal counts (7 findings / 7 files) are the stated coincidence.
  2. The re-raise claim matches the record. Follow-up commit fda39bbc states round 3's findings "were the round-2 finding 4 that commit a83e29a claimed to fix but never edited" — exactly the diff's "re-raised the finding against a commit message that claimed to have fixed it." The date "Measured 2026-08-27" matches both commits' timestamps.
  3. The --stat ellipsization claim is fair and demonstrable on the case itself. git show --stat=40 a83e29a8 prints .../session-start.sh, .../claude-code-hooks.md, .../post-merge/SKILL.md — abbreviated paths that could conflate members (e.g. .../claude-code-hooks.md beside memories/claude-code.md), while --name-only prints full one-per-line paths.
  4. Pronouns resolve. "claimed to have fixed it" — "it" resolves to "the finding", the sentence's subject-side referent; no closer wrong antecedent competes.
  5. Mechanical gates: NLB gate (NLB_BASE_REF=origin/main) exit 0, no missing semantic breaks; check-links.py exit 0 (2605 links, 627 files); non-ASCII scan of all added lines clean (the --- dashes are ASCII triple-hyphens per repo convention).

Verdict: Ready for merge

Reviewed-Commit: 668aeb7

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator Author

@claude review

The prior round declined a verdict solely because CI was still running at 668aeb7f; all check runs at that head have since completed green (both validate runs, both new-line-breaks runs). No code has changed — requesting a fresh review of the same head now that its stated blocker is resolved.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

👀 Picked up by workflow run #33047670063. Setup runs first; Claude itself responds after that.

@github-actions

Copy link
Copy Markdown
Contributor

👀 Claude Review Dispatchedrun reviewing PR #2401 at 668aeb7.

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


Verdict

Ready for merge, pending completion of this review job itself.

Reviewed commit: 668aeb7

No new findings — my independent content review (arithmetic reconstruction against PR #2229's actual a83e29a8 commit via the GitHub REST API, local reproduction of the git show --stat ellipsization claim, ASCII-punctuation scan, redundancy sweep, and Closes #2400 scope check) found nothing beyond what round 2 of this PR's own self-review already verified at this same SHA (#2401 (comment)).

CI status, run through the authoritative instrument rather than inferred: python3 scripts/check-pr-fully-clean.py 2401 -R Morrison-Lab/ai-config currently exits 1 (NOT clean), but both reasons it cites are self-referential to this exact review: the latest posted verdict comment is the prior round's "Blocked on CI" (posted before CI finished), and the one check run it flags as in_progress is review / claude-review job 98435679820 on workflow run 33047737350 — this very job producing this review. I confirmed via the paginated check-runs endpoint (commits/668aeb7f/check-runs, 14 of 14 runs) that every other run — both validate, both new-line-breaks, gather-context, require-review, post-review, preempt-previous, and the earlier review / claude-review run from the prior round — is completed/success; there are no legacy commit statuses. So the instrument cannot yet certify clean because it is waiting on this job's own completion, not because of any defect. Once this review posts a clean verdict, the instrument's stated blockers resolve themselves. I'm reporting this explicitly rather than asserting a "fully clean" claim the instrument itself cannot yet confirm.

Stopping Point: Clean stopping point reached — review completed for the current head (668aeb7), no unaddressed findings, nothing further queued for this review-only run.

Reviewed commit: 668aeb7

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $2.2979 (review) — run

Copy link
Copy Markdown
Collaborator Author

Merging under the standing ai-config mwc grant: fully clean at head 668aeb7f — CI Claude review verdict Ready for merge at this exact commit (its only hedge, "pending completion of this review job itself", resolved when that job completed success), independent adversarial review verdict Ready for merge at the same commit (round 2 above), all 16 check runs completed/success (both validate and new-line-breaks runs green), and the prior round's "Blocked on CI" verdict superseded by the same reviewer's later clean round at the same head. The PR is behind main by unrelated commits with no conflict; its one-file diff (shared/workflow/ardi.md) collides with nothing in flight.

Posted by Claude Code (AI agent) --- not written by a human.


Generated by Claude Code

@d-morrison
d-morrison merged commit 4c26fc8 into main Aug 27, 2026
16 checks passed
@d-morrison
d-morrison deleted the docs/ardi-fix-round-file-set branch August 27, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

UMS: a fix-round commit's file set must match the files its findings name

2 participants