diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index a103df0..1e4b1d0 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -582,19 +582,28 @@ 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 numbers for top10/average. - let mut col_stats: std::collections::HashMap, 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, 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 nums: Vec = (r.start.row + 1..=r.end.row).filter_map(|row| sh.value(CellRef::new(row, col)).as_f64()).collect(); + let mut nums: Vec = (r.start.row + 1..=r.end.row).filter_map(|row| sh.value(CellRef::new(row, col)).as_f64()).collect(); let avg = if nums.is_empty() { 0.0 } else { nums.iter().sum::() / nums.len() as f64 }; - col_stats.insert(*off, (nums, avg)); + let thr = match c { + FilterCriterion::Top10 { bottom, percent, count } => { + nums.sort_by(|a, b| if *bottom { a.partial_cmp(b) } else { b.partial_cmp(a) }.unwrap_or(Ordering::Equal)); + let k = if *percent { ((nums.len() as f64) * (*count as f64) / 100.0).ceil() as usize } else { *count as usize }; + let k = k.clamp(1, nums.len().max(1)); + nums.get(k - 1).copied() + } + _ => None, + }; + 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); @@ -616,18 +625,14 @@ pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) { None => x, } } - FilterCriterion::Top10 { bottom, percent, count } => match (v.as_f64(), col_stats.get(off)) { - (Some(n), Some((nums, _))) => { - let mut sorted = nums.clone(); - sorted.sort_by(|a, b| if *bottom { a.partial_cmp(b) } else { b.partial_cmp(a) }.unwrap_or(Ordering::Equal)); - let k = if *percent { ((sorted.len() as f64) * (*count as f64) / 100.0).ceil() as usize } else { *count as usize }; - let k = k.clamp(1, sorted.len().max(1)); - let thr = sorted.get(k - 1).copied().unwrap_or(n); + 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 diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3f50a7f..a199132 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -805,6 +805,46 @@ fn sort_and_filter() { assert_eq!(r["hiddenRows"], 1); } +/// Top 10 keeps the top or bottom N items or percent, ties at the threshold included. +#[test] +fn filter_top10_items_and_percent() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Score"], [5], [1], [9], [3], [9], [7], ["x"]]})).unwrap(); + s.execute("selection.set", json!({"cell": "A1"})).unwrap(); + s.execute("data.filter", json!({})).unwrap(); + // Top 2 items: 9, 9 (the "x" text row is hidden too). + let r = s.execute("data.filterBy", json!({"column": "A", "top": {"count": 2}})).unwrap(); + assert_eq!(r["hiddenRows"], 5); + // Bottom 2 items: 1, 3. + let r = s.execute("data.filterBy", json!({"column": "A", "top": {"count": 2, "bottom": true}})).unwrap(); + assert_eq!(r["hiddenRows"], 5); + // Top 50 percent of 6 numbers = 3 items; the threshold 7 keeps 9, 9, 7. + let r = s.execute("data.filterBy", json!({"column": "A", "top": {"count": 50, "percent": true}})).unwrap(); + 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 = 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() {