Merge api/retire-index (#73)

README.md

@@ -330,7 +330,7 @@ stay available at any size:
 
 Unknown keys are an error by default, not a silent omission: a team the history
 has never seen cannot produce a confident-looking probability. The error names
-the key, and every key must already be known — pre-filter with `lookup` or
+the key, and every key must already be known — pre-filter with
 `current_skill` if your caller cannot guarantee that.
 
 If predicting for competitors you have never seen is the point rather than a

src/error.rs

@@ -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

@@ -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

@@ -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
     }
 }

tests/api_shape.rs

@@ -41,9 +41,9 @@ fn add_events_bulk_via_iter() {
     h.add_events(events).unwrap();
     let report = h.converge().unwrap();
     assert!(report.converged);
-    assert!(h.lookup(&"a").is_some());
-    assert!(h.lookup(&"b").is_some());
-    assert!(h.lookup(&"c").is_some());
+    assert!(h.current_skill("a").is_some());
+    assert!(h.current_skill("b").is_some());
+    assert!(h.current_skill("c").is_some());
 }
 
 #[test]
@@ -103,9 +103,9 @@ fn fluent_event_builder_basic() {
 
     let report = h.converge().unwrap();
     assert!(report.converged);
-    assert!(h.lookup(&"alice").is_some());
-    assert!(h.lookup(&"bob").is_some());
-    assert!(h.lookup(&"carol").is_some());
+    assert!(h.current_skill("alice").is_some());
+    assert!(h.current_skill("bob").is_some());
+    assert!(h.current_skill("carol").is_some());
 }
 
 #[test]

tests/key_ergonomics.rs

@@ -77,17 +77,18 @@ fn linear_combinations_take_bare_keys() {
     );
 }
 
+/// `lookup` is gone with `Index` (#73); the accessors that answer the same
+/// question all take a borrowed key.
 #[test]
-fn lookup_accepts_a_borrowed_key_like_its_neighbours() {
-    // `lookup` carried `ToOwned<Owned = K>`, copy-pasted from `intern`, which
-    // genuinely needs it to create the entry. `lookup` never creates.
+fn membership_queries_accept_a_borrowed_key() {
     let h = owned();
-    assert!(h.lookup("alice").is_some());
-    assert!(h.lookup("nobody").is_none());
-
-    // Control: its neighbours already accepted this and must still.
     assert!(h.current_skill("alice").is_some());
     assert!(h.rating("alice").is_some());
+    assert!(h.learning_curve("alice").is_some());
+
+    assert!(h.current_skill("nobody").is_none());
+    assert!(h.rating("nobody").is_none());
+    assert!(h.learning_curve("nobody").is_none());
 }
 
 #[test]

tests/record_winner.rs

@@ -17,24 +17,32 @@ fn record_winner_builds_history() {
     h.record_winner(&"alice", &"bob", 1).unwrap();
     let _ = h.converge().unwrap();
 
-    let a_idx = h.lookup(&"alice").unwrap();
-    let b_idx = h.lookup(&"bob").unwrap();
-
-    assert_ne!(a_idx, b_idx);
+    // `lookup` returned an `Index` that nothing public accepted, so the
+    // observable claim is the one worth making: two distinct competitors, each
+    // with their own posterior, and the winner ahead.
+    assert_eq!(h.competitor_count(), 2);
+    let alice = h.current_skill("alice").expect("alice played");
+    let bob = h.current_skill("bob").expect("bob played");
+    assert!(alice.mu() > bob.mu());
 }
 
+/// The same key names the same competitor across events, which is what
+/// interning bought and the only part of it a caller can observe.
 #[test]
-fn intern_is_idempotent() {
+fn a_repeated_key_is_one_competitor() {
     let mut h: History = History::builder().build();
-    let a1 = h.intern(&"alice");
-    let a2 = h.intern(&"alice");
-    assert_eq!(a1, a2);
+    h.record_winner(&"alice", &"bob", 1).unwrap();
+    h.record_winner(&"alice", &"carol", 2).unwrap();
+
+    assert_eq!(h.competitor_count(), 3);
+    assert_eq!(h.learning_curve("alice").expect("known").len(), 2);
 }
 
 #[test]
-fn lookup_returns_none_for_missing() {
+fn an_unknown_key_is_unknown() {
     let h: History = History::builder().build();
-    assert!(h.lookup(&"nobody").is_none());
+    assert!(h.current_skill("nobody").is_none());
+    assert!(h.learning_curve("nobody").is_none());
 }
 
 #[test]
@@ -50,6 +58,6 @@ fn record_draw_with_p_draw_set() {
     h.record_draw(&"alice", &"bob", 1).unwrap();
     let _ = h.converge().unwrap();
 
-    assert!(h.lookup(&"alice").is_some());
-    assert!(h.lookup(&"bob").is_some());
+    assert!(h.current_skill("alice").is_some());
+    assert!(h.current_skill("bob").is_some());
 }