improvise-anr9 and improvise-1125 closed by PR #23; improvise-s1xw closed as
absorbed, verified fact by fact rather than assumed.
anr9's close records the user's P1 and why it was larger than it looked: the
migration did not break a check, it OUTGREW one — the pre-anr9 code compared
slash-joined strings and never had cats to compare. And it was wrong end to end,
because grid_updates diffs against the framed mirror, so an apply that kept the
cells made the producer send nothing at all.
1125's close records that the ticket's stated home was unreachable:
improvise-wasm-client cannot depend on improvise-command, so putting the width
rule in viewport would have guaranteed the third copy the ticket exists to
prevent.
Wave 1 complete: 8 PRs merged, 1189 -> 1309 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WU-A: the grid frame on the wire (improvise-anr9, improvise-s1xw absorbed, improvise-1125).
row_labels/col_labels become one AxisFrame per axis carrying the five facts gotcha 44 enumerated, so the browser draws multi-level headers, repeat suppression, group spans and collapse indicators instead of Food/Alice.
Includes the user's P1: a cell's identity is cats zipped with its tuple, and invalidate_relabelled compared tuples alone — so an axis swap left every CellDisplay and column total standing under a coordinate that no longer named them, and because grid_updates diffs against the framed mirror, the producer then sent nothing at all. A client wrong forever.
improvise-368h, gv9r, 955n and 7v62 closed by PR #26; improvise-08wb closed as
a duplicate of 955n.
955n's close records the finding that mattered most, and it was a regression the
work introduced: hoisting the basis check to the frame hoisted ALL THREE TIERS.
The document and layout tiers are frame-level facts; the record tier is not and
cannot be, because which records are at stake is a property of the command. An
independent reviewer confirmed the lost update by executing a real Host.
7v62 closed won't-fix with the compiler output on the type, which is what the
ticket asked for: a proof discharged before a mutation cannot be represented as
a borrow held across it.
improvise-yqpd filed — clause 4, transactional rollback — carrying the costed
design note, the two non-obvious acceptance criteria the P1 produced, and the
trade nobody had named: interleaving the record check with the applies would
close clause 4's silence for free, but it buys the legitimate double-write and
sells the loud failure on a broken chain.
improvise-748f filed — a drill exited with no edits, and view-back, mark the
document dirty. Unlike the LoadModel case, that is a wrong answer to no
divergence at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WU-C: the host, the session door, and the wire's save (improvise-368h, gv9r, 955n, 08wb dup, 7v62 won't-fix).
Clauses 1-3 of the user's transaction contract: one basis per frame, checked once before anything is applied, commands played in order with each seeing its predecessor's state. Clause 4 (all-or-nothing) is deferred to its own bead with a costed design note.
An independent adversarial reviewer substituted for Copilot (quota exhausted) and found a P1 in the first pass: hoisting the basis check to the frame hoisted all three tiers, so every mutation after the first had its records checked against nothing — a silent lost update, confirmed by execution against a real Host. Fixed and re-reviewed clean.
From the independent review of #23. One fixture line, four comments, and a
set of doc guards it offered as optional.
**The one windowing clause with no guard.** Deleting `pending.clear()` — keep
only the nearest header *above* the window — left the whole workspace green.
`a_window_opening_inside_a_group_names_it_without_painting_it` used a fixture
with exactly one group above the window, so it could not tell "the nearest
comes" from "every one comes". Correctness is untouched either way
(`group_of` still finds the nearest, `body_from` still starts at the first
item); it is a *cost* property, O(the groups a deep scroll crossed), and
example-based correctness tests are blind to those. The test now opens inside
`app_with_consecutive_headers`, which has two headers above the window, and
is renamed `a_window_opening_inside_a_group_names_only_the_nearest`. The
mutant dies: `["[Hidden]", "[Late]", "East"]` against `["[Late]", "East"]`.
**`grouped_model()` claimed coverage it does not have.** Its doc advertised
"multi-level row headers"; `Region` is alone on Row and `Month` alone on
Column, so both axes are single-level and `the_whole_frame_is_unchanged` —
this PR's only byte-exact evidence — exercises no multi-level suppression at
all, which is the whole subject of the change. The fixture is **not** touched:
widening it would destroy the before/after comparison, the same trade as
PR #12. The doc now says what it covers and names what it does not, and
points at `tests/grid_frame_agreement.rs` for the multi-level half.
**`RenderRow::labels()` said a group header has a label of its own**; it
returns `&[]`. Corrected, with the reason the empty slice is right (the label
lives in the `Group` variant) and the consequence for a caller that means
"every label on screen".
**The corrected expectation I wrote down was arithmetically wrong**, which is
worse than writing none — whoever picks up improvise-gd83 would have believed
it. `_Measure` is the *outer* level, so the axis is two `Q1` headers, one per
measure, with the ungrouped `Sep` inside each run rather than after both.
Membership-correct spans are four runs, `[(Q1,2),(None,1),(Q1,2),(None,1)]`,
not two. Now a named constant with the frame dumped beside it, and an
`assert_ne!` that starts failing the day gd83 lands.
**"The only thing the user can click" is not true in a browser.** `web/app.js`
puts no listener on a group `<th>` and `toggle-group-under-cursor` is still in
`the_command_names_that_still_need_the_host_are_these`, so `z` is `Upstream`
into an empty vec — a black hole, not a slow path. The rule is unchanged and
still right: the user must *see* the collapsed group. Three comments now say
seen rather than clicked, and name what would have to land first.
**Taken from the optional list**, because deleting `same_prefix` for this
reason and leaving its four siblings unguarded is inconsistent: `item`,
`label`, `shows_level` and `group_of` have zero production callers and now
each carry a one-line "not for a draw loop — see `header_labels`/`groups`/
`shares_ancestry`". `shows_level` says out loud that it materializes every
item per call and is deliberately *not* made cheaper, since a fast
per-position form is an invitation to the loop `header_labels` exists to
replace.
**Test count corrected.** 1296 passing, 0 failing, 2 ignored, under
`--no-fail-fast`. My 1331 was a miscount and is fully explained: 1296 + 2
ignored + the 33 `test result:` lines my `^test [a-z]` regex also matched.
Slowest binary 1.44s here, sum of binaries 2.31s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-0o6x P2 — body_from(idx) starts at the first ITEM, so a leading
FrameEntry::Group is skipped at offset zero. Pre-existing and byte-identical to
main, so the TUI has always done it and the characterization is honest — but a
COLLAPSED group first on its axis therefore has no header anywhere on screen in
either front end, and no way back.
It sits exactly where improvise-anr9's windowing rule says it must not: that
rule carries all headers between two in-window items precisely because a
collapsed group's header is the only affordance for getting its rows back.
Position zero is the one place the rule does not reach — not because the frame
omits the header, but because body_from drops it on the way to the screen.
The author knew: the agreement fixture orders Carol before Family to dodge the
case and says so in a comment. That knowledge lived in one comment in one test
and is now a ticket, because the moment web/app.js grows a click handler for
group headers, a leading collapsed group becomes a trap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second review pass found the thing neither reviewer nor I predicted:
**interleaving the record check with the applies would close the clause-4
silence.** `[RemoveItem(North), SetCell(North)]` under interleaving is refused
`RecordGone` instead of silently half-applying — half of what clause 4 owes.
So check-before-apply is not a free win. It buys the legitimate double-write
that arm 3 pins (two measures on one record is one record) and **sells** the
loud failure on a broken chain. Clause 4 has to pay that back some other way,
and arm 3 is now what stops someone closing the silence by moving the check
without noticing they made the trade. That is a better reason for that arm to
exist than the one it was added for, and it belongs next to the arm rather
than only in a ticket.
Measured rather than argued, which also corrects my own commit message on
a685503: injecting the interleaving fails **two** tests, not "that arm and
nothing else" — this arm and `a_frame_whose_later_mutation_fails_is_not_yet_undone`
— and the second failing *is* the trade showing itself. I read it as noise at
the time.
Doc-only. 1291 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The second review pass cleared the code and found that **243b9a9 landed its
own instance of the rule this PR enforced in three other files**: it gave
`CheckedBatch` a new "two of the three tiers hoist, and the third does not"
section and left the sibling paragraph in `frame.rs` — the **protocol-facing**
doc, the one a client author reads instead of `improvise-command` — saying the
basis is answered once and later commands are simply applied. A client author
reading only that concludes their later mutations are unchecked, which is the
exact over-generalisation the fix commit was correcting one crate over. Three
files grepped, the fourth written fresh and not grepped.
It now says what is true: the two document tiers are answered once, the record
tier is asked of every mutation, and the exemption is narrower than "later
commands are not checked" — a record that *neither exists nor is claimed* is
one the frame is creating, and only that pair is exempt. A record that exists
is answered for however the claim is written.
Also fixed, flagged by both reviewers and still live: `frame.rs` claimed "a
frame is applied whole or not at all" fifteen lines above "all or nothing is
the contract and is not yet delivered". The first is true of a **refusal** and
false of an apply-time failure, which is the documented hole. The qualifier is
now on the first, and the last paragraph names itself as the other way a frame
can stop half-way.
And one line on the `frame_basis` test helper: it always includes a record
where `BasisCache::claim` omits an unknown one, so pointed at a record that
does not exist it would fabricate `(hash, 0)` — a claim no real client could
compose, and exactly the input that flips the create exemption. No current use
is wrong; the doc now says to name only records that exist.
Doc-only. 1291 passing; clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
[P1, raised by the user on PR #23.] A cell's coordinate is `AxisFrame::cats`
zipped with its item tuple, so `Region = [North]` becoming
`Department = [North]` names a different cell with `old_items == new_items`.
`invalidate_relabelled` compared the tuples and nothing else, so `apply` left
the old `CellDisplay` — **including its editable `CellId::Record`** — and the
column totals standing under the new coordinate.
This is a gap my own migration opened: the pre-anr9 code compared slash-joined
strings, which could not distinguish a category change either but never had
`cats` to compare. improvise-anr9 made it a first-class part of the frame and
did not extend the staleness test to cover it.
Three parts, and the third is why it is a P1 rather than a redraw bug:
- **Every surviving position is stale, not the ones whose tuples differ.** A
`cats` change re-identifies the whole axis at once, including the positions
whose labels are byte-identical — which is exactly the set the tuple
comparison keeps.
- **The totals go too, whichever axis moved.** The column arm already dropped
each total as its column went stale; the row arm did not, and a column total
sums rows that have all been re-identified. It is `apply`'s job and not the
producer's, because an update must be correct with nothing behind it —
gotcha 33(b), documented in the comment immediately above this function.
- **The stale `CellId` is the sharp half.** An axis swap is the case that
bites: two categories may legally share an item name, so the preserved id
stays *valid* and addresses the other record. A client goes on offering an
edit that writes to the cell the old coordinate named — silent, invisible in
any rendering, the family of gotcha 18's drill snapshot. (A category
*rename* in the document is milder: every `RecordHash` moves, so a stale id
fails to resolve and is refused as `RecordGone`.)
**Tests are frame-only, which is the whole point**: a `RowFrame`/`ColFrame`
whose `cats` moved and whose `items` did not, with no `Cells` and no `Totals`
behind it, because the producer's follow-up is what would hide this. Same
shape as `a_cell_update_that_overtakes_its_labels_is_lost`. The `id` is
asserted separately from the text, and the negative arm
(`an_unchanged_category_leaves_the_cells_that_still_fit`) is load-bearing —
without it "blank on every frame update" would pass.
And it was wrong **end to end**, not only in `apply`:
`renaming_an_axis_category_resends_every_cell` drives the producer, which
diffs the cells against the *framed* mirror — so an `apply` that kept them
made `grid_updates` agree they matched and send **nothing at all**
(`left: []` under the injected fault). `project_diff` then assigns
`self.grid = desired` and the client holds re-identified cells with no update
in the stream that could ever correct them: wrong forever, the direction
`Dirty::of` is deliberately sloppy to avoid.
Faults injected, each watched to fail: `recategorized = false` (the pre-fix
behaviour) fails both frame-only tests and the producer test; dropping only
the trailing totals loop fails the row test alone, which is the arm that
needed it.
Neighbourhood re-checked as a class, as asked. `fit_to_labels` preserves by
position and is followed by this function on both frame arms; its other two
callers (`ColWidths`, `Totals`) do not change the frame. `body_from`,
`visual_of` and `group_of` are positional walks with no identity decision.
`header_labels`, `shows_level`, `shares_ancestry` and `header_level_widths`
are drawing and measurement over tuples, and a `cats` change with identical
tuples genuinely draws and measures the same. `grid_updates` compares whole
frames, so `PartialEq` already covered `cats` on the producer side — the
defect was only in what `apply` did with the update it received.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-gd83 P3 — AxisEntry emits a group header on ENTERING a group and no
boundary on leaving one, so an ungrouped item after a grouped run
inherits it. Copilot found it, PR #23 declined it: it is
GridLayout's pre-existing rule, the TUI paints the same span, and
fixing it would destroy the exact-buffer characterization that
was that PR's only evidence the migration changed nothing. The
test already asserts the wrong answer with the corrected
expectation written beside it.
improvise-jq8l P3 — web/app.js calls scrollIntoView nowhere, so a selection can
sit outside the rendered window. Pre-existing, surfaced by the
viewport-reservation argument, and not group-specific.
improvise-plcx P2 — the browser harness renders web/app.js in three browsers but
web/demo.improv has no groups and one category per axis, so the
multi-level, group-span and collapse paths PR #23 added are the
one part of gridTable no test sees. Would be the first test here
able to see that code in a browser.
improvise-4zl7 retitled and re-scoped to the user's ruling: a fitted tree must
route per record and aggregate the results. The question was never about
categorical splits — aggregate-then-route is not route-then-aggregate, so every
fitted tree means one thing on a record row and another one row up.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two conflicts, both expected.
**`invalidate_relabelled` (improvise-hkt6, WU-K).** Both sides kept: the
signature and the item-tuple comparison are this branch's — `old` is an
`AxisFrame` now, and a group header appearing or collapsing must not blank
cells it did not rename — and the totals blanking and its whole argument are
main's. `grid_updates` still diffs the totals against the *framed* mirror,
which is what makes a renamed column whose total did not change send one;
`a_relabelled_column_drops_its_total`,
`a_relabelled_row_leaves_the_totals_alone` and
`a_renamed_column_whose_total_did_not_change_still_gets_its_total_back` all
pass against the new frame.
**`lib.rs`'s composition test.** Main's deletion taken: 18189c1 moved
`client_showing`, `published_basis` and
`a_client_composes_a_write_from_the_id_it_was_given` into `socket.rs` when
it consolidated `set_cell_at` and `typed`.
Main's new `GridUpdate::RowLabels`/`ColLabels` call sites are converted, and
five doc comments naming the retired spellings with them — a fact stated in
several places is a grep, not an edit.
1327 passing, 0 failing. clippy/fmt clean, wasm gate clean, both bundles
build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two measures on one record is one record, so writing the same record twice in
a frame is both legitimate and likely. It works for a reason worth pinning
rather than rediscovering: **every check happens before any command is
applied**, so the second write is compared to the record's *pre-frame*
counter and not to the one its own sibling just bumped.
Were the checks interleaved with the applies — which is what the previous
commit's per-command record check would look like if it were moved into the
apply loop — the frame would refuse itself with `RecordMoved`. Proved by
injecting exactly that interleaving: this arm fails and nothing else does.
1291 passing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The independent review of PR #26 found a **P1 regression 90ea26e
introduced**, confirmed by a probe against a real `Host` rather than by
reading. Hoisting the basis check to the frame hoisted *all three tiers*, so
every mutation after the first was admitted with no record check at all:
A composes [SetCell(North), SetCell(South)] against an honest basis
B writes South = 99
A's frame arrives -> accepted, South = 20, B's write gone, silently
At the base commit the second command was checked and the frame refused. This
is precisely the lost update ADR 001's record tier exists to prevent, and it
is invisible to the other two tiers because `SetCell` is `CellData` and moves
neither of them.
**It cannot be deferred into the clause-4 bead**, and that is the part worth
being sure of: clause 4 never fires on this frame, because *both writes
succeed*. Rollback is about partial application; this is a wrong answer,
fully applied.
**The per-command check was doing two jobs and the commit noticed one.** "Is
the client's picture of the *document* current" is a frame-level fact and
hoists correctly. "Is the client's picture of *this record* current" does
not: which records are at stake is a property of the command, which is why
`records_named` is an exhaustive match over `ModelCommand` (gotcha 43). So
every mutation now answers for its own records, against **the frame's**
basis — the one its first mutation stated, not each command's own field, so a
later command cannot claim a fresher picture than the frame was composed
under.
**Except the records the frame is creating**, or clause 3 is lost again. The
argument that motivated clause 3 is true of records a chain **creates** and
was over-generalised to records it merely **writes** — and those are the ones
the tier is for. The exemption is therefore stated over a *property*, not a
list of commands: a record that **neither resolves in the document nor is
claimed by the basis** is one the frame is about to create
(`will_be_created_by_the_frame`). Every other combination still answers,
including "does not resolve but *is* claimed", which is a client believing in
something that has been removed and stays `RecordGone`. It is asked only
inside a frame — a first command has no predecessor that could have created
anything, so a lone write to a missing record is refused as before.
`stale_record` itself is untouched: one home for what the four combinations
mean, and the frame path differs only in *which* records it asks about.
Three arms, because a fix that refuses everything passes the first alone and
a fix that checks nothing passes the other two. Two mutants injected:
dropping the per-command record check fails
`a_later_write_in_a_chain_still_answers_for_its_own_record`; dropping the
create exemption fails `a_data_dependent_chain_lands_in_order`.
**The fixture is the finding underneath the finding.** `reduce::caught_up`
builds the claim for *one command*, so a frame's records beyond the first are
simply absent from it and come back `RecordUnstated` — my first probe failed
for that reason and not the one it was testing. A real client calls
`BasisCache::claim` once with every record its frame names, which the new
`frame_basis` test helper reproduces.
Also from the review:
- **`a_frame_whose_later_mutation_fails_is_not_yet_undone` did not pin the
hole it is named after** — the repo's signature defect sitting on the one
test whose stated job is to fail loudly if anyone believes the hole is
closed. It asserted North was removed and nothing was rejected, both true
of any *wholly successful* frame; the reviewer retargeted the second
command to a live record and it still passed. The missing assertion is the
one that names the hole: **the second mutation did not land.** Verified by
making the frame wholly successful, which now fails it.
- **Three surviving statements of the removed rule**, gotcha 48's grep rule
again: `basis.rs` (whose "the other TWO classes" over "TWO classes live
here" gave it away), `host.rs`, and `browser_round_trip.rs` — whose live
one-mutation assertion is kept but **re-justified as a characterization of
what `keys::commit` emits**, not a protocol constraint, so a future reader
does not take it for one.
- `ApplyAndClearDrill`'s `dirty = true` is gone: it declares `Model(CellData)`
and `apply_effects` raises the flag off that. Its tests drove `apply`
directly and so measured the effect in isolation rather than the path
production takes; routed through `apply_effects`, one of them reveals that
**a drill exited with no edits marks the document dirty** — the same
over-approximation a failed `LoadModel` gets, true since the flag moved and
invisible until now. Recorded in the assertion.
- `a_loaded_document_is_dirty_because_its_path_still_names_the_old_one` pins
the `LoadModel` behaviour change and *why* it is right: `LoadModel` does not
move `file_path`, so the in-memory document really is a different one from
the file the path names. It gates `autosave_if_needed` and the `:q`
refusal, so it is worth a test.
- `ui/reduce.rs`'s module doc pointed hosts at `Sessions::apply`, which has
had no production caller since `Host::commands` began answering the whole
frame. Now points at `apply_checked`.
Merged `main` (a15cc73). One semantic conflict git merged cleanly: PR #25
added `effect::MarkDirty` in `cmd/fit.rs`, which no longer exists — dropped,
since `AddFormula` beside it is a `Model` effect.
1291 passing; clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WU-M: wire gp-tree into improvise (improvise-szrt, improvise-vx3; improvise-21d's last open child).
improvise-4zl7 stays open and grew: a formula evaluated at an aggregated cell routes on the aggregate, so aggregate-then-route is not route-then-aggregate and EVERY fitted tree — numeric splits included — means one thing on a record cell and another one row up. The bead asked this for categorical splits; it is a question about the feature.
Copilot's eighth pass; both correct, and the second caught a doc that had
drifted from its own code in this branch.
**`compute_row_header_width` cloned every row tuple on every keypress.** It
is reached from `App::cmd_context`, so building an owned `AxisFrame` just to
ask for widths was a full-axis copy per key — where the pre-anr9 version
only borrowed `row_items`. The rule moves out to
`improvise_protocol::header_level_widths`, which takes an **iterator of
borrowed tuples**; `AxisFrame::header_widths` is a one-line call and so is
the layout adapter. One rule, no copy.
**The consecutive-header rule had no test, and the comment describing it was
already wrong.** `axis_frame`'s doc said consecutive headers all come, but
the compaction added earlier in this branch — keep only the nearest header
*above* the window — made that false on one side. It is two rules, not one:
above the window nothing is painted so only the nearest is kept; between two
in-window rows all of them come, because only the nearest wins `group_of`
but a dropped `[Hidden]` header is the only thing on screen the user can
click to get those rows back. Doc corrected, and
`consecutive_headers_between_two_rows_all_come` pins the second arm — the
first is `a_window_opening_inside_a_group_names_it_without_painting_it`.
Injected: compacting unconditionally drops `[Hidden]` and fails it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`match (answered, cmd)` with `_` in both arms said nothing the flag did not.
No behaviour change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The user has settled the contract, and it rejects the "at most one document
mutation per frame" rule this branch shipped. Verbatim: *"a sequence of
commands sent to the server should assume that it's played in order from
start to end and that each command sees the state after the previous command
runs"*, sent as *"a single transaction from a given base"*. Four clauses:
1. A frame states **one basis**, describing the base the client composed
against.
2. That basis is checked **once**, against that base, before anything runs.
3. Commands are applied **in order**, and command N sees the state command
N-1 left. Data dependence between them is expected, not an error.
4. **All or nothing.**
This commit lands clauses 1–3 and removes the rejected rule. **Clause 4 is
not implemented** and is documented as an open hole rather than a closed one;
the design note on PR #26 covers what rollback costs.
**Per-command basis checking is not merely awkward, it is incapable**, which
is the finding that makes clause 3 necessary rather than preferable. A client
cannot claim a counter for a record its own chain is about to create, because
`BasisCache::claim` filters to records the cache has been told about — so
`[AddItem("East"), SetCell(East)]`, the archetypal chain, is refused
`RecordGone`/`RecordUnstated` by a per-command check *in any order*. The base
describes what the client **saw**; the chain's internal coherence is the
transaction's job, not the basis check's.
So `CheckedBatch::against` answers the basis on the first command that
carries one and admits the rest through a private
`Checked::within_an_answered_frame`. Private is what stops it being a bypass:
from outside the module the only doors are still `Checked::against` and
`CheckedBatch::against`.
**Two things the rejected rule got wrong, kept as the lesson.** *Arity is not
a property of a variant* — a future `Command::RunScript` is one variant
performing unbounded mutations, so no count over the vocabulary can ever be
right. And *the transaction boundary was never the frame*: because one
command may already be a chain, clause 4 is needed inside a command as much
as across a frame of them, which is why the design note puts the mechanism at
`App::apply_effects`.
`mutates_the_document` is **deleted rather than kept**, which is a deliberate
deviation from the instruction to keep it: with the rule gone it has no
caller, and dead production code behind an `#[allow]` is worse than none. The
reason it was made exhaustive — a `matches!` answers `false` for a future
variant in silence — is preserved in the `CheckedBatch` doc, where clause 4
will read it when the classifier returns.
`Rejection::FrameNotAtomic` goes with the rule; nothing can produce it. The
taxonomies on `Rejection` and `ServerFrame::Rejected` return to two classes
and name the third that arrives with rollback.
Evidence, both faults injected:
- `a_data_dependent_chain_lands_in_order` — `[AddItem("East"), SetCell(East)]`
through a real `Host`. Restoring the per-command check fails it, which is
the whole point. It asserts **both** commands landed, because asserting
only the first would pass against a host that dropped the second.
- `a_frame_whose_later_mutation_fails_is_not_yet_undone` characterizes the
open hole with `[RemoveItem("North"), SetCell(North)]`: the removal lands,
the write resolves to nothing and `model_effects` returns an **empty**
batch, and the frame is reported accepted having applied half of itself.
Both the partial application and the silence are open. **This test inverts
when clause 4 lands**; until then its job is to fail loudly if anyone
believes the hole is closed.
- The worker's `a_partly_refused_batch_…` fixture moved its injected
staleness to the **first** command, since a later one no longer carries an
answer of its own. Removing the injection still fails it.
The hole is unreachable from any client in this workspace — the only producer
of a multi-command frame is `keys::commit`, one mutation among view commands
— which is what makes it safe to document rather than rush.
Merged `main` (3915994). One semantic conflict: PR #27 added a
`mark_dirty()` call in `commit.rs::add_category`, which no longer exists —
dropped, since `AddCategory` is a `Model` effect and `apply_effects` raises
the flag.
1245 passing; clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot's seventh pass. One applied, one applied by *withdrawing* the
previous fix rather than patching it again.
**`RenderColGroup::label` is an `Option<String>`.** `""` was the
ungrouped-run sentinel, but an empty string is a legal group *name* —
`Category::add_item_in_group` accepts one and the TUI draws its indicator
like any other — so a real group serialized identically to no group at all
and the page dropped its indicator on a truthiness test. `None` is now the
run in no group, and `app.js` checks for null.
**Inline group rows are no longer reserved in the viewport, and the reason
is written down.** Copilot was right twice in a row here, about two
opposite mistakes. Counting the current group rows is a feedback loop (a
smaller window crosses fewer groups and asks for a larger one). Keeping the
maximum is a ratchet: one group header per data row stores an oversized cost
on the first oversized projection and clamps the grid to a single row
*permanently*. Both were on this branch.
The third option — invent a density heuristic — is the one this repo warns
against, in a file no test here can execute. So the reservation keeps only
what depends on the axes and not on the window: the header lines, the group
line, the row-header width. The residue is an over-request bounded by the
groups in view, and the off-screen-selection gap it leaves is neither new
nor specific to groups — nothing in this page calls `scrollIntoView` at all.
Filed rather than guessed at.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-vvl3, improvise-vhcz and improvise-hmws closed by PR #24.
vvl3's close records the finding that outlives the harness: a contract nobody
can execute is prose. storage_contract had been cited in gotcha 28 for months
as the spec OpfsFs must satisfy, and the first time OpfsFs was handed to it, it
failed on its FIRST assertion — on a semantic disagreement about what list
means, sitting in a trait doc everyone had read.
hmws needed neither of its two proposed fixes: an orphan-rule refusal is a fact
about where you are standing, not about the pair of types. The impl is simply
allowed in the crate that owns the trait.
improvise-xf1 narrowed rather than closed, and raised to P2. Chromium and
Firefox now run the full battery; Playwright's WebKit provides no OPFS at all,
so Safari is untested and the ticket's own 16.4+ claim is unverified. Parked as
a runtime probe-skip so it starts running when that changes.
improvise-xps8 filed: the handshake watchdog overwrites a genuine mid-session
disconnect eight seconds later. A third bug in the same family, and it survives
improvise-taxx's fix — confirmed by simulating that fix rather than assuming.
Not a CI guard: its trigger is the host's network stack, and an expected-failure
whose trigger is platform-dependent inverts CI's meaning rather than flaking.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WU-J: a headless browser harness, and the first execution of OpfsFs (improvise-vvl3, improvise-vhcz, improvise-hmws; improvise-xf1 stays open for Safari).
storage_contract had been cited for months as the written-down spec OpfsFs must satisfy in a browser. The first time OpfsFs was handed to it, it failed on its first assertion.
WU-G: the keymap and command layer (improvise-th7m, lae, ct5, 2zq; git/aaw declined with evidence; 0hb deferred with its design note).
Includes the ejection fix the 2zq ruling called for — a virtual on an axis is a configuration, not a vacancy — which also closed a live bug where adding a second regular category evicted _Measure from Column and made every measure vanish from the demo's own view.
Copilot's fifth pass, one finding and it is right.
`into` was checked against `measure_item_names()`, and `Expr::Ref` does not ask
that question: `Model::find_item_category` finds a **regular category's** item
before it falls back into `_Measure` (gotcha 7). So naming the fit after an item
of some category installed cleanly, and every later formula referring to the
fitted measure then read the category coordinate instead — silently, because
both resolve.
The check now asks the document the same question `Expr::Ref` will ask, which is
both stronger than the measure-only form and simpler than it. Two regressions:
one that a category item's name is refused, and one that earns the refusal by
asserting the property it protects — a second formula `Doubled = |Score (fit)| *
2` reads twice the fit, per record, against a real document.
1232 passing (+2); clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An expected-failure is green when the bug is present and red when it is
absent. One whose trigger depends on the host therefore does not merely
flake — it **inverts CI's meaning**. These two failed the browser job twice
with "expected to fail, but passed", and the second time was *after* the
determinism work in the previous commit, which is the evidence that the
problem is not the assertions' precision.
Neither bug reproduces on the GitHub runner:
- improvise-m18f, where the unconditional `close` listener overwrites
"cannot reach …" with "disconnected" — on the runner `close` does not
follow `error` inside the window, even against a server that accepts the
TCP connection and then refuses the upgrade.
- The handshake watchdog replacing a mid-session "disconnected" with
"accepted the connection but sent no document" — on the runner it does not
land, though the page demonstrably opened and then dropped, since both are
asserted as setup.
The trigger for both is the **order and timing of `error`, `close` and a
timer on the host's network stack**. That is not a property of `web/app.js`
and not something this harness can pin, so pretending otherwise was the
mistake. Both are real, both were measured on macOS/Chromium, and the
measurement is recorded in the file with the exact status sequences so the
next person does not re-derive it — including the fact that the watchdog one
**survives** the m18f fix, because that fix is about which message `close`
writes rather than about the timer still running.
What remains is the portable half, and it is the half that was doing the
work: every message the page must write ("bad frame", "cannot reach",
"connected"/"disconnected", "sent no document"), the one case where the
watchdog must stay silent, and the two standalone-deployment tests. Filed
for the orchestrator; `web/app.js` is not this unit's file.
12 passing, 3 skipped (WebKit has no OPFS), 23s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot's sixth pass; both correct, and the second is a front-end
divergence of exactly the kind this PR exists to remove.
**The high-water mark was committed only when the extent changed.** A group
count that rose while a header line fell by the same amount left `next.rows`
equal, so the larger count was discarded and a later repaint under-reserved
— the DOM-scroll bug again. `frameCost` is now assigned unconditionally and
only `sendViewport()` is gated on the extent moving.
**`col_groups` started a new run when `collapsed` differed; the TUI does
not.** `src/ui/grid.rs` breaks a run on `group_name != prev_group` alone.
Collapse is category-scoped, so two adjacent groups from different
categories may legally share a name and differ in it, and the browser would
then have drawn a second indicator the TUI suppresses. Coalesced by name,
with the run taking its first group's indicator, as the TUI paints it.
`column_groups_coalesce_by_name_as_the_tui_does` pins it; restoring the
`(name, collapsed)` key splits the span and fails.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The browser job **failed in CI** on the previous head while passing locally,
with both `test.fail` tests reporting "expected to fail, but passed". That is
the worst possible outcome for an expected-failure: it is a check that is
green when the bug is present and red when it is absent, so a flaky one
inverts CI's meaning rather than merely being noisy. Copilot independently
flagged the same test from a different direction on the same head.
Two timing assumptions, both wrong off my machine:
**A closed port is not a deterministic "refused before open".** Port 9's
`close` timing is a property of the host's network stack, and on the GitHub
runner it did not arrive inside the window, so `#status` still read "cannot
reach" and the assertion passed. Replaced with the harness's **own http
server**: it accepts the TCP connection and then fails the WebSocket upgrade,
so `error` and `close` both fire promptly, everywhere. The page cannot tell
the two apart and neither can `web/app.js` — in both cases the socket never
opened.
**A timer does not order a close after an open.** The watchdog test's mock
closed 300ms in; on a slow host that can land before the page sees `open`, in
which case `armWatchdog` never runs and the test passes having exercised
nothing. Both mocks now close on the page's **first frame**, which the page
sends only after `transport.ready` resolves — so "opened, then dropped" is
true by construction rather than by luck.
And the setup assertions now assert the **precondition of the bug** rather
than that something happened: the m18f test polls the recorded `#status`
history for "cannot reach" before enabling `test.fail`, because a bare
non-empty check is already satisfied by "connecting to …" (Copilot's exact
point). Verified three ways: `--repeat-each=3` green; breaking `paintStatus`
gives 2 real failures, not expected ones; simulating PR #21's fix still
flips the m18f guard to "expected to fail, but passed".
**Four checked-in explanations this branch made false**, also Copilot's, and
gotcha 48's third rule for the second time in this PR — when a fact is stated
in more than one place, changing it is a grep, not an edit:
- `worker-server/src/lib.rs` said `storage_contract` "has still never been
run" against `OpfsFs` and that browser verification is deferred.
- `bindings.rs` said it is "the only part of this crate that no test in the
repository can reach". Still true of the shim, for a narrower reason now
worth stating: the harness drives `OpfsFs` directly and never instantiates
it.
- `vfs.rs::holds_a_document` and its test both said `attach` indexes the
zero-byte entry it creates. It no longer does, which is the whole of the
fix three commits ago — so the check is now belt and braces rather than the
mechanism, and says so.
`web/app.js` and `web/worker.js` carry the same stale "untested JavaScript"
claim and are **not this unit's files**; reported for the orchestrator rather
than edited across the boundary.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four of five fixed. The fifth is the most valuable thing in the review.
**A fitted rule means something different in an aggregated cell, and that is
improvise-4zl7's blocking question in its general form.** A tree is fitted on
individual *records*; an improvise formula is evaluated at a `CellKey`, and in a
cell that aggregates over a hidden category `evaluate_aggregated` aggregates
each `Expr::Ref` **first** and then evaluates the `IF`. Linear formulas survive
that — `SUM(Revenue) - SUM(Cost)` is `SUM(Revenue - Cost)` — and a tree does
not: aggregate-then-route is not route-then-aggregate. 4zl7 asks what a
*categorical* split means where its coordinate is aggregated away; this is the
same question for **every** fitted tree, numeric splits included, and it makes
the bead larger and more clearly blocking rather than smaller.
Characterized rather than answered, because answering it is the user's call.
Worth recording what the fixture actually does: the aggregated cell comes back
`Error("ref:Cost")`, not a wrongly-routed number — the loud failure this repo
prefers, and *not* something to rely on, since nothing states it. The assertion
is therefore the negative (it is not the sum of the per-record predictions), so
it covers both outcomes and fails the day aggregation becomes record-wise.
**An infinite target panicked out of a function that returns `Result`.** A
`.improv` cell holding `inf` parses as an `f64`; `standardize` turns an infinite
target into NaN leaves and `number_text` asserts. Verified by disabling the new
guard and watching `number_text wants a finite value, got inf`. The two
directions differ and the doc says so: an infinite *feature* poisons its range
and quietly yields a constant leaf instead. A panic and a meaningless rule, both
refused now. `NaN` needs no arm — `is_missing` already spells `"NaN"` as
missing, so it is imputed away.
**Only the success line was flattened**; refusals interpolate document names
too, and a name may legally contain a newline — so a failed fit could split the
status bar, which is when a user is most reading it. `one_line` is public and
the command's error path uses it.
**"a constant — no feature matters" overclaimed.** An empty importance ranking
establishes that no shuffle raised the error, which a constant tree satisfies
and so does a tree whose splits do not matter on these rows. It now reports what
the data established: "nothing measurable — no feature changed the error".
**Declined:** the record budget runs after `feature_table` has already grouped
and sorted, and `RecordRow::group` linear-scans (`improvise-core/src/view/
layout.rs:43`) so grouping is O(cells x rows). Correct — but that cost is
pre-existing and shared with records mode, it is in `improvise-core` which is
outside this unit's boundary, and threading a limit into `feature_table` changes
a signature `dataset()` and its tests share. Filed as an indexed-grouping ticket.
1230 passing (+2); clippy, fmt and the wasm gate clean; CLI byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot's fourth round. The substantive one is in its summary rather than in
a thread: `mutates_the_document` was a `matches!`, so a **future** `Command`
variant would answer `false` in silence — and silence is precisely the
failure mode here, since an uncounted document mutation walks straight past
the atomicity guard and is applied against a document a sibling has already
changed. That is the same shape as the three bugs this branch has already
been caught by: a rule stated over the variants that came to mind.
It is now an exhaustive `match`, like `records_named`, so a new variant fails
the build until it says which it is. **Verified by adding one**: a probe
variant on `Command` fails to compile at this function, not merely in the
protocol crate's own fold.
Three documentation findings from the same round, all correct:
- A stray `**` left an unmatched emphasis span that broke the rustdoc for the
rest of the paragraph — in the doc explaining the rule, which is the one
people will read.
- `Rejection`'s "the client re-composes against it" is true of the staleness
class only. A failed save is a fact about the host and a frame carrying two
mutations is answered by splitting it; neither has a fresher basis to
re-compose against. Now scoped.
- The end-to-end test's own doc described the rule as counting
`Command::Model`, one commit after it started counting `SwitchView` too —
so the test that protects the contract mis-stated it.
1208 passing; clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot's fifth pass; both correct.
**Inline group rows are back in `frameCost`.** Omitting them was the same
failure I had just fixed for the header block: they take real table height,
so any viewport containing one asked for more data rows than `#grid` could
show, and the DOM takes its own scroll position while the client's cursor
offsets come from the larger window. Copilot's own suggestion answers the
oscillation objection — the reservation is a **high-water mark**, which
cannot oscillate because it only rises, and it rises in the safe direction:
over-reserving leaves blank space, under-reserving is the bug. It resets on
a view change so one dense document does not make every later view
pessimistic.
**`same_prefix` is replaced by `AxisFrame::shares_ancestry`**, the bulk
form, for the same reason `header_labels` and `groups` are bulk: called per
(column, level) it scanned the frame twice apiece, quadratic in the columns
drawn and growing with the horizontal offset, where the pre-anr9 widget's
materialized lookup was O(1).
`same_prefix` is **deleted** rather than kept beside it — two answers to one
question is the thing this PR exists to remove, and keeping a per-position
form that nothing calls would be an invitation. `shared_ancestry_widens_as_
the_level_rises` checks both arms, because a version comparing whole tuples
passes the leaf arm and a version comparing only the outermost label passes
the outer one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-odbc, improvise-7yx, improvise-g6s1 and improvise-q0vb closed by
PR #22, with the reasoning on each.
improvise-odbc's close records that the ticket's premise was FALSE and that
testing it is what made the ticket cheap: quote_name mirrors bare_name
character for character, so the round-trip property was complete and the
generator needed nothing. The file had been hand-damaged through the
v2025-04-09 migration.
improvise-1ey's design field named bank-info.improv, which PR #22 deleted.
roadmap.org:194 is generated from that field, so the fix belongs at the bd
source and the file is regenerated — Copilot found the stale line and WU-L
correctly declined to hand-edit a generated file.
improvise-l8rh filed: the ambiguous-date heuristic reads American-first, so a
European CSV imports with days and months swapped. Pinned rather than changed
on the user's ruling — but it is now wrong DETERMINISTICALLY, which PR #22's
HashSet fix is what bought.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sibling of bank-info.improv, deleted in PR #22. Both were force-added past
.gitignore's `*.improv`, and both are hand-written fakes with no generator.
foo.improv is worse: it has no version line at all, so it cannot parse under
any grammar version.
Note tests/shipped_documents_load.rs walks examples/ and web/ — the two
directories .gitignore un-ignores — so neither root file was ever covered by
it. Deleting them is what keeps that true rather than an omission.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WU-L: import, persistence and the test gaps (improvise-odbc, improvise-7yx, improvise-g6s1, improvise-q0vb).
Includes a determinism fix nobody asked for: analyze_records iterated a HashSet, so the same CSV imported twice produced two different documents and sometimes a different detected date format.
**A schema change shifted the grid under its own advance.** `add_category`
inserts the new coordinate column immediately before `+cat`, so every index
from `+cat` rightwards moves by one — while `CommitAndAdvance` computed its
advance from `ctx`, the *pre-commit* layout (gotcha 23). On the last row
`Enter` wraps to `col + 1`, which after the insert is `+cat` again.
The failure is not "lands one column off": it **reopens category creation**,
so a user naming two categories in a row never reaches the column they made
and gets an unbounded `+cat` loop. `Tab` reached it the same way.
A schema change that inserts a column now does not advance at all, which is
the useful answer as well as the safe one — the insert happens exactly where
the cursor is, so staying put lands on the new column. The measure `+` shares
none of this and not by luck: it is the last column, so there is no index to
its right to shift into, and both advances already special-case the
bottom-right edge. `inserts_a_column` asks the *layout* what the column is
rather than re-deriving it from the value, so gotcha 1 keeps one answer.
Both arms were watched failing first, and both now assert the column landed
on rather than merely "not `+cat`" — a `assert_ne` alone would pass anywhere.
**And a negative arm that was asserting something false.**
`a_prompt_this_client_cannot_commit_is_never_entered` used `:` to show that
`strands_the_user` had not degenerated into "refuse every mode". But `:` is
itself a dead room: it opens locally and types locally, while Enter is
`[execute-command, clear-buffer]` and `step` has no `execute-command` arm, so
the frame is dropped. The arm was **certifying the behaviour it existed to
rule out**, which is worse than having no arm at all.
`CommandMode` therefore joins the predicate. It costs no working surface —
every ex-command behind `:` is inert for the same single reason — and the
distinction from the two prompts is kept visible because it says who removes
the line: they are dead *by construction* (no `ModelCommand` can name
`Save`/`SaveAs`/`ExportCsv`), this is dead *by omission*, and improvise-e24v
is scheduled to implement `execute-command` client-side. Listing it is
therefore also the named gap gotcha 47 asks for: closing e24v has to come to
this line and delete it, which a comment could not force.
`the_command_line_cannot_be_committed_by_this_client` is the evidence rather
than the assertion.
The arm itself now has **two halves**, and the first version of the rewrite
had neither — it pressed Enter in `Editing`, which never consults
`strands_the_user`, so "refuse every mode" still passed. Half one enters a
mode through `enter-mode` and so exercises the predicate; half two presses
Enter and requires a real `Command::Model` back, because the property is
finishing rather than entering. `?` is honest as half one where `:` was not:
Help has no commit, so nothing typed into it can be swallowed.
Proved in both directions: "refuse everything" fails half one, "refuse
nothing" fails the positive arms and the named-gap list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ClientFrame::Commands` — the doc a third transport reads first — still said
"at most one `Command::Model`" after the rule became "at most one document
mutation", which understates it by exactly the case the previous commit was
fixing, and would have led a new client straight into it.
Found by grepping the rule's own wording rather than by a test, which is
gotcha 48's third paragraph for the third time on this branch: when a fact is
stated in more than one place, changing it is a grep, not an edit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot's third round of PR #26, and the first finding is the **third** time
the same mistake has been caught here: an argument that holds for the command
that came to mind.
**1. `SwitchView` counts.** I excluded it on the grounds that it carries no
basis and a record surrogate is view-independent, so a switch cannot
invalidate an edit beside it. That is true of `SetCell` and `ClearCell` — and
false of *every other* model command, because `TransposeAxes`, `SetAxis`,
`HideItem`, `SetNumberFormat` and their siblings all act on the **active
view**. So `[SwitchView("Second"), TransposeAxes]` passed both checks and
then transposed `Second` rather than the view the transpose was composed
against.
The rule is now "at most one **document mutation** per frame", where
`mutates_the_document` is the two `Command` variants `Sessions::apply` routes
away from the mirror. Deliberately the *document* question rather than "does
it carry a basis", which is exactly the distinction I got wrong. No client
here batches a `SwitchView` with anything, so it costs nothing.
**2. An explicit `ClientFrame::Save` is not also auto-persisted.** A failed
write leaves `dirty` set — deliberately, so the next frame retries — and the
worker's automatic trigger runs after *every* frame. A save frame therefore
attempted the same write twice. On a permanent failure that is duplicate I/O;
on a transient one it is worse, because the answer was composed from the
first attempt, so the client was told `SaveFailed` about a document that was
now on disk. A per-frame question is the right shape here precisely because
the automatic write exists for edits **nobody asked** to save.
**3.** `Rejection`'s summary line now says it answers a *frame*, since two of
its three classes are not refused commands at all.
Evidence, both faults injected:
- `a_frame_carrying_two_document_mutations_is_refused_whole` gains a third
shape. Dropping `SwitchView` from `mutates_the_document` fails it.
- `an_explicit_save_is_not_retried_by_the_automatic_write` asserts the
**number of write attempts**, which took a counting, always-failing
`Storage`. Both properties are load-bearing: "it was refused" is true
whether the write happened once or twice, and a *successful* save clears
`dirty` so the redundant attempt never happens — the bug is invisible from
both the refusal and the happy path. Restoring the retry gives 2 where the
test requires 1.
`a_refused_batch_leaves_the_commands_before_it_unapplied` now leads with
`SetSelected` rather than `SwitchView`, which the new rule would refuse
first. That is not a concession: a view command is the shape a real frame
has, and it is still observable in the asker's mirror.
1208 passing; clippy, fmt and the wasm gate clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two Copilot findings on PR #24, both correct, both applied.
**`test.fail` was masking setup failures.** Applied before navigation, it
made any earlier failure — the page not loading, the script not starting,
`#status` never updating — an *expected* failure, so CI stayed green through
it. That is precisely the check-the-checker defect these tests exist to
catch, wearing the name of its cure. Both tests now assert their setup
first and apply `test.fail` immediately before the one assertion the known
bug owns. Verified by breaking the setup (making `paintStatus` a no-op):
before, 2 passed; after, **2 failed**.
**Firefox now runs the whole battery, and it passes.** improvise-xf1's
acceptance criterion names Chrome, Firefox and Safari, and the reason is
real rather than ceremonial: `createSyncAccessHandle` is the one API this
design rests on (gotcha 28) and each engine implemented it separately, so
Chromium alone was one engine's opinion. `storage_contract` against
`OpfsFs` now passes on **two** engines.
**Safari is not covered, and the criterion is met for two of three.**
Playwright's WebKit build provides no OPFS at all —
`navigator.storage.getDirectory()` itself rejects with `UnknownError` — so
there is nothing there for `OpfsFs` to be right or wrong about. Rather than
delete the project and quietly narrow the claim, it is kept and the spec
skips on a **runtime probe**: the day that build gains OPFS the checks start
running with no CI change. The skip is conditional on the probe and not on
the browser name, and an unexpected skip on any other engine **fails** with
"that is a regression, not a known gap" — a silently skipped check being the
failure this harness exists to prevent.
The shell spec stays Chromium-only on purpose: it is about `web/app.js`'s
own logic, which is engine-independent, and running `routeWebSocket` plus
the worker handshake three times is slower and flakier to no benefit.
14 passing, 3 skipped, 36s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot's two "previously missed" findings, both correct.
**`extent()` reserved one header row and one header column.** That was right
until this PR: the column header is now one line per level plus an optional
group line, and the row header is `row_levels` columns wide. Announcing a
viewport the page cannot show matters because `#grid` has `overflow: auto` —
the DOM would take a scroll position of its own while the client's cursor
offsets were computed from the larger window, putting the selected cell off
screen. The cost is measured off the painted model and re-announced when it
changes.
Inline group *rows* are deliberately **not** counted, although they occupy
space: they depend on how many rows the window asked for, so counting them
would be a feedback loop — a smaller window crosses fewer groups and asks
for a larger one. The three facts that are counted depend only on the axes.
The residue is a little over-fetch, the direction `ROW_PX` already errs in.
**`fit_to_labels` measured its fallback width in Unicode scalars.** It is
documented as the width a column is drawn at while `ColWidths` catches up,
and the authoritative widths are in display cells, so `日本` came out two
cells wide where it needs four. Now measured with `UnicodeWidthStr`, like
`header_widths` and like the TUI.
`a_column_without_its_width_yet_is_measured_in_display_cells` pins it, and
restoring `chars().count()` fails it with `[2]` against `[4]`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
improvise-oai9 closed on evidence with no code written — the report() helper
was already in the tree, and the fault was injected ({e:#} → {e}) before
closing rather than trusting the description.
improvise-r4lo, improvise-taxx and improvise-o8fa closed by PR #21. Two of the
three record that the TICKET was wrong rather than merely that the bug is
fixed: r4lo's offered escape hatch does not exist, and taxx's proposed
readyState mechanism cannot work because both cases arrive as code 1006 with
wasClean:false.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
From WU-I (PR #21) and WU-G (PR #27) hand-backs. Searched the open backlog
first; none of these duplicates an existing bead.
improvise-kc7n P2 — improvise-ws-server never checks Origin. Verified
independently: `grep -rni origin crates/improvise-ws-server/src/`
returns nothing. A WebSocket handshake is exempt from the
same-origin policy, so any page an allowed user visits can
drive their session from their own allowed address — the
attack an IP allow-list structurally cannot see, because the
request genuinely is from an allowed client. Loopback is not
exempt and has no proxy in front of it, so the local path is
the least protected. Related to improvise-kpnr rather than
blocked by it: authentication and CSWSH are different
questions and an answer to either leaves the other open.
improvise-sihr P2 — every minibuffer mode entered from records mode exits to
plain Normal while the layout is still records. Affects `:`,
Ctrl-E and Ctrl-W; only the last is new. Declined in PR #27
because fixing one arm of a three-arm rule is not a fix, and
characterized there instead.
improvise-vvl3 also gains the four assertions improvise-taxx's fix owes, so
that ticket need not re-derive them — including the one that must fail against
the previous unconditional close handler, which is the guard and not the
decoration. Recorded there because vvl3 now owns the JS test-infrastructure
decision that PR #21 declined.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Edward Langley