Declarative competitor configuration: HistoryBuilder::default_rating_for #53

New Issue

2026-09-08T19:14:14Z

logaritmisk commented

2026-09-08 19:14:14 +00:00

Split from #38, which is otherwise delivered. This is the half that needs a design decision rather than work.

What is missing

History::register states configuration for one competitor, before any event. That covers a bot at a known strength or a handful of reference points. It does not cover the motivating case, which is a rule:

every layout is static

ustat has thousands of layout and hole keys. Enumerating them to call register on each means knowing the full key set up front, which a consumer ingesting an event stream generally does not. The shape #38 proposed instead:

let h = History::builder()
    .default_rating_for(|key: &str| {
        key.starts_with("layout_").then(|| /* static rating */)
    })
    .build();

Evaluated where the defaults are applied today, on the create branch in add_events_with_prior. It makes "layouts are static" one statement that cannot be forgotten on an ingestion path, rather than a fact every call site has to remember.

Why it is a decision

It adds a Fn type parameter to History, which already carries four (T, D, O, K). #38 flagged this itself as in tension with the Copy-and-simple posture the crate otherwise keeps. Three shapes, none obviously right:

A fifth generic parameter. Zero-cost, and every signature mentioning History grows again. HistoryBuilder::observer already shows the pattern — it returns a differently-parameterised builder.
A boxed closure. Option<Box<dyn Fn(&K) -> Option<Rating<T, D>>>>. One allocation and one indirect call per competitor creation — not per event, and not per sweep — so the cost is almost certainly irrelevant. Costs History: Clone and Debug unless worked around.
A small trait, RatingPolicy<K, T, D>, defaulting to a ZST. Same generic-parameter cost as (1) but nameable, so consumers can write the type down.

My inclination is (2): the call site is competitor creation, which happens once per key for the lifetime of the history, and the ergonomic win over (1) is large. But the Clone loss is a real cost — HistoryBuilder derives it today — and that is worth your call rather than mine.

Interaction with what shipped

register and the closure would both feed the same declared map, so the conflict rule added alongside them applies unchanged: a rule and an explicit register that disagree about one competitor should be ConflictingCompetitorConfig, not last-wins. Worth deciding whether an explicit register should instead override a rule, since that is the one case where "the specific beats the general" is a defensible reading.

Tests it would need

A key matching the rule reaches the same posterior as configuring it on its first event.
A key not matching gets the history defaults.
The rule fires for a competitor first seen through record_winner, which cannot carry configuration.
register and a rule that disagree resolve however (1) is decided, and a test pins which.

Not urgent. register covers the enumerable cases, and #38's motivating consumer has since said the per-hole claim it wanted this for does not clear its sample size anyway (#51).

Split from #38, which is otherwise delivered. This is the half that needs a design decision rather than work. ## What is missing `History::register` states configuration for **one** competitor, before any event. That covers a bot at a known strength or a handful of reference points. It does not cover the motivating case, which is a *rule*: > every layout is static `ustat` has thousands of layout and hole keys. Enumerating them to call `register` on each means knowing the full key set up front, which a consumer ingesting an event stream generally does not. The shape #38 proposed instead: ```rust let h = History::builder() .default_rating_for(|key: &str| { key.starts_with("layout_").then(|| /* static rating */) }) .build(); ``` Evaluated where the defaults are applied today, on the create branch in `add_events_with_prior`. It makes "layouts are static" one statement that cannot be forgotten on an ingestion path, rather than a fact every call site has to remember. ## Why it is a decision It adds a `Fn` type parameter to `History`, which already carries four (`T`, `D`, `O`, `K`). #38 flagged this itself as in tension with the `Copy`-and-simple posture the crate otherwise keeps. Three shapes, none obviously right: 1. **A fifth generic parameter.** Zero-cost, and every signature mentioning `History` grows again. `HistoryBuilder::observer` already shows the pattern — it returns a differently-parameterised builder. 2. **A boxed closure.** `Option<Box<dyn Fn(&K) -> Option<Rating<T, D>>>>`. One allocation and one indirect call per competitor *creation* — not per event, and not per sweep — so the cost is almost certainly irrelevant. Costs `History: Clone` and `Debug` unless worked around. 3. **A small trait**, `RatingPolicy<K, T, D>`, defaulting to a ZST. Same generic-parameter cost as (1) but nameable, so consumers can write the type down. My inclination is (2): the call site is competitor creation, which happens once per key for the lifetime of the history, and the ergonomic win over (1) is large. But the `Clone` loss is a real cost — `HistoryBuilder` derives it today — and that is worth your call rather than mine. ## Interaction with what shipped `register` and the closure would both feed the same `declared` map, so the conflict rule added alongside them applies unchanged: a rule and an explicit `register` that disagree about one competitor should be `ConflictingCompetitorConfig`, not last-wins. Worth deciding whether an explicit `register` should instead *override* a rule, since that is the one case where "the specific beats the general" is a defensible reading. ## Tests it would need - A key matching the rule reaches the same posterior as configuring it on its first event. - A key not matching gets the history defaults. - The rule fires for a competitor first seen through `record_winner`, which cannot carry configuration. - `register` and a rule that disagree resolve however (1) is decided, and a test pins which. Not urgent. `register` covers the enumerable cases, and #38's motivating consumer has since said the per-hole claim it wanted this for does not clear its sample size anyway (#51).

logaritmisk added the api decision enhancement labels 2026-09-08 19:14:37 +00:00

logaritmisk referenced this issue

2026-09-08 19:15:37 +00:00

No way to configure a competitor before their first event, and getting it wrong is silent and permanent #38

logaritmisk referenced this issue from a commit

2026-09-10 05:17:18 +00:00

feat: HistoryBuilder::default_rating_for, a rule instead of a roll call

logaritmisk closed this issue

2026-09-10 05:17:18 +00:00

logaritmisk referenced this issue from a commit

2026-09-10 05:17:18 +00:00

Merge feat/rating-rule (#53)

logaritmisk commented

2026-09-10 05:17:31 +00:00

Done — c4194b0 (merged as 0b99973). Shape (1), the generic parameter, with two deviations that implementing it forced.

History::builder()
    .default_rating_for(|key: &&str| {
        key.starts_with("layout_").then(|| StartingPoint::new().drift_scale(0.0))
    })
    .build()

Defaulted to NoRule, so History<String> still spells out and nobody who does not use a rule pays for the parameter.

Deviation 1 — a trait, not a bare `Fn` bound

Option (1) as written is R: Fn(&K) -> Option<Rating<T, D>>. A closure's type cannot be written down — and the motivating consumer holds its History in application state, so it must name the type in a struct field. Option (1) makes that impossible, which defeats the point for the one consumer that wanted it.

RatingRule<K> is your option (3) — the nameable version of the same zero-cost parameter — and default_rating_for still takes a closure for the common case, wrapping it in FnRule. tests/rating_rule.rs contains the struct-field case that would not compile under a bare Fn bound:

struct Ladder {
    history: History<&'static str, i64, ConstantDrift, NullObserver, StaticLayouts>,
}

Deviation 2 — the rule returns a `StartingPoint`, not a `Rating`

Two reasons, and the second is a hard one.

A Rating also carries beta and the drift model, which describe the history rather than one competitor — a rule that could vary them would be describing a different model per competitor rather than a starting point within one. What the create branch actually applies is config.prior and config.drift_scale, the same pair a Member may carry, so that is what the rule supplies.

And with Rating<T, D> in the trait signature, HistoryBuilder::drift and time_type stop compiling once a rule is set: R: RatingRule<K, T, D> does not imply R: RatingRule<K, T, D2>, so the type-changing setters have no way to carry the rule across. Keying the trait on K alone removes the problem entirely.

The precedence question you left open

Explicit beats the rule, field by field. ConflictingCompetitorConfig would make one exceptional competitor incompatible with having any rule at all, which is not a usable trade. Two explicit declarations that disagree stay an error — neither is more specific than the other — and two_explicit_declarations_that_disagree_are_still_an_error pins that the new precedence did not weaken the old check.

key_type resets the rule to NoRule, since a RatingRule<K> cannot answer questions about K2. Set the key type first.

All four tests you listed, plus one that corrected me

Your list is covered, including the record_winner case (a route that cannot carry configuration at all, which is the sharpest argument for the feature).

Every test carries a control, and one of them told me I was wrong. I first asserted that a non-matching competitor's posterior was untouched by the rule. It is not, and should not be: alice plays the pinned layout, and what she learns from beating it depends on how sure the model is about it. The control is her configuration — the thing the rule was actually asked about.

Done — c4194b0 (merged as 0b99973). Shape (1), the generic parameter, with two deviations that implementing it forced. ```rust History::builder() .default_rating_for(|key: &&str| { key.starts_with("layout_").then(|| StartingPoint::new().drift_scale(0.0)) }) .build() ``` Defaulted to `NoRule`, so `History<String>` still spells out and nobody who does not use a rule pays for the parameter. ## Deviation 1 — a trait, not a bare `Fn` bound Option (1) as written is `R: Fn(&K) -> Option<Rating<T, D>>`. A closure's type cannot be written down — and the motivating consumer holds its `History` in application state, so it must name the type in a struct field. Option (1) makes that impossible, which defeats the point for the one consumer that wanted it. `RatingRule<K>` is your option (3) — the nameable version of the same zero-cost parameter — and `default_rating_for` still takes a closure for the common case, wrapping it in `FnRule`. `tests/rating_rule.rs` contains the struct-field case that would not compile under a bare `Fn` bound: ```rust struct Ladder { history: History<&'static str, i64, ConstantDrift, NullObserver, StaticLayouts>, } ``` ## Deviation 2 — the rule returns a `StartingPoint`, not a `Rating` Two reasons, and the second is a hard one. A `Rating` also carries `beta` and the drift model, which describe the *history* rather than one competitor — a rule that could vary them would be describing a different model per competitor rather than a starting point within one. What the create branch actually applies is `config.prior` and `config.drift_scale`, the same pair a `Member` may carry, so that is what the rule supplies. And with `Rating<T, D>` in the trait signature, **`HistoryBuilder::drift` and `time_type` stop compiling** once a rule is set: `R: RatingRule<K, T, D>` does not imply `R: RatingRule<K, T, D2>`, so the type-changing setters have no way to carry the rule across. Keying the trait on `K` alone removes the problem entirely. ## The precedence question you left open **Explicit beats the rule, field by field.** `ConflictingCompetitorConfig` would make one exceptional competitor incompatible with having any rule at all, which is not a usable trade. Two *explicit* declarations that disagree stay an error — neither is more specific than the other — and `two_explicit_declarations_that_disagree_are_still_an_error` pins that the new precedence did not weaken the old check. `key_type` resets the rule to `NoRule`, since a `RatingRule<K>` cannot answer questions about `K2`. Set the key type first. ## All four tests you listed, plus one that corrected me Your list is covered, including the `record_winner` case (a route that cannot carry configuration at all, which is the sharpest argument for the feature). Every test carries a control, and one of them told me I was wrong. I first asserted that a non-matching competitor's **posterior** was untouched by the rule. It is not, and should not be: alice plays the pinned layout, and what she learns from beating it depends on how sure the model is about it. The control is her *configuration* — the thing the rule was actually asked about.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#53

Declarative competitor configuration: HistoryBuilder::default_rating_for #53

What is missing

Why it is a decision

Interaction with what shipped

Tests it would need

Deviation 1 — a trait, not a bare Fn bound

Deviation 2 — the rule returns a StartingPoint, not a Rating

The precedence question you left open

All four tests you listed, plus one that corrected me

Declarative competitor configuration: `HistoryBuilder::default_rating_for` #53

Deviation 1 — a trait, not a bare `Fn` bound

Deviation 2 — the rule returns a `StartingPoint`, not a `Rating`