Skip to content

audit:issues: name the phase checkpoints, scale the bundle, fix snapshot provenance - #17

Merged
mmcky merged 1 commit into
mainfrom
audit-checkpoints
Jul 27, 2026
Merged

audit:issues: name the phase checkpoints, scale the bundle, fix snapshot provenance#17
mmcky merged 1 commit into
mainfrom
audit-checkpoints

Conversation

@mmcky

@mmcky mmcky commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Three defects found reviewing /audit:issues against what it promises. The review started from the suspicion that the plugin was over-complicated; that is not what it found, and none of these three is a consolidation. The packaging question is parked on #16 instead, where it can be answered from a run rather than from taste.

The defects

1. The checkpoints had no names. audit/README.md, the skill body and doctrine.md all promise a run that survives a lost session by resuming at the last completed phase — it is the plugin's headline property, and the reason the snapshot script exists. But only phase 1 named its artifacts. Phase 2 produced "per-item findings with evidence tags" and phase 3 "the cluster map", with no filename and no location, so resuming worked only if two independent sessions invented the same file. Phase 2 is also the phase a hundred-item run dies inside rather than between, and it was specified to write on completion — which is no checkpoint at all, precisely where one was needed.

The skill now has a working-directory table (snapshot/, findings.md, links.md, then the bundle), phase 2 appends one entry per item as it is verified in the catalog entry format from deliverables.md — so phase 4 assembles the catalog rather than re-deriving it — and the resume rule is stated: read findings.md, resume at the lowest number in issues.json with no entry, re-verify the last entry rather than trusting a possibly truncated write. Doctrine §4 gains the general form, since it applies to any long audit: a checkpoint owes a findable name and incremental writes, or it is a claim about resumability rather than the property.

2. The bundle did not scale. Four documents were unconditional. deliverables.md hedges the shape ("another audit may need two of these, or none") but the skill did not, so a 20-issue tracker got three files of padding. Padding makes a report less checkable, which is the one thing the four-document shape exists to protect. Below roughly 30 open issues the catalog and the link graph now fold into the report, with the shape used named in the coverage statement.

3. meta["authenticated"] could only ever be true. preflight() exits on every unauthenticated path and returned a bare True for the field to record. A provenance field that cannot vary carries no information — a poor look in the plugin whose doctrine is that every claim carries its evidence class. Replaced with fetched_by, the account the snapshot was taken as, which does vary and matters: visibility is per-account, so two snapshots of a private repo taken by different people can legitimately differ, and knowing who fetched is the only way to tell that from a truncation. Non-fatal — a failure records null rather than exiting, because it means gh's user endpoint was unavailable, not that the snapshot is unauthenticated.

What was deliberately not changed

Worth recording, because the review proposed each of these first and then withdrew them on merit:

Proposed Why not
Delete audit/scripts/README.md qe/scripts/README.md exists for a directory with no scripts yet — it is a convention, not an accident. It is never loaded at run time, so it costs no context, and it carries one command (gh api repos/OWNER/REPO/pulls/N/comments) that appears nowhere else.
Drop the SKILL.md status blockquote benchmark's SKILL.md opens with an identical one. House pattern for recording provenance and the tracking issue.
Trim the script's docstrings The agent runs fetch_tracker.py; it never reads it. That prose is maintainer documentation, and this repo explains its reasoning on purpose.
Fold deliverables.md into the skill Neutral at best, and churn if /audit:prs ever needs it back. Parked on #16.
Collapse repeated facts to one home each Selective, not wholesale. A specification wants one home; an instruction at point of use — the GraphQL gap stated where phase 1 runs — is better repeated than linked.

Validation

Ran against QuantEcon/skills: 8 issues, 7 PRs, numbers 1..15 fully accounted, fetched_by populated. python scripts/validate.py passes all three plugins. Version bumped to 0.1.1 in both plugin.json and the marketplace.json entry.

Follow-up

The larger finding is not addressed here and cannot be: the skill has been executed once, by hand, as a runbook, and never as a skill. Its abstractions are generalised from that one execution. #16 is the validation program — a claims-under-test table, a four-repo run matrix anchored on re-running action-translation and diffing against the hand run, and exit criteria that include capturing the worked example benchmark has and audit does not. The more-structure-or-less question from #12 is deferred to after the QuantEcon/meta run, the case furthest from the original execution and so the sharpest test of what actually generalises.

🤖 Generated with Claude Code

…hot provenance

Three defects found reviewing the skill against what it promises. None is a
consolidation — the review's consolidation proposals mostly did not survive
merit review, and the packaging question is parked on the validation program.

**The checkpoints were unnamed.** README, SKILL.md and doctrine all promise a
run that survives a lost session by resuming at the last completed phase, but
only phase 1 named its artifacts. Phases 2 and 3 produced "per-item findings"
and "the cluster map" with no filename and no location, so resuming worked only
if two sessions independently invented the same file. Phase 2 is also the phase
a hundred-item run dies *inside* rather than between, and it was specified to
write on completion. It now appends per item to a named `findings.md`, in the
catalog entry format so phase 4 assembles rather than re-derives, with a stated
resume rule. Doctrine §4 gains the general form: a checkpoint owes a findable
name and incremental writes, or it is a claim about resumability rather than
the property.

**The bundle did not scale.** Four documents were unconditional. On a tracker
under ~30 open issues that forces three files of padding, and padding makes a
report less checkable — the one thing the shape exists to protect.

**`meta["authenticated"]` could only ever be true**, since preflight exits on
every unauthenticated path. A provenance field that cannot vary is not
evidence, which is a poor look in the plugin whose doctrine is that every
claim carries its evidence class. Replaced with `fetched_by`, the account the
snapshot was taken as — which does vary, and matters because visibility is
per-account on the org's private repos.

Verified against QuantEcon/skills: 8 issues, 7 PRs, 1..15 fully accounted,
`fetched_by` populated. `python scripts/validate.py` passes.

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

This PR updates the /audit:issues skill and its snapshot machinery to make long runs genuinely resumable, scale deliverables to the size of the tracker, and improve snapshot provenance by recording who fetched the data rather than a non-informative auth boolean.

Changes:

  • Document and enforce named, findable phase checkpoints, with phase 2 writing incremental per-item findings to findings.md for reliable resume.
  • Scale the delivered bundle shape for small trackers by folding the catalog and link graph into the report under a stated threshold.
  • Replace meta.authenticated with meta.fetched_by in the snapshot provenance, recorded non-fatally from gh api user.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
audit/skills/issues/SKILL.md Defines the working directory layout, names phase artifacts, specifies incremental phase-2 checkpointing, and documents bundle scaling rules.
audit/scripts/README.md Updates snapshot output provenance documentation to match meta.json’s new fetched_by field.
audit/scripts/fetch_tracker.py Removes the always-true authenticated provenance and records fetched_by (best-effort) in meta.json.
audit/references/doctrine.md Strengthens doctrine §4 to require named artifacts and incremental writes for resumability; updates the phase table to reflect partial phase-2 output.
audit/.claude-plugin/plugin.json Bumps the audit plugin version to 0.1.1.
.claude-plugin/marketplace.json Bumps the marketplace audit entry version to 0.1.1.

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

@mmcky
mmcky merged commit 5272b52 into main Jul 27, 2026
5 checks passed
@mmcky
mmcky deleted the audit-checkpoints branch July 27, 2026 23:52
mmcky added a commit that referenced this pull request Jul 28, 2026
…utorial for running one

Preparing the first real run of /audit:issues (#16) surfaced a contradiction the
plugin has carried since #11, and answering it needed a decision rather than a
convention.

**Finding 0.** doctrine §3 said "no branch or file changes in the audited repo";
deliverables.md says the bundle's first-choice destination is that repo's own
`.dev/audits/`; and 0.1.1's `--out` default writes there too. Together they told
the audit to write into the repo it promised not to touch. §3 is now narrowed to
what it was always protecting — content and history: no commits, no pushes, no
branches, no edits to tracked files — and states that a run may write its own
working directory. Mutation, not writing, is the boundary.

**Where that directory goes** is now discovery-ordered, from contact with a real
repo: action-translation already gitignores `.dev/scratch/*`, so an audit run
there leaves `git status` completely clean and needs no `.gitignore` edit —
which would itself be a change to a tracked file. Prefer an already-ignored
scratch location, fall back to an untracked `.audit/`, then to outside the
checkout.

**docs/tutorial-run-an-audit.md** walks the procedure end to end against
action-translation (228 items), with the phase 1 numbers measured 2026-07-27.
Two things distinguish it from the evaluation tutorial: there is no committed
reference to reproduce, because the skill has never been run as a skill — so the
run is the experiment — and step 4 asks the reader to deliberately interrupt
phase 2 and restart, which is the only test of the checkpoint fix in #17. Step 6
is a ten-point review checklist whose last two items (is the tiering right, would
you act on this) are the ones no self-audit can make.

Also wires audit into docs/using-skills.md, which has had no audit row since the
plugin shipped, plus the AGENTS.md doc map and the site TOC.

NEXT-SESSION.md is added to .gitignore. AGENTS.md already says it must not be
committed, but it sits in the repo root where `git add -A` sweeps it up, and it
routinely holds unsent drafts and correspondence — a convention is the wrong
mechanism for that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Jul 28, 2026
…utorial for running one (#19)

Preparing the first real run of /audit:issues (#16) surfaced a contradiction the
plugin has carried since #11, and answering it needed a decision rather than a
convention.

**Finding 0.** doctrine §3 said "no branch or file changes in the audited repo";
deliverables.md says the bundle's first-choice destination is that repo's own
`.dev/audits/`; and 0.1.1's `--out` default writes there too. Together they told
the audit to write into the repo it promised not to touch. §3 is now narrowed to
what it was always protecting — content and history: no commits, no pushes, no
branches, no edits to tracked files — and states that a run may write its own
working directory. Mutation, not writing, is the boundary.

**Where that directory goes** is now discovery-ordered, from contact with a real
repo: action-translation already gitignores `.dev/scratch/*`, so an audit run
there leaves `git status` completely clean and needs no `.gitignore` edit —
which would itself be a change to a tracked file. Prefer an already-ignored
scratch location, fall back to an untracked `.audit/`, then to outside the
checkout.

**docs/tutorial-run-an-audit.md** walks the procedure end to end against
action-translation (228 items), with the phase 1 numbers measured 2026-07-27.
Two things distinguish it from the evaluation tutorial: there is no committed
reference to reproduce, because the skill has never been run as a skill — so the
run is the experiment — and step 4 asks the reader to deliberately interrupt
phase 2 and restart, which is the only test of the checkpoint fix in #17. Step 6
is a ten-point review checklist whose last two items (is the tiering right, would
you act on this) are the ones no self-audit can make.

Also wires audit into docs/using-skills.md, which has had no audit row since the
plugin shipped, plus the AGENTS.md doc map and the site TOC.

NEXT-SESSION.md is added to .gitignore. AGENTS.md already says it must not be
committed, but it sits in the repo root where `git add -A` sweeps it up, and it
routinely holds unsent drafts and correspondence — a convention is the wrong
mechanism for that.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky added a commit that referenced this pull request Aug 3, 2026
…026-07-28) (#22)

* Reviews: run 1 of the audit validation program (action-translation, 2026-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>

* Run record: label the clock, reconcile the total, anchor the doctrine citations

Copilot review on #22, both comments valid, plus one they did not catch.

The Cost table's "Completed" column carried bare clock times while Setup records
the snapshot as 2026-07-28T06:45:58Z. The two are ten hours apart and look
contradictory; they are the same instant in AEST. In a measurement record that
ambiguity is not cosmetic, so the column now names its timezone.

The total was wrong against the table's own timestamps. 16:45:59 to 17:08:05 is
22 minutes 6 seconds, and the per-phase elapsed figures sum to about the same,
but the Total row said ~23 min. It matters beyond arithmetic: audit/README.md
and #24 both cite "22 minutes" sourced from this record, so as written the
citation disagreed with its evidence — the same defect this record criticises
elsewhere, one level down. Corrected here and in the two places downstream that
restate it as prose.

Both doctrine citations pointed at the top of doctrine.md rather than the section
being cited, which is a poor look in a document whose complaint #1 is a citation
that does not verify. §2 is now #2-evidence-classes.

§4 is anchored to #4-checkpointing, which is deliberately the post-#24 name. #24
renames that section from "Surviving a long run", and merges immediately after
this — so the alternative anchor would be correct for minutes and wrong
permanently. lychee runs without --include-fragments, so CI checks neither.

---------

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