From 86950bc00a3d3c2b47b83865af0f296b24c9e2b0 Mon Sep 17 00:00:00 2001 From: forge Date: Tue, 20 Oct 2026 20:53:27 +0000 Subject: [PATCH] forge: doc_command_check report() tally loop -> Counter (DRY, behavior-preserving) --- tools/doc_command_check.py | 9 +- ...OMMAND_REPORT_DRY_REFACTOR_VERIFICATION.md | 80 +++++++ .../forge/verify_doc_command_report_dry.py | 204 ++++++++++++++++++ 3 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 wizard-work/forge/DOC_COMMAND_REPORT_DRY_REFACTOR_VERIFICATION.md create mode 100644 wizard-work/forge/verify_doc_command_report_dry.py diff --git a/tools/doc_command_check.py b/tools/doc_command_check.py index 33b6749..f9b41cd 100644 --- a/tools/doc_command_check.py +++ b/tools/doc_command_check.py @@ -69,6 +69,7 @@ import re import subprocess import sys import tempfile +from collections import Counter # A documented command: `python3 .py `. The path may be written # as `tools/x.py` from the repo root or as `x.py` from inside `tools/`, which @@ -228,9 +229,11 @@ def report(rep, out=sys.stdout): miss = (" missing=%s" % (r["missing"],)) if r["missing"] else "" print("%-18s %-30s -> %-26s %s%s" % (r["verdict"], r["doc"], r["target"], r["detail"], miss), file=out) - counts = {} - for r in rep["rows"]: - counts[r["verdict"]] = counts.get(r["verdict"], 0) + 1 + # `collections.Counter` replaces the hand-rolled tally loop: every entry's + # verdict is counted exactly as `counts[k] = counts.get(k, 0) + 1` did, and + # `sorted(counts)` still orders the keys, so the rendered summary is + # byte-identical -- this is a deletion of a duplicate loop, not a rewrite. + counts = Counter(r["verdict"] for r in rep["rows"]) print("doc commands at %s: %d command(s) over %d oracle(s) -- %s" % (rep["rev"], len(rep["rows"]), rep["oracles"], ", ".join("%s %d" % (k, counts[k]) for k in sorted(counts)) or "none")) diff --git a/wizard-work/forge/DOC_COMMAND_REPORT_DRY_REFACTOR_VERIFICATION.md b/wizard-work/forge/DOC_COMMAND_REPORT_DRY_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..173af4c --- /dev/null +++ b/wizard-work/forge/DOC_COMMAND_REPORT_DRY_REFACTOR_VERIFICATION.md @@ -0,0 +1,80 @@ +# forge: doc_command_check report() -- replace hand-rolled tally loop with Counter (DRY) + +Target: `tools/doc_command_check.py` +Lane: Refactor (code review and refactors) +Status: DONE -- verified equivalent, behavior preserved. + +## The smell + +`report()` counted verdicts with a hand-rolled loop: + + counts = {} + for r in rep["rows"]: + counts[r["verdict"]] = counts.get(r["verdict"], 0) + 1 + +That is the exact same tally shape `tool_map.collect()` had before the +tool_map DRY -- a `counts.get(name, 0) + 1` loop the house already replaced +with a single-counting abstraction. `report()` is the lone remaining instance +in `doc_command_check.py`, and it is the only place in the file that counts by +hand. + +## The fix (behavior-preserving) + +Replace the loop with `collections.Counter`, the count dict is consumed only +as `sorted(counts)` + `counts[k]`, both of which Counter satisfies identically +(Counter is a `dict` subclass; only keys that occurred appear; a missing key +returns `0` but no key is ever missing because the loop is over its own keys). + + - counts = {} + - for r in rep["rows"]: + - counts[r["verdict"]] = counts.get(r["verdict"], 0) + 1 + + counts = Counter(r["verdict"] for r in rep["rows"]) + +`report()` is the ONLY function touched. `census()`, `documented_commands()`, +`resolve_target()`, `help_options()`, and `selftest()` are byte-identical to +their pre-refactor form. The full module `git diff` is two hunks: the +`from collections import Counter` import and the three-line collapse above. + +## Evidence + +`wizard-work/forge/verify_doc_command_report_dry.py` exercises three verdicts. +All green (exit 0): + + [1] SELFTEST DRIFT GUARD -- `doc_command_check.py --selftest` on BOTH the + original (git HEAD blob, run under its real name) and the refactored + (working tree): + original selftest rc=0 sha=36f0f40814b819f162302a56cd699682... PASS + refactored selftest rc=0 sha=36f0f40814b819f162302a56cd699682... PASS + stdout is byte-identical (sha256 match) to the captured baseline and to + each other -- 15 cells, 15 passed, 0 failed. (The original is run under + the name `doc_command_check.py` because its selftest is self-referential + via `__file__`'s basename; a randomly-named temp file falsely fails those + cells for a harness reason, which the verify script avoids.) + + [2a] MICRO EQUIVALENCE -- on the LIVE census at HEAD, the old inline loop and + the new Counter() produce identical count dicts: + live tally: old loop == Counter PASS counts={'OK': 58} + + [2b] report() byte-identity on synthetic reps covering every branch the + function cares about: + report(synthetic [mixed]) identical PASS (FLAG_NOT_ACCEPTED/NO_ORACLE/OK/TOOL_MISSING) + report(synthetic [all_ok (none fallback)]) PASS (the `-- none` arm) + report(synthetic [empty]) PASS (empty rows -> "0 command(s)... none") + + [3] REPORT OUTPUT DRIFT -- report() on the live census(rep), original vs + refactored, byte-identical: + report(live census) identical PASS bytes=63 + +Note: `report()`'s summary `print()` writes to `sys.stdout` (not the `out=` +parameter the fault-row prints use), so the verify script redirects stdout +into the same buffer as `out` to capture the full text -- including the summary +line whose counts are what this refactor changes. The capture is why [3] +measures 63 bytes rather than 0. + +## Exit contract + +Behavior-preserving refactor. rc 0 on both the original and refactored +`--selftest`, stdout byte-identical (sha256 match), and `report()` output +byte-identical on both synthetic and live census data. No public interface, +argument, or output format changed; the summary line ordering +(`sorted(counts)`) and the `-- none` fallback are unchanged. diff --git a/wizard-work/forge/verify_doc_command_report_dry.py b/wizard-work/forge/verify_doc_command_report_dry.py new file mode 100644 index 0000000..7a2f42a --- /dev/null +++ b/wizard-work/forge/verify_doc_command_report_dry.py @@ -0,0 +1,204 @@ +#!/usr/bin/env python3 +"""verify_doc_command_report_dry.py -- equivalence proof for the report() DRY. + +The refactor replaced the hand-rolled tally loop in tools/doc_command_check.py's +report() with collections.Counter: + + - counts = {} + - for r in rep["rows"]: + - counts[r["verdict"]] = counts.get(r["verdict"], 0) + 1 + + counts = Counter(r["verdict"] for r in rep["rows"]) + +That is behavior-preserving iff report() emits byte-identical text before and +after, because report() is the ONLY function touched, and every other path +(census(), documented_commands(), resolve_target(), help_options()) is unchanged. + +Three verdicts, all must be green: + + 1. SELFTEST DRIFT GUARD -- `doc_command_check.py --selftest` exits 0 on BOTH + the original (git HEAD blob, run under its real name) and the refactored + (working tree), and their stdout is byte-identical (sha256 match) to the + captured baseline. The selftest's 15-cell fixture corpus exercises the + counting branch through report(). + + 2. MICRO EQUIVALENCE -- on the LIVE census at HEAD, the old inline tally loop + and the new Counter() produce identical count dicts, AND report() on every + synthetic rep (mixed verdicts, all-OK, empty) is byte-identical between the + original and refactored modules. + + 3. REPORT OUTPUT DRIFT -- report() on the live census(rep) is byte-identical + between original and refactored: the changed function's rendered text did + not move by one byte. + +Exit code is the verdict: 0 = all green, 1 = any mismatch. +""" +import hashlib +import importlib.util +import io +import os +import subprocess +import sys +import tempfile +from collections import Counter + +REPO = "/root/troupe-standup" +DCC = f"{REPO}/tools/doc_command_check.py" +# Captured from the original (pre-refactor) source at git HEAD, run under its +# real name `doc_command_check.py`: +# python3 tools/doc_command_check.py --selftest +BASELINE_SELFTEST_SHA = "36f0f40814b819f162302a56cd6996820e647ffba255f4f821ce19dc9f3815d2" + + +def load_module(path, name): + spec = importlib.util.spec_from_file_location(name, path) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def run_selftest(src_path, label): + rc = subprocess.run([sys.executable, src_path, "--selftest"], + capture_output=True, text=True) + out = rc.stdout or "" + sha = hashlib.sha256(out.encode()).hexdigest() + ok = rc.returncode == 0 and sha == BASELINE_SELFTEST_SHA + print(f"[1] {label:10s} selftest rc={rc.returncode} sha={sha[:16]}... " + f"{'PASS' if ok else 'FAIL'}") + if not ok: + print(f" expected sha={BASELINE_SELFTEST_SHA[:16]}...") + if rc.stderr: + print(" --- stderr ---") + print(" " + rc.stderr.rstrip().replace("\n", "\n ")) + return ok + + +def old_tally(rep): + """The three-line loop, verbatim from the pre-refactor source.""" + counts = {} + for r in rep["rows"]: + counts[r["verdict"]] = counts.get(r["verdict"], 0) + 1 + return counts + + +def report_text(mod, rep): + """All of report()'s text. Fault-row prints honour `out=`, but the summary + print() hard-wires sys.stdout, so BOTH are captured by redirecting stdout + into the same buffer handed in as `out`.""" + buf = io.StringIO() + old = sys.stdout + sys.stdout = buf + try: + mod.report(rep, out=buf) + finally: + sys.stdout = old + return buf.getvalue() + + +def synthetic_reps(): + """Cover every branch report() cares about: fault rows + multi-key counts.""" + mixed = { + "rev": "HEAD", "oracles": 3, "rows": [ + {"doc": "tools/a.py", "target": "tools/a.py", "verdict": "OK", + "missing": [], "detail": ""}, + {"doc": "tools/a.py", "target": "gone.py", "verdict": "TOOL_MISSING", + "missing": [], "detail": "no such file at HEAD"}, + {"doc": "tools/a.py", "target": "tools/a.py", + "verdict": "FLAG_NOT_ACCEPTED", "missing": ["--gone"], "detail": ""}, + {"doc": "tools/b.py", "target": "tools/b.py", "verdict": "NO_ORACLE", + "missing": [], "detail": "no --help output"}, + {"doc": "tools/a.py", "target": "tools/a.py", "verdict": "OK", + "missing": [], "detail": ""}, + ], + } + all_ok = { + "rev": "HEAD", "oracles": 0, "rows": [ + {"doc": "tools/a.py", "target": "tools/a.py", "verdict": "OK", + "missing": [], "detail": ""}, + {"doc": "tools/a.py", "target": "tools/b.py", "verdict": "OK", + "missing": [], "detail": ""}, + ], + } + empty = {"rev": "HEAD", "oracles": 0, "rows": []} + return [("mixed", mixed), ("all_ok (none fallback)", all_ok), + ("empty", empty)] + + +def main(): + all_ok = True + + # Original source blob at HEAD, materialized under its real name. The + # original's own selftest is self-referential via `__file__`'s basename + # (its docstring-against-itself cells), so the temp file MUST be named + # doc_command_check.py or those cells fail for a harness reason, not a + # behavior one. + orig_src = subprocess.run( + ["git", "-C", REPO, "show", "HEAD:tools/doc_command_check.py"], + capture_output=True, text=True).stdout + if not orig_src: + print("[1] could not read HEAD:tools/doc_command_check.py -- FAIL") + return 1 + orig_dir = tempfile.mkdtemp(prefix="dcc_orig_") + orig_path = os.path.join(orig_dir, "doc_command_check.py") + with open(orig_path, "w") as fh: + fh.write(orig_src) + os.chmod(orig_path, 0o755) + + tm_orig = load_module(orig_path, "dcc_orig") + tm_ref = load_module(DCC, "dcc_ref") + + # [1] selftest drift guard: both versions, byte-identical to baseline. + o_ok = run_selftest(orig_path, "original") + r_ok = run_selftest(DCC, "refactored") + selftest_ok = o_ok and r_ok + all_ok = all_ok and selftest_ok + + # [2a] micro equivalence: live census tally, old loop == new Counter. + live = tm_ref.census(REPO, "HEAD") + old_counts = old_tally(live) + new_counts = Counter(r["verdict"] for r in live["rows"]) + micro_ok = old_counts == new_counts + print(f"[2a] live tally: old loop == Counter " + f"{'PASS' if micro_ok else 'FAIL'} counts={dict(new_counts)}") + if not micro_ok: + all_ok = False + print(f" old={dict(old_counts)} new={dict(new_counts)}") + + # [2b] report() byte-identity on synthetic reps (every verdict branch). + for label, rep in synthetic_reps(): + a = report_text(tm_orig, rep) + b = report_text(tm_ref, rep) + eq = a == b + print(f"[2b] report(synthetic [{label}]) identical " + f"{'PASS' if eq else 'FAIL'}") + if not eq: + all_ok = False + la, lb = a.splitlines(), b.splitlines() + for i in range(max(len(la), len(lb))): + ga = la[i] if i < len(la) else "" + gb = lb[i] if i < len(lb) else "" + if ga != gb: + print(f" diff at line {i+1}: orig={ga!r} ref={gb!r}") + break + + # [3] report() byte-identity on the LIVE census(rep): end-to-end of the + # touched function against the real tree. + ro = report_text(tm_orig, live) + rr = report_text(tm_ref, live) + live_ok = ro == rr + print(f"[3] report(live census) identical " + f"{'PASS' if live_ok else 'FAIL'} bytes={len(rr)}") + if not live_ok: + all_ok = False + print(" --- original (first 600 chars) ---") + print(" " + ro[:600].replace("\n", "\n ")) + print(" --- refactored (first 600 chars) ---") + print(" " + rr[:600].replace("\n", "\n ")) + + print("\nVERDICT:", + "ALL GREEN -- report() behavior preserved" if all_ok + else "MISMATCH -- behavior NOT preserved") + sys.exit(0 if all_ok else 1) + + +if __name__ == "__main__": + main()