improvise-eccl/f2oo: correct §14's premise and state the fold's coverage; evidence

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze

context/code/core.md

@@ -869,11 +869,14 @@ an involution without anything being taught about involution, and neither
 
 ### The cell
 
-`Model::evaluate_aggregated` expands, at its head, any coordinate that names a
-group rather than an item into one key per member (a cross product where several
-coordinates do). Doing it in the **model** is what makes `display_text`, prune,
-column totals, `grid_of`, search, yank and CSV export all follow with no change.
-Three decisions:
+`Model::evaluate_covering` expands, at its head, any coordinate that names a group
+rather than an item into one key per covered member (a cross product where
+several coordinates do). The grid reads every pivot key through
+`GridLayout::evaluate`, which passes the members it draws. Since
+improvise-eccl/f2oo, display text, pruning, column totals, `grid_of`, search,
+yank and CSV export all go through that one evaluation, rather than each calling
+`Model::evaluate_aggregated`. That remains the view-less answer, and it covers
+every member. Five decisions:
 
 - **Membership is read off each item's own `group` field, never off
   `Category::groups`.** `add_item_in_group` registers no `Group`; that list is
@@ -882,14 +885,41 @@ Three decisions:
 - **An item with the same name as the group wins.** `items.contains_key`
   short-circuits the expansion, so an existing document with an item `Q1` reads
   exactly as before.
-- **Stored values fold once over every record the group covers; a derived value
-  folds its members' results.** Folding the members' *aggregates* is right for
-  `Sum` and wrong for `Avg`/`Count` under `Axis::None` — `avg(avg(2,4), 9) = 6`
-  where the group's average is `5`. A formula target has no stored records to
-  pool, so its members' evaluated results are folded with the measure's
-  aggregation; for a ratio formula that is the usual pivot-table limitation, not
-  a new one. Hidden items are included in the fold, as on `Axis::None`: hiding
-  decides what is drawn, not what an aggregate covers.
+- **Stored values fold once over every record the group covers.** Folding the
+  members' *aggregates* is right for `Sum` and wrong for `Avg`/`Count` under
+  `Axis::None`: `avg(avg(2,4), 9) = 6` where the group's average is `5`.
+- **A formula is evaluated over the fold, never folded itself** (improvise-eccl).
+  This bullet used to say a formula target "has no stored records to pool", so
+  its members' results were folded with the measure's aggregation, and it called
+  that "the usual pivot-table limitation". **The premise was wrong.** A formula's
+  *inputs* are stored and can be pooled, and the model already evaluated a
+  formula over pooled inputs whenever a category sat on `Axis::None`:
+  `recompute_formulas` strips the None categories from the stems. Folding the
+  members' results summed ratios instead, so `Margin = (Revenue - Cost) / Revenue`
+  at 0.5 and 0.9 showed **1.4** on the Q1 row where Month-on-None showed **0.8**.
+  Spreadsheet calculated fields do not have that limitation either. The formula
+  is now evaluated at the group key, with each reference read through the fold
+  (`Reach::Fold` in `model/types.rs`):
+  - a raw measure reads the stored fold;
+  - another formula target is evaluated over the fold in turn;
+  - an aggregate inside the formula (`SUM`, `MAX`, …) pools every covered
+    member's records. `MAX(Revenue)` had been *summing* the members' maxima.
+
+  The collapsed row now agrees with the None-axis answer. **Test with a ratio**:
+  a sum agrees under both rules, which is how this was missed. One residual
+  difference is stated rather than hidden. `IS` and a `WHERE` on the grouped
+  category itself see the group's name, so `Month IS Q1` is true on the fold
+  and false on a None axis.
+- **A fold covers the members the grid draws** (improvise-f2oo). Hidden members
+  are not in it. So a fold is exactly the rows it replaces, the totals strip does
+  not move on `z`, and a group whose every member is hidden has no row. That last
+  point was already true, and it contradicted the old rule, under which a hidden
+  value appeared only while a sibling was visible. This deliberately **differs
+  from `Axis::None`**, where hidden items still count, because the two replace
+  different things. A fold replaces *rows the user sees*. A None axis replaces a
+  *dimension*, and hiding an item on it hides nothing that was drawn. The view's
+  hidden set reaches the model as the `covers` predicate `GridLayout::evaluate`
+  passes, since the model knows no view.
 
 ### Settability and drill
 

context/test-evidence/core.md

@@ -1341,8 +1341,8 @@ this fixture and the value is absent — this test, the next, and
 
 Under `Axis::None` (`Store`), `Avg` and `Count` over `Jan/A=2, Jan/B=4, Feb/A=9`.
 The group's average is `5` and its count `3`; folding the members' aggregates gives
-`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (the
-formula-target branch taken for stored values) — this test fails. `Sum` cannot see
+`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (what the
+formula-target branch did before improvise-eccl) — this test fails. `Sum` cannot see
 the difference, which is why the fixture sets `measure_agg`.
 
 ### model::types::model_tests::an_item_named_like_a_group_is_read_as_the_item
@@ -1361,3 +1361,39 @@ fold is the row `z` names the group from. Note the narrowing this implies: an
 **empty** collapsed group now has an empty fold row, which `prune_empty` drops, and
 its header goes with it. lhi4 kept that header because it was the only way back to
 the rows; the fold is now that way back.
+
+### a_folds_ratio_formula_is_the_ratio_of_its_folded_inputs
+
+improvise-eccl. `Margin = (Revenue - Cost) / Revenue` with Jan `10/5` and Feb `30/3`
+under `Q1`, and Apr in `Q2` with no data, so the Q1 fold is row 1. The member ratios
+are 0.5 and 0.9, so the three candidate rules give three different answers: summed
+1.4, averaged 0.7, pooled `(40 - 8) / 40` 0.8. The expected value is the **pooled**
+one, and it is also asserted equal to the same document with Month on `Axis::None`,
+which is the ticket's criterion. The None-axis document works because Apr carries
+no data, so the None axis pools exactly Q1's months. At `a164726f` the fold read
+**1.4**. The ticket's own fixture (0.5 and 0.5) would have separated sum from
+pool, but not mean from pool. The second formula set chains
+(`Profit = Revenue - Cost`, `Margin = Profit / Revenue`), so a reference to
+another formula target is read over the fold as well. A rule that folds a formula
+ref's members while pooling raw refs fails there.
+
+### a_folds_aggregate_formula_pools_its_members_records
+
+The guard on the other side of improvise-eccl. Once a formula at a fold is
+evaluated at the group key, an aggregate inside it (`SUM`, `MAX`) must pool every
+covered member's records, because `Month=Q1` matches no record. `MAX(Revenue)` is
+the discriminating case. It **failed before the change too**: the old rule folded
+the members' `MAX`es with the measure's `Sum`, giving 40 where the answer is 30.
+`SUM` alone agrees under the old rule and the new one. Rival: `contexts_of` stops
+pooling at a fold (`Reach::Fold { .. } => Vec::new()`). This test alone fails.
+
+### collapsing_a_group_with_a_hidden_member_leaves_the_total_where_it_was
+
+improvise-f2oo. `Q1 = {Jan 3, Feb 4}`, `Q2 = {Apr 100}`, Feb hidden. Expanded, the
+drawn rows total 103. At `a164726f` collapsing gave a Q1 fold of 7 and a total of
+**107**: `z` moved the total. The ruling: a fold covers the members the grid
+draws. The test asserts 103 both ways and a fold of 3. It then hides Jan too and
+asserts that there is no Q1 row and a total of 100. That half already held, and it
+is the inconsistency the ticket named: before, a hidden value appeared only while
+a sibling was visible. Rival: `GridLayout::evaluate` covering every member
+(`&|_, _| true`). This test alone fails, with 107.

crates/improvise-command/src/ui/viewport.rs

@@ -6,8 +6,8 @@ use unicode_width::UnicodeWidthStr;
 use improvise_protocol::header_level_widths;
 
 use crate::format::{FormatPlan, NumberFormat};
-use crate::model::cell::{CellKey, CellValue};
 use crate::model::Model;
+use crate::model::cell::{CellKey, CellValue};
 use crate::view::{AxisEntry, GridLayout};
 
 /// Minimum column width — enough for short numbers/labels + 1 char gap.
@@ -110,9 +110,7 @@ fn cell_value(
         true => (None, layout.records_value(row, col)),
         false => {
             let key = layout.cell_key(row, col);
-            let value = key
-                .as_ref()
-                .and_then(|k| layout.evaluate(model, k));
+            let value = key.as_ref().and_then(|k| layout.evaluate(model, k));
             (key, value)
         }
     }

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

@@ -138,7 +138,10 @@ pub type Covers<'a> = &'a dyn Fn(&str, &str) -> bool;
 #[derive(Clone, Copy)]
 enum Reach<'a> {
     Record,
-    Fold { agg: Aggregation, covers: Covers<'a> },
+    Fold {
+        agg: Aggregation,
+        covers: Covers<'a>,
+    },
 }
 
 /// The **record-level** evaluator: no cache, because a member key is not a stem
@@ -644,7 +647,12 @@ impl Model {
     }
 
     /// A group's stored values fold ONCE over every record the group covers.
-    fn fold_stored(&self, key: &CellKey, members: &[CellKey], agg: Aggregation) -> Option<CellValue> {
+    fn fold_stored(
+        &self,
+        key: &CellKey,
+        members: &[CellKey],
+        agg: Aggregation,
+    ) -> Option<CellValue> {
         let values: Vec<&CellValue> = members
             .iter()
             .flat_map(|k| match agg {
@@ -674,7 +682,14 @@ impl Model {
             return self.fold_stored(key, members, agg);
         }
         let reach = Reach::Fold { agg, covers };
-        match eval_expr(&formula.expr, key, self, &formula.target_category, depth, reach) {
+        match eval_expr(
+            &formula.expr,
+            key,
+            self,
+            &formula.target_category,
+            depth,
+            reach,
+        ) {
             Ok(n) => Some(CellValue::Number(n)),
             Err(e) => Some(CellValue::Error(e)),
         }
@@ -986,7 +1001,17 @@ mod agg_helper_tests {
         let empty = Expr::Ref("Revenue".to_string());
         let ctx = CellKey::new(vec![]);
         let never = |_: &Expr, _: &CellKey| unreachable!("a bare Ref never folds per record");
-        let run = |func| eval_agg(&model, &[ctx.clone()], "_Measure", &func, &empty, None, &never);
+        let run = |func| {
+            eval_agg(
+                &model,
+                &[ctx.clone()],
+                "_Measure",
+                &func,
+                &empty,
+                None,
+                &never,
+            )
+        };
 
         assert_eq!(run(AggFunc::Sum), Ok(0.0));
         assert_eq!(run(AggFunc::Count), Ok(0.0));

crates/improvise-core/src/view/layout.rs

@@ -661,7 +661,6 @@ impl GridLayout {
         axis_label(&self.col_items, col)
     }
 
-    /// Display text for (row, col) in either mode.
     /// The value of a pivot key as THIS grid reads it: aggregated over its None
     /// axis, and a fold over the members it draws (core.md §14). One evaluation.
     pub fn evaluate(&self, model: &Model, key: &CellKey) -> Option<CellValue> {
@@ -669,6 +668,7 @@ impl GridLayout {
         model.evaluate_covering(key, Aggregation::of(&self.none_cats), &drawn)
     }
 
+    /// Display text for (row, col) in either mode.
     pub fn display_text(
         &self,
         model: &Model,
@@ -681,8 +681,7 @@ impl GridLayout {
         } else {
             self.cell_key(row, col)
                 .and_then(|key| {
-                    let value =
-                        self.evaluate(model, &key)?;
+                    let value = self.evaluate(model, &key)?;
                     Some(plan.at(Some(&key)).value(Some(&value)))
                 })
                 .unwrap_or_default()
@@ -1803,7 +1802,9 @@ mod tests {
             &["Profit = Revenue - Cost", "Margin = Profit / Revenue"][..],
         ] {
             let mut folded = margin_book(formulas);
-            folded.active_view_mut().toggle_group_collapse("Month", "Q1");
+            folded
+                .active_view_mut()
+                .toggle_group_collapse("Month", "Q1");
             let fold = grid_value(&mut folded, "Q1", "Margin").expect("a Q1 fold");
 
             let mut none = margin_book(formulas);
@@ -1811,7 +1812,10 @@ mod tests {
             let over_none = grid_value(&mut none, "", "Margin").expect("a None-axis cell");
 
             assert!((fold - 0.8).abs() < 1e-9, "{formulas:?}: fold {fold}");
-            assert!((fold - over_none).abs() < 1e-9, "{formulas:?}: {fold} vs {over_none}");
+            assert!(
+                (fold - over_none).abs() < 1e-9,
+                "{formulas:?}: {fold} vs {over_none}"
+            );
         }
     }
 
@@ -1858,7 +1862,11 @@ mod tests {
 
         wb.active_view_mut().toggle_group_collapse("Month", "Q1");
         assert_eq!(column_total(&wb), 103.0, "z moved the total");
-        assert_eq!(grid_value(&mut wb, "Q1", "Food"), Some(3.0), "Feb is hidden");
+        assert_eq!(
+            grid_value(&mut wb, "Q1", "Food"),
+            Some(3.0),
+            "Feb is hidden"
+        );
 
         // Every member hidden: no fold is drawn, and the total still agrees.
         wb.active_view_mut().hide_item("Month", "Jan");