diff --git a/ROADMAP.md b/ROADMAP.md index 070552d07..516a54f90 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and whole-row/column formatting) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-13 · **Change:** minor (conditional formatting no longer scans its whole range for rules that read no statistics) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-13 | Conditional formatting: a rule gathers range statistics only when it reads them (top/bottom, above average, duplicates, scales, bars, icons) and borrows them instead of copying them per cell, so the per-frame lookup of an expression or cell-value rule no longer visits its whole range (#1098, part 1) | | 2026-10-12 | Rows auto-fit oversized fonts: opening a file and whole-row/column formatting grow non-custom rows to their tallest font (#833) | | 2026-10-12 | Number formats: parsing a format code is linear in its length (long runs of `[`, `,` or `/` no longer take quadratic time), and an unclosed `[` is a literal when splitting sections as well as when tokenizing (#810) | | 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | @@ -193,6 +194,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | Progress log: conditional-format lookups no longer scan the range for rules without statistics (#1098) | | 2026-10-12 | minor | Rows auto-fit oversized fonts on open and whole-row/column formatting (#833); progress log row | | 2026-10-12 | minor | Progress log: multi-area row/column delete (#758) | | 2026-10-12 | trivial | Progress log: XLSX chart sheets kept on save (#727) | diff --git a/crates/engine/src/cf.rs b/crates/engine/src/cf.rs index b71ddb8ca..1b0a1fda7 100644 --- a/crates/engine/src/cf.rs +++ b/crates/engine/src/cf.rs @@ -34,7 +34,43 @@ struct RuleStats { counts: HashMap, } -fn numbers_in(sh: &Sheet, ranges: &[RangeRef]) -> (Vec, HashMap) { +/// Which statistics a rule reads. Scanning the rule's whole range for them costs O(range), so +/// rules that read none (cell value, formula, text, blanks, errors, dates) never scan. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum StatsNeed { + /// Sorted numbers, their mean and deviation (top/bottom N, above average, scales, bars, icons). + Numbers, + /// How often each displayed text occurs (duplicate / unique values). + Counts, +} + +fn stats_need(rule: &CfRule) -> Option { + match rule { + CfRule::Duplicate { .. } | CfRule::Unique { .. } => Some(StatsNeed::Counts), + CfRule::Top10 { .. } | CfRule::AboveAverage { .. } | CfRule::ColorScale { .. } | CfRule::DataBar { .. } | CfRule::IconSet { .. } => { + Some(StatsNeed::Numbers) + } + _ => None, + } +} + +impl RuleStats { + fn build(sh: &Sheet, ranges: &[RangeRef], need: StatsNeed) -> RuleStats { + let (sorted, counts) = numbers_in(sh, ranges, need); + let n = sorted.len().max(1) as f64; + let avg = sorted.iter().sum::() / n; + let std = (sorted.iter().map(|x| (x - avg).powi(2)).sum::() / n).sqrt(); + RuleStats { sorted, avg, std, counts } + } +} + +#[cfg(test)] +thread_local! { + /// Cells `numbers_in` visited on this thread (tests count the work a lookup does). + static SCANNED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +fn numbers_in(sh: &Sheet, ranges: &[RangeRef], need: StatsNeed) -> (Vec, HashMap) { let mut nums = Vec::new(); let mut counts = HashMap::new(); for r in ranges { @@ -43,12 +79,20 @@ fn numbers_in(sh: &Sheet, ranges: &[RangeRef]) -> (Vec, HashMap continue, }; for c in r.iter().take(2_000_000) { + #[cfg(test)] + SCANNED.with(|n| n.set(n.get() + 1)); let v = sh.value(c); - if let Value::Number(n) = v { - nums.push(n); - } - if !v.is_empty() { - *counts.entry(v.display().to_lowercase()).or_insert(0) += 1; + match need { + StatsNeed::Numbers => { + if let Value::Number(n) = v { + nums.push(n); + } + } + StatsNeed::Counts => { + if !v.is_empty() { + *counts.entry(v.display().to_lowercase()).or_insert(0) += 1; + } + } } } } @@ -109,19 +153,13 @@ impl CfCache { sh.cond_formats.iter().enumerate().filter(|(_, cf)| cf.ranges.iter().any(|r| r.contains(c))).collect(); rules.sort_by_key(|(_, cf)| cf.priority); let v = sh.value(c); + let no_stats = RuleStats::default(); for (idx, cf) in rules { let first = cf.ranges.first().map(|r| r.start).unwrap_or(c); - let st = self - .stats - .entry(idx) - .or_insert_with(|| { - let (sorted, counts) = numbers_in(sh, &cf.ranges); - let n = sorted.len().max(1) as f64; - let avg = sorted.iter().sum::() / n; - let std = (sorted.iter().map(|x| (x - avg).powi(2)).sum::() / n).sqrt(); - RuleStats { sorted, avg, std, counts } - }) - .clone(); + let st: &RuleStats = match stats_need(&cf.rule) { + Some(need) => &*self.stats.entry(idx).or_insert_with(|| RuleStats::build(sh, &cf.ranges, need)), + None => &no_stats, + }; let text = v.display(); let matched_style: Option<&Style> = match &cf.rule { CfRule::CellIs { op, a, b, style } => { @@ -218,7 +256,7 @@ impl CfCache { if let Some(n) = v.as_f64() { let pts: Vec<(f64, [u8; 3])> = stops .iter() - .map(|(k, col)| (threshold(wb, sheet, c, &st, k), col.resolve(&wb.theme).unwrap_or([255, 255, 255]))) + .map(|(k, col)| (threshold(wb, sheet, c, st, k), col.resolve(&wb.theme).unwrap_or([255, 255, 255]))) .collect(); let rgb = match pts.as_slice() { [(a, ca), (b, cb)] => lerp(*ca, *cb, if b > a { (n - a) / (b - a) } else { 0.0 }), @@ -240,8 +278,8 @@ impl CfCache { } CfRule::DataBar { min, max, color, gradient, show_value } => { if let Some(n) = v.as_f64() { - let lo = threshold(wb, sheet, c, &st, min); - let hi = threshold(wb, sheet, c, &st, max); + let lo = threshold(wb, sheet, c, st, min); + let hi = threshold(wb, sheet, c, st, max); let lo = if lo > 0.0 { 0.0 } else { lo }; let frac = if hi > lo { ((n - lo) / (hi - lo)).clamp(0.0, 1.0) } else { 1.0 }; if out.bar.is_none() { @@ -270,7 +308,7 @@ impl CfCache { defaults.get(1..).unwrap_or_default() } }; - let cuts: Vec = lower.iter().map(|t| threshold(wb, sheet, c, &st, t)).collect(); + let cuts: Vec = lower.iter().map(|t| threshold(wb, sheet, c, st, t)).collect(); let mut idx = cuts.iter().filter(|cut| n >= **cut).count(); if *reverse { idx = k - 1 - idx.min(k - 1); @@ -397,6 +435,105 @@ mod tests { assert_eq!([look("A1"), look("A2"), look("A4")].map(border), [true, false, false]); } + /// The statistics as every rule used to get them: one pass collecting both parts. + fn all_stats_before(sh: &Sheet, ranges: &[RangeRef]) -> RuleStats { + let mut nums = Vec::new(); + let mut counts = HashMap::new(); + for r in ranges { + let Some(r) = sh.used_range().and_then(|u| r.intersection(&u)) else { continue }; + for c in r.iter() { + let v = sh.value(c); + if let Value::Number(n) = v { + nums.push(n); + } + if !v.is_empty() { + *counts.entry(v.display().to_lowercase()).or_insert(0) += 1; + } + } + } + nums.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal)); + let n = nums.len().max(1) as f64; + let avg = nums.iter().sum::() / n; + let std = (nums.iter().map(|x| (x - avg).powi(2)).sum::() / n).sqrt(); + RuleStats { sorted: nums, avg, std, counts } + } + + #[test] + fn needed_statistics_equal_the_full_ones() { + let mut s = crate::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[3, "x"], [7, "X"], [3, null], ["=NA()", 2.5], [" ", 9], [-1, "y"]]})).unwrap(); + let d = s.doc().unwrap(); + let sh = d.wb.sheet(0).unwrap(); + for ranges in ["A1:B6", "A1:A3", "B2:B9", "D1:E4", "A1:A2"].map(|r| vec![RangeRef::parse(r).unwrap()]) { + let full = all_stats_before(sh, &ranges); + let numbers = RuleStats::build(sh, &ranges, StatsNeed::Numbers); + assert_eq!((numbers.sorted, numbers.avg.to_bits(), numbers.std.to_bits()), (full.sorted, full.avg.to_bits(), full.std.to_bits())); + assert_eq!(RuleStats::build(sh, &ranges, StatsNeed::Counts).counts, full.counts); + } + } + + #[test] + fn rules_that_read_statistics_still_match() { + let mut s = crate::Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[1, "a"], [2, "A"], [3, "b"], [10, "c"], [2, null]]})).unwrap(); + let rule = |range: &str, r: serde_json::Value| json!({"range": range, "rule": r}); + s.execute("home.conditionalFormat", rule("B1:B5", json!({"type": "duplicate", "preset": "lightRedFill"}))).unwrap(); + s.execute("home.conditionalFormat", rule("A1:A5", json!({"type": "top10", "rank": 1, "preset": "redBorder"}))).unwrap(); + s.execute("home.conditionalFormat", rule("A1:A5", json!({"type": "colorScale"}))).unwrap(); + let d = s.doc().unwrap(); + let mut cache = CfCache::new(); + let mut look = |a: &str| cache.look(&d.wb, 0, CellRef::parse(a).unwrap()); + let fill = |l: Option| l.and_then(|l| l.style).is_some_and(|st| st.fill.pattern == PatternType::Solid); + // Duplicates are case-insensitive texts: "a" and "A". + assert_eq!(["B1", "B2", "B3", "B4", "B5"].map(|a| fill(look(a))), [true, true, false, false, false]); + // The top value gets the border, the scale runs from the minimum to the maximum. + assert!(look("A4").and_then(|l| l.style).is_some_and(|st| !st.borders.top.is_none())); + assert!(look("A1").and_then(|l| l.style).is_none_or(|st| st.borders.top.is_none())); + assert_eq!(look("A1").and_then(|l| l.scale_fill), Some([0xF8, 0x69, 0x6B])); + assert_eq!(look("A4").and_then(|l| l.scale_fill), Some([0x63, 0xBE, 0x7B])); + } + + /// Looking cells up does work in proportion to the cells asked for, not the rule's range: the + /// statistics are only gathered for rules that read them, and once per rule. + #[test] + fn lookups_do_not_scan_the_range_for_rules_without_statistics() { + let mut s = crate::Session::new(); + s.new_workbook(); + let rows: Vec = (1..=10_000).map(|i| json!([i, i % 7])).collect(); + s.execute("range.setValues", json!({"range": "A1", "values": rows})).unwrap(); + s.execute( + "home.conditionalFormat", + json!({"range": "A1:B10000", "rule": {"type": "expression", "formula": "=MOD(ROW(),2)=0", "preset": "lightRedFill"}}), + ) + .unwrap(); + s.execute( + "home.conditionalFormat", + json!({"range": "A1:B10000", "rule": {"type": "cellIs", "operator": "greater", "value": "5", "preset": "redBorder"}}), + ) + .unwrap(); + s.execute("home.conditionalFormat", json!({"range": "A1:B10000", "rule": {"type": "noBlanks", "preset": "redBorder"}})).unwrap(); + let d = s.doc().unwrap(); + let mut cache = CfCache::new(); + SCANNED.with(|n| n.set(0)); + for r in 1..=50 { + for col in ["A", "B"] { + cache.look(&d.wb, 0, CellRef::parse(&format!("{col}{r}")).unwrap()); + } + } + assert_eq!(SCANNED.with(|n| n.get()), 0); + // A rule that does read them scans its range once, however many cells are looked up. + s.execute("home.conditionalFormat", json!({"range": "A1:A10000", "rule": {"type": "top10", "preset": "redBorder"}})).unwrap(); + let d = s.doc().unwrap(); + let mut cache = CfCache::new(); + SCANNED.with(|n| n.set(0)); + for r in 1..=50 { + cache.look(&d.wb, 0, CellRef::parse(&format!("A{r}")).unwrap()); + } + assert_eq!(SCANNED.with(|n| n.get()), 10_000); + } + #[test] fn cell_value_operator_names() { let mut s = crate::Session::new();