From 87fca8dcca807adc1073dea14a8c1c8f1ec52138 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Tue, 1 Sep 2026 19:30:32 +0200 Subject: [PATCH] docs: correct drifted documentation and compile the README in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four README code blocks no longer compiled: `Player` was renamed `Rating` in T2, the `Drift` trait gained a `T: Time` parameter and a second method, and two blocks were missing imports outright. The `Rating` example needed more than a rename — with the binding unused, `T` is ambiguous because `ConstantDrift` implements `Drift` for every `T`, so it now carries an explicit annotation. Nothing compiled those blocks. `src/lib.rs` gains a `cfg(doctest)` struct carrying `#[doc = include_str!("../README.md")]`, which turns every `rust` block into a doctest without displacing the curated crate docs as the front page. Verified it bites: reintroducing `Player` fails the build with E0432 rather than shipping. Illustrative blocks are fenced `text` — note that a bare fence defaults to `rust` under rustdoc, which is how the `variance_delta = elapsed * γ²` formula became a compile error. Prose fixes: README claimed `Gaussian::forget` takes a square root (it works in variance space) and pointed at a `.gamma()` builder method that does not exist. CLAUDE.md's data-flow diagram spliced the public ingestion shape into the internal one — `Team` is not in that chain — listed `cdf()`/`erfc()` as public when they are `pub(crate)` and private, and called `SkillStore` public when only `CompetitorStore` escapes the crate. Rustdoc fixes: `EventBuilder::scores_with_sigma` claimed a debug-assert that `Outcome::scores_with_sigma` never had and whose own docs contradict; rejection happens at ingestion as `InvalidParameter`. `event.rs` described `add_events_with_prior` as replaced when it is still the ingestion chokepoint. `factors.rs` advertised `Game::custom` without noting it is `#[doc(hidden)]`. Internal T2/T4 milestone labels are dropped from public items; the ones in the private `time_slice` module are left alone. Closes #35 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014b6wy2q8rnFK8U8GPJVQNU --- CLAUDE.md | 34 ++++++++++----- README.md | 102 ++++++++++++++++++++++++++++++------------- src/competitor.rs | 4 +- src/event.rs | 8 ++-- src/event_builder.rs | 5 ++- src/factors.rs | 9 ++-- src/history.rs | 4 +- src/lib.rs | 13 ++++++ src/rating.rs | 4 +- 9 files changed, 130 insertions(+), 53 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6d2c3b5..4771cde 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,10 +32,19 @@ evidence both forward and backward across a history. ### Data flow +Ingestion (public types, `event.rs`): + ``` -History → TimeSlice[] → Event[] → Team[] → Item[] - ↓ - Game (factor graph) → Schedule → BuiltinFactor[] +Event → Team[] → Member[] +``` + +`History::add_events` flattens that into indices; teams survive only as +grouping, not as a value. Inference then runs on the internal shapes: + +``` +History → TimeSlice[] → Event[] → Item[] + ↓ + Game (factor graph) → Schedule → BuiltinFactor[] ``` - **`History`** (`history.rs`) — top level. Interns keys, groups events into @@ -45,9 +54,12 @@ History → TimeSlice[] → Event[] → Team[] → Item[] - **`TimeSlice`** (`time_slice.rs`) — all events at one time. Owns a `SkillStore` and a `ScratchArena`; `iteration()` sweeps its events, using `ColorGroups` to partition independent ones. -- **`Event`** (`time_slice.rs`) — one match. `compute()` runs inference reading - skills immutably; `apply()` folds the result back. The split is what lets a - color group run in parallel with no `unsafe`. +- **`Event`** — two distinct types, do not confuse them. The *public* ingestion + `Event` is in `event.rs` (with `Team`/`Member`); the *internal* + `pub(crate) Event` in `time_slice.rs` is one match during inference, where + `compute()` runs inference reading skills immutably and `apply()` folds the + result back. That split is what lets a color group run in parallel with no + `unsafe`. - **`Game`** (`game.rs`) — a single match's factor graph. `run_chain` builds the diff chain between rank-adjacent teams and drives it to convergence. - **`Gaussian`** (`gaussian.rs`) — natural parameters (`pi = 1/sigma²`, @@ -61,14 +73,16 @@ History → TimeSlice[] → Event[] → Team[] → Item[] the only implementation. - **`Competitor`** (`competitor.rs`) — per-history temporal state (`message`, `last_time`). **`Rating`** (`rating.rs`) — static config (prior, `beta`, drift). -- **`storage/`** — `SkillStore` (per slice) and `CompetitorStore` (per history), - both dense `Vec`s indexed by `Index`. +- **`storage/`** — `SkillStore` (per slice, `pub(crate)`) and `CompetitorStore` + (per history, public), both indexed by `Index`. The module is `pub`, but only + `CompetitorStore` is reachable from outside the crate. - **`KeyTable`** (`key_table.rs`) — user key ↔ `Index`, both directions O(1). - **`Drift`** (`drift.rs`) / **`Time`** (`time.rs`) — traits. `Time` is a *trait* (`i64`, `Untimed`), not an enum. - **`lib.rs`** — public exports, global defaults (`MU`, `SIGMA`, `BETA`, - `GAMMA`, `P_DRAW`, `EPSILON`, `ITERATIONS`), and the standalone `quality()`, - `cdf()`, `erfc()`. + `GAMMA`, `P_DRAW`, `EPSILON`, `ITERATIONS`), and the standalone `quality()`. + The `cdf()` / `erfc()` helpers live here too but are `pub(crate)` and private + respectively — not public API. ### Invariants worth knowing diff --git a/README.md b/README.md index f43d224..cf62dd9 100644 --- a/README.md +++ b/README.md @@ -13,63 +13,92 @@ Rust port of [TrueSkillThroughTime.py](https://github.com/glandfried/TrueSkillTh ## Drift -Skill drift models how a player's true skill can change between appearances. Each time a player reappears after a gap, their skill uncertainty is widened by the drift model before the new evidence is incorporated. +Skill drift models how a competitor's true skill can change between appearances. +Each time they reappear after a gap, their skill uncertainty is widened by the +drift model before the new evidence is incorporated. -Drift is represented by the `Drift` trait: +Drift is represented by the `Drift` trait (`src/drift.rs`), generic over the +history's time type: -```rust -pub trait Drift: Copy + Debug { - fn variance_delta(&self, elapsed: i64) -> f64; +```text +pub trait Drift: Copy + Debug + Send + Sync { + fn variance_delta(&self, from: &T, to: &T) -> f64; + fn variance_for_elapsed(&self, elapsed: i64) -> f64; } ``` -`variance_delta` returns the amount to add to `σ²` given the elapsed time since the player last played. Internally, `Gaussian::forget` uses this to compute the new sigma: `σ_new = sqrt(σ² + variance_delta)`. +Both methods return the amount to add to `σ²`, not to `σ`. `variance_delta` +works from two timestamps; `variance_for_elapsed` takes an already-computed +elapsed count, and is used on the paths that cache it. `Gaussian::forget` +applies the result entirely in variance space — `from_mv(mu, variance() + +variance_delta)` — taking no square root. + +That block is a quotation rather than a doctest. The custom-drift example below +is compiled by CI, so it is what actually pins the signature. ### ConstantDrift -The built-in `ConstantDrift` implements a linear random walk — skill uncertainty grows proportionally to time: +The built-in `ConstantDrift` implements a linear random walk — skill uncertainty +grows proportionally to time: -``` +```text variance_delta = elapsed * γ² ``` -This is the standard TrueSkill Through Time model. Use it by passing a `ConstantDrift(gamma)` when constructing a `Player`: +This is the standard TrueSkill Through Time model. Pass a `ConstantDrift(gamma)` +when constructing a `Rating`: ```rust -use trueskill_tt::{Player, Gaussian, drift::ConstantDrift}; +use trueskill_tt::{ConstantDrift, Gaussian, Rating}; -// gamma = 0.1 means skill can shift ~0.1 per time unit -let player = Player::new(Gaussian::from_ms(0.0, 6.0), 1.0, ConstantDrift(0.1)); +// gamma = 0.1 means skill can shift ~0.1 per time unit. +let rating: Rating = + Rating::new(Gaussian::from_ms(0.0, 6.0), 1.0, ConstantDrift(0.1)); + +assert_eq!(rating.drift().0, 0.1); ``` +The type annotation is load-bearing: `ConstantDrift` implements `Drift` for +every `T: Time`, so without it `T` is ambiguous. + ### Custom drift -Implement `Drift` to express any other model. For example, a drift that saturates after a long absence (uncertainty grows with the square root of elapsed time instead of linearly): +Implement `Drift` to express any other model. For example, a drift that +saturates after a long absence, with uncertainty growing as the square root of +elapsed time instead of linearly: ```rust -use trueskill_tt::drift::Drift; +use trueskill_tt::{Drift, Gaussian, History, Rating, Time}; #[derive(Clone, Copy, Debug)] struct SqrtDrift { gamma: f64, } -impl Drift for SqrtDrift { - fn variance_delta(&self, elapsed: i64) -> f64 { - (elapsed as f64).sqrt() * self.gamma * self.gamma +impl Drift for SqrtDrift { + fn variance_delta(&self, from: &T, to: &T) -> f64 { + let elapsed = from.elapsed_to(to).max(0) as f64; + elapsed.sqrt() * self.gamma * self.gamma + } + + fn variance_for_elapsed(&self, elapsed: i64) -> f64 { + (elapsed.max(0) as f64).sqrt() * self.gamma * self.gamma } } -let player = Player::new(Gaussian::from_ms(0.0, 6.0), 1.0, SqrtDrift { gamma: 0.5 }); +// On a single Rating: +let rating: Rating = + Rating::new(Gaussian::from_ms(0.0, 6.0), 1.0, SqrtDrift { gamma: 0.5 }); + +// Or for a whole History, via the builder: +let history = History::builder().drift(SqrtDrift { gamma: 0.5 }).build(); + +assert_eq!(rating.beta(), 1.0); +assert_eq!(history.log_evidence(), 0.0); ``` -To use a custom drift type with `History`, use the `.drift()` builder method instead of `.gamma()`: - -```rust -let h = History::builder() - .drift(SqrtDrift { gamma: 0.5 }) - .build(); -``` +`HistoryBuilder::drift` is the only way to set a history's drift model; there is +no `gamma()` shorthand. The default is `ConstantDrift(GAMMA)`. ### Per-competitor drift @@ -84,16 +113,25 @@ expressible in the same graph as moving competitors — a bot at a known strength, a rating floor, a course difficulty: ```rust -let events = vec![Event { +use trueskill_tt::{ConstantDrift, Event, History, Member, Outcome, Team}; + +let mut h = History::builder().drift(ConstantDrift(0.1)).build(); + +h.add_events(vec![Event { time: 0, - teams: smallvec![ + teams: [ Team::with_members([Member::new("player")]), // A course does not improve. Pin it, and the round's evidence // lands on the player instead of being split between the two. Team::with_members([Member::new("layout_7").with_drift_scale(0.0)]), - ], + ] + .into_iter() + .collect(), outcome: Outcome::winner(0, 2), -}]; +}]) +.unwrap(); + +h.converge().unwrap(); ``` Like `with_prior`, the scale is **competitor configuration captured at first @@ -101,6 +139,10 @@ appearance** — setting it on a key the history already knows has no effect. It must be finite and non-negative; ingestion otherwise fails with `InferenceError::InvalidParameter`. +Note that the fluent `EventBuilder` (`h.event(t).team([...])`) sets weights but +not `drift_scale` or `prior`; those need the typed `Event` / `Team` / `Member` +shape shown above. + ## Scored outcomes Use `Outcome::scores([...])` when you have continuous per-team scores rather @@ -110,7 +152,7 @@ soft Gaussian evidence about the latent performance diff. Configure (smaller σ = more trust). ```rust -use trueskill_tt::{History, Outcome}; +use trueskill_tt::History; let mut h = History::builder().score_sigma(2.0).build(); h.event(1) diff --git a/src/competitor.rs b/src/competitor.rs index 3b7a4a3..97d3f35 100644 --- a/src/competitor.rs +++ b/src/competitor.rs @@ -7,8 +7,8 @@ use crate::{ /// Per-history, temporal state for someone competing. /// -/// Renamed from `Agent` in T2; the former `.player` field is now -/// `.rating` to match the `Player → Rating` rename. +/// The mutable half of a competitor: `Rating` holds their static +/// configuration, this holds what inference learns as it sweeps. #[derive(Debug)] pub struct Competitor = ConstantDrift> { pub rating: Rating, diff --git a/src/event.rs b/src/event.rs index 1faad02..56a2393 100644 --- a/src/event.rs +++ b/src/event.rs @@ -1,8 +1,10 @@ //! Typed event description for bulk ingestion. //! -//! `Event` is the new public event shape (spec Section 4). Replaces -//! the nested `Vec>>`, `Vec>`, `Vec>>` -//! that the old `add_events_with_prior` took. +//! `Event` is the public event shape taken by `History::add_events`. It +//! is a typed front end, not a replacement: `add_events` flattens it into the +//! nested `Vec>>` / `Vec>` / `Vec>>` that +//! the internal `add_events_with_prior` chokepoint still takes, and which +//! `record_winner` and `record_draw` also route through. use smallvec::SmallVec; diff --git a/src/event_builder.rs b/src/event_builder.rs index ae8687a..01fca05 100644 --- a/src/event_builder.rs +++ b/src/event_builder.rs @@ -107,7 +107,10 @@ where /// Set explicit per-team continuous scores with a per-event noise override. /// /// `sigma` overrides `HistoryBuilder::score_sigma` for this event only. - /// Must be `> 0.0`; debug-asserts otherwise via `Outcome::scores_with_sigma`. + /// Must be `> 0.0`. Constructing the outcome with a non-positive or NaN + /// sigma is allowed; the value is rejected with + /// `InferenceError::InvalidParameter` when the event is ingested, so + /// callers get an error from `commit` rather than a panic. pub fn scores_with_sigma>(mut self, scores: I, sigma: f64) -> Self { self.event.outcome = crate::Outcome::scores_with_sigma(scores, sigma); self diff --git a/src/factors.rs b/src/factors.rs index 05a3d40..4a945e7 100644 --- a/src/factors.rs +++ b/src/factors.rs @@ -1,8 +1,11 @@ //! Factor-graph public API. //! -//! Power users can construct custom factor graphs via `Game::custom` (T2 -//! minimal; full ergonomics in T4) and drive them with custom `Schedule` -//! implementations. +//! The factor types, `VarStore` and the `Schedule` trait are public so custom +//! schedules can be written against them. +//! +//! Building a factor graph by hand goes through `Game::custom`, which is +//! deliberately `#[doc(hidden)]`: it works, but its signature is not yet +//! considered stable API and so is not listed in these docs. pub use crate::{ factor::{ diff --git a/src/history.rs b/src/history.rs index 965cf33..2f34251 100644 --- a/src/history.rs +++ b/src/history.rs @@ -554,13 +554,13 @@ impl, O: Observer, K: Eq + Hash + Clone> History Vec { - assert_eq!(teams.len(), 2, "predict_outcome T2: 2 teams only"); + assert_eq!(teams.len(), 2, "predict_outcome supports exactly 2 teams"); let gather = |team: &[&K]| -> Gaussian { team.iter() .filter_map(|k| self.keys.get(*k)) diff --git a/src/lib.rs b/src/lib.rs index 5813d0f..6ddfaac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -86,6 +86,19 @@ #![forbid(unsafe_code)] +/// Compiles every `rust` block in `README.md` as a doctest. +/// +/// The README is not the crate's front page — the module docs above are — so it +/// is pulled in here rather than via a crate-level `#![doc = ...]`, purely so +/// its examples are type-checked. Without this nothing compiled them, and they +/// had drifted far enough that four blocks no longer built (#35). `cfg(doctest)` +/// means this type exists only while collecting doctests. +/// +/// Blocks that are illustrative rather than runnable are fenced as `text`. +#[cfg(doctest)] +#[doc = include_str!("../README.md")] +pub struct ReadmeDoctests; + use std::{ cmp::Reverse, f64::consts::{FRAC_1_SQRT_2, FRAC_2_SQRT_PI, SQRT_2}, diff --git a/src/rating.rs b/src/rating.rs index a755426..f589dc8 100644 --- a/src/rating.rs +++ b/src/rating.rs @@ -9,8 +9,8 @@ use crate::{ /// Static rating configuration: prior skill, performance noise `beta`, drift. /// -/// Renamed from `Player` in T2; `Rating` better describes the data -/// (a configuration) vs. a person (who's a `Competitor` with state). +/// A configuration rather than a person: the per-history temporal state +/// (messages, last appearance) lives on `Competitor`. #[derive(Clone, Copy, Debug)] pub struct Rating = ConstantDrift> { pub(crate) prior: Gaussian,