fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

A collision is a cell two records write, and every summed cell is reported (improvise-9bit, improvise-4klp)

Browse Source 
Records with disjoint measures on one tuple no longer count as colliding, so
the Refuse default does not refuse a sparse-measure CSV. Only the records
that share a cell are listed and numbered. The Sum confirmation lists every
summed cell rather than three and a count. Adds the date-derived name test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-24 19:00:29 -07:00
parent 92233cff14
commit f00ed3e979
1 changed files with 139 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/improvise-io/src/import/wizard.rs
+139 -48
View File
@@ -271,10 +271,13 @@ impl ImportPipeline {
// earlier one (improvise-2cw9). The map only deduplicates — gotcha 18.
let mut cells: Vec<PendingCell> = vec![];
let mut cell_index: HashMap<CellKey, usize> = HashMap::new();
// How many records each tuple has seen so far, to number the next one.
let mut seen: HashMap<CellKey, usize> = HashMap::new();
// A colliding record's place in its collision; every other record is 1.
let occurrence_of: HashMap<usize, usize> = collisions
.iter()
.flat_map(|c| c.records.iter().enumerate().map(|(k, r)| (*r, k + 1)))
.collect();
for placement in placements {
for (at, placement) in placements.into_iter().enumerate() {
match placement {
Placement::NotObject => skipped_non_objects += 1,
Placement::Missing { cat_idx, partial } => {
@@ -295,8 +298,7 @@ impl ImportPipeline {
unparsed_dates[at] += 1;
}
if let (Some(name), false) = (occurrence, measures.is_empty()) {
let n = seen.entry(CellKey::new(coords.clone())).or_insert(0);
*n += 1;
let n = occurrence_of.get(&(at + 1)).copied().unwrap_or(1);
coords.push((name.to_string(), n.to_string()));
}
for (measure, val) in measures {
@@ -433,10 +435,10 @@ impl Plan<'_> {
}
}
/// The tuples reached by more than one record that carries a value. A record
/// with no measure makes no cell, so it collides with nothing.
/// The tuples where two records write the same cell (improvise-9bit), with
/// only the records that do. Disjoint measures on one tuple sum nothing.
fn collisions_among(placements: &[Placement]) -> Vec<Collision> {
let mut groups: Vec<Collision> = vec![];
let mut tuples: Vec<(&Vec<(String, String)>, Vec<(usize, Vec<&str>)>)> = vec![];
let mut index: HashMap<CellKey, usize> = HashMap::new();
for (at, placement) in placements.iter().enumerate() {
let Placement::Placed {
@@ -445,23 +447,29 @@ fn collisions_among(placements: &[Placement]) -> Vec<Collision> {
else {
continue;
};
if measures.is_empty() {
continue;
}
let written: Vec<&str> = measures.iter().map(|(m, _)| m.as_str()).collect();
let key = CellKey::new(coords.clone());
match index.get(&key) {
Some(&group) => groups[group].records.push(at + 1),
None => {
index.insert(key, groups.len());
groups.push(Collision {
coords: coords.clone(),
records: vec![at + 1],
});
}
}
let slot = *index.entry(key).or_insert_with(|| {
tuples.push((coords, vec![]));
tuples.len() - 1
});
tuples[slot].1.push((at + 1, written));
}
groups.retain(|g| g.records.len() > 1);
groups
tuples
.into_iter()
.filter_map(|(coords, records)| {
let shared = |m: &&str| records.iter().filter(|(_, w)| w.contains(m)).count() > 1;
let records: Vec<usize> = records
.iter()
.filter(|(_, written)| written.iter().any(shared))
.map(|(record, _)| *record)
.collect();
(!records.is_empty()).then(|| Collision {
coords: coords.clone(),
records,
})
})
.collect()
}
/// Why a build with an unanswered collision stops.
@@ -491,24 +499,13 @@ struct PendingCell {
contributors: usize,
}
/// Repeated coordinate tuples are summed, never dropped, and the user is told
/// which cells that happened to (improvise-2cw9). Empty when none repeated.
fn collision_warning(collisions: &[(String, usize)]) -> Option<String> {
let (first, rest) = collisions.split_at(collisions.len().min(3));
let mut details: Vec<String> = first
/// One warning per summed cell, naming it and its record count, none elided:
/// a message the user needs is never truncated (improvise-cgqt).
fn collision_warning(collisions: &[(String, usize)]) -> Vec<String> {
collisions
.iter()
.map(|(label, n)| format!("{label} ({n} records)"))
.collect();
if !rest.is_empty() {
details.push(format!("and {} more", rest.len()));
}
(!collisions.is_empty()).then(|| {
format!(
"Summed {} cell(s) built from duplicate coordinates: {}",
collisions.len(),
details.join("; ")
)
})
.map(|(label, n)| format!("Summed duplicate records into {label} ({n} records)"))
.collect()
}
/// The item name a JSON scalar contributes, or `None` when it carries no value.
@@ -2278,16 +2275,18 @@ mod tests {
}
}
/// `collision_warning` names at most three cells and counts the rest, so a
/// wide collision cannot render an unreadable status line.
/// improvise-cgqt: every summed cell is its own warning, none elided, so
/// the confirm step can list them all. `context/test-evidence/formula-and-io.md`.
#[test]
fn a_long_collision_list_is_truncated_with_a_count() {
fn every_summed_cell_is_reported_in_full() {
let collisions: Vec<(String, usize)> = (0..5).map(|i| (format!("c={i}"), 2)).collect();
let warning = collision_warning(&collisions).unwrap();
assert!(warning.contains("Summed 5 cell(s)"), "{warning}");
assert!(warning.contains("and 2 more"), "{warning}");
assert!(!warning.contains("c=3"), "{warning}");
assert_eq!(collision_warning(&[]), None);
let warnings = collision_warning(&collisions);
assert_eq!(warnings.len(), 5, "{warnings:?}");
for (i, w) in warnings.iter().enumerate() {
assert!(w.contains(&format!("c={i}")), "{w}");
assert!(w.contains("(2 records)"), "{w}");
}
assert!(collision_warning(&[]).is_empty());
}
// ── Colliding records (improvise-cgqt) ──────────────────────────────
@@ -2630,4 +2629,96 @@ mod tests {
w.scroll(-5);
assert_eq!(w.scroll, 0);
}
/// improvise-9bit: a collision is a CELL two records write. Disjoint
/// measures on one tuple sum nothing. `context/test-evidence/formula-and-io.md`.
#[test]
fn records_with_disjoint_measures_on_one_tuple_do_not_collide() {
let raw = json!([
{"region": "East", "revenue": 10.0},
{"region": "East", "cost": 5.0},
]);
let p = ImportPipeline::new(raw);
assert_eq!(p.collisions(), vec![]);
let (wb, warnings) = p.build_model_with_warnings().unwrap();
assert_eq!(wb.model.data.iter_cells().count(), 2);
assert!(warnings.is_empty(), "{warnings:?}");
}
#[test]
fn a_record_with_no_measure_collides_with_nothing() {
let p = ImportPipeline::new(json!([
{"region": "East", "revenue": 10.0},
{"region": "East"},
{"region": "East", "revenue": 20.0},
{"region": "West"},
{"region": "West"},
]));
let pair = |a: &str, b: &str| (a.to_string(), b.to_string());
assert_eq!(
p.collisions(),
vec![Collision {
coords: vec![pair("region", "East")],
records: vec![1, 3],
}]
);
}
#[test]
fn only_the_records_that_share_a_cell_are_listed_and_numbered() {
let raw = json!([
{"region": "East", "cost": 1.0},
{"region": "East", "revenue": 10.0},
{"region": "East", "revenue": 20.0},
]);
let pair = |a: &str, b: &str| (a.to_string(), b.to_string());
let mut p = ImportPipeline::new(raw);
assert_eq!(
p.collisions(),
vec![Collision {
coords: vec![pair("region", "East")],
records: vec![2, 3],
}]
);
p.on_duplicate = DuplicatePolicy::Separate("Occurrence".into());
let wb = p.build_model().unwrap();
assert_eq!(
stored(&wb),
[
"Occurrence=1, _Measure=cost, region=East = 1",
"Occurrence=1, _Measure=revenue, region=East = 10",
"Occurrence=2, _Measure=revenue, region=East = 20",
]
);
}
/// improvise-4klp: a date part is a category the build creates, so its
/// name is taken. `context/test-evidence/formula-and-io.md`.
#[test]
fn the_name_check_covers_categories_derived_from_dates() {
let mut w = ImportWizard::new(json!([
{"region": "East", "when": "2024-01-15", "revenue": 1.0},
{"region": "East", "when": "2024-01-15", "revenue": 2.0},
]));
w.advance();
assert_eq!(w.step, WizardStep::ConfigureDates, "setup");
w.toggle_date_component();
w.advance();
assert_eq!(w.step, WizardStep::ResolveDuplicates, "setup");
w.move_cursor(1);
w.choose_duplicate();
for _ in 0.."Occurrence".len() {
w.pop_duplicate_name_char();
}
for c in "when_Year".chars() {
w.push_duplicate_name_char(c);
}
w.advance();
assert_eq!(w.step, WizardStep::NameDuplicateCategory);
assert!(
w.message.as_deref().unwrap_or("").contains("when_Year"),
"{:?}",
w.message
);
}
}