forge: harden probe_forge verifier against no_main_guard/SystemExit mutant-escape; 12/12 checks

wizard-work/forge/PROBE_FORGE_REFACTOR_VERIFICATION.md

@@ -0,0 +1,98 @@
+PROBE_FORGE_REFACTOR_VERIFICATION.md -- forge refactor verification.
+
+Target: /root/troupe-standup/wizard-work/forge/_probe_forge.py
+        -> /root/troupe-standup/wizard-work/forge/_probe_forge_refactored.py
+Lane:    Refactor (code review + refactors)
+Date:    2026-10-25 (refactor); verifier hardened 2026-10-26
+
+PURPOSE
+  The forge lane refactored `_probe_forge.py` -- a round-trip probe of the
+  forge chunker -- into `_probe_forge_refactored.py`. The refactor is
+  behavior-preserving: it exercises the same chunker API (chunk_text /
+  reconstruct) with the same inputs, emitting byte-identical output.
+
+SMELLS REMOVED (from the original `_probe_forge.py`)
+  1. Import-time side effect: the original ran its entire probe at module
+     top-level (a module-level `for` loop printing immediately on import).
+     There was no `if __name__ == "__main__"` guard, so importing the module
+     executed and printed the probe output. The refactored version moves all
+     execution under `main()` with an explicit `__main__` guard.
+
+  2. Dead variable `shorter`: computed as
+     `shorter = min(len(r), len(text_stripped))` inside the for-else `else`
+     branch but never read -- every subsequent statement used
+     `len(r)` and `len(text_stripped)` directly. Removed; output unchanged.
+
+  3. Inlined first-diff search: the inline `for i in range(min(...))` loop
+     with its for-else side-effects is extracted into
+     `_find_first_diff(actual, expected) -> int | None`, a pure function
+     returning the mismatch index or `None`, decoupling the search from I/O so
+     the control flow reads as intent ("is there a diff, and if so, show it").
+
+  4. Inlined chunk-structure printing: the loop that prints the first 5 chunks
+     with sentinels revealed is extracted into
+     `_print_chunk_structure(chunks, sentinel, n)`, isolating the formatting
+     concern and moving the data (`base`, `text`, `cs`, `r`) inside `main()`
+     instead of at module scope.
+
+VERIFIER HARDENING (the fix, applied after the refactor commit)
+  The verifier's import was done through an unguarded `importlib.import_module`
+  call. A guard-less subject that calls `sys.exit` at module scope raises
+  `SystemExit(0)` during import, which propagated and killed the verifier
+  process with rc 0 BEFORE reaching its final `assert passed == total, sys.exit(1)`.
+  Net effect on a defective tree: output leaked to real stdout and the harness
+  reported green (rc 0). This is exactly the `no_main_guard` mutant it is meant
+  to catch.
+
+  Fix: replaced `_import_refactored()` with `_safe_import()`, which wraps the
+  import in try/except BaseException (so SystemExit is caught), captures
+  stdout/stderr via redirect, and returns `(module, exc, out, err)`. Both call
+  sites (`check_import_side_effect_free` and `check_find_first_diff_correctness`)
+  use it, so a guard-less subject now FAILS loudly instead of escaping. With the
+  real subject carrying a proper `__main__` guard, the import is clean and
+  every check proceeds normally.
+
+SAFETY: byte-identical output
+  The refactor does NOT change behavior. The chunker calls are identical
+  (`chunker.chunk_text(text, max_tokens=64, overlap_tokens=8)` and
+  `chunker.reconstruct(cs, 8)`), the base string is identical (adjacent string
+  literals concatenate to the same value), and `_find_first_diff` preserves the
+  original for-else semantics: returns the index of the first mismatch, or
+  `None` when the common prefix matches (triggering the "no char diff" branch).
+
+VERIFICATION (all executed, real exit codes)
+  1. python3 /root/troupe-standup/wizard-work/forge/verify_probe_forge_refactor.py
+     -> 12/12 checks passed, exit 0
+  2. python3 _probe_forge.py           (orig  stdout: 790 bytes)
+  3. python3 _probe_forge_refactored.py (ref   stdout: 790 bytes)
+  4. diff -> IDENTICAL (no output); the verifier's check_identical_output
+     confirms orig.stdout == ref.stdout, orig.stderr == ref.stderr, and
+     orig.returncode == ref.returncode (0 == 0).
+  5. Round-trip assertion: probe output reports "equal: True", nchunks: 12.
+  6. pytest test_chunker_refactor.py -q -> 13 passed (chunker unchanged).
+
+VERIFIER CHECKS (verify_probe_forge_refactor.py, 12/12)
+  [OK] py_compile passes
+  [OK] identical output (exit 0, 790 bytes)
+  [OK] __main__ guard present
+  [OK] no 'global' statements in code
+  [OK] dead 'shorter' variable removed
+  [OK] _find_first_diff, _print_chunk_structure, main extracted;
+       main calls helpers + chunker API
+  [OK] import is side-effect-free (no probe output at import)
+  [OK] round-trip lossless (equal: True in probe output)
+  [OK] original still exhibits the removed smells (refactor non-vacuous)
+  [OK] _find_first_diff correct on all 10 cases (mutation kill)
+  [OK] 'equal' is computed dynamically from actual == expected (no hardcode)
+  [OK] overlap values consistent (chunk_text=8, reconstruct=8)
+
+NON-VACUITY GUARD
+  The verifier's check 9 confirms the *original* `_probe_forge.py` still
+  exhibits every removed smell: no `__main__` guard, dead `shorter = min(...)`
+  variable present, and a module-level `for` loop. The refactor is therefore
+  non-vacuous -- it addresses real, reproduced defects, not hypotheticals.
+
+VERDICT: probe_forge refactor verified -- behavior-preserving (byte-identical,
+equal: True, 12 chunks), 4 smells removed, and the verifier itself hardened
+against the SystemExit/no_main_guard mutant-escape that previously let a
+defective tree report rc 0. Final harness: 12/12 checks, exit 0.

wizard-work/forge/verify_probe_forge_refactor.py

@@ -86,10 +86,29 @@ def _ensure_path() -> None:
         sys.path.insert(0, WORK_DIR)
 
 
-def _import_refactored():
+def _safe_import():
+    """Import MOD_NAME from WORK_DIR, capturing stdout/stderr and any exception.
+
+    Returns ``(module, exc, out, err)``.  ``exc`` is set when the import raised,
+    including ``SystemExit`` from a guard-less module that calls ``sys.exit``
+    at module scope.
+
+    CENTRALISED GUARD: an unguarded ``importlib.import_module`` used to let a
+    ``SystemExit(0)`` propagate and kill the verifier process with rc 0,
+    masking the ``no_main_guard`` mutant (the probe output leaked to real
+    stdout and the verifier died before reaching its final ``sys.exit(1)``,
+    so a defective tree reported green).  Both call sites below use this
+    helper so neither can forget the try/except.
+    """
     sys.modules.pop(MOD_NAME, None)
     _ensure_path()
-    return importlib.import_module(MOD_NAME)
+    out, err = io.StringIO(), io.StringIO()
+    try:
+        with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err):
+            mod = importlib.import_module(MOD_NAME)
+    except BaseException as exc:
+        return None, exc, out.getvalue(), err.getvalue()
+    return mod, None, out.getvalue(), err.getvalue()
 
 
 def check_py_compile() -> bool:
@@ -187,17 +206,25 @@ def check_helpers_extracted() -> bool:
 
 
 def check_import_side_effect_free() -> bool:
-    """Importing the refactored module must not run the probe or print."""
-    mod = _import_refactored()
-    out, err = io.StringIO(), io.StringIO()
-    # Re-import cleanly to capture any side effects.
-    sys.modules.pop(MOD_NAME, None)
-    _ensure_path()
-    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: "
-              f"stdout={out.getvalue()!r} stderr={err.getvalue()!r}")
+    """Importing the refactored module must not run the probe or print, and must
+    not terminate the process.
+
+    A missing ``__main__`` guard that calls ``sys.exit`` at module scope surfaces
+    here as ``SystemExit`` during import.  Because ``SystemExit(0)`` propagates
+    and exits the verifier with code 0, it used to let the ``no_main_guard``
+    mutant *appear* to pass -- the verifier died before reaching its final
+    ``sys.exit(1)``.  ``_safe_import`` now catches every ``BaseException``
+    (including ``SystemExit``) and treats it as a FAIL, so the defect is
+    reported instead of crashing the harness.
+    """
+    mod, exc, out, err = _safe_import()
+    if exc is not None:
+        print(f"FAIL: import raised {type(exc).__name__}: {exc!r} "
+              f"(stdout={out!r} stderr={err!r})")
+        return False
+    if out or err:
+        print(f"FAIL: import produced output: stdout={out!r} "
+              f"stderr={err!r}")
         return False
     print("OK: import is side-effect-free (no probe output at import)")
     return True
@@ -247,8 +274,17 @@ def check_find_first_diff_correctness() -> bool:
     the return-i path is never exercised by check_identical_output.  By calling
     _find_first_diff directly with crafted strings that DO differ, we force the
     return path and catch the mutants.
+
+    Uses ``_safe_import`` so a guard-less subject (calling ``sys.exit`` at module
+    scope) is caught here as a FAIL rather than crashing the verifier via an
+    unguarded ``SystemExit``.
     """
-    mod = _import_refactored()
+    mod, exc, out, err = _safe_import()
+    if exc is not None:
+        print(f"FAIL: import raised {type(exc).__name__}: {exc!r} "
+              f"(stdout={out!r} stderr={err!r})")
+        return False
+    assert mod is not None  # _safe_import: exc set iff module is None
 
     cases = [
         # (actual, expected, expected_result)