logaritmisk/trueskill-tt
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Activity

refactor!: retire Index, intern and lookup

Browse Source 
`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
This commit is contained in:
logaritmisk
2026-09-09 23:18:54 +02:00
co-authored by Claude Opus 5
parent ddbac87744
commit faa25fb3b1
7 changed files with 57 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/error.rs
+2 -2
View File
@@ -155,7 +155,7 @@ pub enum InferenceError {
/// when a competitor's configuration is a property of the domain.
#[non_exhaustive]
ConflictingCompetitorConfig {
/// The competitor's interned [`Index`](crate::Index) as a raw `usize`,
/// The competitor's interned slot as a raw `usize`,
/// not the user key — the batch is already flattened to indices by the
/// time the conflict is detectable.
competitor: usize,
@@ -323,7 +323,7 @@ impl fmt::Display for InferenceError {
f,
"team {team}, member {member}: no skill recorded for key {key} \
(every key must already be known to the history; pre-filter \
with `lookup` or `current_skill` if that is not guaranteed)"
with `current_skill` if that is not guaranteed)"
)
}
Self::AlreadyRegistered { key } => {
src/history.rs
+9 -22
View File
@@ -538,29 +538,16 @@ impl<T: Time, K: Eq + Hash + Clone> HistoryBuilder<T, ConstantDrift, NullObserve
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O, K> {
/// Promote a key to its [`Index`], creating the entry if it is new.
///
/// Interning a key does not register a competitor or give them a rating —
/// it only reserves the slot. Use [`History::register`] to declare a
/// competitor's configuration up front.
pub fn intern<Q>(&mut self, key: &Q) -> Index
/// Crate-internal since #73: interning reserves a storage slot and nothing
/// more, and no public method ever accepted the handle it returned.
/// [`History::register`] is the public way to declare a competitor.
pub(crate) fn intern<Q>(&mut self, key: &Q) -> Index
where
K: Borrow<Q>,
Q: Hash + Eq + ToOwned<Owned = K> + ?Sized,
{
self.keys.get_or_create(key)
}
/// Resolve an existing key to its [`Index`], or `None` if the history has
/// never seen it.
///
/// The read-only counterpart of [`History::intern`]: it never creates.
#[must_use]
pub fn lookup<Q>(&self, key: &Q) -> Option<Index>
where
K: Borrow<Q>,
Q: Hash + Eq + ?Sized,
{
self.keys.get(key)
}
}
impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O, K> {
@@ -1328,7 +1315,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// Every key must already be known to the history — that is, must have
/// appeared in an ingested event. An unknown key is `UnknownKey`, not a
/// silently dropped member. If your caller cannot guarantee that, pre-filter
/// with [`History::lookup`] or [`History::current_skill`]; treating the
/// with [`History::current_skill`] or [`History::competitors`]; treating the
/// error as "no information" and substituting a neutral value turns a
/// whole-team miss into a plausible constant, which is invisible to any
/// test that does not assert on variation.
@@ -1849,7 +1836,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// Every key must already be known to the history — that is, must have
/// appeared in an ingested event. An unknown key is `UnknownKey`, not a
/// silently dropped member. If your caller cannot guarantee that, pre-filter
/// with [`History::lookup`] or [`History::current_skill`]; treating the
/// with [`History::current_skill`] or [`History::competitors`]; treating the
/// error as "no information" and substituting a neutral value turns a
/// whole-team miss into a plausible constant, which is invisible to any
/// test that does not assert on variation.
@@ -1913,7 +1900,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// Every key must already be known to the history — that is, must have
/// appeared in an ingested event. An unknown key is `UnknownKey`, not a
/// silently dropped member. If your caller cannot guarantee that, pre-filter
/// with [`History::lookup`] or [`History::current_skill`]; treating the
/// with [`History::current_skill`] or [`History::competitors`]; treating the
/// error as "no information" and substituting a neutral value turns a
/// whole-team miss into a plausible constant, which is invisible to any
/// test that does not assert on variation.
@@ -1966,7 +1953,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// Every key must already be known to the history — that is, must have
/// appeared in an ingested event. An unknown key is `UnknownKey`, not a
/// silently dropped member. If your caller cannot guarantee that, pre-filter
/// with [`History::lookup`] or [`History::current_skill`]; treating the
/// with [`History::current_skill`] or [`History::competitors`]; treating the
/// error as "no information" and substituting a neutral value turns a
/// whole-team miss into a plausible constant, which is invisible to any
/// test that does not assert on variation.
@@ -2016,7 +2003,7 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
/// Every key must already be known to the history — that is, must have
/// appeared in an ingested event. An unknown key is `UnknownKey`, not a
/// silently dropped member. If your caller cannot guarantee that, pre-filter
/// with [`History::lookup`] or [`History::current_skill`]; treating the
/// with [`History::current_skill`] or [`History::competitors`]; treating the
/// error as "no information" and substituting a neutral value turns a
/// whole-team miss into a plausible constant, which is invisible to any
/// test that does not assert on variation.
src/lib.rs
+11 -12
View File
@@ -290,23 +290,22 @@ pub(crate) const N_INF: Gaussian = Gaussian::from_ms(0.0, f64::INFINITY);
/// An interned competitor handle: a dense slot number, not a user key.
///
/// [`History`] stores skills and messages by `Index` rather than by `K`, so
/// the hot path never hashes a key. [`History::intern`] promotes a key to one
/// and [`History::lookup`] resolves an existing key without creating.
/// `History` stores skills and messages by `Index` rather than by `K`, so the
/// hot path never hashes a key. Indices are assigned in interning order and
/// are stable for the life of a history; they are not portable between
/// histories, since the same key interns to a different slot under a different
/// ingestion order.
///
/// Indices are assigned in interning order and are stable for the life of a
/// history. They are **not** portable between histories: the same key interns
/// to different slots depending on ingestion order.
/// Crate-internal. It was public, along with `History::intern` and
/// `History::lookup` that produced one — and **nothing public ever accepted
/// one**, so it was a handle with nowhere to go. It also shadowed
/// `std::ops::Index`, which `CompetitorStore` implements. See #73.
#[derive(Copy, Clone, Default, PartialEq, PartialOrd, Eq, Ord, Hash, Debug)]
pub struct Index(usize);
pub(crate) struct Index(usize);
impl Index {
/// The underlying slot number.
///
/// Indices are dense and assigned in interning order, so this is usable as
/// a key into a caller-side side table.
#[must_use]
pub fn get(self) -> usize {
pub(crate) fn get(self) -> usize {
self.0
}
}