H refuses on every fold the host refuses on; carry the fold flag

improvise-4k1j, from PR #275's review. hidden_row_item refused on a fold
only when the NEAREST group header was collapsed. Under two row
categories that header can be the inner one's: with Region's Coast
collapsed and Product's Kit expanded, the client composed
HideItem{Region, "Coast"} and a twin host wrote a group name into the
document, where the host refuses.

The frame cannot answer this on gh/main: FrameGroup names no level and
an inner GroupEnd reads as an outer one. So FrameEntry::Item now carries
the layout's own DataItem::collapsed flag (a fold, or under one, at any
level) and AxisFrame::folds reads it, the same bit
GridLayout::row_folds_a_group reads. FrameEntry::item builds an
unfolded one, as AxisEntry::item does.

hiding_agrees_with_the_host_on_every_row_of_nested_groups presses H on
all 25 rows of four nested grouped grids, in two windows each. It was
red on the nearest-header check at rows 4 and 5, as the review found.

Also fixes the fmt gate the review found red: fmt-workspaces.sh is a
check, not a formatter, and its diff output was misread as success.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-23 18:35:38 -07:00
parent cdefeeb9b8
commit 1e49bc0df7
10 changed files with 186 additions and 38 deletions
+4 -1
View File
@@ -596,7 +596,10 @@ now decline.
the host's effect is, over `row_frame.cats[0]` and the selected tuple's `[0]`.
That is the outer category and its item, exactly as the Cmd reads
`row_cats.first()` and `items[0]`. On a fold the client refuses in the host's
own words. That is why `FOLD_REFUSAL` moved from the command layer into
own words. It knows a fold from `AxisFrame::folds`, the layout's own
per-item flag carried on the frame, never from the nearest group header: under
two row categories that header can be the inner one's (improvise-4k1j,
`context/code/protocol.md`). That is why `FOLD_REFUSAL` moved from the command layer into
`improvise-protocol`: one string, not two copies.
- **`search` (`/`) and `search-navigate` (`n`) decline out loud.** `n` scans the
whole model, and a client holds one window. `/` alone would open a box that
+16 -3
View File
@@ -923,8 +923,8 @@ type and there is nothing else to ask. Note what this cure cost that
shared derivation could not be added beside the old one.
`cats` is the categories outermost-first; `entries` interleaves
`Group(FrameGroup)` headers with `Item(Vec<String>)` label tuples exactly as
`GridLayout::row_items` interleaves them. On the row axis the TUI paints a
`Group(FrameGroup)` headers with `Item { tuple, collapsed }` label tuples exactly
as `GridLayout::row_items` interleaves them. On the row axis the TUI paints a
`Group` as its own row; on the column axis it paints the runs as spans above
the headers. **That difference is painting, and it is the only thing that
differs.**
@@ -935,7 +935,20 @@ one**. `AxisFrame::flat` leaves `cats` empty on purpose, so it is not a way to
smuggle a multi-level axis in flattened.
`FrameGroup::collapsed` is on the wire rather than left to the client because
it is a fact about the *view*, which only the server holds. There is no
it is a fact about the *view*, which only the server holds.
**`Item::collapsed` is on the wire too, and for a sharper reason** (improvise-4k1j).
It is the layout's own `AxisEntry::DataItem::collapsed`, which `GridLayout`
sets on a fold and on every item expanded beneath one, at any level. A client
cannot derive it from the headers. `FrameGroup` names no level, and an inner
`GroupEnd` looks the same as an outer one, so under two row categories the
nearest header can be the inner category's. That is how `H` came to hide a
group name as an item. Tuples cannot answer it either, because a group and an
item may share a name. `AxisFrame::folds(idx)` reads the flag, and it answers
exactly what `GridLayout::row_folds_a_group` answers, because it is the same
bit. improvise-e265 puts a level on headers and ends, which makes per-level
membership derivable. The flag still stays: a client reading the host's fact is
stronger than one re-deriving it, however correct the derivation. There is no
`FrameGroup::cat` any more (improvise-7xyx). Its doc comment promised that a
client echoes it back in `ToggleGroup`, but no client ever did. `resolve.rs`
has no `toggle-group-under-cursor` arm (`z` is on the still-needs-the-host
+24 -1
View File
@@ -659,12 +659,35 @@ row category, where first and last coincide, and there the rival
`cats.first()`→`cats.last()` passed the whole suite. Here it is red, and so is
the rival that takes `t.last()` for the tuple.
### hiding_agrees_with_the_host_on_every_row_of_nested_groups
improvise-4k1j, found reviewing PR #275. There are four fixtures, each with
Region over Product on rows: `Coast` collapsed, `Kit` collapsed, both, and
neither. Every row of each is pressed twice, once in a whole-grid window and
once in a two-row window at an offset. On each press the client's commands go to
a twin host, the real host runs `reduce_key`, and the documents and status lines
must agree. That is 25 rows and 50 presses.
Red with the first fix to `hidden_row_item`, which asked
`frame.group_of(idx)` for a collapsed header. Four presses diverged: rows 4 and 5
with `Coast` collapsed under an expanded `Kit`, in both windows. The nearest
header there is `Kit`, so the client composed
`HideItem { category: "Region", item: "Coast" }`. That writes a group name into
the document as a hidden item, where the host refuses.
Two rivals, each red here:
- `hidden_row_item` asks the nearest header again. This test alone goes red.
- The producer in `project.rs` sends `collapsed: false` for every item. This
test goes red, and so does `hiding_on_a_fold_refuses_as_the_host_does`, since
its single-category fold is also invisible without the flag.
### hiding_on_a_fold_refuses_as_the_host_does
improvise-fo0w. With the cursor on a collapsed group's fold row, the client
composes exactly `[SetStatus(host's status_msg)]` and the document is unchanged.
That status is `FOLD_REFUSAL`, which moved into `improvise-protocol` so that
both ends share one string. Rival: the fold check disabled. The client then no
both ends share one string. The fixture has one row category, so it cannot see
improvise-4k1j, where the nearest header is the wrong level. The test above can. Rival: the fold check disabled. The client then no
longer composes the refusal, and this test goes red. It is the only test that
does.
+8 -5
View File
@@ -229,12 +229,15 @@ fn axis_frame(
// Queued like a header, never clearing one: a COLLAPSED group has
// no item to carry it and reaches the frame only through `pending`.
AxisEntry::GroupEnd => pending.push(FrameEntry::GroupEnd),
AxisEntry::DataItem { tuple, .. } => {
AxisEntry::DataItem { tuple, collapsed } => {
let idx = data_idx;
data_idx += 1;
if window.contains(&idx) {
entries.append(&mut pending);
entries.push(FrameEntry::Item(tuple.clone()));
entries.push(FrameEntry::Item {
tuple: tuple.clone(),
collapsed: *collapsed,
});
}
}
}
@@ -1392,7 +1395,7 @@ mod tests {
.map(|e| match e {
FrameEntry::Group(g) => format!("[{}]", g.name),
FrameEntry::GroupEnd => "[/]".to_string(),
FrameEntry::Item(v) => v.join("+"),
FrameEntry::Item { tuple: v, .. } => v.join("+"),
})
.collect()
}
@@ -1469,7 +1472,7 @@ mod tests {
assert_eq!(group.name, "Late");
assert!(!group.collapsed);
assert!(
matches!(grid.row_frame.body_from(0), [FrameEntry::Item(_)]),
matches!(grid.row_frame.body_from(0), [FrameEntry::Item { .. }]),
"but a renderer starts at the item, not at the header above it"
);
}
@@ -2161,7 +2164,7 @@ mod tests {
row_frame: AxisFrame {
first: 0,
cats: vec!["Region".into()],
entries: vec![FrameEntry::Item(vec!["North".into()])],
entries: vec![FrameEntry::item(vec!["North".into()])],
},
col_frame: AxisFrame::flat(["A", "B"]),
col_widths: vec![8, 8],
+48 -21
View File
@@ -212,7 +212,21 @@ pub enum FrameEntry {
GroupEnd,
/// One label per header level, outermost first. Data indices — the
/// `row`/`col` of every other field on [`GridCache`] — count only these.
Item(Vec<String>),
Item {
tuple: Vec<String>,
/// The layout's own fold flag: `context/code/protocol.md` (improvise-4k1j).
collapsed: bool,
},
}
impl FrameEntry {
/// An ordinary, unfolded item, as `AxisEntry::item` is on the layout side.
pub fn item(tuple: Vec<String>) -> Self {
FrameEntry::Item {
tuple,
collapsed: false,
}
}
}
/// One axis of the grid's *frame*: header levels, label tuples and groups
@@ -237,7 +251,7 @@ impl AxisFrame {
first: 0,
entries: labels
.into_iter()
.map(|l| FrameEntry::Item(vec![l.into()]))
.map(|l| FrameEntry::item(vec![l.into()]))
.collect(),
}
}
@@ -246,7 +260,7 @@ impl AxisFrame {
/// index counts items only, exactly as `GridLayout` does.
pub fn items(&self) -> impl Iterator<Item = &Vec<String>> {
self.entries.iter().filter_map(|e| match e {
FrameEntry::Item(v) => Some(v),
FrameEntry::Item { tuple: v, .. } => Some(v),
FrameEntry::Group(_) | FrameEntry::GroupEnd => None,
})
}
@@ -275,6 +289,19 @@ impl AxisFrame {
self.items().nth(idx)
}
/// Whether the item at `idx` is a fold or lies under one, at any level: the
/// layout's own flag, as `GridLayout::row_folds_a_group` reads it (4k1j).
pub fn folds(&self, idx: usize) -> bool {
self.entries
.iter()
.filter_map(|e| match e {
FrameEntry::Item { collapsed, .. } => Some(*collapsed),
FrameEntry::Group(_) | FrameEntry::GroupEnd => None,
})
.nth(idx)
.unwrap_or(false)
}
/// How many header levels this axis draws. **At least one**: an axis
/// with no categories still draws a single, empty header line.
pub fn levels(&self) -> usize {
@@ -345,7 +372,7 @@ impl AxisFrame {
match entry {
FrameEntry::Group(g) => current = Some(g),
FrameEntry::GroupEnd => current = None,
FrameEntry::Item(_) => out.push(current),
FrameEntry::Item { .. } => out.push(current),
}
}
out
@@ -361,7 +388,7 @@ impl AxisFrame {
.find_map(|e| match e {
FrameEntry::Group(g) => Some(Some(g)),
FrameEntry::GroupEnd => Some(None),
FrameEntry::Item(_) => None,
FrameEntry::Item { .. } => None,
})
.flatten()
}
@@ -396,7 +423,7 @@ impl AxisFrame {
self.entries
.iter()
.enumerate()
.filter(|(_, e)| matches!(e, FrameEntry::Item(_)))
.filter(|(_, e)| matches!(e, FrameEntry::Item { .. }))
.map(|(vi, _)| vi)
.nth(idx)
}
@@ -1168,13 +1195,13 @@ mod tests {
first: 0,
cats: vec!["Type".into(), "Who".into()],
entries: vec![
FrameEntry::Item(vec!["A".into(), "x".into()]),
FrameEntry::Item(vec!["A".into(), "y".into()]),
FrameEntry::item(vec!["A".into(), "x".into()]),
FrameEntry::item(vec!["A".into(), "y".into()]),
FrameEntry::Group(FrameGroup {
name: "Late".into(),
collapsed: true,
}),
FrameEntry::Item(vec!["B".into(), "z".into()]),
FrameEntry::item(vec!["B".into(), "z".into()]),
],
}
}
@@ -1191,11 +1218,11 @@ mod tests {
cats: vec!["Region".into(), "Month".into()],
entries: vec![
FrameEntry::Group(q1()),
FrameEntry::Item(vec!["North".into(), "Jul".into()]),
FrameEntry::Item(vec!["North".into(), "Aug".into()]),
FrameEntry::item(vec!["North".into(), "Jul".into()]),
FrameEntry::item(vec!["North".into(), "Aug".into()]),
FrameEntry::Group(q1()),
FrameEntry::Item(vec!["South".into(), "Jul".into()]),
FrameEntry::Item(vec!["South".into(), "Aug".into()]),
FrameEntry::item(vec!["South".into(), "Jul".into()]),
FrameEntry::item(vec!["South".into(), "Aug".into()]),
],
}
}
@@ -1211,10 +1238,10 @@ mod tests {
name: "Q1".into(),
collapsed: false,
}),
FrameEntry::Item(vec!["Jan".into()]),
FrameEntry::Item(vec!["Feb".into()]),
FrameEntry::item(vec!["Jan".into()]),
FrameEntry::item(vec!["Feb".into()]),
FrameEntry::GroupEnd,
FrameEntry::Item(vec!["Sep".into()]),
FrameEntry::item(vec!["Sep".into()]),
],
}
}
@@ -1249,7 +1276,7 @@ mod tests {
collapsed: true,
}),
FrameEntry::GroupEnd,
FrameEntry::Item(vec!["Sep".into()]),
FrameEntry::item(vec!["Sep".into()]),
],
};
assert_eq!(frame.len(), 1, "the collapsed group contributes no item");
@@ -1399,7 +1426,7 @@ mod tests {
fn body_from_starts_at_the_item_not_at_the_header_above_it() {
let frame = two_level_frame();
assert!(
matches!(frame.body_from(2), [FrameEntry::Item(_)]),
matches!(frame.body_from(2), [FrameEntry::Item { .. }]),
"the trailing group header is above item 2, not inside the body"
);
assert_eq!(frame.body_from(0).len(), 4, "everything, headers included");
@@ -1412,7 +1439,7 @@ mod tests {
let frame = AxisFrame {
first: 0,
cats: vec!["A".into(), "B".into()],
entries: vec![FrameEntry::Item(vec!["Clothing".into(), "日本".into()])],
entries: vec![FrameEntry::item(vec!["Clothing".into(), "日本".into()])],
};
assert_eq!(
frame.header_widths(),
@@ -1450,7 +1477,7 @@ mod tests {
let frame = AxisFrame {
first: 0,
cats: Vec::new(),
entries: vec![FrameEntry::Item(Vec::new())],
entries: vec![FrameEntry::item(Vec::new())],
};
assert_eq!(frame.levels(), 1);
assert_eq!(frame.label(0, 0), "");
@@ -1583,7 +1610,7 @@ mod tests {
cats: vec![cat.into()],
entries: labels
.iter()
.map(|l| FrameEntry::Item(vec![(*l).into()]))
.map(|l| FrameEntry::item(vec![(*l).into()]))
.collect(),
};
GridCache::folded(&[
+1 -1
View File
@@ -780,7 +780,7 @@ fn hidden_row_item(state: &ClientState) -> Option<Vec<Command>> {
};
let idx = state.view.cursor().selected.0.checked_sub(frame.first);
// A fold's row names a group, not an item, so there is nothing to hide.
if idx.and_then(|i| frame.group_of(i)).is_some_and(|g| g.collapsed) {
if idx.is_some_and(|i| frame.folds(i)) {
return Some(vec![Command::SetStatus(crate::FOLD_REFUSAL.to_string())]);
}
let Some(item) = idx.and_then(|i| frame.item(i)).and_then(|t| t.first()) else {
+1 -1
View File
@@ -377,7 +377,7 @@ mod tests {
let rendered: serde_json::Value =
serde_json::from_str(&client.state_json().unwrap()).unwrap();
assert_eq!(
rendered["grid"]["row_frame"]["entries"][0]["Item"][0],
rendered["grid"]["row_frame"]["entries"][0]["Item"]["tuple"][0],
"North"
);
assert_eq!(rendered["view"]["buffers"]["category"], "R");
+2 -2
View File
@@ -257,7 +257,7 @@ pub fn render_model(state: &ClientState) -> RenderModel {
label: group.name.clone(),
collapsed: group.collapsed,
},
FrameEntry::Item(_) => {
FrameEntry::Item { .. } => {
let r = *data_idx;
*data_idx += 1;
RenderRow::Data {
@@ -591,7 +591,7 @@ mod tests {
}
fn item(label: &str) -> FrameEntry {
FrameEntry::Item(vec![label.into()])
FrameEntry::item(vec![label.into()])
}
fn help_page(page: usize) -> Vec<RenderHelpRow> {
+81 -2
View File
@@ -153,7 +153,11 @@ fn a_forwarded_key_reaches_the_host_unchanged() {
let ClientFrame::Key { code, modifiers } = serde_json::from_str(&wire).unwrap() else {
panic!("a forwarded key must stay a key");
};
assert_eq!(modifiers, KeyModifiers::CONTROL, "the modifier survives the wire");
assert_eq!(
modifiers,
KeyModifiers::CONTROL,
"the modifier survives the wire"
);
let mut app = host();
reduce_key(&mut app, KeyEvent::new(code, modifiers));
@@ -2453,7 +2457,10 @@ fn hiding_under_two_row_categories_hides_the_outer_item() {
reduce_key(&mut app, event);
assert_eq!(document_text(&twin), document_text(&app));
assert!(
app.model_state.workbook.active_view().is_hidden("Region", "South"),
app.model_state
.workbook
.active_view()
.is_hidden("Region", "South"),
"row 2 is (South, Widget): the outer category's item is the one hidden"
);
}
@@ -2482,6 +2489,78 @@ fn two_row_categories_host() -> App {
app
}
/// Region over Product, each with one group (`Coast`, `Kit`), either collapsed.
fn nested_groups_host(coast_collapsed: bool, kit_collapsed: bool) -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
wb.add_category("Product").unwrap();
wb.add_item("Region", "North").unwrap();
for item in ["East", "West"] {
wb.add_item_in_group("Region", item, "Coast").unwrap();
}
wb.add_item("Product", "Widget").unwrap();
for item in ["Gadget", "Gizmo"] {
wb.add_item_in_group("Product", item, "Kit").unwrap();
}
wb.add_item("_Measure", "Revenue").unwrap();
let view = wb.active_view_mut();
view.set_axis("Region", Axis::Row);
view.set_axis("Product", Axis::Row);
view.set_axis("_Measure", Axis::Column);
if coast_collapsed {
view.toggle_group_collapse("Region", "Coast");
}
if kit_collapsed {
view.toggle_group_collapse("Product", "Kit");
}
let mut app = App::new(wb, None);
app.term_height = 12 + 8;
app.rebuild_layout();
app
}
/// improvise-4k1j: `H` on every row of two-category grouped grids, whole and
/// windowed, agrees with the host. Evidence: `context/test-evidence/clients.md`.
#[test]
fn hiding_agrees_with_the_host_on_every_row_of_nested_groups() {
let mut disagreements = Vec::new();
let mut rows_pressed = 0;
for (coast, kit) in [(true, false), (false, true), (true, true), (false, false)] {
let rows = nested_groups_host(coast, kit).layout.row_count();
for row in 0..rows {
for window in [rows, 2] {
let mut app = nested_groups_host(coast, kit);
let mut twin = nested_groups_host(coast, kit);
let offset = row.saturating_sub(window - 1);
app.view_state.cursor_mut().selected = (row, 0);
app.view_state.cursor_mut().row_offset = offset;
twin.view_state = app.view_state.clone();
twin.rebuild_layout();
let mut client = furnished_client(&app, (window, 1));
let event = BrowserKey::new("H").to_key_event().unwrap();
let KeyOutcome::Local(cmds) = resolve(&client, event) else {
panic!("H must answer locally at coast={coast} kit={kit} row={row}");
};
cmds.iter().for_each(|c| client.fold(c));
apply_on(&mut twin, &cmds);
reduce_key(&mut app, event);
rows_pressed += 1;
if document_text(&twin) != document_text(&app)
|| client.view.status_msg != app.view_state.status_msg
{
disagreements.push(format!(
"coast={coast} kit={kit} row={row} window={window}: client {cmds:?}, \
host status {:?}",
app.view_state.status_msg
));
}
}
}
}
assert_eq!(rows_pressed, 50, "setup: the four grids have 25 rows");
assert!(disagreements.is_empty(), "{disagreements:#?}");
}
/// A collapsed group of Region items: its fold is the second row.
fn folded_host() -> App {
let mut wb = Workbook::new("Test");
+1 -1
View File
@@ -263,7 +263,7 @@ impl<'a> GridWidget<'a> {
);
}
}
FrameEntry::Item(_) => {
FrameEntry::Item { .. } => {
let ri = data_row_idx;
data_row_idx += 1;