# T2 — New API Surface Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Land the full breaking API redesign from `docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md` Section 7 "T2" — renames, typed `Event` ingestion, generic `Time` / `Drift`, `Observer` + `ConvergenceReport`, `Result<_, InferenceError>` at the boundary, and `pub factors` module — without changing numerical behavior. **Architecture:** T2 is a tiered rename + API rewrite on top of T1's factor-graph internals. No internal inference algorithm changes. Each task leaves the crate building and tests passing on the working branch; the final commit merges as a single breaking change. Because this crate is pre-1.0 with no downstream users, intermediate commits on the T2 branch need not preserve the old API, as long as the branch-tip is consistent. **Tech Stack:** Rust 2024 edition, `approx` crate for float comparisons, `smallvec` for fixed-size-biased event shapes, `criterion` for benchmark gates. Builds on T0 (nat-param Gaussian, dense storage, ScratchArena) and T1 (Factor/Schedule/VarStore, EpsilonOrMax schedule). ## Acceptance criteria - All existing numerical goldens (rewritten in the new API) pass within `1e-6` tolerance. - `cargo bench --bench batch` shows no regression vs T1 (`Batch::iteration` ≤ 23.5 µs on Apple M5 Pro) — the API rewrite is not supposed to change the hot path. - `cargo clippy --all-targets --features approx -- -D warnings` clean. - `cargo +nightly fmt --check` clean. - Public API matches spec Section 4: - `History>` with `Untimed` and `i64` `Time` impls. - `HistoryBuilder` with `.mu().sigma().beta().drift().p_draw().convergence().observer().build()`. - `history.record_winner(&K, &K, T)`, `record_draw(&K, &K, T)`, `add_events(iter)`, `event(time).team([…]).weights([…]).ranking([…]).commit()`. - `history.converge() -> Result`. - `history.learning_curve(&K)`, `learning_curves()`, `current_skill(&K)`, `log_evidence()`, `log_evidence_for(&[&K])`, `predict_quality(...)`, `predict_outcome(...)`, `intern(&K) -> Index`, `lookup(&Q) -> Option`. - `Game::ranked`, `Game::one_v_one`, `Game::free_for_all`, `Game::custom` constructors; `Game::posteriors`, `Game::log_evidence` accessors. - `Observer` trait with `NullObserver` default. - `InferenceError` with `MismatchedShape`, `InvalidProbability`, `ConvergenceFailed`, `NegativePrecision` variants. - `factors` module re-exports `Factor`, `Schedule`, `VarStore`, `VarId`, `BuiltinFactor`, `EpsilonOrMax`, `ScheduleReport`. - Renames completed: `Batch → TimeSlice`, `Player → Rating`, `Agent → Competitor`, `IndexMap → KeyTable`. - Old API (`History::convergence(iters, eps, verbose)`, nested-Vec `add_events(composition, results, times, weights)`, `verbose: bool`, `time: bool`) removed. ## Non-goals (deferred to T3/T4) - `Outcome::Scored` variant + `MarginFactor` (T4). For T2, `Outcome` has only `Ranked`; enum is marked `#[non_exhaustive]` so we can add `Scored` later non-breaking. - `Damped` and `Residual` schedules (T4). - `Send + Sync` trait bounds + Rayon parallelism (T3). - Cross-history parallelism, dirty-bit slice skipping (T3/beyond). - Snapshot / serde support. ## File map **New files:** | Path | Responsibility | |---|---| | `src/time.rs` | `Time` trait, `Untimed` ZST, `impl Time for i64`. | | `src/observer.rs` | `Observer` trait, `NullObserver` ZST. | | `src/outcome.rs` | `Outcome` enum (only `Ranked` in T2; `#[non_exhaustive]`). Convenience constructors `winner`, `draw`, `ranking`. | | `src/event.rs` | Typed `Event`, `Team`, `Member` structs for bulk ingestion. | | `src/convergence.rs` | `ConvergenceOptions`, `ConvergenceReport`. | | `src/key_table.rs` | `KeyTable` (was `IndexMap` in `lib.rs`). | | `src/rating.rs` | `Rating` (was `Player`). | | `src/competitor.rs` | `Competitor` (was `Agent`). | | `src/time_slice.rs` | `TimeSlice` (was `Batch`). | | `src/factors.rs` | Public re-export module: `pub use crate::factor::*` + `pub use crate::schedule::*`. | | `src/event_builder.rs` | Fluent builder returned by `History::event(T)` — `.team([…]).weights([…]).ranking([…]).commit()`. | | `tests/equivalence.rs` | Integration tests: every old-API hardcoded golden is reproduced in the new API. | | `tests/api_shape.rs` | Integration tests: three-tier ingestion, builder ergonomics, error cases. | **Removed files:** - `src/player.rs` → replaced by `rating.rs` - `src/agent.rs` → replaced by `competitor.rs` - `src/batch.rs` → replaced by `time_slice.rs` **Heavily modified:** | Path | What changes | |---|---| | `src/lib.rs` | Remove `IndexMap` (moved); re-export new modules; promote `factor` + `schedule` visibility via `mod factors`. | | `src/drift.rs` | Generify `Drift` over `T: Time`. Widen `ConstantDrift` impl. | | `src/history.rs` | Make generic over `T: Time, D: Drift`. New builder methods. Three-tier ingestion. `converge()` replaces `convergence()`. New query methods. | | `src/error.rs` | Add `MismatchedShape`, `InvalidProbability`, `ConvergenceFailed` variants. | | `src/game.rs` | Add `ranked`, `one_v_one`, `free_for_all`, `custom` constructors and `log_evidence()` accessor. Keep the private `new_with_arena` path for History. | | `src/storage/mod.rs` | Rename `AgentStore` → `CompetitorStore`, `SkillStore` stays. | | `src/arena.rs` | Rename any `Player` → `Rating` references; no structural change. | ## Renaming policy Use `git mv` for file renames so history is preserved. All type-name find-replaces happen in the same commit as the file move. Tests inside `#[cfg(test)] mod tests` are updated in-place. --- ## Task 1: Pre-flight — verify T1 green, capture baseline **Files:** none - [ ] **Step 1: Confirm branch + clean tree** ```bash git status git rev-parse --abbrev-ref HEAD ``` Expected: clean tree on `t0-numerical-parity`. If dirty, stop. - [ ] **Step 2: Create the T2 branch** ```bash git checkout -b t2-new-api-surface ``` - [ ] **Step 3: Confirm all tests pass** ```bash cargo test --features approx --lib cargo test --features approx --doc 2>&1 | tail -5 ``` Expected: `53 passed; 0 failed`. - [ ] **Step 4: Capture a fresh T1 reference bench number (hardware may differ)** ```bash cargo bench --bench batch 2>&1 | grep "Batch::iteration" ``` Record the value — the final task will compare against it. - [ ] **Step 5: No commit** — verification only. --- ## Task 2: Rename `IndexMap` → `KeyTable` **Files:** - Create: `src/key_table.rs` - Modify: `src/lib.rs` (remove the inline `IndexMap`, re-export `KeyTable`) - Modify: every call site across `src/**/*.rs` and tests - [ ] **Step 1: Create `src/key_table.rs`** ```rust use std::{ borrow::{Borrow, ToOwned}, collections::HashMap, hash::Hash, }; use crate::Index; /// Maps user keys to internal `Index` handles. /// /// Renamed from the former `IndexMap` to avoid colliding with the `indexmap` /// crate. Power users can promote `&K` to `Index` via `get_or_create` and /// skip the lookup on subsequent hot-path calls. #[derive(Debug)] pub struct KeyTable(HashMap); impl KeyTable where K: Eq + Hash, { pub fn new() -> Self { Self(HashMap::new()) } pub fn get>(&self, k: &Q) -> Option where K: Borrow, { self.0.get(k).cloned() } pub fn get_or_create>(&mut self, k: &Q) -> Index where K: Borrow, { if let Some(idx) = self.0.get(k) { *idx } else { let idx = Index::from(self.0.len()); self.0.insert(k.to_owned(), idx); idx } } pub fn key(&self, idx: Index) -> Option<&K> { self.0 .iter() .find(|&(_, value)| *value == idx) .map(|(key, _)| key) } pub fn keys(&self) -> impl Iterator { self.0.keys() } pub fn len(&self) -> usize { self.0.len() } pub fn is_empty(&self) -> bool { self.0.is_empty() } } impl Default for KeyTable where K: Eq + Hash, { fn default() -> Self { KeyTable::new() } } ``` - [ ] **Step 2: Remove `IndexMap` from `src/lib.rs`** Delete lines 57–108 (the `IndexMap` struct, impls, and `Default` impl). Remove `use std::{borrow::{Borrow, ToOwned}, collections::HashMap, hash::Hash};` if nothing else uses those imports. Add module declaration near the other `pub mod` lines (alphabetically between `gaussian` and `player`): ```rust pub mod key_table; ``` Add re-export: ```rust pub use key_table::KeyTable; ``` - [ ] **Step 3: Replace all `IndexMap` references with `KeyTable`** ```bash grep -rln 'IndexMap' src/ tests/ benches/ examples/ 2>/dev/null ``` In every file found, replace `IndexMap` → `KeyTable`. Verify no stragglers: ```bash ! grep -rn 'IndexMap' src/ tests/ benches/ examples/ 2>/dev/null ``` - [ ] **Step 4: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` Expected: all 53 tests pass. - [ ] **Step 5: Format + commit** ```bash cargo +nightly fmt git add src/ tests/ benches/ examples/ git commit -m "$(cat <<'EOF' refactor(api): rename IndexMap to KeyTable The former name collided with the popular indexmap crate. KeyTable lives in its own module. Public API unchanged beyond the rename. Part of T2 of docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md. EOF )" ``` --- ## Task 3: Rename `Player` → `Rating` **Files:** - Rename: `src/player.rs` → `src/rating.rs` - Modify: `src/lib.rs` (module decl + re-export) - Modify: every call site - [ ] **Step 1: Move the file** ```bash git mv src/player.rs src/rating.rs ``` - [ ] **Step 2: Rename the type inside `src/rating.rs`** Edit `src/rating.rs` — rename `Player` → `Rating` everywhere, update the doc comment to say what it is now: ```rust use crate::{ BETA, GAMMA, drift::{ConstantDrift, Drift}, gaussian::Gaussian, }; /// 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). #[derive(Clone, Copy, Debug)] pub struct Rating { pub(crate) prior: Gaussian, pub(crate) beta: f64, pub(crate) drift: D, } impl Rating { pub fn new(prior: Gaussian, beta: f64, drift: D) -> Self { Self { prior, beta, drift } } pub(crate) fn performance(&self) -> Gaussian { self.prior.forget(self.beta.powi(2)) } } impl Default for Rating { fn default() -> Self { Self { prior: Gaussian::default(), beta: BETA, drift: ConstantDrift(GAMMA), } } } ``` - [ ] **Step 3: Update `src/lib.rs`** Replace: ```rust pub mod player; ... pub use player::Player; ``` with: ```rust pub mod rating; ... pub use rating::Rating; ``` - [ ] **Step 4: Replace `Player` with `Rating` and `player::` with `rating::` everywhere** Use two passes so we don't touch e.g. `player_b` or `players` — use word-boundary regex: ```bash grep -rln '\bPlayer\b\|::player::\|mod player\|crate::player' src/ tests/ benches/ examples/ 2>/dev/null ``` For each file, edit to replace. Inside test modules, `Player::new(...)` becomes `Rating::new(...)`. The spec keeps the `Rating::new(prior, beta, drift)` signature exactly. Note: `crate::player::Player` → `crate::rating::Rating`. Use `pub(crate)` fields are re-paths only. Verify no stragglers: ```bash ! grep -rn '\bPlayer\b' src/ tests/ benches/ examples/ 2>/dev/null ! grep -rn 'crate::player\|mod player' src/ 2>/dev/null ``` - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` Expected: all 53 tests pass. - [ ] **Step 6: Format + commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' refactor(api): rename Player to Rating The struct holds prior/beta/drift — a rating configuration, not a person. The person-with-temporal-state is the Competitor (renamed in the next task). Resolves Player/Agent ambiguity. Part of T2. EOF )" ``` --- ## Task 4: Rename `Agent` → `Competitor` **Files:** - Rename: `src/agent.rs` → `src/competitor.rs` - Modify: `src/lib.rs`, `src/storage/mod.rs` (the `AgentStore` alias rename), every call site - [ ] **Step 1: Move the file** ```bash git mv src/agent.rs src/competitor.rs ``` - [ ] **Step 2: Rename the type inside `src/competitor.rs`** Replace `Agent` with `Competitor` everywhere in the file. Update the `rating` import (which was `player`): ```rust use crate::{ N_INF, drift::{ConstantDrift, Drift}, gaussian::Gaussian, rating::Rating, }; /// Per-history, temporal state for someone competing. /// /// Renamed from `Agent` in T2. #[derive(Debug)] pub struct Competitor { pub rating: Rating, pub message: Gaussian, pub last_time: i64, } impl Competitor { pub(crate) fn receive(&self, elapsed: i64) -> Gaussian { if self.message != N_INF { self.message .forget(self.rating.drift.variance_delta(elapsed)) } else { self.rating.prior } } } impl Default for Competitor { fn default() -> Self { Self { rating: Rating::default(), message: N_INF, last_time: i64::MIN, } } } pub(crate) fn clean<'a, D: Drift + 'a, A: Iterator>>( competitors: A, last_time: bool, ) { for c in competitors { c.message = N_INF; if last_time { c.last_time = i64::MIN; } } } ``` Note: the field `player` → `rating` is an additional rename that must propagate (`self.player.X` → `self.rating.X` across the codebase). - [ ] **Step 3: Update `src/lib.rs` and `src/storage/mod.rs`** In `src/lib.rs`, replace `pub mod agent;` with `pub mod competitor;`. In `src/storage/mod.rs`, rename `AgentStore` → `CompetitorStore`. Inspect the file (read it first if you haven't) and replace: - type alias/struct `AgentStore` → `CompetitorStore` - any `Agent` generic → `Competitor` - `use crate::agent::Agent` → `use crate::competitor::Competitor` - [ ] **Step 4: Replace all call sites** ```bash grep -rln '\bAgent\b\|AgentStore\|agent::\|mod agent\|crate::agent\|\.player\b' src/ tests/ benches/ examples/ 2>/dev/null ``` Replace: - `Agent` → `Competitor` - `AgentStore` → `CompetitorStore` - `crate::agent::` → `crate::competitor::` - `mod agent` → `mod competitor` - **`.player` field access** → `.rating` (search this one carefully; it's ambiguous with unrelated code in dev-deps) For the `.player` rename, audit carefully — it should only appear inside `src/` on instances of `Competitor`. Grep-replace only inside `src/`: ```bash grep -rln '\.player\b' src/ ``` Do NOT blindly s/\.player/\.rating/ across `tests/` or `benches/` or `examples/` — these may reference external dev-dep types. Only edit matches inside `src/` that are on `Competitor` instances (verify each). Verify: ```bash ! grep -rn '\bAgent\b\|AgentStore\|::agent::\|mod agent' src/ tests/ benches/ examples/ 2>/dev/null ``` - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` - [ ] **Step 6: Format + commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' refactor(api): rename Agent to Competitor and .player field to .rating Competitor holds dynamic per-history state (message, last_time) for someone competing; its configuration lives in a Rating. AgentStore renamed to CompetitorStore to match. Part of T2. EOF )" ``` --- ## Task 5: Rename `Batch` → `TimeSlice` **Files:** - Rename: `src/batch.rs` → `src/time_slice.rs` - Modify: `src/lib.rs`, `src/history.rs`, every call site - [ ] **Step 1: Move the file** ```bash git mv src/batch.rs src/time_slice.rs ``` - [ ] **Step 2: Rename the type** In `src/time_slice.rs`, replace the struct name `Batch` with `TimeSlice`. Update the module-level doc comment: ```rust //! A single time step's worth of events. //! //! Renamed from `Batch` in T2. ``` Rename: - `struct Batch` → `struct TimeSlice` - `impl Batch` → `impl TimeSlice` - `Batch::new` → `TimeSlice::new` The `compute_elapsed` free function keeps its name. - [ ] **Step 3: Update `src/lib.rs`** Replace: ```rust pub mod batch; ``` with: ```rust pub mod time_slice; ``` If `batch::Batch` was re-exported anywhere, update to `time_slice::TimeSlice`. - [ ] **Step 4: Replace all call sites** ```bash grep -rln '\bBatch\b\|crate::batch::\|mod batch\|batch::' src/ tests/ benches/ examples/ 2>/dev/null ``` Replace `Batch` → `TimeSlice` and `batch::` → `time_slice::` and `mod batch` → `mod time_slice`. Note `src/history.rs` uses the name extensively (`self.batches`, `Vec`, function names like `new_forward_info`). For T2 we rename: - `batches: Vec` → `time_slices: Vec` - The field rename must propagate — search `\.batches\b` inside `src/history.rs` and replace with `.time_slices`. Verify: ```bash ! grep -rn '\bBatch\b\|::batch::\|mod batch' src/ tests/ benches/ examples/ 2>/dev/null ! grep -rn '\.batches\b' src/history.rs ``` - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` Tests will still use the old nested-Vec `add_events` signature and `h.batches[…]` access patterns — those are fixed in later tasks. For now tests may need to be patched minimally to use `.time_slices[…]` so they compile. Since the inner shape is the same, this is pure find-replace. - [ ] **Step 6: Format + commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' refactor(api): rename Batch to TimeSlice TimeSlice says what it is: every event sharing one timestamp. The History field batches is renamed to time_slices. Part of T2. EOF )" ``` --- ## Task 6: Introduce `Time` trait and `Untimed` **Files:** - Create: `src/time.rs` - Modify: `src/lib.rs` This task lands the trait. It is *not* yet wired into `History`; that happens in Task 8. - [ ] **Step 1: Create `src/time.rs`** ```rust //! Generic time axis for `History`. //! //! Users pick the `Time` type based on their domain: `Untimed` when no //! time axis is meaningful, `i64` for integer day/second timestamps. //! Additional impls can be added behind feature flags. /// A timestamp on the global ordering axis. /// /// Must be `Ord + Copy` so slices can sort events, and `'static` so /// `History` can store it by value without lifetimes. pub trait Time: Copy + Ord + 'static { /// How much time elapsed between `self` and `later`. /// /// Used by `Drift::variance_delta` to compute skill drift. Returning /// zero means no drift accumulates between the two points. Return value /// must be non-negative for `self <= later`. fn elapsed_to(&self, later: &Self) -> i64; } /// Zero-sized type representing "no time axis." /// /// Used as the default `Time` when events are unordered. Elapsed is always 0, /// so no drift accumulates across slices. #[derive(Copy, Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Untimed; impl Time for Untimed { fn elapsed_to(&self, _later: &Self) -> i64 { 0 } } impl Time for i64 { fn elapsed_to(&self, later: &Self) -> i64 { later - self } } ``` - [ ] **Step 2: Declare the module in `src/lib.rs`** ```rust pub mod time; ``` and re-export: ```rust pub use time::{Time, Untimed}; ``` - [ ] **Step 3: Build** ```bash cargo build --features approx cargo test --features approx --lib ``` All 53 tests still pass (nothing uses `Time` yet). - [ ] **Step 4: Commit** ```bash cargo +nightly fmt git add src/time.rs src/lib.rs git commit -m "$(cat <<'EOF' feat(api): add Time trait with Untimed and i64 impls Foundation for generic History time axis. Untimed is the ZST case (no drift across slices); i64 is the standard timestamp case. Additional impls (time::OffsetDateTime, chrono) can be added behind feature flags in follow-up work. Part of T2. EOF )" ``` --- ## Task 7: Generify `Drift` over `Time` **Files:** - Modify: `src/drift.rs` - Modify: every `Drift` call site (`elapsed: i64` → `from: &T, to: &T`) - [ ] **Step 1: Rewrite `src/drift.rs`** ```rust use std::fmt::Debug; use crate::time::Time; /// Governs how much a competitor's skill can drift between two time points. /// /// Generic over `T: Time` so seasonal or calendar-aware drift is expressible /// without going through `i64`. pub trait Drift: Copy + Debug { /// Variance added to the skill prior for elapsed time `from -> to`. /// /// Called with `from <= to`; returning zero means no drift accumulates. fn variance_delta(&self, from: &T, to: &T) -> f64; } /// Simple constant-per-unit-time drift. /// /// For `Time = i64`: variance added is `(to - from) * gamma^2`. /// For `Time = Untimed`: elapsed is always 0, so drift is always 0. #[derive(Clone, Copy, Debug)] pub struct ConstantDrift(pub f64); impl Drift for ConstantDrift { fn variance_delta(&self, from: &T, to: &T) -> f64 { let elapsed = from.elapsed_to(to).max(0) as f64; elapsed * self.0 * self.0 } } ``` - [ ] **Step 2: Update `Competitor::receive` and `clean` signatures** In `src/competitor.rs`, `Competitor::receive` currently takes `elapsed: i64`. Change the generic bound so it can take `from: &T, to: &T`. Since `Competitor` is parameterized by `D: Drift`, we need to also parameterize by `T: Time`: ```rust use crate::{ N_INF, drift::{ConstantDrift, Drift}, gaussian::Gaussian, rating::Rating, time::Time, }; #[derive(Debug)] pub struct Competitor = ConstantDrift> { pub rating: Rating, pub message: Gaussian, pub last_time: Option, } impl> Competitor { pub(crate) fn receive(&self, now: &T) -> Gaussian { if self.message != N_INF { let elapsed_variance = match &self.last_time { Some(last) => self.rating.drift.variance_delta(last, now), None => 0.0, }; self.message.forget(elapsed_variance) } else { self.rating.prior } } } impl Default for Competitor { fn default() -> Self { Self { rating: Rating::default(), message: N_INF, last_time: None, } } } pub(crate) fn clean<'a, T: Time + 'a, D: Drift + 'a, C: Iterator>>( competitors: C, last_time: bool, ) { for c in competitors { c.message = N_INF; if last_time { c.last_time = None; } } } ``` The `last_time: i64` with `i64::MIN` sentinel becomes `last_time: Option` with `None` sentinel — cleaner and type-safe. - [ ] **Step 3: Parameterize `Rating` likewise** In `src/rating.rs`: ```rust use crate::{ BETA, GAMMA, drift::{ConstantDrift, Drift}, gaussian::Gaussian, time::Time, }; #[derive(Clone, Copy, Debug)] pub struct Rating = ConstantDrift> { pub(crate) prior: Gaussian, pub(crate) beta: f64, pub(crate) drift: D, pub(crate) _time: std::marker::PhantomData, } impl> Rating { pub fn new(prior: Gaussian, beta: f64, drift: D) -> Self { Self { prior, beta, drift, _time: std::marker::PhantomData, } } pub(crate) fn performance(&self) -> Gaussian { self.prior.forget(self.beta.powi(2)) } } impl Default for Rating { fn default() -> Self { Self { prior: Gaussian::default(), beta: BETA, drift: ConstantDrift(GAMMA), _time: std::marker::PhantomData, } } } ``` - [ ] **Step 4: Propagate the generic through the rest of the codebase** Every generic `` becomes `>` where the enclosing type owns `T`. Affected: - `CompetitorStore` → `CompetitorStore` in `src/storage/mod.rs` - `History` → `History` in `src/history.rs` (and every fn sig inside) - `HistoryBuilder` → `HistoryBuilder` - `Game` → `Game` in `src/game.rs` - `TimeSlice` may need `T` if it holds a `time: T` field (it currently holds `time: i64` — change to `time: T`) - `agent::clean` → already updated - `Event` (inside `time_slice.rs`) passes competitors and therefore inherits `T` This is the single biggest refactor in T2. Expect ~100 compiler errors; work through them file by file. The compiler is your friend — each error either needs a `T: Time` parameter added, or a type argument passed. For `Competitor::receive(elapsed: i64)` callers — now `Competitor::receive(now: &T)`. Each caller currently computes `elapsed = compute_elapsed(last_time, batch.time)`; change those to `competitor.receive(&time_slice.time)`. - [ ] **Step 5: Fix test modules** Tests use `i64` timestamps and `ConstantDrift` — the defaults — so most test setup is unchanged. Places that reference `.last_time == i64::MIN` become `.last_time.is_none()`. Places constructing `Competitor { last_time: i64::MIN, ... }` become `Competitor { last_time: None, ... }`. - [ ] **Step 6: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` Expected: all tests still pass, numerically identical. If numerics drift, the Drift implementation has a bug — most likely a negative elapsed was previously allowed. - [ ] **Step 7: Format + commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' refactor(api): generify Drift, Rating, Competitor, TimeSlice, History over T: Time Drift now takes &T -> &T and is generic over the time axis. Untimed impls return elapsed=0. ConstantDrift impl covers all T via the Time trait. Competitor.last_time moves from i64 with MIN sentinel to Option with None sentinel. Part of T2. EOF )" ``` --- ## Task 8: Parameterize `History` — remove `time: bool` **Files:** - Modify: `src/history.rs` The `time: bool` field is a legacy encoding of "no time axis." With `T: Time`, the type system carries that information (`T = Untimed` means no drift). This task removes the bool and fixes the `!time` internal path. - [ ] **Step 1: Remove `time: bool` from `History` and `HistoryBuilder`** In `src/history.rs`, delete the `time: bool` field from both structs. Delete the `.time(bool)` builder method. Update the `Default` impl and `build()` to stop threading `time` through. - [ ] **Step 2: Remove the `!self.time` branches in `add_events_with_prior`** Every `if self.time { … } else { … }` becomes the `self.time` branch — the generic parameter makes this uniform. Specifically, `times` is always required now (it's a `Vec`, not `Vec`), and `sort_time` sorts in `T` order. Rewrite `sort_time` in `src/lib.rs` to be generic: ```rust pub(crate) fn sort_time(xs: &[T], reverse: bool) -> Vec { let mut x: Vec<(usize, T)> = xs.iter().enumerate().map(|(i, &t)| (i, t)).collect(); if reverse { x.sort_by_key(|&(_, t)| std::cmp::Reverse(t)); } else { x.sort_by_key(|&(_, t)| t); } x.into_iter().map(|(i, _)| i).collect() } ``` - [ ] **Step 3: Keep the old nested-Vec `add_events_with_prior` signature** For now. Change its signature only enough to take `times: Vec` instead of `Vec`. Users who previously called with `!time` should switch to `History::` — but the *tests* should switch to `History::` with explicit `[1, 2, 3, ...]` timestamps to preserve the old elapsed=1-per-event numerical behavior. The nested-Vec signature gets fully replaced in Task 15. This keeps the test suite green between tasks. - [ ] **Step 4: Update tests** Tests using `.time(false)` need translation. The mechanical rule: | Old | New | |---|---| | `History::builder().time(false).build()` | `History::::builder().build()` with explicit `vec![1, 2, 3, …]` for `times` in every `add_events` call | | `History::default()` (was `time: true`) | `History::::default()` | For every test that called `add_events(comp, results, vec![], vec![])` under `time: false`, change to `add_events(comp, results, vec![1, 2, 3, …], vec![])` where the length matches `comp.len()`. Numerics are preserved because the old `!time` branch used `i as i64 + 1` timestamps internally. This is mechanical. Each failing test will show which translation is needed. - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` Expected: all tests pass with translated timestamps. - [ ] **Step 6: Format + commit** ```bash cargo +nightly fmt git add src/history.rs src/lib.rs src/time_slice.rs src/storage/ git commit -m "$(cat <<'EOF' refactor(history): parameterize History and remove time: bool The bool encoded 'no time axis' which is now expressed at the type level (T = Untimed). Removed the .time() builder method. Tests that used time(false) now use i64 timestamps 1..=n to preserve numerics. Part of T2. EOF )" ``` --- ## Task 9: Add `Outcome` enum **Files:** - Create: `src/outcome.rs` - Modify: `src/lib.rs` - [ ] **Step 1: Create `src/outcome.rs`** ```rust //! Outcome of a match. //! //! In T2, only `Ranked` is supported; `Scored` will be added together with //! `MarginFactor` in T4. The enum is `#[non_exhaustive]` so adding `Scored` //! is non-breaking for downstream `match` expressions. use smallvec::SmallVec; /// Final outcome of a match. /// /// `Ranked(ranks)`: lower rank = better. Equal ranks mean a tie between those /// teams. `ranks.len()` must equal the number of teams in the event. #[derive(Clone, Debug, PartialEq)] #[non_exhaustive] pub enum Outcome { Ranked(SmallVec<[u32; 4]>), } impl Outcome { /// `N`-team outcome where team `winner` won and everyone else tied for last. /// /// Panics if `winner >= n`. pub fn winner(winner: u32, n: u32) -> Self { assert!(winner < n, "winner index {winner} out of range 0..{n}"); let ranks: SmallVec<[u32; 4]> = (0..n) .map(|i| if i == winner { 0 } else { 1 }) .collect(); Self::Ranked(ranks) } /// All `n` teams tied. pub fn draw(n: u32) -> Self { Self::Ranked(SmallVec::from_vec(vec![0; n as usize])) } /// Explicit per-team ranking. pub fn ranking>(ranks: I) -> Self { Self::Ranked(ranks.into_iter().collect()) } pub fn team_count(&self) -> usize { match self { Self::Ranked(r) => r.len(), } } pub(crate) fn as_ranks(&self) -> &[u32] { match self { Self::Ranked(r) => r, } } } #[cfg(test)] mod tests { use super::*; #[test] fn winner_two_teams() { let o = Outcome::winner(0, 2); assert_eq!(o.as_ranks(), &[0u32, 1]); assert_eq!(o.team_count(), 2); } #[test] fn winner_three_teams_second_wins() { let o = Outcome::winner(1, 3); assert_eq!(o.as_ranks(), &[1u32, 0, 1]); } #[test] fn draw_three_teams() { let o = Outcome::draw(3); assert_eq!(o.as_ranks(), &[0u32, 0, 0]); } #[test] fn ranking_from_iter() { let o = Outcome::ranking([2, 0, 1]); assert_eq!(o.as_ranks(), &[2u32, 0, 1]); } #[test] #[should_panic(expected = "winner index 2 out of range")] fn winner_out_of_range_panics() { let _ = Outcome::winner(2, 2); } } ``` - [ ] **Step 2: Add `smallvec` as a dependency** In `Cargo.toml`, under `[dependencies]`: ```toml smallvec = "1" ``` - [ ] **Step 3: Add `pub mod outcome;` and `pub use outcome::Outcome;` to `src/lib.rs`** - [ ] **Step 4: Build + test** ```bash cargo build --features approx cargo test --features approx --lib outcome ``` Expected: 5 passing tests. - [ ] **Step 5: Format + commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): add Outcome enum with Ranked variant Outcome::winner(i, n), Outcome::draw(n), Outcome::ranking(iter) are the convenience constructors. Marked #[non_exhaustive] so Scored can be added in T4 without breaking match exhaustiveness. Adds smallvec dependency. Part of T2. EOF )" ``` --- ## Task 10: Add `Event`, `Team`, `Member` **Files:** - Create: `src/event.rs` - Modify: `src/lib.rs` - [ ] **Step 1: Create `src/event.rs`** ```rust //! 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. use smallvec::SmallVec; use crate::{gaussian::Gaussian, outcome::Outcome, time::Time}; /// A single match at time `time` involving some number of teams. #[derive(Clone, Debug)] pub struct Event { pub time: T, pub teams: SmallVec<[Team; 4]>, pub outcome: Outcome, } /// A team: list of members competing together. #[derive(Clone, Debug)] pub struct Team { pub members: SmallVec<[Member; 4]>, } impl Team { pub fn new() -> Self { Self { members: SmallVec::new() } } pub fn with_members>>(members: I) -> Self { Self { members: members.into_iter().collect() } } } impl Default for Team { fn default() -> Self { Self::new() } } /// One member of a team, identified by user key `K`. /// /// `weight` defaults to 1.0; a per-event `prior` can override the competitor's /// current skill estimate for this event only. #[derive(Clone, Debug)] pub struct Member { pub key: K, pub weight: f64, pub prior: Option, } impl Member { pub fn new(key: K) -> Self { Self { key, weight: 1.0, prior: None } } pub fn with_weight(mut self, weight: f64) -> Self { self.weight = weight; self } pub fn with_prior(mut self, prior: Gaussian) -> Self { self.prior = Some(prior); self } } /// Convenience: a member is a user key with default weight 1.0 and no prior. impl From for Member { fn from(key: K) -> Self { Self::new(key) } } ``` - [ ] **Step 2: Register in `src/lib.rs`** ```rust pub mod event; ... pub use event::{Event, Member, Team}; ``` - [ ] **Step 3: Build** ```bash cargo build --features approx cargo test --features approx --lib ``` - [ ] **Step 4: Format + commit** ```bash cargo +nightly fmt git add src/event.rs src/lib.rs git commit -m "$(cat <<'EOF' feat(api): add Event, Team, Member typed event description Replaces the old nested Vec>> event description on the public API boundary. Member::from(K) enables ergonomic literal lists. Part of T2. EOF )" ``` --- ## Task 11: Add `Observer` trait and `NullObserver` **Files:** - Create: `src/observer.rs` - Modify: `src/lib.rs` - [ ] **Step 1: Create `src/observer.rs`** ```rust //! Observer trait for progress reporting during convergence. //! //! Replaces the old `verbose: bool` + `println!` path. Callers wire in any //! observer that implements the trait; default methods are no-ops so users //! override only what they need. use crate::time::Time; /// Receives progress callbacks during `History::converge`. /// /// All methods have default no-op implementations; implement only what's /// interesting. Send/Sync is NOT required in T2 (added in T3 along with /// Rayon support). pub trait Observer { /// Called after each convergence iteration across the whole history. fn on_iteration_end(&self, _iter: usize, _max_step: (f64, f64)) {} /// Called after each time slice is processed within an iteration. fn on_batch_processed(&self, _time: &T, _slice_idx: usize, _n_events: usize) {} /// Called once when convergence completes (or max iters is reached). fn on_converged(&self, _iters: usize, _final_step: (f64, f64), _converged: bool) {} } /// ZST no-op observer; the default when none is configured. #[derive(Copy, Clone, Debug, Default)] pub struct NullObserver; impl Observer for NullObserver {} ``` - [ ] **Step 2: Register in `src/lib.rs`** ```rust pub mod observer; ... pub use observer::{NullObserver, Observer}; ``` - [ ] **Step 3: Build** ```bash cargo build --features approx cargo test --features approx --lib ``` - [ ] **Step 4: Commit** ```bash cargo +nightly fmt git add src/observer.rs src/lib.rs git commit -m "$(cat <<'EOF' feat(api): add Observer trait and NullObserver default Observer replaces verbose: bool with structured progress callbacks. NullObserver is a ZST default; users override only the methods they care about. Send + Sync bounds deferred to T3. Part of T2. EOF )" ``` --- ## Task 12: Add `ConvergenceOptions` and `ConvergenceReport`; wire into history **Files:** - Create: `src/convergence.rs` - Modify: `src/history.rs`, `src/lib.rs` - [ ] **Step 1: Create `src/convergence.rs`** ```rust //! Convergence configuration and reporting. use std::time::Duration; use smallvec::SmallVec; #[derive(Clone, Copy, Debug)] pub struct ConvergenceOptions { pub max_iter: usize, pub epsilon: f64, } impl Default for ConvergenceOptions { fn default() -> Self { Self { max_iter: crate::ITERATIONS, epsilon: crate::EPSILON, } } } /// Post-hoc summary of a `History::converge` call. #[derive(Clone, Debug)] pub struct ConvergenceReport { pub iterations: usize, pub final_step: (f64, f64), pub log_evidence: f64, pub converged: bool, pub per_iteration_time: SmallVec<[Duration; 32]>, pub slices_skipped: usize, } ``` - [ ] **Step 2: Register in `src/lib.rs`** ```rust pub mod convergence; ... pub use convergence::{ConvergenceOptions, ConvergenceReport}; ``` - [ ] **Step 3: Add `convergence: ConvergenceOptions` and `observer` to `HistoryBuilder`** In `src/history.rs`, add to `HistoryBuilder`: ```rust pub fn convergence(mut self, opts: ConvergenceOptions) -> Self { self.convergence = opts; self } pub fn observer>(self, observer: O2) -> HistoryBuilder { … } ``` `HistoryBuilder` gains two extra generic parameters (`O: Observer` and observer storage). The simplest shape: ```rust pub struct HistoryBuilder = ConstantDrift, O: Observer = NullObserver> { mu: f64, sigma: f64, beta: f64, drift: D, p_draw: f64, convergence: ConvergenceOptions, observer: O, _time: std::marker::PhantomData, } ``` `History` gains the same `O` parameter; `observer` is stored and called from the convergence loop. - [ ] **Step 4: Add `history.converge() -> Result`** ```rust pub fn converge(&mut self) -> Result { use std::time::Instant; let opts = self.convergence; let mut step = (f64::INFINITY, f64::INFINITY); let mut i = 0; let mut per_iter: SmallVec<[Duration; 32]> = SmallVec::new(); while crate::tuple_gt(step, opts.epsilon) && i < opts.max_iter { let t0 = Instant::now(); step = self.iteration(); per_iter.push(t0.elapsed()); i += 1; self.observer.on_iteration_end(i, step); } let converged = !crate::tuple_gt(step, opts.epsilon); let log_evidence = self.log_evidence_impl(false, &[]); self.observer.on_converged(i, step, converged); Ok(ConvergenceReport { iterations: i, final_step: step, log_evidence, converged, per_iteration_time: per_iter, slices_skipped: 0, }) } ``` Keep the old `convergence(iters, eps, verbose)` method alive for one more task. It is removed in Task 20 alongside test translation. - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` - [ ] **Step 6: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): add ConvergenceOptions, ConvergenceReport, and History::converge HistoryBuilder gains .convergence(opts) and .observer(o). History::converge returns a structured ConvergenceReport with per-iteration timings. The old History::convergence(iters, eps, verbose) still works and is removed in Task 20. Part of T2. EOF )" ``` --- ## Task 13: Expand `InferenceError`; convert boundary panics to `Result` **Files:** - Modify: `src/error.rs` - Modify: `src/history.rs`, `src/game.rs` (where `debug_assert!` or `assert!` guard user input) - [ ] **Step 1: Expand `InferenceError`** Replace `src/error.rs`: ```rust use std::fmt; #[derive(Debug, Clone, PartialEq)] pub enum InferenceError { /// Expected and actual lengths of some array-shaped input differ. MismatchedShape { kind: &'static str, expected: usize, got: usize, }, /// A probability value is outside `[0, 1]`. InvalidProbability { value: f64 }, /// Convergence exceeded `max_iter` without falling below `epsilon`. ConvergenceFailed { last_step: (f64, f64), iterations: usize, }, /// Negative precision: a Gaussian with `pi < 0` slipped into an API call. NegativePrecision { pi: f64 }, } impl fmt::Display for InferenceError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { Self::MismatchedShape { kind, expected, got } => { write!(f, "{kind}: expected length {expected}, got {got}") } Self::InvalidProbability { value } => { write!(f, "probability must be in [0, 1]; got {value}") } Self::ConvergenceFailed { last_step, iterations } => { write!( f, "convergence failed after {iterations} iterations; last step = {last_step:?}" ) } Self::NegativePrecision { pi } => { write!(f, "precision must be non-negative; got {pi}") } } } } impl std::error::Error for InferenceError {} ``` - [ ] **Step 2: Convert boundary panics to `Result`** Search for `debug_assert!` and `assert!` at the API boundary: ```bash grep -rn 'debug_assert!\|^\s*assert!' src/history.rs src/game.rs ``` In `Game::new` (the internal path kept for `History`), the existing `debug_assert!`s remain — they catch invariants, not user errors. But for a public `Game::ranked` (added later in Task 19), return `InferenceError::MismatchedShape` on bad input rather than panicking. In `History::add_events_with_prior`, the four `assert!(...)` calls at the top of the function become `Result<(), InferenceError>` returns. Change the function signature: ```rust pub(crate) fn add_events_with_prior( &mut self, … ) -> Result<(), InferenceError> { if results.len() != composition.len() && !results.is_empty() { return Err(InferenceError::MismatchedShape { kind: "results", expected: composition.len(), got: results.len(), }); } // … other checks … // … rest of body … Ok(()) } ``` Every caller must add `?`. Tests that were calling `h.add_events(…)` (the thin wrapper) now call `h.add_events(…)?` or `.unwrap()` in tests. - [ ] **Step 3: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` - [ ] **Step 4: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(error): expand InferenceError; convert boundary asserts to Result Adds MismatchedShape, InvalidProbability, ConvergenceFailed. History's add_events_with_prior now returns Result<(), InferenceError>. Internal debug_asserts for invariants stay; user-facing shape/bounds checks become errors. Part of T2. EOF )" ``` --- ## Task 14: Add `record_winner` and `record_draw` convenience API **Files:** - Modify: `src/history.rs` - [ ] **Step 1: Add the methods** In `impl, O: Observer> History`: ```rust /// Record a single 1v1 match where `winner` beat `loser` at time `time`. pub fn record_winner(&mut self, winner: &Q, loser: &Q, time: T) -> Result<(), InferenceError> where K: std::borrow::Borrow + std::hash::Hash + Eq + ToOwned, Q: std::hash::Hash + Eq + ?Sized, { let w_idx = self.intern(winner); let l_idx = self.intern(loser); self.add_events_with_prior( vec![vec![vec![w_idx], vec![l_idx]]], vec![vec![1.0, 0.0]], vec![time], vec![], std::collections::HashMap::new(), ) } /// Record a 1v1 tie. pub fn record_draw(&mut self, a: &Q, b: &Q, time: T) -> Result<(), InferenceError> where K: std::borrow::Borrow + std::hash::Hash + Eq + ToOwned, Q: std::hash::Hash + Eq + ?Sized, { let a_idx = self.intern(a); let b_idx = self.intern(b); self.add_events_with_prior( vec![vec![vec![a_idx], vec![b_idx]]], vec![vec![0.0, 0.0]], vec![time], vec![], std::collections::HashMap::new(), ) } /// Get or create an `Index` for a user key. See spec Section 4 "Open question 3." pub fn intern(&mut self, key: &Q) -> Index where K: std::borrow::Borrow + std::hash::Hash + Eq + ToOwned, Q: std::hash::Hash + Eq + ?Sized, { self.keys.get_or_create(key) } /// Look up an `Index` for a key without creating it. pub fn lookup(&self, key: &Q) -> Option where K: std::borrow::Borrow + std::hash::Hash + Eq + ToOwned, Q: std::hash::Hash + Eq + ?Sized, { self.keys.get(key) } ``` Note: `History` does not currently hold a `KeyTable`. This task requires adding `keys: KeyTable` to the `History` struct (which also adds `K: Eq + Hash` bound). The generic parameter `K` is threaded through `History`. This is a significant shape change — work through the compiler errors. - [ ] **Step 2: Add an integration test for `record_winner`** Create `tests/record_winner.rs`: ```rust use trueskill_tt::{ConstantDrift, ConvergenceOptions, History}; #[test] fn record_winner_updates_skills() { let mut h = History::::builder() .mu(25.0) .sigma(25.0 / 3.0) .beta(25.0 / 6.0) .drift(ConstantDrift(25.0 / 300.0)) .build(); h.record_winner(&"alice", &"bob", 0).unwrap(); h.converge().unwrap(); let alice = h.current_skill(&"alice").unwrap(); let bob = h.current_skill(&"bob").unwrap(); assert!(alice.mu() > 25.0, "winner mu should rise: got {}", alice.mu()); assert!(bob.mu() < 25.0, "loser mu should fall: got {}", bob.mu()); } ``` This test depends on `current_skill` (added in Task 17). For now, comment out the last three lines or replace with a `learning_curves()` check; come back to it in Task 17. - [ ] **Step 3: Build + test** ```bash cargo build --features approx cargo test --features approx --test record_winner ``` - [ ] **Step 4: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): add record_winner, record_draw, intern, lookup on History Spec Section 4 "three-tier event ingestion" tier 2: one-off match convenience. History now holds a KeyTable internally; generic parameter K added to History. Part of T2. EOF )" ``` --- ## Task 15: Replace nested-Vec `add_events` with typed `add_events(iter)` **Files:** - Modify: `src/history.rs` - [ ] **Step 1: Add the new `add_events` signature** ```rust pub fn add_events(&mut self, events: I) -> Result<(), InferenceError> where I: IntoIterator>, { // Translate each Event into the internal composition/results/times/weights // triple, then delegate to add_events_with_prior (which becomes pub(crate)). let mut composition: Vec>> = Vec::new(); let mut results: Vec> = Vec::new(); let mut times: Vec = Vec::new(); let mut weights: Vec>> = Vec::new(); let mut priors: HashMap> = HashMap::new(); for ev in events { let mut event_comp: Vec> = Vec::new(); let mut event_weights: Vec> = Vec::new(); for team in &ev.teams { let mut team_indices: Vec = Vec::new(); let mut team_weights: Vec = Vec::new(); for member in &team.members { let idx = self.intern(&member.key); team_indices.push(idx); team_weights.push(member.weight); if let Some(prior) = member.prior { priors.insert( idx, Rating::new(prior, self.beta, self.drift), ); } } event_comp.push(team_indices); event_weights.push(team_weights); } composition.push(event_comp); weights.push(event_weights); // Convert Outcome::Ranked to the legacy "lower number wins" f64 result. // Legacy convention: result[i] is a score; higher score = better rank. // We invert ranks: team with rank 0 gets highest score. let ranks = ev.outcome.as_ranks(); if ranks.len() != ev.teams.len() { return Err(InferenceError::MismatchedShape { kind: "outcome ranks vs teams", expected: ev.teams.len(), got: ranks.len(), }); } let max_rank = ranks.iter().copied().max().unwrap_or(0) as f64; let inverted: Vec = ranks.iter().map(|&r| max_rank - r as f64).collect(); results.push(inverted); times.push(ev.time); } self.add_events_with_prior(composition, results, times, weights, priors) } ``` Rename the old public `add_events` (nested-Vec) to `pub(crate) fn add_events_legacy` or inline it into the internal call path; delete it from the public API surface. - [ ] **Step 2: Add an integration test for the new signature** In `tests/api_shape.rs`: ```rust use trueskill_tt::{Event, History, Member, Outcome, Team}; use smallvec::smallvec; #[test] fn add_events_bulk_via_iter() { let mut h = History::::builder() .mu(0.0).sigma(2.0).beta(1.0).p_draw(0.0) .build(); let events = vec![ Event { time: 1, teams: smallvec![ Team::with_members([Member::new("a")]), Team::with_members([Member::new("b")]), ], outcome: Outcome::winner(0, 2), }, Event { time: 2, teams: smallvec![ Team::with_members([Member::new("b")]), Team::with_members([Member::new("c")]), ], outcome: Outcome::winner(0, 2), }, ]; h.add_events(events).unwrap(); let report = h.converge().unwrap(); assert!(report.converged, "expected convergence, got {report:?}"); } ``` - [ ] **Step 3: Build + test** ```bash cargo build --features approx cargo test --features approx --test api_shape ``` - [ ] **Step 4: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): replace nested-Vec add_events with typed add_events(iter) The old nested shape is gone from the public API. add_events now takes IntoIterator>. Internally routes through add_events_with_prior (now pub(crate)). Part of T2. EOF )" ``` --- ## Task 16: Add `history.event(time).team(...).commit()` fluent builder **Files:** - Create: `src/event_builder.rs` - Modify: `src/history.rs`, `src/lib.rs` - [ ] **Step 1: Create `src/event_builder.rs`** ```rust //! Fluent builder for single-event ingestion. //! //! ```ignore //! history //! .event(time) //! .team(["alice", "bob"]).weights([1.0, 0.7]) //! .team(["carol"]) //! .ranking([1, 0]) //! .commit()?; //! ``` use std::collections::HashMap; use smallvec::SmallVec; use crate::{ InferenceError, Outcome, event::{Event, Member, Team}, history::History, observer::Observer, drift::Drift, time::Time, }; pub struct EventBuilder<'h, T, D, O, K> where T: Time, D: Drift, O: Observer, K: Eq + std::hash::Hash + Clone, { history: &'h mut History, event: Event, current_team_idx: Option, } impl<'h, T, D, O, K> EventBuilder<'h, T, D, O, K> where T: Time, D: Drift, O: Observer, K: Eq + std::hash::Hash + Clone, { pub(crate) fn new(history: &'h mut History, time: T, default_n_teams: usize) -> Self { Self { history, event: Event { time, teams: SmallVec::with_capacity(default_n_teams.max(2)), outcome: Outcome::Ranked(SmallVec::new()), }, current_team_idx: None, } } /// Add a team by its member keys. pub fn team>(mut self, keys: I) -> Self { let members: SmallVec<[Member; 4]> = keys.into_iter().map(Member::new).collect(); self.event.teams.push(Team { members }); self.current_team_idx = Some(self.event.teams.len() - 1); self } /// Set per-member weights for the most recently added team. pub fn weights>(mut self, weights: I) -> Self { let idx = self .current_team_idx .expect(".weights(...) called before any .team(...)"); let ws: Vec = weights.into_iter().collect(); let team = &mut self.event.teams[idx]; debug_assert_eq!( ws.len(), team.members.len(), "weights length must match team size" ); for (m, w) in team.members.iter_mut().zip(ws.into_iter()) { m.weight = w; } self } /// Set explicit ranks per team (length must equal number of teams). pub fn ranking>(mut self, ranks: I) -> Self { self.event.outcome = Outcome::ranking(ranks); self } /// Mark team `winner_idx` as winner of an N-way match, others tied for last. pub fn winner(mut self, winner_idx: u32) -> Self { self.event.outcome = Outcome::winner(winner_idx, self.event.teams.len() as u32); self } /// All teams tied. pub fn draw(mut self) -> Self { self.event.outcome = Outcome::draw(self.event.teams.len() as u32); self } /// Commit the event to the history. pub fn commit(self) -> Result<(), InferenceError> { self.history.add_events(std::iter::once(self.event)) } } ``` - [ ] **Step 2: Add `History::event` method** In `src/history.rs`: ```rust pub fn event(&mut self, time: T) -> EventBuilder<'_, T, D, O, K> { EventBuilder::new(self, time, 2) } ``` - [ ] **Step 3: Register the module in `src/lib.rs`** ```rust pub mod event_builder; pub use event_builder::EventBuilder; ``` - [ ] **Step 4: Add a test** Append to `tests/api_shape.rs`: ```rust #[test] fn fluent_event_builder() { let mut h = History::::builder() .mu(25.0).sigma(25.0 / 3.0).beta(25.0 / 6.0).p_draw(0.0) .build(); h.event(1) .team(["alice", "bob"]) .weights([1.0, 0.7]) .team(["carol"]) .ranking([1, 0]) .commit() .unwrap(); let report = h.converge().unwrap(); assert!(report.converged); } ``` - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --test api_shape fluent_event_builder ``` - [ ] **Step 6: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): add fluent history.event(t).team(...).commit() builder Third tier of the ingestion API (per spec Section 4). Powers one-off events with irregular shapes where neither record_winner nor typed add_events fits cleanly. Part of T2. EOF )" ``` --- ## Task 17: Query methods — `current_skill`, `learning_curve`, `log_evidence_for`, `predict_*` **Files:** - Modify: `src/history.rs`, `src/game.rs`, `src/lib.rs` - [ ] **Step 1: Add `current_skill` and single-key `learning_curve` to History** ```rust impl, O: Observer, K: Eq + std::hash::Hash + Clone> History { /// Skill estimate at the latest time slice the competitor appears in. pub fn current_skill(&self, key: &Q) -> Option where K: std::borrow::Borrow, Q: std::hash::Hash + Eq + ?Sized, { let idx = self.keys.get(key)?; self.time_slices .iter() .rev() .find_map(|ts| ts.skills.get(idx).map(|sk| sk.posterior())) } /// Learning curve for a single key: (time, posterior) pairs in time order. pub fn learning_curve(&self, key: &Q) -> Vec<(T, Gaussian)> where K: std::borrow::Borrow, Q: std::hash::Hash + Eq + ?Sized, { let Some(idx) = self.keys.get(key) else { return Vec::new(); }; self.time_slices .iter() .filter_map(|ts| ts.skills.get(idx).map(|sk| (ts.time, sk.posterior()))) .collect() } } ``` - [ ] **Step 2: Update `learning_curves` to return `HashMap>`** Currently returns `HashMap>`. New shape: ```rust pub fn learning_curves(&self) -> HashMap> { let mut data: HashMap> = HashMap::new(); for slice in &self.time_slices { for (idx, skill) in slice.skills.iter() { if let Some(key) = self.keys.key(idx).cloned() { data.entry(key) .or_default() .push((slice.time, skill.posterior())); } } } data } ``` - [ ] **Step 3: Add `log_evidence()` and `log_evidence_for(&[&K])`** ```rust pub fn log_evidence(&mut self) -> f64 { self.log_evidence_impl(false, &[]) } pub fn log_evidence_for(&mut self, keys: &[&Q]) -> f64 where K: std::borrow::Borrow, Q: std::hash::Hash + Eq + ?Sized, { let targets: Vec = keys .iter() .filter_map(|k| self.keys.get(k)) .collect(); self.log_evidence_impl(false, &targets) } fn log_evidence_impl(&mut self, forward: bool, targets: &[Index]) -> f64 { self.time_slices .iter() .map(|ts| ts.log_evidence(self.online, targets, forward, &self.competitors)) .sum() } ``` The existing public `log_evidence(forward, targets)` method is renamed to `log_evidence_impl` (pub(crate)) and the thin public wrappers take its place. The `forward` bool ("online" mode) stays internal; no public exposure planned for T2. - [ ] **Step 4: Add `predict_quality` and `predict_outcome`** These are light wrappers over the existing `quality()` free function plus a new "predict outcome" that runs a one-off Game without adding to history: ```rust pub fn predict_quality(&self, teams: &[&[&K]]) -> f64 where K: std::borrow::Borrow, { let groups: Vec> = teams .iter() .map(|team| { team.iter() .filter_map(|k| self.keys.get(*k)) .filter_map(|idx| { self.time_slices .iter() .rev() .find_map(|ts| ts.skills.get(idx).map(|s| s.posterior())) }) .collect() }) .collect(); let group_refs: Vec<&[Gaussian]> = groups.iter().map(|g| g.as_slice()).collect(); crate::quality(&group_refs, self.beta) } pub fn predict_outcome(&self, teams: &[&[&K]]) -> Vec { // Win probabilities per team: apply cdf over team-perf sums. // For n teams, return P(team i wins over all others). // Minimal impl for T2: 2-team case only; multi-team deferred to T4. assert_eq!(teams.len(), 2, "predict_outcome T2: 2 teams only"); let gather = |team: &[&K]| -> Gaussian { team.iter() .filter_map(|k| self.keys.get(*k)) .filter_map(|idx| { self.time_slices .iter() .rev() .find_map(|ts| ts.skills.get(idx).map(|s| s.posterior())) }) .fold(crate::N00, |acc, g| acc + g.forget(self.beta.powi(2))) }; let a = gather(teams[0]); let b = gather(teams[1]); let diff = a - b; let p_a = 1.0 - crate::cdf(0.0, diff.mu(), diff.sigma()); vec![p_a, 1.0 - p_a] } ``` Document `predict_outcome` as 2-team-only in T2; N-team lands in T4 alongside `Residual` schedules. - [ ] **Step 5: Add tests** In `tests/api_shape.rs`: ```rust #[test] fn current_skill_learning_curve_learning_curves() { let mut h = History::::builder() .mu(25.0).sigma(25.0 / 3.0).beta(25.0 / 6.0).p_draw(0.0) .build(); h.record_winner(&"a", &"b", 1).unwrap(); h.record_winner(&"a", &"b", 2).unwrap(); h.converge().unwrap(); let a = h.current_skill(&"a").unwrap(); assert!(a.mu() > 25.0); let curve = h.learning_curve(&"a"); assert_eq!(curve.len(), 2); assert_eq!(curve[0].0, 1); assert_eq!(curve[1].0, 2); let all = h.learning_curves(); assert_eq!(all.len(), 2); assert!(all.contains_key("a")); } #[test] fn log_evidence_for_subset() { let mut h = History::::builder() .mu(0.0).sigma(6.0).beta(1.0).p_draw(0.0) .build(); h.record_winner(&"a", &"b", 1).unwrap(); h.record_winner(&"b", &"a", 2).unwrap(); let ev_all = h.log_evidence(); let ev_a = h.log_evidence_for(&[&"a"]); assert_ne!(ev_all, ev_a); } ``` - [ ] **Step 6: Build + test** ```bash cargo build --features approx cargo test --features approx --test api_shape ``` - [ ] **Step 7: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): add current_skill, learning_curve (single), log_evidence_for, predict_* learning_curves now returns HashMap> keyed on the user's key type. log_evidence is public zero-arg; log_evidence_for takes a slice of keys. predict_outcome is T2 2-team-only; N-team deferred. Part of T2. EOF )" ``` --- ## Task 18: Promote `Factor` / `Schedule` / `VarStore` to `pub` under a `factors` module **Files:** - Create: `src/factors.rs` - Modify: `src/factor/mod.rs`, `src/factor/rank_diff.rs`, `src/factor/team_sum.rs`, `src/factor/trunc.rs`, `src/schedule.rs`, `src/lib.rs` - [ ] **Step 1: Promote visibility inside `src/factor/mod.rs`** Change `pub(crate) struct VarId(pub(crate) u32);` → `pub struct VarId(pub u32);`. Similarly: - `pub(crate) struct VarStore` → `pub struct VarStore` (keep `marginals` field `pub(crate)` — implementation detail). - `pub(crate) trait Factor` → `pub trait Factor`. - `pub(crate) enum BuiltinFactor` → `pub enum BuiltinFactor`. - Submodules: `pub(crate) mod team_sum;` → `pub mod team_sum;` and similarly for `rank_diff`, `trunc`. - Inside each submodule, promote the struct and its fields/constructors from `pub(crate)` to `pub`. Remove any `#[allow(dead_code)]` that was guarding visibility warnings. - [ ] **Step 2: Promote `Schedule` and `EpsilonOrMax` in `src/schedule.rs`** ```rust pub trait Schedule { fn run(&self, factors: &mut [BuiltinFactor], vars: &mut VarStore) -> ScheduleReport; } pub struct EpsilonOrMax { pub eps: f64, pub max: usize, } ``` Remove `#[allow(dead_code)]`. - [ ] **Step 3: Create `src/factors.rs` that re-exports the public API** ```rust //! Factor-graph public API. //! //! Power users can construct custom factor graphs via `Game::custom` and //! drive them with a custom `Schedule` implementation. pub use crate::factor::{BuiltinFactor, Factor, VarId, VarStore}; pub use crate::factor::rank_diff::RankDiffFactor; pub use crate::factor::team_sum::TeamSumFactor; pub use crate::factor::trunc::TruncFactor; pub use crate::schedule::{EpsilonOrMax, Schedule, ScheduleReport}; ``` - [ ] **Step 4: Register in `src/lib.rs`** Keep `factor` and `schedule` modules as they are (internal use), and add: ```rust pub mod factors; ``` - [ ] **Step 5: Build + test** ```bash cargo build --features approx cargo test --features approx --lib ``` - [ ] **Step 6: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): promote Factor/Schedule/VarStore to pub in `factors` module Exposes the factor-graph machinery so power users can define custom factors and schedules (see Game::custom in the next task). The internal factor/ and schedule/ modules remain pub(crate) — user- facing API goes through the factors module re-exports. Part of T2. EOF )" ``` --- ## Task 19: `Game::ranked`, `Game::one_v_one`, `Game::free_for_all`, `Game::custom` **Files:** - Modify: `src/game.rs` - [ ] **Step 1: Split the internal `Game::new` into a `Game::ranked_with_arena`** Move the current `Game::new` body into `pub(crate) fn ranked_with_arena(...) -> Self`. The public signatures become: ```rust pub struct GameOptions { pub p_draw: f64, pub convergence: ConvergenceOptions, } impl Default for GameOptions { fn default() -> Self { Self { p_draw: crate::P_DRAW, convergence: ConvergenceOptions::default(), } } } impl<'a, T: Time, D: Drift> Game<'a, T, D> { /// Build a ranked match from borrowed team rosters and an outcome. /// /// Returns `Err(MismatchedShape)` if outcome rank count doesn't match /// team count. Returns `Err(InvalidProbability)` if p_draw is out of range. pub fn ranked( teams: &[&[Rating]], outcome: Outcome, options: &GameOptions, ) -> Result { if !(0.0..1.0).contains(&options.p_draw) { return Err(InferenceError::InvalidProbability { value: options.p_draw }); } if outcome.team_count() != teams.len() { return Err(InferenceError::MismatchedShape { kind: "outcome ranks vs teams", expected: teams.len(), got: outcome.team_count(), }); } // Translate ranks to the legacy "higher f64 = better" result. let ranks = outcome.as_ranks(); let max_rank = ranks.iter().copied().max().unwrap_or(0) as f64; let result: Vec = ranks.iter().map(|&r| max_rank - r as f64).collect(); let teams_owned: Vec>> = teams.iter().map(|t| t.to_vec()).collect(); let weights: Vec> = teams.iter().map(|t| vec![1.0; t.len()]).collect(); let mut arena = ScratchArena::new(); let game = Self::ranked_with_arena( teams_owned, &result, &weights, options.p_draw, &mut arena, ); Ok(game) } /// 1v1 convenience: returns posteriors `(a_post, b_post)` directly. pub fn one_v_one( a: &Rating, b: &Rating, outcome: Outcome, ) -> Result<(Gaussian, Gaussian), InferenceError> { let game = Self::ranked( &[&[*a], &[*b]], outcome, &GameOptions::default(), )?; let post = game.posteriors(); Ok((post[0][0], post[1][0])) } /// FFA: each entry is a single rating; outcome ranks per player. pub fn free_for_all( players: &[&Rating], outcome: Outcome, options: &GameOptions, ) -> Result { let teams: Vec>> = players.iter().map(|p| vec![**p]).collect(); let team_refs: Vec<&[Rating]> = teams.iter().map(|t| t.as_slice()).collect(); Self::ranked(&team_refs, outcome, options) } /// Power-user: build a game from a user-defined factor graph. /// /// The caller owns the `VarStore` and `Vec`. The schedule /// is run once; `posteriors_from_vars` extracts team posteriors by VarId. pub fn custom( teams: Vec>>, vars: VarStore, factors: Vec, team_perf_vars: Vec, weights: Vec>, schedule: &S, ) -> Self { // Partial impl: run schedule, populate likelihoods, evidence. // Full custom-factor support expands in T4. let mut this = Self { teams, result: &[], // Not used for custom — outcome encoded in factors. weights: &[], p_draw: 0.0, likelihoods: Vec::new(), evidence: 1.0, }; this.run_custom(vars, factors, team_perf_vars, weights, schedule); this } /// Log-evidence for this game (zero if there were no Trunc factors). pub fn log_evidence(&self) -> f64 { self.evidence.ln() } } ``` `Game::custom` is the spec's escape hatch for user-defined factors. For T2 it only needs to *work*; its full ergonomics land in T4. Mark it `#[doc(hidden)]` if unfinished. - [ ] **Step 2: Add tests** ```rust #[test] fn game_ranked_1v1() { let a = Rating::new(Gaussian::from_ms(25.0, 25.0/3.0), 25.0/6.0, ConstantDrift(25.0/300.0)); let b = Rating::new(Gaussian::from_ms(25.0, 25.0/3.0), 25.0/6.0, ConstantDrift(25.0/300.0)); let g = Game::::ranked( &[&[a], &[b]], Outcome::winner(0, 2), &GameOptions::default(), ).unwrap(); let p = g.posteriors(); assert_ulps_eq!(p[0][0], Gaussian::from_ms(29.205220, 7.194481), epsilon = 1e-6); assert_ulps_eq!(p[1][0], Gaussian::from_ms(20.794779, 7.194481), epsilon = 1e-6); } #[test] fn game_one_v_one_shortcut() { let a = Rating::::new(Gaussian::from_ms(25.0, 25.0/3.0), 25.0/6.0, ConstantDrift(25.0/300.0)); let b = Rating::::new(Gaussian::from_ms(25.0, 25.0/3.0), 25.0/6.0, ConstantDrift(25.0/300.0)); let (a_post, b_post) = Game::::one_v_one(&a, &b, Outcome::winner(0, 2)).unwrap(); assert_ulps_eq!(a_post, Gaussian::from_ms(29.205220, 7.194481), epsilon = 1e-6); assert_ulps_eq!(b_post, Gaussian::from_ms(20.794779, 7.194481), epsilon = 1e-6); } #[test] fn game_ranked_rejects_bad_p_draw() { let a = Rating::::new(Gaussian::default(), 1.0, ConstantDrift(0.0)); let err = Game::::ranked( &[&[a], &[a]], Outcome::winner(0, 2), &GameOptions { p_draw: 1.5, convergence: Default::default() }, ).unwrap_err(); assert!(matches!(err, InferenceError::InvalidProbability { .. })); } ``` Note `Outcome::winner(0, 2)` makes team 0 the winner and team 1 the loser. The old test used `&[0.0, 1.0]` where higher = worse (team 1 had result 1.0 meaning worse result i.e. team 0 won). Under new convention `Outcome::winner(0, 2)` → ranks `[0, 1]` → mapped to legacy results `[1.0, 0.0]` → team 0 wins. But the old test asserted `p[0][0] = Gaussian::from_ms(20.794779, 7.194481)` (loser) and `p[1][0] = Gaussian::from_ms(29.205220, 7.194481)` (winner). That's because the old convention was higher `result` = worse position. **Double-check this mapping before committing golden values** — run the test and if p[0] vs p[1] flip, swap the expected values. - [ ] **Step 3: Build + test** ```bash cargo build --features approx cargo test --features approx --lib game ``` - [ ] **Step 4: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' feat(api): add Game::ranked, one_v_one, free_for_all, custom constructors Public Game API now returns Result on bad inputs. GameOptions bundles p_draw and convergence config. Game::custom is an escape hatch for user-defined factor graphs (full ergonomics land in T4). Part of T2. EOF )" ``` --- ## Task 20: Translate the full test suite to the new API; delete legacy methods **Files:** - Modify: every `#[cfg(test)] mod tests` across `src/` - Modify: `tests/equivalence.rs` (new file) - Delete: `History::convergence(iters, eps, verbose)`, the legacy `add_events` wrapper - [ ] **Step 1: Inventory remaining legacy callers** ```bash grep -rn 'h\.convergence(\|\.add_events(vec!\|\.time(true)\|\.time(false)\|\.gamma(' src/ tests/ ``` Every hit is a site that must be translated. - [ ] **Step 2: Translation cheat-sheet** Apply this uniformly across every test module. Example old form (from `src/history.rs`): ```rust let mut h = History::builder() .mu(0.0).sigma(2.0).beta(1.0).gamma(0.0).time(false).build(); h.add_events(composition, results, vec![], vec![]); h.convergence(ITERATIONS, EPSILON, false); ``` New form: ```rust let mut h = History::::builder() .mu(0.0).sigma(2.0).beta(1.0).drift(ConstantDrift(0.0)).build(); // Translate nested Vec>> into Vec> let events = translate_to_events(composition, results, /* times = 1..=n */ (1..=n).collect()); h.add_events(events).unwrap(); h.converge().unwrap(); ``` Since the tests use `IndexMap::get_or_create("a")` returning `Index`, they'll need updating to either use `history.intern(&"a")` directly or use `Member::new("a")` in `Event`. Provide a test helper in a `tests/common.rs` module: ```rust pub fn winner_event(time: i64, w: &'static str, l: &'static str) -> Event { use smallvec::smallvec; Event { time, teams: smallvec![ Team::with_members([Member::new(w)]), Team::with_members([Member::new(l)]), ], outcome: Outcome::winner(0, 2), } } pub fn draw_event(time: i64, a: &'static str, b: &'static str) -> Event { use smallvec::smallvec; Event { time, teams: smallvec![ Team::with_members([Member::new(a)]), Team::with_members([Member::new(b)]), ], outcome: Outcome::draw(2), } } ``` Then `h.add_events(vec![winner_event(1, "a", "b"), winner_event(2, "a", "c"), winner_event(3, "b", "c")]).unwrap();`. - [ ] **Step 3: Translate each old test one at a time** Work through `src/history.rs::tests`, `src/game.rs::tests`, `src/time_slice.rs::tests`. Each test retains its hardcoded golden values — numerical behavior is unchanged. Only the *construction* changes. For tests that accessed `h.batches[i].skills.get(idx).unwrap().posterior()`, change to `h.learning_curve(&key)[i].1` or `h.current_skill(&key).unwrap()`. For tests that looked at internal state (`h.batches[0].events.len()`, `get_composition()` etc.), delete those assertions if the new API doesn't expose them, or move them behind `#[cfg(test)]` accessors. - [ ] **Step 4: Create `tests/equivalence.rs` — the regression net** ```rust //! Verifies that the new API produces the same numerical results as the //! hardcoded goldens from the old API (taken from the original Python port). use approx::assert_ulps_eq; use trueskill_tt::{ ConstantDrift, ConvergenceOptions, Event, Gaussian, History, Member, Outcome, Team, }; use smallvec::smallvec; #[test] fn test_1vs1_matches_old_golden() { use trueskill_tt::{Game, GameOptions, Rating}; let a = Rating::::new( Gaussian::from_ms(25.0, 25.0 / 3.0), 25.0 / 6.0, ConstantDrift(25.0 / 300.0), ); let b = Rating::::new( Gaussian::from_ms(25.0, 25.0 / 3.0), 25.0 / 6.0, ConstantDrift(25.0 / 300.0), ); let (a_post, b_post) = Game::::one_v_one(&a, &b, Outcome::winner(0, 2)).unwrap(); assert_ulps_eq!(a_post, Gaussian::from_ms(29.205220, 7.194481), epsilon = 1e-6); assert_ulps_eq!(b_post, Gaussian::from_ms(20.794779, 7.194481), epsilon = 1e-6); } #[test] fn test_env_ttt_matches_old_golden() { let mut h = History::::builder() .mu(25.0) .sigma(25.0 / 3.0) .beta(25.0 / 6.0) .drift(ConstantDrift(25.0 / 300.0)) .convergence(ConvergenceOptions { max_iter: 30, epsilon: 1e-6 }) .build(); let events: Vec> = vec![ Event { time: 1, teams: smallvec![ Team::with_members([Member::new("a")]), Team::with_members([Member::new("b")]), ], outcome: Outcome::winner(0, 2) }, Event { time: 2, teams: smallvec![ Team::with_members([Member::new("a")]), Team::with_members([Member::new("c")]), ], outcome: Outcome::winner(1, 2) }, Event { time: 3, teams: smallvec![ Team::with_members([Member::new("b")]), Team::with_members([Member::new("c")]), ], outcome: Outcome::winner(0, 2) }, ]; h.add_events(events).unwrap(); h.converge().unwrap(); let a_curve = h.learning_curve(&"a"); let b_curve = h.learning_curve(&"b"); assert_ulps_eq!(a_curve[0].1, Gaussian::from_ms(25.000267, 5.419381), epsilon = 1e-6); assert_ulps_eq!(b_curve[0].1, Gaussian::from_ms(24.999465, 5.419425), epsilon = 1e-6); } // Repeat for test_teams, test_add_events, test_only_add_events, test_log_evidence, // test_add_events_with_time, test_1vs1_weighted from the old history.rs tests, // translating construction while keeping every golden value identical. ``` Port every one of the seven original history tests plus the seven game tests. Any test that can't be translated because the new API doesn't expose the needed internals — if its golden is preserved by a translated test, delete it; if not, either add a `pub(crate)` accessor for the test and flag in the spec notes. - [ ] **Step 5: Delete legacy methods** In `src/history.rs`, delete: - `pub fn convergence(iters, eps, verbose) -> ((f64, f64), usize)` — replaced by `converge()`. - The `online: bool` builder accessor, if unused externally (leave `self.online` field internal for now — it's used by `log_evidence_impl`). - The nested-Vec public `add_events` wrapper. Only `pub(crate) fn add_events_with_prior` stays (no longer `pub`). - `HistoryBuilder::gamma(f64)` — now users go through `.drift(ConstantDrift(g))`. - [ ] **Step 6: Build + test** ```bash cargo test --features approx cargo clippy --all-targets --features approx -- -D warnings ``` All tests pass — failures here mean either a translation bug or a legitimate numeric difference. The goldens have been proven stable through T0 and T1, so any drift is translation. - [ ] **Step 7: Commit** ```bash cargo +nightly fmt git add -A git commit -m "$(cat <<'EOF' test(api): translate full test suite to new API; delete legacy methods Every old golden is reproduced in the new API (tests/equivalence.rs plus in-module tests). History::convergence, the nested-Vec add_events, .gamma(f64), .time(bool) all removed. Part of T2. EOF )" ``` --- ## Task 21: Final verification, benchmark capture, update CHANGELOG **Files:** - Modify: `benches/baseline.txt` - Modify: `CHANGELOG.md` - [ ] **Step 1: Full green check** ```bash cargo +nightly fmt --check cargo clippy --all-targets --features approx -- -D warnings cargo test --features approx cargo test --features approx --doc 2>&1 | tail -5 ``` All must pass. - [ ] **Step 2: Capture T2 benchmarks** ```bash cargo bench --bench batch 2>&1 | grep "Batch::iteration" cargo bench --bench gaussian 2>&1 | grep "Gaussian::" ``` Compare to T1 baseline (~23 µs). Must be within 5% (23.5 µs ceiling). T2 is an API refactor; hot path should be unchanged. - [ ] **Step 3: Append T2 block to `benches/baseline.txt`** ``` # After T2 (date, same hardware) Batch::iteration µs (vs T1 23.010 µs: ) Gaussian::add ps (unchanged) Gaussian::sub ps (unchanged) Gaussian::mul ps (unchanged) Gaussian::div ps (unchanged) Gaussian::pi ps (unchanged) Gaussian::tau ps (unchanged) # Notes: # - API-only tier; hot inference path unchanged so numerics match T1 within ULPs. # - Public surface now matches spec Section 4. # - Breaking changes: Batch→TimeSlice, Player→Rating, Agent→Competitor, # IndexMap→KeyTable; Event/Team/Member/Outcome new types; # Time trait; Drift generic; Observer + ConvergenceReport; Result<_, # InferenceError> at API boundary; factors module promoted to pub. ``` - [ ] **Step 4: Add CHANGELOG entry** Read `CHANGELOG.md` (check its existing format first with `head -20 CHANGELOG.md`) and prepend a `## [Unreleased]` section listing the breaking changes bulleted from the spec. - [ ] **Step 5: Commit** ```bash git add benches/baseline.txt CHANGELOG.md git commit -m "$(cat <<'EOF' bench,docs: capture T2 final numbers and update CHANGELOG Batch::iteration: µs (T1 was 23.010 µs). API-only tier; numerics within ULPs. Closes T2 of docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md. EOF )" ``` - [ ] **Step 6: Ready for review** The branch `t2-new-api-surface` is complete. Full test suite green, clippy clean, fmt clean, bench within target. Open the PR with: - Summary linking to spec + plan. - Breaking-change table (spec Section 4½). - Benchmark comparison. - Migration notes for known downstream (there are none — crate is pre-1.0). --- ## Self-review notes **Spec coverage** (against `docs/superpowers/specs/2026-04-23-trueskill-engine-redesign-design.md` Section 7 T2 checklist): - ✅ `Rating`, `TimeSlice`, `Competitor`, `Member`, `Outcome`, `Event`, `KeyTable` (Tasks 2, 3, 4, 5, 9, 10) - ✅ `Time` trait, `History>` (Tasks 6, 7, 8) - ✅ Three-tier API: `record_winner`, `event(...).team(...).commit()`, bulk `add_events(iter)` (Tasks 14, 15, 16) - ✅ `Observer` trait + `ConvergenceReport`; `verbose: bool` deleted (Tasks 11, 12, 20) - ✅ `panic!`/`debug_assert!` at API boundary → `Result<_, InferenceError>` (Task 13) - ✅ `Factor`/`Schedule`/`VarStore` promoted to `pub` under `factors` module (Task 18) - ✅ `Game::ranked`, `one_v_one`, `free_for_all`, `custom` (Task 19) - ✅ Equivalence tests prove identical posteriors (Task 20) - ✅ `intern` / `lookup` for `Index` exposure (Task 14) **Deferred to later tiers (explicitly):** - `Outcome::Scored` + `MarginFactor` — T4 (enum is `#[non_exhaustive]` so adding is non-breaking). - `Damped`, `Residual` schedules — T4. - `Send + Sync` bounds — T3. - `predict_outcome` N-team — T4. - `Game::custom` full ergonomics — T4. **Known hazards during execution:** - **Generic parameter explosion.** After Task 8 + 12 + 14, `History` has four type params. Use trait-object erasure internally if ergonomics suffer. Default types (`i64`, `ConstantDrift`, `NullObserver`) keep common callsites readable. - **`.player` → `.rating` field rename.** `grep -r '\.player'` also matches unrelated names. Audit manually inside `src/` only; **don't auto-replace across tests or dev deps.** - **Outcome → legacy result translation.** The legacy `result` f64 convention is "higher is worse" for descending sort. Our new `Outcome::Ranked` stores ranks where 0 = best. Mapping: `max_rank - rank[i]`. If a test's golden flips winner/loser, this is the likely cause. - **`Time = Untimed` behavior change.** Per spec, `Untimed::elapsed_to` returns 0 — no drift between slices. The old `time=false` mode implicitly used elapsed=1. Tests that used `time(false)` translate to `History::` with explicit `1..=n` timestamps to preserve numerics. - **Test module translation is mechanical but big.** Expect Task 20 to be the longest task (~60–90 minutes). Work through the test files one at a time; commit after each file to keep bisectable history. **Things outside the plan that may bite:** - `Cargo.toml` dev-dep `trueskill-tt = { path = ".", features = ["approx"] }` — if this references removed public names (`Batch`, `Player`), add the old name temporarily as a `type Batch = TimeSlice;` alias during the rename cascade, then remove before Task 21. - `examples/` directory. Confirm no examples exist (`ls examples/` showed nothing at plan-write time), but re-check before Task 20 and include any in the translation pass. - README.md probably shows the old API. Update in Task 21 alongside the CHANGELOG.