refactor: unify convergence defaults, validate builders, clear dead code

Convergence configuration had two disagreeing sources of truth and one
misleading report:

- `EpsilonOrMax::default()` capped at 10 iterations while
  `ConvergenceOptions::default()` allowed 30, and which applied depended on
  whether inference went through `run_chain` or a `Schedule`. The schedule
  default now derives from `ConvergenceOptions`.
- A graph with no iterating factors reported `converged: false` with an
  infinite step, despite being at its fixed point after the setup pass. It
  now reports converged with a zero step.
- `TimeSlice::iterate_to_convergence` hard-coded an epsilon and a
  20-iteration cap matching neither. It reads `self.convergence` and is
  scoped to `#[cfg(test)]`, which is all it was ever used by.

`HistoryBuilder::p_draw` and `::convergence` now validate their arguments
like `score_sigma` already did, instead of accepting a negative `p_draw` or
an `alpha` of zero — the latter leaves every EP update unapplied, so
inference silently returns the priors.

Removing the `#[allow(dead_code)]` masks let the compiler report what they
were hiding: four `OwnedGame` fields that were stored and never read, two
`ColorGroups` helpers and three `SkillStore` helpers used only by tests, and
`iterate_to_convergence` above. Test-only items are now `#[cfg(test)]` and
the unread fields are gone.

Also exported `HistoryBuilder`, which was public but unreachable — callers
could chain `History::builder()` but could not name the type — and added
`Rating::{prior, beta, drift}` and `Index::get`, so handles the API hands
out can be read back.

Two goldens moved, both convergence residuals rather than exact values:
`iterate_to_convergence` now runs to 30 iterations instead of 20, landing
nearer the symmetric truth of 25.0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DnsaJg74eNSva3PJjK2eej

src/color_group.rs

@@ -26,23 +26,22 @@ pub(crate) struct ColorGroups {
 }
 
 impl ColorGroups {
-    #[allow(dead_code)]
     pub(crate) fn new() -> Self {
         Self::default()
     }
 
-    #[allow(dead_code)]
-    pub(crate) fn n_colors(&self) -> usize {
-        self.groups.len()
-    }
-
-    #[allow(dead_code)]
     pub(crate) fn is_empty(&self) -> bool {
         self.groups.is_empty()
     }
 
-    /// Total event count across all colors.
-    #[allow(dead_code)]
+    /// Number of distinct colors in the partition. Test-only.
+    #[cfg(test)]
+    pub(crate) fn n_colors(&self) -> usize {
+        self.groups.len()
+    }
+
+    /// Total event count across all colors. Test-only.
+    #[cfg(test)]
     pub(crate) fn total_events(&self) -> usize {
         self.groups.iter().map(|g| g.len()).sum()
     }

src/game.rs

@@ -88,16 +88,12 @@ impl Default for GameOptions {
 /// Owned variant of `Game` returned by public constructors.
 ///
 /// Unlike `Game<'a, T, D>` (which borrows its result/weights slices from
-/// History's internal state), `OwnedGame<T, D>` owns its inputs so it can
-/// be returned freely from public constructors.
+/// History's internal state), `OwnedGame<T, D>` owns the team ratings, so it
+/// can be returned freely from public constructors. The inference inputs
+/// themselves are not retained — nothing reads them back.
 #[derive(Debug)]
-#[allow(dead_code)]
 pub struct OwnedGame<T: Time, D: Drift<T>> {
     teams: Vec<Vec<Rating<T, D>>>,
-    result: Vec<f64>,
-    weights: Vec<Vec<f64>>,
-    p_draw: f64,
-    pub(crate) convergence: crate::ConvergenceOptions,
     pub(crate) likelihoods: Vec<Vec<Gaussian>>,
     pub(crate) log_evidence: f64,
 }
@@ -119,16 +115,10 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
             convergence,
             &mut arena,
         );
-        let likelihoods = g.likelihoods;
-        let log_evidence = g.log_evidence;
         Self {
             teams,
-            result,
-            weights,
-            p_draw,
-            convergence,
-            likelihoods,
-            log_evidence,
+            likelihoods: g.likelihoods,
+            log_evidence: g.log_evidence,
         }
     }
 
@@ -148,16 +138,10 @@ impl<T: Time, D: Drift<T>> OwnedGame<T, D> {
             convergence,
             &mut arena,
         );
-        let likelihoods = g.likelihoods;
-        let log_evidence = g.log_evidence;
         Self {
             teams,
-            result: scores,
-            weights,
-            p_draw: 0.0,
-            convergence,
-            likelihoods,
-            log_evidence,
+            likelihoods: g.likelihoods,
+            log_evidence: g.log_evidence,
         }
     }
 

src/history.rs

@@ -69,7 +69,20 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
         }
     }
 
+    /// Probability that two evenly-matched sides draw.
+    ///
+    /// Must be in `[0.0, 1.0)`. A zero draw probability asserts that draws
+    /// cannot occur, so ingesting a tied outcome then fails with
+    /// `InferenceError::TieWithoutDrawProbability`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `p_draw` is outside `[0.0, 1.0)` or is NaN.
     pub fn p_draw(mut self, p_draw: f64) -> Self {
+        assert!(
+            (0.0..1.0).contains(&p_draw),
+            "p_draw must be in [0.0, 1.0) (got {p_draw})"
+        );
         self.p_draw = p_draw;
         self
     }
@@ -79,6 +92,11 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
         self
     }
 
+    /// Default observation noise for scored outcomes.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `score_sigma` is not strictly positive.
     pub fn score_sigma(mut self, score_sigma: f64) -> Self {
         assert!(
             score_sigma > 0.0,
@@ -88,7 +106,24 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> HistoryBuilder<
         self
     }
 
+    /// Convergence tolerance, iteration cap, and EP damping.
+    ///
+    /// # Panics
+    ///
+    /// Panics if `alpha` is outside `(0.0, 1.0]`, or if `epsilon` is negative
+    /// or NaN. An `alpha` of zero would leave every EP update unapplied, so
+    /// inference would silently return the priors.
     pub fn convergence(mut self, opts: ConvergenceOptions) -> Self {
+        assert!(
+            opts.alpha > 0.0 && opts.alpha <= 1.0,
+            "convergence alpha must be in (0.0, 1.0] (got {})",
+            opts.alpha
+        );
+        assert!(
+            opts.epsilon >= 0.0,
+            "convergence epsilon must be non-negative (got {})",
+            opts.epsilon
+        );
         self.convergence = opts;
         self
     }

src/lib.rs

@@ -39,7 +39,7 @@ pub use event::{Event, Member, Team};
 pub use event_builder::EventBuilder;
 pub use game::{Game, GameOptions, OwnedGame};
 pub use gaussian::Gaussian;
-pub use history::History;
+pub use history::{History, HistoryBuilder};
 pub use key_table::KeyTable;
 use matrix::Matrix;
 pub use observer::{NullObserver, Observer};
@@ -65,12 +65,29 @@ pub const N_INF: Gaussian = Gaussian::from_ms(0.0, f64::INFINITY);
 #[derive(Copy, Clone, Default, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
 pub struct Index(usize);
 
+impl Index {
+    /// The underlying slot number.
+    ///
+    /// Indices are dense and assigned in interning order, so this is usable as
+    /// a key into a caller-side side table.
+    #[must_use]
+    pub fn get(self) -> usize {
+        self.0
+    }
+}
+
 impl From<usize> for Index {
     fn from(ix: usize) -> Self {
         Self(ix)
     }
 }
 
+impl From<Index> for usize {
+    fn from(idx: Index) -> Self {
+        idx.0
+    }
+}
+
 fn erfc(x: f64) -> f64 {
     let z = x.abs();
     let t = 1.0 / (1.0 + z / 2.0);

src/rating.rs

@@ -29,6 +29,24 @@ impl<T: Time, D: Drift<T>> Rating<T, D> {
         }
     }
 
+    /// The configured prior skill estimate.
+    #[must_use]
+    pub fn prior(&self) -> Gaussian {
+        self.prior
+    }
+
+    /// Performance noise: how much a single showing varies around the skill.
+    #[must_use]
+    pub fn beta(&self) -> f64 {
+        self.beta
+    }
+
+    /// The drift model governing how skill may move between events.
+    #[must_use]
+    pub fn drift(&self) -> D {
+        self.drift
+    }
+
     pub(crate) fn performance(&self) -> Gaussian {
         self.prior.forget(self.beta.powi(2))
     }

src/schedule.rs

@@ -32,8 +32,17 @@ pub struct EpsilonOrMax {
 
 impl Default for EpsilonOrMax {
     fn default() -> Self {
-        // Matches today's hard-coded tolerance and iteration cap.
-        Self { eps: 1e-6, max: 10 }
+        // Derived from `ConvergenceOptions` so there is one source of truth for
+        // the tolerance and iteration cap. These previously disagreed: this
+        // default capped at 10 iterations while `ConvergenceOptions` allowed 30,
+        // and which applied depended on whether inference went through
+        // `run_chain` or a `Schedule`.
+        let defaults = crate::ConvergenceOptions::default();
+
+        Self {
+            eps: defaults.epsilon,
+            max: defaults.max_iter,
+        }
     }
 }
 
@@ -50,10 +59,16 @@ impl Schedule for EpsilonOrMax {
         }
 
         let mut iterations = 0;
-        let mut final_step = (f64::INFINITY, f64::INFINITY);
-        let mut converged = false;
+        // With no iterating factors the graph is already at its fixed point:
+        // the setup pass above is all there is to do. Reporting `converged:
+        // false` with an infinite step for that case gave callers a false
+        // negative.
+        let mut final_step = (0.0, 0.0);
+        let mut converged = true;
 
         if n_setup < factors.len() {
+            final_step = (f64::INFINITY, f64::INFINITY);
+            converged = false;
             for _ in 0..self.max {
                 let mut step = (0.0_f64, 0.0_f64);
 
@@ -113,7 +128,8 @@ mod tests {
 
     #[test]
     fn report_marks_converged_when_no_iterating_factors() {
-        // No iterating factors → 0 iterations, converged stays false (loop never ran).
+        // A graph of only setup factors has nothing to iterate, so it is at its
+        // fixed point after the setup pass: 0 iterations, and converged.
         let mut vars = VarStore::new();
         let out = vars.alloc(N_INF);
         let mut factors = vec![BuiltinFactor::TeamSum(TeamSumFactor {
@@ -122,5 +138,15 @@ mod tests {
         })];
         let report = EpsilonOrMax::default().run(&mut factors, &mut vars);
         assert_eq!(report.iterations, 0);
+        assert!(report.converged);
+        assert_eq!(report.final_step, (0.0, 0.0));
+    }
+
+    #[test]
+    fn default_matches_convergence_options() {
+        let schedule = EpsilonOrMax::default();
+        let options = crate::ConvergenceOptions::default();
+        assert_eq!(schedule.max, options.max_iter);
+        assert_eq!(schedule.eps, options.epsilon);
     }
 }

src/storage/skill_store.rs

@@ -41,6 +41,18 @@ impl SkillStore {
         }
     }
 
+    /// Whether a slot is occupied. Test-only.
+    #[cfg(test)]
+    pub fn contains(&self, idx: Index) -> bool {
+        idx.0 < self.present.len() && self.present[idx.0]
+    }
+
+    /// Number of occupied slots. Test-only.
+    #[cfg(test)]
+    pub fn len(&self) -> usize {
+        self.n_present
+    }
+
     pub fn get_mut(&mut self, idx: Index) -> Option<&mut Skill> {
         if idx.0 < self.present.len() && self.present[idx.0] {
             Some(&mut self.skills[idx.0])
@@ -49,21 +61,6 @@ impl SkillStore {
         }
     }
 
-    #[allow(dead_code)]
-    pub fn contains(&self, idx: Index) -> bool {
-        idx.0 < self.present.len() && self.present[idx.0]
-    }
-
-    #[allow(dead_code)]
-    pub fn len(&self) -> usize {
-        self.n_present
-    }
-
-    #[allow(dead_code)]
-    pub fn is_empty(&self) -> bool {
-        self.n_present == 0
-    }
-
     pub fn iter(&self) -> impl Iterator<Item = (Index, &Skill)> {
         self.present.iter().enumerate().filter_map(|(i, &p)| {
             if p {

src/time_slice.rs

@@ -14,7 +14,6 @@ use crate::{
     rating::Rating,
     storage::{CompetitorStore, SkillStore},
     time::Time,
-    tuple_gt, tuple_max,
 };
 
 #[derive(Debug)]
@@ -504,18 +503,29 @@ impl<T: Time> TimeSlice<T> {
         }
     }
 
-    #[allow(dead_code)]
+    /// Iterate this slice alone until its posteriors stop moving, returning
+    /// the number of iterations taken.
+    ///
+    /// Only used by tests: production convergence is driven across slices by
+    /// `History::converge`.
+    ///
+    /// Honours `self.convergence`; it previously hard-coded an epsilon and a
+    /// 20-iteration cap that matched neither `ConvergenceOptions` nor the
+    /// schedule default.
+    #[cfg(test)]
     pub(crate) fn iterate_to_convergence<D: Drift<T>>(
         &mut self,
         agents: &CompetitorStore<T, D>,
     ) -> usize {
-        let epsilon = 1e-6;
-        let iterations = 20;
+        use crate::{tuple_gt, tuple_max};
+
+        let epsilon = self.convergence.epsilon;
+        let max_iter = self.convergence.max_iter;
 
         let mut step = (f64::INFINITY, f64::INFINITY);
         let mut i = 0;
 
-        while tuple_gt(step, epsilon) && i < iterations {
+        while tuple_gt(step, epsilon) && i < max_iter {
             let old = self.posteriors();
 
             self.iteration(0, agents);
@@ -527,6 +537,10 @@ impl<T: Time> TimeSlice<T> {
             });
 
             i += 1;
+
+            if !crate::step_is_finite(step) {
+                break;
+            }
         }
 
         i
@@ -918,19 +932,24 @@ mod tests {
 
         let post = time_slice.posteriors();
 
+        // These are convergence residuals, not exact values: by symmetry the
+        // true mean is 25.0 and the iteration approaches it from above. The
+        // previous expectation of 25.000003 was the residual after the
+        // hard-coded 20-iteration cap; honouring `ConvergenceOptions` runs to
+        // 30 and lands nearer the truth.
         assert_ulps_eq!(
             post[&a],
-            Gaussian::from_ms(25.000003, 3.880150),
+            Gaussian::from_ms(25.000001, 3.880150),
             epsilon = 1e-6
         );
         assert_ulps_eq!(
             post[&b],
-            Gaussian::from_ms(25.000003, 3.880150),
+            Gaussian::from_ms(25.000001, 3.880150),
             epsilon = 1e-6
         );
         assert_ulps_eq!(
             post[&c],
-            Gaussian::from_ms(25.000003, 3.880150),
+            Gaussian::from_ms(25.000001, 3.880150),
             epsilon = 1e-6
         );
     }