fix(core): a Summary can be built from an engine's parts, and keys stop cloning values (improvise-qddo)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze

context/code/source.md

@@ -70,3 +70,9 @@ implementation, written as a query engine with no index would answer, sharing on
 `DataStore`. `the_store_and_a_scanned_table_answer_every_select_alike` asks both every select over a
 fixture (every subset of every stored key as a base, with no span, one span of each of six item
 lists, and two), at both widths and for all five functions: over 5,000 triples.
+
+## Building a `Summary` from an engine's answer
+
+`Summary::from_parts(Numbers::from_parts(count, exact_sum, least, greatest), Agreement)`. `Agreement`
+is what the non-number values hold: `None`, `One(v)` or `Mixed`. `from_parts` drops extremes that
+contradict the count or are NaN, so a source cannot hand the evaluator a state `add` could not reach.

context/test-evidence/core.md

@@ -2344,3 +2344,20 @@ the member records of an aggregate over an expression
 A select with `exact` and no span is the cell, not the cells containing the pairs. The store's
 `spanned_values` is never called for it (the evaluator reads an exact key through `value_at`), so
 this is the contract a second source implements. Rival: `exact` ignored when there is no span.
+
+### a_summary_built_from_an_engines_parts_reduces_as_one_built_from_the_values
+
+Review of #356 (Codex): an engine outside core could not build a `Summary` without replaying rows. The
+parts (count, exact sum, extremes, agreement of the non-numbers) are computed in the test without
+`Summary::of`, for the 585 sequences of the reference test. Rivals: the greatest taken from the least;
+a NaN allowed as an extreme. Each fails it.
+
+### parts_that_contradict_themselves_are_made_consistent
+
+Extremes with no numbers, and a NaN as an extreme, are dropped by `Numbers::from_parts`.
+
+### keys_of_a_store_are_its_cells_keys_without_the_values
+
+Review of #356 (Codex): `DataStore::keys` went through `cells` and cloned every value. It reads keys
+only now; the test pins the keys and their order against `cells`. No clone-count test: a value clone is
+not observable from outside, so the equality is all that can be asserted.

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

@@ -51,6 +51,23 @@ impl Numbers {
         numbers
     }
 
+    /// Numbers from what an engine computed: how many, their exact sum and the least and
+    /// greatest that are not NaN. Extremes that contradict the count or are NaN are dropped.
+    pub fn from_parts(
+        count: u64,
+        sum: ExactSum,
+        least: Option<f64>,
+        greatest: Option<f64>,
+    ) -> Self {
+        let held = |x: Option<f64>| x.filter(|x| count > 0 && !x.is_nan());
+        Numbers {
+            count,
+            sum,
+            least: held(least),
+            greatest: held(greatest),
+        }
+    }
+
     pub fn add(&mut self, x: f64) {
         self.count += 1;
         self.sum.add(x, 1);
@@ -104,9 +121,9 @@ fn merged(a: Option<f64>, b: Option<f64>, pick: fn(f64, f64) -> f64) -> Option<f
     }
 }
 
-/// The values that are not numbers, as far as unanimity needs them.
+/// What the values that are not numbers agree on: nothing held, one value, or disagreement.
 #[derive(Debug, Clone, Default, PartialEq)]
-enum Others {
+pub enum Agreement {
     #[default]
     None,
     One(CellValue),
@@ -118,7 +135,7 @@ enum Others {
 #[derive(Debug, Clone, Default)]
 pub struct Summary {
     numbers: Numbers,
-    others: Others,
+    others: Agreement,
 }
 
 impl Summary {
@@ -128,6 +145,11 @@ impl Summary {
         summary
     }
 
+    /// A summary from what an engine computed: the numbers, and what the rest agree on.
+    pub fn from_parts(numbers: Numbers, others: Agreement) -> Self {
+        Summary { numbers, others }
+    }
+
     pub fn add(&mut self, value: &CellValue) {
         match value.as_f64() {
             Some(x) => self.numbers.add(x),
@@ -137,18 +159,18 @@ impl Summary {
 
     fn add_other(&mut self, value: &CellValue) {
         self.others = match std::mem::take(&mut self.others) {
-            Others::None => Others::One(value.clone()),
-            Others::One(held) if held == *value => Others::One(held),
-            _ => Others::Mixed,
+            Agreement::None => Agreement::One(value.clone()),
+            Agreement::One(held) if held == *value => Agreement::One(held),
+            _ => Agreement::Mixed,
         };
     }
 
     pub fn merge(&mut self, other: &Summary) {
         self.numbers.merge(&other.numbers);
         match &other.others {
-            Others::None => {}
-            Others::One(v) => self.add_other(v),
-            Others::Mixed => self.others = Others::Mixed,
+            Agreement::None => {}
+            Agreement::One(v) => self.add_other(v),
+            Agreement::Mixed => self.others = Agreement::Mixed,
         }
     }
 
@@ -159,8 +181,8 @@ impl Summary {
 
     pub fn reduce(&self, func: &AggFunc) -> Option<CellValue> {
         match (&self.others, self.numbers.count()) {
-            (Others::None, _) => self.numbers.reduce(func).map(CellValue::Number),
-            (Others::One(only), 0) => Some(only.clone()),
+            (Agreement::None, _) => self.numbers.reduce(func).map(CellValue::Number),
+            (Agreement::One(only), 0) => Some(only.clone()),
             _ => Some(CellValue::Error("mixed".to_string())),
         }
     }
@@ -206,7 +228,10 @@ impl CellSource for DataStore {
         &'a self,
         partial: &'a [(String, String)],
     ) -> Box<dyn Iterator<Item = CellKey> + 'a> {
-        Box::new(self.cells(partial).map(|(key, _)| key))
+        match partial.is_empty() {
+            true => Box::new(self.iter_cells().map(|(key, _)| key)),
+            false => Box::new(self.matching_cells(partial).into_iter().map(|(key, _)| key)),
+        }
     }
 
     fn cells<'a>(
@@ -555,4 +580,61 @@ mod tests {
             "no cell is keyed by R=E alone"
         );
     }
+
+    /// The parts an engine would compute for `values`, found without `Summary::of`.
+    fn parts_of(values: &[CellValue]) -> Summary {
+        let numbers: Vec<f64> = values.iter().filter_map(|v| v.as_f64()).collect();
+        let mut sum = ExactSum::new();
+        numbers.iter().for_each(|x| sum.add(*x, 1));
+        let finite = || numbers.iter().copied().filter(|x| !x.is_nan());
+        let least = finite().min_by(f64::total_cmp);
+        let greatest = finite().max_by(f64::total_cmp);
+        let rest: Vec<&CellValue> = values.iter().filter(|v| v.as_f64().is_none()).collect();
+        let agreement = match rest.split_first() {
+            None => Agreement::None,
+            Some((first, others)) if others.iter().all(|v| v == first) => {
+                Agreement::One((*first).clone())
+            }
+            Some(_) => Agreement::Mixed,
+        };
+        let parts = Numbers::from_parts(numbers.len() as u64, sum, least, greatest);
+        Summary::from_parts(parts, agreement)
+    }
+
+    #[test]
+    fn a_summary_built_from_an_engines_parts_reduces_as_one_built_from_the_values() {
+        for values in sequences(3) {
+            let refs: Vec<&CellValue> = values.iter().collect();
+            for func in &FUNCS {
+                let want = reference(&refs, func);
+                let got = parts_of(&values).reduce(func);
+                assert!(
+                    same(&want, &got),
+                    "{func:?} of {values:?}: {want:?} != {got:?}"
+                );
+            }
+        }
+    }
+
+    #[test]
+    fn parts_that_contradict_themselves_are_made_consistent() {
+        let none = Numbers::from_parts(0, ExactSum::new(), Some(1.0), Some(f64::NAN));
+        assert_eq!(none.reduce(&AggFunc::Min), None, "no numbers, no extremes");
+        let nan = Numbers::from_parts(2, ExactSum::new(), Some(f64::NAN), None);
+        let min = nan.reduce(&AggFunc::Min).unwrap();
+        assert!(min.is_nan(), "a NaN is never an extreme");
+    }
+
+    #[test]
+    fn keys_of_a_store_are_its_cells_keys_without_the_values() {
+        let rows = rows();
+        let store = store_of(&rows);
+        let all: Vec<CellKey> = store.keys(&[]).collect();
+        assert_eq!(all, rows.iter().map(|(k, _)| k.clone()).collect::<Vec<_>>());
+        let east = vec![pair("R", "E")];
+        let under: Vec<CellKey> = store.keys(&east).collect();
+        let want: Vec<CellKey> = store.cells(&east).map(|c| c.0).collect();
+        assert_eq!(under, want);
+        assert!(!under.is_empty());
+    }
 }