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

feat(import): import a folder of CSVs as one sparse model (improvise-lpzu, improvise-8jk6.2)

Browse Source 
A folder argument expands to its *.csv documents through Storage::list,
and a record with no value for a category keeps its cell without that
coordinate instead of being skipped.

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:
Claude (agent-lpzu)
2026-09-24 20:43:31 -07:00
committed by Edward Langley
parent 1719f2cfad
commit e65d87fda9
4 changed files with 412 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/improvise-io/src/import/csv_parser.rs
+141 -4
View File
@@ -1,10 +1,12 @@
use std::collections::HashSet;
use std::path::Path;
use std::path::{Path, PathBuf};
use anyhow::{Context, Result};
use anyhow::{Context, Result, ensure};
use csv::ReaderBuilder;
use serde_json::Value;
use crate::storage::{PhysicalFs, Storage};
pub fn csv_path_p(path: &Path) -> bool {
path.extension()
.is_some_and(|ext| ext.eq_ignore_ascii_case("csv"))
@@ -145,7 +147,21 @@ fn disambiguate_headers(raw: Vec<String>) -> (Vec<String>, Vec<String>) {
/// Parse multiple CSV files and merge into a single JSON array.
/// Each record gets a "File" field set to the filename stem (e.g., "sales" from "sales.csv").
pub fn merge_csvs(paths: &[impl AsRef<Path>]) -> Result<Vec<Value>> {
let (records, warnings) = merge_csvs_from(&PhysicalFs, paths)?;
for warning in &warnings {
eprintln!("warning: {warning}");
}
Ok(records)
}
/// As [`merge_csvs`] over any [`Storage`], returning each file's warnings
/// prefixed with its path. Files may have different headers (improvise-lpzu).
pub fn merge_csvs_from(
fs: &dyn Storage,
paths: &[impl AsRef<Path>],
) -> Result<(Vec<Value>, Vec<String>)> {
let mut all_records = Vec::new();
let mut all_warnings = Vec::new();
for path in paths {
let path = path.as_ref();
let stem = path
@@ -153,7 +169,10 @@ pub fn merge_csvs(paths: &[impl AsRef<Path>]) -> Result<Vec<Value>> {
.and_then(|s| s.to_str())
.unwrap_or("unknown")
.to_string();
let records = parse_csv(path)?;
let bytes = fs.read(path)?;
let (records, warnings) = parse_csv_bytes_with_warnings(&bytes)
.with_context(|| format!("Failed to parse CSV file: {}", path.display()))?;
all_warnings.extend(warnings.iter().map(|w| format!("{}: {w}", path.display())));
for mut record in records {
if let Value::Object(ref mut map) = record {
map.insert("File".to_string(), Value::String(stem.clone()));
@@ -161,7 +180,40 @@ pub fn merge_csvs(paths: &[impl AsRef<Path>]) -> Result<Vec<Value>> {
all_records.push(record);
}
}
Ok(all_records)
Ok((all_records, all_warnings))
}
/// One `improvise import` argument. Whether a path names a folder is the
/// host's question, so the caller classifies and [`expand_import_paths`] lists.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ImportPath {
File(PathBuf),
Folder(PathBuf),
}
/// The files an import reads: each argument in turn, a folder standing for its
/// `*.csv` documents in name order. See `context/code/formula-and-io.md`.
pub fn expand_import_paths(fs: &dyn Storage, args: &[ImportPath]) -> Result<Vec<PathBuf>> {
let mut paths = Vec::new();
for arg in args {
match arg {
ImportPath::File(path) => paths.push(path.clone()),
ImportPath::Folder(dir) => paths.extend(csv_documents_in(fs, dir)?),
}
}
Ok(paths)
}
/// A folder's `*.csv` documents, sorted by `Storage::list`'s contract. A folder
/// with none is refused by name rather than imported as nothing.
fn csv_documents_in(fs: &dyn Storage, dir: &Path) -> Result<Vec<PathBuf>> {
let csvs: Vec<PathBuf> = fs
.list(dir)?
.into_iter()
.filter(|p| csv_path_p(p))
.collect();
ensure!(!csvs.is_empty(), "no CSV files in folder {}", dir.display());
Ok(csvs)
}
fn parse_csv_field(field: &str) -> Value {
@@ -302,6 +354,91 @@ mod tests {
assert_eq!(records[0]["Name"], Value::String("A".to_string()));
}
// ── Folder arguments (improvise-lpzu) ─────────────────────────────
use crate::storage::MemoryFs;
fn folder(files: &[(&str, &str)]) -> MemoryFs {
let fs = MemoryFs::new();
for (path, body) in files {
fs.write(Path::new(path), body.as_bytes()).unwrap();
}
fs
}
fn names(paths: &[PathBuf]) -> Vec<String> {
paths.iter().map(|p| p.display().to_string()).collect()
}
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_folder_expands_to_its_csv_files_in_name_order_and_skips_the_rest() {
let fs = folder(&[
("maat/coupling.csv", "entity\n"),
("maat/notes.txt", "not data"),
("maat/age.CSV", "entity\n"),
("maat/out.improv", "v2025-04-09\n"),
("maat/abs-churn.csv", "date\n"),
("elsewhere/other.csv", "x\n"),
]);
let paths = expand_import_paths(&fs, &[ImportPath::Folder("maat".into())]).unwrap();
assert_eq!(
names(&paths),
["maat/abs-churn.csv", "maat/age.CSV", "maat/coupling.csv"]
);
}
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_folder_with_no_csv_files_is_refused_by_name() {
let only_text = folder(&[("maat/notes.txt", "x"), ("maat/out.improv", "y")]);
let empty = MemoryFs::new();
for fs in [&only_text, &empty] {
let err = expand_import_paths(fs, &[ImportPath::Folder("maat".into())])
.expect_err("a folder with nothing to import is refused");
assert_eq!(err.to_string(), "no CSV files in folder maat");
}
}
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn files_and_folders_expand_in_argument_order() {
let fs = folder(&[
("b/two.csv", "x\n"),
("b/one.csv", "x\n"),
("a.csv", "x\n"),
("z.csv", "x\n"),
]);
let args = [
ImportPath::File("z.csv".into()),
ImportPath::Folder("b".into()),
ImportPath::File("a.csv".into()),
];
let paths = expand_import_paths(&fs, &args).unwrap();
assert_eq!(names(&paths), ["z.csv", "b/one.csv", "b/two.csv", "a.csv"]);
}
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn files_with_different_headers_merge_without_inventing_the_missing_columns() {
let fs = folder(&[
(
"m/coupling.csv",
"entity,coupled,degree\nsrc/a.rs,src/b.rs,80\n",
),
("m/age.csv", "entity,age-months\nsrc/a.rs,2\n"),
]);
let (records, warnings) = merge_csvs_from(&fs, &["m/coupling.csv", "m/age.csv"]).unwrap();
assert!(warnings.is_empty(), "{warnings:?}");
assert_eq!(
records,
[
serde_json::json!({"entity": "src/a.rs", "coupled": "src/b.rs", "degree": 80, "File": "coupling"}),
serde_json::json!({"entity": "src/a.rs", "age-months": 2, "File": "age"}),
]
);
}
// ── RFC 4180 edge cases ───────────────────────────────────────────
#[test]
crates/improvise-io/src/import/wizard.rs
+169 -127
View File
@@ -257,9 +257,6 @@ impl ImportPipeline {
wb.add_item("_Measure", &m.field)?;
}
// Per-category tally of records dropped because that category had no
// value; a record is charged to the first category it is missing.
let mut skipped_per_category = vec![0usize; categories.len()];
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.
@@ -277,12 +274,6 @@ impl ImportPipeline {
for (at, placement) in placements.into_iter().enumerate() {
match placement {
Placement::NotObject => skipped_non_objects += 1,
Placement::Missing { cat_idx, partial } => {
for (cat, item) in &partial {
wb.add_item(cat, item)?;
}
skipped_per_category[cat_idx] += 1;
}
Placement::Placed {
mut coords,
measures,
@@ -339,12 +330,9 @@ impl ImportPipeline {
}
}
let mut warnings = skip_warnings(
self.records.len(),
categories,
&skipped_per_category,
skipped_non_objects,
);
let mut warnings: Vec<String> = skip_warnings(self.records.len(), skipped_non_objects)
.into_iter()
.collect();
warnings.extend(unparsed_date_warnings(date_extractions, &unparsed_dates));
warnings.extend(collision_warning(&summed));
warnings.extend(occurrence.map(|name| separated_note(name, &collisions)));
@@ -371,11 +359,6 @@ enum Placement {
measures: Vec<(String, f64)>,
unparsed: Vec<usize>,
},
/// No value for category `cat_idx`; `partial` is what preceded it.
Missing {
cat_idx: usize,
partial: Vec<(String, String)>,
},
NotObject,
}
@@ -385,17 +368,15 @@ impl Plan<'_> {
return Placement::NotObject;
};
let mut coords: Vec<(String, String)> = vec![];
// 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<usize> = vec![];
for (cat_idx, cat_proposal) in self.categories.iter().enumerate() {
let Some(v) = map.get(&cat_proposal.field).and_then(scalar_name) else {
return Placement::Missing {
cat_idx,
partial: coords,
};
};
// No value is no coordinate: records are sparse (improvise-8jk6.2).
let named = |field: &str| map.get(field).and_then(scalar_name);
for (cat_proposal, v) in self
.categories
.iter()
.filter_map(|p| Some((p, named(&p.field)?)))
{
coords.push((cat_proposal.field.clone(), v.clone()));
// Extract date components from this field's value
@@ -410,13 +391,13 @@ impl Plan<'_> {
}
}
// Attach label values as coords (missing labels become "").
for lab in &self.labels {
let val = map
.get(&lab.field)
.and_then(scalar_name)
.unwrap_or_default();
coords.push((lab.field.clone(), val));
// Labels likewise: a blank label is no coordinate, never an item "".
for (lab, v) in self
.labels
.iter()
.filter_map(|lab| Some((lab, named(&lab.field)?)))
{
coords.push((lab.field.clone(), v));
}
let measures = self
@@ -538,37 +519,12 @@ fn unparsed_date_warnings(
.collect()
}
/// Human-readable warnings for records that could not be placed in the grid.
/// Empty when every record was imported.
fn skip_warnings(
total: usize,
categories: &[&FieldProposal],
skipped_per_category: &[usize],
skipped_non_objects: usize,
) -> Vec<String> {
let mut warnings = vec![];
let missing_total: usize = skipped_per_category.iter().sum();
if missing_total > 0 {
let details: Vec<String> = categories
.iter()
.zip(skipped_per_category)
.filter(|(_, n)| **n > 0)
.map(|(p, n)| format!("{} ({n})", p.field))
.collect();
warnings.push(format!(
"Skipped {missing_total} of {total} records missing category values: {}",
details.join(", ")
));
}
if skipped_non_objects > 0 {
warnings.push(format!(
"Skipped {skipped_non_objects} of {total} records that were not JSON objects"
));
}
warnings
/// The warning for records that could not be placed in the grid. A record
/// missing a category value is placed, so only a non-object is skipped.
fn skip_warnings(total: usize, skipped_non_objects: usize) -> Option<String> {
(skipped_non_objects > 0).then(|| {
format!("Skipped {skipped_non_objects} of {total} records that were not JSON objects")
})
}
// ── Wizard (UI state wrapped around the pipeline) ─────────────────────────────
@@ -1345,10 +1301,9 @@ mod tests {
);
}
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_row_dropped_for_a_missing_category_is_not_charged_to_the_date_tally() {
// `Date` is walked before `Region`, so a bad date is seen first and the
// record is only afterwards dropped for the missing `Region`.
fn a_row_kept_without_a_category_is_charged_to_the_date_tally() {
let mut p = extracting_month_and_year(json!([
{"Date": "01/15/2025", "Region": "East", "Amount": 100.0},
{"Date": "2025-02-05", "Amount": 200.0},
@@ -1367,23 +1322,23 @@ mod tests {
})
.map(|p| p.field.as_str())
.collect();
assert_eq!(
categories,
vec!["Date", "Region"],
"the fixture only discriminates while Date is walked FIRST"
);
assert_eq!(categories, vec!["Date", "Region"]);
let (_, warnings) = p.build_model_with_warnings().unwrap();
let (wb, warnings) = p.build_model_with_warnings().unwrap();
assert!(
warnings.iter().all(|w| !w.contains("Date_Month")),
"the row never landed, so no derived column lost it: {warnings:?}"
stored(&wb).contains(&"Date=2025-02-05, _Measure=Amount = 200".to_string()),
"the row lands without Region or a derived part: {:?}",
stored(&wb)
);
assert_eq!(
warnings.len(),
1,
"only the skipped-record warning: {warnings:?}"
assert_eq!(warnings.len(), 2, "{warnings:?}");
assert!(
warnings[0].contains("'Date_Month' missing from 1 row(s)"),
"{warnings:?}"
);
assert!(
warnings[1].contains("'Date_Year' missing from 1 row(s)"),
"{warnings:?}"
);
assert!(warnings[0].contains("Region"), "{}", warnings[0]);
}
#[test]
@@ -2005,14 +1960,15 @@ mod tests {
"null must not become an item: {items:?}"
);
assert_eq!(
warnings.len(),
1,
"the dropped record is reported: {warnings:?}"
stored(&wb),
[
"_Measure=revenue, region=East = 100",
"_Measure=revenue = 200"
]
);
assert!(
warnings[0].contains("region"),
"warning names the field: {}",
warnings[0]
warnings.is_empty(),
"the record is kept, not skipped: {warnings:?}"
);
}
@@ -2060,9 +2016,15 @@ mod tests {
.collect();
assert!(!items.contains(&""), "no empty-named item: {items:?}");
assert_eq!(
warnings.len(),
1,
"the dropped record is reported: {warnings:?}"
stored(&wb),
[
"_Measure=revenue, region=East = 100",
"_Measure=revenue = 200"
]
);
assert!(
warnings.is_empty(),
"the record is kept, not skipped: {warnings:?}"
);
}
@@ -2090,55 +2052,135 @@ mod tests {
assert!(warnings.is_empty(), "nothing dropped: {warnings:?}");
}
/// improvise-8jk6.2: a record without the field keeps its cell. It was
/// skipped until records became sparse.
#[test]
fn build_model_record_with_missing_category_value_skipped() {
// If a record is missing a category field, it should be skipped
fn build_model_record_with_missing_category_field_is_kept() {
let raw = json!([
{"region": "East", "revenue": 100.0},
{"revenue": 200.0}, // missing "region"
]);
let p = ImportPipeline::new(raw);
let wb = p.build_model().unwrap();
// Only one cell should exist (the East record)
use crate::model::cell::CellKey;
let k = CellKey::new(vec![
("_Measure".to_string(), "revenue".to_string()),
("region".to_string(), "East".to_string()),
]);
assert!(wb.model.get_cell(&k).is_some());
let wb = ImportPipeline::new(raw).build_model().unwrap();
assert_eq!(
stored(&wb),
[
"_Measure=revenue, region=East = 100",
"_Measure=revenue = 200"
]
);
}
/// improvise-5j9: skipping a record for a missing category value was
/// silent. The warning must name the count, the total and the field(s).
/// improvise-8jk6.2: a blank category value used to drop the whole record.
/// Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn build_model_warns_about_records_skipped_for_missing_category() {
fn a_record_with_no_value_for_a_category_is_kept_without_that_coordinate() {
let raw = json!([
{"region": "East", "revenue": 100.0},
{"revenue": 200.0}, // missing "region"
{"revenue": 300.0}, // missing "region"
{"Payee": "Acme", "Settlement Date": "2026-01-05", "Amount": 10.0},
{"Payee": "Bolt", "Settlement Date": null, "Amount": 20.0},
{"Payee": "Acme", "Settlement Date": "", "Amount": 40.0},
]);
let p = ImportPipeline::new(raw);
let (_, warnings) = p.build_model_with_warnings().unwrap();
let mut p = ImportPipeline::new(raw);
force_category(&mut p, "Settlement Date");
let (wb, warnings) = p.build_model_with_warnings().unwrap();
assert_eq!(
warnings.len(),
1,
"one skipped-record warning: {warnings:?}"
stored(&wb),
[
"Payee=Acme, Settlement Date=2026-01-05, _Measure=Amount = 10",
"Payee=Bolt, _Measure=Amount = 20",
"Payee=Acme, _Measure=Amount = 40",
]
);
assert!(
warnings[0].contains('2'),
"warning counts skipped rows: {}",
warnings[0]
let items = &wb.model.category("Settlement Date").unwrap().items;
assert_eq!(items.keys().collect::<Vec<_>>(), ["2026-01-05"]);
assert!(warnings.is_empty(), "{warnings:?}");
}
/// code-maat's coupling and age reports, as a folder import merges them.
fn code_maat_records() -> Value {
use crate::import::csv_parser::merge_csvs_from;
use crate::storage::{MemoryFs, Storage};
let fs = MemoryFs::new();
let files = [
(
"maat/age.csv",
"entity,age-months\nsrc/a.rs,2\nsrc/c.rs,7\n",
),
(
"maat/coupling.csv",
"entity,coupled,degree,average-revs\nsrc/a.rs,src/b.rs,80,5\nsrc/b.rs,src/c.rs,40,3\n",
),
];
for (path, body) in files {
fs.write(std::path::Path::new(path), body.as_bytes())
.unwrap();
}
let (records, warnings) = merge_csvs_from(&fs, &files.map(|(p, _)| p)).unwrap();
assert!(warnings.is_empty(), "{warnings:?}");
Value::Array(records)
}
/// improvise-lpzu: a column a file lacks is absent from its records, never
/// 0, "" or a skip. Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn files_with_different_headers_import_as_one_sparse_model() {
let p = ImportPipeline::new(code_maat_records());
let kinds: Vec<(&str, FieldKind)> = p
.proposals
.iter()
.map(|f| (f.field.as_str(), f.kind.clone()))
.collect();
for (field, kind) in [
("entity", FieldKind::Category),
("coupled", FieldKind::Category),
("File", FieldKind::Category),
("age-months", FieldKind::Measure),
("degree", FieldKind::Measure),
("average-revs", FieldKind::Measure),
] {
assert!(kinds.contains(&(field, kind.clone())), "{field}: {kinds:?}");
}
let (wb, warnings) = p.build_model_with_warnings().unwrap();
let mut cells = stored(&wb);
cells.sort();
assert_eq!(
cells,
[
"File=age, _Measure=age-months, entity=src/a.rs = 2",
"File=age, _Measure=age-months, entity=src/c.rs = 7",
"File=coupling, _Measure=average-revs, coupled=src/b.rs, entity=src/a.rs = 5",
"File=coupling, _Measure=average-revs, coupled=src/c.rs, entity=src/b.rs = 3",
"File=coupling, _Measure=degree, coupled=src/b.rs, entity=src/a.rs = 80",
"File=coupling, _Measure=degree, coupled=src/c.rs, entity=src/b.rs = 40",
]
);
assert!(
warnings[0].contains('3'),
"warning names the total: {}",
warnings[0]
);
assert!(
warnings[0].contains("region"),
"warning names the missing field: {}",
warnings[0]
for cat in wb.model.regular_category_names() {
let items = &wb.model.category(cat).unwrap().items;
assert!(!items.contains_key(""), "{cat} has an empty item");
}
assert!(warnings.is_empty(), "{warnings:?}");
}
/// improvise-lpzu: the label half of the rule — absent, not an item named
/// "". Evidence: `context/test-evidence/formula-and-io.md`.
#[test]
fn a_label_column_a_record_lacks_is_no_coordinate_not_an_empty_item() {
let mut p = ImportPipeline::new(code_maat_records());
p.proposals
.iter_mut()
.find(|f| f.field == "coupled")
.unwrap()
.kind = FieldKind::Label;
let (wb, _) = p.build_model_with_warnings().unwrap();
let mut cells = stored(&wb);
cells.sort();
assert_eq!(
cells[0],
"File=age, _Measure=age-months, entity=src/a.rs = 2"
);
let items = &wb.model.category("coupled").unwrap().items;
assert!(!items.contains_key(""), "{:?}", items.keys());
}
#[test]