Ergonomics: the first program a new user writes does not compile, and the fix is not discoverable #72

New Issue

Closed

opened 2026-09-09 17:55:27 +00:00 by logaritmisk · 2 comments

logaritmisk commented 2026-09-09 17:55:27 +00:00

Owner

Copy Link

Copy Source

Two changes would take the realistic "fit, query, predict, store" program from unusable-without-reading-source to obvious. Both were prototyped and compile.

The headline

let rows: Vec<(String, String, bool)> = load_csv();   // names arrive owned
let mut history = History::default();
history.record_winner(w, l, i as i64)?;

error[E0277]: the trait bound `&str: Borrow<String>` is not satisfied
error[E0271]: type mismatch resolving `<String as ToOwned>::Owned == &str`

Nothing there names the cause: History::default() silently pinned K = &'static str. The fix, after History::builder_with_key::<String>() also fails (E0107 — K is on the impl, not the fn, so it cannot be turbofished), is:

let mut history = History::<i64, _, _, String>::builder_with_key().build();

And at the default key type the natural call is also wrong — h.record_winner("alice", "bob", 1) fails; you must write &"alice".

1. K is the only parameter people change, and it is last

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

Four parameters named, three of them defaults, to change the one that matters. Reordering K first was prototyped and compiles — bounds may reference later parameters, so D: Drift<T> = ConstantDrift is legal in third position:

pub struct History<
    K: Eq + Hash + Clone = &'static str,
    T: Time = i64,
    D: Drift<T> = ConstantDrift,
    O: Observer<T> = NullObserver,
> { .. }

struct Ladder { history: History<String> }   // the whole spelling

Pair it with a turbofishable setter, mirroring the existing type-changing drift/observer:

impl<K, T, D, O> HistoryBuilder<K, T, D, O> {
    pub fn key<K2: Eq + Hash + Clone>(self) -> HistoryBuilder<K2, T, D, O>;
}

let h = History::builder().key::<String>().p_draw(0.05).build();

That retires builder_with_key (see also #68, which needs the same treatment for T).

Also: Joint carries O but never uses it — nothing in its body touches the observer; it rides along inside &'h History. With the reorder, Joint<'h, K> is the whole spelling.

2. &[&[&K]] is the worst shape in the API, and it is a third vocabulary

The crate has four ways to say "some teams of competitors": typed Event/Team/Member, &[&[&K]] (all seven predict_*/expected_*), &[&[Rating]] (Game, free expected_information_gain), and &[&[Gaussian]] (quality).

At the default key type the guessed spelling fails:

h.predict_win_probabilities(&[&["alice", "bob"]])
// error[E0308]: expected `&&str`, found `&str`

At K = String — the realistic case — string literals are impossible and you must build three levels of temporaries that all outlive the call. This is the real code required to ask "who wins":

fn head_to_head(&self, a: &str, b: &str) -> f64 {
    let ta = vec![a.to_string()];
    let tb = vec![b.to_string()];
    let ra: Vec<&String> = ta.iter().collect();
    let rb: Vec<&String> = tb.iter().collect();
    let teams: Vec<&[&String]> = vec![&ra, &rb];
    self.history.predict_win_probabilities(&teams).unwrap()[0]
}

Six lines and four allocations. The same roster said the ingestion way is .team(names) — one expression.

This costs nothing to fix. member_skills and resolve_terms do exactly two things with a key: self.keys.get(*key) and format!("{key:?}"). Neither needs K itself:

pub fn predict_win_probabilities<Q>(&self, teams: &[&[&Q]]) -> Result<Vec<f64>, InferenceError>
where K: Borrow<Q>, Q: Hash + Eq + Debug + ?Sized;

pub fn posterior_of<Q>(&self, terms: &[(&Q, f64)]) -> Result<Gaussian, InferenceError>
where K: Borrow<Q>, Q: Hash + Eq + Debug + ?Sized;

Prototyped and run. It gives the same natural spelling for both key types:

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)])       // no `&` on keys

A Vec<String> roster then needs one .map(String::as_str) instead of three nested collects. Bonus: K: Debug becomes Q: Debug, so the key type no longer has to be Debug to run a prediction.

3. lookup's bound is copy-paste and strictly too narrow

current_skill, rating, learning_curve, filtered_learning_curve and log_evidence_for all accept h.f("alice"). lookup does not:

error[E0271]: type mismatch resolving `<str as ToOwned>::Owned == &str`
note: required by a bound in `History::lookup`

Its body is self.keys.get(key), and KeyTable::get already has the right bound. The ToOwned<Owned = K> came from intern, which genuinely needs it to create the entry. Dropping it from lookup breaks nothing — it strictly widens what compiles.

Suggested extras

• HistoryBuilder::gamma(f64) shorthand — GAMMA is a public constant and drift is the most-tuned parameter after sigma, but setting it requires discovering ConstantDrift. Must pin D = ConstantDrift, so it belongs on that impl only.
• Not boxing the observer: it makes the type name worse, and the reorder already gets you to History<String> at zero runtime cost. The existing Observer for Arc<O>/Box<O>/&O impls already give runtime choice and work.
• Not an owned Joint: the borrow is correct design doing invalidation work a cache would need by hand, and the factorisation is n² floats.

Breaks: every explicit History<..>/Joint<..> spelling; builder_with_key; call sites that relied on &&K inference lose an &.

Found by an API audit, 2026-09-09. Both prototypes compile.

Two changes would take the realistic "fit, query, predict, store" program from unusable-without-reading-source to obvious. Both were prototyped and compile. ## The headline ```rust let rows: Vec<(String, String, bool)> = load_csv(); // names arrive owned let mut history = History::default(); history.record_winner(w, l, i as i64)?; ``` ``` error[E0277]: the trait bound `&str: Borrow<String>` is not satisfied error[E0271]: type mismatch resolving `<String as ToOwned>::Owned == &str` ``` Nothing there names the cause: `History::default()` silently pinned `K = &'static str`. The fix, after `History::builder_with_key::<String>()` also fails (E0107 — `K` is on the `impl`, not the `fn`, so it cannot be turbofished), is: ```rust let mut history = History::<i64, _, _, String>::builder_with_key().build(); ``` And at the *default* key type the natural call is also wrong — `h.record_winner("alice", "bob", 1)` fails; you must write `&"alice"`. ## 1. `K` is the only parameter people change, and it is last ```rust struct Ladder { history: History<i64, ConstantDrift, NullObserver, String> } struct Analysis<'h> { joint: Joint<'h, i64, ConstantDrift, NullObserver, &'static str> } ``` Four parameters named, three of them defaults, to change the one that matters. Reordering `K` first was prototyped and compiles — bounds may reference later parameters, so `D: Drift<T> = ConstantDrift` is legal in third position: ```rust pub struct History< K: Eq + Hash + Clone = &'static str, T: Time = i64, D: Drift<T> = ConstantDrift, O: Observer<T> = NullObserver, > { .. } struct Ladder { history: History<String> } // the whole spelling ``` Pair it with a turbofishable setter, mirroring the existing type-changing `drift`/`observer`: ```rust impl<K, T, D, O> HistoryBuilder<K, T, D, O> { pub fn key<K2: Eq + Hash + Clone>(self) -> HistoryBuilder<K2, T, D, O>; } let h = History::builder().key::<String>().p_draw(0.05).build(); ``` That retires `builder_with_key` (see also #68, which needs the same treatment for `T`). Also: `Joint` carries `O` but never uses it — nothing in its body touches the observer; it rides along inside `&'h History`. With the reorder, `Joint<'h, K>` is the whole spelling. ## 2. `&[&[&K]]` is the worst shape in the API, and it is a *third* vocabulary The crate has four ways to say "some teams of competitors": typed `Event`/`Team`/`Member`, `&[&[&K]]` (all seven `predict_*`/`expected_*`), `&[&[Rating]]` (`Game`, free `expected_information_gain`), and `&[&[Gaussian]]` (`quality`). At the default key type the guessed spelling fails: ```rust h.predict_win_probabilities(&[&["alice", "bob"]]) // error[E0308]: expected `&&str`, found `&str` ``` At `K = String` — the realistic case — string literals are **impossible** and you must build three levels of temporaries that all outlive the call. This is the real code required to ask "who wins": ```rust fn head_to_head(&self, a: &str, b: &str) -> f64 { let ta = vec![a.to_string()]; let tb = vec![b.to_string()]; let ra: Vec<&String> = ta.iter().collect(); let rb: Vec<&String> = tb.iter().collect(); let teams: Vec<&[&String]> = vec![&ra, &rb]; self.history.predict_win_probabilities(&teams).unwrap()[0] } ``` Six lines and four allocations. The same roster said the ingestion way is `.team(names)` — one expression. **This costs nothing to fix.** `member_skills` and `resolve_terms` do exactly two things with a key: `self.keys.get(*key)` and `format!("{key:?}")`. Neither needs `K` itself: ```rust pub fn predict_win_probabilities<Q>(&self, teams: &[&[&Q]]) -> Result<Vec<f64>, InferenceError> where K: Borrow<Q>, Q: Hash + Eq + Debug + ?Sized; pub fn posterior_of<Q>(&self, terms: &[(&Q, f64)]) -> Result<Gaussian, InferenceError> where K: Borrow<Q>, Q: Hash + Eq + Debug + ?Sized; ``` Prototyped and run. It gives the **same natural spelling for both key types**: ```rust 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)]) // no `&` on keys ``` A `Vec<String>` roster then needs one `.map(String::as_str)` instead of three nested collects. Bonus: `K: Debug` becomes `Q: Debug`, so the key type no longer has to be `Debug` to run a prediction. ## 3. `lookup`'s bound is copy-paste and strictly too narrow `current_skill`, `rating`, `learning_curve`, `filtered_learning_curve` and `log_evidence_for` all accept `h.f("alice")`. `lookup` does not: ``` error[E0271]: type mismatch resolving `<str as ToOwned>::Owned == &str` note: required by a bound in `History::lookup` ``` Its body is `self.keys.get(key)`, and `KeyTable::get` already has the right bound. The `ToOwned<Owned = K>` came from `intern`, which genuinely needs it to create the entry. Dropping it from `lookup` **breaks nothing** — it strictly widens what compiles. ## Suggested extras - `HistoryBuilder::gamma(f64)` shorthand — `GAMMA` is a public constant and drift is the most-tuned parameter after `sigma`, but setting it requires discovering `ConstantDrift`. Must pin `D = ConstantDrift`, so it belongs on that impl only. - **Not** boxing the observer: it makes the type name worse, and the reorder already gets you to `History<String>` at zero runtime cost. The existing `Observer for Arc<O>`/`Box<O>`/`&O` impls already give runtime choice and work. - **Not** an owned `Joint`: the borrow is correct design doing invalidation work a cache would need by hand, and the factorisation is n² floats. **Breaks:** every explicit `History<..>`/`Joint<..>` spelling; `builder_with_key`; call sites that relied on `&&K` inference lose an `&`. Found by an API audit, 2026-09-09. Both prototypes compile.

logaritmisk added the apibreaking labels 2026-09-09 17:58:12 +00:00

logaritmisk referenced this issue from a commit 2026-09-09 20:11:29 +00:00

feat!: prediction and joint queries take borrowed keys

logaritmisk referenced this issue from a commit 2026-09-09 20:11:29 +00:00

Merge api/ergonomics (#72)

logaritmisk commented 2026-09-09 20:11:45 +00:00

Author

Owner

Copy Link

Copy Source

Items 2 and 3 are done, plus the gamma shorthand, in 92ae5fc (merged as 4c2e98c). Item 1 — the type-parameter reorder — is still open and is the only piece left.

2 — &[&[&Q]], exactly as prototyped

All seven predict_* / expected_* methods, posterior_of, posterior_of_at and the three Joint mirrors. Verified at both key types:

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)])       // no & on the keys  ✔
h.lookup("alice")                                       // ✔

K: Debug became Q: Debug, so a key type no longer has to be Debug to run a prediction.

Two things worth knowing that the prototype did not surface:

• The old &[&[&"a"]] spelling still compiles at the default key type: Q infers to &str, and &[&[&&str]] is what both shapes resolve to. So at K = &'static str this is not a break at all — only the K = String case changes, where nothing compiled before.
• predict_outcome(&[]) no longer infers Q — nothing in an empty slice names it, so it needs let none: &[&[&str]] = &[];. That is the entire cost, and it bites only on the degenerate call. One test needed the annotation.

3 — lookup

Bound dropped. KeyTable::get already had the right one, and the ToOwned<Owned = K> came from intern, which needs it to create the entry. lookup never creates.

The gamma shorthand

HistoryBuilder::gamma(f64), on the ConstantDrift impl only. It rejects a negative value for the same reason as sigma and beta — it enters squared, so -0.5 would silently behave as 0.5. tests/key_ergonomics.rs pins it against the long spelling and carries a control (a different gamma must give a different answer) so it cannot pass by being a no-op.

Item 1 — left for you

The reorder to History<K, T, D, O> is right and your prototype settles that it compiles. I have not done it because it rewrites every explicit History<..> and Joint<..> spelling in the crate and downstream, and it is the one change here where the shape is a judgement rather than a fix — in particular whether Joint should drop O at the same time (it carries the parameter and never touches the observer, so Joint<'h, K> would be the whole spelling).

Say the word and it is one mechanical pass. It pairs naturally with a turbofishable HistoryBuilder::key<K2>() replacing builder_with_key; note key_type::<K2>() and time_type::<T2>() already exist and do exactly that, so that part may already be covered.

Items 2 and 3 are done, plus the `gamma` shorthand, in 92ae5fc (merged as 4c2e98c). Item 1 — the type-parameter reorder — is still open and is the only piece left. ## 2 — `&[&[&Q]]`, exactly as prototyped All seven `predict_*` / `expected_*` methods, `posterior_of`, `posterior_of_at` and the three `Joint` mirrors. Verified at both key types: ```rust 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)]) // no & on the keys ✔ h.lookup("alice") // ✔ ``` `K: Debug` became `Q: Debug`, so a key type no longer has to be `Debug` to run a prediction. **Two things worth knowing that the prototype did not surface:** - The old `&[&[&"a"]]` spelling **still compiles** at the default key type: `Q` infers to `&str`, and `&[&[&&str]]` is what both shapes resolve to. So at `K = &'static str` this is not a break at all — only the `K = String` case changes, where nothing compiled before. - `predict_outcome(&[])` no longer infers `Q` — nothing in an empty slice names it, so it needs `let none: &[&[&str]] = &[];`. That is the entire cost, and it bites only on the degenerate call. One test needed the annotation. ## 3 — `lookup` Bound dropped. `KeyTable::get` already had the right one, and the `ToOwned<Owned = K>` came from `intern`, which needs it to create the entry. `lookup` never creates. ## The `gamma` shorthand `HistoryBuilder::gamma(f64)`, on the `ConstantDrift` impl only. It rejects a negative value for the same reason as `sigma` and `beta` — it enters squared, so `-0.5` would silently behave as `0.5`. `tests/key_ergonomics.rs` pins it against the long spelling and carries a control (a different gamma must give a different answer) so it cannot pass by being a no-op. ## Item 1 — left for you The reorder to `History<K, T, D, O>` is right and your prototype settles that it compiles. I have not done it because it rewrites every explicit `History<..>` and `Joint<..>` spelling in the crate and downstream, and it is the one change here where the *shape* is a judgement rather than a fix — in particular whether `Joint` should drop `O` at the same time (it carries the parameter and never touches the observer, so `Joint<'h, K>` would be the whole spelling). Say the word and it is one mechanical pass. It pairs naturally with a turbofishable `HistoryBuilder::key<K2>()` replacing `builder_with_key`; note `key_type::<K2>()` and `time_type::<T2>()` already exist and do exactly that, so that part may already be covered.

logaritmisk referenced this issue 2026-09-09 21:19:07 +00:00

Six exported types are unreachable from `History` — removing them shrinks the API for free #73

logaritmisk referenced this issue from a commit 2026-09-10 04:49:27 +00:00

refactor!: K comes first in History, HistoryBuilder and Joint

logaritmisk referenced this issue from a commit 2026-09-10 04:49:27 +00:00

Merge api/param-reorder (#72)

logaritmisk commented 2026-09-10 04:49:44 +00:00

Author

Owner

Copy Link

Copy Source

Item 1 is done, so this closes. b553c63 (merged as 1ad789c).

History<K, T, D, O> and HistoryBuilder<K, T, D, O>, all four defaulted. Your prototype was right that bounds may reference later parameters, so D: Drift<T> = ConstantDrift sits fine in third position. The headline compiles:

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

72 call sites swapped, and the reorder made most of them shorter rather than merely reordered — 18 now read History<String>, and the &'static str ones collapse to bare History. The two turbofished builders went from HistoryBuilder::<Untimed, _, _, String>::new() to HistoryBuilder::<String, Untimed>::new().

One deviation. Joint keeps O, defaulted rather than removed. You are right that it never touches the observer — I checked what it actually reads from the history (beta, competitors, score_sigma, sigma, time_slices, resolve_terms) and none of it is O. But it borrows the whole &'h History<K, T, D, O> and calls History::resolve_terms, so dropping the parameter means introducing a view struct or moving that method off History. The default buys the entire user-visible benefit — Joint<'h> and Joint<'h, String> both spell out — so the refactor would be paying real structural churn for a parameter nobody types. Happy to do it if you want the parameter genuinely gone rather than merely invisible.

builder_with_key was already retired in favour of key_type::<K2>() / time_type::<T2>(), which are the turbofishable setters this item asked for.

Item 1 is done, so this closes. b553c63 (merged as 1ad789c). `History<K, T, D, O>` and `HistoryBuilder<K, T, D, O>`, all four defaulted. Your prototype was right that bounds may reference later parameters, so `D: Drift<T> = ConstantDrift` sits fine in third position. The headline compiles: ```rust struct Ladder { history: History<String> } struct Analysis<'h> { joint: Joint<'h> } ``` 72 call sites swapped, and the reorder made most of them **shorter** rather than merely reordered — 18 now read `History<String>`, and the `&'static str` ones collapse to bare `History`. The two turbofished builders went from `HistoryBuilder::<Untimed, _, _, String>::new()` to `HistoryBuilder::<String, Untimed>::new()`. **One deviation.** `Joint` keeps `O`, defaulted rather than removed. You are right that it never touches the observer — I checked what it actually reads from the history (`beta`, `competitors`, `score_sigma`, `sigma`, `time_slices`, `resolve_terms`) and none of it is `O`. But it borrows the whole `&'h History<K, T, D, O>` and calls `History::resolve_terms`, so dropping the parameter means introducing a view struct or moving that method off `History`. The default buys the entire user-visible benefit — `Joint<'h>` and `Joint<'h, String>` both spell out — so the refactor would be paying real structural churn for a parameter nobody types. Happy to do it if you want the parameter genuinely gone rather than merely invisible. `builder_with_key` was already retired in favour of `key_type::<K2>()` / `time_type::<T2>()`, which are the turbofishable setters this item asked for.

logaritmisk closed this issue 2026-09-10 04:49:47 +00:00

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

v0.9.0

v0.8.0

v0.7.0

v0.6.0

v0.5.0

v0.4.2

v0.4.1

v0.4.0

v0.3.0

v0.2.0

v0.1.2

v0.1.1

v0.1.0

Labels

Clear labels

api

Shape or ergonomics of the public surface

blocked

Waiting on another issue or an external decision

breaking

Resolving it changes the public API; wants a major bump

bug

Behaviour is wrong, including silently wrong

decision

Needs a call to be made, not code to be written

docs

Documentation, including doc comments and the README

enhancement

New capability the crate does not have

infra

CI, release process, packaging

numerics

Floating-point accuracy, cancellation, convergence maths

performance

Speed or memory

refactor

Internal cleanup with no intended behaviour change

testing

Test coverage and test infrastructure

No labels api breaking

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

logaritmisk

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#72