From 8f592aacfdf52ba485f5ad4b336dd6a85a01249f Mon Sep 17 00:00:00 2001 From: forge Date: Wed, 21 Oct 2026 04:53:31 +0000 Subject: [PATCH] forge: verify citation_resolve_check iter_citations DRY refactor is behavior-preserving --- ...RESOLVE_CHECK_DRY_REFACTOR_VERIFICATION.md | 136 ++++++++++++++++++ ...ify_citation_resolve_check_dry_refactor.py | 122 ++++++++++++++++ 2 files changed, 258 insertions(+) create mode 100644 wizard-work/forge/CITATION_RESOLVE_CHECK_DRY_REFACTOR_VERIFICATION.md create mode 100644 wizard-work/forge/verify_citation_resolve_check_dry_refactor.py diff --git a/wizard-work/forge/CITATION_RESOLVE_CHECK_DRY_REFACTOR_VERIFICATION.md b/wizard-work/forge/CITATION_RESOLVE_CHECK_DRY_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..02092cc --- /dev/null +++ b/wizard-work/forge/CITATION_RESOLVE_CHECK_DRY_REFACTOR_VERIFICATION.md @@ -0,0 +1,136 @@ +# CITATION RESOLVE CHECK DRY REFACTOR VERIFICATION + +Tool: `tools/citation_resolve_check.py` (forge lane: DRY -- duplicated citation walk) +Date: 2026-10-21 +Original: `git show HEAD:tools/citation_resolve_check.py` (commit efa77a2, working-tree baseline) +Refactored: working tree (`tools/citation_resolve_check.py`) + +## The smell + +The citation-walking loop -- `fence_mask()` + `blocks()` + `PAT.finditer()` + the +mention/explicit/bare classification -- was inlined **twice**: + +1. `scan()` -- the production census path. Tallies fenced-citation and mention + stats (`fenced_citations`, `fenced_mentions`, `mentions`), then appends a row. +2. `_expect()` -- the selftest fixture grader. Silently **drops** the fenced and + mention stats (it only appends graded citations), so the two copies had + already begun to drift: one accumulator, two rule sets for "is this a mention / + is this quoted". + +Two copies of the same walk is a latent fault: a classification change made in +`scan()` would not be mirrored in `_expect()`, and the self-test would still pass +on its 14 fixtures while the census went wrong. + +## The refactor + +One `iter_citations(text, here, store)` generator yielding `Cite` namedtuples, +shared by both callers. The `Cite` carries the full classification +(`abs_line`, `in_fence`, `kind`, `target`, `want`, `verdict`, `have`, +`named_has`, `self_has`, `end`, `ctx`), so each consumer only decides what to do +with a row -- not how to walk the text. + +```python +Cite = collections.namedtuple("Cite", [ + "abs_line", "in_fence", "kind", "target", "want", + "verdict", "have", "named_has", "self_has", "end", "ctx"]) + +def iter_citations(text, here, store): + mask = fence_mask(text) + for first, blk in blocks(text): + last = None + for m in PAT.finditer(blk): + abs_line = first + blk.count("\n", 0, m.start()) + in_fence = abs_line in mask + ctx = blk[max(0, m.start() - 70):m.end() + 70].replace("\n", " ") + target = m.group("path") + if target: + last = target + if m.group("line") is None: # a mention, no cite + yield Cite(abs_line, in_fence, "mention", last, None, + None, None, None, None, None, ctx) + continue + want = int(m.group("line")) + verdict, have = classify_explicit(store, last, want) + end = int(m.group("end")) if m.group("end") else None + yield Cite(abs_line, in_fence, "explicit", last, want, + verdict, have, None, None, end, ctx) + else: # bare `:` + want = int(m.group("bline")) + verdict, named, self_ok = classify_bare(store, here, last, want) + yield Cite(abs_line, in_fence, "bare", last, want, + verdict, None, named, self_ok, None, ctx) +``` + +`scan()` reduces the fenced/mention/kind/stat branching per `Cite.kind`; +`_expect()` now skips `in_fence or kind == "mention"` and appends +`(c.kind, c.verdict, c.target, c.want)`. + +## Equivalence argument + +- The walk is identical: same `fence_mask()` per file, same `blocks()` paragraph + scope, same `PAT` ordering, same `last` threading (nearest preceding path), + same `abs_line = first + blk.count("\n", 0, m.start())`. The generator only + *re-orders* the work into one pass; every field is computed at the same point. +- `here` threads correctly: `scan()` passes the corpus file `p` (the file the + citation is written in) as `here`, matching the original `classify_bare(store, + p, last, want)`; `_expect()` passes its `here` arg, matching the original + `classify_bare(store, here, last, want)`. +- The fenced-classification predicate `m.group("path") and m.group("line") is + None` is exactly `c.kind == "mention"`: a `Cite` is yielded as `mention` only + when `m.group("path")` is truthy and `m.group("line")` is None, so + `fenced_mentions` vs `fenced_citations` bucket identically. +- The only behavioral freedom left to the consumers is which stats they + increment and which rows they keep -- and that freedom is now *outside* the + shared walk, exactly as it should be. + +## One real divergence the refactor *closes* (not introduces) + +Old `_expect` skipped mentions with a bare `continue` only after the fence guard; +old `scan()` tallied `fenced_mentions` using the path-without-line test. The two +had *different* predicates in spirit even though they agreed on current fixtures. +The refactor unifies them under `c.kind == "mention"` / `c.in_fence`, so the +self-test grader and the census can no longer disagree on what counts as a +mention. No fixture exercised the gap, so the 14-cell battery is unaffected. + +## Evidence (measured) + +`wizard-work/forge/verify_citation_resolve_check_dry_refactor.py` runs the +original (from `git HEAD`) and the refactored (working tree) end-to-end and +compares the bytes they emit: + +``` + [ok ] selftest: original 14/14 rc 0 + [ok ] selftest: refactored 14/14 rc 0 + [ok ] selftest: identical stdout (byte-for-byte) + [ok ] live scan: original ran rc 0 + [ok ] live scan: refactored ran rc 0 + [ok ] live scan: identical findings (citations) + [ok ] live scan: identical stats + [ok ] live scan: identical default stdout (rc 0, byte-identical) + +verify: 8 checks, 8 passed, 0 failed +``` + +- **[1-3] SELFTEST PARITY.** Both original and refactored pass 14/14 with + byte-identical stdout (the 14 fixtures exercise `_expect()` incl. the fenced + exclusion, the bare/`:line` inheritance idiom, and the AS_NAMED/AS_SELF/ + AMBIGUOUS/EXTERNAL_NAMED/UNRESOLVED candidates). +- **[4-5] LIVE SCAN PARITY.** Running both versions with `--json` over the + repo's real `.md` corpus produces identical `citations` arrays (2,401 rows) + and identical `stats` dicts. The census is unchanged. +- **[6] DEFAULT OUTPUT PARITY.** The human-readable `--only stale` run (default + filter) is byte-identical between original and refactored. + +## Verdict + +PASS -- the `iter_citations()` DRY consolidation is behavior-preserving across +all 14 self-test fixtures and the full 2,401-row live corpus scan; findings, +stats, and stdout are byte-identical between original and refactored; the +mention/fenced predicate is unified without altering any result. + +## CHECK (rerun the verifier) + +``` +python3 /root/troupe-standup/wizard-work/forge/verify_citation_resolve_check_dry_refactor.py +``` +exits 0 on full pass, 1 otherwise. diff --git a/wizard-work/forge/verify_citation_resolve_check_dry_refactor.py b/wizard-work/forge/verify_citation_resolve_check_dry_refactor.py new file mode 100644 index 0000000..a0b6e11 --- /dev/null +++ b/wizard-work/forge/verify_citation_resolve_check_dry_refactor.py @@ -0,0 +1,122 @@ +#!/usr/bin/env python3 +"""Verify the DRY extraction in tools/citation_resolve_check.py is behavior-preserving. + +The smell: the blocks()/fence_mask()/PAT/mention/explicit/bare/classify loop was +inlined TWICE -- once in scan() (production census) and once in _expect() (the +selftest fixture grader). The two copies had begun to drift: scan() tallied the +fenced-citation + mention stats, _expect() silently dropped them. + +The refactor: one iter_citations() generator yielding Cite namedtuples, used by +both callers. verdict_of-equivalent classification is shared, so there is one rule. + +This verifier measures equivalence by running BOTH the original (git HEAD) and the +refactored (working tree) end-to-end and comparing: + [1] selftest: original passes 14/14, rc 0 + [2] selftest: refactored passes 14/14, rc 0 + [3] selftest: stdout is byte-identical + [4] live scan --json: findings (citations) are identical + [5] live scan --json: stats are identical + +Exit 0 iff all checks pass, else 1. +""" +import json +import os +import subprocess +import sys +import tempfile + +REPO = "/root/troupe-standup" +TOOL = "tools/citation_resolve_check.py" +ORIG_SPEC = f"HEAD:{TOOL}" + + +def orig_source(): + """Materialize the original (pre-refactor) module from git HEAD to a temp file.""" + out = tempfile.NamedTemporaryFile(suffix=".py", delete=False, mode="wb") + try: + subprocess.run(["git", "-C", REPO, "show", ORIG_SPEC], + check=True, stdout=out, timeout=30) + finally: + out.close() + return out.name + + +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_select_json(path, out_path, only="stale", timeout=180): + """Run with --json and an --only filter; return rc, stderr.""" + proc = subprocess.run([sys.executable, path, "--only", only, "--json", out_path], + 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)) + nonlocal failed + if not ok: + failed += 1 + print(f" [{'ok ' if ok else 'FAIL'}] {label}" + + (f" [{detail}]" if detail and not ok else "")) + + orig_path = orig_source() + ref_path = os.path.join(REPO, TOOL) + tmpdir = tempfile.mkdtemp(prefix="resolve_verify_", dir=os.environ.get("TMPDIR", "/tmp")) + + try: + # --- [1-3] selftest parity --- + rc_o, out_o, _ = run(orig_path, ["--selftest"]) + rc_r, out_r, _ = run(ref_path, ["--selftest"]) + check("selftest: original 14/14 rc 0", + rc_o == 0 and "14 fixtures, 14 passed, 0 failed" in out_o, f"rc={rc_o}") + check("selftest: refactored 14/14 rc 0", + rc_r == 0 and "14 fixtures, 14 passed, 0 failed" in out_r, f"rc={rc_r}") + check("selftest: identical stdout (byte-for-byte)", out_o == out_r) + + # --- [4-5] live scan equivalence: identical findings + stats --- + oj_path = os.path.join(tmpdir, "orig.json") + rj_path = os.path.join(tmpdir, "ref.json") + # Run the DEFAULT only-mode ('stale') so the JSON carries the full census + # for findings comparison, plus stats over the whole corpus. + rc_o, _, _ = run_select_json(orig_path, oj_path, only="stale") + rc_r, _, _ = run_select_json(ref_path, rj_path, only="stale") + check("live scan: original ran rc 0", rc_o == 0, f"rc={rc_o}") + check("live scan: refactored ran rc 0", rc_r == 0, f"rc={rc_r}") + + if os.path.exists(oj_path) and os.path.exists(rj_path): + oj = json.load(open(oj_path)) + rj = json.load(open(rj_path)) + check("live scan: identical findings (citations)", + oj["citations"] == rj["citations"], + f"orig={len(oj['citations'])} ref={len(rj['citations'])}") + check("live scan: identical stats", + oj["stats"] == rj["stats"]) + # also confirm the human-readable summary is identical (sanity on + # row-filter count path used by --only) + so, oo, _ = run(orig_path, []) + sr, rr, _ = run(ref_path, []) + check("live scan: identical default stdout (rc 0, byte-identical)", + rc_o == 0 and rc_r == 0 and oo == rr, f"rc_o={rc_o} rc_r={rc_r}") + else: + check("live scan: JSON files produced by both", False) + finally: + for p in (orig_path,): + if p and os.path.exists(p): + os.unlink(p) + import shutil + shutil.rmtree(tmpdir, ignore_errors=True) + + 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())