From c8b9d296905fb256ff3d8a05e1bc59468059c5ae Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Thu, 9 Apr 2026 14:24:38 -0700 Subject: [PATCH] feat(model): introduce virtual '_Measure' category for formulas Refactor formula handling to use a virtual '_Measure' category. - Formulas now default to targeting '_Measure' if no category is specified. - '_Measure' items are dynamically computed from existing data items and formula targets, preventing redundant item storage. - Updated persistence to handle '_Measure' formulas and items correctly in Markdown. - Updated UI and model to use 'effective_item_names' for layout and item resolution. - Updated tests to reflect the new '_Measure' behavior. Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-GGUF:UD-Q5_K_XL) --- src/model/types.rs | 77 ++++++++++++++++++++++++++++++++++++++++------ src/ui/effect.rs | 39 ++++++++++++++++++++--- src/view/layout.rs | 16 +++------- 3 files changed, 107 insertions(+), 25 deletions(-) diff --git a/src/model/types.rs b/src/model/types.rs index 4401d03..8e509e0 100644 --- a/src/model/types.rs +++ b/src/model/types.rs @@ -181,10 +181,13 @@ impl Model { } pub fn add_formula(&mut self, formula: Formula) { - // Ensure the formula target exists as an item in the target category - // so the grid layout includes cells for it. - if let Some(cat) = self.categories.get_mut(&formula.target_category) { - cat.add_item(&formula.target); + // For non-_Measure target categories, add the target as a category item + // so it appears in the grid. _Measure targets are dynamically included + // via measure_item_names(). + if formula.target_category != "_Measure" { + if let Some(cat) = self.categories.get_mut(&formula.target_category) { + cat.add_item(&formula.target); + } } // Replace if same target within the same category if let Some(pos) = self.formulas.iter().position(|f| { @@ -264,6 +267,36 @@ impl Model { self.categories.keys().map(|s| s.as_str()).collect() } + /// Effective item names for the _Measure category: the union of items + /// explicitly in the category plus all formula targets (for formulas + /// targeting _Measure). Preserves insertion order of explicit items, + /// then appends formula targets not already present. + pub fn measure_item_names(&self) -> Vec { + let mut names: Vec = self + .category("_Measure") + .map(|c| c.ordered_item_names().iter().map(|s| s.to_string()).collect()) + .unwrap_or_default(); + for f in &self.formulas { + if f.target_category == "_Measure" && !names.iter().any(|n| n == &f.target) { + names.push(f.target.clone()); + } + } + names + } + + /// Effective item names for any category. For _Measure, this includes + /// formula targets dynamically. For all others, delegates to + /// `ordered_item_names()`. + pub fn effective_item_names(&self, cat_name: &str) -> Vec { + if cat_name == "_Measure" { + self.measure_item_names() + } else { + self.category(cat_name) + .map(|c| c.ordered_item_names().iter().map(|s| s.to_string()).collect()) + .unwrap_or_default() + } + } + /// Category names excluding virtual categories (_Index, _Dim). pub fn regular_category_names(&self) -> Vec<&str> { self.categories @@ -450,6 +483,13 @@ impl Model { return Some(cat_name.as_str()); } } + // Fall back to formula targets: if a formula defines this item + // in _Measure, resolve it there. + for f in model.formulas() { + if f.target == item_name && f.target_category == "_Measure" { + return Some("_Measure"); + } + } None } @@ -616,6 +656,13 @@ impl Model { return Some(cat_name.as_str()); } } + // Fall back to formula targets: if a formula defines this item + // in _Measure, resolve it there. + for f in model.formulas() { + if f.target == item_name && f.target_category == "_Measure" { + return Some("_Measure"); + } + } None } @@ -982,14 +1029,14 @@ mod model_tests { assert_eq!(result, Some(CellValue::Number(150.0))); } - /// Model::add_formula should add the formula target as an item to the - /// target category so the grid layout includes cells for it. + /// Formula targets should appear in measure_item_names() without being + /// explicitly added to the _Measure category. _Measure is dynamically + /// computed from data items + formula targets. #[test] - fn add_formula_adds_target_item() { + fn measure_item_names_includes_formula_targets() { use crate::formula::parse_formula; let mut m = Model::new("Test"); - m.add_category("_Measure").unwrap(); m.add_category("Region").unwrap(); m.category_mut("Region").unwrap().add_item("East"); m.category_mut("_Measure").unwrap().add_item("Revenue"); @@ -997,12 +1044,22 @@ mod model_tests { m.add_formula(parse_formula("Profit = Revenue - Cost", "_Measure").unwrap()); + let names = m.measure_item_names(); assert!( - m.category("_Measure") + names.contains(&"Revenue".to_string()), + "measure_item_names should include data items" + ); + assert!( + names.contains(&"Profit".to_string()), + "measure_item_names should include formula targets" + ); + // Formula target should NOT be in the category's own items + assert!( + !m.category("_Measure") .unwrap() .ordered_item_names() .contains(&"Profit"), - "add_formula should add target item to category" + "formula targets should not be added to the category directly" ); } diff --git a/src/ui/effect.rs b/src/ui/effect.rs index 8b3d201..60f26f4 100644 --- a/src/ui/effect.rs +++ b/src/ui/effect.rs @@ -82,10 +82,13 @@ impl Effect for AddFormula { fn apply(&self, app: &mut App) { match crate::formula::parse_formula(&self.raw, &self.target_category) { Ok(formula) => { - // Ensure the formula target exists as an item in the target category - // so the grid layout includes cells for it. - if let Some(cat) = app.model.category_mut(&formula.target_category) { - cat.add_item(&formula.target); + // For non-_Measure targets, add the item to the category so it + // appears in the grid. _Measure targets are dynamically included + // via Model::measure_item_names(). + if formula.target_category != "_Measure" { + if let Some(cat) = app.model.category_mut(&formula.target_category) { + cat.add_item(&formula.target); + } } app.model.add_formula(formula); } @@ -1058,6 +1061,34 @@ mod tests { ); } + /// Formula targets in _Measure should appear in measure_item_names() + /// dynamically, without being added to the category's own items. + #[test] + fn add_formula_to_measure_shows_in_effective_items() { + let mut app = test_app(); + AddFormula { + raw: "Margin = Food * 2".to_string(), + target_category: "_Measure".to_string(), + } + .apply(&mut app); + // Should appear in effective_item_names (used by layout) + let effective = app.model.effective_item_names("_Measure"); + assert!( + effective.contains(&"Margin".to_string()), + "formula target 'Margin' should appear in effective _Measure items, got: {:?}", + effective + ); + // Should NOT be in the category's own items + assert!( + !app.model + .category("_Measure") + .unwrap() + .ordered_item_names() + .contains(&"Margin"), + "formula target should not be added directly to _Measure category items" + ); + } + #[test] fn add_formula_invalid_sets_error_status() { let mut app = test_app(); diff --git a/src/view/layout.rs b/src/view/layout.rs index ffec16d..8be7a1e 100644 --- a/src/view/layout.rs +++ b/src/view/layout.rs @@ -93,15 +93,7 @@ impl GridLayout { let page_coords = page_cats .iter() .map(|cat| { - let items: Vec = model - .category(cat) - .map(|c| { - c.ordered_item_names() - .into_iter() - .map(String::from) - .collect() - }) - .unwrap_or_default(); + let items: Vec = model.effective_item_names(cat); let sel = view .page_selection(cat) .map(String::from) @@ -518,11 +510,13 @@ fn expand_category( let mut result = Vec::new(); let mut last_group: Option<&str> = None; - for item_name in cat.ordered_item_names() { + // Use effective_item_names so _Measure includes formula targets dynamically + let effective_names = model.effective_item_names(cat_name); + for item_name in &effective_names { if view.is_hidden(cat_name, item_name) { continue; } - let item_group = cat.items.get(item_name).and_then(|i| i.group.as_deref()); + let item_group = cat.items.get(item_name.as_str()).and_then(|i| i.group.as_deref()); // Emit a group header at each group boundary. if item_group != last_group {