forge: verify PDF skill refactor (pdf_create/pdf_make_form -> _pdf_common), fix harness false-green

wizard-work/forge/PDF_REFACTOR_VERIFICATION.md

@@ -0,0 +1,86 @@
+# Refactor Verification: PDF skill CLI helpers migrated to `_pdf_common`
+
+## Lane
+Refactor — code review and refactors.
+
+## Target
+The two reportlab-based CLI builders `pdf_create.py` and `pdf_make_form.py`
+(in `/root/.hermes/profiles/forge/skills/productivity/pdf/scripts/`) were the
+last holdouts still carrying a duplicated `_reconfigure_stdio()` shim and an
+inlined `print(json.dumps({...}))`. Both have been migrated in place to the
+shared `_pdf_common` helper module, consistent with the three already-migrated
+siblings (`pdf_fill_form.py`, `pdf_secure.py`, `pdf_watermark.py`) and the
+module's documented contract ("Eliminates duplication across the 12 CLI helpers").
+
+## What I verified (real execution)
+Ran `verify_pdf_refactor.py`, which runs the backed-up original (`before/`)
+and the in-place refactored script with the SAME `--output` path and identical
+JSON specs, then asserts behavior preservation:
+
+```
+=== pdf_create.py ===
+  [PASS] stdout byte-identical
+  [PASS] exit code identical
+  [PASS] stderr identical
+  [PASS] sut ran real path (not import-error exit 2)
+  [PASS] pdf page count identical
+  [PASS] pdf extracted text identical
+  [PASS] normalized pdf bytes identical
+  stdout_len orig=95 ref=95 | exit 0/0 | stderr_len 0/0
+  pdf: pages=2/2 text_eq=True norm_bytes_eq=True
+
+=== pdf_make_form.py ===
+  [PASS] stdout byte-identical
+  [PASS] exit code identical
+  [PASS] stderr identical
+  [PASS] sut ran real path (not import-error exit 2)
+  [PASS] pdf page count identical
+  [PASS] pdf extracted text identical
+  [PASS] normalized pdf bytes identical
+  stdout_len orig=298 ref=298 | exit 0/0 | stderr_len 0/0
+  pdf: pages=1/1 text_eq=True norm_bytes_eq=True
+
+SUMMARY: ALL PASS   (exit 0)
+```
+
+`stdout_len` is non-empty (95 / 298 bytes) and exit codes are `0/0` — i.e. the
+SUT actually executed its PDF-generation + `emit_json` path. reportlab's
+per-run `/CreationDate` is stripped via a pypdf re-serialize before hashing.
+
+## The defect I found and fixed (the real work here)
+The previous verification was a FALSE GREEN. `verify_pdf_refactor.py` used
+`PY = sys.executable`. Launched under the system `python3`
+(`/root/.hermes/installs/.../venv/bin/python3`), reportlab and pypdf are NOT
+installed, so BOTH the original and the refactored script failed identically at
+import (`exit 2`, stderr "Missing dependency..."). The gates then compared
+`"" == ""` (empty stdout) -> vacuous PASS. The harness reported success while
+never exercising the refactored `emit_json`/`reconfigure_stdio` code paths.
+
+Fix applied to `verify_pdf_refactor.py`:
+1. **Self-validating interpreter.** On startup, the harness checks that `PY`
+   can import `reportlab` and `pypdf`. If not, it globs for a capable hermes
+   tools `python3.14` and `os.execv`s itself under it; if none is found, it
+   exits non-zero with a clear message instead of false-greening.
+2. **Hard gate against the degenerate case.** Added an explicit
+   `sut ran real path (not import-error exit 2)` gate that fails if either run
+   exits 2, and the `dep-present runs to completion (exit 0)` gate now fails
+   (was a vacuous `True`) when the SUT could not run.
+
+Launched under the dep-less system `python3`, the harness now prints
+`[guard] re-execing under .../python3.14 ...` and passes every REAL gate above.
+Launched directly under the capable interpreter, it runs the same real gates.
+
+## Interpreter note (evidence)
+- `reportlab` 5.0.1 and `pypdf` 6.21.1 are installed under
+  `/root/.hermes/tools/python-3.14.7+202<redacted>-linux-x64/bin/python3.14`.
+- System `python3` (venv) has NEITHER — which is exactly why the original
+  harness false-greened.
+
+## Files
+- `/root/.hermes/wizard-outputs/forge/verify_pdf_refactor.py`  (harness, hardened)
+- `/root/.hermes/wizard-outputs/forge/before/pdf_create.py`    (original backup)
+- `/root/.hermes/wizard-outputs/forge/before/pdf_make_form.py`  (original backup)
+- `/root/.hermes/profiles/forge/skills/productivity/pdf/scripts/pdf_create.py`   (refactored)
+- `/root/.hermes/profiles/forge/skills/productivity/pdf/scripts/pdf_make_form.py` (refactored)
+- `/root/.hermes/profiles/forge/skills/productivity/pdf/scripts/_pdf_common.py`  (shared helper)
+- This report.

wizard-work/forge/_pdf_verify_run.stderr.txt

@@ -0,0 +1 @@
+[guard] re-execing under /root/.hermes/tools/python-3.14.7+20260901-linux-x64/bin/python3.14 (current /root/.hermes/installs/76f6e7e5145b5f39/environments/c5b879e2036d4c909805dd1858587925/venv/bin/python3 lacks reportlab/pypdf)

wizard-work/forge/_pdf_verify_run.stdout.txt

@@ -0,0 +1,24 @@
+=== pdf_create.py ===
+  [PASS] stdout byte-identical
+  [PASS] exit code identical
+  [PASS] stderr identical
+  [PASS] sut ran real path (not import-error exit 2)
+  [PASS] pdf page count identical
+  [PASS] pdf extracted text identical
+  [PASS] normalized pdf bytes identical
+  stdout_len orig=95 ref=95 | exit 0/0 | stderr_len 0/0
+  pdf: pages=2/2 text_eq=True norm_bytes_eq=True
+  -> PASS
+=== pdf_make_form.py ===
+  [PASS] stdout byte-identical
+  [PASS] exit code identical
+  [PASS] stderr identical
+  [PASS] sut ran real path (not import-error exit 2)
+  [PASS] pdf page count identical
+  [PASS] pdf extracted text identical
+  [PASS] normalized pdf bytes identical
+  stdout_len orig=298 ref=298 | exit 0/0 | stderr_len 0/0
+  pdf: pages=1/1 text_eq=True norm_bytes_eq=True
+  -> PASS
+
+SUMMARY: ALL PASS

wizard-work/forge/verify_pdf_refactor.py

@@ -0,0 +1,211 @@
+#!/usr/bin/env python3
+"""Behavior-preservation proof for the pdf_create / pdf_make_form refactor.
+
+Compares the BACKED-UP original (before/) against the refactored in-place script
+(skills/.../) with identical inputs.  The refactor only touches two code paths:
+stdio reconfiguration and JSON emission (both live in _pdf_common).  PDF
+generation bodies are untouched, so we additionally assert page-count + extracted
+text equivalence and byte-identity of the normalized PDF.
+
+The JSON payload embeds the --output path, so both runs are pointed at the SAME
+output-path string (run sequentially; each PDF is copied aside before the next
+run overwrites it).  This makes the embedded `output` field identical across
+runs, yielding a fair byte-level stdout comparison.
+
+reportlab embeds a per-run /CreationDate timestamp, so raw PDF bytes differ even
+for identical specs; we normalize by stripping volatile Info keys before hashing.
+
+FALSE-GREEN GUARD (the defect this harness previously had):
+reportlab and pypdf must be importable in the interpreter running the SUT runs,
+otherwise BOTH runs fail identically at import with exit code 2, stdout is empty
+for both, and the "stdout byte-identical" gate degenerates to empty==empty ->
+a false PASS.  To eliminate that, on startup we:
+  1. confirm `sys.executable` (PY) can import reportlab+pypdf;
+  2. if not, re-exec ourselves under the first discovered hermes tools
+     python3.14 that can import both;
+  3. if none is found, exit non-zero with a clear message (never PASS).
+
+Exits 0 only if every hard gate passes.
+"""
+from __future__ import annotations
+
+import glob
+import json
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+
+CREATE_ORIG = Path("/root/.hermes/wizard-outputs/forge/before/pdf_create.py")
+CREATE_REF = Path("/root/.hermes/profiles/forge/skills/productivity/pdf/scripts/pdf_create.py")
+FORM_ORIG = Path("/root/.hermes/wizard-outputs/forge/before/pdf_make_form.py")
+FORM_REF = Path("/root/.hermes/profiles/forge/skills/productivity/pdf/scripts/pdf_make_form.py")
+
+CREATE_SPEC = {
+    "title": "Ref Test", "author": "forge",
+    "page_size": "A4", "page_numbers": True,
+    "elements": [
+        {"type": "heading", "text": "Section 1", "level": 1},
+        {"type": "paragraph", "text": "Hello world."},
+        {"type": "table", "rows": [["H1", "H2"], ["a", "b"]], "header": True},
+        {"type": "pagebreak"},
+        {"type": "paragraph", "text": "Second page text."},
+    ],
+}
+
+FORM_SPEC = {
+    "title": "Intake", "author": "forge", "page_size": "A4", "page_count": 1,
+    "fields": [
+        {"name": "surname", "type": "text", "page": 1, "label": "Surname",
+         "label_box": [72, 700, 150, 714], "entry_box": [160, 696, 400, 716],
+         "value": "Smith", "tooltip": "Family name"},
+        {"name": "agree", "type": "checkbox", "page": 1, "label": "I agree",
+         "label_box": [72, 660, 150, 674], "entry_box": [160, 658, 176, 674],
+         "checked": True},
+        {"name": "color", "type": "radio", "page": 1, "label": "Color",
+         "label_box": [72, 620, 150, 634], "entry_box": [160, 616, 400, 636],
+         "options": ["red", "blue"], "value": "red"},
+        {"name": "size", "type": "dropdown", "page": 1, "label": "Size",
+         "label_box": [72, 580, 150, 594], "entry_box": [160, 576, 300, 596],
+         "options": ["small", "large"], "value": "small"},
+    ],
+}
+
+
+def _has_deps(interp: str) -> bool:
+    """True iff *interp* can import reportlab and pypdf (the SUT's runtime deps)."""
+    return subprocess.run(
+        [interp, "-c", "import reportlab, pypdf"],
+        capture_output=True,
+    ).returncode == 0
+
+
+def _ensure_capable_interpreter() -> None:
+    """Re-exec under an interpreter that has reportlab+pypdf, or fail loudly.
+
+    This is the fix for the prior false-green: without it, launching under an
+    interpreter lacking the deps made every SUT run fail at import (exit 2) and
+    every gate a vacuous truth.
+    """
+    if _has_deps(sys.executable):
+        return
+    candidates = sorted(glob.glob("/root/.hermes/tools/python-*linux-x64/bin/python3.14"))
+    for c in candidates:
+        if _has_deps(c):
+            print(f"[guard] re-execing under {c} (current {sys.executable} lacks reportlab/pypdf)",
+                  file=sys.stderr)
+            os.execv(c, [c, *sys.argv])
+    print("[guard] ERROR: no interpreter with reportlab+pypdf found "
+          "(cannot run SUT gates). Aborting instead of false-greening.",
+          file=sys.stderr)
+    sys.exit(2)
+
+
+_ensure_capable_interpreter()
+PY = sys.executable  # now guaranteed to import reportlab+pypdf
+
+
+def _run(script: Path, spec_path: Path, out_pdf: Path):
+    return subprocess.run(
+        [PY, str(script), str(spec_path), "-o", str(out_pdf)],
+        capture_output=True, text=True, timeout=120,
+    )
+
+
+def normalize_pdf_bytes(path: Path) -> bytes:
+    """Re-serialize via pypdf and strip volatile Info keys -> deterministic bytes."""
+    from pypdf import PdfReader, PdfWriter
+    r = PdfReader(str(path))
+    w = PdfWriter()
+    w.append(r)
+    info = w._root_object["/Info"] if "/Info" in w._root_object else None
+    if info is not None:
+        for k in ("/CreationDate", "/ModDate", "/Producer", "/Creator"):
+            try:
+                del info[k]
+            except KeyError:
+                pass
+    fd, tmp = tempfile.mkstemp(suffix=".pdf")
+    os.close(fd)
+    try:
+        with open(tmp, "wb") as f:
+            w.write(f)
+        return Path(tmp).read_bytes()
+    finally:
+        Path(tmp).unlink(missing_ok=True)
+
+
+def pdf_text_and_pages(path: Path):
+    from pypdf import PdfReader
+    r = PdfReader(str(path))
+    pages = len(r.pages)
+    text = "".join((p.extract_text() or "") for p in r.pages)
+    return pages, text
+
+
+def check(label, orig_script, ref_script, spec):
+    work = Path(tempfile.mkdtemp())
+    spec_path = work / "spec.json"
+    spec_path.write_text(json.dumps(spec), encoding="utf-8")
+    out_pdf = work / "result.pdf"  # SAME path string used by both runs
+
+    or_ = _run(orig_script, spec_path, out_pdf)
+    orig_pdf = work / "orig.pdf"
+    if out_pdf.exists():
+        shutil.copyfile(out_pdf, orig_pdf)
+        out_pdf.unlink()
+
+    rr = _run(ref_script, spec_path, out_pdf)
+    ref_pdf = work / "ref.pdf"
+    if out_pdf.exists():
+        shutil.copyfile(out_pdf, ref_pdf)
+        out_pdf.unlink()
+
+    gates = [
+        ("stdout byte-identical", or_.stdout == rr.stdout),
+        ("exit code identical", or_.returncode == rr.returncode),
+        ("stderr identical", or_.stderr == rr.stderr),
+    ]
+    # HARD gate: the SUT must actually run its PDF path, not fail at import.
+    if or_.returncode == 2 or rr.returncode == 2:
+        gates.append(("sut ran real path (not import-error exit 2)", False))
+    else:
+        gates.append(("sut ran real path (not import-error exit 2)", True))
+
+    notes = []
+    if or_.returncode == 0 and rr.returncode == 0 and orig_pdf.exists() and ref_pdf.exists():
+        op, ot = pdf_text_and_pages(orig_pdf)
+        rp, rt = pdf_text_and_pages(ref_pdf)
+        gates.append(("pdf page count identical", op == rp))
+        gates.append(("pdf extracted text identical", ot == rt))
+        nb_eq = normalize_pdf_bytes(orig_pdf) == normalize_pdf_bytes(ref_pdf)
+        gates.append(("normalized pdf bytes identical", nb_eq))
+        notes.append(f"pages={op}/{rp} text_eq={ot==rt} norm_bytes_eq={nb_eq}")
+    else:
+        gates.append(("dep-present runs to completion (exit 0)", False))
+
+    print(f"=== {label} ===")
+    for name, ok in gates:
+        print(f"  [{'PASS' if ok else 'FAIL'}] {name}")
+    print(f"  stdout_len orig={len(or_.stdout)} ref={len(rr.stdout)} | "
+          f"exit {or_.returncode}/{rr.returncode} | stderr_len {len(or_.stderr)}/{len(rr.stderr)}")
+    if notes:
+        print(f"  pdf: {' '.join(notes)}")
+    ok_exit2 = or_.returncode == 2 or rr.returncode == 2
+    all_ok = all(ok for _, ok in gates) and not ok_exit2
+    print(f"  -> {'PASS' if all_ok else 'FAIL'}")
+    return all_ok
+
+
+def main() -> int:
+    ok = True
+    ok &= check("pdf_create.py", CREATE_ORIG, CREATE_REF, CREATE_SPEC)
+    ok &= check("pdf_make_form.py", FORM_ORIG, FORM_REF, FORM_SPEC)
+    print("\nSUMMARY:", "ALL PASS" if ok else "FAILURES DETECTED")
+    return 0 if ok else 1
+
+
+if __name__ == "__main__":
+    sys.exit(main())