shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Key the cached Top 10 cutoff by criterion, not column (#661) 

Two Top 10 criteria on one column (possible in native JSON files) shared one
cache entry, so the last cutoff overwrote the first. Reported by @derechopiano.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ut3hr3a13szpQdQ7gbYbfc
This commit is contained in:
Richard 2026-10-13 00:10:54 +11:00
parent bf9a202222
commit ae54c84bc9
2 changed files with 29 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
crates/engine/src/cmd/data.rs
View File

@ -570,9 +570,9 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) {
let Some(af) = sh.autofilter.clone() else { return };
let r = af.range;
let mut hide = Vec::new();
// Precompute per-column top10 threshold and average once, not per row.
let mut col_stats: std::collections::HashMap<u32, (Option<f64>, f64)> = std::collections::HashMap::new();
for (off, c) in &af.criteria {
// Precompute per-criterion top10 threshold and average once, not per row.
let mut col_stats: std::collections::HashMap<usize, (Option<f64>, f64)> = std::collections::HashMap::new();
for (i, (off, c)) in af.criteria.iter().enumerate() {
if matches!(c, FilterCriterion::Top10 { .. } | FilterCriterion::AboveAverage(_)) {
let col = r.start.col + off;
let mut nums: Vec<f64> = (r.start.row + 1..=r.end.row).filter_map(|row| sh.value(CellRef::new(row, col)).as_f64()).collect();
@ -586,12 +586,12 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) {
}
_ => None,
};
col_stats.insert(*off, (thr, avg));
col_stats.insert(i, (thr, avg));
}
}
for row in r.start.row + 1..=r.end.row {
let mut visible = true;
for (off, crit) in &af.criteria {
for (i, (off, crit)) in af.criteria.iter().enumerate() {
let c = CellRef::new(row, r.start.col + off);
let v = sh.value(c);
let text = crate::display::cell_text(wb, sh, c);
@ -613,14 +613,14 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) {
None => x,
}
}
FilterCriterion::Top10 { bottom, .. } => match (v.as_f64(), col_stats.get(off)) {
FilterCriterion::Top10 { bottom, .. } => match (v.as_f64(), col_stats.get(&i)) {
(Some(n), Some((thr, _))) => {
let thr = thr.unwrap_or(n);
if *bottom { n <= thr } else { n >= thr }
}
_ => false,
},
FilterCriterion::AboveAverage(above) => match (v.as_f64(), col_stats.get(off)) {
FilterCriterion::AboveAverage(above) => match (v.as_f64(), col_stats.get(&i)) {
(Some(n), Some((_, avg))) => {
if *above {
n > *avg

22
crates/engine/src/tests.rs
View File

@ -823,6 +823,28 @@ fn filter_top10_items_and_percent() {
assert_eq!(r["hiddenRows"], 4);
}
/// Two Top 10 criteria on one column (possible in a native JSON file) each keep their own
/// cutoff: Top 3 and Bottom 4 of 1..5 keep 3 and 4.
#[test]
fn filter_top10_two_criteria_on_one_column() {
use gridcraft_model::FilterCriterion;
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [["N"], [1], [2], [3], [4], [5]]})).unwrap();
s.execute("selection.set", json!({"cell": "A1"})).unwrap();
s.execute("data.filter", json!({})).unwrap();
let sh = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb).sheet_mut(0).unwrap();
if let Some(af) = sh.autofilter.as_mut() {
af.criteria = vec![
(0, FilterCriterion::Top10 { bottom: false, percent: false, count: 3 }),
(0, FilterCriterion::Top10 { bottom: true, percent: false, count: 4 }),
];
}
s.execute("data.reapply", json!({})).unwrap();
let hidden: Vec<u32> = s.active().unwrap().wb.sheet(0).unwrap().rows.iter().filter(|(_, r)| r.hidden).map(|(k, _)| *k).collect();
// Rows are 0-based: values 1, 2 and 5 sit on rows 1, 2 and 5.
assert_eq!(hidden, vec![1, 2, 5]);
}
/// Turning the filter on with only the header row selected filters the data below it (#201).
#[test]
fn filter_on_a_selected_header_row_covers_the_data_below() {