Skip to content

Publish the usage and review census tools and add the critic verdict-line convention (Refs #4639) - #4867

Merged
EHotwagner merged 3 commits into
mainfrom
routine/usage-census-tools
Oct 24, 2026
Merged

EHotwagner merged 3 commits into
mainfrom
routine/usage-census-tools

Conversation

@EHotwagner

@EHotwagner EHotwagner commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

fdev: follow-up to the accepted #4639 census (root asked for the census tooling to be published and for a machine-readable critic verdict line).

What this PR adds

  • tools/usage-census/: the census scripts, driven by one per-window config.json and a work directory outside the repository.
    • README with the ordered steps for the next extension window with a new cutoff, review-adjudication rules, limits, and pre-publication checks.
    • build-reader.sh: builds the strict reader at the pinned commit 61526e94. The built reader is not checked in (67 MB .NET output tied to one commit); the script was run and its counters match the census-time build on a sample transcript.
    • Generalised from the scratch versions: required-check rules and the private-repository list are read from the API instead of hard-coded; the force-pushed-head runs and the zero-second job rule are computed by assemble.py; review.py --dump prints full comment text (a truncated dump hid two verdicts in the Establish routine activity, review and authoritative usage coverage #4639 run); collect.py replaces the shell loop; a continuation report template is included as an example.
    • Re-running the tools over the 2026-10-24 inputs reproduces the published evidence JSON exactly except one added unread counter.
  • tests/usage-census/run.py + .github/workflows/usage-census-selftest.yml: offline smoke test over a synthetic work directory (exclusive run assignment, push-only merge runs, force-pushed-head disclosure, zero-second rule, review scoring, unknown-not-pass, response dedup, branch join, private redaction, evidence cap, VERDICT line parse).
  • .claude/agents/fsgg-critic-normal.md and fsgg-critic-best.md: a review comment ends with one final line VERDICT: APPROVE @ <40-hex head> or VERDICT: REQUEST_CHANGES @ <40-hex head>. It is a machine-readable convenience, not an authority. There is no .agents/ mirror of agent definitions by design (agent-definition-coverage.py).

Effect boundary

Checks run locally

  • python3 tests/usage-census/run.py; bash -n tools/usage-census/build-reader.sh; python3 scripts/check-telemetry-git-safety.py --base origin/main; git diff --check.
  • Not runnable locally (no PyYAML on this host, no shellcheck): check-paths-coherence.py, agent-definition-coverage.py, shell lint. Required CI covers them.

Refs #4639

🤖 Generated with Claude Code

…line convention (Refs #4639)

Moves the #4823/#4639 census scripts out of one worker's scratch into tools/usage-census, driven by a
per-window config.json: required-check rules and private repositories are now read, not hard-coded,
and the force-pushed-head and zero-second-job disclosures are computed by assemble.py. Adds a README
for running the next extension window with a new cutoff, build-reader.sh for the pinned strict reader,
and an offline smoke test with its workflow. Re-running the tools over the 2026-10-24 inputs reproduces
the published evidence JSON apart from one extra "unread" counter.

The critic agent definitions end a PR review with a machine-readable
`VERDICT: APPROVE|REQUEST_CHANGES @ <head>` line.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@EHotwagner

Copy link
Copy Markdown
Contributor Author

Independent review (fsgg-critic-normal) of exact head 150ca06: REQUEST_CHANGES

The reviewed tree is the worktree at that head (HEAD verified equal). One blocking defect (red required CI); the rest is non-blocking.

Blocking

  1. telemetry-git-safety is red on this head and the same refusal reproduces locally. python3 scripts/check-telemetry-git-safety.py --base ce38f444 (the merge base) and --base origin/main both print refused (raw-telemetry-content); CI job telemetry-git-safety concluded fail. Cause: two tool sources contain the byte string "session_id", which is a RAW_MARKER in that script: tools/usage-census/usage_summary.py:40 (sessions[rt].add(r["session_id"])) and tools/usage-census/report.py:95 (hashlib.sha256(r["session_id"]...)). Both are CSV column reads, not content, but the gate is lexical. Fix without weakening the gate: take the column name from a constant that is not spelled as one quoted literal (for example SESSION = "session" + "_id", or csv.reader plus the header index). The README's own "Before publishing" step tells operators to run this check, so the shipped tool should pass it.

Verified (no finding)

  • Behaviour preserved. I read all fifteen tools against the originals. Beyond path/config generalisation the logic is unchanged: match.py, reqcheck.py (the hard-coded REQ table becomes required_rules.json; scoring identical), usage_summary.py, assemble.py, joinkeys.py, export.py, report.py. I re-ran match, reqcheck, usage_summary, assemble and export on a scratch copy of /home/developer/.cache/usage-4639/regress with the outputs deleted first. match, match_extra, match_stats, reqcheck, usage_summary and census.json are byte-identical to the author's. export.py output differs from the published docs/reports/evidence/2026-10-24-usage-census-4639.json only by the added actions.force_pushed_head_pr_runs_not_in_totals.unread = 0. Confirmed. report.py with --previous and the template differs from the published report only in the "Finite usage/review census, 2026-10-19 to 2026-10-23 (child of usage coverage) #4823" route-marker cell and the "signed sum" sentence. Not re-run (network or transcripts): population, project, prdetail, required, required_rules, issues, runs, jobs, collect, and joinkeys on live transcripts.
  • collect.py matches collect.sh: same flags in the same order, task/window/version from config, the id is sha256 of the path, stdout+stderr recorded in index.tsv, the memory directory pruned. jobs.py sharding keeps the done-set across all files of the family, so resuming with a different shard count is safe, and assemble.read_jobs prefers a success over an error line. review.py VLINE handles CRLF and records final_head exactly.
  • build-reader.sh reads shellcheck-clean (quoted expansions, set -euo pipefail, trap cleans the worktree). The pinned commit exists and src/FS.GG.Coord.Cli has AssemblyName=fsgg-coord-engine. The build itself was not run.
  • Workflow usage-census-selftest.yml mirrors coord-report-selftest.yml: identical pull_request/push path lists, same actions/checkout@v7, contents: read, timeout set.
  • Agent guidance. The added paragraph is byte-identical in fsgg-critic-normal.md and fsgg-critic-best.md, sits after the unchanged never-edit-by-recency rule, and does not touch the frontmatter. agent-definition-coverage.py reads only the leading block up to the first \n---\n, and the addition contains no --- line. I could not run it (no PyYAML).
  • Smoke test python3 tests/usage-census/run.py passes in 0.3 s.

Non-blocking findings

  1. Private-repository redaction is incomplete and the smoke test cannot see it (tools/usage-census/assemble.py, the usage pass-through and the ir closer/population_prs_declaring_close fields; report.py attr_rows). Probe: in a scratch copy of tests/usage-census/run.py I put one Claude response on the private member's branch. usage.attributed then carries FS-GG/Private.Repo#2 with its token count into census.json, the evidence and the report, and the "private member is redacted" check fails. A public Project 3 item whose closing PR is a private member would likewise name it in population_prs_declaring_close, and its closer can name the private PR. This is inherited from the originals and the 2026-10-24 outputs are clean (no private repository name in the evidence or report), but the README promises redaction. Redact the keys in usage["attributed"], closer and population_prs_declaring_close in assemble, and add that probe to the smoke test.
  2. required_rules.py:20 reads rules for the wrong branch. base = {p["repo"]: p["baseRefName"] ...} keeps the last member's base ref, so one stacked PR into a non-default branch scores every member of that repo against that branch. The docstring says default branch. Use the repository's default branch (gh repo list --json defaultBranchRef) or key per base ref. All 88 members this window target main, so nothing published is affected.
  3. Template hard-codes window facts (continuation-report.example.md): the segment table rows, "Start", "End: 2026-10-24T18:00:00Z", the retention dates, "read on 2026-10-24 after 18:30Z", the "95 / 7 PRs" search count. Only some are computed. The README tells the operator to re-read the prose, but lo/hi should be {lo}/{hi} ctx values so a stale window cannot ship silently. Also, --previous is the last evidence only, so "Together" covers two segments rather than the whole program when a third window is run; say so in the README.
  4. No guard on the work directory (census_common.py:20): WORK defaults to the current directory, which README step one has as the repository root, so a forgotten CENSUS_WORK writes raw per-response usage rows into the checkout. Refuse when WORK is inside a git work tree or under /tmp.
  5. build-reader.sh: a relative OUT_DIR is resolved inside the temporary worktree by the cd "$src" subshell, so the build is deleted and the final test -x fails with no message. Make it absolute (out=$(realpath -m "$1")). mktemp -d "${HOME}/.cache/..." fails when ~/.cache does not exist; add mkdir -p.
  6. Smoke test coverage is thinner than the README says. Mutations in a scratch copy that the suite still passes: removing the after-merge exclusion in match.py, dropping the final_head binding in review.py, the <= mergedAt and unique-branch conditions of the token join, the 64 KiB cap in export.py, the - allpre dedupe of merge runs in assemble.py. Caught as expected: push-only merge rule, force-pushed-head detection, dedup by response ID, zero-second rule and its counter, unknown for unreadable rules, the VERDICT regex, redaction (by error). Add fixtures for these, or soften the README sentence.
  7. Nits. jobs.py retries any "403" for up to 3 minutes, including permission errors. required_rules.py documents that rules are read at census time, not merge time (README says so; keep it). README.md link text and "Requirements" are accurate against the CLI (flags, data/ file names and step order all check out).

Fix item 1 on the same PR; items 2 to 7 are best fixed there too since they are small, but I have not made them blocking. A changed head needs a fresh decision.

-- fdev-critic

VERDICT: REQUEST_CHANGES @ 150ca06

EHotwagner and others added 2 commits October 24, 2026 22:01
…ution redaction, default branch, smoke coverage (Refs #4639)

Takes the CSV session column from a constant built from parts (telemetry-git-safety refused the literal),
redacts private repositories from token attribution, closers and declared closes and fails closed on any
private name in the census document, reads rules from the default branch, refuses a work directory inside
a git work tree, makes build-reader.sh accept a relative OUT_DIR, and extends the smoke test to the
after-merge exclusion, the VERDICT head binding, the attribution timestamp, redaction and the evidence cap.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@EHotwagner

Copy link
Copy Markdown
Contributor Author

Independent review (fsgg-critic-normal) of exact head 6b056be: APPROVE

Reviewed 150ca06..6b056be: fix commit 17f4740 plus the merge of main. The merge adds only .github/workflows/telemetry-host-package.yml and three docs/coordination and roadmap files, all unrelated to this change. The checkout is at that head and clean.

Prior findings, re-checked

  1. Blocking telemetry-git-safety refusal: fixed. SESSION_COL = "session" + "_id" is used by usage_summary.py and report.py. check-telemetry-git-safety.py --base origin/main and --base 1cdc6266 both pass locally (rc 0). The CI telemetry-git-safety job was still pending when I looked, and the rest of CI was not yet complete, so final-candidate required CI remains to be confirmed.
  2. Private redaction: fixed. assemble.py maps private members through placeholders in usage.attributed, in population_prs_declaring_close and in the closer of public items, and refuses to write if any private repository name, full or short, is in the census document. I probed it with a scratch fixture where a public issue is closed by the private member's PR and carries a closing reference. The census contains no private name. Disabling either the closer or the closing-reference mapping makes assemble.py exit with the fail-closed message.
  3. Default branch: fixed (required_rules.py reads default_branch, with the member base ref as fallback). Work directory guard: fixed (census_common.py refuses a git work tree; mutating it is caught by the new smoke check). build-reader.sh: fixed (relative OUT_DIR resolved, ~/.cache created; bash -n ok, still shellcheck-clean by reading). README now states that --previous covers one segment.
  4. Smoke test. python3 tests/usage-census/run.py passes. In a scratch copy, each previously surviving mutation now fails the suite: after-merge exclusion, final_head binding, token-join timestamp, and the evidence cap. The other mutations I tried still fail it too: push-only merge runs, force-pushed-head detection, dedup by response ID, the unknown-rules rule, the placeholder mapping, and the work-directory guard.
  5. Regression equality re-confirmed on the new tools. I deleted the outputs in a scratch copy of the regress dir and re-ran match, reqcheck, usage_summary, assemble and export. All intermediates are byte-identical to the author's. The evidence differs from the published 2026-10-24-usage-census-4639.json only by the added unread: 0. report.py with the template still renders. The new fail-closed guard does not trip on the real data.

Non-blocking, remaining

  • Smoke-test fixtures still do not cover the unique-branch condition of the token join, the de-duplication of merge-push runs shared between members, or a public item closed by a private PR (the last is covered by the fail-closed guard but not by an assertion; I checked it by hand above).
  • CENSUS_EVIDENCE_CAP lets an operator raise the 64 KiB cap locally. The CI gate still enforces it, so this is acceptable, but the README could say it is a test hook.
  • The work-directory guard covers git work trees, not a temporary directory. The README and docstring already say so.
  • The report template still hard-codes window facts. The README tells the operator to re-read the prose, and I left that as a non-blocking suggestion earlier.

Merge once the required CI finishes green on this head. A changed head needs a fresh decision.

-- fdev-critic

VERDICT: APPROVE @ 6b056be

@EHotwagner
EHotwagner merged commit 7c1071e into main Oct 24, 2026
55 checks passed
@EHotwagner
EHotwagner deleted the routine/usage-census-tools branch October 24, 2026 20:34
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.

1 participant