From a02bc4dba8785f8903d3d87f291c089f303c772b Mon Sep 17 00:00:00 2001 From: daharoni Date: Wed, 8 Jul 2026 14:07:53 -0700 Subject: [PATCH] fix(solver): reject non-finite input traces at the FFI boundaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A NaN/Inf in an input trace previously propagated silently: total_cmp sorts NaN last (corrupting the rolling-baseline percentile), alpha/PVE come back NaN, and the result is returned with converged=false — indistinguishable from a legitimately hard trace. In the batch path a NaN actually panicked deep in the FFT (Result::unwrap on a non-real spectrum), aborting the call. Add a shared crate::first_nonfinite helper and guard every FFI trace entry, returning a clear typed error (fail loud, no garbage): PyO3 (py_api.rs): - to_f32_vec (covers deconvolve_single, py_indeca_solve_trace, py_seed_trace, py_indeca_estimate_kernel, py_indeca_fit_biexponential) - PySolver::set_trace - deconvolve_batch (inline 2D row conversion bypasses to_f32_vec) - seed_kernel_estimate (inline 2D flat-buffer build) WASM (js_indeca.rs): indeca_solve_trace and seed_trace now return Result and throw on non-finite input. Both CaDecon worker handlers already wrap these in try/catch, so the error surfaces as a job error. The shared Solver core signature is unchanged (native/wasm/pyo3 all compile); validation lives at the entry boundaries. CaTune uses Solver.set_trace directly in the browser — noted as a follow-up (needs a shared-signature change). Tests: Rust first_nonfinite unit tests; Python test_input_validation covers single/batch run_deconvolution, seed_kernel_estimate, and PySolver.set_trace. The batch and seed_kernel_estimate gaps were caught by these tests, not by the 1D guard alone. All suites green (128 Rust, 6 Python, 289 TS). Co-Authored-By: Claude Fable 5 --- crates/solver/src/js_indeca.rs | 24 +++++++++-- crates/solver/src/lib.rs | 28 ++++++++++++ crates/solver/src/py_api.rs | 27 +++++++++++- python/tests/test_input_validation.py | 61 +++++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 6 deletions(-) create mode 100644 python/tests/test_input_validation.py diff --git a/crates/solver/src/js_indeca.rs b/crates/solver/src/js_indeca.rs index ff8fefe..2ff020b 100644 --- a/crates/solver/src/js_indeca.rs +++ b/crates/solver/src/js_indeca.rs @@ -17,6 +17,10 @@ use crate::upsample; /// /// Returns a JsValue containing the serialized InDecaResult: /// { s_counts, alpha, baseline, threshold, pve, iterations, converged } +/// +/// Throws a JS error (rather than returning garbage) if `trace` contains a +/// non-finite value — a NaN/Inf would otherwise propagate silently and yield +/// results indistinguishable from a legitimately hard trace. #[wasm_bindgen] pub fn indeca_solve_trace( trace: &[f32], @@ -30,7 +34,12 @@ pub fn indeca_solve_trace( lp_enabled: bool, warm_counts: &[f32], lambda: f64, -) -> JsValue { +) -> Result { + if let Some(i) = crate::first_nonfinite(trace) { + return Err(JsError::new(&format!( + "indeca_solve_trace: trace contains a non-finite value (NaN or infinity) at index {i}" + ))); + } let warm = if warm_counts.is_empty() { None } else { @@ -49,7 +58,7 @@ pub fn indeca_solve_trace( lp_enabled, lambda, ); - serde_wasm_bindgen::to_value(&result).unwrap_or(JsValue::NULL) + Ok(serde_wasm_bindgen::to_value(&result).unwrap_or(JsValue::NULL)) } /// Estimate a free-form kernel from multiple traces and their spike trains. @@ -158,8 +167,15 @@ pub fn indeca_compute_upsample_factor(fs: f64, target_fs: f64) -> usize { /// /// Returns a JsValue containing the serialized SeedTraceResult: /// { s_counts, alpha, baseline } +/// +/// Throws a JS error if `trace` contains a non-finite value. #[wasm_bindgen] -pub fn seed_trace(trace: &[f32], fs: f64) -> JsValue { +pub fn seed_trace(trace: &[f32], fs: f64) -> Result { + if let Some(i) = crate::first_nonfinite(trace) { + return Err(JsError::new(&format!( + "seed_trace: trace contains a non-finite value (NaN or infinity) at index {i}" + ))); + } let result = peak_seed::seed_trace(trace, fs); - serde_wasm_bindgen::to_value(&result).unwrap_or(JsValue::NULL) + Ok(serde_wasm_bindgen::to_value(&result).unwrap_or(JsValue::NULL)) } diff --git a/crates/solver/src/lib.rs b/crates/solver/src/lib.rs index 25b3b80..8aac929 100644 --- a/crates/solver/src/lib.rs +++ b/crates/solver/src/lib.rs @@ -33,6 +33,16 @@ use std::io::{Cursor, Read}; #[cfg(feature = "jsbindings")] use wasm_bindgen::prelude::*; +/// Index of the first non-finite (NaN or ±infinity) value in `data`, if any. +/// +/// Used by the FFI boundaries (PyO3 / WASM) to reject non-finite input traces +/// up front. A NaN would otherwise propagate silently — e.g. `total_cmp` sorts +/// NaN last, corrupting the rolling-baseline percentile — and yield garbage +/// alpha/PVE results that are indistinguishable from a legitimately hard trace. +pub(crate) fn first_nonfinite(data: &[f32]) -> Option { + data.iter().position(|v| !v.is_finite()) +} + /// Convolution mode for forward/adjoint operations in FISTA. #[derive(Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "jsbindings", wasm_bindgen)] @@ -568,3 +578,21 @@ fn read_f64_le(cur: &mut Cursor<&[u8]>) -> f64 { cur.read_exact(&mut buf).unwrap(); f64::from_le_bytes(buf) } + +#[cfg(test)] +mod finite_guard_tests { + use super::first_nonfinite; + + #[test] + fn clean_slice_has_no_nonfinite() { + assert_eq!(first_nonfinite(&[0.0, 1.5, -2.0, 3.0]), None); + assert_eq!(first_nonfinite(&[]), None); + } + + #[test] + fn detects_nan_and_inf_at_first_index() { + assert_eq!(first_nonfinite(&[0.0, 1.0, f32::NAN, 3.0]), Some(2)); + assert_eq!(first_nonfinite(&[f32::INFINITY, 1.0]), Some(0)); + assert_eq!(first_nonfinite(&[1.0, 2.0, f32::NEG_INFINITY]), Some(2)); + } +} diff --git a/crates/solver/src/py_api.rs b/crates/solver/src/py_api.rs index 1f77ae5..2b3194a 100644 --- a/crates/solver/src/py_api.rs +++ b/crates/solver/src/py_api.rs @@ -9,12 +9,20 @@ const BATCH_SIZE: u32 = 100; const CONTIGUOUS_ERR: &str = "array must be C-contiguous; call numpy.ascontiguousarray() before passing"; -/// Convert a numpy f64 array to a Vec, validating contiguity. +const NONFINITE_ERR: &str = "input array contains a non-finite value (NaN or infinity)"; + +/// Convert a numpy f64 array to a Vec, validating contiguity and finiteness. fn to_f32_vec(arr: &PyReadonlyArray1) -> PyResult> { let slice = arr .as_slice() .map_err(|_| pyo3::exceptions::PyValueError::new_err(CONTIGUOUS_ERR))?; - Ok(slice.iter().map(|&v| v as f32).collect()) + let v: Vec = slice.iter().map(|&x| x as f32).collect(); + if let Some(i) = crate::first_nonfinite(&v) { + return Err(pyo3::exceptions::PyValueError::new_err(format!( + "{NONFINITE_ERR} at index {i}" + ))); + } + Ok(v) } /// Convert an optional numpy f64 array to an optional Vec. @@ -79,6 +87,11 @@ impl PySolver { let slice = trace .as_slice() .map_err(|_| pyo3::exceptions::PyValueError::new_err(CONTIGUOUS_ERR))?; + if let Some(i) = crate::first_nonfinite(slice) { + return Err(pyo3::exceptions::PyValueError::new_err(format!( + "{NONFINITE_ERR} at index {i}" + ))); + } self.inner.set_trace(slice); Ok(()) } @@ -309,6 +322,11 @@ fn deconvolve_batch<'py>( for cell_idx in 0..n_cells { trace_f32.clear(); trace_f32.extend(traces_ref.row(cell_idx).iter().map(|&v| v as f32)); + if let Some(i) = crate::first_nonfinite(&trace_f32) { + return Err(pyo3::exceptions::PyValueError::new_err(format!( + "{NONFINITE_ERR} at row {cell_idx}, index {i}" + ))); + } solver.set_trace(&trace_f32); if hp_enabled || lp_enabled { @@ -378,6 +396,11 @@ fn seed_kernel_estimate<'py>( traces_flat.extend(traces_ref.row(cell_idx).iter().map(|&v| v as f32)); trace_lengths.push(n_timepoints); } + if let Some(i) = crate::first_nonfinite(&traces_flat) { + return Err(pyo3::exceptions::PyValueError::new_err(format!( + "{NONFINITE_ERR} at flattened index {i}" + ))); + } let result = crate::peak_seed::seed_kernel_estimate(&traces_flat, &trace_lengths, fs); diff --git a/python/tests/test_input_validation.py b/python/tests/test_input_validation.py new file mode 100644 index 0000000..a8359fa --- /dev/null +++ b/python/tests/test_input_validation.py @@ -0,0 +1,61 @@ +"""Input-validation tests for the FFI boundary. + +Non-finite (NaN / infinity) trace values must be rejected with a clear error +rather than silently propagating into the solver and returning garbage results. +""" + +from __future__ import annotations + +import numpy as np +import pytest + +from calab import run_deconvolution + +PARAMS = dict(fs=30.0, tau_r=0.02, tau_d=0.4, lam=0.01) + + +def test_run_deconvolution_rejects_nan(): + trace = np.array([0.0, 1.0, np.nan, 2.0], dtype=np.float64) + with pytest.raises(ValueError, match="non-finite"): + run_deconvolution(trace, **PARAMS) + + +def test_run_deconvolution_rejects_inf(): + trace = np.array([0.0, np.inf, 2.0], dtype=np.float64) + with pytest.raises(ValueError, match="non-finite"): + run_deconvolution(trace, **PARAMS) + + +def test_run_deconvolution_rejects_nan_in_batch(): + traces = np.zeros((3, 100), dtype=np.float64) + traces[1, 40] = np.nan + with pytest.raises(ValueError, match="non-finite"): + run_deconvolution(traces, **PARAMS) + + +def test_run_deconvolution_accepts_finite(): + trace = np.zeros(200, dtype=np.float64) + trace[50] = 1.0 + out = run_deconvolution(trace, **PARAMS) + assert out.shape == trace.shape + assert np.all(np.isfinite(out)) + + +def test_seed_kernel_estimate_rejects_nan(): + # The 2D auto-estimate path builds its flat buffer inline (not via the + # shared 1D converter), so it needs its own guard. + import calab._solver as _solver + + traces = np.zeros((2, 100), dtype=np.float64) + traces[0, 30] = np.inf + with pytest.raises(ValueError, match="non-finite"): + _solver.seed_kernel_estimate(traces, 30.0) + + +def test_pysolver_set_trace_rejects_nan(): + import calab._solver as _solver + + solver = _solver.PySolver() + trace = np.array([0.0, 1.0, np.nan], dtype=np.float32) + with pytest.raises(ValueError, match="non-finite"): + solver.set_trace(trace)