From c15e557da1b1fbb0bc1380b5ea9aa1a42c2700c1 Mon Sep 17 00:00:00 2001 From: Tejas Chopra Date: Sun, 26 Jul 2026 11:00:14 -0700 Subject: [PATCH] ci(parity): make the parity harness a real per-PR gate (#2567) Three hardening steps on the Rust-vs-Python parity harness: - Drop the dead maturin/venv step. headroom-parity has no pyo3 dependency, so the venv requirement, the `maturin develop` rebuild, and the CI job's Python toolchain were all overhead. Verified no-op: identical report, exit 0. - Register a text_crusher comparator. 6 recorded fixtures were invisible because the transform was missing from builtin_comparators(); parity-run only walks directories it has a comparator for. All 6 match on the first run. - Promote parity to a blocking per-PR gate. Safe to harden now because parity-run exits non-zero only on a Diff, so the 65 still-stubbed fixtures report Skipped and cannot turn it red. Harness: total=176 matched=111 skipped=65 diffed=0, exit 0. Deliberately not widening the path filter to Python paths: the fixtures are frozen recordings of Python output and the harness never invokes Python, so it measures Rust-vs-snapshot and a Python edit cannot move the result. --- .github/workflows/rust.yml | 37 +++++++++++++------------ Makefile | 12 ++++---- crates/headroom-parity/src/lib.rs | 46 +++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 24 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 80dbd03e9..b50c0bbda 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -21,7 +21,9 @@ on: - 'Makefile' - '.github/workflows/rust.yml' schedule: - # Nightly parity run at 07:17 UTC (weekdays only). Phase 0 allows failure. + # Nightly parity run at 07:17 UTC (weekdays only). Redundant with the + # per-PR gate below, but catches drift from toolchain/dependency updates + # that land without touching any filtered path. - cron: '17 7 * * 1-5' concurrency: @@ -154,28 +156,27 @@ jobs: continue-on-error: true run: cargo deny check licenses - parity-nightly: - name: parity (nightly, allowed to fail during Phase 0) - if: github.event_name == 'schedule' + # Blocking on every PR that touches Rust. Safe to harden now because the + # harness fails only on a Diff — `parity-run` sets `any_diffs` inside the + # diffed loop alone, so the 65 fixtures still served by `stub_comparator!` + # report as Skipped and cannot turn this red. Measured on main today: + # 111 matched / 65 skipped / 0 diffed. + # + # What it protects: the recorded fixtures are frozen Python output, so this + # gate catches the Rust side drifting away from that snapshot — exactly the + # failure mode the ongoing port produces. It cannot detect the Python side + # drifting away from the fixtures; that needs re-recording, not this job. + parity: + name: parity runs-on: ubuntu-latest - continue-on-error: true steps: - uses: actions/checkout@v7 - uses: dtolnay/rust-toolchain@stable with: toolchain: 1.95.0 - - uses: actions/setup-python@v6 - with: - python-version: '3.11' - uses: Swatinem/rust-cache@v2 - - name: Install deps - run: | - python -m venv .venv - source .venv/bin/activate - pip install --upgrade pip - pip install maturin - pip install -e . + # No Python toolchain: headroom-parity links headroom-core directly and + # never crosses into the interpreter, so the venv + maturin + `pip + # install -e .` setup this job used to do was pure overhead. - name: Run parity harness - run: | - source .venv/bin/activate - make test-parity + run: make test-parity diff --git a/Makefile b/Makefile index 5608affb7..3846adc1a 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ FIXTURES ?= tests/parity/fixtures help: @echo "Headroom Rust targets:" @echo " make test - cargo test --workspace" - @echo " make test-parity - maturin develop + parity-run against fixtures" + @echo " make test-parity - parity-run against recorded fixtures" @echo " make bench - cargo bench --workspace" @echo " make build-proxy - release build + strip headroom-proxy, print size" @echo " make build-wheel - release wheel for headroom-py" @@ -36,12 +36,12 @@ help: test: $(CARGO) test --workspace +# headroom-parity has no pyo3 dependency — its comparators call headroom-core +# directly, so this target needs neither a venv nor a built extension module. +# (See crates/headroom-parity/Cargo.toml: "Phase 0 does not invoke Python from +# Rust.") Dropping the `maturin develop` step keeps the harness runnable from a +# bare checkout and takes the Python toolchain off the CI parity job. test-parity: - @if [ -z "$$VIRTUAL_ENV" ]; then \ - echo "error: activate a venv first (e.g. source .venv/bin/activate)"; \ - exit 1; \ - fi - $(MATURIN) develop -m crates/headroom-py/Cargo.toml $(CARGO) run -p headroom-parity -- run --fixtures $(FIXTURES) bench: diff --git a/crates/headroom-parity/src/lib.rs b/crates/headroom-parity/src/lib.rs index 37310e6ff..2f776b3a1 100644 --- a/crates/headroom-parity/src/lib.rs +++ b/crates/headroom-parity/src/lib.rs @@ -466,6 +466,51 @@ impl TransformComparator for ContentDetectorComparator { } } +/// Real comparator for the `text_crusher` transform. The fixture `input` is an +/// object rather than a bare string, mirroring the recorder's three arguments +/// (`tests/parity/record_text_crusher.py`), and `config` is always `null` — the +/// recorder drove the Python default config, so the Rust side uses +/// `TextCrusherConfig::default()` to match. +pub struct TextCrusherComparator; + +impl TransformComparator for TextCrusherComparator { + fn name(&self) -> &str { + "text_crusher" + } + + fn run( + &self, + input: &serde_json::Value, + _config: &serde_json::Value, + ) -> Result { + use headroom_core::transforms::{TextCrusher, TextCrusherConfig}; + + let content = input + .get("content") + .and_then(|v| v.as_str()) + .context("text_crusher fixture input needs a string `content`")?; + let context = input + .get("context") + .and_then(|v| v.as_str()) + .unwrap_or_default(); + // Null in the `short_passthrough` fixture, where the recorder passed no + // ratio at all — `Option` carries that through to the same default + // the Python call used. + let target_ratio = input.get("target_ratio").and_then(|v| v.as_f64()); + + let result = + TextCrusher::new(TextCrusherConfig::default()).compress(content, context, target_ratio); + Ok(serde_json::json!({ + "compressed": result.compressed, + "original_tokens": result.original_tokens, + "compressed_tokens": result.compressed_tokens, + "compression_ratio": result.compression_ratio, + "kept_segments": result.kept_segments, + "total_segments": result.total_segments, + })) + } +} + /// Every built-in comparator, in a stable order. pub fn builtin_comparators() -> Vec> { vec![ @@ -476,6 +521,7 @@ pub fn builtin_comparators() -> Vec> { Box::new(CcrComparator), Box::new(SmartCrusherComparator), Box::new(ContentDetectorComparator), + Box::new(TextCrusherComparator), ] }