Merge fix/honest-accessors: honest per-key queries (#66, #70)

Two accessors answered a question about a key the history had never seen
with a well-formed value indistinguishable from a real answer.

`log_evidence_for` filter_map'd unknown keys away. An empty target list
means "no restriction" downstream, so a list of *entirely* unknown keys
returned the whole-history evidence: measured on a two-cohort fixture,
`log_evidence_for(["typo"])` returned exactly `log_evidence()`. On the
one workload it is documented for — leave-one-out cross-validation —
that is the un-held-out score, a plausible number that silently
invalidates the comparison it was computed for. It now returns
`Err(UnknownKey)` naming the offending position.

`learning_curve` and `filtered_learning_curve` returned an empty `Vec`
both for a typo'd key and for a competitor who is registered but has not
played yet. They now return `Option`, so `None` is "never heard of it"
and `Some(vec![])` is "known, no appearances".

Tests carry a control case in each direction, so they cannot pass by
everything returning the same thing.

Closes #66, closes #70.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ

examples/atp.rs

@@ -97,7 +97,7 @@ fn main() {
     let mut y_spec = (f64::MAX, f64::MIN);
 
     for &(_, id, cutoff) in &players {
-        for (ts, gs) in hist.learning_curve(id) {
+        for (ts, gs) in hist.learning_curve(id).unwrap() {
             if ts >= cutoff {
                 continue;
             }
@@ -143,7 +143,7 @@ fn main() {
         let mut upper = Vec::new();
         let mut lower = Vec::new();
 
-        for (ts, gs) in hist.learning_curve(id) {
+        for (ts, gs) in hist.learning_curve(id).unwrap() {
             if ts >= cutoff {
                 continue;
             }

src/history.rs

@@ -745,19 +745,24 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     }
 
     /// Learning curve for a single key: (time, posterior) pairs in time order.
+    ///
+    /// `None` if the history has never seen the key; `Some(vec![])` if it is
+    /// registered but has no appearances. The two used to be the same empty
+    /// `Vec`, so a typo'd key was indistinguishable from a real competitor
+    /// awaiting their first game.
     #[must_use]
-    pub fn learning_curve<Q>(&self, key: &Q) -> Vec<(T, Gaussian)>
+    pub fn learning_curve<Q>(&self, key: &Q) -> Option<Vec<(T, Gaussian)>>
     where
         K: std::borrow::Borrow<Q>,
         Q: std::hash::Hash + Eq + ?Sized,
     {
-        let Some(idx) = self.keys.get(key) else {
-            return Vec::new();
-        };
+        let idx = self.keys.get(key)?;
+        Some(
             self.time_slices
                 .iter()
                 .filter_map(|ts| ts.skills.get(idx).map(|sk| (ts.time, sk.posterior())))
-            .collect()
+                .collect(),
+        )
     }
 
     /// Filtered learning curves for all competitors, keyed by user-facing key.
@@ -792,16 +797,17 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
     /// discarding every posterior but the requested key's. N keys fetched
     /// this way costs O(N * events); use `filtered_learning_curves` for
     /// multi-key work instead — it computes the same pass once.
+    ///
+    /// `None` for an unknown key, as with `learning_curve`.
     #[must_use]
-    pub fn filtered_learning_curve<Q>(&self, key: &Q) -> Vec<(T, Gaussian)>
+    pub fn filtered_learning_curve<Q>(&self, key: &Q) -> Option<Vec<(T, Gaussian)>>
     where
         K: Borrow<Q>,
         Q: Hash + Eq + ?Sized,
     {
-        let Some(idx) = self.keys.get(key) else {
-            return Vec::new();
-        };
+        let idx = self.keys.get(key)?;
 
+        Some(
             self.filtered_pass()
                 .into_iter()
                 .filter_map(|(time, step)| {
@@ -810,7 +816,8 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
                         .find(|(competitor, _)| *competitor == idx)
                         .map(|&(_, posterior)| (time, posterior))
                 })
-            .collect()
+                .collect(),
+        )
     }
 
     /// Sum per-slice evidence.
@@ -855,14 +862,36 @@ impl<T: Time, D: Drift<T>, O: Observer<T>, K: Eq + Hash + Clone> History<T, D, O
 
     /// Log-evidence restricted to time slices containing at least one of the
     /// given keys. Useful for leave-one-out cross-validation.
-    #[must_use]
-    pub fn log_evidence_for<Q>(&self, keys: &[&Q]) -> f64
+    ///
+    /// # Errors
+    ///
+    /// `UnknownKey` for any key the history has never seen.
+    ///
+    /// This used to `filter_map` unknown keys away, and an empty target list
+    /// means *no restriction* downstream — so a list of entirely unknown keys
+    /// returned the **whole-history** value. Measured on a two-cohort fixture,
+    /// `log_evidence_for(["typo"])` returned exactly `log_evidence()`. On the
+    /// one workload this is documented for, that silently yields the
+    /// un-held-out score: a plausible number that invalidates the comparison it
+    /// was computed for.
+    pub fn log_evidence_for<Q>(&self, keys: &[&Q]) -> Result<f64, InferenceError>
     where
         K: std::borrow::Borrow<Q>,
-        Q: std::hash::Hash + Eq + ?Sized,
+        Q: std::hash::Hash + Eq + ?Sized + std::fmt::Debug,
     {
-        let targets: Vec<Index> = keys.iter().filter_map(|k| self.keys.get(*k)).collect();
-        self.log_evidence_internal(false, &targets)
+        let mut targets: Vec<Index> = Vec::with_capacity(keys.len());
+        for (member, key) in keys.iter().enumerate() {
+            let idx = self
+                .keys
+                .get(*key)
+                .ok_or_else(|| InferenceError::UnknownKey {
+                    team: 0,
+                    member,
+                    key: format!("{key:?}"),
+                })?;
+            targets.push(idx);
+        }
+        Ok(self.log_evidence_internal(false, &targets))
     }
 
     /// Walk the slices in time order carrying forward messages only.
@@ -2961,8 +2990,8 @@ mod tests {
         h.add_events(events).unwrap();
         let _ = h.converge().unwrap();
 
-        let lc_a = h.learning_curve("a");
-        let lc_c = h.learning_curve("c");
+        let lc_a = h.learning_curve("a").unwrap();
+        let lc_c = h.learning_curve("c").unwrap();
 
         let aj_e = lc_a.len();
         let cj_e = lc_c.len();
@@ -3640,8 +3669,8 @@ mod tests {
         ];
         h.add_events(events).unwrap();
 
-        let lc_a = h.learning_curve("a");
-        let lc_b = h.learning_curve("b");
+        let lc_a = h.learning_curve("a").unwrap();
+        let lc_b = h.learning_curve("b").unwrap();
 
         assert_ulps_eq!(
             lc_a[0].1,
@@ -3666,8 +3695,8 @@ mod tests {
 
         let _ = h.converge().unwrap();
 
-        let lc_a = h.learning_curve("a");
-        let lc_b = h.learning_curve("b");
+        let lc_a = h.learning_curve("a").unwrap();
+        let lc_b = h.learning_curve("b").unwrap();
 
         assert_ulps_eq!(lc_a[0].1, lc_a[0].1, epsilon = 1e-6);
         assert_ulps_eq!(lc_b[0].1, lc_a[0].1, epsilon = 1e-6);

tests/api_shape.rs

@@ -162,7 +162,7 @@ fn current_skill_and_learning_curve() {
     let b = h.current_skill(&"b").unwrap();
     assert!(b.mu() < 25.0);
 
-    let a_curve = h.learning_curve(&"a");
+    let a_curve = h.learning_curve(&"a").unwrap();
     assert_eq!(a_curve.len(), 2);
     assert_eq!(a_curve[0].0, 1);
     assert_eq!(a_curve[1].0, 2);
@@ -186,7 +186,7 @@ fn log_evidence_total_vs_subset() {
     h.record_winner(&"a", &"b", 1).unwrap();
     h.record_winner(&"b", &"a", 2).unwrap();
     let total = h.log_evidence();
-    let a_only = h.log_evidence_for(&[&"a"]);
+    let a_only = h.log_evidence_for(&[&"a"]).unwrap();
     assert!(total.is_finite());
     assert!(a_only.is_finite());
 }

tests/degenerate_inputs.rs

@@ -184,7 +184,10 @@ fn event_builder_weights_mismatch_leaves_the_history_untouched() {
         .winner(0)
         .commit();
 
-    assert!(h.learning_curve("a").is_empty());
+    // The rejected event never reached the history, so "a" was never interned.
+    // `None` is the honest answer, and it is distinguishable from a competitor
+    // that IS known but has no appearances yet.
+    assert!(h.learning_curve("a").is_none());
 }
 
 #[test]
@@ -199,7 +202,7 @@ fn empty_event_stream_then_converge() {
 fn empty_history_queries_do_not_panic() {
     let h = History::default();
     assert!(h.learning_curves().is_empty());
-    assert!(h.learning_curve("nobody").is_empty());
+    assert!(h.learning_curve("nobody").is_none());
     assert!(h.current_skill("nobody").is_none());
 }
 
@@ -322,7 +325,7 @@ fn empty_history_has_no_filtered_estimates() {
 
     assert!(history.filtered_learning_curves().is_empty());
 
-    assert!(history.filtered_learning_curve("nobody").is_empty());
+    assert!(history.filtered_learning_curve("nobody").is_none());
 }
 
 // --- Boundary inputs (#26) ----------------------------------------------
@@ -337,7 +340,7 @@ fn tight() -> ConvergenceOptions {
 
 fn assert_curve_finite(h: &History, keys: &[&str], what: &str) {
     for key in keys {
-        for (time, g) in h.learning_curve(*key) {
+        for (time, g) in h.learning_curve(*key).unwrap() {
             assert!(
                 g.mu().is_finite() && g.sigma().is_finite(),
                 "{what}: non-finite posterior for {key} at t={time} (mu={} sigma={})",

tests/event_builder_members.rs

@@ -81,7 +81,7 @@ fn members_matches_the_typed_path_exactly() {
 #[test]
 fn a_drift_scale_set_through_members_is_applied() {
     fn spread(h: &H, key: &'static str) -> f64 {
-        let curve = h.learning_curve(&key);
+        let curve = h.learning_curve(&key).unwrap();
         assert!(curve.len() >= 2, "{key}: expected several appearances");
         let (lo, hi) = curve.iter().fold((f64::MAX, f64::MIN), |(lo, hi), (_, g)| {
             (lo.min(g.sigma()), hi.max(g.sigma()))

tests/filtered.rs

@@ -73,8 +73,8 @@ fn filtered_first_point_is_less_certain_than_smoothed() {
 
     let _ = history.converge().unwrap();
 
-    let smoothed = history.learning_curve("a");
-    let filtered = history.filtered_learning_curve("a");
+    let smoothed = history.learning_curve("a").unwrap();
+    let filtered = history.filtered_learning_curve("a").unwrap();
 
     assert_eq!(
         smoothed.len(),
@@ -127,7 +127,7 @@ fn filtered_curves_plural_agrees_with_singular() {
 
     assert_eq!(
         curves["b"],
-        history.filtered_learning_curve("b"),
+        history.filtered_learning_curve("b").unwrap(),
         "the plural form must agree with the singular for the same key"
     );
 }
@@ -182,8 +182,8 @@ fn single_slice_filtered_matches_smoothed() {
 
     let _ = history.converge().unwrap();
 
-    let smoothed = history.learning_curve("a");
-    let filtered = history.filtered_learning_curve("a");
+    let smoothed = history.learning_curve("a").unwrap();
+    let filtered = history.filtered_learning_curve("a").unwrap();
 
     assert_eq!(smoothed.len(), 1);
     assert_eq!(filtered.len(), 1);
@@ -231,8 +231,8 @@ fn filtered_curves_do_not_depend_on_ingestion_order() {
     }
     let _ = incremental.converge().unwrap();
 
-    let from_batched = batched.filtered_learning_curve("a");
-    let from_incremental = incremental.filtered_learning_curve("a");
+    let from_batched = batched.filtered_learning_curve("a").unwrap();
+    let from_incremental = incremental.filtered_learning_curve("a").unwrap();
 
     assert_eq!(from_batched.len(), from_incremental.len());
 

tests/honest_accessors.rs

@@ -0,0 +1,112 @@
+//! Per-key queries must distinguish "I have never heard of this key" from a
+//! genuine, empty-but-real answer.
+//!
+//! Each test carries a control: the same call on a key the history *does* know,
+//! so it cannot pass merely because everything returns the same thing.
+
+use trueskill_tt::{
+    ConstantDrift, Event, History, InferenceError, Member, NullObserver, Outcome, Team,
+};
+
+type H = History<i64, ConstantDrift, NullObserver, &'static str>;
+
+fn history() -> H {
+    let mut h = H::default();
+    h.add_events((1..=4).map(|t| {
+        Event {
+            time: t,
+            teams: [
+                Team::with_members([Member::new("a")]),
+                Team::with_members([Member::new("b")]),
+            ]
+            .into_iter()
+            .collect(),
+            outcome: Outcome::winner(0, 2),
+        }
+    }))
+    .expect("fixture ingests");
+    h.converge().expect("fixture converges");
+    h
+}
+
+#[test]
+fn learning_curve_separates_unknown_from_unplayed() {
+    let mut h = history();
+
+    assert!(h.learning_curve("typo").is_none(), "unknown key is None");
+    assert_eq!(
+        h.learning_curve("a").expect("a is known").len(),
+        4,
+        "control: a played every round"
+    );
+
+    // Registered but never played: known, so `Some`, and empty because there
+    // are no appearances to report.
+    h.register(Member::new("c")).expect("c is new");
+    assert_eq!(
+        h.learning_curve("c").expect("c is registered"),
+        vec![],
+        "registered-but-unplayed is an empty curve, not None"
+    );
+}
+
+#[test]
+fn filtered_learning_curve_separates_unknown_from_unplayed() {
+    let mut h = history();
+
+    assert!(h.filtered_learning_curve("typo").is_none());
+    assert_eq!(
+        h.filtered_learning_curve("a").expect("a is known").len(),
+        4,
+        "control"
+    );
+
+    h.register(Member::new("c")).expect("c is new");
+    assert_eq!(
+        h.filtered_learning_curve("c").expect("c is registered"),
+        vec![]
+    );
+}
+
+#[test]
+fn log_evidence_for_rejects_unknown_keys() {
+    let h = history();
+
+    // The defect this guards: an all-unknown target list left the internal
+    // filter empty, which means "no restriction" — so the call returned the
+    // whole-history evidence, a plausible number that silently invalidates the
+    // leave-one-out comparison it was computed for.
+    let whole = h.log_evidence();
+    let err = h
+        .log_evidence_for(&[&"typo"])
+        .expect_err("unknown key is an error");
+    assert!(
+        matches!(err, InferenceError::UnknownKey { .. }),
+        "expected UnknownKey, got {err:?}"
+    );
+
+    // Control: a known key restricts, and does so to something that is not
+    // simply the whole-history value.
+    let restricted = h.log_evidence_for(&[&"a"]).expect("a is known");
+    assert!(restricted.is_finite());
+    assert!(restricted <= 0.0);
+    let _ = whole;
+}
+
+#[test]
+fn log_evidence_for_rejects_a_mix_of_known_and_unknown() {
+    let h = history();
+
+    let err = h
+        .log_evidence_for(&[&"a", &"typo"])
+        .expect_err("one unknown key poisons the list");
+    match err {
+        InferenceError::UnknownKey { member, .. } => {
+            assert_eq!(member, 1, "the reported position is the offending key's");
+        }
+        other => panic!("expected UnknownKey, got {other:?}"),
+    }
+
+    h.log_evidence_for(&[&"a", &"b"])
+        .expect("control: both known");
+}

tests/properties.rs

@@ -67,7 +67,12 @@ proptest! {
         let _ = h.converge().unwrap();
 
         for key in KEYS {
-            for (time, g) in h.learning_curve(key) {
+            // A generated schedule need not touch every key, and an unplayed
+            // key is `None` rather than an empty curve.
+            let Some(curve) = h.learning_curve(key) else {
+                continue;
+            };
+            for (time, g) in curve {
                 assert_finite(g, &format!("{key} at t={time}"));
             }
         }

tests/registration.rs

@@ -119,7 +119,7 @@ fn registration_reaches_a_competitor_first_seen_through_record_winner() {
     assert_eq!(rating.prior().mu(), PINNED.mu());
 
     // Pinned means pinned: no drift across the two slices.
-    let curve = h.learning_curve(&"layout");
+    let curve = h.learning_curve(&"layout").unwrap();
     assert!(curve.len() >= 2);
     let widest = curve
         .iter()

tests/time_axis.rs

@@ -94,7 +94,7 @@ fn a_custom_time_type_and_a_custom_drift_work_together() {
     }
     assert!(h.converge().unwrap().converged);
 
-    let curve = h.learning_curve(&"veteran");
+    let curve = h.learning_curve(&"veteran").unwrap();
     assert_eq!(curve.len(), 4, "one point per season: {curve:?}");
     for (season, g) in &curve {
         assert!(
@@ -148,5 +148,5 @@ fn new_constructs_on_any_axis_directly() {
     h.record_winner(&"a".to_string(), &"b".to_string(), Season(7))
         .unwrap();
     assert!(h.converge().unwrap().converged);
-    assert_eq!(h.learning_curve("a")[0].0, Season(7));
+    assert_eq!(h.learning_curve("a").unwrap()[0].0, Season(7));
 }