fix(gpu): recover device-only declines at the remaining cliff sites (R4 DEEP, comp-tree, R3 barycentric)

[ColoCarletti]

Closes #927.

Extends the #914 device-decline recovery to the downstream cliff sites, which today hard-abort under the same transient VRAM pressure (census on rented 5090s: R4 DEEP ×97, comp-tree ×42, R3 ×3):

• R2 comp-poly commit: when both tree-build arms decline, the CPU commit now downloads the H part evals from the resident R2 handle (materialize_composition_parts_host) instead of asserting. The parts handle is folded into the trace session before the commit so the recovery and every downstream consumer read it from one place.
• R3 parts OOD: same parts download when the host arm finds them empty.
• R3 barycentric (main/aux): per-buffer recovery via materialize_lde_trace_host, threaded &mut through get_trace_evaluations_from_lde.
• R4 DEEP: the host loop reads the trace and the part evals — recovers both. round_3/round_4/compute_deep now take &mut Round1/&mut Round2 (single caller).

The asserts survive only for the case where a resident handle cannot serve the data, with the device-only contract's message. New counter GPU_COMPOSITION_PARTS_DOWNLOADS mirrors the downgrade counters (parts side).

Testing: sticky fault hooks in math-cuda (test-faults) on the barycentric/DEEP/comp-tree entries — sticky because the drain-and-retry absorbs one-shot faults — plus three end-to-end tests in cuda_fallback_tests.rs that arm them, require the device-only path on warm-up, and assert the recovery counters moved and the proof verifies.

Validation (RTX 5090): fault tests 5/5 (3 new + 2 existing), cuda_path_integration 7/7 (happy path, zero downgrades), stark cuda lib 220/220; CPU-only stark suite 217/217 and clippy clean (cuda / no-cuda / test-faults).

[ColoCarletti]

/ai-review

[github-actions]

Codex Code Review

• Low — faults.rs:29: load followed by fetch_sub is not atomic as a countdown. Concurrent GPU table dispatches can decrement 1 below zero, which disarms the supposedly sticky fault and lets later calls succeed. Use a compare-exchange/fetch-update loop that saturates at 0.

[claude]

Review

Traced the whole recovery chain — R2 commit → R3 parts OOD / barycentric → R4 DEEP — plus the slab layout, stream ordering and feature wiring. No correctness or safety issues found. Details of what I checked and three minor notes below.

Verified

• Slab layout. download_composition_parts_host de-interleaves slabs[(p*3+k)*lde + r] into interleaved[r*3+k], exactly the inverse of unpack_pinned_slabs_to_ext3 (math-cuda/src/lde.rs:96), which is what fills the host part evals on the non-device-only path. Natural evaluation order is preserved, and nothing between R2 and R4 mutates the parts buffer in place (build_comp_poly_tree_from_slabs_dev bit-reverses inside the leaf kernel, not in buf), so the downloaded evals are byte-identical to what the host consumers would otherwise have had. Both consumers index correctly: lde_evals[i*blowup] in R3, [j][i] in the R4 DEEP loop.
• Host/device sync. Both producers of the parts handle are safe to download cross-stream: coset_lde_batch_ext3_slabs_keep records a ready event on the device-only (outputs: None) branch, and evaluate_poly_coset_batch_ext3_into_keep host-blocks in wait_and_read before returning (ready: None is correct there). wait_ready_on + synchronize on the bound stream covers both. No buffers are dropped on the recovery path, so there is no use-after-free window like the one materialize_aux_trace_host has to drain for.
• Shape guards. h.m != evals.len() and slabs.len() != m*lde*3 both degrade to false → the caller abort, rather than slicing out of bounds. The only producer that leaves empty host part evals is try_decompose_extend_d2_dev(want_host=false), which returns two empty vecs with a matching h.m == 2, so the length check cannot spuriously reject a real recovery.
• Borrows / ordering. Moving set_gpu_composition_parts above the commit is safe (the tree builds only borrow the handle), and the &mut Round1 / &mut Round2 threading is disjoint at every new call site. get_trace_evaluations_from_lde has no callers outside prover.rs and the updated test.
• Sticky hook semantics. check_sticky parks at 0 and keeps returning Err (N=1 fires on the first call), which is what the drain-and-retry arms require; *_fault_fired() reading == 0 is consistent with that. Feature wiring prover/test-cuda-faults → stark/test-cuda-faults → math-cuda/test-faults is correct and the module is fully cfg-d out of production builds. No dead code — every new hook, counter and accessor has a consumer.
• Downstream effect of clearing host_trace_empty. After an R3/R4 recovery the later opening guards (prover.rs:2622/2664/2674) turn from hard aborts into valid host fall-throughs, and the query-0 device/host cross-check at prover.rs:2686 re-enables against data downloaded from the same buffer. Both correct, just slower — consistent with the intent.

Notes (all minor, inline)

1. download_composition_parts_host is serial and copies twice, where the sibling materialize_lde_trace_host parallelizes and reinterprets in place — likely dominates the D2H it follows, on a path the census says fires often.
2. The GPU_DEVICE_ONLY_DOWNGRADES doc still says "every count is a gate miss", which now contradicts both the new call sites and materialize_lde_trace_host own doc.
3. The new tests leave the sticky fault armed if an assert fails, cascading into the rest of the single-threaded run.

One thing to be aware of rather than change: in the R3 arms the recovery is gated on the individual buffer, but materialize_lde_trace_host early-returns true on the table-wide host_trace_empty() flag. A state with the flag clear and one buffer still empty would no-op the recovery and fire the assert with a message about handles that were never consulted. Working through the flag formula in into_round1 that state does not look reachable (any empty side sets the flag; set_host_data only clears it after filling), and the pre-PR code asserted identically there — so not a regression, just a place where the comment promises slightly more than the code delivers.

[github-actions]

AI Review

PR #935 · 10 changed files

Findings

Status	Sev	Location	Finding	Found by
uncertain	medium	crypto/math-cuda/src/faults.rs:25	Race condition in sticky fault check: non-atomic load-then-decrement	nemotron openrouter/nvidia/nemotron-3-ultra-550b-a55b kimi openrouter/moonshotai/kimi-k2.7-code glm openrouter/z-ai/glm-5.2
confirmed	low	crypto/stark/src/trace.rs:708	Doc comment does not mention new mutability contract	kimi openrouter/moonshotai/kimi-k2.7-code

Status column reflects the verdict from the verifier: deepseek-verifier (openrouter/deepseek/deepseek-v4-pro).

AI-003: Race condition in sticky fault check: non-atomic load-then-decrement

• Status: uncertain
• Severity: medium
• Location: crypto/math-cuda/src/faults.rs:25
• Found by: nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b, kimi:openrouter/moonshotai/kimi-k2.7-code, glm:openrouter/z-ai/glm-5.2
• Verified by: -
• Rejected by: -

Claim

The check_sticky function loads the counter, then conditionally decrements, but uses the original loaded value for the fire check. Concurrent calls can both see v=1, both decrement to 0, and both return error (double-fire), or other interleavings can cause the counter to not park at 0 correctly.

Evidence

faults.rs lines 25-38: let v = counter.load(Ordering::Relaxed); ... if v > 0 { counter.fetch_sub(1, Ordering::Relaxed); } if v <= 1 { return Err(...) }. The loaded value v is used for the fire decision, not the return value of fetch_sub. gpu_lde.rs lines 1870, 1883, 1896 define barycentric_fault_fired(), deep_fault_fired(), and comp_tree_fault_fired() as load(...) == 0. Under the race described the counter can become negative, making the fired predicates falsely report not-fired.

Suggested fix

Make the decrement saturate at 0 with a compare-exchange loop, e.g. loop { let v = counter.load(Relaxed); if v < 0 { return Ok; } if v <= 1 { counter.store(0, Relaxed); return Err(...); } if counter.compare_exchange(v, v-1, Relaxed, Relaxed).is_ok() { return Ok; } } (or fetch_update with a closure that maps v>1 to v-1 and v in 0..=1 to 0).

AI-008: Doc comment does not mention new mutability contract

• Status: confirmed
• Severity: low
• Location: crypto/stark/src/trace.rs:708
• Found by: kimi:openrouter/moonshotai/kimi-k2.7-code
• Verified by: deepseek-verifier:openrouter/deepseek/deepseek-v4-pro
• Rejected by: -

Claim

get_trace_evaluations_from_lde now takes &mut LDETraceTable because it may call materialize_lde_trace_host to download resident LDEs into host buffers, but its doc comment still describes the function as if it only reads the table. This makes the new mutability contract easy to miss for maintainers.

Evidence

trace.rs lines 708-715 show the signature changed to lde_trace: &mut LDETraceTable<F, E>, and lines 824-825 and 889-890 call crate::gpu_lde::materialize_lde_trace_host(lde_trace), which mutates the table. The doc block above (lines 693-707) does not mention that the function can mutate the table or download resident data.

Suggested fix

Update the doc comment for get_trace_evaluations_from_lde to state that it may mutate lde_trace by downloading resident device LDEs when a GPU fallback reaches a device-only table.

Reviewer Lanes

Lane	Model	Prompt	Status	Findings
glm	openrouter/z-ai/glm-5.2	general	success	1
kimi	openrouter/moonshotai/kimi-k2.7-code	general	success	2
minimax	minimax/MiniMax-M3	general	error: opencode failed (provider/auth/runtime error) and no findings were submitted	0
moonmath	zro/minimax-m3	general	error: opencode failed (provider/auth/runtime error) and no findings were submitted	0
nemotron	openrouter/nvidia/nemotron-3-ultra-550b-a55b	general	success	5

Verification Lanes

Lane	Model	Status	Confirmed	Rejected	Uncertain
deepseek-verifier	openrouter/deepseek/deepseek-v4-pro	success	1	3	1

Native Codex and Claude reviews run separately and post their own comments. They are not included in this structured provenance report.

Discarded candidates (3) — rejected by the verifier

• Materialization skipped in mixed device-only/host state causing host evaluator to read empty buffers (crypto/stark/src/prover.rs:1674, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The claim states that host_trace_empty() returns false in a mixed state. But the code at trace.rs lines 330-344 shows host_trace_empty is a single boolean flag set to true when the device-only path is taken (line 357), meaning at least one of main_data/aux_data is empty. In a mixed state, the flag is true, not false. The condition at prover.rs line 1674 calls materialize_lde_trace_host when host_trace_empty() is true, which is correct. materialize_lde_trace_host (gpu_lde.rs lines 1550-1570) correctly handles mixed states by downloading only the missing sides — it checks !lde_trace.main_data.is_empty() and !lde_trace.aux_data.is_empty() separately. The claim's premise about the boolean behavior is factually wrong.
• Redundant synchronization in materialize_lde_trace_host aux download (crypto/stark/src/gpu_lde.rs:1585, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — The claim asserts that clone_dtoh in cudarc already synchronizes internally. However, across the entire codebase, clone_dtoh is consistently followed by an explicit stream.synchronize() call (34 occurrences found). This pattern is used in every single GPU kernel dispatch — barycentric.rs, deep.rs, merkle.rs, ntt.rs, lib.rs, fri.rs, gpu_lde.rs, etc. The universal consistency of this pattern is strong evidence that cudarc's clone_dtoh does NOT synchronize the stream, and the explicit synchronize() call at gpu_lde.rs line 1585 is necessary, not redundant.
• Inconsistent error handling in materialize_aux_trace_host vs materialize_lde_trace_host (crypto/stark/src/gpu_lde.rs:1719, found by nemotron:openrouter/nvidia/nemotron-3-ultra-550b-a55b) — Both functions perform the same two checks: (1) whether clone_dtoh + synchronize succeeded, and (2) whether the buffer length matches the expected dimensions. materialize_aux_trace_host (line 1722) combines them: if stream.synchronize().is_err() \|\| raw.len() != rows * cols * 3. materialize_lde_trace_host (lines 1585, 1592-1593) separates them in two if blocks. Both check length AFTER synchronize and BEFORE de-interleaving — the logical order is identical. The only difference is whether the two conditions are combined in one if or separated. This is a trivial stylistic difference with no behavioral impact, not an inconsistency in error handling.

Raw lane outputs, candidates, final issues, and model metrics are uploaded as workflow artifacts.

[MauroToscano]

/bench-gpu

[github-actions]

GPU Benchmark (ABBA) — 7863a6740d vs main (14 pairs)

_{RTX 5090 · AMD Ryzen 9 7950X 16-Core Processor (32 threads) · Vast.ai datacenter @ $0.5480555555555555/hr · prover/cuda · ethrex real block, continuations · drift-free A/B/B/A}

=== ABBA paired result  (improvement: - = PR faster) ===
  pairs: 14   mean A (PR): 67.782s   mean B (base): 67.933s

  [parametric] paired-t   mean -0.22%   sd 1.22%   se 0.32%
               95% CI: [-0.92%, +0.49%]   (t df=13 = 2.16)
  [robust]     median -0.39%   Wilcoxon W+=39 W-=66  p(exact)=0.4263  (z=-0.82)

  --- server stability (this run; compare across servers) ---
  run-to-run jitter:    A CV 0.75%   B CV 0.80%        (lower = steadier)
  within-session drift: +0.60% over the run, 1st->2nd half +0.12%
    (jitter -> Tier-1 cached gate floor; drift -> whether the cached baseline can be trusted)

  VERDICT: INCONCLUSIVE - effect not separable from 0 at n=14.
           Point estimate ~-0.39% (median). Need more pairs to resolve.

  raw pairs: /tmp/abba_run/pairs.csv

_{- = PR faster. Trust the verdict when paired-t and Wilcoxon agree.}

[ColoCarletti]

/bench-gpu