Member::with_prior is silently ignored for competitors the history already knows #10

New Issue

2026-08-04T19:15:05Z

logaritmisk commented

2026-08-04 19:15:05 +00:00

Member::prior is documented as a per-event override (src/event.rs):

weight defaults to 1.0; a per-event prior can override the competitor's current skill estimate for this event only.

Neither half of that sentence is true in the implementation.

What actually happens

History::add_events collects priors into a map keyed by Index (src/history.rs:719-721), then hands it to add_events_with_prior, which consults it in exactly one place (src/history.rs:512-527):

if !self.agents.contains(*agent) {
    self.agents.insert(*agent, Competitor {
        rating: priors.remove(agent).unwrap_or_else(|| Rating::new(/* history defaults */)),
        ...
    });
}

Two consequences:

Known competitor → the prior is dropped on the floor. If the competitor already exists in self.agents, the if is skipped, priors.remove is never called, and the supplied prior has no effect whatsoever. No error, no warning. Since a competitor is inserted the first time they appear, this means a prior only ever applies on a competitor's very first event and is silently discarded on every subsequent one.
Even when it is applied, it is not per-event. It sets Competitor.rating, which is the competitor's standing configuration for the whole history — not an override scoped to one event.

So the feature works only in the single case "brand-new competitor, first appearance", and even there it does something different from what the doc promises.

Why it matters

The natural use is seeding: import a competitor with a known rating from an external system, or pin a reference player's skill for one event. Both silently do nothing if the competitor has been seen before — the caller gets plausible output computed from the default prior instead.

Fix

Decide which of the two documented behaviours is actually wanted:

Per-competitor seeding (probably what the code was reaching for): apply the prior whenever supplied, replacing Competitor.rating even for known competitors. Cheap; but "last write wins" across a batch needs defining, and it makes ingestion order significant.
True per-event override (what the doc says): plumb the prior down to Item/Event so Item::within_prior (src/time_slice.rs:61-78) uses it in place of the skill-derived rating for that event only, leaving the competitor's own state untouched.

Whichever is chosen, the other should be either implemented under a distinct name or removed from the docs. If a supplied prior cannot be honoured, that should be an InferenceError, not a silent drop.

Acceptance

A test asserting a prior supplied for an already-known competitor changes the result.
A test asserting the scope of the override (whole-history vs single-event) matches the documented wording.
src/event.rs doc comment updated to describe what the code does.

`Member::prior` is documented as a per-event override (`src/event.rs`): > `weight` defaults to 1.0; a per-event `prior` can override the competitor's current skill estimate **for this event only**. Neither half of that sentence is true in the implementation. ## What actually happens `History::add_events` collects priors into a map keyed by `Index` (`src/history.rs:719-721`), then hands it to `add_events_with_prior`, which consults it in exactly one place (`src/history.rs:512-527`): ```rust if !self.agents.contains(*agent) { self.agents.insert(*agent, Competitor { rating: priors.remove(agent).unwrap_or_else(|| Rating::new(/* history defaults */)), ... }); } ``` Two consequences: 1. **Known competitor → the prior is dropped on the floor.** If the competitor already exists in `self.agents`, the `if` is skipped, `priors.remove` is never called, and the supplied prior has no effect whatsoever. No error, no warning. Since a competitor is inserted the first time they appear, this means a prior only ever applies on a competitor's *very first* event and is silently discarded on every subsequent one. 2. **Even when it is applied, it is not per-event.** It sets `Competitor.rating`, which is the competitor's standing configuration for the whole history — not an override scoped to one event. So the feature works only in the single case "brand-new competitor, first appearance", and even there it does something different from what the doc promises. ## Why it matters The natural use is seeding: import a competitor with a known rating from an external system, or pin a reference player's skill for one event. Both silently do nothing if the competitor has been seen before — the caller gets plausible output computed from the default prior instead. ## Fix Decide which of the two documented behaviours is actually wanted: - **Per-competitor seeding** (probably what the code was reaching for): apply the prior whenever supplied, replacing `Competitor.rating` even for known competitors. Cheap; but "last write wins" across a batch needs defining, and it makes ingestion order significant. - **True per-event override** (what the doc says): plumb the prior down to `Item`/`Event` so `Item::within_prior` (`src/time_slice.rs:61-78`) uses it in place of the skill-derived rating for that event only, leaving the competitor's own state untouched. Whichever is chosen, the other should be either implemented under a distinct name or removed from the docs. If a supplied prior cannot be honoured, that should be an `InferenceError`, not a silent drop. ## Acceptance - A test asserting a prior supplied for an already-known competitor changes the result. - A test asserting the scope of the override (whole-history vs single-event) matches the documented wording. - `src/event.rs` doc comment updated to describe what the code does.

logaritmisk referenced this issue from a commit

2026-09-07 13:42:41 +00:00

fix!: apply competitor configuration whenever it is supplied

logaritmisk closed this issue

2026-09-07 13:42:41 +00:00

logaritmisk added the api bug labels 2026-09-07 13:53:09 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#10

Member::with_prior is silently ignored for competitors the history already knows #10

What actually happens

Why it matters

Fix

Acceptance

`Member::with_prior` is silently ignored for competitors the history already knows #10