fix(incremental): group-aware occupancy, page-aware lines, one totals condition, deterministic walk tests (improvise-bw4p.3, improvise-bw4p.4)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Claude (dbsp-probes)
2026-10-10 12:10:42 -07:00
committed by Edward Langley
parent d83a4b8330
commit 0e1dd54d83
7 changed files with 328 additions and 52 deletions
+1 -4
View File
@@ -7,7 +7,6 @@ use improvise_protocol::header_level_widths;
use crate::format::{FormatPlan, NumberFormat};
use crate::model::Model;
use crate::model::category::MEASURE_CATEGORY;
use crate::model::cell::{CellKey, CellValue};
use crate::view::{AxisEntry, GridEval, GridLayout};
@@ -50,9 +49,7 @@ pub fn grid_body(
// evaluated once (improvise-h6z1). The model does not change during it.
let eval = layout.evaluator(model);
// A column mixing measures has no total to show: `context/code/command.md`.
let measures_on_rows = layout.row_cats.iter().any(|c| c == MEASURE_CATEGORY);
let totalled = !records && !measures_on_rows && layout.row_count() > 0 && n > 0;
let totals: Vec<Option<CellValue>> = match totalled {
let totals: Vec<Option<CellValue>> = match layout.draws_totals() {
true => (0..n).map(|ci| eval.column_total(ci)).collect(),
false => Vec::new(),
};
+1 -1
View File
@@ -339,7 +339,7 @@ fn eval_is(coordinate: Option<&str>, filter: &Filter) -> f64 {
/// Whether a grid hides dimensions a cell key does not name. The emptiness of
/// the view's `Axis::None` list is the whole question — `context/code/core.md`.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum Aggregation {
/// The key names every dimension, so the cell is read as stored.
Exact,
+7
View File
@@ -991,6 +991,13 @@ impl GridLayout {
Some(CellKey::new(coords))
}
/// Whether the grid draws a total under each column: not in records mode,
/// not when a column mixes measures (`_Measure` on the rows), not when empty.
pub fn draws_totals(&self) -> bool {
let measures_on_rows = self.row_cats.iter().any(|c| c == MEASURE_CATEGORY);
!self.is_records_mode() && !measures_on_rows && self.row_count() > 0 && self.col_count() > 0
}
/// The coordinates a COLUMN answers for once its rows are aggregated away:
/// the page/filter constraints and the column's own tuple (core.md §6).
pub fn column_key(&self, col: usize) -> Option<CellKey> {
+21 -2
View File
@@ -23,6 +23,17 @@ impl Eager {
}
}
impl Eager {
/// Whether `item` names a group of `cat` that covers `held`, a stored item.
fn group_holds(&self, cat: &str, item: &str, held: Option<&str>) -> bool {
let Some(category) = self.model.category(cat) else {
return false;
};
let members = category.items_in_group(item);
!category.items.contains_key(item) && held.is_some_and(|h| members.contains(&h))
}
}
impl Derived for Eager {
fn value(&self, key: &CellKey, agg: Aggregation) -> Option<CellValue> {
self.model.evaluate_aggregated(key, agg)
@@ -50,8 +61,16 @@ impl Derived for Eager {
fn occupancy(&self, partial: &CellKey, reading: &Reading) -> usize {
let drawn = |(cat, item): &(String, String)| reading.covers(cat, item);
let stored = self.model.data.matching_cells(&partial.0);
stored.iter().filter(|(k, _)| k.0.iter().all(drawn)).count()
let named_by = |stored: &CellKey| {
partial.0.iter().all(|(cat, item)| {
let held = stored.get(cat);
held == Some(item.as_str()) || self.group_holds(cat, item, held)
})
};
let cells = self.model.data.iter_cells();
cells
.filter(|(k, _)| named_by(k) && k.0.iter().all(drawn))
.count()
}
}
+78 -23
View File
@@ -13,7 +13,7 @@ pub trait Maintained: Derived {
}
/// One question put to both backends.
#[derive(Debug, Clone, PartialEq)]
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum Probe {
Value(CellKey, Aggregation),
RawFold(CellKey),
@@ -81,14 +81,14 @@ pub const MAX_DROPPED: usize = 3;
/// The questions asked of a backend, once each, in the order first added.
#[derive(Default)]
struct Asked {
seen: HashSet<String>,
seen: HashSet<Probe>,
order: Vec<Probe>,
}
impl Asked {
fn extend(&mut self, probes: impl IntoIterator<Item = Probe>) {
for p in probes {
if self.seen.insert(format!("{p:?}")) {
if self.seen.insert(p.clone()) {
self.order.push(p);
}
}
@@ -161,46 +161,63 @@ pub fn probes(model: &Model) -> Vec<Probe> {
/// Everything each of `views` draws of `model`: cells, column totals and
/// occupancy. Why: `context/code/improvise-incremental.md`, step 4 groundwork.
pub fn drawn_probes(model: &Model, views: &[View]) -> Vec<Probe> {
/// Only views whose None categories are `none_cats` are asked (the oracle's
/// formulas are recomputed under one set); records mode is skipped.
pub fn drawn_probes(model: &Model, views: &[View], none_cats: &[String]) -> Vec<Probe> {
let sorted = |cats: &[String]| {
let mut cats = cats.to_vec();
cats.sort();
cats
};
let oracle_set = sorted(none_cats);
views
.iter()
.map(|view| (view, GridLayout::with_frozen_records(model, view, None)))
.filter(|(_, layout)| !layout.is_records_mode())
.flat_map(|(view, layout)| view_probes(view, &layout))
.filter(|(_, layout)| sorted(&layout.none_cats) == oracle_set)
.flat_map(|(view, layout)| view_probes(model, view, &layout))
.collect()
}
fn axis_partial(cats: &[String], tuple: Option<&[String]>) -> CellKey {
let named = tuple.unwrap_or_default().iter();
CellKey::new(cats.iter().cloned().zip(named.cloned()).collect())
/// A stored cell count is asked of a key only if something is stored under it.
fn occupancy_probe(model: &Model, key: CellKey, reading: &Reading) -> Option<Probe> {
model
.formula_for(&key)
.is_none()
.then(|| Probe::Occupancy(key, reading.clone()))
}
fn view_probes(view: &View, layout: &GridLayout) -> Vec<Probe> {
fn view_probes(model: &Model, view: &View, layout: &GridLayout) -> Vec<Probe> {
let reading = Reading::of_view(view, &layout.none_cats);
let cells = (0..layout.row_count())
.flat_map(|r| (0..layout.col_count()).map(move |c| (r, c)))
.filter_map(|(r, c)| layout.cell_key(r, c))
.flat_map(|key| {
let covered = Probe::Covered(key.clone(), reading.clone());
let occupied = Probe::Occupancy(key.clone(), reading.clone());
all_three(key).into_iter().chain([covered, occupied])
let occupied = occupancy_probe(model, key.clone(), &reading);
all_three(key).into_iter().chain([covered]).chain(occupied)
});
let columns = (0..layout.col_count()).flat_map(|c| {
let total = layout
.column_key(c)
.map(|key| Probe::Total(key, layout.row_cats.clone(), reading.clone()));
let line = axis_partial(&layout.col_cats, layout.col_tuple(c));
total
.into_iter()
.chain([Probe::Occupancy(line, reading.clone())])
let key = layout.column_key(c);
let total = key.clone().filter(|_| layout.draws_totals());
let total = total.map(|k| Probe::Total(k, layout.row_cats.clone(), reading.clone()));
let line = key.and_then(|k| occupancy_probe(model, k, &reading));
total.into_iter().chain(line)
});
let rows = (0..layout.row_count()).map(|r| {
let line = axis_partial(&layout.row_cats, layout.row_tuple(r));
Probe::Occupancy(line, reading.clone())
let rows = (0..layout.row_count()).flat_map(|r| {
let line = layout
.cell_key(r, 0)
.map(|k| without_all(k, &layout.col_cats));
line.and_then(|k| occupancy_probe(model, k, &reading))
});
cells.chain(columns).chain(rows).collect()
}
/// `key` less each of `cats`: a row's line is a cell without its column coordinates.
fn without_all(key: CellKey, cats: &[String]) -> CellKey {
cats.iter().fold(key, |k, cat| k.without(cat))
}
/// What an edit names: its key, asked all three ways, after the edit.
pub fn edit_probes(edit: &Edit) -> Vec<Probe> {
edit.key()
@@ -252,13 +269,13 @@ pub fn differential_in<C: Maintained>(
let mut eager = Eager::new(model.clone(), none_cats.to_vec());
let mut asked = Asked::default();
asked.extend(probes(eager.model()));
asked.extend(drawn_probes(eager.model(), views));
asked.extend(drawn_probes(eager.model(), views, none_cats));
let mut probes_run = compare(0, &eager, &candidate, &asked)?;
for (n, edit) in edits.iter().enumerate() {
eager.apply(edit);
candidate.apply(edit);
asked.extend(probes(eager.model()));
asked.extend(drawn_probes(eager.model(), views));
asked.extend(drawn_probes(eager.model(), views, none_cats));
asked.extend(edit_probes(edit));
probes_run += compare(n + 1, &eager, &candidate, &asked)?;
}
@@ -267,3 +284,41 @@ pub fn differential_in<C: Maintained>(
probes: probes_run,
})
}
#[cfg(test)]
mod tests {
use super::*;
fn old_mask_walk(key: &CellKey) -> Vec<CellKey> {
let n = key.0.len();
(0..1u64 << n)
.filter(|mask| mask.count_ones() as usize <= MAX_DROPPED)
.map(|mask| {
let kept = (0..n).filter(|i| mask >> i & 1 == 0);
CellKey::new(kept.map(|i| key.0[i].clone()).collect())
})
.collect()
}
fn key_of(n: usize) -> CellKey {
CellKey::new((0..n).map(|i| (format!("C{i:02}"), "a".into())).collect())
}
#[test]
fn sub_partials_are_the_keys_the_old_mask_walk_gave() {
for n in 0..=12 {
let mut now: Vec<_> = sub_partials(&key_of(n)).collect();
let mut was = old_mask_walk(&key_of(n));
now.sort();
was.sort();
assert_eq!(now, was, "{n} coordinates");
}
}
#[test]
fn sub_partials_of_sixty_four_coordinates_number_the_choices_of_at_most_three() {
let n = 64;
let choices = 1 + n + n * (n - 1) / 2 + n * (n - 1) * (n - 2) / 6;
assert_eq!(sub_partials(&key_of(n)).count(), choices);
}
}
+3 -2
View File
@@ -5,8 +5,9 @@ use improvise_core::view::View;
use std::collections::BTreeSet;
/// One view's way of reading: its aggregation and its hidden `(category, item)`
/// pairs, which a fold and an occupancy count both leave out.
#[derive(Debug, Clone, PartialEq, Eq)]
/// pairs. A fold over a group or a spanned category leaves them out; a read of a
/// key naming neither does not, and an occupancy count always does.
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Reading {
pub agg: Aggregation,
hidden: BTreeSet<(String, String)>,
+217 -20
View File
@@ -5,7 +5,7 @@ use improvise_core::formula::ast::AggFunc;
use improvise_core::formula::parse_formula;
use improvise_core::model::cell::{CellKey, CellValue};
use improvise_core::model::{Aggregation, Model};
use improvise_core::view::{Axis, View};
use improvise_core::view::{Axis, GridLayout, View};
use improvise_incremental::{
CellDelta, DeltaValue, Derived, Divergence, Eager, Edit, Maintained, Probe, Reading,
cell_delta, differential, differential_in, drawn_probes, probes, same_value,
@@ -220,7 +220,7 @@ fn eager_matches_the_evaluator_on_the_shipped_documents() {
direct.recompute_formulas(&none);
let mut asked = probes(eager.model());
assert!(!asked.is_empty(), "{name}: no probes");
asked.extend(drawn_probes(eager.model(), &views));
asked.extend(drawn_probes(eager.model(), &views, &none));
for probe in asked {
let want = match &probe {
Probe::Value(k, a) => direct.evaluate_aggregated(k, *a),
@@ -232,8 +232,16 @@ fn eager_matches_the_evaluator_on_the_shipped_documents() {
}
Probe::Occupancy(p, r) => {
let drawn = |(c, i): &(String, String)| r.covers(c, i);
let names = |k: &CellKey| {
p.0.iter().all(|(c, i)| {
let held = k.get(c);
let members = direct.category(c).map(|x| x.items_in_group(i));
held == Some(i.as_str())
|| held.is_some_and(|h| members.is_some_and(|m| m.contains(&h)))
})
};
let stored = direct.data.iter_cells().map(|(k, _)| k);
let n = stored.filter(|k| k.matches_partial(&p.0) && k.0.iter().all(drawn));
let n = stored.filter(|k| names(k) && k.0.iter().all(drawn));
Some(CellValue::Number(n.count() as f64))
}
};
@@ -758,6 +766,13 @@ fn wide_workbook() -> improvise_core::workbook::Workbook {
wb
}
/// The None categories of the active view, virtual ones included: the set an
/// `Eager` standing for that view is built with.
fn none_of(wb: &improvise_core::workbook::Workbook) -> Vec<String> {
let none = wb.active_view().categories_on(Axis::None);
none.into_iter().map(String::from).collect()
}
fn wide_cell() -> CellKey {
CellKey::new(vec![
("C0".into(), "a".into()),
@@ -801,7 +816,7 @@ impl Maintained for WrongAt {
#[test]
fn the_harness_checks_a_cell_a_view_draws_that_drops_four_coordinates() {
let wb = wide_workbook();
let none: Vec<String> = ["C1", "C2", "C3", "C4"].map(String::from).to_vec();
let none = none_of(&wb);
let rival = WrongAt(Eager::new(wb.model.clone(), none.clone()), wide_cell());
let views: Vec<_> = wb.views.values().cloned().collect();
let d = differential_in(&wb.model, &views, &none, rival, &[]).unwrap_err();
@@ -888,16 +903,52 @@ impl Maintained for Blind {
fn run_blind(fault: Option<Blindness>) -> Result<improvise_incremental::Report, Divergence> {
let wb = hidden_workbook();
let none = vec!["_Measure".to_string()];
let edit = Edit::SetCell(key("West", "A", "Revenue"), CellValue::Number(6.0));
run_blind_on(&wb, fault, edit)
}
fn run_blind_on(
wb: &improvise_core::workbook::Workbook,
fault: Option<Blindness>,
edit: Edit,
) -> Result<improvise_incremental::Report, Divergence> {
let none = none_of(wb);
let eager = Eager::new(wb.model.clone(), none.clone());
let views: Vec<_> = wb.views.values().cloned().collect();
let edit = Edit::SetCell(key("West", "A", "Revenue"), CellValue::Number(6.0));
match fault {
Some(f) => differential_in(&wb.model, &views, &none, Blind(eager, f), &[edit]),
None => differential_in(&wb.model, &views, &none, eager, &[edit]),
}
}
/// Nothing is hidden and Margin is a ratio: the pooled total of Margin is
/// Revenue over Cost summed across the rows, which no single row's fold gives.
fn ratio_workbook() -> improvise_core::workbook::Workbook {
let mut wb = improvise_core::workbook::Workbook::new("ratio");
for c in ["Region", "Product"] {
wb.add_category(c).unwrap();
}
wb.add_item("Region", "East").unwrap();
wb.add_item("Region", "West").unwrap();
wb.add_item("Product", "A").unwrap();
for m in ["Revenue", "Cost"] {
wb.add_item("_Measure", m).unwrap();
}
for (r, rev, cost) in [("East", 10.0, 4.0), ("West", 5.0, 2.0)] {
wb.model
.set_cell(key(r, "A", "Revenue"), CellValue::Number(rev));
wb.model
.set_cell(key(r, "A", "Cost"), CellValue::Number(cost));
}
wb.model
.add_formula(parse_formula("Margin = Revenue / Cost", "_Measure").unwrap());
let view = wb.active_view_mut();
view.set_axis("Region", Axis::Row);
view.set_axis("_Measure", Axis::Column);
view.set_axis("Product", Axis::None);
wb
}
#[test]
fn the_harness_passes_a_view_reader_that_is_right() {
run_blind(None).unwrap();
@@ -918,8 +969,18 @@ fn the_harness_catches_a_total_that_counts_a_hidden_item() {
#[test]
fn the_harness_catches_a_total_that_does_not_span_its_rows() {
let d = run_blind(Some(Blindness::TotalsDoNotSpan)).unwrap_err();
let edit = Edit::SetCell(key("East", "A", "Cost"), CellValue::Number(5.0));
let run = |fault| run_blind_on(&ratio_workbook(), fault, edit.clone());
run(None).unwrap();
let d = run(Some(Blindness::TotalsDoNotSpan)).unwrap_err();
assert!(matches!(*d.probe, Probe::Total(..)), "{d}");
assert_eq!(measure_of(&d.probe), Some("Margin"), "{d}");
let pooled = 15.0 / 6.0;
assert_eq!(
(d.step, d.eager.clone(), d.candidate.clone()),
(0, num(pooled), None),
"{d}"
);
}
#[test]
@@ -932,7 +993,7 @@ fn the_harness_catches_an_occupancy_that_counts_a_hidden_item() {
fn a_view_is_drawn_as_cells_totals_and_occupancy() {
let wb = hidden_workbook();
let views: Vec<_> = wb.views.values().cloned().collect();
let drawn = drawn_probes(&wb.model, &views);
let drawn = drawn_probes(&wb.model, &views, &none_of(&wb));
let kinds = [
drawn.iter().any(|p| matches!(p, Probe::Covered(..))),
drawn.iter().any(|p| matches!(p, Probe::Total(..))),
@@ -941,19 +1002,155 @@ fn a_view_is_drawn_as_cells_totals_and_occupancy() {
assert_eq!(kinds, [true; 3]);
}
fn coastal_line() -> CellKey {
CellKey::new(vec![("Region".into(), "Coastal".into())])
}
#[test]
fn probing_a_key_of_twenty_six_coordinates_is_not_exponential() {
let coords = (0..26)
.map(|i| (format!("C{i:02}"), "a".to_string()))
.collect();
let mut m = Model::new("deep");
m.set_cell(CellKey::new(coords), CellValue::Number(1.0));
let started = std::time::Instant::now();
let n = probes(&m).len();
assert!(n > 0);
fn a_collapsed_groups_occupancy_counts_its_visible_members() {
let wb = hidden_workbook();
let eager = Eager::new(wb.model.clone(), none_of(&wb));
let reading = Reading::of_view(wb.active_view(), &none_of(&wb));
assert_eq!(eager.occupancy(&coastal_line(), &reading), 2);
let east = CellKey::new(vec![("Region".into(), "East".into())]);
assert_eq!(eager.occupancy(&east, &reading), 0, "East is hidden");
}
fn drawn_of(wb: &improvise_core::workbook::Workbook) -> Vec<Probe> {
let views: Vec<_> = wb.views.values().cloned().collect();
drawn_probes(&wb.model, &views, &none_of(wb))
}
fn occupancy_keys(probes: &[Probe]) -> Vec<CellKey> {
let keyed = |p: &Probe| match p {
Probe::Occupancy(k, _) => Some(k.clone()),
_ => None,
};
probes.iter().filter_map(keyed).collect()
}
fn line(coords: &[(&str, &str)]) -> CellKey {
CellKey::new(
coords
.iter()
.map(|(c, i)| (c.to_string(), i.to_string()))
.collect(),
)
}
/// What the grid's own evaluator reads for every cell and total each view draws.
#[test]
fn eager_reads_what_the_grid_evaluator_reads() {
let mut compared = 0;
for (name, model, none, views) in corpus() {
let eager = Eager::new(model, none.clone());
for view in &views {
let layout = GridLayout::with_frozen_records(eager.model(), view, None);
let mut sorted = layout.none_cats.clone();
sorted.sort();
let mut mine = none.clone();
mine.sort();
if layout.is_records_mode() || sorted != mine {
continue;
}
let reading = Reading::of_view(view, &layout.none_cats);
let grid = layout.evaluator(eager.model());
for (r, c) in
(0..layout.row_count()).flat_map(|r| (0..layout.col_count()).map(move |c| (r, c)))
{
let k = layout.cell_key(r, c).unwrap();
assert_eq!(eager.covered(&k, &reading), grid.cell(&k), "{name}: {k:?}");
compared += 1;
}
for c in (0..layout.col_count()).filter(|_| layout.draws_totals()) {
let k = layout.column_key(c).unwrap();
let want = grid.column_total(c);
assert_eq!(
eager.total(&k, &layout.row_cats, &reading),
want,
"{name}: {k:?}"
);
compared += 1;
}
}
}
assert!(compared > 100, "{compared}");
}
#[test]
fn a_page_selection_narrows_the_line_occupancy_it_is_asked_under() {
let mut wb = hidden_workbook();
let view = wb.active_view_mut();
view.set_axis("Region", Axis::Page);
view.set_axis("Product", Axis::Row);
view.set_page_selection("Region", "West");
let keys = occupancy_keys(&drawn_of(&wb));
assert!(
started.elapsed().as_millis() < 250,
"{:?}",
started.elapsed()
keys.contains(&line(&[("Product", "A"), ("Region", "West")])),
"{keys:?}"
);
assert!(!keys.contains(&line(&[("Product", "A")])), "{keys:?}");
}
#[test]
fn every_row_and_column_line_is_asked_its_occupancy() {
let keys = occupancy_keys(&drawn_of(&hidden_workbook()));
let rows = [
line(&[("Region", "Inland")]),
line(&[("Region", "Coastal")]),
];
assert!(rows.iter().all(|k| keys.contains(k)), "{keys:?}");
assert!(keys.contains(&line(&[("Product", "B")])), "{keys:?}");
}
#[test]
fn a_pruned_view_is_asked_only_about_the_lines_it_draws() {
let mut wb = hidden_workbook();
wb.add_item("Region", "Empty").unwrap();
let empty = line(&[("Region", "Empty")]);
assert!(
occupancy_keys(&drawn_of(&wb)).contains(&empty),
"drawn unpruned"
);
wb.active_view_mut().prune_empty = true;
assert!(
!occupancy_keys(&drawn_of(&wb)).contains(&empty),
"pruned away"
);
}
#[test]
fn a_records_view_is_not_asked_about() {
let mut wb = hidden_workbook();
let view = wb.active_view_mut();
view.set_axis("_Index", Axis::Row);
view.set_axis("_Dim", Axis::Column);
for cat in ["Region", "Product", "_Measure"] {
view.set_axis(cat, Axis::None);
}
assert!(view.is_records_layout());
assert!(drawn_of(&wb).is_empty());
}
#[test]
fn no_total_is_asked_where_the_grid_draws_none() {
let mut wb = ratio_workbook();
let totals = |wb: &improvise_core::workbook::Workbook| {
drawn_of(wb)
.iter()
.filter(|p| matches!(p, Probe::Total(..)))
.count()
};
assert!(totals(&wb) > 0, "totals are drawn here");
let view = wb.active_view_mut();
view.set_axis("_Measure", Axis::Row);
view.set_axis("Region", Axis::Column);
assert_eq!(totals(&wb), 0, "a column mixing measures has no total");
}
#[test]
fn a_view_under_another_none_set_is_not_asked() {
let wb = hidden_workbook();
let views: Vec<_> = wb.views.values().cloned().collect();
assert!(drawn_probes(&wb.model, &views, &["Region".to_string()]).is_empty());
}