improvise-eccl: pool each distinct filtered context once (Codex review on #203)

An inline WHERE on the folded category rewrote every member context to the same item, so SUM(Revenue WHERE Month = Jan) on the Q1 fold counted Jan twice.

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-22 22:35:12 -07:00
parent 14acf46d77
commit 30187208d7
3 changed files with 43 additions and 1 deletions
+12
View File
@@ -1397,3 +1397,15 @@ asserts that there is no Q1 row and a total of 100. That half already held, and
is the inconsistency the ticket named: before, a hidden value appeared only while
a sibling was visible. Rival: `GridLayout::evaluate` covering every member
(`&|_, _| true`). This test alone fails, with 107.
### a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once
Found by the ChatGPT Codex review of PR #203, at head `14acf46d`. Pooling an
aggregate over a fold's member contexts ran `agg_values` once per member, and an
inline `WHERE` on the **folded** category overwrites that coordinate, as it does
at a record. Every member therefore became the same record set.
`SUM(Revenue WHERE Month = "Jan")` on the Q1 fold read **20**, Jan twice. The
test asserts 10, and equality with the Month-on-None document. The fix dedupes
the member contexts after the filter is applied, so each record set is pooled
once. The rival is the pre-fix code, which read 20. `a_folds_aggregate_formula_pools_its_members_records`
could not see this, because none of its aggregates carried a filter.
+10 -1
View File
@@ -62,7 +62,16 @@ fn eval_agg(
agg_filter: Option<&Filter>,
eval_at: &dyn Fn(&Expr, &CellKey) -> Result<f64, String>,
) -> Result<f64, String> {
let values = contexts
// A filter overrides its coordinate, so members it rewrites to one item name
// one record set: pool each distinct context once (core.md §14).
let distinct: IndexSet<CellKey> = contexts
.iter()
.map(|c| match agg_filter {
Some(f) => c.clone().with(&f.category, &f.item),
None => c.clone(),
})
.collect();
let values = distinct
.iter()
.map(|c| agg_values(model, c, target_category, inner, agg_filter, eval_at))
.collect::<Result<Vec<_>, String>>()?
+21
View File
@@ -1829,6 +1829,27 @@ mod tests {
assert_eq!(grid_value(&mut wb, "Q1", "Top"), Some(30.0));
}
/// An inline `WHERE` on the folded category makes every member context the
/// same record set — see context/test-evidence/core.md.
#[test]
fn a_folds_aggregate_filtered_on_the_folded_category_counts_each_record_once() {
let formula = ["Jans = SUM(Revenue WHERE Month = \"Jan\")"];
let mut folded = margin_book(&formula);
folded
.active_view_mut()
.toggle_group_collapse("Month", "Q1");
let fold = grid_value(&mut folded, "Q1", "Jans");
let mut none = margin_book(&formula);
none.active_view_mut().set_axis("Month", Axis::None);
assert_eq!(fold, Some(10.0), "Jan's revenue, once");
assert_eq!(
fold,
grid_value(&mut none, "", "Jans"),
"the None-axis answer"
);
}
/// Every row's value in column 0, summed: the totals strip's arithmetic.
fn column_total(wb: &Workbook) -> f64 {
let layout = GridLayout::new(&wb.model, wb.active_view());