Commit Graph
5 Commits
Author SHA1 Message Date
logaritmiskandClaude Opus 5 b553c630f5 refactor!: K comes first in History, HistoryBuilder and Joint
`K` is the one type parameter people change, and it was last. Naming a
history in a struct field meant writing all four to say one thing:

    struct Ladder { history: History<i64, ConstantDrift, NullObserver, String> }
    struct Analysis<'h> { joint: Joint<'h, i64, ConstantDrift, NullObserver, &'static str> }

Now:

    struct Ladder { history: History<String> }
    struct Analysis<'h> { joint: Joint<'h> }

`History<K, T, D, O>`, all four defaulted. Bounds may reference later
parameters, so `D: Drift<T> = ConstantDrift` is legal in third position.
`Joint` gains the same defaults, so `Joint<'h, String>` spells it.

72 call sites swapped, and the reorder makes most of them shorter: 18
now read `History<String>` and the `&'static str` ones read `History`.
The two turbofished builders shrink from
`HistoryBuilder::<Untimed, _, _, String>::new()` to
`HistoryBuilder::<String, Untimed>::new()`.

`Joint` keeps `O` structurally, defaulted rather than removed. #72 notes
it never touches the observer, which is true — but it borrows the whole
`&'h History<K, T, D, O>` and calls `History::resolve_terms`, so dropping
the parameter means either a view type or moving that method off
`History`. The default already buys the entire user-visible benefit,
which was the spelling.

Refs #72.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-10 06:49:22 +02:00
logaritmiskandClaude Opus 5 e72bf3894c refactor!: the joint is reached through Joint, not mirrored on History
`posterior_of`, `posterior_of_at` and `expected_variance_reduction`
existed twice: once on `Joint`, and once on `History` as one-shot
wrappers whose whole body was `self.joint()?.<same>(..)`.

The wrappers re-factorised on every call — their own docs said so,
warning the reader to take a `Joint` instead — and they were what
smuggled the scored-only precondition onto the flat surface. A user
following the quickstart builds a ranked history, sees `posterior_of` in
the method list, and it never works. `h.joint()?.posterior_of(..)` is
one call longer and tells the truth: you need a joint, and a joint needs
a scored history.

That leaves three tiers instead of a flat surface with a hidden
precondition: `History` fits and reads, `predict_*` forecasts, `Joint`
answers exact joint questions.

`predict_margin` was itself calling `self.posterior_of`; it goes through
`self.joint()?` directly now.

The `Joint` methods' docs referred back to the wrappers for their real
content ("Identical to `History::posterior_of`, without re-paying the
factorisation"), so they now carry it: what a linear functional means,
which appearance each competitor is read at, and why
`expected_variance_reduction` belongs on the handle.

`tests/joint_handle.rs` had three tests comparing the wrapper against
the handle. That comparison is gone, but the property behind it is not —
they now compare a *reused* joint against a *fresh* one per question,
which is the actual correctness claim behind caching the factorisation
(#51), without the wrapper in the middle.

Closes #78.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-10 06:40:31 +02:00
logaritmiskandClaude Opus 5 13a395fdc9 refactor!: scores_with_noise, and History::quality
Two names that described the wrong thing.

`scores_with_sigma(scores, sigma)` reads as "these scores have prior
sigma 2.0". The quantity is observation noise on the score *margin*, in
the units of the scores, and it is spelled `score_sigma` at every config
site — `HistoryBuilder::score_sigma`, `GameOptions::score_sigma`,
`EventKind::Scored { score_sigma }` — so this was the one place the
crate used a third meaning of "sigma" for it. Its own doc had to
disambiguate itself: "`sigma` overrides `HistoryBuilder::score_sigma`".
`scores_with_noise(scores, score_sigma)` on both `Outcome` and
`EventBuilder`.

`predict_quality` predicts nothing. Its own doc says it answers "is this
matchup *fair*", not "what will happen", and the `predict_*` family is
otherwise exactly the methods returning a probability or a distribution
over outcomes. `History::quality` also makes the free/method pair
consistent: free `quality` pairs with `History::quality` the way free
`expected_information_gain` already pairs with
`History::expected_information_gain`. The rule that was already being
followed and never stated — a free function scores a hypothetical from
explicit parameters, the same-named method asks it against the fit — is
now written on the method.

Closes #75. Refs #78 (part 4).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-09 23:23:12 +02:00
logaritmiskandClaude Opus 5 faa25fb3b1 refactor!: retire Index, intern and lookup
`Index` was public, `History::intern` and `History::lookup` returned
one, and no public method anywhere accepted one. It was a handle with
nowhere to go — and `key_table.rs` advertised the hot-path story it was
meant to enable ("power users can promote `&K` to `Index` and skip the
lookup"), which was never reachable through the public API.

It also shadowed `std::ops::Index`, which `CompetitorStore` implements,
so `use trueskill_tt::*` alongside `use std::ops::*` collided.

All three are `pub(crate)` now. `intern` stays internal because
ingestion needs it; `lookup` is gone entirely, since `current_skill`,
`rating` and `learning_curve` already answer "does this history know
this key" and all three take a borrowed key.

The three tests that used them asserted things a caller cannot observe.
They now assert what the interning bought:

- `record_winner_creates_two_competitors` compares posteriors instead of
  comparing two opaque indices for inequality.
- `intern_is_idempotent` becomes `a_repeated_key_is_one_competitor` — a
  key appearing in two events gives one competitor with a two-point
  learning curve, which is the observable form of the same claim.
- `lookup_returns_none_for_missing` becomes `an_unknown_key_is_unknown`.

Closes #73.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
2026-09-09 23:18:54 +02:00
logaritmiskandClaude Opus 5 92ae5fca17 feat!: prediction and joint queries take borrowed keys
`&[&[&K]]` was the worst shape in the API. At `K = String` — the
realistic case, where names arrive owned from a database or CSV — a
string literal was *impossible*, and asking "who wins" cost six lines
and four allocations of temporaries that all had to outlive the call:

    let ta = vec![a.to_string()];
    let ra: Vec<&String> = ta.iter().collect();
    ...
    self.history.predict_win_probabilities(&teams)

All seven `predict_*` / `expected_*` methods, `posterior_of`,
`posterior_of_at` and the `Joint` mirrors are now generic over the
borrowed key, the same way `current_skill` and `learning_curve` already
were. `member_skills` and `resolve_terms` only ever did two things with
a key — `keys.get` and `format!("{key:?}")` — and neither needed `K`.

    h.predict_win_probabilities(&[&["alice"], &["bob"]])   // K = String
    h.predict_win_probabilities(&[&["alice"], &["bob"]])   // K = &'static str
    h.posterior_of(&[("alice", 1.0), ("bob", -1.0)])

One spelling for both key types, and `K: Debug` becomes `Q: Debug`, so a
key type no longer has to be `Debug` to run a prediction. The old
`&[&[&"a"]]` spelling still compiles at the default key type, where `Q`
infers to `&str` and the two shapes coincide.

The one cost: `predict_outcome(&[])` can no longer infer `Q` — nothing
in an empty slice names it. It needs an annotation, and only on that
degenerate call.

`lookup` carried `ToOwned<Owned = K>`, copy-pasted from `intern`, which
genuinely needs it to create the entry. `lookup` never creates, and its
five neighbours all accept `h.f("alice")` already. Dropping the bound
strictly widens what compiles.

`HistoryBuilder::gamma` is shorthand for `.drift(ConstantDrift::new(g))`.
Drift is the most-tuned parameter after `sigma` and `GAMMA` is a public
constant, but setting it meant first discovering `ConstantDrift`, a type
a caller has no other reason to name. On the `ConstantDrift` builder
only, since `gamma` is that model's parameter rather than something
every `Drift` has, and rejecting a negative value for the same reason as
`sigma` and `beta`: it enters squared.

Refs #72 (items 2 and 3, plus the gamma shorthand; the type-parameter
reorder in item 1 is still open).

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