diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index f6fc8f1..4a22c7a 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -9,6 +9,11 @@ use crate::formula::parse_formula; use crate::model::cell::{CellKey, CellValue}; use crate::workbook::Workbook; +/// Mirrors the model's private `MAX_CATEGORIES` limit (regular categories +/// only; virtual `_Index`/`_Dim`/`_Measure` don't count). Used to pre-check +/// proposals before the wizard's confirm step (improvise-mzv). +const MAX_CATEGORIES: usize = 12; + // ── Pipeline (no UI state) ──────────────────────────────────────────────────── /// Pure data + logic for turning a JSON value into a Model. @@ -80,6 +85,22 @@ impl ImportPipeline { } } + /// Number of regular categories `build_model` would create: accepted + /// Category/TimeCategory fields plus derived date-component categories. + /// (Measures and labels map to non-regular categories and don't count.) + pub fn proposed_category_count(&self) -> usize { + self.proposals + .iter() + .filter(|p| p.accepted) + .map(|p| match p.kind { + FieldKind::Category => 1, + FieldKind::TimeCategory if p.date_format.is_some() => 1 + p.date_components.len(), + FieldKind::TimeCategory => 1, + FieldKind::Measure | FieldKind::Label => 0, + }) + .sum() + } + /// Build a Workbook from the current proposals. Pure — no side effects. pub fn build_model(&self) -> Result { let categories: Vec<&FieldProposal> = self @@ -297,6 +318,17 @@ impl ImportWizard { // ── Step transitions ────────────────────────────────────────────────────── pub fn advance(&mut self) { + // Pre-check the category limit where the user can still fix it + // (improvise-mzv): block here instead of failing in build_model() + // after the final confirm. + if matches!( + self.step, + WizardStep::ReviewProposals | WizardStep::ConfigureDates + ) && let Some(msg) = self.category_limit_message() + { + self.message = Some(msg); + return; + } self.step = match self.step { WizardStep::Preview => { if self.pipeline.array_paths.len() > 1 && self.pipeline.needs_path_selection() { @@ -322,6 +354,18 @@ impl ImportWizard { self.message = None; } + /// User-facing message when the accepted proposals would exceed the + /// model's regular-category limit; `None` when within the limit. + fn category_limit_message(&self) -> Option { + let count = self.pipeline.proposed_category_count(); + (count > MAX_CATEGORIES).then(|| { + format!( + "Too many categories: {count} proposed, max {MAX_CATEGORIES}. \ + Mark fields as Measure (c) or toggle them off (Space)." + ) + }) + } + fn has_time_categories(&self) -> bool { self.pipeline .proposals @@ -856,6 +900,73 @@ mod tests { assert!(!w.pipeline.records.is_empty()); } + // ── Category limit pre-check ──────────────────────────────────────── + + /// Build records whose fields `c00..cNN` are all low-cardinality strings, + /// so the analyzer proposes every field as an accepted Category. + fn category_records(n_fields: usize) -> serde_json::Value { + let recs: Vec = (0..3) + .map(|r| { + let mut m = serde_json::Map::new(); + for i in 0..n_fields { + m.insert(format!("c{i:02}"), json!(format!("v{r}"))); + } + serde_json::Value::Object(m) + }) + .collect(); + serde_json::Value::Array(recs) + } + + /// BUG (improvise-mzv): with more than MAX_CATEGORIES (12) accepted + /// Category proposals, the wizard let the user advance through every + /// remaining step; build_model() only failed at the final confirm. + /// Advancing past ReviewProposals must be blocked with a message telling + /// the user to mark fields as Measure or toggle them off. + #[test] + fn wizard_blocks_advance_when_over_category_limit() { + let mut w = ImportWizard::new(category_records(13)); + assert_eq!(w.step, WizardStep::ReviewProposals); + assert_eq!( + w.pipeline + .proposals + .iter() + .filter(|p| p.accepted && p.kind == FieldKind::Category) + .count(), + 13, + "sanity: all 13 fields proposed as accepted categories" + ); + + w.advance(); + assert_eq!( + w.step, + WizardStep::ReviewProposals, + "advance must be blocked while over the category limit" + ); + let msg = w.message.as_deref().expect("a message explaining the block"); + assert!(msg.contains("Measure"), "message should tell the user how to fix it: {msg}"); + } + + #[test] + fn wizard_allows_advance_at_exactly_category_limit() { + let mut w = ImportWizard::new(category_records(12)); + assert_eq!(w.step, WizardStep::ReviewProposals); + w.advance(); + assert_ne!(w.step, WizardStep::ReviewProposals); + } + + #[test] + fn wizard_advance_unblocks_after_marking_field_as_measure() { + let mut w = ImportWizard::new(category_records(13)); + w.advance(); + assert_eq!(w.step, WizardStep::ReviewProposals); + + // Toggle one field off; the wizard should now let the user through. + w.cursor = 0; + w.toggle_proposal(); + w.advance(); + assert_ne!(w.step, WizardStep::ReviewProposals); + } + // ── Formula editing in wizard ─────────────────────────────────────── #[test] diff --git a/src/ui/import_wizard_ui.rs b/src/ui/import_wizard_ui.rs index edf7ccd..599ded3 100644 --- a/src/ui/import_wizard_ui.rs +++ b/src/ui/import_wizard_ui.rs @@ -323,16 +323,18 @@ impl<'a> Widget for ImportWizardWidget<'a> { "Enter to import, Esc to cancel", Style::default().fg(Color::DarkGray), ); - - if let Some(msg) = &self.wizard.message { - let msg_y = inner.y + inner.height - 1; - buf.set_string(x, msg_y, truncate(msg, w), Style::default().fg(Color::Red)); - } } WizardStep::Done => { buf.set_string(x, y, "Import complete!", Style::default().fg(Color::Green)); } } + + // Wizard message (errors, blocked-step explanations) takes the bottom + // line on every step, overriding the key hint while present. + if let Some(msg) = &self.wizard.message { + let msg_y = inner.y + inner.height - 1; + buf.set_string(x, msg_y, truncate(msg, w), Style::default().fg(Color::Red)); + } } }