From 195f3b44b8adb9ae9ec6cfc3f6cfed9748cfa878 Mon Sep 17 00:00:00 2001 From: "Claude (agent-hier)" Date: Thu, 24 Sep 2026 22:15:14 -0700 Subject: [PATCH 1/8] feat(import): a date column imports as one hierarchical category, the flat split a choice (improvise-8jk6.3) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- README.md | 2 +- context/code/formula-and-io.md | 45 +++ context/test-evidence/formula-and-io.md | 38 +++ .../src/command/cmd/wizard.rs | 1 + crates/improvise-command/src/ui/effect.rs | 2 + crates/improvise-command/src/ui/project.rs | 63 +++- crates/improvise-io/src/import/analyzer.rs | 27 ++ crates/improvise-io/src/import/wizard.rs | 301 +++++++++++++++++- crates/improvise-protocol/src/render.rs | 3 + src/main.rs | 21 +- src/ui/import_wizard_ui.rs | 11 +- tests/headless_cli.rs | 53 +++ 12 files changed, 550 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 11e6fa19..3f267d4b 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ improvise import examples/demo.csv \ --no-wizard \ --category Region --category Product --category Customer \ --measure Revenue --measure Cost \ - --time Date --extract Date:Month \ + --time Date --extract Date:Month --split-date Date \ --axis Region:row --axis Product:row \ --axis Date_Month:column --axis Measure:column \ --axis Customer:page --axis Date:none \ diff --git a/context/code/formula-and-io.md b/context/code/formula-and-io.md index 3e101504..b34b7fa4 100644 --- a/context/code/formula-and-io.md +++ b/context/code/formula-and-io.md @@ -613,6 +613,51 @@ different row order on screen, and a different detected date format, since varied. The `HashSet` in `analyze_records` is there to deduplicate and **never to iterate**. +### A date column imports as ONE hierarchical category (improvise-8jk6.3) + +The user: "I think dates should usually be one category and we can use +formulas to crosstab." So the default `DateShape::Hierarchy` builds one +category named after the column, where the old import built up to three flat +ones beside it (`Date_Year`, `Date_Quarter`, `Date_Month`): + +- **Its items are the finest chosen component**, spelled so they stay unique + across years: `2024-01`, `2024-Q1`, `2024`, which are + `extract_date_component`'s existing spellings. A month is never `Jan`. +- **The coarser chosen components are its groups**, coarsest first + (`DateComponent::COARSE_TO_FINE`): a month sits in its quarter, and the + quarter's `>` line names its year. A level left unchosen is skipped rather + than drawn empty, so Year + Month groups months straight into years. +- **It opens folded**, one row per year, with nothing to set, because every + group starts closed (`context/code/core.md` §19). +- **It costs one of the category slots**, not up to four, and + `proposed_category_count` counts it that way. +- **The raw dates are gone from that category.** The day is not a component, so + two records of one month land on one cell and the collision step asks, as it + does for any other collision. `Split` keeps the day. +- **A value the detected format cannot read keeps its raw text** as an + ungrouped item, with a warning naming the column and the row count, rather + than being dropped. The warning is separate from the split shape's, because + that one reports an absent coordinate and this one a coordinate that is there + and ungrouped. +- **Item order is first-seen, not chronological.** Order is a correctness + property here (gotcha 18), and a group need not be contiguous, so a CSV that + is not in date order still nests correctly. It is just drawn in the order + it was read. + +`DateShape::Split` is the old behaviour, unchanged, and stays a choice. `s` on +the wizard's date step toggles the field under the cursor, and the header line +for each field says which it will get ("one category: Year > Quarter > Month" +or "split: Date, Date_Year, …"). It is worded once, in +`project.rs::date_shape_sentence`. Headless, `--split-date ` does the +same, and `--extract` keeps its meaning: which components to build. + +**Crosstabs by formula are not available yet.** A formula can name a month +(`SUM(Amount WHERE Date = "2024-01")`, `Date IS "2024-01"`), but a `WHERE` or +`IS` naming a GROUP matches no record: both compare the coordinate to the name +exactly. `SUM(Amount WHERE Date = "2024")` is an empty sum, and `Date IS "2024"` +is 0 on every month of 2024. That was measured on this branch and reported for +filing, not built here. + ### The ambiguous-date heuristic reads American-first, deliberately `01/02/2025` parses under both `%m/%d/%Y` and `%d/%m/%Y` and nothing in the data diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index 717ba987..0cae6931 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -1648,3 +1648,41 @@ category named `expanded`. closed, the one its `collapsed:` line names and the three it does not, and the line is written back. Rival: the old open default; this test fails with the other eleven in that rival's list. + +### a_date_column_imports_as_one_category_of_months_in_quarters_in_years + +improvise-8jk6.3, the user's "dates should usually be one category". The +fixture has two days of one month, so that month is one summed cell, and +January of the next year, so a month must stay unique across years. Rivals, each +run against the `date_hierarchy` module, and each caught here: +- The split as the default shape. +- Levels taken finest first, so the year hangs under the month. +- Groups registered without their parents. +- The raw dates still added beside the months. +- A hierarchy counted as `1 + components` slots. + +### a_level_left_unchosen_is_skipped_not_emptied + +Year + Month nests months straight into years, and Quarter alone is a flat +category of quarters. Caught the finest-first rival and the raw-dates rival. + +### an_imported_date_hierarchy_opens_folded_to_its_years + +The import sets nothing: the hierarchy opens folded, one row per year, because +every group starts closed (improvise-felo.6). The 2024 row is the fold of its +months, 111. Caught three of the rivals above. + +### an_unparsed_date_keeps_its_value_and_is_warned_about + +A format that reads none of the values keeps each raw value as an ungrouped +item, and the build says so. Its first version passed on the base commit: the +shape was ignored there and the SPLIT's warning happened to contain the same +words. It now also asserts there is no `Date_Year`. Rival: the unparsed value +not counted; this test fails. + +### a_headless_date_import_splits_when_told_to + +`--split-date` is the wizard's `s`, headless. It failed first as an unknown +argument. Rival: the flag parsed and ignored; this test fails, and +`a_headless_date_import_is_one_hierarchical_category` does not, which is the +point of having both. diff --git a/crates/improvise-command/src/command/cmd/wizard.rs b/crates/improvise-command/src/command/cmd/wizard.rs index cb145dfc..66ccb650 100644 --- a/crates/improvise-command/src/command/cmd/wizard.rs +++ b/crates/improvise-command/src/command/cmd/wizard.rs @@ -65,6 +65,7 @@ fn review_proposal_keys(key: KeyCode) -> Vec> { fn configure_date_keys(key: KeyCode) -> Vec> { match key { KeyCode::Char(' ') => op(WizardOp::ToggleDateComponent), + KeyCode::Char('s') => op(WizardOp::ToggleDateShape), KeyCode::Enter => op(WizardOp::Advance), _ => cursor_keys(key), } diff --git a/crates/improvise-command/src/ui/effect.rs b/crates/improvise-command/src/ui/effect.rs index d12b1a58..446fe422 100644 --- a/crates/improvise-command/src/ui/effect.rs +++ b/crates/improvise-command/src/ui/effect.rs @@ -1053,6 +1053,7 @@ pub enum WizardOp { ToggleProposal, CycleProposalKind, ToggleDateComponent, + ToggleDateShape, StartFormulaEdit, DeleteFormula, ConfirmFormula, @@ -1078,6 +1079,7 @@ impl WizardOp { Self::ToggleProposal => wizard.toggle_proposal(), Self::CycleProposalKind => wizard.cycle_proposal_kind(), Self::ToggleDateComponent => wizard.toggle_date_component(), + Self::ToggleDateShape => wizard.toggle_date_shape(), Self::StartFormulaEdit => wizard.start_formula_edit(), Self::DeleteFormula => wizard.delete_formula(), Self::ConfirmFormula => wizard.confirm_formula(), diff --git a/crates/improvise-command/src/ui/project.rs b/crates/improvise-command/src/ui/project.rs index e434bfb5..17588b20 100644 --- a/crates/improvise-command/src/ui/project.rs +++ b/crates/improvise-command/src/ui/project.rs @@ -11,7 +11,7 @@ use improvise_core::model::record::{AggregateHash, RecordHash}; 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::analyzer::{DateComponent, DateShape, FieldKind}; use improvise_io::import::wizard::{DuplicatePolicy, ImportWizard, WizardStep}; use improvise_protocol::{ AxisFrame, Basis, BasisCache, CategoryRow, CategoryTreeCache, CategoryTreeUpdate, CellDisplay, @@ -644,6 +644,7 @@ pub fn wizard_cache(w: &ImportWizard) -> WizardCache { ((*label).to_string(), p.date_components.contains(component)) }) .collect(), + shape: date_shape_sentence(p), }) .collect(), collisions: w @@ -682,6 +683,27 @@ pub fn wizard_cache(w: &ImportWizard) -> WizardCache { } } +/// What a date field's chosen components become, worded for every front end. +fn date_shape_sentence(p: &improvise_io::import::analyzer::FieldProposal) -> String { + let chosen = |order: &[DateComponent]| -> Vec<&str> { + order + .iter() + .filter(|c| p.date_components.contains(c)) + .filter_map(|c| DATE_COMPONENTS.iter().find(|(d, _)| d == c)) + .map(|(_, label)| *label) + .collect() + }; + match (p.date_shape, chosen(&DateComponent::COARSE_TO_FINE)) { + (_, none) if none.is_empty() => "one category of the raw dates".to_string(), + (DateShape::Hierarchy, levels) => format!("one category: {}", levels.join(" > ")), + (DateShape::Split, _) => { + let parts = chosen(&DATE_COMPONENTS.map(|(c, _)| c)); + let names: Vec = parts.iter().map(|l| format!("{}_{l}", p.field)).collect(); + format!("split: {}, {}", p.field, names.join(", ")) + } + } +} + /// The date components the wizard offers, **in the order it offers them**: /// `ImportWizard::date_config_at_cursor` counts these same rows. const DATE_COMPONENTS: [(DateComponent, &str); 3] = [ @@ -6464,3 +6486,42 @@ mod column_totals { ); } } + +/// improvise-8jk6.3: the wizard's date step says what each date field becomes, +/// and `s` switches it between one category and the split. +#[cfg(test)] +mod date_shape { + use super::wizard_cache; + use crate::import::wizard::{ImportWizard, WizardStep}; + use serde_json::json; + + #[test] + fn the_date_step_says_what_a_date_field_becomes_and_s_switches_it() { + let mut w = ImportWizard::new(json!([ + {"Region": "East", "When": "2024-01-15", "Revenue": 1.0}, + {"Region": "West", "When": "2024-04-02", "Revenue": 2.0}, + ])); + w.advance(); + assert_eq!(w.step, WizardStep::ConfigureDates, "setup"); + let shape = |w: &ImportWizard| wizard_cache(w).date_fields[0].shape.clone(); + assert_eq!(shape(&w), "one category of the raw dates"); + w.toggle_date_component(); + w.move_cursor(1); + w.toggle_date_component(); + w.move_cursor(1); + w.toggle_date_component(); + assert_eq!(shape(&w), "one category: Year > Quarter > Month"); + w.toggle_date_shape(); + assert_eq!( + shape(&w), + "split: When, When_Year, When_Month, When_Quarter" + ); + let built = w.pipeline.build_model().unwrap(); + assert!(built.model.category("When_Quarter").is_some()); + w.toggle_date_shape(); + let built = w.pipeline.build_model().unwrap(); + assert!(built.model.category("When_Quarter").is_none()); + let when = built.model.category("When").unwrap(); + assert_eq!(when.item_chain("2024-04"), vec!["2024", "2024-Q2"]); + } +} diff --git a/crates/improvise-io/src/import/analyzer.rs b/crates/improvise-io/src/import/analyzer.rs index d51dac1f..69a39c55 100644 --- a/crates/improvise-io/src/import/analyzer.rs +++ b/crates/improvise-io/src/import/analyzer.rs @@ -22,6 +22,26 @@ pub enum DateComponent { Quarter, } +impl DateComponent { + /// The components as a hierarchy nests them, coarsest first. + pub const COARSE_TO_FINE: [DateComponent; 3] = [ + DateComponent::Year, + DateComponent::Quarter, + DateComponent::Month, + ]; +} + +/// How a date column's components become categories (improvise-8jk6.3). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum DateShape { + /// ONE category named after the column: its items are the finest chosen + /// component, grouped by the coarser ones. The user's default. + #[default] + Hierarchy, + /// The column's raw values, plus one flat category per chosen component. + Split, +} + #[derive(Debug, Clone)] pub struct FieldProposal { pub field: String, @@ -32,6 +52,8 @@ pub struct FieldProposal { pub date_format: Option, /// Which date components to extract as new categories. pub date_components: Vec, + /// Whether those components nest in one category or split into several. + pub date_shape: DateShape, } impl FieldProposal { @@ -132,6 +154,7 @@ pub fn analyze_records(records: &[Value]) -> Vec { accepted: true, date_format: None, date_components: vec![], + date_shape: DateShape::default(), }; } @@ -159,6 +182,7 @@ pub fn analyze_records(records: &[Value]) -> Vec { accepted: true, date_format, date_components: vec![], + date_shape: DateShape::default(), }; } @@ -170,6 +194,7 @@ pub fn analyze_records(records: &[Value]) -> Vec { accepted: true, date_format: None, date_components: vec![], + date_shape: DateShape::default(), }; } @@ -180,6 +205,7 @@ pub fn analyze_records(records: &[Value]) -> Vec { accepted: true, date_format: None, date_components: vec![], + date_shape: DateShape::default(), }; } @@ -191,6 +217,7 @@ pub fn analyze_records(records: &[Value]) -> Vec { accepted: true, date_format: None, date_components: vec![], + date_shape: DateShape::default(), } }) .collect() diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index 28efbdff..39014824 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -4,7 +4,7 @@ use anyhow::{Result, anyhow}; use serde_json::Value; use super::analyzer::{ - DateComponent, FieldKind, FieldProposal, analyze_records, extract_array_at_path, + DateComponent, DateShape, FieldKind, FieldProposal, analyze_records, extract_array_at_path, extract_date_component, find_array_paths, }; use crate::formula::parse_formula; @@ -116,7 +116,10 @@ impl ImportPipeline { .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 if p.date_format.is_some() => match p.date_shape { + DateShape::Hierarchy => 1, + DateShape::Split => 1 + p.date_components.len(), + }, FieldKind::TimeCategory => 1, FieldKind::Measure | FieldKind::Label => 0, }) @@ -177,6 +180,7 @@ impl ImportPipeline { && p.kind == FieldKind::TimeCategory && p.date_format.is_some() && !p.date_components.is_empty() + && p.date_shape == DateShape::Split }) .flat_map(|p| { let fmt = p.date_format.as_deref().unwrap(); @@ -191,11 +195,17 @@ impl ImportPipeline { }) }) .collect(); + let categories = accepted(&[FieldKind::Category, FieldKind::TimeCategory]); + let date_hierarchies = categories + .iter() + .filter_map(|p| DateHierarchy::of(p)) + .collect(); Plan { - categories: accepted(&[FieldKind::Category, FieldKind::TimeCategory]), + categories, measures: accepted(&[FieldKind::Measure]), labels: accepted(&[FieldKind::Label]), date_extractions, + date_hierarchies, } } @@ -222,11 +232,16 @@ impl ImportPipeline { for cat_proposal in categories { wb.add_category(&cat_proposal.field)?; - for val in cat_proposal - .distinct_values + // A hierarchical date column's items are its components, added below. + let raw_items = match plan + .date_hierarchies .iter() - .filter(|v| !v.trim().is_empty()) + .any(|h| h.field == cat_proposal.field) { + true => &[][..], + false => &cat_proposal.distinct_values[..], + }; + for val in raw_items.iter().filter(|v| !v.trim().is_empty()) { wb.add_item(&cat_proposal.field, val)?; } } @@ -263,7 +278,7 @@ impl ImportPipeline { let mut skipped_non_objects = 0usize; // Per-extraction tally of values the detected format could not parse. A // derived coordinate that is merely absent is improvise-yss5. - let mut unparsed_dates = vec![0usize; date_extractions.len()]; + let mut unparsed_dates = vec![0usize; date_extractions.len() + plan.date_hierarchies.len()]; // Two rows sharing every coordinate are one cell; last-row-wins lost the // earlier one (improvise-2cw9). The map only deduplicates — gotcha 18. let mut cells: Vec = vec![]; @@ -287,7 +302,11 @@ impl ImportPipeline { mut coords, measures, unparsed, + chains, } => { + for chain in &chains { + add_date_chain(&mut wb, chain)?; + } for (cat, item) in &coords { wb.add_item(cat, item)?; } @@ -345,7 +364,21 @@ impl ImportPipeline { &skipped_per_category, skipped_non_objects, ); - warnings.extend(unparsed_date_warnings(date_extractions, &unparsed_dates)); + let (split, whole) = unparsed_dates.split_at(date_extractions.len()); + warnings.extend(unparsed_date_warnings(date_extractions, split)); + warnings.extend( + plan.date_hierarchies + .iter() + .zip(whole) + .filter(|(_, n)| **n > 0) + .map(|(h, n)| { + format!( + "'{}' kept its raw value in {n} row(s), ungrouped; it did not match \ + the date format '{}'", + h.field, h.format + ) + }), + ); warnings.extend(collision_warning(&summed)); warnings.extend(occurrence.map(|name| separated_note(name, &collisions))); @@ -359,6 +392,40 @@ struct Plan<'a> { measures: Vec<&'a FieldProposal>, labels: Vec<&'a FieldProposal>, date_extractions: Vec<(&'a str, &'a str, DateComponent, String)>, + date_hierarchies: Vec>, +} + +/// A date column imported as ONE category: its items are the finest chosen +/// component, grouped by the coarser ones (improvise-8jk6.3, core.md §19). +struct DateHierarchy<'a> { + field: &'a str, + format: &'a str, + /// The chosen components, coarsest first. + levels: Vec, +} + +impl<'a> DateHierarchy<'a> { + fn of(p: &'a FieldProposal) -> Option { + let levels: Vec = DateComponent::COARSE_TO_FINE + .into_iter() + .filter(|c| p.date_components.contains(c)) + .collect(); + let hierarchical = + p.kind == FieldKind::TimeCategory && p.date_shape == DateShape::Hierarchy; + (hierarchical && !levels.is_empty()).then_some(Self { + field: &p.field, + format: p.date_format.as_deref()?, + levels, + }) + } + + /// Every level's name for one value, root first; the last is the item. + fn chain(&self, value: &str) -> Option> { + self.levels + .iter() + .map(|c| extract_date_component(value, self.format, *c)) + .collect() + } } /// Where one source record lands. Worked out once, for the build and for the @@ -370,6 +437,7 @@ enum Placement { coords: Vec<(String, String)>, measures: Vec<(String, f64)>, unparsed: Vec, + chains: Vec, }, /// No value for category `cat_idx`; `partial` is what preceded it. Missing { @@ -388,6 +456,7 @@ impl Plan<'_> { // Charged only once this record survives the walk: a row dropped // for a LATER missing category never lands (context/test-evidence). let mut unparsed: Vec = vec![]; + let mut chains: Vec = vec![]; for (cat_idx, cat_proposal) in self.categories.iter().enumerate() { let Some(v) = map.get(&cat_proposal.field).and_then(scalar_name) else { @@ -396,7 +465,24 @@ impl Plan<'_> { partial: coords, }; }; - coords.push((cat_proposal.field.clone(), v.clone())); + let hierarchy = self + .date_hierarchies + .iter() + .enumerate() + .find(|(_, h)| h.field == cat_proposal.field); + let item = match hierarchy.map(|(at, h)| (at, h.chain(&v))) { + Some((_, Some(chain))) => { + let item = chain.last().cloned().unwrap_or_default(); + chains.push((cat_proposal.field.clone(), chain)); + item + } + Some((at, None)) => { + unparsed.push(self.date_extractions.len() + at); + v.clone() + } + None => v.clone(), + }; + coords.push((cat_proposal.field.clone(), item)); // Extract date components from this field's value for (at, (field, fmt, comp, derived_name)) in self.date_extractions.iter().enumerate() { @@ -428,10 +514,33 @@ impl Plan<'_> { coords, measures, unparsed, + chains, } } } +/// One hierarchical date coordinate: its category and its chain, root first. +type DateChain = (String, Vec); + +/// Register a date item under its groups, and each group under its parent. +fn add_date_chain(wb: &mut Workbook, (cat, chain): &DateChain) -> Result<()> { + let Some((item, groups)) = chain.split_last() else { + return Ok(()); + }; + match groups.last() { + Some(group) => wb.add_item_in_group(cat, item, group)?, + None => wb.add_item(cat, item)?, + }; + let category = wb + .model + .category_mut(cat) + .ok_or_else(|| anyhow!("category '{cat}' vanished mid-import"))?; + for pair in groups.windows(2) { + category.add_group(crate::model::category::Group::new(&pair[1]).with_parent(&pair[0])); + } + Ok(()) +} + /// A tuple, and each record on it: its 1-based position and the measures it writes. type OnTuple<'a> = (&'a Vec<(String, String)>, Vec<(usize, Vec<&'a str>)>); @@ -1026,6 +1135,18 @@ impl ImportWizard { } } + /// Switch the field at the cursor between ONE hierarchical category and the + /// flat split into one category per component (improvise-8jk6.3). + pub fn toggle_date_shape(&mut self) { + if let Some((pi, _)) = self.date_config_at_cursor() { + let proposal = &mut self.pipeline.proposals[pi]; + proposal.date_shape = match proposal.date_shape { + DateShape::Hierarchy => DateShape::Split, + DateShape::Split => DateShape::Hierarchy, + }; + } + } + // ── Formula editing ──────────────────────────────────────────────────────── /// Buffer for typing a new formula in the DefineFormulas step. @@ -1327,6 +1448,7 @@ mod tests { prop.date_format = Some("%m/%d/%Y".to_string()); prop.date_components.push(DateComponent::Month); prop.date_components.push(DateComponent::Year); + prop.date_shape = crate::import::analyzer::DateShape::Split; } } p @@ -1431,6 +1553,7 @@ mod tests { for prop in &mut p.proposals { if prop.field == "Date" && prop.kind == FieldKind::TimeCategory { prop.date_components.push(DateComponent::Month); + prop.date_shape = crate::import::analyzer::DateShape::Split; } } let wb = p.build_model().unwrap(); @@ -2197,6 +2320,7 @@ mod tests { for prop in &mut p.proposals { if prop.field == "Date" { prop.date_components.push(DateComponent::Month); + prop.date_shape = crate::import::analyzer::DateShape::Split; } } let wb = p.build_model().unwrap(); @@ -2709,6 +2833,7 @@ mod tests { w.advance(); assert_eq!(w.step, WizardStep::ConfigureDates, "setup"); w.toggle_date_component(); + w.toggle_date_shape(); w.advance(); assert_eq!(w.step, WizardStep::ResolveDuplicates, "setup"); w.move_cursor(1); @@ -2728,3 +2853,161 @@ mod tests { ); } } + +/// improvise-8jk6.3: a date column imports as ONE hierarchical category. +#[cfg(test)] +mod date_hierarchy { + use super::{DuplicatePolicy, ImportPipeline}; + use crate::import::analyzer::{DateComponent, DateShape, FieldKind}; + use crate::model::cell::CellKey; + use crate::view::GridLayout; + use serde_json::json; + + /// Two days of one month, a month in the next quarter, and January of the + /// NEXT year, so a month must stay unique across years. + fn records() -> serde_json::Value { + json!([ + {"Date": "01/15/2024", "Amount": 1.0}, + {"Date": "01/20/2024", "Amount": 10.0}, + {"Date": "04/02/2024", "Amount": 100.0}, + {"Date": "01/03/2025", "Amount": 1000.0}, + ]) + } + + fn pipeline(components: &[DateComponent], shape: DateShape) -> ImportPipeline { + let mut p = ImportPipeline::new(records()); + p.on_duplicate = DuplicatePolicy::Sum; + let date = p.proposals.iter_mut().find(|p| p.field == "Date").unwrap(); + assert_eq!(date.kind, FieldKind::TimeCategory, "precondition"); + date.date_components = components.to_vec(); + date.date_shape = shape; + p + } + + fn cell(month: &str) -> CellKey { + CellKey::new(vec![ + ("Date".into(), month.into()), + ("_Measure".into(), "Amount".into()), + ]) + } + + /// The default shape: ONE category named after the column, its items the + /// months, grouped into quarters grouped into years. + #[test] + fn a_date_column_imports_as_one_category_of_months_in_quarters_in_years() { + use DateComponent::*; + let p = pipeline(&[Month, Year, Quarter], DateShape::default()); + assert_eq!(p.proposed_category_count(), 1, "one slot, not four"); + let wb = p.build_model().unwrap(); + let names: Vec<&str> = wb.model.regular_category_names(); + assert_eq!( + names, + vec!["Date"], + "no Date_Year, Date_Quarter or Date_Month" + ); + let date = wb.model.category("Date").unwrap(); + let items: Vec<&str> = date.items.keys().map(String::as_str).collect(); + assert_eq!(items, vec!["2024-01", "2024-04", "2025-01"]); + assert_eq!(date.item_chain("2024-01"), vec!["2024", "2024-Q1"]); + assert_eq!(date.item_chain("2025-01"), vec!["2025", "2025-Q1"]); + assert_eq!( + wb.model.get_cell(&cell("2024-01")).and_then(|v| v.as_f64()), + Some(11.0), + "two days of one month are one cell" + ); + assert_eq!( + wb.model.get_cell(&cell("2025-01")).and_then(|v| v.as_f64()), + Some(1000.0), + "January 2025 is not January 2024" + ); + } + + /// Chosen levels only: without Quarter, months group straight into years. + #[test] + fn a_level_left_unchosen_is_skipped_not_emptied() { + use DateComponent::*; + let wb = pipeline(&[Year, Month], DateShape::Hierarchy) + .build_model() + .unwrap(); + let date = wb.model.category("Date").unwrap(); + assert_eq!(date.item_chain("2024-04"), vec!["2024"]); + let wb = pipeline(&[Quarter], DateShape::Hierarchy) + .build_model() + .unwrap(); + let date = wb.model.category("Date").unwrap(); + let items: Vec<&str> = date.items.keys().map(String::as_str).collect(); + assert_eq!( + items, + vec!["2024-Q1", "2024-Q2", "2025-Q1"], + "flat quarters" + ); + assert!(date.item_chain("2024-Q1").is_empty()); + } + + /// The user's ruling on improvise-felo.6: the hierarchy opens folded, one + /// row per year. + #[test] + fn an_imported_date_hierarchy_opens_folded_to_its_years() { + use DateComponent::*; + let wb = pipeline(&[Year, Quarter, Month], DateShape::Hierarchy) + .build_model() + .unwrap(); + let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None); + let rows: Vec = (0..layout.row_count()) + .map(|r| layout.row_label(r)) + .collect(); + assert_eq!(rows, vec!["2024", "2025"]); + let v = layout.evaluate(&wb.model, &layout.cell_key(0, 0).unwrap()); + assert_eq!( + v.and_then(|v| v.as_f64()), + Some(111.0), + "2024 folds its months" + ); + } + + /// The flat split stays available as a choice, and is what it was. + #[test] + fn the_split_shape_keeps_the_raw_dates_and_one_category_per_component() { + use DateComponent::*; + let p = pipeline(&[Year, Month], DateShape::Split); + assert_eq!(p.proposed_category_count(), 3); + let wb = p.build_model().unwrap(); + let mut names: Vec<&str> = wb.model.regular_category_names(); + names.sort_unstable(); + assert_eq!(names, vec!["Date", "Date_Month", "Date_Year"]); + assert!( + wb.model + .category("Date") + .unwrap() + .items + .contains_key("01/15/2024") + ); + } + + /// A value the detected format cannot read keeps its raw text as an + /// ungrouped item, and the build says so rather than dropping the record. + #[test] + fn an_unparsed_date_keeps_its_value_and_is_warned_about() { + use DateComponent::*; + let mut p = pipeline(&[Year, Month], DateShape::Hierarchy); + let date = p.proposals.iter_mut().find(|p| p.field == "Date").unwrap(); + date.date_format = Some("%Y-%m-%d".into()); + let (wb, warnings) = p.build_model_with_warnings().unwrap(); + assert!(wb.model.category("Date_Year").is_none(), "one category"); + let date = wb.model.category("Date").unwrap(); + assert!( + date.items.contains_key("01/15/2024"), + "the raw value is kept" + ); + assert!( + date.item_chain("01/15/2024").is_empty(), + "and it is ungrouped" + ); + assert!( + warnings + .iter() + .any(|w| w.contains("'Date' kept its raw value in 4 row(s)")), + "{warnings:?}" + ); + } +} diff --git a/crates/improvise-protocol/src/render.rs b/crates/improvise-protocol/src/render.rs index c8621397..b3d7a367 100644 --- a/crates/improvise-protocol/src/render.rs +++ b/crates/improvise-protocol/src/render.rs @@ -961,6 +961,9 @@ pub struct WizardDateField { /// what is drawn, so the substitution is made once, here. pub format: String, pub components: Vec<(String, bool)>, + /// What the chosen components become, as a sentence: one hierarchical + /// category, or a split (improvise-8jk6.3). Worded once, in the projection. + pub shape: String, } /// Several source records sharing every coordinate (improvise-cgqt). Data, diff --git a/src/main.rs b/src/main.rs index f2144274..3bb84a47 100644 --- a/src/main.rs +++ b/src/main.rs @@ -38,6 +38,8 @@ trait Runnable { fn run(self, model_file: Option) -> Result<()>; } +// Parsed once per process, so a variant's size costs nothing worth boxing for. +#[allow(clippy::large_enum_variant)] #[derive(Subcommand)] #[enum_dispatch(Runnable)] enum Commands { @@ -72,10 +74,16 @@ struct ImportArgs { #[arg(long)] skip: Vec, - /// Extract date component, e.g. "Date:Month" (repeatable) + /// Extract date component, e.g. "Date:Month" (repeatable). The components + /// nest in ONE category named after the column unless --split-date names it #[arg(long)] extract: Vec, + /// Split a date field's components into one flat category each, beside its + /// raw values, rather than one hierarchical category (repeatable) + #[arg(long)] + split_date: Vec, + /// Set category axis, e.g. "Payee:row" (repeatable) #[arg(long)] axis: Vec, @@ -153,6 +161,7 @@ impl Runnable for ImportArgs { time_fields: self.time, skip_fields: self.skip, extractions: parse_colon_pairs(&self.extract), + split_dates: self.split_date, axes: parse_colon_pairs(&self.axis), formulas: self.formula, name: self.name, @@ -187,6 +196,7 @@ struct ImportConfig { time_fields: Vec, skip_fields: Vec, extractions: Vec<(String, String)>, + split_dates: Vec, axes: Vec<(String, String)>, formulas: Vec, name: Option, @@ -205,7 +215,7 @@ fn parse_colon_pairs(args: &[String]) -> Vec<(String, String)> { use import::wizard::DuplicatePolicy; fn apply_config_to_pipeline(pipeline: &mut import::wizard::ImportPipeline, config: &ImportConfig) { - use import::analyzer::{DateComponent, FieldKind}; + use import::analyzer::{DateComponent, DateShape, FieldKind}; // Override field kinds for p in &mut pipeline.proposals { @@ -238,6 +248,13 @@ fn apply_config_to_pipeline(pipeline: &mut import::wizard::ImportPipeline, confi } } + // The wizard's `s`, headless: which date fields split rather than nest. + for p in &mut pipeline.proposals { + if config.split_dates.contains(&p.field) { + p.date_shape = DateShape::Split; + } + } + // Set formulas pipeline.formulas = config.formulas.clone(); diff --git a/src/ui/import_wizard_ui.rs b/src/ui/import_wizard_ui.rs index f9d11183..a3bfc713 100644 --- a/src/ui/import_wizard_ui.rs +++ b/src/ui/import_wizard_ui.rs @@ -190,7 +190,10 @@ impl<'a> Widget for ImportWizardWidget<'a> { if y >= inner.bottom().saturating_sub(2) { break; } - let header = format!(" {} (format: {})", proposal.field, proposal.format); + let header = format!( + " {} (format: {}) — {}", + proposal.field, proposal.format, proposal.shape + ); panel.put( x, y, @@ -231,7 +234,7 @@ impl<'a> Widget for ImportWizardWidget<'a> { x, hint_y, w, - "Space: toggle Enter: next \u{2190} back Esc: cancel", + "Space: toggle s: one category / split Enter: next \u{2190} back Esc: cancel", Style::default().fg(Color::DarkGray), ); } @@ -1188,7 +1191,7 @@ mod tests { "ConfigureDates\n", "┌ Import Wizard — Date Components ─────────────────────────────────────────────┐\n", "│Select date components to extract (Space toggle): │\n", - "│ When (format: %Y-%m-%d) │\n", + "│ When (format: %Y-%m-%d) — one category of the raw dates │\n", "│ [ ] Year │\n", "│ [ ] Month │\n", "│ [ ] Quarter │\n", @@ -1206,7 +1209,7 @@ mod tests { "│ │\n", "│ │\n", "│ │\n", - "│Space: toggle Enter: next ← back Esc: cancel │\n", + "│Space: toggle s: one category / split Enter: next ← back Esc: cancel │\n", "└──────────────────────────────────────────────────────────────────────────────┘\n", "\n", "====\n", diff --git a/tests/headless_cli.rs b/tests/headless_cli.rs index 5af8a6c2..e990c516 100644 --- a/tests/headless_cli.rs +++ b/tests/headless_cli.rs @@ -612,3 +612,56 @@ fn a_headless_import_keeps_duplicates_separate_when_told_to() { assert!(doc.contains("= 10") && doc.contains("= 20"), "{doc}"); assert!(!doc.contains("= 30"), "{doc}"); } + +/// Dates across two years, one per month, so nothing collides. +fn dated_csv(scratch: &Scratch) -> PathBuf { + let csv = scratch.path("dated.csv"); + std::fs::write( + &csv, + "Date,Revenue\n2024-01-15,1\n2024-04-02,10\n2025-01-03,100\n", + ) + .unwrap(); + csv +} + +/// improvise-8jk6.3, headless: `--extract` builds ONE hierarchical category by +/// default, as the wizard does, and it is written with its `>` lines. +#[test] +fn a_headless_date_import_is_one_hierarchical_category() { + let scratch = Scratch::new("date_hierarchy"); + let out_path = scratch.path("out.improv"); + let out = run_import( + &dated_csv(&scratch), + &out_path, + &[ + "--extract", + "Date:Year", + "--extract", + "Date:Quarter", + "--extract", + "Date:Month", + ], + ); + assert!(out.status.success(), "{}", stderr_of(&out)); + let doc = std::fs::read_to_string(&out_path).unwrap(); + assert!(!doc.contains("Date_Month"), "{doc}"); + assert!(doc.contains("- |2024-01|[|2024-Q1|]"), "{doc}"); + assert!(doc.contains("> |2024-Q1|[|2024|]"), "{doc}"); + assert!(!doc.contains("expanded"), "it opens folded: {doc}"); +} + +/// The flat split stays available headless, as `s` makes it in the wizard. +#[test] +fn a_headless_date_import_splits_when_told_to() { + let scratch = Scratch::new("date_split"); + let out_path = scratch.path("out.improv"); + let out = run_import( + &dated_csv(&scratch), + &out_path, + &["--extract", "Date:Month", "--split-date", "Date"], + ); + assert!(out.status.success(), "{}", stderr_of(&out)); + let doc = std::fs::read_to_string(&out_path).unwrap(); + assert!(doc.contains("## Category: Date_Month"), "{doc}"); + assert!(doc.contains("2024-01-15"), "the raw dates stay: {doc}"); +} From b516567e583857c6a2fdf8591591f7b998ca5c88 Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 20:49:26 -0700 Subject: [PATCH 2/8] test(import): a raw date spelled like a generated item or group must not merge (improvise-d0n6) --- crates/improvise-io/src/import/wizard.rs | 45 ++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index 96633e74..622dc3db 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -3476,4 +3476,49 @@ mod date_hierarchy { "{warnings:?}" ); } + + /// A %Y-%m-%d column imported Year > Month, one valid date and one raw + /// value spelled `raw`. + fn with_raw(raw: &str) -> ImportPipeline { + use DateComponent::*; + let mut p = ImportPipeline::new(json!([ + {"Date": "2024-01-15", "Amount": 1.0}, + {"Date": raw, "Amount": 2.0}, + ])); + p.on_duplicate = DuplicatePolicy::Sum; + let date = p.proposals.iter_mut().find(|p| p.field == "Date").unwrap(); + date.kind = FieldKind::TimeCategory; + date.date_format = Some("%Y-%m-%d".into()); + date.date_components = vec![Year, Month]; + date.date_shape = DateShape::Hierarchy; + p + } + + /// A raw value spelled like a generated month would merge with it. + #[test] + fn a_raw_date_spelled_like_a_generated_item_is_refused() { + let err = with_raw("2024-01").build_model_with_warnings().unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("'Date'") && msg.contains("2024-01"), "{msg}"); + assert!(msg.contains("split"), "{msg}"); + } + + /// A raw value spelled like a generated year would sit beside its group. + #[test] + fn a_raw_date_spelled_like_a_generated_group_is_refused() { + let err = with_raw("2024").build_model_with_warnings().unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("'Date'") && msg.contains("2024"), "{msg}"); + } + + /// The control: a raw value spelled like nothing generated still imports, + /// so the refusal above is not a blanket one. + #[test] + fn a_raw_date_spelled_like_nothing_generated_is_kept_ungrouped() { + let (wb, warnings) = with_raw("sometime").build_model_with_warnings().unwrap(); + let date = wb.model.category("Date").unwrap(); + assert!(date.item_chain("sometime").is_empty()); + assert!(date.items.contains_key("2024-01"), "{warnings:?}"); + assert!(warnings.iter().any(|w| w.contains("1 row(s)")), "{warnings:?}"); + } } From c3e12842c2b63a403cc160bc29b2d3d9b161db55 Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 20:50:01 -0700 Subject: [PATCH 3/8] fix(import): refuse a raw date spelled like a generated item or group (improvise-d0n6) --- context/code/formula-and-io.md | 10 +++- context/test-evidence/formula-and-io.md | 15 ++++++ crates/improvise-io/src/import/wizard.rs | 59 +++++++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/context/code/formula-and-io.md b/context/code/formula-and-io.md index adf752eb..1d3bd443 100644 --- a/context/code/formula-and-io.md +++ b/context/code/formula-and-io.md @@ -638,7 +638,15 @@ ones beside it (`Date_Year`, `Date_Quarter`, `Date_Month`): ungrouped item, with a warning naming the column and the row count, rather than being dropped. The warning is separate from the split shape's, because that one reports an absent coordinate and this one a coordinate that is there - and ungrouped. + and ungrouped. **Unless its spelling is one the hierarchy generates** + (improvise-d0n6): a raw `2024-01` or `2024` is the same name as the generated + month or year, so it would join the grouped item and the warning would be + false. The build **refuses**, naming the column and the spellings and pointing + at the split shape. That follows the collision rule below: where two records + would silently be one thing, the user is asked and nothing is defaulted. A + fallback to the split shape was rejected as a silent change of the user's + choice, and a renamed raw item as altering the data. The check is + `Plan::spelling_clash`. - **Item order is first-seen, not chronological.** Order is a correctness property here (gotcha 18), and a group need not be contiguous, so a CSV that is not in date order still nests correctly. It is just drawn in the order diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index 189a7990..cbd1f4f0 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -1904,3 +1904,18 @@ improvise-8jk6.12 (4). A `region` of `" "` is kept as a record with no predates this PR (improvise-ych), but the sparse-record rule now rests on it: without it the value becomes an item named `" "`. Rival F4 (the reviewer's fault) fails it. + +### a_raw_date_spelled_like_a_generated_item_is_refused + +improvise-d0n6 (Codex P2 on PR #322). A `%Y-%m-%d` column imported Year > Month +with `2024-01-15` and a raw `2024-01`. Before the refusal the build succeeded, +put both records on the grouped item `2024-01` (summing them) and warned that +the raw value was "kept ungrouped", which was false. Fault injected: removing +the `spelling_clash` call makes this test fail with `unwrap_err()` on an `Ok`. + +### a_raw_date_spelled_like_a_generated_group_is_refused + +improvise-d0n6. The same with a raw `2024`, a generated group. Removing the +`spelling_clash` call fails it the same way. The control, +`a_raw_date_spelled_like_nothing_generated_is_kept_ungrouped`, passes either +way, so a blanket refusal of every unparsed value fails it. diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index 622dc3db..a66ea166 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use anyhow::{Result, anyhow}; use serde_json::Value; @@ -252,6 +252,9 @@ impl ImportPipeline { } let placements: Vec = self.records.iter().map(|r| plan.place(r)).collect(); + if let Some(clash) = plan.spelling_clash(&placements) { + return Err(anyhow!(clash)); + } let collisions = collisions_among(&placements); let occurrence = match (&self.on_duplicate, collisions.is_empty()) { (_, true) | (DuplicatePolicy::Sum, false) => None, @@ -617,6 +620,60 @@ impl Plan<'_> { } } +impl Plan<'_> { + /// Why the build stops when a value the date format could not read is spelled + /// like an item or group the hierarchy generates (improvise-d0n6). Both would + /// be one name in the category, so the raw record would silently join the + /// grouped item, and the "ungrouped" warning would be false. + fn spelling_clash(&self, placements: &[Placement]) -> Option { + let generated = |field: &str| -> HashSet<&str> { + placements + .iter() + .filter_map(|p| match p { + Placement::Placed { chains, .. } => Some(chains), + Placement::NotObject => None, + }) + .flatten() + .filter(|(cat, _)| cat == field) + .flat_map(|(_, chain)| chain.iter().map(String::as_str)) + .collect() + }; + for (at, h) in self.date_hierarchies.iter().enumerate() { + let tally = self.date_extractions.len() + at; + let names = generated(h.field); + let mut clashing: Vec<&str> = vec![]; + for p in placements { + let Placement::Placed { + coords, unparsed, .. + } = p + else { + continue; + }; + let raw = coords.iter().find(|(c, _)| c == h.field).map(|(_, v)| v); + match raw { + Some(v) if unparsed.contains(&tally) && names.contains(v.as_str()) => { + if !clashing.contains(&v.as_str()) { + clashing.push(v); + } + } + _ => {} + } + } + if !clashing.is_empty() { + return Some(format!( + "'{}' has value(s) that do not match the date format '{}' but are spelled \ + like a month, quarter or year it generates: {}; fix them, or import the \ + column with the split shape", + h.field, + h.format, + clashing.join(", ") + )); + } + } + None + } +} + /// One hierarchical date coordinate: its category and its chain, root first. type DateChain = (String, Vec); From 08b7ab0f350b52c7a73516c9e0d68db71668de1e Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 20:58:51 -0700 Subject: [PATCH 4/8] style(import): format and shorten the spelling-clash check (improvise-d0n6) --- crates/improvise-io/src/import/wizard.rs | 25 ++++++++++++------------ 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index a66ea166..2ad15e00 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -621,10 +621,8 @@ impl Plan<'_> { } impl Plan<'_> { - /// Why the build stops when a value the date format could not read is spelled - /// like an item or group the hierarchy generates (improvise-d0n6). Both would - /// be one name in the category, so the raw record would silently join the - /// grouped item, and the "ungrouped" warning would be false. + /// Why the build stops when an unparsed date is spelled like a generated + /// item or group (improvise-d0n6; formula-and-io.md, date hierarchy). fn spelling_clash(&self, placements: &[Placement]) -> Option { let generated = |field: &str| -> HashSet<&str> { placements @@ -650,14 +648,12 @@ impl Plan<'_> { continue; }; let raw = coords.iter().find(|(c, _)| c == h.field).map(|(_, v)| v); - match raw { - Some(v) if unparsed.contains(&tally) && names.contains(v.as_str()) => { - if !clashing.contains(&v.as_str()) { - clashing.push(v); - } - } - _ => {} - } + let clash = raw.filter(|v| { + unparsed.contains(&tally) + && names.contains(v.as_str()) + && !clashing.contains(&v.as_str()) + }); + clashing.extend(clash.map(String::as_str)); } if !clashing.is_empty() { return Some(format!( @@ -3576,6 +3572,9 @@ mod date_hierarchy { let date = wb.model.category("Date").unwrap(); assert!(date.item_chain("sometime").is_empty()); assert!(date.items.contains_key("2024-01"), "{warnings:?}"); - assert!(warnings.iter().any(|w| w.contains("1 row(s)")), "{warnings:?}"); + assert!( + warnings.iter().any(|w| w.contains("1 row(s)")), + "{warnings:?}" + ); } } From 22d10987bf4057779e1c35eab7ea2ea02da29fb5 Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 21:29:21 -0700 Subject: [PATCH 5/8] test(import): the date step refuses a spelling clash (improvise-d0n6) --- crates/improvise-io/src/import/wizard.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index 2ad15e00..3ceba32f 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -3577,4 +3577,20 @@ mod date_hierarchy { "{warnings:?}" ); } + + /// The clash is answered on the date step, where `s` fixes it, not on the + /// last step three screens later (improvise-907). + #[test] + fn the_date_step_will_not_advance_over_a_spelling_clash() { + use super::{ImportWizard, WizardStep}; + let mut w = ImportWizard::from_pipeline(with_raw("2024")); + w.step = WizardStep::ConfigureDates; + w.advance(); + assert_eq!(w.step, WizardStep::ConfigureDates); + let msg = w.message.clone().unwrap_or_default(); + assert!(msg.contains("'Date'") && msg.contains("2024"), "{msg}"); + w.toggle_date_shape(); + w.advance(); + assert_ne!(w.step, WizardStep::ConfigureDates, "split clears it"); + } } From ec9747cc0eb6e800bcdf9c18e5a676e6d8b45613 Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 21:29:30 -0700 Subject: [PATCH 6/8] fix(import): the date step refuses a spelling clash where the split fix is (improvise-d0n6) --- crates/improvise-io/src/import/wizard.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/improvise-io/src/import/wizard.rs b/crates/improvise-io/src/import/wizard.rs index 3ceba32f..6c8cee29 100644 --- a/crates/improvise-io/src/import/wizard.rs +++ b/crates/improvise-io/src/import/wizard.rs @@ -157,6 +157,13 @@ impl ImportPipeline { .sum() } + /// Why the date step cannot advance: see [`Plan::spelling_clash`]. + pub fn spelling_clash(&self) -> Option { + let plan = self.plan(); + let placements: Vec = self.records.iter().map(|r| plan.place(r)).collect(); + plan.spelling_clash(&placements) + } + /// Every coordinate tuple more than one record lands on, in first-seen /// order. Known before the build, so the wizard can ask (improvise-cgqt). pub fn collisions(&self) -> Vec { @@ -987,7 +994,9 @@ impl ImportWizard { WizardStep::ReviewProposals => self .no_category_message() .or_else(|| self.category_limit_message()), - WizardStep::ConfigureDates => self.category_limit_message(), + WizardStep::ConfigureDates => self + .category_limit_message() + .or_else(|| self.pipeline.spelling_clash()), WizardStep::ResolveDuplicates => match &self.pipeline.on_duplicate { DuplicatePolicy::Sum => None, DuplicatePolicy::Separate(_) => self.separate_blocked(), From 053699b652e37886ca16062224facc9443ede108 Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 21:29:56 -0700 Subject: [PATCH 7/8] test(cli): split-date and axis naming nothing are refused (improvise-tpm8) --- tests/headless_cli.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/headless_cli.rs b/tests/headless_cli.rs index 884c8b3c..65ad2e5c 100644 --- a/tests/headless_cli.rs +++ b/tests/headless_cli.rs @@ -666,6 +666,40 @@ fn a_headless_date_import_splits_when_told_to() { assert!(doc.contains("2024-01-15"), "the raw dates stay: {doc}"); } +/// improvise-8jk6.3 review: a `--split-date` that names nothing splittable +/// said nothing and exited 0. +#[test] +fn split_date_naming_an_unknown_or_non_date_field_is_refused() { + let scratch = Scratch::new("split_date_unknown"); + for field in ["Nope", "Revenue"] { + let out_path = scratch.path("out.improv"); + let out = run_import( + &dated_csv(&scratch), + &out_path, + &["--extract", "Date:Month", "--split-date", field], + ); + assert!(!out.status.success(), "{field}: {}", stderr_of(&out)); + let err = stderr_of(&out); + assert!(err.contains("--split-date") && err.contains(field), "{err}"); + } +} + +/// improvise-8jk6.3 review: `--axis Date_Month:column` names a category the +/// hierarchical import no longer builds, and was dropped without a word. +#[test] +fn an_axis_naming_a_category_the_import_did_not_build_is_refused() { + let scratch = Scratch::new("axis_unknown"); + let out_path = scratch.path("out.improv"); + let out = run_import( + &dated_csv(&scratch), + &out_path, + &["--extract", "Date:Month", "--axis", "Date_Month:column"], + ); + assert!(!out.status.success(), "{}", stderr_of(&out)); + let err = stderr_of(&out); + assert!(err.contains("Date_Month") && err.contains("--split-date"), "{err}"); +} + /// code-maat's reports: three schemas, a stray note, and room for the output. fn code_maat_folder(scratch: &Scratch) -> PathBuf { let dir = scratch.path("maat"); From 0c56a4b9a1a3ea90d494318254b4a84f2971985b Mon Sep 17 00:00:00 2001 From: "Claude (8jk63-fix)" Date: Fri, 2 Oct 2026 21:33:01 -0700 Subject: [PATCH 8/8] fix(cli): refuse a --split-date or --axis that names nothing it built (improvise-tpm8) --- context/test-evidence/formula-and-io.md | 24 ++++++++++++++++++++++++ src/main.rs | 18 ++++++++++++++++++ tests/headless_cli.rs | 5 ++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index cbd1f4f0..5523b05a 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -1919,3 +1919,27 @@ improvise-d0n6. The same with a raw `2024`, a generated group. Removing the `spelling_clash` call fails it the same way. The control, `a_raw_date_spelled_like_nothing_generated_is_kept_ungrouped`, passes either way, so a blanket refusal of every unparsed value fails it. + +### the_date_step_will_not_advance_over_a_spelling_clash + +improvise-d0n6 (Copilot on PR #322, and the adversarial review's B1). The +spelling-clash refusal sat only in the build, so a wizard with a raw `2024` in +a `%Y-%m-%d` column passed ConfigureDates and failed on NameModel, three steps +from the `s` that fixes it. Before the check joined `validation_error` the +test failed with the step already `DefineFormulas`; removing that call again +reproduces it. It also toggles to the split shape and requires the step to +advance, so a check that blocks forever fails. + +### split_date_naming_an_unknown_or_non_date_field_is_refused + +improvise-tpm8 class. `--split-date Nope` and `--split-date Revenue` (a +measure) exited 0 and did nothing. Both now exit 1 naming the flag and the +field; removing the check in `run_headless_import` fails both rows. + +### an_axis_naming_a_category_the_import_did_not_build_is_refused + +improvise-8jk6.3 review. `--extract Date:Month --axis Date_Month:column` used +to build one `Date` category and drop the `--axis` for the vanished +`Date_Month`, exit 0. It now exits 1 naming the category and `--split-date`. +This also refuses any `--axis` naming an absent category, which was silently +ignored before this PR as well. diff --git a/src/main.rs b/src/main.rs index 89342509..ab94e508 100644 --- a/src/main.rs +++ b/src/main.rs @@ -312,6 +312,14 @@ fn run_headless_import( model_file: Option, ) -> Result<()> { apply_config_to_pipeline(&mut pipeline, config); + if let Some(field) = config.split_dates.iter().find(|f| { + !pipeline + .proposals + .iter() + .any(|p| &p.field == *f && p.kind == import::analyzer::FieldKind::TimeCategory) + }) { + anyhow::bail!("--split-date '{field}' is not a date column of this import"); + } if pipeline.on_duplicate == DuplicatePolicy::Refuse { let collisions = pipeline.collisions(); if !collisions.is_empty() { @@ -322,6 +330,16 @@ fn run_headless_import( for warning in &warnings { eprintln!("warning: {warning}"); } + if let Some((cat, _)) = config + .axes + .iter() + .find(|(cat, _)| wb.model.category(cat).is_none()) + { + anyhow::bail!( + "--axis names '{cat}', which this import did not build \ + (a date's parts are one category unless --split-date names it)" + ); + } apply_axis_overrides(&mut wb, &config.axes); if let Some(path) = output.or(model_file) { diff --git a/tests/headless_cli.rs b/tests/headless_cli.rs index 65ad2e5c..572740a2 100644 --- a/tests/headless_cli.rs +++ b/tests/headless_cli.rs @@ -697,7 +697,10 @@ fn an_axis_naming_a_category_the_import_did_not_build_is_refused() { ); assert!(!out.status.success(), "{}", stderr_of(&out)); let err = stderr_of(&out); - assert!(err.contains("Date_Month") && err.contains("--split-date"), "{err}"); + assert!( + err.contains("Date_Month") && err.contains("--split-date"), + "{err}" + ); } /// code-maat's reports: three schemas, a stray note, and room for the output.