jc reliability: enforce the no-NaN contract on non-finite input (follow-up to #709)

.claude/board/EPIPHANIES.md

@@ -1,3 +1,12 @@
+## 2026-07-17 — E-JC-BATTERY-NO-NAN-CONTRACT-1 — the reliability battery's documented no-NaN contract was not enforced for non-finite input; two reviewers caught `Some(NaN)`/finite-garbage (follow-up to P5a)
+
+**Status:** SHIPPED (follow-up PR to #709; `crates/jc/src/reliability.rs`, +2 regression tests, 14 lib + 3 doctests green). Corrects a real gap in E-JC-IS-NOT-A-METRIC-BATTERY-1's shipped code, not that entry's framing (append-only — the merged entry stands).
+
+- **The finding (CodeRabbit Major + Codex, independently):** the module doc promised "`None` … rather than panicking or returning `NaN`", but `spearman(&[1.0, NAN, 2.0], &[1.0, 2.0, 3.0])` returned `Some(1.0)` and `pearson`/`cronbach_alpha`/`icc` returned `Some(NaN)` for non-finite input. Spearman was the sharpest case: `average_ranks` maps `NaN` (via `partial_cmp → Equal`) to an ordinary FINITE rank, so the NaN never reached the delegated `pearson` guard — it produced a finite-but-garbage ρ that would silently poison the D-TRI agreement gates.
+- **The fix:** an `all_finite` helper + an up-front `if !all_finite(..) { return None }` guard on all four public APIs (spearman guards on the RAW values before ranking, not just via `pearson`), plus a trailing `x.is_finite().then_some(x)` result guard so finite-but-overflowing magnitudes (deviations → ±∞ → ratio NaN) also return `None`. Two regression tests: `non_finite_inputs_return_none` (NaN/±∞ across all four, incl. the Codex reproducer) and `overflowing_large_finite_inputs_return_none_not_nan` (1e308 magnitudes).
+- **Doc-count fix:** the "the ten pillars" aside in the module doc became "the registered pillars" (the registry holds 12; a hard count in a doc-comment is a drift magnet — the merged EPIPHANIES entry's "ten pillar-provers" text is append-only and left as-is).
+- Credit: CodeRabbit (thread on #709) + Codex (chatgpt-codex-connector) both flagged it post-merge; the fix ships as a distinct follow-up PR, not a reopen of #709.
+
 ## 2026-07-17 — E-JC-IS-NOT-A-METRIC-BATTERY-1 — `jc` was a FORMAL-SCAFFOLD pillar-prover, NOT a callable reliability/validity battery; the four D-TRI measurement gates had nothing to call (plateau P5a)
 
 **Status:** SHIPPED (plateau P5a; `crates/jc/src/reliability.rs`, 12 tests + 3 doctests green). Corrects the assumption (in the triangle plan §4/§6 and the operator brief "measure with lance-graph/crates/JC ICC Pearson Cronbach alpha spearman") that jc already exposes these.

crates/jc/src/reliability.rs

@@ -4,7 +4,7 @@
 //!
 //! # Why this lives in `jc`
 //!
-//! `jc` is otherwise a proof-in-code harness (the ten pillars). But the
+//! `jc` is otherwise a proof-in-code harness (the registered pillars). But the
 //! workspace repeatedly asks the SAME empirical question — "do two
 //! representations measure the same construct, reliably?" — and until now each
 //! caller rolled its own (e.g. a private `spearman_rho` on rankings in
@@ -62,6 +62,16 @@ fn mean(xs: &[f64]) -> Option<f64> {
     Some(xs.iter().sum::<f64>() / xs.len() as f64)
 }
 
+/// Whether every element is finite (no `NaN`, no `±∞`). The no-`NaN` API
+/// contract is enforced by rejecting non-finite INPUTS up front: a `NaN` sorts
+/// as "equal" in the rank step and would otherwise receive an ordinary rank,
+/// silently producing a finite-but-garbage Spearman ρ (and `Some(NaN)` for the
+/// other three). Every public metric guards on this before computing.
+#[inline]
+fn all_finite(xs: &[f64]) -> bool {
+    xs.iter().all(|v| v.is_finite())
+}
+
 /// Pearson product-moment correlation coefficient of two equal-length series.
 ///
 /// Returns `None` if the series differ in length, have fewer than 2 elements,
@@ -77,6 +87,9 @@ pub fn pearson(x: &[f64], y: &[f64]) -> Option<f64> {
     if x.len() != y.len() || x.len() < 2 {
         return None;
     }
+    if !all_finite(x) || !all_finite(y) {
+        return None; // NaN / ±∞ input → undefined (no-NaN contract)
+    }
     let mx = mean(x)?;
     let my = mean(y)?;
     let mut sxy = 0.0;
@@ -90,10 +103,18 @@ pub fn pearson(x: &[f64], y: &[f64]) -> Option<f64> {
         syy += dy * dy;
     }
     let denom = (sxx * syy).sqrt();
-    if denom == 0.0 {
-        return None; // at least one series is constant
+    if denom == 0.0 || !denom.is_finite() {
+        // `denom == 0` → at least one series is constant. `denom == ∞` → the
+        // squared-deviation product overflowed on large finite input; then
+        // `sxy / ∞ = 0.0` is FINITE-but-wrong, so the trailing `is_finite`
+        // guard would miss it (e.g. `pearson(&[1e100,-1e100],&[1e100,-1e100])`
+        // is perfectly correlated but the ratio collapses to 0.0). Reject here.
+        return None;
     }
-    Some(sxy / denom)
+    let r = sxy / denom;
+    // With a finite non-zero denom, `sxy` can still be ±∞/NaN on overflow —
+    // reject a non-finite ratio too.
+    r.is_finite().then_some(r)
 }
 
 /// Average (fractional) ranks of a series, tie-corrected: tied values receive
@@ -144,6 +165,13 @@ pub fn spearman(x: &[f64], y: &[f64]) -> Option<f64> {
     if x.len() != y.len() || x.len() < 2 {
         return None;
     }
+    if !all_finite(x) || !all_finite(y) {
+        // Guard here, not just in the delegated `pearson`: `average_ranks`
+        // maps a NaN to an ordinary finite rank (partial_cmp → Equal), so the
+        // ranks handed to `pearson` are all finite and the NaN would slip
+        // through as a finite-but-garbage ρ. Reject non-finite input up front.
+        return None;
+    }
     let rx = average_ranks(x);
     let ry = average_ranks(y);
     pearson(&rx, &ry)
@@ -187,6 +215,9 @@ pub fn cronbach_alpha(items: &[Vec<f64>]) -> Option<f64> {
     if n == 0 || items.iter().any(|it| it.len() != n) {
         return None;
     }
+    if items.iter().any(|it| !all_finite(it)) {
+        return None; // NaN / ±∞ input → undefined (no-NaN contract)
+    }
     let sum_item_var: f64 = items.iter().filter_map(|it| pop_var(it)).sum();
     if items.iter().any(|it| pop_var(it).is_none()) {
         return None;
@@ -196,11 +227,17 @@ pub fn cronbach_alpha(items: &[Vec<f64>]) -> Option<f64> {
         .map(|s| items.iter().map(|it| it[s]).sum::<f64>())
         .collect();
     let total_var = pop_var(&totals)?;
-    if total_var == 0.0 {
+    if total_var == 0.0 || !total_var.is_finite() {
+        // 0 → no between-subject variance (α undefined). ∞ → the totals
+        // overflowed on large finite input; `sum_item_var / ∞ = 0.0` would
+        // yield a finite-but-wrong α, so reject the overflowed denominator.
         return None;
     }
     let kf = k as f64;
-    Some((kf / (kf - 1.0)) * (1.0 - sum_item_var / total_var))
+    let alpha = (kf / (kf - 1.0)) * (1.0 - sum_item_var / total_var);
+    // Even with finite inputs, large magnitudes can overflow the variance
+    // sums to ±∞, making α non-finite — reject that too.
+    alpha.is_finite().then_some(alpha)
 }
 
 /// The single-measure ICC forms of Shrout & Fleiss (1979).
@@ -245,6 +282,9 @@ pub fn icc(ratings: &[Vec<f64>], form: IccForm) -> Option<f64> {
     if k < 2 || ratings.iter().any(|r| r.len() != k) {
         return None;
     }
+    if ratings.iter().any(|r| !all_finite(r)) {
+        return None; // NaN / ±∞ input → undefined (no-NaN contract)
+    }
     let nf = n as f64;
     let kf = k as f64;
 
@@ -281,10 +321,16 @@ pub fn icc(ratings: &[Vec<f64>], form: IccForm) -> Option<f64> {
         IccForm::Icc3_1 => ms_r + (kf - 1.0) * ms_e,
         IccForm::Icc2_1 => ms_r + (kf - 1.0) * ms_e + (kf / nf) * (ms_c - ms_e),
     };
-    if denom == 0.0 {
+    if denom == 0.0 || !denom.is_finite() {
+        // 0 → zero-variance degenerate. ∞ → the mean-square sums overflowed on
+        // large finite input; `(ms_r - ms_e) / ∞ = 0.0` would be finite-but-
+        // wrong, so reject the overflowed denominator here.
         return None;
     }
-    Some((ms_r - ms_e) / denom)
+    let v = (ms_r - ms_e) / denom;
+    // A finite non-zero denom can still pair with a non-finite numerator on
+    // overflow — reject a non-finite ratio too.
+    v.is_finite().then_some(v)
 }
 
 #[cfg(test)]
@@ -427,4 +473,50 @@ mod tests {
         assert_eq!(icc(&[vec![1.0], vec![2.0]], IccForm::Icc2_1), None); // k < 2
         assert_eq!(icc(&[vec![1.0, 2.0], vec![1.0]], IccForm::Icc2_1), None); // ragged
     }
+
+    #[test]
+    fn non_finite_inputs_return_none() {
+        // The no-NaN API contract: any NaN / ±∞ in the input yields None, never
+        // a finite-but-garbage estimate or Some(NaN). The Spearman case is the
+        // one two independent reviewers flagged: `average_ranks` maps NaN to a
+        // finite rank, so without the up-front guard it would return Some(1.0).
+        assert_eq!(spearman(&[1.0, f64::NAN, 2.0], &[1.0, 2.0, 3.0]), None);
+        assert_eq!(spearman(&[1.0, 2.0, 3.0], &[1.0, f64::INFINITY, 3.0]), None);
+        assert_eq!(pearson(&[1.0, f64::NAN, 3.0], &[1.0, 2.0, 3.0]), None);
+        assert_eq!(pearson(&[1.0, 2.0, 3.0], &[f64::NEG_INFINITY, 2.0, 3.0]), None);
+
+        let nan_items = vec![vec![1.0, 2.0, f64::NAN], vec![1.0, 2.0, 3.0]];
+        assert_eq!(cronbach_alpha(&nan_items), None);
+        let inf_items = vec![vec![1.0, 2.0, 3.0], vec![1.0, f64::INFINITY, 3.0]];
+        assert_eq!(cronbach_alpha(&inf_items), None);
+
+        let nan_ratings = vec![vec![1.0, 2.0], vec![f64::NAN, 4.0], vec![5.0, 6.0]];
+        assert_eq!(icc(&nan_ratings, IccForm::Icc2_1), None);
+        assert_eq!(icc(&nan_ratings, IccForm::Icc3_1), None);
+    }
+
+    #[test]
+    fn overflowing_large_finite_inputs_return_none_not_nan() {
+        // The Codex reproducer: perfectly-correlated large-magnitude data whose
+        // squared-deviation product overflows the denominator to ∞, so `sxy/∞`
+        // collapses to a FINITE-but-wrong 0.0 that a trailing `is_finite` guard
+        // misses. Must be None (the denom-finiteness guard catches it).
+        assert_eq!(pearson(&[1e100, -1e100], &[1e100, -1e100]), None);
+        // (spearman is immune to this: it Pearson's the tiny 1..n ranks, which
+        // never overflow — `spearman(&[1e100,-1e100],&[1e100,-1e100])` is a
+        // correct `Some(1.0)`, so it is not asserted here.)
+        // Finite but astronomically large magnitudes overflow the squared
+        // deviations / mean-square sums to ±∞, whose ratio is NaN or a
+        // finite-but-wrong 0.0. The denom + result guards must return None.
+        let big = 1e308;
+        for r in [pearson(&[big, -big, big], &[big, big, -big]), spearman(&[big, -big, big], &[big, big, -big])] {
+            assert!(r.map(|v| v.is_finite()).unwrap_or(true), "expected finite or None, got {r:?}");
+        }
+        let big_items = vec![vec![big, -big, big], vec![-big, big, -big]];
+        let a = cronbach_alpha(&big_items);
+        assert!(a.map(|v| v.is_finite()).unwrap_or(true), "cronbach: expected finite or None, got {a:?}");
+        let big_ratings = vec![vec![big, -big], vec![-big, big], vec![big, big]];
+        let v = icc(&big_ratings, IccForm::Icc2_1);
+        assert!(v.map(|x| x.is_finite()).unwrap_or(true), "icc: expected finite or None, got {v:?}");
+    }
 }