Files
logaritmiskandClaude Opus 5 c12bc830a5 feat!: name the unknown key, expose tail probabilities, flag short fits
Three issues from two downstream consumers, all small, all sharing a
theme: the crate had the information and would not hand it over.

#44 — `UnknownKey { team: 0, member: 0 }` did not say which key. A
consumer upgrading 0.1.2 -> 0.4.1 had every one of 5591 predictions
return this error, fell back to a neutral 0.5, and lost its entire
metadata model for a day. Nothing crashed and nothing logged; it was
found by sweeping an unrelated parameter and noticing the output did not
move. The 0.4.0 change that made unknown keys an error was right — the
error was just too anonymous to act on. It now carries the key's `Debug`
rendering, and its `Display` says what to do about it. The precondition
is documented on every prediction entry point, which the reporter said
would alone have saved the day.

#43 — `cdf` was `pub(crate)`, so a consumer asking "is this competitor
below the cutoff" approximated it with a `mu + z * sigma` band and had no
way to say what confidence any `z` bought. Adds
`Gaussian::probability_below` / `probability_above`. The second is
separate on purpose: `1 - cdf` collapses to exactly zero past ~8.3
sigma, and a stopping rule is evaluated precisely there. Both route
through the survival function added in 0.4.1, so this is visibility
rather than new numerics.

#50 — `ConvergenceReport` was not `#[must_use]`, so the one signal that
a fit stopped short was trivially discarded. It now is, and that
immediately found 78 sites doing exactly that — including this crate's
own ATP example, which was capped at 10 sweeps when the history needs
30. The example now reads the report and says so.

`ITERATIONS = 30` is documented as the floor it is, with the three
measurements to hand: 400 events over 100 competitors already stops
there at ~7e-3 against a 1e-6 tolerance, the ATP example needs 30 at a
much looser one, and a consumer's 2000-node model needs 76 to 161.

BREAKING CHANGE: `InferenceError::UnknownKey` gains a `key` field, and
the prediction methods now require `K: Debug` in order to fill it.

Closes #43, #50. Refs #44 — its third ask, an opt-in `UnknownKeys::Skip`
mode, is a live API question and deliberately not answered here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-07 23:41:03 +02:00

162 lines
5.4 KiB
Rust

//! `Observer` callbacks must actually fire.
//!
//! `on_slice_processed` (formerly `on_batch_processed`) was declared on the
//! trait and never called from anywhere, so implementors wired up a callback
//! that could not run. These tests exist so that cannot silently recur.
use std::sync::{Arc, Mutex};
use trueskill_tt::{History, Observer};
/// Plain fields. `Arc<O>` implements `Observer`, so the caller shares the
/// observer itself rather than wrapping each field in its own `Arc`.
#[derive(Default)]
struct Recorder {
iterations: Mutex<Vec<usize>>,
slices: Mutex<Vec<(i64, usize, usize)>>,
converged: Mutex<Vec<(usize, bool)>>,
}
impl Observer<i64> for Recorder {
fn on_iteration_end(&self, iter: usize, _max_step: (f64, f64)) {
self.iterations.lock().unwrap().push(iter);
}
fn on_slice_processed(&self, time: &i64, slice_idx: usize, n_events: usize) {
self.slices
.lock()
.unwrap()
.push((*time, slice_idx, n_events));
}
fn on_converged(&self, iters: usize, _final_step: (f64, f64), converged: bool) {
self.converged.lock().unwrap().push((iters, converged));
}
}
#[test]
fn every_observer_callback_fires() {
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 1).unwrap();
h.record_winner(&"b", &"c", 2).unwrap();
h.record_winner(&"c", &"a", 3).unwrap();
let _ = h.converge().unwrap();
assert!(
!recorder.iterations.lock().unwrap().is_empty(),
"on_iteration_end never fired"
);
assert!(
!recorder.converged.lock().unwrap().is_empty(),
"on_converged never fired"
);
assert!(
!recorder.slices.lock().unwrap().is_empty(),
"on_slice_processed never fired — the defect this test exists for"
);
}
#[test]
fn slice_callbacks_report_the_slice_they_swept() {
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 10).unwrap();
h.record_winner(&"a", &"b", 20).unwrap();
let _ = h.converge().unwrap();
let slices = recorder.slices.lock().unwrap();
// Only the times actually in the history, and each with its own events.
for &(time, idx, events) in slices.iter() {
assert!(time == 10 || time == 20, "unexpected slice time {time}");
assert!(idx < 2, "slice index {idx} out of range");
assert_eq!(events, 1, "each slice holds exactly one event");
}
// Both slices must be reported, not just one end of the sweep.
assert!(
slices.iter().any(|&(t, ..)| t == 10),
"slice 10 never reported"
);
assert!(
slices.iter().any(|&(t, ..)| t == 20),
"slice 20 never reported"
);
}
#[test]
fn a_single_slice_history_still_reports_its_sweep() {
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 1).unwrap();
let _ = h.converge().unwrap();
let slices = recorder.slices.lock().unwrap();
assert!(
!slices.is_empty(),
"the single-slice path must report its sweep too"
);
assert!(slices.iter().all(|&(t, idx, _)| t == 1 && idx == 0));
}
/// The gap #40 closed: without `impl Observer for Arc<O>`, an observer that
/// accumulates anything had to wrap every field in its own `Arc` and derive
/// `Clone`, because `History` consumes the observer and never hands it back.
#[test]
fn a_shared_observer_reaches_the_callers_handle() {
let recorder = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&recorder)).build();
h.record_winner(&"a", &"b", 1).unwrap();
let _ = h.converge().unwrap();
assert!(!recorder.iterations.lock().unwrap().is_empty());
assert!(!recorder.slices.lock().unwrap().is_empty());
assert!(!recorder.converged.lock().unwrap().is_empty());
}
/// `?Sized` on the blanket impls means the observer can be chosen at runtime.
#[test]
fn a_trait_object_observer_works() {
let boxed: Box<dyn Observer<i64>> = Box::new(Recorder::default());
let mut h = History::builder().observer(boxed).build();
h.record_winner(&"a", &"b", 1).unwrap();
let _ = h.converge().unwrap();
let shared: Arc<dyn Observer<i64>> = Arc::new(Recorder::default());
let mut h = History::builder().observer(Arc::clone(&shared)).build();
h.record_winner(&"a", &"b", 1).unwrap();
let _ = h.converge().unwrap();
}
/// A non-shared observer can be reclaimed after convergence instead.
#[test]
fn into_observer_returns_the_accumulated_state() {
let mut h = History::builder().observer(Recorder::default()).build();
h.record_winner(&"a", &"b", 1).unwrap();
let _ = h.converge().unwrap();
// Readable in place...
assert!(!h.observer().iterations.lock().unwrap().is_empty());
// ...and reclaimable by value.
let recorder = h.into_observer();
assert!(!recorder.slices.lock().unwrap().is_empty());
}
/// Borrowing works too, for an observer that outlives the history.
#[test]
fn a_borrowed_observer_works() {
let recorder = Recorder::default();
{
let mut h = History::builder().observer(&recorder).build();
h.record_winner(&"a", &"b", 1).unwrap();
let _ = h.converge().unwrap();
}
assert!(!recorder.iterations.lock().unwrap().is_empty());
}