All three were demonstrated by injection against a green suite — this repo's
characteristic defect, and the PR's own evidence table asserted coverage it did
not have.
- `ClientSession::view_in` had no guard: pointing it at the host's view left
1313 tests green. Only `grid_in` was checked; `category_tree_in` and
`tile_bar_in` were not, and the tile bar is the control a user reassigns axes
with. `a_session_on_another_view_is_projected_that_views_axes` closes it.
- `adopt_deleted_views` and `apply_switch` had vacuous audience assertions:
both bystanders sat on the document default, where a wrongly-addressed relay
folds to a no-op. Both now sit on a view they can be moved off, and both read
the audience off the `Broadcast` rather than inferring it from folded state.
- `ViewBack`/`ViewForward`'s `EffectKind` had no guard — reclassifying either
was silent. `walking_the_view_history_moves_no_document_state` asserts the
consequences (`dirty`, `layout_version`) rather than the declaration.
Four follow-ups filed: improvise-w3bf, improvise-pubt, improvise-h074, and the
reachability trigger appended to improvise-lltt.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by adversarial review of PR #47; I verified the chain rather than taking
it. shell.spec.js's "the standalone deployment" loads the app with no ?ws=,
which spawns the worker (app.js:112), which imports the glue (worker.js:4),
which calls bindings.rs::start (:30) and OpfsFs::open (:35) — and the spec then
asserts a grid draws and a cell is editable. So the whole chain has executed
under Chromium in CI since improvise-vvl3 landed the harness.
Five places claimed it never runs: gotcha 45, the File Inventory line, two
kb.pl facts, and improvise-bck's grooming note. Two of those are corrected here;
kb.pl goes with the wave-end batch, and the bead belongs to the WU-P agent.
Every copy was written before the harness landed and none was re-read after —
gotcha 48's rule ("when a fact is stated in more than one place, changing it is
a grep, not an edit") aimed at exactly this and did not fire, because nobody
grepped for a claim they had no reason to doubt.
The sharper lesson is in the gotcha now: a "never run" written in prose is
unfalsifiable by any check this repo has. kb.pl's CI gate verifies the knowledge
base is CONSISTENT, not that it is TRUE, so a false invariant passes it forever.
And this one understated coverage, so nothing failed and nobody looked — the
comfortable direction to be wrong in, and the reason it lasted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from running /code-review over this branch.
a_filter_axis_is_projected_but_not_cycled builds its own PageAxis by hand, so
it says nothing about what grid_of actually emits — a test that fabricates the
very data the code under test produces, which is this repo's characteristic
defect. a_page_axis_carries_its_items_and_a_filter_axis_does_not is the
producer half; it fails under `let navigable = true`, which is filtering_cats()
leaking into the protocol, the exact trap gotcha 27 names.
improvise-ir7l holds in RECORDS mode only by accident: cell_id's records branch
never asks Model::formula_for, and it is safe purely because records_columns
builds from ordered_item_names (explicit _Measure items) while a formula target
lives only in measure_item_names. a_records_grid_has_no_column_for_a_formula_
target aims a guard one level below the fix, and fails under the plausible
tidy-up that swaps the two — a Profit column appears, and it would project as a
writable Record.
ADR 002 specified a two-variant CellId. It now has four, and a fact stated in
two places is changed by a grep. The ADR gets an Extension section rather than a
rewrite: the decision is unchanged, and both new variants were arrived at by
applying its clause 1 rather than departing from it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on PR #47.
A 120-column cast shrunk into a 390px viewport computed a **4.83px** terminal
font — measured in Chromium against the assembled tree. That was tolerable when
the landing page sat at `/landing/`; since the user's ruling it is the first
thing a phone visitor sees. Under 900px each player now scrolls sideways at a
legible size instead: **11.95px**, measured the same way, with the page's own
`scrollWidth` still equal to its `clientWidth` — the overflow is contained
inside the player, so there is no horizontal PAGE scroll.
That takes `docs/index.html` to 209 lines, past improvise-e61's "under 200".
The criterion is reinterpreted rather than crammed to fit, and said so on the
ticket: the number was a proxy for single file, vanilla, no build step, no
framework, all of which hold. A limit met by one line and checked by nothing was
never really met.
`context/plan.md` carried two instructions that this PR made actively
misleading:
- §2.4 still told a re-recorder to use 100x30. That is the instruction whoever
picks up improvise-2oxt would follow, and it disagrees with both the existing
casts and the player. It now says 120x37, and says the two must be changed
together.
- §4.2 still said to set Pages to branch `main` / folder `/docs`. That used to
fail loudly — there was no `docs/index.html`, so it was a 404 — and since
improvise-e61 it fails QUIETLY: the landing page renders perfectly and its
call to action 404s, because `app/` exists only in the assembled tree. It now
says GitHub Actions, and says why the other setting is worse than it looks.
Verified: 6/6 harness shell specs still pass against the assembled tree, the
four casts still play 37 lines each at the root, and the call to action still
resolves 200. cargo test 1311, both comment checkers clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-tom1. The user has ruled that a remote client must NOT see another
client's staged drill edits, so the invisibility is correct and was never the
defect. What remained was that three places answered "what text is in this
cell" — and only two of them still do.
GridLayout::records_value is now the resolution: it returns the CellValue, and
records_display is one to_string over it, so the text can never disagree with
the value it spells. project::cell_display used to call records_display and
then GUESS numeric-ness with text.parse::<f64>() — the one place the producer
guessed rather than knew, and it was wrong: a records coordinate column holds
an item NAME, so a Year category with items 2024/2025 projected a coordinate
the browser right-aligns as a number. Nothing in the suite could see it,
because every records fixture in the tree names its items Region/East/North.
A correction to the ticket worth more than the code: src/ui/grid.rs is NOT a
third answer. Since improvise-anr9 the TUI grid takes its text off GridCache
and then overlays drill_state.pending_edits — one derivation plus a
session-local overlay, which is exactly what the ruling wants, and
App::display_value_of does the same for the editor. The ticket was written
before anr9 landed.
Both halves of the ruling are now asserted, with the leak injected as the
fault: teaching the producer to prefer pending_edits, which is the plausible
"fix" the ticket exists to forbid.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A second run of benches/grid_projection put the 'large' row's col_widths share at
45% where the first said 64%. 45-46% is what reproduces; 64% was noise, and a
single-run number written down as a fact is the same defect as an argument
written down as a measurement -- which is what the benchmark existed to avoid.
What does not move between runs is what the decision rests on: ratio stays
1.2-1.5x and col_widths stays at roughly half the frame. improvise-jmjz's title
and notes corrected to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three tickets asked one question — where do UI strings live relative to a
serde wire enum — and answering it three times is the disease this unit exists
to cure. One answer, in `improvise-protocol/src/help.rs`:
**A UI string is keyed by a command name, never by a key, and never hangs off
the enum that crosses the wire.** `AppMode::label`/`hint_text` are gone;
`help::mode_label`/`help::hint_line` replace them, so the enum a client
deserializes carries no English at all (improvise-k099). **And wherever a
string names a key, the key is looked up rather than spelled**:
`Hint { keys, command, caption }` takes `keys` off `Keymaps::default_keymaps()`,
`command` is the semantic value a localized front end keys off, and `caption`
is the default English — the `AxisColor`/`axis_fg` shape.
- **improvise-x7sw**: `help::which_key` is the which-key popup's rows, keys and
captions together. The client already holds the keymaps, which is the answer
the ticket preferred, so `ViewState::transient_prefix` stays an address.
- **improvise-ktv1**: the help pages are data — `help::pages()` — with every
key row derived. `src/ui/help.rs` keeps its ratatui styles and loses ~310
lines of content; `web/app.js` paints rows it does not author, and
`harness/tests/shell.spec.js` asserts that `?` really draws them.
**What this deletes is the measure of it.**
`every_hint_key_resolves_in_its_mode` and its 60-line hint tokenizer are gone:
they existed to hold a hand-written string honest against the keymap, and the
string is derived now, so the property is true by construction
(`doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)`).
`Keymap::binding_hints` goes with them — `which_key` is the one walker, and the
binding dump reads it. What replaced the test checks the *other* direction, a
mode advertising a command nothing binds, which was never checkable before.
Two defects the derivation exposed, both fixed here: `key_label` dropped
modifiers, so Ctrl-arrows read as bare arrows, and `Char(' ')` printed a literal
space instead of `Space`.
**What is deliberately not derived**, and says so: the import wizard's keys
(its whole keymap is one `Any` binding into `HandleWizardKey`, which decides
per step inside the command — a real hole, left visible), `gg`/`yy`/`ZZ` and
the ex-commands (`HelpLine::Literal`), and the captions, which no test can
judge. `context/code/protocol.md` §8 has the argument.
Cost, stated: the hints changed shape. They are longer, they name arrow keys
beside `hjkl`, and two sub-lists on the Panels page lost their visual indent.
That is the price of the keys being true.
Evidence: 1321 tests pass, and the 7 Chromium shell tests pass against the
assembled site. Two faults were injected into the help overlay — dropping the
`paintHelp` call, and removing the `mode != Help` guard so it is never hidden —
and each fails the new spec.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-n89z. Three Projection mutations had no producer. They get two
different answers, and the distinction is the finding.
GridUpdate::Invalidated is DELETED. It was not merely unused, it was
REDUNDANT: `grid_updates` diffs, so it always knows which cells moved and sends
Cells, and `GridCache::seed` compacts — there is no state in which a server
knows the contents are stale and not which ones, and if there were, the answer
is a fresh Snapshot through `rebase`. Meanwhile its stated meaning had migrated
into running code, because PR #23 gave `invalidate_relabelled` the job of
blanking exactly what a reframing invalidates, totals included. Three documents
still described it as load-bearing, which is worse than silence: a reader
reasoning about the fold's guarantees assumed something produced it.
CategoryTreeUpdate::Items and ::Axes STAY, with the argument written down.
They are derived rather than invented — the effects that move a category tree
come in three shapes, by the same recipe that built ModelCommand — and the
producer takes the coarse arm on purpose, because Dirty::of cannot say WHICH
category moved (an EffectKind never can), so emitting them needs the tree
observed and diffed for a widget that costs a few hundred bytes.
The two tests that reached the blanking through the retired arm now reach it
through a reframing, which is what really produces it; the row one is strictly
stronger, since blanking everything could not tell "blanks what it reframes"
from "blanks the lot". `the_retired_invalidated_spelling_does_not_parse` keeps
it from returning as a serde alias, with the injected-fault discipline
`the_retired_spellings_do_not_parse` earned: well-formed input first, error
KIND asserted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-review caught three references the deletion commit missed, which is gotcha
48's rule arriving on schedule: when a fact is stated in more than one place,
changing it is a grep, not an edit.
- context/code/command.md still had a `### reduce_full / reduce_key` section
calling it "the signature the three hosts share" -- the exact claim the ticket
was filed against, two hundred lines below the paragraph that corrects it.
- ADR 001 stated in the present tense that apply_command/reduce_full do not
check, and recorded improvise-ondj giving both a Checked signature without
saying that one of them has since gone. The ADR now says why deleting beat
typing: having no caller was the condition that let the defect hide, and a
Checked on a function nobody calls fixes the type and not that.
- The test-evidence entry's "no longer compiles" is now "no longer exists".
Also records the AddItem/AddItemInGroup success gating and the four effects that
reset unconditionally because they cannot report a miss.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-poia. `+` and `+cat` projected `None`, which the protocol defines as
"no projected cell", so a client could not tell a column that creates something
from a padded slot — gotcha 42's conflation, a third time.
The ticket offered two designs and both assumed a cell write. Neither happens:
add_measure emits exactly one effect, AddItem { _Measure, name }, and
add_category emits AddCategory(name) and deliberately writes no coordinate on
the current row (improvise-2zq). The record under the cursor is not part of the
operation and neither is any value — what the user types is a NAME. So there is
no distinct command to invent and no two-command flow to sequence.
What was missing was never an address but a recognition. CellId::Create carries
the kind and nothing else, because the kind plus the typed name is the whole
operand. It is not a SetCell target, so editable stays Record-only and
record_at_cursor still answers None — both by matching, which the compiler now
forces.
No client composes it yet: keys.rs::commit returns early for the whole of
records mode, and that guard is broader than its stated reason. Filed as
improvise-gcvb, with the two things that make it more than a one-liner — an
inserted +cat column means the TUI does not advance, and AddCategory can abort.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Self-review of improvise-9mo9's own commit. AddCategory was gated on success and
AddItem/AddItemInGroup were not: the reset sat after the `if let Err`, so a typo'd
category name bumped cursor_epoch and broadcast Command::ResetCursor to every
client for a change the document never made. The epoch is document-wide, so the
cost was paid by everyone.
SetNumberFormat's negative arm cannot see this -- it tests a command that succeeds
and reshapes nothing, where this is one that reshapes nothing because it was
refused. a_refused_model_command_invalidates_no_cursor is the second arm, proved
by moving the reset back outside the match and watching it fail on the bystander's
cursor.
Also drops apply_command's now-dead `Command::Project(_)` arm: since the fallback
stopped folding, both returned Dirty::NOTHING and the first was unreachable
duplication.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Broadcast::ToAsker` became `Broadcast::To(SessionId, _)` when the deleted-view
case needed a one-session audience that is not the asker; three doc references
still spelled the old name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-drg proposed narrowing each Effect::apply to the slice it touches. It
was filed with three unpicked mechanisms and a note to decide before claiming.
Declined, and the counts are the argument.
Over the 49 impl Effect blocks: 8 touch only view_state, 9 only model_state, 32
cross-cut. At this wave's base commit the split was 9 / 16 / 24, so the narrow
set fell from 25 to 17 in one wave -- and the mover was improvise-9mo9, earlier
on this branch, which made nine reshaping effects call app.reset_cursor(). That
is architectural, not incidental: a reshape lives in model_state.workbook and the
cursor it invalidates lives in ViewState, so the effects that matter most are the
ones that reconcile the two slices.
Three more grounds. The type-level documentation already exists and is checked --
Effect::kind() has no default and apply_effects reads that one answer three times
-- so a narrowed signature would be a second statement of the same fact, free to
disagree. Every offered mechanism weakens a guard: three traits need three queues
(destroying batch order, which gotchas 21/23/43 lean on) or an enum wrapper (a
fourth classification beside EffectKind), and default methods give `apply` a
default whose wrong answer is silent, which is exactly what kind()'s no-default
rule exists to prevent. And finer-grained effect tests are not blocked: the 64
tests in effect.rs already drive apply against a one-line App fixture.
Approach (B) -- rebinding the slice at the top of a body -- stays available as a
per-function style choice and is not mandated.
The bead carries the `human` label, so this is recorded as a recommendation with
its evidence rather than settled unilaterally.
Closes improvise-drg
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-z754. draw_title read workbook.model.name, model_state.dirty and
model_state.file_path; none was projected, so a browser could not draw a title
bar at all.
A projection, not a ViewState field: these are facts about the DOCUMENT, shared
by every client and writable only by the server. On ViewState they would be
document state wearing a session slice's clothes, and reduce_view would need
commands nothing composes. The per-widget cache is the shape for "a local copy
of what a widget draws, replaced by a Projection".
It is the one widget no EffectKind can gate, and that is the whole design.
`dirty` is raised by any EffectKind::Model(_) and cleared by effect::Save,
which is ViewOnly and correctly so; SaveAs moves file_path and is also
ViewOnly. So Dirty::of reports NOTHING for exactly the effects that change this
widget most visibly. ClientSession::document_update therefore OBSERVES — build
the desired cache, diff it against the mirror — the same answer record_versions
got for the same reason.
`stored_as` is a name, never a path. The TUI already drew only file_name(), and
a socket server projecting file_path would hand a remote client the host's
directory layout. `None` carries the fact ClientFrame::Save fails on.
draw_title now draws from project::document_of, so the two front ends share
one derivation. web/app.js deliberately does NOT paint it yet — every harness
fixture mocks a FAILING peer, so a title element added now would be unrun
JavaScript, which gotcha 48 forbids. Filed as improvise-5jna, fixture first.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The host-side half of `adopt_deleted_views`, and a desync that predates the
split: `Workbook::delete_view` moved the document's active view and nothing
moved `ViewState::active_view`, so the cursor was read under a key naming a view
that was gone. It was invisible while `draw_status` read the workbook's copy;
improvise-k69y makes it read the client's, which turns it into a wrong badge.
`ClientSession::view_in` also goes through `App::resolve_view` now, rather than
carrying its own copy of the fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two findings from running the code-review skill on this branch.
`Workbook::delete_view` used to move the document's active view and
`adopt_active_view` pushed the result into every mirror, so deleting the view a
client was on moved that client. The split removed both, leaving that client
naming a view the document has not got — projected the default's grid while its
own slice says otherwise. `Sessions::adopt_deleted_views` answers it, observed
rather than predicted: the check is whether a mirror still names a view the
document has, not whether the command was a `DeleteView`.
That needs a one-session audience that is not the asker, so `Broadcast::ToAsker`
becomes `Broadcast::To(SessionId, Command)` and `Host::relay` reads the address
off the message rather than taking `from`.
And "what does this name name in this document" was answered twice — in
`App::view_name` and again inside `project::grid_in`. `App::resolve_view` is now
the one home both go through.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-i5tk's acceptance criterion 1 was a benchmark, on the ground that "this
can dominate TUI rendering" is exactly as unevidenced as "it is fine".
benches/grid_projection.rs is that benchmark, and it says no.
One grid_of call, dense pivot grids, this machine:
typical 100 x 4 = 400 cells whole-grid 1.902ms ratio 1.2x col_widths 45%
large 1200 x 8 = 9600 cells whole-grid 185.644ms ratio 1.3x col_widths 64%
18k cells (gotcha 41) 3000 x 6 = 18000 cells whole-grid 402.435ms ratio 1.5x col_widths 42%
pathological 9000 x 10 = 90000 cells whole-grid 5.235s ratio 1.5x col_widths 44%
ratio is whole-grid over a 45x12 terminal window: 1.2-1.5x, against a fix whose
stated cost is changing what a cache index MEANS in the TUI and putting every
scroll site in scope. Wrong trade, so i5tk is closed on the measurement.
Why the ratio is so low is the bigger finding. Two of the three whole-grid passes
a frame performs are viewport-independent by construction:
ui/viewport.rs::compute_col_widths takes no viewport and walks every (row, col)
twice -- once for cell text, once for the column totals it must also size -- and a
column total must sum every row including scrolled-past ones (gotcha 44).
grid_of's own cell_display loop is the third pass and the only one windowing would
touch. So the whole grid is evaluated about three times per frame, every result
discarded, and at 18,000 cells a keypress costs four tenths of a second. Filed as
improvise-jmjz; its fix is caching or one fused pass, which this benchmark has now
distinguished from the windowing it rules out.
No criterion crate: plain Instant, and test = false so cargo test does not run a
target that takes seconds by design.
Closes improvise-i5tk
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`context/code/command.md` gains the derivation for improvise-jytm: what moved,
what projects per client, why `effect::SwitchView` is `ViewOnly`, and why
`adopt_active_view`/`Broadcast::ToOthers` are deleted rather than left.
`context/test-evidence/command.md` and `clients.md` follow the tests that were
replaced, each entry naming the fault injected to prove the new check can fail.
Two gaps filed rather than smuggled in: improvise-lltt (a view-scoped
`ModelCommand` names no view) and improvise-azhb (`Initial View` no longer
follows the TUI user).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four findings, all verified against the tree before acting.
F10 — this PR's own edit to design-principles §5 made a sibling note in
context/code/command.md false: it said §5 "still names Sessions::apply" and
recommended apply_checked. A PR whose whole subject is stale prose had left a
new stale copy behind, which is gotcha 48 exactly ("changing it is a grep, not
an edit"). Note deleted; the verified claim above it stands.
F11 — §5 named both apply and apply_checked as "the one door". apply has no
production caller anywhere in the workspace; Host calls apply_checked. Naming
two weakens the claim §5 makes, so it now names the one production uses.
F12 — gotcha 17 said "four call sites". There are six uses across the two
evaluators: a div/0 guard plus the Eq and Ne arms in each. In a file agents
follow literally, a reader who greps and finds six cannot tell whether the
prose or the code moved.
F13 — gotcha 17's sibling said the Dirty::of doc comment "keeps only the
one-line reason LayoutGeometry is spelled out rather than EVERYTHING". It says
no such thing: it points at context/code/command.md, and the EVERYTHING reason
is a two-line comment inside the match body. A correction to prose that
asserted the negation of the code should not itself misdescribe the code.
Two findings deliberately not fixed here and filed instead: improvise-ahdf
(repo-map gotchas 36 and 43 carry the same stale Sessions::apply claim —
pre-existing, and inside long paragraphs that need reading rather than a sed)
and improvise-rm4s (--check-evidence accepts headings naming tests that never
run).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-p1ym. GridCache::page_coords carried (category, selected item), so
PageNext/PagePrev resolved to a command name the browser held but could not
compose an argument for. They reached Host::handle, which answers an empty vec,
so a smoke test found them inert on the live page.
The item list is not a second field beside the selection: page_coords becomes
Vec<PageAxis>, and `items` is Some exactly when the axis is Axis::Page. That
puts gotcha 27's split in the type — a Filter axis constrains without being
navigable, so it carries no list and no caller can compose a cycle for one.
GridLayout::page_coords stays filtering_cats(), which is right for the bar,
and navigation.rs::page_cat_data stops spelling the Page-vs-Filter rule a
second time: it reads the layout's list.
The browser's page_cycle mirrors PageNext/PagePrev line for line, odometer
carry included, and `[`/`]` leave the still-needs-the-host list.
Two decisions stated in context/code/core.md. The list is
effective_item_names — the same call that already chose the default selection,
so selection and list come from one place; page_cat_data used
ordered_item_names before, which for a _Measure page axis let the default
selection be a formula target `[`/`]` could not reach. And the list is NOT
filtered to visible items: the TUI cycles hidden ones, and making the browser
differ would be the divergence rather than the fix.
The lockstep test's first version was vacuous — it rebuilt the client's cache
from the host and compared it against that same host. It now drives two
independent documents and compares View::page_selection; the injection that
reverses the odometer's significance now fails it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`cycle-panel-focus`; the category panel's `cycle-axis-at-cursor`,
`open-item-add-at-cursor`, `filter-to-item`, `toggle-cat-expand` and
`delete-category-at-cursor`; the view panel's `switch-view-at-cursor`,
`create-and-switch-view` and `delete-view-at-cursor`; tile select's
`set-axis-for-tile` and `cycle-axis-for-tile`; the help overlay's
`help-page-next`/`help-page-prev`; `commit-buffer` in all three kinds; and the
two cell writes ADR 002 lets a thin client name, `clear-cell` and `paste`.
Each mirrors its `Cmd` line for line, status sentence included — a client that
emitted the mutation without the sentence beside it would diverge on the status
line and nowhere else. Two orderings are copied rather than rationalised:
`CommitBuffer` writes its optimistic status first (improvise-6t3) and
`TileAxisOp` writes its axis first.
**A divergence found and fixed on the way.** `effect::HelpPageNext` clamps at
the last page and `HelpPagePrev` saturates at zero, while `reduce_view`'s
`Command::HelpPage` arm *wrapped*. Two derivations of one fact, disagreeing,
invisible because `h`/`l` in the help overlay had always been upstream keys and
nothing had ever sent the command. `walking_off_the_end_of_the_help_pages_
agrees_with_the_host` was written first and failed — client 0, host 4 — and the
wire was then changed to match the effect, since the effect is what ships in
the TUI. Whether both should wrap instead is improvise-trd3.
**Three names the ticket listed do need enrichment**, filed as improvise-4bwa:
`toggle-prune-empty` words its status off `View::prune_empty`, which is in no
cache; `delete-formula-at-cursor` needs `(target, target_category)` and
`FormulaCache` carries only the raw text; and `view-back` emits
`ApplyAndClearDrill`, which is deliberately off the wire (gotcha 38) and is
blocked on ADR 001 layer 2 rather than on a projection.
The lockstep harness is new and is the evidence: it applies what the client
composed to a **second authoritative host** and compares the documents as
`.improv` text (`Debug` prints the symbol table's `HashMap`s, so it differs per
process — gotcha 18). Four injected faults — a wrong axis, a wrong status
sentence, a constant view index, and `AddCategory` for `AddItem` — each fail a
named test; the second one did *not* until the fixture was made to sit on a
category header as well as an item row.
`the_command_names_that_still_need_the_host_are_these` shrinks by exactly what
closed. 1318 tests pass, up from 1311.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-jytm, improvise-6c6q, improvise-k69y.
`Workbook::active_view` becomes the document's *default* — what a freshly
connected client opens and what `.improv` writes as `Initial View` — and
`ViewState::active_view` becomes authoritative per client. Two clients may sit
on different views of one document: the model is shared, the UI is not.
- `App::switch_to_view` writes only the client's slice; `App::view()`/
`view_mut()`/`view_name()` resolve it against the workbook.
- `effect::SwitchView`/`ViewBack`/`ViewForward` are `ViewOnly`, which takes
`ViewBack`/`ViewForward` off `ModelCommand` — their operand is one client's
history stack (the `ApplyAndClearDrill` argument).
- `grid_in`/`category_tree_in`/`tile_bar_in` project against the session's view;
`ClientSession` observes its own move through `projected_view`.
- `apply_switch` takes a `SessionId`, moves only the asker's mirror, and relays
`Broadcast::ToAsker` only when the document refused. `adopt_active_view` and
`Broadcast::ToOthers` are deleted: nothing produces them any more.
- `draw_status` reads the client's copy (improvise-k69y).
`the_two_active_view_names_never_diverge` is replaced, not deleted, by
`the_documents_default_view_outlives_this_clients_switches` and
`a_fresh_session_opens_the_documents_default_view`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ui/reduce.rs claimed every deployment mode reached the model through it. That was
aspirational, and it is what let improvise-ondj's missing basis check sit
unnoticed: reduce_full had no production caller anywhere in the workspace, so
nothing exercised its Command::Model arm and nothing could notice it skipped the
check.
Deleted rather than wired up, on the argument in its own doc comment: "Per-client
grid updates need a viewport and come from Sessions::project_all." It returned
half of what a host needs. The browser worker was the natural candidate caller and
reaches the model through Host::handle -> Sessions::apply_checked, which is the
half that has the viewport. Its only caller was a test helper, which now spells
the two lines it was.
apply_command's fallback arm no longer folds. It routed a wire view command into
app.view_state -- the authoritative slice -- which nothing in production did and
nothing should: Sessions::apply_checked folds it into the asking client's mirror,
because the model is shared and the UI is not. A client's SetStatus moving the
host's status line was behaviour that was safe only because the arm was dead.
view_slice_commands_land_in_the_authoritative_view_state pinned exactly that, so
it is inverted rather than deleted, and its fixture writes a value into the status
line first so the assertion can tell "the fold was removed" from "the field was
empty anyway". Restoring the fold fails it.
Sessions::apply is now #[cfg(test)]. Host::commands answers the whole frame with
CheckedBatch::against, so a door that answers one command and then dispatches is
the lost update improvise-955n fixed; cfg(test) makes "no production caller" a
fact the compiler keeps rather than a note verified at one commit.
Closes improvise-s17o
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-ir7l. project::cell_id decided Record vs Aggregate purely on whether
a regular category sat on Axis::None; it never asked whether the coordinate
named a formula target. So a formula cell was a Record, render.rs::editable
said true, and a write to it landed, was stored, and was then shadowed forever
by formula_cache — not a revert, a permanent silent shadow, on the demo's
front page.
Rather than a second "but not really" flag beside CellId (the two-answers
shape ADR 002 exists to forbid), a formula-owned cell now projects a different
kind of identity. Derived carries the formula's target and no surrogate, so
the write is unconstructible rather than refused.
The predicate is Model::formula_for(&CellKey), one home, which already existed
three times open-coded inside model/types.rs. The TUI lands in the same change:
EditOrDrill calls the same predicate and refuses, or the two front ends
diverge.
Two orderings are load-bearing and tested: Derived is asked before the record
branch (the coordinate is stored), and after the aggregate branch (gotcha 10 —
an aggregating formula cell is still drillable).
Two tests were passing while their names were false for the Profit column and
are renamed and two-sided now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Only TransposeAxes and CycleAxis reset the cursor, while eight siblings that
renumber just as thoroughly did not. SetAxis is the sharp case: CycleAxis *is*
SetAxis to Axis::next(), so a pair differing only by an argument disagreed.
reset_cursor is also the only writer of cursor_epoch, which is the only thing
Sessions::adopt_cursor_epoch watches, so each gap was invisible in both
directions at once: the host's own cursor stayed stale and no client was told.
Gotcha 24 records that the second half is strictly worse than doing neither.
SetAxis, HideItem, ShowItem, ToggleGroup, TogglePruneEmpty, AddCategory,
RemoveCategory, AddItem, AddItemInGroup and RemoveItem now reset. The item
effects are in because "an append cannot move an existing position" holds only
of the outermost category on an axis: expand_category runs inside the cross
product, so an item appended to an inner category repeats once per outer block.
SetNumberFormat stays out and keeps the negative arm.
CommitAndAdvance's +cat branch depended on the missing reset — it stayed put so
the cursor would land on the column it had just created (improvise-2zq). It now
says so with an explicit SetSelected, placed after AddCategory's abort so a
refused category still leaves the batch alone.
The new test asserts of every case that row_items/col_items really changed
before asserting anything about a cursor; two faults were injected to show both
arms discriminate. Reasoning in context/code/command.md, evidence in
context/test-evidence/command.md. SetPageSelection under prune_empty is the one
residual and is filed as improvise-7kvw.
Closes improvise-9mo9
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`resolve`/`step` and `ClientState` lived in `improvise-wasm-client`, and
nothing about either was a browser fact: both depend only on `ViewState`, the
protocol widget caches and the announced window, all already protocol types.
The move costs no new dependency and makes design-principles §5 true rather
than aspirational — a new front end now needs exactly two things, its own
event → `KeyEvent` mapping and painting.
- `state.rs` → `improvise-protocol/src/client.rs`
- `keys.rs::{resolve, step, KeyOutcome}` → `improvise-protocol/src/resolve.rs`
- `socket.rs::{typed, typed_cell_command}` → `resolve.rs`, since `step` calls it
- `BrowserKey` stays in `improvise-wasm-client/src/keys.rs`: it *is* the
browser half of the two things a front end still owns
- `render_model` becomes a free function, `ClientState` being foreign now
`tests/lockstep.rs` is kept deliberately. It drives `reduce_key` — the host's
`Cmd`/`Effect` path — against `resolve`; those are still two derivations in two
crates, one unreachable from a browser, so the move changed which crate
`resolve` is spelled in and not that it is a mirror.
`doctrine(a_consolidation_that_keeps_its_compatibility_test_is_unfinished)`
does not fire.
1311 tests pass, unchanged from the baseline: this commit moves code and
changes no behaviour.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Orchestrator doc pass over the comment wave. Each claim was verified against
the tree rather than taken from the queue.
Gotcha 17 recorded the *opposite* of the code: it said float equality was
"currently mixed", with `=`/`!=` on a 1e-10 epsilon and the div-by-zero guard
checking `rv == 0.0` exactly. Both now go through FLOAT_EQ_EPSILON, at four
call sites across eval_expr and its aggregated twin. Rewritten to say so and
to point at context/code/core.md §4.
Gotcha 35 called the doc comment on `Dirty::of` "the only record" of its
derivation. The comment wave moved that reasoning into
context/code/command.md and left the comment holding one line, so the
instruction to re-read on reclassification now names the prose.
design-principles §5 named `Sessions::apply` as dispatch; `apply_checked` is
what `Host` actually calls since improvise-955n answered a whole frame at
once. Both are real, so the row names both.
improvise-mbmf: the_ranking_is_ordered_shares_with_the_unused_features_dropped
asserted a length of 2 from three features — the zero-importance one dropped —
under the message "the zero-importance feature is listed". The message is read
only on failure, which is precisely when it would have described the wrong
behaviour as the requirement. The test name and doc comment were already right.
cargo test --workspace: 1311 passing, unchanged.
Closes improvise-mbmf.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A `###` heading in context/test-evidence/ is a promise that a test of that name
exists. 27 of mine named unannotated helpers -- fixtures, accessors and
tokenizers -- and the shipped --check-evidence accepted them because it matched
any `fn`. Under the strict rule (PR #42) they are orphans.
Each is now a `#### Helper -- `name`` under the #[test] whose scope it belongs
to, opening with a line that says it is not a test. No narrative is deleted: a
helper's scope is the scope of the tests that use it, and that is worth stating
where those tests are described.
Two of the 27 -- `model` and `apply_caught_up` -- are `#[cfg(test)] fn`, which
the fixed checker still accepts as a test. Demoted anyway, and reported.
Verified against gh/fix/checker-silent-success's checker, which was borrowed and
restored rather than committed: 184 entries, all name a #[test].
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
keymap.rs: the dispatch/registry-vs-table split (this file stays the authority
on it) and the improvise-0hb Sequence-flattening decline, with its three
findings.
cmd/**: the two CommitAndAdvance orderings, CommitSave's mode-change-first,
add_category's deliberate no-coordinate-write, EditOrDrill's mode-agnostic
message choice, the SaveAndQuit and CreateAndSwitchView declines, and fit.rs's
whole module doc including the measured wasm bundle cost.
crates/improvise-command now reports no comment run over two lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
host.rs: the one-App-per-document argument, why ClientFrame::Key answers
nothing, why Host::save bypasses Sessions, and the broadcast-before-projections
ordering. The 955n evidence -- including arm 3's check-before-apply trade that
clause 4 must pay back -- goes to the evidence file.
viewport.rs/cat_tree.rs: the residue of two derivations that moved to
improvise-protocol, and why each residue is the shape it is.
ui/ now reports no comment run over two lines.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The diff-not-declare rule, Dirty::of's derivation (now recorded in
context/code/command.md, which gotcha 35 should point at rather than the doc
comment), axis_frame's three group-header edge rules, the Broadcast audience
argument and Applied's three cases all move. Test narratives, including the
injected faults for hkt6 and the renamed-axis case, go to the evidence file.
One orphaned doc paragraph dropped: a struct doc describing a type that no
longer exists at that site had fused with fn diff's. Noted in the evidence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The residue-tag taxonomy, the record-as-unit-of-atomicity argument, the
process-local XOR fingerprint, and apply_effects reading dirtiness,
layout_version and the record fingerprints off one EffectKind, go to
context/code/command.md. Test narratives go to context/test-evidence/command.md.
One comment was wrong rather than long: entering_records_mode_sorts_existing_data
carried a stray unfinished first line from a neighbouring test. Dropped, and
noted in the evidence entry.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
EffectKind's no-default rule and the simple_effect! mandatory-kind argument,
the abort ordering Save/SaveAs/AddCategory rely on, and
ReturnToNormalUnlessMoved's observe-don't-predict, go to
context/code/command.md. Effect test bug-narratives go to
context/test-evidence/command.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Checked/CheckedBatch argument -- one basis per frame, checked once, two
tiers hoist and the record tier cannot, and the create-exemption stated over a
property -- goes to context/code/command.md. Test bug-narratives go to
context/test-evidence/command.md, keyed by test name.
The ```text fence illustrating the E0502 that killed the lifetime-tied Checked
moves with its argument rather than being deleted; it was never a runnable
doctest, only a block the checker exempts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CC-gp: move the gp-tree and improvise-gp reasoning into context/ prose (improvise-9c8c). 184 runs capped to none.
AND THE FINDING OF THE WAVE: cargo test --doc was ZERO across the whole workspace. All six 'doctests' were text or ignore, so none ran — the checker's doctest exemption was protecting nothing while letting 74 lines of CLI documentation claim it.
Both ignore blocks had drifted, which is exactly what ignore cannot catch: the fit sketch called fit::fit(&ds.rows, &y, &spec, ..) — the parallel x/y signature improvise-dkqn REMOVED, against a Dataset with no rows field, so the crate's headline example had not compiled for some time; and the breed sketch called a rank() that has never existed in this crate.
Two are promoted to running doctests (1309 -> 1311, and the 2 ignored become 2 passing). That is the one permitted non-comment change and it earned it.
CC-tui: move the TUI and root-crate reasoning into context/ prose (improvise-9c8c).
src/ui/grid.rs's 33-line module doc — which last week's documentation review called one of the four best-written explanations in the repository — moved whole rather than summarised. The agreement test's six injected faults and the THE_MEMBERSHIP_CORRECT_SPANS derivation are preserved in context/test-evidence/tui.md; that assert_ne! is a live tripwire for improvise-gd83.
mode_style's doc called its wildcard arm 'the intended default chrome'. It is a defect, not an intent: improvise-zx8t records that AppMode exhaustiveness holds for five enum methods and fails here, so SavePrompt took ExportPrompt's DarkGray by accident. Comment now names the ticket; flagged, not fixed.
CC-clients: move the browser-host, client, web/ and harness reasoning into context/ prose (improvise-9c8c).
context/code/clients.md and 38 narratives in context/test-evidence/clients.md, with Playwright titles under #### since they are not Rust fns.
THREE COMMENTS WERE WRONG ABOUT THE CODE, all repaired comment-only: keys.rs cited touches_document, which improvise-gv9r replaced with the dirty flag (gotcha 48's fourth instance — a fact stated in three places, updated in two); registry.rs::handle's doc was attached to a #[cfg(test)] helper that had its own, so handle was undocumented; and transport.rs::connection's doc was detached from its item by a plain // note, so rustdoc dropped it entirely.
ws-server/main.rs's fence was a ```text block — prose claiming the doctest exemption, the second of six.
CC-protocol: move the protocol reasoning into context/ prose (improvise-9c8c).
The fold's three rules, AxisFrame and the four accessors no draw loop may use, the three basis tiers and the one envelope pair are now context/code/protocol.md; 42 bug narratives in context/test-evidence/protocol.md.
FOUR COMMENTS WERE WRONG ABOUT THE CODE. The sharpest: frame.rs said the host runs ClientFrame::Key 'through the keymap against the authoritative App', while host.rs:161 is 'ClientFrame::Key { .. } => Vec::new()'. improvise-bpq2 inverted that behaviour and updated one copy of the argument, not its siblings — a fourth instance of gotcha 48, in a file gotcha 48 does not name. Also: two ModelCommand payload types that had moved, a doc block attached to the wrong item, and a 'seven modes' count in the very function repo-map gotcha 11 warns about (it is eight since SavePrompt).
The four AxisFrame per-position accessors KEEP their two-line 'not for a draw loop' warning inline, deliberately: it prevents a measured quadratic regression found three times on improvise-anr9, and the call site is where the mistake is made.
rustdoc does not execute a ```text fence, so `improvise-gp/src/lib.rs`'s sample
formula and `fit_csv.rs`'s invocation block were prose wearing a fence and were
claiming the checker's doctest exemption. Both move verbatim into
context/code/gp.md (§10 and §17) and the cap now applies to both headers.
What remains fenced in this unit is the two blocks promoted in 15774af, which
rustdoc compiles and runs.
Also takes scripts/check-comment-runs.py from fix/checker-silent-success: a file
argument used to walk nothing and print success, and --check-evidence accepted
any `fn`. Re-ran both per directory against the fixed checker — clean, and 49
evidence headings all name a `#[test]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
web/app.js's transport/watchdog/frameCost arguments and web/worker.js's two
ordering rules — including the one nothing here can test, that an `async`
message listener does not serialize — are now prose in context/code/clients.md.
The harness keeps its own section: why a worker, why a report, the engine
coverage and what `wipe` deliberately cannot clear.
harness/tests/*.spec.js's comments were the record of what each assertion
proves; they are in context/test-evidence/clients.md under the exact test
titles, including the `test.fail` measurement that could NOT become a CI guard
because its trigger is the host's network stack, and the closed-invalid
improvise-xps8 report that a simulated fix appeared to confirm.
ws-server/src/main.rs's ```text fence was prose claiming the doctest
exemption, not a runnable doctest; the synopsis is now two lines and the full
form is in the prose file. No fences remain in this unit.
Verified: node --check on every JS file touched; the non-comment diff for both
'*.rs' and '*.js' is empty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding (P2): `### improvise_eval` named a helper, not a test. The
`--check-evidence` gate passed it only because it accepts every `fn`, so the
entry would have become an orphan the moment the checker was tightened.
The helper's scope is the scope of the tests that use it, so the narrative is
now a `####` under `improvise_evaluation_matches_gp_tree_eval`, and the helper's
own comment points there by name. 49 entries, every one a `#[test]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lib.rs's crate statement, socket.rs's frame/wording/composition arguments
(including the "obligation stated but not discharged" note on `typed`),
render.rs's four silent-if-wrong decisions and state.rs's two dispatch
properties are now prose in context/code/clients.md. The test narratives from
both integration tests — the visual-vs-data row confusion Copilot found on
PR #23, the multi-command frame improvise-955n had to keep accepting, and the
lockstep fixtures' deliberate shapes — are in context/test-evidence/clients.md.
No behaviour change: comments only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ChatGPT on PR #36, and it is the repo's characteristic defect: the check
passed while the thing it names was broken. `defined` held every `fn`, so a
`###` heading naming a HELPER satisfied it — five of this PR's own entries
did — and, worse, a renamed helper let an entry silently retarget a
surviving one, which is exactly the orphan the check exists to catch.
Proved discriminating in both directions before keeping: with the fix, the
five helper headings are reported by name (and are now subordinate `####`
headings, unchecked because a renamed helper is found by the compiler); and
renaming a real test's heading is still caught. 1303 distinct `#[test]`
names over 1309 tests, the difference being duplicate names across files.
CROSS-UNIT NOTE for the wave: this tightens a checker all seven units share.
Any other unit whose evidence file gives a fixture or helper a `###` heading
will now fail `--check-evidence` at merge. That failure is correct and
visible; the alternative was leaving the check lying for six more files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding [P2]: four headings named unannotated helpers —
load_corrupt_gzip, storage_fixture, populated_model, awkward_name. The
current --check-evidence accepts every `fn`, so it reported them as
satisfied; a corrected checker would have failed this branch.
They are now `####` subordinate headings, each saying it is not a test and
naming the tests it supports. The narratives are worth keeping — the
preconditions in load_corrupt_gzip and the reason populated_model is not a
fresh workbook are both "why this test is not vacuous" — so they are demoted
rather than deleted.
Checked against a stricter local checker that accepts only a fn preceded by
`#[test]` or inside a `proptest!` block: 64 headings, 0 orphans.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
keys.rs's 78-line module doc, the 55-line strands_the_user argument and the
per-arm reasoning move to context/code/clients.md; the ten test narratives —
including the two Copilot findings on PR #27 and the "an arm asserting
something false certifies what it was added to rule out" lesson — move to
context/test-evidence/clients.md, keyed by test name.
The named list of command names that still need the host stays in the test, as
gotcha 47 requires; the prose now says so explicitly.
No behaviour change: comments only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Edward Langley