- AGENTS.md: "Three files" is now "These files"; the table lists five. - Digests 43 and 45 regain the traps the review found missing. - protocol.md and formula-and-io.md cite file-inventory.md for the Wasm appendix and the inventory, which moved there. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
65 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/file-inventory.md
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 whenis_bare_nameallows, else pipe-quoted.pipe_quote— unconditional. A.improvtext cell goes through this, not throughquote_name, because a bare42would read back as a number.unescape_pipe_body— the inverse, over the body between the delimiters (what both grammars'pipe_innermatches). 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 on purpose (12 then, 24 since improvise-felo.1), 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.
Folds are view lines: collapsed: and expanded(N): (improvise-felo.6)
Groups start collapsed, so a view persists what it opens. expanded(N): Cat
opens a category's top N levels, expanded(N): Cat/Group opens that group and
N-1 levels beneath it, a bare expanded: is N = 1, and collapsed: Cat/Group
keeps its old meaning. The semantics, the precedence and the refusals are
context/code/core.md §19; what is particular to the format:
- No version bump. The line is additive, and the user ruled that old files
open closed like new ones, so
v2025-04-09still reads every file it did. - N >= 1 is grammar, not validation:
depth_number = @{ '1'..'9' ~ ASCII_DIGIT* }. Spelled with aRangeandASCII_DIGITrather thanASCII_NONZERO_DIGIT, because the generator knows the first two and would emit nothing for the third —expanded():— a file its own grammar refuses. axis_linecomes beforeexpanded_lineinview_entry, so a category namedexpandedkeeps its axis line, and the writer never spells a category depth bare (expanded: rowwould be that axis line).- Two refusals the grammar cannot express — a cycle among
>lines and one group given two folds — are a typedRefusal, whichgenerated_file_parsesaccepts and nothing else does. The generator produces both at ordinary rates. - The generator reaches the new rule, asserted by
the_generator_writes_expanded_lines_with_and_without_a_depthover 512 deterministic files. Unlike the kind-line probe above, it asserts that each spelling occurs at all, not a rate, so a proptest bump fails it only if the walk stopped reaching the rule.
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_funcandparse_cmp_optake the matched text, becauseagg_funcandcmp_opare each one pest rule over string alternatives andpest_derivegenerates no per-alternativeRule. Nothing for amatchto be exhaustive over, so they spell every alternative andunreachable!(GRAMMAR_INVARIANT)on anything else -- loud at the first parse rather than silent forever (improvise-492x). - A
Rule, where one does. The four dispatches onPair::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 toformula.pestis 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
\nor\r— it would start a second line, andRegion\n## Dataa 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/writeuse an already-attached handle;listanswers from an index;deletedrops 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:
attachindexes nothing. What it creates is zero bytes. Indexing there made a fresh origin listdemo.improvbefore anything had been written, so the seed was skipped as "already there" andload_fromgot an empty buffer.Storage::writeindexes the name (and un-queues a pending delete). A written document is the first thing there is to list.refreshrebuilds 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:
getFileHandlefails → the name is not a file.keys()yields directories too, and a directory rejects withTypeMismatchError; an entry removed betweenkeys()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).getFilefails 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 wholerefreshwould let a second tab break the first one's picker. Over-reporting costs one row — the directionDirty::ofis 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).
.improv text is the only native format (improvise-tt22). A legacy JSON
workbook used to be detected by a leading { and loaded read-only through
Workbook's serde derives. That path is gone, on the user's direction on PR #210.
A document that still starts with { is refused with an error that names the
format and says it is no longer supported. The test is
a_legacy_json_workbook_is_refused_by_name. The error is decided rather than
defaulted, because otherwise parse_md would report a grammar error at line 1,
which diagnoses the wrong thing: the file is not malformed .improv. The check
is on the decompressed text, so .improv.gz is covered too.
The serde derives that existed only for that path went with it: Workbook,
Model, View, Category, Item, Group, CategoryKind, CellKey,
DataStore's manual impls and the formula AST. So did indexmap's serde feature
in improvise-core, improvise-io and the root crate, and improvise-formula's
serde dependency. Each was removed and the whole workspace rebuilt, so the
compiler named every other consumer. Two remained, and both are the wire:
Axis (whose rename_all = "lowercase" is its wire spelling) and PageAxis.
A third arrived with improvise-km71 by the same rule: FormulaId, which crosses
the wire in ModelCommand::ReplaceFormula, FormulaSlot and FormulaEntry.
So improvise-formula depends on serde again, for that one type. No derive went
back on the formula AST. A fourth, CategoryKind, went back on when TileEntry
began carrying it (improvise-bqm0; context/code/command.md). It returned as a
wire type, not as a document one: nothing loads a Category through serde.
CellValue was never a candidate, since it is carried on the wire too. JSON
import (StartImportWizard, ImportJsonHeadless) is a separate feature and is
untouched.
One test consumer had to move: the effect-classification tests fingerprinted the
model and the views through serde_json. They now use Debug. format_md was
rejected as the replacement, because it skips the virtual views and sorts the
data, so it would have been a weaker fingerprint.
The first version of this paragraph said Debug was "no weaker" because it
renders every field, including the derived formula_cache the JSON skipped.
That was wrong (improvise-pc3b). A derived Debug prints a HashMap in hash
order, and recompute_formulas clears and refills the cache. So a correct
ViewOnly effect such as RebuildLayout changed the fingerprint in 90 of 200
runs. The suite passed only because the fixture had no formulas, so the cache
was empty. Model now has a hand-written Debug that renders formula_cache
and measure_agg in key order. The fingerprint keeps every field, including the
derived one, and a refill in a different hash order is no longer read as a
change. recomputing_formulas_leaves_the_model_fingerprint_alone pins this.
view_only_effects_leave_shared_state_alone now runs over a formula-bearing
fixture, for the same reason.
That fix was canonical within one instance, not across two (improvise-djcn).
It fixed Model's own maps but not the maps inside its fields. DataStore has
index, records and the symbol table, and View has page_selections,
hidden_items and collapsed_groups. All still printed through a derived
Debug, in hash order. Each HashMap gets its own RandomState, so two equal
documents built separately printed differently in 32 of 32 runs.
- The effect-classification tests compare one instance with itself before and after, so they were unaffected.
a_model_command_lands_where_its_keypress_landscompares twoApps. It passed only because its fixture had no cells and no multi-entry view maps.
Now:
DataStore,SymbolTableandViewhave hand-writtenDebugimpls. Each sorts wherever its storage is a hash map.DataStoreresolves symbols to their strings. ASymbol's number is interning order, not content, so printing it would make two equal stores differ again.SymbolTableprints its interned strings, sorted.cellskeeps itsIndexMaporder, because that order is part of the document (gotcha 18).- Every hand-written
Debug,Model's included, destructuresself. A field added later is then a compile error there, where adebug_structthat names fields one by one would skip it silently. The pc3b version ofModel's impl had exactly that hole.
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.
A date column imports as ONE hierarchical category (improvise-8jk6.3)
The user: "I think dates should usually be one category and we can use
formulas to crosstab." So the default DateShape::Hierarchy builds one
category named after the column, where the old import built up to three flat
ones beside it (Date_Year, Date_Quarter, Date_Month):
- Its items are the finest chosen component, spelled so they stay unique
across years:
2024-01,2024-Q1,2024, which areextract_date_component's existing spellings. A month is neverJan. - The coarser chosen components are its groups, coarsest first
(
DateComponent::COARSE_TO_FINE): a month sits in its quarter, and the quarter's>line names its year. A level left unchosen is skipped rather than drawn empty, so Year + Month groups months straight into years. - It opens folded, one row per year, with nothing to set, because every
group starts closed (
context/code/core.md§19). - It costs one of the category slots, not up to four, and
proposed_category_countcounts it that way. - The raw dates are gone from that category. The day is not a component, so
two records of one month land on one cell and the collision step asks, as it
does for any other collision.
Splitkeeps the day. - A value the detected format cannot read keeps its raw text as an
ungrouped item, with a warning naming the column and the row count, rather
than being dropped. The warning is separate from the split shape's, because
that one reports an absent coordinate and this one a coordinate that is there
and ungrouped. Unless its spelling is one the hierarchy generates
(improvise-d0n6): a raw
2024-01or2024is the same name as the generated month or year, so it would join the grouped item and the warning would be false. The build refuses, naming the column and the spellings and pointing at the split shape. That follows the collision rule below: where two records would silently be one thing, the user is asked and nothing is defaulted. A fallback to the split shape was rejected as a silent change of the user's choice, and a renamed raw item as altering the data. The check isPlan::spelling_clash. - Item order is first-seen, not chronological. Order is a correctness property here (gotcha 18), and a group need not be contiguous, so a CSV that is not in date order still nests correctly. It is just drawn in the order it was read.
DateShape::Split is the old behaviour, unchanged, and stays a choice. s on
the wizard's date step toggles the field under the cursor, and the header line
for each field says which it will get ("one category: Year > Quarter > Month"
or "split: Date, Date_Year, …"). It is worded once, in
project.rs::date_shape_sentence. Headless, --split-date <field> does the
same, and --extract keeps its meaning: which components to build.
Crosstabs by formula are not available yet. A formula can name a month
(SUM(Amount WHERE Date = "2024-01"), Date IS "2024-01"), but a WHERE or
IS naming a GROUP matches no record: both compare the coordinate to the name
exactly. SUM(Amount WHERE Date = "2024") is an empty sum, and Date IS "2024"
is 0 on every month of 2024. That was measured on this branch and reported for
filing, not built here.
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.
A CSV field is text, and the field's role reads it (improvise-8jk6.4)
The parser used to type every field as it read it: text that parsed as an
i64 or a finite f64 became a Value::Number. That is a lossy step, and it
ran before anyone knew what the column was for. A column the user then forced
to Category (--category, or the wizard's c) took its item names from the
number's display, not from the file. So commit 00565796 became item 565796,
and 220944e2 was read as scientific notation and became 22094400.0. The
repo-stats pipeline (improvise-1xfb.5.1) found it.
CSV has no types. Whether 00565796 is a number depends on whether its column
is a measure, and the parser cannot know that. So the parser now yields every
non-empty field as a Value::String of its text (blank is still Null), and
two readers apply the old numeric rule, csv_parser::number_in, only where a
number is wanted:
- Analysis proposes a kind from a typed copy of the records
(
Scalars::typed_record), so a numeric CSV column is still proposed as a Measure, exactly as before. - A measure takes its value through
Scalars::number.
A category or a label is named from the text itself, so it is verbatim.
ImportPipeline carries the reading as a private scalars: Scalars:
Typed for JSON and Text for CSV. JSON cannot share the text rule, because
a JSON string is text by the file's own say. "02139" in JSON is a zip code,
and reading it as a number would propose it as a Measure. A JSON number
forced to Category has no source text to keep, because serde_json does not
keep one. It is named by Value's own display, which is what writing it back
as JSON would say: 42, 1.5, and 1000.0 for 1e3.
The parser returns CsvRecords, not Vec<Value>. It derefs to
[Value], so a caller can read it. It converts to a Vec only inside the
crate, and the only public way into a pipeline is ImportPipeline::from_csv.
This is not caution for its own sake. When the parser first changed, eleven
tests in four files failed. Every one handed parser output to
ImportPipeline::new, the JSON constructor, which would read every column as
text and propose Revenue as a Category. Production had the same shape at all
three CSV entry points: main.rs's single file, ImportPipeline::from_merge
(the merge path, from PR #314), and ImportJsonHeadless. from_merge merged
in with Self::new and now calls Self::from_csv. Now that mistake does not
compile. Two holes remain, and
both take a deliberate step. ImportPipeline::records is still a public
Vec<Value>, and a test helper once copied it out of a CSV pipeline into a
JSON one. And CsvRecords derefs to a slice that .to_vec() will copy.
An import refuses what it cannot keep (improvise-8jk6.5, improvise-8jk6.6)
Two failures used to end in an import that saved and reported success.
A formula that does not parse. build_model_with_warnings ran
if let Ok(formula) = parse_formula(..), so a bad --formula vanished. The
headless run then saved and exited 0, and the wizard kept the bad text in its
list and dropped it the same way at build. Both now go through one
function, import_formula, whose error names the formula and carries pest's
parse error: Formula error in 'Y = x +': …. The prefix matches the TUI
formula minibuffer's Formula error:.
ImportPipeline::parsed_formulasparses them all. If any fails, it refuses and names every formula that failed, one per line (a message the user needs is never truncated). The build calls it before creating anything, so a refusal writes nothing.- The wizard checks at
confirm_formula, the one place the user can still fix the text (gotcha 22). A formula that does not parse stays in the buffer with the error as the step's message, and never enters the list.
Nothing kept. The one skip left after improvise-8jk6.2 is a record that
is not a JSON object. When every record is skipped, there are no fields, so
there are no proposals and no categories. The build did refuse that case, but
by accident, with "At least one category must be accepted". That tells the
user to change a field's kind when there are no fields. The ticket's own repro
([1, 2, "three"]) already exited 1 on the lpzu base, so the headline did not
reproduce there. What did reproduce was the reason. nothing_to_import now
states the rule and runs before the category check: Nothing to import: all 3 records were skipped as not JSON objects, or the source has no records. The
wizard asks it first on ReviewProposals, where another array path can still
be chosen.
One case meets PR #314's "a header-only file is still a file". A folder whose
CSVs are all header-only has no records, only sources. Built from
from_merge's seeded proposals alone, it would save File items, empty
columns and one empty records view per file. It is refused here, because it
kept no record. A header-only file beside files with rows is unaffected,
since some record was kept. The user confirmed this on PR #321: "Refuse (as
#321 does)".
A partial skip stays a warning. Some records kept and some skipped still
builds, exits 0, and warns with a count: Skipped 1 of 3 records that were not JSON objects. The argument: a non-object in an array of records is not a
record that failed to import. It is structurally not a record, such as a
trailing null or a stray scalar in a hand-edited file. Every record that
is a record was kept, and the count is on stderr for a script to read.
Making it fatal would fail every import that carries one stray element.
Making it a distinct non-zero exit that still writes gives a contract where
import … && next stops even though the file was written.
The user ruled on PR #321: "Add a --strict flag". The default stays exit 0
with the counted warning, pinned by
an_import_that_skips_some_records_warns_with_a_count_and_still_writes.
import --no-wizard --strict turns any skipped record into a refusal:
--strict: Skipped 1 of 3 records that were not JSON objects, exit 1.
A strict refusal writes nothing. A refusal that still wrote the file would
be the half-met contract argued against above: && stops while the output
sits there, and a script that checks only the file gets a model that --strict
existed to prevent. So the check runs in run_headless_import before the
build. It reads ImportPipeline::skip_warning, the same function the build's
warning comes from, so the two cannot disagree about what was skipped.
"Skipped" here means a record that is not an object, the only skip left after
8jk6.2. An unparsed date or a short CSV row is a warning about a record that
was kept, and --strict does not refuse it.
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/readsa.csvtwice, 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
Fileproposal'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
_Recordsand_Drill, gets2,3and so on. A view named_Drillwould never be written and would be dropped on load. - Puts
FileonAxis::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_Drillview, but the.improvgrammar has always round-tripped it.per_file_views_survive_a_round_trip_and_repeat_byte_for_byteproves that on a saved per-file view, anda_saved_per_file_view_projects_only_its_fileproves the sharedgrid_inprojection, 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 onNone. The file's records lack those, so nothing splits. - Puts
_Measureon Page, asWorkbook::newdoes, 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.csvandDefault 2.csvbecomeDefault 2andDefault 2 2; without the same-batch check both would be namedDefault 2and 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
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 is the
model's own constant, which counts regular categories only. Until improvise-felo.1
it was a private copy (core.md §9).
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 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)
Two source records that land on the same coordinate tuple are one cell, so
something has to happen to the second one. improvise-2cw9 made that "sum and
warn". The user ruled on 2026-09-25, after an Etrade import summed 31 cells and
reported it in one truncated line, that the wizard asks. It offers three
choices: (a) sum them, (b) keep them separate with a generated category, or
(c) go back. So ImportPipeline::on_duplicate is a DuplicatePolicy
(Sum | Separate(name) | Refuse), and its default is Refuse. Any caller
that never asked therefore gets an error and never a silent sum.
ImportJsonHeadless builds its pipeline as a struct literal, so the compiler
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. 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
tuple plus a measure (improvise-9bit). A Collision names the tuple and only
the records that share one of its cells. A record with no measure makes no
cell, so it collides with nothing. The tuple includes label values, as
CellKey always has.
This PR's first version keyed on the tuple alone. It was superseded in review
(improvise-9bit), and the argument it gave is withdrawn. Under that rule, two
records with disjoint measures on one tuple (one has Revenue, the other
Cost) counted as colliding although no cell sums. The wizard then offered
"Sum them into one cell" where nothing would sum, and with the Refuse default
the CLI refused a sparse-measure CSV that had always imported cleanly. The
question the user is asked is "these values would be added together", so the
rule has to be the one that decides whether anything is added.
The data is structured. A Collision holds coords plus 1-based records
positions. WizardCache::collisions carries it, and
WizardCollision::describe is the one wording, used by the TUI and by the CLI.
A browser wizard must draw the same rows (gotcha 32), not a second sentence.
Where the step sits. It comes after ReviewProposals and after
ConfigureDates when that step is visited, and before DefineFormulas. It
appears only when collisions() is non-empty, and the list is recomputed each
time the step is entered, because it depends on the field choices. The two
field steps are exactly the steps that can change it. Date parts are functions
of a value that is already a coordinate, so they can never split or merge a
collision, but they count toward the category cap, and that is why the question
waits until they are known. The history stack (gotcha 22) handles the new
paths without change: ResolveDuplicates → NameDuplicateCategory → DefineFormulas retraces backwards. The chooser acts on the cursor. Enter
on this step is ChooseDuplicate, never a bare Advance: the policy starts
at Refuse, which validation_error blocks, so a bare advance could never
leave the step.
Separate is an occurrence index, not a row number. This is the user's
preferred shape. The records in a collision get 1, 2, 3… in source order,
and every other record gets 1. That includes a record on the same tuple that
shares no cell with the others: it lands on occurrence 1 beside the first
colliding record, and since the two write different measures, nothing sums. The category holds only as many items as the
deepest collision, rather than one item per record, and every existing
coordinate keeps its meaning. Items are added 1..=n in numeric order
(gotcha 18). The category goes on Axis::None. Left to
on_category_added it would land on Page, and the default grid would then
show only occurrence 1, which silently hides the other records. On None
the grid aggregates across occurrences, so the first screen shows the same
totals Sum would. The records stay distinct, so Avg and records mode see
each one. A policy is moot when nothing collides: no category is added.
The name is suggested and editable: Occurrence, or the first
Occurrence_N that no header (accepted or not) and no category the build will
create already uses. NameDuplicateCategory refuses an empty name or one that
is already a category. suggested_duplicate_category lives on the pipeline, so
the CLI's separate uses the same rule.
At the category cap, Separate is shown and disabled. The reported import
had 12 regular categories, which was MAX_CATEGORIES when this was written.
Option (b) adds a
regular category, and the user declined a hidden virtual one. So at the cap
the option is drawn "(unavailable)" with its reason and remedy: go back and
Skip a column or make it a Measure, or choose Sum. The reason is decided in
ImportWizard::duplicate_options, so every front end words it the same way.
This was the conservative reading, and it left an open question for the user.
The user answered it on 2026-09-25 by raising the cap to 24
(improvise-felo.1), so "keep separate" is available for the reported import
now. The option is still disabled for any import that fills the new cap.
Headless paths refuse by default. improvise import --no-wizard has
nobody to ask, so --on-duplicate refuse|sum|separate defaults to refuse.
It prints every colliding tuple and the flag, exits non-zero, and writes
nothing. Summing by default would repeat exactly the silent outcome the user
called critical. Separating by default would reshape a scripted import's
document and fail outright at the cap. A refusal is the one default that
cannot be wrong without saying so. This changes behaviour: a script that relied
on 2cw9's implicit sum now needs --on-duplicate sum. :import-json has no
flag and refuses, naming :import.
The 2cw9 argument that summing is the right default is superseded here, as its own evidence entry anticipated. Its warning survives as the Sum confirmation.
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).