From c9ffe06f36579a82ebfaf543086d71cead37bcc6 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Tue, 28 Jul 2026 09:37:47 +1000 Subject: [PATCH] audit:issues: name the phase checkpoints, scale the bundle, fix snapshot provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .claude-plugin/marketplace.json | 2 +- audit/.claude-plugin/plugin.json | 2 +- audit/references/doctrine.md | 4 +++- audit/scripts/README.md | 2 +- audit/scripts/fetch_tracker.py | 32 +++++++++++++++++++++----------- audit/skills/issues/SKILL.md | 19 ++++++++++++++++++- 6 files changed, 45 insertions(+), 16 deletions(-) diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index dae865a..ecafcda 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -21,7 +21,7 @@ { "name": "audit", "source": "./audit", - "version": "0.1.0", + "version": "0.1.1", "description": "Bulk, read-only audits of a QuantEcon repository — issue triage, PR review, technical debt, translation parity — each producing an evidence-cited report bundle" } ] diff --git a/audit/.claude-plugin/plugin.json b/audit/.claude-plugin/plugin.json index bd3a268..8d06354 100644 --- a/audit/.claude-plugin/plugin.json +++ b/audit/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "audit", "description": "Bulk, read-only audits of a QuantEcon repository — issue triage, PR review, technical debt, translation parity — each producing an evidence-cited report bundle", - "version": "0.1.0", + "version": "0.1.1", "author": { "name": "QuantEcon" } } diff --git a/audit/references/doctrine.md b/audit/references/doctrine.md index 85cdca3..4e27bb2 100644 --- a/audit/references/doctrine.md +++ b/audit/references/doctrine.md @@ -37,6 +37,8 @@ Corollary: an audit never writes into the plugin directory, and its outputs neve Bulk audits outlive sessions. Context runs out, rate limits bite, machines sleep. One rule follows from that, and it is about checkpointing rather than about structure: **write each phase's output to the working directory before starting the next**, so a lost session resumes where it stopped instead of restarting. How a skill divides itself into phases is its own business — the division below is one that worked, not a template to fill. +Two properties separate a checkpoint from a claim about one, and a skill that promises resumability owes both. The artifact needs a **name the next session can find without guessing** — an unnamed intermediate is only resumable if two sessions independently invent the same file. And a phase that iterates over many items must **append as it works, not write when it finishes**, because the phase long enough to be worth checkpointing is the phase a run dies *inside*. Output that exists only on completion is no checkpoint at all, exactly where one was needed. + Two things are worth doing whatever the division. **Fetch once, and fetch first**, deterministically, in [`../scripts/`](../scripts/) rather than in model judgement. That also **freezes the audit's point in time**: every later claim refers to the snapshot, so "events after the snapshot" becomes a stated property of the report rather than an unnoticed gap. Record the snapshot timestamp; never silently mix fresh API reads into a later phase. `/audit:issues` uses five phases, which suit an audit that must capture a whole tracker, check it item by item, and then write at length: @@ -44,7 +46,7 @@ Two things are worth doing whatever the division. **Fetch once, and fetch first* | Phase | Produces | Resumable from | |---|---|---| | 1. Snapshot | `meta.json`, `issues.json`, `prs.json`, `coverage.json` | — | -| 2. Verify | per-item findings with evidence tags | the snapshot | +| 2. Verify | per-item findings with evidence tags | the snapshot, plus its own partial output | | 3. Relate | the cross-link / parity graph | phase 2 | | 4. Write | the report | phases 2–3 | | 5. Self-audit | the coverage statement, folded back in | all of the above | diff --git a/audit/scripts/README.md b/audit/scripts/README.md index 1c33a3f..110d6be 100644 --- a/audit/scripts/README.md +++ b/audit/scripts/README.md @@ -12,7 +12,7 @@ python ${CLAUDE_PLUGIN_ROOT}/scripts/fetch_tracker.py OWNER/REPO --out /sna | Output | Contents | |---|---| -| `meta.json` | Repo, snapshot time (UTC), `gh` version, auth state, the fields requested | +| `meta.json` | Repo, snapshot time (UTC), `gh` version, the fetching account, the fields requested | | `issues.json` | Every issue, any state, each with its comment thread | | `prs.json` | Every PR, with comments, reviews, and `closingIssuesReferences` | | `coverage.json` | Items against `1..max`, discussion counts by open/closed — comments for issues, comments and reviews for PRs — truncation flag | diff --git a/audit/scripts/fetch_tracker.py b/audit/scripts/fetch_tracker.py index 1da8a86..0cfff79 100644 --- a/audit/scripts/fetch_tracker.py +++ b/audit/scripts/fetch_tracker.py @@ -15,7 +15,7 @@ Writes into `--out`: - meta.json repo, snapshot time (UTC), gh version, auth state, counts + meta.json repo, snapshot time (UTC), gh version, fetching account issues.json every issue, any state, with its full comment thread prs.json every PR, any state, with comments, reviews, and closing refs coverage.json the mechanical half of the coverage self-audit @@ -64,13 +64,7 @@ def run_gh(args): def preflight(repo): - """Fail in the first minute, not the third hour. - - Returns True. It only ever returns — every failure path exits — so the - caller can record authentication in the snapshot's provenance without - re-deriving it from `gh`'s human-readable output, which is free to change - wording or be localized. - """ + """Fail in the first minute, not the third hour.""" if shutil.which("gh") is None: die("the GitHub CLI (`gh`) is not on PATH. Install it, or take the " "unauthenticated route in references/quantecon-context.md — which " @@ -87,7 +81,23 @@ def preflight(repo): die(f"expected OWNER/REPO, got {repo!r}") # Confirms the repo exists and is visible to these credentials. run_gh(["repo", "view", repo, "--json", "name"]) - return True + + +def fetched_by(): + """The account the snapshot was taken as. + + Provenance that carries information, unlike a boolean "authenticated": + preflight exits on unauthenticated, so a flag could only ever record True. + The account matters because visibility is per-account — two snapshots of a + private repo taken by different people can legitimately differ, and the + only way to tell that from a truncation is to know who fetched. Non-fatal: + a failure here means `gh`'s user endpoint was unavailable, not that the + snapshot is unauthenticated, so it records null rather than exiting. + """ + proc = subprocess.run( + ["gh", "api", "user", "--jq", ".login"], capture_output=True, text=True, + ) + return proc.stdout.strip() if proc.returncode == 0 else None def require_thread_objects(items, kind, field, gh_version): @@ -187,7 +197,7 @@ def main(): ) args = parser.parse_args() - authenticated = preflight(args.repo) + preflight(args.repo) out = Path(args.out) out.mkdir(parents=True, exist_ok=True) @@ -222,7 +232,7 @@ def main(): "repo": args.repo, "snapshot_utc": datetime.now(timezone.utc).isoformat(timespec="seconds"), "gh_version": gh_version, - "authenticated": authenticated, + "fetched_by": fetched_by(), "limit": args.limit, "issue_fields": ISSUE_FIELDS, "pr_fields": PR_FIELDS, diff --git a/audit/skills/issues/SKILL.md b/audit/skills/issues/SKILL.md index a3b0b21..9ac1818 100644 --- a/audit/skills/issues/SKILL.md +++ b/audit/skills/issues/SKILL.md @@ -34,6 +34,19 @@ Ask only when discovery is ambiguous — two plausible plan anchors, say — not Then, in the audited repo: the notes system, `CHANGELOG.md`, the latest release notes, and `AGENTS.md`/`CLAUDE.md`. +## Working directory + +Everything the run produces goes under `--out`, defaulting to `.audit/-/`: + +| Path | Written by | Holds | +|---|---|---| +| `snapshot/` | phase 1 | `meta.json`, `issues.json`, `prs.json`, `coverage.json` | +| `findings.md` | phase 2 | one entry per item, appended as each is verified | +| `links.md` | phase 3 | the cross-link graph | +| `01-…` `02-…` `03-…` `README.md` | phase 4 | the delivered bundle | + +**Append to the checkpoint as you go, not when the phase ends** ([doctrine §4](../../references/doctrine.md#4-surviving-a-long-run)). Phase 2 is the long one — a hundred items of item-by-item judgement — so it is the phase a run dies inside rather than between. On restart, read `findings.md` and resume at the lowest number in `issues.json` that has no entry there; re-verify the last entry rather than trusting a possibly truncated write. + ## Phase 1 — snapshot ```bash @@ -58,11 +71,13 @@ Per [doctrine §1](../../references/doctrine.md#1-what-makes-a-bulk-audit-trustw Sibling-repo checks belong here too: for QuantEcon, "resolved in a sibling" and "one step of a rollout" are the two most common wrong conclusions a single-repo audit reaches. +Write each item's finding to `findings.md` as it is verified, in the catalog entry format from [deliverables.md](../../references/deliverables.md#the-auditissues-bundle) — so phase 4 assembles the catalog rather than re-deriving it, and an interrupted run loses one item rather than the phase. + ## Phase 3 — relate Parse every `#N` in all issue **and** PR bodies, both directions. One parsing caveat: a range reference (`#169–#176`) matches its endpoints only, so check milestone membership before declaring the middle numbers orphaned. -Produce the cluster map, the table of missing links worth adding (duplicate pairs, origin↔carrier, complementary checks, family orphans), true orphans and over-dense hubs, and the external cross-link registry. +Produce, into `links.md`: the cluster map, the table of missing links worth adding (duplicate pairs, origin↔carrier, complementary checks, family orphans), true orphans and over-dense hubs, and the external cross-link registry. ## Phase 4 — tier and write @@ -78,6 +93,8 @@ Slot into the repo's existing plan; never invent a parallel one. Tier by repo ty Priority labels only for genuine outliers, a handful either way, and only if the policy provides them. Then write the bundle per [deliverables.md](../../references/deliverables.md), as `01-issue-triage-report.md`, `02-issue-catalog.md`, `03-issue-links.md`, `README.md`. +**Scale the bundle to the tracker.** Four documents suit a tracker big enough that the argument, the enumeration and the graph get in each other's way. Below roughly 30 open issues they do not: fold the catalog and the link graph into the report, keep the `README.md` index, and say in the coverage statement which shape was used. Padding a small audit into four files makes it *less* checkable, which is the one thing the shape exists to protect. + ## Phase 5 — self-audit Run [doctrine §5](../../references/doctrine.md#5-coverage-self-audit) against `coverage.json`: reconcile the counts, explain every unaccounted number, confirm threads were read on both the open and closed sides, and state the residue — inline review comments, GraphQL-only data, anything after the snapshot timestamp. Fold any change back into the documents rather than appending a correction.