5 Commits
Author SHA1 Message Date
logaritmisk 7da2328692 chore: Release trueskill-tt version 0.8.0 2026-09-08 21:18:53 +02:00
logaritmiskandClaude Opus 5 a73afa5f24 Merge branch 'fix/game-boundary'
Reject malformed games at the Game entry point, which does not pass
through History's ingestion chokepoint.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-08 21:13:02 +02:00
logaritmiskandClaude Opus 5 eebf8aacd3 fix!: reject malformed games at the Game boundary too
I fixed this at `History`'s ingestion chokepoint and said the boundary
was complete. It was not. `Game` is a separate public entry point that
does not pass through that chokepoint, and every one of the same four
defects was still live there:

  Game::ranked(&[&[a]], ..)  -> PANIC at src/game.rs:317
  Game::scored(&[&[a]], ..)  -> PANIC at src/game.rs:317
  Game::ranked(&[&[], &[a]]) -> Ok, finite posterior for the opponent
  Game::scored(.., [NaN, 1]) -> Ok

The same panic, from safe API, in release. Fixing one path and
generalising from it is exactly the mistake that produced the
latest-slice joint bug: validating on the shape that cannot expose the
problem, then reporting the property as held.

`Game::validate_teams` is shared by `ranked` and `scored`, with the
non-finite score check in `scored` alongside it. Ranks need no equivalent
— they are `u32`.

`one_v_one` and `free_for_all` build their teams internally and are
unaffected; a test asserts all three well-formed constructors still
succeed, so the check cannot quietly widen.

BREAKING CHANGE: `Game::ranked` and `Game::scored` return
`NotEnoughTeams`, `EmptyTeam` or `InvalidParameter` for inputs they
previously panicked on or silently accepted.

Refs #18, #26

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-08 21:13:02 +02:00
logaritmiskandClaude Opus 5 4e9aa6bdc1 Merge branch 'test/close-coverage-gaps'
Cover non-finite results and color-group disjointness, closing the two
test gaps #26 named.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-08 21:09:25 +02:00
logaritmiskandClaude Opus 5 a18df521eb test: cover non-finite results and color-group disjointness
The two gaps #26 named that were never filled.

NonFiniteResult had no test at all — the name appeared in `tests/` only
inside a doc comment, and it is the sub-claim in that issue's title. It
turns out to be very much reachable, and from *finite* inputs: sigma at
1e300, beta at 1e300, sigma at 1e-300, score_sigma at 1e-300, and scores
at 1e308 all overflow inside inference, where the boundary checks cannot
see them. That matters because the failure is silent by default — NaN
fails every comparison, so a naive `step < epsilon` reads a NaN step as
converged, which is why the crate has `step_converged`/`step_is_finite`.
Pinned from outside, including that `converge_partial` does not launder a
breakdown into an `Ok`, and with a control asserting merely extreme
parameters still converge so the suite cannot pass by always failing.

Color-group disjointness was #26's fourth acceptance criterion and had
only five hand-written cases. Now a proptest over three shapes: a dense
pool where collisions force colors to multiply, a sparse one where most
events are independent, and repeated members within a single event.

Two of my first assertions were wrong about the code rather than the
reverse. A competitor named twice *within* one event is not a collision —
`color_greedy` collects each event's members into a set for that reason.
And contiguity is not a property of `color_greedy`: it holds only after
`recompute_color_groups` reorders events so each color occupies one
range. The test now asserts what is actually promised — that the reorder
is always *possible*, since the parallel sweep slices `&mut` sub-ranges
from those groups and overlapping ranges would be unsound.

Refs #26

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-08 21:09:25 +02:00
6 changed files with 426 additions and 1 deletions
+39
View File
@@ -2,12 +2,51 @@
All notable changes to this project will be documented in this file. All notable changes to this project will be documented in this file.
## 0.8.0 - 2026-09-08
### Breaking Changes
- feat!: make a short fit an error and raise the default iteration cap
- feat!: validate mu, sigma and beta on HistoryBuilder
- feat!: add History::register and History::rating, and reject config conflicts across batches
- fix!: reject non-finite weights at ingestion
- fix!: reject malformed games at the Game boundary too
### Bug Fixes
- fix: reject malformed events at the ingestion boundary
### Documentation
- docs: record the rayon opt-in deviation in spec section 6
- docs: state what the joint's cost actually scales in
### Features
- feat: add EventBuilder::members for per-member configuration
### Other (unconventional)
- Merge branch 'fix/ingestion-shape'
- Merge branch 'feat/convergence-strictness'
- Merge branch 'fix/non-finite-weights'
- Merge branch 'test/close-coverage-gaps'
- Merge branch 'fix/game-boundary'
### Testing
- test: cover non-finite results and color-group disjointness
## 0.7.0 - 2026-09-08 ## 0.7.0 - 2026-09-08
### Features ### Features
- feat: factorise the joint once with History::joint - feat: factorise the joint once with History::joint
### Miscellaneous Tasks
- chore: Release trueskill-tt version 0.7.0
### Other (unconventional) ### Other (unconventional)
- Merge branch 'feat/joint-handle' - Merge branch 'feat/joint-handle'
+1 -1
View File
@@ -1,6 +1,6 @@
[package] [package]
name = "trueskill-tt" name = "trueskill-tt"
version = "0.7.0" version = "0.8.0"
edition = "2024" edition = "2024"
rust-version = "1.85" rust-version = "1.85"
description = "TrueSkill Through Time: Bayesian skill rating that tracks how skill evolves over time, via Gaussian message passing" description = "TrueSkill Through Time: Bayesian skill rating that tracks how skill evolves over time, via Gaussian message passing"
+118
View File
@@ -191,3 +191,121 @@ mod tests {
assert_eq!(cg.total_events(), 4); assert_eq!(cg.total_events(), 4);
} }
} }
#[cfg(test)]
mod properties {
use std::collections::HashSet;
use proptest::prelude::*;
use super::*;
/// The property the whole parallel sweep rests on: two events sharing a
/// competitor must never land in the same color, because a color group is
/// run concurrently and two events touching one competitor would race.
///
/// Hand-written cases cover the shapes someone thought of. This covers the
/// ones nobody did — the correctness of `sweep_color_groups` depends on it
/// holding for every input, not for five.
fn check(events: &[Vec<usize>]) {
let groups = color_greedy(events.len(), |ev| {
events[ev]
.iter()
.copied()
.map(Index::from)
.collect::<Vec<_>>()
});
// Disjointness *between events* within a color. Deduplicated per
// event, because one event legitimately naming a competitor twice is
// not a collision — `color_greedy` collects each event's members into
// a set for exactly that reason.
for color in 0..groups.n_colors() {
let mut seen: HashSet<usize> = HashSet::new();
for &ev in &groups.groups[color] {
let members: HashSet<usize> = events[ev].iter().copied().collect();
for competitor in members {
assert!(
seen.insert(competitor),
"competitor {competitor} shared by two events in color {color}"
);
}
}
}
// Every event is assigned exactly once. Without this, a partition that
// dropped events would satisfy disjointness trivially.
let mut assigned: Vec<usize> = groups.groups.iter().flatten().copied().collect();
assigned.sort_unstable();
assert_eq!(assigned, (0..events.len()).collect::<Vec<_>>());
assert_eq!(groups.total_events(), events.len());
// No empty colors: one would waste a sweep and make `n_colors`
// misleading.
for (color, group) in groups.groups.iter().enumerate() {
assert!(!group.is_empty(), "color {color} is empty");
}
// Contiguity is not a property of `color_greedy` — it holds only after
// `recompute_color_groups` reorders the events so each color occupies
// one range. What must always hold is that the reorder is *possible*:
// relabelling events in group order yields contiguous groups. The
// parallel sweep slices `&mut` sub-ranges from those, so if this ever
// failed the reorder would produce overlapping ranges.
let mut next = 0usize;
let relabelled: Vec<Vec<usize>> = groups
.groups
.iter()
.map(|group| {
group
.iter()
.map(|_| {
let i = next;
next += 1;
i
})
.collect()
})
.collect();
assert!(ColorGroups { groups: relabelled }.groups_are_contiguous());
}
proptest! {
#![proptest_config(ProptestConfig::with_cases(512))]
/// Small competitor pool, so collisions are common and colors are
/// forced to multiply.
#[test]
fn colors_are_disjoint_on_a_dense_pool(
events in prop::collection::vec(
prop::collection::vec(0usize..6, 1..4),
0..20,
)
) {
check(&events);
}
/// Wide pool, so most events are independent and land in one color.
#[test]
fn colors_are_disjoint_on_a_sparse_pool(
events in prop::collection::vec(
prop::collection::vec(0usize..200, 1..6),
0..30,
)
) {
check(&events);
}
/// Repeated competitors within one event must not confuse the
/// member-set bookkeeping.
#[test]
fn colors_are_disjoint_with_repeated_members(
events in prop::collection::vec(
prop::collection::vec(0usize..3, 1..8),
0..15,
)
) {
check(&events);
}
}
}
+39
View File
@@ -431,6 +431,29 @@ impl<'a, T: Time, D: Drift<T>> Game<'a, T, D> {
} }
impl<T: Time, D: Drift<T>> Game<'_, T, D> { impl<T: Time, D: Drift<T>> Game<'_, T, D> {
/// Reject the team shapes inference cannot represent.
///
/// `run_chain` builds one diff link per adjacent pair of teams, so fewer
/// than two teams leaves it indexing `links[1..]` on an empty vector — a
/// panic, in release, from safe API. An empty team is the quiet half: it
/// contributes no performance, so a malformed game returns a finite,
/// plausible-looking posterior for whoever it was matched against.
///
/// `History` validates the same two things at its own ingestion
/// chokepoint. `Game` is a separate public entry point that does not pass
/// through it, so it needs its own check rather than inheriting one.
fn validate_teams(teams: &[&[Rating<T, D>]]) -> Result<(), crate::InferenceError> {
if teams.len() < 2 {
return Err(crate::InferenceError::NotEnoughTeams { got: teams.len() });
}
for (team, members) in teams.iter().enumerate() {
if members.is_empty() {
return Err(crate::InferenceError::EmptyTeam { team });
}
}
Ok(())
}
/// # Errors /// # Errors
/// ///
/// - `InvalidParameter` if `options.convergence` is out of range — an /// - `InvalidParameter` if `options.convergence` is out of range — an
@@ -442,12 +465,15 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
/// - `TieWithoutDrawProbability` if the outcome ties two teams while /// - `TieWithoutDrawProbability` if the outcome ties two teams while
/// `p_draw` is zero: the truncation margin is then zero and the two-sided /// `p_draw` is zero: the truncation margin is then zero and the two-sided
/// tie update evaluates `0/0`. /// tie update evaluates `0/0`.
/// - `NotEnoughTeams` for fewer than two teams, and `EmptyTeam` for a team
/// with no members.
pub fn ranked( pub fn ranked(
teams: &[&[Rating<T, D>]], teams: &[&[Rating<T, D>]],
outcome: crate::Outcome, outcome: crate::Outcome,
options: &GameOptions, options: &GameOptions,
) -> Result<OwnedGame<T, D>, crate::InferenceError> { ) -> Result<OwnedGame<T, D>, crate::InferenceError> {
options.convergence.validate()?; options.convergence.validate()?;
Self::validate_teams(teams)?;
if !(0.0..1.0).contains(&options.p_draw) { if !(0.0..1.0).contains(&options.p_draw) {
return Err(crate::InferenceError::InvalidProbability { return Err(crate::InferenceError::InvalidProbability {
value: options.p_draw, value: options.p_draw,
@@ -499,12 +525,15 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
/// or is NaN, or if `options.convergence` is out of range. /// or is NaN, or if `options.convergence` is out of range.
/// - `MismatchedShape` if the outcome's score count differs from `teams.len()`. /// - `MismatchedShape` if the outcome's score count differs from `teams.len()`.
/// - `WrongOutcomeKind` if `outcome` is not `Outcome::Scored`. /// - `WrongOutcomeKind` if `outcome` is not `Outcome::Scored`.
/// - `NotEnoughTeams` for fewer than two teams, `EmptyTeam` for a team with
/// no members, and `InvalidParameter` for a non-finite score.
pub fn scored( pub fn scored(
teams: &[&[Rating<T, D>]], teams: &[&[Rating<T, D>]],
outcome: crate::Outcome, outcome: crate::Outcome,
options: &GameOptions, options: &GameOptions,
) -> Result<OwnedGame<T, D>, crate::InferenceError> { ) -> Result<OwnedGame<T, D>, crate::InferenceError> {
options.convergence.validate()?; options.convergence.validate()?;
Self::validate_teams(teams)?;
if options.score_sigma <= 0.0 || options.score_sigma.is_nan() { if options.score_sigma <= 0.0 || options.score_sigma.is_nan() {
return Err(crate::InferenceError::InvalidParameter { return Err(crate::InferenceError::InvalidParameter {
name: "score_sigma", name: "score_sigma",
@@ -526,6 +555,16 @@ impl<T: Time, D: Drift<T>> Game<'_, T, D> {
got: "Outcome::Ranked", got: "Outcome::Ranked",
})? })?
.to_vec(); .to_vec();
// A non-finite score poisons the chain rather than failing it. Ranks
// need no equivalent: they are `u32`.
for value in &scores {
if !value.is_finite() {
return Err(crate::InferenceError::InvalidParameter {
name: "score",
value: *value,
});
}
}
let teams_owned: Vec<Vec<Rating<T, D>>> = teams.iter().map(|t| t.to_vec()).collect(); let teams_owned: Vec<Vec<Rating<T, D>>> = teams.iter().map(|t| t.to_vec()).collect();
let weights: Vec<Vec<f64>> = teams.iter().map(|t| vec![1.0; t.len()]).collect(); let weights: Vec<Vec<f64>> = teams.iter().map(|t| vec![1.0; t.len()]).collect();
Ok(OwnedGame::new_scored( Ok(OwnedGame::new_scored(
+112
View File
@@ -138,3 +138,115 @@ fn one_v_one_honours_convergence_options() {
let (a_post, _) = Game::<i64, _>::one_v_one(&a, &b, Outcome::winner(0, 2), &options).unwrap(); let (a_post, _) = Game::<i64, _>::one_v_one(&a, &b, Outcome::winner(0, 2), &options).unwrap();
assert!(a_post.mu() > 25.0); assert!(a_post.mu() > 25.0);
} }
/// `Game` is a public entry point that does not pass through `History`'s
/// ingestion chokepoint, so it needs its own boundary — and did not have one.
///
/// A one-team game panicked at `src/game.rs:317` with "range start index 1 out
/// of range for slice of length 0", in release, from safe API. This is the
/// same defect `tests/ingestion_shape.rs` covers for `History`; fixing that
/// path left this one open, because they share no validation.
mod malformed_games {
use super::*;
#[test]
fn a_one_team_ranked_game_is_an_error_not_a_panic() {
let a = default_rating();
let err = Game::<i64, _>::ranked(&[&[a]], Outcome::winner(0, 1), &GameOptions::default())
.unwrap_err();
assert!(
matches!(err, InferenceError::NotEnoughTeams { got: 1 }),
"{err:?}"
);
}
#[test]
fn a_one_team_scored_game_is_an_error_not_a_panic() {
let a = default_rating();
let err = Game::<i64, _>::scored(
&[&[a]],
Outcome::scores([1.0]),
&GameOptions {
score_sigma: 1.0,
..GameOptions::default()
},
)
.unwrap_err();
assert!(
matches!(err, InferenceError::NotEnoughTeams { got: 1 }),
"{err:?}"
);
}
#[test]
fn a_zero_team_game_is_an_error() {
let err =
Game::<i64, ConstantDrift>::ranked(&[], Outcome::ranking([]), &GameOptions::default())
.unwrap_err();
assert!(
matches!(err, InferenceError::NotEnoughTeams { got: 0 }),
"{err:?}"
);
}
/// The quiet half: an empty team contributed no performance, so the game
/// returned a finite posterior for its opponent as though it had won one.
#[test]
fn an_empty_team_is_an_error() {
let a = default_rating();
let err =
Game::<i64, _>::ranked(&[&[], &[a]], Outcome::winner(0, 2), &GameOptions::default())
.unwrap_err();
assert!(
matches!(err, InferenceError::EmptyTeam { team: 0 }),
"{err:?}"
);
}
#[test]
fn a_non_finite_score_is_an_error() {
let a = default_rating();
for bad in [f64::NAN, f64::INFINITY, f64::NEG_INFINITY] {
let err = Game::<i64, _>::scored(
&[&[a], &[a]],
Outcome::scores([bad, 1.0]),
&GameOptions {
score_sigma: 1.0,
..GameOptions::default()
},
)
.unwrap_err();
assert!(
matches!(err, InferenceError::InvalidParameter { name: "score", .. }),
"{bad}: {err:?}"
);
}
}
/// `free_for_all` and `one_v_one` build their teams internally, so they
/// must keep working — the check must not catch well-formed games.
#[test]
fn well_formed_games_are_untouched() {
let a = default_rating();
assert!(
Game::<i64, _>::ranked(
&[&[a], &[a]],
Outcome::winner(0, 2),
&GameOptions::default()
)
.is_ok()
);
assert!(
Game::<i64, _>::free_for_all(
&[&a, &a, &a],
Outcome::ranking([0, 1, 2]),
&GameOptions::default()
)
.is_ok()
);
assert!(
Game::<i64, _>::one_v_one(&a, &a, Outcome::winner(0, 2), &GameOptions::default())
.is_ok()
);
}
}
+117
View File
@@ -0,0 +1,117 @@
//! Inference must report numerical breakdown rather than call it convergence.
//!
//! The boundary rejects inputs that are *not numbers*, but finite inputs can
//! still overflow during inference — `beta.powi(2)` at 1e300 is infinite, and
//! infinity minus infinity is NaN. `NonFiniteResult` is the guard for that, and
//! it matters because the alternative is silent: NaN fails every comparison, so
//! a naive `step < epsilon` check reads a NaN step as *converged*.
//!
//! That is why the crate has `step_converged` / `step_is_finite` rather than
//! `!tuple_gt(..)`. These tests pin the guard from outside.
use smallvec::smallvec;
use trueskill_tt::{Event, Gaussian, History, InferenceError, Member, Outcome, Team};
fn scored_fit(
sigma: f64,
beta: f64,
score_sigma: f64,
scores: [f64; 2],
) -> Result<bool, InferenceError> {
let mut h = History::builder()
.mu(0.0)
.sigma(sigma)
.beta(beta)
.score_sigma(score_sigma)
.build();
h.add_events(vec![Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a")]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::scores(scores),
}])?;
h.converge().map(|r| r.converged)
}
/// Every one of these is built from finite, individually legal parameters. The
/// overflow happens inside inference, which is exactly the case the boundary
/// checks cannot catch.
///
/// Matched rather than merely `is_err()`: an assertion that only checks "some
/// error" would keep passing if these started failing at the boundary for an
/// unrelated reason, and would then be testing nothing.
#[test]
fn overflow_during_inference_is_reported_not_hidden() {
let cases: [(&str, f64, f64, f64, [f64; 2]); 5] = [
("huge sigma", 1e300, 1.0, 1.0, [3.0, 1.0]),
("huge beta", 6.0, 1e300, 1.0, [3.0, 1.0]),
("tiny sigma", 1e-300, 1.0, 1.0, [3.0, 1.0]),
("tiny score_sigma", 6.0, 1.0, 1e-300, [3.0, 1.0]),
("huge scores", 6.0, 1.0, 1.0, [1e308, -1e308]),
];
for (name, sigma, beta, score_sigma, scores) in cases {
match scored_fit(sigma, beta, score_sigma, scores) {
Err(InferenceError::NonFiniteResult { context, step }) => {
assert_eq!(context, "History::converge", "{name}");
assert!(
!step.0.is_finite() || !step.1.is_finite(),
"{name}: reported NonFiniteResult with a finite step {step:?}"
);
}
other => panic!("{name}: expected NonFiniteResult, got {other:?}"),
}
}
}
/// The trap the invariant exists for: NaN fails every comparison, so a naive
/// `step < epsilon` test reads a NaN step as converged. A breakdown must never
/// come back as a successful fit.
#[test]
fn a_broken_fit_is_never_reported_as_converged() {
let mut h = History::builder().build();
h.add_events(vec![Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(1e300, 1e-300))]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::winner(0, 2),
}])
.unwrap();
let err = h.converge().unwrap_err();
assert!(
matches!(err, InferenceError::NonFiniteResult { .. }),
"a breakdown must not be reported as convergence: {err:?}"
);
// `converge_partial` must not launder it into an `Ok` either — the
// permissive path is permissive about *stopping short*, not about NaN.
let mut h2 = History::builder().build();
h2.add_events(vec![Event {
time: 1i64,
teams: smallvec![
Team::with_members([Member::new("a").with_prior(Gaussian::from_ms(1e300, 1e-300))]),
Team::with_members([Member::new("b")]),
],
outcome: Outcome::winner(0, 2),
}])
.unwrap();
assert!(matches!(
h2.converge_partial().unwrap_err(),
InferenceError::NonFiniteResult { .. }
));
}
/// The neighbouring case, so the tests above cannot pass by the fit simply
/// always failing: ordinary extreme-but-workable parameters still converge.
#[test]
fn merely_extreme_parameters_still_converge() {
assert!(scored_fit(1e6, 1.0, 1.0, [3.0, 1.0]).unwrap());
assert!(scored_fit(1e-6, 1.0, 1.0, [3.0, 1.0]).unwrap());
assert!(scored_fit(6.0, 1.0, 1e6, [3.0, 1.0]).unwrap());
assert!(scored_fit(6.0, 1.0, 1.0, [1e150, -1e150]).unwrap());
}