From 4f225e88fa181adfc6e3bbc696d709cfa43bc6c0 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 20:50:50 -0700 Subject: [PATCH 1/4] WIP improvise-6qet: a collapsed group contributes one aggregate row Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .../improvise-command/src/command/cmd/grid.rs | 57 ++++- .../improvise-command/src/command/cmd/mode.rs | 6 + crates/improvise-command/src/ui/app.rs | 3 +- crates/improvise-command/src/ui/project.rs | 17 +- crates/improvise-command/src/ui/viewport.rs | 4 +- crates/improvise-core/src/model/category.rs | 10 + crates/improvise-core/src/model/types.rs | 48 ++++ crates/improvise-core/src/view/layout.rs | 223 +++++++++++++++--- 8 files changed, 314 insertions(+), 54 deletions(-) diff --git a/crates/improvise-command/src/command/cmd/grid.rs b/crates/improvise-command/src/command/cmd/grid.rs index ef471310..3097273c 100644 --- a/crates/improvise-command/src/command/cmd/grid.rs +++ b/crates/improvise-command/src/command/cmd/grid.rs @@ -1,9 +1,10 @@ use crate::model::cell::CellValue; use crate::ui::app::AppMode; use crate::ui::effect::{self, Effect, ViewTarget}; -use crate::view::AxisEntry; +use crate::view::{AxisEntry, GridLayout}; use super::core::{Cmd, CmdContext}; +use super::navigation::viewport_effects; #[cfg(test)] mod tests { @@ -338,11 +339,57 @@ impl Cmd for ToggleGroupAtCursor { let Some((cat, group)) = lookup else { return vec![]; }; - vec![Box::new(effect::ToggleGroup { + let landing = self.landing(ctx, &cat, &group); + // The toggle reframes and so resets the cursor; the placement must come + // AFTER it or it is thrown away (gotcha 24, core.md §14). + let mut effects: Vec> = vec![Box::new(effect::ToggleGroup { view: ViewTarget::Active, category: cat, group, - })] + })]; + effects.extend(landing.map(|at| self.place_cursor(ctx, at)).unwrap_or_default()); + effects + } +} + +impl ToggleGroupAtCursor { + /// Where the toggled group sits once toggled, asked of the layout the toggle + /// will produce — collapsing an inner level shifts every earlier run (§14). + fn landing(&self, ctx: &CmdContext, cat: &str, group: &str) -> Option { + let (cats, tuple) = match self.is_row { + true => (&ctx.layout.row_cats, ctx.layout.row_tuple(ctx.selected.0)?), + false => (&ctx.layout.col_cats, ctx.layout.col_tuple(ctx.selected.1)?), + }; + let prefix = tuple.get(..cats.iter().position(|c| c == cat)?)?; + let mut view = ctx.view.clone(); + view.toggle_group_collapse(cat, group); + let after = GridLayout::with_frozen_records(ctx.model, &view, None); + match self.is_row { + true => after.row_run_start(cat, group, prefix), + false => after.col_run_start(cat, group, prefix), + } + } + + /// The cursor on `at`, the scroll restored where the toggle zeroed it and + /// moved only as far as `at` needs. + fn place_cursor(&self, ctx: &CmdContext, at: usize) -> Vec> { + let (row, col) = match self.is_row { + true => (at, ctx.selected.1), + false => (ctx.selected.0, at), + }; + let mut effects: Vec> = vec![ + Box::new(effect::SetRowOffset(ctx.row_offset)), + Box::new(effect::SetColOffset(ctx.col_offset)), + ]; + effects.extend(viewport_effects( + row, + col, + ctx.row_offset, + ctx.col_offset, + ctx.visible_rows, + ctx.visible_cols, + )); + effects } } @@ -363,8 +410,8 @@ impl Cmd for HideSelectedRowItem { .row_items .iter() .filter_map(|e| { - if let AxisEntry::DataItem(v) = e { - Some(v) + if let AxisEntry::DataItem { tuple, .. } = e { + Some(tuple) } else { None } diff --git a/crates/improvise-command/src/command/cmd/mode.rs b/crates/improvise-command/src/command/cmd/mode.rs index 043e4b73..bdf80cc3 100644 --- a/crates/improvise-command/src/command/cmd/mode.rs +++ b/crates/improvise-command/src/command/cmd/mode.rs @@ -320,6 +320,12 @@ impl Cmd for EditOrDrill { "edit-or-drill" } fn execute(&self, ctx: &CmdContext) -> Vec> { + // Neither edit nor drill: no record carries a group's name (core.md §14). + if ctx.layout.folds_a_group(ctx.selected.0, ctx.selected.1) { + return vec![effect::set_status( + "Collapsed group — press z to expand it, then edit an item", + )]; + } // Only **regular** categories on None are true aggregation: virtuals like // `_Index`/`_Dim` are always None in pivot mode and imply nothing. let regular_none = ctx.none_cats().iter().any(|c| { diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index b026516f..56f6fb8c 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -101,9 +101,10 @@ fn frame_of(layout: &crate::view::layout::GridLayout, view: &View) -> u64 { group_name.hash(&mut h); view.is_group_collapsed(cat_name, group_name).hash(&mut h); } - crate::view::layout::AxisEntry::DataItem(tuple) => { + crate::view::layout::AxisEntry::DataItem { tuple, collapsed } => { 1u8.hash(&mut h); tuple.hash(&mut h); + collapsed.hash(&mut h); } // Drawn as nothing, but it ends a group's run (improvise-gd83), // so where it sits is what a row's group attribution depends on. diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index 7d33657b..6ee0b6f3 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -205,7 +205,7 @@ fn axis_frame( ) -> AxisFrame { let total = items .iter() - .filter(|e| matches!(e, AxisEntry::DataItem(_))) + .filter(|e| matches!(e, AxisEntry::DataItem { .. })) .count(); let mut data_idx = 0usize; let mut entries: Vec = Vec::new(); @@ -230,7 +230,7 @@ fn axis_frame( // Queued like a header, never clearing one: a COLLAPSED group has // no item to carry it and reaches the frame only through `pending`. AxisEntry::GroupEnd => pending.push(FrameEntry::GroupEnd), - AxisEntry::DataItem(tuple) => { + AxisEntry::DataItem { tuple, .. } => { let idx = data_idx; data_idx += 1; if window.contains(&idx) { @@ -330,11 +330,14 @@ fn cell_id(app: &App, layout: &GridLayout, row: usize, col: usize) -> Option Vec { let mut widths = vec![0u16; n]; let items = layout.col_items.iter().filter_map(|e| match e { - AxisEntry::DataItem(v) => Some(v), + AxisEntry::DataItem { tuple, .. } => Some(tuple), AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None, }); for (ci, levels) in items.enumerate().take(n) { @@ -152,7 +152,7 @@ pub fn compute_col_widths(model: &Model, layout: &GridLayout, plan: &FormatPlan) /// keypress-path caller that borrows tuples — see `context/code/command.md`. pub fn compute_row_header_width(layout: &GridLayout) -> u16 { let items = layout.row_items.iter().filter_map(|e| match e { - AxisEntry::DataItem(v) => Some(v), + AxisEntry::DataItem { tuple, .. } => Some(tuple), AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None, }); header_level_widths(items, layout.row_cats.len().max(1)) diff --git a/crates/improvise-core/src/model/category.rs b/crates/improvise-core/src/model/category.rs index a2748df6..500ad680 100644 --- a/crates/improvise-core/src/model/category.rs +++ b/crates/improvise-core/src/model/category.rs @@ -199,6 +199,16 @@ impl Category { pub fn ordered_item_names(&self) -> Vec<&str> { self.items.keys().map(|s| s.as_str()).collect() } + + /// The items whose own `group` names `group`, in insertion order. Read off the + /// items, never `groups`: `add_item_in_group` registers no `Group` (core.md §14). + pub fn items_in_group(&self, group: &str) -> Vec<&str> { + self.items + .values() + .filter(|i| i.group.as_deref() == Some(group)) + .map(|i| i.name.as_str()) + .collect() + } } #[cfg(test)] diff --git a/crates/improvise-core/src/model/types.rs b/crates/improvise-core/src/model/types.rs index 522f7da6..b68c4a07 100644 --- a/crates/improvise-core/src/model/types.rs +++ b/crates/improvise-core/src/model/types.rs @@ -523,6 +523,10 @@ impl Model { } pub fn evaluate_aggregated(&self, key: &CellKey, agg: Aggregation) -> Option { + let members = self.group_expansion(key); + if !members.is_empty() { + return self.evaluate_group(key, &members, agg); + } if agg == Aggregation::Exact { return self.evaluate(key); } @@ -536,6 +540,50 @@ impl Model { aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key)) } + /// One key per member where `key` names a collapsed GROUP rather than an item — + /// the cross product over every such coordinate. Empty when none does (§14). + fn group_expansion(&self, key: &CellKey) -> Vec { + let mut expanded = vec![key.clone()]; + for (cat_name, name) in &key.0 { + let members = self + .category(cat_name) + .filter(|cat| !cat.items.contains_key(name.as_str())) + .map(|cat| cat.items_in_group(name)) + .unwrap_or_default(); + if members.is_empty() { + continue; + } + expanded = expanded + .iter() + .flat_map(|k| members.iter().map(|m| k.clone().with(cat_name, *m))) + .collect(); + } + match expanded.as_slice() { + [only] if only == key => Vec::new(), + _ => expanded, + } + } + + /// A group's cell: stored values fold ONCE over every record the group covers, + /// a derived one folds its members' results — why the two differ: core.md §14. + fn evaluate_group(&self, key: &CellKey, members: &[CellKey], agg: Aggregation) -> Option { + let values: Vec = match self.formula_for(key) { + Some(_) => members + .iter() + .filter_map(|k| self.evaluate_aggregated(k, agg)) + .collect(), + None => members + .iter() + .flat_map(|k| match agg { + Aggregation::Exact => self.data.get(k).into_iter().collect(), + Aggregation::OverHidden => self.data.matching_values(&k.0), + }) + .cloned() + .collect(), + }; + aggregate_cell_values(&values.iter().collect::>(), self.agg_for(key)) + } + /// [`Self::evaluate_aggregated`] as `f64`, with `0.0` for an empty cell. pub fn evaluate_aggregated_f64(&self, key: &CellKey, agg: Aggregation) -> f64 { self.evaluate_aggregated(key, agg) diff --git a/crates/improvise-core/src/view/layout.rs b/crates/improvise-core/src/view/layout.rs index d6ab4c00..c93e4d08 100644 --- a/crates/improvise-core/src/view/layout.rs +++ b/crates/improvise-core/src/view/layout.rs @@ -187,7 +187,7 @@ pub enum RecordCol { } /// One entry on a grid axis. A `GroupHeader` is always visible so its label -/// and collapse state show; a `DataItem` is absent when its group collapses. +/// and collapse state show; a collapsed group is ONE `DataItem` (core.md §14). #[derive(Debug, Clone, PartialEq)] pub enum AxisEntry { GroupHeader { @@ -197,18 +197,73 @@ pub enum AxisEntry { /// The run opened by the last `GroupHeader` ends here (improvise-gd83). /// Nothing is drawn for it; it exists so membership is not inherited. GroupEnd, - DataItem(Vec), + /// `collapsed` is not derivable from `tuple`: a group and an item may share a + /// name. Set for a folded group and for everything expanded beneath one. + DataItem { tuple: Vec, collapsed: bool }, +} + +impl AxisEntry { + /// An ordinary, uncollapsed data item. + pub fn item(tuple: Vec) -> Self { + AxisEntry::DataItem { + tuple, + collapsed: false, + } + } } /// The data-item tuples on an axis, skipping group headers. Every public /// `row`/`col` argument indexes this sequence, not the visual entry list. fn data_items(items: &[AxisEntry]) -> impl Iterator> { items.iter().filter_map(|e| match e { - AxisEntry::DataItem(v) => Some(v), + AxisEntry::DataItem { tuple, .. } => Some(tuple), AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None, }) } +/// Whether the data item at `idx` folds a collapsed group. +fn folds(items: &[AxisEntry], idx: usize) -> bool { + items + .iter() + .filter_map(|e| match e { + AxisEntry::DataItem { collapsed, .. } => Some(*collapsed), + AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None, + }) + .nth(idx) + .unwrap_or(false) +} + +/// The data index of the first item in the run `cat`/`group` opens under +/// `prefix` — where a toggle's cursor lands, collapsing or expanding (§14). +fn run_start( + cats: &[String], + items: &[AxisEntry], + cat: &str, + group: &str, + prefix: &[String], +) -> Option { + let level = cats.iter().position(|c| c == cat)?; + let mut data_idx = 0usize; + let mut armed = false; + for entry in items { + match entry { + AxisEntry::GroupHeader { + cat_name, + group_name, + } => armed |= cat_name == cat && group_name == group, + AxisEntry::GroupEnd => {} + AxisEntry::DataItem { tuple, .. } => { + if armed && tuple.get(..level) == Some(prefix) { + return Some(data_idx); + } + armed = false; + data_idx += 1; + } + } + } + None +} + /// The display label of the data item at `idx`, or empty when out of bounds. fn axis_label(items: &[AxisEntry], idx: usize) -> String { data_items(items) @@ -222,7 +277,7 @@ fn data_index_to_visual(items: &[AxisEntry], data_idx: usize) -> Option { items .iter() .enumerate() - .filter(|(_, e)| matches!(e, AxisEntry::DataItem(_))) + .filter(|(_, e)| matches!(e, AxisEntry::DataItem { .. })) .map(|(vi, _)| vi) .nth(data_idx) } @@ -267,7 +322,7 @@ fn prune_axis_items(items: &mut Vec, keep: impl Fn(usize) -> bool) { run_open = false; } } - AxisEntry::DataItem(_) => { + AxisEntry::DataItem { .. } => { run_had_item = true; if keep(data_idx) { if let Some(h) = pending_header.take() { @@ -344,7 +399,7 @@ impl GridLayout { && let Some(records) = frozen_records { let row_items: Vec = (0..records.len()) - .map(|i| AxisEntry::DataItem(vec![i.to_string()])) + .map(|i| AxisEntry::item(vec![i.to_string()])) .collect(); layout.row_items = row_items; layout.records = Some(records); @@ -427,13 +482,13 @@ impl GridLayout { let records = Self::group_records(model, &page_constraints(&page_coords)); let row_items: Vec = (0..records.len()) - .map(|i| AxisEntry::DataItem(vec![i.to_string()])) + .map(|i| AxisEntry::item(vec![i.to_string()])) .collect(); let record_cols = Self::records_columns(model); let col_items: Vec = record_cols .iter() - .map(|c| AxisEntry::DataItem(vec![Self::record_col_label(c)])) + .map(|c| AxisEntry::item(vec![Self::record_col_label(c)])) .collect(); Self { @@ -687,11 +742,38 @@ impl GridLayout { group_name, } => Some(Some((cat_name.clone(), group_name.clone()))), AxisEntry::GroupEnd => Some(None), - AxisEntry::DataItem(_) => None, + AxisEntry::DataItem { .. } => None, }) .flatten() } + /// Whether the cell at `(row, col)` folds a collapsed group: an aggregate + /// with no record under it. The one home of that answer (§14). + pub fn folds_a_group(&self, row: usize, col: usize) -> bool { + folds(&self.row_items, row) || folds(&self.col_items, col) + } + + /// The first data row of `cat`/`group`'s run under `prefix` — where a toggle + /// of that group puts the cursor, asked of the layout AFTER the toggle (§14). + pub fn row_run_start(&self, cat: &str, group: &str, prefix: &[String]) -> Option { + run_start(&self.row_cats, &self.row_items, cat, group, prefix) + } + + /// [`Self::row_run_start`] for a column group. + pub fn col_run_start(&self, cat: &str, group: &str, prefix: &[String]) -> Option { + run_start(&self.col_cats, &self.col_items, cat, group, prefix) + } + + /// The coordinate tuple of the Nth data row. + pub fn row_tuple(&self, row: usize) -> Option<&[String]> { + data_items(&self.row_items).nth(row).map(Vec::as_slice) + } + + /// The coordinate tuple of the Nth data column. + pub fn col_tuple(&self, col: usize) -> Option<&[String]> { + data_items(&self.col_items).nth(col).map(Vec::as_slice) + } + /// The `(cat_name, group_name)` of the nearest `GroupHeader` preceding /// the Nth data column. pub fn col_group_for(&self, data_col: usize) -> Option<(String, String)> { @@ -705,25 +787,28 @@ impl GridLayout { group_name, } => Some(Some((cat_name.clone(), group_name.clone()))), AxisEntry::GroupEnd => Some(None), - AxisEntry::DataItem(_) => None, + AxisEntry::DataItem { .. } => None, }) .flatten() } } /// Expand one category into `AxisEntry` values under a coordinate prefix: a -/// `GroupHeader` per boundary, then the visible, non-collapsed items. +/// `GroupHeader` per boundary, then each visible item or collapsed group (§14). fn expand_category( model: &Model, view: &View, cat_name: &str, prefix: Vec, + prefix_collapsed: bool, ) -> Vec { let Some(cat) = model.category(cat_name) else { return vec![]; }; let mut result = Vec::new(); let mut last_group: Option<&str> = None; + // A group's items need not be contiguous, so a collapsed one folds ONCE. + let mut folded: Vec<&str> = Vec::new(); // effective_item_names, so _Measure picks up formula targets. let effective_names = model.effective_item_names(cat_name); @@ -735,6 +820,10 @@ fn expand_category( .items .get(item_name.as_str()) .and_then(|i| i.group.as_deref()); + let collapsed = item_group.filter(|g| view.is_group_collapsed(cat_name, g)); + if collapsed.is_some_and(|g| folded.contains(&g)) { + continue; + } // A header on entering a group; a `GroupEnd` only on leaving one for // NO group — another group's header already ends the run. @@ -751,14 +840,18 @@ fn expand_category( last_group = item_group; } - // Skip the data item if its group is collapsed. - if item_group.is_some_and(|g| view.is_group_collapsed(cat_name, g)) { - continue; + let mut tuple = prefix.clone(); + match collapsed { + Some(g) => { + folded.push(g); + tuple.push(g.to_string()); + } + None => tuple.push(item_name.to_string()), } - - let mut row = prefix.clone(); - row.push(item_name.to_string()); - result.push(AxisEntry::DataItem(row)); + result.push(AxisEntry::DataItem { + tuple, + collapsed: prefix_collapsed || collapsed.is_some(), + }); } // The expansion is the end of the run too: the next prefix's items follow // immediately and must not inherit it (improvise-gd83). @@ -772,14 +865,16 @@ fn expand_category( /// `vec![DataItem(vec![])]` when `cats` is empty. fn cross_product(model: &Model, view: &View, cats: &[String]) -> Vec { if cats.is_empty() { - return vec![AxisEntry::DataItem(vec![])]; + return vec![AxisEntry::item(vec![])]; } - let mut result: Vec = vec![AxisEntry::DataItem(vec![])]; + let mut result: Vec = vec![AxisEntry::item(vec![])]; for cat_name in cats { result = result .into_iter() .flat_map(|entry| match entry { - AxisEntry::DataItem(prefix) => expand_category(model, view, cat_name, prefix), + AxisEntry::DataItem { tuple, collapsed } => { + expand_category(model, view, cat_name, tuple, collapsed) + } marker @ (AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd) => vec![marker], }) .collect(); @@ -832,13 +927,13 @@ mod tests { #[test] fn prune_axis_items_indexes_data_positions_not_visual_ones() { let mut items = vec![ - AxisEntry::DataItem(vec!["A".into()]), + AxisEntry::item(vec!["A".into()]), AxisEntry::GroupHeader { cat_name: "C".into(), group_name: "G".into(), }, - AxisEntry::DataItem(vec!["B".into()]), - AxisEntry::DataItem(vec!["C".into()]), + AxisEntry::item(vec!["B".into()]), + AxisEntry::item(vec!["C".into()]), ]; // Data index 1 is "B": the header does not consume an index. If the // predicate saw visual indices, "A" would be the one dropped. @@ -861,8 +956,8 @@ mod tests { cat_name: "C".into(), group_name: "G".into(), }, - AxisEntry::DataItem(vec!["A".into()]), - AxisEntry::DataItem(vec!["B".into()]), + AxisEntry::item(vec!["A".into()]), + AxisEntry::item(vec!["B".into()]), ]; prune_axis_items(&mut items, |_| false); assert!(items.is_empty()); @@ -1256,29 +1351,70 @@ mod tests { ); } + /// The inversion of `collapsed_group_has_header_but_no_data_items`: a + /// collapsed group is ONE row. Evidence: context/test-evidence/core.md. #[test] - fn collapsed_group_has_header_but_no_data_items() { + fn a_collapsed_group_contributes_one_aggregate_row() { let mut wb = Workbook::new("T"); wb.add_category("Month").unwrap(); wb.add_category("Type").unwrap(); wb.add_item_in_group("Month", "Jan", "Q1").unwrap(); wb.add_item_in_group("Month", "Feb", "Q1").unwrap(); wb.add_item_in_group("Month", "Apr", "Q2").unwrap(); + wb.add_item_in_group("Month", "May", "Q2").unwrap(); wb.add_item("Type", "Food").unwrap(); wb.active_view_mut().toggle_group_collapse("Month", "Q1"); let layout = GridLayout::new(&wb.model, wb.active_view()); - // Q1 collapsed: header present, Jan and Feb absent; Q2 intact - assert_eq!(layout.row_count(), 1); // only Apr - let q1_header = layout + let labels: Vec = (0..layout.row_count()).map(|r| layout.row_label(r)).collect(); + assert_eq!(labels, ["Q1", "Apr", "May"], "Q1 folds into one row; Q2 stays open"); + assert!(layout.folds_a_group(0, 0), "the Q1 row is a fold"); + assert!(!layout.folds_a_group(1, 0), "Apr, under the OPEN group, is not"); + assert_eq!( + layout.row_group_for(0), + Some(("Month".into(), "Q1".into())), + "the fold sits under its own header, so z can name it" + ); + } + + /// Evidence: context/test-evidence/core.md. + #[test] + fn a_non_contiguous_collapsed_group_folds_once() { + let mut wb = Workbook::new("T"); + wb.add_category("Month").unwrap(); + wb.add_category("Type").unwrap(); + wb.add_item_in_group("Month", "A", "G1").unwrap(); + wb.add_item_in_group("Month", "B", "G2").unwrap(); + wb.add_item_in_group("Month", "C", "G1").unwrap(); + wb.add_item("Type", "Food").unwrap(); + wb.active_view_mut().toggle_group_collapse("Month", "G1"); + let layout = GridLayout::new(&wb.model, wb.active_view()); + let labels: Vec = (0..layout.row_count()).map(|r| layout.row_label(r)).collect(); + assert_eq!(labels, ["G1", "B"], "one G1 row, not one per run"); + let g1_headers = layout .row_items .iter() - .find(|e| matches!(e, AxisEntry::GroupHeader { group_name, .. } if group_name == "Q1")); - assert!(q1_header.is_some()); - let jan = layout - .row_items - .iter() - .find(|e| matches!(e, AxisEntry::DataItem(v) if v.contains(&"Jan".to_string()))); - assert!(jan.is_none()); + .filter(|e| matches!(e, AxisEntry::GroupHeader { group_name, .. } if group_name == "G1")) + .count(); + assert_eq!(g1_headers, 1, "a second G1 header would open a run with no row"); + } + + /// Evidence: context/test-evidence/core.md. + #[test] + fn a_category_nested_under_a_collapsed_group_is_folded_too() { + let mut wb = Workbook::new("T"); + wb.add_category("Month").unwrap(); + wb.add_category("Region").unwrap(); + wb.add_category("Type").unwrap(); + wb.add_item_in_group("Month", "Jan", "Q1").unwrap(); + wb.add_item_in_group("Month", "Apr", "Q2").unwrap(); + wb.add_item("Region", "North").unwrap(); + wb.add_item("Type", "Food").unwrap(); + wb.active_view_mut().set_axis("Region", Axis::Row); + wb.active_view_mut().toggle_group_collapse("Month", "Q1"); + let layout = GridLayout::new(&wb.model, wb.active_view()); + assert_eq!(layout.row_tuple(0), Some(&["Q1".to_string(), "North".to_string()][..])); + assert!(layout.folds_a_group(0, 0), "Q1/North has no record under it either"); + assert!(!layout.folds_a_group(1, 0), "Apr/North does"); } /// improvise-gd83: two arms — an ungrouped item AFTER a grouped run, and @@ -1513,13 +1649,22 @@ mod tests { cat_name: "Month".into(), group_name: "Q1".into() }, + AxisEntry::DataItem { + tuple: vec!["Q1".into()], + collapsed: true + }, AxisEntry::GroupEnd, - AxisEntry::DataItem(vec!["Sep".into()]), + AxisEntry::item(vec!["Sep".into()]), ], - "Q1 is collapsed, not empty — its header is the only way back, and Mar pruned" + "Q1 folds to one funded row under its header, and Mar pruned" ); assert_eq!( layout.row_group_for(0), + Some(("Month".into(), "Q1".into())), + "the fold is where z names the collapsed group from" + ); + assert_eq!( + layout.row_group_for(1), None, "Sep must not inherit the collapsed run" ); From bd2ed2fc1f47e0c18f73d3e2057711fa185455c5 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 20:58:34 -0700 Subject: [PATCH 2/4] WIP improvise-6qet: tests for the fold, the cursor landing and settability Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .../improvise-command/src/command/cmd/grid.rs | 136 +++++++++++++----- .../improvise-command/src/command/cmd/mode.rs | 33 +++++ crates/improvise-command/src/ui/project.rs | 94 +++++++----- crates/improvise-core/src/model/types.rs | 74 ++++++++++ 4 files changed, 267 insertions(+), 70 deletions(-) diff --git a/crates/improvise-command/src/command/cmd/grid.rs b/crates/improvise-command/src/command/cmd/grid.rs index 3097273c..0d02e7cd 100644 --- a/crates/improvise-command/src/command/cmd/grid.rs +++ b/crates/improvise-command/src/command/cmd/grid.rs @@ -34,10 +34,12 @@ mod tests { wb } + /// The real path: the App's own context, the command, then `apply_effects`, + /// so a placement the toggle's reframe throws away is seen. fn toggle_group_at_cursor(app: &mut crate::ui::app::App) -> String { + use crate::command::key::{KeyCode, KeyModifiers}; let effects = { - let reg = make_registry(); - let ctx = make_ctx(&app.model_state.workbook, &app.layout, ®); + let ctx = app.cmd_context(KeyCode::Char('z'), KeyModifiers::NONE); ToggleGroupAtCursor { is_row: true }.execute(&ctx) }; let dbg = effects_debug(&effects); @@ -45,6 +47,12 @@ mod tests { dbg } + fn row_labels(app: &crate::ui::app::App) -> Vec { + (0..app.layout.row_count()) + .map(|r| app.layout.row_label(r)) + .collect() + } + /// Evidence: `context/test-evidence/command.md`. #[test] fn toggling_at_the_cursor_names_the_group_the_cursor_sits_in_and_collapses_it() { @@ -52,68 +60,122 @@ mod tests { let mut app = App::new(two_groups_on_the_row_axis(), None); app.rebuild_layout(); + // Off row 0, or a toggle that ignores the cursor names Q1 and passes. + app.cursor_mut().selected = (2, 0); assert!( !app.model_state .workbook .active_view() - .is_group_collapsed("Month", "Q1"), + .is_group_collapsed("Month", "Q2"), "the fixture starts expanded, or the assertion below proves nothing" ); let dbg = toggle_group_at_cursor(&mut app); assert!( - dbg.contains(r#"ToggleGroup { view: Active, category: "Month", group: "Q1" }"#), - "the cursor sits on Jan, which is in Q1, got: {dbg}" - ); - assert!( - app.model_state - .workbook - .active_view() - .is_group_collapsed("Month", "Q1"), - "the effect was returned but the view did not move" - ); - } - - /// Evidence: `context/test-evidence/command.md`. - #[test] - fn a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one() { - use crate::ui::app::App; - - let mut app = App::new(two_groups_on_the_row_axis(), None); - app.rebuild_layout(); - toggle_group_at_cursor(&mut app); - - let dbg = toggle_group_at_cursor(&mut app); - - assert!( - dbg.contains(r#"group: "Q2""#), - "collapsing Q1 removes its rows, so data row 0 is now Apr in Q2, got: {dbg}" + dbg.contains(r#"ToggleGroup { view: Active, category: "Month", group: "Q2" }"#), + "the cursor sits on Apr, which is in Q2, got: {dbg}" ); let view = app.model_state.workbook.active_view(); assert!( - view.is_group_collapsed("Month", "Q1") && view.is_group_collapsed("Month", "Q2"), - "pressing twice collapses both groups; it is not an involution" + view.is_group_collapsed("Month", "Q2"), + "the effect was returned but the view did not move" ); + assert!(!view.is_group_collapsed("Month", "Q1"), "the neighbour was left alone"); } /// Evidence: `context/test-evidence/command.md`. #[test] - fn a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address() { + fn toggling_a_group_collapses_that_group_and_the_cursor_follows_it() { use crate::ui::app::App; let mut app = App::new(two_groups_on_the_row_axis(), None); app.rebuild_layout(); - toggle_group_at_cursor(&mut app); - toggle_group_at_cursor(&mut app); - assert_eq!(app.layout.row_count(), 0); + app.cursor_mut().selected = (2, 0); + assert_eq!(app.layout.row_label(2), "Apr", "the cursor starts inside Q2"); let dbg = toggle_group_at_cursor(&mut app); + assert!(dbg.contains(r#"group: "Q2""#), "Apr is in Q2, got: {dbg}"); + assert_eq!(row_labels(&app), ["Jan", "Feb", "Q2"]); + assert_eq!(app.cursor().selected, (2, 0), "the cursor is on the Q2 row"); + + let dbg = toggle_group_at_cursor(&mut app); + + assert!(dbg.contains(r#"group: "Q2""#), "z again names Q2, got: {dbg}"); + assert_eq!(row_labels(&app), ["Jan", "Feb", "Apr", "May"], "z z is a no-op"); + assert_eq!(app.cursor().selected, (2, 0), "back on Q2's first item"); + } + + /// Evidence: `context/test-evidence/command.md`. + #[test] + fn a_collapse_from_the_tail_of_a_run_lands_on_the_runs_first_row() { + use crate::ui::app::App; + + let mut app = App::new(two_groups_on_the_row_axis(), None); + app.rebuild_layout(); + app.cursor_mut().selected = (3, 0); + assert_eq!(app.layout.row_label(3), "May"); + + toggle_group_at_cursor(&mut app); + + assert_eq!(row_labels(&app), ["Jan", "Feb", "Q2"]); + assert_eq!(app.cursor().selected, (2, 0), "May folded into the Q2 row"); + } + + /// Evidence: `context/test-evidence/command.md`. + #[test] + fn collapsing_an_inner_group_lands_where_every_earlier_run_shrank() { + use crate::ui::app::App; + use crate::view::Axis; + + let mut wb = crate::workbook::Workbook::new("T"); + wb.add_category("Region").unwrap(); + wb.add_category("Month").unwrap(); + wb.add_category("Type").unwrap(); + wb.add_item("Region", "North").unwrap(); + wb.add_item("Region", "South").unwrap(); + for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] { + wb.add_item_in_group("Month", month, quarter).unwrap(); + } + wb.add_item("Type", "Food").unwrap(); + wb.active_view_mut().set_axis("Region", Axis::Row); + wb.active_view_mut().set_axis("Month", Axis::Row); + wb.active_view_mut().set_axis("Type", Axis::Column); + let mut app = App::new(wb, None); + app.rebuild_layout(); + assert_eq!(app.layout.row_cats, ["Region", "Month"], "Month is the INNER level"); + app.cursor_mut().selected = (6, 0); + assert_eq!(app.layout.row_tuple(6), Some(&["South".into(), "Apr".into()][..])); + + toggle_group_at_cursor(&mut app); + assert_eq!( - dbg, "[]", - "there is no row under the cursor to name a group" + row_labels(&app), + ["North/Jan", "North/Feb", "North/Q2", "South/Jan", "South/Feb", "South/Q2"] ); + assert_eq!(app.cursor().selected, (5, 0), "South's Q2, not the old index 6"); + } + + /// Evidence: `context/test-evidence/command.md`. + #[test] + fn a_collapsed_group_can_be_re_expanded_from_the_grid() { + use crate::ui::app::App; + + let mut app = App::new(two_groups_on_the_row_axis(), None); + app.rebuild_layout(); + app.cursor_mut().selected = (2, 0); + toggle_group_at_cursor(&mut app); + app.cursor_mut().selected = (1, 0); + toggle_group_at_cursor(&mut app); + assert_eq!(row_labels(&app), ["Q1", "Q2"], "both groups collapsed"); + app.cursor_mut().selected = (1, 0); + + let dbg = toggle_group_at_cursor(&mut app); + + assert!(dbg.contains(r#"group: "Q2""#), "the Q2 row names Q2, got: {dbg}"); + assert_eq!(row_labels(&app), ["Q1", "Apr", "May"]); + assert_eq!(app.cursor().selected, (1, 0), "on Q2's first item"); } #[test] diff --git a/crates/improvise-command/src/command/cmd/mode.rs b/crates/improvise-command/src/command/cmd/mode.rs index bdf80cc3..6d5d31b7 100644 --- a/crates/improvise-command/src/command/cmd/mode.rs +++ b/crates/improvise-command/src/command/cmd/mode.rs @@ -188,6 +188,39 @@ mod tests { ); } + /// Evidence: `context/test-evidence/command.md`. + #[test] + fn edit_or_drill_declines_a_collapsed_groups_row() { + let mut wb = crate::workbook::Workbook::new("T"); + wb.add_category("Month").unwrap(); + wb.add_category("Type").unwrap(); + for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] { + wb.add_item_in_group("Month", month, quarter).unwrap(); + } + wb.add_item("Type", "Food").unwrap(); + wb.active_view_mut().toggle_group_collapse("Month", "Q2"); + let layout = make_layout(&wb); + let reg = make_registry(); + let mut ctx = make_ctx(&wb, &layout, ®); + let edit = EditOrDrill { + edit_mode: AppMode::editing(), + }; + ctx.selected = (1, 0); + assert!( + effects_debug(&edit.execute(&ctx)).contains("Editing"), + "Feb, beside the fold, still edits" + ); + + ctx.selected = (2, 0); + let dbg = effects_debug(&edit.execute(&ctx)); + + assert_eq!(layout.row_label(2), "Q2"); + assert!( + dbg.contains("Collapsed group") && !dbg.contains("Editing") && !dbg.contains("Drill"), + "the fold names no record to edit or drill into, got: {dbg}" + ); + } + /// improvise-ir7l: the cell under the cursor is a formula target, so there is /// nothing to type into. `context/test-evidence/command.md`. #[test] diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index 6ee0b6f3..40792ba2 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -1319,8 +1319,8 @@ mod tests { .collect() } - /// A collapsed group at the bottom has no items to hang off and is still the - /// only row saying those rows exist, so the last window carries it. + /// A collapsed group at the bottom folds into one row, and the last window + /// carries its header above that row. Evidence: context/test-evidence/command.md. #[test] fn a_window_reaching_the_last_row_carries_the_trailing_group_header() { let app = app_with_a_trailing_group(true); @@ -1335,13 +1335,13 @@ mod tests { ); assert_eq!( grid.row_frame.len(), - 2, - "East is inside the collapsed group" + 3, + "East folds into the collapsed group's one row" ); assert_eq!( frame_shape(&grid.row_frame), - vec!["North", "South", "[Late]"], - "the header is the last entry, and it is what the user expands" + vec!["North", "South", "[Late]", "Late"], + "the header, then the row the group collapsed into" ); } @@ -1370,11 +1370,11 @@ mod tests { /// header comes. Two headers above, or the compaction is invisible. #[test] fn a_window_opening_inside_a_group_names_only_the_nearest() { - let app = app_with_consecutive_headers(); + let app = app_with_a_collapsed_group_between_rows(); let grid = grid_of( &app, Viewport { - row_offset: 1, + row_offset: 2, col_offset: 0, rows: 20, cols: 20, @@ -1396,9 +1396,9 @@ mod tests { ); } - /// Two consecutive group headers between two rows: `Hidden` is collapsed, so it - /// emits a header and no items, and `Late`'s follows immediately. - fn app_with_consecutive_headers() -> App { + /// Two group headers between `North` and `East`: `Hidden`, collapsed into one + /// row, and `Late`, open. + fn app_with_a_collapsed_group_between_rows() -> App { let mut wb = Workbook::new("Consecutive"); wb.add_category("Region").unwrap(); wb.add_item("Region", "North").unwrap(); @@ -1421,11 +1421,11 @@ mod tests { app } - /// Headers between two in-window rows all come, not just the nearest: the far - /// one is the only row saying those items exist. Above the window, the opposite. + /// Headers between two in-window rows all come, each above the row it opens. + /// Evidence: context/test-evidence/command.md. #[test] - fn consecutive_headers_between_two_rows_all_come() { - let app = app_with_consecutive_headers(); + fn every_header_between_two_in_window_rows_comes() { + let app = app_with_a_collapsed_group_between_rows(); let grid = grid_of( &app, Viewport { @@ -1437,25 +1437,14 @@ mod tests { ); assert_eq!( frame_shape(&grid.row_frame), - vec!["North", "[Hidden]", "[Late]", "East"], - "both headers sit between the two rows and both are drawn" - ); - let group = grid.row_frame.group_of(1).expect("East is under Late"); - assert_eq!(group.name, "Late", "the NEAREST header wins membership"); - let hidden = grid - .row_frame - .entries - .iter() - .find_map(|e| match e { - FrameEntry::Group(g) if g.name == "Hidden" => Some(g), - _ => None, - }) - .expect("kept"); - assert!( - hidden.collapsed, - "and it arrives collapsed, which is what makes it the row that \ - says those rows still exist" + vec!["North", "[Hidden]", "Hidden", "[Late]", "East"], + "both headers are drawn, the collapsed one above its folded row" ); + let hidden = grid.row_frame.group_of(1).expect("the fold is under Hidden"); + assert_eq!(hidden.name, "Hidden"); + assert!(hidden.collapsed, "and it arrives collapsed"); + let late = grid.row_frame.group_of(2).expect("East is under Late"); + assert_eq!(late.name, "Late", "the NEAREST header wins membership"); } fn app_with_data() -> App { @@ -4303,6 +4292,45 @@ mod cell_identity { .unwrap_or_else(|| panic!("cell ({row}, {col}) was projected with no identity")) } + /// Nothing regular on None, so only the fold can make the row an aggregate. + /// Evidence: context/test-evidence/command.md. + #[test] + fn a_collapsed_groups_row_is_projected_as_an_aggregate() { + let mut wb = Workbook::new("Folded"); + wb.add_category("Region").unwrap(); + wb.add_item("Region", "North").unwrap(); + wb.add_item_in_group("Region", "East", "Coastal").unwrap(); + wb.add_item_in_group("Region", "West", "Coastal").unwrap(); + wb.add_item("_Measure", "Revenue").unwrap(); + wb.active_view_mut().set_axis("Region", Axis::Row); + wb.active_view_mut().set_axis("_Measure", Axis::Column); + for (region, value) in [("North", 1.0), ("East", 3.0), ("West", 4.0)] { + wb.model.set_cell( + CellKey::new(vec![ + ("Region".to_string(), region.to_string()), + ("_Measure".to_string(), "Revenue".to_string()), + ]), + CellValue::Number(value), + ); + } + wb.active_view_mut() + .toggle_group_collapse("Region", "Coastal"); + let mut app = App::new(wb, None); + app.rebuild_layout(); + assert_eq!(app.layout.row_label(1), "Coastal"); + + assert!( + matches!(id_at(&app, 0, 0), CellId::Record(_)), + "North is stored, so the none-axis arm is not what answers below" + ); + assert!( + matches!(id_at(&app, 1, 0), CellId::Aggregate(_)), + "a fold has no record under it and must not be offered as settable" + ); + let grid = grid_of(&app, WHOLE); + assert_eq!(grid.cell(1, 0).expect("the fold's cell").text.trim(), "7.00"); + } + fn record_of_id(id: CellId) -> RecordCell { match id { CellId::Record(cell) => cell, diff --git a/crates/improvise-core/src/model/types.rs b/crates/improvise-core/src/model/types.rs index b68c4a07..173e2a38 100644 --- a/crates/improvise-core/src/model/types.rs +++ b/crates/improvise-core/src/model/types.rs @@ -1451,6 +1451,80 @@ mod model_tests { let result = m.evaluate_aggregated(&key, Aggregation::of(&none_cats)); assert_eq!(result, Some(CellValue::Number(20.0))); // avg(10, 30) = 20 } + + /// `Month` with `Q1 = {Jan, Feb}` built through `add_item_in_group`, which + /// registers no `Group` — the shape a document built in code has. + fn quarter_model(cells: &[(&str, &str, f64)]) -> Model { + let mut m = Model::new("T"); + m.add_category("Month").unwrap(); + m.add_category("Store").unwrap(); + for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2")] { + m.category_mut("Month") + .unwrap() + .add_item_in_group(month, quarter); + } + for store in ["A", "B"] { + m.category_mut("Store").unwrap().add_item(store); + } + m.category_mut("_Measure").unwrap().add_item("Revenue"); + for (month, store, value) in cells { + m.set_cell( + coord(&[("Month", month), ("Store", store), ("_Measure", "Revenue")]), + CellValue::Number(*value), + ); + } + m + } + + /// Evidence: context/test-evidence/core.md. + #[test] + fn a_collapsed_groups_cell_aggregates_its_items() { + let m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0), ("Apr", "A", 100.0)]); + let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "Revenue")]); + assert_eq!( + m.evaluate_aggregated(&key, Aggregation::Exact), + Some(CellValue::Number(7.0)), + "Jan + Feb; not absent, not Jan alone, not a count, not Q2's Apr" + ); + } + + /// Evidence: context/test-evidence/core.md. + #[test] + fn a_groups_stored_values_fold_once_not_per_member() { + use crate::formula::AggFunc; + let mut m = quarter_model(&[("Jan", "A", 2.0), ("Jan", "B", 4.0), ("Feb", "A", 9.0)]); + let key = coord(&[("Month", "Q1"), ("_Measure", "Revenue")]); + let over_store = Aggregation::of(&["Store".to_string()]); + m.measure_agg.insert("Revenue".to_string(), AggFunc::Avg); + assert_eq!( + m.evaluate_aggregated(&key, over_store), + Some(CellValue::Number(5.0)), + "avg(2, 4, 9); folding the members' averages gives avg(3, 9) = 6" + ); + m.measure_agg.insert("Revenue".to_string(), AggFunc::Count); + assert_eq!( + m.evaluate_aggregated(&key, over_store), + Some(CellValue::Number(3.0)), + "three records; counting the members' counts gives 2" + ); + } + + /// Evidence: context/test-evidence/core.md. + #[test] + fn an_item_named_like_a_group_is_read_as_the_item() { + let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]); + m.category_mut("Month").unwrap().add_item("Q1"); + m.set_cell( + coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "Revenue")]), + CellValue::Number(50.0), + ); + let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "Revenue")]); + assert_eq!( + m.evaluate_aggregated(&key, Aggregation::Exact), + Some(CellValue::Number(50.0)), + "the stored item, not Jan + Feb" + ); + } } #[cfg(test)] From 1a359b4ae606636b0be6a26e589dc6ae467058b9 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 21:06:41 -0700 Subject: [PATCH 3/4] improvise-6qet: CSV export test, formatting, and the reasoning in context/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A collapsed group is written to CSV as its own total row, because an export writes what the grid draws (improvise-iot6). core.md §14 carries the argument; the evidence entries record each rival and where it failed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- context/code/core.md | 102 +++++++++++++++ context/test-evidence/command.md | 123 ++++++++++++------ context/test-evidence/core.md | 68 ++++++++++ context/test-evidence/formula-and-io.md | 10 ++ .../improvise-command/src/command/cmd/grid.rs | 59 +++++++-- crates/improvise-command/src/ui/project.rs | 10 +- crates/improvise-core/src/model/types.rs | 7 +- crates/improvise-core/src/view/layout.rs | 38 ++++-- crates/improvise-io/src/persistence/export.rs | 29 +++++ 9 files changed, 388 insertions(+), 58 deletions(-) diff --git a/context/code/core.md b/context/code/core.md index 27c6dfc3..e27a3c18 100644 --- a/context/code/core.md +++ b/context/code/core.md @@ -832,6 +832,108 @@ that item. Under route-then-aggregate that arm is reached only where a record genuinely lacks the coordinate, because the fold hands the expression a record key. Evaluated bare, outside a `SUM`, it degrades exactly as `Formula::filter` does — which is why a fitted tree is always emitted wrapped. +## 14. A collapsed group is one row, and its cell folds the group (improvise-6qet) + +**The ruling.** "When collapsing a group, I'd generally expect the cursor to move +to whichever place the group collapsed into" (user, 2026-09-23). The shape chosen +from a presented fork: **a collapsed group contributes one data row, whose cell is +an aggregate over the group.** Before this, `expand_category` emitted the group's +`GroupHeader` and then `continue`d past every item, so a collapsed group had no +data row at all. `row_group_for` is indexed by data row, so nothing on the grid +could name the group once it was collapsed: `z` pressed twice collapsed the +*neighbour*, and a collapsed group could be reopened only by `:toggle-group`. + +The fix is to emit **one** `DataItem` where the `continue` was, and every +cursor-addressed question then answers itself by arithmetic. With +`Q1={Jan,Feb}, Q2={Apr,May}` and the cursor on `Apr` (row 2), collapsing Q2 gives +rows `[Jan, Feb, Q2]`, so row 2 *is* Q2 and a second `z` names Q2 again. `z z` is +an involution without anything being taught about involution, and neither +`ViewCursor` nor `row_group_for` changed. + +### The row + +- **Its tuple is the group name** (`Month=Q1`). `AxisEntry::DataItem` became a + struct variant `{ tuple, collapsed }` rather than growing a second variant, + because a new variant would be silently missed by `matches!(e, DataItem(_))` + sites; changing the shape made the compiler list all six external matches. +- **`collapsed` cannot be derived from the tuple**: a group and an item may + legally share a name. A parallel `Vec` was rejected for the reason gotcha + 1 gives `prune_axis_items` a predicate — pruning shifts indices. +- **The flag is inherited** down `cross_product`, so a category *nested under* a + collapsed group (`Q1/North`) is also a fold: no record carries `Month=Q1` + whichever inner coordinate sits beside it. +- **A group's items need not be contiguous** (`A[G1], B[G2], C[G1]` is legal), so + `expand_category` keeps a `folded` list: a collapsed non-contiguous group folds + **once**, and its header is not re-emitted on the repeat — that header would + open a run with no row in it. + +### 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: + +- **Membership is read off each item's own `group` field, never off + `Category::groups`.** `add_item_in_group` registers no `Group`; that list is + filled only by `.improv` `>` lines. Asking it answers "not a group" for every + document built in code, and the symptom is the fold's row pruned as empty. +- **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. + +### Settability and drill + +The row is not a record, so `project.rs::cell_id` answers +`CellId::Aggregate` when `GridLayout::folds_a_group(row, col)` — the one home of +that question — whatever is on `Axis::None`. Without it, a document with nothing +regular on None projected the fold as `CellId::Record`, offered it for editing, +and a `SetCell` would have written the coordinate `Month=Q1`, which is not an +item. `render.rs::editable` is untouched (ADR 002: the variant decides). +`EditOrDrill` asks the same predicate and declines out loud: drilling would filter +on `Month=Q1`, which no record carries, and show an empty list. + +### The cursor, and why the order is the fix + +`ToggleGroup` is `Model(LayoutGeometry)`, and `apply_effects` resets the cursor +when it observes the frame move *inside* the loop (gotcha 24). So +`ToggleGroupAtCursor` emits `ToggleGroup` **first** and the cursor placement +**after**; the swap is the rival, and it lands every toggle at `(0, 0)`. The +placement also restores the scroll offsets the reset zeroed. + +Where the cursor lands is asked of the layout the toggle **will** produce +(`GridLayout::row_run_start(cat, group, prefix)` on a view with the toggle +applied), not of the current one. The same answer serves collapse and expand — +the group's first row — and it matters when the group is at an inner level: +collapsing `Month` under `Region` shrinks every earlier `Region`'s run too, so +`South/Apr` at row 6 lands on `South/Q2` at row 5, not row 6. That is the only +test the "ask the current layout" rival fails. + +### What this does not do + +- **Part 2 of the ruling** (a toggle carrying the group it acted on, so a later + command in the same batch can be invalidated) is not added: nothing binds `z` + as a `Binding::Sequence`, and if something did, both toggles would resolve + against one pre-batch context and name the same group, so the pair is a no-op — + which is what `z z` means. A mechanism with no reachable case is not built. +- **CSV export writes the fold as a row**, because an export writes what the grid + draws (improvise-iot6); a collapsed group is its own total row there too. +- Under `prune_empty` an **empty** collapsed group now disappears header and all, + where improvise-lhi4 kept its header. It is a narrowing, and deliberate: the + header was kept because it was the only way back to the rows, and the fold row + is now that way back — an empty fold has nothing to show. +- The browser still cannot press `z` (`toggle-group-under-cursor` resolves + `Upstream`), so the cursor half takes effect in the TUI only. + ## `PageAxis` — what constrains, and what the user may turn (improvise-p1ym) `GridLayout::page_coords` answers two questions that gotcha 27 says must not be diff --git a/context/test-evidence/command.md b/context/test-evidence/command.md index efcda9b0..4c7f0346 100644 --- a/context/test-evidence/command.md +++ b/context/test-evidence/command.md @@ -1257,9 +1257,11 @@ substring. ### a_window_reaching_the_last_row_carries_the_trailing_group_header -A collapsed group at the bottom of the grid has no items to hang off, and is still -the only row that says those rows exist. A window that reaches the last row -therefore carries it. +A collapsed group at the bottom of the grid folds into one row (improvise-6qet, +`context/code/core.md` §14), and the header comes above it. A window that reaches +the last row therefore carries both: the frame is `[North, South, [Late], Late]`. +Before 6qet a collapsed group had no row, and this test asserted the header was the +*last* entry — the premise 6qet changed, so the length moved from 2 to 3. It must be *seen*; it is not yet actionable in a browser. `web/app.js` registers no listener on a group ``, and `toggle-group-under-cursor` is still in @@ -1289,23 +1291,30 @@ say which group a column or row belongs to — while `body_from` declines to pai header that sits above the window. **And only the nearest one comes.** The fixture puts **two** headers above the -window (`Hidden` is collapsed, so it emits a header and no items, and `Late`'s -follows it immediately) because that is the only way to tell "the nearest header -comes" from "every header above comes". With one group above, deleting the +window (`Hidden`, whose collapsed group is now one row, and `Late`) because that is +the only way to tell "the nearest header comes" from "every header above comes". +Since improvise-6qet the fold is itself a row, so the window opens at row **2** +(`East`), past both `Hidden`'s header and its folded row; at row 1 the window would +start on the fold and bring `Hidden`'s header with it. With one group above, deleting the compaction leaves this green — it is a *cost* property, O(the groups a deep scroll crossed), and correctness alone cannot see it: `group_of` still finds the nearest and `body_from` still starts at the first item either way. -#### Helper — `app_with_consecutive_headers` +#### Helper — `app_with_a_collapsed_group_between_rows` -Not a test — the fixture for that test and for `consecutive_headers_between_two_rows_all_come`. +Not a test — the fixture for that test and for `every_header_between_two_in_window_rows_comes`. -Two consecutive group headers between two rows: `Hidden` is collapsed, so it emits -a header and no items, and `Late`'s header follows it immediately. +Two group headers between `North` and `East`: `Hidden`, collapsed, and `Late`, open. +It was `app_with_consecutive_headers` until improvise-6qet: a collapsed group now +contributes one row, so a collapse no longer produces two *adjacent* headers and +the old name described a shape the layout cannot emit. -### consecutive_headers_between_two_rows_all_come +### every_header_between_two_in_window_rows_comes -Headers between two in-window rows all come, not just the nearest. +Headers between two in-window rows all come, not just the nearest — the frame is +`[North, [Hidden], Hidden, [Late], East]`, each header above the row it opens. +Renamed from `consecutive_headers_between_two_rows_all_come` for the reason the +fixture was. Only the nearest wins `group_of`, so a compaction that kept one would still answer every question about *membership* correctly — and would silently lose the `Hidden` @@ -4778,40 +4787,80 @@ cursor at data row 0: 3. The row axis is now empty, so `row_group_for(0)` resolves nothing and `z` returns no effects at all. -So the cursor-addressed toggle is not an involution, and a collapsed group cannot -be reopened from the grid — only by `:toggle-group`, which names the group. That -behaviour is unchanged by this work and is filed separately as improvise-6qet; the -three tests below pin what the command *does*, which is what the old test claimed -to and could not. +So the cursor-addressed toggle was not an involution, and a collapsed group could +not be reopened from the grid. **improvise-6qet changed that**, and the tests below +are its guards; the two that pinned the old behaviour +(`a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one`, +`a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address`) were replaced +rather than kept, since the behaviour they pinned was the bug. The old law stays +deleted; a real determinism law over two independently built contexts is +improvise-hzjo. + +improvise-6eqi found the survivors of ub4o all seated the cursor at data row 0, so a +toggle that ignored the cursor (`row_group_for(0)`) passed the whole workspace. Every +toggle test below starts off row 0; that rival was injected and fails five of them. ### toggling_at_the_cursor_names_the_group_the_cursor_sits_in_and_collapses_it -Asserts the effect names `category: "Month", group: "Q1"` — a category and a group -by name, which no assertion in the tile/grid suite had ever done — and that the -view's collapsed set actually moved afterwards. The fixture is asserted expanded -first, so the post-condition is not supplied by the default. +Asserts the effect names `category: "Month", group: "Q2"` — a category and a group +by name — and that the view's collapsed set actually moved, and that the neighbour +`Q1` did not. The cursor sits on `Apr` (row 2): at row 0 the neighbour and the +subject are the same answer. Fixture asserted expanded first, so the post-condition +is not supplied by the default. -Fault injected: `execute` returning `vec![]` after resolving the lookup. The old -law passed (both sides empty, as they already were); this test fails on the -missing effect, and the two below fail with it. +Faults injected: `execute` returning `vec![]` after resolving the lookup, and +`row_group_for(0)` in place of the cursor row (improvise-6eqi). Both fail it. -### a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one +### toggling_a_group_collapses_that_group_and_the_cursor_follows_it -The test the old one could not be: it requires the second `execute` to see a -layout the first one changed. +improvise-6qet, driven through the real path (`App::cmd_context` → `execute` → +`apply_effects`). Cursor at row 2 (`Apr`); `z` gives rows `[Jan, Feb, Q2]` with the +cursor on `Q2` at row 2, and `z` again gives `[Jan, Feb, Apr, May]` with the cursor +back on row 2 — the involution the old law claimed. -Fault injected: a stateless `execute` that always returns -`ToggleGroup { category: "Month", group: "Q1" }` — i.e. one that ignores the -applied state, which is exactly what "does not apply effects between calls" hides. -Run against it, the old law **passes** (`first == second` by construction) and this -test fails, as does the one below. That rival is one-directional: it can only ever -name `Q1`, so a test that merely asserts "some group is named" is satisfied by it. +Faults injected: **the swap** (cursor placement emitted before `ToggleGroup`) fails +it `left: (0, 0) right: (2, 0)` — the toggle's observed reframe resets the cursor +after the placement. `row_group_for(0)` fails it too. -### a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address +### a_collapse_from_the_tail_of_a_run_lands_on_the_runs_first_row + +Cursor on `May` (row 3). Collapsing Q2 lands on row 2, the fold. The rival "leave +the cursor where it was" is right for a cursor already on the run's first row and +wrong here (row 3 is past the end of `[Jan, Feb, Q2]`); injected, it fails this and +the next test only. + +### collapsing_an_inner_group_lands_where_every_earlier_run_shrank + +`Region × Month` on Row, `Month` the inner level. Cursor on `South/Apr` (row 6). +Collapsing Q2 shrinks `North`'s run as well, so `South/Q2` is at row **5**. The only +test that separates "ask the layout the toggle will produce" from "ask the current +layout": that rival returns 6 (`left: (6, 0) right: (5, 0)`). + +### a_collapsed_group_can_be_re_expanded_from_the_grid + +Both groups collapsed (`[Q1, Q2]`), cursor on the `Q2` row: `z` names Q2 and +reopens it, landing on `Apr` at row 1. Before 6qet a collapsed group had no row and +this was impossible from the grid. The swap fails it at `(0, 0)`. + +### edit_or_drill_declines_a_collapsed_groups_row + +A fold is neither editable nor drillable: no record carries `Month=Q2`. The same +context at `Feb` (row 1) still opens the editor, so the decline is about the row, +not the fixture. Rival: the `folds_a_group` guard removed — the command opens an +editor on the fold, and the test fails. + +## `ui/project.rs` — a collapsed group's row is an aggregate (improvise-6qet) + +### ui::project::cell_identity::a_collapsed_groups_row_is_projected_as_an_aggregate + +Nothing regular is on `Axis::None`, so the none-axis arm of `cell_id` cannot supply +`Aggregate` — and `North` in the same grid is asserted `Record` to prove it. The +`Coastal` fold must be `Aggregate`, and its text `7.00` (East 3 + West 4). Rival: +`folds_a_group` dropped from `cell_id` — the fold is projected `Record`, which +`render.rs::editable` would offer for a write to the non-item `Region=Coastal`. It +also fails when group membership is read off `Category::groups` instead of each +item's `group`, because a document built with `add_item_in_group` registers none. -Pins step 3 — `row_count() == 0` is asserted before the call, so the empty effect -vector is the command declining to name a group rather than the fixture never -having had one. Both rivals above fail it. ## `ui/project.rs` — a formula lost with its target ITEM (improvise-en4s) `Dirty::of(ModelChange::CellData)` reports `formulas: true`. That was written as diff --git a/context/test-evidence/core.md b/context/test-evidence/core.md index 299a152c..d27feb93 100644 --- a/context/test-evidence/core.md +++ b/context/test-evidence/core.md @@ -1293,3 +1293,71 @@ at different assertions — the first at the East count, the second at the total The fixture is deliberately asymmetric: one matching cell under the scoped key and three under the unscoped one, so no single constant can satisfy both. + +## `view/layout.rs`, `model/types.rs` — a collapsed group is one aggregate row (improvise-6qet) + +Reasoning in `context/code/core.md` §14. The bug is one statement: `expand_category` +`continue`d past every item of a collapsed group, so the group had no data row and +the cursor-addressed `z` could never name it. Restoring that `continue` against the +fixed tree fails fifteen tests across `improvise-core`, `improvise-io` and +`improvise-command` — every test below that touches the layout. + +### a_collapsed_group_contributes_one_aggregate_row + +The inversion of `collapsed_group_has_header_but_no_data_items`, which asserted the +premise 6qet changed and was rewritten rather than deleted. **Two** groups, one +collapsed and one not, so `folds_a_group` is asserted `true` for the fold and +`false` for `Apr` under the open group — a predicate that answered `true` for +everything under a header would pass a one-group fixture. Also asserts the fold +sits under its own header (`row_group_for(0) == Q1`), which is what lets `z` name +it. + +### a_non_contiguous_collapsed_group_folds_once + +`A[G1], B[G2], C[G1]`: a group's items need not be contiguous. Rival: dedup only +against the *previous* group (right for the contiguous case, which every other +fixture is) — `G1` then folds twice, under two headers, and only this test fails. + +### a_category_nested_under_a_collapsed_group_is_folded_too + +`Month × Region` on Row, `Q1` collapsed: `Q1/North` is a fold too, because no +record carries `Month=Q1` whatever sits beside it. Rival: the `collapsed` flag not +inherited down `cross_product` — only this test fails, and without it the TUI would +offer `Q1/North` for editing. + +### model::types::model_tests::a_collapsed_groups_cell_aggregates_its_items + +`Jan=3, Feb=4` under `Q1`, `Apr=100` under `Q2`: the fold is `7`. That value +separates the real fold from absent (`None`), from pass-through of the first member +(`3`), from a count (`2`) and from a fold that leaks another group (`107`). + +Also the witness for the trap the first implementation of this ticket fell into: +reading membership off `Category::groups` rather than each item's `group` field. +`add_item_in_group` registers no `Group`, so that rival answers "not a group" for +this fixture and the value is absent — this test, the next, and +`a_collapsed_groups_header_survives_pruning` fail (the fold's row prunes as empty). + +### model::types::model_tests::a_groups_stored_values_fold_once_not_per_member + +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 +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 + +An item literally named `Q1` beside the group `Q1`: the item wins and reads `50`, so +an existing document with such an item is unchanged. Rival: drop the +`items.contains_key` short-circuit — the key expands to `Jan + Feb = 7` and this +test fails. + +#### `a_collapsed_groups_header_survives_pruning`, re-aimed + +Re-aimed by improvise-6qet (the entry above, under improvise-lhi4, still holds for +why the fixture carries an unfunded `Mar`). The expected sequence now includes the +fold, `DataItem { ["Q1"], collapsed: true }`, and `row_group_for(0)` is `Q1` — the +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. diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index 91930f47..daf7a527 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -1424,3 +1424,13 @@ This is the second time in this ticket that the interesting question was *which rows the count is over*. The first was one warning per derived column rather than one overall; this is one warning per **landed** row rather than per attempted one. + +### export_writes_a_collapsed_group_as_its_own_total_row + +improvise-6qet (`context/code/core.md` §14). An export writes what the grid draws +(improvise-iot6), and the grid now draws a collapsed group as one row whose cell +folds the group, so the CSV body is exactly `["Q1,7.00"]` (Jan 3 + Feb 4). Decided +rather than defaulted: the alternative — expanding the group in the export — would +reintroduce the iot6 divergence between what the user sees and what they export. +On the pre-6qet tree the body is empty (the group had no row, and pruning left +nothing), so the test fails there. diff --git a/crates/improvise-command/src/command/cmd/grid.rs b/crates/improvise-command/src/command/cmd/grid.rs index 0d02e7cd..b06b8570 100644 --- a/crates/improvise-command/src/command/cmd/grid.rs +++ b/crates/improvise-command/src/command/cmd/grid.rs @@ -81,7 +81,10 @@ mod tests { view.is_group_collapsed("Month", "Q2"), "the effect was returned but the view did not move" ); - assert!(!view.is_group_collapsed("Month", "Q1"), "the neighbour was left alone"); + assert!( + !view.is_group_collapsed("Month", "Q1"), + "the neighbour was left alone" + ); } /// Evidence: `context/test-evidence/command.md`. @@ -92,7 +95,11 @@ mod tests { let mut app = App::new(two_groups_on_the_row_axis(), None); app.rebuild_layout(); app.cursor_mut().selected = (2, 0); - assert_eq!(app.layout.row_label(2), "Apr", "the cursor starts inside Q2"); + assert_eq!( + app.layout.row_label(2), + "Apr", + "the cursor starts inside Q2" + ); let dbg = toggle_group_at_cursor(&mut app); @@ -102,8 +109,15 @@ mod tests { let dbg = toggle_group_at_cursor(&mut app); - assert!(dbg.contains(r#"group: "Q2""#), "z again names Q2, got: {dbg}"); - assert_eq!(row_labels(&app), ["Jan", "Feb", "Apr", "May"], "z z is a no-op"); + assert!( + dbg.contains(r#"group: "Q2""#), + "z again names Q2, got: {dbg}" + ); + assert_eq!( + row_labels(&app), + ["Jan", "Feb", "Apr", "May"], + "z z is a no-op" + ); assert_eq!(app.cursor().selected, (2, 0), "back on Q2's first item"); } @@ -144,17 +158,35 @@ mod tests { wb.active_view_mut().set_axis("Type", Axis::Column); let mut app = App::new(wb, None); app.rebuild_layout(); - assert_eq!(app.layout.row_cats, ["Region", "Month"], "Month is the INNER level"); + assert_eq!( + app.layout.row_cats, + ["Region", "Month"], + "Month is the INNER level" + ); app.cursor_mut().selected = (6, 0); - assert_eq!(app.layout.row_tuple(6), Some(&["South".into(), "Apr".into()][..])); + assert_eq!( + app.layout.row_tuple(6), + Some(&["South".into(), "Apr".into()][..]) + ); toggle_group_at_cursor(&mut app); assert_eq!( row_labels(&app), - ["North/Jan", "North/Feb", "North/Q2", "South/Jan", "South/Feb", "South/Q2"] + [ + "North/Jan", + "North/Feb", + "North/Q2", + "South/Jan", + "South/Feb", + "South/Q2" + ] + ); + assert_eq!( + app.cursor().selected, + (5, 0), + "South's Q2, not the old index 6" ); - assert_eq!(app.cursor().selected, (5, 0), "South's Q2, not the old index 6"); } /// Evidence: `context/test-evidence/command.md`. @@ -173,7 +205,10 @@ mod tests { let dbg = toggle_group_at_cursor(&mut app); - assert!(dbg.contains(r#"group: "Q2""#), "the Q2 row names Q2, got: {dbg}"); + assert!( + dbg.contains(r#"group: "Q2""#), + "the Q2 row names Q2, got: {dbg}" + ); assert_eq!(row_labels(&app), ["Q1", "Apr", "May"]); assert_eq!(app.cursor().selected, (1, 0), "on Q2's first item"); } @@ -409,7 +444,11 @@ impl Cmd for ToggleGroupAtCursor { category: cat, group, })]; - effects.extend(landing.map(|at| self.place_cursor(ctx, at)).unwrap_or_default()); + effects.extend( + landing + .map(|at| self.place_cursor(ctx, at)) + .unwrap_or_default(), + ); effects } } diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index 40792ba2..7481fd7b 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -1440,7 +1440,10 @@ mod tests { vec!["North", "[Hidden]", "Hidden", "[Late]", "East"], "both headers are drawn, the collapsed one above its folded row" ); - let hidden = grid.row_frame.group_of(1).expect("the fold is under Hidden"); + let hidden = grid + .row_frame + .group_of(1) + .expect("the fold is under Hidden"); assert_eq!(hidden.name, "Hidden"); assert!(hidden.collapsed, "and it arrives collapsed"); let late = grid.row_frame.group_of(2).expect("East is under Late"); @@ -4328,7 +4331,10 @@ mod cell_identity { "a fold has no record under it and must not be offered as settable" ); let grid = grid_of(&app, WHOLE); - assert_eq!(grid.cell(1, 0).expect("the fold's cell").text.trim(), "7.00"); + assert_eq!( + grid.cell(1, 0).expect("the fold's cell").text.trim(), + "7.00" + ); } fn record_of_id(id: CellId) -> RecordCell { diff --git a/crates/improvise-core/src/model/types.rs b/crates/improvise-core/src/model/types.rs index 173e2a38..ea394dbe 100644 --- a/crates/improvise-core/src/model/types.rs +++ b/crates/improvise-core/src/model/types.rs @@ -566,7 +566,12 @@ impl Model { /// A group's cell: stored values fold ONCE over every record the group covers, /// a derived one folds its members' results — why the two differ: core.md §14. - fn evaluate_group(&self, key: &CellKey, members: &[CellKey], agg: Aggregation) -> Option { + fn evaluate_group( + &self, + key: &CellKey, + members: &[CellKey], + agg: Aggregation, + ) -> Option { let values: Vec = match self.formula_for(key) { Some(_) => members .iter() diff --git a/crates/improvise-core/src/view/layout.rs b/crates/improvise-core/src/view/layout.rs index c93e4d08..495c3967 100644 --- a/crates/improvise-core/src/view/layout.rs +++ b/crates/improvise-core/src/view/layout.rs @@ -1365,10 +1365,19 @@ mod tests { wb.add_item("Type", "Food").unwrap(); wb.active_view_mut().toggle_group_collapse("Month", "Q1"); let layout = GridLayout::new(&wb.model, wb.active_view()); - let labels: Vec = (0..layout.row_count()).map(|r| layout.row_label(r)).collect(); - assert_eq!(labels, ["Q1", "Apr", "May"], "Q1 folds into one row; Q2 stays open"); + let labels: Vec = (0..layout.row_count()) + .map(|r| layout.row_label(r)) + .collect(); + assert_eq!( + labels, + ["Q1", "Apr", "May"], + "Q1 folds into one row; Q2 stays open" + ); assert!(layout.folds_a_group(0, 0), "the Q1 row is a fold"); - assert!(!layout.folds_a_group(1, 0), "Apr, under the OPEN group, is not"); + assert!( + !layout.folds_a_group(1, 0), + "Apr, under the OPEN group, is not" + ); assert_eq!( layout.row_group_for(0), Some(("Month".into(), "Q1".into())), @@ -1388,14 +1397,21 @@ mod tests { wb.add_item("Type", "Food").unwrap(); wb.active_view_mut().toggle_group_collapse("Month", "G1"); let layout = GridLayout::new(&wb.model, wb.active_view()); - let labels: Vec = (0..layout.row_count()).map(|r| layout.row_label(r)).collect(); + let labels: Vec = (0..layout.row_count()) + .map(|r| layout.row_label(r)) + .collect(); assert_eq!(labels, ["G1", "B"], "one G1 row, not one per run"); let g1_headers = layout .row_items .iter() - .filter(|e| matches!(e, AxisEntry::GroupHeader { group_name, .. } if group_name == "G1")) + .filter( + |e| matches!(e, AxisEntry::GroupHeader { group_name, .. } if group_name == "G1"), + ) .count(); - assert_eq!(g1_headers, 1, "a second G1 header would open a run with no row"); + assert_eq!( + g1_headers, 1, + "a second G1 header would open a run with no row" + ); } /// Evidence: context/test-evidence/core.md. @@ -1412,8 +1428,14 @@ mod tests { wb.active_view_mut().set_axis("Region", Axis::Row); wb.active_view_mut().toggle_group_collapse("Month", "Q1"); let layout = GridLayout::new(&wb.model, wb.active_view()); - assert_eq!(layout.row_tuple(0), Some(&["Q1".to_string(), "North".to_string()][..])); - assert!(layout.folds_a_group(0, 0), "Q1/North has no record under it either"); + assert_eq!( + layout.row_tuple(0), + Some(&["Q1".to_string(), "North".to_string()][..]) + ); + assert!( + layout.folds_a_group(0, 0), + "Q1/North has no record under it either" + ); assert!(!layout.folds_a_group(1, 0), "Apr/North does"); } diff --git a/crates/improvise-io/src/persistence/export.rs b/crates/improvise-io/src/persistence/export.rs index 993de670..30a50c13 100644 --- a/crates/improvise-io/src/persistence/export.rs +++ b/crates/improvise-io/src/persistence/export.rs @@ -204,6 +204,35 @@ mod tests { assert_eq!(header, ["Jan"], "CSV wrote a column the grid does not draw"); } + /// An export writes what the grid draws (improvise-iot6), so a collapsed group + /// is one total row. Evidence: context/test-evidence/formula-and-io.md. + #[test] + fn export_writes_a_collapsed_group_as_its_own_total_row() { + use crate::view::Axis; + let mut wb = Workbook::new("T"); + wb.add_category("Month").unwrap(); + wb.add_category("Type").unwrap(); + wb.add_item_in_group("Month", "Jan", "Q1").unwrap(); + wb.add_item_in_group("Month", "Feb", "Q1").unwrap(); + wb.add_item("Type", "Food").unwrap(); + for (month, value) in [("Jan", 3.0), ("Feb", 4.0)] { + wb.model.set_cell( + coord(&[("Month", month), ("Type", "Food")]), + CellValue::Number(value), + ); + } + { + let v = wb.active_view_mut(); + v.set_axis("Month", Axis::Row); + v.set_axis("Type", Axis::Column); + v.prune_empty = true; + v.toggle_group_collapse("Month", "Q1"); + } + let csv = String::from_utf8(super::render_csv(&wb, "Default").unwrap()).unwrap(); + let rows: Vec<&str> = csv.lines().skip(1).collect(); + assert_eq!(rows, ["Q1,7.00"], "one folded row, summing Jan and Feb"); + } + #[test] fn export_csv_unknown_view_returns_error() { let m = two_cat_model(); From 216b14860d9b8f2de896c981033b9e12afe6ba4c Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Tue, 22 Sep 2026 21:31:06 -0700 Subject: [PATCH 4/4] improvise-6qet: the grouped harness spec expects the collapsed group's row grouped.spec.js asserted a collapsed group contributes no rows (8 data rows). It now contributes one per Store (10), labelled Upland, and its cells are not editable. The editable assertion was checked against cell_id without folds_a_group: it fails with 6 editable cells where 0 are expected. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- harness/tests/grouped.spec.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/harness/tests/grouped.spec.js b/harness/tests/grouped.spec.js index a5fbd8df..55295ca5 100644 --- a/harness/tests/grouped.spec.js +++ b/harness/tests/grouped.spec.js @@ -146,18 +146,21 @@ test.describe("a grouped, multi-level document", () => { expect(await groupRows.locator("td").count()).toBe(0); await expect(groupRows.first().locator("th")).toHaveCount(1); - // The collapse is the document's, and it must have REMOVED rows: Upland's - // two regions are absent while Inland's two are here. Without this, a - // page that drew the indicator and ignored the frame would pass. + // The collapse is the document's: Upland's two regions are gone and the + // group is ONE row in their place (improvise-6qet, core.md §14). const labels = await page.locator("#grid tbody tr th[scope=row]").allTextContents(); expect(labels).toContain("East"); expect(labels).toContain("West"); + expect(labels).toContain("Upland"); expect(labels).not.toContain("Ridge"); expect(labels).not.toContain("Valley"); - // 4 regions visible x 2 stores. Stated as a count as well, because a row - // drawn with empty labels would satisfy the four assertions above. - await expect(page.locator("#grid tbody tr").filter({ hasNot: page.locator("th.group") })).toHaveCount( - 8, - ); + // (4 regions + the Upland fold) x 2 stores. A count too, because a row + // drawn with empty labels would satisfy the assertions above. + const dataRows = page.locator("#grid tbody tr").filter({ hasNot: page.locator("th.group") }); + await expect(dataRows).toHaveCount(10); + // The fold is an aggregate: no record carries Region=Upland, so nothing + // in its rows may open an editor, where a stored row beside it does. + await expect(dataRows.first().locator("td.editable")).not.toHaveCount(0); + await expect(dataRows.last().locator("td.editable")).toHaveCount(0); }); });