forge: citation_hash_class verdict_of() DRY extraction, behavior-preserving
This commit is contained in:
forge committed
1 parent
f1d40d1da7
commit
033acf3298
2 files changed
+226
No files matched your search
@@ -0,0 +1,92 @@
|
||||
# CITATION_HASH_CLASS DRY REFACTOR VERIFICATION
|
||||
|
||||
Tool: `tools/citation_hash_class.py` (forge lane: DRY, verdict-classification logic)
|
||||
Date: 2026-10-21
|
||||
Original: `git show HEAD:tools/citation_hash_class.py`
|
||||
Refactored: working tree (`tools/citation_hash_class.py`)
|
||||
|
||||
## The smell
|
||||
|
||||
The verdict-classification logic -- the four-branch `if/elif/else` chain that
|
||||
maps `(bf, fam, after_size)` to one of `BOUND_UNRESOLVED`, `CLASS_MISMATCH`,
|
||||
`CLASSLESS_AFTER_SIZE`, or `ok` -- was inlined **twice**: once in `scan()`
|
||||
(the production census path) and once in `selftest()` (the fixture verifier).
|
||||
The two copies had **quietly diverged**:
|
||||
|
||||
- `scan()` guarded CLASS_MISMATCH with `fam not in ('AMBIGUOUS', 'UNRESOLVED')`
|
||||
- `selftest()` omitted that guard, so `AMBIGUOUS + bf != fam` would have been
|
||||
classified `CLASS_MISMATCH` in the test where it is `ok` in production.
|
||||
|
||||
No existing fixture exercised the AMBIGUOUS+bound path, so the divergence was
|
||||
invisible. Two copies of the same rule is a latent fault waiting for a fixture
|
||||
to cover it.
|
||||
|
||||
## The refactor
|
||||
|
||||
One `verdict_of(bf, fam, after_size)` function, replacing both inline blocks.
|
||||
The canonical logic is the one `scan()` already used (the production path), so
|
||||
the selftest copy is now corrected to match, not the other way around.
|
||||
|
||||
```python
|
||||
def verdict_of(bf, fam, after_size):
|
||||
if bf and fam == 'UNRESOLVED':
|
||||
return 'BOUND_UNRESOLVED'
|
||||
if bf and fam not in ('AMBIGUOUS', 'UNRESOLVED') and bf != fam:
|
||||
return 'CLASS_MISMATCH'
|
||||
if not bf and after_size and fam in ('BLOB_OID', 'CONTENT_SHA256'):
|
||||
return 'CLASSLESS_AFTER_SIZE'
|
||||
return 'ok'
|
||||
```
|
||||
|
||||
`scan()` now calls `verdict = verdict_of(bf, fam, after_size)`.
|
||||
`selftest()` now calls `got = verdict_of(bf, fam, after)`.
|
||||
|
||||
## Equivalence argument
|
||||
|
||||
- The verdict is a pure function of `(bf, fam, after_size)`. Both callers
|
||||
compute `bf` from `bound_family()`, `fam` from `resolve()`, and `after_size`/`after`
|
||||
from the same `AFTER_SIZE` regex on the same window -- all unchanged.
|
||||
- `verdict_of()` encodes the `scan()` branch conditions verbatim, so every
|
||||
production result is identical by construction.
|
||||
- The `selftest()` branch for CLASS_MISMATCH is the only one that changed (it
|
||||
gains the `fam not in ('AMBIGUOUS', 'UNRESOLVED')` guard), and no fixture
|
||||
covers AMBIGUOUS+bound, so the 19-cell battery is unaffected.
|
||||
|
||||
## Evidence (measured)
|
||||
|
||||
`wizard-work/forge/verify_citation_hash_class_dry_refactor.py`:
|
||||
|
||||
```
|
||||
[ok ] selftest: original 19/19 rc 0
|
||||
[ok ] selftest: refactored 19/19 rc 0
|
||||
[ok ] selftest: identical stdout
|
||||
[ok ] verdict_of matches old inline logic on all VERDICT_FIXTURES
|
||||
[ok ] divergence: AMBIGUOUS+bound now 'ok' (not CLASS_MISMATCH)
|
||||
[ok ] live scan: identical findings JSON
|
||||
[ok ] live scan: identical stats
|
||||
|
||||
verify: 7 checks, 7 passed, 0 failed
|
||||
```
|
||||
|
||||
### Verdict breakdown
|
||||
|
||||
- **[1-3] SELFTEST PARITY.** Both original and refactored pass 19/19 with
|
||||
byte-identical stdout. The refactoring did not alter the selftest contract.
|
||||
|
||||
- **[4] VERDICT OF AGREEMENT.** `verdict_of()` returns the same verdict as the
|
||||
old inline `scan()` logic on every existing VERDICT_FIXTURE (4 rows:
|
||||
ok, CLASSLESS_AFTER_SIZE x2, CLASS_MISMATCH).
|
||||
|
||||
- **[5] DIVERGENCE CLOSED.** The refactored `verdict_of()` is confirmed to
|
||||
agree with the old `scan()` guard for the AMBIGUOUS+bound case: returns
|
||||
`'ok'`, not `'CLASS_MISMATCH'`. The selftest copy now matches production.
|
||||
|
||||
- **[6-7] LIVE SCAN PARITY.** Running both the original and refactored tool
|
||||
end-to-end with `--json` over the repo's actual corpus produces identical
|
||||
findings arrays and identical stats dicts. The census is unchanged.
|
||||
|
||||
## Verdict
|
||||
|
||||
PASS -- verdict-classification DRY consolidation is behavior-preserving across
|
||||
all 19 selftest fixtures and the full live corpus scan; the AMBIGUOUS+bound
|
||||
divergence between the two old copies is closed.
|
||||
@@ -0,0 +1,134 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Verify the verdict_of() DRY extraction from citation_hash_class.py is behavior-preserving.
|
||||
|
||||
Compares the ORIGINAL (pre-refactor) verdict logic against the REFACTORED version:
|
||||
1. selftest output is byte-identical (19/19 both)
|
||||
2. verdict_of() agrees with the old inline logic on every existing fixture
|
||||
3. The live scan on the repo produces identical JSON census
|
||||
4. The AMBIGUOUS+bound divergence (which the old selftest had but scan() didn't)
|
||||
is now FIXED: refactored uses one logic, not two
|
||||
"""
|
||||
import importlib.util
|
||||
import json
|
||||
import subprocess
|
||||
import sys
|
||||
|
||||
REPO = "/root/troupe-standup"
|
||||
ORIG = "/root/.hermes/wizard-outputs/forge/citation_hash_class.orig.py"
|
||||
|
||||
|
||||
def load_module(name, path):
|
||||
spec = importlib.util.spec_from_file_location(name, path)
|
||||
mod = importlib.util.module_from_spec(spec)
|
||||
spec.loader.exec_module(mod)
|
||||
return mod
|
||||
|
||||
|
||||
def run_selftest(path):
|
||||
"""Run --selftest on the given path; return (rc, stdout, stderr)."""
|
||||
proc = subprocess.run(
|
||||
[sys.executable, path, "--selftest"],
|
||||
capture_output=True, text=True, cwd=REPO, timeout=120)
|
||||
return proc.returncode, proc.stdout, proc.stderr
|
||||
|
||||
|
||||
def main():
|
||||
cells, failed = [], 0
|
||||
|
||||
def check(label, ok, detail=""):
|
||||
cells.append((label, ok))
|
||||
nonlocal failed
|
||||
if not ok:
|
||||
failed += 1
|
||||
print(f" [{'ok ' if ok else 'FAIL'}] {label}"
|
||||
+ (f" [{detail}]" if detail and not ok else ""))
|
||||
|
||||
orig = load_module("orig", ORIG)
|
||||
ref = load_module("ref", f"{REPO}/tools/citation_hash_class.py")
|
||||
|
||||
# --- [1] selftest: both pass 19/19, identical summary line ---
|
||||
rc_o, out_o, _ = run_selftest(ORIG)
|
||||
rc_r, out_r, _ = run_selftest(f"{REPO}/tools/citation_hash_class.py")
|
||||
check("selftest: original 19/19 rc 0",
|
||||
rc_o == 0 and "19 passed, 0 failed" in out_o,
|
||||
f"rc={rc_o}")
|
||||
check("selftest: refactored 19/19 rc 0",
|
||||
rc_r == 0 and "19 passed, 0 failed" in out_r,
|
||||
f"rc={rc_r}")
|
||||
check("selftest: identical stdout", out_o == out_r)
|
||||
|
||||
# --- [2] verdict_of matches the old inline scan() logic on VERDICT_FIXTURES ---
|
||||
# The old scan() logic (from the original source) for verdict:
|
||||
# if bf and fam == 'UNRESOLVED': -> 'BOUND_UNRESOLVED'
|
||||
# elif bf and fam not in ('AMBIGUOUS','UNRESOLVED') and bf != fam: -> 'CLASS_MISMATCH'
|
||||
# elif not bf and after_size and fam in ('BLOB_OID','CONTENT_SHA256'): -> 'CLASSLESS_AFTER_SIZE'
|
||||
# else: 'ok'
|
||||
def old_veridict(bf, fam, after_size):
|
||||
if bf and fam == 'UNRESOLVED':
|
||||
return 'BOUND_UNRESOLVED'
|
||||
if bf and fam not in ('AMBIGUOUS', 'UNRESOLVED') and bf != fam:
|
||||
return 'CLASS_MISMATCH'
|
||||
if not bf and after_size and fam in ('BLOB_OID', 'CONTENT_SHA256'):
|
||||
return 'CLASSLESS_AFTER_SIZE'
|
||||
return 'ok'
|
||||
|
||||
all_ok = True
|
||||
for text, tok, fam, want in orig.VERDICT_FIXTURES:
|
||||
i = text.index(tok)
|
||||
bf, _ = orig.bound_family(text, i, i + len(tok), 40)
|
||||
after = bool(orig.AFTER_SIZE.search(text[max(0, i - 40):i]))
|
||||
old_v = old_veridict(bf, fam, after)
|
||||
new_v = ref.verdict_of(bf, fam, after)
|
||||
if old_v != new_v:
|
||||
all_ok = False
|
||||
print(f" DIVERGE on {text!r}: old={old_v} new={new_v}")
|
||||
check("verdict_of matches old inline logic on all VERDICT_FIXTURES",
|
||||
all_ok)
|
||||
|
||||
# --- [3] The AMBIGUOUS+bound divergence is resolved ---
|
||||
# Old scan(): AMBIGUOUS + bf != fam -> 'ok' (excluded by guard)
|
||||
# Old selftest(): AMBIGUOUS + bf != fam -> 'CLASS_MISMATCH' (guard missing)
|
||||
# Refactored: verdict_of uses the scan() guard, so AMBIGUOUS+bound -> 'ok'
|
||||
check("divergence: AMBIGUOUS+bound now 'ok' (not CLASS_MISMATCH)",
|
||||
ref.verdict_of('BLOB_OID', 'AMBIGUOUS', False) == 'ok'
|
||||
and ref.verdict_of('BLOB_OID', 'AMBIGUOUS', False)
|
||||
== old_veridict('BLOB_OID', 'AMBIGUOUS', False))
|
||||
|
||||
# --- [4] Live scan JSON on the repo is identical ---
|
||||
# Run both versions producing JSON and compare the findings.
|
||||
import tempfile, os
|
||||
with tempfile.NamedTemporaryFile(suffix='.json', delete=False, mode='w') as f:
|
||||
orig_json_path = f.name
|
||||
with tempfile.NamedTemporaryFile(suffix='.json', delete=False, mode='w') as f:
|
||||
ref_json_path = f.name
|
||||
try:
|
||||
subprocess.run([sys.executable, ORIG, "--json", orig_json_path],
|
||||
capture_output=True, text=True, cwd=REPO, timeout=120,
|
||||
check=True)
|
||||
subprocess.run([sys.executable, f"{REPO}/tools/citation_hash_class.py",
|
||||
"--json", ref_json_path],
|
||||
capture_output=True, text=True, cwd=REPO, timeout=120,
|
||||
check=True)
|
||||
with open(orig_json_path) as f:
|
||||
oj = json.load(f)
|
||||
with open(ref_json_path) as f:
|
||||
rj = json.load(f)
|
||||
# Compare findings (rows) -- these are the verdicts produced by scan()
|
||||
same_findings = oj["findings"] == rj["findings"]
|
||||
check("live scan: identical findings JSON", same_findings,
|
||||
f"orig={len(oj['findings'])} ref={len(rj['findings'])}")
|
||||
# Compare stats
|
||||
same_stats = oj["stats"] == rj["stats"]
|
||||
check("live scan: identical stats", same_stats)
|
||||
finally:
|
||||
os.unlink(orig_json_path)
|
||||
os.unlink(ref_json_path)
|
||||
|
||||
total = len(cells)
|
||||
passed = total - failed
|
||||
print(f"\nverify: {total} checks, {passed} passed, {failed} failed")
|
||||
return 1 if failed else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
sys.exit(main())
|
||||
Reference in new issue
Block a user