fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(browser): the tile bar reaches records mode, where every key was silent

Browse Source 
`declined()` answers `toggle-records-mode` out loud and the code read as though
that closed the route. It did not. The tile bar carries the virtual `_Index` and
`_Dim` (`View::tileable_cats`), `set-axis-for-tile` composes locally, and records
mode auto-detects from `_Index` on Row plus `_Dim` on Column — so a handful of
undeclined keys land a browser in `RecordsNormal`, where `i` did nothing and said
nothing.

The silence had TWO sources, and the ticket predicted one. On a measure column
the cell is a `CellId::Record`, so the editor opened and `Enter` died in
`commit`'s `grid.records` early return. On a COORDINATE column the projected cell
carries no `CellId` at all, so `edit_or_drill` short-circuited before reaching any
arm — improvise-fdk1 seen from the key path, since a coordinate rename names a
record the `RecordHash` surrogate cannot address.

So the guard goes at the entrance of `edit_or_drill`, before the cell is looked
at, rather than on the `Record` arm: a guard on the arm answers "is this cell one
we cannot commit" where the question is "is this grid one we cannot commit in".
It reads `grid.records` and not `mode.is_records()` for the reason `commit`
already did — the grid is the observed fact and the mode settles from it.

`CellId::Create` now declines instead of returning `None`, so a creating column
reaching this function is a door rather than a hole. `create_column`
(improvise-gcvb) was already unreachable and is now explicitly so.

The regression guard walks every column of the records row and prints each
cell's `CellId`, because a fixture on the measure column — the one the ticket's
transcript stands on — cannot see the other two. Two rivals were injected and
each fails on a different assertion; both are recorded in the evidence file.

Closes improvise-bd9o.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-22 00:54:22 -07:00
parent d46ec59984
commit d124f18c2e
4 changed files with 227 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/clients.md
+62
View File
@@ -587,6 +587,68 @@ The classes that remain:
- **Drill.** Session-local residue, and the one case that does **not** go
upstream silently — see below.
#### The records-mode decline is on the entrance, not the exit (improvise-bd9o)
`declined()` answers `toggle-records-mode` with "records mode is not available
in the browser yet", and for a while the code read as though that closed the
route. **It did not, and the ticket's own account of the remaining silence was
one case short of complete.**
The route is the tile bar. `project::tile_entries` walks `View::tileable_cats`,
which includes the virtual `_Index` and `_Dim` (gotcha 27), and
`set-axis-for-tile` is fully composable here. Records mode **auto-detects** from
`_Index` on Row plus `_Dim` on Column (gotcha 1), so a handful of ordinary,
undeclined keys — `T`, a tile cursor walk, `r`, another walk, `c`, `Esc` — put a
browser in `RecordsNormal` with `grid.records` true. This is failure-catalogue
#4, an unreachability claim only as wide as the routes its author enumerated:
the *toggle* was considered and the *axis assignment* was not.
Once there, `i` was silent, and **from two different sources** — which is why a
fixture standing on one of them cannot see the other:
- On a **measure** column the cell is a `CellId::Record`, so `edit_or_drill`
opened the editor; `commit` then returned `None` for `state.grid.records`, so
`Enter` went upstream and `Host::handle` answered `ClientFrame::Key` with an
explicit empty vec (gotcha 47). The editor stayed open holding the typed text.
This is the case the ticket describes.
- On a **coordinate** column — the leftmost columns, the ones carrying the row's
category items — the projected cell carries **no `CellId` at all**. Per gotcha
42 a `None` id means "no projected cell", so `edit_or_drill`'s
`cell.id.as_ref()?` returned `None` before reaching any arm, and `i` did not
even open an editor. Nothing happened and nothing was said. The ticket did not
predict this one, and it is not a second bug: it is improvise-fdk1 seen from
the key path, since a coordinate rename has no wire form precisely because the
`RecordHash` surrogate names records that already exist.
So the guard goes at the **entrance** — the first statement of `edit_or_drill`,
before the cell is even looked at — and not on the `CellId::Record` arm where
the ticket's case lives. A guard on the arm answers *is this cell one we cannot
commit*, where the question is *is this grid one we cannot commit in*; the
coordinate column separates the two, and `the_tile_bar_route_into_records_mode_does_not_strand_the_user`
walks every column of the row for exactly that reason.
It tests `state.grid.records` rather than `state.view.mode.is_records()` for the
reason `commit` already did: the grid is the **observed** fact and the mode is
settled from it (`settled_mode`), so a mode-shaped guard would be a second
description of records-ness free to disagree with the first.
**What this costs, stated rather than discovered later.** `create_column`
(improvise-gcvb) is now **explicitly** dead rather than accidentally so. It was
already unreachable — `edit_or_drill`'s `CellId::Create` arm returned `None`,
and `o` is declined at its `add-record-row` step, so nothing opened
`RecordsEditing` in a browser at all — but the deadness was an accident of two
unrelated arms rather than a decision. The `CellId::Create` arm now *declines*
instead of returning `None`, so that if a creating column ever reaches this
function from outside records mode it is a door rather than a hole. Reviving
`create_column` for real means opening the editor on `CellId::Create` while
still refusing the other columns, which is a change worth making on its own
evidence and not as a side effect of closing a silence.
And the existing `toggle-records-mode` decline is now **true**, where before it
was a claim the tile bar contradicted. That is the standing lesson from
improvise-cfly arriving from the other direction: there, a decline outlived its
reason; here, a decline was never as wide as its wording.
#### The three improvise-e24v listed as free, and what each really costs
improvise-4bwa. Each of the three is short exactly one fact, and the three
context/test-evidence/clients.md
+57
View File
@@ -3598,3 +3598,60 @@ It asserts the whole sequence each sink saw, not the last value: the fault it
separates from is `onShell` **losing** its text, and a test reading only the
final `#status` would pass against a page that wrote the model's status there
and then happened to be asked again.
### the_tile_bar_route_into_records_mode_does_not_strand_the_user
improvise-bd9o. `declined()` answers `toggle-records-mode` out loud, and the
code read as though that closed the route into records mode. It did not: the
tile bar carries `_Index` and `_Dim` (`View::tileable_cats`), `set-axis-for-tile`
composes locally, and records mode auto-detects from `_Index` on Row plus `_Dim`
on Column — so `T`, a tile walk, `r`, another walk, `c`, `Esc` lands a browser in
`RecordsNormal` with nothing declined on the way.
**What failed before the fix.** At `d46ec59` the test's own five-key preamble
reached `grid.records == true`, and then `i` changed *nothing*: status stayed at
the tile bar's `"_Dim → Col"` and the mode stayed `RecordsNormal`. Verified by
running the test against unmodified `resolve.rs`.
**Two sources, and the fixture walks every column because of it.** A probe of
the projected row showed them to be different failures:
| column | `CellId` | what `i` did |
|---|---|---|
| 0 (coordinate) | `None` | nothing at all — `edit_or_drill`'s `cell.id.as_ref()?` short-circuited |
| 1 (`+cat`) | `Create(Category)` | nothing — the `Create` arm returned `None` |
| 2–3 (measures) | `Record(_)` | opened the editor; `Enter` then died in `commit`'s `grid.records` early return |
Only the third is the case improvise-bd9o describes. The first is improvise-fdk1
seen from the key path: a coordinate column has no surrogate because a rename
names a record that does not exist yet. A fixture placed on a measure column —
the obvious one to write, since that is where the ticket's transcript stands —
cannot see either of the others, so the test iterates `0..extent.cols` and
prints the `CellId` in each failure message.
**Two rivals, each failing on a different assertion.**
- *Guard the arm, not the grid*: `CellId::Record(_) if state.grid.records =>
decline`. This is the shape the ticket's own acceptance criterion reads like,
and it is what a fixture on a measure column would have accepted. It fails
here at **column 0 (`None`)** on the status assertion — the coordinate column
never reaches an arm.
- *Say it and open anyway*: return the decline **plus** `ChangeMode(mode)`. It
fails at **column 0** on the second assertion, `!mode.is_editing()`. This is
the rival that matters most, because "the user was told something" is the
assertion one naturally writes, and alone it is satisfied by a client that
speaks and then strands the user in an editor whose `Enter` is still a black
hole.
**Why the status line is cleared with a sentinel and not a keypress.** Each
column needs a fresh "was anything said", and every candidate key for clearing
it (`x`, `Escape`) is itself a key this client might decline or act on — which
would either supply the status the assertion is looking for or move the cursor
out from under it. `state_mut().view.status_msg` is written directly to a value
no code path produces.
**Why the preamble reads the tile bar rather than counting `l` presses.** The
ticket's transcript spells the route as `T r l c Esc`, which depends on
`tileable_cats`' order. `tile_to` finds the named category's index in
`state().tiles` and walks to it, so the test pins *the route exists*, not a
particular ordering the producer is free to change.
crates/improvise-protocol/src/resolve.rs
+8 -3
View File
@@ -635,6 +635,11 @@ fn cell_at_cursor(state: &ClientState) -> Option<&crate::CellDisplay> {
/// `EditOrDrill::execute`, with ADR 002 answering the branch, and drill
/// declining out loud rather than upstream. See `context/code/clients.md`.
fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
// The grid, not the mode: a records GRID is the observed fact and the mode
// follows it (improvise-bd9o, `context/code/clients.md`).
if state.grid.records {
return Some(vec![Command::SetStatus(not_yet("editing in records mode"))]);
}
let cell = cell_at_cursor(state)?;
match cell.id.as_ref()? {
// `edit_text`, never `text`: the drawn text has been through
@@ -652,9 +657,9 @@ fn edit_or_drill(state: &ClientState, mode: AppMode) -> Option<Vec<Command>> {
CellId::Derived { target } => Some(vec![Command::SetStatus(format!(
"{target} is computed by a formula — edit the formula, not the cell"
))]),
// A creating column takes a name, and committing one is the records
// fork this client does not have (improvise-poia).
CellId::Create(_) => None,
// Both creating columns are records-only, so the guard above takes
// them; declining rather than `None` keeps that a door, not a hole.
CellId::Create(_) => Some(vec![Command::SetStatus(not_yet("creating a column"))]),
}
}
crates/improvise-wasm-client/tests/browser_round_trip.rs
+100
View File
@@ -388,3 +388,103 @@ fn a_commit_keypress_is_one_mutation_among_view_commands_and_is_accepted() {
a host that dropped the frame"
);
}
// ── the tile-bar route into records mode (improvise-bd9o) ───────────────
/// Drive one key all the way round: the client resolves it, the host answers
/// whatever it composed, and every frame comes back. Evidence file.
fn press(
host: &mut Host,
id: improvise_command::ui::project::SessionId,
client: &mut Client,
key: &str,
) {
let Some(frame) = client.key(&improvise_wasm_client::BrowserKey::new(key)) else {
return;
};
let json = serde_json::to_string(&frame).expect("a client frame serializes");
let frames = send(host, id, &json);
deliver(client, &frames);
}
/// Walk the tile cursor to a named category and put it on `axis`'s key. The
/// tile bar's own order is the host's, so it is read rather than assumed.
fn tile_to(
host: &mut Host,
id: improvise_command::ui::project::SessionId,
client: &mut Client,
category: &str,
key: &str,
) {
let at = client
.state()
.tiles
.tiles
.iter()
.position(|t| t.category == category)
.unwrap_or_else(|| panic!("the tile bar must carry {category}"));
while client.state().view.tile_cat_idx < at {
press(host, id, client, "l");
}
while client.state().view.tile_cat_idx > at {
press(host, id, client, "h");
}
press(host, id, client, key);
}
/// `toggle-records-mode` is declined and the tile bar reaches records mode
/// anyway (improvise-bd9o). Evidence file.
#[test]
fn the_tile_bar_route_into_records_mode_does_not_strand_the_user() {
let mut host = host();
let mut client = Client::new();
let handshake: ClientFrame = serde_json::from_str(&client.set_viewport(10, 10))
.expect("the viewport frame must be one the server reads");
let ClientFrame::Viewport { rows, cols } = handshake else {
panic!("set_viewport must produce a Viewport frame");
};
let (id, snapshot) = host.connect(rows, cols);
deliver(&mut client, &[snapshot]);
press(&mut host, id, &mut client, "T");
tile_to(&mut host, id, &mut client, "_Index", "r");
tile_to(&mut host, id, &mut client, "_Dim", "c");
press(&mut host, id, &mut client, "Escape");
assert!(
client.state().grid.records,
"the tile-bar route must actually reach records mode, or this test is \
not standing where the ticket stood"
);
// Every column of the row, because the silence had TWO sources and a
// fixture on one of them cannot see the other. Evidence file.
let cols = client.state().grid.extent.cols;
assert!(
cols >= 3,
"the fixture must reach past the coordinate column"
);
for col in 0..cols {
while client.state().view.cursor().selected.1 < col {
press(&mut host, id, &mut client, "l");
}
let kind = format!(
"{:?}",
client.state().grid.cell(0, col).and_then(|c| c.id.clone())
);
// A sentinel rather than a key: clearing the line must not itself be
// a keypress this client might decline.
client.state_mut().view.status_msg = "\u{0}unsaid".to_string();
press(&mut host, id, &mut client, "i");
assert_ne!(
client.state().view.status_msg,
"\u{0}unsaid",
"column {col} ({kind}): a key whose commit this client cannot \
compose must say so"
);
assert!(
!client.state().view.mode.is_editing(),
"column {col} ({kind}): the editor must not open on a cell this \
client cannot commit — an Enter that is a black hole"
);
}
}