From 35d7512557988cd07b9c1e9abcef3f8af73dc8e4 Mon Sep 17 00:00:00 2001
From: Anders Olsson <anders.e.olsson@gmail.com>
Date: Tue, 8 Sep 2026 01:20:48 +0200
Subject: [PATCH] fix(test): the ingestion-order property was comparing two
 truncated fits
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`ingestion_order_does_not_change_the_answer` failed with a 1.2e-6 gap in
mu and read as a violation of the invariant. It was not. Both sides ran
`max_iter: 200` against `epsilon: 1e-10`, and the batched side stopped
at the cap with a step of 3.4e-9 — so the test compared two fits that
had not converged and attributed the difference to ingestion order.

Raised to 20_000, at which both converge and the property holds. Runtime
is unchanged at 0.05s, because converging is what the iterations were
for.

The test now asserts `report.converged` on both sides before comparing.
That is the part worth keeping: any test that compares two fits for
equality is measuring truncation unless it first establishes that both
reached a fixed point. `ingestion_equivalence.rs` already did this;
`properties.rs` did not.

Found because c12bc83 made `ConvergenceReport` `#[must_use]`, which is
the same failure #50 describes — a short fit is wrong by a little and
looks entirely plausible. The blanket `let _ =` that commit applied to
78 call sites was too blunt here: binding the report to `_` silenced the
one signal that would have caught this, in a test whose whole purpose is
to compare two fits.

Refs #50

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011hcFjNDmHXZF8URGLku5zZ
---
 tests/properties.rs | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tests/properties.rs b/tests/properties.rs
index 91ec38d..9f04310 100644
--- a/tests/properties.rs
+++ b/tests/properties.rs
@@ -26,7 +26,10 @@ const KEYS: [&str; 8] = ["a", "b", "c", "d", "e", "f", "g", "h"];
 fn history_from(games: &[(usize, usize)]) -> History {
     let mut h = History::builder()
         .convergence(ConvergenceOptions {
-            max_iter: 200,
+            // 200 was not enough: the batched side stopped at the cap with a
+            // step of 3.4e-9, so this test was comparing two truncated fits and
+            // attributing the gap to ingestion order.
+            max_iter: 20_000,
             epsilon: 1e-10,
             ..ConvergenceOptions::default()
         })
@@ -114,14 +117,21 @@ proptest! {
     fn ingestion_order_does_not_change_the_answer(games in pairs()) {
         let batched = {
             let mut h = history_from(&games);
-            let _ = h.converge().unwrap();
+            let report = h.converge().unwrap();
+            prop_assert!(
+                report.converged,
+                "batched side stopped at {} iterations with step {:?}; comparing \
+                 two fits that have not converged measures truncation, not order",
+                report.iterations,
+                report.final_step
+            );
             h
         };
 
         let incremental = {
             let mut h = History::builder()
                 .convergence(ConvergenceOptions {
-                    max_iter: 200,
+                    max_iter: 20_000,
                     epsilon: 1e-10,
                     ..ConvergenceOptions::default()
                 })
@@ -139,7 +149,13 @@ proptest! {
                 .unwrap();
             }
 
-            let _ = h.converge().unwrap();
+            let report = h.converge().unwrap();
+            prop_assert!(
+                report.converged,
+                "incremental side stopped at {} iterations with step {:?}",
+                report.iterations,
+                report.final_step
+            );
             h
         };