diff --git a/README.md b/README.md index 11e6fa19..95094b06 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,14 @@ opening one fails with an error that says so. JSON *import* is unaffected. The import wizard analyzes CSV columns and proposes each as a category (string-valued), measure (numeric), time dimension (date-like, with optional year/month/quarter extraction), or skip. Multiple CSVs merge automatically -with a synthetic "File" category. +with a synthetic "File" category, even when their headers differ. A record +simply has no value for a column its file lacks. A folder argument imports +every `*.csv` in it, in name order, and each merged file also gets its own view, +filtered to that file and showing only its columns: + +```sh +improvise import reports/ --no-wizard -o reports.improv +``` All model mutations go through a typed command registry, so the same operations that back the TUI work headless: diff --git a/context/code/cli.md b/context/code/cli.md index 5b5ab034..c9f6fec2 100644 --- a/context/code/cli.md +++ b/context/code/cli.md @@ -369,3 +369,24 @@ non-zero and writes nothing. It is checked before the build rather than read off the build's error, so the listing is structured data and not a parsed sentence. `separate` uses `ImportPipeline::suggested_duplicate_category`, and there is deliberately no flag to name the category yet. + +## 12 — `import` takes folders, and the CLI only says which arguments are folders (improvise-lpzu) + +`improvise import` takes any mix of files and folders. A folder stands for its +`*.csv` files, in name order, so `import dir/` is `import dir/*.csv`. The rules +and their reasons are in `context/code/formula-and-io.md`, "A folder of CSVs is +one sparse model, with a view per file". The CLI's part is the one question +`Storage` cannot answer: whether a path is a folder. `get_import_data` asks +`Path::is_dir` for each argument, hands the resulting `ImportPath`s to +`expand_import_paths` over `PhysicalFs`, and from there takes the path a list +of files always took. A folder with no CSV in it is refused before anything is +read. The message names the folder, and the run exits non-zero and writes +nothing. + +A merge now reaches the build as an `ImportPipeline` carrying the merged files' +`sources`, rather than as a bare JSON value. Both `--no-wizard` and the TUI +wizard (`run_tui` → `App::start_import`) therefore build the per-file views. +The headless path matters most, because the code-maat pipeline +(improvise-1xfb.5.1) runs there. `a_headless_import_of_a_folder_merges_its_csv_files_and_repeats_byte_for_byte` +pins the views, the sparse Data lines and the byte-identical second run, with +the first run's output sitting in the folder it reads. diff --git a/context/code/formula-and-io.md b/context/code/formula-and-io.md index 2d07d8e6..90c8563f 100644 --- a/context/code/formula-and-io.md +++ b/context/code/formula-and-io.md @@ -632,6 +632,88 @@ one proves nothing. backend rather than only a file (improvise-fuj); the path-taking wrappers remain for the native CLI. +### A folder of CSVs is one sparse model, with a view per file (improvise-lpzu) + +`improvise import ` exists for improvise-1xfb.5.1. code-maat writes one +CSV per analysis into a directory, and the schemas differ: coupling has +`entity,coupled,degree,average-revs`, age has `entity,age-months`, and +abs-churn has `date,added,deleted,commits`. The user ruled (2026-09-25): +"Records are already sparse: the model can just leave null/empty values for +columns not in a given CSV file." So files with different headers merge into +**one** model. It is not refused, and it is not split into one model per schema. + +**A folder argument means exactly its `*.csv` files.** `expand_import_paths` +takes the arguments in order and replaces each folder with its `*.csv` +entries. The extension check is case-insensitive, the listing is not recursive, +and the order is `Storage::list`'s, which is sorted by contract. Then the +existing path runs unchanged: two or more files merge through `merge_csvs_from` +with a `File` category, and one file imports alone. So `import dir/` is +`import dir/*.csv`, including when the folder holds a single CSV. Four cases +follow from that equivalence rather than being chosen one by one: + +- **Non-CSV entries are skipped**, not refused, because a glob skips them. This + includes the output `.improv`, which a second run finds in the folder. +- **A folder with no CSV at all is refused**, naming the folder. A glob that + matches nothing is an error too, and importing nothing would write an empty + document. +- **Files and folders mix freely**, in argument order. +- **A file named twice is read twice.** For example, `dir/a.csv dir/` reads + `a.csv` twice, and its records collide with themselves. The headless path + refuses that like any other collision. Nothing deduplicates, because the + shell glob would not either. + +The listing goes through `Storage::list` so the browser host can reuse it +(gotcha 2). `Storage` knows documents, not folders, so **whether an argument +is a folder is the caller's question**. The CLI answers it at the edge with +`Path::is_dir` and passes an `ImportPath::{File, Folder}`. An OPFS host would +answer it from its own UI. + +**Sparse merge.** A record from a file without some column has no key for it, +so `Plan::place` gives it no coordinate there (improvise-8jk6.2 above), and no +measure cell. Nothing is fabricated: no `0` and no `""`. The analyzer already +classified each column over the values present (improvise-l1f5), so a column +only one file has is judged by that file's values alone. The category cap +counts the **union** of every file's categories. Rows from different files +cannot collide, because `File` differs. + +**Each file gets a view** (the user, 2026-09-25: "dir import should do this: +one view filtered by the file column and projected to the columns in that +file"). `merge_csvs_from` returns a `Merged` whose `sources` list each `File` +item with the columns its header named. Two files with one stem share an item, +so their columns are unioned. The pipeline carries `sources`, and the build +adds one view per source after the merged `Default` view, which stays the +initial view. Each per-file view: + +- **Is named after the file's stem.** A name already taken, including the + virtual `_Records` and `_Drill`, gets ` 2`, ` 3` and so on. A view named + `_Drill` would never be written and would be dropped on load. +- **Puts `File` on `Axis::Filter`, fixed at that file's item.** Filter and + not Page, because the view *is* that file's projection. Paging to another + file would show that file's records through this file's columns and hidden + measures, which is wrong on every page but one. Filter constrains without + being navigable (gotcha 27). Until now it appeared only in the never-saved + `_Drill` view, but the `.improv` grammar has always round-tripped it. + `per_file_views_survive_a_round_trip_and_repeat_byte_for_byte` proves that on + a saved per-file view, and `a_saved_per_file_view_projects_only_its_file` + proves the shared `grid_in` projection, which both the TUI and the browser + draw from, carries it as a fixed coordinate with nothing to cycle. +- **Seats the file's own categories** (its columns, plus date parts derived + from them) through `View::on_category_added`, in model order: the rule a + single-file import uses. Every other category goes on `None`. The file's + records lack those, so nothing splits. +- **Puts `_Measure` on Page**, as `Workbook::new` does, **with other files' + measures hidden** and the page selection on the file's first measure. A + formula target comes from no file, so it stays visible. + +A per-file view is made only while `File` is an accepted category. A user who +skipped or reclassified it has no filter to build views from. + +Two things this does not do. Page cycling still steps onto a hidden item and +draws an empty grid: `[`/`]` in a per-file view can reach another file's +measure (improvise-yd2t; the Default view already behaved this way). And a +CSV with its own `File` column has it overwritten by the merge's, which is the +existing behaviour of `merge_csvs`. + ### The wizard's back path is a history stack `advance` pushes the step it leaves; `previous` pops it. Keeping the path @@ -658,10 +740,33 @@ and a blank string are **absences, not names**; stringifying them used to invent an item literally called "null" (improvise-ych). Every other value keeps its printed form. -A record missing a value for an accepted Category field has no place in the grid -and is dropped; `build_model_with_warnings` counts it against the *first* -category it is missing and reports the total, because without a warning that -loss is invisible in a thousand-row import (improvise-5j9). +**A record with no value for a category keeps its cell, without that +coordinate** (improvise-8jk6.2). Records are sparse: a `CellKey` names only the +coordinates it has. So a missing field, a `null` and a blank string all mean +"no coordinate here". None of them makes an item: not `"null"`, not `""`, and +no synthetic placeholder. The same rule covers labels, which used to become an +item named `""`. The user's words, 2026-09-25: "this limit in import should be +fixed, it dates to when this wasn't sparse." That limit was improvise-5j9's +rule: such a record "has no place in the grid" and was dropped, counted and +reported as `Skipped N of M records missing category values`. 5j9's warning +made the loss visible, and the loss is now gone, so the warning went with it. +The only record still skipped is one that is not a JSON object. + +Where such a record shows up is decided by the axes, not by the import. The +test is `a_record_with_no_coordinate_is_drawn_only_where_that_category_is_off_the_grid`: + +- **The category on Row or Column:** no position holds the record. Its other + coordinates may still draw a row, but the cell under the category's items is + empty, and the column total leaves it out. +- **The category on Page:** no selection shows it. +- **The category on None:** it is aggregated like any other record. +- **Records mode:** its row shows a blank cell in that category's column. +- **`.improv`:** its Data line omits the category, and it round-trips. +- **Drill:** a drill that does not constrain the category reaches it. + +Whether Row/Column/Page should give such records a place of their own (a +"(none)" position) is **open, and it is the user's call**. It is raised on the +lpzu PR. Nothing here invents one. ### Colliding records are a question, not a default (improvise-cgqt) @@ -678,7 +783,9 @@ made it choose too. It chose `Refuse` and points at `:import`. **Detection comes before the build and shares the build's placement.** Where a record lands is `Plan::place`: its categories, then its date parts, then its -labels, then its measure values, or a reason it is dropped. `collisions()` and +labels, then its measure values. Only a record that is not an object is +dropped, since a missing value is no coordinate rather than a reason to drop +(improvise-8jk6.2). `collisions()` and `build_model_with_warnings` both call it, so the question the wizard asks and the thing the build does cannot disagree. Neither has a second walk of the record. **A collision is a cell that more than one record writes**, meaning a diff --git a/context/test-evidence/cli.md b/context/test-evidence/cli.md index be0c036f..424792e3 100644 --- a/context/test-evidence/cli.md +++ b/context/test-evidence/cli.md @@ -407,3 +407,41 @@ named, and no output file. It has two siblings. `..._sums_duplicates_when_told_t asserts `= 30` and no `Occurrence`. `..._keeps_duplicates_separate_when_told_to` asserts the category, `= 10` and `= 20`, and no `= 30`. So the two flags cannot both be summing. + +### a_headless_import_of_a_folder_merges_its_csv_files_and_repeats_byte_for_byte + +improvise-lpzu, at the process boundary. The folder holds code-maat's three +schemas (coupling, age and abs-churn) and a `README.txt`. At `e65d87fd`'s +parent the run failed with `Cannot read '…/maat': Is a directory (os error +21)`, because a single path went straight to `read_to_string`. The test +asserts: + +- sparse Data lines for all three schemas, with no README; +- the four views, `Default` first and initial, then one per file; +- `File: filter, coupling`; +- a second run, with the first run's `maat.improv` now inside the folder, + writing the same bytes. + +Rivals R2, R5, R6 and R7 each fail it. + +### a_headless_import_takes_files_and_folders_together_and_refuses_an_empty_folder + +improvise-lpzu. A file argument and a folder argument together are one merge, +and the extra file's records carry its own `File` item. Before the fix this +failed with `Multi-file import only supports CSV files`, because the folder was +not a `.csv` path. An empty folder exits non-zero, writes nothing, and names +itself: `no CSV files in folder …/empty`. Rivals R2, R4 and R5 each fail it. + +### a_saved_per_file_view_projects_only_its_file + +improvise-lpzu (`tests/folder_import_views.rs`). A folder import is saved, +reloaded and projected through `project::grid_in`, the one derivation the TUI +grid and the browser both draw from (gotcha 44). This is why no separate TUI +or browser test was written. For each file's view: + +- the page bar carries `File` fixed at that file with nothing to cycle + (`items: None`, gotcha 27); +- the drawn cells are that file's alone: `2.00, 7.00` for age and + `5.00, 3.00` for coupling. + +Rivals R2, R7 and R9 each fail it. diff --git a/context/test-evidence/formula-and-io.md b/context/test-evidence/formula-and-io.md index 3ef8badc..71710e03 100644 --- a/context/test-evidence/formula-and-io.md +++ b/context/test-evidence/formula-and-io.md @@ -1026,6 +1026,14 @@ every blank column value in a CSV import grew a phantom "null" item. Null means *absent*: the record must be skipped and counted, exactly as if the key were missing. +**The skip is superseded by improvise-8jk6.2.** Null still means absent, and +still invents no item. But an absent coordinate no longer drops the record: it +keeps its cell without that coordinate. So the test now asserts both cells +(`region=East` and the bare `_Measure=revenue`) and no warning. The rival that +restores the skip (a present-but-blank category returns the record as +unplaceable) fails here, beside +`a_record_with_no_value_for_a_category_is_kept_without_that_coordinate`. + ### build_model_literal_string_null_is_a_real_category_item The other side of improvise-ych: the *string* `"null"` is an ordinary value — a @@ -1038,7 +1046,9 @@ null" would pass the test above. improvise-ych, third arm. An empty string is the JSON spelling of a blank cell — the CSV parser already turns a blank field into `Value::Null`, so the two inputs must agree. An item with an empty name is unnameable in the grid, so it is -treated as missing rather than created. +treated as missing rather than created. Since improvise-8jk6.2 "missing" +means the record keeps its cell without a `region` coordinate. It is no longer +skipped, and the same skip-restoring rival fails here. ### build_model_sums_records_that_share_every_coordinate @@ -1107,12 +1117,19 @@ and scrolls them. It was red at compile time against the old `Option` signature, then red against a `Vec` that still held one elided sentence. The rival **elided after three** fails it. -### build_model_warns_about_records_skipped_for_missing_category +### improvise-5j9's skip warning, retired by improvise-8jk6.2 improvise-5j9: skipping a record for a missing category value used to be silent, -so a 1000-row import could drop rows with no user-visible sign. The warning must -name the count, the total, and the offending field(s) — a bare "some records were -skipped" is not actionable. +so a 1000-row import could drop rows with no user-visible sign. The warning had +to name the count, the total, and the offending field(s); a bare "some records +were skipped" is not actionable. Its test, +`build_model_warns_about_records_skipped_for_missing_category`, was **deleted** +by improvise-8jk6.2. A record missing a category value is now kept without that +coordinate, so there is no skip left to report. Its successor is +`a_record_with_no_value_for_a_category_is_kept_without_that_coordinate`, which +asserts no warning at all. `build_model_record_with_missing_category_value_skipped` +became `build_model_record_with_missing_category_field_is_kept` and now asserts +the kept cell rather than only the East one. ### is_does_not_swallow_the_start_of_a_longer_word @@ -1436,7 +1453,16 @@ The fix reports and does not relocate: the row still lands with the coordinates it does have, exactly as before. Changing where such a row is placed is a behaviour change the ticket did not ask for. -### import::wizard::tests::a_row_dropped_for_a_missing_category_is_not_charged_to_the_date_tally +### import::wizard::tests::a_row_kept_without_a_category_is_charged_to_the_date_tally + +**Renamed from `a_row_dropped_for_a_missing_category_is_not_charged_to_the_date_tally` +by improvise-8jk6.2.** Its premise went with the skip. A row missing `Region` +now lands, so its unparseable `Date` *is* a derived column's loss, and it is +charged once to each of `Date_Month` and `Date_Year`. The test asserts the +landed cell (`Date=2025-02-05, _Measure=Amount = 200`, with no `Region` and no +derived part) and exactly those two warnings. The history below is kept +because the rule it established (charge only landed rows) still holds. Every +object row now lands. improvise-yss5, found by ChatGPT Codex reviewing PR #150 at `af7fae0e` and correct. The first fix incremented `unparsed_dates` while walking a record's @@ -1617,3 +1643,127 @@ that name, which must be refused. It was green on arrival, so its evidence is the reviewer's rival, **date-derived names dropped from the taken names**, which fails it. That fault would have let `add_category` reuse the existing category, and every cell key would then name `when_Year` twice. + +### a_record_with_no_value_for_a_category_is_kept_without_that_coordinate + +improvise-8jk6.2, reported by the user from the wizard: "Skipped 6 of 342 +records missing category values: Settlement Date (6)". `Plan::place` returned +`Placement::Missing` for a category with no value, so the build dropped the +whole record, measures and all. The fixture has one complete record, one with +`"Settlement Date": null` and one with `""`, for two different payees. At +`e65d87fd`'s parent the build stored only the first cell. Now the stored cells +are exactly the three records, the two blanks with no date coordinate, and +there is no warning. `Settlement Date` holds only its one real item, so neither +`"null"` nor `""` became one. Rival R1, a record with a present-but-blank +category treated as unplaceable, fails this test and the two improvise-ych +tests. + +### files_with_different_headers_import_as_one_sparse_model + +improvise-lpzu. code-maat's `age.csv` (`entity,age-months`) and `coupling.csv` +(`entity,coupled,degree,average-revs`) merged through `merge_csvs_from`. Before +the fix, the age records had no `coupled` key, so they took the same `Missing` +path and **every age record was dropped**. The build stored the four coupling +cells and nothing else. The test pins the proposals (each column's kind, +judged over its present values), all six stored cells sorted, no empty-named +item in any category, and no warning. Rival R2, a record whose file lacks a +category column skipped, fails it along with the two CLI folder tests, the +projection test and the per-file view test. + +### a_label_column_a_record_lacks_is_no_coordinate_not_an_empty_item + +improvise-lpzu, the label half. Labels were attached with +`scalar_name(..).unwrap_or_default()`, so an age record, whose file has no +`coupled`, stored `coupled=` as a coordinate, and `coupled` grew an item named +`""`. Before the fix the first sorted cell was +`File=age, _Measure=age-months, coupled=, entity=src/a.rs = 2`. Rival R3 +(`unwrap_or_default` restored) fails it. + +### a_folder_expands_to_its_csv_files_in_name_order_and_skips_the_rest + +improvise-lpzu. The folder holds three CSVs written out of name order, one +with an upper-case `.CSV`, plus a `.txt`, an `.improv` (the output a second run +finds) and a CSV in *another* folder. Only the three are returned, sorted. +`MemoryFs` stores in a `HashMap`, so the order comes from `Storage::list`'s +contract and not from insertion. Rival R5 (the extension filter dropped) and +rival R6 (the listing passed through a `HashSet`) each fail it. + +### a_folder_with_no_csv_files_is_refused_by_name + +improvise-lpzu. Both an empty folder and a folder holding only non-CSV files +must refuse with `no CSV files in folder maat`. Importing nothing would write +an empty document. Rival R4 (the `ensure!` removed) fails it, and so does R5, +because the text-only folder then "has" files. + +### files_and_folders_expand_in_argument_order + +improvise-lpzu. `[z.csv, b/, a.csv]` must expand to +`[z.csv, b/one.csv, b/two.csv, a.csv]`. Argument order is kept, and a folder +is sorted within itself. A fixture listing only folders, or files in name +order, could not tell "argument order" from "sort everything". R6 fails it. + +### files_with_different_headers_merge_without_inventing_the_missing_columns + +improvise-lpzu, at the parser. Each merged record carries exactly its own +file's keys plus `File`: no key, `null` or `0` for a column another file has. +Each `SourceFile` lists its file's columns in header order. The per-file views +are built from those lists. + +### a_record_with_no_coordinate_is_drawn_only_where_that_category_is_off_the_grid + +improvise-8jk6.2's downstream question, asked of the real layout rather than +argued. Bolt has no `Date`: + +- With `Date` on Column or on Page, the grid draws only Acme's `10`. +- With `Date` on None, it draws both `10` and `20`. +- In records mode, Bolt's row reads `["", "Bolt", "", "20", ""]`, a blank + `Date` cell. +- The Data line is `Payee=Bolt, _Measure=Amount = 20`, and it round-trips. +- A drill on `Payee=Bolt` reaches the record. + +This is a characterization of the model's existing sparse semantics. It found +no bug. Its purpose is to make the Row/Column answer ("invisible") a stated +fact the user can overrule, rather than an accident. R2 fails it. + +### a_merge_gives_each_file_a_view_projected_to_its_columns + +improvise-lpzu, the user's "one view filtered by the file column and +projected to the columns in that file". The test asserts: + +- the view order (`Default`, then the files in read order), with `Default` + still active; +- `File` on Filter at the file's item; +- the file's own categories seated, and the other file's on None; +- `_Measure` on Page at the file's first measure, with the other file's + measures hidden; +- the drawn values. + +The age view draws `[2, 7]`. The coupling view draws `[5, 3]`, because its +first measure in model order is `average-revs`: serde_json's map orders keys +alphabetically. Failed at `bb505f8f`'s parent, where no view existed. Rivals +R7 (File on None), R9 (every category seated) and R10 (nothing hidden) each +fail it. + +### a_files_view_holds_only_that_files_records + +improvise-lpzu. The code-maat fixture cannot show that the *filter* works, +because its files share no measure, so the measure page alone would isolate +them. Two quarterly files with the same `Region,Revenue` schema can: without +the filter, `q1` would draw `30` and `3`. Rival R7 fails it. R9 and R10 do +not, and are not meant to. + +### a_files_view_name_skips_names_already_held + +improvise-lpzu. Two stems collide with names already held: `Default`, the +merged view, and `_Drill`, a virtual view that is never written and is dropped +on load. The first becomes `Default 2` and the second `_Drill 2`, and each +still filters to its own item. Rival R8 (the virtual names not checked) fails +it: the file's view would have been saved as `_Drill` and lost. + +### per_file_views_survive_a_round_trip_and_repeat_byte_for_byte + +improvise-lpzu. Two builds from two independently written `MemoryFs` give +identical `.improv` text, and `format(parse(text)) == text`. The reloaded +document still has the three views in order, `File: filter` at the file's item, +and the same drawn values. This is the first persisted, non-virtual use of +`Axis::Filter` (gotcha 27). R7 and R9 fail it.