Merge PR #242 (1c6600c3) into the 2026-09-24 integration wave

# Conflicts:
#	context/test-evidence/command.md
This commit is contained in:
Edward Langley
2026-09-24 02:43:51 -07:00
16 changed files with 314 additions and 31 deletions
+5 -3
View File
@@ -1122,9 +1122,11 @@ implemented rule is "clicking the already-selected tile cycles it; clicking any
other selects it", where *already selected* is `mode == TileSelect` **and**
`tile_cat_idx == index`. The tempting summaries — "a click never mutates on its
own", "a **second** click cycles" — both have the same counterexample, found on
review: `T` enters `TileSelect` leaving `tile_cat_idx` at **0**, so a click on
the **leftmost** tile is a click on the already-selected one and cycles on the
*first* gesture. Two adjacent tiles therefore answer the identical first click
review: `T` enters `TileSelect` with `tile_cat_idx` at the tile
`opening_tile` picks (the first Regular category, improvise-oplp; see
`context/code/command.md`), so a click on **that** tile is a click on the
already-selected one and cycles on the *first* gesture. Before improvise-oplp
this was the leftmost tile, `_Index`. Two adjacent tiles therefore answer the identical first click
differently, and that is correct rather than a hole — a click does what `Enter`
does there, and `Enter` in `TileSelect` cycles the tile under the cursor. The
resolver cannot see *how* the state arose and must not try to.
+45 -5
View File
@@ -3187,11 +3187,51 @@ for three reasons, each checked rather than argued:
`renders_each_tile_with_its_axis_label` fail, as well as the two characterization
tests here.
What the default cursor does cost, stated: one cycle key on tile 0 puts `_Index`
on Row alone, and in a pivot view that empties the grid (0 rows on
`web/demo.improv`). The status line says `_Index → Row`, and three more presses
undo it. If that turns out to matter, the fix is the order the tile bar lists
categories in, not taking entries out of it.
**Where the tile cursor opens (improvise-oplp, improvise-bqm0).** It used to
open on tile 0, which is `_Index`, so one cycle key (`T Enter`, read as
"confirm", or `T r`) put `_Index` alone on Row and emptied a pivot grid (0 rows
on `web/demo.improv`). The browser worker saves every accepted model command, so
there the empty grid became the stored document at once.
The fix places the cursor and keeps the order. Both ends call
`improvise_protocol::opening_tile`, which opens on the best-ranked tile (the
leftmost on a tie): a Regular category first, then `_Measure`, then a Label, and
`_Index`/`_Dim` last. **Placing the cursor beat reordering the tiles** because
the order is itself a decided, tested output. The browser's click targets are
tile *indices*, so reordering would move every click position. And
`the_tile_bar_offers_every_category_by_name_measure_and_records_axes_included`
pins the offered list, which is the characterization #192 made. Moving the cursor
changes only where one keypress lands.
**The ranking, and why it is an enum and not a bool.** `TileEntry` carries the
host's `CategoryKind`, because a client cannot tell `_Index` from a user category
without sniffing names (design-principles §2). #231 first carried
`regular: bool`, which answers only one question. It could not say what to open
on when a view has *no* Regular category, and so fell back to tile 0, `_Index`,
which is the original bug (improvise-bqm0, confirmed on a document holding only
`_Measure` data). The rank is an exhaustive `match`, so a new kind forces a
decision. `_Measure` is the fallback, not the primary: it is on an axis in every
shipped document, and `Enter` reads as "confirm" on the user's own dimension. It
ranks above a Label because a Label is a high-cardinality per-row field, and
cycling it onto Row would explode the grid rather than empty it. The fallback is
**not** always `_Measure` (improvise-h3cv). In the `_Drill` view `drill_steps`
puts every coordinate of the drilled key on `Axis::Filter`, `_Measure` included,
and `tileable_cats` drops Filter, so `_Measure` is not a tile there. A drill's
tiles are `_Index`, `_Dim` and whatever sits on None, so the Label rank and the
`_Index` fallback are live exactly in a drill: a Label opens if there is one,
and otherwise `_Index`, because nothing better is on offer.
`in_a_drill_tile_select_opens_on_a_label_not_on_index` and
`in_a_drill_with_nothing_better_tile_select_opens_on_index` drive both through
the TUI's own keys. They are not lockstep tests, because a browser declines `>`.
**The tests had to separate the rule from its neighbours** (improvise-52od). In
every early fixture the first Regular tile was also the first tile on Row, so a
rival `position(axis == Row)` passed the whole workspace. Records mode is where
they part: `_Index` holds Row there, and the rival cycles it, dropping the user
out of records mode. `tile_select_opens_on_a_regular_category_even_when_a_virtual_holds_row`
and `in_records_mode_tile_select_opens_on_a_regular_category_on_both_ends` pin
that; the evidence is in `context/test-evidence/protocol.md` and
`clients.md`.
**`_Measure` must stay tileable.** It is on an axis in every shipped document, so
`regular_category_names()`, the advice for *prompts*, is the wrong filter here.
+3 -1
View File
@@ -493,7 +493,9 @@ compiler named every other consumer. Two remained, and both are the wire:
A third arrived with improvise-km71 by the same rule: `FormulaId`, which crosses
the wire in `ModelCommand::ReplaceFormula`, `FormulaSlot` and `FormulaEntry`.
So `improvise-formula` depends on serde again, for that one type. No derive went
back on the formula AST.
back on the formula AST. A fourth, `CategoryKind`, went back on when `TileEntry`
began carrying it (improvise-bqm0; `context/code/command.md`). It returned as a
wire type, not as a document one: nothing loads a `Category` through serde.
`CellValue` was never a candidate, since it is carried on the wire too. JSON
*import* (`StartImportWizard`, `ImportJsonHeadless`) is a separate feature and is
untouched.
+19
View File
@@ -5175,6 +5175,25 @@ is touched. Enter must then land on (2, 0), and the folded `opened_editor` must
be exactly `OpenedEditor::on("800")`. **Injection:** `resolve::advanced`
carrying the previous editor's `touched` flag. This test fails alone.
### in_records_mode_tile_select_opens_on_a_regular_category_on_both_ends
improvise-52od, on both ends. `records_pair()` puts host and twin in records mode
(`_Index` on Row, `_Dim` on Column). Then `T Enter` through `both_ends`, and the
case asserts that `_Index` and `_Dim` kept their records axes, that `Region` (the
tile `T` opens on) moved, and that the twin agrees. Injected rival
`opening_tile = position(axis == Row)`: this case fails, and so does the
protocol unit test. `tile_select_opens_on_a_regular_category_on_both_ends`
*still passes* under that rival, which is the ticket's finding in one line: its
fixture had the two rules agree.
### with_no_regular_category_tile_select_opens_on_measure_on_both_ends
improvise-bqm0, on both ends. A document holding only `_Measure` data (one item,
one cell), then `T Enter`. `_Index` must keep its axis, and `_Measure` must be
the tile that moved. Injected #231's rule (`first Regular, else 0`): this case
fails, because tile select opened on `_Index` and cycled it onto Row, which is
the bug reproduced headless in the ticket.
### `enter_enter_leaves_every_cell_unchanged_on_both_ends`
**improvise-fy9o**, browser and host in lockstep. On the 2×2 `precise_host`,
+21
View File
@@ -6469,6 +6469,27 @@ shipped build.
Each rival re-derives one half's chain, so the count moves off 18. The file
was restored after each and compared byte for byte.
### in_a_drill_tile_select_opens_on_a_label_not_on_index
improvise-h3cv. `context/code/command.md` claimed tile select's fallback is
always `_Measure`. In a drill `_Measure` is on Filter and is not a tile, so the
tiles are `_Index`, `_Dim` and a Label `Note` on None. The test drills with `>`
on a one-cell document, asserts `_Measure` really is on Filter (so the fixture
cannot silently stop being a drill), presses `T`, and requires the cursor on
`Note`. Two rivals, each injected into `opening_tile` and restored after: #231's
`position(kind == Regular).unwrap_or(0)` opens on `_Index` and fails; ranking a
Label below `_Index`/`_Dim` also opens on `_Index` and fails. Neither rival fails
the companion test below.
### in_a_drill_with_nothing_better_tile_select_opens_on_index
improvise-h3cv's other half. The same drill with no Label leaves only `_Index`
and `_Dim`, so the ranking has nothing better and the cursor opens on `_Index`,
the first of the two. This pins that the ranking does not panic or pick `_Dim`
when every tile has the lowest rank. It is a characterization, not a bug guard.
Neither this nor its sibling is a lockstep test, because the browser declines
`>` (drill-into-cell), so only the TUI can reach a drill.
## `ui/host.rs` — the abort arm of `Host::mutating_frame` (improvise-we1a)
### every_failing_model_effect_that_aborts_also_refuses
+19
View File
@@ -1191,3 +1191,22 @@ compares **content in order**: the drawn keys rows must equal
`help_bindings()` looked up through the same `keys`, row for row. The same
injection now fails. What still passes is a bypass that draws exactly what the
builders asked for, in the order they asked — which is harmless by definition.
### tile_select_opens_on_a_regular_category_even_when_a_virtual_holds_row
improvise-52od. `opening_tile`'s rule is "the best-ranked tile", and in every
earlier fixture its top rank, the first Regular tile, was also the first tile on
Row. So the rival `position(|t| t.axis == Axis::Row).unwrap_or(0)`, which needs
no wire field at all, passed the whole workspace. The fixture is a records-shaped
bar: `_Index` on Row, `_Dim` on Column, `_Measure` on None, `Region` on Page and
`Product` on Row. The rule answers 3 (`Region`). The Row rival answers 0
(`_Index`), and a "first Regular on Row" rival answers 4 (`Product`), so all
three are separated. Injected: the Row rival fails this test.
### with_no_regular_category_tile_select_opens_on_measure_not_index
improvise-bqm0. #231's rule, `position(regular).unwrap_or(0)`, fell back to tile
0, which is `_Index`, when a view has no Regular category, which is exactly the
bug #231 fixed. The fixture is `_Index`, `_Dim`, a Label and `_Measure`. The rule
answers 3 (`_Measure`), then 2 (the Label) with `_Measure` removed, and 0 for an
empty bar. Injected: #231's rule answers 0 and fails. The Row rival fails too.
+52
View File
@@ -5176,3 +5176,55 @@ mod formula_edit_tests {
assert_eq!(buffer(&app), Some("Sum = East + West"));
}
}
/// Where `T` opens inside a drill, where `_Measure` is on Filter and so is no tile
/// (improvise-h3cv). `context/test-evidence/command.md`.
#[cfg(test)]
mod drill_tile_select {
use super::*;
use crate::command::key::{KeyCode, KeyEvent, KeyModifiers};
use crate::model::cell::{CellKey, CellValue};
use crate::view::Axis;
use crate::workbook::Workbook;
fn drilled(with_label: bool) -> App {
let mut wb = Workbook::new("Drill");
wb.add_category("Region").unwrap();
wb.add_item("Region", "North").unwrap();
wb.add_item("_Measure", "Revenue").unwrap();
if with_label {
wb.add_label_category("Note").unwrap();
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), "North".into()),
("_Measure".into(), "Revenue".into()),
]),
CellValue::Number(5.0),
);
let mut app = App::new(wb, None);
let key = |c| KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE);
app.handle_key(key('>')).expect("drill");
assert_eq!(app.view().axis_of("_Measure"), Axis::Filter, "a real drill");
app.handle_key(key('T')).expect("tile select");
app
}
fn opened_on(app: &App) -> String {
let names = app.model_state.workbook.model.category_names();
let tiles = app.view().tileable_cats(&names);
tiles[app.view_state.tile_cat_idx].to_string()
}
#[test]
fn in_a_drill_tile_select_opens_on_a_label_not_on_index() {
assert_eq!(opened_on(&drilled(true)), "Note");
}
#[test]
fn in_a_drill_with_nothing_better_tile_select_opens_on_index() {
assert_eq!(opened_on(&drilled(false)), "_Index");
}
}
+4 -2
View File
@@ -5,7 +5,7 @@ use std::collections::BTreeMap;
use improvise_core::format::FormatPlan;
use improvise_core::model::Model;
use improvise_core::model::category::MEASURE_CATEGORY;
use improvise_core::model::category::{CategoryKind, MEASURE_CATEGORY};
use improvise_core::model::cell::CellValue;
use improvise_core::model::record::{AggregateHash, RecordHash};
use improvise_core::view::layout::{AxisEntry, GridLayout, RecordCol};
@@ -429,7 +429,9 @@ pub fn tiles_of(model: &Model, view: &View) -> Vec<TileEntry> {
.map(|cat| TileEntry {
category: cat.to_string(),
axis: view.try_axis_of(cat).unwrap_or(Axis::None),
regular: model.category(cat).is_some_and(|c| c.kind.is_regular()),
kind: model
.category(cat)
.map_or_else(|| CategoryKind::of_name(cat), |c| c.kind),
})
.collect()
}
+2 -1
View File
@@ -1,4 +1,5 @@
use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
pub type CategoryId = usize;
pub type ItemId = usize;
@@ -49,7 +50,7 @@ impl Group {
/// Regular categories store their items explicitly; virtual ones are
/// synthesized at query time by the layout layer. context/code/core.md §9.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, Default)]
pub enum CategoryKind {
#[default]
Regular,
+1 -1
View File
@@ -300,7 +300,7 @@ mod tests {
])));
live.fold(&Command::Project(Projection::TileBar(vec![TileEntry {
category: "Region".into(),
regular: true,
kind: crate::CategoryKind::Regular,
axis: Axis::Row,
}])));
live.fold(&Command::Project(Projection::ViewList(vec![ViewSummary {
+2
View File
@@ -43,6 +43,8 @@ pub use viewmodel::{CatTreeEntry, flatten_cat_tree, view_axis_summary};
// `improvise-core`'s, because the index that resolves one lives in
// `DataStore`. Re-exported so a client depends on the protocol alone.
pub use improvise_core::model::record::{AggregateHash, RecordHash};
// A tile carries its kind so a client can rank it without sniffing names.
pub use improvise_core::model::category::CategoryKind;
pub use improvise_core::view::layout::PageAxis;
// On the wire in `ModelCommand::SetCell`, so a client needs to name it
+1 -1
View File
@@ -512,7 +512,7 @@ mod tests {
}])),
Command::Project(Projection::TileBar(vec![TileEntry {
category: "Region".into(),
regular: true,
kind: crate::CategoryKind::Regular,
axis: Axis::Page,
}])),
// One populated value per payload family: `RecordCell`, `Axis`,
+65 -9
View File
@@ -4,6 +4,7 @@
use serde::{Deserialize, Serialize};
use unicode_width::UnicodeWidthStr;
use improvise_core::model::category::CategoryKind;
use improvise_core::model::cell::CellValue;
use improvise_core::model::record::{AggregateHash, RecordHash};
use improvise_core::view::Axis;
@@ -793,19 +794,29 @@ impl WidgetCache for ViewListCache {
}
}
/// One tile in the tile bar. `regular` is the host's `CategoryKind` answer: a
/// client cannot tell `_Index` from a user category without sniffing names.
/// One tile in the tile bar. `kind` is the host's answer: a client cannot tell
/// `_Index` from a user category without sniffing names (improvise-bqm0).
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct TileEntry {
pub category: String,
pub axis: Axis,
pub regular: bool,
pub kind: CategoryKind,
}
/// Where tile select opens: the first regular category, so `T Enter` cannot
/// put `_Index` alone on Row (improvise-oplp). Virtuals stay reachable.
/// Where tile select opens: the best-ranked tile, the leftmost on a tie, so
/// `T Enter` never puts `_Index` alone on Row. `context/code/command.md`.
pub fn opening_tile(tiles: &[TileEntry]) -> usize {
tiles.iter().position(|tile| tile.regular).unwrap_or(0)
let rank = |kind: CategoryKind| match kind {
CategoryKind::Regular => 0,
CategoryKind::VirtualMeasure => 1,
CategoryKind::Label => 2,
CategoryKind::VirtualIndex | CategoryKind::VirtualDim => 3,
};
tiles
.iter()
.enumerate()
.min_by_key(|(i, tile)| (rank(tile.kind), *i))
.map_or(0, |(i, _)| i)
}
/// The tile bar's tileable categories and axes (improvise-pca). Its overlap
@@ -931,6 +942,51 @@ pub struct Snapshot {
mod tests {
use super::*;
fn tile(category: &str, axis: Axis, kind: CategoryKind) -> TileEntry {
TileEntry {
category: category.to_string(),
axis,
kind,
}
}
/// Records mode puts `_Index` on Row and the first regular elsewhere, so
/// "first regular" and "first on Row" part (improvise-52od, protocol.md).
#[test]
fn tile_select_opens_on_a_regular_category_even_when_a_virtual_holds_row() {
let tiles = [
tile("_Index", Axis::Row, CategoryKind::VirtualIndex),
tile("_Dim", Axis::Column, CategoryKind::VirtualDim),
tile("_Measure", Axis::None, CategoryKind::VirtualMeasure),
tile("Region", Axis::Page, CategoryKind::Regular),
tile("Product", Axis::Row, CategoryKind::Regular),
];
assert_eq!(opening_tile(&tiles), 3, "Region, the first regular tile");
}
/// No regular category: open on `_Measure`, never on `_Index` or `_Dim`
/// (improvise-bqm0). Why this ranking: `context/code/command.md`.
#[test]
fn with_no_regular_category_tile_select_opens_on_measure_not_index() {
let tiles = [
tile("_Index", Axis::None, CategoryKind::VirtualIndex),
tile("_Dim", Axis::None, CategoryKind::VirtualDim),
tile("Note", Axis::None, CategoryKind::Label),
tile("_Measure", Axis::Column, CategoryKind::VirtualMeasure),
];
assert_eq!(
opening_tile(&tiles),
3,
"_Measure outranks a label and the records axes"
);
assert_eq!(
opening_tile(&tiles[..3]),
2,
"a label outranks _Index and _Dim"
);
assert_eq!(opening_tile(&[]), 0, "an empty bar opens at 0");
}
fn valued(value: CellValue) -> CellDisplay {
CellDisplay {
value: Some(value),
@@ -1533,7 +1589,7 @@ mod tests {
let tiles = TileBarCache::folded(&[vec![TileEntry {
category: "Region".into(),
axis: Axis::Page,
regular: true,
kind: crate::CategoryKind::Regular,
}]]);
assert_eq!(tiles.tiles[0].axis, Axis::Page);
}
@@ -1931,7 +1987,7 @@ mod tests {
Projection::TileBar(vec![TileEntry {
category: "Region".into(),
axis: Axis::Page,
regular: true,
kind: crate::CategoryKind::Regular,
}]),
];
for projection in &projections {
@@ -1968,7 +2024,7 @@ mod tests {
Projection::TileBar(vec![TileEntry {
category: "Region".into(),
axis: Axis::Page,
regular: true,
kind: crate::CategoryKind::Regular,
}]),
],
};
+4 -4
View File
@@ -1693,12 +1693,12 @@ mod tests {
crate::TileEntry {
category: "Region".into(),
axis: Axis::Row,
regular: true,
kind: crate::CategoryKind::Regular,
},
crate::TileEntry {
category: "Product".into(),
axis: Axis::Column,
regular: true,
kind: crate::CategoryKind::Regular,
},
];
state
@@ -2357,7 +2357,7 @@ mod tests {
.map(|category| crate::TileEntry {
category: category.into(),
axis: Axis::Row,
regular: true,
kind: crate::CategoryKind::Regular,
})
.collect();
state.view.tile_cat_idx = 1;
@@ -3298,7 +3298,7 @@ mod tests {
tiles.tiles.tiles = vec![crate::TileEntry {
category: "Region".into(),
axis: improvise_core::view::Axis::Row,
regular: true,
kind: crate::CategoryKind::Regular,
}];
assert_eq!(
resolve(&tiles, ev(KeyCode::Char('h'))),
+4 -4
View File
@@ -857,12 +857,12 @@ mod tests {
TileEntry {
category: "Region".into(),
axis: Axis::Row,
regular: true,
kind: improvise_protocol::CategoryKind::Regular,
},
TileEntry {
category: "Product".into(),
axis: Axis::None,
regular: true,
kind: improvise_protocol::CategoryKind::Regular,
},
])));
client
@@ -1182,12 +1182,12 @@ mod tests {
TileEntry {
category: "Quarter".into(),
axis: Axis::Page,
regular: true,
kind: improvise_protocol::CategoryKind::Regular,
},
TileEntry {
category: "Region".into(),
axis: Axis::Column,
regular: true,
kind: improvise_protocol::CategoryKind::Regular,
},
])));
let tiles = client.model().tiles;
@@ -901,6 +901,73 @@ fn tile_select_opens_on_a_regular_category_on_both_ends() {
}
}
/// Records mode: `_Index` holds Row, so a "first tile on Row" rule would cycle
/// it and drop the user out of records mode (improvise-52od, clients.md).
#[test]
fn in_records_mode_tile_select_opens_on_a_regular_category_on_both_ends() {
let (mut app, mut twin) = records_pair();
let mut client = furnished_client(&app, (8, 8));
let axis_of = |app: &App, cat: &str| app.view().axis_of(cat);
assert!(
app.view_state.mode.is_records(),
"the fixture must be in records mode"
);
assert_eq!(axis_of(&app, "_Index"), Axis::Row);
let region_before = axis_of(&app, "Region");
both_ends(&mut app, &mut twin, &mut client, "T");
both_ends(&mut app, &mut twin, &mut client, "Enter");
assert_eq!(axis_of(&app, "_Index"), Axis::Row, "T Enter moved _Index");
assert_eq!(axis_of(&app, "_Dim"), Axis::Column, "T Enter moved _Dim");
assert_ne!(
axis_of(&app, "Region"),
region_before,
"Region is the tile T opens on"
);
assert_eq!(axis_of(&twin, "Region"), axis_of(&app, "Region"));
}
/// A document with no regular category: tile select must still not open on
/// `_Index` (improvise-bqm0). Why `_Measure`: `context/code/command.md`.
#[test]
fn with_no_regular_category_tile_select_opens_on_measure_on_both_ends() {
let bare = || {
let mut wb = Workbook::new("Test");
wb.add_item("_Measure", "Revenue").unwrap();
wb.active_view_mut().set_axis("_Measure", Axis::Column);
wb.model.set_cell(
improvise_command::model::cell::CellKey::new(vec![(
"_Measure".to_string(),
"Revenue".to_string(),
)]),
improvise_protocol::CellValue::Number(5.0),
);
let mut app = App::new(wb, None);
app.term_height = 2 + 8;
app.rebuild_layout();
app
};
let (mut app, mut twin) = (bare(), bare());
let mut client = furnished_client(&app, (2, 1));
let axis_of = |app: &App, cat: &str| app.view().axis_of(cat);
let index_before = axis_of(&app, "_Index");
both_ends(&mut app, &mut twin, &mut client, "T");
both_ends(&mut app, &mut twin, &mut client, "Enter");
assert_eq!(
axis_of(&app, "_Index"),
index_before,
"T Enter moved _Index"
);
assert_ne!(
axis_of(&app, "_Measure"),
Axis::Column,
"_Measure is the tile T opens on"
);
}
/// The category panel's four bindings, each of which reads the flattened tree
/// both ends share (improvise-edp): Space, `a`, Enter and `d`.
#[test]