From 98fa4427f1131c1f0f912f7fa9758d8afb8ae0ce Mon Sep 17 00:00:00 2001 From: ApiliumDevTeam Date: Wed, 29 Jul 2026 11:51:04 +0200 Subject: [PATCH] fix(ai): stop a single NaN from silently disabling two subsystems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both sites carried a clamp-shaped expression that clippy wanted rewritten. Following that advice mechanically would have introduced a bug in one and papered over a worse one in the other, so each is settled here instead. SurpriseGate. `min`/`max` discard NaN and return the other operand; `clamp` propagates it. But the real entry point is upstream: `record_surprise` feeds Welford's algorithm, and one non-finite sample makes the running mean NaN permanently — every later update keeps it, the adaptive threshold inherits it, and because every comparison against NaN is false the gate stops firing with nothing to show for it. Reject non-finite samples at the boundary, and fall back to the CONFIGURED threshold — the non-adaptive baseline this type already has — rather than to an invented constant, if unusable statistics arrive some other way (deserialised state is not covered by the entry guard). TransactionClassifier::confidence. `partial_cmp().unwrap()` panics on a NaN distance, which `l2_distance` produces from a corrupt centroid, so the process died inside a confidence calculation. `total_cmp` is total and also sorts NaN last, so a corrupt centroid can no longer pass itself off as the nearest one. And with every distance unusable the old code fell past the `second_dist > 0.0` test to return 1.0 — FULL confidence at the exact moment there is no information. It now answers 0.5, the same neutral value it already returns when there are no centroids at all. Five tests, each pinning a failure that was silent: a poisoned mean, a NaN threshold, the panic, and full confidence from no information. --- crates/aingle_ai/src/ineru/surprise_gate.rs | 89 +++++++++++++++---- .../src/nested_learning/transaction_level.rs | 60 ++++++++++--- 2 files changed, 121 insertions(+), 28 deletions(-) diff --git a/crates/aingle_ai/src/ineru/surprise_gate.rs b/crates/aingle_ai/src/ineru/surprise_gate.rs index 677eccd..f459e7a 100644 --- a/crates/aingle_ai/src/ineru/surprise_gate.rs +++ b/crates/aingle_ai/src/ineru/surprise_gate.rs @@ -75,7 +75,17 @@ impl SurpriseGate { } /// Record a surprise value for calibration + /// + /// Non-finite samples are dropped. Welford's algorithm has no way back from + /// one: `mean += delta / n` with a NaN delta makes the mean NaN, every + /// later update keeps it NaN, and `adaptive_threshold` inherits it. A NaN + /// threshold is worse than any number, because every comparison against it + /// is false — the gate stops firing and nothing says so. pub fn record_surprise(&mut self, surprise: f32) { + if !surprise.is_finite() { + return; + } + // Add to recent window self.recent_surprises.push_back(surprise); if self.recent_surprises.len() > self.calibration_window { @@ -131,24 +141,22 @@ impl SurpriseGate { /// Get adaptive threshold based on recent statistics /// - /// NOT `clamp`, and not by accident. `f32::min(NaN, 1.0)` returns `1.0` — - /// `min`/`max` discard NaN and return the other operand — so a NaN mean or - /// std yields a threshold of `1.0` here. `NaN.clamp(0.1, 1.0)` returns NaN, - /// and a NaN threshold is worse than either bound: every comparison against - /// it is false, so the gate would stop firing and do so silently. - /// `get_std()` can produce NaN from an empty history or from a negative - /// variance caused by floating-point error. - /// - /// The right fix is neither expression: branch on `is_nan()` explicitly and - /// pin the chosen fallback with a test. Which fallback is correct — today's - /// 1.0, meaning the gate almost never fires, or 0.1, meaning it almost - /// always does — belongs to whoever owns this gate, so it is left as a - /// decision rather than made silently by a lint fix. - #[allow(clippy::manual_clamp)] + /// Falls back to the configured [`Self::set_threshold`] value when the + /// running statistics cannot produce a usable number. `record_surprise` + /// already refuses non-finite samples, so this should be unreachable — but + /// it is the difference between a wrong threshold and a NaN one, and a NaN + /// threshold disables the gate silently. Deserialised state is not covered + /// by the entry guard either. pub fn adaptive_threshold(&self) -> f32 { // Use mean + 1 std as adaptive threshold let std = self.get_std(); - (self.mean_surprise + std).min(1.0).max(0.1) + let adaptive = self.mean_surprise + std; + + if !adaptive.is_finite() { + return self.threshold.clamp(0.1, 1.0); + } + + adaptive.clamp(0.1, 1.0) } /// Get statistics @@ -255,4 +263,55 @@ mod tests { let adaptive = gate.adaptive_threshold(); assert!(adaptive > 0.0 && adaptive < 1.0); } + + #[test] + fn a_non_finite_sample_cannot_poison_the_running_statistics() { + // Welford has no way back from a NaN: `mean += delta / n` with a NaN + // delta makes the mean NaN and every later update keeps it that way. + let mut gate = SurpriseGate::new(0.5); + for _ in 0..20 { + gate.record_surprise(0.4); + } + let before = gate.adaptive_threshold(); + + gate.record_surprise(f32::NAN); + gate.record_surprise(f32::INFINITY); + gate.record_surprise(f32::NEG_INFINITY); + + assert_eq!( + gate.adaptive_threshold(), + before, + "a non-finite sample must be dropped, not folded into the mean" + ); + } + + #[test] + fn the_adaptive_threshold_is_always_a_usable_number() { + // The failure this guards is silent: every comparison against a NaN + // threshold is false, so `should_update` would stop firing and the + // memory would quietly stop learning. + let mut gate = SurpriseGate::new(0.5); + gate.record_surprise(f32::NAN); + for s in [0.0, 1.0, 0.5, f32::NAN, 0.9] { + gate.record_surprise(s); + } + + let t = gate.adaptive_threshold(); + assert!(t.is_finite(), "threshold was {t}"); + assert!( + (0.1..=1.0).contains(&t), + "threshold {t} left its documented range" + ); + } + + #[test] + fn unusable_statistics_fall_back_to_the_configured_threshold() { + // Not to an invented constant: the value the caller set is the + // non-adaptive baseline this gate already has. + let mut gate = SurpriseGate::new(0.5); + gate.set_threshold(0.7); + gate.mean_surprise = f32::NAN; + + assert!((gate.adaptive_threshold() - 0.7).abs() < 1e-6); + } } diff --git a/crates/aingle_ai/src/nested_learning/transaction_level.rs b/crates/aingle_ai/src/nested_learning/transaction_level.rs index 19d8350..7a85a25 100644 --- a/crates/aingle_ai/src/nested_learning/transaction_level.rs +++ b/crates/aingle_ai/src/nested_learning/transaction_level.rs @@ -191,25 +191,29 @@ impl TransactionClassifier { return 0.5; } - // Find min and second min distance + // Find min and second min distance. + // + // `total_cmp`, not `partial_cmp().unwrap()`: the latter PANICS on a NaN + // distance, which `l2_distance` yields from a centroid holding one. A + // total order also sorts NaN last, so a corrupt centroid cannot pass + // itself off as the nearest one. let mut sorted = distances.clone(); - sorted.sort_by(|a, b| a.partial_cmp(b).unwrap()); + sorted.sort_by(f32::total_cmp); let min_dist = sorted.first().copied().unwrap_or(1.0); let second_dist = sorted.get(1).copied().unwrap_or(min_dist); - // Confidence based on separation. - // - // NOT `clamp`: `max`/`min` discard NaN and `clamp` propagates it, so the - // two differ exactly when the input is NaN. Here the division is guarded - // by `second_dist > 0.0`, so NaN should be unreachable — but that rests - // on the `partial_cmp().unwrap()` in the sort above, which PANICS on a - // NaN distance rather than producing one. Substituting `clamp` would be - // safe and would also settle nothing; the real work is making that sort - // total (`total_cmp`) and then deciding what a NaN distance means. - #[allow(clippy::manual_clamp)] + // Nothing usable to measure a separation with. Answer what this function + // already answers when there are no centroids at all, rather than let + // the `> 0.0` test below fall through to 1.0 — which would report FULL + // confidence precisely when there is no information. + if !min_dist.is_finite() || !second_dist.is_finite() { + return 0.5; + } + + // Confidence based on separation if second_dist > 0.0 { - (1.0 - min_dist / second_dist).max(0.0).min(1.0) + (1.0 - min_dist / second_dist).clamp(0.0, 1.0) } else { 1.0 } @@ -316,4 +320,34 @@ mod tests { let processed2 = tl.process(tx); assert!(processed2.confidence > 0.0); } + + #[test] + fn a_nan_centroid_does_not_panic_and_does_not_win() { + // `partial_cmp().unwrap()` panicked here on any NaN distance, taking the + // process down from inside a confidence calculation. + let mut classifier = TransactionClassifier::new(); + classifier.centroids.clear(); + classifier.centroids.insert("good".into(), vec![0.0; 16]); + classifier + .centroids + .insert("corrupt".into(), vec![f32::NAN; 16]); + + let c = classifier.confidence(&[0.5; 16]); + + assert!(c.is_finite(), "confidence was {c}"); + assert!((0.0..=1.0).contains(&c), "confidence {c} left its range"); + } + + #[test] + fn no_measurable_separation_reports_neutral_confidence_not_full() { + // With every distance unusable the old code fell through the + // `second_dist > 0.0` test and returned 1.0 — maximum confidence at the + // exact moment there was no information at all. + let mut classifier = TransactionClassifier::new(); + classifier.centroids.clear(); + classifier.centroids.insert("a".into(), vec![f32::NAN; 16]); + classifier.centroids.insert("b".into(), vec![f32::NAN; 16]); + + assert_eq!(classifier.confidence(&[0.5; 16]), 0.5); + } }