Skip to content

Reviews: run 1 of the audit validation program (action-translation, 2026-07-28) - #22

Open
mmcky wants to merge 1 commit into
mainfrom
audit-run1-findings
Open

Reviews: run 1 of the audit validation program (action-translation, 2026-07-28)#22
mmcky wants to merge 1 commit into
mainfrom
audit-run1-findings

Conversation

@mmcky

@mmcky mmcky commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The run record required by #16 — the first execution of /audit:issues as a skill, against QuantEcon/action-translation (230 items). Defects filed separately as #21; the .dev/audits/ convention question as QuantEcon/QuantEcon.manual#140.

What held

Eight of the ten review checks. The two that matter most for trusting a long report both held cleanly: the coverage table matches coverage.json on every number, with a six-item residue and an explicit "bounds this audit placed on itself: none by sampling"; and the read-only boundary heldgit status stayed empty across a run that wrote 127 KB into the audited repo, which is the discovery-ordered working directory from #19 doing exactly what it claimed.

I also independently re-verified the audit's headline finding rather than taking it on trust: PR #197 provably never touched the file it was supposed to fix, so the wave escape is real.

What broke

A [verified] citation that does not verify — on that same headline finding. It cites a commit that is real, does touch the right file, and is not on main; it lives on an unmerged branch, while the audit's header says "verified against main". The conclusion survives; the citation does not, and a tag that invites trust while pointing at nothing is worse than no tag. doctrine §2 permits this because it never says to check reachability.

Half of phase 2 was never checkpointed. findings.md held the 56 open issues; the 62 closed ones went straight to the catalog. That is a defect in #17 — my fix — and it would have broken the resume rule had the run died in the closed-set pass.

The finding that cuts against this repo's own decisions

~23 minutes for 230 items, against docs promising "hours, not minutes, on a repo with a hundred items". doctrine §4 justifies the whole checkpointing apparatus on "bulk audits outlive sessions" — at 23 minutes they do not.

I argued for that machinery, so to be explicit about what the measurement costs: the machinery should stay (a 1000-item tracker scales up, interruption remains possible, and the incremental log proved independently useful as an audit trail), but it can no longer be justified by a premise the first measurement contradicts. Fixing the premise is the honest move; quietly keeping it is not.

The gap run 1 did not close

Resumability is still untested. Tutorial step 4 asks the operator to interrupt phase 2 and restart; this run went straight through. The checkpoint provably materialises — it did not exist at all before #17 — but resuming from it remains unvalidated, which is the program's headline claim. Run 2 should be interrupted deliberately.

Checks 9 and 10 — is the tiering right, would you act on this — are the maintainer's and are marked pending rather than answered.

🤖 Generated with Claude Code

…026-07-28)

The first execution of /audit:issues as a skill rather than as a hand-run
runbook, recorded per #16's protocol.

Result: the bundle is good and the plugin has seven defects (#21), one of them
in the checkpoint fix that shipped the day before in #17. Eight of the ten
review checks held, including the read-only boundary — git status stayed empty
across a run that wrote 127 KB into the audited repo — and a coverage table that
matches coverage.json on every number.

Two findings are worth naming here because they cut against decisions this repo
made recently. A [verified] citation on the audit's headline finding pointed at
a commit that is not on main, which doctrine §2 permits because it never says
to check reachability. And the run took ~23 minutes for 230 items, against docs
promising "hours, not minutes" — which undercuts doctrine §4's stated premise
that bulk audits outlive sessions, and so the stated justification for the
checkpointing machinery. The honest correction is to keep the machinery and fix
the premise, not to pretend the measurement did not happen.

Checks 9 and 10 — is the tiering right, would you act on this — are the
maintainer's and remain open. Resumability is still untested: the run was not
interrupted, so the program's headline claim survives run 1 unvalidated.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the required run record for validation program run #1 of /audit:issues (executed as a skill) against QuantEcon/action-translation, capturing setup, measured cost, claim verdicts, and a defect list to support follow-up work tracked elsewhere.

Changes:

  • Add a new validation run document under reviews/ for /audit:issues run 1 (2026-07-28).
  • Record measured phase timings, claim verdicts, and the seven defects found (with links to the tracking issue and related discussions).
Comments suppressed due to low confidence (1)

reviews/audit-run-action-translation-2026-07-28.md:58

  • This link references doctrine §2 but points to the top of doctrine.md. Linking to the specific “Evidence classes” section improves checkability for readers following the citation.
1. **A `[verified]` citation that does not verify, on the headline finding.** The #91 wave-escape entry cites `b99b431` as where `docs/user/heading-maps.md`'s history ends. That commit is real and does touch the file — but it is **not on `main`**; it exists only on `origin/fix/heading-map-position-fallback`, unmerged, dated 2026-03-24. The last commit touching that path on `main` is `0ea2539`. The audit's own header says "verified against `main` @ `2c3d624`". The conclusion survives independent checking and is arguably strengthened, but a reviewer who checks the citation finds nothing — worse than no citation. [doctrine §2](../audit/references/doctrine.md) says cite `file:line`, a merged PR, or a tag; it never says *confirm the commit is an ancestor of the ref you named*. One `git merge-base --is-ancestor` closes it.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +23 to +30
| Phase | Completed | Elapsed |
|---|---|---|
| 1 — snapshot | 16:45:59 | ~11 s |
| 2 — verify (56 open issues) | 16:54:57 | ~9 min |
| 3 — relate | 16:58:24 | ~3 min |
| 4 — write (3 documents + index) | 17:08:05 | ~10 min |
| **Total** | | **~23 min** for 230 items |


[`audit/README.md`](../audit/README.md) says *"Expect hours, not minutes, on a repo with a hundred items"*; [`SKILL.md`](../audit/skills/issues/SKILL.md) says *"a hundred-issue repo is a multi-hour run"*. Both are wrong by roughly an order of magnitude on this repo.

This is uncomfortable rather than merely inaccurate, because it partly undercuts its own machinery. [doctrine §4](../audit/references/doctrine.md) justifies phase checkpointing on the premise that *"bulk audits outlive sessions"*. At 23 minutes they do not. Checkpointing is not thereby worthless — a 1000-item tracker scales up, interruption remains possible, and the incremental log turned out to be independently useful as an audit trail — but the stated *reason* is false and should be corrected rather than quietly retained. See defect 4.
mmcky added a commit that referenced this pull request Jul 28, 2026
The relative form failed the links check because the file lives on #22's branch,
not this one. Absolute is the repo's convention for cross-boundary references
anyway (developing-skills § Conventions), and it resolves once #22 merges —
matching how doctrine.md and audit/README.md already cite the same record.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants