This commit implements the download flow for SaveAs and ExportCsv per the
USER RULING on improvise-2mqz. The correct shape is:
- A request frame asks the host for BYTES
- ServerFrame returns the bytes (or Rejected)
- Browser triggers downloads via Blob + <a download>
Key changes:
Protocol layer (improvise-protocol):
- Add Request::DownloadDocument and Request::DownloadCsv variants
- Add ClientFrame::DownloadDocument and ClientFrame::DownloadCsv
- Add ServerFrame::DocumentBytes and ServerFrame::CsvBytes
- Add KeyOutcome::LocalThenRequest variant for operations that need
both local fold (mode change, buffer clear) AND a server request
- Update stranding() to unstrand SavePrompt and ExportPrompt modes
- Update keymap to use rebind() instead of commit() for commit-save
and commit-export (because LocalThenRequest can't be in a Sequence)
Host layer (improvise-command):
- Handle new ClientFrames in host.rs returning bytes or Rejected
- Add Rejection::ExportFailed variant for CSV export failures
- Update CommitSave and CommitExport to clear their buffers
- Make render_csv() public in improvise-io for host use
WASM client layer (improvise-wasm-client):
- Add PendingDownload type with filename, bytes, and kind
- Store pending downloads for JavaScript retrieval via take_pending_download()
Browser layer (web/app.js):
- Add triggerDownload() function using Blob + <a download>
- Poll for pending downloads after frame handling
This unstrands SavePrompt and ExportPrompt which were previously refused
by stranding() due to missing wire forms.
Refs: improvise-2mqz
Co-authored-by: Ed Langley <fiddlerwoaroof@users.noreply.github.com>
- 8jk6.6, the user's ruling on PR #321 ("Add a --strict flag"): the
default stays exit 0 with the counted warning. With --strict, any
skipped record is a refusal that exits 1 and writes nothing. The check
runs before the build and reads ImportPipeline::skip_warning, the same
function the build's warning now comes from.
- 8jk6.17: the headless suite now sees three faults that only io unit
tests caught at efaa5050:
- naming only the first bad formula (two bad --formula flags);
- dropping CSV decimal measures;
- import-json reading a CSV through the JSON constructor.
- Docs record the user's three answers.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
After merging PR #314, a header-only file's seeded File proposal means the
category check alone no longer refuses an import that kept no record.
nothing_to_import does, and this pins it. The docs now name from_merge
(switched to from_csv in the merge) among the CSV entry points.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
- 8jk6.4: the CSV parser yields source text (CsvRecords), and only the
role reads it as a number. A forced category keeps 00565796 and
220944e2 verbatim; measures and analysis read the text through
csv_parser::number_in. CsvRecords reaches a pipeline only through
ImportPipeline::from_csv, so handing CSV text to the JSON constructor
no longer compiles.
- 8jk6.5: an import formula that does not parse is refused by name with
its parse error: the build refuses (headless exits non-zero and writes
nothing), and the wizard keeps it out of the list with the text still
in the buffer.
- 8jk6.6: an import that kept no record refuses and says why, from the
build and from the wizard's review step. A partial skip stays a
warning with a count and exit 0.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
ImportPipeline::from_merge seeds the File items and every file's columns
from Merged::sources, so a header-only CSV keeps its File item and its
columns (unseen columns are proposed as Labels, as an all-null column
is). A file with no measure column gets a records view filtered to it
instead of paging onto another file's hidden measure. Four per-file-view
rules the review found untested now each have a test that fails when
the rule is broken.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
A CSV merge now records each file's columns, and the build adds one view
per file: File on Filter at that file's item, its own categories seated
as a fresh import seats them, the rest on None, and other files'
measures hidden. The merged Default view stays the initial view. The
headless and wizard paths both carry the sources.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
A folder argument expands to its *.csv documents through Storage::list,
and a record with no value for a category keeps its cell without that
coordinate instead of being skipped.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
The duplicate step's cap check already reads the one constant after the
merge; its tests filled 12 categories and now fill to the cap. An
injected literal-12 check fails keeping_duplicates_separate_is_refused_at_the_category_cap.
core.md section 9 gains the inventory of what degrades as categories grow.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
The wizard's private copy of the limit is gone; it reads the model's
now-public constant. Tests that fill to the cap derive it. Why 12 was
chosen and what 24 costs: context/code/core.md section 9.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Every wizard write goes through Panel, which clips to the panel and saturates
its height arithmetic, so no step panics on a small terminal. The bottom row
truncates in cells, so a wrapped message with a multibyte character is drawn
whole. A scrolling list's range is named on its header row, where no item can
cover it. Tests: a multibyte message at every width, wrap bytes vs cells, a
no-panic sweep of every step at 20-100 x 3-20, the first item filling the list.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Records with disjoint measures on one tuple no longer count as colliding, so
the Refuse default does not refuse a sparse-measure CSV. Only the records
that share a cell are listed and numbered. The Sum confirmation lists every
summed cell rather than three and a count. Adds the date-derived name test.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
When source records collide on every coordinate, the import wizard now asks:
sum them, keep them separate with a generated occurrence category, or go
back. Collisions are found before the build by the same placement pass the
build uses, and are carried to the TUI as structured data (WizardCache) and
listed in full, scrollable. At the 12-category cap, "keep separate" is shown
disabled with its reason and remedy.
Every wizard warning and message now wraps to the panel instead of being
truncated, and long lists scroll.
Headless paths no longer sum silently: `import --no-wizard` takes
`--on-duplicate refuse|sum|separate` (default refuse), and `:import-json`
refuses and points at `:import`.
Reasoning: context/code/formula-and-io.md, "Colliding records are a
question, not a default"; context/code/tui.md; context/code/cli.md §11.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
improvise-6yvg reproduced. A JSON import in improvise-command and a legacy
JSON .improv load in improvise-io stored 0.24815714285714285 as
0.24815714285714283 in each crate's own test build. The root binary's
`improvise import` used the same lossy parser. Now float_roundtrip is on every
normal serde_json dependency in the workspace: root, improvise-command,
improvise-io, and the optional test-support dependency of improvise-protocol.
improvise-dz8c: the hand-listed cargo-tree check over three frame-parsing
crates is replaced by tests/json_numbers.rs. It reads
`cargo metadata --no-deps` and names every package, including target-specific
entries, whose normal serde_json dependency lacks the feature. The rule and why
it beats "parses a frame", the build graph, and enabling the feature once in
the protocol crate are in context/code/protocol.md.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
A collapsed group is written to CSV as its own total row, because an export
writes what the grid draws (improvise-iot6). core.md §14 carries the argument;
the evidence entries record each rival and where it failed.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
`Repr::Nonfinite(&'a str)` could only be produced by a deserializer that
borrows from its input, so the new wire spelling round-tripped through
`from_str` and `from_slice` and through nothing else. Measured on the
pre-fix head:
from_str {"Number":"inf"} -> Ok
from_str {"Number":"inf"} -> Err (the SAME JSON document)
to_value then from_value -> Err (does not round-trip its own output)
from_reader -> Err
The escaped form is the serious half: escaping is not a distinction JSON
makes, so the wire accepted a subset of the encodings of the spelling it
claims to share with the file. `Cow<'a, str>` fixes all four; the serialized
form is unchanged.
The PR's own tests could not see this — both use `from_str` exclusively,
which is precisely the one path that borrows. A check adjacent to the
question it names, inside the change that defines the question.
Also tightened `a_string_that_is_not_one_of_the_three_tokens_is_refused`: its
second disjunct accepted `data did not match any variant`, which is exactly
the message the above bug produces, so it passed in both worlds.
Conflict was `context/test-evidence/core.md` only, against #143. Both entries
kept and verified line-by-line — neither test cites the file, so a dropped
entry would pass `--check-evidence`.
Gates on the merge: 1728 passed / 0 failed / 7 ignored over 38 binaries,
clippy -D warnings clean, fmt clean, and `cargo check --target
wasm32-unknown-unknown -p improvise-core` clean — the one gate the review
asked for and could not run.
Found by adversarial review (confirmed by injection, not reasoned).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
The comment added with the previous commit ran to three lines and
scripts/check-comment-runs.py failed the `test` job on it. The reason it was not
caught locally is worth recording: --check-evidence is ADDITIVE, so the run does
answer the cap, but the output was read through `tail -1` and the cap's line is
the FIRST of the two. A gate filtered down to the line you expect is not a gate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Found by ChatGPT Codex reviewing PR #150 at af7fae0e, and correct. The tally was
incremented while walking a record's categories, before the record was known to
survive that walk. A row whose Date is unparseable and whose LATER required
category is missing is skipped entirely — it never lands in the imported model —
yet it had already been charged to the derived column. So the warning counted a
row nothing else in the import reports, and its value depended on the order the
categories happen to be walked in.
Mismatches are now collected per record and merged only after missing.is_none().
The regression asserts the fixture's category order explicitly, because it only
discriminates while Date precedes the missing category: a later change to
proposal order would otherwise make the test pass by never reaching the bad
date. Before the fix it reports three warnings where one is correct; the rival —
merging before the missing check rather than after — reproduces exactly that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
serde_json writes a non-finite f64 as null, and null does not read back as one,
so CellValue::Number(INFINITY) serialized to {"Number":null} and the frame
carrying it was unreadable. The websocket reader skips an unparseable frame and
the worker logs a console error, so one poisoned cell silently dropped a whole
batch — and to_string returned Ok, so the send site could not notice either.
The ticket offered two answers and the format had already chosen. Measured
before changing anything: inf, -inf and nan all round-trip losslessly through
.improv — format_number writes them, the grammar's bare_value accepts them, the
walker maps them back. So refusing them before sending would make a legal saved
document unprojectable to a browser, which is a new failure rather than a fix.
It also settles the ticket's second location: resolve.rs's typed() accepting inf
is correct, not half a defect.
Number now serializes through an untagged adaptor: a JSON number when finite,
one of the three tokens when not. A finite cell's bytes are unchanged, and a
JSON integer still reads as a number rather than as a token.
The tokens are one home — nonfinite_token/parse_nonfinite beside CellValue,
called by the adaptor and by persistence/{format,parse}.rs. Three copies stood
before and a fourth was about to be written. format_f64 is deliberately not
folded in: it is the display layer and these three are a format.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
extract_date_component returns None for a value the detected format cannot
parse, and the caller's `if let Some(..)` simply did not push the derived
coordinate — so the row silently left the derived Date_Month/Date_Year category
with nothing reported. Same shape as improvise-2cw9/jc95/5hfh: a value the code
cannot interpret became an absence rather than a report.
Narrower than the ticket reads, and the reason is a property of one function.
detect_date_format accepts a format only when EVERY distinct non-empty sample
parses, and extract_date_component makes the identical parse call — so through
the shipped CLI and TUI wizard a mismatch cannot occur. The reachable door is
ImportPipeline's `pub proposals`, where a library caller sets date_format
itself. a_column_of_mixed_date_formats_is_not_proposed_as_a_time_category guards
that reachability claim rather than leaving it as prose.
One warning per derived column, in the house wording csv_parser already uses.
The row still lands with the coordinates it does have: this reports, it does not
relocate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-dy3j (P1). CategoryKind::Label had no spelling in `.improv` at
all: format_md wrote every category as a bare `## Category: Name` and
build_category read every section back through add_category. The kind was
dropped on the way out AND on the way in, so a round-trip equality test
could not see it.
A label is outside MAX_CATEGORIES = 12 by design and the import wizard
scores it as zero, so twelve categories plus a label is legally
constructible. On load every label came back Regular, the thirteenth was
refused, and parse_md failed: any such document was permanently
unloadable. Below the limit the loss was quieter — a label silently
became a pivot category.
Grammar first. The ticket proposed `## Category: Note [label]`, and
improvise-pczv forbids it: a section header takes its name as the WHOLE
line, so the suffix is indistinguishable from a category named
`Note [label]`. The kind is therefore an entry inside the section,
`kind: label`, spelled with a real token rather than a lookahead — the
grammar-walking generator skips predicates. Absence means `regular`, so
every existing file parses unchanged and a pivot category's section is
byte-identical to before.
Tests assert the parsed Category::kind, never the emitted line. Both
rival faults were injected against the fixed tree and both fail two
tests: a writer that omits the line, and a reader that discards the kind
it parsed (whose file text is correct, so any text-comparing test passes
it). Reasoning in context/code/formula-and-io.md; evidence in
context/test-evidence/formula-and-io.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-e0ex. `Margin = Profit / Revenue` drew as `1`: `View::new` seeded
`,.0` — zero decimals — so a 0.66 ratio rounded half-away-from-zero. The cause
is the format, not the arithmetic; exact decimal math would not change the
rendering by one character, so nothing here touches the evaluator and gotcha 8
is unchanged.
Two halves, per the user's ruling:
* `View::new` seeds `,.2`. Revenue draws `18,400.00` and Margin `0.66`. The
noise on whole numbers is the cheaper error — a currency column with two
redundant zeroes is legible, a ratio with none is wrong — and a format cannot
be guessed from a value.
* A formula may carry its own format, which wins for its target; a raw measure
keeps the view's. `formula.pest` gains
`format_clause = ${ ^"FORMAT" ~ sp+ ~ format_spec }`, with `sp+` for `WHERE`'s
reason (a predicate is invisible to the grammar-walking generator) and
`FORMAT` added to `quote::KEYWORDS`.
`improv.pest` is deliberately unchanged: `formula_raw` already carries the
`## Formulas` line verbatim, so the format rides in the formula text with
nothing to add — and the generator, which reads `improv.pest`, sees no new
spelling to violate.
Resolution has one home. `FormatPlan::of(model, view)` replaces the
`(comma, decimals)` pair every render path threaded — `GridLayout::display_text`,
`viewport::grid_body`/`compute_col_widths`, `project::grid_in` and
`persistence/export.rs::render_csv` — rather than being added beside it, which
would be two answers to "what format is this cell". It is asked of the cell's
`CellKey`, so a cell aggregating over `_Measure` keeps the view's format by
construction. `GridBody::col_formats` carries the answer to the totals strip,
which is formatted on its own path.
Persistence is untouched except for metadata: `format_f64` is display,
`persistence/format.rs::format_number` writes round-trip precision, and the two
have no call between them. Newly created views write `format: ,.2`; existing
documents are unaffected, because `format_md` has always written the line
explicitly. `web/demo.improv` moves to `,.2` on both views.
The trap this sets is gotcha 34's: `format_md` writes `Formula::raw`, so a
`FORMAT` suffix round-trips whether or not the walker parsed it. The round-trip
test therefore asserts the parsed field — injecting a walker that drops the
clause fails that one test and nothing else in `improvise-io`, 242 tests
including the whole characterization suite and the grammar properties.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review by fault injection, not reading. Three findings, one a real bug
this branch would otherwise have shipped and two vacuous tests of my own.
1. A REAL BUG THIS BRANCH CREATED THE DURABILITY OF. Model::remove_item left
measure_agg holding an entry for a measure it had just deleted -- exactly
improvise-qx1z's defect one layer down, in the same branch that fixed the
View half and did not look at the Model half. Harmless while improvise-ppru
was unfixed, because nothing persisted the map; persisting it turned a
stale entry into a permanent one, so the document grows an Aggregation line
naming a measure it does not have and re-adding that measure brings back an
aggregation nobody asked for. A fix that introduces a format for state that
outlives what it names is how a latent bug becomes a permanent one, so the
sweep ships with the writer.
2. A VACUOUS CONTROL. a_file_naming_no_view_keeps_the_seeded_default PASSED
under the over-broad "always drop Default" it exists to catch, because
delete_view refuses the LAST view -- a guard one level away from the
condition under test. It answered "does delete_view refuse the last view"
where it meant "does drop_unnamed_default decline to fire". Split in two:
a discriminating case naming Default AND a sibling, where the wrong
condition really deletes, and the legacy no-views case kept separate and
marked as the weaker statement it is.
3. A VACUOUS NEW TEST. a_category_named_prune_is_not_read_as_the_prune_flag
passed under the rejected presence-style grammar that swallows "prune: row"
whole, because the fixture used Row and on_category_added RE-DERIVES Row for
the first regular category on reload -- the axis matched by reinvention, not
by being read. Now uses Page plus a page selection, neither of which any
default supplies.
Also verified and unchanged: the generator reaches all three new grammar
branches, including the scalar data line, each proved by a separate injection;
awkward measure names (|A: B|, |sum|, true) round-trip through the Aggregation
section; and improvise-nvq4 still does not reproduce.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Self-review with the code-review skill, against kb.pl's doctrine. All three are
tests that passed while the thing they named was not pinned.
F1. `doctrine(inject_the_rival_fix_not_only_the_bug)`. The jcfs test used one
twelve-value fixture, and `take(100)` — raising the cap rather than removing it
— passed the ENTIRE suite. That pins a behaviour, not the rule: the rule is that
detection does not depend on how many values there are, and any fixed fixture
pins one side of some cap. It now runs at 11 and 500 leading values, so every
`take(N)` fails one arm. Renamed to say what it asserts.
F2. `doctrine(an_injection_that_does_not_fail_names_an_untested_path)`.
`an_all_null_column_is_a_label` was never proved able to fail. Under the l1f5
fault it PASSES — an all-null column was Label before the fix too — so its
neighbours' injection says nothing about it. Injecting its own fault (dropping
the `!present.is_empty()` conjunct) fails it alone. The rival fix for l1f5 was
also injected: `is_number() || is_null()` over the full slice repairs Revenue
and leaves Region broken, which is why both `all_` bindings had to move and
`a_blank_value_does_not_demote_a_string_column` is the separating input.
F3. `doctrine(an_order_test_needs_two_histories)`. The cell-order test used one
insertion history five times, which cannot tell "listed its own order" from
"emitted one canonical order that matches this fixture". Same six regions now
imported forwards and backwards, each asserting its own. Reaching the second
direction needed the loop re-ordered under the fault, since an injection that
fails at iteration 1 leaves iteration 2 untested.
Also: `duplicate_csv_headers_not_silent` asserted its two values by map
ITERATION ORDER; now by key, which is also what it means.
Four controls were verified able to fail rather than assumed:
an_ordinary_float_is_still_a_number (float branch deleted),
distinct_headers_are_not_renamed_and_warn_nothing (rename every header),
an_all_null_column_is_a_label (F2), and the long-row control.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Edward Langley
Cursor Agent