aleleba/gridcraft
1
0
Fork 0
mirror of https://github.com/storytold/gridcraft.git synced 2026-10-24 10:46:50 -06:00
Code Issues Packages Projects Releases Wiki Activity

Conditional formats: statistics only for rules that read them, borrowed not cloned per cell (#1098)

Browse Source 
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
vangel76
2026-10-13 20:11:02 +02:00
co-authored by Claude Sonnet 5.5
parent 84acf6293b
commit 1539500b7f
2 changed files with 161 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ROADMAP.md
+3 -1
View File
@@ -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) |
crates/engine/src/cf.rs
+158 -21
View File
@@ -34,7 +34,43 @@ struct RuleStats {
counts: HashMap<String, usize>,
}
fn numbers_in(sh: &Sheet, ranges: &[RangeRef]) -> (Vec<f64>, HashMap<String, usize>) {
/// 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<StatsNeed> {
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::<f64>() / n;
let std = (sorted.iter().map(|x| (x - avg).powi(2)).sum::<f64>() / 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<usize> = const { std::cell::Cell::new(0) };
}
fn numbers_in(sh: &Sheet, ranges: &[RangeRef], need: StatsNeed) -> (Vec<f64>, HashMap<String, usize>) {
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<f64>, HashMap<String, usi
None => 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::<f64>() / n;
let std = (sorted.iter().map(|x| (x - avg).powi(2)).sum::<f64>() / 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<f64> = lower.iter().map(|t| threshold(wb, sheet, c, &st, t)).collect();
let cuts: Vec<f64> = 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::<f64>() / n;
let std = (nums.iter().map(|x| (x - avg).powi(2)).sum::<f64>() / 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<CfLook>| 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<serde_json::Value> = (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();