logaritmisk/trueskill-tt
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Activity

History has no Debug, and trait impls are inconsistent across the public value types #76

New Issue
Closed
opened 2026-09-09 17:56:50 +00:00 by logaritmisk · 1 comment
logaritmisk commented 2026-09-09 17:56:50 +00:00
Owner
Copy Link
Copy Source

The concrete cost

A consumer cannot derive Debug on anything holding a History:

#[derive(Debug)]
struct MyApp { history: History }

error[E0277]: `trueskill_tt::History` doesn't implement `Debug`
5 |     history: History,
  |     ^^^^^^^^^^^^^^^^ the trait `Debug` is not implemented

Both known consumers hold a History in application state, so both hit this. HistoryBuilder has the same gap while deriving Clone.

The survey

Measured from a consumer crate:

Type Derives Missing
History (none) — Send, Sync, Default are impl'd Debug, Clone
HistoryBuilder Clone Debug
Joint manual Debug —
Gaussian Clone, Copy, PartialEq, Debug —
Rating Clone, Copy, Debug PartialEq
Event / Team / Member Clone, Debug PartialEq
ConvergenceReport Clone, Debug PartialEq
Outcome Clone, Debug, PartialEq —
GameOptions, ConvergenceOptions Clone, Copy, Debug + Default —
InferenceError Debug, Clone, PartialEq —

Send + Sync + Default on History are all present, which is what a server needs — so this is a gap rather than a deliberate pattern.

The inconsistencies

  • Gaussian has PartialEq but Rating does not, though both are pure value/config types and Rating is just a Gaussian plus three scalars. Comparing two ratings is a natural thing to want.
  • Event/Team/Member are input value types with no way to compare them — which makes ingestion round-trip tests awkward for consumers.
  • HistoryBuilder has Clone but not Debug, the opposite of the usual pairing.

Suggested

  • Debug on History and HistoryBuilder. History's should be hand-written and summarising — competitor count, slice count, whether converged — not a dump of every skill store, in the same spirit as the manual Debug already written for Joint (which deliberately omits the n² factorisation).
  • PartialEq on Rating, Event, Team, Member, ConvergenceReport.
  • Clone on History is a judgement call — it is expensive and rarely wanted; leaving it off is defensible, but worth being a decision rather than an omission.

Breaks: nothing. All additive.

Found by an API audit, 2026-09-09; the Debug failure reproduced from a consumer crate.

## The concrete cost A consumer cannot derive `Debug` on anything holding a `History`: ```rust #[derive(Debug)] struct MyApp { history: History } ``` ``` error[E0277]: `trueskill_tt::History` doesn't implement `Debug` 5 | history: History, | ^^^^^^^^^^^^^^^^ the trait `Debug` is not implemented ``` Both known consumers hold a `History` in application state, so both hit this. `HistoryBuilder` has the same gap while deriving `Clone`. ## The survey Measured from a consumer crate: | Type | Derives | Missing | |---|---|---| | `History` | *(none)* — `Send`, `Sync`, `Default` are impl'd | **`Debug`**, `Clone` | | `HistoryBuilder` | `Clone` | **`Debug`** | | `Joint` | manual `Debug` | — | | `Gaussian` | `Clone, Copy, PartialEq, Debug` | — | | `Rating` | `Clone, Copy, Debug` | **`PartialEq`** | | `Event` / `Team` / `Member` | `Clone, Debug` | **`PartialEq`** | | `ConvergenceReport` | `Clone, Debug` | `PartialEq` | | `Outcome` | `Clone, Debug, PartialEq` | — | | `GameOptions`, `ConvergenceOptions` | `Clone, Copy, Debug` + `Default` | — | | `InferenceError` | `Debug, Clone, PartialEq` | — | `Send + Sync + Default` on `History` are all present, which is what a server needs — so this is a gap rather than a deliberate pattern. ## The inconsistencies - **`Gaussian` has `PartialEq` but `Rating` does not**, though both are pure value/config types and `Rating` is just a `Gaussian` plus three scalars. Comparing two ratings is a natural thing to want. - **`Event`/`Team`/`Member` are input value types with no way to compare them** — which makes ingestion round-trip tests awkward for consumers. - **`HistoryBuilder` has `Clone` but not `Debug`**, the opposite of the usual pairing. ## Suggested - `Debug` on `History` and `HistoryBuilder`. `History`'s should be hand-written and summarising — competitor count, slice count, whether converged — not a dump of every skill store, in the same spirit as the manual `Debug` already written for `Joint` (which deliberately omits the n² factorisation). - `PartialEq` on `Rating`, `Event`, `Team`, `Member`, `ConvergenceReport`. - `Clone` on `History` is a judgement call — it is expensive and rarely wanted; leaving it off is defensible, but worth being a decision rather than an omission. **Breaks:** nothing. All additive. Found by an API audit, 2026-09-09; the `Debug` failure reproduced from a consumer crate.
logaritmisk added the apienhancement labels 2026-09-09 17:58:26 +00:00
logaritmisk commented 2026-09-09 19:29:30 +00:00
Author
Owner
Copy Link
Copy Source

Closed by 7ca0daa (merged as 9d629d0), together with earlier api/cleanup work.

Already landed before this commit: hand-written summarising Debug for History (competitor count, event count, slice count, the configured priors — not a dump of the skill stores, matching what Joint's manual Debug does with its n² factorisation), Debug on HistoryBuilder, and PartialEq on Rating, Event, Team, Member, ConvergenceReport.

One correction to the survey. Rating derived PartialEq — and it was unusable. The derive puts a D: PartialEq bound on the impl, and ConstantDrift, the crate's own only Drift implementation, did not satisfy it:

error[E0369]: binary operation `==` cannot be applied to type `Rating`
   | pub struct ConstantDrift(f64);
   | ------------------------ `trueskill_tt::ConstantDrift` is defined in another crate

Reading the derive list said this was done; writing the comparison from a consumer's position said otherwise. ConstantDrift now derives it, as do ConvergenceOptions and GameOptions — all three are pure configuration and nothing about them makes equality ambiguous.

tests/trait_impls.rs pins the surface, written in the shape the failure was reported (a consumer struct holding a History), and asserts History's Debug output summarises rather than dumps — so a future #[derive(Debug)] cannot quietly replace the manual impl.

Clone on History stays off, as a decision rather than an omission: a history owns every slice's skill store and scratch arena, so a clone costs the whole fit, and neither consumer has wanted one. Reopen this if that changes.

Closed by 7ca0daa (merged as 9d629d0), together with earlier `api/cleanup` work. Already landed before this commit: hand-written summarising `Debug` for `History` (competitor count, event count, slice count, the configured priors — not a dump of the skill stores, matching what `Joint`'s manual `Debug` does with its n² factorisation), `Debug` on `HistoryBuilder`, and `PartialEq` on `Rating`, `Event`, `Team`, `Member`, `ConvergenceReport`. **One correction to the survey.** `Rating` derived `PartialEq` — and it was unusable. The derive puts a `D: PartialEq` bound on the impl, and `ConstantDrift`, the crate's own only `Drift` implementation, did not satisfy it: ``` error[E0369]: binary operation `==` cannot be applied to type `Rating` | pub struct ConstantDrift(f64); | ------------------------ `trueskill_tt::ConstantDrift` is defined in another crate ``` Reading the derive list said this was done; writing the comparison from a consumer's position said otherwise. `ConstantDrift` now derives it, as do `ConvergenceOptions` and `GameOptions` — all three are pure configuration and nothing about them makes equality ambiguous. `tests/trait_impls.rs` pins the surface, written in the shape the failure was reported (a consumer struct *holding* a `History`), and asserts `History`'s `Debug` output summarises rather than dumps — so a future `#[derive(Debug)]` cannot quietly replace the manual impl. **`Clone` on `History` stays off**, as a decision rather than an omission: a history owns every slice's skill store and scratch arena, so a clone costs the whole fit, and neither consumer has wanted one. Reopen this if that changes.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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 enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
logaritmisk
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: logaritmisk/trueskill-tt#76