c4bf74608f
group_runs coalesced adjacent positions by group NAME, so two runs under two different headers sharing a name read as one and both front ends drew a single span across the boundary. THE TICKET'S PROPOSED FIX DOES NOT WORK, and that is worth more than the fix. h0iv suggested comparing the (cat, name) pair, FrameGroup carrying cat for exactly that reason. But its own reachability case is ONE category -- Month grouped into Q1 nested under Region, giving North/Q1 and South/Q1 adjacent -- so both headers are (Month, Q1), equal as values, and the pair separates nothing. A test pins that so the next reader does not re-propose it. What does separate them is that they are two distinct FrameEntry::Group entries, which groups() already borrows from self.entries: std::ptr::eq on the header is the literal statement of "the same header opened both". THE PRODUCT QUESTION, decided here and reversible in one comparison. Whether two adjacent same-named runs are one span a user wants spanned is a matter of taste, which is why #77 filed rather than fixed. What settled it is not the span but the indicator: the existing wasm-client test merged Month/Q1 (expanded) with Fiscal/Q1 (COLLAPSED) and its own message said the merged span is drawn with the first one's indicator -- a collapsed group rendered as expanded, with no affordance saying otherwise. A span boundary is taste; a wrong collapse glyph is a wrong fact. That test's justification was also spent: it said the rule must be name-only "because that is what src/ui/grid.rs uses", and since improvise-8qlr grid.rs calls group_run_heads. There is no second copy left to agree with, which is the general point -- a consolidation's frozen behaviour reads as decided afterwards, and nothing says it was frozen only for the duration of the move. The agreement fixture gains the case it could not see: two_level_rows minus the ungrouped Sep, so the two Q1 runs abut, end to end through a real GridLayout and both renderers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
480 lines
17 KiB
Rust
480 lines
17 KiB
Rust
//! The two front ends draw the same grid frame (improvise-anr9/s1xw). The
|
||
//! shipped divergence and the injected faults: `context/test-evidence/tui.md`.
|
||
|
||
use improvise::model::cell::{CellKey, CellValue};
|
||
use improvise::ui::app::{App, AppMode, ViewCursor};
|
||
use improvise::ui::grid::GridWidget;
|
||
use improvise::view::{Axis, GridLayout};
|
||
use improvise::workbook::Workbook;
|
||
use improvise_command::ui::project;
|
||
use improvise_protocol::{FrameEntry, GridCache, Snapshot, ViewState};
|
||
use improvise_wasm_client::render::{RenderModel, RenderRow};
|
||
use ratatui::{buffer::Buffer, layout::Rect, widgets::Widget};
|
||
|
||
fn coord(pairs: &[(&str, &str)]) -> CellKey {
|
||
CellKey::new(
|
||
pairs
|
||
.iter()
|
||
.map(|(c, i)| (c.to_string(), i.to_string()))
|
||
.collect(),
|
||
)
|
||
}
|
||
|
||
/// Both axes multi-level and grouped. Its two deliberate asymmetries — Carol
|
||
/// outside Family, Sep ungrouped — are in `context/test-evidence/tui.md`.
|
||
fn two_level_rows() -> Workbook {
|
||
let mut m = Workbook::new("Test");
|
||
m.add_category("Type").unwrap();
|
||
m.add_category("Month").unwrap();
|
||
m.add_category("Recipient").unwrap();
|
||
m.add_item("Type", "Food").unwrap();
|
||
m.add_item("Type", "Clothing").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"] {
|
||
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
|
||
}
|
||
|
||
/// The one cache both ends are driven from — built exactly as the TUI builds
|
||
/// it, over the whole grid at offsets zero (gotcha 44).
|
||
fn whole_grid(wb: &mut Workbook) -> (App, GridCache) {
|
||
let none_cats = wb.active_view().none_cats();
|
||
wb.model.recompute_formulas(&none_cats);
|
||
let app = App::new(wb.clone(), None);
|
||
let cache = project::grid_of(
|
||
&app,
|
||
project::Viewport {
|
||
row_offset: 0,
|
||
col_offset: 0,
|
||
rows: app.layout.row_count(),
|
||
cols: app.layout.col_count(),
|
||
},
|
||
);
|
||
(app, cache)
|
||
}
|
||
|
||
/// The row-header text the TUI *paints*, sliced by `AxisFrame::header_widths`
|
||
/// — the widget's own rule, never a second reading (improvise-1125).
|
||
fn tui_row_headers(wb: &Workbook, cache: &GridCache) -> Vec<Vec<String>> {
|
||
let lines = tui_lines(wb, cache);
|
||
let sep = header_rule(&lines);
|
||
let widths = cache.row_frame.header_widths();
|
||
lines[sep + 1..]
|
||
.iter()
|
||
// The body ends at the blank space below the grid, or at the rule
|
||
// above the totals strip — which is not a row of the frame.
|
||
.take_while(|l| !l.trim().is_empty() && !l.trim_end().chars().all(|c| c == '─'))
|
||
.map(|line| {
|
||
let chars: Vec<char> = line.chars().collect();
|
||
let mut at = 0usize;
|
||
widths
|
||
.iter()
|
||
.map(|w| {
|
||
let end = (at + *w as usize).min(chars.len());
|
||
let cell: String = chars[at.min(chars.len())..end].iter().collect();
|
||
at = end;
|
||
cell.trim_end().to_string()
|
||
})
|
||
.collect()
|
||
})
|
||
.collect()
|
||
}
|
||
|
||
/// One painted header line, cut by the cache's own `col_widths` for the same
|
||
/// reason. Labels are right-aligned into `width - 1`, so a trim recovers them.
|
||
fn tui_header_cells(line: &str, cache: &GridCache) -> Vec<String> {
|
||
let chars: Vec<char> = line.chars().collect();
|
||
let mut at: usize = cache
|
||
.row_frame
|
||
.header_widths()
|
||
.iter()
|
||
.map(|w| *w as usize)
|
||
.sum();
|
||
cache
|
||
.col_widths
|
||
.iter()
|
||
.map(|w| {
|
||
let end = (at + *w as usize).min(chars.len());
|
||
let cell: String = chars[at.min(chars.len())..end].iter().collect();
|
||
at = end;
|
||
cell.trim().to_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(),
|
||
seed: cache
|
||
.seed()
|
||
.into_iter()
|
||
.map(improvise_protocol::Projection::Grid)
|
||
.collect(),
|
||
});
|
||
client.model()
|
||
}
|
||
|
||
#[test]
|
||
fn the_two_front_ends_draw_the_same_row_header() {
|
||
let mut wb = two_level_rows();
|
||
let (_app, cache) = whole_grid(&mut wb);
|
||
|
||
let tui = tui_row_headers(&wb, &cache);
|
||
let browser = browser_rows(&cache);
|
||
|
||
assert_eq!(
|
||
tui.len(),
|
||
browser.len(),
|
||
"the two ends drew a different number of rows\nTUI: {tui:?}\nbrowser: {browser:?}"
|
||
);
|
||
assert!(
|
||
tui.len() > 4,
|
||
"precondition: the fixture must have group headers AND several data \
|
||
rows, or nothing here is being compared: {tui:?}"
|
||
);
|
||
|
||
for (i, (painted, decided)) in tui.iter().zip(browser.iter()).enumerate() {
|
||
// A group label lands in the first sub-column, a data row's tuple one
|
||
// per level, so the group arm compares the joined non-empty text.
|
||
let painted_text = painted.join("").trim().to_string();
|
||
match decided {
|
||
// Glyph AND name: checking 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,
|
||
"row {i}: the TUI painted {painted:?} and the browser decided \
|
||
{labels:?} — one of them re-derived the frame"
|
||
),
|
||
}
|
||
}
|
||
}
|
||
|
||
/// 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, separate because the row test drives ONE arm
|
||
/// (`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"
|
||
);
|
||
|
||
// Compared EXACTLY: a `contains` is one-directional and can see neither a
|
||
// wrongly-blanked label nor one painted under the wrong column.
|
||
for (level, labels) in model.col_levels.iter().enumerate() {
|
||
assert_eq!(
|
||
&tui_header_cells(&lines[rule - levels + level], &cache),
|
||
labels,
|
||
"level {level}: the TUI's header columns and the browser's \
|
||
{labels:?} disagree — on suppression, on placement, or on both"
|
||
);
|
||
}
|
||
|
||
// 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<(Option<&str>, usize)> = model
|
||
.col_groups
|
||
.iter()
|
||
.map(|g| (g.label.as_deref(), g.span))
|
||
.collect();
|
||
// improvise-gd83: MEMBERSHIP, not nearest-preceding. Sep is ungrouped, so
|
||
// the run ends before it and resumes under the next measure's Q1 header.
|
||
assert_eq!(
|
||
named,
|
||
vec![(Some("Q1"), 2), (None, 1), (Some("Q1"), 2), (None, 1)],
|
||
"the two Q1 runs are separated by the ungrouped Sep; a run that \
|
||
swallowed it would be `expand_category` leaking a group across its \
|
||
own boundary (see `AxisFrame::group_of`)"
|
||
);
|
||
assert!(
|
||
named.iter().any(|(label, _)| label.is_none()),
|
||
"precondition: the fixture must contain an UNGROUPED column, or this \
|
||
test cannot see a leaked group at all"
|
||
);
|
||
|
||
// improvise-8qlr: the TUI's group line, compared EXACTLY. A `contains`
|
||
// passes while the TUI labels every column, which is a different grid.
|
||
let group_line = &lines[rule - levels - 1];
|
||
let expected: Vec<String> = model
|
||
.col_groups
|
||
.iter()
|
||
.flat_map(|g| {
|
||
let head = match &g.label {
|
||
Some(name) => format!("{} {name}", indicator(g.collapsed)),
|
||
None => String::new(),
|
||
};
|
||
std::iter::once(head).chain(std::iter::repeat_n(String::new(), g.span - 1))
|
||
})
|
||
.collect();
|
||
assert_eq!(
|
||
&tui_header_cells(group_line, &cache),
|
||
&expected,
|
||
"the TUI's group line and the browser's runs disagree — one of them \
|
||
coalesces the runs itself instead of asking `group_runs`"
|
||
);
|
||
}
|
||
|
||
/// `two_level_rows` without `Sep`, so the two `Q1` runs ABUT. The existing
|
||
/// fixture is saved by the ungrouped month between them — improvise-h0iv.
|
||
fn abutting_column_groups() -> Workbook {
|
||
let mut m = two_level_rows();
|
||
m.remove_item("Month", "Sep");
|
||
m
|
||
}
|
||
|
||
/// improvise-h0iv, end to end: two `Q1` runs under two measures are two spans,
|
||
/// and both ends draw them. See `context/test-evidence/tui.md`.
|
||
#[test]
|
||
fn two_abutting_runs_of_one_group_name_are_two_spans_at_both_ends() {
|
||
let mut wb = abutting_column_groups();
|
||
let (_app, cache) = whole_grid(&mut wb);
|
||
let model = browser_model(&cache);
|
||
let lines = tui_lines(&wb, &cache);
|
||
let rule = header_rule(&lines);
|
||
let levels = model.col_levels.len();
|
||
|
||
let named: Vec<(Option<&str>, usize)> = model
|
||
.col_groups
|
||
.iter()
|
||
.map(|g| (g.label.as_deref(), g.span))
|
||
.collect();
|
||
assert!(
|
||
named.iter().all(|(label, _)| label.is_some()),
|
||
"precondition: NOTHING ungrouped may separate the runs, which is the \
|
||
whole difference from the fixture above: {named:?}"
|
||
);
|
||
assert_eq!(
|
||
named,
|
||
vec![(Some("Q1"), 2), (Some("Q1"), 2)],
|
||
"one span of four draws a single header across the measure boundary"
|
||
);
|
||
|
||
let group_line = &lines[rule - levels - 1];
|
||
let expected: Vec<String> = model
|
||
.col_groups
|
||
.iter()
|
||
.flat_map(|g| {
|
||
let head = match &g.label {
|
||
Some(name) => format!("{} {name}", indicator(g.collapsed)),
|
||
None => String::new(),
|
||
};
|
||
std::iter::once(head).chain(std::iter::repeat_n(String::new(), g.span - 1))
|
||
})
|
||
.collect();
|
||
assert_eq!(
|
||
&tui_header_cells(group_line, &cache),
|
||
&expected,
|
||
"the TUI draws one group line and the browser decided another"
|
||
);
|
||
}
|
||
|
||
/// improvise-0o6x: a group FIRST on its axis. Separate from `two_level_rows`,
|
||
/// which orders Carol first to dodge this case on purpose.
|
||
fn leading_group_rows() -> Workbook {
|
||
let mut m = Workbook::new("Test");
|
||
m.add_category("Recipient").unwrap();
|
||
m.add_item_in_group("Recipient", "Alice", "Fam").unwrap();
|
||
m.add_item_in_group("Recipient", "Bob", "Fam").unwrap();
|
||
m.add_item("Recipient", "Carol").unwrap();
|
||
m.add_item("_Measure", "Revenue").unwrap();
|
||
m.active_view_mut().set_axis("Recipient", Axis::Row);
|
||
m.active_view_mut().set_axis("_Measure", Axis::Column);
|
||
for r in ["Alice", "Bob", "Carol"] {
|
||
m.model.set_cell(
|
||
coord(&[("Recipient", r), ("_Measure", "Revenue")]),
|
||
CellValue::Number(1.0),
|
||
);
|
||
}
|
||
m
|
||
}
|
||
|
||
/// A collapsed group at position zero is otherwise a trap: its rows are hidden
|
||
/// and its header is the only affordance. context/test-evidence/tui.md.
|
||
#[test]
|
||
fn a_group_header_first_on_its_axis_reaches_both_front_ends() {
|
||
for collapse in [false, true] {
|
||
let mut wb = leading_group_rows();
|
||
if collapse {
|
||
wb.active_view_mut()
|
||
.toggle_group_collapse("Recipient", "Fam");
|
||
}
|
||
let (_app, cache) = whole_grid(&mut wb);
|
||
|
||
assert!(
|
||
matches!(cache.row_frame.entries.first(), Some(FrameEntry::Group(g)) if g.name == "Fam"),
|
||
"precondition (collapse={collapse}): the frame must LEAD with the \
|
||
header, or body_from has nothing at position zero to drop"
|
||
);
|
||
|
||
let browser = browser_rows(&cache);
|
||
assert!(
|
||
matches!(browser.first(), Some(RenderRow::Group { label, .. }) if label == "Fam"),
|
||
"the browser dropped the leading group header: {browser:?}"
|
||
);
|
||
|
||
let painted = tui_row_headers(&wb, &cache);
|
||
assert_eq!(
|
||
painted.first().map(|r| r.join("").trim().to_string()),
|
||
Some(format!("{} Fam", indicator(collapse))),
|
||
"the TUI dropped the leading group header: {painted:?}"
|
||
);
|
||
}
|
||
}
|
||
|
||
/// The bug's own words, separate because the agreement above would also hold
|
||
/// if *both* ends started slash-joining.
|
||
#[test]
|
||
fn neither_front_end_produces_a_slash_joined_label() {
|
||
let mut wb = two_level_rows();
|
||
let (_app, cache) = whole_grid(&mut wb);
|
||
|
||
for row in browser_rows(&cache) {
|
||
for label in row.labels() {
|
||
assert!(
|
||
!label.contains('/'),
|
||
"the browser drew the joined label {label:?}"
|
||
);
|
||
}
|
||
}
|
||
for painted in tui_row_headers(&wb, &cache) {
|
||
for label in painted {
|
||
assert!(
|
||
!label.contains('/'),
|
||
"the TUI painted the joined label {label:?}"
|
||
);
|
||
}
|
||
}
|
||
}
|
||
|
||
/// The collapse state crosses as a boolean, not a glyph (gotcha 26). BOTH
|
||
/// states, because a hard-wired `false` would pass the first half.
|
||
#[test]
|
||
fn the_collapse_state_crosses_as_a_fact_not_a_glyph() {
|
||
for collapse in [false, true] {
|
||
let mut wb = two_level_rows();
|
||
if collapse {
|
||
wb.active_view_mut()
|
||
.toggle_group_collapse("Recipient", "Family");
|
||
}
|
||
let (_app, cache) = whole_grid(&mut wb);
|
||
|
||
let groups: Vec<(String, bool)> = browser_rows(&cache)
|
||
.into_iter()
|
||
.filter_map(|r| match r {
|
||
RenderRow::Group { label, collapsed } => Some((label, collapsed)),
|
||
RenderRow::Data { .. } => None,
|
||
})
|
||
.collect();
|
||
assert!(
|
||
!groups.is_empty(),
|
||
"precondition (collapse={collapse}): a collapsed group still owes \
|
||
the user a header to expand again"
|
||
);
|
||
for (label, collapsed) in groups {
|
||
assert_eq!(label, "Family");
|
||
assert_eq!(collapsed, collapse, "the frame states the view's answer");
|
||
}
|
||
}
|
||
}
|