From 34e58642fba121a834a54993cf009c86b05889c8 Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Wed, 10 Jun 2026 12:12:40 +0200 Subject: [PATCH] feat(testing): bake order-sensitive determinism check into fill The order_sensitive marker docstring claimed the determinism check "generates this vector twice and diffs the output", but no such logic existed in the plugin. The only real gate lived in a justfile recipe invoked by CI, so a plain `uv run fill` never verified determinism and contributors relied on CI. Bake the two-seed check into the fill command itself. After a successful fill, regenerate the order_sensitive subset under PYTHONHASHSEED=1 and =2 in throwaway directories and byte-diff them. The mocked prover is forced so proof bytes stay deterministic, and a single process pins each seed cleanly. A difference fails the command and lists the offending fixtures. Add --no-check-determinism to opt out for fast local iteration. Drop the now-redundant CI step: `just fill-ci` runs fill, which performs the identical subset check by default. Repurpose the fill-determinism recipe as the standalone, wide-scope audit the baked-in check cannot give: it runs without a full fill and can cover the whole tree to catch a filler that should be marked but is not. Pass --no-check-determinism in the recipe so its own fill calls do not nest the per-fill gate. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 3 - Justfile | 13 ++- .../testing/src/consensus_testing/cli/fill.py | 98 ++++++++++++++++++- .../pytest_plugins/filler.py | 2 +- 4 files changed, 106 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e90047d09..a843d7774 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -133,9 +133,6 @@ jobs: - name: Fill test fixtures run: just fill-ci - - name: Check fixture determinism (order-sensitive vectors) - run: just fill-determinism - interop-tests: name: Interop tests - Multi-node consensus runs-on: macos-latest diff --git a/Justfile b/Justfile index cfef1782e..db72fc8e8 100644 --- a/Justfile +++ b/Justfile @@ -85,11 +85,13 @@ test-consensus *args: fill-ci *args: uv run --group test fill --fork=Lstar --clean -n auto --dist=worksteal "$@" -# Generate the order-sensitive vectors twice under different hash seeds and diff. -# Only the vectors marked order_sensitive run, so this stays cheap. +# Standalone determinism audit: regenerate vectors twice under different hash seeds and diff. +# The fill command already gates the order_sensitive subset on every run. +# This recipe runs that audit on its own, or widens it past the marked subset. +# Pass a path to cover the whole tree (for example tests/consensus) and catch a +# filler that should be marked order_sensitive but is not. # A difference means an emitted vector depends on set or dict iteration order, # which is hash-seeded and would break cross-client reproducibility. -# Pass a path to widen the scope (for example the whole tests/consensus tree). [group('tests')] fill-determinism *args: #!/usr/bin/env bash @@ -101,8 +103,9 @@ fill-determinism *args: trap 'rm -rf "$first" "$second"' EXIT # Single process: the marked subset is small, so xdist worker startup would # cost more than it saves, and one process pins the hash seed cleanly. - PYTHONHASHSEED=1 uv run --group test fill --fork=Lstar --clean -n 0 -o "$first" $target -q - PYTHONHASHSEED=2 uv run --group test fill --fork=Lstar --clean -n 0 -o "$second" $target -q + # This recipe is itself the determinism check, so the per-fill gate is skipped. + PYTHONHASHSEED=1 uv run --group test fill --fork=Lstar --clean --no-check-determinism -n 0 -o "$first" $target -q + PYTHONHASHSEED=2 uv run --group test fill --fork=Lstar --clean --no-check-determinism -n 0 -o "$second" $target -q if diff -rq "$first" "$second"; then echo "Determinism check passed: fixtures are byte-identical across hash seeds." else diff --git a/packages/testing/src/consensus_testing/cli/fill.py b/packages/testing/src/consensus_testing/cli/fill.py index 0bc1d3df9..5e2bcc793 100644 --- a/packages/testing/src/consensus_testing/cli/fill.py +++ b/packages/testing/src/consensus_testing/cli/fill.py @@ -3,6 +3,7 @@ import os import subprocess import sys +import tempfile from collections.abc import Sequence from pathlib import Path @@ -47,6 +48,12 @@ default="mocked", help="Aggregation prover mode (default: mocked; pass real for the authoritative set)", ) +@click.option( + "--check-determinism/--no-check-determinism", + default=True, + help="After filling, regenerate the order-sensitive vectors under two hash " + "seeds and fail if the emitted bytes differ (default: on)", +) @click.pass_context def fill( ctx: click.Context, @@ -56,6 +63,7 @@ def fill( clean: bool, scheme: str, crypto: str, + check_determinism: bool, ) -> None: """ Generate consensus test fixtures from test specifications. @@ -121,7 +129,95 @@ def fill( # Why a subprocess: a fresh interpreter imports the spec config anew. # Only then does the scheme exported above take effect. exit_code = subprocess.run([sys.executable, "-m", "pytest", *args]).returncode - sys.exit(exit_code) + if exit_code != 0: + sys.exit(exit_code) + + if check_determinism: + verify_order_sensitive_determinism(config_path, project_root, fork) + + sys.exit(0) + + +def verify_order_sensitive_determinism(config_path: Path, project_root: Path, fork: str) -> None: + """ + Regenerate the order-sensitive vectors under two hash seeds and diff them. + + Set and dict iteration order is randomized per process by PYTHONHASHSEED. + A vector whose bytes depend on that order is not reproducible across clients. + Two seeds producing byte-identical output proves the marked subset is order-free. + + The mocked prover is forced so proof bytes stay deterministic across both runs. + A single process pins each seed cleanly, so distribution is disabled. + """ + consensus_tests = project_root / "tests" / "consensus" + emitted_under_seed: list[Path] = [] + + with tempfile.TemporaryDirectory() as scratch_root: + for hash_seed in ("1", "2"): + output_directory = Path(scratch_root) / f"seed-{hash_seed}" + child_args = [ + "-c", + str(config_path), + f"--rootdir={project_root}", + f"--output={output_directory}", + f"--fork={fork}", + "--crypto=mocked", + "--clean", + str(consensus_tests), + "-m", + "order_sensitive", + "-n", + "0", + "-q", + ] + child_environment = {**os.environ, "PYTHONHASHSEED": hash_seed} + child_exit_code = subprocess.run( + [sys.executable, "-m", "pytest", *child_args], + env=child_environment, + ).returncode + + # Exit code 5 means no test matched the marker, so there is nothing to check. + if child_exit_code == 5: + click.echo("Determinism check skipped: no order-sensitive vectors selected.") + return + if child_exit_code != 0: + click.echo( + "Determinism check could not generate the order-sensitive subset.", + err=True, + ) + sys.exit(child_exit_code) + emitted_under_seed.append(output_directory) + + differing_fixtures = diff_fixture_trees(emitted_under_seed[0], emitted_under_seed[1]) + if differing_fixtures: + click.echo( + "Determinism check FAILED: order-sensitive vectors differ across hash seeds.", + err=True, + ) + for relative_path in differing_fixtures: + click.echo(f" differs: {relative_path}", err=True) + sys.exit(1) + + click.echo( + "Determinism check passed: order-sensitive vectors are byte-identical across hash seeds." + ) + + +def diff_fixture_trees(first_tree: Path, second_tree: Path) -> list[str]: + """Return the relative paths of fixtures whose bytes differ between two trees.""" + first_files = { + path.relative_to(first_tree): path for path in first_tree.rglob("*") if path.is_file() + } + second_files = { + path.relative_to(second_tree): path for path in second_tree.rglob("*") if path.is_file() + } + return sorted( + str(relative_path) + for relative_path in first_files.keys() | second_files.keys() + if relative_path not in first_files + or relative_path not in second_files + or first_files[relative_path].read_bytes() != second_files[relative_path].read_bytes() + ) if __name__ == "__main__": diff --git a/packages/testing/src/consensus_testing/pytest_plugins/filler.py b/packages/testing/src/consensus_testing/pytest_plugins/filler.py index 14e9561d2..e192c8d83 100644 --- a/packages/testing/src/consensus_testing/pytest_plugins/filler.py +++ b/packages/testing/src/consensus_testing/pytest_plugins/filler.py @@ -197,7 +197,7 @@ def pytest_configure(config: pytest.Config) -> None: config.addinivalue_line( "markers", "order_sensitive: emission could depend on set or dict iteration order; " - "the determinism check generates this vector twice and diffs the output", + "the fill command regenerates these vectors under two hash seeds and diffs the output", ) # Crypto mode is chosen explicitly and applies to either scheme.