Merge pull request #354 from fiddlerwoaroof/dbsp/eager-math

fix(core): Min/Max, NaN and signed zero follow the math, not the old fold

context/code/core.md

@@ -94,7 +94,8 @@ target category and every `none_cat` stripped), and iterates
 `stem × formula target` until nothing changes, bounded by `MAX_EVAL_DEPTH = 16`.
 Chained formulas (`Margin = Profit / Revenue` over `Profit = Revenue - Cost`)
 converge naturally; circular references converge to `CellValue::Error("circular")`
-rather than overflowing the stack.
+rather than overflowing the stack. "Nothing changed" is `model::types::settled`: numbers by bits (a
+zero that changes sign is a change) with every NaN one value, so a NaN can settle.
 
 **A cell still moving when the bound runs out has not converged, and the cached
 path had to be told so** (improvise-7iww). The direct evaluator gets this free:

context/code/improvise-incremental.md

@@ -175,7 +175,7 @@ taken where clause 4 and clause 7 leave a choice; evidence is in
    `SetMeasureAgg` rebuilds nothing. A read of several groups (a collapsed group, a total) folds
    their union without merging them: `fold_groups` sums the counts and takes the smallest of the
    groups' minima, the largest of their maxima, or the one value they share.
-4. **Eager's rules are reproduced, not improved.** An all-numbers group reduces; any other group
+4. **Eager's rules are reproduced, not improved** (superseded for NaN and zeros by "Eager's math, fixed" below). An all-numbers group reduces; any other group
    is unanimous or `Error("mixed")`, so `Count` of a group holding text is that text. `Min` and
    `Max` skip NaN and are `+inf`/`-inf` over only NaN, as `fold(INFINITY, f64::min)` is. The
    multiset orders by the delta's total order, with NaN last, so the skip is a step past one
@@ -218,8 +218,8 @@ taken where clause 4 and clause 7 leave a choice; evidence is in
     sums are fed. Eager's `f64::min` and `f64::max` leave the sign of a tie between the two zeros to
     the platform and the order (`fmin` on ARM returns `-0.0`; x86 may not). The backend reproduces the
     part that does not depend on either: all zeros negative gives `-0.0`, all positive gives `+0.0`.
-    With both, `Min` is `-0.0` and `Max` is `+0.0`, which is what ARM's `fmin`/`fmax` return; on other
-    platforms eager may differ there, and `tests/random.rs` keeps `+0.0` out of its pool for that reason.
+    With both, `Min` is `-0.0` and `Max` is `+0.0`; eager now returns the same (see "Eager's math,
+    fixed").
 
 ### Not built in step 4
 
@@ -265,17 +265,11 @@ is followed in outline, and the places it is not are the reason for this section
    cell under the stem changes, not adjusted for the one record. That needs the evaluator to
    accept a maintained aggregate in place of folding, which is a change to core's evaluator and
    to the order its records are visited in (insertion order, not `CellKey` order). It is a bead.
-5. **Dependency order is an optimisation and is checked against the plain passes.** `Order` is the
+5. **Dependency order is an optimisation and is checked against the plain passes** (the NaN and zero differences below are gone: "Eager's math, fixed"). `Order` is the
    formulas' dependency graph by name. When it has no cycle and there are fewer formulas than
    passes, one evaluation per cell in dependency order reaches the fixed point eager's passes
-   reach (a chain of fifteen links needs fifteen passes and a sixteenth to see no change). It
-   differs from the passes in two places. A cell holding NaN is never equal to itself, so eager's
-   passes never see it settle and call it `circular`, and the cells that read it keep what they
-   computed from the NaN; `mark_nan_circular` does the same afterwards. And eager stops on `==`,
-   which calls `-0.0` equal to the `0.0` an early pass read from a stored cell, so a stem with a
-   zero cell can settle on a sign dependency order never sees (a stored `0.0` at `F1`, `F3 = F1`,
-   `F4 = F3 ^ -1`, then `F1 = -0`: eager's `F4` is `inf`). A stem whose ordered evaluation holds any
-   zero is therefore evaluated again by the passes (`holds_a_zero`). `Evaluation::Plain` always
+   reach (a chain of fifteen links needs fifteen passes and a sixteenth to see no change). It is the same fixed point, because Eager's convergence test compares by bits ("Eager's math, fixed").
+   `Evaluation::Plain` always
    runs the passes; `tests/ordering.rs` compares the two after every edit, zero signs included, and
    `tests/formulas.rs` compares both with eager.
 6. **A formula edit rebuilds the stems.** Adding, removing or replacing a formula (the fingerprint
@@ -352,3 +346,45 @@ of that step, are gone; formula-owned keys still go to the model, whose cache st
 
 - The hidden overlay and kept totals (clause 7).
 - A group that does not pay for a sum it never reads (item 6).
+
+## Eager's math, fixed (user ruling 2026-10-10)
+
+"Don't reproduce math bugs you might find in the Eager mode." Every place the earlier steps matched
+Eager's arithmetic "as eager does" was reviewed. The items below supersede the sentences in steps 4 and 5
+that say otherwise (step 4 items 4 and 10, step 5 item 5). Each is a user-visible numeric change.
+
+1. **Min and Max of both zeros** is `-0.0` and `+0.0`, whatever the order or the platform. Core's fold used
+   `f64::min` and `f64::max`, whose tie between the zeros is left to the platform; it now orders by
+   `total_cmp` over the non-NaN values. Incremental already returned this. (improvise-bw4p.20)
+2. **Min and Max over only NaN** are NaN, where they were `+inf` and `-inf` (the seed of the old fold leaking
+   out). NaN among numbers is still skipped.
+3. **A NaN formula result is a value, not a cycle.** The fixed point compared cached cells with `!=`, so a
+   cell that settled on NaN never settled and became `circular`, dragging the cells that read it. It now
+   compares with `model::types::settled`: numbers by bits, every NaN one value. A cell that really oscillates
+   is still `circular` after sixteen passes. `mark_nan_circular` is gone.
+4. **A zero's sign is a change.** `settled` compares by bits, so `0.0` becoming `-0.0` between passes moves
+   the cell, and eager's fixed point and dependency order's agree. Dependency order is the unique fixed point
+   of an acyclic set under bit equality, up to NaN payload and sign (`settled` merges NaNs): each cell is a
+   function of the final values it reads. So the
+   `holds_a_zero` fallback is dropped and `Ordered` needs no extra precondition.
+5. **Empty `SUM`** is `+0.0` (step 6), listed here for completeness.
+6. **Cycles over zeros and NaN.** A cycle that only flips a zero's sign (`X = -X`, `X = X * -1`,
+   `IF(X < 1, -X, X)`, `A = -B` with `B = A` over zeros) used to settle, because `-0.0 == 0.0`; it is now
+   `circular`, as the same cycle over nonzero data always was. A cycle fed a NaN (`A = A + 1`, or `A = A`
+   with `A` NaN) used to be `circular` and now settles on NaN.
+7. **`Min` and `Max` skip NaN while `Sum` and `Avg` propagate it.** Kept: it is IEEE's `minimumNumber` and
+   `maximumNumber` for the extremes and ordinary addition for the sums, and consistent with ADR 005.
+   Whether a spreadsheet user expects one rule for both is a product question (bead text in the hand-back).
+
+### Left alone, with the reasoning
+
+- **`Count` of a group that holds text** returns the text (the unanimity branch of
+  `aggregate_cell_values`), not the number of cells. That is probably not what a user wants, but it is a
+  decision about what a `Count` measure means for non-numbers, not an arithmetic error, so it is filed
+  rather than changed.
+- **The division and equality epsilon** (`FLOAT_EQ_EPSILON`, 1e-10) is shared and deliberate (gotcha 17).
+- **A group holding NaN beside text** is `mixed`: NaN is a number and text is not, so no unanimity exists.
+- **`Avg` and `Sum`** are already exact and correctly rounded (step 6).
+- **Overflow of the exact sum to infinity** follows `ExactSum::value` (round to nearest, so an overflowing
+  total is infinite): correct IEEE behaviour.
+

context/test-evidence/improvise-incremental.md

@@ -314,8 +314,8 @@ because they change nothing observable (below); every other is named by at least
   `group::tests::an_emptied_group_is_absent_and_keeps_no_entries`,
   `incremental_matches_eager_on_the_shipped_documents`, `incremental_equals_eager_over_random_edit_sequences`;
 - a group never counts its numbers: `a_group_read_comes_from_a_maintained_group`,
-  `group::tests::min_and_max_skip_nan_and_are_infinite_over_nothing_else` and eight differentials;
-- `Max` takes the first key: `group::tests::min_and_max_skip_nan_and_are_infinite_over_nothing_else`,
+  `group::tests::min_and_max_skip_nan_and_are_nan_over_nothing_else` and eight differentials;
+- `Max` takes the first key: `group::tests::min_and_max_skip_nan_and_are_nan_over_nothing_else`,
   `an_overwrite_costs_two_updates_per_shape_whatever_the_table_size` and five differentials;
 - `Count` counts one: `a_collapsed_group_reads_one_group_per_member_it_covers`, `group::tests::*`;
 - unanimity ignores disagreement: `group::tests::text_is_unanimous_or_mixed_across_groups`,
@@ -437,9 +437,10 @@ A multiset that ignores a negative weight keeps the minimum it should have lost.
 
 After the last retraction the group reads as absent and its map holds no zero-weight entry.
 
-### group::tests::min_and_max_skip_nan_and_are_infinite_over_nothing_else
+### group::tests::min_and_max_skip_nan_and_are_nan_over_nothing_else
 
-NaN is skipped by `Min` and `Max`, a group of only NaN gives `+inf` and `-inf` and still counts.
+NaN is skipped by `Min` and `Max`; a group of only NaN gives NaN (it gave `+inf` and `-inf` before the
+Eager-math follow-up) and still counts.
 
 ### group::tests::a_fold_over_several_groups_is_the_fold_of_their_union
 
@@ -472,7 +473,6 @@ What each fault was and the test that names it:
   `a_formula_that_keeps_moving_for_sixteen_passes_is_circular`,
   `dependency_order_and_the_plain_passes_leave_the_same_cache`;
 - dependencies after dependents: `dependency_order_and_the_plain_passes_leave_the_same_cache` and five more;
-- NaN is not marked circular: `a_formula_cell_holding_nan_is_circular_and_the_cells_that_read_it_keep_the_nan_they_read`;
 - fifteen passes / seventeen passes: `a_chain_added_last_link_first_is_circular_exactly_where_eager_says`;
 - the passes end without marking the cells still moving, or begin from the old cache:
   `a_chain_added_last_link_first_is_circular_exactly_where_eager_says`,
@@ -557,9 +557,12 @@ the whole recompute.
 `IF(Revenue > 1000, B, Revenue)`: at the stem of a big product it reads the cache of the stem of B,
 which is evaluated after it. Fails when such a formula set is not wide.
 
-### a_formula_cell_holding_nan_is_circular_and_the_cells_that_read_it_keep_the_nan_they_read
+### a_formula_cell_holding_nan_is_nan_and_not_circular
 
-NaN is not equal to itself, so eager's passes never see it settle. Fails when the ordered path leaves it.
+A NaN formula result is a value: `Profit` and `Margin` over a NaN cell read NaN under both evaluations and
+eager. It used to be `circular` in eager, because NaN is not equal to itself and never settled; the
+fixed point now compares with `settled` (follow-up, "Eager's math, fixed"). Fails when NaN is
+marked `circular`.
 
 ### a_new_aggregation_for_the_measure_a_filtered_formula_falls_back_to_changes_it
 
@@ -645,14 +648,21 @@ the stems of formulas that name the measure, and when `Stems::carrying` ignores
 
 ### a_new_aggregation_reaches_a_product_targeted_filtered_sum
 
-The reviewer's second repro, a Product-targeted `SUM(F0 WHERE Region = East)`. It already agreed on this
-fixture and stays as the second shape of the same hazard.
+A Product-targeted `SUM(F0 WHERE Region = East)` with no None category, and `SetMeasureAgg Cost Min`.
+It agreed with eager before the fix too, so it pins the shape and not the fault.
+
+### a_new_aggregation_reaches_a_filtered_sum_on_the_fallback_path
+
+The reviewer's shape: `P0 = SUM(Revenue) WHERE Region = East` on `_Measure` with Time as the None
+category, then `SetMeasureAgg Revenue Max`, so the fallback to the raw group at the formula's own target
+reads the new aggregation. Fails when a new aggregation does not reach the stems carrying the measure.
 
 ### ordered_evaluation_keeps_the_zero_eager_settles_on
 
-A stored `0.0` at `F1`, then `F3 = F1`, `F4 = F3 ^ -1`, then `F1 = -0`: eager settles on `F3 = 0.0` and `F4 = inf`
-because it calls `-0.0` equal to the `0.0` it first read; the first ordered evaluation gave `-inf`. Fails
-(only `Ordered`) when a stem with a zero is not re-evaluated by the passes. The ordering property's pool
+A stored `0.0` at `F1`, then `F3 = F1`, `F4 = F3 ^ -1`, then `F1 = -0`. Eager used to settle on `F4 = inf`
+because it called `-0.0` equal to the `0.0` it first read, while the ordered evaluation gave `-inf`; with
+zero signs settled by bits both give `F3 = -0.0` and `F4 = -inf`. Fails (only `Ordered`) when a stem
+with a zero is not re-evaluated by the passes, and in core when zeros settle equal. The ordering property's pool
 now holds `-0`, `Units * -1` and `-Cost`, and compares after every edit, but did not find this fault
 alone in 1,500 cases: this test is the pin.
 
@@ -766,3 +776,28 @@ average of a rounded sum.
 
 Infinities and NaN read as `ExactSum::value` does, and a zero keeps its sign.
 
+## Eager's math, fixed (follow-up to steps 4 to 6)
+
+Ruling (user, 2026-10-10): do not reproduce math bugs found in Eager; fix Eager. Six faults were
+injected into core and the groups (mixed zeros, Min over NaN, NaN never settling, zeros settling equal, and
+the groups' Min and Max over NaN); `nan-never-settles` survived the
+first pass because NaN produced by one evaluation always has the same bits, and is closed by a unit test of
+`settled`. Each fix has a regression test named after the bug:
+- `model::types::eager_math_tests::min_and_max_of_both_zeros_do_not_depend_on_their_order` and
+  `with_both_zeros_min_is_negative_and_max_is_positive_whatever_the_order`: a naive `f64::min` fold
+  passes on ARM and fails on other platforms, so the rival is a fold that keeps the first of two zeros;
+- `model::types::eager_math_tests::min_and_max_over_only_nan_are_nan_not_infinite`,
+  `min_and_max_over_only_nan_are_nan_in_both_backends`, `group::tests::min_and_max_skip_nan_and_are_nan_over_nothing_else`;
+- `model::types::eager_math_tests::a_formula_that_settles_on_nan_is_nan_and_not_circular`,
+  `a_formula_cell_holding_nan_is_nan_and_not_circular`, `settled_treats_every_nan_as_one_value_and_the_zeros_as_two`;
+- `model::types::eager_math_tests::a_zero_that_changes_sign_between_passes_settles_on_the_new_sign` and
+  `ordered_evaluation_keeps_the_zero_eager_settles_on` (now with the sign settled by bits, no fallback);
+- `model::types::eager_math_tests::a_formula_that_never_settles_is_still_circular`: the guard that a real
+  oscillation is still `circular`.
+The random and differential suites compare numbers by bits and their pools hold both zeros again.
+
+User-visible changes this evidence also covers: a cycle that only flips a zero's sign (`X = -X`,
+`X = X * -1`, `IF(X < 1, -X, X)`, `A = -B` with `B = A` over zeros) was `-0` and is now `circular`, as the
+same cycle over nonzero data always was; a cycle fed a NaN (`A = A + 1`, `A = A` with `A` NaN) settles on
+NaN instead of `circular`.
+

crates/improvise-core/src/model/types.rs

@@ -32,14 +32,32 @@ fn exact(values: &[f64]) -> numtower::ExactSum {
     sum
 }
 
+/// The least of `values` under `order`, skipping NaN; NaN if nothing else is left. Zeros order
+/// by sign (`-0.0` below `+0.0`), so the answer does not depend on the order or the platform.
+fn extreme(values: &[f64], order: impl Fn(&f64, &f64) -> std::cmp::Ordering) -> f64 {
+    let numbers = values.iter().copied().filter(|v| !v.is_nan());
+    numbers.min_by(order).unwrap_or(f64::NAN)
+}
+
+/// Whether two cached cells are the same value for a fixed point: numbers by bits, so a zero
+/// that changes sign is a change, except that every NaN is one value, so a NaN can settle.
+pub fn settled(a: Option<&CellValue>, b: Option<&CellValue>) -> bool {
+    match (a, b) {
+        (Some(CellValue::Number(x)), Some(CellValue::Number(y))) => {
+            x.to_bits() == y.to_bits() || (x.is_nan() && y.is_nan())
+        }
+        _ => a == b,
+    }
+}
+
 /// Reduce raw values. `None` on empty: an empty cell reads as **absent**, not
 /// as zero. Both aggregation paths share it (improvise-kdp, core.md §2).
 fn apply_agg(values: &[f64], func: &AggFunc) -> Option<f64> {
     (!values.is_empty()).then(|| match func {
         AggFunc::Sum => exact_sum(values),
         AggFunc::Avg => exact_mean(values),
-        AggFunc::Min => values.iter().copied().fold(f64::INFINITY, f64::min),
-        AggFunc::Max => values.iter().copied().fold(f64::NEG_INFINITY, f64::max),
+        AggFunc::Min => extreme(values, f64::total_cmp),
+        AggFunc::Max => extreme(values, |a, b| b.total_cmp(a)),
         AggFunc::Count => values.len() as f64,
     })
 }
@@ -1046,7 +1064,7 @@ impl Model {
                     // whatever the cache already holds.
                     let new_val = self.evaluate_formula_cell(&key, none_cats);
                     let old_val = self.formula_cache.get(&key);
-                    if old_val != new_val.as_ref() {
+                    if !settled(old_val, new_val.as_ref()) {
                         moving.push(key.clone());
                         if let Some(v) = new_val {
                             self.formula_cache.insert(key, v);
@@ -3709,3 +3727,98 @@ mod exact_sum_tests {
         assert_eq!(got.to_bits(), (1.0f64 / 3.0).to_bits());
     }
 }
+
+/// Eager's math, fixed (ADR 009 follow-up): none of these depends on the order, the platform or
+/// on NaN being unequal to itself.
+#[cfg(test)]
+mod eager_math_tests {
+    use super::*;
+    use crate::formula::parse_formula;
+
+    fn bits(v: Option<f64>) -> Option<u64> {
+        v.map(f64::to_bits)
+    }
+
+    #[test]
+    fn min_and_max_of_both_zeros_do_not_depend_on_their_order() {
+        for values in [[0.0, -0.0], [-0.0, 0.0]] {
+            let min = apply_agg(&values, &AggFunc::Min);
+            let max = apply_agg(&values, &AggFunc::Max);
+            assert_eq!(bits(min), Some((-0.0f64).to_bits()), "{values:?}");
+            assert_eq!(bits(max), Some(0.0f64.to_bits()), "{values:?}");
+        }
+    }
+
+    #[test]
+    fn min_and_max_over_only_nan_are_nan_not_infinite() {
+        for func in [AggFunc::Min, AggFunc::Max] {
+            let got = apply_agg(&[f64::NAN, -f64::NAN], &func).unwrap();
+            assert!(got.is_nan(), "{func:?} gave {got}");
+        }
+        assert_eq!(apply_agg(&[f64::NAN, 2.0], &AggFunc::Min), Some(2.0));
+        assert_eq!(apply_agg(&[f64::NAN, 2.0], &AggFunc::Max), Some(2.0));
+    }
+
+    fn model_with(cells: &[(&str, f64)], formulas: &[&str]) -> Model {
+        let mut m = Model::new("m");
+        m.add_category("Region").unwrap();
+        m.category_mut("Region").unwrap().add_item("East");
+        for (measure, v) in cells {
+            m.category_mut("_Measure").unwrap().add_item(*measure);
+            let key = CellKey::new(vec![
+                ("Region".into(), "East".into()),
+                ("_Measure".into(), (*measure).into()),
+            ]);
+            m.set_cell(key, CellValue::Number(*v));
+        }
+        for raw in formulas {
+            m.add_formula(parse_formula(raw, "_Measure").unwrap());
+        }
+        m.recompute_formulas(&[]);
+        m
+    }
+
+    fn at(m: &Model, measure: &str) -> Option<CellValue> {
+        let key = CellKey::new(vec![
+            ("Region".into(), "East".into()),
+            ("_Measure".into(), measure.into()),
+        ]);
+        m.evaluate_aggregated(&key, Aggregation::OverHidden)
+    }
+
+    #[test]
+    fn a_formula_that_settles_on_nan_is_nan_and_not_circular() {
+        let m = model_with(&[("Revenue", f64::NAN)], &["Profit = Revenue * 1"]);
+        let Some(CellValue::Number(n)) = at(&m, "Profit") else {
+            panic!("a NaN, not {:?}", at(&m, "Profit"));
+        };
+        assert!(n.is_nan());
+    }
+
+    #[test]
+    fn a_zero_that_changes_sign_between_passes_settles_on_the_new_sign() {
+        let m = model_with(&[("F1", 0.0)], &["F3 = F1", "F4 = F3 ^ -1", "F1 = -0"]);
+        let f4 = at(&m, "F4");
+        assert_eq!(f4, Some(CellValue::Number(f64::NEG_INFINITY)));
+        let Some(CellValue::Number(f3)) = at(&m, "F3") else {
+            panic!("a number")
+        };
+        assert_eq!(f3.to_bits(), (-0.0f64).to_bits());
+    }
+
+    #[test]
+    fn settled_treats_every_nan_as_one_value_and_the_zeros_as_two() {
+        let n = |x: f64| Some(CellValue::Number(x));
+        assert!(settled(n(f64::NAN).as_ref(), n(-f64::NAN).as_ref()));
+        assert!(settled(n(1.5).as_ref(), n(1.5).as_ref()));
+        assert!(!settled(n(0.0).as_ref(), n(-0.0).as_ref()));
+        assert!(!settled(n(f64::NAN).as_ref(), n(1.0).as_ref()));
+        assert!(!settled(n(1.0).as_ref(), None));
+    }
+
+    #[test]
+    fn a_formula_that_never_settles_is_still_circular() {
+        let m = model_with(&[("X", 1.0)], &["X = X + 1"]);
+        assert_eq!(at(&m, "X"), Some(CellValue::Error("circular".into())));
+    }
+}

crates/improvise-incremental/src/group.rs

@@ -119,7 +119,7 @@ pub fn fold_groups(groups: &[&Group], func: Reduce) -> Option<CellValue> {
     }
 }
 
-/// `Min` and `Max` skip NaN, and are infinite when nothing else is left, as eager's folds are.
+/// `Min` and `Max` skip NaN and are NaN when nothing else is left.
 fn reduce(groups: &[&Group], func: Reduce, cells: i64) -> f64 {
     match func {
         Reduce::Count => cells as f64,
@@ -127,13 +127,13 @@ fn reduce(groups: &[&Group], func: Reduce, cells: i64) -> f64 {
         Reduce::Avg => total(groups).mean(cells as u64),
         Reduce::Min => {
             let smallest = groups.iter().filter_map(|g| g.smallest());
-            let min = smallest.fold(f64::INFINITY, f64::min);
+            let min = smallest.reduce(f64::min).unwrap_or(f64::NAN);
             let negative: i64 = groups.iter().map(|g| g.negative_zeros).sum();
             signed_zero(min, negative > 0)
         }
         Reduce::Max => {
             let largest = groups.iter().filter_map(|g| g.largest());
-            let max = largest.fold(f64::NEG_INFINITY, f64::max);
+            let max = largest.reduce(f64::max).unwrap_or(f64::NAN);
             let (zeros, negative): (i64, i64) = (
                 groups.iter().map(|g| g.zeros()).sum(),
                 groups.iter().map(|g| g.negative_zeros).sum(),
@@ -202,19 +202,18 @@ mod tests {
     }
 
     #[test]
-    fn min_and_max_skip_nan_and_are_infinite_over_nothing_else() {
+    fn min_and_max_skip_nan_and_are_nan_over_nothing_else() {
         let some = group(&[num(f64::NAN), num(3.0), num(-1.0)]);
         assert_eq!(fold_groups(&[&some], Reduce::Min), Some(num(-1.0)));
         assert_eq!(fold_groups(&[&some], Reduce::Max), Some(num(3.0)));
         let only_nan = group(&[num(f64::NAN), num(-f64::NAN)]);
-        assert_eq!(
-            fold_groups(&[&only_nan], Reduce::Min),
-            Some(num(f64::INFINITY))
-        );
-        assert_eq!(
-            fold_groups(&[&only_nan], Reduce::Max),
-            Some(num(f64::NEG_INFINITY))
+        for func in [Reduce::Min, Reduce::Max] {
+            let got = fold_groups(&[&only_nan], func);
+            assert!(
+                matches!(got, Some(CellValue::Number(n)) if n.is_nan()),
+                "{func:?}"
             );
+        }
         assert_eq!(fold_groups(&[&only_nan], Reduce::Count), Some(num(2.0)));
     }
 

crates/improvise-incremental/src/incremental.rs

@@ -9,7 +9,7 @@ use crate::{Derived, Edit, Maintained, Reading, cell_delta};
 use improvise_core::formula::ast::AggFunc;
 use improvise_core::model::category::MEASURE_CATEGORY;
 use improvise_core::model::cell::{CellChange, CellKey, CellValue};
-use improvise_core::model::types::Covers;
+use improvise_core::model::types::{Covers, settled};
 use improvise_core::model::{Aggregation, Model};
 use std::collections::BTreeMap;
 
@@ -253,14 +253,6 @@ impl Incremental {
                     let value = self.evaluate_cell(&keys[i]);
                     self.model.set_formula_cell(keys[i].clone(), value);
                 }
-                if self.holds_a_zero(&keys) {
-                    for key in &keys {
-                        self.model.set_formula_cell(key.clone(), None);
-                    }
-                    self.passes(&keys);
-                } else {
-                    self.mark_nan_circular(&keys);
-                }
             }
             _ => self.passes(&keys),
         }
@@ -277,7 +269,7 @@ impl Incremental {
             moving.clear();
             for key in keys {
                 let new = self.evaluate_cell(key);
-                if self.model.formula_cell(key) != new.as_ref() {
+                if !settled(self.model.formula_cell(key), new.as_ref()) {
                     moving.push(key);
                     self.model
                         .set_formula_cell(key.clone(), new.or_else(|| self.keep(key)));
@@ -293,24 +285,6 @@ impl Incremental {
         }
     }
 
-    /// Eager stops on `==`, which calls `-0.0` equal to the `0.0` an early pass read, so a stem
-    /// with a zero cell can settle on a sign the dependency order never sees: it runs the passes.
-    fn holds_a_zero(&self, keys: &[CellKey]) -> bool {
-        let zero = |k: &CellKey| matches!(self.model.formula_cell(k), Some(CellValue::Number(n)) if *n == 0.0);
-        keys.iter().any(zero)
-    }
-
-    /// A NaN is not equal to itself, so eager's passes never see it settle and call it circular
-    /// once they run out; the cells that read it keep what they computed from the NaN.
-    fn mark_nan_circular(&mut self, keys: &[CellKey]) {
-        for key in keys {
-            if matches!(self.model.formula_cell(key), Some(CellValue::Number(n)) if n.is_nan()) {
-                let circular = CellValue::Error("circular".into());
-                self.model.set_formula_cell(key.clone(), Some(circular));
-            }
-        }
-    }
-
     /// A cell whose new value is absent keeps the one it has.
     fn keep(&self, key: &CellKey) -> Option<CellValue> {
         self.model.formula_cell(key).cloned()

crates/improvise-incremental/tests/formulas.rs

@@ -236,7 +236,7 @@ fn a_name_in_another_category_reads_another_stems_cell() {
 }
 
 #[test]
-fn a_formula_cell_holding_nan_is_circular_and_the_cells_that_read_it_keep_the_nan_they_read() {
+fn a_formula_cell_holding_nan_is_nan_and_not_circular() {
     let wb = formula_workbook();
     let none = vec!["Region".to_string()];
     let nan = bump("East", "A", "Revenue", f64::NAN);
@@ -248,15 +248,13 @@ fn a_formula_cell_holding_nan_is_circular_and_the_cells_that_read_it_keep_the_na
             ("_Measure".into(), m.into()),
         ])
     };
-    let circular = Some(CellValue::Error("circular".into()));
-    assert_eq!(
-        backend.value(&at("Profit"), Aggregation::OverHidden),
-        circular
-    );
-    assert_eq!(
-        backend.value(&at("Margin"), Aggregation::OverHidden),
-        circular
+    for measure in ["Profit", "Margin"] {
+        let got = backend.value(&at(measure), Aggregation::OverHidden);
+        assert!(
+            matches!(got, Some(CellValue::Number(n)) if n.is_nan()),
+            "{measure}: {got:?}"
         );
+    }
     for by in [Evaluation::Ordered, Evaluation::Plain] {
         same_model(&wb.model, &none, std::slice::from_ref(&nan), by).unwrap();
     }
@@ -345,6 +343,42 @@ fn a_new_aggregation_reaches_the_stems_of_a_formula_that_targets_another_categor
     );
 }
 
+#[test]
+fn a_new_aggregation_reaches_a_filtered_sum_on_the_fallback_path() {
+    let mut model = region_target_model();
+    model.add_formula(parse_formula("P0 = SUM(Revenue) WHERE Region = East", "_Measure").unwrap());
+    let none = vec!["Time".to_string()];
+    let edits = [Edit::SetMeasureAgg {
+        measure: "Revenue".into(),
+        func: improvise_core::formula::ast::AggFunc::Max,
+    }];
+    for by in [Evaluation::Ordered, Evaluation::Plain] {
+        same_model(&model, &none, &edits, by).unwrap();
+    }
+}
+
+#[test]
+fn ordered_evaluation_keeps_the_zero_eager_settles_on() {
+    let mut model = Model::new("zeros");
+    model.add_category("Region").unwrap();
+    model.category_mut("Region").unwrap().add_item("East");
+    let key = CellKey::new(vec![
+        ("Region".into(), "East".into()),
+        ("_Measure".into(), "F1".into()),
+    ]);
+    model.set_cell(key, CellValue::Number(0.0));
+    let edits: Vec<Edit> = [
+        ("F3 = F1", "_Measure"),
+        ("F4 = F3 ^ -1", "_Measure"),
+        ("F1 = -0", "_Measure"),
+    ]
+    .iter()
+    .map(|(raw, cat)| Edit::AddFormula(parse_formula(raw, cat).unwrap()))
+    .collect();
+    for by in [Evaluation::Ordered, Evaluation::Plain] {
+        same_model(&model, &[], &edits, by).unwrap_or_else(|e| panic!("{by:?}: {e}"));
+    }
+}
 #[test]
 fn a_new_aggregation_reaches_a_product_targeted_filtered_sum() {
     let mut model = Model::new("products");
@@ -373,28 +407,3 @@ fn a_new_aggregation_reaches_a_product_targeted_filtered_sum() {
         same_model(&model, &none, &edits, by).unwrap();
     }
 }
-
-/// A stored `0.0` at `F1`, then `F3 = F1`, `F4 = F3 ^ -1` and last `F1 = -0`: eager's first
-/// pass reads the stored zero, and its later passes call `-0.0` equal to it and stop.
-#[test]
-fn ordered_evaluation_keeps_the_zero_eager_settles_on() {
-    let mut model = Model::new("zeros");
-    model.add_category("Region").unwrap();
-    model.category_mut("Region").unwrap().add_item("East");
-    let key = CellKey::new(vec![
-        ("Region".into(), "East".into()),
-        ("_Measure".into(), "F1".into()),
-    ]);
-    model.set_cell(key, CellValue::Number(0.0));
-    let edits: Vec<Edit> = [
-        ("F3 = F1", "_Measure"),
-        ("F4 = F3 ^ -1", "_Measure"),
-        ("F1 = -0", "_Measure"),
-    ]
-    .iter()
-    .map(|(raw, cat)| Edit::AddFormula(parse_formula(raw, cat).unwrap()))
-    .collect();
-    for by in [Evaluation::Ordered, Evaluation::Plain] {
-        same_model(&model, &[], &edits, by).unwrap_or_else(|e| panic!("{by:?}: {e}"));
-    }
-}

crates/improvise-incremental/tests/random.rs

@@ -16,8 +16,7 @@ use support::{
 
 fn pool() -> impl Strategy<Value = CellValue> {
     prop_oneof![
-        // No +0.0: with both zeros in a group eager's Min and Max leave the sign to the platform.
-        prop_oneof![-3i32..0, 1i32..4].prop_map(|n| CellValue::Number(f64::from(n))),
+        (-3i32..4).prop_map(|n| CellValue::Number(f64::from(n))),
         Just(CellValue::Number(-0.0)),
         Just(CellValue::Number(f64::NAN)),
         Just(CellValue::Number(-f64::NAN)),

crates/improvise-incremental/tests/support/mod.rs

@@ -339,8 +339,7 @@ pub fn same_model(
 
 fn any_formula_value() -> impl Strategy<Value = CellValue> {
     prop_oneof![
-        // No +0.0 beside -0.0: eager's Min and Max of both zeros is platform-defined.
-        prop_oneof![-2i32..0, 1i32..4].prop_map(|n| CellValue::Number(f64::from(n))),
+        (-2i32..4).prop_map(|n| CellValue::Number(f64::from(n))),
         Just(CellValue::Number(-0.0)),
         Just(CellValue::Number(f64::NAN)),
         prop::sample::select(vec![0.1, 1e16, -1e16]).prop_map(CellValue::Number),

crates/improvise-incremental/tests/zeros.rs

@@ -83,12 +83,22 @@ fn overwriting_a_zero_with_the_other_zero_moves_the_sign_of_min_and_max() {
 
 #[test]
 fn with_both_zeros_min_is_negative_and_max_is_positive_whatever_the_order() {
-    let at = |values: &[f64], func| {
-        let backend = Incremental::new(model_of(values, func), vec![]);
-        bits(&backend)
-    };
     for values in [[0.0, -0.0], [-0.0, 0.0]] {
-        assert_eq!(at(&values, AggFunc::Min), Some((-0.0f64).to_bits()));
-        assert_eq!(at(&values, AggFunc::Max), Some(0.0f64.to_bits()));
+        let min = Some((-0.0f64).to_bits());
+        let max = Some(0.0f64.to_bits());
+        assert_eq!(both(&values, AggFunc::Min), (min, min), "{values:?}");
+        assert_eq!(both(&values, AggFunc::Max), (max, max), "{values:?}");
+    }
+}
+
+#[test]
+fn min_and_max_over_only_nan_are_nan_in_both_backends() {
+    for func in [AggFunc::Min, AggFunc::Max] {
+        let (eager, incremental) = both(&[f64::NAN, -f64::NAN], func.clone());
+        let nan = |b: Option<u64>| b.is_some_and(|b| f64::from_bits(b).is_nan());
+        assert!(
+            nan(eager) && nan(incremental),
+            "{func:?}: {eager:?} {incremental:?}"
+        );
     }
 }