Public API gaps: predict_outcome panics and is 2-team-only, HistoryBuilder isn't exported, Rating/Index are write-only #21

New Issue

2026-08-04T19:19:55Z

logaritmisk commented

2026-08-04 19:19:55 +00:00

A collection of API-surface problems found while auditing. Individually small, collectively they make the crate awkward to consume.

1. `predict_outcome` panics, is 2-team-only, and ignores draws

src/history.rs:408-429:

/// Panics if `teams.len() != 2`. N-team support lands in T4.
pub fn predict_outcome(&self, teams: &[&[&K]]) -> Vec<f64> {
    assert_eq!(teams.len(), 2, "predict_outcome T2: 2 teams only");

T4 has shipped; the N-team support did not.
It panics rather than returning InferenceError, in a crate that has one.
It returns Vec<f64> of length 2 where a [f64; 2] or a named struct would be self-describing.
It ignores p_draw entirely: [p_a, 1.0 - p_a] allocates no probability mass to a draw even when the history is configured with p_draw > 0. For a draw-enabled model the numbers are simply wrong.
It silently drops unknown keys via filter_map (src/history.rs:415), same defect as predict_quality in #9 — a team of entirely unknown players yields a confident-looking probability.

2. `HistoryBuilder` is not exported

src/lib.rs:40 re-exports History but not HistoryBuilder, which is pub in src/history.rs:21. History::builder() works for inline chains, but callers cannot name the type:

use trueskill_tt::HistoryBuilder;   // error[E0432]: unresolved import

so no fn make_builder() -> HistoryBuilder<…>, no storing a partially-configured builder. Same for ConvergenceOptions' companion EpsilonOrMax, which is reachable only via crate::factors.

3. `Rating` is write-only

Rating's fields are all pub(crate) (src/rating.rs:15-20) with no accessors. A caller can construct one with Rating::new(prior, beta, drift) and hand it to Game::ranked, but cannot read back prior, beta, or drift — so a Rating obtained from anywhere can't be inspected, logged, or serialised. Add pub fn prior(), beta(), drift().

4. `Index` is opaque with no way back to `usize`

pub struct Index(usize) (src/lib.rs:64) has From<usize> but no inverse and no accessor. History::intern hands callers an Index and KeyTable is documented as letting "power users promote &K to Index and skip the lookup on the hot path" (src/key_table.rs:12-13) — but the returned handle can't be stored in an external side table keyed by integer, printed meaningfully, or round-tripped. Add pub fn get(self) -> usize (or impl From<Index> for usize).

Note Index is exported at the crate root but is not in the pub use list — it is public only by virtue of being declared pub in lib.rs, which is easy to miss.

5. `Game::one_v_one` hardcodes default options

src/game.rs:517-525 ignores any caller configuration:

let game = Self::ranked(&[&[*a], &[*b]], outcome, &GameOptions::default())?;

So a 1v1 with a draw cannot set p_draw, and no 1v1 can set convergence options. Given #8, passing Outcome::draw(2) here returns NaN with no way for the caller to avoid it. Take &GameOptions.

6. `Observer::on_batch_processed` is never called

Declared in the trait (src/observer.rs:20) but grep finds no call site — only on_iteration_end and on_converged are invoked (src/history.rs:446, 450). Implementors get a callback that never fires. Either call it from the slice sweep or remove it. The name is also pre-T2 ("batch" → TimeSlice); on_slice_processed matches the current vocabulary.

7. `factor` vs `factors` module naming

pub(crate) mod factor (src/lib.rs:19) and pub mod factors (src/lib.rs:20) sit adjacent, differing by one character — the latter being the public re-export facade of the former. Confusing to read and easy to mistype. Consider factor (private impl) + factors → something like graph or custom.

Acceptance

predict_outcome supports N teams, returns Result, accounts for p_draw, and errors on unknown keys.
HistoryBuilder is exported; Rating and Index have accessors.
one_v_one accepts GameOptions.
No public trait method is unreachable.
CHANGELOG.md records the breaking changes.

A collection of API-surface problems found while auditing. Individually small, collectively they make the crate awkward to consume. ## 1. `predict_outcome` panics, is 2-team-only, and ignores draws `src/history.rs:408-429`: ```rust /// Panics if `teams.len() != 2`. N-team support lands in T4. pub fn predict_outcome(&self, teams: &[&[&K]]) -> Vec<f64> { assert_eq!(teams.len(), 2, "predict_outcome T2: 2 teams only"); ``` - T4 has shipped; the N-team support did not. - It panics rather than returning `InferenceError`, in a crate that has one. - It returns `Vec<f64>` of length 2 where a `[f64; 2]` or a named struct would be self-describing. - It ignores `p_draw` entirely: `[p_a, 1.0 - p_a]` allocates no probability mass to a draw even when the history is configured with `p_draw > 0`. For a draw-enabled model the numbers are simply wrong. - It silently drops unknown keys via `filter_map` (`src/history.rs:415`), same defect as `predict_quality` in #9 — a team of entirely unknown players yields a confident-looking probability. ## 2. `HistoryBuilder` is not exported `src/lib.rs:40` re-exports `History` but not `HistoryBuilder`, which is `pub` in `src/history.rs:21`. `History::builder()` works for inline chains, but callers cannot name the type: ```rust use trueskill_tt::HistoryBuilder; // error[E0432]: unresolved import ``` so no `fn make_builder() -> HistoryBuilder<…>`, no storing a partially-configured builder. Same for `ConvergenceOptions`' companion `EpsilonOrMax`, which is reachable only via `crate::factors`. ## 3. `Rating` is write-only `Rating`'s fields are all `pub(crate)` (`src/rating.rs:15-20`) with no accessors. A caller can construct one with `Rating::new(prior, beta, drift)` and hand it to `Game::ranked`, but cannot read back `prior`, `beta`, or `drift` — so a `Rating` obtained from anywhere can't be inspected, logged, or serialised. Add `pub fn prior()`, `beta()`, `drift()`. ## 4. `Index` is opaque with no way back to `usize` `pub struct Index(usize)` (`src/lib.rs:64`) has `From<usize>` but no inverse and no accessor. `History::intern` hands callers an `Index` and `KeyTable` is documented as letting "power users promote `&K` to `Index` and skip the lookup on the hot path" (`src/key_table.rs:12-13`) — but the returned handle can't be stored in an external side table keyed by integer, printed meaningfully, or round-tripped. Add `pub fn get(self) -> usize` (or `impl From<Index> for usize`). Note `Index` is exported at the crate root but is not in the `pub use` list — it is public only by virtue of being declared `pub` in `lib.rs`, which is easy to miss. ## 5. `Game::one_v_one` hardcodes default options `src/game.rs:517-525` ignores any caller configuration: ```rust let game = Self::ranked(&[&[*a], &[*b]], outcome, &GameOptions::default())?; ``` So a 1v1 with a draw cannot set `p_draw`, and no 1v1 can set convergence options. Given #8, passing `Outcome::draw(2)` here returns NaN with no way for the caller to avoid it. Take `&GameOptions`. ## 6. `Observer::on_batch_processed` is never called Declared in the trait (`src/observer.rs:20`) but `grep` finds no call site — only `on_iteration_end` and `on_converged` are invoked (`src/history.rs:446`, `450`). Implementors get a callback that never fires. Either call it from the slice sweep or remove it. The name is also pre-T2 ("batch" → `TimeSlice`); `on_slice_processed` matches the current vocabulary. ## 7. `factor` vs `factors` module naming `pub(crate) mod factor` (`src/lib.rs:19`) and `pub mod factors` (`src/lib.rs:20`) sit adjacent, differing by one character — the latter being the public re-export facade of the former. Confusing to read and easy to mistype. Consider `factor` (private impl) + `factors` → something like `graph` or `custom`. ## Acceptance - `predict_outcome` supports N teams, returns `Result`, accounts for `p_draw`, and errors on unknown keys. - `HistoryBuilder` is exported; `Rating` and `Index` have accessors. - `one_v_one` accepts `GameOptions`. - No public trait method is unreachable. - `CHANGELOG.md` records the breaking changes.

logaritmisk referenced this issue

2026-08-04 19:20:44 +00:00

Dead and duplicated code: a stale `iterate_to_convergence`, a copy-pasted sweep, and `#[allow(dead_code)]` used as a mute button #23

logaritmisk referenced this issue

2026-08-04 19:21:19 +00:00

`cargo publish` would fail: no `description`, `license`, or `repository` — and the crate has no top-level docs #25

logaritmisk referenced this issue

2026-08-04 20:11:20 +00:00

`quality()` and `predict_quality()` panic for anything other than exactly 2 rating groups #9

logaritmisk referenced this issue

2026-08-04 20:12:32 +00:00

`cargo publish` would fail: no `description`, `license`, or `repository` — and the crate has no top-level docs #25

logaritmisk commented

2026-08-04 20:13:02 +00:00

Partly done — staying open for the items that are breaking or need a design call.

Done (all additive, no breakage):

2. HistoryBuilder is exported. It was pub but unreachable — use trueskill_tt::HistoryBuilder; failed to resolve.
3. Rating::{prior, beta, drift} accessors added.
4. Index::get() plus impl From<Index> for usize, so the handle intern hands out can be read back.
History::time_slices_len() added (small, needed by the new ingest benchmark).

Still open:

1. predict_outcome — still panics on ≠2 teams, still ignores p_draw, still drops unknown keys. All three want a signature change to Result plus a decision on how draw mass is reported for an N-team prediction. That is design work, not cleanup.
5. one_v_one still hardcodes GameOptions::default(). Fixing it means changing the signature. Less urgent than it was: passing Outcome::draw(2) there now returns a clean TieWithoutDrawProbability error rather than NaN (#8), so the trap is visible.
6. Observer::on_batch_processed is still never called, and still carries the pre-T2 "batch" name. Whether to wire it up or delete it is a call about what the observer contract should be.
7. factor vs factors module naming, unchanged.

predict_quality now documents its panics rather than silently dropping unknown keys — a stopgap until it can return Result alongside predict_outcome.

Partly done — **staying open** for the items that are breaking or need a design call. Done (all additive, no breakage): - **2.** `HistoryBuilder` is exported. It was `pub` but unreachable — `use trueskill_tt::HistoryBuilder;` failed to resolve. - **3.** `Rating::{prior, beta, drift}` accessors added. - **4.** `Index::get()` plus `impl From<Index> for usize`, so the handle `intern` hands out can be read back. - `History::time_slices_len()` added (small, needed by the new ingest benchmark). **Still open:** - **1.** `predict_outcome` — still panics on ≠2 teams, still ignores `p_draw`, still drops unknown keys. All three want a signature change to `Result` plus a decision on how draw mass is reported for an N-team prediction. That is design work, not cleanup. - **5.** `one_v_one` still hardcodes `GameOptions::default()`. Fixing it means changing the signature. Less urgent than it was: passing `Outcome::draw(2)` there now returns a clean `TieWithoutDrawProbability` error rather than NaN (#8), so the trap is visible. - **6.** `Observer::on_batch_processed` is still never called, and still carries the pre-T2 "batch" name. Whether to wire it up or delete it is a call about what the observer contract should be. - **7.** `factor` vs `factors` module naming, unchanged. `predict_quality` now documents its panics rather than silently dropping unknown keys — a stopgap until it can return `Result` alongside `predict_outcome`.

logaritmisk referenced this issue

2026-09-07 12:21:22 +00:00

No way to ask "which comparison should I run next?" — active-learning / information-gain primitives are absent #39

logaritmisk referenced this issue

2026-09-07 12:29:09 +00:00

No way to ask "which comparison should I run next?" — active-learning / information-gain primitives are absent #39

logaritmisk referenced this issue from a commit

2026-09-07 12:59:46 +00:00

feat!: N-team outcome prediction with draw mass, replacing the 2-team panic

logaritmisk referenced this issue from a commit

2026-09-07 12:59:46 +00:00

refactor!: close the remaining API gaps from #21