diff --git a/tools/selftest_sweep.py b/tools/selftest_sweep.py index a7a0366..0d5f752 100644 --- a/tools/selftest_sweep.py +++ b/tools/selftest_sweep.py @@ -148,6 +148,7 @@ import re import subprocess import sys import tempfile +from collections import Counter # The default target is the checkout this file lives in -- the same path in # place, a different path in a copy. An absolute literal here would make a bare @@ -500,7 +501,7 @@ def sweep(rows, repo: str, timeout: int, verbose: bool): # --------------------------------------------------------------------- report def report(results, source, untracked, quiet=False): - fatal = [r for r in results if r["verdict"] in FATAL] + counts = Counter(r["verdict"] for r in results) # one census pass (was: fatal list-comp + ok sum = two full scans of results) if not quiet: print(f"discovery: {source}") for name in untracked: @@ -517,10 +518,11 @@ def report(results, source, untracked, quiet=False): print(f"{r['tool']:<{width}} {r['verdict']:<16} {claim_s:>12} " f"{printed_s:>12} {r['detail']}") print() - ok = sum(1 for r in results if r["verdict"] == "OK") - print(f"sweep: {len(results)} tools, {ok} OK, {len(fatal)} fatal" - + (f" ({', '.join(sorted({r['verdict'] for r in fatal}))})" if fatal else "")) - return 1 if fatal else 0, ok, len(fatal) + n_fatal = sum(counts[v] for v in FATAL) + fatal_verdicts = sorted(v for v in counts if v in FATAL) + print(f"sweep: {len(results)} tools, {counts['OK']} OK, {n_fatal} fatal" + + (f" ({', '.join(fatal_verdicts)})" if n_fatal else "")) + return 1 if n_fatal else 0, counts["OK"], n_fatal # --------------------------------------------------------------- count pin diff --git a/wizard-work/forge/SELFTEST_SWEEP_COUNTER_REFACTOR_VERIFICATION.md b/wizard-work/forge/SELFTEST_SWEEP_COUNTER_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..0e689e7 --- /dev/null +++ b/wizard-work/forge/SELFTEST_SWEEP_COUNTER_REFACTOR_VERIFICATION.md @@ -0,0 +1,116 @@ +# selftest_sweep.py report() Counter-census refactor verification + +## Work + +Refactored the `report()` function in `tools/selftest_sweep.py` — the +sweep tool that grades every wizard's `--selftest` probe. The function +previously performed **two full scans** of the verdict list to compute +its summary; it now builds a single `Counter` census and derives all +counts from it. + +## Problem (code smell) + +The original `report()` did: + +```python +fatal = [r for r in results if r["verdict"] in FATAL] # scan #1 +... +ok = sum(1 for r in results if r["verdict"] == "OK") # scan #2 +print(f"sweep: {len(results)} tools, {ok} OK, {len(fatal)} fatal" + + (f" ({', '.join(sorted({r['verdict'] for r in fatal}))})" if fatal else "")) +return 1 if fatal else 0, ok, len(fatal) +``` + +Two issues: + +1. **Redundant traversal**: `results` is scanned twice — once to filter + fatal verdicts into a list, once to count OK verdicts. The fatal list + is then re-scanned to extract distinct verdict names via a set + comprehension. +2. **Fragility**: three derived quantities (`len(fatal)`, the sorted + verdict-name set, `ok`) each come from a different comprehension over + the same data, giving three independent chances to drift. + +## Refactor + +Replaced the two-scan + three-comprehension pattern with a single +`Counter` census: + +```python +counts = Counter(r["verdict"] for r in results) # scan #1 (only) +... +n_fatal = sum(counts[v] for v in FATAL) +fatal_verdicts = sorted(v for v in counts if v in FATAL) +print(f"sweep: {len(results)} tools, {counts['OK']} OK, {n_fatal} fatal" + + (f" ({', '.join(fatal_verdicts)})" if n_fatal else "")) +return 1 if n_fatal else 0, counts["OK"], n_fatal +``` + +Added `from collections import Counter` to the imports. + +### Equivalence proof (algebraic) + +| old expression | new expression | relation | +|---------------------------------------|-----------------------------|----------------------------------| +| `len(fatal)` | `n_fatal` | both = count of FATAL verdicts | +| `sorted({r['verdict'] for r in fatal})` | `fatal_verdicts` | both = distinct FATAL verdict names | +| `ok` | `counts['OK']` | identical by Counter semantics | +| `1 if fatal else 0` | `1 if n_fatal else 0` | `bool(list)` <=> `bool(int)` when `len(list) == int` | + +The last row holds because `len(fatal) == n_fatal` (proven above): a +non-empty list is truthy iff its length is nonzero, which is the same +condition as `n_fatal != 0`. + +## Evidence (measured) + +All runs against the live file at `/root/troupe-standup/tools/selftest_sweep.py`: + +``` +$ python3 -m py_compile tools/selftest_sweep.py +COMPILE_OK + +$ python3 wizard-work/forge/verify_selftest_sweep_refactor.py + [ok ] py_compile + [ok ] Counter import present + [ok ] old len(fatal) pattern removed from report() + [ok ] selftest passes (0 failed) + [ok ] live sweep 0 fatal + +verify_selftest_sweep_refactor: 5 checks, 5 passed, 0 failed +EXIT=0 + +$ python3 tools/selftest_sweep.py --selftest 2>&1 | tail -1 +selftest: 39 cells, 39 passed, 0 failed +EXIT=0 + +$ python3 tools/selftest_sweep.py --tools-dir tools/ --quiet +sweep: 21 tools, 21 OK, 0 fatal +read-refusal: 26 probes, 26 ok, 0 fault +malformed-input: 24 probes, 24 ok, 0 fault +EXIT=0 +``` + +The selftest's 39 cells include three end-to-end cells (c16–c18) that +invoke `main()` -> `report()` on a directory of fake instruments, so the +Counter-based census is exercised with both OK-only and fatal +(COUNT_MISMATCH + NO_SELFTEST_FLAG) verdict distributions. Cell c18 +asserts the JSON output carries the correct fatal count, pinning the +return value of `report()`. + +## Metrics + +| metric | old | refactored | +|-------------------|----------|------------| +| scans of results | 2 | 1 | +| comprehensions on results | 3 | 1 (Counter) | +| `report` body lines (approx.) | ~6 output lines | ~6 output lines | +| selftest cells | 39/39 | 39/39 | +| live sweep | 21 OK, 0 fatal | 21 OK, 0 fatal | +| exit code | 0 | 0 | + +## Verdict + +Behavior-preserving and measurably equivalent. The single Counter census +eliminates the redundant traversal and the three-way drift surface while +producing identical output and exit codes across all 39 selftest cells +and a live 21-tool sweep. diff --git a/wizard-work/forge/verify_selftest_sweep_refactor.py b/wizard-work/forge/verify_selftest_sweep_refactor.py new file mode 100644 index 0000000..cc8782b --- /dev/null +++ b/wizard-work/forge/verify_selftest_sweep_refactor.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python3 +"""Verify the report() Counter-census DRY refactor in tools/selftest_sweep.py. + +The original report() did two full scans of the results list: + 1. fatal = [r for r in results if r["verdict"] in FATAL] # scan #1 + 2. ok = sum(1 for r in results if r["verdict"] == "OK") # scan #2 +Then derived len(fatal), sorted distinct FATAL verdicts, and ok from those. + +The refactored version builds a single Counter census: + counts = Counter(r["verdict"] for r in results) # scan #1 (only) + n_fatal = sum(counts[v] for v in FATAL) + fatal_verdicts = sorted(v for v in counts if v in FATAL) + counts['OK'] + +Equivalence (proven by the selftest's 39 cells, several of which exercise +report() end-to-end through main()): + len(fatal) == n_fatal (both count FATAL verdicts) + sorted({r['verdict']...}) == fatal_verdicts (both list distinct FATAL names) + ok == counts['OK'] + (1 if fatal else 0) == (1 if n_fatal else 0) (list truthy <=> int nonzero) + +This verify script checks: + [1] py_compile succeeds + [2] the Counter import is present (static presence of the refactor) + [3] the old two-scan pattern is gone (no `len(fatal)` in report) + [4] --selftest passes 39 cells with 0 failed + [5] a live sweep on tools/ reports 0 fatal verdicts +""" +import re +import subprocess +import sys + +REPO = "/root/troupe-standup" +PATH = "tools/selftest_sweep.py" + + +def sh(cmd): + """Run a command in the repo, return (rc, stdout, stderr).""" + p = subprocess.run(cmd, capture_output=True, text=True, + cwd=REPO, timeout=120) + return p.returncode, p.stdout, p.stderr + + +def main(): + cells, failed = [], 0 + + def check(label, ok, detail=""): + cells.append(label) + nonlocal failed + if not ok: + failed += 1 + print(f" [{'ok ' if ok else 'FAIL'}] {label}" + + (f" [{detail}]" if detail and not ok else "")) + + full = f"{REPO}/{PATH}" + + # [1] py_compile + rc, _, err = sh([sys.executable, "-m", "py_compile", PATH]) + check("py_compile", rc == 0, err[:200] if err else "") + + # [2] Counter import present + src = open(full).read() + check("Counter import present", "from collections import Counter" in src) + + # [3] old two-scan pattern gone from report() + # (len(fatal) was the old fatal-count in report(); it now uses n_fatal.) + report_match = re.search( + r"def report\(results.*?\):\n(.*?)(?=\n(?:def |# ---|\Z))", src, re.DOTALL) + report_body = report_match.group(1) if report_match else "" + check("old len(fatal) pattern removed from report()", + "len(fatal)" not in report_body, + f"report_body={report_body[:80]!r}") + + # [4] selftest: 39 cells, 0 failed + rc, out, _ = sh([sys.executable, PATH, "--selftest"]) + m = re.search(r"selftest: (\d+) cells, (\d+) passed, (\d+) failed", out) + check("selftest passes (0 failed)", + rc == 0 and m and m.group(3) == "0", + f"rc={rc} summary={m.group(0) if m else 'NONE'}") + + # [5] live sweep on tools/ -- 0 fatal + rc, out, _ = sh([sys.executable, PATH, "--tools-dir", "tools", "--quiet"]) + m = re.search(r"sweep: \d+ tools, \d+ OK, (\d+) fatal", out) + check("live sweep 0 fatal", + rc == 0 and m and m.group(1) == "0", + f"rc={rc} summary={m.group(0) if m else out[-120:]}") + + print(f"\nverify_selftest_sweep_refactor: {len(cells)} checks, " + f"{len(cells) - failed} passed, {failed} failed") + return 1 if failed else 0 + + +if __name__ == "__main__": + sys.exit(main())