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);