Merge commit '4f38a35e945f54e1a0bbde3155ba518d4f298b71' into feat/felo1-max-categories-24

crates/improvise-command/src/command/cmd/wizard.rs

@@ -27,6 +27,8 @@ fn step_effects(wizard: &ImportWizard, key: KeyCode) -> Vec<Box<dyn Effect>> {
         WizardStep::SelectArrayPath => select_path_keys(key),
         WizardStep::ReviewProposals => review_proposal_keys(key),
         WizardStep::ConfigureDates => configure_date_keys(key),
+        WizardStep::ResolveDuplicates => resolve_duplicate_keys(key),
+        WizardStep::NameDuplicateCategory => name_duplicate_category_keys(key),
         WizardStep::DefineFormulas if wizard.formula_editing => formula_edit_keys(key),
         WizardStep::DefineFormulas => formula_list_keys(key),
         WizardStep::NameModel => name_model_keys(key),
@@ -68,6 +70,24 @@ fn configure_date_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
     }
 }
 
+/// Up/Down choose an option; PageUp/PageDown scroll the colliding records.
+fn resolve_duplicate_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
+    match key {
+        KeyCode::Enter => op(WizardOp::ChooseDuplicate),
+        _ => scroll_keys(key).unwrap_or_else(|| cursor_keys(key)),
+    }
+}
+
+fn name_duplicate_category_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
+    match key {
+        KeyCode::Enter => op(WizardOp::Advance),
+        KeyCode::Backspace => op(WizardOp::PopDuplicateNameChar),
+        KeyCode::Char(c) => op(WizardOp::PushDuplicateNameChar(c)),
+        KeyCode::Esc => close(),
+        _ => back_key(key),
+    }
+}
+
 fn formula_list_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
     match key {
         KeyCode::Char('n') => op(WizardOp::StartFormulaEdit),
@@ -103,7 +123,18 @@ fn done_keys(key: KeyCode) -> Vec<Box<dyn Effect>> {
     match key {
         KeyCode::Enter => vec![Box::new(effect::AcceptWizardModel)],
         KeyCode::Esc => op(WizardOp::Back),
-        _ => back_key(key),
+        KeyCode::Down | KeyCode::Char('j') => op(WizardOp::Scroll(1)),
+        KeyCode::Up | KeyCode::Char('k') => op(WizardOp::Scroll(-1)),
+        _ => scroll_keys(key).unwrap_or_else(|| back_key(key)),
+    }
+}
+
+/// PageUp/PageDown move a step's scrolling list by one item.
+fn scroll_keys(key: KeyCode) -> Option<Vec<Box<dyn Effect>>> {
+    match key {
+        KeyCode::PageDown => Some(op(WizardOp::Scroll(1))),
+        KeyCode::PageUp => Some(op(WizardOp::Scroll(-1))),
+        _ => None,
     }
 }
 
@@ -214,6 +245,8 @@ mod tests {
             WizardStep::SelectArrayPath,
             WizardStep::ReviewProposals,
             WizardStep::ConfigureDates,
+            WizardStep::ResolveDuplicates,
+            WizardStep::NameDuplicateCategory,
             WizardStep::DefineFormulas,
             WizardStep::NameModel,
             WizardStep::Done,
@@ -236,6 +269,35 @@ mod tests {
         assert_eq!(step_effects(&w, KeyCode::Char('z')).len(), 0);
     }
 
+    /// improvise-cgqt: Enter takes the option under the cursor, never a bare
+    /// advance, which would skip the question; the page keys scroll the list.
+    #[test]
+    fn the_duplicates_step_chooses_on_enter_and_scrolls_on_the_page_keys() {
+        let w = wizard_at(WizardStep::ResolveDuplicates);
+        assert_eq!(ops(&w, KeyCode::Enter), "[WizardApply(ChooseDuplicate)]");
+        assert!(ops(&w, KeyCode::Down).contains("MoveCursor(1)"));
+        assert!(ops(&w, KeyCode::PageDown).contains("Scroll(1)"));
+        assert!(ops(&w, KeyCode::PageUp).contains("Scroll(-1)"));
+        assert!(ops(&w, KeyCode::Esc).contains("CloseWizard"));
+    }
+
+    /// The name step is text entry: `j` is a letter here, not a cursor key.
+    #[test]
+    fn the_category_name_step_types_into_the_name() {
+        let w = wizard_at(WizardStep::NameDuplicateCategory);
+        assert!(ops(&w, KeyCode::Char('j')).contains("PushDuplicateNameChar('j')"));
+        assert!(ops(&w, KeyCode::Backspace).contains("PopDuplicateNameChar"));
+        assert!(ops(&w, KeyCode::Enter).contains("Advance"));
+    }
+
+    /// The confirm step's warnings scroll, so a long list is never cut off.
+    #[test]
+    fn done_scrolls_its_warnings() {
+        let w = wizard_at(WizardStep::Done);
+        assert!(ops(&w, KeyCode::Down).contains("Scroll(1)"));
+        assert!(ops(&w, KeyCode::PageUp).contains("Scroll(-1)"));
+    }
+
     #[test]
     fn unbound_keys_emit_nothing() {
         let w = wizard_at(WizardStep::ReviewProposals);

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

@@ -1055,6 +1055,10 @@ pub enum WizardOp {
     PopFormulaChar,
     PushNameChar(char),
     PopNameChar,
+    ChooseDuplicate,
+    PushDuplicateNameChar(char),
+    PopDuplicateNameChar,
+    Scroll(i32),
 }
 
 impl WizardOp {
@@ -1076,6 +1080,10 @@ impl WizardOp {
             Self::PopFormulaChar => wizard.pop_formula_char(),
             Self::PushNameChar(c) => wizard.push_name_char(*c),
             Self::PopNameChar => wizard.pop_name_char(),
+            Self::ChooseDuplicate => wizard.choose_duplicate(),
+            Self::PushDuplicateNameChar(c) => wizard.push_duplicate_name_char(*c),
+            Self::PopDuplicateNameChar => wizard.pop_duplicate_name_char(),
+            Self::Scroll(delta) => wizard.scroll(*delta),
         }
     }
 }
@@ -1232,7 +1240,7 @@ pub struct ImportJsonHeadless {
 impl Effect for ImportJsonHeadless {
     fn apply(&self, app: &mut App) {
         use crate::import::analyzer::{FieldKind, analyze_records};
-        use crate::import::wizard::ImportPipeline;
+        use crate::import::wizard::{DuplicatePolicy, ImportPipeline};
 
         let is_csv = self
             .path
@@ -1305,6 +1313,8 @@ impl Effect for ImportJsonHeadless {
                 .unwrap_or("Imported Model")
                 .to_string(),
             formulas: vec![],
+            // Headless: nobody to ask, so a collision refuses (improvise-cgqt).
+            on_duplicate: DuplicatePolicy::Refuse,
         };
 
         match pipeline.build_model_with_warnings() {
@@ -1318,7 +1328,11 @@ impl Effect for ImportJsonHeadless {
                 };
             }
             Err(e) => {
-                refuse_and_stop(app, format!("Import error: {e}"));
+                let hint = match pipeline.collisions().is_empty() {
+                    true => "",
+                    false => " (use :import to choose)",
+                };
+                refuse_and_stop(app, format!("Import error: {e}{hint}"));
             }
         }
     }
@@ -3938,6 +3952,36 @@ mod tests {
         );
     }
 
+    /// improvise-cgqt: `:import-json` cannot ask, so colliding records refuse
+    /// rather than sum, and the refusal points at the wizard that can.
+    #[test]
+    fn import_headless_refuses_records_that_collide() {
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("data.csv");
+        std::fs::write(&path, "Region,Revenue\nEast,10\nEast,20\n").unwrap();
+
+        let mut app = test_app();
+        let before = app.model_state.workbook.model.data.iter_cells().count();
+        ImportJsonHeadless {
+            path,
+            model_name: None,
+            array_path: None,
+        }
+        .apply(&mut app);
+
+        let status = &app.view_state.status_msg;
+        assert!(status.contains("shared by 2 records"), "{status}");
+        assert!(
+            status.contains(":import"),
+            "names the way to choose: {status}"
+        );
+        assert_eq!(
+            app.model_state.workbook.model.data.iter_cells().count(),
+            before,
+            "the document is untouched"
+        );
+    }
+
     #[test]
     fn import_headless_csv_full_rows_report_plain_success() {
         let dir = tempfile::tempdir().unwrap();

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

@@ -12,13 +12,14 @@ use improvise_core::view::layout::{AxisEntry, GridLayout, RecordCol};
 use improvise_core::view::{Axis, View};
 use improvise_core::workbook::Workbook;
 use improvise_io::import::analyzer::{DateComponent, FieldKind};
-use improvise_io::import::wizard::{ImportWizard, WizardStep};
+use improvise_io::import::wizard::{DuplicatePolicy, ImportWizard, WizardStep};
 use improvise_protocol::{
     AxisFrame, Basis, BasisCache, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay,
     CellId, Command, CreateKind, DocumentCache, FormulaCache, FormulaEntry, FrameEntry, FrameGroup,
     GridCache, GridExtent, GridUpdate, Projection, RecordCell, Rejection, Snapshot, TileBarCache,
-    TileEntry, ViewListCache, ViewState, ViewSummary, WidgetCache, WizardCache, WizardDateField,
-    WizardFieldKind, WizardProposal, WizardStepKind, basis, reduce_view,
+    TileEntry, ViewListCache, ViewState, ViewSummary, WidgetCache, WizardCache, WizardChoice,
+    WizardCollision, WizardDateField, WizardFieldKind, WizardProposal, WizardStepKind, basis,
+    reduce_view,
 };
 
 use crate::ui::app::App;
@@ -642,6 +643,27 @@ pub fn wizard_cache(w: &ImportWizard) -> WizardCache {
                     .collect(),
             })
             .collect(),
+        collisions: w
+            .duplicates
+            .iter()
+            .map(|c| WizardCollision {
+                coords: c.coords.clone(),
+                records: c.records.clone(),
+            })
+            .collect(),
+        duplicate_choices: w
+            .duplicate_options()
+            .into_iter()
+            .map(|o| WizardChoice {
+                label: o.label,
+                disabled: o.disabled,
+            })
+            .collect(),
+        duplicate_category: match &w.pipeline.on_duplicate {
+            DuplicatePolicy::Separate(name) => name.clone(),
+            DuplicatePolicy::Sum | DuplicatePolicy::Refuse => String::new(),
+        },
+        scroll: w.scroll,
         formulas: w.pipeline.formulas.clone(),
         sample_formulas: w.sample_formulas(),
         formula_editing: w.formula_editing,
@@ -671,6 +693,8 @@ fn step_kind(step: &WizardStep) -> WizardStepKind {
         WizardStep::SelectArrayPath => WizardStepKind::SelectArrayPath,
         WizardStep::ReviewProposals => WizardStepKind::ReviewProposals,
         WizardStep::ConfigureDates => WizardStepKind::ConfigureDates,
+        WizardStep::ResolveDuplicates => WizardStepKind::ResolveDuplicates,
+        WizardStep::NameDuplicateCategory => WizardStepKind::NameDuplicateCategory,
         WizardStep::DefineFormulas => WizardStepKind::DefineFormulas,
         WizardStep::NameModel => WizardStepKind::NameModel,
         WizardStep::Done => WizardStepKind::Done,

crates/improvise-protocol/src/lib.rs

@@ -35,9 +35,9 @@ pub use render::{
     AxisFrame, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay, CellId, CreateKind,
     DocumentCache, FormulaCache, FormulaEntry, FrameEntry, FrameGroup, GridCache, GridExtent,
     GridUpdate, MAX_ROW_HEADER_W, MIN_ROW_HEADER_W, Projection, RecordCell, Snapshot, TileBarCache,
-    TileEntry, ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardDateField,
-    WizardFieldKind, WizardProposal, WizardStepKind, header_level_widths, non_item_refusal,
-    opening_tile,
+    TileEntry, ViewListCache, ViewSummary, WidgetCache, WizardCache, WizardChoice, WizardCollision,
+    WizardDateField, WizardFieldKind, WizardProposal, WizardStepKind, header_level_widths,
+    non_item_refusal, opening_tile,
 };
 pub use resolve::{
     Click, KeyOutcome, OpenedEditor, Request, click, edited, read_buffer, resolve, typed,

crates/improvise-protocol/src/render.rs

@@ -918,6 +918,8 @@ pub enum WizardStepKind {
     SelectArrayPath,
     ReviewProposals,
     ConfigureDates,
+    ResolveDuplicates,
+    NameDuplicateCategory,
     DefineFormulas,
     NameModel,
     Done,
@@ -955,6 +957,41 @@ pub struct WizardDateField {
     pub components: Vec<(String, bool)>,
 }
 
+/// Several source records sharing every coordinate (improvise-cgqt). Data,
+/// so every front end lists the same rows; [`Self::describe`] words it.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct WizardCollision {
+    pub coords: Vec<(String, String)>,
+    /// 1-based positions of the colliding records in the source.
+    pub records: Vec<usize>,
+}
+
+impl WizardCollision {
+    /// One line naming the records and the tuple they share, never
+    /// abbreviated: a front end wraps it rather than cutting it.
+    pub fn describe(&self) -> String {
+        let records: Vec<String> = self.records.iter().map(|r| format!("#{r}")).collect();
+        let coords: Vec<String> = self
+            .coords
+            .iter()
+            .map(|(c, i)| format!("{c}={i}"))
+            .collect();
+        format!(
+            "{} records ({}): {}",
+            self.records.len(),
+            records.join(", "),
+            coords.join(", ")
+        )
+    }
+}
+
+/// One option on the duplicates step; `disabled` says why it cannot be taken.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct WizardChoice {
+    pub label: String,
+    pub disabled: Option<String>,
+}
+
 /// The import wizard as the widget draws it (improvise-764). **Deliberately
 /// not a [`Projection`]** — that absence is the argument; see §3.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
@@ -974,6 +1011,14 @@ pub struct WizardCache {
     pub proposals: Vec<WizardProposal>,
     /// `ConfigureDates`: the accepted time fields and their components.
     pub date_fields: Vec<WizardDateField>,
+    /// `ResolveDuplicates`: every colliding tuple, in first-seen order.
+    pub collisions: Vec<WizardCollision>,
+    /// `ResolveDuplicates`: sum, keep separate, go back — in cursor order.
+    pub duplicate_choices: Vec<WizardChoice>,
+    /// `NameDuplicateCategory`: the name being typed.
+    pub duplicate_category: String,
+    /// The first item drawn of the step's scrolling list.
+    pub scroll: usize,
     /// `DefineFormulas`: the formulas defined so far.
     pub formulas: Vec<String>,
     /// `DefineFormulas`: suggestions derived from the accepted measures.