From ae54c84bc90cda1032df8b7af03d6c82b09f9d0a Mon Sep 17 00:00:00 2001
From: Richard <richard1912@users.noreply.github.com>
Date: Tue, 13 Oct 2026 00:10:54 +1100
Subject: [PATCH] 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
---
 crates/engine/src/cmd/data.rs | 14 +++++++-------
 crates/engine/src/tests.rs    | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs
index 0effb86..f9b831c 100644
--- a/crates/engine/src/cmd/data.rs
+++ b/crates/engine/src/cmd/data.rs
@@ -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
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index 2d12822..8c8b08f 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -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() {