fix(bpsk): the GPU demodulator never cancelled the crossfade ISI

docs/dev/project/traceability.md

@@ -15,6 +15,62 @@ and the actually-observed results per change.
 
 ---
 
+## 2026-09-22 — the GPU BPSK demodulator never cancelled the crossfade ISI (#1433)
+
+**Requirement/change.** `BpskPlugin::demodulate` dispatches to the GPU when a context exists
+(`plugins/bpsk/src/lib.rs:112`), and `bpsk_demodulate_with_gpu`'s non-RRC branch went GPU timing
+search → `bpsk_iq_demod_gpu` → slice → `differential_decode` with **no `cancel_crossfade_isi`**. The
+CPU arm cancels at `demodulate.rs:114`. The GPU path landed 2026-05-04 (`664122b9`); #821 added the
+cancellation 2026-07-13 to `symbol_stream_with_expected` only.
+
+**Blast radius: the shipped daemon.** `crates/openpulse-daemon/Cargo.toml:28` is `default = ["gpu"]`
+and `server.rs:142` registers `BpskPlugin::with_gpu(ctx)` whenever an adapter is present. So on a
+GPU daemon the coded BPSK receive took the uncancelled arm — and `demodulate_soft` has no GPU path
+and already skips cancellation by design (#832), so **both** arms were uncancelled there.
+
+**Design decision.** Cancel on the whole symbol stream immediately after `bpsk_iq_demod_gpu`, before
+the preamble/tail slice — mirroring the CPU ordering, because the cancellation is a backward
+substitution and running it on a slice changes the boundary symbol. The RRC branch is untouched: it
+returns before this point and RRC does not crossfade.
+
+**Measured, 200 B BPSK250, 16 seeds, total-power SNR:**
+
+| SNR | CPU BER | GPU BER before | ratio | CPU ok | GPU ok before | GPU ok after |
+|---|---|---|---|---|---|---|
+| 0 dB | 0.00023 | 0.00328 | 14.0× | 11/16 | **0/16** | 11/16 |
+| 2 dB | 0.00000 | 0.00047 | — | 16/16 | **9/16** | 16/16 |
+| 4 dB | 0.00000 | 0.00000 | — | 16/16 | 16/16 | 16/16 |
+
+After the fix the two arms agree bit-for-bit (BER 0.00023 both at 0 dB).
+
+**Why nothing caught it, and the fixture that replaces it.** `gpu_and_cpu_agree_under_noise` swept
+4–20 dB on a 27-byte payload. BPSK250 at 8 kHz is 32 samples/symbol (~15 dB processing gain), so its
+lowest cell sits near 19 dB Eb/N0 — and **4 dB is measured above as the first SNR at which the
+difference vanishes** (16/16 both ways). The fixture's easiest cell was exactly the boundary. The new
+`gpu_and_cpu_agree_where_the_cancellation_decides_the_frame` runs 0 and 2 dB, asserts the mechanism
+(BER ratio ≤ 2×) *and* the outcome (decode counts), and guards against going vacuous by requiring the
+cell to be one where the CPU both decodes and errs.
+
+**A correction to #1080's record.** `gpu_and_cpu_agree_under_a_carrier_offset` printed "GPU decoded 6
+of 30 frames the CPU did not… the two searches still disagree off-frequency. See #1080." After this
+fix it prints 0. That divergence was this defect, not the timing search.
+
+**Twins swept, with reasons rather than absence.** `psk8_demodulate_gpu` returns `None` for non-RRC
+modes (`demodulate.rs:377`) and RRC does not crossfade; `qpsk`'s `demodulate` never dispatches to the
+GPU; `64qam` has no crossfade canceller. BPSK was the only affected plugin.
+
+**Evidence tier — stated because it is lower than usual.** These tests are `#![cfg(feature = "gpu")]`
+and the workspace gate runs `--no-default-features`, so **the gate cannot run them**; `gate.sh`'s
+`--all-features` pass is compile + lint only, and CI's `gpu` job was removed in #1380. The numbers
+above were run by hand on a host with a working adapter (`the_adapter_is_available_or_this_file_
+proves_nothing` passes here). That absence of an automatic gate is why this survived four months.
+
+**Tests → results.** `cargo test -p bpsk-plugin --features gpu --test gpu_cpu_equivalence` —
+5 passed, 0 failed. The new test **fails before the fix** (0/16 against 11/16, with the other four
+passing), which is the discriminating pair. Workspace gate: see the PR.
+
+---
+
 ## 2026-09-22 — the uncoded BPSK path misses #821's own bar by 1.7×; #1429
 
 **Change.** A characterisation test and a corrected comment; **no behaviour change**, deliberately.

docs/dev/reviews/review-1433-gpu-crossfade.md

@@ -0,0 +1,91 @@
+---
+project: openpulsehf
+doc: docs/dev/reviews/review-1433-gpu-crossfade.md
+status: resolved
+last_updated: 2026-09-22
+---
+
+# Review provenance — the GPU crossfade-cancellation defect (#1433)
+
+This defect was **not found by reading the GPU code**. It was found by an adversarial review of an
+unrelated design — the #1428 union — which asked what would have to be true for that design to work
+on the shipped binary. The answer was that it would not, and why.
+
+Recording the provenance because the finding's shape is more reusable than the fix.
+
+## Prompt
+
+Not a review of this defect — there was no prompt about the GPU path, which is the point. The
+defect surfaced inside a review of the **#1428 union design**, whose prompt asked where to seam the
+union and with what scope, and closed with: *"Anything else wrong or unproven in the framing above —
+in particular any place I have asserted a property of the code that is not actually there."*
+
+That last clause is what produced it. The framing asserted that the coded receive path takes the
+cancelled arm; the reviewer checked the assertion against the shipped build configuration rather
+than the source alone, and found it false on any host with a GPU adapter.
+
+## Verdict
+
+Confirmed, and independently re-derived before any action was taken (table above). The union design
+as proposed would have been **degenerate on the binary that runs on air**, because both of its arms
+are uncancelled there — and the more consequential half is unrelated to the union: an 84-frame swing
+at −2 dB AWGN, live since 2026-07-13.
+
+Two of the review's other findings were also confirmed and changed the union's design: the proposed
+seam (`receive_from_samples_with_fec_inner`) is not on the path the #1428 A/B measured, and there is
+no single hard-decode seam to place it at — `stage_demodulate_payload` has eleven callers across
+nine distinct frame-decode chains. Those are recorded against #1428, not here.
+
+## Consumer
+
+- `plugins/bpsk/src/demodulate.rs` `bpsk_demodulate_with_gpu` — the site.
+- `crates/openpulse-daemon/src/server.rs:142` — registers `BpskPlugin::with_gpu(ctx)` when an
+  adapter exists; `Cargo.toml:28` is `default = ["gpu"]`. This is what makes it a shipped defect
+  rather than a latent one.
+- #1428's union and #1429's arm question both assume the coded path is the *cancelled* arm. On a GPU
+  daemon it was not, so both issues' premises were wrong there.
+
+## Prior art
+
+`gh issue list --state all --search "crossfade GPU"` returned nothing; the control search on
+`crossfade` alone returned #1363, #1429, #1361, #1428, #923 — so the filter works and there was no
+existing issue. #1080's GPU-divergence sweep found three defects (I-only timing search, psk8's
+reversed LLR bit order, modulator drift) and not this one.
+
+## Twins
+
+Swept, with a reason for each rather than an absence: `psk8_demodulate_gpu` returns `None` for
+non-RRC modes (`plugins/psk8/src/demodulate.rs:377`) and RRC does not crossfade; `qpsk`'s
+`demodulate` never dispatches to the GPU; `64qam` has no crossfade canceller at all. BPSK was the
+only affected plugin.
+
+## What the review contributed, and what I verified before acting
+
+The review asserted the defect and the daemon's exposure. Per the standing rule that a subagent's
+structural claims are not taken at face value, every step was re-derived against the source before
+anything was filed or changed:
+
+| claim | how it was checked |
+|---|---|
+| GPU branch does not cancel | read `demodulate.rs:522-585`; control — the CPU path at `:114` does |
+| daemon defaults to GPU | `Cargo.toml:28`, `server.rs:126/142` |
+| `demodulate` dispatches to it | `lib.rs:112-116` |
+| the existing gate cannot see it | ran it: 4–20 dB, 27 B, "byte-level disagreements 0" |
+| it actually costs frames | measured: 0 dB → CPU 11/16, GPU **0/16** |
+
+The last row is the one that mattered. The review argued from processing gain that the fixture was
+too easy; the measurement showed **4 dB is the first SNR at which the difference vanishes**, i.e.
+the existing fixture's easiest cell sits exactly on the boundary. That is a sharper statement than
+the argument, and it is what the new test's cells are chosen from.
+
+## The lesson worth keeping
+
+An equivalence test between an accelerated path and its reference must run in a cell where **the
+property under test decides the outcome**. Comparing decode success where both arms succeed
+certifies nothing — and "both arms succeed" is the comfortable place a fixture drifts to, because it
+is also where the test is fast and never flakes. The probe: *at what input does this test's
+assertion start to depend on the thing it is named after?* If the answer is "outside the swept
+range", the test is decorative.
+
+This is the artificially-easy-fixture archetype with an accelerator twist: the reference arm hid the
+defect, because the test only ever asked whether the two agreed, never whether either was right.

plugins/bpsk/src/demodulate.rs

@@ -553,11 +553,23 @@ pub fn bpsk_demodulate_with_gpu(
     };
 
     let effective = &samples[offset.min(samples.len())..];
-    let (i_syms, q_syms) = match openpulse_gpu::bpsk_iq_demod_gpu(ctx, effective, n, fc, fs, offset)
-    {
-        Some(iq) => iq,
-        None => return bpsk_demodulate(samples, config),
-    };
+    let (mut i_syms, mut q_syms) =
+        match openpulse_gpu::bpsk_iq_demod_gpu(ctx, effective, n, fc, fs, offset) {
+            Some(iq) => iq,
+            None => return bpsk_demodulate(samples, config),
+        };
+
+    // #1433: the CPU arm cancels the crossfade ISI inside `symbol_stream_with_expected`
+    // (`demodulate.rs`, the `cancel_crossfade_isi` call after `demodulate_iq`); this path landed
+    // 2026-05-04 and #821 added the cancellation 2026-07-13 to that function only, so the GPU arm
+    // decoded the uncancelled `r_k = a_k + β·a_{k+1}` for four months. Measured on a 200 B frame at
+    // 0 dB total-power SNR: uncancelled 0/16 frames against the CPU arm's 11/16.
+    //
+    // Applied to the WHOLE symbol stream before the preamble/tail slice below, because the
+    // cancellation is a backward substitution — running it on a slice changes the boundary symbol.
+    // That is the CPU ordering, and `gpu_and_cpu_agree_where_the_cancellation_decides_the_frame`
+    // is what holds the two together.
+    cancel_crossfade_isi(&mut i_syms, &mut q_syms);
 
     if i_syms.len() <= PREAMBLE_SYMS + TAIL_SYMS {
         return Err(ModemError::Demodulation(

plugins/bpsk/tests/gpu_cpu_equivalence.rs

@@ -184,3 +184,91 @@ fn gpu_and_cpu_agree_under_a_carrier_offset() {
         );
     }
 }
+
+/// Bit errors of `got` against `want`, over `want`'s length. `None` if the demod produced too little.
+fn bit_errors(got: Option<Vec<u8>>, want: &[u8]) -> Option<(u32, u32)> {
+    let g = got?;
+    if g.len() < want.len() {
+        return None;
+    }
+    let bad = g[..want.len()]
+        .iter()
+        .zip(want)
+        .map(|(a, b)| (a ^ b).count_ones())
+        .sum();
+    Some((bad, (want.len() * 8) as u32))
+}
+
+/// The two arms must agree **where the crossfade cancellation actually decides the frame** (#1433).
+///
+/// `gpu_and_cpu_agree_under_noise` sweeps 4…20 dB *total-power* SNR on a 27-byte payload. BPSK250 at
+/// 8 kHz is 32 samples/symbol (~15 dB of processing gain), so its lowest cell sits near 19 dB Eb/N0 —
+/// both arms decode every seed there and a BER difference cannot show up. Measured on this host, 4 dB
+/// is the *first* SNR at which the difference vanishes: 16/16 both ways. One and two dB lower it is
+/// the whole frame.
+///
+/// Pre-fix measurement (GPU not cancelling), 200 B, 16 seeds:
+///
+/// | total-power SNR | CPU BER | GPU BER | ratio | CPU ok | GPU ok |
+/// |---|---|---|---|---|---|
+/// | 0 dB | 0.00023 | 0.00328 | 14.0 | 11/16 | **0/16** |
+/// | 2 dB | 0.00000 | 0.00047 | — | 16/16 | **9/16** |
+/// | 4 dB | 0.00000 | 0.00000 | — | 16/16 | 16/16 |
+///
+/// So this asserts the mechanism (the BER ratio) *and* the outcome (decode counts), and carries its
+/// own guards against going vacuous: the cell is required to be one where the CPU both decodes some
+/// frames and makes some errors, or the ratio would be meaningless and the counts uninformative.
+#[test]
+fn gpu_and_cpu_agree_where_the_cancellation_decides_the_frame() {
+    let Some(c) = ctx() else { return };
+    let cfg = config("BPSK250", 1500.0);
+    let payload: Vec<u8> = (0..200u32)
+        .map(|i| (i.wrapping_mul(2654435761) >> 13) as u8)
+        .collect();
+    let tx = bpsk_modulate(&payload, &cfg).expect("modulate");
+
+    for snr_db in [0.0f32, 2.0] {
+        let (mut cpu_bad, mut cpu_tot, mut gpu_bad, mut gpu_tot) = (0u32, 0u32, 0u32, 0u32);
+        let (mut cpu_ok, mut gpu_ok) = (0u32, 0u32);
+        let seeds = 16u64;
+        for seed in 0..seeds {
+            let rx = add_awgn(&tx, snr_db, seed * 7919 + 13);
+            let cpu = bpsk_demodulate(&rx, &cfg).ok();
+            let gpu = bpsk_demodulate_with_gpu(&rx, &cfg, &c).ok();
+            if let Some((b, t)) = bit_errors(cpu.clone(), &payload) {
+                cpu_bad += b;
+                cpu_tot += t;
+            }
+            if let Some((b, t)) = bit_errors(gpu.clone(), &payload) {
+                gpu_bad += b;
+                gpu_tot += t;
+            }
+            cpu_ok += u32::from(cpu.is_some_and(|b| b.starts_with(&payload)));
+            gpu_ok += u32::from(gpu.is_some_and(|b| b.starts_with(&payload)));
+        }
+        let cpu_ber = f64::from(cpu_bad) / f64::from(cpu_tot.max(1));
+        let gpu_ber = f64::from(gpu_bad) / f64::from(gpu_tot.max(1));
+        println!(
+            "  {snr_db:4.1} dB: cpu BER {cpu_ber:.5} ok {cpu_ok}/{seeds} | gpu BER {gpu_ber:.5} ok {gpu_ok}/{seeds}"
+        );
+
+        assert!(
+            cpu_ok > 0,
+            "{snr_db} dB is below the CPU arm's own cliff (cpu_ok = 0) — the cell cannot \
+             discriminate between the arms, so this assertion would be vacuous"
+        );
+        assert!(
+            gpu_ok + 2 >= cpu_ok,
+            "{snr_db} dB: the GPU arm decoded {gpu_ok}/{seeds} against the CPU arm's \
+             {cpu_ok}/{seeds}. The GPU demodulator is skipping cancel_crossfade_isi (#1433)"
+        );
+        if cpu_ber > 0.0 {
+            assert!(
+                gpu_ber <= cpu_ber * 2.0,
+                "{snr_db} dB: GPU BER {gpu_ber:.5} is {:.1}x the CPU's {cpu_ber:.5}. That is the \
+                 crossfade-ISI bias the CPU arm cancels and the GPU arm does not (#1433)",
+                gpu_ber / cpu_ber
+            );
+        }
+    }
+}