Merge gh/main; fix the whitespace run in the refusal message
Conflict was `context/test-evidence/gp.md` only — this PR and #146 (improvise-ipqu) appended at the same point. Both entries kept, verified line-by-line against both parents, because --check-evidence cannot see a dropped entry: it checks that every heading names a running test, and neither of this PR's tests cites the file, so losing one would pass the gate. The format string was broken across source lines with no `\` continuation, so eighteen spaces were baked into what a user reads on --predict against a corrupt model. Found by adversarial review, which printed the message rather than reading the source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
+74
-38
File diff suppressed because one or more lines are too long
@@ -115,3 +115,43 @@ What it still cannot see, and should be closed if the welcome text moves again:
|
||||
`draw.rs::draw_welcome` builds its lines as a local array inside a ratatui
|
||||
render function, so nothing can walk them. Its `:formula` line was corrected by
|
||||
hand.
|
||||
|
||||
## 7 — a script's command is built against the cursor it will run against
|
||||
|
||||
`improvise cmd` and `improvise script` used to resolve every command through
|
||||
`CmdRegistry::parse` — the **pure** arm, which builds `CursorState::default()`:
|
||||
row 0, col 0, and a grid of zero rows and zero columns. The interactive arm,
|
||||
which builds `CursorState::from_ctx(ctx)`, was never reached from a script.
|
||||
|
||||
So every `Move` (`move-selection`, `scroll-rows`, `page-scroll`, the four
|
||||
`jump-*`) and `enter-advance` snapshotted a zeroed cursor at **parse** time,
|
||||
clamped to `(0, 0)`, and emitted `SetSelected(0, 0)`. That is worse than a
|
||||
no-op: it *resets* the cursor. Nothing could position it, so every
|
||||
cursor-reading command — `clear-cell`, `yank`, `paste`, `cycle-axis-at-cursor`,
|
||||
`hide-selected-row-item`, `edit-or-drill`, `drill-into-cell`,
|
||||
`toggle-group-under-cursor`, the three `delete-*-at-cursor`,
|
||||
`commit-and-advance` — was pinned to the origin (improvise-6uva). It was
|
||||
introduced with `CursorState` and the two-arm registry in April and predates the
|
||||
browser work by five months.
|
||||
|
||||
**A registry with two construction arms silently demotes every caller of the
|
||||
weaker arm, and nothing type-checks which arm a caller deserves.** That is the
|
||||
shape to remember, and it has a second instance in the same fix: the
|
||||
`register_pure` entries had an interactive arm that refused **empty** args while
|
||||
their pure arm accepted them, so `:w` (save to the current path) parsed one way
|
||||
and refused the other. Both arms are now `parse`, which is what "needs no
|
||||
interactive context" was supposed to mean;
|
||||
`a_pure_commands_two_arms_agree_on_no_arguments` is the guard.
|
||||
|
||||
The repair is `parse::segments` plus `parse::parse_segment_interactive`, and the
|
||||
loop re-parses **per segment**, because command *n* must see command *n-1*'s
|
||||
cursor — which means construction cannot be hoisted out of the loop. Line
|
||||
atomicity is preserved by keeping the existing whole-line `parse_line` as a
|
||||
**syntax gate** before anything in the line runs: a line that parsed before
|
||||
still parses, and a line that did not still does not, so the only behavioural
|
||||
change is which cursor the commands see.
|
||||
|
||||
The TUI's `:` line had the same defect and is fixed the same way, with one
|
||||
limitation that is not new: `ExecuteCommand` returns one effect batch, so the
|
||||
segments of `:a . b` all see the pre-batch cursor, exactly as a
|
||||
`Binding::Sequence` does (gotchas 21 and 23).
|
||||
|
||||
@@ -692,6 +692,42 @@ does, so the branch ends with an explicit `SetSelected` back to where it was —
|
||||
which states the intent instead of inheriting it from an absence, and still sits
|
||||
after `AddCategory`'s abort so a refused category leaves the batch unchanged.
|
||||
|
||||
### `SwitchView` — the history records a move that happened (improvise-swzk)
|
||||
|
||||
`switch_to_view` is guarded: it writes `active_view` only if the document has the
|
||||
view. `SwitchView` used to push the leaving frame onto `view_back_stack` **before**
|
||||
calling it, under the prediction `previous != self.0` — which answers *did the user
|
||||
ask for a different name*, where the history means *did this client go somewhere*.
|
||||
Those two questions differ on exactly one input, the name the document has not got,
|
||||
and there `:view NoSuchView` buried a real frame under a frame naming the view the
|
||||
user was already on. `<` then "went back" to where it already was, and the frame
|
||||
behind it needed a second `<` nobody knew to press.
|
||||
|
||||
The wire path was never wrong here and is not the model for the fix: `reduce_view`
|
||||
**cannot** check — the client holds no `Workbook` — so `apply_switch` relays the
|
||||
exact inverse, `[ViewBack, ForgetView]` (improvise-6c6q, and see "A refused switch
|
||||
is undone, not overwritten"). The host holds the document, so it does not need an
|
||||
inverse; it needs to *look*. `apply` now clones `previous`, calls `switch_to_view`,
|
||||
and pushes only if `active_view` actually differs afterwards.
|
||||
|
||||
That is `ReturnToNormalUnlessMoved`'s rule one effect over, and it is the same
|
||||
answer `cursor_epoch` gives instead of matching on `CycleAxis` and
|
||||
`document_generation` gives instead of matching on `LoadModel` (gotchas 24, 39, 45):
|
||||
**observe the state that moved, never predict it from the request.** It also
|
||||
subsumes the case the prediction was originally written for — switching to the view
|
||||
you are already on is a `switch_to_view` that changes nothing — so the special case
|
||||
is gone rather than joined by a second one. A guard spelled here as
|
||||
`workbook.views.contains_key(&self.0)` would have been correct today and a second
|
||||
copy of `switch_to_view`'s own condition, free to disagree with it the day that
|
||||
condition grows a clause.
|
||||
|
||||
Not fixed, and reachable only by construction: `ViewBack` and `ViewForward` call
|
||||
`switch_to_view` just as unguardedly, pushing the current view onto the opposite
|
||||
stack before the switch they cannot be sure of. They are safe **because
|
||||
`ViewState::forget_view` purges both stacks when a view is deleted**, so a popped
|
||||
frame always names a live view — a guard living one level away from the hazard,
|
||||
which is the shape that keeps returning here.
|
||||
|
||||
### `ReturnToNormalUnlessMoved` — observation, not prediction (improvise-toc)
|
||||
|
||||
Return to `Normal` unless something earlier in this batch already moved the mode
|
||||
|
||||
@@ -154,6 +154,72 @@ composes both: the prompt and buffer on the left, any pending `status_msg`
|
||||
right-aligned. The alternatives and why they lost are in
|
||||
`context/test-evidence/tui.md`.
|
||||
|
||||
### `fill_line` — which segment yields is a per-call-site decision
|
||||
|
||||
Three rows in the TUI are one left segment and one right segment sharing a line:
|
||||
the title bar, `minibuffer_line` and `draw_status`. `fill_line` composes them,
|
||||
and it owes two things that it did not originally do — it must measure in
|
||||
**display cells** (it padded with `left.len()`, bytes, and the title's own `·`
|
||||
is two of them, so every document's right segment sat one cell in from its edge)
|
||||
and it must **truncate**, because concatenating and letting ratatui clip clips
|
||||
the *right*.
|
||||
|
||||
What clipping the right costs depends on the row, which is why `Keeps` is a
|
||||
parameter and not a rule:
|
||||
|
||||
- **The title bar keeps the right.** Its left segment is decoration whose length
|
||||
is unbounded — a document name plus a filename — and its right segment is the
|
||||
fixed `" ?:help :q quit "`, the only thing on screen telling a stuck user how
|
||||
to get out. Unconditional left priority made the escape hatch the first thing
|
||||
sacrificed, at 100 columns with an ordinary document name (improvise-t14d).
|
||||
- **Both bottom-bar rows keep the left**, which is what they already did. There
|
||||
the left is the user's own typing plus the cursor (`minibuffer_line`) or the
|
||||
status message (`draw_status`), and the right is a view badge or the message's
|
||||
own margin. Right priority there would let a long message eat the text being
|
||||
edited — the same mistake in the other direction.
|
||||
|
||||
So the rule is not "the right wins" but "the affordance wins over the
|
||||
decoration", and which is which is local. Truncation goes through
|
||||
`ui::text::truncate` (improvise-8l0c), so an overflow is a `…` rather than a
|
||||
mid-token clip of `?:`.
|
||||
|
||||
### One key table on screen, and it is the one derived from the keymap
|
||||
|
||||
The TUI stacks the tile bar (row 3) directly above the status bar (row 4), and
|
||||
the import wizard's overlay sits above the status bar too, so a mode can put
|
||||
**two** key tables on screen at once. When it does, they disagree — different
|
||||
notation, different content, and in one case a flat contradiction.
|
||||
|
||||
The rule is not "the overlay wins" or "the status bar wins". It is **keep
|
||||
whichever table is derived from the thing it describes** (improvise-rr8n):
|
||||
|
||||
- **Tile select: the status bar wins.** `hint_line(&mode)` is read off the
|
||||
keymap, so it cannot go stale, and it is a strict superset of what the tile
|
||||
bar's literal carried — `Space`, `h`/`l` and `Tab` were only ever on the
|
||||
derived line. The tile bar's own hints are therefore **deleted**, both of
|
||||
them, and `TileBar::render` now advertises no keys at all. The one it drew in
|
||||
Normal was false: it named `Ctrl+↑↓←→` as "to move tiles" while those four
|
||||
keys are bound to `enter-tile-select`, which moves nothing.
|
||||
- **The import wizard: the overlay wins.** `hint_line(&AppMode::ImportWizard)`
|
||||
is derived too, but from the *wrong input* — the wizard's real keys are
|
||||
resolved per step inside `HandleWizardKey`, and one static line off the mode's
|
||||
keymap is wrong on four of its eight steps, including telling the user `Esc`
|
||||
cancels on `Done` where `Esc` goes back. So `draw_status` suppresses its hint
|
||||
fallback for that one mode and lets the overlay's per-step hints stand alone.
|
||||
|
||||
Two things worth keeping from how this survived. `every_hint_key_resolves_in_its_mode`
|
||||
checks `AppMode::hint_text()` against the keymaps and **cannot see a string
|
||||
literal in a widget**, which is the same blind spot improvise-wk18 names for
|
||||
`help.rs`. And the widget's own characterization test renders at 80 columns,
|
||||
where the hint does not fit and is never drawn — so the test that pins this
|
||||
widget's output was structurally incapable of showing either hint. A fixture
|
||||
width is part of what a test can see.
|
||||
|
||||
The longer-term shape the ticket names is still open: the wizard's per-step keys
|
||||
belong in `advertised()` keyed by step, which would make them derived like
|
||||
everything else and let the status bar carry them correctly instead of being
|
||||
switched off.
|
||||
|
||||
### The caches are built here, per frame
|
||||
|
||||
`project::formulas_of`, `category_tree_of`, `view_list_of`, `tile_bar_of`,
|
||||
@@ -272,3 +338,44 @@ draw one cache and nothing else. Two facts worth keeping:
|
||||
the selected tile still fits in the available width, and draws `‹`/`›` overflow
|
||||
indicators. That is the file's only piece of real arithmetic and it is local
|
||||
geometry, not a shared derivation.
|
||||
|
||||
## `,` walks the view history forward (improvise-htl6)
|
||||
|
||||
`view-forward` was bound in **no** keymap, in either front end, from the day it
|
||||
was written. The TUI reached it as `:view-forward` and a browser could not reach
|
||||
it at all, because `declined()` refuses to enter `CommandMode` at the door. It
|
||||
survived because pressing `<` twice is the only way to observe the forward stack
|
||||
in the first place, and until PR #115 no client maintained one.
|
||||
|
||||
**The key is `,`, and `>` is why the obvious answer is wrong.** `<` is
|
||||
`view-back` and `>` is `drill-into-cell`, so the pair that *looks* symmetric is
|
||||
not one — `>` moves within the grid, `<` moves between views. Binding the
|
||||
forward half to anything that reads as `>`'s partner would deepen exactly the
|
||||
confusion that hid the gap. `,` is the unshifted twin of `<`, so both halves of
|
||||
the history sit on one physical key, and it carries no direction-in-the-grid
|
||||
meaning to collide with.
|
||||
|
||||
**It is deliberately not a `Ctrl-` key.** `Ctrl-R`, the universal "forward
|
||||
through a history" spelling, is browser reload; `Ctrl-S` and `Ctrl-W` are
|
||||
already bound here and `Ctrl-S` is already a filed bug for precisely that reason
|
||||
(improvise-clso, "Ctrl-S saves and also opens the browser's own save dialog").
|
||||
The keymap is **shared** between the two front ends, so a binding chosen for the
|
||||
terminal is a binding the browser must also live with, and adding a second
|
||||
instance of an open bug is not a thing to do by accident.
|
||||
|
||||
Nothing else moved. `help.rs` keys its lines off the command *name* and looks
|
||||
the key up in the keymap, so the overlay and the hint line follow the binding;
|
||||
`every_bound_command_name_is_registered` already covers the new entry. The
|
||||
absence assertion PR #117 put in
|
||||
`resolve::tests::the_forward_step_mirrors_its_cmd_and_no_key_reaches_it` was
|
||||
**inverted rather than deleted** — the test is renamed `…_and_one_key_reaches_it`
|
||||
and now asserts the binding exists, with `view-back` kept as the control so it
|
||||
cannot pass against a table that binds everything or nothing. Coming there and
|
||||
saying so was the point of that assertion, and an inverted guard keeps the
|
||||
property under watch where deleting it would have let the key quietly go away
|
||||
again.
|
||||
|
||||
`,` is not advertised in `AppMode::Normal`'s hint line, and neither is `<`: the
|
||||
hint is width-constrained and the history lives on help page 5 under "View
|
||||
history" with both halves. That is the #149 lesson — a hint that does not fit is
|
||||
never drawn, so the test pinning the widget cannot see it.
|
||||
|
||||
+6
-4
@@ -194,11 +194,11 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
|
||||
|
||||
26. **Axis presentation lives on `Axis`, colors resolve in `draw.rs`.** Four vocabularies hang off the enum in `view/axis.rs` — `plain_label()` ("Row"/"Col"/"Page"/"None"), `glyph()` (which `Display` appends, giving "Row ↕"), `short_label()` (three columns for the tile bar; `None` is "·", *not* a truncation), and `initial()` (one letter for the view panel; `None` is empty). Plus `color() -> AxisColor` and `next()` for the Row→Column→Page→None rotation. `AxisColor` is a semantic palette: `improvise-core` must not name a ratatui type, so `draw.rs::axis_fg` does the one mapping to a terminal color, exactly as `minibuf_fg` does for `MinibufColor` (improvise-rml). The tile bar and category panel each carried their own copy of the color table before, with nothing keeping them in agreement. Not covered here: `persistence/format.rs` maps `Axis` to the on-disk keywords ("row"/"column"/"page"), which is a format concern and stays in the io layer.
|
||||
|
||||
27. **`Axis::Filter` constrains without being navigable.** Drilling fixes its coordinates on `Filter`, not `Page` (improvise-dz8): both restrict which records are visible, but only `Page` is something the user may move or page through. The split shows up as two different questions, and picking the wrong one is the trap: **"what data is visible" is `View::filtering_cats()`** (Page ∪ Filter — used by `GridLayout` and by `add-record-row` when it builds a key from the current constraints), while **"what can the user act on" is `categories_on(Axis::Page)`** (used by `[`/`]` page cycling) or `View::tileable_cats()` (used by all three tile-select sites). `Axis::next()` maps `Filter` to itself and no navigable axis rotates into it, so tile select cannot hand one out. The `.improv` grammar accepts `filter` and round-trips a selection with it — a Filter axis only ever occurs in the never-persisted `_Drill` view, but the alternative was writing it out as some other axis.
|
||||
27. **`Axis::Filter` constrains without being navigable.** Drilling fixes its coordinates on `Filter`, not `Page` (improvise-dz8): both restrict which records are visible, but only `Page` is something the user may move or page through. The split shows up as two different questions, and picking the wrong one is the trap: **"what data is visible" is `View::filtering_cats()`** (Page ∪ Filter — used by `GridLayout` and by `add-record-row` when it builds a key from the current constraints), while **"what can the user act on" is `categories_on(Axis::Page)`** (used by `[`/`]` page cycling) or `View::tileable_cats()` (used by all three tile-select sites). `Axis::next()` maps `Filter` to itself and no navigable axis rotates into it, so tile select cannot hand one out. The `.improv` grammar accepts `filter` and round-trips a selection with it — a Filter axis only ever occurs in the never-persisted `_Drill` view, but the alternative was writing it out as some other axis. **What a virtual view must *be* is model-side and has one home** (improvise-gd28): `view::{records_spine, records_steps, drill_steps}` return `Vec<ViewStep>` — pure data naming no `Effect`, no `App` and no front end — and `cmd/grid.rs::steps_into_active_view` is the whole of *how the TUI gets there*, twenty lines a browser answers its own way. Before the split, the drill's filter placement and the records spine were spelled out twice in `cmd/grid.rs` with nothing keeping them in agreement. `DrillState` deliberately did **not** move: it holds an `Rc` and its `pending_edits` are uncommitted writes, so gotcha 30's session-local tag already places it. Reasoning: `context/code/core.md` §11.
|
||||
|
||||
28. **The browser backend is sync on purpose, with an async seam.** `OpfsFs` (`improvise-io/src/opfs.rs`, wasm32 only) implements the same synchronous `Storage` trait as `PhysicalFs`, because `FileSystemFileHandle.createSyncAccessHandle()` yields a handle whose read/write/truncate/flush **are** blocking — available only inside a dedicated Web Worker, which is where improvise-9x6 puts the app. So `Storage` never became async. What is still async splits out into four explicit methods the worker awaits from its message loop: `open()`, `attach(path)` (must precede any read/write of that document — acquiring a handle is async even though using it is not), `refresh()` (repopulates the index `list` answers from), and `commit_deletes()`. Consequence: **`delete` is visible immediately but lands in OPFS at the end of the batch** — it drops the name from the index and queues a `removeEntry`. Handles are exclusive per file, so `attach` closes any handle it replaces and `detach` releases the lock. `storage_contract` in `storage.rs` is the behaviour every backend owes; it runs against both host backends and is the written-down spec `OpfsFs` must satisfy in a browser, where the harness cannot reach. **And the premise is dedicated-worker-only, which is narrower than this paragraph used to read.** `createSyncAccessHandle` is spec'd `[Exposed=DedicatedWorker]`, and measured in a module `SharedWorker` on Chromium and Firefox it is **absent** — not rejected, not flagged, not transient: `getDirectory()` and `getFileHandle()` both succeed and the method is simply not on the object (improvise-12ha). So `OpfsFs` cannot be hosted by a `SharedWorker` as written, and "a browser worker" in this paragraph means a *dedicated* one everywhere it appears. `OpfsFs::open`'s own guard does not enforce that — it `dyn_into`s `WorkerGlobalScope`, which a `SharedWorkerGlobalScope` **satisfies**, so a shared worker sails past a check whose message names the right requirement and fails later as a raw `TypeError` deep in the glue (improvise-7gip). The consequence for multi-tab is that the machinery everyone reaches for first is the storage seam, not `Sessions`: `Sessions`, the `Addressed` filter and the one `frame` pair are all ready, and none of that helps, because the no is entirely on the handle.
|
||||
|
||||
29. **"Which view is active" is stored twice, and the copies are now allowed to differ.** `Workbook::active_view` is the document's **default** — what a freshly connected client opens, and what `.improv` writes as `Initial View`. `ViewState::active_view` is **authoritative for that client**, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi); the *split* is improvise-jytm answering improvise-g0f, on the user's ruling that **the model is shared and the UI is not**. Two clients may sit on different views of one document. Three things follow. **`App::resolve_view` is the one answer** to what a view name names in this document, falling back to the default so a slice naming a deleted view cannot panic in `rebuild_layout`; `view()`, `view_mut()`, `view_name()`, `project::grid_in` and `ClientSession::view_in` all go through it, and a second copy of that fallback was the first thing review found. **The guard was replaced, not deleted** — `the_two_active_view_names_never_diverge` became `the_documents_default_view_outlives_this_clients_switches` plus `a_fresh_session_opens_the_documents_default_view`, because a deliberate split with no guard is how one silently regresses into the other. And **a deleted view is the case the split creates**: `DeleteView` can take away the view another client is on, so `Sessions::adopt_deleted_views` moves that mirror to the default and tells it, and `effect::DeleteView` does the same for the host — observed ("does this mirror still name a view the document has"), never predicted from the command variant. **Testing any of it needs a bystander that can be moved**: both audience tests first sat theirs on the default, where a wrongly-addressed relay folds to a no-op, and review demonstrated the vacuity by injecting `ToAll` twice. Two gaps remain, both filed: a view-scoped `ModelCommand` names no view, so a connected client's `SetAxis` lands on the host's view (improvise-lltt — deferred on a *condition*, that no shipped client emits `SwitchView`, while the browser already emits `TransposeAxes`), and `Initial View` no longer follows the TUI user (improvise-azhb). Follow mode is deliberately not foreclosed.
|
||||
29. **"Which view is active" is stored twice, and the copies are now allowed to differ.** `Workbook::active_view` is the document's **default** — what a freshly connected client opens, and what `.improv` writes as `Initial View`. `ViewState::active_view` is **authoritative for that client**, and it is what keys `ViewState::cursors`. The second copy exists because `reduce_view` runs on a client that holds no `Workbook` (improvise-cqi); the *split* is improvise-jytm answering improvise-g0f, on the user's ruling that **the model is shared and the UI is not**. Two clients may sit on different views of one document. Three things follow. **`App::resolve_view` is the one answer** to what a view name names in this document, falling back to the default so a slice naming a deleted view cannot panic in `rebuild_layout`; `view()`, `view_mut()`, `view_name()`, `project::grid_in` and `ClientSession::view_in` all go through it, and a second copy of that fallback was the first thing review found. **The guard was replaced, not deleted** — `the_two_active_view_names_never_diverge` became `the_documents_default_view_outlives_this_clients_switches` plus `a_fresh_session_opens_the_documents_default_view`, because a deliberate split with no guard is how one silently regresses into the other. And **a deleted view is the case the split creates**: `DeleteView` can take away the view another client is on, so `Sessions::adopt_deleted_views` moves that mirror to the default and tells it, and `effect::DeleteView` does the same for the host — observed ("does this mirror still name a view the document has"), never predicted from the command variant. **Testing any of it needs a bystander that can be moved**: both audience tests first sat theirs on the default, where a wrongly-addressed relay folds to a no-op, and review demonstrated the vacuity by injecting `ToAll` twice. One gap remains: `Initial View` no longer follows the TUI user (improvise-azhb). The other — a view-scoped `ModelCommand` naming no view — is **closed** (improvise-lltt), and this paragraph described it as an open gap for a day after: every view-scoped variant carries `view: String` and `model_effects` maps it to `ViewTarget::Named`, so a connected client's `SetAxis` lands on the view it names. `ViewTarget::Active` resolves through `App::view_mut()` and is therefore the *host's* view — but **the wire never constructs one**, which is what closed it. Follow mode is deliberately not foreclosed.
|
||||
|
||||
30. **Two tests gate `ViewState` membership.** `ViewState` is the *portable* session slice — the one thing serialized into the Snapshot a newly-connected client gets (improvise-q08). A field may live there only if it is **portable** (plain data from the allowlisted crates, no `Rc`/`Arc`) and **snapshot-safe** (a fresh client inheriting the value is correct). `wizard` fails both — `ImportWizard` is an `improvise-io` type, and a half-open dialog is not something to inherit — and `drill_state` fails both — it holds an `Rc`, and `pending_edits` are uncommitted writes belonging to whoever staged them. Both live on `App` under the **session-local** residue tag (improvise-h0b). The `ViewState: Send` static assertion in `app.rs`'s tests is the compile-time proxy for portability: `Rc` is not `Send`, so a re-added shared pointer fails the build (the `Deserialize` derive from gotcha 25 now rejects it too, serde's `rc` feature being off). `Send` does *not* catch the origin-crate or snapshot-safety halves — check those by hand.
|
||||
|
||||
@@ -221,7 +221,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
|
||||
39. **`Applied` is an enum because a document swap is not a diff.** `Sessions::apply_checked` returns `Applied::Diff { dirty, broadcast }`, `Applied::Rebased { snapshots }` or — since ADR 001 layer 1 — `Applied::Rejected { reason }`, and the enum *is* the mechanism: a host that wants the `Dirty` must match, and matching names the case where a `Snapshot` is owed. The server's mirrors self-heal — `ClientSession::project` reseeds on the `document_generation` bump — but `ViewState::cursors` lives on the **client**, keyed by the outgoing document's view names (gotcha 24), and only a fresh `Snapshot` replaces it. `Command::Model` put the three `WholeWorkbook` mutations on the wire (gotcha 38), so the host that must send the snapshots is no longer the host that asked. `apply_model` therefore calls `rebase` itself rather than instructing the host to, exactly as `apply_switch` adopts the accepted view itself. **The trigger is the generation moving, not the command variant**: a `LoadModel` that cannot read its path leaves the document alone, and reseeding on the variant would cost a client its cursors for a *failed* load — `a_load_that_fails_is_not_a_rebase` guards that arm. A `rebase: bool` beside `dirty` was rejected, because the host that forgets to call `rebase` is the host that forgets to read the flag. The **third** case is a *normal outcome*, not an error, and it is in the enum for the same reason the other two are: a host that wants the `Dirty` must match, and matching names it. `ServerMsg::Rejected` is the matching frame, addressed to **the asker alone** — nothing happened, so nobody else is owed anything — and the client keeps it as a `Rejection` rather than a sentence, because force needs to know *which* record and *which* kind (gotcha 43).
|
||||
**Fixture warning.** When testing a document swap, the incoming document's active view must have the **same name** as the outgoing one. A rename makes `apply_model` broadcast a `SwitchView`; the client folds it, and the stale cursor is then read under a key nothing has written — right *by accident*, with the bug invisible. `other_document()` renames, and a first version of this test passed because of it; `a_document_shaped_differently_under_the_same_view_name()` keeps the name and changes only the shape, so there is nothing to hide behind.
|
||||
|
||||
40. **A batch's basis is three tiers, and only two of them can be read off the effect.** `App` carries `document_generation` (bumped by `install_workbook`), `layout_version` (per `ModelChange::LayoutGeometry` effect) and `record_versions` (per *record*) — ADR 001's stale-basis check, and shared document state, so never on `ViewState` (gotcha 30). **The unit is the record**: `record_of(key)` is the coordinate tuple minus `_Measure` (`record_row_identity`), because renaming a category coordinate in wide records mode moves every measure cell in the row (gotcha 1) — a cell-level unit is violated by one legal edit, so the unit must be at least as coarse as the coarsest legal write. Both tiers are read in `apply_effects` off the same `EffectKind` the `Dirty` is: one classification, two readers, no second table to drift from gotcha 35's. `layout_version` bumps **inside** the loop for the reason `Dirty` accumulates there — an aborted batch must not claim effects it never ran (gotcha 21). `record_versions` **cannot**, because an `EffectKind` says *that* a batch writes cells, never *which*; so the records are **observed**, via a per-record XOR fingerprint of the raw data taken when the batch declares a `CellData` effect and diffed after it. Three consequences. A write landing the value already there **bumps nothing**, which is correct — no basis was invalidated. A `LayoutGeometry` effect that drops cells **will** bump their records when it shares a batch with a `CellData` one: an over-bump costing at most an unnecessary rejection, the same direction `Dirty::of` is deliberately sloppy in. And only *raw* data is fingerprinted — formula results live in `Model::formula_cache`, and a record whose derived value moved because someone edited elsewhere has not itself been written. The diff collects into a **set**: chaining `before.keys()` with `after.keys()` counts any rewritten record twice, a bug that survived one test run.
|
||||
40. **A batch's basis is three tiers, and only two of them can be read off the effect.** `App` carries `document_generation` (bumped by `install_workbook`), `layout_version` (per *view shape*) and `record_versions` (per *record*) — ADR 001's stale-basis check, and shared document state, so never on `ViewState` (gotcha 30). **The unit is the record**: `record_of(key)` is the coordinate tuple minus `_Measure` (`record_row_identity`), because renaming a category coordinate in wide records mode moves every measure cell in the row (gotcha 1) — a cell-level unit is violated by one legal edit, so the unit must be at least as coarse as the coarsest legal write. **Only the `Dirty` is read off the `EffectKind`; both lower tiers are *observed* around the loop** — the source says so at `apply_effects` and this paragraph said the opposite until improvise-ilvl's audit. The `EffectKind` decides only *whether to look*: `touches_model` arms the shape snapshot, `writes_cells` the record fingerprints. `record_versions` **must** be observed, because an `EffectKind` says *that* a batch writes cells, never *which*; `layout_version` is observed for a better reason than necessity — `observe_reshape` diffs `view_shapes` before and after and bumps only where a view's shape actually moved (improvise-pdvm/nynk), so it is a **fact about the document** rather than a claim about the batch. Two consequences the declared form did not have. An **idempotent** `set-axis` bumps nothing, and batching two real changes into one line or two gives the same `2`. And the gotcha 21 argument is subsumed rather than restated: an aborted batch moved no shape, so there is nothing to not-claim — which is why observing once **after** the loop is correct where accumulating `Dirty` inside it is. Note the arming is deliberately wider than the tier: a `CellData`-only batch that prunes a row out of a view **does** bump `layout_version`, because the shape moved. Three further consequences. A write landing the value already there **bumps nothing**, which is correct — no basis was invalidated. A `LayoutGeometry` effect that drops cells **will** bump their records when it shares a batch with a `CellData` one: an over-bump costing at most an unnecessary rejection, the same direction `Dirty::of` is deliberately sloppy in. And only *raw* data is fingerprinted — formula results live in `Model::formula_cache`, and a record whose derived value moved because someone edited elsewhere has not itself been written. The diff collects into a **set**: chaining `before.keys()` with `after.keys()` counts any rewritten record twice, a bug that survived one test run.
|
||||
|
||||
41. **`Storage` is not a VFS, and that closes the SQLite question.** The trait is whole-document `read`/`write` bytes (gotcha 2, `invariant(storage_is_two_ops)`) — no offsets, seeking, handles or locking — so an `sqlite3_vfs`, which needs `xRead(offset,len)`/`xWrite`/`xTruncate`/`xLock`, has nothing to bind to. SQLite *can* still sit behind it, as an in-memory database serialized to a blob (`rusqlite` `serialize`/`deserialize_read_exact`, proven with a probe before declining — declining on a capability that actually exists would have been the wrong decline). It is worth nothing: measured on 18,000 cells the SQLite container is **22% larger** than `.improv`, **4.7× larger gzipped**, ~**160×** larger at the floor (24 KB against ~150 bytes), and still rewritten whole on every save, so the one dimension SQLite normally wins — incremental page writes — is unavailable. improvise-9ix was declined on that evidence. Two things generalise past SQLite. **A second serialization format is a second implementation of the grammar**, obliged to track `improv.pest` forever; and **its own round-trip test cannot detect drift from `.improv`**, because it never compares the two — that missing cross-format equality test is what made the ticket look cheap. If a structured-query story is ever wanted it is a one-way `export_sqlite` beside `export_csv`, native-only and feature-gated: `libsqlite3-sys`' C build fails under this repo's nix cc-wrapper for `wasm32-unknown-unknown`, which would break the `wasm` CI gate the day it landed.
|
||||
|
||||
@@ -237,6 +237,8 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
|
||||
|
||||
44. **The TUI grid draws from its cache, and since improvise-anr9 the cache frames it too.** `GridWidget` holds no `&Model` and no `&View`: cell text, aggregate-ness (`CellId::Aggregate`, ADR 002), column widths, the page bar, the totals strip and now the **frame** all come off `GridCache`. The TUI still builds that cache over the **whole** grid (offsets zero), so a cache position is an absolute position — the browser's `Viewport` is a real window, the TUI's is not, and the two clients therefore use it differently; column totals **must** be projected rather than client-derived, because the sum runs over every row *including the ones a viewport scrolled past*. The frame is an `AxisFrame` per axis — `cats` (the categories, outermost first) plus `entries: Vec<FrameEntry>` interleaving `Group(FrameGroup)` headers with `Item(Vec<String>)` label *tuples*, exactly as `GridLayout::row_items` interleaves them. That replaced a `Vec<String>` built by `GridLayout::row_label`, which slash-joined the tuple — the exact rendering `two_row_categories_produce_cross_product_labels` says must not appear — so the browser drew `Food/Alice` where the TUI drew two header columns, with no groups and no collapse indicators. It is **one** type and not a `labels` plus a `levels` beside it: two answers to "what are the labels" is the disease (improvise-edp), and this is the second cure, which cost a *type* change rather than an addition. Three things to know before touching it. **`leaf_labels()` is not "the labels"** — it is the innermost label, what a one-line summary calls a row by; a draw loop wants `header_labels(first_rendered)`, `groups()` and `shares_ancestry(sel)`, each of which is one pass, because `item`/`label`/`shows_level`/`group_of` scan `entries` from the start and asking them per position is quadratic in the frame *and grows with the TUI's absolute scroll offset* (found three times on anr9; the per-position forms survive for one-off lookups and say so). **The width rule has one home**, `improvise_protocol::header_level_widths` — widest label per level, plus a gap, clamped — reached by `AxisFrame::header_widths` and by `ui::viewport::compute_row_header_width`, which borrows level tuples off a `GridLayout` rather than building a frame on the keypress path. **A `cats` change re-identifies every position**: a cell's identity is `cats` zipped with its tuple, so `invalidate_relabelled` blanks the whole axis when the categories move, not merely the positions whose tuples differ, and blanks the **totals** on a recategorization of *either* axis — a column total is a sum over the rows, so re-identifying the rows leaves a number answering coordinates the grid no longer has (gotcha 33(b): the rule lives in `apply`, not in a producer's memory). What is still **not** on the cache is `record_col`, and that is correct: it keys pending drill edits and is session-local (gotcha 1). `body_from(idx)` takes an argument only because the two front ends scroll differently — a browser's frame is already windowed and passes `0`, the TUI's covers the whole grid and passes its row offset — not because the rule differs. `group_of` answers group **membership**: the nearest preceding header *unless* an `AxisEntry::GroupEnd` closed the run first (improvise-gd83, landed in PR #77). It previously answered "nearest preceding header" unconditionally, which gave an ungrouped item the preceding group. `GroupEnd` draws nothing and exists only so membership is not inherited. Both of the gaps this sentence used to list are **closed**, and it named them as open for a day after: `group_runs` coalesces by **header identity** (`std::ptr::eq`), not by name, so two runs under two headers equal as values are two runs (improvise-h0iv); and `AxisFrame::first` carries **no** `serde(default)` — its doc comment says why, since zero is a claim and not an absence (improvise-1wl0). `doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)` is about exactly this paragraph, twice now. And `project::grid_of` evaluates every cell per frame where the widget evaluated only the visible ones — a widening of an existing whole-grid cost. Since improvise-jytm `grid_in` may additionally **derive a `GridLayout`** for a session on a view the host is not on; the host's layout is reused where the names agree, which is the common case and the only one that can carry a drill snapshot.
|
||||
|
||||
**`RecordHash` is document-scoped; `AggregateHash` is layout-scoped and cannot be otherwise** (improvise-gd28). A `RecordHash` falls back to `DataStore`'s maintained index and so resolves against the **document**, which is why a connected client's `SetCell` works from any view. An aggregate has no existence outside the layout that derives it, so there is no index to fall back to and none can be built — `App::resolve_aggregate` therefore answers only for the **host's** active view, which since improvise-jytm is not the client's. **That, and not ADR 001 layer 2, is what blocks browser drill.** The ticket's blocking argument was right about the *commit* (`ApplyAndClearDrill` is addressed to one session, gotcha 38) and wrong about the *entry*: entering a drill needs no commit, and `CreateView`/`SetAxis`/`SetPageSelection` are already unaddressed `ModelCommand`s that name their view. The gap is one level down from where it was looked for — **the mutation and its operand have different scopes**: the mutation configures the shared `_Drill` and is addressed to nobody, while the operand exists only inside one session's layout. So the fix is not a new mechanism but the `view: String` convention every other view-scoped variant already follows, resolved against a layout derived for that name (improvise-obtu). Two things travel with it: `improvise-fdk1` is **not** needed, because `edit_or_drill` declines records-mode editing out loud before reaching a cell, so a browser cannot edit inside a drill grid at all; and closing it makes improvise-t219 live, `_Drill` being one shared view in `Workbook::views` that two clients drilling would contend for — the same masking relationship improvise-6uva has with improvise-up8g. Guarded by `an_aggregate_a_session_was_shown_does_not_resolve_against_the_hosts_view`, whose **control** is the load-bearing half — the same surrogate resolves once the host is moved onto that view, so a blanket `None` cannot pass.
|
||||
|
||||
45. **One session loop, two transports — and the browser saves itself.** `Host` is `App` + `Sessions` + `handle(id, ClientFrame) -> Vec<Addressed>` and lives in `improvise_command::ui::host`, **not** in `improvise-ws-server`, which re-exports it. It moved for improvise-9x6: the worker hosts the same authoritative `App` over a `MessagePort` and cannot depend on tokio, and a second copy would have been exactly the mirror improvise-xks6 had just deleted for the frames — a drifting `Applied`/`Broadcast` match is invisible until a client sees it. `registry` stayed on the server, because a worker hosts **one** document. What the worker adds is only what a `MessagePort` differs in: **one client by construction** (the `Addressed` filter is honoured anyway, against a future `SharedWorker`), and **a handshake with no close** — `ClientFrame::Viewport` must still come first, but a pre-handshake frame is *dropped* rather than closing a connection that does not exist.
|
||||
|
||||
**Saving is a frame, and dirtiness is read off the classification the effect already carries.** `effect::Save` is `ViewOnly` (gotcha 3) — correctly, since it writes *this host's* `file_path` through *this host's* `Storage` — so the derivation that builds `ModelCommand` from the model effects cannot name it, and for a while that meant no client on any transport could ask its host to save. `ClientFrame::Save` closes it (improvise-368h): a **request** carrying no path, because a client cannot see the host's storage, answered `ServerFrame::Rejected(NoDocumentPath | SaveFailed)` to the asker alone and **silently on success** — nothing moved, so no widget is dirty. It is a frame and not a `Command` for one concrete reason: as a `Command` it put a runtime-fallible operation inside a batch `ClientFrame::Commands` promises is all-or-nothing, so `[Save, SetCell]` reported a failed write and then applied the edit. It also fails the test every `Command` passes — it mutates no shared state and no client's slice, so `reduce_view` has nothing to fold and `Checked` has no basis to answer for. **And `MarkDirty` is gone.** It was a separate effect the *command* layer composed beside each mutation — reasonable on the key path, unreachable from anywhere else, because improvise-kmgs derived `ModelCommand` from the effects **one at a time** and so put `SetCell` on the wire without its companion (improvise-gv9r): a derivation correct per element and wrong for a set, whose symptom was that "save if dirty" never fired. `App::apply_effects` now raises the flag for any `EffectKind::Model(_)`, all three `ModelChange` levels included, which makes omitting it unrepresentable rather than validated. It over-approximates in the same direction `layout_version` does — a `LoadModel` that cannot read its path leaves the flag raised, costing one redundant write — and `Save` clears it in loop order, so a batch that edits and then saves ends clean. The worker's persist trigger is therefore the **flag**, not a table of command variants: an observation rather than a second description of which commands change the document, for the reason `cursor_epoch` beats matching on `CycleAxis` (gotcha 24) and `document_generation` beats matching on `LoadModel` (gotcha 39). It is also strictly tighter — a refused batch raises nothing, so the "was anything rejected" guard went with it. A browser still never *sends* `ClientFrame::Save`, and that is the answer rather than a gap: there is no file the user owns and no Ctrl-S semantics, the document *is* the OPFS entry, so the worker persists on its own behalf through the one existing `Save` effect. The cost that remains: `Storage` is whole-document (gotcha 41), so OPFS rewrites the file per accepted model command.
|
||||
@@ -251,7 +253,7 @@ Region=West, Measure=Revenue = |pending| # pipe-quoted text
|
||||
|
||||
47. **Keys become commands in the browser, and the server must not know the keymap.** The user's ruling on improvise-bpq2, following from "the model is shared, the UI is not" — and it buys two clients different bindings over one document. What made it urgent is **not** latency. `a_key_resolved_on_the_host_moves_state_no_client_can_hear` shows nothing in the projection stream carries the host's own `ViewState`, so host-side resolution **diverges**; and `Host::handle` answers `ClientFrame::Key` with an explicit empty vec, so **an upstream key is a black hole, not a slow path** — every `KeyOutcome::Upstream` names a binding that does nothing at all in a browser. That test still holds and was not flipped: bpq2 closes the divergence by *avoiding* it. **The method is the ticket's** — enumerate the **bindings**, not the commands (`doctrine(binding_dump_diff)`) — and where a client cannot compose the `Command`, **enrich the projection** rather than moving resolution back.
|
||||
|
||||
Three things that cost. **`GridExtent` on `GridCache`** — the whole grid, of which the labels are one window — because a viewport-indexed cache cannot tell "the last visible row" from "the last row". It is deliberately **not framed by the labels** (`fit_to_labels` leaves it alone) and its batch position is not load-bearing the way `RowFrame`-before-`Cells` is, since it addresses no position; the producer reads `GridLayout::row_count()`, and reading it off the projected ranges is **invisible in the TUI** (which builds at offsets zero, gotcha 44) and is exactly the browser bug. **The window is the client's own fact**, recorded by `Client::set_viewport` from what it announced — *not derivable*, since the row frame's own length is the window **clipped to the grid** and so differs precisely at the bottom edge where the scroll arithmetic matters. `Client::viewport` (associated, no recording) still exists and is the one that quietly turns cursor keys off; `web/app.js` does call `set_viewport`. **`ClientFrame::Commands(Vec<Command>)`** replaces the singleton, because one cursor key is `SetSelected` plus the offsets `viewport_effects` returns and a host that folds the selection but not the offset projects the wrong window. It is **answered whole, then applied whole** (improvise-955n): `CheckedBatch::against` checks the frame before anything runs, so there is no longer a moment at which part of a frame has landed and the rest has been refused, and `Applied::Rejected` is unreachable through `Host::commands` — still matched, because a host that wants the `Dirty` must name every case (gotcha 39). The contract is four clauses and **clause 4 is not delivered**: one basis per frame, checked once, applied in order with each command seeing the last one's state, and all-or-nothing. The fourth is open because `App::apply_effects` aborts without undoing (gotcha 21 — an abort is not a rollback), so `[RemoveItem("North"), SetCell(the North record)]` lands the removal, resolves the write to nothing, and reports the frame accepted having applied half of itself — partial *and* silent. It is unreachable from any client in this workspace, the only multi-command producer being `keys::commit`; **reachability is a reason not to rush a fix and not a reason not to notice one** — the lost update `CheckedBatch` fixed had the same reachability argument and none of the documentation, which is how it shipped.
|
||||
Three things that cost. **`GridExtent` on `GridCache`** — the whole grid, of which the labels are one window — because a viewport-indexed cache cannot tell "the last visible row" from "the last row". It is deliberately **not framed by the labels** (`fit_to_labels` leaves it alone) and its batch position is not load-bearing the way `RowFrame`-before-`Cells` is, since it addresses no position; the producer reads `GridLayout::row_count()`, and reading it off the projected ranges is **invisible in the TUI** (which builds at offsets zero, gotcha 44) and is exactly the browser bug. **The window is the client's own fact**, recorded by `Client::set_viewport` from what it announced — *not derivable*, since the row frame's own length is the window **clipped to the grid** and so differs precisely at the bottom edge where the scroll arithmetic matters. `Client::viewport` (associated, no recording) still exists and is the one that quietly turns cursor keys off; `web/app.js` does call `set_viewport`. **`ClientFrame::Commands(Vec<Command>)`** replaces the singleton, because one cursor key is `SetSelected` plus the offsets `viewport_effects` returns and a host that folds the selection but not the offset projects the wrong window. It is **answered whole, then applied whole** (improvise-955n): `CheckedBatch::against` checks the frame before anything runs, so there is no longer a moment at which part of a frame has landed and the rest has been refused, and `Applied::Rejected` is unreachable through `Host::commands` — still matched, because a host that wants the `Dirty` must name every case (gotcha 39). The contract is four clauses and **all four are now delivered** — clause 4 by improvise-yqpd on 2026-09-21, whose mechanism is `App::snapshot`/`restore` plus a `Sessions` clone taken in `Host::mutating_frame` and put back by `undo_frame`, deliberately *not* inside `apply_effects`, which runs on every keypress. **This paragraph said it was undelivered for a day after it landed**, and an orchestrator brief repeated that from here, so read the clause list as a claim to check. A delivered clause 4 is also what makes the CLIENT's rollback well-defined (improvise-nxah): a refusal means the host applied none of the frame and restored every mirror, so a client keeping its optimistic fold is the only thing left out of step. What rolls back there is `ViewState`, never a cache — a `Command::Model` is a no-op in `reduce_view`, so the optimistic fold never reached one. The old text, kept because the hazard it names is still the right shape to fear: one basis per frame, checked once, applied in order with each command seeing the last one's state, and all-or-nothing. was that the fourth was open because `App::apply_effects` aborts without undoing (gotcha 21 — an abort is not a rollback), so `[RemoveItem("North"), SetCell(the North record)]` would land the removal, resolve the write to nothing, and report the frame accepted having applied half of itself — partial *and* silent. That is what `yqpd` closed, and it closed it **outside** `apply_effects` for the reason the old text implies: the abort lives on the keypress path and must stay cheap. The old note that it was *unreachable* from any client here, the only multi-command producer being `keys::commit`, still reads correctly as a caution — **reachability is a reason not to rush a fix and not a reason not to notice one**, the lost update `CheckedBatch` fixed having had the same reachability argument and none of the documentation, which is how it shipped.
|
||||
|
||||
Two corrections to the ticket's own gap list, both worth more than the code. **Panel cursor movement was never a gap**: improvise-edp had already made `viewmodel::flatten_cat_tree` the one home both ends call, so it is the *shared* derivation, not a second one — worth re-checking the "known gaps" in other old tickets against what has landed since. And the "unmodified characters only" rule was **mis-scoped, not wrong**: it was about `KeyPattern::AnyChar`, which is bound only in the text-entry modes, so `Ctrl-d` in Normal has an exact binding and mirroring it is a faithful subset. Each resolver mirrors its `Cmd` **line for line, including where siblings disagree** — `Move` and `MoveTileCursor` emit even when they clamp, `MovePanelCursor` returns nothing when the cursor does not move. Tidying that into one rule would make the client a second derivation, which is what `tests/lockstep.rs` exists to rule out. What still needs the host is a **named list** in `keys.rs` and in `the_normal_mode_keys_that_still_need_the_host_are_these`, so closing one has to come there and say so.
|
||||
|
||||
|
||||
@@ -135,3 +135,106 @@ spawned child at a stable path.
|
||||
Runs `add-category Region` to create the document. It asserts the file exists
|
||||
afterwards rather than trusting the seed, because every test that follows would
|
||||
otherwise misreport as "the command did nothing" when the seed was what failed.
|
||||
|
||||
### a_script_can_move_the_cursor_and_the_next_command_sees_it
|
||||
|
||||
`improvise cmd` and `improvise script` resolved every command through
|
||||
`CmdRegistry::parse` — the **pure** arm, which builds `CursorState::default()`
|
||||
(row 0, col 0, zero rows, zero columns). The interactive arm, which builds
|
||||
`CursorState::from_ctx(ctx)`, was never reached from a script, so every `Move`
|
||||
snapshotted a zeroed cursor at *parse* time, clamped to `(0, 0)` and emitted
|
||||
`SetSelected(0, 0)` — worse than a no-op, since it **resets** the cursor
|
||||
(improvise-6uva). The reasoning is `context/code/cli.md` section 7.
|
||||
|
||||
Reproduced at `a9cad15` before the fix, on a seeded document with `Region` on
|
||||
row carrying `North, South, East`: `move-selection 2 0 . hide-selected-row-item`
|
||||
and a bare `hide-selected-row-item` both wrote `hidden: Region/North`. After the
|
||||
fix the first writes `hidden: Region/East`.
|
||||
|
||||
**Why the test has two halves.** Asserting only that the moved script hides
|
||||
`East` cannot separate "the move worked" from "the origin happens to be East".
|
||||
The second document asserts the origin is `North`, so the first assertion is
|
||||
discriminating, and its message says what breaks if that ever changes — three
|
||||
identical replays of a fixture whose moves are silent no-ops prove nothing,
|
||||
which is how this survived five months.
|
||||
|
||||
Two faults were injected, one per layer, and both fail this test:
|
||||
|
||||
- **`parse_segment_interactive` calls `ctx.registry.parse` instead of
|
||||
`.interactive`** — the bug itself, one level down.
|
||||
`left: "hidden: Region/North" right: "hidden: Region/East"`.
|
||||
- **`run_headless_commands` builds the segment up front with `parse_line`**
|
||||
rather than per segment against a live `CmdContext` — the composition half.
|
||||
Same failure. The unit test in `parse.rs` passes under this second one, which
|
||||
is why both tests exist.
|
||||
|
||||
### a_script_cursor_move_survives_the_line_that_made_it
|
||||
|
||||
The same property across two `-f` arguments rather than one dot-separated line,
|
||||
because `improvise cmd` takes several and `improvise script` is one per line:
|
||||
the cursor is `App` state and a line boundary is not a reset.
|
||||
|
||||
### a_segment_is_built_against_the_cursor_it_will_run_against
|
||||
|
||||
The unit test for the layer that owns the bug. It seats a `CmdContext` at
|
||||
`(1, 0)` in a two-by-two grid and asserts `move-selection 0 1` executes to
|
||||
`SetSelected(1, 1)`.
|
||||
|
||||
Its second half is what keeps it honest: it builds the *same* command through
|
||||
`registry.parse` and asserts that one gives `SetSelected(0, 0)`. If the pure arm
|
||||
ever learns the cursor, the first assertion stops discriminating and this half
|
||||
fails first, saying so.
|
||||
|
||||
### a_pure_commands_two_arms_agree_on_no_arguments
|
||||
|
||||
A second instance of the same shape, found by the first fix rather than
|
||||
predicted by the ticket. `CmdRegistry::register_pure` built an interactive arm
|
||||
that refused **empty** args while its own pure arm accepted them, so `w` with no
|
||||
path — save to the current document — parsed one way and refused the other. Once
|
||||
a script re-parsed through the interactive arm, `improvise cmd 'w'` and `:w`
|
||||
both broke; the suite caught it as
|
||||
`a_failed_colon_write_lands_in_normal_where_its_error_is_drawn` reporting
|
||||
`":w — w: this command requires arguments"`.
|
||||
|
||||
Both arms are now `parse`, which is what "needs no interactive context" was
|
||||
supposed to mean. Restoring the empty-args guard fails this test on the first
|
||||
`expect`: the interactive arm refuses a bare `w` the pure arm accepts.
|
||||
|
||||
### segments_drops_comments_and_blanks
|
||||
|
||||
`segments` is the split half of `parse_line_with`, extracted so the headless
|
||||
loop can re-parse one segment at a time. It answers the same three cases the
|
||||
whole-line parser did — a `#` comment, a `//` comment and a blank line are no
|
||||
commands — plus the empty segment in `a . . b`.
|
||||
|
||||
### an_unknown_segment_is_an_error_not_a_silent_skip
|
||||
|
||||
`parse_segment_interactive` returns `Ok(None)` for a segment with no word, which
|
||||
is a shape an unknown *word* must not be able to reach: a silently skipped
|
||||
command in a script is the partial-and-silent failure this repo keeps finding.
|
||||
The error also names the word, matching what `CmdRegistry::parse` does.
|
||||
|
||||
### an_explicit_coordinate_still_beats_the_cursor
|
||||
|
||||
Found by the ChatGPT Codex review of PR #151 at head `64bcb82`, and correct: the
|
||||
first version of improvise-6uva's fix introduced a regression it was not looking
|
||||
for. Four registry entries — `clear-cell`, `yank`, `paste`, `drill-into-cell` —
|
||||
were registered with `register`, and their **interactive** arms took `|_args,
|
||||
ctx|`: they discarded the arguments and used `ctx.cell_key()`. That was harmless
|
||||
while a script only ever reached the *pure* arm, which reads the coordinates. The
|
||||
moment the headless loop re-parsed interactively, `clear-cell Region/East` passed
|
||||
the syntax gate and then cleared whatever the cursor was sitting on — the origin,
|
||||
`Region/North`.
|
||||
|
||||
So improvise-6uva's own lesson had a **third** instance in its own fix: two
|
||||
construction arms that mean different things demote whichever caller reaches the
|
||||
weaker one, and here the "weaker" one was the arm that had the context. The four
|
||||
arms now go through `key_from_args_or_cursor`: explicit coordinates win, the
|
||||
cursor is the fallback, which is also what the keymap path wants since a binding
|
||||
carries no arguments.
|
||||
|
||||
The fixture writes three cells and clears the one the cursor is **not** on, so
|
||||
the assertion separates "cleared the named cell" from "cleared the cursor cell"
|
||||
— clearing `North` by name would have passed against the broken code. Restoring
|
||||
the arg-discarding arm fails it: `the cursor cell was cleared instead of the
|
||||
named one: ["Region=East = 3", "Region=South = 2"]`.
|
||||
|
||||
@@ -3253,7 +3253,7 @@ on where you can go next, and only the second press shows it. Its empty arm is
|
||||
pressed first and asserted against the host's own status, so a fixture that
|
||||
silently had history would be caught rather than quietly testing one arm twice.
|
||||
|
||||
### resolve::tests::the_forward_step_mirrors_its_cmd_and_no_key_reaches_it
|
||||
### resolve::tests::the_forward_step_mirrors_its_cmd_and_one_key_reaches_it
|
||||
|
||||
**`view-forward` is bound in no keymap, in either front end** — not a browser
|
||||
gap, and not something this ticket predicted. `Keymaps::default_keymaps()` binds
|
||||
@@ -3266,12 +3266,29 @@ only way to observe the forward stack at all.
|
||||
The arm is added anyway and tested by name, because `composed` keys on the
|
||||
command name and the keymap is the *shared* table both ends read: the day a key
|
||||
binds `view-forward`, nothing else must be touched. What keeps that from being
|
||||
unreachable code with no guard is the second half of this test, which asserts
|
||||
unreachable code with no guard is the second half of this test, which asserted
|
||||
that no keymap binds `view-forward` **and** that one binds `view-back`. The
|
||||
control matters: without it the check passes against a misspelling, which is the
|
||||
neighbouring-question defect §7 names. When a binding is added, this test fails
|
||||
and is where the gap stops being one.
|
||||
|
||||
**improvise-htl6 is that day, and the assertion was inverted rather than
|
||||
deleted.** `,` now binds `view-forward` (the argument for that key over `>` and
|
||||
over `Ctrl-R` is in `context/code/tui.md`), the test is renamed, and the second
|
||||
half now asserts the binding **exists** — with `view-back` kept as the control,
|
||||
which now guards the opposite failure: a table that binds everything, or a
|
||||
sweep that reports names it never read. A third assertion pins the key itself,
|
||||
`composed(&state, ev(Char(',')), "view-forward", &[]) == [Command::ViewForward]`,
|
||||
so the binding cannot drift onto some other key while the presence check stays
|
||||
happy.
|
||||
|
||||
**The rival: bind `,` to `view-back` instead** — the misspelling the original
|
||||
control was written against, and the one a copy-paste actually produces. It
|
||||
fails this test *and* `ui::app::tests::the_view_history_walks_both_ways_from_the_keyboard`,
|
||||
because with an empty back stack `view-back` leaves the active view where it is.
|
||||
Deleting the binding outright is the fault the pre-fix run already
|
||||
demonstrated.
|
||||
|
||||
### "the deployment notice starts collapsed and remembers being opened"
|
||||
|
||||
The page's `#backend-notice` was a static `<p>` carrying roughly ten dense
|
||||
|
||||
@@ -275,8 +275,60 @@ than decided here: a TUI user who switches view and saves now writes the
|
||||
|
||||
---
|
||||
|
||||
## `ui/app.rs` — the view history from the keyboard
|
||||
|
||||
### the_view_history_walks_both_ways_from_the_keyboard
|
||||
|
||||
improvise-htl6: `view-forward` was bound in no keymap, in either front end, so
|
||||
the forward half of the view history was reachable only as `:view-forward` in
|
||||
the TUI and not at all in the browser. `,` now binds it. Reasoning:
|
||||
`context/code/tui.md`, "`,` walks the view history forward".
|
||||
|
||||
This is the TUI half of the claim and it is deliberately end-to-end through
|
||||
`App::handle_key` rather than a keymap-table lookup: the table is what was
|
||||
wrong, so a test that reads the table cannot be the only witness. It switches
|
||||
views, presses `<`, asserts it landed back, then presses `,` and asserts it
|
||||
returned.
|
||||
|
||||
**`<` is the control and it is load-bearing.** Without it, a keymap that bound
|
||||
*nothing* would fail on the same assertion as one that bound `,` wrongly, and
|
||||
the two are different defects. **The rival injected** is `,` bound to
|
||||
`view-back` — the misspelling the sibling test's control was originally written
|
||||
against — which leaves the active view on `Default` because the back stack is
|
||||
empty by then, and fails here and in
|
||||
`resolve::tests::the_forward_step_mirrors_its_cmd_and_one_key_reaches_it`.
|
||||
|
||||
The browser half needs nothing: the keymap is the shared table both ends read,
|
||||
and `resolve.rs` has had the `view-forward` arm since improvise-cfly.
|
||||
|
||||
## `ui/effect.rs`
|
||||
|
||||
### a_switch_the_document_refuses_leaves_the_back_stack_alone
|
||||
|
||||
improvise-swzk: `SwitchView` pushed the leaving frame onto `view_back_stack`
|
||||
before calling the guarded `App::switch_to_view`, so `:view NoSuchView` buried a
|
||||
real history frame under a frame naming the view the user was already on. `<`
|
||||
then returned to where it already was. Reasoning: `context/code/command.md`,
|
||||
"`SwitchView` — the history records a move that happened".
|
||||
|
||||
The test switches to a real second view, then to a name the document has not
|
||||
got, asserts the back stack is still one deep, then presses `<` and asserts it
|
||||
lands on the **first** view.
|
||||
|
||||
**Two faults injected, and the second is the one the brief asks for.** Making the
|
||||
push unconditional (`if true`) fails this test and
|
||||
`switch_view_to_same_does_not_push_stack`. Suppressing it entirely (`if false`) —
|
||||
the over-reach a fix aimed at this bug could plausibly commit — fails eleven
|
||||
tests including this one's own first assertion, which is why the assertion that
|
||||
one frame *survives* is here and not only the assertion about where `<` lands.
|
||||
|
||||
**The fixture is load-bearing and was measured, not assumed.** A variant with no
|
||||
real second view — refuse a switch from `Default`, then `<` — **passes against
|
||||
the unfixed code**, because the phantom frame it pops names `Default` and
|
||||
`Default` is where you already are. That is the vacuity review demonstrated
|
||||
twice on PR #48, and the ticket warned about it: a bystander sitting on the view
|
||||
it would land on anyway cannot see this.
|
||||
|
||||
### the_document_surface_runs_on_a_non_filesystem_backend
|
||||
|
||||
improvise-fuj: every effect that touches a document goes through `App::storage`,
|
||||
@@ -4217,6 +4269,66 @@ The neighbouring test `a_refused_switch_corrects_the_asker_and_nobody_else`
|
||||
compared `active_view` alone and stayed green through all of this: the check
|
||||
that answers a neighbouring question, in the place the doctrine warns about.
|
||||
|
||||
### an_aggregate_a_session_was_shown_does_not_resolve_against_the_hosts_view
|
||||
|
||||
**The blocker for browser drill, demonstrated rather than argued.** The ticket
|
||||
records drill as blocked on ADR 001 layer 2, on the grounds that
|
||||
`ApplyAndClearDrill` is session-local and unaddressable. That reasoning is
|
||||
about the **commit** path. Entering a drill needs no commit, and after the
|
||||
view-stack work above a browser already has `<`; what is missing is only the
|
||||
step that builds the `_Drill` view.
|
||||
|
||||
Everything that step needs is on the wire already: `CreateView`, `SetAxis` and
|
||||
`SetPageSelection` are `ModelCommand`s that **name their view** (improvise-lltt),
|
||||
and `SwitchView` is view-half and per-client. What a browser cannot supply is
|
||||
the *operand* — it holds `CellId::Aggregate(AggregateHash)`, an opaque u128, so
|
||||
it cannot compose the filter coordinates itself. That is ADR 002 working as
|
||||
designed, and it points at the host resolving the surrogate.
|
||||
|
||||
`App::resolve_aggregate` already does that and is tested — but **only where the
|
||||
host stands.** A `RecordHash` falls back to `DataStore`'s maintained index and
|
||||
so resolves against the *document*, which is why a connected client's `SetCell`
|
||||
works from any view. An `AggregateHash` has no such index and cannot have one:
|
||||
an aggregate has no existence outside the layout that derives it. So it is
|
||||
layout-scoped, and the layout is `app.layout` — built from the **host's** active
|
||||
view, which since improvise-jytm is not the client's.
|
||||
|
||||
This test seats a session on a view the host is not on, takes an
|
||||
`AggregateHash` out of the grid `grid_in` derives *for that session*, and shows
|
||||
the host answering `None` to a surrogate it had itself just projected. The
|
||||
control is what makes it discriminating: the **same** surrogate resolves once
|
||||
the host is moved onto that view, so the failure is the host's view and not the
|
||||
surrogate. Without the control a blanket `None` — a typo in the hash, an empty
|
||||
layout — would pass.
|
||||
|
||||
The fixture's second view is not a transpose of the first: it puts `Product` on
|
||||
Row and `Region` on `None`, so its aggregates constrain on a coordinate the
|
||||
host's layout never names. A fixture that merely reordered the same axes could
|
||||
resolve by coincidence.
|
||||
|
||||
**The first version of this test had the defect it was written to expose, and
|
||||
review found it.** It derived the grid from a hard-coded `grid_in(&app, "Other",
|
||||
…)`, so the session, the switch and `apply_checked` could all be **deleted** and
|
||||
every assertion still passed — it proved a grid built for a *name*, where its
|
||||
own name claims a grid a *session was shown*. The view is now read off
|
||||
`sessions.get(id).view().active_view`, asserted to have moved, and asserted to
|
||||
differ from the host's; the viewport is the session's own. Injected fault:
|
||||
delete the `apply_checked` line and the test fails with `left: "Default"`,
|
||||
which it could not do before.
|
||||
|
||||
**What this does and does not say about the ticket.** It is *not* ADR 001 layer
|
||||
2: the mutation a drill performs is addressed to no one session — it configures
|
||||
the shared `_Drill` view exactly as `CreateView`/`SetAxis` already do
|
||||
unaddressed — and only the resolution of its operand is session-scoped. Nor is
|
||||
it improvise-fdk1: a browser cannot edit inside a drill grid at all, because
|
||||
`edit_or_drill` declines records-mode editing out loud before reaching the
|
||||
cell, so no coordinate rename is reachable and no divergent staging scheme is
|
||||
created. The single missing piece is that the resolution must run against the
|
||||
view the aggregate was **seen** in. The cheap shape is the one every other
|
||||
view-scoped variant already uses: let the command name its view, as
|
||||
`SetAxis { view, .. }` does, and resolve against a layout derived for that name
|
||||
rather than against `app.layout`.
|
||||
|
||||
## `command/keymap.rs` — a bound key that builds nothing (improvise-55l5)
|
||||
|
||||
### a_bound_key_whose_command_cannot_be_built_says_so
|
||||
@@ -4496,3 +4608,153 @@ So the discriminating input is the **buffer**, not the column: an empty commit
|
||||
creates nothing, inserts no column, and must still start the next row. Injecting
|
||||
the value-blind predicate above turns the branch off entirely and this test fails
|
||||
with `row_count` 1 where 2 is owed, while the headline test above stays green.
|
||||
|
||||
## improvise-lm5e — a type name is not a field
|
||||
|
||||
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*
|
||||
(`doctrine(check_the_checker)`). The ticket's own reproduction step was 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 `Type/Clothing`, `set-axis` ignoring its argument,
|
||||
`:w <path>` saving to `/tmp/wrong.improv`, paste writing a constant instead of
|
||||
the yanked value, the tile bar acting on `Month` whatever the cursor says, page
|
||||
cycling always landing on the first item, the panel cursor landing on 0 of the
|
||||
wrong panel, and the formula/view panels deleting and switching to `"Other"`.
|
||||
|
||||
Against that command layer, `cargo test -p improvise-command --lib` reported
|
||||
**525 passed, 6 failed**. None of the six were in this list except
|
||||
`commit_item_add_files_the_item_under_the_mode_s_category`, which already
|
||||
checked one field by substring; the other three were in `ui/app.rs` and two in
|
||||
`ui/reduce.rs`, so the layer that would have caught a wrong field was never the
|
||||
command layer. With the strengthened assertions and the same rival: **510
|
||||
passed, 21 failed** — all sixteen fire, and the five incidental ones remain.
|
||||
|
||||
Each strengthened test 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
|
||||
(`SaveAs`, `RemoveFormula`, `SetPanelCursor`, `SwitchView`, `DeleteView`).
|
||||
Assertions naming a **fieldless** effect — `Quit`, `Save`, `RebuildLayout`,
|
||||
`TransposeAxes`, `AbortChain`, `TogglePruneEmpty` — were left alone: for those
|
||||
the type name *is* the whole claim, so they are complete rather than smoke.
|
||||
|
||||
### add_category_cmd_produces_add_category_effect
|
||||
|
||||
Folds the effect and asserts the full `regular_category_names()`, which pins
|
||||
both that `Region` arrived and that nothing else moved. See above for the rival.
|
||||
|
||||
### set_cell_cmd_parses_coords_correctly
|
||||
|
||||
The test's own name claimed the coordinates were checked; it asserted only
|
||||
`SetCell`. It now folds and reads the cell back at `Type/Food, Month/Jan`.
|
||||
|
||||
### set_axis_cmd_recognizes_column_alias
|
||||
|
||||
Asserts `axis_of("Type") == Column` after the fold, and that `Month` is on
|
||||
Column with it — so an implementation that moved the wrong category, or that
|
||||
ignored the `col` alias and left `Type` on Row, is separated from one that did
|
||||
neither.
|
||||
|
||||
### write_cmd_with_path_saves_as
|
||||
|
||||
`SaveAs` is not folded — it would write a file — so the path is asserted on the
|
||||
effect's own `Debug`.
|
||||
|
||||
### paste_with_yanked_value_produces_set_cell
|
||||
|
||||
The fixture seeds `Type/Clothing, Month/Feb` with `42.0` and yanks `99.0`, so
|
||||
asserting the post-fold value at the command's key separates "wrote the yanked
|
||||
value" from "wrote whatever was already there".
|
||||
|
||||
### tile_axis_cycle_produces_cycle_effect
|
||||
|
||||
Reading the effects showed what no assertion had: `TileAxisOp` indexes
|
||||
`tileable_cats(&ctx.model.category_names())`, **all** categories including the
|
||||
virtual ones, so tile cursor 0 is `_Index` and not the first regular category.
|
||||
The test now says so, asserts `_Index` moves to `before.next()`, and asserts
|
||||
`Type` and `Month` do not move.
|
||||
|
||||
### tile_axis_set_produces_set_axis_effect
|
||||
|
||||
The same, for the explicit-axis arm: `_Index` lands on `Page`, the two regular
|
||||
categories stay where they were.
|
||||
|
||||
### page_next_cycles_through_page_items
|
||||
|
||||
The name claimed cycling; the assertion was `SetPageSelection`. `Region` carries
|
||||
`North, South, East`, so the post-fold selection must be `South`.
|
||||
|
||||
### page_prev_cycles_backward
|
||||
|
||||
Its mirror, and the one that pins the wrap: from the first item `[` must land on
|
||||
`East`, the last.
|
||||
|
||||
### move_panel_cursor_down_from_zero
|
||||
|
||||
`SetPanelCursor` carries a panel *and* a cursor and neither was checked. Both
|
||||
are now named, so moving the right cursor on the wrong panel fails.
|
||||
|
||||
### delete_formula_at_cursor_with_formulas
|
||||
|
||||
Asserts the `RemoveFormula` names `Profit` in `Type` — the formula the cursor is
|
||||
on — rather than merely that a formula was removed.
|
||||
|
||||
### switch_view_at_cursor_with_valid_cursor
|
||||
|
||||
Names `Default`, the view under the cursor.
|
||||
|
||||
### delete_view_at_cursor_zero_does_not_adjust_cursor
|
||||
|
||||
Names `Default` too. Its existing negative half — no `SetPanelCursor` at
|
||||
position 0 — is unchanged.
|
||||
|
||||
### commit_category_add_with_name_produces_add_effect
|
||||
|
||||
Folds and asserts the whole category list, so the name in the minibuffer is the
|
||||
name that arrives.
|
||||
|
||||
### commit_item_add_files_the_item_under_the_mode_s_category
|
||||
|
||||
This one already checked `"Month"` by substring, which is why it was among the
|
||||
six the rival caught. It now folds and asserts `Month`'s items are
|
||||
`Jan, Feb, March` **and** that `Type`'s are untouched — a substring `"Month"`
|
||||
cannot tell the category field from the item field.
|
||||
|
||||
### execute_command_valid_runs_command
|
||||
|
||||
`:add-category Region` must name `Region`; the assertion said only
|
||||
`AddCategory`.
|
||||
### a_sequence_of_moves_folds_in_order_and_the_last_write_wins
|
||||
|
||||
Replaces `law_sequence_associativity` (improvise-gjat), which could not fail. That
|
||||
test built three effect vectors from one `CmdContext` and asserted
|
||||
`effects_debug((a ++ b) ++ c) == effects_debug(a ++ (b ++ c))` — the associativity
|
||||
of `Vec::extend`, true for any deterministic `execute`, and
|
||||
`doctrine(fold_tests_must_test_order_not_the_axiom)` names exactly this shape. It
|
||||
never applied an effect, so no command behaviour could move it.
|
||||
|
||||
The property that can fail is the one production actually relies on: a
|
||||
`Binding::Sequence` flattens into **one** effect vec (gotcha 21), the batch folds
|
||||
onto the `App` in order, and each `Move` composed against the *same* ctx emits an
|
||||
**absolute** `SetSelected` — so the moves do not compose and the last write wins.
|
||||
The test folds one batch per ordering and asserts the resulting cursor, with
|
||||
`c, b, a` landing somewhere `a, b, c` does not, so an implementation that ignores
|
||||
order is separated from one that honours it.
|
||||
|
||||
Two faults were injected, and both leave the old assertion passing:
|
||||
|
||||
- **`Move::execute` returns `Vec::new()` when the computed position equals the
|
||||
current one** — a plausible "skip a move that moves nothing" optimisation, and
|
||||
one that would break `tests/lockstep.rs`, since the browser resolver mirrors
|
||||
`Move` line for line including where it clamps (gotcha 47). The old test still
|
||||
passed (`c` contributed nothing to either grouping, so the two remained equal);
|
||||
the new one failed `left: (0, 1) right: (0, 0)` — `b` had become the last write.
|
||||
- **`App::apply_effects` folds the batch in reverse.** The old test passed
|
||||
untouched, because it formats the vector and never applies it; the new one
|
||||
failed `left: (1, 0) right: (0, 0)`.
|
||||
|
||||
The second fault is why the batch is flattened rather than applied one command at
|
||||
a time. A first version of this test called `apply_effects` per command; each
|
||||
single-`Move` batch reverses to the same cursor, so the reversed fold was
|
||||
invisible and the test passed against it. One batch is also the shape a
|
||||
`Binding::Sequence` really has.
|
||||
|
||||
@@ -1217,3 +1217,29 @@ view a fresh workbook creates. Making `Workbook::create_view` refuse or rename
|
||||
a name it already holds passes this test and breaks records mode, because both
|
||||
rebuilders depend on the replacement — which is why the fix is a helper beside
|
||||
`create_view` and not a change to it.
|
||||
|
||||
### model::types::formula_tests::count_aggregation
|
||||
|
||||
improvise-a36c. The test asserted `val >= 1.0` for `COUNT(Sales)` at
|
||||
`Region=East`, where the fixture's answer is exactly `1.0` — so every inflated
|
||||
count cleared the bar. The ticket's own fault injection (`values.len() + 999.0`
|
||||
in `eval_agg`) is not the worst case: it fails one *other* test,
|
||||
`model::types::agg_helper_tests::eval_agg_over_no_rows_is_zero_only_where_an_identity_exists`,
|
||||
which sees the empty case move from `0` to `999`. Preserve the empty case
|
||||
(`if values.is_empty() { 0.0 } else { len + 999.0 }`) and the **entire
|
||||
workspace of 1699 tests passes** while `COUNT` over any non-empty set is wrong
|
||||
by 999. That measurement, not the ticket's, is what this entry exists to record:
|
||||
the only thing standing between a wrong `COUNT` and a green suite was a check on
|
||||
the identity element, which is the one input `COUNT` gets right by accident.
|
||||
|
||||
Two assertions, because one does not separate the neighbouring question.
|
||||
`assert_eq!(at East, 1.0)` answers *is the count inflated* — it fails the
|
||||
`+999` rival at `left: Some(1000.0)`. It does **not** answer *does `COUNT` count
|
||||
anything*: a `Count` arm returning a constant `1.0` satisfies it, and at a
|
||||
fixture whose expected value is `1` that rival is invisible by construction.
|
||||
The second assertion, `COUNT` unscoped over all three regions being `3.0`,
|
||||
is what fails it, at `left: Some(1.0)`. Both rivals were injected and both fail,
|
||||
at different assertions — the first at the East count, the second at the total.
|
||||
|
||||
The fixture is deliberately asymmetric: one matching cell under the scoped key
|
||||
and three under the unscoped one, so no single constant can satisfy both.
|
||||
|
||||
@@ -1354,3 +1354,73 @@ is the *seating* — with the `improvise-cj4u` fix reverted, `create_view`
|
||||
pivots `Note` onto Column in the source workbook, the file faithfully records
|
||||
`Note: column`, and this test fails. It was written while cj4u was still open
|
||||
and failed then, which is how that was established rather than assumed.
|
||||
|
||||
### import::wizard::tests::build_model_warns_about_values_that_do_not_match_the_date_format
|
||||
|
||||
improvise-yss5, and the ticket said plainly it had **not** been reproduced. It
|
||||
is real and it is narrower than it reads. `extract_date_component` returns
|
||||
`None` for a value the detected format cannot parse, and the caller's `if let
|
||||
Some(..)` simply does not push the derived coordinate — so the row silently
|
||||
leaves the derived `Date_Month`/`Date_Year` category with nothing reported, the
|
||||
same shape improvise-2cw9/jc95/5hfh closed elsewhere in this path.
|
||||
|
||||
**Through the shipped CLI and TUI wizard it is unreachable**, and the reason is
|
||||
worth writing down because it is a property of one function.
|
||||
`detect_date_format` accepts a format only when **every** distinct non-empty
|
||||
sample parses, and `extract_date_component` then makes the identical
|
||||
`NaiveDate::parse_from_str` call — so a mismatch requires the format to come
|
||||
from somewhere other than detection over the same values. That door is
|
||||
`ImportPipeline`'s `pub proposals`: a library caller may set `date_format`
|
||||
itself, and the test does. `a_column_of_mixed_date_formats_is_not_proposed_as_a_time_category`
|
||||
guards the reachability claim rather than leaving it as prose, per
|
||||
`doctrine(an_unreachability_claim_is_only_as_wide_as_the_routes_enumerated)`.
|
||||
|
||||
The fixture derives **two** columns (Month and Year) from one `Date` whose three
|
||||
values include two the format rejects, and the assertion is on the full warning
|
||||
vector rather than on a substring — so "one located warning per derived column"
|
||||
is separable from "one line saying some dates did not parse".
|
||||
|
||||
Three rivals, each caught at a different assertion:
|
||||
|
||||
- **Dropping the `count > 0` filter** — warn unconditionally — passed the entire
|
||||
suite before `extracting_a_date_component_from_values_that_all_parse_warns_about_nothing`
|
||||
was added, because no existing test enabled date extraction *and* asserted
|
||||
silence. That is the one-directional fault
|
||||
`context/design-principles.md` §7 names: a check that only asserts the warning
|
||||
appears is satisfied by a writer that always warns. It now fails there.
|
||||
- **Tallying into one counter instead of one per extraction** fails the vector
|
||||
comparison, which a single-derived-column fixture could not see.
|
||||
- **Weakening `detect_date_format` from `all` to `any`** fails the reachability
|
||||
guard here and three analyzer tests, which is the evidence that guard is
|
||||
load-bearing and not decoration.
|
||||
|
||||
The fix reports and does not relocate: the row still lands with the coordinates
|
||||
it does have, exactly as before. Changing where such a row is placed is a
|
||||
behaviour change the ticket did not ask for.
|
||||
|
||||
### import::wizard::tests::a_row_dropped_for_a_missing_category_is_not_charged_to_the_date_tally
|
||||
|
||||
improvise-yss5, found by ChatGPT Codex reviewing PR #150 at `af7fae0e` and
|
||||
correct. The first fix incremented `unparsed_dates` 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's tally. 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.
|
||||
|
||||
The mismatches are now collected per record and merged only after
|
||||
`missing.is_none()`. The fixture only discriminates while `Date` precedes the
|
||||
missing category, so the test asserts that ordering explicitly rather than
|
||||
relying on it — a later change to proposal order would otherwise make the test
|
||||
pass by walking the missing category first and never reaching the bad date.
|
||||
|
||||
Before the fix it reported three warnings where one is correct, naming
|
||||
`Date_Month` and `Date_Year` for a row that does not exist in the model.
|
||||
Injecting the rival — merging the per-record tally *before* the `missing` check
|
||||
rather than after — reproduces exactly that and fails here.
|
||||
|
||||
This is the second time in this ticket that the interesting question was *which
|
||||
rows the count is over*. The first was one warning per derived column rather
|
||||
than one overall; this is one warning per **landed** row rather than per
|
||||
attempted one.
|
||||
|
||||
@@ -1221,3 +1221,42 @@ Three rivals, each caught at a different assertion:
|
||||
- **Dropping `#[serde(flatten)]`** fails at the shape assertion with
|
||||
`"mean" ... : null`, since the map would then nest under a `scaling` key and
|
||||
every previously saved model would stop loading.
|
||||
### fit::tests::a_budget_notice_precedes_a_table_warning_which_precedes_a_preprocess_one
|
||||
|
||||
improvise-ipqu. `FitOutcome::warnings` is three tiers concatenated by two
|
||||
`splice(0..0, ..)` calls in `fit_measure`, and the resulting order — the
|
||||
budget's notices, then the feature table's, then `preprocess`'s — is stated in
|
||||
the field's own doc comment **and** in `context/code/gp.md` §14, and is joined
|
||||
into a user-facing status line. It was checked nowhere. Swapping the two
|
||||
statements left all 74 `improvise-gp` tests green, measured before and after.
|
||||
This is the shape gotcha 43 names — a rule "held up only by the order of two
|
||||
statements" — and the repair is the same one
|
||||
`a_published_basis_follows_the_grid_updates_it_describes` made: inject the swap
|
||||
and watch a test fail.
|
||||
|
||||
**No pre-existing fixture produced more than one tier**, which is why nothing
|
||||
caught it, and building one is the whole of the work. `all_three_warning_tiers_at_once`
|
||||
extends `two_categories_of_different_widths` — 60 `Wide` items against a
|
||||
64-feature budget, so `Wide` is dropped and a notice is emitted — with two
|
||||
further things, each aimed at exactly one of the other tiers. A `Blank` item on
|
||||
`_Measure` carrying **no cell anywhere** is a header column (`raw_measures`
|
||||
reads item names, not data) that is empty in every row, which is what
|
||||
`preprocess` warns about. And a `Cost` cell at `(w0, n1)`, a coordinate the base
|
||||
fixture does not use, is one visible record with no `Score` value, which
|
||||
`feature_table` skips and warns about. Neither addition disturbs the budget
|
||||
arithmetic: the skipped record contributes no row, so `Wide` is still 60 wide,
|
||||
and `Blank` costs 2, leaving `60 + 8 + 1 + 2 = 71` over budget by `Wide` alone.
|
||||
|
||||
Asserting that the concatenation *contains* all three would be vacuous
|
||||
(`doctrine(fold_tests_must_test_order_not_the_axiom)`), so the assertion is on
|
||||
the three positions. The helper resolving a tier to its index **panics** when a
|
||||
tier is absent rather than returning a sentinel, so a future change that stops
|
||||
one tier speaking fails here loudly instead of making the ordering assertion
|
||||
trivially true over two elements.
|
||||
|
||||
Two rivals, each moving a different boundary, and each caught only by this test
|
||||
(74 passed, 1 failed, both times). Swapping the two `splice` statements gives
|
||||
table → notice → preprocess and fails at `notice < table`. Replacing the first
|
||||
`splice(0..0, table.warnings)` with `extend(table.warnings)` gives notice →
|
||||
preprocess → table and fails at `table < preprocessed` — which is the rival that
|
||||
earns the third tier, since the swap alone never moves `preprocess`'s.
|
||||
|
||||
@@ -644,3 +644,195 @@ Fault injected 2026-09-21: restoring the name-only comparison in
|
||||
`AxisFrame::group_runs` fails this with `left: [(Some("Q1"), 4)]` while the five
|
||||
existing agreement tests all pass — the ungrouped `Sep` in their fixture is
|
||||
precisely why they cannot see it.
|
||||
|
||||
---
|
||||
|
||||
## `src/draw.rs::title_bar_tests` — the title bar's escape hatch
|
||||
|
||||
improvise-t14d. `fill_line` composed a left and a right segment into one row and
|
||||
did **neither** of the two things that composition needs. It padded with
|
||||
`left.len() + right.len()` — **bytes** — and it never truncated, so on overflow
|
||||
it simply concatenated and left ratatui to clip. ratatui clips the *right*, and
|
||||
the right segment of the title bar is `" ?:help :q quit "`: the only thing on
|
||||
screen telling a stuck user how to get help or get out.
|
||||
|
||||
**The priority was backwards.** The left segment is decoration — the product
|
||||
name, the document name, the filename, a dirty marker — and its length is
|
||||
unbounded because a document name is. The right segment is a fixed 17 cells and
|
||||
is the escape hatch. Giving the unbounded decoration unconditional priority over
|
||||
the fixed affordance means the affordance is the first thing sacrificed, and it
|
||||
is sacrificed at ordinary widths: measured through a real `TestBackend`, a
|
||||
document named `Quarterly Regional Sales Analysis` stored as
|
||||
`quarterly-regional-sales-2026.improv` loses `:q quit` **at 100 columns**.
|
||||
|
||||
The byte fault is separate and universal. The title's own format string spells
|
||||
`·`, two bytes and one cell, so `left.len()` over-counts by one for *every*
|
||||
document — the padding is one column short and the right segment sits one cell
|
||||
in from the edge it is anchored to, before any non-ASCII document name makes it
|
||||
worse.
|
||||
|
||||
`Keeps` is the fix's shape: which segment survives is a per-call-site decision,
|
||||
not a global rule. The title bar keeps the **right** (the hatch outranks the
|
||||
name). The two bottom-bar call sites keep the **left**, which is what they did
|
||||
before — there the left is the user's own typing (`minibuffer_line`) or the
|
||||
status message (`draw_status`), and the right is a view badge, so right-priority
|
||||
would have let a long message eat the text the user is editing along with the
|
||||
cursor. Widths are `ui::text::truncate`'s display cells throughout, so the
|
||||
overflow is a graceful `…` rather than a mid-token clip of `?:`.
|
||||
|
||||
### the_escape_hatch_outlives_a_long_document_name
|
||||
|
||||
The defect itself, at the five widths the ticket measured (36, 44, 52, 60, 100).
|
||||
Rendered through a `TestBackend` and read off the terminal buffer, not off
|
||||
`fill_line`'s return value: the loss happens in ratatui's clip, so a helper's
|
||||
return value cannot see it. **A test at 80 columns with a short fixture name
|
||||
passes against the broken code** — title length is the variable, not width.
|
||||
|
||||
Fault injected: `Keeps::Right` → `Keeps::Left` at the `draw_title` call site.
|
||||
Fails at width 36 with `" improvise · Quarterly Regional S"`; the other four
|
||||
tests in the module pass, so this one and only this one names the priority.
|
||||
|
||||
### the_document_name_still_leads_the_row
|
||||
|
||||
The other direction, which the escape-hatch assertion alone cannot supply: a fix
|
||||
that dropped the left segment entirely, or that put the hatch first, satisfies
|
||||
it. At 80 columns with a short name nothing is truncated, so this pins that the
|
||||
name leads, the hatch follows, and the filename survives intact.
|
||||
|
||||
### a_multibyte_title_does_not_shift_the_right_segment
|
||||
|
||||
The byte-versus-cell fault. Asserts the row *ends with* the right segment — the
|
||||
edge it is anchored to — for an ASCII name, an accented one and a CJK one. The
|
||||
ASCII case is the discriminating one and the surprise: it fails against the
|
||||
broken code too, because the `·` is in the format string rather than in the
|
||||
document's name.
|
||||
|
||||
Fault injected: `width - left.width() - right.width()` →
|
||||
`width.saturating_sub(left.len() + right.len())`, keeping the truncation. Fails
|
||||
on `"Sales"` with the right segment one cell in from the edge, and takes
|
||||
`a_composed_line_is_exactly_its_width` with it. The escape-hatch test does not
|
||||
see it, which is why the two faults need two tests.
|
||||
|
||||
### a_composed_line_is_exactly_its_width
|
||||
|
||||
The property both directions owe, over 0..40 columns, three segment pairs and
|
||||
both `Keeps` arms: a wide-character left segment, an empty left, and an empty
|
||||
right. Width 0 and the band where the two segments cannot both fit are the cases
|
||||
`fill_line`'s arithmetic can get wrong — it subtracts rather than saturating, so
|
||||
an off-by-one here is a panic and not a cosmetic slip.
|
||||
|
||||
### the_kept_side_is_the_side_that_survives
|
||||
|
||||
`Keeps::Left` has no other witness in this module: every rendering test above
|
||||
drives the title bar, which keeps the right. Without this, both arms could be
|
||||
the same code and the suite would stay green — which is what the enum exists to
|
||||
prevent.
|
||||
|
||||
Fault injected: the `Keeps::Left` arm rewritten to the body of the `Keeps::Right`
|
||||
arm. This test fails; the other four pass.
|
||||
|
||||
---
|
||||
|
||||
## `src/ui/tile_bar.rs` + `src/draw.rs` — one hint table on screen, and it is the derived one
|
||||
|
||||
improvise-rr8n. Two hand-written key tables were drawn **beside** derived ones,
|
||||
in a different notation, and one of them was outright false.
|
||||
|
||||
**The tile bar's non-TileSelect hint named keys that do something else.** It read
|
||||
`" Ctrl+↑↓←→ to move tiles"`, while `Ctrl-Left/Right/Up/Down` are bound in Normal
|
||||
to `enter-tile-select` — pressing one *enters* tile mode and moves nothing. It
|
||||
was also the only persistent on-screen documentation of those keys, and it spelt
|
||||
`Ctrl+` where `keymap::key_label`, and therefore every derived hint and help row,
|
||||
spells `Ctrl-`.
|
||||
|
||||
**Nothing could have caught it.** `every_hint_key_resolves_in_its_mode` checks
|
||||
`AppMode::hint_text()` against the keymaps; a string literal in a widget is
|
||||
invisible to it. And the shipped fixture `renders_each_tile_with_its_axis_label`
|
||||
renders at **80 columns**, where the hint does not fit (`x + hint.width() <=
|
||||
area.x + area.width` is false) and is never drawn — so the one test that pins
|
||||
this widget's output could not see either hint at all. That is why the new tests
|
||||
render at 140.
|
||||
|
||||
**The fix deletes rather than rewrites, and that departs from the ticket's AC.**
|
||||
improvise-rr8n proposed rewriting the string to `" Ctrl-↑↓←→ or T to pick a
|
||||
tile"` and suppressing the *status bar's* hint in TileSelect. Measured against
|
||||
the ticket's own second finding, that is backwards:
|
||||
|
||||
```
|
||||
tile bar (hand-written): [Enter] cycle axis [r/c/p] set axis [←→] select [Esc] cancel
|
||||
status bar (derived): hl←→:select Enter/Space:cycle r:row c:col p:page Esc/Tab:back
|
||||
```
|
||||
|
||||
The derived line is a strict **superset** — it carries `Space`, `h`/`l` and
|
||||
`Tab`, which the tile bar's omits — and it cannot go stale, being read off the
|
||||
keymap. Suppressing it would have deleted information and kept the table that
|
||||
had just been shown to rot. So the tile bar now advertises nothing, and in
|
||||
Normal the derived line already says `T:tiles`; only the `Ctrl-`arrow spelling
|
||||
is lost, and it was never documented correctly.
|
||||
|
||||
**The wizard goes the other way, and there the ticket is right.** Its overlay
|
||||
draws correct per-step hints, and the status bar drew one static
|
||||
`Space:toggle c:cycle Enter:next Esc:cancel` under it — wrong on four of eight
|
||||
steps, including an outright contradiction on `Done`, where the bar says `Esc`
|
||||
cancels and `Esc` goes *back*. It is static because it is derived from the
|
||||
`ImportWizard` keymap, while the wizard's real keys are resolved per step inside
|
||||
`HandleWizardKey`; so the derivation is sound and its *input* is the wrong thing.
|
||||
`draw_status` therefore suppresses the hint fallback for that one mode.
|
||||
|
||||
The asymmetry is the point: **keep whichever table is derived from the thing it
|
||||
describes.** For tile select that is the status line; for the wizard it is the
|
||||
overlay.
|
||||
|
||||
### the_tile_bar_advertises_no_keys_of_its_own
|
||||
|
||||
Renders the bar in Normal and TileSelect at 140 columns and requires that neither
|
||||
row names `Ctrl`, `Enter`, `Esc`, `←→` or `move tiles`. An absence assertion, so
|
||||
it is paired with the two below that require the information to still exist
|
||||
elsewhere.
|
||||
|
||||
Two faults injected, both caught here and by nothing else:
|
||||
|
||||
- The original `else` branch restored — fails in Normal on `"Ctrl"`, quoting the
|
||||
whole false line.
|
||||
- Only the `else` branch deleted, keeping the TileSelect literal (the narrow
|
||||
reading of finding 1) — fails in TileSelect on `"Enter"`. This is the rival
|
||||
that matters, because it is the fix a reader of finding 1 alone would write,
|
||||
and it leaves two disagreeing dialects stacked on adjacent rows.
|
||||
|
||||
Note it would also fail against the ticket's own proposed replacement string,
|
||||
which still contains `Ctrl`. That is deliberate rather than accidental: the
|
||||
property asserted is "this widget names no keys", not "this widget's keys are
|
||||
currently right".
|
||||
|
||||
### the_tile_bar_still_draws_its_tiles_at_every_width_it_used_to
|
||||
|
||||
The counterweight. Deleting a `set_string` at the end of a render function is
|
||||
the kind of edit that takes a neighbour with it, so this pins the tiles and the
|
||||
`Tiles:` prefix at 80 and 140. It also covers the dead `x += 1` the deletion
|
||||
orphaned — clippy caught that one, but the test is what says the drawing still
|
||||
lands.
|
||||
|
||||
### the_wizard_overlay_is_the_only_key_hint_on_screen
|
||||
|
||||
The wizard half. Requires the bottom row in `ImportWizard` to name none of
|
||||
`Space:toggle`, `c:cycle`, `Esc:cancel`. Fails against the unfixed code quoting
|
||||
the full row, including the `Esc:cancel` that contradicts the overlay.
|
||||
|
||||
### suppressing_the_wizard_hint_leaves_the_rest_of_the_bar
|
||||
|
||||
The direction the suppression could overshoot: the mode label `IMPORT` and a real
|
||||
`status_msg` are not hints and must survive. A fix that blanked the bar in that
|
||||
mode, or that suppressed the *message* along with the hint, fails here.
|
||||
|
||||
### the_status_bar_still_advertises_the_tile_keys
|
||||
|
||||
The counterweight to the tile bar deletion, and the test that rejects the
|
||||
ticket's own AC: Normal must still say `T:tiles`, and TileSelect must still carry
|
||||
`Enter/Space:cycle`, `r:row` and `Esc/Tab:back`.
|
||||
|
||||
Fault injected: suppressing the hint fallback unconditionally
|
||||
(`true => String::new()`) rather than for `ImportWizard` alone. All three wizard
|
||||
tests pass — including both of the ones *aimed* at the suppression — and only
|
||||
this one fails. That is the shape design-principles §7 names: the assertion next
|
||||
door is satisfied by the wrong fix, and the discriminating input is the mode the
|
||||
change was not supposed to touch.
|
||||
|
||||
@@ -215,7 +215,8 @@ impl TryFrom<RawScaling> for Scaling {
|
||||
fn try_from(raw: RawScaling) -> Result<Self, Self::Error> {
|
||||
Scaling::new(raw.mean, raw.std).ok_or_else(|| {
|
||||
format!(
|
||||
"the saved affine map is not finite (mean {}, std {}), so predictions from it would be NaN. Refit the model",
|
||||
"the saved affine map is not finite (mean {}, std {}), so predictions \
|
||||
from it would be NaN. Refit the model",
|
||||
raw.mean, raw.std
|
||||
)
|
||||
})
|
||||
|
||||
@@ -64,11 +64,16 @@ mod tests {
|
||||
("Type".into(), "Clothing".into()),
|
||||
("Month".into(), "Feb".into()),
|
||||
]);
|
||||
let cmd = PasteCell { key };
|
||||
let cmd = PasteCell { key: key.clone() };
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("SetCell"), "Expected SetCell, got: {dbg}");
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
assert_eq!(
|
||||
app.model_state.workbook.model.get_cell(&key),
|
||||
Some(&CellValue::Number(99.0)),
|
||||
"paste must write the YANKED value at the command's own key"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -96,10 +96,12 @@ mod tests {
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.buffers = &bufs;
|
||||
let effects = commit(CommitKind::Category).execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("AddCategory"),
|
||||
"Expected AddCategory, got: {dbg}"
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
assert_eq!(
|
||||
app.model_state.workbook.model.regular_category_names(),
|
||||
vec!["Type", "Month", "Region"],
|
||||
"the category named in the buffer must be the one added"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -136,9 +138,15 @@ mod tests {
|
||||
let cmd = reg
|
||||
.interactive("commit-buffer", &["item".to_string()], &ctx)
|
||||
.expect("item-add mode should supply the category");
|
||||
let dbg = effects_debug(&cmd.execute(&ctx));
|
||||
assert!(dbg.contains("AddItem"), "Expected AddItem, got: {dbg}");
|
||||
assert!(dbg.contains("Month"), "Expected category Month, got: {dbg}");
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(cmd.execute(&ctx));
|
||||
let model = &app.model_state.workbook.model;
|
||||
assert_eq!(
|
||||
model.effective_item_names("Month"),
|
||||
vec!["Jan", "Feb", "March"],
|
||||
"the item named in the buffer must land in the MODE's category"
|
||||
);
|
||||
assert_eq!(model.effective_item_names("Type"), vec!["Food", "Clothing"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -193,19 +193,13 @@ impl CmdRegistry {
|
||||
});
|
||||
}
|
||||
|
||||
/// Register a command that needs no interactive context: with args it delegates
|
||||
/// to `parse`, and without them it errors.
|
||||
/// Register a command that needs no interactive context: BOTH arms are
|
||||
/// `parse`, so neither can demote the other — `context/code/command.md`.
|
||||
pub fn register_pure(&mut self, prototype: &dyn Cmd, parse: ParseFn) {
|
||||
self.entries.push(CmdEntry {
|
||||
name: prototype.name(),
|
||||
parse: Box::new(parse),
|
||||
interactive: Box::new(move |args, _ctx| {
|
||||
if args.is_empty() {
|
||||
Err("this command requires arguments".into())
|
||||
} else {
|
||||
parse(args)
|
||||
}
|
||||
}),
|
||||
interactive: Box::new(move |args, _ctx| parse(args)),
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -18,10 +18,11 @@ mod tests {
|
||||
let cmd = AddCategoryCmd(vec!["Region".to_string()]);
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("AddCategory"),
|
||||
"Expected AddCategory, got: {dbg}"
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
assert_eq!(
|
||||
app.model_state.workbook.model.regular_category_names(),
|
||||
vec!["Type", "Month", "Region"]
|
||||
);
|
||||
}
|
||||
|
||||
@@ -38,8 +39,16 @@ mod tests {
|
||||
]);
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("SetCell"), "Expected SetCell, got: {dbg}");
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
let key = crate::model::cell::CellKey::new(vec![
|
||||
("Type".into(), "Food".into()),
|
||||
("Month".into(), "Jan".into()),
|
||||
]);
|
||||
assert_eq!(
|
||||
app.model_state.workbook.model.get_cell(&key),
|
||||
Some(&CellValue::Number(42.0))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -51,8 +60,11 @@ mod tests {
|
||||
let cmd = SetAxisCmd(vec!["Type".to_string(), "col".to_string()]);
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("SetAxis"), "Expected SetAxis, got: {dbg}");
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
let view = app.model_state.workbook.active_view();
|
||||
assert_eq!(view.axis_of("Type"), Axis::Column);
|
||||
assert_eq!(view.categories_on(Axis::Column), vec!["Type", "Month"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -78,7 +90,10 @@ mod tests {
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("SaveAs"), "Expected SaveAs, got: {dbg}");
|
||||
assert!(
|
||||
dbg.contains("SaveAs(\"/tmp/out.improv\")"),
|
||||
"SaveAs must carry the path it was given, got: {dbg}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -384,52 +384,45 @@ mod tests {
|
||||
assert_eq!(first, second, "ToStart(Row) should be idempotent");
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn law_sequence_associativity() {
|
||||
fn a_sequence_of_moves_folds_in_order_and_the_last_write_wins() {
|
||||
let m = two_cat_model();
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
assert!(ctx.row_count() >= 2 && ctx.col_count() >= 2);
|
||||
|
||||
let mk_a = || {
|
||||
let mk = |kind: MoveKind, cmd_name: &'static str| {
|
||||
Move {
|
||||
kind: MoveKind::Relative(1, 0),
|
||||
kind,
|
||||
cursor: CursorState::from_ctx(&ctx),
|
||||
cmd_name: "move-selection",
|
||||
cmd_name,
|
||||
}
|
||||
.execute(&ctx)
|
||||
};
|
||||
let mk_b = || {
|
||||
Move {
|
||||
kind: MoveKind::Relative(0, 1),
|
||||
cursor: CursorState::from_ctx(&ctx),
|
||||
cmd_name: "move-selection",
|
||||
}
|
||||
.execute(&ctx)
|
||||
};
|
||||
let mk_c = || {
|
||||
Move {
|
||||
kind: MoveKind::ToStart(true),
|
||||
cursor: CursorState::from_ctx(&ctx),
|
||||
cmd_name: "jump-first-row",
|
||||
}
|
||||
.execute(&ctx)
|
||||
let a = || mk(MoveKind::Relative(1, 0), "move-selection");
|
||||
let b = || mk(MoveKind::Relative(0, 1), "move-selection");
|
||||
let c = || mk(MoveKind::ToStart(true), "jump-first-row");
|
||||
|
||||
// A `Binding::Sequence` flattens into ONE effect vec (gotcha 21), so the
|
||||
// fold under test is a single batch.
|
||||
let seq = |steps: Vec<Vec<Box<dyn Effect>>>| {
|
||||
let batch: Vec<Box<dyn Effect>> = steps.into_iter().flatten().collect();
|
||||
assert!(!batch.is_empty());
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(batch);
|
||||
app.cursor().selected
|
||||
};
|
||||
|
||||
let mut ab_c = mk_a();
|
||||
ab_c.extend(mk_b());
|
||||
ab_c.extend(mk_c());
|
||||
assert_eq!(seq(vec![a()]), (1, 0));
|
||||
assert_eq!(seq(vec![b()]), (0, 1));
|
||||
assert_eq!(seq(vec![c()]), (0, 0));
|
||||
|
||||
let mut bc = mk_b();
|
||||
bc.extend(mk_c());
|
||||
let mut a_bc = mk_a();
|
||||
a_bc.extend(bc);
|
||||
|
||||
assert_eq!(
|
||||
effects_debug(&ab_c),
|
||||
effects_debug(&a_bc),
|
||||
"Sequence concatenation should be associative"
|
||||
);
|
||||
assert_eq!(seq(vec![a(), b(), c()]), (0, 0));
|
||||
assert_eq!(seq(vec![c(), b(), a()]), (1, 0));
|
||||
assert_eq!(seq(vec![b(), a(), c()]), (0, 0));
|
||||
assert_eq!(seq(vec![c(), a(), b()]), (0, 1));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -480,10 +473,15 @@ mod tests {
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let effects = PageNext.execute(&ctx);
|
||||
assert!(!effects.is_empty());
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SetPageSelection"),
|
||||
"Expected SetPageSelection, got: {dbg}"
|
||||
let mut app = crate::ui::app::App::new(three_cat_model_with_page(), None);
|
||||
app.apply_effects(effects);
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.active_view()
|
||||
.page_selection("Region"),
|
||||
Some("South"),
|
||||
"`]` must advance to the NEXT item of the page category"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -495,10 +493,15 @@ mod tests {
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let effects = PagePrev.execute(&ctx);
|
||||
assert!(!effects.is_empty());
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SetPageSelection"),
|
||||
"Expected SetPageSelection, got: {dbg}"
|
||||
let mut app = crate::ui::app::App::new(three_cat_model_with_page(), None);
|
||||
app.apply_effects(effects);
|
||||
assert_eq!(
|
||||
app.model_state
|
||||
.workbook
|
||||
.active_view()
|
||||
.page_selection("Region"),
|
||||
Some("East"),
|
||||
"`[` from the first item must wrap to the LAST"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -360,8 +360,8 @@ mod tests {
|
||||
assert_eq!(effects.len(), 1);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SetPanelCursor"),
|
||||
"Expected SetPanelCursor, got: {dbg}"
|
||||
dbg.contains("SetPanelCursor { panel: Formula, cursor: 1 }"),
|
||||
"must move THE FORMULA panel's cursor to 1, got: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -414,8 +414,8 @@ mod tests {
|
||||
let effects = DeleteFormulaAtCursor.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("RemoveFormula"),
|
||||
"Expected RemoveFormula, got: {dbg}"
|
||||
dbg.contains("RemoveFormula { target: \"Profit\", target_category: \"Type\" }"),
|
||||
"must remove the formula AT THE CURSOR, got: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -428,8 +428,8 @@ mod tests {
|
||||
let effects = SwitchViewAtCursor.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("SwitchView"),
|
||||
"Expected SwitchView, got: {dbg}"
|
||||
dbg.contains("SwitchView(\"Default\")"),
|
||||
"must switch to the view UNDER THE CURSOR, got: {dbg}"
|
||||
);
|
||||
assert!(dbg.contains("Normal"), "Expected Normal mode, got: {dbg}");
|
||||
}
|
||||
@@ -515,8 +515,8 @@ mod tests {
|
||||
let effects = DeleteViewAtCursor.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("DeleteView"),
|
||||
"Expected DeleteView, got: {dbg}"
|
||||
dbg.contains("DeleteView(\"Default\")"),
|
||||
"must delete the view UNDER THE CURSOR, got: {dbg}"
|
||||
);
|
||||
assert!(
|
||||
!dbg.contains("SetPanelCursor"),
|
||||
|
||||
@@ -22,6 +22,15 @@ use super::text_buffer::*;
|
||||
use super::tile::*;
|
||||
use super::wizard::*;
|
||||
|
||||
/// Explicit coordinates win; the cursor is the fallback — `context/code/cli.md`.
|
||||
fn key_from_args_or_cursor(args: &[String], ctx: &CmdContext) -> Result<CellKey, String> {
|
||||
if args.is_empty() {
|
||||
ctx.cell_key().clone().ok_or("no cell at cursor".into())
|
||||
} else {
|
||||
Ok(parse_cell_key_from_args(args))
|
||||
}
|
||||
}
|
||||
|
||||
/// Build the default command registry with all commands.
|
||||
/// Registry names MUST match the `Cmd::name()` return value.
|
||||
pub fn default_registry() -> CmdRegistry {
|
||||
@@ -46,8 +55,8 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|_args, ctx| {
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(ClearCellCommand { key }))
|
||||
},
|
||||
);
|
||||
@@ -240,8 +249,8 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|_args, ctx| {
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(YankCell { key }))
|
||||
},
|
||||
);
|
||||
@@ -257,8 +266,8 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|_args, ctx| {
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(PasteCell { key }))
|
||||
},
|
||||
);
|
||||
@@ -308,8 +317,8 @@ pub fn default_registry() -> CmdRegistry {
|
||||
key: parse_cell_key_from_args(args),
|
||||
}))
|
||||
},
|
||||
|_args, ctx| {
|
||||
let key = ctx.cell_key().clone().ok_or("no cell at cursor")?;
|
||||
|args, ctx| {
|
||||
let key = key_from_args_or_cursor(args, ctx)?;
|
||||
Ok(Box::new(DrillIntoCell { key }))
|
||||
},
|
||||
);
|
||||
|
||||
@@ -177,8 +177,8 @@ mod tests {
|
||||
let effects = ExecuteCommand.execute(&ctx);
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(
|
||||
dbg.contains("AddCategory"),
|
||||
"Expected AddCategory effect, got: {dbg}"
|
||||
dbg.contains("AddCategory(\"Region\")"),
|
||||
"must add the category the LINE named, got: {dbg}"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -359,7 +359,13 @@ impl Cmd for ExecuteCommand {
|
||||
return vec![effect::change_mode(AppMode::Normal)];
|
||||
}
|
||||
|
||||
match crate::command::parse::parse_line_with(ctx.registry, &raw) {
|
||||
// Built against the live cursor, exactly as a script's is — improvise-6uva.
|
||||
let built: Result<Vec<Box<dyn Cmd>>, String> = crate::command::parse::segments(&raw)
|
||||
.into_iter()
|
||||
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.map(|built| built.into_iter().flatten().collect());
|
||||
match built {
|
||||
Ok(cmds) => {
|
||||
// Leave the command line **first**, before anything that can fail:
|
||||
// an abort must stop the chain, never the bookkeeping (gotcha 21).
|
||||
|
||||
@@ -17,8 +17,15 @@ mod tests {
|
||||
let cmd = TileAxisOp { axis: None };
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(!effects.is_empty());
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("CycleAxis"), "Expected CycleAxis, got: {dbg}");
|
||||
// The tile cursor indexes EVERY tileable category, virtual ones included,
|
||||
// so cursor 0 is `_Index` rather than the first regular category.
|
||||
let before = m.active_view().axis_of("_Index");
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
let view = app.model_state.workbook.active_view();
|
||||
assert_eq!(view.axis_of("_Index"), before.next());
|
||||
assert_eq!(view.axis_of("Type"), m.active_view().axis_of("Type"));
|
||||
assert_eq!(view.axis_of("Month"), m.active_view().axis_of("Month"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -32,8 +39,12 @@ mod tests {
|
||||
};
|
||||
let effects = cmd.execute(&ctx);
|
||||
assert!(!effects.is_empty());
|
||||
let dbg = effects_debug(&effects);
|
||||
assert!(dbg.contains("SetAxis"), "Expected SetAxis, got: {dbg}");
|
||||
let mut app = crate::ui::app::App::new(two_cat_model(), None);
|
||||
app.apply_effects(effects);
|
||||
let view = app.model_state.workbook.active_view();
|
||||
assert_eq!(view.axis_of("_Index"), Axis::Page);
|
||||
assert_eq!(view.axis_of("Type"), m.active_view().axis_of("Type"));
|
||||
assert_eq!(view.axis_of("Month"), m.active_view().axis_of("Month"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Quasi-lisp prefix command parser: `word arg1 arg2 …`, `.` between commands,
|
||||
//! `Category/Item` coordinate pairs, `"quoted strings"`.
|
||||
|
||||
use super::cmd::{Cmd, CmdRegistry, default_registry};
|
||||
use super::cmd::{Cmd, CmdContext, CmdRegistry, default_registry};
|
||||
|
||||
/// Parse a line into commands using the default registry.
|
||||
pub fn parse_line(line: &str) -> Result<Vec<Box<dyn Cmd>>, String> {
|
||||
@@ -33,6 +33,40 @@ pub fn parse_line_with(registry: &CmdRegistry, line: &str) -> Result<Vec<Box<dyn
|
||||
Ok(commands)
|
||||
}
|
||||
|
||||
/// The command segments of a line, with comments and blank segments dropped.
|
||||
pub fn segments(line: &str) -> Vec<&str> {
|
||||
let line = line.trim();
|
||||
if line.is_empty() || line.starts_with('#') || line.starts_with("//") {
|
||||
return Vec::new();
|
||||
}
|
||||
split_on_dot(line)
|
||||
.into_iter()
|
||||
.map(str::trim)
|
||||
.filter(|s| !s.is_empty())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Build one segment's command against a LIVE context — `context/code/cli.md`.
|
||||
pub fn parse_segment_interactive(
|
||||
segment: &str,
|
||||
ctx: &CmdContext,
|
||||
) -> Result<Option<Box<dyn Cmd>>, String> {
|
||||
let tokens = tokenize(segment.trim());
|
||||
let Some(word) = tokens.first() else {
|
||||
return Ok(None);
|
||||
};
|
||||
ctx.registry
|
||||
.interactive(word, &tokens[1..], ctx)
|
||||
.map(Some)
|
||||
.map_err(|err| {
|
||||
if err.starts_with("Unknown command") {
|
||||
err
|
||||
} else {
|
||||
format!("{word}: {err}")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Split a line on ` . ` separators (dot must be a standalone word,
|
||||
/// surrounded by whitespace or at line boundaries). Respects quoted strings.
|
||||
fn split_on_dot(line: &str) -> Vec<&str> {
|
||||
@@ -229,4 +263,88 @@ mod tests {
|
||||
assert!(parse_line("add-items").is_err());
|
||||
assert!(parse_line("add-items Region").is_err());
|
||||
}
|
||||
|
||||
// ── a segment is built against the cursor it will run against ────────
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_segment_is_built_against_the_cursor_it_will_run_against() {
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
|
||||
let m = two_cat_model();
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let mut ctx = make_ctx(&m, &layout, ®);
|
||||
ctx.selected = (1, 0);
|
||||
assert!(ctx.row_count() >= 2 && ctx.col_count() >= 2);
|
||||
|
||||
let built = parse_segment_interactive("move-selection 0 1", &ctx)
|
||||
.expect("move-selection parses")
|
||||
.expect("a segment with a word builds a command");
|
||||
let dbg = format!("{:?}", built.execute(&ctx));
|
||||
assert!(
|
||||
dbg.contains("SetSelected(1, 1)"),
|
||||
"a segment built against the live cursor must move FROM it, not \
|
||||
from the origin (improvise-6uva); got: {dbg}"
|
||||
);
|
||||
|
||||
// The pure arm is what a script used to go through, and it is what the
|
||||
// interactive arm must not be mistaken for.
|
||||
let pure = reg
|
||||
.parse("move-selection", &["0".to_string(), "1".to_string()])
|
||||
.expect("the pure arm parses too");
|
||||
assert!(
|
||||
format!("{:?}", pure.execute(&ctx)).contains("SetSelected(0, 0)"),
|
||||
"if the pure arm ever learns the cursor, the assertion above stops \
|
||||
discriminating"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn segments_drops_comments_and_blanks() {
|
||||
assert_eq!(segments("# a comment"), Vec::<&str>::new());
|
||||
assert_eq!(segments("// also a comment"), Vec::<&str>::new());
|
||||
assert_eq!(segments(" "), Vec::<&str>::new());
|
||||
assert_eq!(segments("a . . b"), vec!["a", "b"]);
|
||||
assert_eq!(segments("move-selection 2 0"), vec!["move-selection 2 0"]);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_pure_commands_two_arms_agree_on_no_arguments() {
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
|
||||
let m = two_cat_model();
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
|
||||
// `w` with no path saves to the current document. The interactive arm
|
||||
// used to refuse exactly that, which is the same demotion one level down.
|
||||
let by_parse = reg
|
||||
.parse("w", &[])
|
||||
.expect("the pure arm accepts a bare `w`");
|
||||
let by_ctx = parse_segment_interactive("w", &ctx)
|
||||
.expect("so must the interactive arm")
|
||||
.expect("a segment with a word builds a command");
|
||||
assert_eq!(by_parse.name(), by_ctx.name());
|
||||
assert_eq!(
|
||||
format!("{:?}", by_parse.execute(&ctx)),
|
||||
format!("{:?}", by_ctx.execute(&ctx)),
|
||||
"the two arms of a pure command must build the same thing"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unknown_segment_is_an_error_not_a_silent_skip() {
|
||||
use crate::command::cmd::test_helpers::*;
|
||||
|
||||
let m = two_cat_model();
|
||||
let layout = make_layout(&m);
|
||||
let reg = make_registry();
|
||||
let ctx = make_ctx(&m, &layout, ®);
|
||||
let err = parse_segment_interactive("no-such-command", &ctx)
|
||||
.expect_err("an unknown word is refused");
|
||||
assert!(err.contains("no-such-command"), "got: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1704,6 +1704,35 @@ mod tests {
|
||||
App::new(wb, None)
|
||||
}
|
||||
|
||||
/// improvise-htl6. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn the_view_history_walks_both_ways_from_the_keyboard() {
|
||||
let mut app = two_col_model();
|
||||
app.model_state.workbook.create_view("Second");
|
||||
app.apply_effects(vec![Box::new(crate::ui::effect::SwitchView(
|
||||
"Second".into(),
|
||||
))]);
|
||||
assert_eq!(app.view_state.active_view.as_str(), "Second");
|
||||
|
||||
app.handle_key(KeyEvent::new(KeyCode::Char('<'), KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
app.view_state.active_view.as_str(),
|
||||
"Default",
|
||||
"the control: `<` was bound all along, so a table binding nothing \
|
||||
cannot pass this test"
|
||||
);
|
||||
|
||||
app.handle_key(KeyEvent::new(KeyCode::Char(','), KeyModifiers::NONE))
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
app.view_state.active_view.as_str(),
|
||||
"Second",
|
||||
"`,` walks the history forward: before improvise-htl6 no key did, \
|
||||
and the forward stack was reachable only as `:view-forward`"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-8uz: the cursor is **per view**. A characterization test kept
|
||||
/// verbatim across the move to `ViewState::cursors`.
|
||||
#[test]
|
||||
|
||||
@@ -335,15 +335,17 @@ impl Effect for DeleteView {
|
||||
pub struct SwitchView(pub String);
|
||||
impl Effect for SwitchView {
|
||||
fn apply(&self, app: &mut App) {
|
||||
let current = app.view_state.active_view.clone();
|
||||
if current != self.0 {
|
||||
// History records that the view MOVED, observed after the fact — the
|
||||
// document may refuse the name (improvise-swzk, `context/code/command.md`).
|
||||
let previous = app.view_state.active_view.clone();
|
||||
app.switch_to_view(&self.0);
|
||||
if app.view_state.active_view != previous {
|
||||
app.view_state.view_back_stack.push(ViewFrame {
|
||||
view_name: current,
|
||||
view_name: previous,
|
||||
mode: app.view_state.mode.clone(),
|
||||
});
|
||||
app.view_state.view_forward_stack.clear();
|
||||
}
|
||||
app.switch_to_view(&self.0);
|
||||
}
|
||||
// Which view this client is looking at is UI, not document (improvise-jytm).
|
||||
// `Dirty` is therefore not the door: see `ClientSession::project`.
|
||||
@@ -1771,6 +1773,33 @@ mod tests {
|
||||
assert!(app.view_state.view_back_stack.is_empty());
|
||||
}
|
||||
|
||||
/// improvise-swzk. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn a_switch_the_document_refuses_leaves_the_back_stack_alone() {
|
||||
let mut app = test_app();
|
||||
app.model_state.workbook.create_view("View 2");
|
||||
SwitchView("View 2".to_string()).apply(&mut app);
|
||||
|
||||
SwitchView("NoSuchView".to_string()).apply(&mut app);
|
||||
assert_eq!(
|
||||
app.view_state.active_view.as_str(),
|
||||
"View 2",
|
||||
"the fixture must actually refuse: the document has no such view"
|
||||
);
|
||||
assert_eq!(
|
||||
app.view_state.view_back_stack.len(),
|
||||
1,
|
||||
"nothing was left, so nothing is behind us"
|
||||
);
|
||||
|
||||
ViewBack.apply(&mut app);
|
||||
assert_eq!(
|
||||
app.view_state.active_view.as_str(),
|
||||
"Default",
|
||||
"`<` must reach the real frame, not the one a refused switch buried it under"
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-2avl: the end-of-batch `layout_version` bump was argued safe
|
||||
/// and never tested. Both arms — `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
|
||||
@@ -4326,6 +4326,71 @@ mod cell_identity {
|
||||
}
|
||||
}
|
||||
|
||||
/// A `RecordHash` is document-scoped; an `AggregateHash` cannot be, and so
|
||||
/// resolves only where the host stands. Evidence: `context/test-evidence/command.md`.
|
||||
#[test]
|
||||
fn an_aggregate_a_session_was_shown_does_not_resolve_against_the_hosts_view() {
|
||||
let mut app = aggregated_app();
|
||||
app.model_state.workbook.create_view("Other".to_string());
|
||||
{
|
||||
let v = app.model_state.workbook.views.get_mut("Other").unwrap();
|
||||
v.set_axis("Product", Axis::Row);
|
||||
v.set_axis("_Measure", Axis::Column);
|
||||
v.set_axis("Region", Axis::None);
|
||||
}
|
||||
app.rebuild_layout();
|
||||
|
||||
let mut sessions = Sessions::default();
|
||||
let (id, _snap) = sessions.connect(&app, 20, 20);
|
||||
let switch = Command::SwitchView("Other".to_string());
|
||||
let checked = Checked::against(&app, &switch).unwrap();
|
||||
sessions.apply_checked(&mut app, id, checked);
|
||||
|
||||
// What the host itself is still on.
|
||||
assert_eq!(app.view_name(), "Default", "the host must not have moved");
|
||||
|
||||
// Read the view OFF the session, so deleting the switch cannot leave
|
||||
// this test green (found by review: the name was hard-coded).
|
||||
let session = sessions.get(id).expect("the session we just connected");
|
||||
let seen = session.view().active_view.clone();
|
||||
assert_eq!(
|
||||
seen, "Other",
|
||||
"the switch did not move this session's mirror"
|
||||
);
|
||||
assert_ne!(
|
||||
seen,
|
||||
app.view_name(),
|
||||
"then the two views are not different"
|
||||
);
|
||||
|
||||
let grid = grid_in(&app, &seen, session.viewport());
|
||||
let hash = grid
|
||||
.cells
|
||||
.iter()
|
||||
.flatten()
|
||||
.find_map(|c| match c.id.clone()? {
|
||||
CellId::Aggregate(h) => Some(h),
|
||||
_ => None,
|
||||
})
|
||||
.expect("the Other view aggregates over Region, so it has aggregates");
|
||||
|
||||
assert_eq!(
|
||||
app.resolve_aggregate(hash),
|
||||
None,
|
||||
"if this resolves, the host's layout can see the session's aggregate \
|
||||
and the scoping hole is not real"
|
||||
);
|
||||
|
||||
// The control: the SAME surrogate, once the host stands where the
|
||||
// session does. Without it, a blanket `None` would pass above.
|
||||
app.switch_to_view("Other");
|
||||
app.rebuild_layout();
|
||||
assert!(
|
||||
app.resolve_aggregate(hash).is_some(),
|
||||
"the surrogate is resolvable in principle; only the host's VIEW made it not"
|
||||
);
|
||||
}
|
||||
|
||||
/// An aggregate names a coordinate *constraint* and must come back as the same
|
||||
/// one, page and filter coordinates included (gotcha 27).
|
||||
#[test]
|
||||
|
||||
@@ -1765,11 +1765,12 @@ mod formula_tests {
|
||||
);
|
||||
}
|
||||
m.add_formula(parse_formula("Count = COUNT(Sales)", "_Measure").unwrap());
|
||||
let val = m
|
||||
.evaluate(&coord(&[("_Measure", "Count"), ("Region", "East")]))
|
||||
.and_then(|v| v.as_f64())
|
||||
.unwrap();
|
||||
assert!(val >= 1.0);
|
||||
let count_at = |k: CellKey| m.evaluate(&k).and_then(|v| v.as_f64());
|
||||
assert_eq!(
|
||||
count_at(coord(&[("_Measure", "Count"), ("Region", "East")])),
|
||||
Some(1.0)
|
||||
);
|
||||
assert_eq!(count_at(coord(&[("_Measure", "Count")])), Some(3.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -785,6 +785,51 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-ipqu: the three warning tiers are concatenated, and the order
|
||||
/// is user-facing. `context/test-evidence/gp.md`.
|
||||
#[test]
|
||||
fn a_budget_notice_precedes_a_table_warning_which_precedes_a_preprocess_one() {
|
||||
let (model, view) = all_three_warning_tiers_at_once();
|
||||
let outcome = fit_measure(&model, &view, "Score", FIT_INTO, &request())
|
||||
.expect("dropping the wide column is enough");
|
||||
let at = |needle: &str| {
|
||||
outcome
|
||||
.warnings
|
||||
.iter()
|
||||
.position(|w| w.contains(needle))
|
||||
.unwrap_or_else(|| panic!("no {needle:?} warning in {:?}", outcome.warnings))
|
||||
};
|
||||
let (notice, table, preprocessed) = (at("budget"), at("no Score value"), at("Blank"));
|
||||
assert!(
|
||||
notice < table && table < preprocessed,
|
||||
"budget notice, then the table's, then preprocess's: {:?}",
|
||||
outcome.warnings
|
||||
);
|
||||
}
|
||||
|
||||
/// [`two_categories_of_different_widths`] plus the two things that make the
|
||||
/// other two tiers speak. Reasoning: `context/test-evidence/gp.md`.
|
||||
fn all_three_warning_tiers_at_once() -> (Model, View) {
|
||||
let (mut model, view) = two_categories_of_different_widths();
|
||||
// A measure with no cell anywhere is a header column that is entirely
|
||||
// missing in every row, which is what `preprocess` warns about.
|
||||
model
|
||||
.category_mut(MEASURE)
|
||||
.expect("_Measure always exists")
|
||||
.add_item("Blank");
|
||||
// A coordinate the base fixture does not use, carrying `Cost` and no
|
||||
// `Score`: one visible record `feature_table` must skip.
|
||||
model.set_cell(
|
||||
CellKey::new(vec![
|
||||
("Wide".to_string(), "w0".to_string()),
|
||||
("Narrow".to_string(), "n1".to_string()),
|
||||
(MEASURE.to_string(), "Cost".to_string()),
|
||||
]),
|
||||
CellValue::Number(1.0),
|
||||
);
|
||||
(model, view)
|
||||
}
|
||||
|
||||
/// 60 `Wide` items and 8 `Narrow` ones over 60 records, so the two columns
|
||||
/// are 60 and 8 and only one of them has to go.
|
||||
fn two_categories_of_different_widths() -> (Model, View) {
|
||||
|
||||
@@ -187,6 +187,9 @@ impl ImportPipeline {
|
||||
// value; a record is charged to the first category it is missing.
|
||||
let mut skipped_per_category = vec![0usize; categories.len()];
|
||||
let mut skipped_non_objects = 0usize;
|
||||
// Per-extraction tally of values the detected format could not parse. A
|
||||
// derived coordinate that is merely absent is improvise-yss5.
|
||||
let mut unparsed_dates = vec![0usize; date_extractions.len()];
|
||||
// Two rows sharing every coordinate are one cell; last-row-wins lost the
|
||||
// earlier one (improvise-2cw9). The map only deduplicates — gotcha 18.
|
||||
let mut cells: Vec<PendingCell> = vec![];
|
||||
@@ -196,6 +199,9 @@ impl ImportPipeline {
|
||||
if let Value::Object(map) = record {
|
||||
let mut coords: Vec<(String, String)> = vec![];
|
||||
let mut missing: Option<usize> = None;
|
||||
// Charged only once this record survives the walk: a row dropped
|
||||
// for a LATER missing category never lands (context/test-evidence).
|
||||
let mut unparsed_here: Vec<usize> = vec![];
|
||||
|
||||
for (cat_idx, cat_proposal) in categories.iter().enumerate() {
|
||||
let val = map.get(&cat_proposal.field).and_then(scalar_name);
|
||||
@@ -205,12 +211,18 @@ impl ImportPipeline {
|
||||
coords.push((cat_proposal.field.clone(), v.clone()));
|
||||
|
||||
// Extract date components from this field's value
|
||||
for (field, fmt, comp, derived_name) in &date_extractions {
|
||||
if *field == cat_proposal.field
|
||||
&& let Some(derived_val) = extract_date_component(&v, fmt, *comp)
|
||||
{
|
||||
wb.add_item(derived_name, &derived_val)?;
|
||||
coords.push((derived_name.clone(), derived_val));
|
||||
for (at, (field, fmt, comp, derived_name)) in
|
||||
date_extractions.iter().enumerate()
|
||||
{
|
||||
if *field != cat_proposal.field {
|
||||
continue;
|
||||
}
|
||||
match extract_date_component(&v, fmt, *comp) {
|
||||
Some(derived_val) => {
|
||||
wb.add_item(derived_name, &derived_val)?;
|
||||
coords.push((derived_name.clone(), derived_val));
|
||||
}
|
||||
None => unparsed_here.push(at),
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -223,6 +235,9 @@ impl ImportPipeline {
|
||||
skipped_per_category[cat_idx] += 1;
|
||||
continue;
|
||||
}
|
||||
for at in unparsed_here {
|
||||
unparsed_dates[at] += 1;
|
||||
}
|
||||
|
||||
// Attach label values as coords (missing labels become "").
|
||||
for lab in &labels {
|
||||
@@ -284,6 +299,7 @@ impl ImportPipeline {
|
||||
&skipped_per_category,
|
||||
skipped_non_objects,
|
||||
);
|
||||
warnings.extend(unparsed_date_warnings(&date_extractions, &unparsed_dates));
|
||||
warnings.extend(collision_warning(&collisions));
|
||||
|
||||
Ok((wb, warnings))
|
||||
@@ -329,6 +345,25 @@ fn scalar_name(v: &Value) -> Option<String> {
|
||||
(!name.trim().is_empty()).then_some(name)
|
||||
}
|
||||
|
||||
/// One warning per derived date column whose source value some row could not
|
||||
/// parse: the coordinate is absent, not wrong, so nothing else can report it.
|
||||
fn unparsed_date_warnings(
|
||||
date_extractions: &[(&str, &str, DateComponent, String)],
|
||||
unparsed: &[usize],
|
||||
) -> Vec<String> {
|
||||
date_extractions
|
||||
.iter()
|
||||
.zip(unparsed)
|
||||
.filter(|&(_, &count)| count > 0)
|
||||
.map(|((field, fmt, _, derived_name), count)| {
|
||||
format!(
|
||||
"column '{derived_name}' missing from {count} row(s); \
|
||||
'{field}' did not match the date format '{fmt}'"
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Human-readable warnings for records that could not be placed in the grid.
|
||||
/// Empty when every record was imported.
|
||||
fn skip_warnings(
|
||||
@@ -939,6 +974,127 @@ mod tests {
|
||||
assert_eq!(val, Some(60.0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_column_of_mixed_date_formats_is_not_proposed_as_a_time_category() {
|
||||
let raw = json!([
|
||||
{"Date": "01/15/2025", "Amount": 100.0},
|
||||
{"Date": "2025-02-05", "Amount": 200.0},
|
||||
]);
|
||||
let p = ImportPipeline::new(raw);
|
||||
let date = p
|
||||
.proposals
|
||||
.iter()
|
||||
.find(|p| p.field == "Date")
|
||||
.expect("Date");
|
||||
assert_eq!(
|
||||
date.date_format, None,
|
||||
"detect_date_format requires EVERY distinct sample to parse, which is why \
|
||||
the honest CLI/wizard route cannot hand build_model a value it will refuse"
|
||||
);
|
||||
}
|
||||
|
||||
/// A pipeline extracting Month and Year from `Date` under `%m/%d/%Y`.
|
||||
fn extracting_month_and_year(raw: serde_json::Value) -> ImportPipeline {
|
||||
use crate::import::analyzer::DateComponent;
|
||||
|
||||
let mut p = ImportPipeline::new(raw);
|
||||
for prop in &mut p.proposals {
|
||||
if prop.field == "Date" {
|
||||
prop.kind = FieldKind::TimeCategory;
|
||||
prop.accepted = true;
|
||||
prop.date_format = Some("%m/%d/%Y".to_string());
|
||||
prop.date_components.push(DateComponent::Month);
|
||||
prop.date_components.push(DateComponent::Year);
|
||||
}
|
||||
}
|
||||
p
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extracting_a_date_component_from_values_that_all_parse_warns_about_nothing() {
|
||||
let p = extracting_month_and_year(json!([
|
||||
{"Date": "01/15/2025", "Amount": 100.0},
|
||||
{"Date": "02/05/2025", "Amount": 200.0},
|
||||
]));
|
||||
let (_, warnings) = p.build_model_with_warnings().unwrap();
|
||||
assert!(
|
||||
warnings.is_empty(),
|
||||
"a clean import must stay silent: {warnings:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_row_dropped_for_a_missing_category_is_not_charged_to_the_date_tally() {
|
||||
// `Date` is walked before `Region`, so a bad date is seen first and the
|
||||
// record is only afterwards dropped for the missing `Region`.
|
||||
let mut p = extracting_month_and_year(json!([
|
||||
{"Date": "01/15/2025", "Region": "East", "Amount": 100.0},
|
||||
{"Date": "2025-02-05", "Amount": 200.0},
|
||||
]));
|
||||
for prop in &mut p.proposals {
|
||||
if prop.field == "Region" {
|
||||
prop.kind = FieldKind::Category;
|
||||
prop.accepted = true;
|
||||
}
|
||||
}
|
||||
let categories: Vec<&str> = p
|
||||
.proposals
|
||||
.iter()
|
||||
.filter(|p| {
|
||||
p.accepted && matches!(p.kind, FieldKind::Category | FieldKind::TimeCategory)
|
||||
})
|
||||
.map(|p| p.field.as_str())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
categories,
|
||||
vec!["Date", "Region"],
|
||||
"the fixture only discriminates while Date is walked FIRST"
|
||||
);
|
||||
|
||||
let (_, warnings) = p.build_model_with_warnings().unwrap();
|
||||
assert!(
|
||||
warnings.iter().all(|w| !w.contains("Date_Month")),
|
||||
"the row never landed, so no derived column lost it: {warnings:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
warnings.len(),
|
||||
1,
|
||||
"only the skipped-record warning: {warnings:?}"
|
||||
);
|
||||
assert!(warnings[0].contains("Region"), "{}", warnings[0]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_model_warns_about_values_that_do_not_match_the_date_format() {
|
||||
let p = extracting_month_and_year(json!([
|
||||
{"Date": "01/15/2025", "Amount": 100.0},
|
||||
{"Date": "2025-02-05", "Amount": 200.0},
|
||||
{"Date": "2025-03-05", "Amount": 300.0},
|
||||
]));
|
||||
let (wb, warnings) = p.build_model_with_warnings().unwrap();
|
||||
let derived = wb
|
||||
.model
|
||||
.category("Date_Month")
|
||||
.expect("the derived category");
|
||||
assert_eq!(
|
||||
derived.items.keys().map(String::as_str).collect::<Vec<_>>(),
|
||||
vec!["2025-01"],
|
||||
"two of the three values did not parse, so they contribute no item"
|
||||
);
|
||||
// Two derived columns lost the same two rows, and each says so: a single
|
||||
// "some dates did not parse" line cannot be told from this one.
|
||||
assert_eq!(
|
||||
warnings,
|
||||
vec![
|
||||
"column 'Date_Month' missing from 2 row(s); 'Date' did not match the date \
|
||||
format '%m/%d/%Y'",
|
||||
"column 'Date_Year' missing from 2 row(s); 'Date' did not match the date \
|
||||
format '%m/%d/%Y'",
|
||||
],
|
||||
"one located warning per derived column"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_model_extracts_date_month_component() {
|
||||
use crate::import::analyzer::DateComponent;
|
||||
|
||||
@@ -289,6 +289,7 @@ pub fn describe(command: &'static str) -> &'static str {
|
||||
"toggle-records-mode" => "records mode",
|
||||
"transpose" => "swap rows and columns",
|
||||
"view-back" => "back",
|
||||
"view-forward" => "forward",
|
||||
"yank" => "yank the cell",
|
||||
other => other,
|
||||
}
|
||||
@@ -498,6 +499,7 @@ fn navigation() -> Vec<HelpLine> {
|
||||
Some(HelpLine::Blank),
|
||||
keys(n, "drill-into-cell", None, "Drill into selected cell"),
|
||||
keys(n, "view-back", None, "Go back to previous view"),
|
||||
keys(n, "view-forward", None, "Go forward again after going back"),
|
||||
]
|
||||
.into_iter()
|
||||
.flatten()
|
||||
|
||||
@@ -634,6 +634,9 @@ impl Keymaps {
|
||||
// Drill into aggregated cell / view history
|
||||
normal.bind(KeyCode::Char('>'), none, "drill-into-cell");
|
||||
normal.bind(KeyCode::Char('<'), none, "view-back");
|
||||
// `,` and not `>`: `>` drills into the grid, so the pair that LOOKS
|
||||
// symmetric is not one (improvise-htl6, `context/code/tui.md`).
|
||||
normal.bind(KeyCode::Char(','), none, "view-forward");
|
||||
|
||||
// Records mode toggle and prune toggle
|
||||
normal.bind(KeyCode::Char('R'), none, "toggle-records-mode");
|
||||
|
||||
@@ -2327,10 +2327,10 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The forward sibling, reached by name: no keymap binds `view-forward`
|
||||
/// in either front end (improvise-cfly). See the evidence file.
|
||||
/// The forward sibling, and the key that reaches it in both front ends
|
||||
/// (improvise-cfly, improvise-htl6). See the evidence file.
|
||||
#[test]
|
||||
fn the_forward_step_mirrors_its_cmd_and_no_key_reaches_it() {
|
||||
fn the_forward_step_mirrors_its_cmd_and_one_key_reaches_it() {
|
||||
let mut state = in_mode(AppMode::Normal);
|
||||
assert_eq!(
|
||||
composed(&state, ev(KeyCode::Char('<')), "view-forward", &[]),
|
||||
@@ -2351,15 +2351,24 @@ mod tests {
|
||||
for (_, keymap) in Keymaps::shared().iter() {
|
||||
bound_commands(keymap, &mut bound);
|
||||
}
|
||||
// improvise-htl6 inverted this: it asserted view-forward was bound by
|
||||
// NOTHING, which is where the gap was meant to stop being one.
|
||||
assert!(
|
||||
!bound.iter().any(|(name, _)| name == "view-forward"),
|
||||
"a key now reaches view-forward: this test is where that stops \
|
||||
being a gap, and the arm above stops being unreachable"
|
||||
bound.iter().any(|(name, _)| name == "view-forward"),
|
||||
"view-forward has lost its key again and the arm above is \
|
||||
unreachable: the TUI is back to :view-forward and the browser to \
|
||||
nothing at all"
|
||||
);
|
||||
assert!(
|
||||
bound.iter().any(|(name, _)| name == "view-back"),
|
||||
"and the control: view-back IS bound, so the check above is not \
|
||||
vacuous over a misspelling"
|
||||
"and the control: view-back is bound too, so the check above is \
|
||||
not vacuous over a table that binds everything or nothing"
|
||||
);
|
||||
assert_eq!(
|
||||
composed(&state, ev(KeyCode::Char(',')), "view-forward", &[]),
|
||||
Some(vec![Command::ViewForward]),
|
||||
"and the key is `,` — the unshifted twin of `<`, so both halves of \
|
||||
the history sit on one physical key"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -52,6 +52,7 @@ location(wasm_ci,'.github/workflows/ci.yml','wasm job').
|
||||
location(view_state_membership,'crates/improvise-command/src/ui/app.rs','ViewState doc comment states both membership tests; App carries the session-local residue tag').
|
||||
location(axis_presentation,'crates/improvise-core/src/view/axis.rs','plain_label/glyph/short_label/initial/color/next on Axis; AxisColor semantic palette').
|
||||
location(axis_color_resolution,'src/draw.rs','axis_fg maps AxisColor to a ratatui Color, beside minibuf_fg which does the same for MinibufColor').
|
||||
location(tui_truncate,'src/ui/text.rs',truncate).
|
||||
invariant(cellkey_sorted,'CellKey coords always sorted by category; build via CellKey::new/with/without').
|
||||
invariant(display_rounding,'format_f64 is render-only; never feed rounded values back into eval').
|
||||
invariant(fixedpoint_eval,'recompute_formulas iterates to stability; circular -> CellValue::Error(\"circular\")').
|
||||
@@ -124,6 +125,18 @@ invariant(normal_mode_is_resolved_not_carried,'AppMode::Normal is resolved throu
|
||||
invariant(the_frame_basis_follows_active_view,'The cursor frame basis re-bases AT THE SWITCH, in the loop, as soon as active_view moves -- not at the next geometry effect. Guarding the COMPARISON on the two names agreeing (PR #78 round 2) exempted the first geometry effect after a mid-batch SwitchView from the check altogether, which is under-reporting and gotcha 35 expensive side. SwitchView stays ViewOnly: the basis is App own bookkeeping and none of it crosses the wire, so this needed no protocol change. Costs one extra view_frame per batch that both switches and moves geometry, DrillIntoCell paying most.').
|
||||
invariant(encoded_width_mirrors_every_preprocess_branch,'The preflight runs BEFORE preprocess and the exact check AFTER it, so the exact check bounds the error message and not the allocation. encoded_width must mirror EVERY arm of preprocess inference -- Numeric, Date (5 parts, 7 with a time) and Categorical (distinct plus the reserved (missing)) -- plus the .missing flag. It may over-count; it must never under-count. improvise-x1ns was TWO under-counts one inference-branch apart: the categorical (missing) one-hot, and the entire Date arm. The guard is that encoded_width matches Forced WITH NO WILDCARD, so a fourth variant is a compile error rather than a silent categorical fallback -- which is exactly how Date was missed.').
|
||||
invariant(opfs_sync_handles_are_dedicated_worker_only,'createSyncAccessHandle is spec-ed [Exposed=DedicatedWorker] and is ABSENT -- not rejected, not flagged, not transient -- on a file handle obtained inside a module SharedWorker. Measured 2026-09-21 on Chromium and Firefox: navigator.storage.getDirectory() and getFileHandle() both SUCCEED there and the method is simply not on the object; the real OpfsFs bundle, run unmodified in a shared worker, fails at exactly that call. Playwright WebKit has no OPFS in any context, so it is out one step earlier. Consequence: gotcha 28-s premise -- the handle-s read/write/truncate/flush ARE blocking, which is the whole reason Storage stayed synchronous -- is a DEDICATED-worker-only premise, so a SharedWorker cannot host OpfsFs as written and every \"browser worker\" in that argument means a dedicated one. The trap for anyone costing multi-tab: Sessions, the Addressed filter and the single frame pair are all ready and none of it helps, because the no is entirely on the storage seam. See improvise-12ha for the three probed ways out (async seam / nested worker / relay) and improvise-7gip for the guard that fails to enforce this.').
|
||||
invariant(one_display_width_truncator,'src/ui/text.rs::truncate is the one display-width-safe truncator the TUI widgets share; grid.rs and formula_panel.rs call it. import_wizard_ui.rs keeps a separate BYTE-measured truncate deliberately, because its measure decides where every wizard line wraps and bytes are never fewer than cells. The sibling hazard is a byte count used as a COLUMN OFFSET (inner.x + name.len()), which never panics because Buffer::set_string clips, and so shows only as a gap widening with each non-ASCII character.').
|
||||
invariant(page_items_ride_inside_cycle,'RenderPageCycle carries the item list INSIDE cycle rather than beside it, so PageAxis::cycle is one predicate and not two (improvise-rw6l). A Filter axis has no cycle, and therefore gets no down-arrow, no menu, no items, is painted as a span rather than a button, and Click::PageItem composes nothing if a click reaches it -- four answers off one home, which is gotcha 27 holding. items.is_some() appears nowhere in the browser. This is stronger than the guard originally asked for (a test that a Filter axis gets no arrow): the wrong state is not constructible.').
|
||||
invariant(one_format_plan_per_render,'FormatPlan REPLACED the (comma, decimals) pair in every render path -- GridLayout::display_text, viewport::grid_body and compute_col_widths, project::grid_in, export.rs::render_csv, ui/app.rs -- rather than being added beside it. Two answers to \"what format is this cell\" is the disease (design-principles section 5, improvise-edp).').
|
||||
invariant(format_ownership_is_formula_for_order,'A cell format comes from the FIRST formula in model order whose target coordinate matches, the same rule and order that decides whose VALUE it is. A match naming no format falls through to the view -- a match, not a miss. Found by review: a HashMap keyed by target made the SPELLING hash-dependent while the value was not (gotcha 18 shape in a new place), and skipping unformatted formulas let a later FORMAT reach a cell it does not produce.').
|
||||
invariant(a_column_total_is_resolved_from_its_column,'GridLayout::column_key(col) -- the page and filter constraints plus the column tuple, the coordinates surviving the ROW aggregation. Reading row 0 let a row-axis formula format reach a total that had summed that row away, and made the spelling depend on row order. Found by review after context/code/core.md had ARGUED the row-0 shortcut was safe.').
|
||||
invariant(baseline_is_measured_in_your_own_worktree,'A dispatch brief carried 1638 as the test baseline; 7470b61 measures 1645. The orchestrator had copied the number from the PREVIOUS agent report, taken at 364052b, before PR #123 merged and added seven tests. An agent trusting it would have read +11 for four new tests and gone hunting. Measuring the baseline at YOUR OWN base commit costs one run and is the only thing that makes the delta checkable -- a number carried forward from another report is a claim about a commit you are not on.').
|
||||
invariant(a_hidden_toast_keeps_its_text,'The toast element is the record of what was last said and is NOT emptied when it hides (improvise-n6t8). harness/tests/drill.spec.js discriminates between two refusals by reading it, so an emptied node would satisfy not.toContainText(\"aggregate\") VACUOUSLY the moment the timer fired -- a passing assertion about an element with no content. Argued in web/index.html and context/code/clients.md.').
|
||||
invariant(headless_cmd_and_script_are_one_function,'run_headless_script DELEGATES to run_headless_commands, so \`improvise cmd\` and \`improvise script\` cannot diverge in what they print, what they exit with, or when they write the document back. Asserted by a_script_reports_exactly_as_cmd_does rather than left to the delegation, because a future caller could inline it.').
|
||||
invariant(the_creating_columns_insert_and_so_suppress_the_advance,'A +cat or +measure commit that NAMES something inserts a column at the cursor own index, so an advance computed from the PRE-commit layout is wrong for both -- CursorState::col_count is stale because apply_effects rebuilds only after the batch (gotcha 23). inserts_a_column covers both creating columns and takes the committed VALUE, because an empty name creates nothing and is genuinely at the right edge. created_name is the one home of that rule, read by add_measure, add_category and the predicate.').
|
||||
invariant(a_reported_share_cannot_overflow_its_scale,'gp_tree::fit::importance_ranking sums and divides in f64 and narrows only the resulting share, which is bounded by 100 and therefore always finite (improvise-k844). TWO overflows, not one: 100.0*imp[j] associates left so a lone importance over f32::MAX/100 is inf before the divide that would have returned it to range; and the f32 TOTAL itself overflows when several are large, which reassociating to 100.0*(imp[j]/total) turns from NaN% into a SILENT 0% FOR EVERY FEATURE. f64 closes both, since no sum of finite f32s can overflow it. The residual guard answers only an importance that ARRIVES non-finite -- an infinite total apportions nothing, so the ranking is empty rather than NaN, and note NaN <= 0.0 is false so the pre-existing early return passed NaN straight through. Reachable PAST improvise-pv3g because both callers score importance in the target OWN units, making it an MSE difference that grows as the target square.').
|
||||
invariant(viewport_extent_is_external_on_both_front_ends,'The viewport extent is an EXTERNAL INPUT that reaches the document, and it is not a browser problem. run_headless_commands reads crossterm::terminal::size() into app.term_{width,height} and CmdContext::{visible_rows,visible_cols} derive from it: 80x24 gives visible_rows 16, 200x60 gives 52. Measured on a 40-row grid, the SAME command sequence -- one PageDown then clear-cell at the cursor -- deleted R12 at 80x24 and R39 at 200x60. So Client::set_viewport is not a browser-introduced input; it is the browser SPELLING of one the TUI has always had, and gotcha 47 \"not derivable\" is true of both front ends. Any ADR treating extent as a browser problem produces a log that replays in a browser and not in a terminal. Cheapest of the five external inputs to close -- one pair of integers with an existing wire spelling, ClientFrame::Viewport -- but it must land in the SAME CHANGE as improvise-6uva, which currently masks it. A test must drive TWO sizes; one cannot see this.').
|
||||
invariant(aggregate_hash_is_layout_scoped,'A RecordHash falls back to DataStore maintained index and so resolves against the DOCUMENT, which is why a connected client SetCell works from any view. An AggregateHash has no such index AND CANNOT HAVE ONE -- an aggregate has no existence outside the layout that derives it. So App::resolve_aggregate answers only for the HOST active view, which since improvise-jytm is not the client. Guarded by an_aggregate_a_session_was_shown_does_not_resolve_against_the_hosts_view, whose CONTROL is the load-bearing half: the same surrogate resolves once the host is moved onto that view, so a blanket None -- a typo in the hash, an empty layout -- cannot pass.').
|
||||
doctrine(bug_first,'write a failing test demonstrating the bug before fixing it').
|
||||
doctrine(unit_over_e2e,'prefer small unit tests over integration/e2e').
|
||||
doctrine(suite_speed,'Two numbers, because the ambiguity between them is what let this drift: NO SINGLE TEST BINARY may exceed 2 seconds, and the WORKSPACE WALL CLOCK is the sum of them. Measured at 22d43b2: 1309 tests over 33 binaries, ~2.2s wall, longest single binary 1.42s -- so the workspace is already past 2s and the rule that bites is the per-binary one. Re-MEASURE rather than re-assert: cargo test --workspace prints a \"finished in\" per binary. Excludes compilation, and excludes harness/ (Playwright, minutes, its own CI job).').
|
||||
@@ -313,6 +326,48 @@ doctrine(a_merge_prints_pop_whether_or_not_it_stashed,'On a conflicted merge wit
|
||||
doctrine(a_missing_loose_ref_is_not_a_deleted_branch,'A watcher reading .git/refs/heads/** directly -- which a worktree-isolated agent must, because the shell guard refuses git in a loop -- cannot distinguish a DELETED branch from one that pack-refs moved into .git/packed-refs. Both present as a vanished file. Observed together in a single event: wave/q-keymap was genuinely deleted by its PR merge while wave/r-browser-page was merely packed. So a MISSING event is a PROMPT TO CHECK WITH GIT, never a conclusion; pair it with git branch --list before acting.').
|
||||
doctrine(rebuild_the_integration_on_the_new_base_and_diff_the_trees,'When part of a wave merges upstream while an integration is held, rebuilding the integration on the NEW base and diffing the two trees is the strongest available evidence that the upstream merges lost nothing. 2026-09-21: git diff --stat between the locally-integrated preview (old base + four local merges) and gh/main-plus-the-remaining-branch was EMPTY -- byte-identical. That answers the clean-auto-merge hazard DIRECTLY rather than by inference, and every gate run on the first tree transfers to the second by identity. Cheaper than re-gating and strictly better than assuming, because a clean auto-merge can delete tests with no conflict and no compile error.').
|
||||
doctrine(a_reported_sha_is_correct_and_short_lived,'Three times in one session an integration report named SHAs that were accurate when measured and stale within minutes -- twice because agents pushed self-review fix commits, once because three PRs merged upstream while the preview was held. The remedy is a watcher, not diligence: heads were verified at the start AND the end of a pass and the table was still stale, because the moves landed after the last check. A report that names no SHA is not checkable; a report that names one has a shelf life. Re-read immediately before acting on it -- and the same applies to a SHA written into a PR body.').
|
||||
doctrine(an_empty_actions_log_is_retention_not_history,'gh run list came back empty for the improvise release.yml workflow and was read as \"only one tag push ever ran this\". GitHub Actions logs age out at 90 days and the rc1/rc2 tags were five months old; both carry COMPLETE published releases (all four targets, sha256 files, dist-manifest.json, source.tar.gz, installer), so the workflow had demonstrably run end to end the last time a tag and the version agreed. The artefact a run PUBLISHED outlives the run, so ask gh release before gh run -- and gh run list --workflow=release.yml --event=push keeps the tag history even where the logs themselves are gone. Two further claims in the same passage failed the same way and are worth knowing as a set: a recurrence was inferred from run DURATIONS when only one of the three failures had ever been diagnosed (an identical signature is not an identical cause -- the rc1 logs aged out and its failures sit on initial-setup commits, where plan could have failed on an incomplete dist config), and the remotes a tag sat on were asserted rather than asked. Ask; then note that the asking has a shelf life too.').
|
||||
doctrine(a_verified_remote_fact_has_a_shelf_life,'v0.1.0-rc3 was measured three times in one session and read differently each time: present on all three remotes and locally (asserted, from the aa4e901 commit message), then on gh alone (git ls-remote), then nowhere at all -- the tag was deleted mid-review. Only the FIRST reading was careless. The second and third were each taken correctly, by two different people, and they contradict. So the rule is NOT ask-rather-than-assert: a verified answer about a remote, a tag, a deployment or a dashboard DECAYS, and prose cannot distinguish a stale line from a live one, so a reader following a checklist at 2am cannot tell which they hold. A document that records an inventory is therefore wrong on a schedule it does not control. Record HOW TO ASK (git tag -l, git ls-remote --tags <remote>) and say in the document that no inventory is recorded, so the blank reads as deliberate rather than as an omission to helpfully fill in. THE KEEPER IS THE ONE WHOSE SUBJECT CANNOT CHANGE: \"the commit message was wrong\" is durable, because its subject is a fixed text; \"the tag is on gh alone\" was true and is now false. That is the test for whether a sentence belongs in context/ at all. The half-lesson that stops one step short, and the reason this fact exists: the gh-alone reading was FOUND by running the command, from which the obvious conclusion is run-the-command -- and that was written into two files as though it settled the matter. Running the command is necessary and INSUFFICIENT, because the answer expires. Four copies of the volatile half had to be removed, one of them in the header of scripts/release-preflight.sh, which is the file a reader opens first.').
|
||||
doctrine(a_ready_ticket_may_already_be_fixed,'improvise-pv3g was dispatched to an agent as an open P1 and was already fixed on main: f616487 was an ancestor of the very commit the agent was told to branch from, and bd still showed the ticket OPEN because nobody closed it when the work landed. improvise-uej8 looks the same, named in 7a44616 via PR #114. So bd ready is a list of tickets nobody closed, NOT a list of work outstanding -- the two coincide only while closing is perfect, and in a session with several agents landing several PRs it is not. THE CHECK IS CHEAP AND BELONGS IN EVERY BRIEF: reproduce the bug at HEAD before implementing, and if it does not reproduce, find the commit that fixed it, confirm it is an ancestor of the base, and report the SHA instead of opening a PR. The agent that found this built the ticket fixture and ran the SHIPPED BINARY at the commit before the suspected fix, in a throwaway worktree, showing the bug in both directions rather than inferring from a commit message. The failure this avoids is not wasted time: it is a fabricated fix for an already-fixed bug, which can undo the real one and costs a reviewer trust.').
|
||||
doctrine(a_refusal_the_headless_path_discards_is_not_a_refusal,'improvise-pv3g acceptance asked for an actionable refusal naming the cause, and the wording was written, tested and correct. run_headless_commands (src/main.rs) applies effects and never prints view_state.status_msg, and exit_code is set to 1 only for PARSE errors -- so through \`improvise cmd\` a refused command is indistinguishable from a successful one: silence and exit 0 either way. The TUI renders the status line, so the criterion is genuinely met there and the headless path is where it evaporates. No test sees it because a_refusal_names_what_went_wrong_and_stores_nothing asserts the EFFECT status, and the effect is correct; nothing asserts the binary emits it. Filed as improvise-j6x8. The shape to carry: when acceptance says a user is TOLD something, name the surface -- an effect carrying a message is not a user being told, and the layer that drops it is usually one the test suite does not drive.').
|
||||
doctrine(a_prose_count_is_checked_by_no_one,'context/code/tui.md said mode_style spells FOURTEEN default-chrome AppMode variants; it spells thirteen, verified against src/draw.rs. The match is exhaustive and sound -- only the sentence was wrong, and nothing in this repo can check a number written in prose. Same failure mode as gotcha 38 subtracting SwitchView and forgetting its two siblings, and gotcha 44 listing gaps that had already closed. THE REPAIR SHAPE: write the ARITHMETIC beside the count so a reader can check it (13 + Editing/RecordsEditing + CommandMode + TileSelect = 17 = the AppMode variant count), and say when it was wrong.').
|
||||
doctrine(a_rendered_string_is_pinned_wherever_anything_reads_the_screen,'improvise-771e changed the browser tile bar from Axis::short_label to plain_label. The orchestrator pointed the agent at tests/grid_frame_agreement.rs as the thing that might pin the vocabulary; it does not mention the tile bar at all. harness/tests/drill.spec.js DID -- it asserted the tile text exactly, twice, with \"Quarter Pag\" AS THE EXPECTED VALUE, a Playwright spec in a directory the ticket never named, asserting the bug as its expectation. So when changing a rendered string, GREP FOR THE STRING, not for the function that produces it: a screen-reading test pins the output wherever it lives, and the producer name will not find it. A now-false comment naming short_label in contract.spec.js was found the same way.').
|
||||
doctrine(a_duplicated_inheritance_edge_is_invisible_to_a_behavioural_test,'improvise-wiew: parenting RecordsNormal to document_keymap DIRECTLY, when it already reaches it through Normal, is a fault the behavioural test cannot see -- the key still works, so pressing it proves nothing. Only records_normal_inherits_the_document_layer_once_through_normal, which asserts the CHAIN IDENTITY (exactly four deep), catches it. The same asymmetry appeared in PR #112: removing a mode keymap parent failed ONLY the Arc::ptr_eq chain guard while 38 behavioural keymap tests passed, because the mode own AnyChar answered identically. A keymap chain needs a structural assertion beside the behavioural one; the behavioural one is satisfied by any arrangement that happens to route the key.').
|
||||
doctrine(distrust_a_context_paragraph_that_argues_a_case_cannot_arise,'context/code/core.md claimed the row-0 totals shortcut was safe \"for the case it cannot arise in\". It arises: a formula target category is any category, so it can own a row-axis item. Such a paragraph is doing the work a type or a test should, and nothing checks it. Note who wrote it: the same agent that had just read the code and had, in the same edit, correctly closed a gap core.md itself had named. Reading the code is what produces both the good paragraph and the wrong one.').
|
||||
doctrine(a_clean_merge_is_not_a_tested_merge,'harness/tests/pages.spec.js arrived on main from PR #123 while PR #125 was in flight, asserting a display literal the new decimal default changes. The merge was CLEAN and the combination was WRONG, and only CI merge ref had ever run both sides -- the local run was green on a branch that had never seen the file. So a PR whose change is cross-cutting should merge main and RE-RUN THE GATES before calling itself green, not after CI says otherwise. This is design-principles section 7 third case, arriving again.').
|
||||
doctrine(rebuild_the_assembled_site_after_merging_web,'harness/serve is gitignored and built once. After a merge touching web/, Playwright failed 8 tests with \"Quarter Pag\" vs \"Quarter Page\" and missing elements -- nothing was broken: harness/build.sh had run BEFORE the merge, so serve/ held pre-merge web/*.js against a post-merge spec. Re-run web/build.sh and harness/build.sh after any merge touching web/, or debug a product that is not what is being served. The failure mode is indistinguishable from a real regression.').
|
||||
doctrine(a_none_that_carries_two_facts_is_a_black_hole,'KeymapSet::dispatch spelled construction as registry.interactive(name,args,ctx).ok()?, so Option::None meant BOTH \"no binding matched\" and \"a binding matched but would not build\" -- and App::dispatch_key answers None with Dirty::NOTHING and no status, so a bound key was silent. improvise-55l5 split them: None means no binding matched and nothing else; a construction failure returns Some(vec![set_status]). THE TEST PAIR IS THE POINT, because one test cannot separate two facts one value carried: the rival that swallows the error fails a_bound_key_whose_command_cannot_be_built_says_so, and the rival that ALSO reports a failed lookup fails an_unbound_key_stays_silent -- without that control the fix writes a status for every unmapped character in every mode.').
|
||||
doctrine(surface_the_error_class_no_static_check_can_see,'improvise-55l5 separated the constructor errors into two classes. CLASS A reaches a user and nothing static can see it: \"no cell at cursor\" from six registrations reading ctx.cell_key(), which is STATE-dependent, so no registration-time check applies -- and the worst case is commit-cell-edit, where the user has typed a value in Editing and the batch is discarded. CLASS B is keymap-authoring mistakes that every_bound_command_name_is_registered already refuses at test time, checking both halves of the old expression (gotcha 15). SO THE REPRO THE TICKET ITSELF PROPOSED WAS THE ONE CASE THAT CANNOT HAPPEN. Class B surfaces anyway, deliberately: a list of blessed errors would be a second description of which failures matter, free to disagree with the first.').
|
||||
doctrine(cleanup_done_by_one_of_two_call_sites_belongs_to_the_command,'AddRecordRow returned SetCell and never placed the cursor, so the frame observation invalidated it to the origin mid-batch and o opened an editor on ROW 0 -- renaming an unrelated record and moving every measure cell in it (gotchas 1 and 24). But CommitAndAdvance Tab-at-bottom-right path had ALWAYS placed the cursor, at the CALL SITE. One of two callers doing the cleanup is the tell: the placement was a property of the caller where it is a property of the command. Look for this wherever two call sites of one command differ in what they do AFTERWARDS.').
|
||||
doctrine(a_reserve_that_caps_hides_its_own_failure,'A fixed-size slot sized by eye needs a guard, and the guard can only see an overflow that GROWS the box. A width or max-width absorbs an over-long value inside a box that never moves, so every bounding-box measurement stays identical and the regression is invisible. Reserve with a FLOOR (min-width) and let the measurement catch the day the reserve stops fitting. improvise-n6t8: web/style.css #mode is min-width: 21ch, and removing it fails both \"the bar moved in CATEGORIES\" and the 16-char check-the-checker. The general form: when a guard measures a consequence, choose the mechanism whose failure PRODUCES that consequence rather than absorbing it.').
|
||||
doctrine(dismiss_a_repaint_driven_surface_by_identity_not_by_timer,'A transient UI over a value that is STATE rather than an event must key its dismissal on what it last ACCEPTED, not on a timer. ViewState::status_msg is re-offered by every repaint -- web/app.js calls the sink unconditionally and repaint runs on every keydown -- so a setTimeout alone resurrects the message on the next keypress, which is worse than a message that simply stays. improvise-n6t8 keeps what createToasts last accepted, and keeps it AFTER the toast hides, so an equal offer is recognised as a repaint and dropped. COST, STATED RATHER THAN HIDDEN: two identical messages in a row show once. That is forced -- nothing beside status_msg carries a sequence and model.revision moves for a cursor key as readily as for a re-issued command, so of the two readings of an ambiguous offer only one avoids reintroducing the bug.').
|
||||
doctrine(a_user_route_cannot_separate_what_a_user_cannot_separate,'improvise-bd9o guards on state.grid.records rather than on the mode, and the whole argument turns on those two facts being different things. TWO INJECTED RIVALS -- view.mode.is_records() and the keymap mode.is_records() -- PASSED THE ENTIRE WORKSPACE. The end-to-end route test could not see them for a structural reason: it enters records mode the way a user does, where the grid and the mode AGREE. An end-to-end test is the right test for \"is this route silent\" and the WRONG test for \"on which of two agreeing facts does this guard depend\". When a fix is argued on a distinction only DRIFT can expose, the test must STATE the drift rather than reach it -- build the client whose grid and mode disagree. The tell to look for: a paragraph arguing a choice that no existing fixture can exhibit.').
|
||||
doctrine(guard_the_entrance_not_the_arm,'improvise-bd9o: a guard on the CellId::Record arm answers \"is this CELL one we cannot commit\" where the question is \"is this GRID one we cannot commit in\". The separating case is the coordinate column, which carries NO CellId at all -- cell.id.as_ref()? short-circuits, so an arm guard never runs and the key does nothing silently. The guard therefore sits before cell_at_cursor, on the observed grid. The general form: when a refusal is about the CONTEXT, guarding a branch inside the context leaves every path that never reaches the branch unguarded, and those paths are the ones that fail silently.').
|
||||
doctrine(a_kind_a_name_cannot_carry_must_be_passed_not_re_derived,'CategoryKind::of_name reads Label as Regular BY DESIGN, and that single fact produced improvise-dy3j and improvise-cj4u at two different layers: a file format with nowhere to write the kind, and View::on_category_added asking of_name instead of being told. The cj4u fix is THE SIGNATURE -- passing the kind is what made the compiler name every call site -- and it deleted the two Workbook set_axis(name, Axis::None) corrections that were a second answer to \"where does this category go\", of which only ONE of two callers knew to write it: add_label_category fixed the views existing at that moment while create_view, holding only names, pivoted the label onto Row. Ask of any stand-in accessor which distinctions it deliberately collapses, and then which callers those distinctions matter to.').
|
||||
doctrine(a_section_header_is_not_a_place_to_put_a_modifier,'improvise-dy3j proposed \"## Category: Note [label]\" and improvise-pczv forbids it: header_name takes the name as the WHOLE line, totally, so a suffix is indistinguishable from a category literally named \"Note [label]\" -- adopting it would make a legal name unspellable and re-open the ambiguity header_name exists to close. The modifier goes INSIDE the section as an entry line, with a real token (\"kind: \") rather than a lookahead, because the grammar-walking generator reads the grammar through pest_meta and SKIPS predicates. A ticket that proposes a format syntax has usually read the writer and not the grammar.').
|
||||
doctrine(agreement_between_two_wrong_copies_is_not_evidence,'A lockstep test that only compares two front ends PASSES when both derive the same WRONG answer -- which is the EXPECTED outcome of a duplicated derivation, not a passing grade. improvise-vw47: both_ends passed against the bug, because cmd/panel.rs and resolve.rs both computed format!(\"View {}\", len + 1) and were wrong identically, so the two documents agreed perfectly. Only the assertions on the RESULTING DOCUMENT caught it -- the view count must rise, the survivor format must be untouched. \"The two ends agree\" and \"the two ends are right\" are different questions, and a duplicated derivation is exactly the case that separates them. A cross-front-end test must assert that what the ends agree on is RIGHT, as well as that they agree.').
|
||||
doctrine(a_field_with_no_severity_cannot_be_split_by_guessing,'improvise-q914: the orchestrator brief proposed stdout for a status and stderr for a refusal. ViewState::status_msg is a bare String with NO severity, and BOTH candidate classifiers are disproved by code already in the tree -- \"a refusal aborted\" is false, because effect::AddItem writes its Err to status_msg and does not set abort_effects, so that rule labels a refusal a SUCCESS; and \"a refusal is anything that spoke\" is false, because cmd/fit.rs sets a status on the SUCCESS path carrying the reproducing seed, so that rule labels a deliverable a FAILURE. Both verified in-tree by the orchestrator. Choosing either is design-principles section 7 adjacent-check defect. The honest answer was one undivided stream, and the real fix is a severity on the field -- a wire change, filed as improvise-udt2 naming both disproved rivals so the next attempt does not rediscover them.').
|
||||
doctrine(a_narrow_exit_contract_beats_an_overpromised_one,'improvise-q914 asked for a non-zero exit when an effect is refused. Delivered: exit 1 on a parse error or App::last_batch_aborted -- both REAL flags -- and no further, with a status-only refusal printed but still exiting 0 because nothing can classify it. README states it plainly: exit 0 means \"nothing aborted\", not \"everything worked\". A contract that promises less and is TRUE is worth more than one that promises the ACs wording and is guessing, because a script author who trusts the wrong one proceeds on a no-op. The shortfall is recorded on the ticket and filed rather than glossed.').
|
||||
doctrine(observe_dirty_per_command_because_save_clears_it,'The headless write-back gate accumulates model_state.dirty PER COMMAND rather than reading it once at the end, because effect::Save CLEARS the flag (gotcha 45). Reading it once would make \`[add-item East, w other.improv]\` look unchanged and SILENTLY DROP the edit from the -f document. Same shape as the other observation gates in this repo: what is observed must be sampled where the thing being observed happens, not where it is convenient to look.').
|
||||
doctrine(key_outcome_variants_are_protocol_surface,'improvise-h3y7 audited the wire vocabulary for COMPLETENESS, enumerating Command and ModelCommand against the effects in both directions, and found nothing wrong here. The gap was in the SHAPE of what a resolver may RETURN: KeyOutcome was Local | Upstream | Ignored, with no way to say \"send this frame\" -- so ClientFrame::Save and its Host::handle arm both existed, since improvise-368h, with NO KEY ABLE TO REACH EITHER. An enumeration that stops at the frames and the commands will miss the next one the same way. KeyOutcome variants are part of the protocol surface and belong in any completeness pass.').
|
||||
doctrine(a_self_constructed_frame_is_not_reach,'a_client_can_ask_its_host_to_save BUILDS the frame it feeds to the host, so it proves the host ANSWERS the frame and never that anything can SEND one. improvise-368h therefore shipped a feature no keypress could reach, with a passing test over it -- \"a green suite is not evidence\" in its purest form. The repair is an end-to-end test from the INPUT: improvise-jz6w asserts Ctrl-S on a real Client produces the frame, a real Host answers it, and the document comes back off storage. WORTH SWEEPING: check whether other Host::handle arms have only self-constructed-frame coverage.').
|
||||
doctrine(a_review_comments_commit_id_is_per_comment_not_per_review,'Measured on PR #122 and re-verified by the orchestrator against the live API: ONE review submitted 19:48-19:53 carries five inline comments, THREE naming 246e8f4a and TWO naming 6a4c7dcd, while its body names 246e8f4a. GitHub pins each comment to the file version it was written against, so a comment on an outdated diff position keeps the older commit while its siblings move on. Two consequences: a per-entry \"EARLIER head\" flag can make a CURRENT review look partly stale, and a head comparison against the review BODY is not even a proxy for its own comments -- the old pr-reviews head check was answering about the WRONG OBJECT, not merely reading too few of them.').
|
||||
doctrine(a_multi_assignment_local_is_not_left_to_right_on_bash_5,'\`local n=\"$1\" name=\"$2\" f=\"$DIR/$n.$name.json\"\` creates EVERY name before running the assignments on bash 5, so the third clause reads an unbound variable and set -u kills the shell. It works on macOS bash 3.2. scripts/pr-reviews-test.sh therefore passed 19/19 LOCALLY and 2/19 IN CI ON THE SAME COMMIT. Give each local its own statement -- and note the real lesson: a shell checker must be wired into CI in the SAME change that adds it, because the platform difference is invisible to the machine that wrote it.').
|
||||
doctrine(a_dead_command_substitution_wears_jqs_exit_code,'Both bash-5 bugs in scripts/pr-reviews failed as EXIT 2 WITH NO OUTPUT AT ALL, and 2 is jq answering an invalid --argjson with a USAGE error. When set -u kills a command substitution, or gh api --paginate returns several concatenated documents, the empty or malformed string reaches jq and the shell fault at the TOP of the script is reported by the JSON tool at the BOTTOM. Exit 2 plus total silence means \"something upstream of jq produced nothing\", never \"jq computed a wrong answer\".').
|
||||
doctrine(a_fixture_must_be_able_to_hold_the_shape_the_live_path_produces,'gh api --paginate emits one JSON array PER PAGE, so a long thread is several concatenated documents. scripts/pr-reviews flattens the live AND the fixture path through one pages_to_array, so a fixture file can carry two concatenated arrays. A fixture that could only hold a single array would exercise a shape the live fetch never produces -- a check adjacent to the thing it names. No PR in this repo has yet reached a second page, so the test is the only thing that will see it before a user does.').
|
||||
doctrine(a_prose_argument_is_code_and_can_be_the_bug,'context/code/command.md stated that the measure + \"is the last column, so there is no column to its right for an index to shift into, and both advances are already special-cased at the bottom-right edge\". Verified verbatim on main before the fix. THE FIRST CLAUSE WAS FALSE -- the column that shifts is +measure itself -- and THE SECOND NAMED THE BUG AS A SAFEGUARD. improvise-0ygd is the defect that argument authorised. A wrong argument in context/ is not documentation debt: it is the defect, written down, and it will be read as a reason not to look. This repo keeps its most expensive artefacts in prose that nothing checks, so a paragraph asserting why a case cannot arise is the shape to distrust -- the third instance this session, after the row-0 totals shortcut and gotcha 38 arithmetic.').
|
||||
doctrine(a_missing_affordance_claim_must_render_the_screen,'improvise-p8ls claimed a TUI user has no on-screen route to help or out, and was demonstrated VIA THE HELP PRINTOUT -- a function return value -- where the claim was about WHAT IS ON SCREEN. Those are adjacent questions. draw_title has right-aligned \" ?:help :q quit \" unconditionally, every mode, every frame, since da93145 on 2026-04-01; a printout of advertised() cannot see a string draw_title renders. A ratatui TestBackend is cheap, already precedented in src/draw.rs (bottom_bar_tests::bottom_row), and settles it in one run. The ticket EVIDENCE was accurate and its CONCLUSION did not follow: the author checked advertised() and draw_welcome and concluded those were the only homes, where the title bar is a third and is the one always visible -- grepping the FACT rather than the function finds four.').
|
||||
doctrine(fix_every_sibling_sentence_or_the_paragraph_contradicts_itself,'The orchestrator corrected gotcha 47 to say ADR 001 clause 4 IS delivered, and left the next three sentences reading \"The fourth is open because App::apply_effects aborts without undoing\" -- a paragraph contradicting itself within one screen, produced by the very edit that was fixing the contradiction. Caught by reading the patched line back rather than trusting the substitution count. This is gotcha 48 turned on its author: a fact stated in more than one place is changed by a grep, and the places include the SENTENCES AROUND THE ONE BEING EDITED, not only other files.').
|
||||
doctrine(a_rollback_needs_a_positive_acknowledgement_not_only_a_refusal,'improvise-nxah: a client with two mutating frames in flight cannot tell which one a Rejected answers, so ServerFrame::Accepted is pushed once per accepted mutating frame, LAST, after everything the frame produced -- the same ordering argument Projection::Basis has, and the same shape as the publication gap ADR 001 originally left. And a SINGLE undo slot is unsound rather than merely imprecise: a snapshot taken before the first of two outstanding frames discards the second, which the host DID apply, diverging in the opposite direction. Hence settled plus an in_flight queue that drops one frame and replays the rest.').
|
||||
doctrine(a_hard_floor_across_widths_names_the_container_not_the_child,'improvise-tg27 suspected a flex min-width:auto trap swallowing #grid overflow. documentElement.scrollWidth read 1011 AT EVERY WINDOW NARROWER THAN 1011px -- a FLOOR, not a slope. A floor is the signature of a sum of flex:none RESERVES; a slope would have meant a clipped child. The reserves were the app bar (#build-version 14ch + #doc-name 34ch + #view-name 16ch + #mode 21ch + a non-shrinking #status), added the day before by improvise-n6t8, so the PAGE grew the scrollbar while main stayed window-width. Read the SHAPE of the measurement across the parameter before reading the code: constant-then-correct names a floor, and a floor names whoever set it.').
|
||||
doctrine(never_hide_the_overflow_you_are_asserting_the_absence_of,'improvise-tg27 rejected body{overflow-x:hidden}, which would have made scrollWidth == clientWidth UNCONDITIONALLY and rendered its whole spec vacuous -- every assertion still passing, the bar still not fitting. The check would then answer \"is overflow hidden\" where it means \"does the content fit\". A fix that makes the measurement unable to fail is not a fix, and it is especially tempting in CSS, where suppressing a symptom and correcting a layout look equally like one line.').
|
||||
doctrine(a_deferral_cost_must_be_measured_not_named,'improvise-k844 sat open because context/code/gp.md section 7 deferred it on the claim that reordering the arithmetic \"moves the bit patterns misclass_rate_and_importance_are_bit_for_bit_what_they_were pins\". That test pins permutation_importance RAW SCORES; importance_ranking consumes them AFTERWARDS, so no path it witnesses runs through the share -- and scoring_known_answers.rs is byte-identical across the actual fix, verified by the orchestrator as absent from the diff. A COST PREDICTED FROM A TEST NAME RATHER THAN FROM WHAT IT CALLS is how a one-line fix stays filed, and it reads as diligence, which is why it survives review. Measure the claimed cost or state none. Pairs with an_invariant_is_made_unrepresentable_not_validated: the clamp improvise-pv3g added instead was a call-site check that left inf% in the data behind it.').
|
||||
doctrine(a_reported_number_can_overflow_while_every_input_is_finite,'The gp crates learned this twice. standardize sums (t-mean)^2 in f32 (improvise-pv3g) and importance_ranking scaled a share by 100.0* in f32 (improvise-k844), and the second was REACHABLE PAST THE FIRST. Both are fixed by making the arithmetic incapable of it -- a Scaling that refuses a non-finite part, an f64 accumulator -- never by a call-site check, and pv3g own f32 clamp WAS that call-site check: it stopped the abort and left inf% in the data behind it. The silent twin is the half to look for: pv3g had cv_regress pooling a fold it could not standardize, k844 had an f32 total going inf with no panic to announce it. Ask what ELSE consumes the derived quantity.').
|
||||
doctrine(a_dev_shell_is_a_third_environment,'.envrc puts nix bash 5.3 on PATH -- the RIGHT INTERPRETER, CI semantics -- and pkgs.mkShell ALSO exports about thirty generically-named lowercase variables the runner does not have: name, out, outputs, system, shell, builder, stdenv, phases, buildInputs, doCheck and more. A script reading one by accident under set -u is SILENT IN THE DEV SHELL AND FATAL IN CI. improvise-f9qf: pr-reviews-test.sh passed 19/19 under macOS bash 3.2 AND 19/19 under the dev shell bash 5.3, while failing 2/19 in CI -- two different reasons for green, one of them the correct interpreter, and unsetting \`name\` alone reproduced CI 2/17 exactly. Note nix shell does NOT export name and nix develop DOES (verified with env -u name as the control), so the dev shell is the one nix entry point that masks this class. The orchestrator own verification of this was itself confounded by inheriting name from a direnv-loaded shell -- it answered \"does name end up set\" where the question is \"does nix shell set it\".').
|
||||
doctrine(an_observation_is_a_function_of_the_trajectory_not_the_state,'improvise-ilvl claimed \"an observation is a pure function of state\". It is not -- layout_version, cursor_epoch and model_state.dirty all differ between two routes to an IDENTICAL document (0/0/false against 2/2/true). The weaker true claim, that they are reproducible FROM THE LOG, is all replay needs. But the stronger false one is exactly what would justify SNAPSHOT-PLUS-SUFFIX LOG COMPACTION, and that is wrong silently in the direction that looks like it works: Basis puts document_generation and layout_version ON THE WIRE and clients compare them for EQUALITY, so a compacted replay is not basis-compatible with any client holding a claim, and dirty is the worker OPFS persist trigger so the same document reached by a different route persists a different number of times.').
|
||||
doctrine(a_determinism_check_can_pass_because_the_divergent_path_is_unreachable,'Three identical replays of a 26-line script fixture proved the script path deterministic and proved NOTHING, because the fixture cursor moves were silent no-ops (improvise-6uva) -- the divergence had no way to occur. The defect surfaced only when clear-cell FAILED TO PARSE for want of coordinates. Sharper form of \"a green suite is not evidence\": a passing determinism or idempotence check must be shown to be capable of FAILING, by driving the input the check exists to discriminate. It is also how one defect MASKS another -- improvise-up8g viewport hazard is invisible in improvise script precisely because 6uva discards the cursor, so fixing 6uva makes up8g live and they must be closed together.').
|
||||
doctrine(an_injected_fault_is_not_reverted_until_the_full_suite_is_rerun,'An agent injected two rivals and reverted one onto the WRONG OCCURRENCE: replace(\".is_measure()\",\".is_index_or_dim()\",1) hits the FIRST match in the file, which was drill_constraint pre-existing predicate rather than the injected one. The commit shipped with TWO faults where it had demonstrated one. Every gate re-run afterwards PASSED over the broken logic -- clippy, fmt, check-evidence, both node suites, the eight-crate wasm gate, the full Playwright run -- because the one gate that could see it, cargo test --workspace, had been measured BEFORE the injections and never again. The PR body therefore carried a figure that was checkable and false. Caught by ChatGPT Codex review. THE RULE: the targeted test going green says nothing about what the revert did on its way past, so the full suite must be re-run after every revert.').
|
||||
doctrine(a_fixture_that_names_its_own_precondition_must_read_it_back,'A test asserting that a session was shown something must READ THE VIEW OFF THE SESSION, not hard-code the name. A first version derived its grid from grid_in(&app,\"Other\",..), so the session, the switch and apply_checked could ALL BE DELETED and every assertion still passed -- it proved a grid built for a NAME while its name claimed a grid a SESSION WAS SHOWN. The repair reads sessions.get(id).view().active_view, asserts it MOVED, and asserts it DIFFERS from the host: the assert_ne! is what pins the premise, since assert_eq!(seen,\"Other\") alone passes if the host moved too. Found by review inside a PR whose whole subject was a scoping distinction -- the characteristic defect appearing in the change that documents it.').
|
||||
work_done('improvise-8tk',bug_fix,'ZZ/:wq quit even when the save failed, silently discarding the model. Save now aborts the effect batch on failure; save_to_current_path/1 returns Result<String,String>; three tests in ui/effect.rs.').
|
||||
work_done('improvise-907',bug_fix,'Import wizard step machine redesign: validation_error() gate at ReviewProposals (no-category and over-limit), history-stack previous() bound to Left on every step, and Done made a real confirmation step holding the built workbook. Superseded improvise-gfq, improvise-o1c, improvise-c6j.').
|
||||
work_done('improvise-6sq',wont_do,'Merging EditOrDrill and EnterEditAtCursorCmd into one EnterEdit{mode} is not safe: they differ in WHEN the cell value is read, not in a mode parameter. Added ui/app.rs::tests::i_and_o_diverge_on_an_empty_records_grid as executable evidence.').
|
||||
@@ -344,6 +399,26 @@ work_done('improvise-2f97',feature,'kb_error(dangling_citation/3): a name cited
|
||||
work_done('improvise-dj1l',bug_fix,'scripts/kb-merge.pl matched clause keys by unification, which lost and corrupted clauses: two variant-headed rules in one delta went two in, one out (536 before, 2 in delta, 537 after), and a non-ground rule over a predicate holding facts deleted one AND was written with its variable bound. See invariant(key_matching_is_variant_equality_never_unification). Five regression cases added to scripts/kb-test.sh, including one that injects a dropping fold to prove check_total fires; scripts/kb gained a KB_MERGE seam so a broken merge can be injected.').
|
||||
work_done('pr82-fingerprint-guards-2026-09-20',test,'PR #82, follow-up to #76. Added assert!(app.view().prune_empty) to selecting_the_page_already_selected_invalidates_no_cursor (paged_model(false) plus deleted moved-conjunct had left it GREEN); moved its page-selection precondition ahead of the effect and onto App::view(); corrected the test-evidence entry for every_layout_geometry_effect_is_visible_to_the_narrow_fingerprint, whose stated mechanism measurement contradicts, and recorded which two of its four prepare steps are load-bearing. 1425 passing, unchanged. Key is a dated slug and not an improvise- id, because bd issued none: an ID-shaped non-ID is worse than a dead id, the diagnosis being identical and the repair not.').
|
||||
work_done('pr90-io-gp-correctness-2026-09-21',fix,'PR #90, wave K. improvise-iot6: render_csv built GridLayout::new, which skips prune_empty, so :export and the CLI export path wrote rows AND columns the grid does not draw -- FOURTH instance of the trap and the first a user could see; fixed with with_frozen_records(m, v, None), the PR #78 grid_in fix line for line. A sweep of the remaining GridLayout::new call sites found all four are test code, so this was the last production instance. improvise-zv8t: fit_measure handed preprocess an Overrides::default(), so a numeric- or date-valued document category was inferred rather than forced categorical and the fit refused as an encoding artefact. Two self-review rounds and one adversarial round on the branch; the adversarial round found F2 (forcing by name, budgeting by index), which this PR itself introduced. 1458 -> 1464 passing.').
|
||||
work_done('improvise-oujb',feature,'Every release now carries improvise-web.tar.gz (the assembled landing page + browser app, one improvise-web/ directory, ~1.6 MB) via [[dist.extra-artifacts]] running scripts/build-web-release.sh. web/index.html gained a build-version span, stamped by web/assemble.sh with the same token as the bundles and verified by the same scripts/assemble-stamp.sh gate (fault-injected: assembly exits 1). The wasm-pack and node pins moved to scripts/web-toolchain-versions.env, read by the ci.yml build job via GITHUB_ENV and sourced by the release script. NOT verified: a real tag push -- and note dist SKIPS build-global-artifacts on a PR run, so CI green on the PR is no evidence about the capture; the substitute evidence is a local dist build --artifacts=global, which did produce the artifact in target/distrib. Also unrun: the Linux toolchain branch of the script.').
|
||||
work_done('improvise-nn4s',docs,'ADR 004 drafted (context/adr/004-faces-and-themes.md), PROPOSED not accepted -- PR #122, held open because adopting a theming architecture is the user decision. Faces name roles; a theme binds them and is client-local (not a Command, not in ViewState, not projected, not in .improv); inheritance resolves once in improvise-protocol; five portable attributes (fg, bg, weight, slant, underline) and an unrenderable one is ignored. Colour is palette-relative (Default|Named|Indexed) or absolute (Rgb), and the SHIPPED theme is never absolute. Migration needs no flag day because the default theme reproduces today rendering exactly, making each call-site move a TestBackend-verifiable no-op. Format deferred on a stated condition: improvise has no configuration file of any kind, so the first theme file would be the first config file -- a larger decision than theming.').
|
||||
work_done('improvise-8l0c',bugfix,'formula_panel.rs cut by bytes and subtracted 1 from an unchecked inner.width. REACHABILITY IS WIDER THAN THE TICKET: draw.rs:297 lays the panel out as [Min(40),Length(side_w)] so a terminal at or below ~42 columns gives it width ZERO, and Frame::render_widget has no empty-area guard -- so the crash needed no Unicode at all and resizing was the common path. grid.rs display-width truncate moved to src/ui/text.rs and shared; prefix_by_width replaced a chars().take tail that over-ran by a cell at width 1. Searching for the CLASS rather than the symptom found two more sites placing a string at inner.x + name.len(). Two rival fixes injected and watched fail: a char-count truncator over-runs on wide characters, and repairing only the subtraction still panics at the boundary and at width 1 -- a test written only at width 0 would have accepted it. Filed improvise-n8vh. PR #124, merge e8f66e6.').
|
||||
work_done('improvise-rw6l',feature,'PR #123, merge 7470b61. Page bar chips became buttons carrying a down-arrow; clicking opens a menu of the axis items and choosing one composes SetPageSelection via a new Click::PageItem (indices, like every existing Click variant, so the page never spells a name). No wire change. The menu is the third keyboard-owning state of the kind editing already was. Codex found a real bug -- the menu branch sat above the metaKey guard, blocking Cmd-R, and Ctrl-j moved the menu two lines below a comment saying Ctrl/Alt are the browser\`s -- fixed by ordering plus one bail, and verified on the merged branch by the orchestrator. Shipped with improvise-771e (browser tile bar reads plain_label) and improvise-wiew (document_keymap).').
|
||||
work_done('improvise-e0ex',feature,'PR #125, merge 5fdce21. View::new seeds \",.2\"; a FORMAT clause in formula.pest overrides per formula target; FormatPlan in improvise-core replaces the (comma,decimals) pair in every render path; GridLayout::column_key resolves a total format from its own column; web/demo.improv moved to \",.2\". improv.pest untouched. Two Codex findings fixed, both real. Injecting a walker that discards the format clause fails ONE test and passes 242 others in improvise-io -- the measurement that shows raw cannot be the witness. Two fixtures pinned to \",.0\" rather than re-baselined, one of which would have gone VACUOUS WHILE PASSING under the new default. No .improv data line changed. 1654 tests, 37 binaries.').
|
||||
work_done('improvise-55l5',bugfix,'PR #126, merge 4625593. dispatch None now means no binding matched and nothing else; a construction failure returns a status. Two rivals injected, each failing exactly one of the paired tests.').
|
||||
work_done('improvise-o9uq',bugfix,'PR #126, merge 4625593. add-record-row places its own cursor after the SetCell; target_row handles the already-exists arm. Reproduced through App::handle_key rather than a hand-placed state: o N o r t h Enter deleted East Revenue cell and renamed East to EastNorth. Not blocked by and does not block improvise-fdk1.').
|
||||
work_done('improvise-n6t8',feature,'PR #127, merge d46ec59. Every header child that changes with a keystroke is reserved, with #hint the single elastic child; ViewState::status_msg moved to a toast in a fixed #bottom-dock anchored at its bottom so it stacks ABOVE the minibuffer, whose box is byte-identical with and without one. harness/tests/header.spec.js measures the app bar bounding boxes across the NINE modes a browser can reach plus a status message, asserting #mode before each measurement so a key that failed to reach its mode cannot make the comparison vacuous. Three faults injected, all caught. Layout leaves gotcha 48 list of what the harness cannot see; prefers-color-scheme joins it.').
|
||||
work_done('improvise-bd9o',bugfix,'PR #128, merge 76d909e. Eleven lines: an entrance guard on state.grid.records in edit_or_drill, before cell_at_cursor, plus CellId::Create declining out loud instead of returning None. Reproduced at HEAD -- improvise-55l5 fixed none of it, because Host::handle answers ClientFrame::Key with an empty vec so the key never reaches dispatch. The silence had TWO sources and the ticket predicted one: the coordinate column carries no CellId at all, which is improvise-fdk1 seen from the key path. Four rivals injected; two (mode.is_records in either place) passed the whole workspace at 1659 and are now caught by a deliberately DRIFTED client. Filed improvise-x0xd.').
|
||||
work_done('improvise-dy3j',bugfix,'PR #129, merge bf83294. A Label category is persisted as a \"kind: label\" entry inside its ## Category: section; grammar first, then format.rs and the parse.rs walker. Closes a P1 where regular + label > 12 was PERMANENTLY UNLOADABLE, because every label reloaded as Regular and the 13th was refused. Second worked instance of doctrine(a_roundtrip_equality_is_blind_to_a_loss_both_sides_share), whose text cites only improvise-349m: the kind was dropped on BOTH sides, so the whole characterization suite was silent. The discriminating rival is a reader that parses the kind and discards it -- the file text is then correct and every output-comparing test passes.').
|
||||
work_done('improvise-cj4u',bugfix,'PR #129, merge bf83294. View::on_category_added takes a CategoryKind beside the name and both Workbook set_axis corrections are deleted. An injected blanket unseated=true satisfies EVERY label assertion in the repository, so the discriminating controls are that a regular category still takes Row and that a label does not consume the slot the next one wants.').
|
||||
work_done('improvise-vw47',bugfix,'PR #130, merge 69c873d. A new view takes the first UNUSED \"View <n>\" rather than views.len() + 1; view::unused_view_name in improvise-core is the one derivation and both resolvers call it. Two rivals injected: fixing panel.rs alone fails both_ends on the document text (host writes View 5 with View 4 format intact, client replaces View 4 and resets it), and starting the search at 1 fails the pre-existing incrementally-names test. Baseline measured at 1667 and final 1670 -- exactly the three tests added.').
|
||||
work_done('improvise-q914',bugfix,'PR #131, merge 9d2bdd3, closing improvise-j6x8 as the same fix. improvise cmd and script now print every non-empty status_msg to stderr, clear it per command, exit 1 on a parse error or an aborted batch, and write the document back only if some command raised dirty (accumulated per command). Three clap doc strings stopped advertising JSON. The injected rival -- print on success, stay silent on refusal -- fails 5 of 12 CLI tests, and a test asking only \"was anything printed\" would pass it. tests/headless_cli.rs is the contract at the process boundary, which exists nowhere below std::process::exit.').
|
||||
work_done('improvise-jz6w',feature,'PR #134, merge a799e24. KeyOutcome::Request(Request::Save) lets a resolver say \"ask the host\"; Ctrl-S bound through it and removed from the_command_names_that_still_need_the_host_are_these in the same commit. End-to-end guard from a real Client through a real Host to MemoryFs. Three rivals injected and each watched fail. Consequence found and filed rather than patched: Ctrl-S now saves AND opens the browser save dialog, because app.js preventDefaults only unmodified keys -- pre-existing behaviour, newly visible (improvise-clso).').
|
||||
work_done('improvise-xgra',tooling,'PR #133, merge 22ab58f. scripts/pr-reviews reads review bodies, INLINE comments and issue comments, interleaved in time order; scripts/pr-reviews-test.sh is a fixture-driven check_the_checker with 22 cases in CI kb job. Six faults injected, each caught by the case that names it. Two bugs were found by Codex on the PR and were bash-5-only, invisible on the macOS shell that wrote them. The agent also walked into this PR own trap while building it: pr-watch > f 2>&1; echo EXIT=$? reported 0 over a FAILING kb job, because the echo took the status.').
|
||||
work_done('improvise-0ygd',bugfix,'PR #135, merge 42ec30d, TUI half only. AdvanceDir::Right judged at_bottom_right from the pre-commit col_count, so Tab on +measure ran AddRecordRow and jumped to (row+1,0) although the commit had just inserted a column at the cursor -- the measure just named left unreachable. Enter escaped only because EnterAdvance aborts at bottom-right and the SetSelected restore at the head of the batch held the cursor. The browser half is worse (the whole command vector is discarded through Upstream) and is UNREACHABLE at this head, because improvise-bd9o declines at the entrance of edit_or_drill -- so no browser regression test can be written before improvise-x0xd.').
|
||||
work_done('improvise-nxah',bugfix,'PR #136, merge eefd992. The client rolls back its optimistic fold on ServerFrame::Rejected; ServerFrame::Accepted added so an answer addresses a known frame. Five rivals injected, each failing exactly the check aimed at it -- including the single-snapshot design, caught by refusing_one_of_two_frames_in_flight_keeps_the_other. The repro compares against the HOST OWN MIRROR rather than a second client derivation (improvise-vw47 trap) and is total over ViewState rather than a hand-kept field list, asserting the two ends agree BEFORE the refusal so it cannot pass by never having disagreed.').
|
||||
work_done('improvise-tg27',bugfix,'PR #138, merge 327a76b. header flex-wrap over a breakpoint; main min-width:0; panels capped at 40vw; harness/tests/narrow.spec.js asserts documentElement.scrollWidth <= clientWidth at 360/520/768, so the page own overflow leaves gotcha 48 list of what the harness cannot see. Fault injected by restoring style.css to 22ab58f: 6 of 9 assertions failed, and the two that passed are the two that should. Merge with improvise-nxah verified STRUCTURALLY -- 198 insertions and ZERO deletions across context/, with evidence entries rising 812 to 818 as the independent witness. Found not predicted: the keyboard already reached every clipped column, so what was broken was a FALSE scroll affordance beside a working one.').
|
||||
work_done('improvise-k844',bugfix,'PR #139, merge 1ae2d2e. importance_ranking sums and divides in f64, narrowing only the bounded share. The injected rival is THE TICKET OWN TITLE -- reassociating to 100.0*(imp/total) in f32 passes the lone-importance and sums-to-100 tests and fails the twin and the guard, 58 passed 2 failed, converting NaN% into a serene 0% for every feature. Scoring bit patterns byte-identical, which is the evidence the fix is confined. context/code/gp.md section 7 corrected in place: it had deferred this bug on a cost that does not exist.').
|
||||
work_done('improvise-f9qf',tooling,'PR #137, merge a9cad15. scripts/checkers.sh plus scripts/checkers-test.sh (12 cases, each INJECTING rather than asserting). The proof: restoring the multi-assignment local gives 22 passed under the old local gate, 22 passed under direnv, and 2 passed / 20 failed / exit 1 under checkers.sh. The second undetected split is closed -- the four #!/bin/sh and one #!/usr/bin/env sh scripts are now bash, a strict superset needing no porting. Unrun and stated: scripts/build-web-release.sh executes only under dist in build-global-artifacts, which no branch push exercises.').
|
||||
work_followup('improvise-8tk','improvise-lae','no-file-path UX: prompt for a path rather than only erroring. ExportPrompt is CSV-only, so a new SavePrompt TextEntry mode committing to SaveAs is the likely shape.').
|
||||
work_followup('improvise-6sq','improvise-ct5','i on an empty records grid reports \'cannot drill\', leaking a pivot-mode concept into records mode').
|
||||
work_followup('improvise-h0b','improvise-a2x','ViewState field types (AppMode, MinibufferConfig, ViewFrame, KeyCode, KeyModifiers) still derive no Serialize, and improvise-command depends only on serde_json. Deferred: nothing serializes ViewState until improvise-cqi builds a Snapshot, so the derives would have no consumer.').
|
||||
@@ -423,6 +498,27 @@ decision('improvise-ni6k',cursor_invalidation_is_observed_not_declared,'No effec
|
||||
decision('improvise-864a',pages_artifact_reaped_by_a_foreign_successful_deploy,'Measured over 100 push-to-main runs 2026-09-19..2026-09-21: 12 of 12 artifact-not-found deploy failures had ANOTHER run SUCCESSFUL Pages deployment complete inside their own upload-to-deploy window, against 2 of 56 successes. Direction proved by the oldest run of the 17:47Z batch failing BEFORE the next deploy started and before any deployment existed, since deploy-pages fetches artifact metadata before creating one. NOT queue depth, NOT supersession, NOT retention, NOT the action deleting it. PR #46 tip-of-main guard aimed at the right quantity for the WRONG REASON and still must not ship: the association is not a mechanism, and 5f871f7 survived a foreign deploy inside its own window. Reproduce deliberately before fixing.').
|
||||
decision('improvise-zv8t',a_document_category_is_never_inferred,'fit_measure now builds its Overrides from the same header[..category_columns] slice it already handed Features::of, forcing each to Forced::Categorical, so the one place the document own answer was known is the only place either caller reads it from. Closes Artefact::NumericCategory and DatePart-over-a-category from the only production entry point. The Artefact arms STAY: crate::dataset is pub and takes the caller Overrides, so a caller that builds one without forcing still produces a Numeric role over a category column -- deleting the arm would derive the classification from one caller habits rather than from the roles. Cost accepted and filed as improvise-e8no: a wide numeric category now one-hots past MAX_FEATURES and refuses a fit that used to ignore it. Do NOT close e8no by making the forcing conditional on cardinality -- that is inference by another name, and makes the ENCODING depend on a budget rather than on what the document says.').
|
||||
decision('improvise-e5pf',the_mode_settles_where_the_grid_does,'resolve::changed_mode is DELETED. Every composition site emits the plain AppMode::Normal, and ClientState::fold settles against the grid it HOLDS via improvise_protocol::settled_mode -- one rule, also called by ClientSession::fold and App::settle_normal_mode, with App::normal_mode deleted as its host-side duplicate. Composition-time settling was PREDICTION: switch_view_at composes [SwitchView(target), ChangeMode(Normal)] and read the OUTGOING grid. Read-time derivation was rejected for having the same window and for moving the enumeration from composition sites to READERS, where a missed reader is silent. settled_mode settles the Normal pair ONLY -- Editing/RecordsEditing pass through -- which is why a records decision must ask the grid and never the mode. Same shape the repo already prefers: cursor_epoch over matching on CycleAxis, document_generation over matching on LoadModel, the dirty flag over a table of command variants.').
|
||||
decision('improvise-oujb',pages_tracks_main_release_captures_a_copy,'User ruling 2026-09-22: \"I like Pages showing the latest main for now.\" So pages-deploy is unchanged -- still push to refs/heads/main -- and a tag CAPTURES the assembled site as improvise-web.tar.gz rather than deploying one. The artifact is a self-hostable copy and a rollback target, NOT what a visitor runs; README says so, because a reader who assumes otherwise is wrong in a way that costs an afternoon. The divergence between a binary user and a site visitor is now deliberate, which is what makes the build-version stamp load-bearing rather than cosmetic. \"For now\" is recorded deliberately: a current preference, not a permanent architecture decision.').
|
||||
decision('improvise-oujb',extra_artifacts_not_a_hand_edit,'release.yml is autogenerated by cargo-dist and a hand edit is clobbered by the next dist init, so the web capture is [[dist.extra-artifacts]] in dist-workspace.toml. Verified against dist 0.30.4 -- the pinned version, downloaded, because dist REFUSES to run at all on a cargo-dist-version mismatch so a 0.32.0 test would have proved nothing: dist plan lists improvise-web.tar.gz and dist generate leaves release.yml byte-identical. The build command calls web/build.sh and web/assemble.sh rather than assembling its own way, which is what the ci.yml build job comment exists to protect.').
|
||||
decision('improvise-n6t8',reserve_space_do_not_fit_it,'User ruling 2026-09-22: \"As a general rule, there shouldnt be motion of layout elements I expect to be fixed.\" So an element whose CONTENT varies while its POSITION is read as fixed must have its space RESERVED rather than fitted to the text. In the browser header that covers the mode pill (AppMode::label -- NORMAL, CATEGORIES, FORMULA all differ in width, so every mode change shifts everything after it), the hint line (one per mode, reflowing on the same event and compounding it), the status line, the document title and the view chip. It is also the reason a transient message must be a toast that is OUT OF FLOW rather than merely short: shortening reduces the motion and does not remove it. The fragile fix is a hardcoded min-width sized by eye to todays longest label, which silently resumes the jitter the day a longer AppMode variant is added -- the compiler forces label() to be filled in for a new variant and nothing checks that the CSS still fits it, so the guard must be a rendered-layout assertion that walks the modes, with a too-long label injected to prove it discriminates.').
|
||||
decision('improvise-pv3g',guard_the_derived_quantity_not_the_inputs,'A finite f32 target can still overflow the spread: standardize sums (t-mean)^2 in f32, so eight rows at a 1.5e20 spread give 1.8e40, past f32::MAX, and std becomes inf -- after which denormalize computes 0.0*inf + mean = NaN, surfacing two crates away in improvise-gp number_text. fit_measure checked every INPUT and saw nothing, because every input was finite. The fix makes the poisoned map unrepresentable rather than validated: Scaling has private fields and one constructor refusing a non-finite part, standardize returns Option, and cv_regress pools nothing for a fold it cannot standardize -- which closes the SILENT half, where the same overflow corrupted R-squared with no panic to announce it. Note the crashing spread SHRINKS as records are added, so guarding the inputs is guarding the wrong side.').
|
||||
decision('improvise-x0dc',panel_open_is_not_panel_focus,'RECOMMEND DECLINING as written, pending a user ruling; left open. Esc from a panel mode leaves the panel OPEN, which reproduces, but is deliberate: category_panel_open starts TRUE in Normal mode, so an open panel in Normal is the state the app BOOTS IN -- if that were the bug, the app would boot in the bug. Three independent corroborations: F/C/V are toggle-panel-and-focus while Ctrl-F/C/V are a separate toggle-panel-visibility; Tab in Normal is cycle-panel-focus, meaningless unless panels stay open unfocused; and the panel hint spells enter-mode normal as \"back\", not \"close\". The genuine defect its own evidence points at is improvise-df2w: render.rs formula_rows returns an empty vec when closed and app.js paintPanel hides on rows.length === 0, so an OPEN BUT EMPTY panel is invisible in the browser while the TUI draws its empty message -- two answers to \"is this panel visible\".').
|
||||
decision('improvise-wiew',keymap_layer_named_for_its_subject,'document_keymap sits between global and Normal / the three panels / TileSelect; RecordsNormal reaches it THROUGH Normal; Help, ImportWizard and the text-entry modes take global directly. The orchestrator framed the split as \"is the document the subject right now\", which is a judgement; the implementing agent found a STRUCTURAL line instead: every key the global binds is a UI action, whereas page-next resolves to SetPageSelection, which is EffectKind::Model(LayoutGeometry) -- the first shared keys that MUTATE. Under the help overlay that is an invisible change to the grid it covers, so the rule is \"may a fallthrough key mutate the document\" rather than taste. The NAME is the only guard on membership, since nothing in the code stops a mode parenting the layer: document_keymap, never common or shared. The layer owes its own Char-only constraint for the identical fallthrough reason as the global, written down BECAUSE no descendant binds AnyChar today.').
|
||||
decision('improvise-e0ex',decimals_by_default,'View::new seeds \",.2\". A ratio under \",.0\" draws as 1, and a format cannot be guessed from a value, so noise on whole numbers (18,400.00) is the cheaper error. The override is spelled IN THE FORMULA per the user ruling -- a formula target IS a _Measure item (gotcha 7), so per-formula and per-measure differ only for a measure no formula owns, and that is raw data whose format is the view job. improv.pest is UNCHANGED: formula_raw already carries the line verbatim, so format_clause lives in formula.pest and the grammar-walking generator sees no new spelling. sp+ and not a lookahead, for WHERE reason -- the generator skips predicates.').
|
||||
decision('improvise-o9uq',order_is_the_fix,'add-record-row appends viewport_effects(target_row,0,..) AFTER the SetCell. The order is the entire fix: the same two effects the other way round are the bug verbatim, because the frame observation inside apply_effects invalidates a cursor placed before the append. The acceptance criteria said \"append viewport_effects\" and did not say where, and the two orders are indistinguishable by reading -- so the injected rival is the SWAP, not the omission. target_row is row_count() unless the record already exists, in which case it is that record index: predicting row_count() unconditionally puts the cursor one past the end in that arm.').
|
||||
decision('improvise-n6t8',two_status_sinks_rather_than_one_composed_string,'The transport fact and the document fact go to two ELEMENTS, not to one string joined with a separator. composeStatus is deleted. It had kept gotcha 46 by CONVENTION -- a repaint that wrote the whole string could erase a connection error, and the join was what stopped it; two sinks make it STRUCTURAL, because a repaint reaches createShell onModel and cannot address onShell at all. The join unit test was REPLACED and not dropped (\"a repaint model status cannot reach the transport sink at all\"): a separation with no guard is how one regresses into a single sink silently.').
|
||||
decision('improvise-dy3j',absence_is_the_compatible_reading_of_a_new_format_line,'kind_kw spells only \"label\" and format_md emits the line only for a CategoryKind::Label, so every file written before the line existed parses unchanged and every pivot category section is byte-identical -- no migration and no moved round-trip expectation. parse_kind_kw is an exhaustive Result-returning match rather than a bool read off the rule PRESENCE, so a future second keyword fails at the walker instead of being read as \"label\" because it arrived on a kind_line.').
|
||||
decision('improvise-vw47',unused_name_helper_not_a_create_view_refusal,'Workbook::create_view REPLACES on purpose and must not be touched. Verified: it has exactly two production callers -- ui/effect.rs:310 inside effect::CreateView, and persistence/parse.rs:419 guarded by contains_key so it cannot observe replacement at all. The _Records/_Drill rebuilders are NOT callers; they reach it through effect::CreateView, emitting a fixed name on every entry, which is the SAME effect the user-facing path uses -- so the effect cannot distinguish them and the name must be fixed at the RESOLVER. Refusing or renaming inside create_view turns \"a view was silently reset\" into \"the key does nothing\", a worse bug wearing a better error message. view::unused_view_name takes NAMES rather than a Workbook because the browser holds only ViewListCache, and project::view_summaries maps workbook.views one-for-one including virtual views, so both ends see the same list.').
|
||||
decision('improvise-z05h',the_registry_names_the_refusal_not_the_validator,'require_args drops its \`word\` parameter and CmdRegistry::parse prefixes \"{typed}: \", so a refusal names what the USER TYPED rather than what the validator was called about. That deletes 41 command-name literals from cmd/ -- the same argument gotcha 15 makes about registry.rs, where every register* call reads its name off a prototype so a typo is impossible. \`interactive\` deliberately does NOT prefix, because dispatch already does since PR #126 and both would double it.').
|
||||
decision('improvise-jz6w',key_outcome_request_is_narrow,'KeyOutcome::Request carries a narrow Request enum (Request::Save, with Request::frame the one mapping to an envelope) and NOT a ClientFrame. Two reasons, the second serious: an arbitrary frame lets a resolver compose a Viewport, whose extent is the PAGE s fact and reaches no derivation from a keymap; and it lets a resolver compose a Commands batch, which is INDISTINGUISHABLE ON THE WIRE FROM A Local while skipping the local fold Local s contract requires -- a desync with nothing to observe it. Guarded rather than asserted: \`requested\` is checked first in step so no name sits in two lists, and a request cannot travel inside a Binding::Sequence (a sequence is one frame), with no_binding_puts_a_request_inside_a_sequence keeping that refusal arm unreachable rather than untested. Not a Command: gotcha 45 reason holds.').
|
||||
decision('improvise-xgra',an_inline_comment_at_the_head_satisfies_the_wait,'A pulls/{n}/comments entry carries a commit_id, so it is a claim about WHICH TREE WAS READ, and it is exactly the case that failed on PR #122. The clincher is that GitHub \"Add single comment\" CREATES A REVIEW WHOSE BODY IS EMPTY -- under the old rule that reviewer exit code was right while their words were invisible, so the surfaces and the wait had to move together or the tool would be honest and useless. An ISSUE-LEVEL comment does NOT satisfy it: no commit, so no claim about a tree, and \"I am rebasing shortly\" satisfying a wait for a review is the widening that looks like the fix. There is a fixture and an injected fault for exactly that.').
|
||||
decision('improvise-0ygd',split_on_the_buffer_not_the_column,'In records mode +measure is ALWAYS the rightmost column, so cursor.col >= col_max names that one cell and nothing else -- which means the entire \"Tab at the bottom-right starts the next record\" bulk-entry contract is reachable ONLY from that cell with an EMPTY buffer. A guard keyed on the column alone fixes improvise-0ygd and SILENTLY DELETES BULK ENTRY. The injected rival proves it: dropping the value-awareness passes the new test and fails tab_on_an_empty_new_measure_column_still_starts_a_record_row. When one cell is the only member of a class, a guard on the class is a guard on the cell.').
|
||||
decision('improvise-nxah',a_refused_frame_is_rolled_back_by_the_fold_not_a_cache,'What rolls back on ServerFrame::Rejected is ViewState -- the thing the fold produces -- and never a cache, because a Command::Model is A NO-OP IN reduce_view: the optimistic fold never touches a cache at all, and page_coords moves only under Projection::Grid. That single fact refutes the bug report own evidence (which blamed page_coords) and dissolves the wave recorded contradiction between nxah and improvise-3al1 -- the staged typing 3al1 needs rides in SetBuffer frames, which carry no model command, are never refused, and have already settled, so rolling back the COMMIT restores the buffer TO THE TYPING. Only a frame carrying a Command::Model is rejectable, stated exhaustively by Checked::against.').
|
||||
decision('improvise-tg27',a_reserve_is_a_floor_and_must_reflow_not_widen,'header{flex-wrap:wrap} over a @media breakpoint: a flex item wraps exactly when the line cannot hold it, so the desktop bar is untouched BY CONSTRUCTION rather than by a chosen number -- no magic width to drift. Plus main{min-width:0}, 40vw caps on the side panels, and max-width caps so a reserve cannot exceed its own line. The reflow threshold is deliberately asserted NOWHERE, because #status composes the DOCUMENT NAME: the same 1024px window is one line under a short name and two under a long one. Only 1280 is pinned, being where improvise-n6t8 stability spec runs.').
|
||||
decision('improvise-f9qf',one_checker_entry_point,'scripts/checkers.sh is the ONE entry point for every check_the_checker, called verbatim by CI and by the local gate so there is no second invocation to drift. It pins a bash >= 5 (ambient first, nix only when the ambient one is older, exit 3 when neither), SCRUBS THE ENVIRONMENT to an allowlist (env -i plus PATH/HOME/TMPDIR/NIX_*/GITHUB_*, extensible via CHECKERS_KEEP), and refuses any shell shebang in scripts/ that is not #!/usr/bin/env bash. The pinned bash goes on the FRONT of PATH so scripts the suite invokes get it too -- which is where the fault actually lived. CI logs \"checkers: bash 5.2 at /usr/bin/bash\": ambient, no nix, zero startup.').
|
||||
decision('improvise-f9qf',nix_shebang_declined_on_availability_not_speed,'Measured warm, 20 invocations: ambient bash -c true 5.1ms; nix shell nixpkgs#bash --command 61.8ms; nix-shell -p bash --run, THE MANPAGE FORM THE USER POINTED AT, 675.2ms -- 132x, and that inside git merge machinery for beads-merge-driver.sh. But the DECIDER is availability: nix is installed on ONLY the kb job. build, test, wasm, js, harness-lint, browser and pages-deploy have none, so a nix shebang needs install-nix-action added to three more jobs, paid every run, to reach a bash those runners already ship. zsh declined too -- not a shebang change (word splitting, 1-based arrays, globbing), sixteen scripts to read, and no zsh on the runner either.').
|
||||
decision('improvise-6uva',script_replay_lost_its_context_at_35946af,'improvise script and improvise cmd resolve every command through CmdRegistry::parse -- the PURE arm, which builds CursorState::default() (0,0,0,0) -- never the interactive arm that builds CursorState::from_ctx. So every Move command and enter-advance snapshots a ZEROED cursor AT PARSE TIME, clamps to (0,0) and emits SetSelected(0,0): worse than a no-op, it RESETS the cursor, and no cursor-reading command is reachable from a script. Broken by 35946af (2026-04-04), a performance refactor introducing the register/register_pure/register_nullary split, FIVE MONTHS BEFORE the first wasm-client commit -- so the web refactor is innocent. Both directions demonstrated: at the parent 649d80c a move-then-clear cleared row 2 and a bare clear cleared row 0; at HEAD both clear row 0. THE SHAPE: a registry with two construction arms silently demotes every caller of the weaker arm, and nothing type-checks which arm a caller deserves.').
|
||||
decision('improvise-gd28',browser_drill_blocked_by_surrogate_scope_not_layer_2,'The ticket recorded drill as blocked on ADR 001 layer 2 because ApplyAndClearDrill is session-local and unaddressable. That reasoning is about the COMMIT path. Entering a drill needs no commit, and CreateView/SetAxis/SetPageSelection are already unaddressed ModelCommands that NAME THEIR VIEW. What a browser cannot supply is the OPERAND -- CellId::Aggregate(AggregateHash), an opaque u128, which is ADR 002 working as designed. The real gap is one level down from where it was looked for: THE MUTATION AND ITS OPERAND HAVE DIFFERENT SCOPES. The mutation configures the shared _Drill and is addressed to nobody; the operand exists only inside one session layout. So the fix is not a new mechanism but the view:String convention every other view-scoped variant already follows, resolved against a layout derived for that name -- no SessionId needed. fdk1 is also not needed: edit_or_drill declines records-mode editing OUT LOUD before reaching a cell, so a browser cannot edit inside a drill grid at all and no divergent staging scheme is reachable.').
|
||||
wire_carries(command,both_directions,'user intent up, projection commands down').
|
||||
wire_carries(snapshot,on_connect_only,'improvise-q08: full ViewState + render cache filled from current viewport').
|
||||
wire_carries(command,both_directions,'user intent client->server, projections server->client').
|
||||
@@ -565,6 +661,22 @@ kb_issue_found(ticket_premise_false_8uz,'improvise-8uz asserted selected/row_off
|
||||
kb_issue_found(ticket_premise_false_rml,'improvise-rml called category_panel.rs an \"exact copy\" of tile_bar.rs. It was not: the colors were identical but the labels differed (\"Row ↕\" vs \"Row\", \"None ∅\" vs \"·\"). Only the color table was true duplication; the four label sets are four deliberate vocabularies and were preserved as four methods.').
|
||||
kb_issue_found(duplicated_active_view,'adding ViewState::active_view created a second copy of \"which view is active\". The improvise-8uz characterization test caught the desync immediately because it called workbook.switch_view directly. Held in lockstep by two writers until improvise-jytm, which made the divergence deliberate: the guard test the_two_active_view_names_never_diverge was REPLACED, not deleted, by the_documents_default_view_outlives_this_clients_switches and a_fresh_session_opens_the_documents_default_view -- a deliberate split with no guard is how one silently regresses into the other.').
|
||||
kb_issue_found(copilot_stopped_reviewing_this_repo,'Measured 2026-09-20: PRs #12, #21 and #27 carry 2, 8 and 4 copilot-pull-request-reviewer[bot] reviews; #61, #66, #68 and #70 carry none. Both request paths report success and register nothing -- the GraphQL requestReviews mutation with botIds BOT_kgDOCnlnWA returns clientMutationId null, and POST /pulls/<n>/requested_reviewers with copilot-pull-request-reviewer[bot] returns 200 with requested_reviewers empty -- and no review_requested timeline event appears. Cause unknown; something changed between #27 and #61. Consequence for the process: AGENTS.md said \"merge only once Copilot passes\", which is an instruction to wait forever on a review that was never requested, since an ABSENT review and an UNREQUESTED one look identical from outside. That is doctrine(review_is_requested_not_inherited) seen from the requesters side, and doctrine(an_advisory_gate_is_not_a_gate) with the gate removed rather than never added. AGENTS.md now says review is an adversarial agent the orchestrator dispatches, naming the head SHA, and keeps the Copilot mechanics for the day it returns.').
|
||||
kb_issue_found(github_build_setup_injects_into_the_wrong_job,'dist github-build-setup is documented as running extra steps before dist own build step, and is the obvious way to give a release job a wasm toolchain. Measured on dist 0.30.4 by injecting a named probe step and regenerating: it lands in build-local-artifacts ONLY -- the four-platform matrix job -- and never in build-global-artifacts, which is where a GLOBAL [[dist.extra-artifacts]] is actually built. So it cannot provision anything for a global extra artifact, and the provisioning must live inside the build command itself. The hook reports success and simply lands elsewhere: nothing distinguishes \"injected somewhere useless\" from \"not injected\", which is doctrine(an_empty_actions_log_is_retention_not_history) in its configuration form. NOTE ON CITING: the proposing agent named that doctrine from memory and invented a plausible key for a fact that exists under a different one -- the kb form of the dead-ticket-ID hazard. scripts/kb refused the delta over it, which is the gate doing its job; grep a key before citing it.').
|
||||
kb_issue_found(minibufcolor_roles_are_not_what_they_look_like,'MinibufColor Green/Cyan/Yellow was read by the orchestrator as editing/destructive/search. BOTH HALVES ARE WRONG at the eight MinibufferConfig construction sites. Cyan is the FORMULA prompt; there is no search minibuffer at all, SearchMode being Normal plus a flag (gotcha 14) with the search MATCH drawn Black-on-Yellow in the grid. And green/yellow does not split on editing-vs-prompting: item_add is GREEN while category_add is YELLOW, the same act -- typing a name into the document -- on opposite sides. Consequence for improvise-nn4s: its own cheapest proposed step, renaming the three variants to roles, CANNOT BE DONE HONESTLY, because no three-way split fits eight sites; the honest move is a face per text-entry mode, eight faces with three resolving alike today. The general point: a colour-named enum does not merely fail to be themeable, it actively MISLEADS the next reader about what the roles are -- the orchestrator read roles off the colour names and was wrong twice in one sentence.').
|
||||
kb_issue_found(stale_count_in_tui_md_default_chrome_arm,'context/code/tui.md line 125 says mode_style spells out \"the fourteen variants that take the default chrome\". The arm lists THIRTEEN -- verified by the orchestrator against src/draw.rs: Normal, RecordsNormal, FormulaEdit, FormulaPanel, CategoryPanel, CategoryAdd, ItemAdd, ViewPanel, ImportWizard, ExportPrompt, SavePrompt, Help, Quit. 13 + Editing + RecordsEditing + CommandMode + TileSelect is the 17 AppMode variants, so the arithmetic confirms thirteen. Nothing checks a count written in prose, which is why it survived; the exhaustive match itself is sound and only the sentence about it is wrong.').
|
||||
kb_issue_found(twelve_sites_hardcode_black_ink_on_an_accent,'Twelve TUI sites hand-roll reverse video as fg(Color::Black).bg(<accent>) -- mode_style in src/draw.rs is one, visible in full. Black ink is hardcoded rather than being the terminal background, which is a latent bug on a light terminal: the accent is palette-relative and survives the user theme, the ink does not. Found while deriving the face vocabulary for ADR 004, which raises it as open question 3: whether reverse is a portable attribute (honest for these twelve, but makes resolution stateful for CSS, Swift and Qt) or whether the answer is a named ink face.').
|
||||
kb_issue_found(five_tickets_closed_by_one_unrecorded_commit,'7a44616 (\"fix(panel): five category-panel and axis defects, four of them P1\") closed improvise-uej8, improvise-3b2b, improvise-g3no and improvise-koh6, and ALL FOUR were still OPEN in bd when agents were dispatched at them -- two of them costing a dispatch each. The commit message says \"five defects\" and names none of the ticket ids in a form bd could act on. A commit that closes several tickets should cite every id, because the export is the only thing a later reader greps; see also doctrine(a_ready_ticket_may_already_be_fixed). One misreading compounded it: an earlier agent wrote that a command \"left the host-needed list WITH improvise-koh6\", which reads as context rather than as the close it actually was.').
|
||||
kb_issue_found(the_compounding_header_element_was_status_not_the_mode_pill,'improvise-n6t8 was filed naming the mode pill, following the user report. The spec failed first on the header HEIGHT: #status was flex: 0 1 auto beside a flex: 1 1 auto hint, so a longer hint took the shrink out of #status, which wrapped to two and then three lines -- 52.5px to 71.0px. THE OLD HEADER WAS TWO LINES TALL AT NORMAL WITH NO MESSAGE AT ALL. Reserving the pill alone would have left the bar moving and the PR would have shipped looking correct. Fixed with flex: 1 1 0 on the hint and nowrap plus flex: 0 0 auto on #status. The lesson is about the ORDER of work: writing the measurement first is what found an element neither the reporter nor the ticket named.').
|
||||
kb_issue_found(the_edit_that_created_the_drift_was_the_one_that_added_the_third_place,'improvise-bd9o added a new section to context/code/clients.md, and ChatGPT Codex caught two sibling passages the new section falsified. gotcha 48 says a fact stated in more than one place is changed by a grep and not an edit -- and this is its sharpest form: WRITING A NEW SECTION IS EXACTLY WHEN ITS SIBLINGS STOP BEING SEARCHED FOR, because the author is composing rather than amending and has no edit site to search from. The same session produced two other instances: a gotcha-38 correction whose own arithmetic went stale, and a release document whose tag inventory was wrong within hours.').
|
||||
kb_issue_found(generator_coverage_of_a_new_production_is_checkable_and_unchecked,'context/code/formula-and-io.md says plainly that a new .improv production needs grammar-generator coverage or the proptests quietly stop describing the format -- and nothing measures it. improvise-dy3j PROBED: 14 of 500 generated files carry the new kind: label line and all 14 parse to a CategoryKind::Label, about 7 per 256-case run. Genuine coverage, and free because kind_line is a category_entry alternative. The probe was REMOVED rather than kept, because its rate depends on proptest RNG and as a standing assertion it would fail on a dependency bump for an unrelated reason -- the measurement lives in the argument where it can be re-run. A one-line --probe mode on the existing generator would make this a habit rather than something an agent happened to do.').
|
||||
kb_issue_found(the_bug_lived_underneath_a_test_that_named_it,'create_and_switch_view_names_incrementally has asserted \"the name follows the workbook\" since improvise-aaw, and improvise-vw47 lived underneath it for that whole time: every fixture the test uses leaves len + 1 FREE, so it could not separate \"the count\" from \"the first unused name\". It did earn its keep at the fix -- the rival that starts the search at 1 fails it -- but a test whose fixtures never exercise the distinction its NAME claims is a test that will not notice the distinction breaking. When adding a fixture to an existing test, ask which of the properties in its name the new fixture can actually separate.').
|
||||
kb_issue_found(a_new_context_doc_must_be_added_to_the_doc_site_sections,'scripts/build-doc-site.mjs (improvise-res9) requires every context/**/*.md to be either published or an explicitly named omission, and it is reachable ONLY through web/assemble.sh in CI -- so a new context/ document fails the \`Build all bundles\` job with \"neither published nor a named omission\", a gate that appears in no brief and in no local gate list. ChatGPT Codex found the same thing about four minutes after the push. Worth adding to the standard brief: a new context/ doc needs a SECTIONS entry. Noted in passing: context/code/doc-site.md was itself already stale in two counts (\"the seven context/code/*.md documents\" where twelve exist, and \"501 entries\" where the checker reports 804), both de-numbered rather than re-asserted.').
|
||||
kb_issue_found(a_storage_assertion_cannot_separate_reaching_from_reaching_wrongly,'improvise-jz6w injected Request::Save => ClientFrame::Commands(vec![]) as a rival. The end-to-end test asserts the document comes back off storage -- and an EMPTY BATCH WRITES NOTHING EITHER WAY, so the storage assertion alone cannot separate \"reached the host\" from \"reached the host as the wrong kind of thing\". Asserting the FRAME is the discriminator. The general form: when a test asserts a downstream EFFECT, ask which wrong upstream values produce the same effect, and assert the value where they are not distinguishable by the effect.').
|
||||
kb_issue_found(repo_map_said_clause_four_was_undelivered_for_a_day_after_it_landed,'improvise-yqpd delivered ADR 001 clause 4 on 2026-09-21 -- App::snapshot/restore plus a Sessions clone in Host::mutating_frame, restored by undo_frame -- and context/repo-map.md gotcha 47 went on saying \"clause 4 is not delivered\" afterwards. The orchestrator then COPIED that claim into an agent brief as a live constraint, and the agent caught it by reading the tree. Verified: mutating_frame, undo_frame, App::snapshot and App::restore all exist and improvise-yqpd is closed. Corrected, with the correction recorded in the gotcha itself so the next reader sees that the clause list is a claim to check rather than a fact.').
|
||||
kb_issue_found(a_layout_measured_before_the_status_settles_is_not_the_layout,'improvise-tg27 first claimed its fix made #hint legible, and its own spec ASSERTED THAT AND PASSED -- because it measured immediately after the grid painted, before #status had composed the transport fact: 186px where the settled value is 1px. The spec now waits for #status before measuring anything. The general form: a page that composes a line from two asynchronous facts has an intermediate layout that is real, renderable and WRONG, and a measurement taken there is a measurement of a state no user sees for more than a frame.').
|
||||
kb_issue_found(check_evidence_cannot_see_a_templated_test_name,'scripts/check-comment-runs.py --check-evidence scrapes LITERAL test(\"...\") strings out of *.spec.js, so a Playwright test named by a template literal -- test(\`at ${width}px ...\`) -- is invisible to it and its evidence heading reports as an orphan. improvise-tg27 hit this and restructured to five literally-named tests, each looping widths internally with the width in the assertion message. Parameterise INSIDE one literally-named test rather than looping test().').
|
||||
kb_issue_found(k844_description_was_verbatim_e8no_body,'improvise-k844 stored description was a verbatim copy of improvise-e8no body -- the wide-numeric-category budget -- contradicting its own title. Confirmed by diffing the two DESCRIPTION blocks: identical. The agent worked the TITLE, which was right, and the defect had one good consequence: reading e8no text while working k844 is what made it check whether e8no was done, and e8no turned out to be STALE-OPEN with both its guard tests passing. A ticket carrying the wrong body is worse than an empty one, because an agent that trusts it works the wrong problem while believing it has a specification.').
|
||||
kb_issue_found(the_old_checker_gate_overrode_every_shebang,'The gate in the orchestrator brief -- \`for t in scripts/*-test.sh; do sh \"$t\"; done\` -- OVERRIDES EVERY SHEBANG with /bin/sh, so on macOS it ran every suite under bash 3.2 in sh-mode regardless of what the file declared. The gate was the disease it was meant to catch. Seven further sites ran a repo script through an explicit \`sh\`, and ONE OF THEM IS dist-workspace.toml, which a first-line shebang gate STRUCTURALLY CANNOT SEE: the shebang rule closes DECLARATIONS and never INVOCATIONS, and that one was found by grepping for the CALL after fixing the declaration, not by the gate going green. Also: scripts/node-test.sh matches *-test.sh and is a test RUNNER, so a naive glob ran it with no arguments and it printed its usage INSIDE A GREEN GATE -- the rule is now that a suite is X-test.sh where X or X.sh exists beside it.').
|
||||
functional(invariant/2).
|
||||
functional(location/3).
|
||||
functional(doctrine/2).
|
||||
@@ -673,3 +785,4 @@ citation_resolves(decision,A):-decision(B,A,C).
|
||||
citation_body(A,B):-sub_atom(A,C,1,D,')'),!,C>0,C<200,sub_atom(A,0,C,E,B).
|
||||
trim_left(A,B):-sub_atom(A,0,1,C,' ')->sub_atom(A,1,D,0,E),trim_left(E,B);B=A.
|
||||
decision_name(A,B):-citation_body(A,C),sub_atom(C,D,1,E,','),!,F is D+1,sub_atom(C,F,G,0,H),trim_left(H,B).
|
||||
work_done('improvise-gd28','Virtual-view shape extracted to improvise-core as ViewStep/records_spine/records_steps/drill_steps; cmd/grid.rs keeps only steps_into_active_view, twenty lines of effect translation. No test expectation moved and none was added -- the existing drill tests (Filter placement asserted on the effect Debug, per-view cursor, formula drill) already pinned it. PR 140.').
|
||||
|
||||
+198
-5
@@ -24,11 +24,13 @@ use crate::ui::grid::GridWidget;
|
||||
use crate::ui::help::HelpWidget;
|
||||
use crate::ui::import_wizard_ui::ImportWizardWidget;
|
||||
use crate::ui::panel::Panel;
|
||||
use crate::ui::text::truncate;
|
||||
use crate::ui::tile_bar::TileBar;
|
||||
use crate::ui::view_panel::ViewContent;
|
||||
use crate::ui::which_key::WhichKeyWidget;
|
||||
use crate::view::AxisColor;
|
||||
use improvise_command::ui::project;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
struct TuiContext<'a> {
|
||||
terminal: Terminal<CrosstermBackend<&'a mut Stdout>>,
|
||||
@@ -169,8 +171,29 @@ fn minibuf_fg(color: MinibufColor) -> Color {
|
||||
}
|
||||
}
|
||||
|
||||
fn fill_line(left: String, right: &str, width: u16) -> String {
|
||||
let pad = " ".repeat((width as usize).saturating_sub(left.len() + right.len()));
|
||||
/// Which segment survives when the two cannot both fit. The title bar's escape
|
||||
/// hatch outranks the document name (improvise-t14d) — `context/code/tui.md`.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
enum Keeps {
|
||||
Left,
|
||||
Right,
|
||||
}
|
||||
|
||||
fn fill_line(left: String, right: &str, width: u16, keeps: Keeps) -> String {
|
||||
let width = width as usize;
|
||||
let (left, right) = match keeps {
|
||||
Keeps::Right => {
|
||||
let right = truncate(right, width);
|
||||
let left = truncate(&left, width - right.width());
|
||||
(left, right)
|
||||
}
|
||||
Keeps::Left => {
|
||||
let left = truncate(&left, width);
|
||||
let right = truncate(right, width - left.width());
|
||||
(left, right)
|
||||
}
|
||||
};
|
||||
let pad = " ".repeat(width - left.width() - right.width());
|
||||
format!("{left}{pad}{right}")
|
||||
}
|
||||
|
||||
@@ -272,7 +295,7 @@ fn draw_title(f: &mut Frame, area: Rect, app: &App) {
|
||||
let file = doc.stored_as.map(|n| format!(" ({n})")).unwrap_or_default();
|
||||
let title = format!(" improvise · {}{}{} ", doc.name, file, dirty);
|
||||
let right = " ?:help :q quit ";
|
||||
let line = fill_line(title, right, area.width);
|
||||
let line = fill_line(title, right, area.width, Keeps::Right);
|
||||
f.render_widget(
|
||||
Paragraph::new(line).style(
|
||||
Style::default()
|
||||
@@ -396,7 +419,7 @@ fn minibuffer_line(prompt: &str, buf: &str, status_msg: &str, width: u16) -> Str
|
||||
true => String::new(),
|
||||
false => format!(" {status_msg} "),
|
||||
};
|
||||
fill_line(left, &right, width)
|
||||
fill_line(left, &right, width, Keeps::Left)
|
||||
}
|
||||
|
||||
fn draw_bottom_bar(f: &mut Frame, area: Rect, app: &App) {
|
||||
@@ -431,6 +454,9 @@ fn draw_status(f: &mut Frame, area: Rect, app: &App) {
|
||||
|
||||
let msg = match app.view_state.status_msg.is_empty() {
|
||||
false => app.view_state.status_msg.clone(),
|
||||
// The wizard overlay draws its own per-step hints; one static table
|
||||
// under it contradicts them on four of eight steps (improvise-rr8n).
|
||||
true if matches!(app.view_state.mode, AppMode::ImportWizard) => String::new(),
|
||||
true => app.hint_text(),
|
||||
};
|
||||
|
||||
@@ -445,7 +471,7 @@ fn draw_status(f: &mut Frame, area: Rect, app: &App) {
|
||||
" {}{search_part} {msg}",
|
||||
crate::protocol::mode_label(&app.view_state.mode)
|
||||
);
|
||||
let line = fill_line(left, &view_badge, area.width);
|
||||
let line = fill_line(left, &view_badge, area.width, Keeps::Left);
|
||||
|
||||
f.render_widget(
|
||||
Paragraph::new(line).style(mode_style(&app.view_state.mode)),
|
||||
@@ -561,6 +587,61 @@ mod bottom_bar_tests {
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The wizard overlay draws correct per-step hints; the status bar stacked a
|
||||
/// second static table under it (improvise-rr8n) — `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn the_wizard_overlay_is_the_only_key_hint_on_screen() {
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.view_state.mode = AppMode::ImportWizard;
|
||||
let row = bottom_row(&app, 100);
|
||||
for token in ["Space:toggle", "c:cycle", "Esc:cancel"] {
|
||||
assert!(
|
||||
!row.contains(token),
|
||||
"the status bar names {token:?} under an overlay that contradicts \
|
||||
it on four of eight steps: {row:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Suppressing the hint must not blank the bar: the mode label and any
|
||||
/// status message still belong there. improvise-rr8n.
|
||||
#[test]
|
||||
fn suppressing_the_wizard_hint_leaves_the_rest_of_the_bar() {
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.view_state.mode = AppMode::ImportWizard;
|
||||
assert!(
|
||||
bottom_row(&app, 100).contains("IMPORT"),
|
||||
"the mode label is not a hint and must survive"
|
||||
);
|
||||
|
||||
app.view_state.status_msg = "no array found in that JSON".to_string();
|
||||
assert!(
|
||||
bottom_row(&app, 100).contains("no array found in that JSON"),
|
||||
"a real status message must still reach the bar"
|
||||
);
|
||||
}
|
||||
|
||||
/// The counterweight to deleting the tile bar's own hint: the tile keys are
|
||||
/// still advertised, by the derived line. improvise-rr8n.
|
||||
#[test]
|
||||
fn the_status_bar_still_advertises_the_tile_keys() {
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
assert!(
|
||||
bottom_row(&app, 200).contains("T:tiles"),
|
||||
"Normal must still say how to reach tile select"
|
||||
);
|
||||
|
||||
app.view_state.mode = AppMode::TileSelect;
|
||||
let row = bottom_row(&app, 200);
|
||||
for token in ["Enter/Space:cycle", "r:row", "Esc/Tab:back"] {
|
||||
assert!(
|
||||
row.contains(token),
|
||||
"TileSelect lost {token:?}, which the tile bar's own hint never \
|
||||
carried in full: {row:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn app_with_minibuffer_and_message() -> App {
|
||||
let mut app = App::new(Workbook::new("T"), None);
|
||||
app.view_state.mode = AppMode::category_add();
|
||||
@@ -687,3 +768,115 @@ mod key_conversion_tests {
|
||||
assert_eq!(got, key::KeyModifiers::ALT);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod title_bar_tests {
|
||||
use super::{Keeps, draw_title, fill_line};
|
||||
use crate::ui::app::App;
|
||||
use crate::workbook::Workbook;
|
||||
use ratatui::{Terminal, backend::TestBackend, layout::Rect};
|
||||
use std::path::PathBuf;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
/// The title row as the terminal really receives it: ratatui clips the
|
||||
/// right, so a helper's return value cannot see the defect.
|
||||
fn title_row(app: &App, width: u16) -> String {
|
||||
let mut terminal = Terminal::new(TestBackend::new(width, 1)).expect("test backend");
|
||||
terminal
|
||||
.draw(|f| draw_title(f, Rect::new(0, 0, width, 1), app))
|
||||
.expect("draw");
|
||||
terminal
|
||||
.backend()
|
||||
.buffer()
|
||||
.content()
|
||||
.iter()
|
||||
.map(|c| c.symbol())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn app_named(name: &str, file: &str) -> App {
|
||||
App::new(Workbook::new(name), Some(PathBuf::from(file)))
|
||||
}
|
||||
|
||||
/// improvise-t14d — `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn the_escape_hatch_outlives_a_long_document_name() {
|
||||
let app = app_named(
|
||||
"Quarterly Regional Sales Analysis",
|
||||
"quarterly-regional-sales-2026.improv",
|
||||
);
|
||||
for width in [36u16, 44, 52, 60, 100] {
|
||||
let row = title_row(&app, width);
|
||||
assert!(
|
||||
row.contains("?:help") && row.contains(":q quit"),
|
||||
"the escape hatch is gone at width {width}: {row:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The name must still lead where it fits, or a fix that simply dropped it
|
||||
/// would satisfy the assertion above. improvise-t14d.
|
||||
#[test]
|
||||
fn the_document_name_still_leads_the_row() {
|
||||
let app = app_named("Sales", "sales.improv");
|
||||
let row = title_row(&app, 80);
|
||||
let name_at = row.find("Sales").expect("the name is drawn");
|
||||
let hatch_at = row.find("?:help").expect("the hatch is drawn");
|
||||
assert!(
|
||||
name_at < hatch_at,
|
||||
"the name leads and the hatch follows: {row:?}"
|
||||
);
|
||||
assert!(
|
||||
row.contains("(sales.improv)"),
|
||||
"a name that fits is not truncated: {row:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Display cells, not bytes — the ASCII case is the discriminating one.
|
||||
/// improvise-t14d, `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn a_multibyte_title_does_not_shift_the_right_segment() {
|
||||
for name in ["Sales", "Ventes régionales", "日本語"] {
|
||||
let app = app_named(name, "m.improv");
|
||||
let row = title_row(&app, 80);
|
||||
assert!(
|
||||
row.ends_with(":q quit "),
|
||||
"the right segment must sit on the right edge for {name:?}: {row:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Whichever segment yields, the composed line is exactly the width it was
|
||||
/// given — the property both call-site directions owe. improvise-t14d.
|
||||
#[test]
|
||||
fn a_composed_line_is_exactly_its_width() {
|
||||
for width in 0u16..40 {
|
||||
for (left, right) in [
|
||||
(" improvise · Ünïcödé ", " ?:help :q quit "),
|
||||
("", " badge "),
|
||||
("日本語 is a wide left segment", ""),
|
||||
] {
|
||||
for keeps in [Keeps::Left, Keeps::Right] {
|
||||
let line = fill_line(left.to_string(), right, width, keeps);
|
||||
assert_eq!(
|
||||
line.width(),
|
||||
width as usize,
|
||||
"fill_line({left:?}, {right:?}, {width}, {keeps:?}) = {line:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `Keeps::Left` is the other direction, and nothing else in this module
|
||||
/// separates it from `Keeps::Right`. improvise-t14d.
|
||||
#[test]
|
||||
fn the_kept_side_is_the_side_that_survives() {
|
||||
let (left, right) = ("LEFTLEFTLEFT", "RIGHTRIGHT");
|
||||
assert_eq!(fill_line(left.to_string(), right, 12, Keeps::Left), left);
|
||||
assert!(
|
||||
fill_line(left.to_string(), right, 12, Keeps::Right).ends_with(right),
|
||||
"the right segment is kept whole when it is the one that wins"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
+33
-19
@@ -344,29 +344,43 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> Result<()>
|
||||
let mut changed = false;
|
||||
|
||||
for line in cmds {
|
||||
match command::parse_line(line) {
|
||||
Ok(parsed_cmds) => {
|
||||
for cmd in &parsed_cmds {
|
||||
let effects = {
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
cmd.execute(&ctx)
|
||||
};
|
||||
app.apply_effects(effects);
|
||||
changed |= app.model_state.dirty;
|
||||
// The status line is the only channel a command has to
|
||||
// report anything — `context/code/cli.md`.
|
||||
let said = std::mem::take(&mut app.view_state.status_msg);
|
||||
if !said.trim().is_empty() {
|
||||
eprintln!("{said}");
|
||||
}
|
||||
if app.last_batch_aborted() {
|
||||
exit_code = 1;
|
||||
// The whole line is syntax-checked before any of it runs, exactly as
|
||||
// before; only CONSTRUCTION moved — `context/code/cli.md`.
|
||||
if let Err(e) = command::parse_line(line) {
|
||||
eprintln!("Parse error: {e}");
|
||||
exit_code = 1;
|
||||
continue;
|
||||
}
|
||||
for segment in command::parse::segments(line) {
|
||||
let mut failed = None;
|
||||
let effects = {
|
||||
// Built against the cursor it is about to run against, so a
|
||||
// command that snapshots one sees command n-1's (improvise-6uva).
|
||||
let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE);
|
||||
match command::parse::parse_segment_interactive(segment, &ctx) {
|
||||
Ok(Some(cmd)) => cmd.execute(&ctx),
|
||||
Ok(None) => Vec::new(),
|
||||
Err(e) => {
|
||||
failed = Some(e);
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
};
|
||||
if let Some(e) = failed {
|
||||
eprintln!("Parse error: {e}");
|
||||
exit_code = 1;
|
||||
break;
|
||||
}
|
||||
app.apply_effects(effects);
|
||||
changed |= app.model_state.dirty;
|
||||
// The status line is the only channel a command has to
|
||||
// report anything — `context/code/cli.md`.
|
||||
let said = std::mem::take(&mut app.view_state.status_msg);
|
||||
if !said.trim().is_empty() {
|
||||
eprintln!("{said}");
|
||||
}
|
||||
if app.last_batch_aborted() {
|
||||
exit_code = 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+43
-13
@@ -126,21 +126,10 @@ impl<'a> Widget for TileBar<'a> {
|
||||
// Draw overflow indicator if tiles remain after the visible area
|
||||
if last_drawn + 1 < labels.len() && x < area.x + area.width {
|
||||
buf.set_string(x, area.y, "▶", Style::default().fg(Color::DarkGray));
|
||||
x += 1;
|
||||
}
|
||||
|
||||
// Hint
|
||||
if matches!(self.mode, AppMode::TileSelect) {
|
||||
let hint = " [Enter] cycle axis [r/c/p] set axis [←→] select [Esc] cancel";
|
||||
if x + hint.width() as u16 <= area.x + area.width {
|
||||
buf.set_string(x, area.y, hint, Style::default().fg(Color::DarkGray));
|
||||
}
|
||||
} else {
|
||||
let hint = " Ctrl+↑↓←→ to move tiles";
|
||||
if x + hint.width() as u16 <= area.x + area.width {
|
||||
buf.set_string(x, area.y, hint, Style::default().fg(Color::DarkGray));
|
||||
}
|
||||
}
|
||||
// The bar draws tiles and advertises no keys: the status line's hint is
|
||||
// derived from the keymap and this one was not (improvise-rr8n).
|
||||
}
|
||||
}
|
||||
|
||||
@@ -218,4 +207,45 @@ mod tests {
|
||||
" Tiles: [_Index ·] [_Dim ·] [_Measure Col] [Region Row] [Product Col]"
|
||||
);
|
||||
}
|
||||
|
||||
/// The tile bar advertised keys that do something else (improvise-rr8n) —
|
||||
/// `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn the_tile_bar_advertises_no_keys_of_its_own() {
|
||||
let app = app_with_two_categories();
|
||||
let cache = project::tile_bar_of(&app);
|
||||
for mode in [AppMode::Normal, AppMode::TileSelect] {
|
||||
// Wide enough that a hint would fit: the shipped 80-column fixture
|
||||
// is too narrow to draw one, which is how the false claim survived.
|
||||
let area = Rect::new(0, 0, 140, 1);
|
||||
let mut buf = Buffer::empty(area);
|
||||
TileBar::new(&cache, &mode, 0).render(area, &mut buf);
|
||||
let text = buf_text(&buf);
|
||||
for token in ["Ctrl", "Enter", "Esc", "←→", "move tiles"] {
|
||||
assert!(
|
||||
!text.contains(token),
|
||||
"the tile bar names {token:?} in {mode:?}, and nothing checks \
|
||||
that claim against the keymap: {text:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The tiles themselves are what the bar is for, and deleting the hint must
|
||||
/// not delete them. improvise-rr8n.
|
||||
#[test]
|
||||
fn the_tile_bar_still_draws_its_tiles_at_every_width_it_used_to() {
|
||||
let app = app_with_two_categories();
|
||||
let cache = project::tile_bar_of(&app);
|
||||
for width in [80u16, 140] {
|
||||
let area = Rect::new(0, 0, width, 1);
|
||||
let mut buf = Buffer::empty(area);
|
||||
TileBar::new(&cache, &AppMode::TileSelect, 0).render(area, &mut buf);
|
||||
let text = buf_text(&buf);
|
||||
assert!(
|
||||
text.contains("[Region Row]") && text.contains("Tiles:"),
|
||||
"at width {width} the bar lost its tiles: {text:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -266,3 +266,94 @@ fn a_run_that_changed_the_document_still_writes_it() {
|
||||
"the -f document lost an edit because `:w` elsewhere cleared the flag: {text}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A document whose row axis carries three items, so a cursor move has
|
||||
/// somewhere to go and the three landing places are distinguishable.
|
||||
fn three_rows(scratch: &Scratch, name: &str) -> PathBuf {
|
||||
let doc = scratch.path(name);
|
||||
run_cmd(&["add-category Region"], &doc);
|
||||
run_cmd(&["add-items Region North South East"], &doc);
|
||||
doc
|
||||
}
|
||||
|
||||
fn hidden_line(doc: &Path) -> String {
|
||||
std::fs::read_to_string(doc)
|
||||
.expect("the document")
|
||||
.lines()
|
||||
.find(|l| l.starts_with("hidden:"))
|
||||
.unwrap_or("(nothing hidden)")
|
||||
.to_string()
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_script_can_move_the_cursor_and_the_next_command_sees_it() {
|
||||
let scratch = Scratch::new("cursor");
|
||||
let moved = three_rows(&scratch, "moved.improv");
|
||||
let still = three_rows(&scratch, "still.improv");
|
||||
|
||||
run_cmd(&["move-selection 2 0 . hide-selected-row-item"], &moved);
|
||||
run_cmd(&["hide-selected-row-item"], &still);
|
||||
|
||||
assert_eq!(
|
||||
hidden_line(&still),
|
||||
"hidden: Region/North",
|
||||
"without a move the cursor is at the origin; if this changes, the other \
|
||||
half of this test stops meaning anything"
|
||||
);
|
||||
assert_eq!(
|
||||
hidden_line(&moved),
|
||||
"hidden: Region/East",
|
||||
"`move-selection 2 0` must move the cursor two rows down before the \
|
||||
next command reads it (improvise-6uva)"
|
||||
);
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn a_script_cursor_move_survives_the_line_that_made_it() {
|
||||
let scratch = Scratch::new("cursor-lines");
|
||||
let doc = three_rows(&scratch, "lines.improv");
|
||||
run_cmd(&["move-selection 1 0", "hide-selected-row-item"], &doc);
|
||||
assert_eq!(hidden_line(&doc), "hidden: Region/South");
|
||||
}
|
||||
|
||||
fn cell_lines(doc: &Path) -> Vec<String> {
|
||||
std::fs::read_to_string(doc)
|
||||
.expect("the document")
|
||||
.lines()
|
||||
.filter(|l| l.contains(" = "))
|
||||
.map(|l| l.trim().to_string())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Evidence: `context/test-evidence/cli.md`.
|
||||
#[test]
|
||||
fn an_explicit_coordinate_still_beats_the_cursor() {
|
||||
let scratch = Scratch::new("explicit");
|
||||
let doc = scratch.path("cells.improv");
|
||||
run_cmd(&["add-category Region"], &doc);
|
||||
run_cmd(&["add-items Region North South East"], &doc);
|
||||
run_cmd(
|
||||
&[
|
||||
"set-cell 1 Region/North",
|
||||
"set-cell 2 Region/South",
|
||||
"set-cell 3 Region/East",
|
||||
],
|
||||
&doc,
|
||||
);
|
||||
assert_eq!(cell_lines(&doc).len(), 3, "the fixture wrote three cells");
|
||||
|
||||
// The cursor is at the origin, which is North. Clearing East by name must
|
||||
// clear East, not whatever the cursor happens to be sitting on.
|
||||
run_cmd(&["clear-cell Region/East"], &doc);
|
||||
let left = cell_lines(&doc);
|
||||
assert!(
|
||||
left.iter().any(|l| l.contains("North")),
|
||||
"the cursor cell was cleared instead of the named one: {left:?}"
|
||||
);
|
||||
assert!(
|
||||
!left.iter().any(|l| l.contains("East")),
|
||||
"the named cell survived: {left:?}"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user