diff --git a/wizard-work/forge/TRIAGE_DUMP_PENDING_REFACTOR_VERIFICATION.md b/wizard-work/forge/TRIAGE_DUMP_PENDING_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..d4391f4 --- /dev/null +++ b/wizard-work/forge/TRIAGE_DUMP_PENDING_REFACTOR_VERIFICATION.md @@ -0,0 +1,106 @@ +# forge — triage_dump_pending probe: behavior-preserving refactor (VERIFICATION) + +**Lane:** Refactor — code review and refactors +**Artifact committed to:** `wizard-work/forge/` (`_triage_dump_pending.py`, `_triage_dump_pending_refactored.py`, `verify_triage_dump_pending_refactor.py`) +**Verdict:** 9/9 checks pass; probe output byte-identical to the original (7932 bytes). + +## Target + +`_triage_dump_pending.py` dumps the decided/pending items from the triage inbox +seed `inbox/seed-2026-09-26.json` (9484 bytes: 1 decided, 12 pending). The +original ran its **entire** probe at module top level: it opened the JSON, loaded +it into a module global `path`/`d`, and printed everything at import time, with +no `if __name__ == "__main__"` guard, no function definitions, and a hardcoded +path at module scope. Importing the module therefore executed the probe and +wrote to stdout. + +## Smells fixed (behavior-preserving) + +1. **Module-level execution.** All file I/O, JSON loading, and printing now live + under `main()`. The module imports with zero side effects; the entry point is + explicitly guarded with `if __name__ == "__main__": sys.exit(main())`. + - *Measured:* `check_import_side_effect_free` — importing the refactored + module produces no stdout/stderr (original prints 7932 bytes at import). + +2. **Hardcoded path at module scope.** The bare `path = "/root/.../seed-...json"` + assignment is promoted to the `DEFAULT_PATH` constant and a `path` parameter + of `main(path=DEFAULT_PATH)`. Callers/tests can override it without editing + source; the default is unchanged, so output is byte-identical when run with no + args. + - *Measured:* `check_main_path_parameter` — `main` AST has a `path` arg. + +3. **No function extraction.** Three inline print blocks (summary header, + decided iteration, pending iteration) are extracted into pure helpers + `_print_summary`, `_print_decided`, `_print_pending` (plus `load_data`); data + flows from `main` into the helpers as parameters instead of the module global + `d`. + - *Measured:* `check_helpers_extracted` — all five module-level functions + exist and `main` calls the three print helpers. + +4. **Implicit module-global coupling.** The module-level `d` variable is gone. + - *Measured:* `check_no_globals` — no `global` statements in code + (`_code_only` strips docstring/comments to avoid false positives from the + refactored module's docstring, which *names* the smells). + +## Verification harness + +`verify_triage_dump_pending_refactor.py` — 9 checks, each a real subprocess/AST +assertion (no tautology): + +| # | check | what it proves | +|---|-------|-----------------| +| 1 | `py_compile` | refactored file is syntactically valid | +| 2 | identical output | `original` vs `refactored` produce byte-identical stdout+stderr AND exit code | +| 3 | `__main__` guard | refactored has the guard (original's smell) | +| 4 | no `global` | no `global` statements in code | +| 5 | import side-effect-free | importing produces no output | +| 6 | helpers extracted | `_print_summary/_print_decided/_print_pending/load_data/main` exist; `main` calls the three printers | +| 7 | `main(path)` param | `main` accepts `path` (configurable, not module-scope) | +| 8 | original still smells | non-vacuous: original still lacks guard, still has module-scope path, still has module-level `for`, still has NO function defs | +| 9 | path override honoured | `main(path=custom)` prints the custom file, not `DEFAULT_PATH` — kills the "accepts-but-ignores-path" mutation, which is byte-identical on the real seed and thus evades check 2 | + +## Evidence (measured, this run) + +``` +$ python3 wizard-work/forge/verify_triage_dump_pending_refactor.py +OK: py_compile passes +OK: identical output (exit 0, 7932 bytes) +OK: __main__ guard present +OK: no 'global' statements in code +OK: import is side-effect-free (no probe output at import) +OK: _print_summary, _print_decided, _print_pending, load_data, main extracted; main calls all three print helpers +OK: main(path) accepts a configurable path parameter +OK: original still exhibits the removed smells (refactor is non-vacuous) +OK: main(path=custom) honours the override (prints custom file, not DEFAULT_PATH) + +=== Verification: 9/9 checks passed === (exit 0) +``` + +Byte-identical proof (independent of the harness): + +``` +$ diff <(python3 _triage_dump_pending.py 2>&1) <(python3 _triage_dump_pending_refactored.py 2>&1) && echo IDENTICAL +IDENTICAL (diff exit 0) +``` + +Original's smell surface (0 function defs, 0 `__name__` references): + +``` +$ grep -c "def " _triage_dump_pending.py -> 0 +$ grep -c '__name__' _triage_dump_pending.py -> 0 +``` + +## Non-vacuity + +Check 8 guarantees the refactor actually removed something: the *original* +still exhibits every smell the refactored version no longer does (no `__main__` +guard, hardcoded path at module scope, module-level `for` loop, zero function +definitions). A no-op "refactor" would fail this. + +## Mutation kill + +Check 9 (path override) is the key discriminator the byte-comparison cannot see: +a `path` parameter that is accepted but ignored (always reads `DEFAULT_PATH`) +produces byte-identical output on the real seed but prints the wrong data on a +custom file. The check feeds a synthetic JSON and asserts the custom data +appears and the real path does not. diff --git a/wizard-work/forge/_triage_dump_pending.py b/wizard-work/forge/_triage_dump_pending.py new file mode 100644 index 0000000..3a9f55e --- /dev/null +++ b/wizard-work/forge/_triage_dump_pending.py @@ -0,0 +1,26 @@ +import json + +path = "/root/troupe-standup/inbox/seed-2026-09-26.json" +with open(path) as f: + d = json.load(f) + +print("SOURCE:", repr(d.get("source"))) +print("exported_at:", d.get("exported_at")) +print("decided_count:", len(d.get("decided", []))) +print("pending_count:", len(d.get("pending", []))) +print() + +print("=== DECIDED ===") +for x in d.get("decided", []): + print("id:", x.get("id"), "|", x.get("summary")) + print("decision:", x.get("decision"), "| escalated_by:", x.get("escalated_by")) + +print() +print("=== PENDING ===") +for x in d.get("pending", []): + print("id:", x.get("id")) + print(" summary:", x.get("summary")) + print(" escalated_by:", x.get("escalated_by")) + print(" proposed_action:", x.get("proposed_action")) + print(" evidence:", (x.get("evidence") or "")) + print() diff --git a/wizard-work/forge/_triage_dump_pending_refactored.py b/wizard-work/forge/_triage_dump_pending_refactored.py new file mode 100644 index 0000000..6177918 --- /dev/null +++ b/wizard-work/forge/_triage_dump_pending_refactored.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 +"""Triage dump-pending probe (refactored, forge-lane Refactor). + +Refactored from ``_triage_dump_pending.py`` (a copy of the repo-root +``.triage_dump_pending.py``). The original ran its entire probe at **module +top level** -- opening the JSON file, loading it, and printing all output at +import time, with no ``if __name__ == "__main__"`` guard. Importing it (for a +harness or monkeypatch) executed the probe and printed to stdout. + +Smells fixed in this refactor: + + * **Module-level execution** -- all file I/O, JSON loading, and printing now + live under ``main()``; the module imports with zero side effects and the + entry point is explicitly guarded. ``import _triage_dump_pending_refactored`` + produces no output and does not terminate the process. + + * **Hardcoded path at module scope** -- the JSON path was a bare module-level + assignment (``path = "/root/troupe-standup/..."``). It is promoted to the + named constant ``DEFAULT_PATH`` and made the ``path`` parameter of ``main``, + so callers (including tests) can override it without editing source. The + default is unchanged, so output is byte-identical when run with no args. + + * **No function extraction** -- the three inline print blocks (summary + header, decided iteration, pending iteration) are extracted into + ``_print_summary``, ``_print_decided``, and ``_print_pending``: pure + formatting helpers that take their data as arguments rather than reading + the module-global ``d``. + + * **Implicit module-global coupling** -- the module-level ``d`` variable is + removed: data flows from ``main`` into the formatting helpers as parameters. + +Output is byte-identical to the original on the current inbox seed +(7932 bytes, sha256 +``74d9d22e5ba47b6cb4250eaa08ee3516b9f7d86f3404e857de0dce1d8b364cd9``). +""" +from __future__ import annotations + +import json +import sys +from typing import Any + +DEFAULT_PATH = "/root/troupe-standup/inbox/seed-2026-09-26.json" + + +def load_data(path: str) -> dict[str, Any]: + """Load the triage seed JSON from *path*.""" + with open(path) as f: + return json.load(f) + + +def _print_summary(d: dict[str, Any]) -> None: + """Print the seed metadata and counts line, then a blank separator.""" + print("SOURCE:", repr(d.get("source"))) + print("exported_at:", d.get("exported_at")) + print("decided_count:", len(d.get("decided", []))) + print("pending_count:", len(d.get("pending", []))) + print() + + +def _print_decided(items: list[dict[str, Any]]) -> None: + """Print the decided items (one summary line + one decision line each).""" + print("=== DECIDED ===") + for x in items: + print("id:", x.get("id"), "|", x.get("summary")) + print("decision:", x.get("decision"), "| escalated_by:", x.get("escalated_by")) + + +def _print_pending(items: list[dict[str, Any]]) -> None: + """Print the pending items: a blank separator, the header, then a block per item.""" + print() + print("=== PENDING ===") + for x in items: + print("id:", x.get("id")) + print(" summary:", x.get("summary")) + print(" escalated_by:", x.get("escalated_by")) + print(" proposed_action:", x.get("proposed_action")) + print(" evidence:", (x.get("evidence") or "")) + print() + + +def main(path: str = DEFAULT_PATH) -> int: + """Load the triage seed at *path* and print its decided/pending contents. + + Returns 0 on success or 1 if the seed file cannot be read. + """ + try: + d = load_data(path) + except OSError as exc: + print(f"triage_dump_pending: cannot read -- {exc}", file=sys.stderr) + return 1 + _print_summary(d) + _print_decided(d.get("decided", [])) + _print_pending(d.get("pending", [])) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/wizard-work/forge/verify_triage_dump_pending_refactor.py b/wizard-work/forge/verify_triage_dump_pending_refactor.py new file mode 100644 index 0000000..aa6a37d --- /dev/null +++ b/wizard-work/forge/verify_triage_dump_pending_refactor.py @@ -0,0 +1,281 @@ +#!/usr/bin/env python3 +"""Verification harness for the _triage_dump_pending_refactored.py refactor. + +Exit 0 = all checks pass; exit 1 = at least one failure. + +The target ``_triage_dump_pending.py`` is a probe that dumps the decided/pending +items from the triage inbox seed. The refactor is *behavior-preserving* on the +probe itself (same JSON input, same output). Checks: + + 1. Refactored file compiles (py_compile). + 2. Original and refactored produce byte-identical stdout+stderr AND exit code. + 3. ``if __name__ == "__main__"`` guard present (no probe at import time). + 4. No ``global`` statements in code. + 5. Import of the refactored module is side-effect-free (no stdout/stderr at + import -- the original's primary smell). + 6. Pure helpers ``_print_summary``, ``_print_decided``, ``_print_pending``, + ``load_data``, and ``main`` extracted as module-level functions, and + ``main`` calls the three print helpers (not inline prints). + 7. ``main`` accepts a ``path`` parameter (the hardcoded path is now + configurable, not a module-level assignment that runs on import). + 8. Non-vacuity: the *original* ``_triage_dump_pending.py`` still exhibits the + removed smells (no __main__ guard, hardcoded path at module scope, + module-level for-loop, no function definitions). + 9. ``main(path)`` honours the override: calling with a custom temp JSON path + prints the custom file's contents. This kills the mutation of a + ``path`` parameter that is accepted but ignored (byte-identical on the + real seed would not catch this). +""" +from __future__ import annotations + +import ast +import contextlib +import difflib +import importlib +import io +import json +import os +import subprocess +import sys +import tempfile + +# Resolve paths from this file's location (wizard-work/forge/). +WORK_DIR = os.path.dirname(os.path.abspath(__file__)) +ORIGINAL = "_triage_dump_pending.py" +REFACTORED = "_triage_dump_pending_refactored.py" +MOD_NAME = REFACTORED[:-3] # strip .py + + +def _run(*args: str) -> subprocess.CompletedProcess: + return subprocess.run(args, cwd=WORK_DIR, capture_output=True, text=True) + + +def _read(name: str) -> str: + with open(os.path.join(WORK_DIR, name), encoding="utf-8") as f: + return f.read() + + +def _code_only(content: str) -> str: + """Strip the module docstring and comment-only lines. + + Smell-absence checks must inspect code, not documentation: the refactored + module's docstring *names* the patterns it removes, which would be false + positives as residual smells. + """ + tree = ast.parse(content) + skip: set[int] = set() + if (tree.body + and isinstance(tree.body[0], ast.Expr) + and isinstance(tree.body[0].value, ast.Constant) + and isinstance(tree.body[0].value.value, str)): + ds = tree.body[0] + for ln in range(ds.lineno, (ds.end_lineno or ds.lineno) + 1): + skip.add(ln) + kept = [] + for i, line in enumerate(content.splitlines(), 1): + if i in skip or line.lstrip().startswith("#"): + continue + kept.append(line) + return "\n".join(kept) + + +def check_py_compile() -> bool: + r = _run("python3", "-m", "py_compile", REFACTORED) + if r.returncode != 0: + print(f"FAIL: py_compile failed: {r.stderr}") + return False + print("OK: py_compile passes") + return True + + +def check_identical_output() -> bool: + orig = _run("python3", ORIGINAL) + ref = _run("python3", REFACTORED) + ot = orig.stdout + orig.stderr + rt = ref.stdout + ref.stderr + if ot != rt: + print("FAIL: original and refactored output differ") + print("".join(difflib.unified_diff( + ot.splitlines(keepends=True), rt.splitlines(keepends=True), + fromfile="original (" + ORIGINAL + ")", + tofile="refactored (" + REFACTORED + ")", + ))) + return False + if orig.returncode != ref.returncode: + print(f"FAIL: exit codes differ ({orig.returncode} vs {ref.returncode})") + return False + print(f"OK: identical output (exit {orig.returncode}, {len(ot)} bytes)") + return True + + +def check_main_guard() -> bool: + content = _read(REFACTORED) + if '__name__ == "__main__"' not in content: + print("FAIL: no __main__ guard") + return False + print("OK: __main__ guard present") + return True + + +def check_no_globals() -> bool: + code = _code_only(_read(REFACTORED)) + for i, line in enumerate(code.splitlines(), 1): + if line.lstrip().startswith("global "): + print(f"FAIL: 'global' found at code line {i}: {line.strip()}") + return False + print("OK: no 'global' statements in code") + return True + + +def check_import_side_effect_free() -> bool: + """Importing the refactored module must not run the probe or print.""" + sys.modules.pop(MOD_NAME, None) + if WORK_DIR not in sys.path: + sys.path.insert(0, WORK_DIR) + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + importlib.import_module(MOD_NAME) + if out.getvalue() or err.getvalue(): + print(f"FAIL: import produced output: stdout={out.getvalue()!r} " + f"stderr={err.getvalue()!r}") + return False + print("OK: import is side-effect-free (no probe output at import)") + return True + + +def check_helpers_extracted() -> bool: + tree = ast.parse(_read(REFACTORED)) + mod_funcs = { + n.name for n in tree.body + if isinstance(n, ast.FunctionDef) + } + for required in ("_print_summary", "_print_decided", "_print_pending", + "load_data", "main"): + if required not in mod_funcs: + print(f"FAIL: {required} not defined as module-level function") + return False + main_func = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "main") + name_calls = { + n.func.id for n in ast.walk(main_func) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Name) + } + for needed in ("_print_summary", "_print_decided", "_print_pending"): + if needed not in name_calls: + print(f"FAIL: main() does not call {needed}()") + return False + print("OK: _print_summary, _print_decided, _print_pending, load_data, " + "main extracted; main calls all three print helpers") + return True + + +def check_main_path_parameter() -> bool: + """``main`` must accept a ``path`` parameter so the JSON location is + configurable rather than hardcoded at module scope.""" + tree = ast.parse(_read(REFACTORED)) + main_func = next(n for n in tree.body + if isinstance(n, ast.FunctionDef) and n.name == "main") + params = {a.arg for a in main_func.args.args} + if "path" not in params: + print(f"FAIL: main() does not accept a 'path' parameter (params={params})") + return False + print("OK: main(path) accepts a configurable path parameter") + return True + + +def check_original_still_smells() -> bool: + """The original must still carry the smells we removed (non-vacuous).""" + src = _read(ORIGINAL) + code = _code_only(src) + problems = [] + # The original must LACK a __main__ guard (that is the smell we fixed). + if '__name__ == "__main__"' in src: + problems.append("original already has __main__ guard (refactor not needed)") + # The hardcoded path must still be at module scope in the original. + if "/root/troupe-standup/inbox/seed-2026-09-26.json" not in code: + problems.append("hardcoded path missing from original (expected)") + # The original must execute the probe at import time: module-level for-loop. + tree = ast.parse(src) + has_module_level_for = any(isinstance(n, ast.For) for n in tree.body) + if not has_module_level_for: + problems.append("no module-level for loop in original (expected)") + # The original must have NO function definitions (all logic is module-level). + func_defs = [n for n in tree.body if isinstance(n, ast.FunctionDef)] + if func_defs: + problems.append(f"original has function definitions: " + f"{[f.name for f in func_defs]}") + if problems: + print("FAIL: original does not exhibit the expected smells:") + for p in problems: + print(f" - {p}") + return False + print("OK: original still exhibits the removed smells " + "(refactor is non-vacuous)") + return True + + +def check_path_override_honoured() -> bool: + """Calling ``main(path=...)`` with a custom file must print that file's data. + + A mutation that accepts a ``path`` parameter but ignores it (always reads + DEFAULT_PATH) would pass check 2 (byte-identical on the real seed) but + fail here: the custom file's contents appear in the output. + """ + tmp_dir = tempfile.mkdtemp(prefix="triage_override_") + custom = os.path.join(tmp_dir, "custom.json") + data = { + "source": "override test", + "exported_at": "2099-01-01T00:00:00Z", + "decided": [{"id": 99, "summary": "overridden", "decision": "x", + "escalated_by": "tester"}], + "pending": [], + } + with open(custom, "w", encoding="utf-8") as f: + json.dump(data, f) + try: + sys.modules.pop(MOD_NAME, None) + if WORK_DIR not in sys.path: + sys.path.insert(0, WORK_DIR) + mod = importlib.import_module(MOD_NAME) + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = mod.main(path=custom) + combined = out.getvalue() + err.getvalue() + if "overridden" not in combined: + print(f"FAIL: main(path=custom) did not read the custom file " + f"(rc={rc}, output={combined!r})") + return False + if "/root/troupe-standup/inbox/seed-2026-09-26.json" in combined: + print("FAIL: main(path=custom) printed data from DEFAULT_PATH, not " + "the custom path") + return False + print("OK: main(path=custom) honours the override " + "(prints custom file, not DEFAULT_PATH)") + return True + finally: + import shutil + shutil.rmtree(tmp_dir, ignore_errors=True) + + +def main() -> None: + checks = [ + check_py_compile, + check_identical_output, + check_main_guard, + check_no_globals, + check_import_side_effect_free, + check_helpers_extracted, + check_main_path_parameter, + check_original_still_smells, + check_path_override_honoured, + ] + results = [fn() for fn in checks] + passed = sum(results) + total = len(results) + print(f"\n=== Verification: {passed}/{total} checks passed ===") + if passed != total: + sys.exit(1) + + +if __name__ == "__main__": + main()