Merge commit 'c91cc055c8ad81afd8f055b71c74af6819020637' into chore/1xfb-5-1-repo-stats
This commit is contained in:
Claude (agent-stats)
@@ -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, even when it holds a single CSV. Each 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:
|
||||
|
||||
@@ -369,3 +369,26 @@ 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. A folder always merges, even when it holds a
|
||||
single CSV, so that file keeps its `File` item and view. A single file named
|
||||
directly still imports alone (the user's ruling on the PR). 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.
|
||||
|
||||
@@ -632,6 +632,144 @@ 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 <folder>` 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, with one difference: **a folder always merges**. Two or more
|
||||
files merge through `merge_csvs_from` with a `File` category, and so does a
|
||||
folder holding a single CSV. That gives the one file its `File` item and its
|
||||
per-file view.
|
||||
|
||||
The single-CSV rule is **the user's ruling (2026-09-25)**, which replaced this
|
||||
PR's first answer, "`import dir/` is exactly `import dir/*.csv`". Their words:
|
||||
"yes, we might want to have some kind of update import eventually (don't plan
|
||||
this yet, but we'd want file identity for this)". A folder names a collection
|
||||
that may grow, so a document built from it should record which file each
|
||||
record came from from the start.
|
||||
|
||||
**A single CSV named directly, not through a folder, still imports alone,
|
||||
without `File`.** That is deliberate. Naming one file asks for that file, not
|
||||
for a collection, and the shipped recipes (`examples/demo.improv` from
|
||||
`examples/demo.csv`) and every existing single-file script would otherwise gain
|
||||
a `File` category and a second view. File identity for update-import can be
|
||||
asked for by naming a folder. The test is
|
||||
`a_folder_of_one_csv_still_gets_a_file_category_and_its_view_but_a_lone_file_does_not`.
|
||||
|
||||
Apart from that, a folder still behaves as the glob would, and four cases follow
|
||||
from that 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, header-only files included
|
||||
(see the next paragraph). Rows from different files cannot collide, because
|
||||
`File` differs. One caveat on "judged by its values": a JSON `""` is not yet
|
||||
dropped by the analyzer the way `null` is, so a JSON numeric column holding
|
||||
`""` is proposed as a Label (improvise-8jk6.11). CSV is unaffected, because
|
||||
its parser turns a blank field into `null`.
|
||||
|
||||
**A header-only file is still a file** (the Codex P2 on PR #314). code-maat
|
||||
writes a header and no rows whenever nothing passes its threshold, so
|
||||
`coupling.csv` is often exactly that. The pipeline is analyzed from *records*,
|
||||
and such a file has none, so it had no `File` item, its columns never reached
|
||||
the model, and its view filtered to an item that did not exist.
|
||||
`ImportPipeline::from_merge` therefore **seeds from `Merged::sources`**:
|
||||
|
||||
- the `File` proposal's items are the sources' items, in file order, so every
|
||||
file has its item whether it has rows or not;
|
||||
- a source column that no record has a value in is proposed as a **Label**.
|
||||
That is the analyzer's own verdict for an all-null column (improvise-l1f5),
|
||||
and a column with no values is exactly that: nothing can be classified, so it
|
||||
takes no pivot slot and does not count toward the category cap until the user
|
||||
reclassifies it in the wizard or with `--category`/`--measure`.
|
||||
|
||||
This is also the file identity the user wants kept for a future update-import.
|
||||
|
||||
**A file with no measure column gets a records view** (improvise-8jk6.13). A
|
||||
record with no measure makes no cell, so such a file contributes items and no
|
||||
data. Its view had nothing to page `_Measure` to, and paged onto another file's
|
||||
measure, which the same view hides; the cell there was a `Record`, editable,
|
||||
under this file's `File`. Now `source_view` gives such a file the records
|
||||
layout (`view::records_steps`, the one home of what a records view is) with
|
||||
`File` still on Filter at the file's item. It draws zero rows, because there
|
||||
are no cells. A header-only file whose columns are
|
||||
all Labels is the same case. In records mode the columns are the whole model's
|
||||
categories and measures, not just this file's: records mode has no per-view
|
||||
column filter. With no rows that is cosmetic.
|
||||
|
||||
**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 file with no measure
|
||||
gets a records view instead (above).
|
||||
- **Is never named like another view from the same import.** `Default.csv` and
|
||||
`Default 2.csv` become `Default 2` and `Default 2 2`; without the same-batch
|
||||
check both would be named `Default 2` and one view would silently replace the
|
||||
other.
|
||||
|
||||
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 +796,35 @@ 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.
|
||||
|
||||
**No "(none)" position, by the user's decision (2026-09-25).** They were asked
|
||||
whether Row, Column or Page should give such records a place of their own. Their
|
||||
answer: "I think leaving it out for now is fine, maybe a UI element somewhere
|
||||
that shows the number of records underneath the current view?" That indicator
|
||||
is improvise-gtn7.10, and it is not an import concern.
|
||||
|
||||
### Colliding records are a question, not a default (improvise-cgqt)
|
||||
|
||||
@@ -678,7 +841,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
|
||||
|
||||
@@ -407,3 +407,54 @@ 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. Since the review of `4e68372c` the fixture
|
||||
also holds a header-only `soc.csv`. Its view must carry `File` at `soc`, be a
|
||||
records view, and draw zero rows. Rivals F7 and F8 fail that half.
|
||||
|
||||
### a_folder_of_one_csv_still_gets_a_file_category_and_its_view_but_a_lone_file_does_not
|
||||
|
||||
improvise-lpzu, after the user's ruling on PR #314 ("we'd want file identity").
|
||||
At `4e68372c`, a folder holding one CSV took the single-file path, because
|
||||
`import dir/` was exactly `dir/*.csv`. That document had no `File` category and
|
||||
no per-file view, and the test failed at the `## Category: File` assertion. The
|
||||
other half pins the direction the ruling did *not* change: the same CSV named
|
||||
directly imports without any `File`. The rival that merges every all-CSV
|
||||
import, lone files included, fails that half ("a lone file keeps today's
|
||||
shape"). So the test separates "a folder merges" from "everything merges".
|
||||
|
||||
@@ -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<String>`
|
||||
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,184 @@ 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.
|
||||
|
||||
### two_files_with_one_stem_union_their_columns
|
||||
|
||||
improvise-8jk6.12 (3). `a/x.csv` (`entity,one`) and `b/x.csv` (`entity,two`) are
|
||||
one `File` item, `x`, whose columns are `entity, one, two`. Rival F3 (the
|
||||
reviewer's fault: clear the stem's columns before each file) leaves
|
||||
`entity, two`, and the `x` view would then hide the measure `one` that half its
|
||||
records carry.
|
||||
|
||||
### 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.
|
||||
|
||||
### a_header_only_file_keeps_its_file_item_its_columns_and_its_view
|
||||
|
||||
The ChatGPT Codex P2 on PR #314 at `4e68372c`, confirmed by the adversarial
|
||||
review. `age.csv` (one row), a header-only `coupling.csv` and `rev.csv` (one
|
||||
row). At `005b7a76` the test failed at the first assertion: `File` held only
|
||||
`age` and `rev`, so the `coupling` view filtered to an item that did not exist,
|
||||
and `coupled`, `degree` and `average-revs` were not in the model. Now `File`
|
||||
reads `age, coupling, rev` in file order, the three columns exist (as Labels),
|
||||
and the `coupling` view is a zero-row records view filtered to `coupling`, while
|
||||
the `age` view still draws `2`. Rival F5 (sources not seeded) and F6 (unseen
|
||||
columns not proposed) each fail it, and so do F7 and F8 below.
|
||||
|
||||
### a_file_with_no_measure_gets_a_records_view_not_another_files_measure
|
||||
|
||||
improvise-8jk6.13, from the review of PR #314. `a.csv` (`entity,x`) and
|
||||
`tags.csv` (`entity,tag`). At `005b7a76` the `tags` view had `_Measure` on Page
|
||||
with no selection, so the layout paged onto `x`, a measure the same view hides.
|
||||
The test failed there, with `row_count() == 1`: `a`'s record, drawn under
|
||||
`File=tags`'s view. Now the view has no `_Measure` page, is a records layout,
|
||||
and draws zero rows, while `a`'s view is still a pivot. Rival F7 (the old
|
||||
paging) fails it, and F8 (the File selection lost when `records_steps` moves
|
||||
`File` off Filter) fails it with the same `1`. F8 is the mistake the first
|
||||
version of the fix made.
|
||||
|
||||
### a_files_view_seats_the_date_parts_of_its_own_date_column
|
||||
|
||||
improvise-8jk6.12 (1). The reviewer replaced the date-parts clause of
|
||||
`source_view`'s `own` with `false` and every test passed, because no folder
|
||||
fixture extracted a date part. Here `abs-churn.csv` has a `date` column,
|
||||
extracted to `date_Month`, and `age.csv` does not. `date_Month` is seated in the
|
||||
`abs-churn` view and on None in the `age` view. Rival F1 (the reviewer's fault)
|
||||
puts it on None in both and fails the first assertion.
|
||||
|
||||
### two_files_whose_names_collide_in_one_batch_keep_a_view_each
|
||||
|
||||
improvise-8jk6.12 (2). `Default.csv` and `Default 2.csv`. The first takes
|
||||
`Default 2`, so the second must take `Default 2 2`. Without the same-batch
|
||||
check both are named `Default 2`, and `IndexMap::insert` silently replaces the
|
||||
first, leaving two views where three are owed. The test asserts all three names
|
||||
with their filters. Rival F2 (the reviewer's fault) fails it.
|
||||
|
||||
### a_whitespace_only_category_value_is_no_coordinate
|
||||
|
||||
improvise-8jk6.12 (4). A `region` of `" "` is kept as a record with no
|
||||
`region` coordinate, and `region` holds only `East`. The trim in `scalar_name`
|
||||
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.
|
||||
|
||||
@@ -1224,7 +1224,12 @@ impl App {
|
||||
}
|
||||
|
||||
pub fn start_import_wizard(&mut self, json: serde_json::Value) {
|
||||
self.wizard = Some(ImportWizard::new(json));
|
||||
self.start_import(crate::import::wizard::ImportPipeline::new(json));
|
||||
}
|
||||
|
||||
/// Open the wizard over a pipeline the host built, sources and all.
|
||||
pub fn start_import(&mut self, pipeline: crate::import::wizard::ImportPipeline) {
|
||||
self.wizard = Some(ImportWizard::from_pipeline(pipeline));
|
||||
self.view_state.mode = AppMode::ImportWizard;
|
||||
}
|
||||
|
||||
|
||||
@@ -1315,6 +1315,7 @@ impl Effect for ImportJsonHeadless {
|
||||
formulas: vec![],
|
||||
// Headless: nobody to ask, so a collision refuses (improvise-cgqt).
|
||||
on_duplicate: DuplicatePolicy::Refuse,
|
||||
sources: vec![],
|
||||
};
|
||||
|
||||
match pipeline.build_model_with_warnings() {
|
||||
|
||||
@@ -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"))
|
||||
@@ -31,6 +33,12 @@ pub fn parse_csv_with_warnings(path: &Path) -> Result<(Vec<Value>, Vec<String>)>
|
||||
/// As [`parse_csv_with_warnings`] but over bytes, so import can come from any
|
||||
/// [`Storage`](crate::storage::Storage) backend, not only a file (improvise-fuj).
|
||||
pub fn parse_csv_bytes_with_warnings(bytes: &[u8]) -> Result<(Vec<Value>, Vec<String>)> {
|
||||
let (_, records, warnings) = parse_csv_bytes(bytes)?;
|
||||
Ok((records, warnings))
|
||||
}
|
||||
|
||||
/// The header (after renaming), the records and the warnings.
|
||||
fn parse_csv_bytes(bytes: &[u8]) -> Result<(Vec<String>, Vec<Value>, Vec<String>)> {
|
||||
let mut reader = ReaderBuilder::new()
|
||||
.has_headers(true)
|
||||
.flexible(true)
|
||||
@@ -110,7 +118,7 @@ pub fn parse_csv_bytes_with_warnings(bytes: &[u8]) -> Result<(Vec<Value>, Vec<St
|
||||
}),
|
||||
);
|
||||
|
||||
Ok((records, warnings))
|
||||
Ok((headers, records, warnings))
|
||||
}
|
||||
|
||||
/// A header name keys the record object, so two equal names were one column and
|
||||
@@ -145,7 +153,41 @@ 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 mut all_records = Vec::new();
|
||||
let merged = merge_csvs_from(&PhysicalFs, paths)?;
|
||||
for warning in &merged.warnings {
|
||||
eprintln!("warning: {warning}");
|
||||
}
|
||||
Ok(merged.records)
|
||||
}
|
||||
|
||||
/// The category every merged record names its source file by.
|
||||
pub const FILE_CATEGORY: &str = "File";
|
||||
|
||||
/// One merged file: its `File` item and the columns its header named.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SourceFile {
|
||||
pub item: String,
|
||||
pub columns: Vec<String>,
|
||||
}
|
||||
|
||||
/// Several CSV files as one record set, with each file's own columns kept.
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub struct Merged {
|
||||
pub records: Vec<Value>,
|
||||
/// One per distinct `File` item, in the order the files were read.
|
||||
pub sources: Vec<SourceFile>,
|
||||
/// Each file's parse warnings, prefixed with its path.
|
||||
pub warnings: Vec<String>,
|
||||
}
|
||||
|
||||
/// As [`merge_csvs`] over any [`Storage`]. Files may have different headers,
|
||||
/// and two files with one stem share its item and its columns (improvise-lpzu).
|
||||
pub fn merge_csvs_from(fs: &dyn Storage, paths: &[impl AsRef<Path>]) -> Result<Merged> {
|
||||
let mut merged = Merged {
|
||||
records: Vec::new(),
|
||||
sources: Vec::new(),
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
for path in paths {
|
||||
let path = path.as_ref();
|
||||
let stem = path
|
||||
@@ -153,15 +195,75 @@ 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 (headers, records, warnings) = parse_csv_bytes(&bytes)
|
||||
.with_context(|| format!("Failed to parse CSV file: {}", path.display()))?;
|
||||
let at = merged.source_for(&stem);
|
||||
let columns = &mut merged.sources[at].columns;
|
||||
for header in headers {
|
||||
if header != FILE_CATEGORY && !columns.contains(&header) {
|
||||
columns.push(header);
|
||||
}
|
||||
}
|
||||
merged
|
||||
.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()));
|
||||
map.insert(FILE_CATEGORY.to_string(), Value::String(stem.clone()));
|
||||
}
|
||||
all_records.push(record);
|
||||
merged.records.push(record);
|
||||
}
|
||||
}
|
||||
Ok(all_records)
|
||||
Ok(merged)
|
||||
}
|
||||
|
||||
impl Merged {
|
||||
fn source_for(&mut self, item: &str) -> usize {
|
||||
match self.sources.iter().position(|s| s.item == item) {
|
||||
Some(at) => at,
|
||||
None => {
|
||||
self.sources.push(SourceFile {
|
||||
item: item.to_string(),
|
||||
columns: vec![],
|
||||
});
|
||||
self.sources.len() - 1
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// 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 +404,118 @@ 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 merged = merge_csvs_from(&fs, &["m/coupling.csv", "m/age.csv"]).unwrap();
|
||||
assert!(merged.warnings.is_empty(), "{:?}", merged.warnings);
|
||||
let source = |item: &str, columns: &[&str]| SourceFile {
|
||||
item: item.to_string(),
|
||||
columns: columns.iter().map(|c| c.to_string()).collect(),
|
||||
};
|
||||
assert_eq!(
|
||||
merged.sources,
|
||||
[
|
||||
source("coupling", &["entity", "coupled", "degree"]),
|
||||
source("age", &["entity", "age-months"]),
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
merged.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"}),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-8jk6.12 (3): two files with one stem are one item, and its
|
||||
/// columns are both files'. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn two_files_with_one_stem_union_their_columns() {
|
||||
let fs = folder(&[
|
||||
("a/x.csv", "entity,one\ne,1\n"),
|
||||
("b/x.csv", "entity,two\ne,2\n"),
|
||||
]);
|
||||
let merged = merge_csvs_from(&fs, &["a/x.csv", "b/x.csv"]).unwrap();
|
||||
let expected = SourceFile {
|
||||
item: "x".to_string(),
|
||||
columns: ["entity", "one", "two"].map(String::from).to_vec(),
|
||||
};
|
||||
assert_eq!(merged.sources, [expected]);
|
||||
}
|
||||
|
||||
// ── RFC 4180 edge cases ───────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -7,10 +7,11 @@ use super::analyzer::{
|
||||
DateComponent, FieldKind, FieldProposal, analyze_records, extract_array_at_path,
|
||||
extract_date_component, find_array_paths,
|
||||
};
|
||||
use super::csv_parser::{FILE_CATEGORY, Merged, SourceFile};
|
||||
use crate::formula::parse_formula;
|
||||
use crate::model::MAX_CATEGORIES;
|
||||
use crate::model::cell::{CellKey, CellValue};
|
||||
use crate::view::Axis;
|
||||
use crate::view::{Axis, VIRTUAL_VIEW_NAMES, View, ViewStep, records_steps};
|
||||
use crate::workbook::Workbook;
|
||||
|
||||
// ── Pipeline (no UI state) ────────────────────────────────────────────────────
|
||||
@@ -29,6 +30,9 @@ pub struct ImportPipeline {
|
||||
pub formulas: Vec<String>,
|
||||
/// What to do with records that share every coordinate (improvise-cgqt).
|
||||
pub on_duplicate: DuplicatePolicy,
|
||||
/// The files a CSV merge read, each of which gets its own view. Empty for
|
||||
/// a single file or a JSON import (improvise-lpzu).
|
||||
pub sources: Vec<SourceFile>,
|
||||
}
|
||||
|
||||
/// Several source records that land on one coordinate tuple (improvise-cgqt).
|
||||
@@ -64,6 +68,7 @@ impl ImportPipeline {
|
||||
model_name: "Imported Model".to_string(),
|
||||
formulas: vec![],
|
||||
on_duplicate: DuplicatePolicy::Refuse,
|
||||
sources: vec![],
|
||||
};
|
||||
|
||||
// Auto-select if root is an array or there is exactly one candidate path.
|
||||
@@ -77,6 +82,32 @@ impl ImportPipeline {
|
||||
pipeline
|
||||
}
|
||||
|
||||
/// A pipeline over a CSV merge, carrying its sources for the per-file views.
|
||||
pub fn from_merge(merged: Merged) -> Self {
|
||||
let mut pipeline = Self::new(Value::Array(merged.records));
|
||||
pipeline.seed_sources(merged.sources);
|
||||
pipeline
|
||||
}
|
||||
|
||||
/// Every file keeps its `File` item and its columns, rows or none: a
|
||||
/// header-only file is still a file (improvise-lpzu).
|
||||
fn seed_sources(&mut self, sources: Vec<SourceFile>) {
|
||||
let items: Vec<String> = sources.iter().map(|s| s.item.clone()).collect();
|
||||
match self.proposals.iter_mut().find(|p| p.field == FILE_CATEGORY) {
|
||||
Some(file) => file.distinct_values = items,
|
||||
None => self
|
||||
.proposals
|
||||
.push(unseen(FILE_CATEGORY, FieldKind::Category, items)),
|
||||
}
|
||||
for column in sources.iter().flat_map(|s| &s.columns) {
|
||||
if !self.proposals.iter().any(|p| &p.field == column) {
|
||||
self.proposals
|
||||
.push(unseen(column, FieldKind::Label, vec![]));
|
||||
}
|
||||
}
|
||||
self.sources = sources;
|
||||
}
|
||||
|
||||
pub fn select_path(&mut self, path: &str) {
|
||||
self.selected_path = path.to_string();
|
||||
if let Some(arr) = extract_array_at_path(&self.raw, path) {
|
||||
@@ -257,9 +288,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 +305,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 +361,13 @@ impl ImportPipeline {
|
||||
}
|
||||
}
|
||||
|
||||
let mut warnings = skip_warnings(
|
||||
self.records.len(),
|
||||
categories,
|
||||
&skipped_per_category,
|
||||
skipped_non_objects,
|
||||
);
|
||||
for view in source_views(&wb, &self.sources, &plan) {
|
||||
wb.views.insert(view.name.clone(), view);
|
||||
}
|
||||
|
||||
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)));
|
||||
@@ -353,6 +376,87 @@ impl ImportPipeline {
|
||||
}
|
||||
}
|
||||
|
||||
/// A proposal for a column no record has a value in, which is what an
|
||||
/// all-null column already gets from the analyzer (improvise-l1f5).
|
||||
fn unseen(field: &str, kind: FieldKind, distinct_values: Vec<String>) -> FieldProposal {
|
||||
FieldProposal {
|
||||
field: field.to_string(),
|
||||
kind,
|
||||
distinct_values,
|
||||
accepted: true,
|
||||
date_format: None,
|
||||
date_components: vec![],
|
||||
}
|
||||
}
|
||||
|
||||
/// One view per merged file, in file order, while `File` is a category. The
|
||||
/// shape and why Filter: `context/code/formula-and-io.md` (improvise-lpzu).
|
||||
fn source_views(wb: &Workbook, sources: &[SourceFile], plan: &Plan) -> Vec<View> {
|
||||
let file_is_a_category = plan.categories.iter().any(|c| c.field == FILE_CATEGORY);
|
||||
let mut views: Vec<View> = vec![];
|
||||
for source in sources.iter().filter(|_| file_is_a_category) {
|
||||
let taken = |name: &str| {
|
||||
wb.views.contains_key(name)
|
||||
|| VIRTUAL_VIEW_NAMES.contains(&name)
|
||||
|| views.iter().any(|v| v.name == name)
|
||||
};
|
||||
let name = std::iter::once(source.item.clone())
|
||||
.chain((2usize..).map(|n| format!("{} {n}", source.item)))
|
||||
.find(|name| !taken(name))
|
||||
.expect("a free name exists, there being finitely many views");
|
||||
views.push(source_view(wb, source, plan, name));
|
||||
}
|
||||
views
|
||||
}
|
||||
|
||||
/// Filtered to the file; its own categories seated as a fresh import seats
|
||||
/// them, every other one on None; other files' measures hidden.
|
||||
fn source_view(wb: &Workbook, source: &SourceFile, plan: &Plan, name: String) -> View {
|
||||
let has = |field: &str| source.columns.iter().any(|c| c == field);
|
||||
let own = |cat: &str| {
|
||||
has(cat)
|
||||
|| plan
|
||||
.date_extractions
|
||||
.iter()
|
||||
.any(|(field, _, _, derived)| derived == cat && has(field))
|
||||
};
|
||||
let mut view = View::new(name);
|
||||
for cat in wb.model.categories.values() {
|
||||
match cat.kind.is_virtual() || own(&cat.name) {
|
||||
true => view.on_category_added(&cat.name, cat.kind),
|
||||
false => {
|
||||
view.category_axes.insert(cat.name.clone(), Axis::None);
|
||||
}
|
||||
}
|
||||
}
|
||||
view.set_axis("_Measure", Axis::Page);
|
||||
view.set_axis(FILE_CATEGORY, Axis::Filter);
|
||||
view.set_page_selection(FILE_CATEGORY, &source.item);
|
||||
let (mine, theirs): (Vec<&FieldProposal>, Vec<&FieldProposal>) =
|
||||
plan.measures.iter().partition(|m| has(&m.field));
|
||||
for measure in theirs {
|
||||
view.hide_item("_Measure", &measure.field);
|
||||
}
|
||||
match mine.first() {
|
||||
Some(first) => view.set_page_selection("_Measure", &first.field),
|
||||
None => into_records(&mut view, wb, &source.item),
|
||||
}
|
||||
view
|
||||
}
|
||||
|
||||
/// A file with no measure has no measure to page to: its view is its records,
|
||||
/// still filtered to the file (improvise-8jk6.13).
|
||||
fn into_records(view: &mut View, wb: &Workbook, item: &str) {
|
||||
for step in records_steps(&wb.model) {
|
||||
match step {
|
||||
ViewStep::Axis { category, axis } => view.set_axis(&category, axis),
|
||||
ViewStep::PageSelection { category, item } => view.set_page_selection(&category, &item),
|
||||
}
|
||||
}
|
||||
view.set_axis(FILE_CATEGORY, Axis::Filter);
|
||||
view.set_page_selection(FILE_CATEGORY, item);
|
||||
}
|
||||
|
||||
/// The accepted fields by role, and the date parts derived from them.
|
||||
struct Plan<'a> {
|
||||
categories: Vec<&'a FieldProposal>,
|
||||
@@ -371,11 +475,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 +484,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 +507,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 +635,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) ─────────────────────────────
|
||||
@@ -625,8 +697,12 @@ pub struct ImportWizard {
|
||||
|
||||
impl ImportWizard {
|
||||
pub fn new(raw: Value) -> Self {
|
||||
let pipeline = ImportPipeline::new(raw);
|
||||
Self::from_pipeline(ImportPipeline::new(raw))
|
||||
}
|
||||
|
||||
/// A wizard over a pipeline the caller built, such as a CSV merge that
|
||||
/// carries its sources (improvise-lpzu).
|
||||
pub fn from_pipeline(pipeline: ImportPipeline) -> Self {
|
||||
let step = if pipeline.needs_path_selection() {
|
||||
WizardStep::SelectArrayPath
|
||||
} else if pipeline.records.is_empty() {
|
||||
@@ -1112,6 +1188,7 @@ impl ImportWizard {
|
||||
mod tests {
|
||||
use super::{ImportPipeline, collision_warning};
|
||||
use crate::import::analyzer::FieldKind;
|
||||
use crate::view::Axis;
|
||||
use serde_json::{Value, json};
|
||||
|
||||
#[test]
|
||||
@@ -1345,10 +1422,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 +1443,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 +2081,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 +2137,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,57 +2173,411 @@ 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",
|
||||
]
|
||||
);
|
||||
let items = &wb.model.category("Settlement Date").unwrap().items;
|
||||
assert_eq!(items.keys().collect::<Vec<_>>(), ["2026-01-05"]);
|
||||
assert!(warnings.is_empty(), "{warnings:?}");
|
||||
}
|
||||
|
||||
/// CSV files as a folder import merges them, sources and all.
|
||||
fn merged_pipeline(files: &[(&str, &str)]) -> ImportPipeline {
|
||||
use crate::import::csv_parser::merge_csvs_from;
|
||||
use crate::storage::{MemoryFs, Storage};
|
||||
let fs = MemoryFs::new();
|
||||
for (path, body) in files {
|
||||
fs.write(std::path::Path::new(path), body.as_bytes())
|
||||
.unwrap();
|
||||
}
|
||||
let paths: Vec<&str> = files.iter().map(|(p, _)| *p).collect();
|
||||
let merged = merge_csvs_from(&fs, &paths).unwrap();
|
||||
assert!(merged.warnings.is_empty(), "{:?}", merged.warnings);
|
||||
ImportPipeline::from_merge(merged)
|
||||
}
|
||||
|
||||
/// code-maat's age and coupling reports.
|
||||
const CODE_MAAT: [(&str, &str); 2] = [
|
||||
(
|
||||
"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",
|
||||
),
|
||||
];
|
||||
|
||||
fn code_maat_records() -> Value {
|
||||
Value::Array(merged_pipeline(&CODE_MAAT).records)
|
||||
}
|
||||
|
||||
/// Every non-empty cell a view's grid draws, row by row.
|
||||
fn drawn(wb: &crate::workbook::Workbook, view: &str) -> Vec<f64> {
|
||||
use crate::view::GridLayout;
|
||||
let layout = GridLayout::with_frozen_records(&wb.model, &wb.views[view], None);
|
||||
(0..layout.row_count())
|
||||
.flat_map(|r| (0..layout.col_count()).map(move |c| (r, c)))
|
||||
.filter_map(|(r, c)| layout.evaluate(&wb.model, &layout.cell_key(r, c)?))
|
||||
.filter_map(|v| v.as_f64())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// improvise-8jk6.2: where a record with no `Date` shows up, axis by axis.
|
||||
/// Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_record_with_no_coordinate_is_drawn_only_where_that_category_is_off_the_grid() {
|
||||
use crate::model::cell::CellKey;
|
||||
use crate::persistence::{format_md, parse_md};
|
||||
use crate::view::{GridLayout, drill_snapshot};
|
||||
let mut p = ImportPipeline::new(json!([
|
||||
{"Payee": "Acme", "Date": "d1", "Amount": 10.0},
|
||||
{"Payee": "Bolt", "Amount": 20.0},
|
||||
]));
|
||||
force_category(&mut p, "Date");
|
||||
let (mut wb, _) = p.build_model_with_warnings().unwrap();
|
||||
let mut seat = |date: Axis, payee: Axis| {
|
||||
let view = wb.active_view_mut();
|
||||
view.set_axis("_Measure", Axis::None);
|
||||
view.set_axis("Payee", payee);
|
||||
view.set_axis("Date", date);
|
||||
(drawn(&wb, "Default"), wb.clone())
|
||||
};
|
||||
assert_eq!(
|
||||
seat(Axis::Column, Axis::Row).0,
|
||||
[10.0],
|
||||
"no column holds Bolt"
|
||||
);
|
||||
assert_eq!(seat(Axis::Page, Axis::Row).0, [10.0], "no page holds Bolt");
|
||||
assert_eq!(
|
||||
seat(Axis::None, Axis::Row).0,
|
||||
[10.0, 20.0],
|
||||
"None counts it"
|
||||
);
|
||||
|
||||
let (_, mut records) = seat(Axis::None, Axis::None);
|
||||
records.active_view_mut().set_axis("_Index", Axis::Row);
|
||||
records.active_view_mut().set_axis("_Dim", Axis::Column);
|
||||
let layout = GridLayout::with_frozen_records(&records.model, records.active_view(), None);
|
||||
let row: Vec<String> = (0..layout.col_count())
|
||||
.map(|c| layout.records_display(1, c).unwrap_or_default())
|
||||
.collect();
|
||||
assert_eq!(row, ["", "Bolt", "", "20", ""], "a blank Date cell");
|
||||
|
||||
let text = format_md(&records);
|
||||
assert!(
|
||||
warnings[0].contains('2'),
|
||||
"warning counts skipped rows: {}",
|
||||
warnings[0]
|
||||
text.contains("\nPayee=Bolt, _Measure=Amount = 20\n"),
|
||||
"{text}"
|
||||
);
|
||||
assert!(
|
||||
warnings[0].contains('3'),
|
||||
"warning names the total: {}",
|
||||
warnings[0]
|
||||
assert_eq!(format_md(&parse_md(&text).unwrap()), text);
|
||||
|
||||
let drill = |cat: &str, item: &str| {
|
||||
let key = CellKey::new(vec![(cat.to_string(), item.to_string())]);
|
||||
drill_snapshot(&records.model, &key).len()
|
||||
};
|
||||
assert_eq!((drill("Payee", "Bolt"), drill("Date", "d1")), (1, 1));
|
||||
}
|
||||
|
||||
/// improvise-lpzu, the Codex P2 on PR #314: a header-only file keeps its
|
||||
/// identity. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_header_only_file_keeps_its_file_item_its_columns_and_its_view() {
|
||||
use crate::view::GridLayout;
|
||||
let (wb, _) = merged_pipeline(&[
|
||||
("maat/age.csv", "entity,age-months\nsrc/a.rs,2\n"),
|
||||
("maat/coupling.csv", "entity,coupled,degree,average-revs\n"),
|
||||
("maat/rev.csv", "entity,n-revs\nsrc/a.rs,9\n"),
|
||||
])
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
let files: Vec<&String> = wb.model.category("File").unwrap().items.keys().collect();
|
||||
assert_eq!(files, ["age", "coupling", "rev"]);
|
||||
for column in ["coupled", "degree", "average-revs"] {
|
||||
assert!(wb.model.category(column).is_some(), "{column} is missing");
|
||||
}
|
||||
let view = &wb.views["coupling"];
|
||||
assert_eq!(view.page_selection("File"), Some("coupling"));
|
||||
let layout = GridLayout::with_frozen_records(&wb.model, view, None);
|
||||
assert!(layout.is_records_mode() && layout.row_count() == 0);
|
||||
assert_eq!(drawn(&wb, "age"), [2.0]);
|
||||
}
|
||||
|
||||
/// improvise-8jk6.13: a view never pages onto a measure it hides.
|
||||
/// Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_file_with_no_measure_gets_a_records_view_not_another_files_measure() {
|
||||
use crate::view::GridLayout;
|
||||
let (wb, _) = merged_pipeline(&[
|
||||
("t/a.csv", "entity,x\ne1,1\n"),
|
||||
("t/tags.csv", "entity,tag\ne1,red\n"),
|
||||
])
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
let tags = &wb.views["tags"];
|
||||
assert_eq!(tags.page_selection("_Measure"), None);
|
||||
assert_ne!(tags.axis_of("_Measure"), Axis::Page, "{tags:?}");
|
||||
let layout = GridLayout::with_frozen_records(&wb.model, tags, None);
|
||||
assert!(layout.is_records_mode(), "{tags:?}");
|
||||
assert_eq!(
|
||||
layout.row_count(),
|
||||
0,
|
||||
"a record with no measure has no cell"
|
||||
);
|
||||
assert!(
|
||||
warnings[0].contains("region"),
|
||||
"warning names the missing field: {}",
|
||||
warnings[0]
|
||||
assert!(!wb.views["a"].is_records_layout());
|
||||
}
|
||||
|
||||
/// improvise-8jk6.12 (1): a date part derived from the file's own column
|
||||
/// is the file's. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_files_view_seats_the_date_parts_of_its_own_date_column() {
|
||||
use crate::import::analyzer::DateComponent;
|
||||
let mut p = merged_pipeline(&[
|
||||
("m/abs-churn.csv", "date,added\n2026-01-02,10\n"),
|
||||
("m/age.csv", "entity,age-months\nsrc/a.rs,2\n"),
|
||||
]);
|
||||
let date = p.proposals.iter_mut().find(|f| f.field == "date").unwrap();
|
||||
assert_eq!(date.kind, FieldKind::TimeCategory);
|
||||
date.date_components = vec![DateComponent::Month];
|
||||
let (wb, _) = p.build_model_with_warnings().unwrap();
|
||||
assert_ne!(wb.views["abs-churn"].axis_of("date_Month"), Axis::None);
|
||||
assert_eq!(wb.views["age"].axis_of("date_Month"), Axis::None);
|
||||
}
|
||||
|
||||
/// improvise-8jk6.12 (2): two files of one batch never share a view name.
|
||||
/// Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn two_files_whose_names_collide_in_one_batch_keep_a_view_each() {
|
||||
let (wb, _) = merged_pipeline(&[
|
||||
("d/Default.csv", "Region,Revenue\nEast,1\n"),
|
||||
("d/Default 2.csv", "Region,Revenue\nEast,2\n"),
|
||||
])
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
let filters: Vec<(&str, Option<&str>)> = wb
|
||||
.views
|
||||
.values()
|
||||
.map(|v| (v.name.as_str(), v.page_selection("File")))
|
||||
.collect();
|
||||
assert_eq!(
|
||||
filters,
|
||||
[
|
||||
("Default", None),
|
||||
("Default 2", Some("Default")),
|
||||
("Default 2 2", Some("Default 2")),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-8jk6.12 (4): a whitespace-only value is no coordinate, never
|
||||
/// an item. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_whitespace_only_category_value_is_no_coordinate() {
|
||||
let (wb, _) = ImportPipeline::new(json!([
|
||||
{"region": "East", "revenue": 1.0},
|
||||
{"region": " ", "revenue": 2.0},
|
||||
]))
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
let items: Vec<&String> = wb.model.category("region").unwrap().items.keys().collect();
|
||||
assert_eq!(items, ["East"]);
|
||||
assert_eq!(
|
||||
stored(&wb),
|
||||
["_Measure=revenue, region=East = 1", "_Measure=revenue = 2"]
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-lpzu: each merged file gets a view of its own records and
|
||||
/// columns. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_merge_gives_each_file_a_view_projected_to_its_columns() {
|
||||
let (wb, _) = merged_pipeline(&CODE_MAAT)
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
wb.views.keys().collect::<Vec<_>>(),
|
||||
["Default", "age", "coupling"]
|
||||
);
|
||||
assert_eq!(wb.active_view, "Default");
|
||||
|
||||
let age = &wb.views["age"];
|
||||
assert_eq!(age.axis_of("File"), Axis::Filter);
|
||||
assert_eq!(age.page_selection("File"), Some("age"));
|
||||
assert_eq!(age.axis_of("entity"), Axis::Row);
|
||||
assert_eq!(age.axis_of("coupled"), Axis::None);
|
||||
assert_eq!(age.axis_of("_Measure"), Axis::Page);
|
||||
assert_eq!(age.page_selection("_Measure"), Some("age-months"));
|
||||
assert!(age.is_hidden("_Measure", "degree") && age.is_hidden("_Measure", "average-revs"));
|
||||
assert!(!age.is_hidden("_Measure", "age-months"));
|
||||
assert_eq!(drawn(&wb, "age"), [2.0, 7.0]);
|
||||
|
||||
let coupling = &wb.views["coupling"];
|
||||
assert_eq!(coupling.page_selection("File"), Some("coupling"));
|
||||
assert_eq!(coupling.axis_of("entity"), Axis::Row);
|
||||
assert_eq!(coupling.axis_of("coupled"), Axis::Column);
|
||||
assert_eq!(coupling.page_selection("_Measure"), Some("average-revs"));
|
||||
assert!(coupling.is_hidden("_Measure", "age-months"));
|
||||
assert!(!coupling.is_hidden("_Measure", "average-revs"));
|
||||
assert_eq!(drawn(&wb, "coupling"), [5.0, 3.0]);
|
||||
}
|
||||
|
||||
/// improvise-lpzu: two files of one schema are told apart by the filter,
|
||||
/// not by their columns. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_files_view_holds_only_that_files_records() {
|
||||
let (wb, _) = merged_pipeline(&[
|
||||
("s/q1.csv", "Region,Revenue\nEast,10\nWest,1\n"),
|
||||
("s/q2.csv", "Region,Revenue\nEast,20\nWest,2\n"),
|
||||
])
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
assert_eq!(drawn(&wb, "q1"), [10.0, 1.0]);
|
||||
assert_eq!(drawn(&wb, "q2"), [20.0, 2.0]);
|
||||
}
|
||||
|
||||
/// improvise-lpzu: a view name is taken from the file only while it is
|
||||
/// free. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn a_files_view_name_skips_names_already_held() {
|
||||
let (wb, _) = merged_pipeline(&[
|
||||
("d/Default.csv", "Region,Revenue\nEast,1\n"),
|
||||
("d/_Drill.csv", "Region,Revenue\nEast,2\n"),
|
||||
])
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
wb.views.keys().collect::<Vec<_>>(),
|
||||
["Default", "Default 2", "_Drill 2"]
|
||||
);
|
||||
assert_eq!(
|
||||
wb.views["Default 2"].page_selection("File"),
|
||||
Some("Default")
|
||||
);
|
||||
assert_eq!(wb.views["_Drill 2"].page_selection("File"), Some("_Drill"));
|
||||
}
|
||||
|
||||
/// improvise-lpzu: the per-file views persist, and in the same order on
|
||||
/// every import. Evidence: `context/test-evidence/formula-and-io.md`.
|
||||
#[test]
|
||||
fn per_file_views_survive_a_round_trip_and_repeat_byte_for_byte() {
|
||||
use crate::persistence::{format_md, parse_md};
|
||||
let build = || {
|
||||
let (wb, _) = merged_pipeline(&CODE_MAAT)
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
format_md(&wb)
|
||||
};
|
||||
let text = build();
|
||||
assert_eq!(build(), text);
|
||||
let reread = parse_md(&text).unwrap();
|
||||
assert_eq!(format_md(&reread), text);
|
||||
assert_eq!(
|
||||
reread.views.keys().collect::<Vec<_>>(),
|
||||
["Default", "age", "coupling"]
|
||||
);
|
||||
assert_eq!(reread.views["age"].axis_of("File"), Axis::Filter);
|
||||
assert_eq!(reread.views["age"].page_selection("File"), Some("age"));
|
||||
assert_eq!(drawn(&reread, "coupling"), [5.0, 3.0]);
|
||||
}
|
||||
|
||||
/// 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",
|
||||
]
|
||||
);
|
||||
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]
|
||||
fn build_model_warns_nothing_when_every_record_is_complete() {
|
||||
let raw = json!([
|
||||
|
||||
+3
-3
@@ -58,7 +58,7 @@ impl<'a> Drop for TuiContext<'a> {
|
||||
pub fn run_tui(
|
||||
workbook: crate::workbook::Workbook,
|
||||
file_path: Option<PathBuf>,
|
||||
import_value: Option<serde_json::Value>,
|
||||
import: Option<crate::import::wizard::ImportPipeline>,
|
||||
) -> Result<()> {
|
||||
let mut stdout = io::stdout();
|
||||
let mut tui_context = TuiContext::enter(&mut stdout)?;
|
||||
@@ -68,8 +68,8 @@ pub fn run_tui(
|
||||
app.term_height = h;
|
||||
}
|
||||
|
||||
if let Some(json) = import_value {
|
||||
app.start_import_wizard(json);
|
||||
if let Some(pipeline) = import {
|
||||
app.start_import(pipeline);
|
||||
} else if app.is_empty_model() {
|
||||
app.view_state.mode = AppMode::Help;
|
||||
}
|
||||
|
||||
+41
-15
@@ -144,7 +144,7 @@ impl Runnable for ImportArgs {
|
||||
if self.files.is_empty() {
|
||||
anyhow::bail!("No files specified for import");
|
||||
}
|
||||
let import_value = get_import_data(&self.files)
|
||||
let pipeline = get_import_data(&self.files)
|
||||
.ok_or_else(|| anyhow::anyhow!("Failed to parse import files"))?;
|
||||
|
||||
let config = ImportConfig {
|
||||
@@ -160,9 +160,9 @@ impl Runnable for ImportArgs {
|
||||
};
|
||||
|
||||
if self.no_wizard {
|
||||
run_headless_import(import_value, &config, self.output, model_file)
|
||||
run_headless_import(pipeline, &config, self.output, model_file)
|
||||
} else {
|
||||
run_wizard_import(import_value, &config, model_file)
|
||||
run_wizard_import(pipeline, &config, model_file)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -202,7 +202,7 @@ fn parse_colon_pairs(args: &[String]) -> Vec<(String, String)> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
use import::wizard::DuplicatePolicy;
|
||||
use import::wizard::{DuplicatePolicy, ImportPipeline};
|
||||
|
||||
fn apply_config_to_pipeline(pipeline: &mut import::wizard::ImportPipeline, config: &ImportConfig) {
|
||||
use import::analyzer::{DateComponent, FieldKind};
|
||||
@@ -289,12 +289,11 @@ fn apply_axis_overrides(wb: &mut Workbook, axes: &[(String, String)]) {
|
||||
}
|
||||
|
||||
fn run_headless_import(
|
||||
import_value: Value,
|
||||
mut pipeline: ImportPipeline,
|
||||
config: &ImportConfig,
|
||||
output: Option<PathBuf>,
|
||||
model_file: Option<PathBuf>,
|
||||
) -> Result<()> {
|
||||
let mut pipeline = import::wizard::ImportPipeline::new(import_value);
|
||||
apply_config_to_pipeline(&mut pipeline, config);
|
||||
if pipeline.on_duplicate == DuplicatePolicy::Refuse {
|
||||
let collisions = pipeline.collisions();
|
||||
@@ -318,30 +317,57 @@ fn run_headless_import(
|
||||
}
|
||||
|
||||
fn run_wizard_import(
|
||||
import_value: Value,
|
||||
pipeline: ImportPipeline,
|
||||
_config: &ImportConfig,
|
||||
model_file: Option<PathBuf>,
|
||||
) -> Result<()> {
|
||||
let workbook = get_initial_workbook(&model_file)?;
|
||||
// The wizard configures itself inside the TUI.
|
||||
// TODO: pass `_config` to it for pre-population.
|
||||
run_tui(workbook, model_file, Some(import_value))
|
||||
run_tui(workbook, model_file, Some(pipeline))
|
||||
}
|
||||
|
||||
// ── Import data loading ──────────────────────────────────────────────────────
|
||||
|
||||
fn get_import_data(paths: &[PathBuf]) -> Option<Value> {
|
||||
fn get_import_data(args: &[PathBuf]) -> Option<ImportPipeline> {
|
||||
use improvise::import::csv_parser::{ImportPath, expand_import_paths};
|
||||
// Whether an argument is a folder is the one filesystem question asked here.
|
||||
let classified: Vec<ImportPath> = args
|
||||
.iter()
|
||||
.map(|p| match p.is_dir() {
|
||||
true => ImportPath::Folder(p.clone()),
|
||||
false => ImportPath::File(p.clone()),
|
||||
})
|
||||
.collect();
|
||||
let paths = match expand_import_paths(&improvise_io::storage::PhysicalFs, &classified) {
|
||||
Ok(paths) => paths,
|
||||
Err(e) => {
|
||||
eprintln!("{e:#}");
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let paths = paths.as_slice();
|
||||
let all_csv = paths.iter().all(|p| csv_path_p(p));
|
||||
// A folder always merges, so even one CSV keeps its file identity (lpzu).
|
||||
let a_folder = classified
|
||||
.iter()
|
||||
.any(|a| matches!(a, ImportPath::Folder(_)));
|
||||
|
||||
if paths.len() > 1 {
|
||||
if paths.len() > 1 || a_folder {
|
||||
if !all_csv {
|
||||
eprintln!("Multi-file import only supports CSV files");
|
||||
return None;
|
||||
}
|
||||
match crate::import::csv_parser::merge_csvs(paths) {
|
||||
Ok(records) => Some(Value::Array(records)),
|
||||
let fs = improvise_io::storage::PhysicalFs;
|
||||
match crate::import::csv_parser::merge_csvs_from(&fs, paths) {
|
||||
Ok(merged) => {
|
||||
for warning in &merged.warnings {
|
||||
eprintln!("warning: {warning}");
|
||||
}
|
||||
Some(ImportPipeline::from_merge(merged))
|
||||
}
|
||||
Err(e) => {
|
||||
eprintln!("CSV merge error: {e}");
|
||||
eprintln!("CSV merge error: {e:#}");
|
||||
None
|
||||
}
|
||||
}
|
||||
@@ -355,7 +381,7 @@ fn get_import_data(paths: &[PathBuf]) -> Option<Value> {
|
||||
Ok(content) => {
|
||||
if csv_path_p(path) {
|
||||
match crate::import::csv_parser::parse_csv(path) {
|
||||
Ok(records) => Some(Value::Array(records)),
|
||||
Ok(records) => Some(ImportPipeline::new(Value::Array(records))),
|
||||
Err(e) => {
|
||||
eprintln!("CSV parse error: {e}");
|
||||
None
|
||||
@@ -367,7 +393,7 @@ fn get_import_data(paths: &[PathBuf]) -> Option<Value> {
|
||||
eprintln!("JSON parse error: {e}");
|
||||
None
|
||||
}
|
||||
Ok(json) => Some(json),
|
||||
Ok(json) => Some(ImportPipeline::new(json)),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
//! A folder import's per-file views, saved and reloaded, as both front ends
|
||||
//! project them (improvise-lpzu). Evidence: `context/test-evidence/cli.md`.
|
||||
|
||||
use improvise::import::csv_parser::merge_csvs_from;
|
||||
use improvise::import::wizard::ImportPipeline;
|
||||
use improvise::persistence::{format_md, parse_md};
|
||||
use improvise::ui::app::App;
|
||||
use improvise_command::ui::project::{Viewport, grid_in};
|
||||
use improvise_io::storage::{MemoryFs, Storage};
|
||||
|
||||
fn reloaded_folder_import() -> App {
|
||||
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",
|
||||
),
|
||||
("maat/soc.csv", "entity,soc\n"),
|
||||
];
|
||||
for (path, body) in files {
|
||||
fs.write(std::path::Path::new(path), body.as_bytes())
|
||||
.unwrap();
|
||||
}
|
||||
let merged = merge_csvs_from(&fs, &files.map(|(p, _)| p)).unwrap();
|
||||
let (wb, _) = ImportPipeline::from_merge(merged)
|
||||
.build_model_with_warnings()
|
||||
.unwrap();
|
||||
App::new(parse_md(&format_md(&wb)).unwrap(), None)
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_saved_per_file_view_projects_only_its_file() {
|
||||
let app = reloaded_folder_import();
|
||||
let whole = Viewport {
|
||||
row_offset: 0,
|
||||
col_offset: 0,
|
||||
rows: 100,
|
||||
cols: 100,
|
||||
};
|
||||
for (view, texts) in [("age", ["2.00", "7.00"]), ("coupling", ["5.00", "3.00"])] {
|
||||
let grid = grid_in(&app, view, whole);
|
||||
let file = grid.page_coords.iter().find(|p| p.cat == "File").unwrap();
|
||||
assert_eq!((file.selected.as_str(), &file.items), (view, &None));
|
||||
let drawn: Vec<&str> = grid
|
||||
.cells
|
||||
.iter()
|
||||
.flatten()
|
||||
.map(|c| c.text.as_str())
|
||||
.filter(|t| !t.is_empty())
|
||||
.collect();
|
||||
assert_eq!(drawn, texts, "{view}");
|
||||
}
|
||||
|
||||
// A header-only file: its item exists, and its view has nothing editable.
|
||||
let soc = grid_in(&app, "soc", whole);
|
||||
let file = soc.page_coords.iter().find(|p| p.cat == "File").unwrap();
|
||||
assert_eq!(file.selected, "soc");
|
||||
assert!(soc.records, "a file with no measure opens in records mode");
|
||||
assert_eq!(soc.extent.rows, 0, "{soc:?}");
|
||||
}
|
||||
@@ -612,3 +612,130 @@ fn a_headless_import_keeps_duplicates_separate_when_told_to() {
|
||||
assert!(doc.contains("= 10") && doc.contains("= 20"), "{doc}");
|
||||
assert!(!doc.contains("= 30"), "{doc}");
|
||||
}
|
||||
|
||||
/// 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");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
for (name, body) in [
|
||||
(
|
||||
"coupling.csv",
|
||||
"entity,coupled,degree,average-revs\nsrc/a.rs,src/b.rs,80,5\n",
|
||||
),
|
||||
("age.csv", "entity,age-months\nsrc/a.rs,2\nsrc/b.rs,9\n"),
|
||||
(
|
||||
"abs-churn.csv",
|
||||
"date,added,deleted,commits\n2026-01-02,10,3,2\n",
|
||||
),
|
||||
("README.txt", "not a report\n"),
|
||||
] {
|
||||
std::fs::write(dir.join(name), body).unwrap();
|
||||
}
|
||||
dir
|
||||
}
|
||||
|
||||
/// improvise-lpzu: a folder argument is its `*.csv` files, merged sparsely and
|
||||
/// in name order. Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_headless_import_of_a_folder_merges_its_csv_files_and_repeats_byte_for_byte() {
|
||||
let scratch = Scratch::new("folder_import");
|
||||
let dir = code_maat_folder(&scratch);
|
||||
let out_path = dir.join("maat.improv");
|
||||
|
||||
let first = run_import(&dir, &out_path, &[]);
|
||||
assert!(first.status.success(), "{}", stderr_of(&first));
|
||||
let doc = std::fs::read_to_string(&out_path).unwrap();
|
||||
for line in [
|
||||
"File=coupling, _Measure=degree, coupled=|src/b.rs|, entity=|src/a.rs| = 80",
|
||||
"File=age, _Measure=age-months, entity=|src/b.rs| = 9",
|
||||
"File=abs-churn, _Measure=added, date=|2026-01-02| = 10",
|
||||
] {
|
||||
assert!(doc.lines().any(|l| l == line), "{line} missing from\n{doc}");
|
||||
}
|
||||
assert!(!doc.contains("README"), "{doc}");
|
||||
let views: Vec<&str> = doc.lines().filter(|l| l.starts_with("## View: ")).collect();
|
||||
assert_eq!(
|
||||
views,
|
||||
[
|
||||
"## View: Default",
|
||||
"## View: abs-churn",
|
||||
"## View: age",
|
||||
"## View: coupling"
|
||||
]
|
||||
);
|
||||
assert!(doc.lines().any(|l| l == "File: filter, coupling"), "{doc}");
|
||||
assert!(doc.contains("Initial View: Default"), "{doc}");
|
||||
|
||||
// The output now sits in the folder; a second run must not read it.
|
||||
let second = run_import(&dir, &out_path, &[]);
|
||||
assert!(second.status.success(), "{}", stderr_of(&second));
|
||||
assert_eq!(std::fs::read_to_string(&out_path).unwrap(), doc);
|
||||
}
|
||||
|
||||
/// improvise-lpzu: a file and a folder together are one merge, and a folder
|
||||
/// with nothing to import is named. Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_headless_import_takes_files_and_folders_together_and_refuses_an_empty_folder() {
|
||||
let scratch = Scratch::new("folder_mixed");
|
||||
let dir = code_maat_folder(&scratch);
|
||||
let extra = scratch.path("extra.csv");
|
||||
std::fs::write(&extra, "entity,age-months\nsrc/z.rs,4\n").unwrap();
|
||||
let out_path = scratch.path("out.improv");
|
||||
|
||||
let out = Command::new(bin())
|
||||
.args(["import", "--no-wizard", "-o"])
|
||||
.arg(&out_path)
|
||||
.arg(&extra)
|
||||
.arg(&dir)
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(out.status.success(), "{}", stderr_of(&out));
|
||||
let doc = std::fs::read_to_string(&out_path).unwrap();
|
||||
assert!(
|
||||
doc.contains("File=extra, _Measure=age-months, entity=|src/z.rs| = 4"),
|
||||
"{doc}"
|
||||
);
|
||||
assert!(doc.contains("File=coupling"), "{doc}");
|
||||
|
||||
let empty = scratch.path("empty");
|
||||
std::fs::create_dir_all(&empty).unwrap();
|
||||
let refused = run_import(&empty, &scratch.path("never.improv"), &[]);
|
||||
assert!(!refused.status.success());
|
||||
let err = stderr_of(&refused);
|
||||
assert!(
|
||||
err.contains(&format!("no CSV files in folder {}", empty.display())),
|
||||
"{err}"
|
||||
);
|
||||
assert!(!scratch.path("never.improv").exists());
|
||||
}
|
||||
|
||||
/// improvise-lpzu, the user's ruling: a folder always merges, so even one CSV
|
||||
/// keeps its file identity. Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_folder_of_one_csv_still_gets_a_file_category_and_its_view_but_a_lone_file_does_not() {
|
||||
let scratch = Scratch::new("folder_single");
|
||||
let dir = scratch.path("one");
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let csv = dir.join("age.csv");
|
||||
std::fs::write(&csv, "entity,age-months\nsrc/a.rs,2\n").unwrap();
|
||||
|
||||
let from_folder = scratch.path("folder.improv");
|
||||
let out = run_import(&dir, &from_folder, &[]);
|
||||
assert!(out.status.success(), "{}", stderr_of(&out));
|
||||
let doc = std::fs::read_to_string(&from_folder).unwrap();
|
||||
assert!(doc.contains("## Category: File\n- age\n"), "{doc}");
|
||||
assert!(doc.lines().any(|l| l == "File: filter, age"), "{doc}");
|
||||
assert!(
|
||||
doc.contains("File=age, _Measure=age-months, entity=|src/a.rs| = 2"),
|
||||
"{doc}"
|
||||
);
|
||||
|
||||
let from_file = scratch.path("file.improv");
|
||||
let out = run_import(&csv, &from_file, &[]);
|
||||
assert!(out.status.success(), "{}", stderr_of(&out));
|
||||
let doc = std::fs::read_to_string(&from_file).unwrap();
|
||||
assert!(
|
||||
!doc.contains("File"),
|
||||
"a lone file keeps today's shape: {doc}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user