From cf80507de79a117229acd75d07505443765bea27 Mon Sep 17 00:00:00 2001 From: forge Date: Fri, 23 Oct 2026 09:33:18 +0000 Subject: [PATCH] forge: landing_check.py DRY consolidation refactor (Counter single census pass) + equivalence verifier --- tools/landing_check.py | 25 +- .../LANDING_CHECK_REFACTOR_VERIFICATION.md | 135 ++++++++++ .../forge/verify_landing_check_refactor.py | 244 ++++++++++++++++++ 3 files changed, 393 insertions(+), 11 deletions(-) create mode 100644 wizard-work/forge/LANDING_CHECK_REFACTOR_VERIFICATION.md create mode 100644 wizard-work/forge/verify_landing_check_refactor.py diff --git a/tools/landing_check.py b/tools/landing_check.py index 559d133..201138b 100644 --- a/tools/landing_check.py +++ b/tools/landing_check.py @@ -65,6 +65,7 @@ import shutil import subprocess import sys import tempfile +from collections import Counter DEFAULT_BASE = ("https://forge.alexanderwhitestone.com/git/stackchain/" "troupe-standup/raw") @@ -193,14 +194,15 @@ def build_rows(repo, rev, branch, forge, paths=None, max_bytes=4 * 1024 * 1024): def summarise(rows): - counts = {} - for row in rows: - counts[row["verdict"]] = counts.get(row["verdict"], 0) + 1 - return counts + """Verdict census as a {verdict: n} mapping -- one pass via Counter, + replacing the original's per-call dict.get(k,0)+1 loop.""" + return Counter(row["verdict"] for row in rows) -def summary_line(rows): - counts = summarise(rows) +def summary_line(rows, counts=None): + """One-line tally. Pass a precomputed census via `counts` to avoid + recomputing it; run() reuses its own census instead of re-scanning.""" + counts = counts if counts is not None else summarise(rows) order = ["OK", "DIFFERS", "MISSING", "HTTP_ERROR", "UNREACHABLE", "READ_FAILED", "SKIPPED_LARGE"] parts = ["%d %s" % (counts[k], k) for k in order if counts.get(k)] @@ -212,6 +214,8 @@ def run(args): rows = build_rows(args.repo, args.rev, args.branch, Forge(args.base_url, args.timeout), args.path, args.max_bytes) + counts = summarise(rows) # single census pass + fatal = sum(counts.get(k, 0) for k in FATAL) # derived once if not args.quiet: width = max([len("path")] + [len(r["path"]) for r in rows]) if rows else 4 print("%-*s %-12s %s" % (width, "path", "verdict", "detail")) @@ -219,13 +223,12 @@ def run(args): print("%-*s %-12s %s" % (width, row["path"], row["verdict"], row["detail"])) print() - line = summary_line(rows) - print(line) + print(summary_line(rows, counts)) # reuse the census if args.json: payload = {"rev": args.rev, "branch": args.branch, "base_url": args.base_url, "rows": rows, - "summary": summarise(rows), - "fatal": sum(row["verdict"] in FATAL for row in rows)} + "summary": counts, + "fatal": fatal} try: with open(args.json, "w", encoding="utf-8") as fh: json.dump(payload, fh, indent=2, sort_keys=True) @@ -233,7 +236,7 @@ def run(args): except OSError as exc: print("cannot write --json " + args.json + ": " + str(exc), file=sys.stderr) return 2 - return 1 if any(row["verdict"] in FATAL for row in rows) else 0 + return 1 if fatal else 0 def selftest(): diff --git a/wizard-work/forge/LANDING_CHECK_REFACTOR_VERIFICATION.md b/wizard-work/forge/LANDING_CHECK_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..bf0b6ef --- /dev/null +++ b/wizard-work/forge/LANDING_CHECK_REFACTOR_VERIFICATION.md @@ -0,0 +1,135 @@ +# landing_check.py DRY-consolidation refactor — equivalence verification (forge lane: Refactor) + +## The shipped artifact +`tools/landing_check.py` (committed at HEAD by Volt, commit `4034645`): the +"every file at a revision, byte-for-byte against what the forge serves" gate. +Its own `verify`-style posture was never shipped — the `--selftest` it carries +(26 cells) was the only lock, and no equivalence proof covered the census path +`run()` -> `summarise()` -> `summary_line()`. + +## The smell (what the refactor fixes) +`run()` derived its own verdict census by walking the row list **four times**: + +```python +def summarise(rows): + counts = {} + for row in rows: + counts[row["verdict"]] = counts.get(row["verdict"], 0) + 1 + return counts + +def summary_line(rows): + counts = summarise(rows) # pass 1 (inside summary_line) + ... + +def run(args): + ... + line = summary_line(rows) # pass 1: build the census + print(line) + if args.json: + payload = {..., + "summary": summarise(rows), # pass 2: rebuild it + "fatal": sum(row["verdict"] in FATAL for row in rows)} # pass 3: re-scan for fatal + return 1 if any(row["verdict"] in FATAL for row in rows) else 0 # pass 4: re-scan for fatal +``` + +Three smells, the family `tool_map.py` files under "DRY consolidation": +- `summarise()` uses a hand-rolled `dict.get(k,0)+1` loop instead of `Counter`. +- `summarise(rows)` is materialised **twice** per `run()` (once for the printed + summary, once for the JSON `summary`), then **re-scanned** twice more (`sum(...)` + and `any(...)` both iterate every row just to test fatality). +- `summary_line()` cannot receive a precomputed census, so it re-builds its own. + +## The refactor (working tree, uncommitted at time of writing) +```python +from collections import import Counter # ... already gated the rest of run() + +def summarise(rows): + return Counter(row["verdict"] for row in rows) # one pass + +def summary_line(rows, counts=None): # accept a precomputed census + counts = counts if counts is not None else summarise(rows) + ... + +def run(args): + rows = build_rows(...) + counts = summarise(rows) # single census pass + fatal = sum(counts.get(k, 0) for k in FATAL) # derived from the census, 0 row scans + ... + print(summary_line(rows, counts)) # reuse + if args.json: + payload = {..., + "summary": counts, # reuse + "fatal": fatal} # reuse + return 1 if fatal else 0 # reuse +``` + +Net: **4 row-scan passes -> 1 pass** over the rows for the census; **0** re-scans +for fatality. `summarise()` is `summarise()`'s only caller in `run()` now. +The output surface is unchanged: the printed table, the summary line, the JSON +shape, and every exit code are byte-for-byte identical (proven below). + +## ORIGINAL vs REFACTORED +- ORIGINAL = newest pre-refactor commit in `tools/landing_check.py`'s history, + located by `find_original_commit()` walking `git log -- ` for the newest + commit carrying the `dict.get` smell (`counts.get(row["verdict"], 0) + 1`) and + **not** the Counter refactor (`return Counter(row["verdict"] for row in rows)`). + Anchoring on HEAD is a false-red once the refactor lands: HEAD then carries the + refactor, so a HEAD-anchored "original" would compare the refactor to itself. + Same history-walk pattern as the citation_hash_class / path_claim verifiers. +- REFACTORED = working tree. + +Pinned pre-refactor original: commit `2556dbb0` ("tools: the write path -- an +unwritable --json is rc 2, one line, never a traceback"). + +## What this verifier proves (13 checks, all pass, exit 0) + +``` + [ok ] selftest: original 26/26 rc 0 + [ok ] selftest: refactored 26/26 rc 0 + [ok ] selftest: identical stdout (byte-for-byte) + [ok ] live scan: original ran (rc != 2, JSON produced) + [ok ] live scan: refactored ran (rc != 2, JSON produced) + [ok ] live scan: identical JSON (byte-for-byte) + [ok ] refactored imports Counter; original does not + [ok ] refactored summarise() returns a Counter (one pass) + [ok ] original summarise() carries the dict.get smell + [ok ] refactored run() computes counts once, derives fatal, reuses them + [ok ] original run() re-runs summarise for JSON + re-scans rows for fatal + [ok ] selftest non-vacuous (3 fixture rows graded) + [ok ] live scan non-vacuous (real row, measured verdict) + +verify: 13 checks, 13 passed, 0 failed +original source commit: 2556dbb0aa236e986be22a7eb07297f7252a9323 +``` + +Legs: +- [1-3] **selftest parity** (deterministic, no network). Both original (pin `2556dbb0`) + and refactored pass 26/26 rc 0 with **byte-identical stdout**. The selftest is + the comprehensive equivalence surface: it exercises `build_rows`/`grade`/`summarise`/ + `summary_line`/`run`/JSON/exit-codes/`--quiet`/`--path` across the OK, DIFFERS, + MISSING, HTTP_ERROR, UNREACHABLE and SKIPPED_LARGE verdicts on a fixture repo. +- [4] **live scan equivalence** on `--path README.md` against the real forge + (`forge.alexanderwhitestone.com`). Both ran rc 0 and produced **byte-identical JSON + (561 B)**, one real row (`README.md`, 1409 B, sha256 `87313bb2…`, verdict OK). +- [5] **code-level**: the smell marker (`counts.get(row["verdict"], 0) + 1`) lives + only in the original; the fix marker (`return Counter(row["verdict"] for row in rows)`, + `counts = summarise(rows)`, `fatal = sum(counts.get(k, 0) for k in FATAL)`, + `summary_line(rows, counts)`, `"summary": counts`, `"fatal": fatal`, + `return 1 if fatal else 0`) lives only in the refactored. +- [6] **non-vacuity**: the selftest grades 3 real fixture rows (`landing: 3 files, + 3 OK, 0 fatal`); the live scan yields 1 real row with a measured verdict. + +## Non-tautology proof +Against the pre-refactor source (pin `2556dbb0`) the refactor markers are absent: +`return Counter(...)` not in original, and `counts.get(row["verdict"], 0) + 1` +IS in original. A HEAD-anchored "original" would read the refactor's own +`Counter` line as "original" and the smell-leg would read False — so the +history-walk pin is what makes the check red on the buggy side and green on the +fixed side. The live JSON is a real 561-B document produced by a real forge fetch, +not a synthetic constant. + +## Artifacts +- Refactored tool: `tools/landing_check.py` (working tree; committed with this pass) +- Equivalence verifier: `wizard-work/forge/verify_landing_check_refactor.py` +- Post-commit CHECK: `python3 wizard-work/forge/verify_landing_check_refactor.py` +- Report: this file diff --git a/wizard-work/forge/verify_landing_check_refactor.py b/wizard-work/forge/verify_landing_check_refactor.py new file mode 100644 index 0000000..6aa282d --- /dev/null +++ b/wizard-work/forge/verify_landing_check_refactor.py @@ -0,0 +1,244 @@ +#!/usr/bin/env python3 +"""verify_landing_check_refactor.py + +Equivalence proof for the behavior-preserving refactor in +tools/landing_check.py (forge lane: Refactor). + +THE SMELL + run() walked the row list THREE times to produce its own census, and a fourth + time to test fatality: + - summary_line(rows) -> summarise(rows) [pass 1: dict] + - "summary": summarise(rows) [pass 2: rebuild it] + - "fatal": sum(row["verdict"] in FATAL for row in rows) [pass 3] + - return 1 if any(row["verdict"] in FATAL for row in rows) [pass 4] + summarise() itself used a per-call dict.get(k,0)+1 loop. tool_map.py names the + family "DRY consolidation": derive one aggregate from one pass, reuse it. + +THE REFACTOR (working tree; see landing_check.py lines 196-239) + - summarise() returns Counter(row["verdict"] for row in rows) [one pass] + - summary_line(rows, counts=None) accepts a precomputed census + - run() computes `counts = summarise(rows)` ONCE, derives + `fatal = sum(counts.get(k, 0) for k in FATAL)` from it, and reuses `counts` + for the printed summary_line AND the JSON "summary" / "fatal" fields + Net: 4 passes -> 1 pass over the rows; 0 row re-scans for fatal. + +ORIGINAL vs REFACTORED + ORIGINAL = newest pre-refactor commit in TOOL's history (find_original_commit + walks the file's commit history for the newest commit still carrying + the dict.get smell and NOT the Counter refactor). Anchoring on HEAD + is a false-red once the refactor lands: HEAD then carries the + refactor, so the code-level portability leg would compare the + refactor against itself. Same history-walk pattern as + verify_citation_hash_class_portable_default.py and + verify_path_claim_dry_refactor.py. + REFACTORED = working tree. + +WHAT THIS VERIFIER PROVES + [1] selftest: original passes 26/26, rc 0 + [2] selftest: refactored passes 26/26, rc 0 + [3] selftest: stdout byte-identical (covers build_rows/grade/summarise/ + summary_line/run/JSON/exit-codes/quiet/--path across the OK, DIFFERS, + MISSING, HTTP_ERROR, UNREACHABLE and SKIPPED_LARGE verdicts) + [4] live scan (--path README.md, real forge): original & refactored both run + clean, JSON byte-identical (equivalence on real served bytes) + [5] code-level: refactored uses Counter + one census pass + counts param; + original carries the dict.get smell and re-runs summarise for JSON / + re-scans rows for fatal (the 4-pass smell) + [6] non-vacuity: selftest grades 3 real fixture rows; live scan yields a real + row with a measured verdict (not an empty census) + +ORIGINAL marker (the smell): `counts.get(row["verdict"], 0) + 1` +REFACTORED marker: `return Counter(row["verdict"] for row in rows)` + +Exit 0 iff all checks pass, else 1. +""" +import json +import os +import shutil +import subprocess +import sys +import tempfile + +REPO = "/root/troupe-standup" +TOOL = "tools/landing_check.py" +SMILE_ORIGINAL = 'counts.get(row["verdict"], 0) + 1' # the dict.get loop, original-only +REFACTORED_MARKER = 'return Counter(row["verdict"] for row in rows)' # refactored-only +SELFTEST_SUMMARY = "selftest: 26 cells, 26 passed, 0 failed" + + +class Verdict(Exception): + pass + + +def code_lines(text): + """Strip naive full-line comments (#) and triple-quote doc-opens so a smell + that lives only in prose is not counted as code -- same convention the other + forge verifiers use. landing_check's summarise docstring mentions + `dict.get(k,0)+1` but not the exact SMILE_ORIGINAL, so this is safe.""" + out = [] + for ln in text.splitlines(): + s = ln.lstrip() + if s.startswith("#") or s.startswith('"""'): + continue + out.append(ln) + return "\n".join(out) + + +def git_commits_touching(path): + """Newest-first list of commit shas that touched in REPO.""" + proc = subprocess.run(["git", "-C", REPO, "log", "--format=%H", "--", path], + capture_output=True, text=True, check=True, timeout=60) + return [s for s in proc.stdout.split() if s] + + +def find_original_commit(path=TOOL): + """NEWEST commit in 's history whose summarise() still carries the + dict.get smell (SMILE_ORIGINAL) and NOT the Counter refactor + (REFACTORED_MARKER) -- the genuine pre-refactor original. Robust to the + refactor landing in a later commit: HEAD then carries REFACTORED_MARKER and + is skipped, so the pin always points at the true pre-refactor source.""" + for sha in git_commits_touching(path): + txt = subprocess.run(["git", "-C", REPO, "show", "%s:%s" % (sha, path)], + capture_output=True, text=True, timeout=60).stdout + if SMILE_ORIGINAL in code_lines(txt) and REFACTORED_MARKER not in txt: + return sha + return None + + +def materialize_original(path=TOOL): + """Write the PRE-refactor module (genuine history pin) to a temp file under + its real .py name so subprocess can execute it; return (dir, path, sha).""" + sha = find_original_commit(path) + if not sha: + raise Verdict("could not find a pre-refactor original for %s in history" % path) + out = tempfile.NamedTemporaryFile(suffix=".py", delete=False, mode="wb") + out.close() + subprocess.run(["git", "-C", REPO, "show", "%s:%s" % (sha, path)], + check=True, stdout=open(out.name, "wb"), timeout=60) + return os.path.dirname(out.name), out.name, sha + + +def run(path, args, cwd=REPO, timeout=180): + proc = subprocess.run([sys.executable, path, *args], + capture_output=True, text=True, cwd=cwd, timeout=timeout) + return proc.returncode, proc.stdout, proc.stderr + + +def run_json(path, out_path, repo=REPO, only_path=None, timeout=180): + """Run landing_check.py --repo --json [--path ]; + the JSON is written to out_path. Returns (rc, stdout, stderr).""" + args = ["--repo", repo, "--rev", "HEAD", "--json", out_path] + if only_path is not None: + args += ["--path", only_path] + proc = subprocess.run([sys.executable, path, *args], + capture_output=True, text=True, cwd=REPO, timeout=timeout) + return proc.returncode, proc.stdout, proc.stderr + + +def main(): + cells, failed = [], 0 + + def check(label, ok, detail=""): + cells.append((label, ok)) + if not ok: + failed += 1 + print(" [%s] %s%s" + % ("ok " if ok else "FAIL", label, + (f" [{detail}]" if detail and not ok else ""))) + + orig_dir, orig_path, orig_sha = materialize_original() + ref_path = os.path.join(REPO, TOOL) + tmpdir = tempfile.mkdtemp(prefix="lc_verify_", + dir=os.environ.get("TMPDIR", "/tmp")) + try: + # ---- [1-3] selftest parity (deterministic, no network) --------------- + rc_o, out_o, _ = run(orig_path, ["--selftest"]) + rc_r, out_r, _ = run(ref_path, ["--selftest"]) + check("selftest: original 26/26 rc 0", + rc_o == 0 and SELFTEST_SUMMARY in out_o, f"rc={rc_o}") + check("selftest: refactored 26/26 rc 0", + rc_r == 0 and SELFTEST_SUMMARY in out_r, f"rc={rc_r}") + check("selftest: identical stdout (byte-for-byte)", + out_o == out_r, + f"len_o={len(out_o)} len_r={len(out_r)}") + + # ---- [4] live scan equivalence (real forge, single tracked file) ------ + oj_path = os.path.join(tmpdir, "orig.json") + rj_path = os.path.join(tmpdir, "ref.json") + rc_o, so, se_o = run_json(orig_path, oj_path, only_path="README.md") + rc_r, sr, se_r = run_json(ref_path, rj_path, only_path="README.md") + check("live scan: original ran (rc != 2, JSON produced)", + rc_o != 2 and os.path.exists(oj_path), + f"rc={rc_o} {se_o[:140]}") + check("live scan: refactored ran (rc != 2, JSON produced)", + rc_r != 2 and os.path.exists(rj_path), + f"rc={rc_r} {se_r[:140]}") + if os.path.exists(oj_path) and os.path.exists(rj_path): + ob = open(oj_path, "rb").read() + rb = open(rj_path, "rb").read() + check("live scan: identical JSON (byte-for-byte)", + ob == rb, f"orig={len(ob)}B ref={len(rb)}B") + else: + check("live scan: identical JSON (byte-for-byte)", False, + "JSON not produced by one side") + + # ---- [5] code-level: the smell vs. the fix --------------------------- + ref_code = code_lines(open(ref_path, encoding="utf-8").read()) + orig_code = code_lines(open(orig_path, encoding="utf-8").read()) + check("refactored imports Counter; original does not", + "from collections import Counter" in ref_code + and "from collections import Counter" not in orig_code) + check("refactored summarise() returns a Counter (one pass)", + REFACTORED_MARKER in ref_code) + check("original summarise() carries the dict.get smell", + SMILE_ORIGINAL in orig_code and REFACTORED_MARKER not in orig_code) + check("refactored run() computes counts once, derives fatal, reuses them", + "counts = summarise(rows)" in ref_code + and "fatal = sum(counts.get(k, 0) for k in FATAL)" in ref_code + and "summary_line(rows, counts)" in ref_code + and '"summary": counts' in ref_code + and '"fatal": fatal' in ref_code + and "return 1 if fatal else 0" in ref_code) + check("original run() re-runs summarise for JSON + re-scans rows for fatal", + '"summary": summarise(rows)' in orig_code + and 'sum(row["verdict"] in FATAL for row in rows)' in orig_code + and "any(row[\"verdict\"] in FATAL for row in rows)" in orig_code + and "return 1 if any" in orig_code) + + # ---- [6] non-vacuity ------------------------------------------------- + # selftest grades 3 real fixture rows (3 OK) -- the census did real work + check("selftest non-vacuous (3 fixture rows graded)", + "landing: 3 files, 3 OK, 0 fatal" in out_r) + if os.path.exists(rj_path): + rj = json.load(open(rj_path)) + live_rows = len(rj.get("rows", [])) + live_sum = sum(rj.get("summary", {}).values()) + check("live scan non-vacuous (real row, measured verdict)", + live_rows == 1 and live_sum >= 1 and + all(r.get("verdict") for r in rj.get("rows", [])), + f"rows={live_rows} sum={live_sum}") + else: + check("live scan non-vacuous (real row, measured verdict)", False, + "JSON not produced") + finally: + shutil.rmtree(orig_dir, ignore_errors=True) + shutil.rmtree(tmpdir, ignore_errors=True) + for p in ("/tmp/lc_orig.py",): + try: + os.remove(p) + except OSError: + pass + + total = len(cells) + passed = total - failed + print("\nverify: %d checks, %d passed, %d failed" % (total, passed, failed)) + print("original source commit: %s" % orig_sha) + return 1 if failed else 0 + + +if __name__ == "__main__": + try: + sys.exit(main()) + except Verdict as e: + print("\nVERDICT: MISMATCH -- %s" % e) + sys.exit(1)