Merge collapse-fu/wm20 (500d4609) into collapse-fu/h6z1

# Conflicts:
#	benches/grid_projection.rs

benches/grid_projection.rs

@@ -105,6 +105,51 @@ fn chain_app(regions: usize, products: usize) -> App {
     app
 }
 
+/// Days in collapsed groups of `per_group` on Row, with Product: every row is a
+/// fold over a few days, never the whole category (improvise-jq0m).
+fn grouped_app(days: usize, per_group: usize, products: usize, measures: usize) -> App {
+    let mut wb = Workbook::new("Grouped");
+    wb.add_category("Day").unwrap();
+    wb.add_category("Product").unwrap();
+    for d in 0..days {
+        let group = format!("G{}", d / per_group);
+        wb.add_item_in_group("Day", format!("Day{d}"), group)
+            .unwrap();
+    }
+    for p in 0..products {
+        wb.add_item("Product", format!("Product{p}")).unwrap();
+    }
+    for m in 0..measures {
+        wb.add_item("_Measure", format!("Measure{m}")).unwrap();
+    }
+    {
+        let v = wb.active_view_mut();
+        v.set_axis("Day", Axis::Row);
+        v.set_axis("Product", Axis::Row);
+        v.set_axis("_Measure", Axis::Column);
+        for g in 0..days.div_ceil(per_group) {
+            v.toggle_group_collapse("Day", &format!("G{g}"));
+        }
+    }
+    let mut n = 0.0f64;
+    for d in 0..days {
+        for p in 0..products {
+            for m in 0..measures {
+                n += 1.0;
+                let key = CellKey::new(vec![
+                    ("Day".to_string(), format!("Day{d}")),
+                    ("Product".to_string(), format!("Product{p}")),
+                    ("_Measure".to_string(), format!("Measure{m}")),
+                ]);
+                wb.model.set_cell(key, CellValue::Number(n));
+            }
+        }
+    }
+    let mut app = App::new(wb, None);
+    app.rebuild_layout();
+    app
+}
+
 fn whole_grid(app: &App) -> Viewport {
     Viewport {
         row_offset: 0,
@@ -196,4 +241,8 @@ fn main() {
     println!("\nfolded rows and a five-formula chain (improvise-h6z1)\n");
     report("folded chain ~18k", chain_app(1000, 30), 5);
     report("folded chain ~90k", chain_app(5000, 30), 3);
+    println!("\ncollapsed small groups on the row axis (improvise-jq0m)\n");
+    report("364 days in weeks", grouped_app(364, 7, 20, 6), 5);
+    report("364 days in pairs", grouped_app(364, 2, 20, 6), 5);
+    report("1000 days in pairs", grouped_app(1000, 2, 10, 4), 3);
 }

context/code/command.md

@@ -1805,11 +1805,44 @@ one `CellKey` per member, the cross product of every spanned category's covered
 items (3,000 keys for an 18k column), and looked each one up. Now
 `group_expansion` returns the expansion as data: a `base` key and the covered
 items of each expanded category. `fold_stored` hands that to
-`DataStore::spanned_values`, which narrows by `base` through the index once and
-filters each candidate by membership. In `Exact` mode a candidate must also
-carry no coordinate beyond those, which is what `get(member)` answered. Only a
-formula still enumerates members (`contexts_of`), since its references are
-evaluated per context.
+`DataStore::spanned_values`, which narrows through the index and filters each
+candidate by membership. In `Exact` mode a candidate must also carry no
+coordinate beyond those, which is what `get(member)` answered. Only a formula
+still enumerates members (`contexts_of`), since its references are evaluated
+per context.
+
+**How it narrows is a choice, and the first version got it wrong
+(improvise-jq0m).** Narrowing by `base` alone is right for a column total, which
+spans *every* item of each row category. It is wrong for a collapsed group,
+which spans two or seven items of a category with hundreds: `base` drops the
+grouped category entirely, so every fold cell scanned every record of the
+column, and a grid of small groups went quadratic. The review measured 1000
+days in pairs at 11–15.7 s a keypress, against 0.44–0.51 s before #239. So
+`spanned_positions` now prices both narrowings off the index's own set sizes
+and takes the cheaper: once by `base`, or once per covered item of the span
+with the fewest items, each intersected with `base`. The per-item positions are
+disjoint (a record has one item per category) and are sorted afterwards, so
+both paths return the store's order. A `base` naming a name the store never
+interned matches nothing on both paths.
+
+The bench now carries the grouped cases, which it could not see before: Day in
+collapsed groups on Row, with Product, and `_Measure` on Column. The machine is
+an Apple M4 Max (16 cores, 64 GB) at load average 15–63. The figures are
+keypress (`whole-grid + col_widths`), min-of-N, from two alternating rounds,
+the second reversed. Each cell gives both rounds:
+
+| case | before #239 (`5932ad74`) | `f668ddc9` | jq0m | jq0m ÷ before |
+|---|---|---|---|---|
+| 4.8k dense | 33.9 / 39.1 ms | 40.7 / 40.2 ms | 36.3 / 33.3 ms | 1.07 / 0.85 |
+| 18k dense | 237 / 239 ms | 238 / 232 ms | 196 / 211 ms | 0.83 / 0.88 |
+| 90k dense | 3.04 / 3.46 s | 3.24 / 2.70 s | 2.68 / 2.84 s | 0.88 / 0.82 |
+| 364 days in weeks | 483 / 415 ms | 1.42 / 1.18 s | 356 / 399 ms | 0.74 / 0.96 |
+| 364 days in pairs | 715 / 539 ms | 5.02 / 4.62 s | 471 / 518 ms | 0.66 / 0.96 |
+| 1000 days in pairs | 406 / 399 ms | 10.8 / 20.2 s | 352 / 364 ms | 0.87 / 0.91 |
+
+The grouped cases now come in **below** the pre-#239 figures. The old
+per-member path looked up each member through a full `CellKey`, and the
+per-item narrowing intersects index sets directly.
 
 Two levers were **not** taken. Combining the walk's row values for decomposable
 aggregators was rejected under §5 as a second derivation of the total. It is

context/test-evidence/core.md

@@ -1551,3 +1551,30 @@ Three faults were injected, each run against the full `improvise-core` and
   (collapsed-group, hidden-row and CSV-agreement tests).
 - A category the store never interned is skipped instead of matching nothing.
   **Only this test fails.**
+
+### a_span_finds_what_its_members_find_in_store_order
+
+improvise-jq0m. `spanned_values` now chooses between two narrowings, and a
+fixed example can only ever take one of them. This property builds random
+stores over `R`, `P` and `M`. Some records lack `R`, some carry an extra `X`,
+and `r6` is a covered item the store never interned. It spans `R` and sometimes
+`P`, in both modes, and compares the result with the per-member definition,
+**in store order**: the values the members' own lookups find, read back in
+`iter_cells` order. Store and span sizes vary across cases, so both narrowings
+are taken.
+
+Three faults were injected, each run against the full `improvise-core` and
+`improvise-command` suites:
+
+- The per-item narrowing drops the base, so it keeps each item's pair alone.
+  This test and `a_spanned_scan_finds_what_a_lookup_per_member_finds` fail.
+  **Nothing in either crate's grid or command tests does.** Those fixtures never
+  hold a second value of the base's coordinate within a small span.
+- The base narrowing drops the base. This test fails, along with eight
+  fold and totals tests.
+- The per-item positions are not sorted after they are merged. **Only this
+  test fails.** Values stay correct and only their order moves, which changes
+  a Sum or Avg in the last bits.
+
+The regression itself is a cost, which no test here can see. The evidence for
+it is the `grid_projection` bench's grouped cases, added for this ticket.

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

@@ -378,16 +378,30 @@ impl DataStore {
     /// Positions in `cells` of every cell matching `partial`, **in insertion
     /// order**. The index narrows; the primary map orders (§8).
     fn matching_positions(&self, partial: &[(String, String)]) -> Vec<usize> {
-        // Intern the partial key (lookup only, no new symbols)
-        let interned_partial: Vec<(Symbol, Symbol)> = partial
-            .iter()
-            .filter_map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
-            .collect();
-
-        if interned_partial.len() < partial.len() {
-            return vec![];
+        self.intern_partial(partial)
+            .map(|pairs| self.positions_of(&pairs))
+            .unwrap_or_default()
     }
 
+    /// `partial` as symbols, or `None` when a name was never interned, which
+    /// no stored cell can then match.
+    fn intern_partial(&self, partial: &[(String, String)]) -> Option<Vec<(Symbol, Symbol)>> {
+        partial
+            .iter()
+            .map(|(c, i)| Some((self.symbols.get(c)?, self.symbols.get(i)?)))
+            .collect()
+    }
+
+    /// How many candidates narrowing by `pairs` would visit: its smallest index set.
+    fn narrowing_cost(&self, pairs: &[(Symbol, Symbol)]) -> usize {
+        pairs
+            .iter()
+            .map(|pair| self.index.get(pair).map_or(0, HashSet::len))
+            .min()
+            .unwrap_or(self.cells.len())
+    }
+
+    fn positions_of(&self, interned_partial: &[(Symbol, Symbol)]) -> Vec<usize> {
         let mut sets: Vec<&HashSet<InternedKey>> = interned_partial
             .iter()
             .filter_map(|pair| self.index.get(pair))
@@ -450,7 +464,7 @@ impl DataStore {
                     .iter()
                     .all(|(cat, items)| ikey.0.iter().any(|(c, i)| c == cat && items.contains(i)))
         };
-        self.matching_positions(partial)
+        self.spanned_positions(partial, &wanted)
             .into_iter()
             .filter_map(|n| self.cells.get_index(n))
             .filter(|(ikey, _)| admits(ikey))
@@ -458,6 +472,39 @@ impl DataStore {
             .collect()
     }
 
+    /// Candidates for a span: narrowed once by `partial`, or once per covered item
+    /// of its smallest span, whichever visits fewer. Sorted (improvise-jq0m).
+    fn spanned_positions(
+        &self,
+        partial: &[(String, String)],
+        wanted: &[(Symbol, HashSet<Symbol>)],
+    ) -> Vec<usize> {
+        self.intern_partial(partial)
+            .map(|base| self.narrowed(&base, wanted))
+            .unwrap_or_default()
+    }
+
+    fn narrowed(
+        &self,
+        base: &[(Symbol, Symbol)],
+        wanted: &[(Symbol, HashSet<Symbol>)],
+    ) -> Vec<usize> {
+        let with = |cat: Symbol, item: Symbol| [base, &[(cat, item)]].concat();
+        let per_item: Vec<Vec<(Symbol, Symbol)>> = wanted
+            .iter()
+            .min_by_key(|(_, items)| items.len())
+            .map(|(cat, items)| items.iter().map(|i| with(*cat, *i)).collect())
+            .unwrap_or_default();
+        let item_cost: usize = per_item.iter().map(|p| self.narrowing_cost(p)).sum();
+        let narrow_per_item = !per_item.is_empty() && item_cost < self.narrowing_cost(base);
+        let mut positions: Vec<usize> = match narrow_per_item {
+            true => per_item.iter().flat_map(|p| self.positions_of(p)).collect(),
+            false => self.positions_of(base),
+        };
+        positions.sort_unstable();
+        positions
+    }
+
     /// Every cell matching `partial`, in insertion order. Allocates a
     /// `CellKey` per match — prefer `matching_values` when values suffice.
     #[allow(dead_code)]
@@ -1008,6 +1055,60 @@ mod prop_tests {
                     "returned key {result_key} does not match partial {partial:?}");
             }
         }
+
+        /// improvise-jq0m: whichever narrowing it picks, a span returns what a lookup
+        /// per member returns, in store order. context/test-evidence/core.md.
+        #[test]
+        fn a_span_finds_what_its_members_find_in_store_order(
+            cells in prop::collection::vec((0u8..6, 0u8..3, 0u8..2, any::<bool>()), 0..40),
+            covered_r in prop::collection::vec(any::<bool>(), 7),
+            covered_p in prop::option::of(prop::collection::vec(any::<bool>(), 3)),
+            base_m in 0u8..2,
+            exact in any::<bool>(),
+        ) {
+            let mut store = DataStore::new();
+            for (n, (r, p, m, extra)) in cells.iter().enumerate() {
+                let mut key = CellKey::new(vec![
+                    ("P".into(), format!("p{p}")),
+                    ("M".into(), format!("m{m}")),
+                ]);
+                // r = 5 leaves the record without the spanned category at all.
+                key = match r { 5 => key, r => key.with("R", format!("r{r}")) };
+                key = match extra { true => key.with("X", "x"), false => key };
+                store.set(key, CellValue::Number(n as f64));
+            }
+            // r6 is never stored, so it is a covered item the store never interned.
+            let pick = |prefix: &str, flags: &[bool]| -> Vec<String> {
+                flags.iter().enumerate().filter(|(_, f)| **f)
+                    .map(|(i, _)| format!("{prefix}{i}")).collect()
+            };
+            let rs = pick("r", &covered_r);
+            let ps = covered_p.as_deref().map(|f| pick("p", f));
+            let base = CellKey::new(vec![("M".into(), format!("m{base_m}"))]);
+            let mut spans: Vec<(&str, Vec<&str>)> = vec![("R", rs.iter().map(String::as_str).collect())];
+            if let Some(ps) = &ps {
+                spans.push(("P", ps.iter().map(String::as_str).collect()));
+            }
+            let mut members = vec![base.clone()];
+            for (cat, items) in &spans {
+                members = members.iter()
+                    .flat_map(|k| items.iter().map(|i| k.clone().with(*cat, *i)))
+                    .collect();
+            }
+            let matched: std::collections::HashSet<CellKey> = members.iter()
+                .flat_map(|k| match exact {
+                    true => store.get(k).map(|_| k.clone()).into_iter().collect::<Vec<_>>(),
+                    false => store.matching_cells(&k.0).into_iter().map(|(k, _)| k).collect(),
+                })
+                .collect();
+            let expected: Vec<CellValue> = store.iter_cells()
+                .filter(|(k, _)| matched.contains(k))
+                .map(|(_, v)| v.clone())
+                .collect();
+            let got: Vec<CellValue> = store.spanned_values(&base.0, &spans, exact)
+                .into_iter().cloned().collect();
+            prop_assert_eq!(got, expected);
+        }
     }
 }