From 9d3e002be36ea43d9fd56aa75f53dded3e274221 Mon Sep 17 00:00:00 2001
From: Anders Olsson <anders.e.olsson@gmail.com>
Date: Wed, 9 Sep 2026 21:40:39 +0200
Subject: [PATCH] fix!: no prediction path answers from a fit it cannot answer
 from
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`converge` refuses to report a NaN fit. Nothing stopped a caller from
ignoring that error and predicting anyway, and every prediction path was
differently wrong when they did. Measured on a point-mass-prior history
with `beta(0.0)`, after `converge` returned `NonFiniteResult`:

    predict_quality           = Ok(NaN)
    predict_outcome().total() = NaN
    predict_win_probabilities = Ok([0.0, 0.0])

The third is the dangerous one: finite, plausible, and summing to zero
against a doc that promises one at `p_draw == 0`. A caller checking
`total() ≈ 1` catches the second and misses it.

The same parameters on a *scored* event converge cleanly and leave
legitimate point-mass posteriors. There `predict_quality` **panicked** —
"cannot invert a singular matrix", out of a method returning `Result` —
because the contrast covariance `beta²AᵀA + AᵀSA` is exactly singular,
and `predict_win_probabilities` again returned `Ok([0.0, 0.0])`. That
promise assumes continuous performances, where an exact tie has measure
zero; point masses break the assumption, not the arithmetic.

Both checks now live at `member_skills`, the one gate every prediction
path reads skills through, rather than being repeated per method.

The finiteness check is on `mu` / `sigma`, not on the natural parameters.
The first attempt checked `pi` and `tau`, and measurement showed it
rejected a *legitimate* point mass — `pi = inf`, `mu = 0`, `sigma = 0` —
turning a working prediction into an error. The question is whether the
usable moments exist, and those are what predictions consume.

Docs: `converge_partial` omitted the drift-variance `InvalidParameter` it
validates before sweeping, and the free `expected_information_gain`
omitted `GridTooCoarse`, which comes from `outcome_distribution` and so
is not covered by its "anything `Game::ranked` returns" clause.

Refs #78 (parts 1 and 2; the layering and `predict_quality` rename
questions are still open).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
---
 src/acquisition.rs         |   4 +
 src/history.rs             | 101 +++++++++++++++++++++++-
 tests/prediction_guards.rs | 152 +++++++++++++++++++++++++++++++++++++
 3 files changed, 254 insertions(+), 3 deletions(-)
 create mode 100644 tests/prediction_guards.rs

diff --git a/src/acquisition.rs b/src/acquisition.rs
index 23d1eea..c21bab7 100644
--- a/src/acquisition.rs
+++ b/src/acquisition.rs
@@ -124,6 +124,10 @@ fn u_minus_ln1p(u: f64) -> f64 {
 /// - `TooManyTeams` if the outcome space is too large to enumerate; see
 ///   [`MAX_PREDICTED_TEAMS`](crate::MAX_PREDICTED_TEAMS).
 /// - `InvalidProbability` if `options.p_draw` is outside `[0.0, 1.0)`.
+/// - `GridTooCoarse` when the performance sigmas are too far apart to
+///   integrate on one grid. This comes from `outcome_distribution`, which runs
+///   before any inference — so it is not covered by "anything `Game::ranked`
+///   returns" below.
 /// - Anything [`Game::ranked`](crate::Game::ranked) returns for a hypothetical
 ///   outcome.
 pub fn expected_information_gain<T: Time, D: Drift<T>>(
diff --git a/src/history.rs b/src/history.rs
index e697d25..ecc03f6 100644
--- a/src/history.rs
+++ b/src/history.rs
@@ -1071,7 +1071,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
                         .find_map(|ts| ts.skills.get(index).map(|s| s.posterior()))
                 });
 
-                members.push(match skill {
+                let skill = match skill {
                     Some(skill) => skill,
                     None => match self.unknown_keys {
                         crate::UnknownKeys::Prior => Gaussian::from_ms(self.mu, self.sigma),
@@ -1083,12 +1083,73 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
                             });
                         }
                     },
-                });
+                };
+
+                // The single gate every prediction path reads skills through.
+                //
+                // `converge` refuses to report a NaN fit, but nothing stopped a
+                // caller ignoring that error and predicting anyway. Measured on
+                // a point-mass-prior history with `beta(0.0)`, after `converge`
+                // returned `NonFiniteResult`: `predict_quality` gave `Ok(NaN)`,
+                // `predict_outcome().total()` gave `NaN`, and
+                // `predict_win_probabilities` gave `Ok([0.0, 0.0])` — finite,
+                // plausible, and summing to zero against a doc that promises
+                // one. That last shape is the dangerous one, and it is exactly
+                // what a caller checking `total() ≈ 1` would catch on the
+                // others and miss here.
+                //
+                // Checked on `mu` / `sigma`, not on the natural parameters.
+                // Measured: a point-mass posterior is `pi = inf`, which is a
+                // legitimate converged state that reads back as `mu = 0`,
+                // `sigma = 0` — a natural-parameter finiteness check rejects
+                // it and turns a working prediction into an error. The
+                // question here is whether the *usable* moments exist, and
+                // `mu()` / `sigma()` are what every prediction path consumes.
+                //
+                // This also catches an improper skill (`pi <= 0`), which
+                // `sigma()` reports as infinite. Nothing produces one as a
+                // stored posterior today, and a prediction from an
+                // uninformative skill would be meaningless if it did.
+                if !skill.mu().is_finite() || !skill.sigma().is_finite() {
+                    return Err(InferenceError::NonFiniteResult {
+                        context: "prediction read a skill with no usable mean or \
+                                  variance; the fit did not converge",
+                        step: (skill.mu(), skill.sigma()),
+                    });
+                }
+
+                members.push(skill);
             }
 
             gathered.push(members);
         }
 
+        // Degenerate performances: `beta == 0` with every skill a point mass.
+        //
+        // Every prediction here is a statement about how performances *vary*,
+        // and in this configuration nothing varies. The consequences were three
+        // different wrong answers rather than one error. `predict_quality`
+        // **panicked** — "cannot invert a singular matrix", from a
+        // `Result`-returning method, on a history that had converged cleanly —
+        // because the contrast covariance `beta^2 A^T A + A^T S A` is exactly
+        // singular. `predict_win_probabilities` returned `Ok([0.0, 0.0])`
+        // against a doc that promises they sum to one at `p_draw == 0`; the
+        // promise assumes continuous performances, where an exact tie has
+        // measure zero, and point masses break that assumption rather than the
+        // arithmetic.
+        //
+        // Checked once here, at the gate every prediction path reads skills
+        // through, rather than per method — the condition is the same one each
+        // time and it is a property of the parameters, not a numerical
+        // accident.
+        if self.beta == 0.0 && gathered.iter().flatten().all(|g| g.sigma() == 0.0) {
+            return Err(InferenceError::InvalidParameter {
+                name: "beta is zero and every skill is a point mass, so there is \
+                       no performance distribution to predict from",
+                value: 0.0,
+            });
+        }
+
         Ok(gathered)
     }
 
@@ -1159,7 +1220,13 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     ///
     /// # Errors
     ///
-    /// `NotEnoughTeams`, `EmptyTeam`, or `UnknownKey`.
+    /// `NotEnoughTeams`, `EmptyTeam` or `UnknownKey`.
+    ///
+    /// Every prediction reads skills through one gate, which adds two errors to
+    /// all of them: `NonFiniteResult` if a skill has no usable mean or variance
+    /// — the fit did not converge — and `InvalidParameter` if `beta` is zero
+    /// and every skill is a point mass, leaving no performance distribution to
+    /// predict from.
     pub fn predict_quality(&self, teams: &[&[&K]]) -> Result<f64, InferenceError>
     where
         K: std::fmt::Debug,
@@ -1668,6 +1735,12 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     /// shape of the request, `GridTooCoarse` when the performance sigmas are
     /// too far apart to integrate on one grid, and anything inference returns
     /// for a hypothetical outcome.
+    ///
+    /// Every prediction reads skills through one gate, which adds two errors to
+    /// all of them: `NonFiniteResult` if a skill has no usable mean or variance
+    /// — the fit did not converge — and `InvalidParameter` if `beta` is zero
+    /// and every skill is a point mass, leaving no performance distribution to
+    /// predict from.
     pub fn expected_information_gain(&self, teams: &[&[&K]]) -> Result<f64, InferenceError>
     where
         K: std::fmt::Debug,
@@ -1722,6 +1795,12 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     /// # Errors
     ///
     /// `NotEnoughTeams`, `EmptyTeam`, or `UnknownKey`.
+    ///
+    /// Every prediction reads skills through one gate, which adds two errors to
+    /// all of them: `NonFiniteResult` if a skill has no usable mean or variance
+    /// — the fit did not converge — and `InvalidParameter` if `beta` is zero
+    /// and every skill is a point mass, leaving no performance distribution to
+    /// predict from.
     pub fn predict_win_probabilities(&self, teams: &[&[&K]]) -> Result<Vec<f64>, InferenceError>
     where
         K: std::fmt::Debug,
@@ -1770,6 +1849,12 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     /// `NotEnoughTeams`, `EmptyTeam`, `UnknownKey`, or `TooManyTeams`.
     /// `GridTooCoarse` when the performance sigmas are too far apart to
     /// integrate on one grid.
+    ///
+    /// Every prediction reads skills through one gate, which adds two errors to
+    /// all of them: `NonFiniteResult` if a skill has no usable mean or variance
+    /// — the fit did not converge — and `InvalidParameter` if `beta` is zero
+    /// and every skill is a point mass, leaving no performance distribution to
+    /// predict from.
     pub fn predict_outcome(&self, teams: &[&[&K]]) -> Result<Prediction, InferenceError>
     where
         K: std::fmt::Debug,
@@ -1813,6 +1898,12 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     /// `NotEnoughTeams`, `EmptyTeam`, `UnknownKey`, or `MismatchedShape` if
     /// `ranks` does not have one entry per team. `GridTooCoarse` when the
     /// performance sigmas are too far apart to integrate on one grid.
+    ///
+    /// Every prediction reads skills through one gate, which adds two errors to
+    /// all of them: `NonFiniteResult` if a skill has no usable mean or variance
+    /// — the fit did not converge — and `InvalidParameter` if `beta` is zero
+    /// and every skill is a point mass, leaving no performance distribution to
+    /// predict from.
     pub fn predict_ranking(&self, teams: &[&[&K]], ranks: &[u32]) -> Result<f64, InferenceError>
     where
         K: std::fmt::Debug,
@@ -1888,6 +1979,10 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     /// # Errors
     ///
     /// `NonFiniteResult` if a sweep produces a NaN or infinite step.
+    ///
+    /// `InvalidParameter` if a competitor's drift model yields a negative or
+    /// non-finite variance. Checked here, before any sweeping, so it applies to
+    /// `converge` too.
     #[must_use = "this fit may have stopped at `max_iter` — check `converged`, \
                   or bind it to `_` to say you have decided not to"]
     pub fn converge_partial(&mut self) -> Result<ConvergenceReport, InferenceError> {
diff --git a/tests/prediction_guards.rs b/tests/prediction_guards.rs
new file mode 100644
index 0000000..7eaff7e
--- /dev/null
+++ b/tests/prediction_guards.rs
@@ -0,0 +1,152 @@
+//! No prediction path may answer from a fit it cannot answer from.
+//!
+//! `converge` grew a `NonFiniteResult` guard; nothing stopped a caller from
+//! ignoring that error and predicting anyway. The three failures that produced
+//! were each differently wrong: `Ok(NaN)`, a panic out of a `Result`-returning
+//! method, and `Ok([0.0, 0.0])` — finite, plausible, summing to zero against a
+//! doc that promises one.
+//!
+//! Every test here has a healthy control, so none can pass by everything
+//! returning `Err`.
+
+use trueskill_tt::{
+    ConstantDrift, Event, Gaussian, History, InferenceError, Member, NullObserver, Outcome, Team,
+};
+
+type H = History<i64, ConstantDrift, NullObserver, &'static str>;
+
+fn build(beta: f64, prior: Option<Gaussian>, outcome: Outcome) -> H {
+    let mut h: H = History::builder()
+        .beta(beta)
+        .drift(ConstantDrift::new(0.0))
+        .build();
+
+    let member = |k: &'static str| match prior {
+        Some(p) => Member::new(k).with_prior(p),
+        None => Member::new(k),
+    };
+
+    let _ = h.add_events(vec![Event {
+        time: 1,
+        teams: [
+            Team::with_members([member("a")]),
+            Team::with_members([member("b")]),
+        ]
+        .into_iter()
+        .collect(),
+        outcome,
+    }]);
+    h
+}
+
+/// Point-mass priors with `beta(0.0)` on a *ranked* event: `converge` reports
+/// `NonFiniteResult` and the stored posteriors are `pi: NaN, tau: NaN`.
+fn nan_poisoned() -> H {
+    let mut h = build(
+        0.0,
+        Some(Gaussian::from_ms(0.0, 0.0)),
+        Outcome::winner(0, 2),
+    );
+    let err = h.converge().expect_err("this fixture must not converge");
+    assert!(
+        matches!(err, InferenceError::NonFiniteResult { .. }),
+        "{err:?}"
+    );
+    h
+}
+
+/// The same degenerate parameters on a *scored* event, where inference
+/// converges cleanly and leaves legitimate point-mass posteriors behind. The
+/// fit is fine; it is prediction that has nothing to work with.
+fn degenerate_but_converged() -> H {
+    let mut h = build(
+        0.0,
+        Some(Gaussian::from_ms(0.0, 0.0)),
+        Outcome::scores([1.0, 0.0]),
+    );
+    h.converge().expect("this fixture converges");
+    h
+}
+
+fn healthy() -> H {
+    let mut h = build(1.0, None, Outcome::winner(0, 2));
+    h.converge().expect("control converges");
+    h
+}
+
+macro_rules! all_predictions {
+    ($h:ident, $f:expr) => {{
+        let teams: &[&[&&'static str]] = &[&[&"a"], &[&"b"]];
+        let f = $f;
+        f("predict_quality", $h.predict_quality(teams).map(|_| ()));
+        f(
+            "predict_win_probabilities",
+            $h.predict_win_probabilities(teams).map(|_| ()),
+        );
+        f("predict_outcome", $h.predict_outcome(teams).map(|_| ()));
+        f(
+            "predict_ranking",
+            $h.predict_ranking(teams, &[0, 1]).map(|_| ()),
+        );
+        f(
+            "expected_information_gain",
+            $h.expected_information_gain(teams).map(|_| ()),
+        );
+    }};
+}
+
+#[test]
+fn a_nan_poisoned_fit_is_refused_by_every_prediction_path() {
+    let h = nan_poisoned();
+
+    all_predictions!(h, |name: &str, r: Result<(), InferenceError>| {
+        match r {
+            Err(InferenceError::NonFiniteResult { .. }) => {}
+            other => panic!("{name} answered from a NaN fit: {other:?}"),
+        }
+    });
+}
+
+#[test]
+fn degenerate_performances_are_refused_rather_than_answered_wrongly() {
+    let h = degenerate_but_converged();
+
+    // The fit itself is sound — the posteriors are point masses, not NaN.
+    let skill = h.current_skill("a").expect("a played");
+    assert_eq!(skill.sigma(), 0.0);
+    assert!(skill.mu().is_finite());
+
+    // `predict_quality` previously PANICKED here, out of a method that returns
+    // `Result`: the contrast covariance is exactly singular when beta is zero
+    // and every skill is a point mass.
+    all_predictions!(h, |name: &str, r: Result<(), InferenceError>| {
+        match r {
+            Err(InferenceError::InvalidParameter { .. }) => {}
+            other => panic!("{name} predicted from a degenerate fit: {other:?}"),
+        }
+    });
+}
+
+#[test]
+fn the_control_history_answers_every_prediction() {
+    let h = healthy();
+
+    all_predictions!(h, |name: &str, r: Result<(), InferenceError>| {
+        assert!(r.is_ok(), "{name} failed on a healthy history: {r:?}");
+    });
+}
+
+#[test]
+fn win_probabilities_sum_to_one_on_the_control() {
+    // The promise the `Ok([0.0, 0.0])` case broke. Asserted on the control so
+    // the guard above cannot be "fixed" by making every path error.
+    let h = healthy();
+    let p = h
+        .predict_win_probabilities(&[&[&"a"], &[&"b"]])
+        .expect("control predicts");
+    let total: f64 = p.iter().sum();
+    assert!(
+        (total - 1.0).abs() < 1e-6,
+        "win probabilities sum to {total}"
+    );
+}