From db4889dc76a6660404b65e92dd5a198903da73fe Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 13:26:03 +0000 Subject: [PATCH 1/2] jc reliability: enforce the no-NaN contract on non-finite input The battery's module doc promised `None` "rather than returning NaN", but non-finite input slipped through: spearman(&[1.0, NAN, 2.0], ...) returned Some(1.0) (average_ranks maps NaN to a finite rank, so it never reached the delegated pearson guard) and pearson/cronbach_alpha/icc returned Some(NaN). Add an `all_finite` helper + up-front finite guards on all four public APIs (spearman guards the raw values before ranking), plus trailing result guards so finite-but-overflowing magnitudes (deviations -> +/-inf -> ratio NaN) also return None. Two regression tests cover NaN/+/-inf (incl. the reported reproducer) and 1e308-overflow inputs. Also soften the "ten pillars" aside in the module doc to "registered pillars" (the registry holds 12; a hard count in a doc-comment is a drift magnet). Found post-merge by CodeRabbit (Major) and Codex independently on #709. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Awg6TXocHcwTtc6eGsHcdD --- .claude/board/EPIPHANIES.md | 9 ++++ crates/jc/src/reliability.rs | 81 ++++++++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/.claude/board/EPIPHANIES.md b/.claude/board/EPIPHANIES.md index 976397da..9f14e391 100644 --- a/.claude/board/EPIPHANIES.md +++ b/.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. diff --git a/crates/jc/src/reliability.rs b/crates/jc/src/reliability.rs index 5c233dae..80753e15 100644 --- a/crates/jc/src/reliability.rs +++ b/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 { Some(xs.iter().sum::() / 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 { 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; @@ -93,7 +106,10 @@ pub fn pearson(x: &[f64], y: &[f64]) -> Option { if denom == 0.0 { return None; // at least one series is constant } - Some(sxy / denom) + let r = sxy / denom; + // Even with finite inputs, large magnitudes can overflow the squared + // deviations to ±∞, making the ratio non-finite — reject that too. + r.is_finite().then_some(r) } /// Average (fractional) ranks of a series, tie-corrected: tied values receive @@ -144,6 +160,13 @@ pub fn spearman(x: &[f64], y: &[f64]) -> Option { 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 +210,9 @@ pub fn cronbach_alpha(items: &[Vec]) -> Option { 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; @@ -200,7 +226,10 @@ pub fn cronbach_alpha(items: &[Vec]) -> Option { 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 +274,9 @@ pub fn icc(ratings: &[Vec], form: IccForm) -> Option { 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; @@ -284,7 +316,10 @@ pub fn icc(ratings: &[Vec], form: IccForm) -> Option { if denom == 0.0 { return None; } - Some((ms_r - ms_e) / denom) + let v = (ms_r - ms_e) / denom; + // Overflow of the mean-square sums to ±∞ can make the ratio non-finite + // even for finite inputs — reject that too. + v.is_finite().then_some(v) } #[cfg(test)] @@ -427,4 +462,42 @@ 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() { + // Finite but astronomically large magnitudes overflow the squared + // deviations / mean-square sums to ±∞, whose ratio is NaN. The result + // guards must catch that and return None, never Some(NaN). + 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:?}"); + } } From 0f0bba122ba2feed18eb72db819385e4642f1b73 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Jul 2026 13:32:35 +0000 Subject: [PATCH 2/2] jc reliability: reject overflowed denominators (Codex P2 on #710) The trailing is_finite() guards missed one finite-but-wrong case: when the squared-deviation product (pearson), subject-total variance (cronbach), or mean-square sum (icc) overflows a finite input to +inf, the denominator becomes inf and numerator/inf collapses to a FINITE 0.0. Example flagged by Codex: pearson(&[1e100,-1e100],&[1e100,-1e100]) is perfectly correlated but returned Some(0.0) instead of None. Guard `denom == 0.0 || !denom.is_finite()` in all three ratio computations so an overflowed denominator returns None. The existing result-finiteness guards stay for the non-finite-numerator case. Extend the overflow test with the exact Codex reproducer; note in-test why spearman is immune (it Pearson's the bounded 1..n ranks, which never overflow). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Awg6TXocHcwTtc6eGsHcdD --- crates/jc/src/reliability.rs | 39 +++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/crates/jc/src/reliability.rs b/crates/jc/src/reliability.rs index 80753e15..8c15a7b3 100644 --- a/crates/jc/src/reliability.rs +++ b/crates/jc/src/reliability.rs @@ -103,12 +103,17 @@ pub fn pearson(x: &[f64], y: &[f64]) -> Option { 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; } let r = sxy / denom; - // Even with finite inputs, large magnitudes can overflow the squared - // deviations to ±∞, making the ratio non-finite — reject that too. + // 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) } @@ -222,7 +227,10 @@ pub fn cronbach_alpha(items: &[Vec]) -> Option { .map(|s| items.iter().map(|it| it[s]).sum::()) .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; @@ -313,12 +321,15 @@ pub fn icc(ratings: &[Vec], form: IccForm) -> Option { 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; } let v = (ms_r - ms_e) / denom; - // Overflow of the mean-square sums to ±∞ can make the ratio non-finite - // even for finite inputs — reject that too. + // 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) } @@ -486,9 +497,17 @@ mod tests { #[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. The result - // guards must catch that and return None, never Some(NaN). + // 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:?}");