The browser asks the grid what kind of grid it is (improvise-e5pf)
One substitution at three decision points: wherever the host asks layout.is_records_mode(), resolve.rs asked state.view.mode.is_records(). The two agree only while the mode and the layout agree, and Command::ChangeMode(AppMode::Normal) is the message that separates them -- the host settles Normal against the layout the batch ends on, reduce_view folds the variant it was handed. GridCache::records carries the fact, projected by grid_of off layout.is_records_mode() and diffed like the extent. Deriving it on the client from row_frame.cats would have been a second implementation of is_records_mode (improvise-edp's disease); deriving it from the mode is the bug. Like Extent it frames nothing, so apply does not re-fit and its batch position is not load-bearing. The client's settling has one home, resolve::changed_mode, reached by every site that can emit AppMode::Normal -- including entered(), which is how a bound `enter-mode normal` gets there. The three decision points ask state.grid.records, so a commit at a records cell declines rather than composing typed_cell_command against a grid whose columns are [coords.., +cat, measures.., +]. lockstep now DRIVES the keys: in_lockstep presses on both ends and refreshes the client from a real projection rather than from the host's slice, which is what let the old records coverage start from agreement. The reachable path is Tab (category panel) then Esc; c/f/v are Ignored in RecordsNormal and R is a stranding decline, which a first draft of the test found the hard way. Two resolve.rs fixtures put a records mode over a non-records grid -- a state no host can produce -- and had to be corrected with the fix. Evidence and both injected faults: context/test-evidence/clients.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -854,6 +854,36 @@ invariant §3(c) is really about, at **every step** of a fold that widens before
|
||||
its widths arrive and narrows before they shrink — not merely at the end, which
|
||||
is where a message-count assertion looks.
|
||||
|
||||
### `GridCache::records` — what kind of grid this is (improvise-e5pf)
|
||||
|
||||
The host answers "is this the wide records layout" with
|
||||
`GridLayout::is_records_mode()`, which reads the axes. A thin client has no
|
||||
`GridLayout`, and it was answering the neighbouring question instead —
|
||||
`ViewState::mode.is_records()` — at three decision points in `resolve.rs`. The
|
||||
two agree only while the mode and the layout agree, and
|
||||
`Command::ChangeMode(AppMode::Normal)` is precisely the message that separates
|
||||
them: the host settles `Normal` against the layout the batch *ends* on
|
||||
(`App::settle_normal_mode`), and `reduce_view` folds the variant it was handed.
|
||||
|
||||
So the fact is **projected**, not derived: `GridCache::records`, carried by
|
||||
`GridUpdate::Records`. Deriving it on the client from `row_frame.cats`
|
||||
containing `_Index` would have been a second implementation of
|
||||
`is_records_mode` — improvise-edp's disease — and deriving it from the mode is
|
||||
the bug itself.
|
||||
|
||||
Like `GridUpdate::Extent` it **frames nothing**, so `apply` does not re-fit and
|
||||
its position in a batch is not load-bearing: it describes what kind of grid the
|
||||
frames describe, not any position in one.
|
||||
|
||||
The client's own settling has **one home**, `resolve::changed_mode`, which every
|
||||
composition site that can emit `AppMode::Normal` goes through — including
|
||||
`entered`, which is how a bound `enter-mode normal` reaches it. The three
|
||||
decision points now ask `state.grid.records`. What that buys concretely: the
|
||||
commit at a records cell **declines** rather than composing
|
||||
`typed_cell_command` against a grid whose columns are
|
||||
`[coords…, +cat, measures…, +]` (gotcha 1), which was a wrong write and strictly
|
||||
worse than the black hole it was written to produce.
|
||||
|
||||
## §3a. The frame's group boundaries and its position in the axis
|
||||
|
||||
Two facts `AxisFrame` gained after `improvise-anr9`, both about group runs.
|
||||
|
||||
@@ -2427,3 +2427,64 @@ does not show — which `claim` would then silently omit, and
|
||||
`Rejection::RecordUnstated` is what the server answers with. The fixtures no
|
||||
longer fabricate, which was the ticket; they do still assume a cursor at the
|
||||
origin.
|
||||
|
||||
### leaving_a_panel_in_records_mode_lands_where_the_host_lands
|
||||
|
||||
improvise-e5pf. `AppMode::Normal` is settled by the host against the layout the
|
||||
batch ends on; the wire carries `Command::ChangeMode(AppMode::Normal)` and
|
||||
`reduce_view` folds it verbatim. So a client that opened a panel from records
|
||||
mode and pressed Esc landed in `Normal` while its host landed in
|
||||
`RecordsNormal` — and `RecordsNormal` has its own keymap, so the divergence
|
||||
compounds on the next keypress.
|
||||
|
||||
The reachable path, which the ticket did not name: `Tab` in `RecordsNormal`
|
||||
resolves locally to `ChangeMode(CategoryPanel)`, and `Esc` there resolves to
|
||||
`ChangeMode(Normal)`. `c`/`f`/`v` are `Ignored` in `RecordsNormal` and `R`
|
||||
itself is a `stranding` decline, so the panel keys a first draft of this test
|
||||
used never separated anything — the test panicked on "the client must answer
|
||||
this key itself" rather than on the drift.
|
||||
|
||||
**Why lockstep could not see it before.** Its records coverage (`typing_at`,
|
||||
`records_pair`) set `host.view_state.mode` by assignment and seeded the client
|
||||
with `client.view = app.view_state.clone()`, so it started where the mode and
|
||||
the layout already agree, and every divergence here requires them to disagree —
|
||||
`doctrine(a_guard_seeded_on_agreement_cannot_detect_drift)`. `in_lockstep`
|
||||
drives the key on both ends and then refreshes the client's grid from a **real
|
||||
projection**, never from the host's slice, which is the only seeding that can
|
||||
show the two apart.
|
||||
|
||||
**Fault injected**: `resolve::changed_mode` reduced to
|
||||
`Command::ChangeMode(mode)`. The test fails on `close: the client and the host
|
||||
disagree about the mode`, `Normal` against `RecordsNormal`.
|
||||
|
||||
Two existing `resolve.rs` fixtures had the same shape and had to be corrected
|
||||
with the fix: `furnished` and `a_records_mode_commit_is_the_hosts` put the
|
||||
client in a records *mode* over a non-records *grid*, which is a state no host
|
||||
can produce. Both now project `GridUpdate::Records` as well. That they had to
|
||||
change is the evidence that the substitution was load-bearing rather than
|
||||
incidental.
|
||||
|
||||
### a_records_commit_is_declined_off_the_grid_and_not_off_the_mode
|
||||
|
||||
improvise-e5pf's damage half. `resolve::commit` is written to **decline** a
|
||||
records commit it cannot compose (it needs `record_col`, gotcha 1). It asked
|
||||
`state.view.mode.is_records()`; with the mode drifted to plain `Editing` over a
|
||||
records grid it did not decline, and composed `typed_cell_command` plus a
|
||||
`Basis` claim against a grid whose columns are `[coords…, +cat, measures…, +]`.
|
||||
A wrong write, strictly worse than the black hole it exists to produce.
|
||||
|
||||
The test states the drift rather than reaching it — reaching it takes the key
|
||||
sequence the test above drives — and asserts no `Command::Model` is composed.
|
||||
|
||||
**The control is the point.** A first version of this test passed against the
|
||||
*broken* code, because `client_of` publishes no basis and `BasisCache::claim`
|
||||
returns `None` until one arrives, so `commit` returned `None` for a reason that
|
||||
had nothing to do with records mode:
|
||||
`doctrine(an_injection_that_does_not_fail_names_an_untested_path)`, caught by
|
||||
injecting the old spelling and watching the test still pass. `records_client`
|
||||
now publishes a basis naming every record the grid projects, and the test
|
||||
carries a positive control — the same `Editing` mode over a **regular** grid
|
||||
must still compose a write — so "this client never writes" cannot pass it.
|
||||
|
||||
**Fault injected**: the decline restored to `state.view.mode.is_records()`. The
|
||||
test fails on `a records commit this client cannot compose was composed anyway`.
|
||||
|
||||
@@ -176,6 +176,9 @@ pub fn grid_in(app: &App, view_name: &str, viewport: Viewport) -> GridCache {
|
||||
rows: layout.row_count(),
|
||||
cols: layout.col_count(),
|
||||
},
|
||||
// Asked of the layout, the same question the host's own `normal_mode`
|
||||
// asks, so the two front ends cannot answer it differently (e5pf).
|
||||
records: layout.is_records_mode(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -613,6 +616,9 @@ pub fn grid_updates(mirror: &GridCache, desired: &GridCache) -> Vec<GridUpdate>
|
||||
if mirror.extent != desired.extent {
|
||||
updates.push(GridUpdate::Extent(desired.extent));
|
||||
}
|
||||
if mirror.records != desired.records {
|
||||
updates.push(GridUpdate::Records(desired.records));
|
||||
}
|
||||
if mirror.row_frame != desired.row_frame {
|
||||
updates.push(GridUpdate::RowFrame(desired.row_frame.clone()));
|
||||
}
|
||||
|
||||
@@ -433,6 +433,9 @@ pub struct GridCache {
|
||||
/// How big the **whole** grid is, of which the frames are one window
|
||||
/// (improvise-bpq2). Not derivable, and not framed — see §3.
|
||||
pub extent: GridExtent,
|
||||
/// Is this the wide records layout (gotcha 1)? **Projected, never derived
|
||||
/// from the axes or from a mode** — improvise-e5pf, and §3.
|
||||
pub records: bool,
|
||||
}
|
||||
|
||||
/// The size of the whole grid: what a cursor clamps against. A named pair,
|
||||
@@ -464,6 +467,7 @@ impl GridCache {
|
||||
.collect();
|
||||
vec![
|
||||
GridUpdate::Extent(self.extent),
|
||||
GridUpdate::Records(self.records),
|
||||
GridUpdate::RowFrame(self.row_frame.clone()),
|
||||
GridUpdate::ColFrame(self.col_frame.clone()),
|
||||
GridUpdate::ColWidths(self.col_widths.clone()),
|
||||
@@ -575,6 +579,9 @@ pub enum GridUpdate {
|
||||
/// How big the whole grid is (improvise-bpq2). Alone among these arms
|
||||
/// its batch position is **not** load-bearing — see §3.
|
||||
Extent(GridExtent),
|
||||
/// Whether this is the wide records layout (improvise-e5pf). Frames
|
||||
/// nothing, like [`GridUpdate::Extent`].
|
||||
Records(bool),
|
||||
}
|
||||
|
||||
impl WidgetCache for GridCache {
|
||||
@@ -613,6 +620,8 @@ impl WidgetCache for GridCache {
|
||||
// No `fit_to_labels`: the extent is the size of the grid the
|
||||
// frames are a window onto, not something they frame.
|
||||
GridUpdate::Extent(extent) => self.extent = *extent,
|
||||
// Nor this: it is what KIND of grid the frames describe.
|
||||
GridUpdate::Records(records) => self.records = *records,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -290,7 +290,7 @@ fn composed(
|
||||
let mode = AppMode::from_name(args.first()?)?;
|
||||
// `declined` answers this first, and both halves hold the rule:
|
||||
// no path opens a room this client cannot commit.
|
||||
stranding(&mode).is_none().then(|| entered(mode))
|
||||
stranding(&mode).is_none().then(|| entered(state, mode))
|
||||
}
|
||||
|
||||
// ── panels ───────────────────────────────────────────────────
|
||||
@@ -312,10 +312,13 @@ fn composed(
|
||||
panel,
|
||||
open: !closing,
|
||||
},
|
||||
Command::ChangeMode(match closing {
|
||||
true => AppMode::Normal,
|
||||
false => panel.mode(),
|
||||
}),
|
||||
changed_mode(
|
||||
state,
|
||||
match closing {
|
||||
true => AppMode::Normal,
|
||||
false => panel.mode(),
|
||||
},
|
||||
),
|
||||
])
|
||||
}
|
||||
|
||||
@@ -374,7 +377,7 @@ fn composed(
|
||||
Some(vec![
|
||||
model(state, ModelCommand::CreateView(name.clone()))?,
|
||||
Command::SwitchView(name),
|
||||
Command::ChangeMode(AppMode::Normal),
|
||||
changed_mode(state, AppMode::Normal),
|
||||
])
|
||||
}
|
||||
"delete-view-at-cursor" => delete_view_at_cursor(state),
|
||||
@@ -456,15 +459,25 @@ fn stranding(mode: &AppMode) -> Option<&'static str> {
|
||||
}
|
||||
|
||||
/// `EnterMode::execute`: clear the incoming mode's minibuffer, then change.
|
||||
fn entered(mode: AppMode) -> Vec<Command> {
|
||||
fn entered(state: &ClientState, mode: AppMode) -> Vec<Command> {
|
||||
let mut commands = Vec::new();
|
||||
if let Some(mb) = mode.minibuffer() {
|
||||
commands.push(write_buffer(&mb.buffer_key, String::new()));
|
||||
}
|
||||
commands.push(Command::ChangeMode(mode));
|
||||
commands.push(changed_mode(state, mode));
|
||||
commands
|
||||
}
|
||||
|
||||
/// `AppMode::Normal` settled against the GRID, the way `App::settle_normal_mode`
|
||||
/// settles it against the layout (improvise-e5pf). The **one** home.
|
||||
fn changed_mode(state: &ClientState, mode: AppMode) -> Command {
|
||||
let settled = match mode {
|
||||
AppMode::Normal if state.grid.records => AppMode::RecordsNormal,
|
||||
other => other,
|
||||
};
|
||||
Command::ChangeMode(settled)
|
||||
}
|
||||
|
||||
// ── the grid cursor ─────────────────────────────────────────────────────
|
||||
|
||||
/// A mirror of `navigation.rs`'s `MoveKind`, spelled again because that crate
|
||||
@@ -691,14 +704,14 @@ fn commit(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option<V
|
||||
if let CellId::Create(kind) = id {
|
||||
return create_column(state, edit_mode, advance, kind, &text);
|
||||
}
|
||||
if state.view.mode.is_records() {
|
||||
if state.grid.records {
|
||||
return None;
|
||||
}
|
||||
let mut commands = vec![Command::Model {
|
||||
basis: state.basis.claim([record_at_cursor(state)?])?,
|
||||
cmd: typed_cell_command(id, &text)?,
|
||||
}];
|
||||
commands.push(Command::ChangeMode(AppMode::Normal));
|
||||
commands.push(changed_mode(state, AppMode::Normal));
|
||||
commands.extend(advanced(state, edit_mode, advance)?);
|
||||
Some(commands)
|
||||
}
|
||||
@@ -714,10 +727,7 @@ fn create_column(
|
||||
) -> Option<Vec<Command>> {
|
||||
// `exit_mode_for`: the tidy-up goes FIRST, where an aborting
|
||||
// `AddCategory` cannot reach it (gotcha 21).
|
||||
let mut commands = vec![Command::ChangeMode(match state.view.mode.is_records() {
|
||||
true => AppMode::RecordsNormal,
|
||||
false => AppMode::Normal,
|
||||
})];
|
||||
let mut commands = vec![changed_mode(state, AppMode::Normal)];
|
||||
let name = text.trim();
|
||||
if !name.is_empty() {
|
||||
commands.push(model(
|
||||
@@ -749,11 +759,7 @@ fn advanced(state: &ClientState, edit_mode: AppMode, advance: Advance) -> Option
|
||||
let row_max = rows.saturating_sub(1);
|
||||
let col_max = cols.saturating_sub(1);
|
||||
let (row, col) = state.view.cursor().selected;
|
||||
if matches!(advance, Advance::Right)
|
||||
&& state.view.mode.is_records()
|
||||
&& row >= row_max
|
||||
&& col >= col_max
|
||||
{
|
||||
if matches!(advance, Advance::Right) && state.grid.records && row >= row_max && col >= col_max {
|
||||
return None;
|
||||
}
|
||||
let landed = match advance {
|
||||
@@ -980,7 +986,7 @@ fn switch_view_at(state: &ClientState, index: usize) -> Option<Vec<Command>> {
|
||||
let summary = state.views.views.get(index)?;
|
||||
Some(vec![
|
||||
Command::SwitchView(summary.name.clone()),
|
||||
Command::ChangeMode(AppMode::Normal),
|
||||
changed_mode(state, AppMode::Normal),
|
||||
])
|
||||
}
|
||||
|
||||
@@ -1662,6 +1668,9 @@ mod tests {
|
||||
/// an **unhandled command** and never a state that said no (improvise-rmvx).
|
||||
fn furnished(mode: AppMode) -> ClientState {
|
||||
let mut state = client_on(record_cell());
|
||||
// A records MODE over a non-records GRID is a state no host can
|
||||
// produce, so the fixture projects both (improvise-e5pf).
|
||||
crate::WidgetCache::apply(&mut state.grid, &GridUpdate::Records(mode.is_records()));
|
||||
state.view.mode = mode;
|
||||
state.grid.page_coords = vec![crate::PageAxis {
|
||||
cat: "Quarter".into(),
|
||||
@@ -2490,6 +2499,7 @@ mod tests {
|
||||
#[test]
|
||||
fn a_records_mode_commit_is_the_hosts() {
|
||||
let mut client = client_on(record_cell());
|
||||
crate::WidgetCache::apply(&mut client.grid, &GridUpdate::Records(true));
|
||||
client.view.mode = AppMode::records_editing();
|
||||
assert_eq!(
|
||||
resolve(&client, ev(KeyCode::Enter)),
|
||||
|
||||
@@ -1094,6 +1094,146 @@ fn records_pair() -> (App, App) {
|
||||
(app, twin)
|
||||
}
|
||||
|
||||
/// One key on both ends, the client refreshed from a real projection rather
|
||||
/// than from the host's own slice — which is what lets them drift.
|
||||
fn in_lockstep(app: &mut App, client: &mut ClientState, key: KeyEvent, what: &str) {
|
||||
let KeyOutcome::Local(cmds) = resolve(client, key) else {
|
||||
panic!("{what}: the client must answer this key itself");
|
||||
};
|
||||
for cmd in &cmds {
|
||||
client.fold(cmd);
|
||||
}
|
||||
reduce_key(app, key);
|
||||
let window = client.window;
|
||||
let fresh = grid_of(
|
||||
app,
|
||||
Viewport {
|
||||
row_offset: client.view.cursor().row_offset,
|
||||
col_offset: client.view.cursor().col_offset,
|
||||
rows: window.0,
|
||||
cols: window.1,
|
||||
},
|
||||
);
|
||||
for update in fresh.seed() {
|
||||
improvise_protocol::WidgetCache::apply(&mut client.grid, &update);
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-e5pf: `AppMode::Normal` is settled against the GRID, so the client
|
||||
/// must settle it too. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn leaving_a_panel_in_records_mode_lands_where_the_host_lands() {
|
||||
let (mut app, _) = records_pair();
|
||||
assert!(
|
||||
app.view_state.mode.is_records(),
|
||||
"the fixture must really be in records mode, or nothing separates the \
|
||||
two answers"
|
||||
);
|
||||
let mut client = records_client(&app);
|
||||
assert!(
|
||||
client.grid.records,
|
||||
"and the client must be told so by the projection, not by its own mode"
|
||||
);
|
||||
|
||||
for (key, what) in [
|
||||
(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE), "open"),
|
||||
(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE), "close"),
|
||||
] {
|
||||
in_lockstep(&mut app, &mut client, key, what);
|
||||
assert_eq!(
|
||||
format!("{:?}", client.view.mode),
|
||||
format!("{:?}", app.view_state.mode),
|
||||
"{what}: the client and the host disagree about the mode"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-e5pf: the commit declines off the GRID, so a client whose mode has
|
||||
/// drifted still declines. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_records_commit_is_declined_off_the_grid_and_not_off_the_mode() {
|
||||
let (app, _) = records_pair();
|
||||
let mut drifted = records_client(&app);
|
||||
// The drift itself, stated rather than reached: a plain `Editing` over a
|
||||
// records grid is what the substitution produced.
|
||||
drifted.view.mode = AppMode::editing();
|
||||
drifted
|
||||
.view
|
||||
.buffers
|
||||
.insert("edit".into(), "typed".to_string());
|
||||
drifted.view.cursor_mut().selected = stored_cell_at(&drifted);
|
||||
|
||||
let enter = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE);
|
||||
assert!(
|
||||
!composes_a_write(&drifted, enter),
|
||||
"a records commit this client cannot compose was composed anyway"
|
||||
);
|
||||
|
||||
// The control: the same mode over a REGULAR grid must still commit, or the
|
||||
// decline above is "this client never writes" and proves nothing.
|
||||
let plain = furnished_host();
|
||||
let mut ordinary = paged_client(&plain);
|
||||
ordinary.view.mode = AppMode::editing();
|
||||
ordinary
|
||||
.view
|
||||
.buffers
|
||||
.insert("edit".into(), "42".to_string());
|
||||
ordinary.view.cursor_mut().selected = stored_cell_at(&ordinary);
|
||||
assert!(!ordinary.grid.records, "the control must be a regular grid");
|
||||
assert!(
|
||||
composes_a_write(&ordinary, enter),
|
||||
"the control never wrote either, so the decline above is vacuous"
|
||||
);
|
||||
}
|
||||
|
||||
/// A records client seeded the way a real one is: from a projection, with a
|
||||
/// basis published, so a declined commit is declined for the stated reason.
|
||||
fn records_client(app: &App) -> ClientState {
|
||||
let mut client = client_of(app, (8, 8));
|
||||
improvise_protocol::WidgetCache::apply(
|
||||
&mut client.basis,
|
||||
&improvise_protocol::basis::published(
|
||||
app.document_generation(),
|
||||
app.layout_version(),
|
||||
client
|
||||
.grid
|
||||
.cells
|
||||
.iter()
|
||||
.flatten()
|
||||
.filter_map(|cell| match &cell.id {
|
||||
Some(improvise_protocol::CellId::Record(rc)) => Some((rc.record, 0)),
|
||||
_ => None,
|
||||
})
|
||||
.collect(),
|
||||
),
|
||||
);
|
||||
client
|
||||
}
|
||||
|
||||
/// The first cell the client's own projection says is a stored record.
|
||||
fn stored_cell_at(client: &ClientState) -> (usize, usize) {
|
||||
client
|
||||
.grid
|
||||
.cells
|
||||
.iter()
|
||||
.enumerate()
|
||||
.find_map(|(r, row)| {
|
||||
row.iter()
|
||||
.position(|cell| matches!(cell.id, Some(improvise_protocol::CellId::Record(_))))
|
||||
.map(|c| (r, c))
|
||||
})
|
||||
.expect("the fixture must project a stored cell")
|
||||
}
|
||||
|
||||
fn composes_a_write(client: &ClientState, key: KeyEvent) -> bool {
|
||||
match resolve(client, key) {
|
||||
KeyOutcome::Local(cmds) => cmds
|
||||
.iter()
|
||||
.any(|c| matches!(c, improvise_protocol::Command::Model { .. })),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
/// Where the client's own projected grid puts a creating column. Asked of the
|
||||
/// projection, never re-derived from the axes — that is ADR 002's whole point.
|
||||
fn creating_cell(client: &ClientState, kind: improvise_protocol::CreateKind) -> (usize, usize) {
|
||||
|
||||
Reference in New Issue
Block a user