diff --git a/tools/tool_map.py b/tools/tool_map.py index 7a589f2..339b71a 100644 --- a/tools/tool_map.py +++ b/tools/tool_map.py @@ -210,6 +210,22 @@ def flags_of(repo, rev, name): return out, "" +def _count_by_tool(entries): + """Count probes per tool name (first element of each entry tuple). + + All three probe tables share the same shape for tallying: each entry's + first element is the tool name, and the rest is payload the tally ignores. + READ_PROBES / NOT_UTF8_PROBES entries are (tool, argv); + MALFORMED_PROBES entries are (tool, label, ...). This helper replaces the + three identical loops that previously counted each table by hand. + """ + counts = {} + for entry in entries: + name = entry[0] + counts[name] = counts.get(name, 0) + 1 + return counts + + def collect(repo, rev): """The whole map as data: one dict per tool plus the gate's legs.""" names = tracked_tools(repo, rev) @@ -218,13 +234,9 @@ def collect(repo, rev): sweep = literal_assignments(repo, rev, "tools/selftest_sweep.py", {"READ_PROBES", "NOT_UTF8_PROBES", "MALFORMED_PROBES"}) - read_n, utf8_n, mal_n = {}, {}, {} - for name, _argv in sweep.get("READ_PROBES", ()): # (tool, argv) - read_n[name] = read_n.get(name, 0) + 1 - for name, _argv in sweep.get("NOT_UTF8_PROBES", ()): - utf8_n[name] = utf8_n.get(name, 0) + 1 - for name, *_rest in sweep.get("MALFORMED_PROBES", ()): # (tool, label, ...) - mal_n[name] = mal_n.get(name, 0) + 1 + read_n = _count_by_tool(sweep.get("READ_PROBES", ())) + utf8_n = _count_by_tool(sweep.get("NOT_UTF8_PROBES", ())) + mal_n = _count_by_tool(sweep.get("MALFORMED_PROBES", ())) legs = literal_assignments(repo, rev, "tools/verify.py", {"LEGS"}).get("LEGS", ()) leg_of = {} for leg in legs: diff --git a/wizard-work/forge/TOOL_MAP_DRY_REFACTOR_VERIFICATION.md b/wizard-work/forge/TOOL_MAP_DRY_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..af2966d --- /dev/null +++ b/wizard-work/forge/TOOL_MAP_DRY_REFACTOR_VERIFICATION.md @@ -0,0 +1,116 @@ +# forge: tool_map DRY -- collapse three identical counting loops into _count_by_tool + +Target: `tools/tool_map.py` (`collect()`) +Lane: Refactor (code review and refactors) +Status: DONE -- verified behavior-preserving, drift guard green, output byte-identical. + +## The smell + +`collect()` read three probe tables from `selftest_sweep.py` and tallied each one +with a hand-rolled loop, all three doing exactly the same thing -- increment a +per-tool name count: + + read_n, utf8_n, mal_n = {}, {}, {} + for name, _argv in sweep.get("READ_PROBES", ()): # (tool, argv) + read_n[name] = read_n.get(name, 0) + 1 + for name, _argv in sweep.get("NOT_UTF8_PROBES", ()): + utf8_n[name] = utf8_n.get(name, 0) + 1 + for name, *_rest in sweep.get("MALFORMED_PROBES", ()): # (tool, label, ...) + mal_n[name] = mal_n.get(name, 0) + 1 + +The only variation was the unpacking shape: READ_PROBES / NOT_UTF8_PROBES entries +are 2-tuples `(tool, argv)`, MALFORMED_PROBES entries are 5-tuples +`(tool, label, argv, rc, substring)`. In every case the tally only needs +`entry[0]`. Three copies of "count the first element per tool name" is the smell. + +## The fix + +One helper, three call sites: + + def _count_by_tool(entries): + counts = {} + for entry in entries: + name = entry[0] + counts[name] = counts.get(name, 0) + 1 + return counts + + read_n = _count_by_tool(sweep.get("READ_PROBES", ())) + utf8_n = _count_by_tool(sweep.get("NOT_UTF8_PROBES", ())) + mal_n = _count_by_tool(sweep.get("MALFORMED_PROBES", ())) + +No public interface, argument, or output shape changed. The probe tables feed +`collect()` only via `read_n.get(name, 0)` / `not_utf8` / `malformed` per row, so +two count dicts that agree on every tool produce the same render and the same +`--json`. The helper is strictly more uniform than the loops: it indexes `entry[0]` +regardless of tuple width, which is exactly what all three loops already did. + +## Evidence + +Three independent verdicts, all green. The verifier is +`wizard-work/forge/verify_tool_map_dry_refactor.py`; run it (and the `CHECK:` +line below) to reproduce. + +### Check 1 -- the shipped drift guard (end-to-end) + +`python3 tools/tool_map.py --selftest` re-renders `docs/TOOLS.md` at HEAD and +compares it to the document on disk; the whole map (purpose, flags, probe counts, +gate legs) is byte-derived from the tree. Its currency cell, "HEAD: docs/TOOLS.md +is the render of this tree (the drift guard)", is the refactor's real contract: + + selftest: 16 cells, 16 passed, 0 failed + (drift guard cell: ok) + +### Check 2 -- direct equivalence, micro level + +Old inline loops vs new `_count_by_tool` on the LIVE probe tables parsed from +`selftest_sweep.py` (the exact data `collect()` consumes): + + READ_PROBES: {'append_only_audit.py': 1, 'append_only_owners.py': 1, + 'census_row_delta.py': 1, 'check_lint.py': 1, 'checkin_check.py': 1, + 'citation_hash_class.py': 1, 'citation_resolve_check.py': 1, + 'doc_census_check.py': 1, 'doc_command_check.py': 1, 'landing_check.py': 1, + 'queue_append_audit.py': 1, 'queue_claim_check.py': 1, 'queue_index.py': 1, + 'reply_check.py': 1, 'selftest_sweep.py': 1, 'tool_map.py': 1, 'verify.py': 1} + NOT_UTF8_PROBES: {'append_only_owners.py': 1, 'census_row_delta.py': 1, + 'checkin_check.py': 1, 'queue_claim_check.py': 1, 'queue_index.py': 1} + MALFORMED_PROBES: {'append_only_audit.py': 1, 'append_only_owners.py': 2, + 'census_row_delta.py': 2, 'check_lint.py': 2, 'checkin_check.py': 1, + 'citation_hash_class.py': 1, 'citation_resolve_check.py': 1, + 'doc_census_check.py': 1, 'doc_command_check.py': 1, 'landing_check.py': 1, + 'queue_append_audit.py': 1, 'queue_claim_check.py': 1, 'queue_index.py': 1, + 'reply_check.py': 1, 'selftest_sweep.py': 1, 'tool_map.py': 1, 'verify.py': 1} + +`old_read == new_read` AND `old_utf8 == new_utf8` AND `old_mal == new_mal` -- +identical, including the MALFORMED_PROBES table where `append_only_owners.py` +and `census_row_delta.py` each appear twice (two malformed-input contracts per +tool), proving the helper respects the `*_rest` discard shape. + +### Check 3 -- direct equivalence, full output + +`collect(repo, HEAD)` JSON-dumped with `sort_keys=True` from (a) the ORIGINAL +module (`git show HEAD:tools/tool_map.py`) and (b) the REFACTORED working-tree +module, both reading the SAME tree. The only code that differs between the two +is the counting logic, so an identical hash isolates the refactor as the sole +variable: + + original.sha256 21653a858a6968448aad7cc3b629c51e0dba3fb3885a21235b2fd0396d54ab41 + refactored.sha256 21653a858a6968448aad7cc3b629c51e0dba3fb3885a21235b2fd0396d54ab41 + => BYTE-IDENTICAL. + +### Check 4 -- the redundancy actually collapsed + +`grep -cE '\[name\] = .*\.get\(name, 0\) \+ 1'` (the tally pattern shared by all +three old loops, whose only per-iteration body is `NAME[name] = NAME.get(name, 0) + 1`): + + original (HEAD): 3 occurrences (read_n / utf8_n / mal_n, one per table) + refactored: 1 occurrence (inside _count_by_tool, accumulator renamed `counts`) + +No inline tallying loop remains in `collect()`; the only surviving count loop +lives in the helper. + +## Exit contract + +Behavior-preserving DRY refactor. `--selftest` 16/16; `collect()` JSON +byte-identical between the shipped module at HEAD and the refactored working +tree. No output changed. The probe counts flow unchanged into the rendered +`docs/TOOLS.md`, which the drift guard confirms stays current. diff --git a/wizard-work/forge/verify_tool_map_dry_refactor.py b/wizard-work/forge/verify_tool_map_dry_refactor.py new file mode 100644 index 0000000..de1d9b7 --- /dev/null +++ b/wizard-work/forge/verify_tool_map_dry_refactor.py @@ -0,0 +1,144 @@ +#!/usr/bin/env python3 +"""verify_tool_map_dry_refactor.py -- equivalence proof for the _count_by_tool DRY. + +The refactor replaced three hand-rolled counting loops in tools/tool_map.py's +collect() with a single _count_by_tool() helper. That is behavior-preserving +iff collect() on the ORIGINAL (git HEAD:tools/tool_map.py) and collect() on the +REFACTORED (working-tree) file, both run against the SAME repo+rev, emit +byte-identical data -- because the probe tables they tally are read by the +unchanged literal_assignments() from selftest_sweep.py, so the ONLY variable is +the counting logic itself. + +Two verdicts are produced, both must be green: + + 1. DRIFT GUARD -- `tool_map.py --selftest` exits 0 (its "docs/TOOLS.md is the + render of this tree" cell). This is the tool's own shipped contract: the + rendered map (which embeds the probe counts) is unchanged. + + 2. DIRECT EQUIVALENCE -- collect(repo, HEAD) JSON is byte-identical between + the original module and the refactored module (sha256 match), AND the + per-tool count dicts from the OLD inline loops vs the NEW helper are + identical on the live probe tables. + +Exit code is the verdict: 0 = both green, 1 = any mismatch. +""" +import hashlib +import importlib.util +import json +import subprocess +import sys +import tempfile + +REPO = "/root/troupe-standup" +TM = f"{REPO}/tools/tool_map.py" +SWEET = f"{REPO}/tools/selftest_sweep.py" + + +def run_selftest(): + rc = subprocess.run([sys.executable, TM, "--selftest"], + capture_output=True, text=True) + ok = rc.returncode == 0 + print(f"[1] selftest (drift guard) rc={rc.returncode} " + f"{'PASS' if ok else 'FAIL'}") + if not ok: + print(" --- selftest stdout ---") + print(" " + rc.stdout.rstrip().replace("\n", "\n ")) + if rc.stderr: + print(" --- selftest stderr ---") + print(" " + rc.stderr.rstrip().replace("\n", "\n ")) + return ok + + +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 old_counts(sweep): + """The three inline loops, verbatim from the pre-refactor code.""" + read_n, utf8_n, mal_n = {}, {}, {} + for name, _argv in sweep.get("READ_PROBES", ()): + read_n[name] = read_n.get(name, 0) + 1 + for name, _argv in sweep.get("NOT_UTF8_PROBES", ()): + utf8_n[name] = utf8_n.get(name, 0) + 1 + for name, *_rest in sweep.get("MALFORMED_PROBES", ()): + mal_n[name] = mal_n.get(name, 0) + 1 + return read_n, utf8_n, mal_n + + +def collect_json(mod): + data = mod.collect(REPO, "HEAD") + return json.dumps(data, sort_keys=True, indent=2) + + +def main(): + all_ok = True + + # [2a] micro-equivalence: old loops vs new helper on the LIVE probe tables + tm_ref = load_module(TM, "tm_ref") + sweep = tm_ref.literal_assignments( + REPO, "HEAD", "tools/selftest_sweep.py", + {"READ_PROBES", "NOT_UTF8_PROBES", "MALFORMED_PROBES"}) + old_read, old_utf8, old_mal = old_counts(sweep) + new_read = tm_ref._count_by_tool(sweep.get("READ_PROBES", ())) + new_utf8 = tm_ref._count_by_tool(sweep.get("NOT_UTF8_PROBES", ())) + new_mal = tm_ref._count_by_tool(sweep.get("MALFORMED_PROBES", ())) + micro_ok = (old_read == new_read and old_utf8 == new_utf8 + and old_mal == new_mal) + print(f"[2a] micro: old inline loops == _count_by_tool on live tables " + f"{'PASS' if micro_ok else 'FAIL'}") + if not micro_ok: + all_ok = False + for label, o, n in [("READ", old_read, new_read), + ("NOT_UTF8", old_utf8, new_utf8), + ("MALFORMED", old_mal, new_mal)]: + if o != n: + print(f" {label} MISMATCH old={o} new={n}") + + # [2b] full collect() equivalence: original (git HEAD) vs refactored + orig_src = subprocess.run( + ["git", "-C", REPO, "show", "HEAD:tools/tool_map.py"], + capture_output=True, text=True).stdout + if not orig_src: + print("[2b] could not read HEAD:tools/tool_map.py -- FAIL") + all_ok = False + else: + with tempfile.NamedTemporaryFile("w", suffix="_tm_orig.py", + delete=False) as fh: + fh.write(orig_src) + orig_path = fh.name + tm_orig = load_module(orig_path, "tm_orig") + j_orig = collect_json(tm_orig) + j_ref = collect_json(tm_ref) + h_orig = hashlib.sha256(j_orig.encode()).hexdigest() + h_ref = hashlib.sha256(j_ref.encode()).hexdigest() + full_ok = j_orig == j_ref and h_orig == h_ref + print(f"[2b] collect() JSON original.sha256={h_orig[:16]}… " + f"refactored.sha256={h_ref[:16]}… " + f"{'PASS' if full_ok else 'FAIL'}") + if not full_ok: + all_ok = False + # show first diff line for a targeted signal + a, b = j_orig.splitlines(), j_ref.splitlines() + for i in range(max(len(a), len(b))): + ga = a[i] if i < len(a) else "" + gb = b[i] if i < len(b) else "" + if ga != gb: + print(f" first diff at json line {i+1}:") + print(f" original: {ga}") + print(f" refactored: {gb}") + break + + selftest_ok = run_selftest() + if not selftest_ok: + all_ok = False + + print("\nVERDICT:", "ALL GREEN -- behavior preserved" if all_ok + else "MISMATCH -- behavior NOT preserved") + sys.exit(0 if all_ok else 1) + + +if __name__ == "__main__": + main()