diff --git a/changelog.d/20260929_lab-5341.md b/changelog.d/20260929_lab-5341.md new file mode 100644 index 0000000..6295d88 --- /dev/null +++ b/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. diff --git a/spec/wire-format.md b/spec/wire-format.md index 13d3e6d..f8006a7 100644 --- a/spec/wire-format.md +++ b/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) ``` diff --git a/tools/python-frame-reference.py b/tools/python-frame-reference.py index ee7997d..04f5b20 100644 --- a/tools/python-frame-reference.py +++ b/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"] diff --git a/tools/test_python_frame_reference.py b/tools/test_python_frame_reference.py index 87e0737..64eb6cc 100644 --- a/tools/test_python_frame_reference.py +++ b/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"}]