Files
improvise/context/code/formula-and-io.md
T
Edward Langley afbe7e7d1f docs: record the measured generator coverage of the new kind_line rule
context/code/formula-and-io.md already says a new grammar production needs
generator coverage or the proptests quietly stop describing the format.
`kind_line` is an alternative of `category_entry`, so the grammar walk
reaches it with no generator change — but that is precisely the claim the
section exists to say must be checked rather than assumed.

Probed over 500 files from generator::improv_file(): 14 carried a
`kind: label` line and all 14 parsed to a CategoryKind::Label, so each
256-case proptest run exercises the production about seven times.

The probe is not kept. Its rate depends on proptest's internal RNG, so as
a standing assertion it would fail on a dependency bump for a reason that
has nothing to do with the format — the measurement belongs in the
argument, where it can be re-run and corrected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
2026-09-22 01:15:47 -07:00

33 KiB

improvise-formula and improvise-io

The arguments that used to live in the comments of these two crates. Written for a reader with the code open; the code says what, this says why, and why the obvious alternative is wrong. Bug narratives keyed by test name are in context/test-evidence/formula-and-io.md.


Pipe quoting has exactly one home

improvise_formula::quote owns both halves of the CL-style |…| convention — writing and reading — for both grammars. formula.pest's bare_ident/pipe_quoted and improv.pest's bare_name/pipe_quoted mirror each other character for character, and quote.rs is the one Rust statement of that rule.

It lives in improvise-formula because that crate owns the grammar that says what a bare name is, and because every emitter needs it: a name that came from a user's data has to be spelled so the parser hands it back unchanged. improvise-gp re-exports it, improvise-io calls it from persistence/{format,parse}.rs, and parser.rs::unquote_pipe is a thin shim over unescape_pipe_body.

Before improvise-i2m5 there were three copies and they had drifted, over the keyword rule below. improvise-io no longer has one — its persistence/quoting.rs is deleted. context/repo-map.md's file inventory kept claiming io "still has its own unguarded copy" long after the deletion — it was corrected in wave 1, but any other doc or comment still saying so is wrong, and that is the shape this particular fact goes stale in.

Three entry points, because the callers do not want the same thing:

  • quote_name — a name as the grammar wants it: bare when is_bare_name allows, else pipe-quoted.
  • pipe_quote — unconditional. A .improv text cell goes through this, not through quote_name, because a bare 42 would read back as a number.
  • unescape_pipe_body — the inverse, over the body between the delimiters (what both grammars' pipe_inner matches). The escape table is written once, in the last two.

The keyword guard is canonical, user-confirmed — do not revert it

An item literally named SUM is written |SUM|. is_bare_name is deliberately stricter than the grammar strictly requires: bare keywords (WHERE SUM AVG MIN MAX COUNT IF IS) do parse today, but only because agg_call needs a ( and PEG backtracking then falls through to ref_expr — an emergent property of rule ordering, not a stated rule. A pipe-quoted name is an identifier in every position, with nothing to fall through, so a name colliding with a keyword is quoted and stops depending on that ordering.

IS joined the list with is_expr (improvise-4zl7), and it is the one keyword whose omission would have been silently wrong rather than merely fragile: a category literally named IS written bare turns IS IS East into a predicate on a category called East. Adding a production to formula.pest means adding its keyword here — the two are one change.

The cost is worth paying because a ## Formulas line carries formula text verbatim: a name format_md writes is later read by parse_formula. One namespace, one spelling rule.

The escape table, and the one input outside both grammars

\| → |, \\ → \, \n → newline; any other \X is preserved verbatim. The trailing arms of unescape_pipe_body are unreachable from either grammar (pipe_inner is ("\\" ~ ANY | !"|" ~ ANY)*, so a backslash is always followed by a character). They are written as a total function rather than a panic so a caller arriving by some other route gets its bytes back. The two pre-i2m5 unquoting copies differed only on a trailing lone backslash, where io returned it and parser.rs panicked; the survivor returns it.


Cat IS Item: a coordinate predicate in the expression grammar

is_expr = { identifier ~ ^"IS" ~ filter_value } sits in primary before ref_expr, which would otherwise consume the category name and leave IS Item unparseable. A bare name still parses as a reference: is_expr needs the keyword, fails without it, and PEG backtracks into ref_expr.

It reuses where_clause's two operands — an identifier for the category and a filter_value for the item — and therefore build_filter, which already reads exactly that pair. The ^"IS" literal is a string in the grammar rather than a rule, so it contributes no pair and into_inner() yields the two operands and nothing else. One builder, one quoting rule, both halves through improvise_formula::quote.

The grammar-walking generator needs no new case: it reads formula.pest through pest_meta at test time, so a production added to primary is generated and round-tripped from the day it exists.

What the evaluator does with it, and why a fitted tree is always wrapped in a SUM, is context/code/core.md §13.

WHERE carries the same boundary, and the mechanism has to be generatable

where_clause and inline_where are compound-atomic with sp+ after the keyword, exactly as is_expr is (improvise-r08u). Non-atomic, pest auto-skipped WHITESPACE after ^"WHERE", so SUM(Rev WHEREAS x = y) read as a WHERE on a category AS — a silently wrong filter where it should have been a syntax error, the same class of defect IS had.

Two things decided the shape, and the second is the one worth carrying. A negative lookahead !bare_char with sp* would have been strictly more permissive — it keeps WHERE|Region|=East, which the old rule accepted — but the grammar-walking generator cannot honour a predicate: it walks formula.pest through pest_meta and emits WHEREa="", text the grammar then refuses, so generated_formula_parses goes red. A rule whose only guard is a lookahead is a rule the generator can violate. sp+ is generatable, matches IS, and costs exactly one accepted spelling: a category may no longer abut the keyword, so WHERE |Region| needs its space.

Note compound-atomicity alone closes the ticket's own reproducer, because AS x cannot be one identifier once implicit whitespace is gone. sp+ is what closes the residue, WHEREAS=y, and the negative test names both.

A formula target is unquoted, exactly as a reference is

parse_formula strips the pipes off the target the same way it does off an Expr::Ref, because the target names a _Measure item. A target that kept its delimiters made Model::add_formula create an item literally called |Total Revenue| — a ghost column the user's real item never matched, and that Expr::Ref("Total Revenue") never resolved to.

Both the TUI formula minibuffer (AddFormula) and .improv load go through that one call, so both were affected. The writer emits Formula::raw, never target, so the file text is unaffected either way — which is exactly why the round-trip tests never saw it, and why the fix repairs existing files on next load with no migration.


The .improv grammar is the single source of truth

persistence/improv.pest defines the format; parse.rs is a tree-walker over pest's parse tree, not a line scanner. Adding a format feature means changing the grammar first.

The test generator (parse.rs::generator, and its twin in parser.rs::generator for formula.pest) reads the grammar's AST at test time via pest_meta and emits random valid input. A new production therefore needs generator coverage, or the proptests quietly stop describing the format: an Expr variant the emitter does not handle produces nothing, and generated_file_parses then passes over input that never exercised the new rule. formula.pest's generator is explicit about this — lookaheads emit nothing on purpose, and any unused Expr variant falls through the _ => {} arm; if the grammar starts using one, generated formulas fail to parse and the gap surfaces.

Two generator details worth not re-deriving. The atomic flag in the formula generator controls whether a Seq inserts a space between children, so non-atomic rules produce the whitespace-separated tokens pest's implicit WHITESPACE accepts. And Choice is 50/50 between two branches, so rules with three or more alternatives (nested Choices) pick deeper branches less often — good enough for random generation, not a uniform distribution.

The category kind is a line, not a header suffix (improvise-dy3j)

CategoryKind::Label had no spelling in the format at all. format_md wrote every category as a bare ## Category: Name and build_category read every section back through add_category, so the kind was dropped on the way out and on the way in. A round-trip equality test cannot see a loss both sides share, which is why the existing suite was silent — the same shape as the Formula::raw traps above, and the reason the new tests assert the parsed Category::kind rather than the emitted text.

The consequence was a lost document, not a lost flag. Label sits outside MAX_CATEGORIES = 12 on purpose, and the import wizard's own precondition scores a label as zero, so it will happily build twelve categories plus a label. On load every label came back Regular, the thirteenth was refused, and parse_md failed: any document where regular + label exceeded twelve was permanently unloadable. Below the limit the failure was quieter — a label silently became a pivot category, entering regular_category_names() pickers and gaining axis auto-assignment.

The ticket proposed ## Category: Note [label], and the grammar forbids it. A section header takes its name as the whole line, totally (improvise-pczv) — header_name is pipe_quoted ~ &NEWLINE | line_name — so a suffix is not a suffix, it is part of a category literally named Note [label]. The two readings are indistinguishable, and adopting the suffix would have made a legal name unspellable while quietly re-opening the ambiguity header_name exists to close.

So the kind is an entry inside the section, beside the item lines:

## Category: Note
kind: label
- see the record view

Three properties earned it. It uses a real token ("kind: "), not a lookahead, because the grammar-walking generator reads the grammar through pest_meta and skips predicates — a rule guarded only by a lookahead is a rule the generator can violate, which is why WHERE carries sp+. It cannot collide with item_list, grouped_item or group_hierarchy, each of which needs a - or > lead. And absence is the compatible reading: kind_kw spells only label, the writer emits the line only for a label, so every file written before this commit parses unchanged and every pivot category's section is byte-identical to what it was. That is also why parse_kind_kw is an exhaustive match returning Result rather than a bool read off the rule's mere presence — a second keyword must fail loudly at the walker, not be read as label because it arrived on a kind_line.

One case the format already allowed and this had to answer: two ## Category: sections naming one category are one category (improvise-349m), merged in pass 1. Absence is not a claim, so the merge keeps a stated kind rather than letting a later kindless section erase it.

The generator coverage this section demands was measured, not assumed. kind_line is an alternative of category_entry, which the grammar walk reaches generically, so nothing in the generator needed changing — but that is the kind of claim this section exists to say must be checked. Probed with a throwaway test over 500 files from generator::improv_file(): 14 carried a kind: label line and all 14 parsed to a CategoryKind::Label, so at the default 256 cases each proptest run exercises the new production about seven times. The probe was removed rather than kept: its rate depends on proptest's internal RNG, so as a permanent assertion it would fail on a dependency bump for a reason unrelated to the format.

The tree walkers are infallible by construction

Every build_* in parser.rs operates on a Pair pest has already validated. "A formula has exactly one target", "a comparison has lhs, op and rhs" — pest guarantees these before the walker sees the pair. So every .expect(GRAMMAR_INVARIANT) is a bug marker, not a runtime condition: if one fires, the grammar and the walker have diverged.

That reasoning used to be extended to the final else arm of each dispatch, and the extension was wrong. The sentence here said the unlabelled arm was safe because ref_expr is the only remaining alternative of primary, COUNT the only remaining agg_func and >= the only remaining cmp_op. Each clause was true of the grammar as it stood, which is exactly the property a grammar edit changes -- so what the arm really encoded was a snapshot, silently re-interpreted as an invariant. "pest validated the pair" says the pair matches some alternative; it never said which, and an unlabelled arm reads the answer off a count the next commit is free to change.

Two different repairs, and which one is available is a property of the input:

  • A &str, where no compile error exists. parse_agg_func and parse_cmp_op take the matched text, because agg_func and cmp_op are each one pest rule over string alternatives and pest_derive generates no per-alternative Rule. Nothing for a match to be exhaustive over, so they spell every alternative and unreachable!(GRAMMAR_INVARIANT) on anything else -- loud at the first parse rather than silent forever (improvise-492x).
  • A Rule, where one does. The four dispatches on Pair::as_rule -- build_formula, build_unary, build_primary, filter_value_to_string -- match over an enum, so they name every rule, including a final or-pattern over the ones that cannot appear. Adding any rule to formula.pest is now a build failure at all four until someone decides what it means.

The greppable form: == Rule:: appears nowhere in parser.rs. An if rule == Rule::x chain cannot be exhaustive, and it is how the snapshot got written down as an invariant in the first place.

The cost is four or-patterns of ~28 enum names, which is genuinely noisy, and that is the price of the guarantee -- an invariant made unrepresentable rather than validated, applied to a grammar instead of to a type. The noise is also the tell that a walker has grown a dispatch, which is worth noticing.

build_primary is where this was demonstrated rather than argued, and the injection has to be chosen carefully: a new alternative that is an atomic rule is caught by accident, because first_inner then finds no child and panics on its own GRAMMAR_INVARIANT. Only a non-atomic alternative reaches the silent path. With pct_literal = { "#" ~ identifier } placed first in primary, the pre-fix tree built clean, the whole crate suite passed, and parse_formula("X = #Revenue") returned Ref("Revenue") -- the # discarded and the formula quietly wrong. An injection into an atomic rule would have reported the code safe when it was not.

parse.rs's walkers are the opposite and deliberately so: they return Result and name the section in the error, because the .improv file is hand-editable and a failure there is a user's problem, not a programmer's. parse_tree renders pest's error as a located message (line number plus the offending line), because "Parse error: {pest dump}" is unusable to somebody fixing a file by hand.

Two passes, because sections may appear in any order

Pass 1 turns each ## section into a plain collector (PCategory, PView) or a flat list; pass 2 builds the Workbook. That split is what makes the format order-independent — a ## View: may name a category defined further down. PCategory::items is a Vec in file order because IndexMap insertion order is load-bearing (gotcha 18), so pass 2 must replay the list as written.

Two ## Category: sections naming one category are one category, and the merge is a concatenation performed in pass 1 (ParsedFile::push_category, improvise-349m). Merging in pass 2 — building each section in turn — was not idempotent. A group is created either by the item that names it or by a > line, and build_category creates its own section's item-groups before its own > lines; so the first section's > groups landed ahead of the second section's item-groups, while format_md writes one section with all the items before all the > lines. The reparse then built the other order, and the two format_md outputs differed. Concatenating first makes the built order the written order by construction. Refusing a duplicate header was the alternative and would have made a legal hand-written file an error for no gain.

A section header takes its name as the whole line, totally

header_name = { pipe_quoted ~ &NEWLINE | line_name } (improvise-pczv). The &NEWLINE lookahead is load-bearing: without it, ## Category: |A|B parses as A and silently drops B. line_name catches everything else, so a header name can no longer fail to parse. Four rules share it — model_name, initial_view, category_section, view_section — which is why it is one rule and not four copies.

The writing half is format.rs::quote_header, and it is deliberately not quote_name: that rule is for a name embedded among other tokens, and applying it to a header would pipe-quote every ordinary spaced title the line carries perfectly well, rewriting the header of every existing document for no correctness gain. quote_header quotes exactly four things, each one thing the reader does:

  • the empty name, and any name with leading or trailing whitespace — the reader trims, so the category comes back renamed while every data coordinate still names the untrimmed original: silent orphaning;
  • a name containing \n or \r — it would start a second line, and Region\n## Data a second section;
  • a name already spelled |…| — the reader would take the pipes off.

So a header is written raw where a reference is quoted. The grammar accepts both arms, so that asymmetry is a writer convention, not a format rule.


Storage is whole-document read/write, and that closed the SQLite question

Saving or loading a workbook only ever touches one whole document, so a backend needs no seeking, metadata or partial reads (improvise-6mq). It does need to enumerate and remove documents, because a browser build has no OS file manager behind it — hence list and delete beside read and write (improvise-i34).

Paths are opaque document names, not a directory tree: a backend may treat them as filesystem paths, OPFS keys or map entries. The trait doc says "backends that are not trees treat dir as a name prefix" — keep that sentence reachable. It is the sentence that turned out to be right while the OpfsFs impl was wrong, and the bug was found by reading it.

Because the trait is two ops over whole documents, an sqlite3_vfs — which needs xRead(offset,len), xWrite, xTruncate, xLock — has nothing to bind to. See gotcha 41 for the measurements that declined improvise-9ix.

storage_contract is a public function, not a test helper

It reads like a test helper and it was one: #[cfg(test)] in storage.rs, run against MemoryFs and PhysicalFs so they could not drift. The one backend that most needed it could never be handed to it — OpfsFs is cfg(target_arch = "wasm32") and its sync access handles exist in no host harness — so for that backend the contract lived only in a doc comment.

Moving it out of cfg(test) is what lets a browser run this sequence rather than a second transcription of it (improvise-vvl3/xf1). A second copy of a contract is the disease this codebase keeps curing. It returns Result rather than asserting because a failure has to travel out of a Web Worker as a string; the host tests .unwrap() it and get the same message.

Two bugs fell out of its first actual execution — see the dir parameter and the index rules below.

dir is a parameter, not a constant, because the three backends must agree at every dir, not only at the flat root the browser happens to use (improvise-vhcz). A backend whose list answered in a different path shape passes at Path::new("") — where joining is the identity — and fails at anything else.

impl Storage for Rc<T> lives in improvise-io because of the orphan rule

OpfsFs keeps four genuinely async operations — open, attach, refresh, commit_deletes — off the trait, so Storage can stay synchronous (gotcha 28). But App::storage is a Box<dyn Storage>, and a trait object can name none of the four: handing the backend to the App puts the async seam permanently out of reach. The host must hold the concrete backend and give the App a Storage, which means sharing one.

impl Storage for Rc<OpfsFs> is two foreign types in any other crate, so improvise-9x6 wrote a forwarding newtype in the worker and every future worker would have written it again. Here the trait is local, so the impl is allowed, and it is blanket rather than OpfsFs-specific because nothing about it is OPFS-shaped: it is the general statement that a shared handle to a backend is usable as that backend (improvise-hmws). Rc and not Arc because the one caller is a Web Worker — a single thread, and wasm32-unknown-unknown has no others. An Arc impl would be speculative.


OpfsFs: why a browser backend can be synchronous, and what the index holds

OPFS has two APIs. The promise-based one (getFileHandle, createWritable) is async, but FileSystemFileHandle::createSyncAccessHandle() yields a handle whose read/write/truncate/flush/close are ordinary blocking calls. It exists so wasm can do blocking file I/O — it is what SQLite's wasm build uses — and is available only inside a dedicated Web Worker, which is where improvise-9x6 puts the whole app. So Storage never had to become async.

Acquiring a handle is still async, as are directory listing and removal, so the backend splits in two:

  • async, explicit: open, attach, refresh, commit_deletes, called by the worker from its message loop where awaiting is free;
  • sync, via the trait: read/write use an already-attached handle; list answers from an index; delete drops the name from that index and queues the removal.

So a delete is visible to read and list immediately and lands in OPFS at the end of the batch. That ordering is the price of a synchronous trait and is invisible to a single worker. write therefore un-queues any pending delete of the same name: otherwise delete-then-write would be listed and readable and then removed by commit_deletes at the end of the batch — the one ordering the deferral can get wrong.

The index holds documents, not entries

Storage::list promises document names, and documents and entries stopped being the same thing the moment attach had to create a file to get a handle for a document that does not exist yet (improvise-vvl3). Three writers, and the split between them is the whole rule:

  • attach indexes nothing. What it creates is zero bytes. Indexing there made a fresh origin list demo.improv before anything had been written, so the seed was skipped as "already there" and load_from got an empty buffer.
  • Storage::write indexes the name (and un-queues a pending delete). A written document is the first thing there is to list.
  • refresh rebuilds from OPFS, skipping zero-byte entries — and counting an entry it cannot measure as a document, since the one cause of that is another tab holding the exclusive handle.

holds_bytes is where the last clause lives, and its two failure modes get opposite answers, which is the whole function:

  • getFileHandle fails → the name is not a file. keys() yields directories too, and a directory rejects with TypeMismatchError; an entry removed between keys() and here rejects as well. Neither is a document and neither can ever be attached or read, so listing one is the same picker bug this filter exists to remove (Copilot, PR #24).
  • getFile fails on a real file handle → another context of the same origin holds the exclusive sync access handle (two tabs). An entry somebody else has open is certainly a document, so this fails open. Dropping it would hide a real document; failing the whole refresh would let a second tab break the first one's picker. Over-reporting costs one row — the direction Dirty::of is deliberately sloppy in.

Handles are exclusive per file, so attach closes any handle it replaces and detach releases the lock.

list joins dir like the other backends

The join is not cosmetic (improvise-vhcz). MemoryFs and PhysicalFs answer with dir-joined paths — paths a caller can hand straight back to read — and OpfsFs used to answer with bare names. At Path::new("") the two shapes coincide, which is the only dir the worker ever passes, so the disagreement was invisible: a worker that organised documents under docs/ would have got a full picker in every host test and an empty one in a browser, with nothing saying why.

dir is decoration rather than a filter, because this backend is flat by construction: entry_name reduces every path to its file name, so docs/a.improv and a.improv are the same OPFS entry and an entry name may not contain a separator at all. That is the trait's own "treat dir as a name prefix" — the doc was right and the impl was wrong.


Loading: decompression and decoding are two failures with two answers

load_from decompresses, then decodes UTF-8, with a separate Context on each. read_to_string would fuse them, and a valid gzip stream holding invalid UTF-8 would then be reported as unreadable gzip — a wrong diagnosis of a document that decompressed perfectly well. Decoding once, afterwards, also leaves one answer to "is this UTF-8" rather than one per branch.

The .gz suffix selects gzip regardless of backend: that is a property of the document name, not of the filesystem (improvise-6mq). Legacy JSON is detected by a leading { and is never written.

parse_number_cell returns CellValue::Error naming the offending text rather than a fabricated 0.0. No token the current grammar admits actually fails f64::from_str (overflow saturates to ±inf), but if the two ever drift the mismatch must surface — never silently store a wrong number.


Persistence write order and the virtual views

format_md writes Views → Formulas → Categories → Data: views first because they are typically small and orient the reader, data last because it is the largest section. The parser tolerates any order (see the two-pass note above).

_Records and _Drill are scratch views the app rebuilds on demand. They put _Index/_Dim on axes, and those categories are never written — so persisting the views would produce axis lines naming categories the loader will not find. They are skipped on write, and dropped on load if an older file carries them, rather than reinstating a view the app would rebuild anyway; Initial View then falls through to the default, since the name it pointed at is gone. persisted_initial_view is the writing half: saving from records or drill mode falls back to the first real view rather than naming one the file will not contain (improvise-60z).

_Measure persists only its non-formula items — formula targets are rebuilt from the ## Formulas section, so format_md collects the targets and excludes them.

Page and Filter both carry the selected item after the axis keyword; the other axes have nothing to select. Axis::Filter round-trips even though it occurs only inside the never-written _Drill view, because the alternative was silently writing a Filter axis as some other axis (improvise-dz8, gotcha 27).


Import: order is a correctness property, not tidiness

analyzer::analyze_records emits distinct values in first-seen order, never HashSet order (improvise-7yx, gotcha 18). build_model adds items in exactly that order and IndexMap preserves it, so a hash order made the same CSV import to a different document on every run — different .improv bytes, different row order on screen, and a different detected date format, since detect_date_format only inspects the first ten samples and which ten it saw varied. The HashSet in analyze_records is there to deduplicate and never to iterate.

The ambiguous-date heuristic reads American-first, deliberately

01/02/2025 parses under both %m/%d/%Y and %d/%m/%Y and nothing in the data can tell them apart. detect_date_format is first-match over DATE_FORMATS, so the American reading wins. That is a defensible answer and not obviously the right one: a European CSV imports with days and months swapped, silently, and the derived _Month/_Quarter categories inherit the error. What makes it tolerable is that the wizard shows the detected format and lets the user change it — the same argument two-digit years already rested on. It is pinned by a test so that changing the answer is a decision somebody makes rather than a side effect of reordering the table.

Ruled a known limit (improvise-l8rh, the user, 2026-09-20). Two things the ruling rests on. Changing the default to day-first only moves which half of the world imports silently wrong; and the honest fix is larger than the heuristic — an ambiguous column should surface its ambiguity rather than resolve it, which changes FieldProposal's shape and the wizard step that displays it, the way Dataset::warnings returns a decision the user should hear about as data. Worth doing deliberately, not as a patch.

What makes the limit reasonable to live with is that it is now deterministic: analyze_records iterated a HashSet until improvise-7yx, so the ten samples detect_date_format inspects were a different ten every run and a column whose eleventh value ruled out %m/%d/%Y detected a different format run to run. The ten-sample window itself is unchanged and is part of the limit.

CSV parsing

Short rows keep every header column, missing trailing fields filled with Value::Null, and each affected column is reported as a warning with a row count. The reader is .flexible(true), so without this a short row parsed cleanly and the trailing columns were simply absent from the record map — no error, no warning, and skewed value counts in field-kind analysis (improvise-k8i).

A header name becomes a category name and survives into the .improv file, so anything riding along invisibly in one — a byte-order mark, a stray carriage return — is silent corruption rather than cosmetics. That is why the adversarial inputs are tested with controls: a check that cannot see a BOM where there is one proves nothing.

parse_csv_bytes_with_warnings exists so CSV import can come from any Storage backend rather than only a file (improvise-fuj); the path-taking wrappers remain for the native CLI.

The wizard's back path is a history stack

advance pushes the step it leaves; previous pops it. Keeping the path actually taken, rather than re-deriving the reverse of each conditional transition, means back can never disagree with how the user really got here (improvise-907).

Preconditions live in validation_error() and are checked on the step where the user can still fix them — never deferred to build_model after the final confirm. Two gates earn that: too many categories (improvise-mzv) and no category at all (improvise-gfq); both are fixable with c at ReviewProposals, and both used to refuse only at the last keystroke. MAX_CATEGORIES here mirrors the model's private limit and counts regular categories only.

NameModel is terminal for advance: leaving it is a build, not a step change. build() parks the workbook in built on Done; a build failure leaves the user on NameModel with the error. Going back from Done discards built, so accepting an import always reflects the latest field decisions.

scalar_name decides what a JSON scalar contributes as an item name. Value::Null — which is also what the CSV parser produces for a blank field — 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).


CSV export

render_csv is separated from writing so the bytes can go to any Storage backend, not just a file (improvise-fuj). It goes through the csv crate rather than joining with commas, and it honours the view's number format — a value of 1200 under a comma-grouping format renders 1,200, and a label like Smith, Jr. splits into two fields, corrupting every row (improvise-1cz).