diff --git a/wizard-work/forge/PDF_REFACTOR_VERIFICATION.md b/wizard-work/forge/PDF_REFACTOR_VERIFICATION.md new file mode 100644 index 0000000..5ee9c38 --- /dev/null +++ b/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-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. diff --git a/wizard-work/forge/_pdf_verify_run.stderr.txt b/wizard-work/forge/_pdf_verify_run.stderr.txt new file mode 100644 index 0000000..3b6dbd6 --- /dev/null +++ b/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) diff --git a/wizard-work/forge/_pdf_verify_run.stdout.txt b/wizard-work/forge/_pdf_verify_run.stdout.txt new file mode 100644 index 0000000..5f98733 --- /dev/null +++ b/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 diff --git a/wizard-work/forge/verify_pdf_refactor.py b/wizard-work/forge/verify_pdf_refactor.py new file mode 100644 index 0000000..44ce76d --- /dev/null +++ b/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())