Merge gh/main into fix/6p8q-records-yank-paste

Resolves the append-at-EOF conflict in context/test-evidence/command.md,
keeping gh/main's entries and then this branch's.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-22 21:43:13 -07:00
39 changed files with 2628 additions and 320 deletions
+35 -3
View File
@@ -78,9 +78,41 @@ examples and history belong; prose docs should stay operational.
- **Reading through the MCP is fine.** `loadSession` at session start is still
how a session gets the model in hand; only writing moved.
Every write verifies the result reads back as intended and runs `kb_consistent`
on the candidate, so a bad delta leaves the file untouched. `count` before and
after is still the cheapest evidence a write did what you meant.
Every write checks three things before it replaces the file: the result reads
back as intended, it still holds every clause **HEAD** has, and `kb_consistent`
passes. So a bad delta leaves the file untouched. `count` before and after is
still the cheapest evidence a write did what you meant.
## When a write is refused
Exit codes: `0` done, `1` refused, `2` usage, `3` HEAD's `kb.pl` could not be
read, so nothing was compared, `127` no `tpl`. Only `1` is a verdict.
**"clauses HEAD has are gone, with no kb rm recorded"** means the working-tree
`kb.pl` is short of the committed one. Your delta did not cause it. The file
was already damaged, by a killed write, a bad merge resolution or an edit. The
refusal lists the clauses that are missing. The check counts clauses **per
key**, so losing one of two clauses that share a key is caught too.
- **Recover:** `git show HEAD:prolog-sessions/kb.pl > prolog-sessions/kb.pl`,
then re-run your `add`/`replace`/`rm` deltas.
- **Do not commit the short file to get past the refusal.** HEAD is what the
check compares against, so a committed loss becomes the new floor and is
never reported again.
- **To remove a clause on purpose, use `kb rm`.** It records the removal in a
per-worktree ledger (`git rev-parse --git-path kb-removed.pl`, never
committed), and that record is what lets later writes accept the missing
clause. A hand deletion has no record, so it is refused.
The ledger vouches only for the exact `kb.pl` file the tool last wrote. Any
rewrite outside the tool voids it, including a restore, `git checkout`, `cp`
or an editor. After an `rm`, a restore therefore also undoes the `rm`, and you
must redo it. That is deliberate. Otherwise the abandoned `rm` would go on
excusing a later, accidental loss of the same clause.
A killed write can leave `kb.pl.cand.*` files and a `kb.pl.lock` directory in
`prolog-sessions/`. Git ignores them. The lock makes the next write wait 30
seconds and then fail, so delete it if nothing else is writing.
`context/code/kb-tool.md` has the argument. `scripts/kb-test.sh` proves each
refusal by injecting the fault it names.
+35 -3
View File
@@ -78,9 +78,41 @@ examples and history belong; prose docs should stay operational.
- **Reading through the MCP is fine.** `loadSession` at session start is still
how a session gets the model in hand; only writing moved.
Every write verifies the result reads back as intended and runs `kb_consistent`
on the candidate, so a bad delta leaves the file untouched. `count` before and
after is still the cheapest evidence a write did what you meant.
Every write checks three things before it replaces the file: the result reads
back as intended, it still holds every clause **HEAD** has, and `kb_consistent`
passes. So a bad delta leaves the file untouched. `count` before and after is
still the cheapest evidence a write did what you meant.
## When a write is refused
Exit codes: `0` done, `1` refused, `2` usage, `3` HEAD's `kb.pl` could not be
read, so nothing was compared, `127` no `tpl`. Only `1` is a verdict.
**"clauses HEAD has are gone, with no kb rm recorded"** means the working-tree
`kb.pl` is short of the committed one. Your delta did not cause it. The file
was already damaged, by a killed write, a bad merge resolution or an edit. The
refusal lists the clauses that are missing. The check counts clauses **per
key**, so losing one of two clauses that share a key is caught too.
- **Recover:** `git show HEAD:prolog-sessions/kb.pl > prolog-sessions/kb.pl`,
then re-run your `add`/`replace`/`rm` deltas.
- **Do not commit the short file to get past the refusal.** HEAD is what the
check compares against, so a committed loss becomes the new floor and is
never reported again.
- **To remove a clause on purpose, use `kb rm`.** It records the removal in a
per-worktree ledger (`git rev-parse --git-path kb-removed.pl`, never
committed), and that record is what lets later writes accept the missing
clause. A hand deletion has no record, so it is refused.
The ledger vouches only for the exact `kb.pl` file the tool last wrote. Any
rewrite outside the tool voids it, including a restore, `git checkout`, `cp`
or an editor. After an `rm`, a restore therefore also undoes the `rm`, and you
must redo it. That is deliberate. Otherwise the abandoned `rm` would go on
excusing a later, accidental loss of the same clause.
A killed write can leave `kb.pl.cand.*` files and a `kb.pl.lock` directory in
`prolog-sessions/`. Git ignores them. The lock makes the next write wait 30
seconds and then fail, so delete it if nothing else is writing.
`context/code/kb-tool.md` has the argument. `scripts/kb-test.sh` proves each
refusal by injecting the fault it names.
+54
View File
@@ -124,6 +124,47 @@ So this is the browser's own answer, and `ClientFrame::Save` is the other one
for a socket client editing a TUI-hosted document, where there *is* a file
somebody owns.
### A read-only session refuses edits; it does not accept them and fail to save (improvise-j4qt)
A second tab on the same document cannot take the exclusive OPFS handle, so it
boots **read-only** (improvise-i0v9). It reads the bytes, paints the grid, and
navigates. Until improvise-j4qt, "read-only" described only the *storage*. The
session was writable: an edit was applied to the tab's own in-memory `App` and
projected, so the grid **painted a value the document did not have**
(`18,400.00` became `184,004,242.00`). Only then did the automatic persist
fail, with `NOT SAVED` replacing the read-only reason in the status. The
first tab never saw the edit.
The fix sets the session's access at boot: `Worker::read_only` versus
`Worker::new`, as a private `Access`. A read-only worker answers any
`ClientFrame::Commands` that contains a `Command::Model` with
`Rejected(Rejection::ReadOnly)` **before the host sees it**. So nothing is
applied, nothing is projected, and nothing is persisted. Three things make
this the right place:
- **It is decided up front, not observed afterwards.** The persist failure is
an observation, and it arrives *after* the paint. By the time a failed save
can be seen, the user has already been shown the number.
- **The client's rollback is already correct for it** (improvise-nxah).
`Rejection::ReadOnly` answers `refused_an_edit() == true`, so the client
pops that frame's entry and replays `ViewState` from its settled slice. No
cache needs rolling back: `Command::Model` is a no-op in `reduce_view`, so
the optimistic fold never painted the value. The paint came from the host's
projection, and that projection is exactly what the refusal prevents.
- **Only the document is refused.** A frame of view commands, such as a cursor
move, a panel or a help page, is answered as before. Read-only is a claim
about the document, not about this client's own slice.
It is a property of the *boot*, not the backend. The backend in both cases is
a `vfs::ReadOnly`, and `worker.rs`'s `a_persist_that_fails_raises_a_notice…`
still uses one as a disk-full injection over a *writable* session. Reading
access off the storage would have made those two indistinguishable.
`ClientFrame::Save` is unchanged: a read-only tab that asks to save is still
answered `SaveFailed` by the backend.
What this does **not** do is i0v9's gap 2. There is still no upgrade path when
the first tab closes, and the tab stays read-only until it is reloaded.
### Ordering
`Worker::on_frame` returns the frames for one inbound frame, in order, and a
@@ -1491,6 +1532,19 @@ what says it is closed**. That is the shape `col_groups` already uses for "this
grid has no column groups" — an absent list rather than a flag the page has to
interpret alongside a mode it would then have to understand.
**That shape is only sound where "open" implies "non-empty"**, and help is the
one place it does — its content is static. `views` and `formulas` borrowed it
and were wrong: a document with no formulas opened a formula panel that built
the same `RenderModel` as a closed one, so the page hid it under a mode reading
FORMULAS (improvise-df2w). Both are now `Option<Vec<_>>` — `None` closed,
`Some(vec![])` open and empty — one field, so the two facts cannot disagree,
where a `formula_panel_open` flag beside the rows (the `category_panel_open`
shape) would have been two. The same mistake as improvise-xg1z: an absence
spent on encoding a second fact. What the page does **not** yet draw is the
TUI's empty-state line (`(no formulas — press 'n' to add)`); that string lives
in `src/ui/formula_panel.rs`, and copying it into `render.rs` would be a second
home for it.
The rows themselves are `improvise_protocol::help::pages()`, the same ones
`src/ui/help.rs` draws. `RenderHelpRow` carries a `kind` string
(`heading`, `keys`, `accent`, …) which is a *semantic* name the page maps to a
+102
View File
@@ -832,6 +832,108 @@ that item. Under route-then-aggregate that arm is reached only where a record
genuinely lacks the coordinate, because the fold hands the expression a record
key. Evaluated bare, outside a `SUM`, it degrades exactly as `Formula::filter`
does — which is why a fitted tree is always emitted wrapped.
## 14. A collapsed group is one row, and its cell folds the group (improvise-6qet)
**The ruling.** "When collapsing a group, I'd generally expect the cursor to move
to whichever place the group collapsed into" (user, 2026-09-23). The shape chosen
from a presented fork: **a collapsed group contributes one data row, whose cell is
an aggregate over the group.** Before this, `expand_category` emitted the group's
`GroupHeader` and then `continue`d past every item, so a collapsed group had no
data row at all. `row_group_for` is indexed by data row, so nothing on the grid
could name the group once it was collapsed: `z` pressed twice collapsed the
*neighbour*, and a collapsed group could be reopened only by `:toggle-group`.
The fix is to emit **one** `DataItem` where the `continue` was, and every
cursor-addressed question then answers itself by arithmetic. With
`Q1={Jan,Feb}, Q2={Apr,May}` and the cursor on `Apr` (row 2), collapsing Q2 gives
rows `[Jan, Feb, Q2]`, so row 2 *is* Q2 and a second `z` names Q2 again. `z z` is
an involution without anything being taught about involution, and neither
`ViewCursor` nor `row_group_for` changed.
### The row
- **Its tuple is the group name** (`Month=Q1`). `AxisEntry::DataItem` became a
struct variant `{ tuple, collapsed }` rather than growing a second variant,
because a new variant would be silently missed by `matches!(e, DataItem(_))`
sites; changing the shape made the compiler list all six external matches.
- **`collapsed` cannot be derived from the tuple**: a group and an item may
legally share a name. A parallel `Vec<bool>` was rejected for the reason gotcha
1 gives `prune_axis_items` a predicate — pruning shifts indices.
- **The flag is inherited** down `cross_product`, so a category *nested under* a
collapsed group (`Q1/North`) is also a fold: no record carries `Month=Q1`
whichever inner coordinate sits beside it.
- **A group's items need not be contiguous** (`A[G1], B[G2], C[G1]` is legal), so
`expand_category` keeps a `folded` list: a collapsed non-contiguous group folds
**once**, and its header is not re-emitted on the repeat — that header would
open a run with no row in it.
### The cell
`Model::evaluate_aggregated` expands, at its head, any coordinate that names a
group rather than an item into one key per member (a cross product where several
coordinates do). Doing it in the **model** is what makes `display_text`, prune,
column totals, `grid_of`, search, yank and CSV export all follow with no change.
Three decisions:
- **Membership is read off each item's own `group` field, never off
`Category::groups`.** `add_item_in_group` registers no `Group`; that list is
filled only by `.improv` `>` lines. Asking it answers "not a group" for every
document built in code, and the symptom is the fold's row pruned as empty.
- **An item with the same name as the group wins.** `items.contains_key`
short-circuits the expansion, so an existing document with an item `Q1` reads
exactly as before.
- **Stored values fold once over every record the group covers; a derived value
folds its members' results.** Folding the members' *aggregates* is right for
`Sum` and wrong for `Avg`/`Count` under `Axis::None` — `avg(avg(2,4), 9) = 6`
where the group's average is `5`. A formula target has no stored records to
pool, so its members' evaluated results are folded with the measure's
aggregation; for a ratio formula that is the usual pivot-table limitation, not
a new one. Hidden items are included in the fold, as on `Axis::None`: hiding
decides what is drawn, not what an aggregate covers.
### Settability and drill
The row is not a record, so `project.rs::cell_id` answers
`CellId::Aggregate` when `GridLayout::folds_a_group(row, col)` — the one home of
that question — whatever is on `Axis::None`. Without it, a document with nothing
regular on None projected the fold as `CellId::Record`, offered it for editing,
and a `SetCell` would have written the coordinate `Month=Q1`, which is not an
item. `render.rs::editable` is untouched (ADR 002: the variant decides).
`EditOrDrill` asks the same predicate and declines out loud: drilling would filter
on `Month=Q1`, which no record carries, and show an empty list.
### The cursor, and why the order is the fix
`ToggleGroup` is `Model(LayoutGeometry)`, and `apply_effects` resets the cursor
when it observes the frame move *inside* the loop (gotcha 24). So
`ToggleGroupAtCursor` emits `ToggleGroup` **first** and the cursor placement
**after**; the swap is the rival, and it lands every toggle at `(0, 0)`. The
placement also restores the scroll offsets the reset zeroed.
Where the cursor lands is asked of the layout the toggle **will** produce
(`GridLayout::row_run_start(cat, group, prefix)` on a view with the toggle
applied), not of the current one. The same answer serves collapse and expand —
the group's first row — and it matters when the group is at an inner level:
collapsing `Month` under `Region` shrinks every earlier `Region`'s run too, so
`South/Apr` at row 6 lands on `South/Q2` at row 5, not row 6. That is the only
test the "ask the current layout" rival fails.
### What this does not do
- **Part 2 of the ruling** (a toggle carrying the group it acted on, so a later
command in the same batch can be invalidated) is not added: nothing binds `z`
as a `Binding::Sequence`, and if something did, both toggles would resolve
against one pre-batch context and name the same group, so the pair is a no-op —
which is what `z z` means. A mechanism with no reachable case is not built.
- **CSV export writes the fold as a row**, because an export writes what the grid
draws (improvise-iot6); a collapsed group is its own total row there too.
- Under `prune_empty` an **empty** collapsed group now disappears header and all,
where improvise-lhi4 kept its header. It is a narrowing, and deliberate: the
header was kept because it was the only way back to the rows, and the fold row
is now that way back — an empty fold has nothing to show.
- The browser still cannot press `z` (`toggle-group-under-cursor` resolves
`Upstream`), so the cursor half takes effect in the TUI only.
## `PageAxis` — what constrains, and what the user may turn (improvise-p1ym)
`GridLayout::page_coords` answers two questions that gotcha 27 says must not be
+93 -17
View File
@@ -99,34 +99,98 @@ refuses unless every clause HEAD has is still present **by key**. It runs first
so that the refusal names the loss rather than whatever consistency error the
loss happens to cause.
It compares keys rather than counts, although the ticket asked for a count,
because a count can be masked: uncommitted adds in one predicate offset a
It compares keys rather than totals, although the ticket asked for a count,
because a total can be masked: uncommitted adds in one predicate offset a
truncation in another, and the count comes out even. Keys cost the same (0.26s
for a write, most of it trealla start-up) and the message still names both
totals and the number missing.
**Keys are counted, not collected** (improvise-ih3k). The guard first compared
the *set* of keys, and a set cannot see one of two clauses sharing a key.
`work_done/3`, `decision/3` and `work_followup/3` key on two arguments and are
not `functional/1`, so `kb_consistent` accepts two clauses under one key. `kb
add` refuses to *write* such a pair, but a git merge is not a `kb add`: two
branches that each record `work_done(T, bug_fix, …)` conflict at the end of the
block, and keep-both is the natural resolution. Once HEAD holds a pair, losing
either clause left the key present and the write was accepted, with both totals
printed equal. So the guard counts clauses per key, and a key is short when the
result holds fewer than HEAD does. The refusal names the HEAD clauses the result
holds no copy of, which is the one that went.
Two alternatives were rejected. Comparing whole clauses would refuse every
`replace`, whose purpose is to change the text under a key. Making the three
predicates functional would refuse the pair itself at `kb_consistent`. That
would force a merge to resolve it, and may be worth doing, but it is a change to
the gate in `kb.pl`, not to this tool. `functional/1` also keys an arity-3
predicate on its first argument, which is wrong for `decision/3`, so it would
need a new two-argument form. And the tool must not lose a clause whatever the
gate allows, so counting is needed either way.
A pair also had no way out through the tool. `rm` checked its result against
`before - clauses in the delta`, which assumes one clause per key, so removing a
paired key was refused as "a clause was dropped". The expected total is now
`before - clauses the delta's keys match`, which is still counted separately
from the fold that removes them. `replace` of a paired key is still refused by
its total. That is the right answer, because it would silently collapse two
clauses into one. The route for a pair is `kb rm` the key, then `kb add` the one
clause to keep.
**How the count legitimately falls.** `add` never lowers it and `replace`
overwrites in place, so neither can trip the guard. `kb rm` is the only
intended removal, and it records the keys it removed in a **ledger** that later
writes read: a clause missing from the working tree is excused only if its key
is in the ledger. The ledger lives in the git dir (`git rev-parse --git-path
writes read: a key the result is short of is excused only if the ledger names
it. The ledger lives in the git dir (`git rev-parse --git-path
kb-removed.pl`), so it is per-worktree and can never be committed; with
`KB_FILE` set it sits beside that file. It prunes itself on every write, keeping
only keys HEAD still has and the candidate does not, so an entry drops once the
removal is committed, and once the key is re-added. That second case matters:
`KB_FILE` set it sits beside that file. Every write rewrites it, keeping only
the keys the result is still short of, so an entry drops once the removal is
committed and once the key is back to HEAD's count. That second case matters:
without it, a re-added clause that was lost again would still be excused. There
is deliberately no override flag. The way out of a refusal is to restore
`kb.pl` from HEAD and re-apply, or to commit the removal so HEAD moves.
is deliberately no override flag.
The way out of a refusal is to restore `kb.pl` from HEAD and re-apply your
deltas. Do **not** commit the short `kb.pl` to get past it. The refusal
message used to offer that, and taken on a loss nobody intended it makes the
loss permanent: the committed file becomes the floor this guard compares
against, and nothing ever reports it again. A removal you intend goes through
`kb rm`, onto a restored file.
**A ledger speaks only for the file its write published** (improvise-yea8).
Pruning happens only when the tool writes. A commit, a `git checkout --
prolog-sessions/kb.pl`, a `cp`, and a branch switch are not writes. So an
entry from an `rm` that was later abandoned would excuse a later loss of that
same clause by any path. That is easy to reach, because the recommended
recovery is exactly a restore. Pruning against the kb read at the next write
cannot close it, because that kb has already lost the clause. So the ledger's
first line is a `% kb-stamp` comment naming the kb file the write published,
and a later write reads the entries only if the kb it finds is still that file.
The stamp is the content hash **and** the file's inode and nanosecond mtime.
Content alone is not enough, and the test that showed it was the ticket's own
reproduction. `kb.pl` is kept in canonical form, so `grep -v` of one clause
from HEAD produces output **byte-identical** to what `kb rm` of that clause
writes. After `rm X`, a restore, and then the same loss by hand, a content-only
stamp still matched. `rename(2)` carries the candidate's inode and mtime onto
`kb.pl`. Anything else that rewrites the file, whether a `cp` onto it, a git
checkout or an editor, gives a new mtime and usually a new inode. `stat -c` is
tried before `stat -f` because GNU `stat -f` means *filesystem* status and
would silently stamp the wrong thing. It is not keyed by HEAD commit, because a
commit that does not touch `kb.pl` does not make the ledger wrong. And an
uncommitted `rm` carried across a branch switch, with `kb.pl` untouched, is
still exactly the kb it describes.
The cost is a false refusal after an intended `rm` followed by anything that
rewrites the file without the tool. That refusal is loud, and the recovery is
the usual one: restore and redo the `rm`.
**The write is a rename.** The candidate used to come from `mktemp` in
`$TMPDIR`, and `mv` from there onto the repository crosses a device wherever
`/tmp` is its own filesystem. That makes `mv` a copy and an unlink, so a kill
partway through leaves a truncated `kb.pl`. The candidate is now a sibling of
`kb.pl` (`kb.pl.cand.XXXXXX`), so publishing it is `rename(2)`. The ledger is
renamed first. A kill between the two renames then leaves a ledger naming keys
the kb still has, and the next write prunes those entries. Until it does, they
would excuse the loss of exactly those clauses, which is a narrower hole than a
kb.pl that is already short.
renamed first. A kill between the two renames leaves a ledger stamped for a
candidate that was never published. It therefore matches nothing, and the next
write ignores it. That can refuse a write an earlier `rm` should have excused.
It cannot excuse a loss, which was the hole before the stamp.
**Exit codes.** 0 done, 1 refused, 2 usage, 3 HEAD's `kb.pl` could not be read,
127 no `tpl`. Only 1 is a verdict. 3 is separate because "could not compare"
@@ -139,10 +203,22 @@ The completeness guard trusts HEAD. A loss that has been **committed** is
invisible to it by construction. That was the right trade for this incident,
where HEAD held all 801, but a bad commit of `kb.pl` becomes the new floor.
A `SIGKILL` leaves its `kb.pl.cand.*` and `kb.pl.lock` behind, because no trap
runs. The lock already had that property. The stray candidate is new: it is
never read, but it shows as untracked in `prolog-sessions/`, where a `git add
-A` would pick it up.
A `SIGKILL` leaves six files behind, because no trap runs: the candidate
`kb.pl.cand.XXXXXX`, its `.base`, `.ledger`, `.removed` and `.stamped`
siblings, and the `kb.pl.lock` directory. None of them is ever read again,
and **git does not see them**. `.gitignore` ignores `prolog-sessions/*`, so
neither `git status --untracked-files=all` nor `git add -A` lists them. That
was measured, and an earlier version of this paragraph warned about `git add
-A` staging them, which does not happen. The one that matters is the lock: the
next write waits 30 seconds for it and then exits 1 with "remove it if stale".
Delete it and the candidates by hand.
The ledger's stamp names a file by inode and nanosecond mtime. A rewrite that
preserves both, such as a `touch -r`, a filesystem with coarse timestamps
rewriting within one tick, or an inode reused at the same mtime, with the bytes
also identical to what the tool published, is indistinguishable from the file
the tool wrote. That takes deliberate effort or a filesystem this repository
does not run on.
The key rule is looser than `functional/1` for `crate/3`, which keys on the
first argument alone; `kb_consistent` is the backstop for that case rather than
+80 -8
View File
@@ -218,14 +218,86 @@ its verdict. The list is checked in **both** directions, so an entry that starts
being composed fails just as loudly as one that stops
(`doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)`).
Two limits are worth stating, because the check is a source scan.
`#[cfg(test)]` items are dropped by a line scan that relies on rustfmt putting
an item's closing brace at the attribute's own indentation; over-stripping would
**fail** the test rather than pass it, which is the safe direction. And a
`Command::X` inside `matches!` or `if let` is read as a construction rather than
a pattern, which can only make the check *weaker* for that variant — never raise
a false alarm. A variant nothing composes at all is immune to both, since its
only pattern occurrence is its own fold arm, which `=>` identifies.
The check is a source scan, and **what counts as source has one home**:
`improvise_protocol::source_scan::production_code`, shared with the forward
guard below (improvise-c7mu). It removes three things a scan must not see.
- **Comments and literals.** Both are blanked, and newlines are kept. Before
this, a `// TODO: Command::SetYanked(...)` left where the real composer had
been deleted still passed all four tests. That is the *weaker* direction, the
one a scan weakness was supposed to be unable to go. It was live in a small
way: a `///` in `project.rs` named `Command::SwitchView`, which has real
composers, so no verdict was wrong yet. It also had a mirror: a comment
explaining that a `NOT_COMPOSED` variant is held back would have raised a
false "stale entry" alarm.
- **Test items inside a file.** A line scan that relies on rustfmt putting an
item's closing brace at the attribute's own indentation. It treats
`cfg(any(test, …))` as test code and `cfg(not(test))` as production.
Over-stripping **fails** the test rather than passing it, which is the safe
direction.
- **Test-only files.** A `#[cfg(test)] mod x;` makes `x.rs` and everything under
`x/` test code, even though nothing inside the file says so. Before
improvise-c7mu, this checker's own file and `improvise-io`'s
`roundtrip_tests.rs` were scanned as production.
One limit remains. A `Command::X` inside `matches!` or `if let` is read as a
construction rather than a pattern. That can only make the check *weaker* for
that variant; it never raises a false alarm. A variant nothing composes at all
is immune, since its only pattern occurrence is its own fold arm, which `=>`
identifies.
### And forwards: every effect is on the wire or has a verdict (improvise-h3y7)
`vocabulary_is_composed.rs` checks that the vocabulary does not name more than
production composes. The other direction is *does the vocabulary name
everything it should?* That was the direction `ViewBack` and `ViewForward`
failed: they were missing, and no client had a back history. Until improvise-h3y7
nothing checked it, and the audit that finally ran it by hand under-reported.
Brace-matching `impl Effect for` blocks misses the eight `simple_effect!`
expansions and `SetTransientPrefix` in `command/keymap.rs`. All nine are
`ViewOnly`, which is the half the `ViewBack` bug was in.
`improvise-command/src/ui/reduce/vocabulary_is_complete.rs` is that direction.
It scans the crate's `src/` for both ways an effect is declared, reading
each name past any attributes and doc comments before it. The macro accepts
those, and missing them hid effects (improvise-94x2). It requires
each name to be a `Command` or `ModelCommand` variant (read off serde's derive)
or to appear in `NOT_ON_THE_WIRE` with a verdict. A verdict is one of four
kinds, and the check verifies each kind:
- `Command(v)`: on the wire under another name (`HelpPageNext`/`Prev`/`Set` →
`Command::HelpPage`). `v` must be a real variant.
- `Frame(v)`: answered by a `ClientFrame` that is not a `Command` (`Save` →
`ClientFrame::Save`, gotcha 45). `v` must be a real frame.
- `Off(why)`: excluded on purpose, with the argument: session-local residue
(gotcha 30), host-local caches, or batch-relative control flow.
- `Gap(ticket)`: missing, and a ticket owns closing it (`SaveAs`, `ExportCsv` →
improvise-2mqz).
The list is also checked in reverse. An entry that is now on the wire fails,
and so does an entry that names no effect. The scan is `production_code`, so
the comments, strings and test fakes that name an effect do not count as
effects. If they did, a stale verdict could outlive the effect it names.
What it does **not** check is whether a same-named pair *agrees*: whether
`reduce_view`'s arm does what the effect does. The name match is the
completeness half. The semantic half stays a per-pair read (improvise-h3y7's
notes put it at about 21 pairs). The resolver lists in `resolve.rs`
(`declined()` and the host-only names) are also not checked against the
vocabulary yet. The audit found them stale in three places.
### No `ModelCommand` field is a position (improvise-hkbc)
improvise-6nsd closed "a stale layout tier misdirects a mutation" on one
condition: no `ModelCommand` carries a row or column index. `position_free`
in `reduce.rs`'s tests enforces that at compile time. It destructures every
variant with no `..`, down into `RecordCell`, and hands each field to a
function that accepts only a name, a path, a surrogate, a value or an axis. So
a new variant, a new field, or a field retyped to `usize` fails to build, and
the build failure forces the ADR conversation. It **cannot** see a position
spelled as a `String`. It also does not make the client position-free:
improvise-2wu3 resolves a painted `(row, col)` against the current cache before
it ever builds a command.
### Why two model effects are key-path only
+66 -2
View File
@@ -1130,8 +1130,8 @@ Normal and never closes the panel, exactly as `src/draw.rs` keeps drawing it
off `view_panel_open` alone. Asserting it hidden would have pinned the browser
to behaviour the TUI does not have.
The gating rule itself — an empty list is what says a panel is closed, the same
rule `help` follows — is asserted in Rust as well, by
The gating rule itself — `None` is what says a panel is closed (it was an empty
list, the rule `help` follows, until improvise-df2w) — is asserted in Rust as well, by
`a_closed_panel_projects_no_rows_and_an_open_one_does` and
`the_tile_bar_is_always_present_and_only_its_cursor_is_modal`, because a browser
test cannot distinguish "closed" from "the cache is empty" and those two tests
@@ -4231,6 +4231,70 @@ and converged.
**Fault injected:** the same `Binding::Prefix` → `Upstream` reversion. Fails on
the second press.
### `a_read_only_session_refuses_an_edit_before_anything_is_painted`
improvise-j4qt. The finder measured this in Chromium and Firefox: two tabs, one
document. The second tab boots read-only. Typing `4242` into a cell applied it
to that tab's in-memory `App` and projected it, so the grid showed
`184,004,242.00`. Only then did the persist fail and `NOT SAVED` replace the
read-only reason. The first tab never saw the edit.
The test builds the read-only boot the way `bindings::read_only` does: the
document's bytes over a `vfs::ReadOnly` backend. It sends a caught-up `SetCell`
and asserts four things:
- the whole reply is `[Rejected(ReadOnly)]`;
- the client's folded grid still shows the old text;
- no notice was raised (no persist ran);
- the document is not dirty.
**Red first**: the same test over `Worker::new`, which was the boot's
constructor before this change, failed. Its reply was a `Commands` frame of
projections carrying the new value.
The control is `a_read_only_session_still_answers_a_view_command`: a
`SetSelected` frame is not refused. Without it, a worker that refused *every*
frame would pass the test above.
End to end: `harness/tests/shell.spec.js`, "a read-only tab refuses an edit and
never paints it". It opens two real tabs on one document in Chromium, edits in
the second, and asserts four things:
- the toast says `read-only`;
- the cell keeps its text;
- the grid contains no `4,242`;
- `#status` still says `read-only` and not `NOT SAVED`.
Fault injected: `bindings.rs` reverted to `Worker::new(app)` for the read-only
boot, then `harness/build.sh` rebuilt. The spec then failed on the toast, which
received `""` because no refusal arrived. Reverted and rebuilt, and the whole
of `shell.spec.js` passed (18).
### `an_open_empty_panel_is_not_a_closed_one`
improvise-df2w. `views` and `formulas` said "closed" by being empty, so an open
formula panel over a document with no formulas built a `RenderModel` identical
to a closed one, and the page hid it while the mode read FORMULAS. The test
compares the two models whole rather than asserting a field, so it does not
depend on how the difference is spelled.
**Reproduced:** fails at HEAD before the fix with `left == right` on the formula
half — the two models were byte-identical. The existing
`a_closed_panel_projects_no_rows_and_an_open_one_does` could not see it, because
it fills both caches before opening the panels.
### the formula panel opens over a document with no formulas
The browser half of improvise-df2w, in `harness/tests/contract.spec.js`, over
`harness/fixtures/tall.improv`, which has no `## Formulas` section. It asserts
**both** directions, because each rival fails a different one.
**Reproduced:** against a bundle built from the unfixed `render.rs` and
`app.js`, fails at `#formulas` `toBeVisible()`, immediately after `#mode` reads
FORMULAS — the exact disagreement the ticket describes.
**Fault injected:** `paintPanel` writing `host.hidden = false` (a page that
stops inferring closed and simply never hides). Fails at the launch-time
`toBeHidden()` before the panel is opened.
## improvise-ktlk — the overlay and the commit read one buffer
PR #162 (improvise-xg1z) changed what the overlay **draws** for an `Editing`
+125 -37
View File
@@ -1257,9 +1257,11 @@ substring.
### a_window_reaching_the_last_row_carries_the_trailing_group_header
A collapsed group at the bottom of the grid has no items to hang off, and is still
the only row that says those rows exist. A window that reaches the last row
therefore carries it.
A collapsed group at the bottom of the grid folds into one row (improvise-6qet,
`context/code/core.md` §14), and the header comes above it. A window that reaches
the last row therefore carries both: the frame is `[North, South, [Late], Late]`.
Before 6qet a collapsed group had no row, and this test asserted the header was the
*last* entry — the premise 6qet changed, so the length moved from 2 to 3.
It must be *seen*; it is not yet actionable in a browser. `web/app.js` registers no
listener on a group `<th>`, and `toggle-group-under-cursor` is still in
@@ -1289,23 +1291,30 @@ say which group a column or row belongs to — while `body_from` declines to pai
header that sits above the window.
**And only the nearest one comes.** The fixture puts **two** headers above the
window (`Hidden` is collapsed, so it emits a header and no items, and `Late`'s
follows it immediately) because that is the only way to tell "the nearest header
comes" from "every header above comes". With one group above, deleting the
window (`Hidden`, whose collapsed group is now one row, and `Late`) because that is
the only way to tell "the nearest header comes" from "every header above comes".
Since improvise-6qet the fold is itself a row, so the window opens at row **2**
(`East`), past both `Hidden`'s header and its folded row; at row 1 the window would
start on the fold and bring `Hidden`'s header with it. With one group above, deleting the
compaction leaves this green — it is a *cost* property, O(the groups a deep scroll
crossed), and correctness alone cannot see it: `group_of` still finds the nearest
and `body_from` still starts at the first item either way.
#### Helper — `app_with_consecutive_headers`
#### Helper — `app_with_a_collapsed_group_between_rows`
Not a test — the fixture for that test and for `consecutive_headers_between_two_rows_all_come`.
Not a test — the fixture for that test and for `every_header_between_two_in_window_rows_comes`.
Two consecutive group headers between two rows: `Hidden` is collapsed, so it emits
a header and no items, and `Late`'s header follows it immediately.
Two group headers between `North` and `East`: `Hidden`, collapsed, and `Late`, open.
It was `app_with_consecutive_headers` until improvise-6qet: a collapsed group now
contributes one row, so a collapse no longer produces two *adjacent* headers and
the old name described a shape the layout cannot emit.
### consecutive_headers_between_two_rows_all_come
### every_header_between_two_in_window_rows_comes
Headers between two in-window rows all come, not just the nearest.
Headers between two in-window rows all come, not just the nearest — the frame is
`[North, [Hidden], Hidden, [Late], East]`, each header above the row it opens.
Renamed from `consecutive_headers_between_two_rows_all_come` for the reason the
fixture was.
Only the nearest wins `group_of`, so a compaction that kept one would still answer
every question about *membership* correctly — and would silently lose the `Hidden`
@@ -4803,40 +4812,80 @@ cursor at data row 0:
3. The row axis is now empty, so `row_group_for(0)` resolves nothing and `z`
returns no effects at all.
So the cursor-addressed toggle is not an involution, and a collapsed group cannot
be reopened from the grid — only by `:toggle-group`, which names the group. That
behaviour is unchanged by this work and is filed separately as improvise-6qet; the
three tests below pin what the command *does*, which is what the old test claimed
to and could not.
So the cursor-addressed toggle was not an involution, and a collapsed group could
not be reopened from the grid. **improvise-6qet changed that**, and the tests below
are its guards; the two that pinned the old behaviour
(`a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one`,
`a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address`) were replaced
rather than kept, since the behaviour they pinned was the bug. The old law stays
deleted; a real determinism law over two independently built contexts is
improvise-hzjo.
improvise-6eqi found the survivors of ub4o all seated the cursor at data row 0, so a
toggle that ignored the cursor (`row_group_for(0)`) passed the whole workspace. Every
toggle test below starts off row 0; that rival was injected and fails five of them.
### toggling_at_the_cursor_names_the_group_the_cursor_sits_in_and_collapses_it
Asserts the effect names `category: "Month", group: "Q1"` — a category and a group
by name, which no assertion in the tile/grid suite had ever done — and that the
view's collapsed set actually moved afterwards. The fixture is asserted expanded
first, so the post-condition is not supplied by the default.
Asserts the effect names `category: "Month", group: "Q2"` — a category and a group
by name — and that the view's collapsed set actually moved, and that the neighbour
`Q1` did not. The cursor sits on `Apr` (row 2): at row 0 the neighbour and the
subject are the same answer. Fixture asserted expanded first, so the post-condition
is not supplied by the default.
Fault injected: `execute` returning `vec![]` after resolving the lookup. The old
law passed (both sides empty, as they already were); this test fails on the
missing effect, and the two below fail with it.
Faults injected: `execute` returning `vec![]` after resolving the lookup, and
`row_group_for(0)` in place of the cursor row (improvise-6eqi). Both fail it.
### a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one
### toggling_a_group_collapses_that_group_and_the_cursor_follows_it
The test the old one could not be: it requires the second `execute` to see a
layout the first one changed.
improvise-6qet, driven through the real path (`App::cmd_context` → `execute` →
`apply_effects`). Cursor at row 2 (`Apr`); `z` gives rows `[Jan, Feb, Q2]` with the
cursor on `Q2` at row 2, and `z` again gives `[Jan, Feb, Apr, May]` with the cursor
back on row 2 — the involution the old law claimed.
Fault injected: a stateless `execute` that always returns
`ToggleGroup { category: "Month", group: "Q1" }` — i.e. one that ignores the
applied state, which is exactly what "does not apply effects between calls" hides.
Run against it, the old law **passes** (`first == second` by construction) and this
test fails, as does the one below. That rival is one-directional: it can only ever
name `Q1`, so a test that merely asserts "some group is named" is satisfied by it.
Faults injected: **the swap** (cursor placement emitted before `ToggleGroup`) fails
it `left: (0, 0) right: (2, 0)` — the toggle's observed reframe resets the cursor
after the placement. `row_group_for(0)` fails it too.
### a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address
### a_collapse_from_the_tail_of_a_run_lands_on_the_runs_first_row
Cursor on `May` (row 3). Collapsing Q2 lands on row 2, the fold. The rival "leave
the cursor where it was" is right for a cursor already on the run's first row and
wrong here (row 3 is past the end of `[Jan, Feb, Q2]`); injected, it fails this and
the next test only.
### collapsing_an_inner_group_lands_where_every_earlier_run_shrank
`Region × Month` on Row, `Month` the inner level. Cursor on `South/Apr` (row 6).
Collapsing Q2 shrinks `North`'s run as well, so `South/Q2` is at row **5**. The only
test that separates "ask the layout the toggle will produce" from "ask the current
layout": that rival returns 6 (`left: (6, 0) right: (5, 0)`).
### a_collapsed_group_can_be_re_expanded_from_the_grid
Both groups collapsed (`[Q1, Q2]`), cursor on the `Q2` row: `z` names Q2 and
reopens it, landing on `Apr` at row 1. Before 6qet a collapsed group had no row and
this was impossible from the grid. The swap fails it at `(0, 0)`.
### edit_or_drill_declines_a_collapsed_groups_row
A fold is neither editable nor drillable: no record carries `Month=Q2`. The same
context at `Feb` (row 1) still opens the editor, so the decline is about the row,
not the fixture. Rival: the `folds_a_group` guard removed — the command opens an
editor on the fold, and the test fails.
## `ui/project.rs` — a collapsed group's row is an aggregate (improvise-6qet)
### ui::project::cell_identity::a_collapsed_groups_row_is_projected_as_an_aggregate
Nothing regular is on `Axis::None`, so the none-axis arm of `cell_id` cannot supply
`Aggregate` — and `North` in the same grid is asserted `Record` to prove it. The
`Coastal` fold must be `Aggregate`, and its text `7.00` (East 3 + West 4). Rival:
`folds_a_group` dropped from `cell_id` — the fold is projected `Record`, which
`render.rs::editable` would offer for a write to the non-item `Region=Coastal`. It
also fails when group membership is read off `Category::groups` instead of each
item's `group`, because a document built with `add_item_in_group` registers none.
Pins step 3 — `row_count() == 0` is asserted before the call, so the empty effect
vector is the command declining to name a group rather than the fixture never
having had one. Both rivals above fail it.
## `ui/project.rs` — a formula lost with its target ITEM (improvise-en4s)
`Dirty::of(ModelChange::CellData)` reports `formulas: true`. That was written as
@@ -4934,6 +4983,45 @@ in so that a change making the fixture's load stop rebasing fails *here*, where
the message says what actually broke, rather than turning the rollback test
green by emptying it.
### ui::reduce::vocabulary_is_complete::every_effect_is_on_the_wire_or_has_a_verdict
improvise-h3y7, the forward direction of the vocabulary derivation. It is the
direction `ViewBack`/`ViewForward` failed: they were missing, so no client ever
had a back history. The test scans this crate's production `src/` for
`impl Effect for X` and `simple_effect!(X`. That covers both spellings, and the
h3y7 audit's brace-matcher missed nine effects by seeing only the first. It
then requires each name to be a `Command` or `ModelCommand` variant (read off
serde's derive) or to carry a `NOT_ON_THE_WIRE` verdict.
Faults injected, together: `HelpPageNext`'s verdict replaced by one for
`SetSelected`. This test failed naming `HelpPageNext`, and
`the_verdict_list_names_nothing_that_is_on_the_wire` failed naming
`SetSelected`. Separately, `HelpPageSet`'s verdict was pointed at
`Command::HelpPag`, and
`the_verdict_list_names_only_real_effects_and_real_variants` failed.
`the_scan_reaches_every_way_an_effect_is_declared` pins one effect of each
spelling plus `SetTransientPrefix` (which lives in `command/keymap.rs`). A
broken walk therefore fails that test instead of shrinking the set.
`an_effect_named_only_in_a_comment_or_string_is_not_an_effect` shares
improvise-c7mu's fault. With `code_only` made the identity, it failed with
`["SaveAs", "ExportCsv", "Real", "SetYanked", "AlsoReal"]`.
**improvise-94x2, found by the PR #172 review.** The reader took the
identifier *immediately* after `simple_effect!(`. The macro's own matcher is
`$(#[$meta:meta])* $name:ident(…)`, so a doc comment before the name (blanked
to whitespace by `code_only`), an attribute, or a line break after `(` left
nothing to read, and the effect was invisible. The reviewer injected a
documented `simple_effect!(\n /// Rival.\n SetBogusRival(bool) …)` into
`effect.rs`, and all five tests passed. Re-injected here, it also passed all
five before the fix. After the fix
`every_effect_is_on_the_wire_or_has_a_verdict` fails naming `SetBogusRival`.
The rival was removed afterwards. `name_after` now skips whitespace and every
bracket-matched `#[…]` before reading the name.
`an_attributed_or_wrapped_effect_is_still_an_effect` pins that on a fixture:
a doc comment, two attributes (one with nested brackets), and a wrapped call.
With the whitespace skip removed it fails with `[]`.
### yy_then_p_in_records_mode_copies_between_real_records
improvise-6p8q. At `gh/main` `9e78051` it failed at the yank: `left: None,
+68
View File
@@ -1293,3 +1293,71 @@ 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.
## `view/layout.rs`, `model/types.rs` — a collapsed group is one aggregate row (improvise-6qet)
Reasoning in `context/code/core.md` §14. The bug is one statement: `expand_category`
`continue`d past every item of a collapsed group, so the group had no data row and
the cursor-addressed `z` could never name it. Restoring that `continue` against the
fixed tree fails fifteen tests across `improvise-core`, `improvise-io` and
`improvise-command` — every test below that touches the layout.
### a_collapsed_group_contributes_one_aggregate_row
The inversion of `collapsed_group_has_header_but_no_data_items`, which asserted the
premise 6qet changed and was rewritten rather than deleted. **Two** groups, one
collapsed and one not, so `folds_a_group` is asserted `true` for the fold and
`false` for `Apr` under the open group — a predicate that answered `true` for
everything under a header would pass a one-group fixture. Also asserts the fold
sits under its own header (`row_group_for(0) == Q1`), which is what lets `z` name
it.
### a_non_contiguous_collapsed_group_folds_once
`A[G1], B[G2], C[G1]`: a group's items need not be contiguous. Rival: dedup only
against the *previous* group (right for the contiguous case, which every other
fixture is) — `G1` then folds twice, under two headers, and only this test fails.
### a_category_nested_under_a_collapsed_group_is_folded_too
`Month × Region` on Row, `Q1` collapsed: `Q1/North` is a fold too, because no
record carries `Month=Q1` whatever sits beside it. Rival: the `collapsed` flag not
inherited down `cross_product` — only this test fails, and without it the TUI would
offer `Q1/North` for editing.
### model::types::model_tests::a_collapsed_groups_cell_aggregates_its_items
`Jan=3, Feb=4` under `Q1`, `Apr=100` under `Q2`: the fold is `7`. That value
separates the real fold from absent (`None`), from pass-through of the first member
(`3`), from a count (`2`) and from a fold that leaks another group (`107`).
Also the witness for the trap the first implementation of this ticket fell into:
reading membership off `Category::groups` rather than each item's `group` field.
`add_item_in_group` registers no `Group`, so that rival answers "not a group" for
this fixture and the value is absent — this test, the next, and
`a_collapsed_groups_header_survives_pruning` fail (the fold's row prunes as empty).
### model::types::model_tests::a_groups_stored_values_fold_once_not_per_member
Under `Axis::None` (`Store`), `Avg` and `Count` over `Jan/A=2, Jan/B=4, Feb/A=9`.
The group's average is `5` and its count `3`; folding the members' aggregates gives
`avg(3, 9) = 6` and `count(2, 1) = 2`. Rival: always fold members' results (the
formula-target branch taken for stored values) — this test fails. `Sum` cannot see
the difference, which is why the fixture sets `measure_agg`.
### model::types::model_tests::an_item_named_like_a_group_is_read_as_the_item
An item literally named `Q1` beside the group `Q1`: the item wins and reads `50`, so
an existing document with such an item is unchanged. Rival: drop the
`items.contains_key` short-circuit — the key expands to `Jan + Feb = 7` and this
test fails.
#### `a_collapsed_groups_header_survives_pruning`, re-aimed
Re-aimed by improvise-6qet (the entry above, under improvise-lhi4, still holds for
why the fixture carries an unfunded `Mar`). The expected sequence now includes the
fold, `DataItem { ["Q1"], collapsed: true }`, and `row_group_for(0)` is `Q1` — the
fold is the row `z` names the group from. Note the narrowing this implies: an
**empty** collapsed group now has an empty fold row, which `prune_empty` drops, and
its header goes with it. lhi4 kept that header because it was the only way back to
the rows; the fold is now that way back.
+10
View File
@@ -1424,3 +1424,13 @@ 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.
### export_writes_a_collapsed_group_as_its_own_total_row
improvise-6qet (`context/code/core.md` §14). An export writes what the grid draws
(improvise-iot6), and the grid now draws a collapsed group as one row whose cell
folds the group, so the CSV body is exactly `["Q1,7.00"]` (Jan 3 + Feb 4). Decided
rather than defaulted: the alternative — expanding the group in the export — would
reintroduce the iot6 divergence between what the user sees and what they export.
On the pre-6qet tree the body is empty (the group had no row, and pruning left
nothing), so the test fails there.
+60
View File
@@ -969,3 +969,63 @@ That assertion is the discriminating one: a fold arm that removed the key would
give `None` and fail. Asserting only that `"command"` survived would pass under
either behaviour, which is what the old spelling amounted to for the cleared
key.
### vocabulary_is_composed::a_variant_named_only_in_a_comment_or_string_is_not_composed
improvise-c7mu, found by the post-merge review of PR #160. `constructed_in` ran
over raw source text, so any `Command::X` counted as a composer, including one
inside a comment or a string. The review deleted the real `Command::SetYanked`
composer from `resolve.rs`, left `// TODO: Command::SetYanked(value)` in its
place, and all four tests passed. The test feeds the whole per-file pipeline a
line comment, a block comment and a string that each name a variant, and
asserts that none of them counts. A positive control asserts that a real
construction still does.
Fault injected: `code_only` made the identity function. This test failed with
`["SetYanked", "SetSearchMode", "ChangeMode"]`, and so did
`source_scan::tests::comments_and_literals_are_blanked_and_code_is_kept`. The
four whole-tree tests stayed green under that fault. This confirms the review's
finding that no live comment gives a wrong verdict *today*, and it is the
reason the property has to be pinned on a fixture rather than on the tree.
The fault on the real tree (deleting `resolve.rs`'s composer) was not rerun
here. `resolve.rs` is outside this lane, and the edit was refused as another
agent's ground.
### source_scan::tests::a_test_only_module_file_is_not_production_code
improvise-c7mu, the second half. `#[cfg(test)] mod x;` makes a whole file test
code, and nothing inside the file says so. The old scan saw it as production,
and that included this checker's own file. The test builds a fixture tree:
`lib.rs` declares `mod p;` and `#[cfg(test)] mod t;`, and `t.rs` declares a
nested `mod deeper;`. It asserts that `production_code` returns exactly
`lib.rs` and `p.rs`.
Fault injected: the file filter in `production_code` made always-true. This
test failed with `["lib.rs", "p.rs", "t.rs", "t/deeper.rs"]`. Before this
fixture existed, the `improvise-protocol` and `improvise-command` lib suites
**stayed green** under the same fault, and so did
`test_only_items_and_modules_are_not_production`, which checks the helpers one
by one. The wiring between them was untested until this fixture existed.
### reduce::tests::every_model_command_field_is_position_free
improvise-hkbc. improvise-6nsd's "a stale layout tier cannot misdirect a
mutation" holds only while no `ModelCommand` carries a row or column. The guard
is the signature of `position_free`. It destructures every variant with no `..`,
down into `RecordCell`, and hands each field to a function that accepts only
`&str`, a path, a `RecordHash`, a `CellValue`, an `Axis` or an optional name.
The `#[test]` only keeps that signature compiled and reached.
Faults injected, one per half, each compiling only `improvise-protocol`:
- A new variant `ScrollTo { row: usize }`, also added to `records_named`'s
or-pattern the way an author would. The lib built. The test build failed
with `non-exhaustive patterns: ScrollTo` **at `position_free` and nowhere
else**.
- The same variant classified as `ScrollTo { row } => name(row)`. The test
build failed with `expected &str, found &usize`.
A field added to an existing variant is caught too, but not in isolation,
because every constructor of that variant also fails. Reverted with
`git restore`.
+59
View File
@@ -551,3 +551,62 @@ forward.
**Checked by hand, not in the suite:** the ledger prunes an entry once the key
is re-added and once HEAD stops having it (a committed removal).
### The kb completeness guard's follow-ups — `scripts/kb-test.sh` cases 19b, 20b, 23–25
improvise-ih3k, improvise-yea8 and improvise-za97, filed by the review of PR
#166. The argument is in `context/code/kb-tool.md`. This entry covers what each
case was watched to catch.
**Reproduced at `f909cad` before any fix.** The new cases 23, 23c and 24 gave
40 passed, 3 failed. 23 failed because an add onto a kb that had lost one of
two `work_done(<fixture id>,bug_fix,…)` clauses exited 0. 23c failed
because `rm` of that paired key was refused as "a clause was dropped": its
expected total assumed one clause per key. The tickets did not predict that
one, and it meant a pair could not be resolved through the tool at all. 24
failed because the stale ledger entry excused the loss.
**The ticket's own reproduction defeats a content stamp.** The first fix
stamped the ledger with the kb's sha256, and case 24 still failed. `kb.pl` is
canonical, so `grep -v` of one clause from HEAD is byte-identical to what `kb
rm` of that clause writes. The stamp therefore also carries the inode and the
nanosecond mtime.
**Case 20b had to change, and why that is not a weakening.** It made its
second loss with `sed -i` on the kb. Under the fix, any edit outside the tool
voids the ledger, so 20b then refused two clauses instead of one and failed on
its "1 of" assertion. It was testing the new voiding rather than what its
comment names. The second loss now comes from giving the *baseline* an extra
clause (`KB_BASELINE`). The kb file stays exactly as the tool wrote it, the
ledger stays valid, and the case again isolates "excuses only what it names".
It also now asserts the unexcused clause is named.
**Rivals injected, one at a time. Each failed only the cases listed:**
- keys compared as a set (`min(1, N)` per key) in `complete`: 23 only.
- `rm`'s expected total back to `before - |delta|`: 23c's rm step only.
- ledger pruned by key presence rather than count: 23c's final write only.
- ledger honoured whenever it exists: 24 and 25.
- ledger stamped by content alone: 24 only. 25 cannot see it, because its
intervening `add` makes the restored-then-lost file differ in content
anyway. 24 is the case that carries identity.
- ledger re-stamped only by `rm`, keeping the old stamp on other writes: 24b
(as named), 20b and 23c. All three write again after an add.
- ledger stamped with the kb as read rather than the candidate: 20, 20b, 23c,
24b and 25. That is wider than any one case, as expected, since no later
write can ever match.
- the `gh/main` `scripts/kb` (old refusal text, no stamp): 19b, 24 and 25.
**Case 25 is the first to run the production paths.** Every other case sets
`KB_FILE`, so `ledger_path`'s `git rev-parse --git-path` branch and
`baseline_to`'s `git show HEAD` were unexercised. It copies the tool into a
throwaway `git init` repo and checks four things. `rm` records to the git-path
ledger. `git status` shows only `kb.pl` modified, so the ledger is invisible
to git. A later `add` is excused. After `git checkout --` and a hand loss, the
`add` is refused.
**Checked by hand, not in the suite:** a `SIGKILL` at the publish point (the
`KB_TEST_KILL` seam) on a copy inside `prolog-sessions/` leaves the candidate,
its `.base`, `.ledger`, `.removed` and `.stamped` siblings and `.lock`. `git
status --untracked-files=all` and `git add -A --dry-run` both show none of
them, and `git check-ignore -v` names `.gitignore:40` (`prolog-sessions/*`).
+175 -27
View File
@@ -1,9 +1,10 @@
use crate::model::cell::CellValue;
use crate::ui::app::AppMode;
use crate::ui::effect::{self, Effect, ViewTarget};
use crate::view::AxisEntry;
use crate::view::{AxisEntry, GridLayout};
use super::core::{Cmd, CmdContext};
use super::navigation::viewport_effects;
#[cfg(test)]
mod tests {
@@ -33,10 +34,12 @@ mod tests {
wb
}
/// The real path: the App's own context, the command, then `apply_effects`,
/// so a placement the toggle's reframe throws away is seen.
fn toggle_group_at_cursor(app: &mut crate::ui::app::App) -> String {
use crate::command::key::{KeyCode, KeyModifiers};
let effects = {
let reg = make_registry();
let ctx = make_ctx(&app.model_state.workbook, &app.layout, &reg);
let ctx = app.cmd_context(KeyCode::Char('z'), KeyModifiers::NONE);
ToggleGroupAtCursor { is_row: true }.execute(&ctx)
};
let dbg = effects_debug(&effects);
@@ -44,6 +47,12 @@ mod tests {
dbg
}
fn row_labels(app: &crate::ui::app::App) -> Vec<String> {
(0..app.layout.row_count())
.map(|r| app.layout.row_label(r))
.collect()
}
/// Evidence: `context/test-evidence/command.md`.
#[test]
fn toggling_at_the_cursor_names_the_group_the_cursor_sits_in_and_collapses_it() {
@@ -51,68 +60,157 @@ mod tests {
let mut app = App::new(two_groups_on_the_row_axis(), None);
app.rebuild_layout();
// Off row 0, or a toggle that ignores the cursor names Q1 and passes.
app.cursor_mut().selected = (2, 0);
assert!(
!app.model_state
.workbook
.active_view()
.is_group_collapsed("Month", "Q1"),
.is_group_collapsed("Month", "Q2"),
"the fixture starts expanded, or the assertion below proves nothing"
);
let dbg = toggle_group_at_cursor(&mut app);
assert!(
dbg.contains(r#"ToggleGroup { view: Active, category: "Month", group: "Q1" }"#),
"the cursor sits on Jan, which is in Q1, got: {dbg}"
dbg.contains(r#"ToggleGroup { view: Active, category: "Month", group: "Q2" }"#),
"the cursor sits on Apr, which is in Q2, got: {dbg}"
);
let view = app.model_state.workbook.active_view();
assert!(
view.is_group_collapsed("Month", "Q2"),
"the effect was returned but the view did not move"
);
assert!(
app.model_state
.workbook
.active_view()
.is_group_collapsed("Month", "Q1"),
"the effect was returned but the view did not move"
!view.is_group_collapsed("Month", "Q1"),
"the neighbour was left alone"
);
}
/// Evidence: `context/test-evidence/command.md`.
#[test]
fn a_second_toggle_at_the_same_cursor_names_the_next_group_not_the_collapsed_one() {
fn toggling_a_group_collapses_that_group_and_the_cursor_follows_it() {
use crate::ui::app::App;
let mut app = App::new(two_groups_on_the_row_axis(), None);
app.rebuild_layout();
toggle_group_at_cursor(&mut app);
app.cursor_mut().selected = (2, 0);
assert_eq!(
app.layout.row_label(2),
"Apr",
"the cursor starts inside Q2"
);
let dbg = toggle_group_at_cursor(&mut app);
assert!(dbg.contains(r#"group: "Q2""#), "Apr is in Q2, got: {dbg}");
assert_eq!(row_labels(&app), ["Jan", "Feb", "Q2"]);
assert_eq!(app.cursor().selected, (2, 0), "the cursor is on the Q2 row");
let dbg = toggle_group_at_cursor(&mut app);
assert!(
dbg.contains(r#"group: "Q2""#),
"collapsing Q1 removes its rows, so data row 0 is now Apr in Q2, got: {dbg}"
"z again names Q2, got: {dbg}"
);
let view = app.model_state.workbook.active_view();
assert!(
view.is_group_collapsed("Month", "Q1") && view.is_group_collapsed("Month", "Q2"),
"pressing twice collapses both groups; it is not an involution"
assert_eq!(
row_labels(&app),
["Jan", "Feb", "Apr", "May"],
"z z is a no-op"
);
assert_eq!(app.cursor().selected, (2, 0), "back on Q2's first item");
}
/// Evidence: `context/test-evidence/command.md`.
#[test]
fn a_collapse_from_the_tail_of_a_run_lands_on_the_runs_first_row() {
use crate::ui::app::App;
let mut app = App::new(two_groups_on_the_row_axis(), None);
app.rebuild_layout();
app.cursor_mut().selected = (3, 0);
assert_eq!(app.layout.row_label(3), "May");
toggle_group_at_cursor(&mut app);
assert_eq!(row_labels(&app), ["Jan", "Feb", "Q2"]);
assert_eq!(app.cursor().selected, (2, 0), "May folded into the Q2 row");
}
/// Evidence: `context/test-evidence/command.md`.
#[test]
fn collapsing_an_inner_group_lands_where_every_earlier_run_shrank() {
use crate::ui::app::App;
use crate::view::Axis;
let mut wb = crate::workbook::Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
wb.add_item("Region", "North").unwrap();
wb.add_item("Region", "South").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
wb.add_item_in_group("Month", month, quarter).unwrap();
}
wb.add_item("Type", "Food").unwrap();
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("Type", Axis::Column);
let mut app = App::new(wb, None);
app.rebuild_layout();
assert_eq!(
app.layout.row_cats,
["Region", "Month"],
"Month is the INNER level"
);
app.cursor_mut().selected = (6, 0);
assert_eq!(
app.layout.row_tuple(6),
Some(&["South".into(), "Apr".into()][..])
);
toggle_group_at_cursor(&mut app);
assert_eq!(
row_labels(&app),
[
"North/Jan",
"North/Feb",
"North/Q2",
"South/Jan",
"South/Feb",
"South/Q2"
]
);
assert_eq!(
app.cursor().selected,
(5, 0),
"South's Q2, not the old index 6"
);
}
/// Evidence: `context/test-evidence/command.md`.
#[test]
fn a_fully_collapsed_row_axis_leaves_the_command_nothing_to_address() {
fn a_collapsed_group_can_be_re_expanded_from_the_grid() {
use crate::ui::app::App;
let mut app = App::new(two_groups_on_the_row_axis(), None);
app.rebuild_layout();
app.cursor_mut().selected = (2, 0);
toggle_group_at_cursor(&mut app);
app.cursor_mut().selected = (1, 0);
toggle_group_at_cursor(&mut app);
assert_eq!(app.layout.row_count(), 0);
assert_eq!(row_labels(&app), ["Q1", "Q2"], "both groups collapsed");
app.cursor_mut().selected = (1, 0);
let dbg = toggle_group_at_cursor(&mut app);
assert_eq!(
dbg, "[]",
"there is no row under the cursor to name a group"
assert!(
dbg.contains(r#"group: "Q2""#),
"the Q2 row names Q2, got: {dbg}"
);
assert_eq!(row_labels(&app), ["Q1", "Apr", "May"]);
assert_eq!(app.cursor().selected, (1, 0), "on Q2's first item");
}
#[test]
@@ -338,11 +436,61 @@ impl Cmd for ToggleGroupAtCursor {
let Some((cat, group)) = lookup else {
return vec![];
};
vec![Box::new(effect::ToggleGroup {
let landing = self.landing(ctx, &cat, &group);
// The toggle reframes and so resets the cursor; the placement must come
// AFTER it or it is thrown away (gotcha 24, core.md §14).
let mut effects: Vec<Box<dyn Effect>> = vec![Box::new(effect::ToggleGroup {
view: ViewTarget::Active,
category: cat,
group,
})]
})];
effects.extend(
landing
.map(|at| self.place_cursor(ctx, at))
.unwrap_or_default(),
);
effects
}
}
impl ToggleGroupAtCursor {
/// Where the toggled group sits once toggled, asked of the layout the toggle
/// will produce — collapsing an inner level shifts every earlier run (§14).
fn landing(&self, ctx: &CmdContext, cat: &str, group: &str) -> Option<usize> {
let (cats, tuple) = match self.is_row {
true => (&ctx.layout.row_cats, ctx.layout.row_tuple(ctx.selected.0)?),
false => (&ctx.layout.col_cats, ctx.layout.col_tuple(ctx.selected.1)?),
};
let prefix = tuple.get(..cats.iter().position(|c| c == cat)?)?;
let mut view = ctx.view.clone();
view.toggle_group_collapse(cat, group);
let after = GridLayout::with_frozen_records(ctx.model, &view, None);
match self.is_row {
true => after.row_run_start(cat, group, prefix),
false => after.col_run_start(cat, group, prefix),
}
}
/// The cursor on `at`, the scroll restored where the toggle zeroed it and
/// moved only as far as `at` needs.
fn place_cursor(&self, ctx: &CmdContext, at: usize) -> Vec<Box<dyn Effect>> {
let (row, col) = match self.is_row {
true => (at, ctx.selected.1),
false => (ctx.selected.0, at),
};
let mut effects: Vec<Box<dyn Effect>> = vec![
Box::new(effect::SetRowOffset(ctx.row_offset)),
Box::new(effect::SetColOffset(ctx.col_offset)),
];
effects.extend(viewport_effects(
row,
col,
ctx.row_offset,
ctx.col_offset,
ctx.visible_rows,
ctx.visible_cols,
));
effects
}
}
@@ -363,8 +511,8 @@ impl Cmd for HideSelectedRowItem {
.row_items
.iter()
.filter_map(|e| {
if let AxisEntry::DataItem(v) = e {
Some(v)
if let AxisEntry::DataItem { tuple, .. } = e {
Some(tuple)
} else {
None
}
@@ -188,6 +188,39 @@ mod tests {
);
}
/// Evidence: `context/test-evidence/command.md`.
#[test]
fn edit_or_drill_declines_a_collapsed_groups_row() {
let mut wb = crate::workbook::Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2"), ("May", "Q2")] {
wb.add_item_in_group("Month", month, quarter).unwrap();
}
wb.add_item("Type", "Food").unwrap();
wb.active_view_mut().toggle_group_collapse("Month", "Q2");
let layout = make_layout(&wb);
let reg = make_registry();
let mut ctx = make_ctx(&wb, &layout, &reg);
let edit = EditOrDrill {
edit_mode: AppMode::editing(),
};
ctx.selected = (1, 0);
assert!(
effects_debug(&edit.execute(&ctx)).contains("Editing"),
"Feb, beside the fold, still edits"
);
ctx.selected = (2, 0);
let dbg = effects_debug(&edit.execute(&ctx));
assert_eq!(layout.row_label(2), "Q2");
assert!(
dbg.contains("Collapsed group") && !dbg.contains("Editing") && !dbg.contains("Drill"),
"the fold names no record to edit or drill into, got: {dbg}"
);
}
/// improvise-ir7l: the cell under the cursor is a formula target, so there is
/// nothing to type into. `context/test-evidence/command.md`.
#[test]
@@ -320,6 +353,12 @@ impl Cmd for EditOrDrill {
"edit-or-drill"
}
fn execute(&self, ctx: &CmdContext) -> Vec<Box<dyn Effect>> {
// Neither edit nor drill: no record carries a group's name (core.md §14).
if ctx.layout.folds_a_group(ctx.selected.0, ctx.selected.1) {
return vec![effect::set_status(
"Collapsed group — press z to expand it, then edit an item",
)];
}
// Only **regular** categories on None are true aggregation: virtuals like
// `_Index`/`_Dim` are always None in pivot mode and imply nothing.
let regular_none = ctx.none_cats().iter().any(|c| {
+2 -1
View File
@@ -101,9 +101,10 @@ fn frame_of(layout: &crate::view::layout::GridLayout, view: &View) -> u64 {
group_name.hash(&mut h);
view.is_group_collapsed(cat_name, group_name).hash(&mut h);
}
crate::view::layout::AxisEntry::DataItem(tuple) => {
crate::view::layout::AxisEntry::DataItem { tuple, collapsed } => {
1u8.hash(&mut h);
tuple.hash(&mut h);
collapsed.hash(&mut h);
}
// Drawn as nothing, but it ends a group's run (improvise-gd83),
// so where it sits is what a row's group attribution depends on.
+75 -38
View File
@@ -205,7 +205,7 @@ fn axis_frame(
) -> AxisFrame {
let total = items
.iter()
.filter(|e| matches!(e, AxisEntry::DataItem(_)))
.filter(|e| matches!(e, AxisEntry::DataItem { .. }))
.count();
let mut data_idx = 0usize;
let mut entries: Vec<FrameEntry> = Vec::new();
@@ -230,7 +230,7 @@ fn axis_frame(
// Queued like a header, never clearing one: a COLLAPSED group has
// no item to carry it and reaches the frame only through `pending`.
AxisEntry::GroupEnd => pending.push(FrameEntry::GroupEnd),
AxisEntry::DataItem(tuple) => {
AxisEntry::DataItem { tuple, .. } => {
let idx = data_idx;
data_idx += 1;
if window.contains(&idx) {
@@ -332,11 +332,14 @@ fn cell_id(app: &App, layout: &GridLayout, row: usize, col: usize) -> Option<Cel
}));
}
let key = layout.cell_key(row, col)?;
let aggregated = layout.none_cats.iter().any(|cat| {
model
.category(cat)
.is_some_and(|category| category.kind.is_regular())
});
// A folded group is an aggregate whatever is on None: no record carries
// its coordinate, so a `SetCell` there would write a non-item (§14).
let aggregated = layout.folds_a_group(row, col)
|| layout.none_cats.iter().any(|cat| {
model
.category(cat)
.is_some_and(|category| category.kind.is_regular())
});
// A derived cell is asked about before a stored one: a formula target is a
// real coordinate, so the record branch would otherwise claim it (ir7l).
let derived = model.formula_for(&key).map(|f| CellId::Derived {
@@ -1318,8 +1321,8 @@ mod tests {
.collect()
}
/// A collapsed group at the bottom has no items to hang off and is still the
/// only row saying those rows exist, so the last window carries it.
/// A collapsed group at the bottom folds into one row, and the last window
/// carries its header above that row. Evidence: context/test-evidence/command.md.
#[test]
fn a_window_reaching_the_last_row_carries_the_trailing_group_header() {
let app = app_with_a_trailing_group(true);
@@ -1334,13 +1337,13 @@ mod tests {
);
assert_eq!(
grid.row_frame.len(),
2,
"East is inside the collapsed group"
3,
"East folds into the collapsed group's one row"
);
assert_eq!(
frame_shape(&grid.row_frame),
vec!["North", "South", "[Late]"],
"the header is the last entry, and it is what the user expands"
vec!["North", "South", "[Late]", "Late"],
"the header, then the row the group collapsed into"
);
}
@@ -1369,11 +1372,11 @@ mod tests {
/// header comes. Two headers above, or the compaction is invisible.
#[test]
fn a_window_opening_inside_a_group_names_only_the_nearest() {
let app = app_with_consecutive_headers();
let app = app_with_a_collapsed_group_between_rows();
let grid = grid_of(
&app,
Viewport {
row_offset: 1,
row_offset: 2,
col_offset: 0,
rows: 20,
cols: 20,
@@ -1395,9 +1398,9 @@ mod tests {
);
}
/// Two consecutive group headers between two rows: `Hidden` is collapsed, so it
/// emits a header and no items, and `Late`'s follows immediately.
fn app_with_consecutive_headers() -> App {
/// Two group headers between `North` and `East`: `Hidden`, collapsed into one
/// row, and `Late`, open.
fn app_with_a_collapsed_group_between_rows() -> App {
let mut wb = Workbook::new("Consecutive");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
@@ -1420,11 +1423,11 @@ mod tests {
app
}
/// Headers between two in-window rows all come, not just the nearest: the far
/// one is the only row saying those items exist. Above the window, the opposite.
/// Headers between two in-window rows all come, each above the row it opens.
/// Evidence: context/test-evidence/command.md.
#[test]
fn consecutive_headers_between_two_rows_all_come() {
let app = app_with_consecutive_headers();
fn every_header_between_two_in_window_rows_comes() {
let app = app_with_a_collapsed_group_between_rows();
let grid = grid_of(
&app,
Viewport {
@@ -1436,25 +1439,17 @@ mod tests {
);
assert_eq!(
frame_shape(&grid.row_frame),
vec!["North", "[Hidden]", "[Late]", "East"],
"both headers sit between the two rows and both are drawn"
vec!["North", "[Hidden]", "Hidden", "[Late]", "East"],
"both headers are drawn, the collapsed one above its folded row"
);
let group = grid.row_frame.group_of(1).expect("East is under Late");
assert_eq!(group.name, "Late", "the NEAREST header wins membership");
let hidden = grid
.row_frame
.entries
.iter()
.find_map(|e| match e {
FrameEntry::Group(g) if g.name == "Hidden" => Some(g),
_ => None,
})
.expect("kept");
assert!(
hidden.collapsed,
"and it arrives collapsed, which is what makes it the row that \
says those rows still exist"
);
.group_of(1)
.expect("the fold is under Hidden");
assert_eq!(hidden.name, "Hidden");
assert!(hidden.collapsed, "and it arrives collapsed");
let late = grid.row_frame.group_of(2).expect("East is under Late");
assert_eq!(late.name, "Late", "the NEAREST header wins membership");
}
fn app_with_data() -> App {
@@ -4302,6 +4297,48 @@ mod cell_identity {
.unwrap_or_else(|| panic!("cell ({row}, {col}) was projected with no identity"))
}
/// Nothing regular on None, so only the fold can make the row an aggregate.
/// Evidence: context/test-evidence/command.md.
#[test]
fn a_collapsed_groups_row_is_projected_as_an_aggregate() {
let mut wb = Workbook::new("Folded");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
wb.add_item_in_group("Region", "East", "Coastal").unwrap();
wb.add_item_in_group("Region", "West", "Coastal").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
for (region, value) in [("North", 1.0), ("East", 3.0), ("West", 4.0)] {
wb.model.set_cell(
CellKey::new(vec![
("Region".to_string(), region.to_string()),
("_Measure".to_string(), "Revenue".to_string()),
]),
CellValue::Number(value),
);
}
wb.active_view_mut()
.toggle_group_collapse("Region", "Coastal");
let mut app = App::new(wb, None);
app.rebuild_layout();
assert_eq!(app.layout.row_label(1), "Coastal");
assert!(
matches!(id_at(&app, 0, 0), CellId::Record(_)),
"North is stored, so the none-axis arm is not what answers below"
);
assert!(
matches!(id_at(&app, 1, 0), CellId::Aggregate(_)),
"a fold has no record under it and must not be offered as settable"
);
let grid = grid_of(&app, WHOLE);
assert_eq!(
grid.cell(1, 0).expect("the fold's cell").text.trim(),
"7.00"
);
}
fn record_of_id(id: CellId) -> RecordCell {
match id {
CellId::Record(cell) => cell,
@@ -10,6 +10,9 @@ use crate::ui::app::App;
use crate::ui::effect::{self, Effect, ViewTarget};
use crate::ui::project::{Dirty, document_projections};
#[cfg(test)]
mod vocabulary_is_complete;
/// A wire [`Command`] whose basis the authoritative state has answered for
/// (improvise-ondj/7v62, ADR 001). See `context/code/command.md`.
pub struct Checked<'a>(&'a Command);
@@ -0,0 +1,247 @@
//! Every effect is on the wire under its own name, or named below with its
//! verdict: the forward twin of `vocabulary_is_composed.rs`. `context/code/protocol.md` §2.
use std::collections::BTreeSet;
use std::path::Path;
use improvise_protocol::source_scan::{production_code, production_text};
use improvise_protocol::variants::declared;
use improvise_protocol::{ClientFrame, Command, ModelCommand};
/// Why an effect has no same-named `Command`/`ModelCommand` variant.
enum Verdict {
/// On the wire as this `Command` variant, under another name.
Command(&'static str),
/// Answered by this `ClientFrame` variant, which is not a `Command`.
Frame(&'static str),
/// Deliberately off the wire, and the argument.
Off(&'static str),
/// Missing, and the ticket that owns closing it.
Gap(&'static str),
}
/// Checked in all directions: an entry must name a real effect, must not be on
/// the wire by name, and a `Command`/`Frame` verdict must name a real variant.
const NOT_ON_THE_WIRE: &[(&str, Verdict)] = &[
("HelpPageNext", Verdict::Command("HelpPage")),
("HelpPagePrev", Verdict::Command("HelpPage")),
("HelpPageSet", Verdict::Command("HelpPage")),
("Save", Verdict::Frame("Save")),
(
"SaveAs",
Verdict::Gap("improvise-2mqz: writes the host's Storage and has no frame"),
),
(
"ExportCsv",
Verdict::Gap("improvise-2mqz: writes the host's Storage and has no frame"),
),
(
"ApplyAndClearDrill",
Verdict::Off("operand is one session's staged drill edits (gotcha 38, improvise-3x3u)"),
),
(
"AcceptWizardModel",
Verdict::Off("operand is one session's built wizard workbook (gotcha 38, improvise-3x3u)"),
),
(
"StartDrill",
Verdict::Off("writes drill_state, session-local residue (gotcha 30)"),
),
(
"SetDrillPendingEdit",
Verdict::Off("writes drill_state.pending_edits, session-local residue (gotcha 30)"),
),
(
"StartImportWizard",
Verdict::Off("writes App::wizard, session-local residue (gotcha 30, improvise-764)"),
),
(
"WizardApply",
Verdict::Off("writes App::wizard, session-local residue (gotcha 30, improvise-764)"),
),
(
"CloseWizard",
Verdict::Off("writes App::wizard, session-local residue (gotcha 30, improvise-764)"),
),
(
"RebuildLayout",
Verdict::Off(
"refreshes the host's layout cache mid-batch; a client holds none (gotcha 23)",
),
),
(
"AbortChain",
Verdict::Off("control flow inside one apply_effects batch; it writes no state (gotcha 21)"),
),
(
"ReturnToNormalUnlessMoved",
Verdict::Off("relative to the batch it ends (gotcha 31); a client composes ChangeMode"),
),
(
"EnterEditAtCursor",
Verdict::Off(
"reads the host's layout; a client composes SetBuffer + ChangeMode off its own cache",
),
),
];
#[test]
fn every_effect_is_on_the_wire_or_has_a_verdict() {
let on_the_wire = wire_names();
let missing: Vec<String> = effects_in_production()
.into_iter()
.filter(|name| !on_the_wire.contains(name))
.filter(|name| !NOT_ON_THE_WIRE.iter().any(|(n, _)| n == name))
.collect();
assert!(
missing.is_empty(),
"effect::{missing:?} has no Command or ModelCommand of its name and no \
verdict. Put it on the wire, or name it in NOT_ON_THE_WIRE with the \
argument (improvise-h3y7) — gotcha 38's ViewBack was exactly this"
);
}
#[test]
fn the_verdict_list_names_nothing_that_is_on_the_wire() {
let on_the_wire = wire_names();
let stale: Vec<&str> = NOT_ON_THE_WIRE
.iter()
.map(|(name, _)| *name)
.filter(|name| on_the_wire.contains(*name))
.collect();
assert!(
stale.is_empty(),
"effect::{stale:?} has a verdict and IS on the wire. \
doctrine(a_stale_list_of_what_is_missing_is_worse_than_no_list)"
);
}
#[test]
fn the_verdict_list_names_only_real_effects_and_real_variants() {
let effects = effects_in_production();
let commands = declared::<Command>();
let frames = declared::<ClientFrame>();
for (name, verdict) in NOT_ON_THE_WIRE {
assert!(effects.contains(*name), "no effect is named {name}");
match verdict {
Verdict::Command(variant) => assert!(
commands.iter().any(|c| c == variant),
"{name}'s verdict names Command::{variant}, which does not exist"
),
Verdict::Frame(variant) => assert!(
frames.iter().any(|f| f == variant),
"{name}'s verdict names ClientFrame::{variant}, which does not exist"
),
Verdict::Off(why) | Verdict::Gap(why) => {
assert!(!why.is_empty(), "{name}'s verdict gives no argument")
}
}
}
}
/// The scan must reach both spellings of an effect and every file that holds
/// one — the h3y7 audit's brace-matcher missed nine that way (check_the_checker).
#[test]
fn the_scan_reaches_every_way_an_effect_is_declared() {
let effects = effects_in_production();
for name in ["TransposeAxes", "SetRowOffset", "SetTransientPrefix"] {
assert!(
effects.contains(name),
"the scan missed {name}, so it under-reports: {effects:?}"
);
}
}
fn wire_names() -> BTreeSet<String> {
declared::<Command>()
.into_iter()
.chain(declared::<ModelCommand>())
.collect()
}
/// Every `impl Effect for X` and `simple_effect!(X` in this crate's production
/// `src/`: the one scanner both vocabulary guards share (improvise-c7mu).
fn effects_in_production() -> BTreeSet<String> {
production_code(&[Path::new(env!("CARGO_MANIFEST_DIR")).join("src")])
.iter()
.flat_map(|(_, code)| effects_named(code))
.collect()
}
/// One file's effects. A comment, a string or a test fake naming one is not
/// one, or a stale verdict could outlive the effect it names.
fn effects_in(text: &str) -> Vec<String> {
effects_named(&production_text(text))
}
fn effects_named(code: &str) -> Vec<String> {
let mut found = Vec::new();
for marker in ["impl Effect for ", "simple_effect!("] {
for (at, _) in code.match_indices(marker) {
let name = name_after(&code[at + marker.len()..]);
if name.starts_with(|c: char| c.is_ascii_uppercase()) {
found.push(name);
}
}
}
found
}
/// The identifier a marker introduces. `simple_effect!` takes `$(#[$meta])*`
/// before the name, and a doc comment is one: skip them (improvise-94x2).
fn name_after(rest: &str) -> String {
let mut rest = rest.trim_start();
while let Some(attr) = rest.strip_prefix("#[") {
rest = attr[closing_bracket(attr) + 1..].trim_start();
}
rest.chars()
.take_while(|c| c.is_ascii_alphanumeric() || *c == '_')
.collect()
}
/// Where the `]` that closes an attribute's `#[` falls in `inside`, the text
/// after it. Nested brackets count; an unclosed one runs to the end.
fn closing_bracket(inside: &str) -> usize {
let mut depth = 1usize;
for (i, c) in inside.char_indices() {
match c {
'[' => depth += 1,
']' if depth == 1 => return i,
']' => depth -= 1,
_ => {}
}
}
inside.len().saturating_sub(1)
}
/// The spelling the macro itself documents, a doc comment and an attribute
/// before the name, and a line break after `(`. Each hid an effect.
#[test]
fn an_attributed_or_wrapped_effect_is_still_an_effect() {
let text = concat!(
"simple_effect!(\n",
" /// Documented, as the macro's own matcher allows.\n",
" #[allow(dead_code)]\n",
" #[cfg_attr(all(), doc = \"x\")]\n",
" Documented(bool) => |app| x, K\n",
");\n",
"simple_effect!(\n Wrapped(bool) => |app| y, K);\n",
);
assert_eq!(effects_in(text), vec!["Documented", "Wrapped"]);
}
#[test]
fn an_effect_named_only_in_a_comment_or_string_is_not_an_effect() {
let text = concat!(
"// impl Effect for SaveAs {}\n",
"/// simple_effect!(SetYanked(Option<CellValue>) => |app| x, K);\n",
"let s = \"impl Effect for ExportCsv\";\n",
"impl Effect for Real {}\n",
"simple_effect!(AlsoReal(bool) => |app| y, K);\n",
"#[cfg(test)]\n",
"mod tests {\n",
" impl Effect for TestFake {}\n",
"}\n",
);
assert_eq!(effects_in(text), vec!["Real", "AlsoReal"]);
}
+2 -2
View File
@@ -123,7 +123,7 @@ fn cell_value(
fn header_level_seed(layout: &GridLayout, n: usize) -> Vec<u16> {
let mut widths = vec![0u16; n];
let items = layout.col_items.iter().filter_map(|e| match e {
AxisEntry::DataItem(v) => Some(v),
AxisEntry::DataItem { tuple, .. } => Some(tuple),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None,
});
for (ci, levels) in items.enumerate().take(n) {
@@ -152,7 +152,7 @@ pub fn compute_col_widths(model: &Model, layout: &GridLayout, plan: &FormatPlan)
/// keypress-path caller that borrows tuples — see `context/code/command.md`.
pub fn compute_row_header_width(layout: &GridLayout) -> u16 {
let items = layout.row_items.iter().filter_map(|e| match e {
AxisEntry::DataItem(v) => Some(v),
AxisEntry::DataItem { tuple, .. } => Some(tuple),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None,
});
header_level_widths(items, layout.row_cats.len().max(1))
@@ -199,6 +199,16 @@ impl Category {
pub fn ordered_item_names(&self) -> Vec<&str> {
self.items.keys().map(|s| s.as_str()).collect()
}
/// The items whose own `group` names `group`, in insertion order. Read off the
/// items, never `groups`: `add_item_in_group` registers no `Group` (core.md §14).
pub fn items_in_group(&self, group: &str) -> Vec<&str> {
self.items
.values()
.filter(|i| i.group.as_deref() == Some(group))
.map(|i| i.name.as_str())
.collect()
}
}
#[cfg(test)]
+127
View File
@@ -523,6 +523,10 @@ impl Model {
}
pub fn evaluate_aggregated(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
let members = self.group_expansion(key);
if !members.is_empty() {
return self.evaluate_group(key, &members, agg);
}
if agg == Aggregation::Exact {
return self.evaluate(key);
}
@@ -536,6 +540,55 @@ impl Model {
aggregate_cell_values(&self.data.matching_values(&key.0), self.agg_for(key))
}
/// One key per member where `key` names a collapsed GROUP rather than an item —
/// the cross product over every such coordinate. Empty when none does (§14).
fn group_expansion(&self, key: &CellKey) -> Vec<CellKey> {
let mut expanded = vec![key.clone()];
for (cat_name, name) in &key.0 {
let members = self
.category(cat_name)
.filter(|cat| !cat.items.contains_key(name.as_str()))
.map(|cat| cat.items_in_group(name))
.unwrap_or_default();
if members.is_empty() {
continue;
}
expanded = expanded
.iter()
.flat_map(|k| members.iter().map(|m| k.clone().with(cat_name, *m)))
.collect();
}
match expanded.as_slice() {
[only] if only == key => Vec::new(),
_ => expanded,
}
}
/// A group's cell: stored values fold ONCE over every record the group covers,
/// a derived one folds its members' results — why the two differ: core.md §14.
fn evaluate_group(
&self,
key: &CellKey,
members: &[CellKey],
agg: Aggregation,
) -> Option<CellValue> {
let values: Vec<CellValue> = match self.formula_for(key) {
Some(_) => members
.iter()
.filter_map(|k| self.evaluate_aggregated(k, agg))
.collect(),
None => members
.iter()
.flat_map(|k| match agg {
Aggregation::Exact => self.data.get(k).into_iter().collect(),
Aggregation::OverHidden => self.data.matching_values(&k.0),
})
.cloned()
.collect(),
};
aggregate_cell_values(&values.iter().collect::<Vec<_>>(), self.agg_for(key))
}
/// [`Self::evaluate_aggregated`] as `f64`, with `0.0` for an empty cell.
pub fn evaluate_aggregated_f64(&self, key: &CellKey, agg: Aggregation) -> f64 {
self.evaluate_aggregated(key, agg)
@@ -1403,6 +1456,80 @@ mod model_tests {
let result = m.evaluate_aggregated(&key, Aggregation::of(&none_cats));
assert_eq!(result, Some(CellValue::Number(20.0))); // avg(10, 30) = 20
}
/// `Month` with `Q1 = {Jan, Feb}` built through `add_item_in_group`, which
/// registers no `Group` — the shape a document built in code has.
fn quarter_model(cells: &[(&str, &str, f64)]) -> Model {
let mut m = Model::new("T");
m.add_category("Month").unwrap();
m.add_category("Store").unwrap();
for (month, quarter) in [("Jan", "Q1"), ("Feb", "Q1"), ("Apr", "Q2")] {
m.category_mut("Month")
.unwrap()
.add_item_in_group(month, quarter);
}
for store in ["A", "B"] {
m.category_mut("Store").unwrap().add_item(store);
}
m.category_mut("_Measure").unwrap().add_item("Revenue");
for (month, store, value) in cells {
m.set_cell(
coord(&[("Month", month), ("Store", store), ("_Measure", "Revenue")]),
CellValue::Number(*value),
);
}
m
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn a_collapsed_groups_cell_aggregates_its_items() {
let m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0), ("Apr", "A", 100.0)]);
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "Revenue")]);
assert_eq!(
m.evaluate_aggregated(&key, Aggregation::Exact),
Some(CellValue::Number(7.0)),
"Jan + Feb; not absent, not Jan alone, not a count, not Q2's Apr"
);
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn a_groups_stored_values_fold_once_not_per_member() {
use crate::formula::AggFunc;
let mut m = quarter_model(&[("Jan", "A", 2.0), ("Jan", "B", 4.0), ("Feb", "A", 9.0)]);
let key = coord(&[("Month", "Q1"), ("_Measure", "Revenue")]);
let over_store = Aggregation::of(&["Store".to_string()]);
m.measure_agg.insert("Revenue".to_string(), AggFunc::Avg);
assert_eq!(
m.evaluate_aggregated(&key, over_store),
Some(CellValue::Number(5.0)),
"avg(2, 4, 9); folding the members' averages gives avg(3, 9) = 6"
);
m.measure_agg.insert("Revenue".to_string(), AggFunc::Count);
assert_eq!(
m.evaluate_aggregated(&key, over_store),
Some(CellValue::Number(3.0)),
"three records; counting the members' counts gives 2"
);
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn an_item_named_like_a_group_is_read_as_the_item() {
let mut m = quarter_model(&[("Jan", "A", 3.0), ("Feb", "A", 4.0)]);
m.category_mut("Month").unwrap().add_item("Q1");
m.set_cell(
coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "Revenue")]),
CellValue::Number(50.0),
);
let key = coord(&[("Month", "Q1"), ("Store", "A"), ("_Measure", "Revenue")]);
assert_eq!(
m.evaluate_aggregated(&key, Aggregation::Exact),
Some(CellValue::Number(50.0)),
"the stored item, not Jan + Feb"
);
}
}
#[cfg(test)]
+206 -39
View File
@@ -187,7 +187,7 @@ pub enum RecordCol {
}
/// One entry on a grid axis. A `GroupHeader` is always visible so its label
/// and collapse state show; a `DataItem` is absent when its group collapses.
/// and collapse state show; a collapsed group is ONE `DataItem` (core.md §14).
#[derive(Debug, Clone, PartialEq)]
pub enum AxisEntry {
GroupHeader {
@@ -197,18 +197,73 @@ pub enum AxisEntry {
/// The run opened by the last `GroupHeader` ends here (improvise-gd83).
/// Nothing is drawn for it; it exists so membership is not inherited.
GroupEnd,
DataItem(Vec<String>),
/// `collapsed` is not derivable from `tuple`: a group and an item may share a
/// name. Set for a folded group and for everything expanded beneath one.
DataItem { tuple: Vec<String>, collapsed: bool },
}
impl AxisEntry {
/// An ordinary, uncollapsed data item.
pub fn item(tuple: Vec<String>) -> Self {
AxisEntry::DataItem {
tuple,
collapsed: false,
}
}
}
/// The data-item tuples on an axis, skipping group headers. Every public
/// `row`/`col` argument indexes this sequence, not the visual entry list.
fn data_items(items: &[AxisEntry]) -> impl Iterator<Item = &Vec<String>> {
items.iter().filter_map(|e| match e {
AxisEntry::DataItem(v) => Some(v),
AxisEntry::DataItem { tuple, .. } => Some(tuple),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None,
})
}
/// Whether the data item at `idx` folds a collapsed group.
fn folds(items: &[AxisEntry], idx: usize) -> bool {
items
.iter()
.filter_map(|e| match e {
AxisEntry::DataItem { collapsed, .. } => Some(*collapsed),
AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd => None,
})
.nth(idx)
.unwrap_or(false)
}
/// The data index of the first item in the run `cat`/`group` opens under
/// `prefix` — where a toggle's cursor lands, collapsing or expanding (§14).
fn run_start(
cats: &[String],
items: &[AxisEntry],
cat: &str,
group: &str,
prefix: &[String],
) -> Option<usize> {
let level = cats.iter().position(|c| c == cat)?;
let mut data_idx = 0usize;
let mut armed = false;
for entry in items {
match entry {
AxisEntry::GroupHeader {
cat_name,
group_name,
} => armed |= cat_name == cat && group_name == group,
AxisEntry::GroupEnd => {}
AxisEntry::DataItem { tuple, .. } => {
if armed && tuple.get(..level) == Some(prefix) {
return Some(data_idx);
}
armed = false;
data_idx += 1;
}
}
}
None
}
/// The display label of the data item at `idx`, or empty when out of bounds.
fn axis_label(items: &[AxisEntry], idx: usize) -> String {
data_items(items)
@@ -222,7 +277,7 @@ fn data_index_to_visual(items: &[AxisEntry], data_idx: usize) -> Option<usize> {
items
.iter()
.enumerate()
.filter(|(_, e)| matches!(e, AxisEntry::DataItem(_)))
.filter(|(_, e)| matches!(e, AxisEntry::DataItem { .. }))
.map(|(vi, _)| vi)
.nth(data_idx)
}
@@ -267,7 +322,7 @@ fn prune_axis_items(items: &mut Vec<AxisEntry>, keep: impl Fn(usize) -> bool) {
run_open = false;
}
}
AxisEntry::DataItem(_) => {
AxisEntry::DataItem { .. } => {
run_had_item = true;
if keep(data_idx) {
if let Some(h) = pending_header.take() {
@@ -344,7 +399,7 @@ impl GridLayout {
&& let Some(records) = frozen_records
{
let row_items: Vec<AxisEntry> = (0..records.len())
.map(|i| AxisEntry::DataItem(vec![i.to_string()]))
.map(|i| AxisEntry::item(vec![i.to_string()]))
.collect();
layout.row_items = row_items;
layout.records = Some(records);
@@ -427,13 +482,13 @@ impl GridLayout {
let records = Self::group_records(model, &page_constraints(&page_coords));
let row_items: Vec<AxisEntry> = (0..records.len())
.map(|i| AxisEntry::DataItem(vec![i.to_string()]))
.map(|i| AxisEntry::item(vec![i.to_string()]))
.collect();
let record_cols = Self::records_columns(model);
let col_items: Vec<AxisEntry> = record_cols
.iter()
.map(|c| AxisEntry::DataItem(vec![Self::record_col_label(c)]))
.map(|c| AxisEntry::item(vec![Self::record_col_label(c)]))
.collect();
Self {
@@ -687,11 +742,38 @@ impl GridLayout {
group_name,
} => Some(Some((cat_name.clone(), group_name.clone()))),
AxisEntry::GroupEnd => Some(None),
AxisEntry::DataItem(_) => None,
AxisEntry::DataItem { .. } => None,
})
.flatten()
}
/// Whether the cell at `(row, col)` folds a collapsed group: an aggregate
/// with no record under it. The one home of that answer (§14).
pub fn folds_a_group(&self, row: usize, col: usize) -> bool {
folds(&self.row_items, row) || folds(&self.col_items, col)
}
/// The first data row of `cat`/`group`'s run under `prefix` — where a toggle
/// of that group puts the cursor, asked of the layout AFTER the toggle (§14).
pub fn row_run_start(&self, cat: &str, group: &str, prefix: &[String]) -> Option<usize> {
run_start(&self.row_cats, &self.row_items, cat, group, prefix)
}
/// [`Self::row_run_start`] for a column group.
pub fn col_run_start(&self, cat: &str, group: &str, prefix: &[String]) -> Option<usize> {
run_start(&self.col_cats, &self.col_items, cat, group, prefix)
}
/// The coordinate tuple of the Nth data row.
pub fn row_tuple(&self, row: usize) -> Option<&[String]> {
data_items(&self.row_items).nth(row).map(Vec::as_slice)
}
/// The coordinate tuple of the Nth data column.
pub fn col_tuple(&self, col: usize) -> Option<&[String]> {
data_items(&self.col_items).nth(col).map(Vec::as_slice)
}
/// The `(cat_name, group_name)` of the nearest `GroupHeader` preceding
/// the Nth data column.
pub fn col_group_for(&self, data_col: usize) -> Option<(String, String)> {
@@ -705,25 +787,28 @@ impl GridLayout {
group_name,
} => Some(Some((cat_name.clone(), group_name.clone()))),
AxisEntry::GroupEnd => Some(None),
AxisEntry::DataItem(_) => None,
AxisEntry::DataItem { .. } => None,
})
.flatten()
}
}
/// Expand one category into `AxisEntry` values under a coordinate prefix: a
/// `GroupHeader` per boundary, then the visible, non-collapsed items.
/// `GroupHeader` per boundary, then each visible item or collapsed group (§14).
fn expand_category(
model: &Model,
view: &View,
cat_name: &str,
prefix: Vec<String>,
prefix_collapsed: bool,
) -> Vec<AxisEntry> {
let Some(cat) = model.category(cat_name) else {
return vec![];
};
let mut result = Vec::new();
let mut last_group: Option<&str> = None;
// A group's items need not be contiguous, so a collapsed one folds ONCE.
let mut folded: Vec<&str> = Vec::new();
// effective_item_names, so _Measure picks up formula targets.
let effective_names = model.effective_item_names(cat_name);
@@ -735,6 +820,10 @@ fn expand_category(
.items
.get(item_name.as_str())
.and_then(|i| i.group.as_deref());
let collapsed = item_group.filter(|g| view.is_group_collapsed(cat_name, g));
if collapsed.is_some_and(|g| folded.contains(&g)) {
continue;
}
// A header on entering a group; a `GroupEnd` only on leaving one for
// NO group — another group's header already ends the run.
@@ -751,14 +840,18 @@ fn expand_category(
last_group = item_group;
}
// Skip the data item if its group is collapsed.
if item_group.is_some_and(|g| view.is_group_collapsed(cat_name, g)) {
continue;
let mut tuple = prefix.clone();
match collapsed {
Some(g) => {
folded.push(g);
tuple.push(g.to_string());
}
None => tuple.push(item_name.to_string()),
}
let mut row = prefix.clone();
row.push(item_name.to_string());
result.push(AxisEntry::DataItem(row));
result.push(AxisEntry::DataItem {
tuple,
collapsed: prefix_collapsed || collapsed.is_some(),
});
}
// The expansion is the end of the run too: the next prefix's items follow
// immediately and must not inherit it (improvise-gd83).
@@ -772,14 +865,16 @@ fn expand_category(
/// `vec![DataItem(vec![])]` when `cats` is empty.
fn cross_product(model: &Model, view: &View, cats: &[String]) -> Vec<AxisEntry> {
if cats.is_empty() {
return vec![AxisEntry::DataItem(vec![])];
return vec![AxisEntry::item(vec![])];
}
let mut result: Vec<AxisEntry> = vec![AxisEntry::DataItem(vec![])];
let mut result: Vec<AxisEntry> = vec![AxisEntry::item(vec![])];
for cat_name in cats {
result = result
.into_iter()
.flat_map(|entry| match entry {
AxisEntry::DataItem(prefix) => expand_category(model, view, cat_name, prefix),
AxisEntry::DataItem { tuple, collapsed } => {
expand_category(model, view, cat_name, tuple, collapsed)
}
marker @ (AxisEntry::GroupHeader { .. } | AxisEntry::GroupEnd) => vec![marker],
})
.collect();
@@ -832,13 +927,13 @@ mod tests {
#[test]
fn prune_axis_items_indexes_data_positions_not_visual_ones() {
let mut items = vec![
AxisEntry::DataItem(vec!["A".into()]),
AxisEntry::item(vec!["A".into()]),
AxisEntry::GroupHeader {
cat_name: "C".into(),
group_name: "G".into(),
},
AxisEntry::DataItem(vec!["B".into()]),
AxisEntry::DataItem(vec!["C".into()]),
AxisEntry::item(vec!["B".into()]),
AxisEntry::item(vec!["C".into()]),
];
// Data index 1 is "B": the header does not consume an index. If the
// predicate saw visual indices, "A" would be the one dropped.
@@ -861,8 +956,8 @@ mod tests {
cat_name: "C".into(),
group_name: "G".into(),
},
AxisEntry::DataItem(vec!["A".into()]),
AxisEntry::DataItem(vec!["B".into()]),
AxisEntry::item(vec!["A".into()]),
AxisEntry::item(vec!["B".into()]),
];
prune_axis_items(&mut items, |_| false);
assert!(items.is_empty());
@@ -1256,29 +1351,92 @@ mod tests {
);
}
/// The inversion of `collapsed_group_has_header_but_no_data_items`: a
/// collapsed group is ONE row. Evidence: context/test-evidence/core.md.
#[test]
fn collapsed_group_has_header_but_no_data_items() {
fn a_collapsed_group_contributes_one_aggregate_row() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
wb.add_item_in_group("Month", "Jan", "Q1").unwrap();
wb.add_item_in_group("Month", "Feb", "Q1").unwrap();
wb.add_item_in_group("Month", "Apr", "Q2").unwrap();
wb.add_item_in_group("Month", "May", "Q2").unwrap();
wb.add_item("Type", "Food").unwrap();
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
let layout = GridLayout::new(&wb.model, wb.active_view());
// Q1 collapsed: header present, Jan and Feb absent; Q2 intact
assert_eq!(layout.row_count(), 1); // only Apr
let q1_header = layout
let labels: Vec<String> = (0..layout.row_count())
.map(|r| layout.row_label(r))
.collect();
assert_eq!(
labels,
["Q1", "Apr", "May"],
"Q1 folds into one row; Q2 stays open"
);
assert!(layout.folds_a_group(0, 0), "the Q1 row is a fold");
assert!(
!layout.folds_a_group(1, 0),
"Apr, under the OPEN group, is not"
);
assert_eq!(
layout.row_group_for(0),
Some(("Month".into(), "Q1".into())),
"the fold sits under its own header, so z can name it"
);
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn a_non_contiguous_collapsed_group_folds_once() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
wb.add_item_in_group("Month", "A", "G1").unwrap();
wb.add_item_in_group("Month", "B", "G2").unwrap();
wb.add_item_in_group("Month", "C", "G1").unwrap();
wb.add_item("Type", "Food").unwrap();
wb.active_view_mut().toggle_group_collapse("Month", "G1");
let layout = GridLayout::new(&wb.model, wb.active_view());
let labels: Vec<String> = (0..layout.row_count())
.map(|r| layout.row_label(r))
.collect();
assert_eq!(labels, ["G1", "B"], "one G1 row, not one per run");
let g1_headers = layout
.row_items
.iter()
.find(|e| matches!(e, AxisEntry::GroupHeader { group_name, .. } if group_name == "Q1"));
assert!(q1_header.is_some());
let jan = layout
.row_items
.iter()
.find(|e| matches!(e, AxisEntry::DataItem(v) if v.contains(&"Jan".to_string())));
assert!(jan.is_none());
.filter(
|e| matches!(e, AxisEntry::GroupHeader { group_name, .. } if group_name == "G1"),
)
.count();
assert_eq!(
g1_headers, 1,
"a second G1 header would open a run with no row"
);
}
/// Evidence: context/test-evidence/core.md.
#[test]
fn a_category_nested_under_a_collapsed_group_is_folded_too() {
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Region").unwrap();
wb.add_category("Type").unwrap();
wb.add_item_in_group("Month", "Jan", "Q1").unwrap();
wb.add_item_in_group("Month", "Apr", "Q2").unwrap();
wb.add_item("Region", "North").unwrap();
wb.add_item("Type", "Food").unwrap();
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().toggle_group_collapse("Month", "Q1");
let layout = GridLayout::new(&wb.model, wb.active_view());
assert_eq!(
layout.row_tuple(0),
Some(&["Q1".to_string(), "North".to_string()][..])
);
assert!(
layout.folds_a_group(0, 0),
"Q1/North has no record under it either"
);
assert!(!layout.folds_a_group(1, 0), "Apr/North does");
}
/// improvise-gd83: two arms — an ungrouped item AFTER a grouped run, and
@@ -1513,13 +1671,22 @@ mod tests {
cat_name: "Month".into(),
group_name: "Q1".into()
},
AxisEntry::DataItem {
tuple: vec!["Q1".into()],
collapsed: true
},
AxisEntry::GroupEnd,
AxisEntry::DataItem(vec!["Sep".into()]),
AxisEntry::item(vec!["Sep".into()]),
],
"Q1 is collapsed, not empty — its header is the only way back, and Mar pruned"
"Q1 folds to one funded row under its header, and Mar pruned"
);
assert_eq!(
layout.row_group_for(0),
Some(("Month".into(), "Q1".into())),
"the fold is where z names the collapsed group from"
);
assert_eq!(
layout.row_group_for(1),
None,
"Sep must not inherit the collapsed run"
);
@@ -204,6 +204,35 @@ mod tests {
assert_eq!(header, ["Jan"], "CSV wrote a column the grid does not draw");
}
/// An export writes what the grid draws (improvise-iot6), so a collapsed group
/// is one total row. Evidence: context/test-evidence/formula-and-io.md.
#[test]
fn export_writes_a_collapsed_group_as_its_own_total_row() {
use crate::view::Axis;
let mut wb = Workbook::new("T");
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
wb.add_item_in_group("Month", "Jan", "Q1").unwrap();
wb.add_item_in_group("Month", "Feb", "Q1").unwrap();
wb.add_item("Type", "Food").unwrap();
for (month, value) in [("Jan", 3.0), ("Feb", 4.0)] {
wb.model.set_cell(
coord(&[("Month", month), ("Type", "Food")]),
CellValue::Number(value),
);
}
{
let v = wb.active_view_mut();
v.set_axis("Month", Axis::Row);
v.set_axis("Type", Axis::Column);
v.prune_empty = true;
v.toggle_group_collapse("Month", "Q1");
}
let csv = String::from_utf8(super::render_csv(&wb, "Default").unwrap()).unwrap();
let rows: Vec<&str> = csv.lines().skip(1).collect();
assert_eq!(rows, ["Q1,7.00"], "one folded row, summing Jan and Feb");
}
#[test]
fn export_csv_unknown_view_returns_error() {
let m = two_cat_model();
+7 -2
View File
@@ -54,6 +54,9 @@ pub enum Rejection {
/// A mutation in the frame refused to apply at all. The frame is undone;
/// the refusal's own explanation is in the host's status line.
MutationRefused,
/// This session cannot write the document, so it takes no edit at all
/// (improvise-j4qt). An edit it could not keep must never be painted.
ReadOnly,
/// A [`ClientFrame::Save`](crate::ClientFrame::Save) reached a host with
/// no path (improvise-368h). Decided before anything runs.
@@ -109,7 +112,8 @@ impl Rejection {
| Self::RecordMoved(_)
| Self::RecordUnstated(_)
| Self::ChainBroken(_)
| Self::MutationRefused => true,
| Self::MutationRefused
| Self::ReadOnly => true,
Self::NoDocumentPath | Self::SaveFailed => false,
}
}
@@ -136,7 +140,8 @@ impl Rejection {
Self::RecordMoved(_) => Self::RecordUnstated(sample),
Self::RecordUnstated(_) => Self::ChainBroken(sample),
Self::ChainBroken(_) => Self::MutationRefused,
Self::MutationRefused => Self::NoDocumentPath,
Self::MutationRefused => Self::ReadOnly,
Self::ReadOnly => Self::NoDocumentPath,
Self::NoDocumentPath => Self::SaveFailed,
Self::SaveFailed => Self::DocumentReplaced,
}
+2
View File
@@ -14,6 +14,8 @@ mod reduce;
pub mod render;
pub mod resolve;
#[cfg(any(test, feature = "test-support"))]
pub mod source_scan;
#[cfg(any(test, feature = "test-support"))]
pub mod variants;
pub mod viewmodel;
#[cfg(test)]
+96
View File
@@ -577,4 +577,100 @@ mod tests {
assert_eq!(cursor, 7, "{panel:?}'s cursor is not the field it names");
}
}
/// improvise-hkbc: every field of every `ModelCommand` names its target by
/// name, by path or by surrogate. A row or column index fails to COMPILE.
fn position_free(cmd: &ModelCommand) -> usize {
fn name(_: &str) -> usize {
1
}
fn surrogate(_: &RecordHash) -> usize {
1
}
fn record(RecordCell { record, measure }: &RecordCell) -> usize {
surrogate(record) + optional_name(measure)
}
fn path(_: &std::path::Path) -> usize {
1
}
fn value(_: &CellValue) -> usize {
1
}
fn axis(_: &Axis) -> usize {
1
}
fn optional_name(_: &Option<String>) -> usize {
1
}
// No `..` anywhere: a field added to a variant must be classified here.
match cmd {
ModelCommand::SetCell { cell, value: v } => record(cell) + value(v),
ModelCommand::ClearCell(cell) => record(cell),
ModelCommand::AddItem { category, item }
| ModelCommand::RemoveItem { category, item } => name(category) + name(item),
ModelCommand::AddItemInGroup {
category,
item,
group,
} => name(category) + name(item) + name(group),
ModelCommand::AddFormula {
raw,
target_category,
} => name(raw) + name(target_category),
ModelCommand::RemoveFormula {
target,
target_category,
} => name(target) + name(target_category),
ModelCommand::CleanEmptyRecords | ModelCommand::SortData => 0,
ModelCommand::AddCategory(category)
| ModelCommand::RemoveCategory(category)
| ModelCommand::CreateView(category)
| ModelCommand::DeleteView(category) => name(category),
ModelCommand::SetAxis {
view,
category,
axis: a,
} => name(view) + name(category) + axis(a),
ModelCommand::CycleAxis { view, category } => name(view) + name(category),
ModelCommand::TransposeAxes { view } | ModelCommand::TogglePruneEmpty { view } => {
name(view)
}
ModelCommand::SetPageSelection {
view,
category,
item,
}
| ModelCommand::HideItem {
view,
category,
item,
}
| ModelCommand::ShowItem {
view,
category,
item,
} => name(view) + name(category) + name(item),
ModelCommand::ToggleGroup {
view,
category,
group,
} => name(view) + name(category) + name(group),
ModelCommand::SetNumberFormat { view, format } => name(view) + name(format),
ModelCommand::LoadModel(p) => path(p),
ModelCommand::ImportJsonHeadless {
path: p,
model_name,
array_path,
} => path(p) + optional_name(model_name) + optional_name(array_path),
}
}
/// The guard is the signature above; this only keeps it compiled and reached.
#[test]
fn every_model_command_field_is_position_free() {
assert_eq!(
position_free(&ModelCommand::TransposeAxes { view: "V".into() }),
1
);
}
}
@@ -0,0 +1,346 @@
//! Production Rust source, for the guards that read source text: no comments,
//! no literals, no test code. The one home of that rule, `context/code/protocol.md` §2.
use std::collections::BTreeSet;
use std::path::{Path, PathBuf};
/// Every `.rs` file under `roots` that is compiled outside tests, with its
/// comments, literals and test-only items blanked or dropped.
pub fn production_code(roots: &[PathBuf]) -> Vec<(PathBuf, String)> {
let mut files = Vec::new();
for root in roots {
collect_rust(root, &mut files);
}
let code: Vec<(PathBuf, String)> = files
.into_iter()
.map(|file| {
let text = std::fs::read_to_string(&file).expect("a listed source reads");
let code = code_only(&text);
(file, code)
})
.collect();
let test_modules: BTreeSet<PathBuf> = code
.iter()
.flat_map(|(file, code)| test_module_paths(file, code))
.collect();
code.into_iter()
.filter(|(file, _)| !test_modules.iter().any(|m| is_module_file(m, file)))
.map(|(file, code)| (file, strip_test_items(&code)))
.collect()
}
/// One file's text as [`production_code`] cleans it. Which *files* are test
/// modules needs their declaring file too, so only `production_code` knows.
pub fn production_text(text: &str) -> String {
strip_test_items(&code_only(text))
}
fn collect_rust(dir: &Path, into: &mut Vec<PathBuf>) {
let Ok(entries) = std::fs::read_dir(dir) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
match path.is_dir() {
true => collect_rust(&path, into),
false if path.extension().is_some_and(|e| e == "rs") => into.push(path),
false => {}
}
}
}
/// `m` is a module path without extension: its file is `m.rs`, `m/mod.rs`,
/// or anything under `m/`.
fn is_module_file(m: &Path, file: &Path) -> bool {
file == m.with_extension("rs") || file.starts_with(m)
}
/// The out-of-line modules `file` declares under a test-only `cfg`, as paths
/// without extension (`#[cfg(test)] mod x;` is `<dir>/x`).
fn test_module_paths(file: &Path, code: &str) -> Vec<PathBuf> {
let dir = match file.file_stem().and_then(|s| s.to_str()) {
Some("lib" | "main" | "mod") => file.parent().map(Path::to_path_buf),
Some(stem) => file.parent().map(|p| p.join(stem)),
None => None,
}
.expect("a source file has a directory and a stem");
let lines: Vec<&str> = code.lines().map(str::trim).collect();
let mut found = Vec::new();
for (i, line) in lines.iter().enumerate() {
if !is_test_cfg(line) {
continue;
}
let item = lines[i + 1..]
.iter()
.find(|l| !l.starts_with("#[") && !l.is_empty());
if let Some(name) = item.and_then(|l| out_of_line_mod(l)) {
found.push(dir.join(name));
}
}
found
}
/// `mod x;`, `pub mod x;`, `pub(crate) mod x;` → `x`.
fn out_of_line_mod(line: &str) -> Option<&str> {
let rest = line.split_once("mod ")?.1.strip_suffix(';')?;
let prefix = line.split_once("mod ")?.0.trim();
(prefix.is_empty() || prefix.starts_with("pub")).then_some(rest.trim())
}
/// A `cfg` attribute that compiles its item only under `test`. `cfg(not(test))`
/// is production, and `"test-support"` is a literal, already blanked.
fn is_test_cfg(line: &str) -> bool {
let line = line.trim();
line.starts_with("#[cfg(")
&& !line.contains("not(")
&& line
.split(|c: char| !c.is_alphanumeric() && c != '_')
.any(|word| word == "test")
}
/// Drop every test-only item. rustfmt puts an item's closing brace at the
/// attribute's own indentation, which is what makes this a line scan (§2).
fn strip_test_items(text: &str) -> String {
let lines: Vec<&str> = text.lines().collect();
let mut kept = String::new();
let mut i = 0;
while i < lines.len() {
let line = lines[i];
match is_test_cfg(line) {
false => {
kept.push_str(line);
kept.push('\n');
i += 1;
}
true => {
let indent = line.len() - line.trim_start().len();
let close = format!("{}}}", " ".repeat(indent));
i += 1;
while i < lines.len() && lines[i].trim_start().starts_with("#[") {
i += 1;
}
match lines.get(i).is_some_and(|l| l.trim_end().ends_with(';')) {
true => i += 1,
false => {
while i < lines.len() && lines[i].trim_end() != close {
i += 1;
}
i += 1;
}
}
}
}
}
kept
}
/// `text` with comments, string, byte-string, raw-string and char literals
/// replaced by spaces. Newlines survive, so line-based scans still line up.
pub fn code_only(text: &str) -> String {
let chars: Vec<char> = text.chars().collect();
let mut out = String::with_capacity(text.len());
let mut i = 0;
while i < chars.len() {
let end = literal_or_comment_end(&chars, i);
match end {
Some(end) => {
for &c in &chars[i..end] {
out.push(if c == '\n' { '\n' } else { ' ' });
}
i = end;
}
None => {
out.push(chars[i]);
i += 1;
}
}
}
out
}
/// Where the comment or literal starting at `i` ends, if one starts there.
fn literal_or_comment_end(c: &[char], i: usize) -> Option<usize> {
let at = |k: usize| c.get(k).copied();
let word_before = i > 0 && (c[i - 1].is_alphanumeric() || c[i - 1] == '_');
match (at(i), at(i + 1)) {
(Some('/'), Some('/')) => Some(line_end(c, i)),
(Some('/'), Some('*')) => Some(block_end(c, i)),
(Some('"'), _) => Some(quoted_end(c, i + 1, '"')),
(Some('b'), Some('"')) if !word_before => Some(quoted_end(c, i + 2, '"')),
(Some('r' | 'b'), Some('r' | '#' | '"')) if !word_before => raw_end(c, i),
(Some('\''), _) => char_end(c, i),
_ => None,
}
}
fn line_end(c: &[char], from: usize) -> usize {
(from..c.len()).find(|&k| c[k] == '\n').unwrap_or(c.len())
}
/// Block comments nest in Rust.
fn block_end(c: &[char], from: usize) -> usize {
let mut depth = 0usize;
let mut k = from;
while k + 1 < c.len() {
match (c[k], c[k + 1]) {
('/', '*') => {
depth += 1;
k += 2;
}
('*', '/') => {
depth -= 1;
k += 2;
if depth == 0 {
return k;
}
}
_ => k += 1,
}
}
c.len()
}
fn quoted_end(c: &[char], mut k: usize, quote: char) -> usize {
while k < c.len() {
match c[k] {
'\\' => k += 2,
q if q == quote => return k + 1,
_ => k += 1,
}
}
c.len()
}
/// `r"…"`, `r#"…"#`, `br#"…"#`. `None` when the `r` is only an identifier.
fn raw_end(c: &[char], i: usize) -> Option<usize> {
let mut k = i + usize::from(c[i] == 'b');
if c.get(k) != Some(&'r') {
return None;
}
k += 1;
let hashes = c[k..].iter().take_while(|&&h| h == '#').count();
k += hashes;
if c.get(k) != Some(&'"') {
return None;
}
k += 1;
while k < c.len() {
if c[k] == '"'
&& c[k + 1..]
.iter()
.take(hashes)
.filter(|&&h| h == '#')
.count()
== hashes
{
return Some(k + 1 + hashes);
}
k += 1;
}
Some(c.len())
}
/// A char literal, or `None` for a lifetime (`'a` with no closing quote).
fn char_end(c: &[char], i: usize) -> Option<usize> {
match (c.get(i + 1), c.get(i + 2)) {
(Some('\\'), _) => Some(quoted_end(c, i + 1, '\'')),
(Some(_), Some('\'')) => Some(i + 3),
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;
/// The walk itself drops a `#[cfg(test)] mod x;` file, not only the parts
/// that find one. A fixture tree, because the wiring is the thing tested.
#[test]
fn a_test_only_module_file_is_not_production_code() {
let root = std::env::temp_dir().join(format!("source_scan_{}", std::process::id()));
let src = root.join("src");
std::fs::create_dir_all(src.join("t")).unwrap();
std::fs::write(src.join("lib.rs"), "mod p;\n#[cfg(test)]\nmod t;\n").unwrap();
std::fs::write(src.join("p.rs"), "fn production() {}\n").unwrap();
std::fs::write(src.join("t.rs"), "fn test_only() {}\nmod deeper;\n").unwrap();
std::fs::write(src.join("t").join("deeper.rs"), "fn test_too() {}\n").unwrap();
let mut found: Vec<String> = production_code(std::slice::from_ref(&src))
.into_iter()
.map(|(file, _)| file.strip_prefix(&src).unwrap().display().to_string())
.collect();
found.sort();
std::fs::remove_dir_all(&root).unwrap();
assert_eq!(found, vec!["lib.rs", "p.rs"]);
}
#[test]
fn test_only_items_and_modules_are_not_production() {
let text = concat!(
"fn kept() {}\n",
"#[cfg(test)]\n",
"mod inline {\n",
" fn gone_inline() {}\n",
"}\n",
"#[cfg(test)]\n",
"mod out_of_line;\n",
"#[cfg(any(test, feature = \"x\"))]\n",
"pub mod gated;\n",
"#[cfg(not(test))]\n",
"fn production_only() {}\n",
"fn also_kept() {}\n",
);
let code = code_only(text);
let stripped = strip_test_items(&code);
for kept in ["fn kept", "fn production_only", "fn also_kept"] {
assert!(stripped.contains(kept), "{kept} was dropped: {stripped}");
}
assert!(!stripped.contains("gone_inline"), "{stripped}");
let lib = Path::new("/c/src/lib.rs");
assert_eq!(
test_module_paths(lib, &code),
vec![
PathBuf::from("/c/src/out_of_line"),
PathBuf::from("/c/src/gated")
]
);
let nested = Path::new("/c/src/ui/reduce.rs");
assert_eq!(
test_module_paths(nested, &code)[0],
PathBuf::from("/c/src/ui/reduce/out_of_line")
);
assert!(is_module_file(
Path::new("/c/src/gated"),
Path::new("/c/src/gated.rs")
));
assert!(is_module_file(
Path::new("/c/src/gated"),
Path::new("/c/src/gated/a.rs")
));
assert!(!is_module_file(
Path::new("/c/src/gated"),
Path::new("/c/src/gatedx.rs")
));
}
#[test]
fn comments_and_literals_are_blanked_and_code_is_kept() {
let text = concat!(
"let a = Command::Kept(1); // Command::LineComment(2)\n",
"/* Command::Block( /* nested */ Command::StillBlock ) */\n",
"let s = \"Command::InString(\\\"x\\\")\";\n",
"let r = r#\"Command::InRaw(\"q\")\"#;\n",
"let q = '\"'; let l: &'static str = \"\"; Command::AfterChar(3)\n",
);
let kept = code_only(text);
assert!(kept.contains("Command::Kept(1)"));
assert!(kept.contains("Command::AfterChar(3)"));
assert!(
kept.contains("&'static str"),
"a lifetime is not a char literal"
);
for gone in ["LineComment", "Block", "StillBlock", "InString", "InRaw"] {
assert!(!kept.contains(gone), "{gone} survived: {kept}");
}
assert_eq!(kept.lines().count(), text.lines().count());
}
}
@@ -5,6 +5,7 @@ use std::collections::BTreeSet;
use std::path::{Path, PathBuf};
use crate::reduce::Command;
use crate::source_scan::{production_code, production_text};
/// A variant production composes nowhere, and why. Checked in **both**
/// directions, so a composed entry fails here too (improvise-m0sx).
@@ -62,6 +63,24 @@ fn the_uncomposed_list_names_only_real_variants() {
);
}
/// The #160 review's fault: a variant named only in a comment or a string
/// is not composed. See `context/test-evidence/protocol.md`.
#[test]
fn a_variant_named_only_in_a_comment_or_string_is_not_composed() {
let text = concat!(
"fn f() {\n",
" // TODO: Command::SetYanked(value)\n",
" /* Command::SetSearchMode(true) */\n",
" let s = \"Command::ChangeMode(AppMode::Normal)\";\n",
"}\n",
);
assert_eq!(composers_in(text), Vec::<String>::new());
assert_eq!(
composers_in("let c = Command::SetYanked(None);"),
vec!["SetYanked"]
);
}
/// The scan must find *something*, or a broken walk reads as a clean bill of
/// health — the failure direction `check_the_checker` exists for.
#[test]
@@ -74,21 +93,22 @@ fn the_scan_reaches_production_code_at_all() {
);
}
/// Every `Command::<Name>` in a **value** position, in non-test source.
fn composed_in_production() -> BTreeSet<String> {
let mut found = BTreeSet::new();
for file in production_sources() {
let text = std::fs::read_to_string(&file).expect("a listed source reads");
for name in constructed_in(&strip_test_items(&text)) {
found.insert(name);
}
}
found
/// One file's composers: no comment, literal or test item is one (improvise-c7mu).
fn composers_in(text: &str) -> Vec<String> {
constructed_in(&production_text(text))
}
/// Rust under `src/`, workspace-wide. `tests/` is excluded by construction:
/// Every `Command::<Name>` in a **value** position, in production source.
fn composed_in_production() -> BTreeSet<String> {
production_code(&source_roots())
.iter()
.flat_map(|(_, code)| constructed_in(code))
.collect()
}
/// `src/` of every workspace crate. `tests/` is excluded by construction:
/// a variant only a test composes is exactly what this file looks for.
fn production_sources() -> Vec<PathBuf> {
fn source_roots() -> Vec<PathBuf> {
let root = Path::new(env!("CARGO_MANIFEST_DIR"))
.parent()
.and_then(Path::parent)
@@ -98,53 +118,7 @@ fn production_sources() -> Vec<PathBuf> {
for entry in std::fs::read_dir(root.join("crates")).expect("crates/ is readable") {
roots.push(entry.expect("a crates/ entry reads").path().join("src"));
}
let mut files = Vec::new();
for dir in roots {
collect_rust(&dir, &mut files);
}
files
}
fn collect_rust(dir: &Path, into: &mut Vec<PathBuf>) {
let Ok(entries) = std::fs::read_dir(dir) else {
return;
};
for entry in entries.flatten() {
let path = entry.path();
match path.is_dir() {
true => collect_rust(&path, into),
false if path.extension().is_some_and(|e| e == "rs") => into.push(path),
false => {}
}
}
}
/// Drop every `#[cfg(test)]` item. rustfmt puts an item's closing brace at the
/// attribute's own indentation, which is what makes this a line scan (§2).
fn strip_test_items(text: &str) -> String {
let lines: Vec<&str> = text.lines().collect();
let mut kept = String::new();
let mut i = 0;
while i < lines.len() {
let line = lines[i];
match line.trim_start().starts_with("#[cfg(test)]") {
false => {
kept.push_str(line);
kept.push('\n');
i += 1;
}
true => {
let indent = line.len() - line.trim_start().len();
let close = format!("{}}}", " ".repeat(indent));
i += 1;
while i < lines.len() && lines[i] != close {
i += 1;
}
i += 1;
}
}
}
kept
roots
}
/// `Command::<Name>` occurrences that are not `match` patterns. A pattern is
+61 -49
View File
@@ -42,11 +42,11 @@ pub struct RenderModel {
pub categories: Vec<RenderCatRow>,
/// Whether the panel is open, so the page does not decide.
pub category_panel_open: bool,
/// The view panel's rows, **empty unless it is open** — the same rule the
/// help overlay follows, so the page asks no question about the mode.
pub views: Vec<RenderViewRow>,
/// The formula panel's rows, empty unless it is open.
pub formulas: Vec<RenderFormulaRow>,
/// The view panel's rows, **`None` when it is closed** and possibly empty
/// when open: an absence is not an empty list (improvise-df2w).
pub views: Option<Vec<RenderViewRow>>,
/// The formula panel's rows, on the same terms as `views`.
pub formulas: Option<Vec<RenderFormulaRow>>,
/// The tile bar, drawn on every frame as the TUI draws it. Which tile is
/// `selected` is the mode's answer, not the page's.
pub tiles: Vec<RenderTile>,
@@ -341,41 +341,39 @@ fn page_axes(state: &ClientState) -> Vec<RenderPageAxis> {
.collect()
}
/// The view panel's rows while it is open. The axis line is
/// The view panel's rows, or `None` while it is closed. The axis line is
/// `viewmodel::view_axis_summary`, which the TUI's own panel calls too.
fn view_rows(state: &ClientState) -> Vec<RenderViewRow> {
if !state.view.view_panel_open {
return Vec::new();
}
state
.views
.views
.iter()
.enumerate()
.map(|(i, summary)| RenderViewRow {
name: summary.name.clone(),
axes: improvise_protocol::viewmodel::view_axis_summary(summary),
active: summary.name == state.view.active_view,
selected: i == state.view.view_panel_cursor,
})
.collect()
fn view_rows(state: &ClientState) -> Option<Vec<RenderViewRow>> {
state.view.view_panel_open.then(|| {
state
.views
.views
.iter()
.enumerate()
.map(|(i, summary)| RenderViewRow {
name: summary.name.clone(),
axes: improvise_protocol::viewmodel::view_axis_summary(summary),
active: summary.name == state.view.active_view,
selected: i == state.view.view_panel_cursor,
})
.collect()
})
}
/// The formula panel's rows while it is open.
fn formula_rows(state: &ClientState) -> Vec<RenderFormulaRow> {
if !state.view.formula_panel_open {
return Vec::new();
}
state
.formulas
.formulas
.iter()
.enumerate()
.map(|(i, text)| RenderFormulaRow {
text: text.clone(),
selected: i == state.view.formula_cursor,
})
.collect()
/// The formula panel's rows, or `None` while it is closed.
fn formula_rows(state: &ClientState) -> Option<Vec<RenderFormulaRow>> {
state.view.formula_panel_open.then(|| {
state
.formulas
.formulas
.iter()
.enumerate()
.map(|(i, text)| RenderFormulaRow {
text: text.clone(),
selected: i == state.view.formula_cursor,
})
.collect()
})
}
/// The tile bar, always. Only the *selection* is conditional, exactly as
@@ -797,34 +795,48 @@ mod tests {
client
}
/// An empty list is what says a panel is closed — the same rule the help
/// overlay follows, so the page asks no question about the mode.
/// `None` is what says a panel is closed, so the page asks no question
/// about the mode.
#[test]
fn a_closed_panel_projects_no_rows_and_an_open_one_does() {
let mut client = client_with_panels();
let closed = client.model();
assert!(
closed.views.is_empty(),
closed.views.is_none(),
"the cache is full but the panel is shut"
);
assert!(closed.formulas.is_empty());
assert!(closed.formulas.is_none());
client.state_mut().view.view_panel_open = true;
client.state_mut().view.formula_panel_open = true;
client.state_mut().view.view_panel_cursor = 1;
client.state_mut().view.active_view = "Default".into();
let open = client.model();
assert_eq!(open.views.len(), 2);
let (views, formulas) = (open.views.unwrap(), open.formulas.unwrap());
assert_eq!(views.len(), 2);
assert_eq!(
open.views[0].axes, "R:Region",
views[0].axes, "R:Region",
"the axis line is the shared derivation, and _Dim is not part of it"
);
assert!(
open.views[0].active,
"active is ViewState's, not the cache's"
);
assert!(open.views[1].selected, "and selected is the panel cursor's");
assert_eq!(open.formulas[0].text, "Profit = Revenue - Cost");
assert!(views[0].active, "active is ViewState's, not the cache's");
assert!(views[1].selected, "and selected is the panel cursor's");
assert_eq!(formulas[0].text, "Profit = Revenue - Cost");
}
/// Open with nothing in it is not closed: the page must be able to tell
/// them apart from the model alone (improvise-df2w).
#[test]
fn an_open_empty_panel_is_not_a_closed_one() {
let mut client = init();
let closed = client.model();
client.state_mut().view.formula_panel_open = true;
let formulas_open = client.model();
assert_ne!(formulas_open, closed, "an open, empty formula panel");
client.state_mut().view.formula_panel_open = false;
client.state_mut().view.view_panel_open = true;
let views_open = client.model();
assert_ne!(views_open, closed, "an open, empty view panel");
}
/// The tile bar is drawn always; only the *selection* waits for the mode,
@@ -29,6 +29,10 @@ fn rejection_message(rejection: &Rejection) -> String {
Rejection::MutationRefused => {
"part of that batch was refused — the batch was not applied".into()
}
Rejection::ReadOnly => {
"this tab is read-only: another tab has the document open — your edit was not applied"
.into()
}
// Not staleness, so neither offers a retry: they are worded as facts
// about the host, because that is what they are (improvise-368h).
Rejection::NoDocumentPath => {
@@ -111,7 +111,7 @@ async fn read_only(storage: &OpfsFs, path: PathBuf, why: String) -> Result<Worke
let mut app = App::with_storage(opened.workbook, Some(path.clone()), Box::new(fs));
app.rebuild_layout();
Ok(WorkerServer {
worker: Worker::new(app),
worker: Worker::read_only(app),
storage: None,
notice: Some(Notice::new(
NoticeKind::ReadOnly,
+103 -1
View File
@@ -5,7 +5,7 @@ use improvise_command::ui::app::App;
use improvise_command::ui::effect;
use improvise_command::ui::host::{Addressed, Host};
use improvise_command::ui::project::SessionId;
use improvise_protocol::{ClientFrame, ServerFrame};
use improvise_protocol::{ClientFrame, Command, Rejection, ServerFrame};
use crate::boot::{Notice, NoticeKind};
@@ -18,16 +18,50 @@ pub struct Worker {
/// Raised by a persist nobody asked for and drained by the shim; see
/// [`Worker::take_notice`] and `context/code/clients.md`.
notice: Option<Notice>,
access: Access,
}
/// Whether this session may change the document. A read-only boot still
/// answers every view command; `context/code/clients.md` says why.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum Access {
Writable,
/// Another context holds the handle, so no edit could ever be kept.
ReadOnly,
}
impl Access {
/// Decided before the host sees the frame, so nothing is applied and
/// nothing projected: an edit that cannot be saved is never painted.
fn refuses(self, frame: &ClientFrame) -> bool {
match (self, frame) {
(Self::ReadOnly, ClientFrame::Commands(cmds)) => {
cmds.iter().any(|c| matches!(c, Command::Model { .. }))
}
(Self::ReadOnly, _) | (Self::Writable, _) => false,
}
}
}
impl Worker {
/// Take an `App` already pointed at its document and storage backend;
/// building that is [`crate::vfs`]'s job and the shim's, not this loop's.
pub fn new(app: App) -> Self {
Self::with_access(app, Access::Writable)
}
/// A session over a document another context holds (improvise-i0v9). It
/// paints and navigates, and refuses every edit (improvise-j4qt).
pub fn read_only(app: App) -> Self {
Self::with_access(app, Access::ReadOnly)
}
fn with_access(app: App, access: Access) -> Self {
Self {
host: Host::new(app),
client: None,
notice: None,
access,
}
}
@@ -48,6 +82,9 @@ impl Worker {
// Anything else before the handshake: composed against a document
// the client was never sent, so nothing to apply and nothing to say.
(None, _) => Vec::new(),
(Some(_), frame) if self.access.refuses(&frame) => {
vec![ServerFrame::Rejected(Rejection::ReadOnly)]
}
(Some(id), frame) => {
// A save the client asked for is already answered; retrying it
// here double-writes. See `context/code/clients.md`.
@@ -272,6 +309,71 @@ mod tests {
);
}
/// The read-only boot `bindings::read_only` performs: the document's bytes
/// over a backend that refuses every write, because another tab holds it.
fn read_only_worker() -> Worker {
let fs = MemoryFs::new();
let path = PathBuf::from("demo.improv");
fs.write(&path, &demo_bytes()).unwrap();
let locked =
crate::vfs::ReadOnly::new(fs, "another tab or window is holding this document");
let wb = vfs::open(&locked, &Opening::named(&path)).unwrap().workbook;
let mut app = App::with_storage(wb, Some(path), Box::new(locked));
app.rebuild_layout();
Worker::read_only(app)
}
/// improvise-j4qt: a read-only tab applied an edit and PAINTED a value the
/// document never had. Evidence: `context/test-evidence/clients.md`.
#[test]
fn a_read_only_session_refuses_an_edit_before_anything_is_painted() {
let mut worker = read_only_worker();
let mut grid = connect(&mut worker);
let was = grid.cells[0][0].text.clone();
let cmd = reduce::caught_up(
worker.app(),
ModelCommand::SetCell {
cell: addressed(&grid, 0, 0),
value: CellValue::Number(4242.0),
},
);
let out = worker.on_frame(ClientFrame::command(cmd));
let _ = folded(&mut grid, &out);
assert_eq!(
out,
vec![ServerFrame::Rejected(Rejection::ReadOnly)],
"a session that cannot save must refuse the edit, and say only that"
);
assert_eq!(grid.cells[0][0].text, was, "the refused value was painted");
assert_eq!(
worker.take_notice(),
None,
"nothing was applied, so no persist ran and none failed"
);
assert!(
!worker.app().model_state.dirty,
"the edit reached the document"
);
}
/// The control: read-only is about the DOCUMENT. Moving the cursor writes
/// only this client's slice, so it must still be answered.
#[test]
fn a_read_only_session_still_answers_a_view_command() {
let mut worker = read_only_worker();
let _ = connect(&mut worker);
let out = worker.on_frame(ClientFrame::command(Command::SetSelected {
row: 1,
col: 0,
}));
assert!(
!out.iter().any(|f| matches!(f, ServerFrame::Rejected(_))),
"a read-only tab refused a cursor move: {out:?}"
);
}
/// improvise-zre1: the failure stayed in a `status_msg` the worker has no
/// UI for, so the client could not tell "save failed" from "save pending".
#[test]
+22 -1
View File
@@ -70,7 +70,7 @@ test.describe("every field the render model projects reaches the screen", () =>
await expect(page.locator("#categories")).toBeVisible();
await page.keyboard.press("Escape");
// `views`: empty unless open, so the page asks no question about the mode.
// `views`: `null` unless open, so the page asks no question about the mode.
await expect(page.locator("#views")).toBeHidden();
await page.keyboard.press("Shift+V");
await expect(page.locator("#views li")).toHaveCount(2);
@@ -356,6 +356,27 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
});
});
test.describe("an open panel is drawn even when it has nothing in it", () => {
// improvise-df2w: `formulas` said "closed" by being empty, so a document with
// no formulas could open the panel and never see it. `tall.improv` has none.
test("the formula panel opens over a document with no formulas", async ({ page }) => {
await page.goto("/app/index.html?doc=df2w-empty.improv&seed=tall.improv");
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await page.locator("#grid").focus();
await expect(page.locator("#formulas")).toBeHidden();
await page.keyboard.press("Shift+F");
await expect(page.locator("#mode")).toHaveText("FORMULAS");
await expect(page.locator("#formulas")).toBeVisible();
await expect(page.locator("#formulas li")).toHaveCount(0);
// And closed is still closed: the other direction, which a page that
// simply never hid the panel would fail.
await page.keyboard.press("Shift+F");
await expect(page.locator("#formulas")).toBeHidden();
});
});
test.describe("the selection is on screen", () => {
// improvise-jq8l: `web/app.js` called `scrollIntoView` NOWHERE, so a selection
// moved past the bottom of `#grid` simply stayed there, invisible, with no way
+11 -8
View File
@@ -146,18 +146,21 @@ test.describe("a grouped, multi-level document", () => {
expect(await groupRows.locator("td").count()).toBe(0);
await expect(groupRows.first().locator("th")).toHaveCount(1);
// The collapse is the document's, and it must have REMOVED rows: Upland's
// two regions are absent while Inland's two are here. Without this, a
// page that drew the indicator and ignored the frame would pass.
// The collapse is the document's: Upland's two regions are gone and the
// group is ONE row in their place (improvise-6qet, core.md §14).
const labels = await page.locator("#grid tbody tr th[scope=row]").allTextContents();
expect(labels).toContain("East");
expect(labels).toContain("West");
expect(labels).toContain("Upland");
expect(labels).not.toContain("Ridge");
expect(labels).not.toContain("Valley");
// 4 regions visible x 2 stores. Stated as a count as well, because a row
// drawn with empty labels would satisfy the four assertions above.
await expect(page.locator("#grid tbody tr").filter({ hasNot: page.locator("th.group") })).toHaveCount(
8,
);
// (4 regions + the Upland fold) x 2 stores. A count too, because a row
// drawn with empty labels would satisfy the assertions above.
const dataRows = page.locator("#grid tbody tr").filter({ hasNot: page.locator("th.group") });
await expect(dataRows).toHaveCount(10);
// The fold is an aggregate: no record carries Region=Upland, so nothing
// in its rows may open an editor, where a stored row beside it does.
await expect(dataRows.first().locator("td.editable")).not.toHaveCount(0);
await expect(dataRows.last().locator("td.editable")).toHaveCount(0);
});
});
+34
View File
@@ -257,6 +257,40 @@ test.describe("the standalone deployment", () => {
await first.close();
});
// improvise-j4qt: the read-only tab used to APPLY an edit to its own copy
// and paint it, a number the document never had. It must refuse the edit.
test("a read-only tab refuses an edit and never paints it", async ({ context }) => {
const doc = `read-only-edit-${Date.now()}.improv`;
const first = await context.newPage();
await first.goto(`/app/index.html?doc=${doc}`);
await expect(first.locator("#grid table")).toBeVisible({ timeout: 20_000 });
const second = await context.newPage();
await second.goto(`/app/index.html?doc=${doc}`);
await expect(second.locator("#grid table")).toBeVisible({ timeout: 20_000 });
await expect(second.locator("#status")).toContainText("read-only");
const cell = second.locator("#grid td.editable").first();
const before = await cell.textContent();
await cell.click();
const input = cell.locator("input");
await expect(input).toBeFocused();
await input.fill("4242");
await input.press("Enter");
// The refusal is what arrives, and a document message is a toast. Waiting
// on it first means the grid below is asserted AFTER the answer landed.
await expect(second.locator("#toast")).toContainText("read-only", { timeout: 10_000 });
await expect(cell).toHaveText(before);
await expect(second.locator("#grid")).not.toContainText("4,242");
// The boot's reason stands; no failed save replaced it (the old symptom).
await expect(second.locator("#status")).toContainText("read-only");
await expect(second.locator("#status")).not.toContainText("NOT SAVED");
await second.close();
await first.close();
});
// What lands on screen is the HOST's value, not the string typed — the
// round trip through a real worker, which no unit test can stand in for.
test("committing an edit sends one frame and shows the host's value", async ({
+15 -4
View File
@@ -72,7 +72,7 @@ acquire_lock() {
scratch=()
new_cand() {
cand="$(mktemp "$KB.cand.XXXXXX")"
scratch+=("$cand" "$cand.delta" "$cand.removed" "$cand.ledger" "$cand.base")
scratch+=("$cand" "$cand.delta" "$cand.removed" "$cand.ledger" "$cand.base" "$cand.stamped")
}
# What `kb rm` removed that HEAD still has: the record that tells an intended
@@ -83,6 +83,14 @@ ledger_path() {
fi
}
# A ledger speaks only for the kb FILE this tool last published, named on its
# first line; bytes alone are not enough, since a restore can reproduce them.
file_stamp() {
printf '%s %s' "$(stamp_of "$1")" \
"$(stat -c '%i %.9Y' "$1" 2>/dev/null || stat -f '%i %Fm' "$1")"
}
ledger_holds() { [ -f "$1" ] && [ "$(head -n 1 "$1")" = "% kb-stamp $(file_stamp "$2")" ]; }
# The committed knowledge base, which no write may end up short of.
# KB_BASELINE is the test seam, naming a file.
baseline_to() {
@@ -140,19 +148,22 @@ commit() { # $1 = candidate, $2 = stamp of the kb as read, $3 = keys a rm remove
echo "kb: cannot read the committed kb.pl to compare against; $KB is unchanged" >&2
exit 3
fi
{ if [ -f "$ledger" ]; then cat "$ledger"; fi
{ if ledger_holds "$ledger" "$KB"; then cat "$ledger"; fi
if [ -n "$removing" ]; then cat "$removing"; fi; } > "$cand.removed"
if ! merge complete "$cand.base" "$cand" "$cand.removed" "$cand.ledger"; then
echo "kb: REFUSED -- clauses HEAD has are gone, with no kb rm recorded; $KB is unchanged." >&2
echo "kb: restore it from HEAD and re-apply your deltas, or commit the removal." >&2
echo "kb: restore it from HEAD (git show HEAD:prolog-sessions/kb.pl) and re-apply your deltas." >&2
echo "kb: do NOT commit this kb.pl: a committed loss becomes the floor this check compares against." >&2
echo "kb: to drop a clause on purpose, restore first and then kb rm it." >&2
exit 1
fi
if ! check_file "$cand"; then
echo "kb: REFUSED -- kb_consistent fails on the result; $KB is unchanged" >&2
exit 1
fi
{ printf '%% kb-stamp %s\n' "$(file_stamp "$cand")"; cat "$cand.ledger"; } > "$cand.stamped"
kill_hook "$cand"
mv -f "$cand.ledger" "$ledger"
mv -f "$cand.stamped" "$ledger"
mv -f "$cand" "$KB"
}
+40 -10
View File
@@ -35,8 +35,11 @@ run(remove, KbFile, DeltaFile, OutFile) :-
halt(2)
),
foldl_remove(Funcs, Doomed, Old, Kept),
length(Old, NOld), length(Doomed, NDoomed),
ExpectK is NOld - NDoomed,
length(Old, NOld),
findall(x, ( member(O, Old), clause_key(Funcs, O, KO),
member(T, Doomed), clause_key(Funcs, T, KT), same_key(KO, KT) ), Hits),
length(Hits, NHits),
ExpectK is NOld - NHits,
check_total(Kept, ExpectK),
write_terms(OutFile, Kept),
verify(OutFile, Kept),
@@ -325,24 +328,27 @@ count_only(KbFile) :-
% Consistency is not completeness: a kb that lost 276 clauses still passed
% kb_consistent. Why by key and against HEAD: context/code/kb-tool.md.
% Keys are counted, not collected: a merge can commit two clauses under one
% key, and a set of keys cannot see one of them go (improvise-ih3k).
complete(BaseFile, CandFile, ExcFile, LedgerOut) :-
terms_of(BaseFile, Base),
terms_of(CandFile, Cand),
terms_of(ExcFile, Exc),
ground_keys(Cand, CandKeys),
key_counts(Base, BaseCounts),
key_counts(Cand, CandCounts),
ground_keys(Exc, ExcKeys),
findall(T, ( member(T, Base), ground_key(T, K),
\+ memberchk(K, CandKeys), \+ memberchk(K, ExcKeys) ), Missing),
findall(K-D, ( member(K-NBK, BaseCounts), count_of(K, CandCounts, NCK),
D is NBK - NCK, D > 0 ), Short),
findall(K-D, ( member(K-D, Short), \+ memberchk(K, ExcKeys) ), Unexcused),
findall(T, ( member(K-_, Unexcused), lost_under(K, Base, Cand, T) ), Missing),
length(Base, NB), length(Cand, NC),
( Missing == []
-> ground_keys(Base, BaseKeys),
findall(E, ( member(E, Exc), ground_key(E, K),
memberchk(K, BaseKeys), \+ memberchk(K, CandKeys) ), Kept),
( Unexcused == []
-> findall(E, ( member(E, Exc), ground_key(E, K), memberchk(K-_, Short) ), Kept),
write_terms(LedgerOut, Kept),
length(Kept, NK),
format("kb-merge: complete -- baseline ~w, result ~w, ~w removal(s) recorded~n",
[NB, NC, NK])
; length(Missing, NM),
; foldl(add_deficit, Unexcused, 0, NM),
format("kb-merge: REFUSED -- the baseline has ~w clauses and the result ~w; ~w of the baseline's are missing and no kb rm recorded their removal:~n",
[NB, NC, NM]),
forall(( nth1(I, Missing, T), I =< 10 ), format(" ~q~n", [T])),
@@ -356,5 +362,29 @@ ground_keys(Terms, Sorted) :-
findall(C, ( member(T, Terms), ground_key(T, C) ), Cs),
sort(Cs, Sorted).
key_counts(Terms, Counts) :-
findall(C, ( member(T, Terms), ground_key(T, C) ), Cs),
msort(Cs, Sorted),
run_lengths(Sorted, Counts).
run_lengths([], []).
run_lengths([K|T], [K-N|Rest]) :-
same_run(K, T, 1, N, After),
run_lengths(After, Rest).
same_run(K, [X|T], N0, N, After) :- X == K, !, N1 is N0 + 1, same_run(K, T, N1, N, After).
same_run(_, After, N, N, After).
count_of(K, Counts, N) :- ( memberchk(K-N0, Counts) -> N = N0 ; N = 0 ).
add_deficit(_-D, N0, N) :- N is N0 + D.
% What to name for a short key: the baseline's clauses under it that the
% result holds no variant of, or the first one when every text survives.
lost_under(K, Base, Cand, T) :-
findall(B, ( member(B, Base), ground_key(B, KB), KB == K ), Under),
findall(B, ( member(B, Under), \+ ( member(C, Cand), same_key(B, C) ) ), Gone),
( Gone == [] -> Under = [T|_] ; member(T, Gone) ).
die(Msg) :- format("kb-merge: ~w~n", [Msg]), halt(2).
die_file(F) :- format("kb-merge: cannot read ~w~n", [F]), halt(2).
+147 -4
View File
@@ -553,6 +553,17 @@ else
bad "the truncation was refused, but not by the completeness guard"
fi
# 19b. ...and the refusal must not offer committing the loss as the way out:
# a committed loss becomes the floor, and is never reported again
# (improvise-za97).
if said "$work/out" "commit the removal"; then
bad "the refusal still offers committing an unexplained loss"
elif said "$work/out" "do NOT commit"; then
ok "the refusal warns against committing the loss"
else
bad "the refusal says nothing about committing the short kb"
fi
# 20. rm is how the count legitimately falls, and the ledger is how a LATER
# write knows it: rm a clause HEAD has, then add -- the add must land. A
# ledger that is never written, or is ignored, refuses here.
@@ -568,13 +579,19 @@ fi
# 20b. ...and the record excuses exactly what it names. A second, unrecorded
# loss on top of a recorded rm must still refuse; a guard that stands down
# whenever a ledger exists passes 20 and fails here.
sed -i.bak '/^location(add_command,/d' "$work/kb.pl"; rm -f "$work/kb.pl.bak"
# whenever a ledger exists passes 20 and fails here. The second loss is
# made by giving the BASELINE a clause, not by editing the kb: an edit
# voids the ledger (case 24), which would test that instead.
awk -v x="location(a_probe_only_head_has,'src/nowhere.rs',probe)." '
{ line[NR] = $0 } /^location\(/ { last = NR }
END { for (i = 1; i <= NR; i++) { print line[i]; if (i == last) print x } }
' "$KB" > "$work/head-plus.pl"
cp "$work/kb.pl" "$work/twice.pl"
printf "doctrine(another_probe_after_the_rm,'x').\n" > "$work/new2.pl"
if kb add "$work/new2.pl" >"$work/out" 2>&1; then
if KB_BASELINE="$work/head-plus.pl" kb add "$work/new2.pl" >"$work/out" 2>&1; then
bad "a recorded rm excused an unrecorded loss beside it"
elif cmp -s "$work/kb.pl" "$work/twice.pl" && said "$work/out" "1 of the baseline's are missing"; then
elif cmp -s "$work/kb.pl" "$work/twice.pl" && said "$work/out" "1 of the baseline's are missing" \
&& said "$work/out" "a_probe_only_head_has"; then
ok "a recorded rm excuses only the clause it removed"
else
bad "the unrecorded loss was refused, but not by name, or the kb changed"
@@ -611,5 +628,131 @@ else
ok "a write killed before its rename leaves the old kb byte-identical"
fi
echo "kb-test: what the ledger and the key comparison can be fooled by"
# 23. Keys compared as a SET cannot see one of two clauses sharing a key
# (improvise-ih3k). work_done/3 keys on (id, kind) and is not functional/1,
# so a git merge can commit two clauses under one key. Dropping one then
# leaves the key present and the loss unseen. The baseline carries the pair.
awk -v a="work_done('improvise-zzzz',bug_fix,'first fix, merged from branch A')." \
-v b="work_done('improvise-zzzz',bug_fix,'review fix, merged from branch B')." '
{ line[NR] = $0 } /^work_done\(/ { last = NR }
END { for (i = 1; i <= NR; i++) { print line[i]; if (i == last) { print a; print b } } }
' "$KB" > "$work/pair-base.pl"
grep -v "^work_done('improvise-zzzz',bug_fix,'review fix" "$work/pair-base.pl" > "$work/pair-lost.pl"
pkb() { KB_FILE="$work/kb.pl" KB_BASELINE="$work/pair-base.pl" "$here/kb" "$@"; }
fresh
if ! KB_FILE="$work/pair-base.pl" "$here/kb" check >/dev/null 2>&1; then
bad "the paired baseline fails kb_consistent -- case 23 proves nothing"
elif [ "$(grep -c "^work_done('improvise-zzzz'" "$work/pair-lost.pl")" -ne 1 ]; then
bad "the lossy fixture does not hold exactly one of the pair -- case 23 proves nothing"
else
cp "$work/pair-lost.pl" "$work/kb.pl"
if pkb add "$work/new.pl" >"$work/out" 2>&1; then
bad "add wrote onto a kb that lost one of two clauses sharing a key"
elif ! cmp -s "$work/kb.pl" "$work/pair-lost.pl"; then
bad "the lost twin was refused but the kb was MODIFIED"
elif said "$work/out" "review fix, merged from branch B"; then
ok "losing one of two clauses under one key is refused, naming the one lost"
else
bad "the lost twin was refused, but the refusal does not name it"
fi
# The pair itself is not a loss: an add onto the intact pair must land, or
# the guard has turned "HEAD holds a duplicate" into "nothing may be written".
cp "$work/pair-base.pl" "$work/kb.pl"
if pkb add "$work/new.pl" >"$work/out" 2>&1; then
ok "an add onto a kb that still holds both clauses of a pair is accepted"
else
bad "the intact pair was refused: $(tail -1 "$work/out")"
fi
# And the pair has a way out through the tool: rm the key (both go, recorded),
# add back the survivor, and a later write must still land.
printf "work_done('improvise-zzzz',bug_fix,_).\n" > "$work/pair-key.pl"
printf "work_done('improvise-zzzz',bug_fix,'the fix, both halves').\n" > "$work/pair-one.pl"
printf "doctrine(a_probe_after_the_pair_is_resolved,'x').\n" > "$work/after-pair.pl"
if ! pkb rm "$work/pair-key.pl" >"$work/out" 2>&1; then
bad "rm of a key HEAD holds twice was refused: $(tail -1 "$work/out")"
elif ! pkb add "$work/pair-one.pl" >"$work/out" 2>&1; then
bad "re-adding one clause for a removed pair was refused: $(tail -1 "$work/out")"
elif ! pkb add "$work/after-pair.pl" >"$work/out" 2>&1; then
bad "a write after resolving the pair was refused: $(tail -1 "$work/out")"
else
ok "a pair is resolved by rm then add, and later writes still land"
fi
fi
# 24. A ledger entry excuses only the kb it was recorded against
# (improvise-yea8). rm X, then restore kb.pl the way the refusal message
# recommends -- not a write, so nothing prunes -- then lose X some other
# way. The ledger entry from the abandoned rm must not excuse that loss.
fresh
printf "location(cli_args,'src/main.rs',_).\n" > "$work/drop-x.pl"
grep -v "^location(cli_args," "$KB" > "$work/lost-x.pl"
if ! kb rm "$work/drop-x.pl" >"$work/out" 2>&1; then
bad "rm of a clause HEAD has was refused -- case 24 proves nothing"
elif [ ! -s "$work/kb.pl.removed" ]; then
bad "rm left no ledger -- case 24 proves nothing"
else
cp "$work/lost-x.pl" "$work/kb.pl"
if kb add "$work/new.pl" >"$work/out" 2>&1; then
bad "a ledger entry from an abandoned rm excused a later loss of that clause"
elif ! cmp -s "$work/kb.pl" "$work/lost-x.pl"; then
bad "the stale excuse was refused but the kb was MODIFIED"
elif said "$work/out" "location(cli_args,'src/main.rs',clap)"; then
ok "a ledger entry does not outlive the kb its rm produced"
else
bad "the loss was refused, but not by naming the lost clause"
fi
fi
# 24b. ...and still excuses it across the tool's own writes: the stamp moves
# with each write, so rm then add then add must all land. A ledger stamped
# only by rm passes 24 and refuses the second add here.
fresh
printf "doctrine(a_second_probe_after_the_rm,'x').\n" > "$work/new3.pl"
if ! kb rm "$work/drop-x.pl" >"$work/out" 2>&1; then
bad "rm refused -- case 24b proves nothing"
elif ! kb add "$work/new.pl" >"$work/out" 2>&1; then
bad "the first add after an rm was refused: $(tail -1 "$work/out")"
elif ! kb add "$work/new3.pl" >"$work/out" 2>&1; then
bad "the second add after an rm was refused: the ledger did not follow the write"
else
ok "a recorded rm stays excused across the tool's later writes"
fi
# 25. The production paths, which every case above bypasses by setting KB_FILE:
# the baseline is `git show HEAD` and the ledger is `git rev-parse
# --git-path`. Run in a throwaway repo holding a copy of the tool.
r="$work/repo"
mkdir -p "$r/scripts" "$r/prolog-sessions"
cp "$here/kb" "$here/kb-merge.pl" "$r/scripts/"
cp "$KB" "$r/prolog-sessions/kb.pl"
g() { git -C "$r" -c user.name=kb-test -c user.email=kb-test@invalid "$@"; }
rkb() { (unset KB_FILE KB_BASELINE; "$r/scripts/kb" "$@"); }
if ! { g init -q && g add -A && g commit -qm base; } >/dev/null 2>&1; then
bad "could not build the throwaway repo -- case 25 proves nothing"
elif ! rkb rm "$work/drop-x.pl" >"$work/out" 2>&1; then
bad "rm in a real repo was refused: $(tail -1 "$work/out")"
else
ledger="$(g rev-parse --path-format=absolute --git-path kb-removed.pl)"
if ! grep -q "^location(cli_args," "$ledger" 2>/dev/null; then
bad "rm in a real repo did not record its removal at the git-path ledger"
elif [ "$(g status --porcelain --untracked-files=all)" != " M prolog-sessions/kb.pl" ]; then
bad "the real-repo write left something git sees: $(g status --porcelain --untracked-files=all | tr '\n' ' ')"
elif ! rkb add "$work/new.pl" >"$work/out" 2>&1; then
bad "the git-path ledger did not excuse its own rm: $(tail -1 "$work/out")"
else
g checkout -q -- prolog-sessions/kb.pl
cp "$work/lost-x.pl" "$r/prolog-sessions/kb.pl"
if rkb add "$work/new.pl" >"$work/out" 2>&1; then
bad "after git checkout restored the kb, the git-path ledger excused a new loss"
elif said "$work/out" "location(cli_args,'src/main.rs',clap)"; then
ok "the git-path ledger and HEAD baseline work, and a loss after a restore is refused"
else
bad "the real-repo loss was refused, but not by naming it"
fi
fi
fi
printf '\nkb-test: %d passed, %d failed\n' "$pass" "$fail"
[ "$fail" -eq 0 ]
+4 -4
View File
@@ -594,12 +594,12 @@ function edit(td, row, col, seed) {
});
}
/** A side panel. An empty row list is what says it is closed — the page asks
* no question about the mode, exactly as `paintHelp` does not. */
/** A side panel. `null` is what says it is closed, and `[]` is open and empty:
* two facts, never inferred from a count (improvise-df2w). */
function paintPanel(host, rows, build) {
host.hidden = rows.length === 0;
host.hidden = rows === null;
const list = document.createElement("ul");
rows.forEach((row, index) => {
(rows ?? []).forEach((row, index) => {
const li = build(row);
li.dataset.index = String(index);
list.appendChild(li);