fix(tools): harden stdlib python-frame verify (LAB-5341)

changelog.d/20260929_lab-5341.md

@@ -0,0 +1,16 @@
+### Tooling — stdlib python-frame verify holds on its own (LAB-5341)
+
+- `tools/python-frame-reference.py verify` now LZ4-decompresses each envelope vector's
+  `compressed_data` (the strict block decoder from `tools/interop-v2-reference.py`) and
+  rejects a `payload_envelope.inner_msgpack_hex` that does not match. Before, the field was
+  only compared twin against twin, so two twins carrying the same wrong value passed.
+- The `twin_of` compare of `value_json` is type-strict (`true` no longer equals `1`).
+- A `twin_of` pair where either side lacks `payload_envelope.envelope_encoding` fails the twin
+  gate in `verify` and warns in `generate`, naming the missing field. Before, the twin gate
+  counted a one-sided missing encoding as a distinct one: the twin printed `ok` (`verify`
+  still failed on the vector's own encoding check) and `generate` stayed silent.
+- A `payload_envelope` that is present but not an object is a FAIL line in `verify` and a
+  warning in `generate`. Before, a list or string raised a traceback, and a present `null`
+  passed by skipping every envelope check.
+- `spec/wire-format.md`'s `Verify:` block lists the mutation suite CI already runs.
+- `test-vectors/python-frame.json` is unchanged.

spec/wire-format.md

@@ -616,7 +616,8 @@ emits the array-of-integers envelope any more) as legacy-read proof.
 Verify:
 
 ```bash
-python3 tools/python-frame-reference.py           # stdlib-only structural verify
+python3 tools/test_python_frame_reference.py      # mutation suite for the stdlib verify
+python3 tools/python-frame-reference.py           # stdlib-only verify (frame, envelope, LZ4 -> inner msgpack)
 node tools/frame-crosscheck.mjs                   # independent zero-dep JS reader (full round-trip)
 ```
 

tools/python-frame-reference.py

@@ -11,8 +11,9 @@
 Modes:
     verify    (default) stdlib-only. Re-parses every frame vector with an
               independent minimal parser (no cachekit import) and checks the
-              expected header/payload; checks every error vector is rejected.
-              Runs in CI.
+              expected header/payload, including the ByteStorage envelope down
+              to the LZ4-decompressed inner msgpack (inner_msgpack_hex);
+              checks every error vector is rejected. Runs in CI.
     generate  Upserts the vector file by vector name (LAB-1203): every vector
               the installed wheel can reproduce is rebuilt, and rewritten only
               if its content actually changed; every other committed vector is
@@ -51,7 +52,10 @@
 The ByteStorage envelope codec is NOT reimplemented here: encode/decode come
 from tools/wire-format-reference.py, the single shared implementation of the
 encoding these fixtures exist to pin (stdlib-only, so `verify` stays
-dependency-free).
+dependency-free). LZ4 decompression of compressed_data likewise comes from
+tools/interop-v2-reference.py's strict block decoder, which rejects truncation,
+bad offsets and any output length other than original_size: a reader-lenient
+decoder here would silently weaken the inner_msgpack_hex check.
 """
 
 from __future__ import annotations
@@ -80,18 +84,19 @@
 )
 
 
-def _load_wire_format_codec() -> ModuleType:
-    """Load tools/wire-format-reference.py as a module (hyphenated filename)."""
-    path = Path(__file__).resolve().parent / "wire-format-reference.py"
-    spec = importlib.util.spec_from_file_location("wire_format_reference", path)
+def _load_tool(filename: str, module_name: str) -> ModuleType:
+    """Load a sibling stdlib-only reference tool as a module (hyphenated filename)."""
+    path = Path(__file__).resolve().parent / filename
+    spec = importlib.util.spec_from_file_location(module_name, path)
     if spec is None or spec.loader is None:
-        raise ImportError(f"cannot load envelope codec from {path}")
+        raise ImportError(f"cannot load {path}")
     module = importlib.util.module_from_spec(spec)
     spec.loader.exec_module(module)
     return module
 
 
-_wire = _load_wire_format_codec()
+_wire = _load_tool("wire-format-reference.py", "wire_format_reference")
+_lz4_block_decompress = _load_tool("interop-v2-reference.py", "interop_v2_reference").lz4_block_decompress
 
 
 class FrameError(ValueError):
@@ -137,6 +142,8 @@ def parse_frame(frame: bytes) -> tuple[dict, bytes]:
     return header, frame[header_end:]
 
 
+# Fields a twin must share with its base. envelope_encoding must DIFFER, so it is
+# not listed here; _twin_divergence requires its presence separately.
 _TWIN_ENVELOPE_FIELDS = ("compressed_data_hex", "checksum_hex", "original_size", "format", "inner_msgpack_hex")
 
 
@@ -165,19 +172,27 @@ def _twin_divergence(twin: dict, by_name: dict[str, dict]) -> str | None:
     if base is None:
         return f"twin_of names unknown vector {twin['twin_of']!r}"
     for side in (twin, base):
-        missing = [k for k in ("value_json", "frame_hex", "expected_payload_hex", "payload_envelope") if side.get(k) is None]
-        env = side.get("payload_envelope") or {}
-        missing += [f"payload_envelope.{f}" for f in _TWIN_ENVELOPE_FIELDS if env.get(f) is None]
+        missing = [k for k in ("value_json", "frame_hex", "expected_payload_hex") if side.get(k) is None]
+        env = side.get("payload_envelope")
+        if isinstance(env, dict):
+            # envelope_encoding too: a missing one would otherwise count as a
+            # distinct encoding and satisfy "differs in encoding" vacuously.
+            required = ("envelope_encoding", *_TWIN_ENVELOPE_FIELDS)
+            missing += [f"payload_envelope.{f}" for f in required if env.get(f) is None]
+        else:
+            missing.append("payload_envelope (object)")
         if missing:
             return f"twin_of requires envelope vectors on both sides; {side['name']!r} lacks {', '.join(missing)}"
     twin_env, base_env = twin["payload_envelope"], base["payload_envelope"]
-    if twin_env.get("envelope_encoding") == base_env.get("envelope_encoding"):
+    if twin_env["envelope_encoding"] == base_env["envelope_encoding"]:
         return (
             f"declared twin_of {base['name']!r} but both carry envelope_encoding "
-            f"{twin_env.get('envelope_encoding')!r} — a twin must differ from its base in encoding"
+            f"{twin_env['envelope_encoding']!r} — a twin must differ from its base in encoding"
         )
     mismatches: list[str] = []
-    if twin["value_json"] != base["value_json"]:
+    # Serialised, not `!=`: Python has True == 1 == 1.0, so a twin carrying
+    # `1` against a base carrying `true` would compare equal.
+    if json.dumps(twin["value_json"], sort_keys=True) != json.dumps(base["value_json"], sort_keys=True):
         mismatches.append("value_json")
     if _frame_prefix_hex(twin) != _frame_prefix_hex(base):
         mismatches.append("frame prefix (magic/version/header bytes)")
@@ -216,8 +231,13 @@ def verify() -> int:
         if "expected_payload_hex" in vec and payload.hex() != vec["expected_payload_hex"]:
             print(f"FAIL {name}: payload mismatch")
             vec_failed += 1
+        # Keyed on presence, not on None: a present JSON null is a non-object,
+        # not "absent", so it cannot skip every envelope check below.
         env = vec.get("payload_envelope")
-        if env:
+        if "payload_envelope" in vec and not isinstance(env, dict):
+            print(f"FAIL {name}: payload_envelope must be an object, got {type(env).__name__}")
+            vec_failed += 1
+        elif "payload_envelope" in vec:
             declared = env.get("envelope_encoding")
             if declared is None:
                 print(f"FAIL {name}: payload_envelope must declare envelope_encoding ('bin' or 'int-array')")
@@ -260,7 +280,20 @@ def verify() -> int:
                             print(f"FAIL {name}: payload_envelope field(s) disagree with the envelope bytes: {', '.join(drifted)}")
                             vec_failed += 1
                         else:
-                            observed_encodings.add(actual)
+                            # Checked against the bytes, not only twin against
+                            # twin: two twins carrying the same wrong value (or
+                            # one vector with no twin) must still fail here.
+                            try:
+                                inner = _lz4_block_decompress(data, size)
+                            except ValueError as e:
+                                print(f"FAIL {name}: LZ4 decompress: {e}")
+                                vec_failed += 1
+                            else:
+                                if env.get("inner_msgpack_hex") != inner.hex():
+                                    print(f"FAIL {name}: decompressed payload does not match payload_envelope.inner_msgpack_hex")
+                                    vec_failed += 1
+                                else:
+                                    observed_encodings.add(actual)
         det = vec.get("arrow_detection")
         if det:
             off = det["ipc_magic_offset"]

tools/test_python_frame_reference.py

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-"""Mutation suite for the `twin_of` machinery in python-frame-reference.py (LAB-3967).
+"""Mutation suite for python-frame-reference.py verify: `twin_of` machinery (LAB-3967) and envelope checks.
 
 Design and rationale: python-frame-reference.py, "Twin declarations". This
 suite mutates a copy of the COMMITTED fixture and proves verify() fails on
@@ -20,6 +20,7 @@
 import json
 import sys
 import tempfile
+import traceback
 from pathlib import Path
 
 HERE = Path(__file__).resolve().parent
@@ -119,7 +120,7 @@ def mutate(twin: dict) -> None:
     "payload_envelope.inner_msgpack_hex": env_mutation("inner_msgpack_hex", flip_last_nibble),
 }
 # Fields NO other verify check covers: here the twin gate is the only thing standing.
-ONLY_TWIN_GATE = {"value_json", "frame prefix (magic/version/header bytes)", "payload_envelope.inner_msgpack_hex"}
+ONLY_TWIN_GATE = {"value_json", "frame prefix (magic/version/header bytes)"}
 
 for field, mutate in MUTATIONS.items():
     doc, _ = mutated(mutate)
@@ -131,6 +132,49 @@ def mutate(twin: dict) -> None:
         fail_lines = [line for line in out.splitlines() if line.startswith("FAIL")]
         check(f"mutate {field}: twin gate is the ONLY check that fires", fail_lines == twin_lines)
 
+# --- value_json compares type-strictly: Python's True == 1 must not pass the twin claim ---
+doc, _ = mutated(lambda t: t["value_json"].__setitem__("active", 1))
+rc, out = run_verify(doc)
+check("value_json true -> 1 in the twin: verify exits 1", rc == 1)
+check("value_json true -> 1 in the twin: twin gate names value_json", f"FAIL {BIN_NAME}" in out and "value_json" in out)
+
+# --- inner_msgpack_hex is checked against the decompressed bytes, not only twin against twin ---
+INNER_FAIL = "decompressed payload does not match payload_envelope.inner_msgpack_hex"
+doc, twin = mutated(env_mutation("inner_msgpack_hex", flip_last_nibble))
+legacy = next(v for v in doc["frame_vectors"] if v["name"] == LEGACY_NAME)
+legacy["payload_envelope"]["inner_msgpack_hex"] = twin["payload_envelope"]["inner_msgpack_hex"]
+rc, out = run_verify(doc)
+check("same wrong inner_msgpack_hex on both twins: verify exits 1", rc == 1)
+check(
+    "same wrong inner_msgpack_hex on both twins: both vectors FAIL on the bytes",
+    f"FAIL {BIN_NAME}: {INNER_FAIL}" in out and f"FAIL {LEGACY_NAME}: {INNER_FAIL}" in out,
+)
+doc, twin = mutated(env_mutation("inner_msgpack_hex", flip_last_nibble))
+del twin["twin_of"]
+rc, out = run_verify(doc)
+check("wrong inner_msgpack_hex with twin_of dropped: verify exits 1", rc == 1 and f"FAIL {BIN_NAME}: {INNER_FAIL}" in out)
+
+# --- a non-object payload_envelope is a FAIL line, never an AttributeError traceback ---
+for bad in (["not", "an", "object"], "not an object"):
+    kind = type(bad).__name__
+    doc, _ = mutated(lambda t, bad=bad: t.__setitem__("payload_envelope", bad))
+    try:
+        rc, out = run_verify(doc)
+    except Exception:  # noqa: BLE001 - any traceback is the failure under test
+        traceback.print_exc(file=sys.stdout)  # shows where it raised: verify() or this harness
+        rc, out = None, ""
+    check(f"{kind} payload_envelope: verify exits 1 with a FAIL line", rc == 1 and "payload_envelope must be an object" in out)
+# A present null is a non-object too, not "absent". On a vector outside the twin
+# pair nothing else fires and the encoding coverage floor still holds.
+doc = copy.deepcopy(COMMITTED)
+next(v for v in doc["frame_vectors"] if v["name"] == "raw_payload_frame")["payload_envelope"] = None
+rc, out = run_verify(doc)
+fail_lines = [line for line in out.splitlines() if line.startswith("FAIL")]
+check(
+    "null payload_envelope on a non-twin vector: verify exits 1, and its FAIL line is the only one",
+    rc == 1 and fail_lines == ["FAIL raw_payload_frame: payload_envelope must be an object, got NoneType"],
+)
+
 # --- a dangling declaration is a failure, not a silent skip ---
 doc, _ = mutated(lambda t: t.__setitem__("twin_of", "no_such_vector"))
 rc, out = run_verify(doc)
@@ -169,6 +213,16 @@ def mutate(twin: dict) -> None:
 rc, out = run_verify(doc)
 check("null envelope subfield on both sides: verify exits 1", rc == 1 and "lacks payload_envelope.inner_msgpack_hex" in out)
 
+# A missing envelope_encoding is "lacking", not a distinct encoding that satisfies "differs in encoding".
+doc = copy.deepcopy(COMMITTED)
+next(v for v in doc["frame_vectors"] if v["name"] == LEGACY_NAME)["payload_envelope"].pop("envelope_encoding")
+rc, out = run_verify(doc)
+check(
+    "base lacking envelope_encoding: the twin FAILs too, naming it",
+    rc == 1 and f"FAIL {BIN_NAME}: twin_of requires envelope vectors on both sides; "
+    f"{LEGACY_NAME!r} lacks payload_envelope.envelope_encoding" in out,
+)
+
 doc, _ = mutated(lambda t: t.__setitem__("twin_of", [LEGACY_NAME]))
 rc, out = run_verify(doc)
 check("non-string twin_of: verify exits 1 with a FAIL line", rc == 1 and "must be a vector-name string" in out)
@@ -219,7 +273,8 @@ def warn_output(vectors: list[dict]) -> tuple[bool, str]:
     try:
         with contextlib.redirect_stderr(buf):
             pfr._warn_twin_divergence(vectors)
-    except ValueError:
+    except Exception:  # noqa: BLE001 - generate must never raise here, whatever the type
+        traceback.print_exc(file=sys.stdout)
         raised = True
     return raised, buf.getvalue()
 
@@ -238,6 +293,20 @@ def warn_output(vectors: list[dict]) -> tuple[bool, str]:
 # Reachable only via generate: verify() indexes frame_hex for every vector before the twin gate runs.
 raised, err = warn_output([{k: v for k, v in SYNTH_LEGACY.items() if k != "frame_hex"}, SYNTH_TWIN])
 check("generate: base lacking frame_hex -> warns, does not raise", not raised and "lacks frame_hex" in err)
+for bad in (["not", "an", "object"], "not an object"):
+    raised, err = warn_output([SYNTH_LEGACY, {**SYNTH_TWIN, "payload_envelope": bad}])
+    check(
+        f"generate: {type(bad).__name__} payload_envelope -> warns, does not raise",
+        not raised and "lacks payload_envelope (object)" in err,
+    )
+raised, err = warn_output([SYNTH_LEGACY, {**SYNTH_TWIN, "value_json": {"a": True}}])
+check("generate: value_json 1 vs true -> warns (type-strict compare)", not raised and "value_json" in err)
+no_encoding = {k: v for k, v in SYNTH_LEGACY["payload_envelope"].items() if k != "envelope_encoding"}
+raised, err = warn_output([{**SYNTH_LEGACY, "payload_envelope": no_encoding}, SYNTH_TWIN])
+check(
+    "generate: base lacking envelope_encoding -> warns, does not raise",
+    not raised and "lacks payload_envelope.envelope_encoding" in err,
+)
 
 # --- _upsert: the declaration survives a rebuild and never causes churn ---
 committed = [copy.deepcopy(SYNTH_TWIN) | {"generator": "old wheel"}]