fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #200 from fiddlerwoaroof/collapse-fu/b5zh

Browse Source 
improvise-b5zh: an outer group's header is pruned with its rows
This commit is contained in:
Ed Langley
2026-09-22 23:34:50 -07:00
committed by GitHub
parent de7037365e 0ecb1d89a9
commit 7767cee9b6
2 changed files with 89 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/test-evidence/core.md
+29 -1
View File
@@ -939,7 +939,12 @@ the regression was live whenever `prune_empty` was on.
The fix distinguishes the two emptinesses **without consulting the `View`**,
which `GridLayout` has already dropped: a run that contained no `DataItem` at
all was never expanded, i.e. collapsed, and its header survives; a run that
contained items the predicate rejected is genuinely empty and goes. A
contained items the predicate rejected is genuinely empty and goes. **Superseded
twice** (see `an_outer_group_whose_rows_all_prune_loses_its_header`). Since
improvise-6qet a collapsed group emits its fold as a `DataItem`, so no flat run
is item-less. The one run that still was, an outer group's header followed
directly by an inner group's header, was kept by that arm when it should have
gone (improvise-b5zh). The arm is deleted. A
`GroupEnd` is kept whenever a run is open in the output, so a re-opened earlier
header is closed even though the header between them was pruned. That keeps
gotcha 1's shape — still a predicate over the data index, no `&View`
@@ -1362,6 +1367,29 @@ fold is the row `z` names the group from. Note the narrowing this implies: an
its header goes with it. lhi4 kept that header because it was the only way back to
the rows; the fold is now that way back.
### an_outer_group_whose_rows_all_prune_loses_its_header
improvise-b5zh asked whether `close_pending_run`'s "a header whose run saw no item
is collapsed, and survives" arm was dead since improvise-6qet, and proposed
deleting it. It was **not dead**, and where it ran it was wrong. With two grouped
categories on one axis (`Region` with `North[Upper]`, `South[Lower]`, outside
`Month` with `Jan`, `Feb` in `Q1`), `cross_product` puts each outer header
directly before an inner one: `[Upper, Q1, North/Jan, …]`. The inner header
closed the outer header's run as "saw no item", so `Upper` was kept even with
every North row pruned. At `1a6b877` the pruned axis was
`[Upper, GroupEnd, Lower, Q1, South/Jan, South/Feb]`, an orphan header drawn
over nothing. The fix keeps the pending headers as a stack: a header closes the
pending header of its **own** category and everything nested inside it, a kept
item flushes the whole stack, and a `GroupEnd` drops the innermost. Rival: a
header clears every pending header, which is the flat reading of "a header ends
the previous run". Only this test fails, because `Lower` is lost when `Q1`
arrives. The flat-case tests (lhi4, u6gc, the partly-pruned group) pass
unchanged under both versions, which is why none of them could see this.
One pre-existing shape is left alone and stated: two consecutive `GroupEnd`s
(an inner and an outer run ending together) keep only the first, because
`run_open` is a single flag. That was true before this change, pruned or not.
### records_steps_are_the_spine_then_every_non_index_dim_category_on_none
improvise-t0ma. Before this, `records_steps` was guarded by one incidental test: the
crates/improvise-core/src/view/layout.rs
+60 -27
View File
@@ -282,60 +282,53 @@ fn data_index_to_visual(items: &[AxisEntry], data_idx: usize) -> Option<usize> {
.nth(data_idx)
}
/// The run a header opened has ended. A header still pending saw either no
/// item at all — it is collapsed, and survives — or only rejected ones.
fn close_pending_run(
kept: &mut Vec<AxisEntry>,
pending: Option<AxisEntry>,
had_item: bool,
) -> bool {
pending
.filter(|_| !had_item)
.map(|h| kept.push(h))
.is_some()
/// A header ends the pending run of its OWN category and every run nested inside
/// it; an outer category's header is still waiting for an item (improvise-b5zh).
fn close_runs_of(pending: &mut Vec<AxisEntry>, header: &AxisEntry) {
let cat_of = |e: &AxisEntry| match e {
AxisEntry::GroupHeader { cat_name, .. } => Some(cat_name.clone()),
AxisEntry::GroupEnd | AxisEntry::DataItem { .. } => None,
};
if let Some(at) = pending.iter().position(|h| cat_of(h) == cat_of(header)) {
pending.truncate(at);
}
}
/// Drop the data items `keep` rejects, keeping a header only if an item under
/// it survives or the group is collapsed. A predicate — why: core.md §6.
/// it survives. A predicate — why: core.md §6.
fn prune_axis_items(items: &mut Vec<AxisEntry>, keep: impl Fn(usize) -> bool) {
let mut kept: Vec<AxisEntry> = Vec::new();
let mut pending_header: Option<AxisEntry> = None;
let mut run_had_item = false;
// Headers no kept item has reached yet, outermost first.
let mut pending: Vec<AxisEntry> = Vec::new();
let mut run_open = false;
let mut data_idx = 0usize;
for entry in items.drain(..) {
match &entry {
// A header ends the previous run: another group's header is where
// `expand_category` stops emitting a `GroupEnd` (improvise-gd83).
// Another group's header is where `expand_category` stops emitting a
// `GroupEnd` (improvise-gd83), so it closes that group's run.
AxisEntry::GroupHeader { .. } => {
run_open |= close_pending_run(&mut kept, pending_header.take(), run_had_item);
pending_header = Some(entry);
run_had_item = false;
close_runs_of(&mut pending, &entry);
pending.push(entry);
}
// Kept whenever a run is open in `kept`, even where the header it
// closed was pruned: an earlier one is then re-opened (u6gc).
AxisEntry::GroupEnd => {
run_open |= close_pending_run(&mut kept, pending_header.take(), run_had_item);
run_had_item = false;
pending.pop();
if run_open {
kept.push(entry);
run_open = false;
}
}
AxisEntry::DataItem { .. } => {
run_had_item = true;
if keep(data_idx) {
if let Some(h) = pending_header.take() {
kept.push(h);
run_open = true;
}
run_open |= !pending.is_empty();
kept.append(&mut pending);
kept.push(entry);
}
data_idx += 1;
}
}
}
close_pending_run(&mut kept, pending_header.take(), run_had_item);
*items = kept;
}
@@ -1697,6 +1690,46 @@ mod tests {
);
}
/// improvise-b5zh: an OUTER header is followed by an inner one, not by an item
/// — see context/test-evidence/core.md.
#[test]
fn an_outer_group_whose_rows_all_prune_loses_its_header() {
let mut wb = Workbook::new("T");
wb.add_category("Region").unwrap();
wb.add_category("Month").unwrap();
wb.add_category("Type").unwrap();
wb.add_item_in_group("Region", "North", "Upper").unwrap();
wb.add_item_in_group("Region", "South", "Lower").unwrap();
wb.add_item_in_group("Month", "Jan", "Q1").unwrap();
wb.add_item_in_group("Month", "Feb", "Q1").unwrap();
wb.add_item("Type", "Food").unwrap();
for month in ["Jan", "Feb"] {
wb.model.set_cell(
CellKey::new(vec![
("Region".into(), "South".into()),
("Month".into(), month.into()),
("Type".into(), "Food".into()),
]),
CellValue::Number(1.0),
);
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("Month", Axis::Row);
wb.active_view_mut().set_axis("Type", Axis::Column);
wb.active_view_mut().prune_empty = true;
let layout = GridLayout::with_frozen_records(&wb.model, wb.active_view(), None);
assert_eq!(layout.row_count(), 2, "only South's rows are funded");
assert!(
has_header(&layout.row_items, "Lower"),
"South's group stays"
);
assert!(
!has_header(&layout.row_items, "Upper"),
"every North row pruned, so Upper has nothing under it: {:?}",
layout.row_items
);
}
/// The commonest case and the one the other three cannot see: SOME of a
/// group's items prune. context/test-evidence/core.md.
#[test]