refactor!: typed discriminators for InferenceError
Six of fifteen variants carried a `&'static str` discriminator, about
thirty magic strings between them, and the only thing a caller could do
with one was print it. Four new enums replace them:
Parameter 13 variants, replacing 9 strings in InvalidParameter
Shape 4 variants, replacing 10 in MismatchedShape
OutcomeKind 2 variants, replacing WrongOutcomeKind's three fields
CompetitorField 2 variants, replacing ConflictingCompetitorConfig's
`InvalidProbability` folds into `InvalidParameter` as
`Parameter::PDraw`. It was a bespoke variant for one scalar while every
other scalar shared `InvalidParameter`, and it omitted the parameter
name — so the same parameter had two mechanisms.
`JointUnavailable { reason: &'static str }` splits into `EmptyHistory`,
`JointRequiresScoredEvents` and `NotPositiveDefinite`. The three are
conditions a caller branches on differently — add events, use
`predict_win_probabilities`, or reconsider the priors — and telling them
apart used to mean string-matching English. One test already proved the
distinction was load-bearing: the blanket conversion mapped the
empty-history case onto the ranked one and `an_empty_history_has_no_joint`
caught it immediately.
`NonFiniteResult` splits into `NonFiniteStep { context, step }` and
`NonFiniteSkill { mu, sigma }`. One `step: (f64, f64)` field was
carrying a sweep step from `converge` and a skill's own moments from a
prediction — two situations in one variant, and a field name that could
only be right for one of them.
`InvalidParameter { name: "beta with point-mass skills" }` becomes
`NoPerformanceVariance`. It was never a parameter out of range: both
values are individually valid and it is their combination that leaves
nothing varying.
Three `Display` impls did not meet the standard the others set, and the
typed data is what makes fixing them possible:
before drift variance is invalid: NaN
after drift variance must be finite and non-negative (got NaN)
before kinds: expected length 3, got 2
after the outcome describes a different number of teams than the
event has: expected 3, got 2
before Game::ranked: expected Outcome::Ranked, got Outcome::Scored
after expected Outcome::Ranked, got Outcome::Scored; call
Game::scored for a scored outcome
`Parameter::range()` states each parameter's actual bounds, which no
`&'static str` name could have. `error::message_tests` renders every one
and asserts each is a sentence rather than a label, and that the three
above now carry a range or a next step.
The four internal `MismatchedShape` kinds — `results`, `times`, `kinds`,
and the weights array — collapse to `Shape::Internal`, whose `Display`
says plainly that reaching it is a bug in this crate. They are checks on
`add_events_with_prior`'s own parallel arrays and are unreachable
through the public API; they stay checked rather than becoming
`debug_assert!`s, because release is where this crate's defects hide.
Closes #74.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
This commit is contained in:
@@ -184,7 +184,10 @@ fn a_batch_declaring_two_different_priors_is_rejected() {
|
||||
assert!(
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::ConflictingCompetitorConfig { field: "prior", .. }
|
||||
InferenceError::ConflictingCompetitorConfig {
|
||||
field: trueskill_tt::CompetitorField::Prior,
|
||||
..
|
||||
}
|
||||
),
|
||||
"got {err:?}"
|
||||
);
|
||||
|
||||
@@ -157,7 +157,7 @@ fn event_builder_rejects_a_weights_length_mismatch() {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::MismatchedShape {
|
||||
kind: "weights",
|
||||
shape: trueskill_tt::Shape::Weights,
|
||||
expected: 1,
|
||||
got: 2,
|
||||
..
|
||||
@@ -228,7 +228,7 @@ fn scored_event_rejects_non_positive_sigma() {
|
||||
assert!(matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
name: "score_sigma",
|
||||
parameter: trueskill_tt::Parameter::ScoreSigma,
|
||||
..
|
||||
}
|
||||
));
|
||||
|
||||
@@ -279,7 +279,7 @@ fn reject(scale: f64) -> InferenceError {
|
||||
fn negative_scale_is_rejected() {
|
||||
assert!(matches!(
|
||||
reject(-1.0),
|
||||
InferenceError::InvalidParameter { name: "drift_scale", value, .. }
|
||||
InferenceError::InvalidParameter { parameter: trueskill_tt::Parameter::DriftScale, value, .. }
|
||||
if value == -1.0
|
||||
));
|
||||
}
|
||||
@@ -291,7 +291,7 @@ fn non_finite_scale_is_rejected() {
|
||||
matches!(
|
||||
reject(scale),
|
||||
InferenceError::InvalidParameter {
|
||||
name: "drift_scale",
|
||||
parameter: trueskill_tt::Parameter::DriftScale,
|
||||
..
|
||||
}
|
||||
),
|
||||
@@ -488,7 +488,7 @@ fn a_batch_that_contradicts_itself_is_rejected() {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::ConflictingCompetitorConfig {
|
||||
field: "drift_scale",
|
||||
field: trueskill_tt::CompetitorField::DriftScale,
|
||||
..
|
||||
}
|
||||
),
|
||||
|
||||
@@ -136,7 +136,7 @@ fn weights_still_guards_a_members_team() {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::MismatchedShape {
|
||||
kind: "weights",
|
||||
shape: trueskill_tt::Shape::Weights,
|
||||
expected: 2,
|
||||
got: 1,
|
||||
..
|
||||
@@ -164,7 +164,7 @@ fn an_invalid_drift_scale_surfaces_from_commit() {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
name: "drift_scale",
|
||||
parameter: trueskill_tt::Parameter::DriftScale,
|
||||
..
|
||||
}
|
||||
),
|
||||
|
||||
+8
-2
@@ -57,7 +57,7 @@ fn game_ranked_rejects_bad_p_draw() {
|
||||
},
|
||||
)
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, InferenceError::InvalidProbability { .. }));
|
||||
assert!(matches!(err, InferenceError::InvalidParameter { .. }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -227,7 +227,13 @@ mod malformed_games {
|
||||
)
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "score", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Score,
|
||||
..
|
||||
}
|
||||
),
|
||||
"{bad}: {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -112,7 +112,13 @@ fn a_non_finite_score_is_rejected_at_ingestion() {
|
||||
}])
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "score", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Score,
|
||||
..
|
||||
}
|
||||
),
|
||||
"{bad}: {err:?}"
|
||||
);
|
||||
assert!(h.current_skill(&"a").is_none(), "{bad} was recorded anyway");
|
||||
@@ -136,7 +142,13 @@ fn a_non_finite_weight_is_rejected_at_ingestion() {
|
||||
.commit()
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "weight", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Weight,
|
||||
..
|
||||
}
|
||||
),
|
||||
"{bad}: {err:?}"
|
||||
);
|
||||
assert!(h.current_skill(&"a").is_none(), "{bad} reached the history");
|
||||
|
||||
@@ -231,16 +231,20 @@ fn a_ranked_history_has_no_exact_joint() {
|
||||
let _ = h.converge().unwrap();
|
||||
assert!(matches!(
|
||||
h.joint().unwrap_err(),
|
||||
InferenceError::JointUnavailable { .. }
|
||||
InferenceError::JointRequiresScoredEvents
|
||||
));
|
||||
}
|
||||
|
||||
/// Distinguishable from the ranked case, which is the point of splitting
|
||||
/// `JointUnavailable { reason: &str }` into three variants (#74): "add events"
|
||||
/// and "use predict_win_probabilities" are different instructions, and telling
|
||||
/// them apart used to mean matching on English prose.
|
||||
#[test]
|
||||
fn an_empty_history_has_no_joint() {
|
||||
let h = history(UnknownKeys::Reject);
|
||||
assert!(matches!(
|
||||
h.joint().unwrap_err(),
|
||||
InferenceError::JointUnavailable { .. }
|
||||
InferenceError::EmptyHistory
|
||||
));
|
||||
}
|
||||
|
||||
|
||||
@@ -56,7 +56,7 @@ fn overflow_during_inference_is_reported_not_hidden() {
|
||||
|
||||
for (name, sigma, beta, score_sigma, scores) in cases {
|
||||
match scored_fit(sigma, beta, score_sigma, scores) {
|
||||
Err(InferenceError::NonFiniteResult { context, step, .. }) => {
|
||||
Err(InferenceError::NonFiniteStep { context, step, .. }) => {
|
||||
assert_eq!(context, "History::converge", "{name}");
|
||||
assert!(
|
||||
!step.0.is_finite() || !step.1.is_finite(),
|
||||
@@ -86,7 +86,7 @@ fn a_broken_fit_is_never_reported_as_converged() {
|
||||
|
||||
let err = h.converge().unwrap_err();
|
||||
assert!(
|
||||
matches!(err, InferenceError::NonFiniteResult { .. }),
|
||||
matches!(err, InferenceError::NonFiniteStep { .. }),
|
||||
"a breakdown must not be reported as convergence: {err:?}"
|
||||
);
|
||||
|
||||
@@ -104,7 +104,7 @@ fn a_broken_fit_is_never_reported_as_converged() {
|
||||
.unwrap();
|
||||
assert!(matches!(
|
||||
h2.converge_partial().unwrap_err(),
|
||||
InferenceError::NonFiniteResult { .. }
|
||||
InferenceError::NonFiniteStep { .. }
|
||||
));
|
||||
}
|
||||
|
||||
@@ -161,7 +161,7 @@ fn a_nan_competitor_is_not_masked_by_a_healthy_one() {
|
||||
.converge()
|
||||
.expect_err("a NaN fit must never be reported as converged");
|
||||
assert!(
|
||||
matches!(err, InferenceError::NonFiniteResult { .. }),
|
||||
matches!(err, InferenceError::NonFiniteStep { .. }),
|
||||
"{err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -49,7 +49,7 @@ fn nan_poisoned() -> H {
|
||||
);
|
||||
let err = h.converge().expect_err("this fixture must not converge");
|
||||
assert!(
|
||||
matches!(err, InferenceError::NonFiniteResult { .. }),
|
||||
matches!(err, InferenceError::NonFiniteStep { .. }),
|
||||
"{err:?}"
|
||||
);
|
||||
h
|
||||
@@ -101,7 +101,7 @@ fn a_nan_poisoned_fit_is_refused_by_every_prediction_path() {
|
||||
|
||||
all_predictions!(h, |name: &str, r: Result<(), InferenceError>| {
|
||||
match r {
|
||||
Err(InferenceError::NonFiniteResult { .. }) => {}
|
||||
Err(InferenceError::NonFiniteSkill { .. }) => {}
|
||||
other => panic!("{name} answered from a NaN fit: {other:?}"),
|
||||
}
|
||||
});
|
||||
@@ -121,7 +121,7 @@ fn degenerate_performances_are_refused_rather_than_answered_wrongly() {
|
||||
// and every skill is a point mass.
|
||||
all_predictions!(h, |name: &str, r: Result<(), InferenceError>| {
|
||||
match r {
|
||||
Err(InferenceError::InvalidParameter { .. }) => {}
|
||||
Err(InferenceError::NoPerformanceVariance) => {}
|
||||
other => panic!("{name} predicted from a degenerate fit: {other:?}"),
|
||||
}
|
||||
});
|
||||
|
||||
+10
-4
@@ -172,7 +172,13 @@ fn a_weight_on_a_registration_is_rejected() {
|
||||
.register(Member::new("layout").with_weight(0.5))
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "weight", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Weight,
|
||||
..
|
||||
}
|
||||
),
|
||||
"{err:?}"
|
||||
);
|
||||
}
|
||||
@@ -188,7 +194,7 @@ fn an_invalid_drift_scale_on_a_registration_is_rejected() {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
name: "drift_scale",
|
||||
parameter: trueskill_tt::Parameter::DriftScale,
|
||||
..
|
||||
}
|
||||
),
|
||||
@@ -280,7 +286,7 @@ mod conflicting_configuration {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::ConflictingCompetitorConfig {
|
||||
field: "drift_scale",
|
||||
field: trueskill_tt::CompetitorField::DriftScale,
|
||||
..
|
||||
}
|
||||
),
|
||||
@@ -297,7 +303,7 @@ mod conflicting_configuration {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::ConflictingCompetitorConfig {
|
||||
field: "drift_scale",
|
||||
field: trueskill_tt::CompetitorField::DriftScale,
|
||||
..
|
||||
}
|
||||
),
|
||||
|
||||
+22
-4
@@ -49,7 +49,13 @@ fn ranked_rejects_a_zero_damping_factor() {
|
||||
)
|
||||
.expect_err("alpha = 0 must be rejected");
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "alpha", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Alpha,
|
||||
..
|
||||
}
|
||||
),
|
||||
"got {err:?}"
|
||||
);
|
||||
}
|
||||
@@ -65,7 +71,13 @@ fn ranked_rejects_an_out_of_range_damping_factor() {
|
||||
)
|
||||
.expect_err("alpha out of (0, 1] must be rejected");
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "alpha", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Alpha,
|
||||
..
|
||||
}
|
||||
),
|
||||
"alpha={alpha}: got {err:?}"
|
||||
);
|
||||
}
|
||||
@@ -81,7 +93,13 @@ fn scored_rejects_a_bad_damping_factor() {
|
||||
)
|
||||
.expect_err("alpha = 0 must be rejected");
|
||||
assert!(
|
||||
matches!(err, InferenceError::InvalidParameter { name: "alpha", .. }),
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
parameter: trueskill_tt::Parameter::Alpha,
|
||||
..
|
||||
}
|
||||
),
|
||||
"got {err:?}"
|
||||
);
|
||||
}
|
||||
@@ -360,7 +378,7 @@ mod constructor_parameters {
|
||||
matches!(
|
||||
err,
|
||||
InferenceError::InvalidParameter {
|
||||
name: "drift variance",
|
||||
parameter: trueskill_tt::Parameter::DriftVariance,
|
||||
..
|
||||
}
|
||||
),
|
||||
|
||||
Reference in New Issue
Block a user