From 1ac3b21db59c10efd407f0b03592d1116ed14cb2 Mon Sep 17 00:00:00 2001 From: Anders Olsson Date: Thu, 27 Aug 2026 17:53:37 +0200 Subject: [PATCH] fix: enforce EventBuilder weight/team length in release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #18. `EventBuilder::weights` guarded the length match with a `debug_assert!`, so release builds accepted a mismatch, silently dropped the weights, and ingested the event anyway. That is the exact shape #18 is about: validation that exists only where it is least needed. The setters return `Self` to keep the chain fluent, so they cannot return a `Result`. The builder now records the first failure and `commit` returns it as `MismatchedShape`. The weights are not applied on mismatch either, so a partially-weighted team cannot reach the history by another route. Two tests in tests/degenerate_inputs.rs, whose CI job runs in release — which is the only place the old behaviour differed. The second test needed strengthening before it was worth anything. As first written it committed a ONE-team event, which ingestion rejects for an unrelated reason, so it passed under a mutation that disabled the whole check. It now uses two teams, so ingestion would otherwise succeed and the assertion is actually load-bearing. Both tests were then mutation-proved together: disabling the error path in `commit` fails both in release. #18 stays open. The remaining debug_asserts live in `ranked_with_arena` and `scored_with_arena`, and promoting those means threading `Result` up through `Event::compute`, `TimeSlice::iteration`, `log_evidence` and `filtered_step` — which lands on the public API as `log_evidence() -> Result` and `filtered_learning_curve() -> Result<...>`. That is a trade-off about what the query API should look like, not a mechanical change, so it is not mine to decide. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01T5SYDExxL4vZgvunrcNSMc --- src/event_builder.rs | 43 +++++++++++++++++++++++++++------- tests/degenerate_inputs.rs | 48 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/event_builder.rs b/src/event_builder.rs index b5fcb54..ae8687a 100644 --- a/src/event_builder.rs +++ b/src/event_builder.rs @@ -19,6 +19,14 @@ where history: &'h mut History, event: Event, current_team_idx: Option, + /// First validation failure seen while building, surfaced by `commit`. + /// + /// The setters return `Self` so the chain stays fluent; they cannot return + /// a `Result` without breaking that. Recording the failure and reporting it + /// at `commit` keeps the check enforced in release, where the previous + /// `debug_assert!` was compiled out and a mismatched event was ingested + /// silently. + error: Option, } impl<'h, T, D, O, K> EventBuilder<'h, T, D, O, K> @@ -37,6 +45,7 @@ where outcome: Outcome::Ranked(SmallVec::new()), }, current_team_idx: None, + error: None, } } @@ -50,24 +59,36 @@ where /// Set per-member weights for the most recently added team. /// + /// A length mismatch is recorded and returned by [`EventBuilder::commit`] + /// as `InferenceError::MismatchedShape`, in both debug and release. The + /// weights are not applied in that case, so a partially-weighted team + /// cannot reach the history. + /// /// # Panics /// - /// Panics if called before any `.team(...)`. In debug builds, also panics - /// if the number of weights does not match the team's member count. + /// Panics if called before any `.team(...)`. pub fn weights>(mut self, weights: I) -> Self { let idx = self .current_team_idx .expect(".weights(...) called before any .team(...)"); + let ws: Vec = weights.into_iter().collect(); let team = &mut self.event.teams[idx]; - debug_assert_eq!( - ws.len(), - team.members.len(), - "weights length must match team size" - ); + + if ws.len() != team.members.len() { + self.error.get_or_insert(InferenceError::MismatchedShape { + kind: "weights", + expected: team.members.len(), + got: ws.len(), + }); + + return self; + } + for (m, w) in team.members.iter_mut().zip(ws) { m.weight = w; } + self } @@ -108,8 +129,14 @@ where /// /// # Errors /// - /// Forwards to [`History::add_events`] and returns its errors. + /// Returns the first validation failure recorded while building — see + /// [`EventBuilder::weights`] — otherwise forwards to + /// [`History::add_events`] and returns its errors. pub fn commit(self) -> Result<(), InferenceError> { + if let Some(error) = self.error { + return Err(error); + } + self.history.add_events(std::iter::once(self.event)) } } diff --git a/tests/degenerate_inputs.rs b/tests/degenerate_inputs.rs index 1a6bb16..d4edc50 100644 --- a/tests/degenerate_inputs.rs +++ b/tests/degenerate_inputs.rs @@ -141,6 +141,54 @@ fn converge_on_an_empty_history_with_owned_keys() { assert!(report.converged); } +/// A weights/team length mismatch used to be a `debug_assert!`, so release +/// builds ingested the event with the weights silently unapplied. This file's +/// CI job runs in release too, which is the point of pinning it here. +#[test] +fn event_builder_rejects_a_weights_length_mismatch() { + let mut h = History::default(); + + let err = h + .event(1) + .team(["a"]) + .weights([1.0, 2.0]) + .team(["b"]) + .winner(0) + .commit() + .unwrap_err(); + + assert!( + matches!( + err, + InferenceError::MismatchedShape { + kind: "weights", + expected: 1, + got: 2, + } + ), + "expected a weights MismatchedShape, got {err:?}" + ); +} + +/// The mismatch must not be applied even partially — a half-weighted team +/// reaching the history would be worse than the error. +#[test] +fn event_builder_weights_mismatch_leaves_the_history_untouched() { + let mut h = History::default(); + + // Two teams, so ingestion would otherwise succeed — a one-team event is + // rejected for an unrelated reason and would pass this vacuously. + let _ = h + .event(1) + .team(["a"]) + .weights([1.0, 2.0]) + .team(["b"]) + .winner(0) + .commit(); + + assert!(h.learning_curve("a").is_empty()); +} + #[test] fn empty_event_stream_then_converge() { let mut h = History::default();