stackchain/troupe-standup
2
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

forge: landing_check.py DRY consolidation refactor (Counter single census pass) + equivalence verifier

Browse Source
This commit is contained in:
forge committed 2026-10-23 09:33:18 +00:00
1 parent af1542ca93
commit cf80507de7
3 files changed
+393 -11
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

tools/landing_check.py
+14 -11
View File
@@ -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():
wizard-work/forge/LANDING_CHECK_REFACTOR_VERIFICATION.md
+135
View File
@@ -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 -- <tool>` 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
wizard-work/forge/verify_landing_check_refactor.py
+244
View File
@@ -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 <path> 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 <path>'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 <repo> --json <out> [--path <only_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)