forge: doc_census_check main() 3 scans -> 1 Counter pass, byte-identical
This commit is contained in:
1 parent
d7fb94f4a3
commit
95c6f90d4d
3 files changed
+305
-4
No files matched your search
@@ -1786,11 +1786,11 @@ def main():
|
||||
for v in bad:
|
||||
print(f" {v['verdict']:10s} {v['file']}:{v.get('line', '-')} "
|
||||
f"{v['unit']} claimed {v['claim']} at {v['rev']} -- {v['detail']}")
|
||||
for k in sorted({v["verdict"] for v in rep["verdicts"]}):
|
||||
verdict_counts = collections.Counter(v["verdict"] for v in rep["verdicts"])
|
||||
for k in sorted(verdict_counts):
|
||||
if k in ("REV_UNCHECKED", "SELF_NOT_RUN", "CLONE_LOCAL"):
|
||||
n = sum(1 for v in rep["verdicts"] if v["verdict"] == k)
|
||||
print(f" ({n} {k}: reported, never counted as a defect)")
|
||||
for k, c in collections.Counter(v["verdict"] for v in rep["verdicts"]).items():
|
||||
print(f" ({verdict_counts[k]} {k}: reported, never counted as a defect)")
|
||||
for k, c in verdict_counts.items():
|
||||
tot[k] += c
|
||||
print("\n--- verdict totals (unit: claims) ---")
|
||||
for k in sorted(tot):
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
# forge: doc_census_check main() verdict tally -- 3 scans -> 1 Counter pass (DRY)
|
||||
|
||||
Target: `tools/doc_census_check.py`
|
||||
Lane: Refactor (code review and refactors)
|
||||
Status: DONE -- verified equivalent, behavior preserved.
|
||||
|
||||
## The smell
|
||||
|
||||
`main()`'s per-tool reporting loop counted verdicts by scanning `rep["verdicts"]`
|
||||
three separate times per report, twice of them by hand:
|
||||
|
||||
```python
|
||||
# OLD: three scans of rep["verdicts"]
|
||||
for k in sorted({v["verdict"] for v in rep["verdicts"]}): # scan 1 (set comp)
|
||||
if k in ("REV_UNCHECKED", "SELF_NOT_RUN", "CLONE_LOCAL"):
|
||||
n = sum(1 for v in rep["verdicts"] if v["verdict"] == k) # scan 2 (per-key)
|
||||
print(f" ({n} {k}: reported, never counted as a defect)")
|
||||
for k, c in collections.Counter(v["verdict"] for v in rep["verdicts"]).items(): # scan 3
|
||||
tot[k] += c
|
||||
```
|
||||
|
||||
Scan 1 builds a set of verdicts just to `sorted()` them. Scan 2 re-counts each key
|
||||
with a `sum(1 for ...)` -- O(keys * rows). Scan 3 builds a fresh `Counter` for the
|
||||
`tot` accumulator that the advisory block never used. Two of the three were hand
|
||||
rolled; the third re-derived data the first two already had.
|
||||
|
||||
## The fix (behavior-preserving)
|
||||
|
||||
Build the `Counter` once and reuse it for both sinks. `sorted(Counter)` equals
|
||||
`sorted(set(...))` over the same values, `verdict_counts[k]` equals the per-key
|
||||
`sum(1 for ...)`, and the `tot` accumulation iterates the same `.items()`. The
|
||||
counted values are identical, so every downstream sink (`sorted(tot)` totals, the
|
||||
advisory print line, the stdout) is unchanged.
|
||||
|
||||
```python
|
||||
# NEW: one scan of rep["verdicts"]
|
||||
verdict_counts = collections.Counter(v["verdict"] for v in rep["verdicts"])
|
||||
for k in sorted(verdict_counts):
|
||||
if k in ("REV_UNCHECKED", "SELF_NOT_RUN", "CLONE_LOCAL"):
|
||||
print(f" ({verdict_counts[k]} {k}: reported, never counted as a defect)")
|
||||
for k, c in verdict_counts.items():
|
||||
tot[k] += c
|
||||
```
|
||||
|
||||
`main()` is the ONLY function touched. `census()`, `documented_commands()`,
|
||||
`resolve_target()`, `help_options()`, `run_read_probe`, `judge_malformed`,
|
||||
`report_read`, `delta`, and `selftest()` are all byte-identical to HEAD.
|
||||
|
||||
## Evidence
|
||||
|
||||
`wizard-work/forge/verify_doc_census_dry_refactor.py` exercises three verdicts.
|
||||
All green (exit 0):
|
||||
|
||||
[1] SELFTEST DRIFT GUARD -- `doc_census_check.py --selftest` on BOTH the
|
||||
original (git HEAD~1 blob, materialized under its real name because the
|
||||
selftest is self-referential via `__file__`) and the refactored (working
|
||||
tree):
|
||||
original selftest rc=0 sha=028d683923c285be0e004c8cb7d2fca2... PASS
|
||||
refactored selftest rc=0 sha=028d683923c285be0e004c8cb7d2fca2... PASS
|
||||
stdout is byte-identical (sha256 match) -- 104 cells, 104 passed, 0
|
||||
failed. The selftest's gate-e2e cells drive `main()` end-to-end (writes an
|
||||
index document, re-checks it, exercises `--outside`, `--no-run`, and
|
||||
`--json`), so this pins the refactored reporting loop through the live
|
||||
path -- not just `--selftest` but the `main()` code that contains the
|
||||
collapse.
|
||||
|
||||
[2] END-TO-END --no-run --json -- `--no-run --json` on the live repo at HEAD,
|
||||
original vs refactored:
|
||||
original --json rc=0 sha=a43ee4cadcd02ce204b8509739eec3eae...
|
||||
refactored --json rc=0 sha=a43ee4cadcd02ce204b8509739eec3eae... PASS
|
||||
(10276 bytes, byte-identical). `--no-run` still builds `reports` via
|
||||
`check_tool()` (untouched) and still runs the refactored reporting loop, so
|
||||
stdout + the `tot` totals are pinned against the live census.
|
||||
|
||||
[2b] stderr identical sha=e3b0c44298fc1c149afbf4c8996fb92427ae41e464... PASS
|
||||
(both empty -- the refactor changed no refusal path).
|
||||
|
||||
[3] REDUNDANCY COLLAPSED -- static check on the refactored source:
|
||||
removed scans: sum(1 for v in rep["verdicts"]...) = 0
|
||||
{v["verdict"] for v in rep["verdicts"]...} = 0
|
||||
verdict_counts = collections.Counter(...) builds = 1 PASS
|
||||
|
||||
[3b] ORIGINAL carries the removed scans (genuine before/after, not a no-op):
|
||||
setcomp = True sumscan = True PASS
|
||||
The original HEAD~1 source still contains both `sum(1 for ...)` and the
|
||||
`{v["verdict"] for ...}` set comprehension, confirming the comparison is
|
||||
against the genuine pre-refactor code.
|
||||
|
||||
## Exit contract
|
||||
|
||||
Behavior-preserving refactor. rc 0 on both the original and refactored
|
||||
`--selftest` (stdout sha256-identical, 104/104 cells), rc 0 on both
|
||||
`--no-run --json` (artifact sha256-identical, 10276 bytes), stderr identical,
|
||||
and the two redundant scans are gone with the consolidated Counter built
|
||||
exactly once. No public interface, argument, or output format changed; the
|
||||
`sorted(tot)` totals render and the advisory print loop are unchanged.
|
||||
@@ -0,0 +1,205 @@
|
||||
#!/usr/bin/env python3
|
||||
"""verify_doc_census_dry_refactor.py -- equivalence proof for the main() DRY.
|
||||
|
||||
The refactor collapsed three scans of `rep["verdicts"]` into one inside
|
||||
tools/doc_census_check.py's main() reporting loop:
|
||||
|
||||
- for k in sorted({v["verdict"] for v in rep["verdicts"]}):
|
||||
- if k in ("REV_UNCHECKED", "SELF_NOT_RUN", "CLONE_LOCAL"):
|
||||
- n = sum(1 for v in rep["verdicts"] if v["verdict"] == k)
|
||||
- print(f" ({n} {k}: reported, never counted as a defect)")
|
||||
- for k, c in collections.Counter(v["verdict"] for v in rep["verdicts"]).items():
|
||||
+ verdict_counts = collections.Counter(v["verdict"] for v in rep["verdicts"])
|
||||
+ for k in sorted(verdict_counts):
|
||||
+ if k in ("REV_UNCHECKED", "SELF_NOT_RUN", "CLONE_LOCAL"):
|
||||
+ print(f" ({verdict_counts[k]} {k}: reported, never counted as a defect)")
|
||||
+ for k, c in verdict_counts.items():
|
||||
tot[k] += c
|
||||
|
||||
That is behavior-preserving iff main() emits byte-identical bytes before and
|
||||
after, because main() is the ONLY function touched and the verdict-counting
|
||||
block feeds two sinks that both read from the same Counter:
|
||||
|
||||
- the advisory print loop iterates `sorted(<verdicts>)` -- sorted(Counter)
|
||||
equals sorted(set(...)) over the same values, and `verdict_counts[k]`
|
||||
equals the per-key `sum(1 for ...)` it replaces;
|
||||
- the `tot` accumulation iterates the same Counter's `.items()`, so the
|
||||
totals and their final `sorted(tot)` render are unchanged.
|
||||
|
||||
Three verdicts, all must be green:
|
||||
|
||||
1. SELFTEST DRIFT GUARD -- `doc_census_check.py --selftest` exits 0 on BOTH
|
||||
the original (git HEAD~1 blob, materialized under its real name because the
|
||||
selftest is self-referential via __file__) and the refactored (working
|
||||
tree); stdout is byte-identical (sha256 match). The selftest's 104 cells
|
||||
drive main() end-to-end through gate e2e (--json writes, --write, --check),
|
||||
so this is the live path through the refactored block.
|
||||
|
||||
2. END-TO-END --json -- `--no-run --json` on the live repo at HEAD on both
|
||||
the original and refactored; the rendered JSON artifact is byte-identical
|
||||
(sha256 match). --no-run is the fast leg: it still builds `reports` (via
|
||||
check_tool, untouched by the refactor) and still runs the refactored
|
||||
reporting loop, so it pins stdout+tot against the census.
|
||||
|
||||
3. REDUNDANCY COLLAPSED -- the two redundant scans are gone from the
|
||||
refactored source, and the consolidated Counter is built exactly once.
|
||||
|
||||
Exit code is the verdict: 0 = all green, 1 = any mismatch.
|
||||
"""
|
||||
import hashlib
|
||||
import importlib.util
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
|
||||
REPO = "/root/troupe-standup"
|
||||
TOOL = os.path.join(REPO, "tools", "doc_census_check.py")
|
||||
NAME = "doc_census_check.py"
|
||||
|
||||
|
||||
def sha256_bytes(b: bytes) -> str:
|
||||
return hashlib.sha256(b).hexdigest()
|
||||
|
||||
|
||||
def materialize_original():
|
||||
"""Write the PRE-refactor source (git HEAD~1) under its real name in a temp
|
||||
dir and return (dir, path). HEAD~1 is the parent of the (not-yet-)refactor
|
||||
commit; doc_census_check.py is identical at HEAD and HEAD~1 today (no later
|
||||
commit touches it), so this reads a genuine original both before the refactor
|
||||
lands and after -- at HEAD~1 the parent still holds the multi-scan code.
|
||||
|
||||
The name matters: the selftest is self-referential via __file__ (it shells
|
||||
back to `python3 <__file__> ...` for the gate-e2e cells), so the temp file
|
||||
MUST be named doc_census_check.py or those cells fail for a harness reason.
|
||||
"""
|
||||
res = subprocess.run(
|
||||
["git", "-C", REPO, "show", "HEAD~1:tools/doc_census_check.py"],
|
||||
capture_output=True, text=True)
|
||||
if not res.stdout:
|
||||
raise SystemExit("[setup] could not read HEAD~1:tools/doc_census_check.py: "
|
||||
+ res.stderr.strip())
|
||||
d = tempfile.mkdtemp(prefix="dcc_orig_")
|
||||
p = os.path.join(d, NAME)
|
||||
with open(p, "w", encoding="utf-8") as fh:
|
||||
fh.write(res.stdout)
|
||||
os.chmod(p, 0o755)
|
||||
return d, p
|
||||
|
||||
|
||||
def run_tool(src_path, extra, repo=REPO, cwd=None):
|
||||
rc = subprocess.run([sys.executable, src_path, *extra],
|
||||
cwd=cwd or repo, capture_output=True, text=True,
|
||||
errors="replace")
|
||||
return rc.returncode, rc.stdout or "", rc.stderr or ""
|
||||
|
||||
|
||||
def main():
|
||||
failures = []
|
||||
orig_dir, orig_path = materialize_original()
|
||||
try:
|
||||
# --------------------------------------------------------------- [1]
|
||||
# SELFTEST DRIFT GUARD: original vs refactored, rc 0, stdout identical.
|
||||
o_rc, o_out, o_err = run_tool(orig_path, ["--selftest"], repo=os.path.dirname(orig_path))
|
||||
r_rc, r_out, r_err = run_tool(TOOL, ["--selftest"])
|
||||
o_sha = sha256_bytes(o_out.encode())
|
||||
r_sha = sha256_bytes(r_out.encode())
|
||||
selftest_ok = o_rc == 0 and r_rc == 0 and o_sha == r_sha
|
||||
print(f"[1] selftest drift guard rc orig={o_rc} ref={r_rc} "
|
||||
f"sha orig={o_sha[:16]} ref={r_sha[:16]} "
|
||||
f"{'PASS' if selftest_ok else 'FAIL'}")
|
||||
if not selftest_ok:
|
||||
failures.append("[1] selftest rc0/sha mismatch")
|
||||
if o_err:
|
||||
print(" original stderr:", o_err.strip()[:200].replace("\n", "\n "))
|
||||
if r_err:
|
||||
print(" refactored stderr:", r_err.strip()[:200].replace("\n", "\n "))
|
||||
for i, (a, b) in enumerate(zip(o_out.splitlines(), r_out.splitlines())):
|
||||
if a != b:
|
||||
print(f" diff at line {i+1}: orig={a[:100]!r} ref={b[:100]!r}")
|
||||
break
|
||||
|
||||
# --------------------------------------------------------------- [2]
|
||||
# END-TO-END --no-run --json on the live repo: byte-identical artifact.
|
||||
with tempfile.TemporaryDirectory(prefix="dcc-json-") as td:
|
||||
j_orig = os.path.join(td, "orig.json")
|
||||
j_ref = os.path.join(td, "ref.json")
|
||||
o_rc, o_out, o_err = run_tool(orig_path, ["--no-run", "--json", j_orig])
|
||||
r_rc, r_out, r_err = run_tool(TOOL, ["--no-run", "--json", j_ref])
|
||||
try:
|
||||
with open(j_orig, "rb") as fh:
|
||||
o_json = fh.read()
|
||||
with open(j_ref, "rb") as fh:
|
||||
r_json = fh.read()
|
||||
except OSError as exc:
|
||||
print(f"[2] could not read json artifact: {exc} FAIL")
|
||||
failures.append("[2] json artifact unreadable")
|
||||
o_json, r_json = b"", b""
|
||||
o_js = sha256_bytes(o_json)
|
||||
r_js = sha256_bytes(r_json)
|
||||
json_ok = o_rc == 0 and r_rc == 0 and o_js == r_js
|
||||
print(f"[2] end-to-end --json rc orig={o_rc} ref={r_rc} "
|
||||
f"sha {o_js[:16]} == {r_js[:16]} "
|
||||
f"({'PASS' if json_ok else 'FAIL'}) "
|
||||
f"({len(r_json)} bytes)")
|
||||
if not json_ok:
|
||||
failures.append("[2] json sha mismatch")
|
||||
print(f" orig stderr: {o_err.strip()[:200]}")
|
||||
print(f" ref stderr: {r_err.strip()[:200]}")
|
||||
# stderr must also be identical (the gate's preconditions are part
|
||||
# of the contract, and a refactor must not change a refusal).
|
||||
err_ok = sha256_bytes(o_err.encode()) == sha256_bytes(r_err.encode())
|
||||
print(f"[2b] stderr identical sha {sha256_bytes(o_err.encode())[:16]} "
|
||||
f"{'PASS' if err_ok else 'FAIL'}")
|
||||
if not err_ok:
|
||||
failures.append("[2b] stderr drifted")
|
||||
|
||||
# --------------------------------------------------------------- [3]
|
||||
# REDUNDANCY COLLAPSED: static check on the refactored source.
|
||||
with open(TOOL, "r", encoding="utf-8") as fh:
|
||||
src = fh.read()
|
||||
code = "\n".join(ln for ln in src.splitlines()
|
||||
if not ln.lstrip().startswith("#"))
|
||||
# The two scans the refactor removed: a per-key sum scan and the set
|
||||
# comprehension that fed sorted(). Both must be absent from main().
|
||||
removed_patterns = [
|
||||
"sum(1 for v in rep[\\\"verdicts\\\"]",
|
||||
"{v[\\\"verdict\\\"] for v in rep[\\\"verdicts\\\"]}",
|
||||
]
|
||||
gone = {p: code.count(p) for p in removed_patterns}
|
||||
# The consolidated Counter is built exactly once (not twice, not zero).
|
||||
builds = code.count("verdict_counts = collections.Counter(v[")
|
||||
red_ok = all(v == 0 for v in gone.values()) and builds == 1
|
||||
print(f"[3] redundancy removed scans={gone} verdict_counts builds={builds} "
|
||||
f"{'PASS' if red_ok else 'FAIL'}")
|
||||
if not red_ok:
|
||||
failures.append("[3] redundancy not collapsed")
|
||||
|
||||
# Cross-check: the ORIGINAL (HEAD~1) must still carry the removed scans,
|
||||
# i.e. this is a genuine before/after, not a no-op comparison.
|
||||
with open(orig_path, "r", encoding="utf-8") as fh:
|
||||
osrc = fh.read()
|
||||
ocode = "\n".join(ln for ln in osrc.splitlines()
|
||||
if not ln.lstrip().startswith("#"))
|
||||
had_setcomp = "{v[\"verdict\"] for v in rep[\"verdicts\"]}" in ocode
|
||||
had_sumscan = "sum(1 for v in rep[\"verdicts\"] if v[\"verdict\"] == k)" in ocode
|
||||
original_ok = had_setcomp and had_sumscan
|
||||
print(f"[3b] original carries the removed scans "
|
||||
f"setcomp={had_setcomp} sumscan={had_sumscan} "
|
||||
f"{'PASS' if original_ok else 'FAIL'}")
|
||||
if not original_ok:
|
||||
failures.append("[3b] original does not carry the pre-refactor scans")
|
||||
|
||||
finally:
|
||||
shutil.rmtree(orig_dir, ignore_errors=True)
|
||||
|
||||
print("\nVERDICT:",
|
||||
"ALL GREEN -- doc_census main() 3 scans -> 1 Counter pass, "
|
||||
"selftest byte-identical, --json byte-identical" if not failures
|
||||
else "MISMATCH -- " + "; ".join(failures))
|
||||
return 0 if not failures else 1
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in new issue
Block a user