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>
Conflicts:
- client.rs accepted(): both settles, in `fold`'s order:
settle_editor (fy9o/p6v6 awaited seed), settle_opened (fdk1 capture),
then the revision bump.
- resolve.rs edit_or_drill: a Coordinate opens like a Record, through
p6v6's `opened_on(state, ..)`, which waits while a write is unanswered.
- resolve.rs rename_coordinate: fy9o unwelded `clear-buffer` from the
commit keys, so the rename clears its own buffer on the success path,
after ChangeMode(Normal). It passes `advanced(.., writes)`, where
`writes` is whether the frame composed the rename. The waiting decline
still returns before any clear (the joed constraint).
- resolve.rs reopened_at: the decline arm first (Coordinate { .. }), then
`awaiting_seed` after a write, else `opened_on(state, ..)`.
- lockstep.rs: both sides' new tests kept.
- context/test-evidence/clients.md: both sides kept.
- browser_round_trip `a_second_rename_after_a_reorder_renames_the_row_the_host_shows`:
the reopened editor is now seeded from the answered cell. The test
asserts that seed, then clears it before typing. Evidence amended.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Composes fdk1's coordinate rename with #205's opened editor (improvise-ixxx):
a coordinate editor opens through opened_on and records SetOpenedEditor like
any value editor, and an untouched rename writes nothing, as the host's
CommitAndAdvance already did. The record/version capture for the basis claim
stays in ClientState::opened; context/code/clients.md argues why it cannot
live in OpenedEditor. Also: core.rs E0533 (CellId::Coordinate is a struct
variant since #197), and the ModelCommand count prose (25).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
The cell-editing commits own their buffer clear at their head instead of
the keymap welding one after them, so the re-seed survives and Enter,
Enter writes nothing.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Enter or e in the formula panel opens FormulaEdit on the formula under the
cursor, seeded with its text. The editor names the formula by a FormulaId
captured when it opens, and commits through a new ReplaceFormula effect and
ModelCommand. The edit keeps the formula's position, renames it in place when
the target changes, and refuses out loud if the formula was removed or
rewritten meanwhile, if the new target belongs to another formula, or if the
text does not parse. A refusal leaves the editor open over the user's text.
FormulaCache now carries each formula's id, so the browser composes the same
edit, and a formula-row click opens the editor on the clicked row.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
ApplyAndClearDrill walked pending_edits, a HashMap, and applied each edit on
its own. A rename moves the row's cells, so a measure edit replayed after it
wrote the old key. That left a ghost record there and a stale value in the moved
record. A second rename on the same row always found nothing and was dropped.
Across rows, a swap lost one record's data in either order.
RecordRewrite::of builds each row's final record once. It reads unedited values
off the model before anything is written, and applies every rename to every
cell. RecordRewrite::merged puts every clear before every write, in snapshot row
order. The plain records path calls the same function with its one edit, so
rename_record_coordinate and the drill's own copy are gone (gotcha 19).
Reasoning: context/code/command.md, RecordRewrite.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
CellValue's Display writes a non-whole number {:.4}, and every editor seed
was that string, so i then Enter on an untouched 1.23456789 stored 1.2346,
in the TUI (pivot and records) and in the browser, which seeds from the
projected edit_text. CellValue::edit_text is now the one home of the seed:
the shortest decimal that reads back as the same f64 (non-finite values
take the core.md §16 token). display_value_in's two arms and
cell_display's records arm all go through it; the now-orphaned
GridLayout::resolve_display is removed.
Text that reads as a number (007) is exact in the seed but still
reclassified by typed(); filed as improvise-ixxx.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
The headless exit code read `last_batch_aborted()`, which answers "did a chain
stop", not "did the request happen" — wrong both ways. `commit-cell-edit` at
bottom-right lands its write, saves, and then `EnterAdvance`'s `AbortChain`
exited 1 (c5ip); `add-item` into a missing category refuses without aborting
and exited 0 (udt2).
`App` gains a per-batch `batch_refused`, reset beside `batch_aborted` and
written only by `App::refuse(msg)`, which sets the status and the flag
together; commands reach it through `effect::refuse`. `main.rs` exits 1 on
`last_batch_refused()` only. Not a severity on `status_msg`: that would be
portable ViewState and would go stale across its ~60 writers.
Strays into ui/app.rs (four small additions) — flagged in the hand-back.
Unconverted refusals are filed as improvise-i3o5; headless `q` as improvise-ep5o.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
Sixteen command tests asserted `effects_debug(..).contains("<EffectName>")` and
nothing about the effect's fields, so they answered *which effect was emitted*
where they meant *which category, item, view, path, cell or axis it named*.
Run rather than argued: one patch made fourteen command sites emit the right
effect type carrying a wrong constant field — add-category naming "Wrong",
set-cell writing 0.0 at the wrong key, set-axis ignoring its argument,
`:w <path>` saving elsewhere, paste writing a constant instead of the yanked
value, the tile bar acting on a fixed category, page cycling always landing on
the first item, the panel cursor landing on 0 of the wrong panel, and the
formula and view panels deleting and switching to "Other". Against that,
`cargo test -p improvise-command --lib` reported 525 passed, 6 failed, and
fourteen of the sixteen were among the survivors. With the strengthened
assertions and the same rival: 510 passed, 21 failed.
Each test now asserts post-fold `App` state where the effect is cheap and safe
to apply, and the effect's own fields by `Debug` where it is not. Assertions
naming a fieldless effect are left alone: there the type name is the whole
claim.
One thing the ticket did not predict: reading the effects showed `TileAxisOp`
indexes every tileable category including the virtual ones, so tile cursor 0 is
`_Index` and not the first regular category. The tests now say so.
Evidence: context/test-evidence/command.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
Typing a name into the trailing `+` column of a records grid and pressing Tab
created the measure and then threw the user across the grid: it added a record
row nobody asked for and parked the cursor at (row + 1, 0), leaving the column
the commit had just made unreachable. Enter on the same cell was correct.
The cause is a stale index, not an abort and not an ordering. `AdvanceDir::Right`
reads `at_bottom_right` off `CursorState::col_count`, the PRE-commit column count
— `apply_effects` rebuilds the layout only after the whole batch (gotcha 23). A
`+measure` commit inserts its column at the cursor's own index and pushes the `+`
one to the right, exactly as `+cat` does, so after the commit the cursor is no
longer at the right edge.
`context/code/command.md` argued the opposite in prose: that the measure `+` has
no column to its right for an index to shift into, and that both advances are
already special-cased at the bottom-right edge. The first is false — the column
that shifts is `+measure` itself — and the second names the bug rather than a
safeguard. That paragraph is corrected here.
`inserts_a_column` therefore covers both creating columns, and takes the value,
because an empty name creates nothing: `created_name` is now the one home of that
rule, read by `add_measure`, `add_category` and the predicate. Splitting on the
buffer rather than the column is what keeps bulk entry, whose Tab-at-the-bottom-
right path is reachable only from `+measure` — it is always the rightmost column
— by tabbing onto the `+` with nothing typed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
`main` moved to #76 (improvise-748f, improvise-7kvw) while this branch was in
review. #76 approaches cursor invalidation from the per-effect side; this
branch (improvise-ni6k) replaced that table with an observation. The merge is
the interesting part.
TWO CONFLICTS, both resolved keeping both sides:
- `document_content` (#76's new O(cells) hash for the CellData arm) lands
beside `document_config`, which stays `pub(crate)` for improvise-1xf4's test.
- Three pre-loop bases now: #76's `content_basis` (writes_cells) and
`config_basis` (moves_geometry, their narrower gate), plus this branch's
`frame_basis`. Mine stays on `touches_model` and the comment says why: a
`CellData` write under `prune_empty` reframes.
THE ONE THAT DID NOT CONFLICT IS THE ONE THAT MATTERED. `effect.rs` auto-merged
clean and left #76's new per-effect `SetPageSelection` reset_cursor standing
beside the observation — two live mechanisms, 1435 tests green, nothing failing.
That is the failure mode, not a merge error. The stray is removed; one mechanism
remains, and `reset_cursor` now has exactly one caller in the crate.
#76's OWN TESTS ARE NOW THE GUARD, which is the outcome to want:
`a_page_flip_under_prune_empty_invalidates_the_cursor` and its negative twin
both pass against the observation with the per-effect call deleted, and
disabling the observation fails the positive arm while the negative one still
passes. So the observation subsumes the per-effect fix rather than merely
coexisting with it.
A MEASURED REGRESSION, FILED AS improvise-mcq4, NOT HIDDEN. 748f exists to keep
a no-cell batch off an O(cells) walk. `view_frame` builds through
`with_frozen_records`, which applies `prune_empty`, which walks cells. One
`CycleAxis` batch with pruning on, fingerprint off vs on: 3.0ms -> 12.4ms at
18,000 cells and 28.5ms -> 91.7ms at 90,000 — 3-4x, crossing the 16.7ms budget
where the tree before it sat at 3.0ms.
It was nearly missed twice, both times by a fixture that could not reach the
expensive branch: the SetCell bench shows +2-5% because #76's own walk dominates
that arm, and the geometry bench, written first against the existing fixture,
had `prune_empty` OFF and read 17-176us. `benches/reshape_observation.rs` now
carries the held-key case with pruning ON, and context/code/command.md carries
the argument.
Test function names diffed BY NAME across the merge, whole workspace including
harness/: nothing lost from either side. The seven present at 6978a4e and absent
after are this PR's own renames (j7v6, 8uj8, 1xf4), the two list helpers
improvise-g9jb replaced, and the two no-op guards improvise-ni6k orphaned.
Note: `git stash list` shows an `autostash` holding ci.yml, AGENTS.md and skills
files. It is not this session's — the tree was verifiably clean before the merge
— and it has been left untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-748f — a drill exited with no edits, and view-back on a document with
no drill, marked it dirty. `Effect::kind` takes no `App`, so the classification
cannot be state-dependent; the answer is to observe. The ticket proposed
observing the per-record raw-cell fingerprint, and that is **unsound**: five of
the ten `CellData` effects change the document without writing a raw cell —
`AddItem`, `AddItemInGroup`, `SortData` (order is in the file), `AddFormula` and
`RemoveFormula`. Observing raw cells would have left every one reading clean,
which is a lost edit rather than a spurious prompt. So `document_config` is
widened into `document_content` — cells in iteration order and formulas as well
as the categories, items, groups and view configuration it already carried — and
`CellData` joins `LayoutGeometry` in being read against it, in the loop, at the
position gotcha 45 requires. `WholeWorkbook` stays declared, and that is the
remaining over-report: a `LoadModel` does not move `file_path`, so the in-memory
document really does differ from the file the path names.
`start_drill_and_apply_clear_drill_with_no_edits` pinned the old behaviour and
is inverted.
improvise-z05r — `CommitExport` did not bail on an empty path where
`CommitBuffer`'s three kinds all do, and `ExportCsv` did not abort the batch
where `Save` and `SaveAs` both do. All three writers now report through one
`report_file_write`, and the test names the three so a fourth has to come there.
The commit reorders its batch too: the mode change goes at the head, because an
abort at the tail strands the user in `ExportPrompt`, where the minibuffer is
drawn *instead of* the status line and the export error is never seen (gotcha 21).
improvise-7kvw — `SetPageSelection` renumbers the grid under `prune_empty`,
because which rows survive pruning is a function of the page. It now resets the
cursor in exactly that case, and only when the selection really moved: sending
the cursor home on every `[`/`]` with pruning off is worse than the hole.
improvise-pubt — the ticket argued the drill arm of `display_value_in` was
probably unreachable with a foreign layout. It is reachable: records mode
auto-detects from the axes, so an ordinary saved view gives `GridLayout::new` a
records layout whose `cell_key` returns the synthetic key the arm matches. The
test fails on the old code. `staged_drill_edit` now consults `drill_state` only
for the host's own layout, by pointer identity, which is the question `grid_in`
is actually asking.
improvise-h074 — `resolve_view`'s fallback carries back the `.expect` message
`Workbook::active_view()` had. The second half, the host-side `DeleteView`
bypass of `adopt_deleted_views`, is untouched: it is a decision about
`Sessions`/`Host`.
Every guard here was proved by injecting the fault it names and watching it fail
by name; `context/test-evidence/command.md` records each injection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
improvise-j7v6: the rule turning a typed string into a `CellValue` was
written twice — `resolve::typed` (already moved into improvise-protocol
since the ticket was filed) and `commit_regular_cell_value`'s
if/else-if/else on the host's key path. The host now calls
`improvise_protocol::typed`, which is `pub` for that reason.
Per doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished),
`a_typed_string_is_classified_as_the_host_classifies_it` did not survive
as a comparison: type identity replaced its claim. It is re-scoped to the
thing this crate still owes — the FRAME the string composes — and renamed
`a_typed_string_composes_the_frame_that_carries_it`.
The ticket's flagged risk (gotcha 34: a shared classifier that disagrees
with the persistence layer is a third answer) is now asserted rather than
argued: tests/typed_string_agreement.rs round-trips what `typed` calls
text and what it calls a number through format_md/parse_md. Proved
discriminating: dropping `pipe_quote` from the writer fails it.
improvise-8uj8: `every_rejection_message_says_the_edit_did_not_land`
drove a hand-written five of seven `Rejection` variants while claiming
the fact held "over all of them". `Rejection::all` (the same cyclic walk
`AppMode::all` uses) and `Rejection::refused_an_edit` (exhaustive) now
drive it, and its claim is re-scoped to what is true: a rejection that
refused an EDIT says nothing was applied, and the two Save answers —
which refused no edit — say the document was not written. Renamed
`every_rejection_message_says_what_it_refused`.
Proved both ways: wording SaveFailed as "hmm" fails the test, which the
old list could not have seen; adding a Rejection variant fails the build
at basis.rs:101 and :126.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
loop-of-one test
F1 (HIGH). Gating the reset on Ok is not the same as gating it on "something
changed". Six accepted commands changed nothing and reset every cursor anyway:
SetAxis to the axis already held, HideItem on an already-hidden item, ShowItem on
a never-hidden one, AddItem of an existing item (Category::add_item dedupes and
returns Ok), and RemoveItem/RemoveCategory of something absent. Each now asks the
state before mutating. The case that made it urgent needs no second client:
TileSelect binds `r` to set-axis-for-tile row, which emits SetAxis with no
comparison against the current axis, so `r` on a Row category sent the user's own
cursor to the origin.
F2 (REGRESSION this PR introduced). Naming a measure in the trailing `+` column
at the bottom-right of a records grid sent the cursor home: AddItem now resets,
inserts_a_column matches only NewCategory, and EnterAdvance at the bottom-right
returns AbortChain with no SetSelected. The restore moves to BEFORE the advance,
where an aborting advance cannot skip it -- gotcha 21's rule applied to the
cursor rather than the mode -- which also subsumes the +cat arm's copy.
F3. a_refused_model_command_invalidates_no_cursor named a class and tested one
member; injecting reset_cursor into AddCategory's Err arm left it green. Now a
table over all three refusable effects, and that injection fails it by name.
F5. The benchmark could not reproduce its own figures: it took the mean over a
few iterations, and review measured the same share at 8% and at 131% -- a part
slower than the whole containing it, printed flat. It also passed (false, 2)
where grid_of passes the view's (true, 0), so it timed a call that never happens.
Now min-of-N with the view's own format, and a share above 100% prints UNSTABLE.
Figures are stable across runs at ratio 1.2-1.5x, col_widths 45-49%.
F6. The drg counts omitted the 8 simple_effect! invocations and misclassified
AddItem at base through a regex that cannot see `app` newline `.model_state` --
doctrine(derive_then_verify) catching the person citing it. Correct: 57 effects,
narrow set 32 -> 25, not 25 -> 17 of 49. The direction holds and is still the
argument; the headline was overstated by about half again. Corrected in
context/code/command.md and on the bead, not quietly restated.
F9. a_model_command_that_leaves_the_active_view_alone_broadcasts_nothing asserts
a ResetCursor broadcast, so the name read as a lie: ..._broadcasts_no_switch.
F4 filed as improvise-pdvm: AddItem/RemoveItem declare CellData so layout_version
does not bump while cursor_epoch does, and this PR's own reshape assertion is the
first evidence for it. Related to improvise-75ld, same question other direction.
Faults injected for every new guard: SetAxis's moved check, AddItem's fresh
check, AddCategory's Err arm, and the pre-fix advance ordering ((0,0) vs (0,3)).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only TransposeAxes and CycleAxis reset the cursor, while eight siblings that
renumber just as thoroughly did not. SetAxis is the sharp case: CycleAxis *is*
SetAxis to Axis::next(), so a pair differing only by an argument disagreed.
reset_cursor is also the only writer of cursor_epoch, which is the only thing
Sessions::adopt_cursor_epoch watches, so each gap was invisible in both
directions at once: the host's own cursor stayed stale and no client was told.
Gotcha 24 records that the second half is strictly worse than doing neither.
SetAxis, HideItem, ShowItem, ToggleGroup, TogglePruneEmpty, AddCategory,
RemoveCategory, AddItem, AddItemInGroup and RemoveItem now reset. The item
effects are in because "an append cannot move an existing position" holds only
of the outermost category on an axis: expand_category runs inside the cross
product, so an item appended to an inner category repeats once per outer block.
SetNumberFormat stays out and keeps the negative arm.
CommitAndAdvance's +cat branch depended on the missing reset — it stayed put so
the cursor would land on the column it had just created (improvise-2zq). It now
says so with an explicit SetSelected, placed after AddCategory's abort so a
refused category still leaves the batch alone.
The new test asserts of every case that row_items/col_items really changed
before asserting anything about a cursor; two faults were injected to show both
arms discriminate. Reasoning in context/code/command.md, evidence in
context/test-evidence/command.md. SetPageSelection under prune_empty is the one
residual and is filed as improvise-7kvw.
Closes improvise-9mo9
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
keymap.rs: the dispatch/registry-vs-table split (this file stays the authority
on it) and the improvise-0hb Sequence-flattening decline, with its three
findings.
cmd/**: the two CommitAndAdvance orderings, CommitSave's mode-change-first,
add_category's deliberate no-coordinate-write, EditOrDrill's mode-agnostic
message choice, the SaveAndQuit and CreateAndSwitchView declines, and fit.rs's
whole module doc including the measured wasm bundle cost.
crates/improvise-command now reports no comment run over two lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The user has settled the contract, and it rejects the "at most one document
mutation per frame" rule this branch shipped. Verbatim: *"a sequence of
commands sent to the server should assume that it's played in order from
start to end and that each command sees the state after the previous command
runs"*, sent as *"a single transaction from a given base"*. Four clauses:
1. A frame states **one basis**, describing the base the client composed
against.
2. That basis is checked **once**, against that base, before anything runs.
3. Commands are applied **in order**, and command N sees the state command
N-1 left. Data dependence between them is expected, not an error.
4. **All or nothing.**
This commit lands clauses 1–3 and removes the rejected rule. **Clause 4 is
not implemented** and is documented as an open hole rather than a closed one;
the design note on PR #26 covers what rollback costs.
**Per-command basis checking is not merely awkward, it is incapable**, which
is the finding that makes clause 3 necessary rather than preferable. A client
cannot claim a counter for a record its own chain is about to create, because
`BasisCache::claim` filters to records the cache has been told about — so
`[AddItem("East"), SetCell(East)]`, the archetypal chain, is refused
`RecordGone`/`RecordUnstated` by a per-command check *in any order*. The base
describes what the client **saw**; the chain's internal coherence is the
transaction's job, not the basis check's.
So `CheckedBatch::against` answers the basis on the first command that
carries one and admits the rest through a private
`Checked::within_an_answered_frame`. Private is what stops it being a bypass:
from outside the module the only doors are still `Checked::against` and
`CheckedBatch::against`.
**Two things the rejected rule got wrong, kept as the lesson.** *Arity is not
a property of a variant* — a future `Command::RunScript` is one variant
performing unbounded mutations, so no count over the vocabulary can ever be
right. And *the transaction boundary was never the frame*: because one
command may already be a chain, clause 4 is needed inside a command as much
as across a frame of them, which is why the design note puts the mechanism at
`App::apply_effects`.
`mutates_the_document` is **deleted rather than kept**, which is a deliberate
deviation from the instruction to keep it: with the rule gone it has no
caller, and dead production code behind an `#[allow]` is worse than none. The
reason it was made exhaustive — a `matches!` answers `false` for a future
variant in silence — is preserved in the `CheckedBatch` doc, where clause 4
will read it when the classifier returns.
`Rejection::FrameNotAtomic` goes with the rule; nothing can produce it. The
taxonomies on `Rejection` and `ServerFrame::Rejected` return to two classes
and name the third that arrives with rollback.
Evidence, both faults injected:
- `a_data_dependent_chain_lands_in_order` — `[AddItem("East"), SetCell(East)]`
through a real `Host`. Restoring the per-command check fails it, which is
the whole point. It asserts **both** commands landed, because asserting
only the first would pass against a host that dropped the second.
- `a_frame_whose_later_mutation_fails_is_not_yet_undone` characterizes the
open hole with `[RemoveItem("North"), SetCell(North)]`: the removal lands,
the write resolves to nothing and `model_effects` returns an **empty**
batch, and the frame is reported accepted having applied half of itself.
Both the partial application and the silence are open. **This test inverts
when clause 4 lands**; until then its job is to fail loudly if anyone
believes the hole is closed.
- The worker's `a_partly_refused_batch_…` fixture moved its injected
staleness to the **first** command, since a later one no longer carries an
answer of its own. Removing the injection still fails it.
The hole is unreachable from any client in this workspace — the only producer
of a multi-command frame is `keys::commit`, one mutation among view commands
— which is what makes it safe to document rather than rush.
Merged `main` (3915994). One semantic conflict: PR #27 added a
`mark_dirty()` call in `commit.rs::add_category`, which no longer exists —
dropped, since `AddCategory` is a `Model` effect and `apply_effects` raises
the flag.
1245 passing; clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
**A schema change shifted the grid under its own advance.** `add_category`
inserts the new coordinate column immediately before `+cat`, so every index
from `+cat` rightwards moves by one — while `CommitAndAdvance` computed its
advance from `ctx`, the *pre-commit* layout (gotcha 23). On the last row
`Enter` wraps to `col + 1`, which after the insert is `+cat` again.
The failure is not "lands one column off": it **reopens category creation**,
so a user naming two categories in a row never reaches the column they made
and gets an unbounded `+cat` loop. `Tab` reached it the same way.
A schema change that inserts a column now does not advance at all, which is
the useful answer as well as the safe one — the insert happens exactly where
the cursor is, so staying put lands on the new column. The measure `+` shares
none of this and not by luck: it is the last column, so there is no index to
its right to shift into, and both advances already special-case the
bottom-right edge. `inserts_a_column` asks the *layout* what the column is
rather than re-deriving it from the value, so gotcha 1 keeps one answer.
Both arms were watched failing first, and both now assert the column landed
on rather than merely "not `+cat`" — a `assert_ne` alone would pass anywhere.
**And a negative arm that was asserting something false.**
`a_prompt_this_client_cannot_commit_is_never_entered` used `:` to show that
`strands_the_user` had not degenerated into "refuse every mode". But `:` is
itself a dead room: it opens locally and types locally, while Enter is
`[execute-command, clear-buffer]` and `step` has no `execute-command` arm, so
the frame is dropped. The arm was **certifying the behaviour it existed to
rule out**, which is worse than having no arm at all.
`CommandMode` therefore joins the predicate. It costs no working surface —
every ex-command behind `:` is inert for the same single reason — and the
distinction from the two prompts is kept visible because it says who removes
the line: they are dead *by construction* (no `ModelCommand` can name
`Save`/`SaveAs`/`ExportCsv`), this is dead *by omission*, and improvise-e24v
is scheduled to implement `execute-command` client-side. Listing it is
therefore also the named gap gotcha 47 asks for: closing e24v has to come to
this line and delete it, which a comment could not force.
`the_command_line_cannot_be_committed_by_this_client` is the evidence rather
than the assertion.
The arm itself now has **two halves**, and the first version of the rewrite
had neither — it pressed Enter in `Editing`, which never consults
`strands_the_user`, so "refuse every mode" still passed. Half one enters a
mode through `enter-mode` and so exercises the predicate; half two presses
Enter and requires a real `Command::Model` back, because the property is
finishing rather than entering. `?` is honest as half one where `:` was not:
Help has no commit, so nothing typed into it can be swallowed.
Proved in both directions: "refuse everything" fails half one, "refuse
nothing" fails the positive arms and the named-gap list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-2zq step 2, on the user's ruling. Step 1 made this coherent by
stopping `on_category_added` from ejecting the user; this adds the column.
Typing a name into `+cat` creates that category, and because
`GridLayout::records_columns` filters on `kind.is_regular()` and never
looks at items or at an axis, the coordinate column appears immediately
and is typeable on the next keystroke.
**The two questions the ruling left open, and the answers.**
*Where the column goes.* Not a second trailing column, and not one `+`
disambiguated by a prefix. A prefix would put a sigil into the name space
— and a category and a measure may legitimately share a name, which is
the exact ambiguity `_Dim` already carries a column *index* rather than a
label to avoid (gotcha 1). A key was rejected too: the name still has to
be typed somewhere, and a minibuffer lands in the records-exit class bug
this PR characterizes but does not fix.
So each `+` sits adjacent to what it adds to — `[coords…, +cat,
measures…, +]`. Position tells the two apart instead of a label, which is
the clutter the ticket was worried about. It costs a mid-grid column
index, and the consequence worth naming is that a **blank** records grid
now opens with the cursor on `+cat` rather than `+`: a record needs
coordinates before it needs measures, so that order is defensible, but it
does change what the first keystroke in an empty grid does.
*Whether committing also sets the coordinate on the current row.* It does
not. It is symmetric with the measure `+`, which creates an empty column
and writes no value; and setting it would move **every** measure cell in
the row onto the new coordinate (gotcha 1) — a data migration across N
cells attached to the same Enter that made a schema change, in an app
with no undo. The follow-up edit is one column away.
**The refusal is visible, which took a change of order.** `MAX_CATEGORIES`
is reachable here rather than theoretical. `AddCategory` now aborts the
batch on refusal — same reason a failed save does (gotcha 21): what
follows assumes the category exists, and the trailing `EnterEditAtCursor`
would re-open the minibuffer *over* the message, since `draw_bottom_bar`
renders the minibuffer instead of the status line. And
`CommitAndAdvance` now emits its mode change **before** the commit rather
than after, so an abort cannot eat it. That is the third time this PR has
applied one rule — tidy-up goes where an abort cannot reach it — and the
comment at that site had already described the pattern for the *advance*
failing; the commit failing was simply not considered. The limit itself
is still checked in exactly one place, `Model::add_category`.
Evidence. Three new tests drive real keys:
`the_new_category_column_creates_a_category_and_its_column` (the column
is usable immediately), `..._writes_no_coordinate` (the deliberate
answer to question 2, asserted over every row), and
`..._refuses_past_the_limit_where_the_user_can_see_it`, which asserts the
mode has **no minibuffer** — proved discriminating by swapping the commit
back in front of the mode change and watching it report
"Maximum of 12 categories reached" from inside `RecordsEditing`, where
nothing would have drawn it.
Four existing characterization tests were updated knowingly, not
incidentally: the shared records fixture's column list (which accounted
for 8 of the 9 initial failures on its own), the core layout and
persistence smoke expectations, and the blank-model bootstrap, which now
has to step to the `+` column and so records the cursor-position
consequence above.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both correct, both applied, and both wider than the line they were raised
on.
**A prompt a browser cannot commit is never entered.** `enter-mode` is
trivially resolvable in the wasm client, so `Ctrl-W` opened `SavePrompt`
there — and Enter then resolved `[commit-save, clear-buffer]`, whose
first step has no arm in `keys::step`, went `Upstream`, and was dropped
by `Host::handle`'s empty vec. The user types a path into a prompt that
swallows it. `doctrine(an_upstream_key_is_a_black_hole)` already says an
upstream key does nothing; what this adds is that a *doorway* to such a
key is worse than the key, because the door costs one inert keypress and
the room costs work the user believes was done.
`Ctrl-E` into `ExportPrompt` had the identical trap and predates this
branch, so both are fixed: fixing one arm of a rule with two is the shape
`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`
names. Neither is an omission to close later — `Save`, `SaveAs` and
`ExportCsv` are all `ViewOnly`, so no `ModelCommand` names them and no
client on any transport can ask a host to write a file (gotcha 45).
Named as a predicate, `strands_the_user`, rather than matched at the call
site (`doctrine(two_questions_one_enum)`): "can this client finish what
entering the mode starts" is a different question from "is this a
text-entry mode".
**And the TUI reported a failed save where nothing draws it.** `SaveAs`
aborts the batch on failure, so with `CommitSave` emitting
`[SaveAs, ChangeMode(Normal)]` the app stayed in `SavePrompt` — where
`draw_bottom_bar` renders the minibuffer *instead of* the status line, so
the "Save error:" set a statement earlier was never drawn. The save
appeared to do nothing at all. Leading with the mode change puts the
error in `Normal`, where the bottom row is the status bar. It costs the
typed path, which the welded `clear-buffer` takes on success anyway and
`EnterMode` clears on the next `Ctrl-W` regardless (gotcha 11).
Evidence, both injected. `a_prompt_this_client_cannot_commit_is_never_entered`
fails on both arms when `strands_the_user` returns false, and carries a
negative arm (`:` still opens the command line) so "refuse every mode"
cannot pass it. The end-to-end test now asserts the mode *left* the
prompt — the opposite of what it asserted an hour ago, deliberately — and
fails when the two effects are swapped back.
Copilot's third finding, that `context/repo-map.md` and
`prolog-sessions/kb.pl` still say 16 AppMode variants and seven
text-entry modes, is correct and is **not** applied here: both files are
orchestrator-only in this wave. The exact edits are in the hand-back.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A client's edit left the authoritative document reporting **clean**, on
every transport. `reduce::model_effects` maps `ModelCommand::SetCell` to
`effect::SetCell`, which writes the cell and never touched
`model_state.dirty`; dirtiness was a *separate* `MarkDirty` effect that only
the command/key path composed. So "save if dirty" was a trigger that never
fired — a host built on it loses every client edit, silently — and anything
else that read the flag about a client-edited document got the wrong answer.
The interesting part is not the missing line, it is **why** it was missing.
improvise-kmgs derived `ModelCommand` from the effects **one at a time**, and
per element that derivation is right: `SetCell` really is the effect a
`SetCell` command names. What it could not see is that the key path never
emits `SetCell` alone — it emits the *pair*. **A derivation correct per
element and wrong for a set.** The pairing existed nowhere except at 17
hand-written call sites, which is exactly the kind of obligation this
codebase makes unrepresentable rather than validating.
So dirtiness now comes from `App::apply_effects`, off the same `EffectKind`
the `Dirty` and the two lower basis tiers are already read from (gotcha 40):
one classification, three readers, no second table to drift from. There is
no longer any way to compose a document change that omits it — on the wire,
from a keypress, or from a host acting on its own behalf. `MarkDirty`,
`effect::mark_dirty()` and every call site are **deleted**; leaving them
would have been a second answer to one question, and the next reader would
have believed the call sites were the mechanism.
All three `ModelChange` levels raise it, `WholeWorkbook` included. A replaced
document is not the document `file_path` names — `LoadModel` does not move
`file_path` — so in both the wizard and the load case the bytes on disk and
the bytes in memory have parted company, which is all `dirty` claims.
`AcceptWizardModel`'s own `dirty = true` line goes for the same reason the
call sites do.
**The worker's persist trigger moves with it.** `touches_document`
enumerated command variants *because* the flag was unusable, and it said so.
With the flag trustworthy, the trigger is the observation — the same choice
the cursor reset makes by watching `cursor_epoch` instead of matching on
`CycleAxis` (gotcha 24), and the rebase by watching `document_generation`
instead of matching on `LoadModel` (gotcha 39). A table of variants is a
second description of which commands change the document, free to disagree.
It is also strictly tighter: a refused batch changes nothing and so raises
nothing, which retires the "was anything rejected" guard beside it.
Evidence:
- `a_wire_model_command_marks_the_document_dirty` (`ui/reduce.rs`) — written
first and watched to fail on the old code. It asserts all three
`ModelChange` levels separately, so an effect reached by only one of them
cannot pass on a sibling's evidence.
- `a_view_only_batch_leaves_the_document_clean` (`ui/app.rs`) — the negative
arm, without which "raise it unconditionally" would pass. It is aimed at
`apply_effects` and **not** at a `Command`, because a first draft written
against the session door was **vacuous**: a view-slice command is folded
straight into `ViewState` and never reaches the loop at all, so it passed
with the raise made unconditional. Injecting that fault is how the vacuity
was found; against the effect batch the same injection fails it.
- The worker's `an_edit_is_persisted_without_the_client_asking_to_save`
fails when the new trigger is disabled.
Three `cmd/cell.rs` tests counted the dropped companion effect and are
adjusted; two are renamed, since they no longer assert anything about dirty.
1193 passing, from a 1189 baseline.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-8tk made ZZ/:wq refuse to quit when the save fails, which is
correct and unhelpful: with no file path the user is left holding
"No file path — use :w <path>" and has to know to type a path inline.
There was no save-as prompt to send them to. `AppMode::ExportPrompt` is
not one — `CommitExport` writes CSV, so binding it here would have
written a CSV file under whatever .improv name the user typed.
This is the ticket's preferred option (a): a `SavePrompt` minibuffer mode
built by the `TextEntry` builder, committing to `SaveAs` — `SaveAs` and
not `Save`, because the prompt exists precisely when there is no path for
`Save` to use, and because `SaveAs` adopts the path so the next Ctrl-S
works. Bound to Ctrl-W in Normal, beside Ctrl-E's export prompt, and
reachable as `:enter-mode save-prompt`. The status message now names it.
`ZZ` is deliberately NOT routed into the prompt automatically; see the
hand-back for the reasoning. The abort of improvise-8tk is untouched.
Evidence. The binding dump diff is purely additive — one Normal binding
(inherited by RecordsNormal) and the new mode's own keymap; no existing
line moved. The mode is carried by the existing meta-tests rather than by
new bespoke ones, and both were proved to reach it by injection:
every_hint_key_resolves_in_its_mode fails when the new hint names an
unbound Tab, and every_bound_command_name_is_registered fails when the
commit name is typo'd. commit_save_writes_the_model_to_the_typed_path
pins SaveAs-not-ExportCsv, and commit_save_on_an_empty_path_writes_nothing
pins the empty-path bail.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records mode showed long format: a row per cell, a _Measure column naming
which measure it was, and a synthetic "Value" column holding the number.
Entering data meant sitting on "Value", and editing the measure column
renamed a coordinate rather than a value.
Wide format instead: a row is one coordinate tuple — the cells sharing every
coordinate but _Measure — and each raw measure gets its own editable column.
Two measures on one coordinate are now ONE row.
The case matrix came from a Prolog model rather than a read-through, which
surfaced the load-bearing consequence: a row now groups several cells, so
editing a category column must move all of them. The long-format code moved
only the cell under the cursor, which here would leave half a row's measures
under the old coordinate and half under the new. Both the plain-records and
staged-drill paths implement it.
_Dim in the synthetic key now carries the column index rather than a label,
because a category and a measure item may share a name and a label could not
say which column an edit landed on. GridLayout::record_col resolves it to a
RecordCol, and drill pending edits are keyed by (row, RecordCol) — which also
frees ApplyAndClearDrill from depending on the layout still being the drill
layout.
A blank model has no measures, so wide format alone would leave nowhere to
type a number and break the onboarding path. A trailing "+" column creates a
measure by name; being a schema change, it applies immediately rather than
staging inside a drill.
Formula targets are not columns — they are computed, so there is nothing to
type into them.
prune_empty's records branch went with it: the function early-returns in
records mode, so that tail could never run. Filed as improvise-2yk in case
the early return is itself the bug.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01V69PGQfMJuGRmtSWpfVAqn
CommitFormula, CommitCategoryAdd and CommitItemAdd collapse into a single
CommitBuffer { kind: CommitKind }, registered as
`commit-buffer <formula|category|item>`. The five keymap bindings pass
the kind as an argument, giving improvise-w3q a uniform primitive to
build its text-entry template on.
The three did not have identical shape — they differed in buffer name,
empty-buffer landing mode, post-commit landing mode, status text and add
effect, and two of those are semantic (formula returns to its panel;
category/item stay open for batch entry). CommitKind carries those as
methods rather than flattening them, so this costs ~24 lines rather than
saving 40. The win is one home for minibuffer commit semantics.
CommitKind::Item carries its target category, resolved from
AppMode::ItemAdd by parse_commit_kind at registry time. execute() is
therefore a pure function of kind + buffer, the shared helper's
Option<Box<dyn Effect>> is gone, and the old silent no-op outside
ItemAdd mode becomes a real error on the `:` line.
Fixes a bug found on the way: the commit pushed set_status("Formula
added") after AddFormula, clobbering the parse error that
AddFormula::apply writes to the status bar — an unparseable formula was
dropped while the UI reported success. The add effect now goes last, so
the only party that knows whether it worked gets the last word.
Also adds a keymap test that dispatches Enter in each minibuffer mode:
Keymap::dispatch swallows an unresolvable command name with .ok()?, so
the existing lookup-only smoke tests would have passed even had the
rename missed the registry.
Filed improvise-djn: AddCategory still swallows its Result.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
EnterEditAtCursor called app.rebuild_layout() and then built a whole
CmdContext just to read one field, so effect semantics depended on apply
order and the read cost a column-width computation.
Split the two concerns the command layer should own:
- new RebuildLayout effect, emitted by EnterEditAtCursorCmd and
CommitAndAdvance ahead of the re-edit, so the ordering is visible at
the command level instead of hidden inside apply
- App::display_value_at_cursor() lifts the display-value block out of
cmd_context; both the context and the effect now call it
Behavior is unchanged. The new effect-level test pins it: removing the
row-0 item slides row 1 under the cursor, and the edit buffer must hold
that cell's value — it reads "" instead of "2" if the rebuild is dropped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Command layer + semantic App state move below the TUI: command/ (cmd,
keymap, parse, key), ui/{app,effect,cat_tree}.rs, and the pure viewport
helpers now live in crates/improvise-command, which depends on core +
formula + io and provably not on ratatui/crossterm (cargo tree clean).
The root crate remains the TUI binary and re-exports improvise_command
under the old crate::command / crate::ui::{app,effect,cat_tree} paths,
so widgets, draw.rs, and main.rs are unchanged. App moves whole (per
the improvise-ltq design note) rather than splitting an AppState out —
its fields are all command-safe after the key/color decoupling.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cursor Agent
Edward Langley