fix: four Copilot findings on the grid frame

All four were right; two more were right and are declined on the threads.

**A frame's pre-window headers were O(offset).** `axis_frame` accumulated
every group header above the window and flushed the run at the first
in-window item, so a deep scroll past many groups serialized headers that
`body_from` discards on arrival. Only the *nearest* one can ever be answered
with, so only that one is kept.

**`web/app.js` captured the row counter, not its value.** `r` is one binding
shared by every click-handler closure, so after the loop every editable cell
would have called `edit` with the last data row. The group-aware counter was
right; capturing it was not. It is a bug this PR introduced — the old code
took `r` as a `forEach` parameter, which is per-call.

**The agreement suite drove one arm.** It only read `rows`, and its fixture
had no grouped column axis, so `col_levels` and `col_groups` could regress
behind a green suite
(`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`).
The fixture now groups two of three months under `Q1` and leaves `Sep`
outside, with `_Measure` giving a second column level, and
`the_two_front_ends_draw_the_same_column_header` checks the levels against
the painted header lines and the spans against the column count. One span
per column, and a dropped level suppression, were each injected and watched
to fail.

**The group branch did not compare the label.** It asserted only that the
painted text began with an indicator, so any group name passed. It now
compares the whole painted header against the browser's glyph-plus-label.

The column test also *demonstrated* the finding this PR declines: `Q1` spans
all six columns including the two under the ungrouped `Sep`, because
`expand_category` emits a header on entering a group and no boundary on
leaving one. That is `GridLayout`'s rule and the TUI paints the same span.
It is asserted as it is, with the defect and the corrected expectation
written next to it, and stated on `AxisFrame::group_of`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

crates/improvise-command/src/ui/project.rs

@@ -237,11 +237,13 @@ pub fn grid_of(app: &App, viewport: Viewport) -> GridCache {
 /// a header occupies a row without consuming a data index (gotcha 1's
 /// cousin: the data index counts items, the visual position counts entries).
 ///
-/// - **A header above the window comes with it.** A column window opening
+/// - **One header above the window comes with it.** A column window opening
 ///   inside `Q1` must still be able to say the column is in `Q1`
-///   (`AxisFrame::group_of`), so the headers since the last in-window item
-///   are carried. `AxisFrame::body_from` then declines to *paint* them,
-///   which is the cut `GridLayout::data_row_to_visual` has always made.
+///   (`AxisFrame::group_of`), so the nearest header above the first in-window
+///   item is carried — and only that one, or the payload grows with the
+///   groups a deep scroll passed. `AxisFrame::body_from` then declines to
+///   *paint* it, which is the cut `GridLayout::data_row_to_visual` always
+///   made.
 /// - **Consecutive headers all come.** `[Q1 (collapsed), Q2, Jan]` is a real
 ///   shape: only the nearest wins `group_of`, but dropping the far one would
 ///   lose a collapsed group the user has to be able to expand again.
@@ -267,11 +269,23 @@ fn axis_frame(
             AxisEntry::GroupHeader {
                 cat_name,
                 group_name,
-            } => pending.push(FrameEntry::Group(FrameGroup {
-                cat: cat_name.clone(),
-                name: group_name.clone(),
-                collapsed: view.is_group_collapsed(cat_name, group_name),
-            })),
+            } => {
+                // Before the window opens, only the NEAREST header matters:
+                // it is the one `group_of` answers with, and `body_from`
+                // discards every header above the first in-window item
+                // anyway. Keeping the run would make the payload O(offset)
+                // in the groups above a deeply scrolled viewport, all of it
+                // discarded on arrival. `entries.is_empty()` is true exactly
+                // until the first in-window item lands.
+                if entries.is_empty() {
+                    pending.clear();
+                }
+                pending.push(FrameEntry::Group(FrameGroup {
+                    cat: cat_name.clone(),
+                    name: group_name.clone(),
+                    collapsed: view.is_group_collapsed(cat_name, group_name),
+                }));
+            }
             AxisEntry::DataItem(tuple) => {
                 let idx = data_idx;
                 data_idx += 1;

crates/improvise-protocol/src/render.rs

@@ -326,12 +326,23 @@ impl AxisFrame {
         }
     }
 
-    /// The group the data item at `idx` sits in: the nearest [`FrameEntry::Group`]
-    /// before it. `None` for an item outside any group.
+    /// The nearest [`FrameEntry::Group`] before the data item at `idx`, or
+    /// `None` when no header precedes it at all.
     ///
     /// This is `GridLayout::col_group_for` / `row_group_for`, answered off
     /// the frame instead of off the model — which is what lets a client that
     /// holds no model draw column group spans and a collapse indicator.
+    ///
+    /// **"Nearest preceding" is not "the item's group", and deliberately so:
+    /// it is what the TUI has always painted.** `expand_category` emits a
+    /// header on *entering* a group and no boundary on leaving one, so an
+    /// ungrouped item following a grouped run is answered with the run's
+    /// group — and the TUI's column-group row spans it under that label
+    /// today. Encoding membership, or an end boundary, means changing
+    /// `AxisEntry` in `improvise-core` and changing what the TUI paints;
+    /// that is a fix to the layout, not to this projection of it, and it is
+    /// filed rather than smuggled into the migration that made the two ends
+    /// agree (improvise-anr9, raised by Copilot on PR #23).
     pub fn group_of(&self, idx: usize) -> Option<&FrameGroup> {
         let visual = self.visual_of(idx)?;
         self.entries[..visual].iter().rev().find_map(|e| match e {

tests/grid_frame_agreement.rs

@@ -27,7 +27,7 @@ use improvise::view::{Axis, GridLayout};
 use improvise::workbook::Workbook;
 use improvise_command::ui::project;
 use improvise_protocol::{GridCache, Snapshot, ViewState};
-use improvise_wasm_client::render::RenderRow;
+use improvise_wasm_client::render::{RenderModel, RenderRow};
 use ratatui::{buffer::Buffer, layout::Rect, widgets::Widget};
 
 fn coord(pairs: &[(&str, &str)]) -> CellKey {
@@ -39,13 +39,17 @@ fn coord(pairs: &[(&str, &str)]) -> CellKey {
     )
 }
 
-/// Two categories on Row — the cross product the slash-join used to flatten
-/// — with two of the three `Recipient`s in a group, so the frame carries
-/// header levels, repeat suppression *and* a group with a collapse state in
-/// one fixture. `Carol` sits outside the group and *before* it, so that
-/// collapsing `Family` still leaves a row: an all-collapsed axis draws no
-/// group headers at all, in both front ends and before this change (see the
-/// hand-back), and a fixture that hit it would be testing that bug.
+/// Both axes multi-level and grouped — the cross product the slash-join used
+/// to flatten, on rows *and* columns, so the frame carries header levels,
+/// repeat suppression, group spans and a collapse state in one fixture.
+///
+/// Two deliberate asymmetries. `Carol` sits outside the `Family` group and
+/// *before* it, so collapsing `Family` still leaves a row: an all-collapsed
+/// axis draws no group headers at all, in both front ends and before this
+/// change (see the hand-back), and a fixture that hit it would be testing
+/// that bug. And `Month` has one grouped quarter and one ungrouped month, so
+/// the column runs are not uniform — a `col_groups` that emitted one span
+/// per column, or one span for the whole axis, would pass a uniform fixture.
 fn two_level_rows() -> Workbook {
     let mut m = Workbook::new("Test");
     m.add_category("Type").unwrap();
@@ -53,17 +57,31 @@ fn two_level_rows() -> Workbook {
     m.add_category("Recipient").unwrap();
     m.add_item("Type", "Food").unwrap();
     m.add_item("Type", "Clothing").unwrap();
-    m.add_item("Month", "Jan").unwrap();
+    m.add_item_in_group("Month", "Jan", "Q1").unwrap();
+    m.add_item_in_group("Month", "Feb", "Q1").unwrap();
+    m.add_item("Month", "Sep").unwrap();
     m.add_item("Recipient", "Carol").unwrap();
     m.add_item_in_group("Recipient", "Alice", "Family").unwrap();
     m.add_item_in_group("Recipient", "Bob", "Family").unwrap();
     m.active_view_mut().set_axis("Recipient", Axis::Row);
+    m.add_item("_Measure", "Revenue").unwrap();
+    m.add_item("_Measure", "Cost").unwrap();
+    m.active_view_mut().set_axis("_Measure", Axis::Column);
     for t in ["Food", "Clothing"] {
         for r in ["Carol", "Alice", "Bob"] {
-            m.model.set_cell(
-                coord(&[("Type", t), ("Month", "Jan"), ("Recipient", r)]),
-                CellValue::Number(1.0),
-            );
+            for mo in ["Jan", "Feb", "Sep"] {
+                for meas in ["Revenue", "Cost"] {
+                    m.model.set_cell(
+                        coord(&[
+                            ("Type", t),
+                            ("Month", mo),
+                            ("Recipient", r),
+                            ("_Measure", meas),
+                        ]),
+                        CellValue::Number(1.0),
+                    );
+                }
+            }
         }
     }
     m
@@ -94,42 +112,8 @@ fn whole_grid(wb: &mut Workbook) -> (App, GridCache) {
 /// widget lays the sub-columns out with (improvise-1125) — reading the
 /// widths a second way here would be the very duplication under test.
 fn tui_row_headers(wb: &Workbook, cache: &GridCache) -> Vec<Vec<String>> {
-    let area = Rect::new(0, 0, 60, 24);
-    let mut buf = Buffer::empty(area);
-    let layout = GridLayout::new(&wb.model, wb.active_view());
-    let view_name = wb.active_view.clone();
-    let buffers = std::collections::HashMap::new();
-    GridWidget::new(
-        cache,
-        ViewCursor::default(),
-        &view_name,
-        &layout,
-        &AppMode::Normal,
-        "",
-        &buffers,
-        None,
-    )
-    .render(area, &mut buf);
-
-    let width = area.width as usize;
-    let lines: Vec<String> = buf
-        .content()
-        .chunks(width)
-        // Strip the block's left and right border columns.
-        .map(|row| {
-            row[1..width - 1]
-                .iter()
-                .map(|c| c.symbol())
-                .collect::<String>()
-        })
-        .collect();
-
-    // The body starts after the one line that is nothing but the header rule.
-    let sep = lines
-        .iter()
-        .position(|l| !l.trim().is_empty() && l.chars().all(|c| c == '─'))
-        .expect("the header separator");
-
+    let lines = tui_lines(wb, cache);
+    let sep = header_rule(&lines);
     let widths = cache.row_frame.header_widths();
     lines[sep + 1..]
         .iter()
@@ -152,9 +136,54 @@ fn tui_row_headers(wb: &Workbook, cache: &GridCache) -> Vec<Vec<String>> {
         .collect()
 }
 
+/// The index of the rule under the column headers: the one line that is
+/// nothing else.
+fn header_rule(lines: &[String]) -> usize {
+    lines
+        .iter()
+        .position(|l| !l.trim().is_empty() && l.chars().all(|c| c == '─'))
+        .expect("the header separator")
+}
+
+/// The buffer the widget paints, one string per line, borders stripped.
+fn tui_lines(wb: &Workbook, cache: &GridCache) -> Vec<String> {
+    let area = Rect::new(0, 0, 100, 30);
+    let mut buf = Buffer::empty(area);
+    let layout = GridLayout::new(&wb.model, wb.active_view());
+    let view_name = wb.active_view.clone();
+    let buffers = std::collections::HashMap::new();
+    GridWidget::new(
+        cache,
+        ViewCursor::default(),
+        &view_name,
+        &layout,
+        &AppMode::Normal,
+        "",
+        &buffers,
+        None,
+    )
+    .render(area, &mut buf);
+
+    let width = area.width as usize;
+    buf.content()
+        .chunks(width)
+        // Strip the block's left and right border columns.
+        .map(|row| {
+            row[1..width - 1]
+                .iter()
+                .map(|c| c.symbol())
+                .collect::<String>()
+        })
+        .collect()
+}
+
 /// What the browser client decides to draw, from a `Snapshot` of the same
 /// cache — the real bootstrap path, not a hand-built client state.
 fn browser_rows(cache: &GridCache) -> Vec<RenderRow> {
+    browser_model(cache).rows
+}
+
+fn browser_model(cache: &GridCache) -> RenderModel {
     let mut client = improvise_wasm_client::init();
     client.bootstrap(Snapshot {
         view: ViewState::default(),
@@ -164,7 +193,7 @@ fn browser_rows(cache: &GridCache) -> Vec<RenderRow> {
             .map(improvise_protocol::Projection::Grid)
             .collect(),
     });
-    client.model().rows
+    client.model()
 }
 
 #[test]
@@ -192,10 +221,12 @@ fn the_two_front_ends_draw_the_same_row_header() {
         // in the first sub-column, so compare the joined non-empty text.
         let painted_text = painted.join("").trim().to_string();
         match decided {
-            RenderRow::Group { label, .. } => assert!(
-                painted_text.starts_with('▼') || painted_text.starts_with('▶'),
-                "row {i}: the browser calls it the group {label:?}, the TUI \
-                 painted {painted_text:?}"
+            // The full painted string, glyph and name: a branch that checked
+            // only the glyph would pass on any group name whatsoever.
+            RenderRow::Group { label, collapsed } => assert_eq!(
+                painted_text,
+                format!("{} {label}", indicator(*collapsed)),
+                "row {i}: the two ends disagree about the group header"
             ),
             RenderRow::Data { labels, .. } => assert_eq!(
                 painted, labels,
@@ -206,6 +237,84 @@ fn the_two_front_ends_draw_the_same_row_header() {
     }
 }
 
+/// The TUI's glyph, spelled here so the comparison above is against what is
+/// really painted rather than against a prefix test.
+fn indicator(collapsed: bool) -> &'static str {
+    match collapsed {
+        true => "▶",
+        false => "▼",
+    }
+}
+
+/// The column side of the same agreement, and it is a separate test because
+/// `the_two_front_ends_draw_the_same_row_header` drives **one arm**: a
+/// `col_levels` that stopped suppressing repeats, or a `col_groups` that
+/// emitted one span per column, would sit green behind it
+/// (`doctrine(one_arm_of_a_symmetric_rule_is_evidence_about_that_arm_only)`).
+#[test]
+fn the_two_front_ends_draw_the_same_column_header() {
+    let mut wb = two_level_rows();
+    let (_app, cache) = whole_grid(&mut wb);
+    let model = browser_model(&cache);
+    let lines = tui_lines(&wb, &cache);
+    let rule = header_rule(&lines);
+
+    // The header block is the group line (when there is one) plus one line
+    // per level, ending at the rule.
+    let levels = model.col_levels.len();
+    assert_eq!(levels, 2, "precondition: Month × _Measure is two levels");
+    assert!(
+        !model.col_groups.is_empty(),
+        "precondition: Q1 groups two of the three months"
+    );
+
+    // Every label the browser decided to draw is painted by the TUI on the
+    // matching header line, and every one it blanked is absent from it.
+    for (level, labels) in model.col_levels.iter().enumerate() {
+        let line = &lines[rule - levels + level];
+        for label in labels.iter().filter(|l| !l.is_empty()) {
+            assert!(
+                line.contains(label.as_str()),
+                "level {level}: the browser drew {label:?}, the TUI painted \
+                 {line:?}"
+            );
+        }
+    }
+
+    // The spans partition the columns, and they are not all one column wide
+    // — the fixture has a grouped run of two and an ungrouped run of one.
+    let total: usize = model.col_groups.iter().map(|g| g.span).sum();
+    assert_eq!(
+        total,
+        cache.col_frame.len(),
+        "the spans must cover every column, or a colspan layout leaves a hole"
+    );
+    let named: Vec<(&str, usize)> = model
+        .col_groups
+        .iter()
+        .map(|g| (g.label.as_str(), g.span))
+        .collect();
+    assert_eq!(
+        named,
+        vec![("Q1", 6)],
+        "Q1 covers all six columns, INCLUDING the two under the ungrouped \
+         Sep — `expand_category` emits a header on entering a group and no \
+         boundary on leaving one, so the nearest-preceding walk keeps \
+         answering Q1 (see `AxisFrame::group_of`). That is `GridLayout`'s \
+         rule and the TUI paints the same span; it is reproduced here rather \
+         than corrected, because correcting it changes what the TUI draws. \
+         When it is fixed this line becomes [(\"Q1\", 4), (\"\", 2)] and this \
+         is the test that says so."
+    );
+
+    // And the group line the TUI paints names the same group.
+    let group_line = &lines[rule - levels - 1];
+    assert!(
+        group_line.contains("▼ Q1"),
+        "the TUI paints the same group above the same columns: {group_line:?}"
+    );
+}
+
 /// The joined form appears in neither, which is the bug's own words. It is a
 /// separate assertion because the agreement above would also hold if *both*
 /// ends started slash-joining.

web/app.js

@@ -300,6 +300,9 @@ function gridTable(model) {
       continue;
     }
     r += 1;
+    // `r` is one binding shared by every closure below, so a click handler
+    // that captured it would fire with the LAST row's index. Snapshot it.
+    const dataRow = r;
 
     for (const label of row.labels) {
       const header = document.createElement("th");
@@ -326,7 +329,8 @@ function gridTable(model) {
       // cell. An aggregate is a drill target and drill is not on the wire
       // yet, so it is simply not offered here.
       td.classList.toggle("editable", cell.editable);
-      if (cell.editable) td.addEventListener("click", () => edit(td, r, c, cell.edit_text));
+      if (cell.editable)
+        td.addEventListener("click", () => edit(td, dataRow, c, cell.edit_text));
     });
   }
   return table;