Files
troupe-standup/wizard-work/forge/LANDING_CHECK_REFACTOR_VERIFICATION.md
T

6.5 KiB

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:

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)

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