From 4852e8c2966c19faeb57297a17203993ec2907f4 Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:25:49 +1200 Subject: [PATCH 1/4] fix(statistical): MIN returns #REF! for an empty array, matching MAX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `=MIN({})` answered 0 while `=MAX({})` answered #REF!, so the two reducers disagreed about whether the same empty input is an error. A plausible-looking 0 propagates silently into whatever consumes it; an error is visible. `min_fn` now returns #REF! when an argument is an empty array, the same rule `max_fn` already carries. The rule is deliberately narrower than MAX's. `max_fn` errors whenever it saw an array and found no numbers at all; MIN cannot, because the fixtures pin `=IFERROR(MIN({"a","b","c"}),"no numbers")` to the number 0. So a *populated* array holding nothing numeric still answers 0, and only a genuinely empty one is #REF!. Verified with a differential over 14,200 cases (MIN, MAX, MINA, MAXA across scalars, numeric/text/boolean/mixed/blank arrays, empty strings, numeric text, nested and 2-D shapes, leading and trailing errors, zoned instants, sparklines, and range- and name-delivered forms through a seeded resolver, as 1-, 2- and 3-argument calls). 554 cases changed; every one is a MIN call carrying an empty-array argument and every one now returns #REF!. MAX, MINA and MAXA are byte-identical. The `=MIN(SPARKLINE({1,2,3}),{})` row in bugs.tsv now passes (2055 → 2056 passing, 182 → 181 open). It is left in place: relocating it to its category TSV is a fixtures-only change and cannot ride in a commit that also touches code. closes #771 --- .../src/eval/functions/statistical/min/mod.rs | 11 +++++- .../functions/statistical/min/tests/edge.rs | 37 +++++++++++++++++++ crates/core/tests/sparkline.rs | 16 +++++--- 3 files changed, 57 insertions(+), 7 deletions(-) diff --git a/crates/core/src/eval/functions/statistical/min/mod.rs b/crates/core/src/eval/functions/statistical/min/mod.rs index f7bfbb501..f9d87e96b 100644 --- a/crates/core/src/eval/functions/statistical/min/mod.rs +++ b/crates/core/src/eval/functions/statistical/min/mod.rs @@ -3,7 +3,13 @@ use crate::types::{ErrorKind, Value}; /// `MIN(value1, ...)` — smallest numeric value in the arguments. /// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. /// Array elements: Numbers only; text/Bool → skip; errors propagate. -/// No numbers → 0.0. +/// Empty array arg → `#REF!` (matching MAX). No numbers → 0.0. +/// +/// Note the two rules are distinct: an *empty* array is `#REF!`, but a +/// non-empty array holding nothing numeric still answers 0 — the fixtures +/// pin `=IFERROR(MIN({"a","b","c"}),"no numbers")` to the number 0, so MIN +/// deliberately does not carry MAX's broader "saw an array, found no +/// numbers → #REF!" rule. pub fn min_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -34,6 +40,9 @@ pub fn min_fn(args: &[Value]) -> Value { } Value::Empty => {} Value::Array(elems) => { + if elems.is_empty() { + return Value::Error(ErrorKind::Ref); + } // Recurse into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays) so every // cell is visited. diff --git a/crates/core/src/eval/functions/statistical/min/tests/edge.rs b/crates/core/src/eval/functions/statistical/min/tests/edge.rs index a24575726..11ab9cf9a 100644 --- a/crates/core/src/eval/functions/statistical/min/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/min/tests/edge.rs @@ -14,6 +14,43 @@ fn min_text_in_args_returns_value_error() { ); } +#[test] +fn min_empty_array_is_ref_error() { + // Matches MAX and Google Sheets: an empty array argument is #REF!, + // not a silent 0. + assert_eq!(min_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); +} + +#[test] +fn min_empty_array_beside_a_number_is_ref_error() { + assert_eq!( + min_fn(&[Value::Number(1.0), Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); + assert_eq!( + min_fn(&[Value::Array(vec![]), Value::Number(1.0)]), + Value::Error(ErrorKind::Ref) + ); +} + +#[test] +fn min_non_empty_array_without_numbers_still_returns_zero() { + // Distinct from the empty-array rule: the fixtures pin + // `=IFERROR(MIN({"a","b","c"}),"no numbers")` to the number 0, so a + // populated-but-numberless array must not become #REF!. + assert_eq!( + min_fn(&[Value::Array(vec![ + Value::Text("a".to_string()), + Value::Text("b".to_string()), + ])]), + Value::Number(0.0) + ); + assert_eq!( + min_fn(&[Value::Array(vec![Value::Empty, Value::Empty])]), + Value::Number(0.0) + ); +} + #[test] fn min_negative_numbers() { assert_eq!( diff --git a/crates/core/tests/sparkline.rs b/crates/core/tests/sparkline.rs index 916423f3a..f1353e416 100644 --- a/crates/core/tests/sparkline.rs +++ b/crates/core/tests/sparkline.rs @@ -738,12 +738,16 @@ fn an_empty_array_argument_outranks_the_sparkline_skip() { eval("=MAX(SPARKLINE({1,2,3}),{\"a\"})"), Value::Number(0.0) ); - // MIN's counterpart row (`=MIN(SPARKLINE({1,2,3}),{})` → #REF!) is a known - // divergence recorded in bugs.tsv: MIN has no empty-array rule at all, so - // it answers 0 — for `=MIN({})` too, with no sparkline in sight. Fixing - // that would move MIN for inputs unrelated to this work. - assert_eq!(eval("=MIN(SPARKLINE({1,2,3}),{})"), Value::Number(0.0)); - assert_eq!(eval("=MIN({})"), Value::Number(0.0)); + // MIN now carries the same empty-array rule, so its counterpart row + // agrees with MAX's. + assert_eq!( + eval("=MIN(SPARKLINE({1,2,3}),{})"), + Value::Error(ErrorKind::Ref) + ); + assert_eq!(eval("=MIN({})"), Value::Error(ErrorKind::Ref)); + // MIN keeps answering 0 for a populated array holding nothing numeric — + // the empty-array rule is narrower than MAX's. + assert_eq!(eval("=MIN(SPARKLINE({1,2,3}),{\"a\"})"), Value::Number(0.0)); } // ── Registry surface ──────────────────────────────────────────────────────── From 097df702c71f9f6310f8a5e6b46fdf6c04a55ec3 Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:49:27 +1200 Subject: [PATCH 2/4] fix(statistical): apply the absent-vs-numberless rule to MAX, MINA and MAXA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Google Sheets was asked directly about all four reducers, and the rule is uniform: {} {"a","b"} blank range MAX #REF! 0 #REF! MIN #REF! 0 #REF! MAXA #REF! 0 — MINA #REF! 0 — So "no numbers" is two conditions, not one. An *absent* argument is #REF!; a *populated* argument that happens to hold nothing numeric is 0. A range of blank cells sides with the empty array rather than the text array — blanks are absent, not present-and-unusable. MAX collapsed both into a single `had_array && result.is_none()` check and so answered #REF! for `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})`, which are 0. It now also tracks whether an array held anything other than a blank, and errors only when it did not — leaving `=MAX()` at #REF! where it already belonged. MIN gained the blank-array half of the same rule, which its previous empty-array-only fix did not cover. MINA and MAXA needed the empty-array check alone. They reach the other answers by a different mechanism — text folds in as 0 rather than being skipped, so `=MAXA({"a","b",5,3})` is 5 and `=MAXA({"a","b",-5,-3})` is 0 — and a populated array is therefore never numberless for them. Their agreement with MAX on the two probed rows is a coincidence of values, not shared semantics, so they keep their own fold helpers. The blank-range answer for MINA and MAXA has no captured row, so both keep today's #N/A there rather than being moved to match their non-A counterparts on a guess. Verified with a differential over 16,320 cases across all four functions (scalars, numeric/text/boolean/blank/mixed arrays, empty strings, numeric text, nested and 2-D shapes, flat and nested-row blank ranges, leading and trailing errors, zoned instants, sparklines, and range- and name-delivered forms through a seeded resolver, as 1-, 2- and 3-argument calls). 1,879 cases changed: MIN 624 → #REF!, every one carrying an absent or numberless argument MAX 199 #REF! → 0, every one carrying a numberless argument, and no case whose arguments are all absent moved at all MINA 528 → #REF!, every one carrying an empty array MAXA 528 → #REF!, every one carrying an empty array No case changed in any other direction, and none changed without a triggering argument. Every captured row above is reproduced, including the two that must not move: `=MAX()` stays #REF! and `=MAX(SPARKLINE({1,2,3}),{})` stays #REF!. bugs.tsv is unchanged at 2056 passing / 181 open — the same single row that this branch already flipped, with no row regressing. The zone-aware short-circuit is untouched: `=MIN(TZDATETIME(...),{})` still returns the instant rather than #REF!, because `zoned_extreme` runs before the argument loop. No captured row covers it. --- .../src/eval/functions/statistical/max/mod.rs | 55 +++++++++++++++---- .../functions/statistical/max/tests/edge.rs | 55 +++++++++++++++++++ .../eval/functions/statistical/maxa/mod.rs | 9 +++ .../functions/statistical/maxa/tests/edge.rs | 27 ++++++++- .../src/eval/functions/statistical/min/mod.rs | 46 ++++++++++++---- .../functions/statistical/min/tests/edge.rs | 44 +++++++++++++-- .../eval/functions/statistical/mina/mod.rs | 9 +++ .../functions/statistical/mina/tests/edge.rs | 27 ++++++++- 8 files changed, 246 insertions(+), 26 deletions(-) diff --git a/crates/core/src/eval/functions/statistical/max/mod.rs b/crates/core/src/eval/functions/statistical/max/mod.rs index 6318ef65b..e771f38e3 100644 --- a/crates/core/src/eval/functions/statistical/max/mod.rs +++ b/crates/core/src/eval/functions/statistical/max/mod.rs @@ -3,7 +3,17 @@ use crate::types::{ErrorKind, Value}; /// `MAX(value1, ...)` — largest numeric value in the arguments. /// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. /// Array elements: Numbers only; text/Bool → skip; errors propagate. -/// Empty array arg → #REF!. No numbers → 0.0. +/// +/// "No numbers" is *two* rules, not one — the fixtures separate them: +/// +/// - an **absent** argument is `#REF!`: `=MAX({})` and `=MAX()`; +/// - a **populated** argument holding nothing numeric is 0: `=MAX({"a","b"})` +/// and `=MAX({TRUE,FALSE})` are both 0, even though neither text nor +/// booleans contribute a number in array context. +/// +/// So blanks read as absent while text and booleans read as +/// present-but-unusable, which is why `array_had_content` ignores `Empty` but +/// counts everything else. pub fn max_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -15,6 +25,7 @@ pub fn max_fn(args: &[Value]) -> Value { } let mut result: Option = None; let mut had_array = false; + let mut array_had_content = false; let mut skipped_sparkline = false; for arg in args { match arg { @@ -44,7 +55,12 @@ pub fn max_fn(args: &[Value]) -> Value { // Recurse into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays) so every // cell is visited. - if let Err(e) = max_array_into(elems, &mut result, &mut skipped_sparkline) { + if let Err(e) = max_array_into( + elems, + &mut result, + &mut skipped_sparkline, + &mut array_had_content, + ) { return e; } } @@ -54,14 +70,19 @@ pub fn max_fn(args: &[Value]) -> Value { } } // A skipped sparkline is not "nothing usable": the aggregate had something - // in scope, so it answers 0 rather than falling into the numberless-array - // rule below (google.tsv: `=MAX(Data!K1:K1)` is 0). Scoped to a sparkline - // so `=MAX({"a"})` and friends keep their pre-existing `#REF!`. + // in scope, so it answers 0 rather than falling into the absent-argument + // rule below. A sparkline *inside* an array now counts as content on its + // own, so this only still decides a direct sparkline argument sitting + // beside an otherwise-blank array — a shape no fixture row covers, left + // answering 0 as it always has. if skipped_sparkline && result.is_none() { return Value::Number(0.0); } - // Empty array with no numbers → Ref - if had_array && result.is_none() { + // Absent argument → #REF!. An array that held only blanks is as absent as + // `{}` is (`=MAX()` is #REF!), but one holding text + // or booleans is present-but-unusable and answers 0 (`=MAX({"a","b"})` + // and `=MAX({TRUE,FALSE})` are both 0). + if had_array && !array_had_content && result.is_none() { return Value::Error(ErrorKind::Ref); } Value::Number(result.unwrap_or(0.0)) @@ -74,21 +95,35 @@ pub fn max_fn(args: &[Value]) -> Value { /// direct argument or through a range (google.tsv: `=MAX(SPARKLINE({1,2,3}))` /// and `=MAX(Data!K1:K1)` are both 0). The flag is what distinguishes "skipped a /// sparkline" from "saw nothing usable at all", which stay different answers. +/// +/// `had_content` records whether the array held *anything* other than a +/// blank. Text and booleans do not contribute a number here, but they do make +/// the array present rather than absent, which is what separates +/// `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` (both 0) from +/// `=MAX()` (#REF!). fn max_array_into( elems: &[Value], result: &mut Option, skipped_sparkline: &mut bool, + had_content: &mut bool, ) -> Result<(), Value> { for elem in elems { match elem { Value::Number(n) => { + *had_content = true; *result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } - Value::Sparkline(_) => *skipped_sparkline = true, + Value::Sparkline(_) => { + *had_content = true; + *skipped_sparkline = true; + } Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - Value::Array(inner) => max_array_into(inner, result, skipped_sparkline)?, - _ => {} + Value::Array(inner) => { + max_array_into(inner, result, skipped_sparkline, had_content)? + } + Value::Empty => {} + _ => *had_content = true, } } Ok(()) diff --git a/crates/core/src/eval/functions/statistical/max/tests/edge.rs b/crates/core/src/eval/functions/statistical/max/tests/edge.rs index 2026974bf..29fdd29bf 100644 --- a/crates/core/src/eval/functions/statistical/max/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/max/tests/edge.rs @@ -14,6 +14,61 @@ fn max_text_in_args_returns_value_error() { ); } +#[test] +fn max_empty_array_is_ref_error() { + assert_eq!(max_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); +} + +#[test] +fn max_non_empty_array_without_numbers_returns_zero() { + // `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` are both 0: neither text nor + // booleans contribute a number in array context, but they do make the + // argument present rather than absent. + assert_eq!( + max_fn(&[Value::Array(vec![ + Value::Text("a".to_string()), + Value::Text("b".to_string()), + ])]), + Value::Number(0.0) + ); + assert_eq!( + max_fn(&[Value::Array(vec![Value::Bool(true), Value::Bool(false)])]), + Value::Number(0.0) + ); +} + +#[test] +fn max_array_of_only_blanks_is_ref_error() { + // `=MAX()` stays #REF! — this is the half the + // narrowing above must not disturb. + assert_eq!( + max_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), + Value::Error(ErrorKind::Ref) + ); + assert_eq!( + max_fn(&[Value::Array(vec![ + Value::Array(vec![Value::Empty]), + Value::Array(vec![Value::Empty]), + ])]), + Value::Error(ErrorKind::Ref) + ); +} + +#[test] +fn max_blanks_beside_content_are_not_absent() { + assert_eq!( + max_fn(&[Value::Array(vec![ + Value::Empty, + Value::Text("z".to_string()), + ])]), + Value::Number(0.0) + ); + assert_eq!( + max_fn(&[Value::Array(vec![Value::Empty, Value::Number(4.0)])]), + Value::Number(4.0) + ); +} + #[test] fn max_negative_numbers() { assert_eq!( diff --git a/crates/core/src/eval/functions/statistical/maxa/mod.rs b/crates/core/src/eval/functions/statistical/maxa/mod.rs index 9756b7f3c..43d10a279 100644 --- a/crates/core/src/eval/functions/statistical/maxa/mod.rs +++ b/crates/core/src/eval/functions/statistical/maxa/mod.rs @@ -5,6 +5,7 @@ use crate::types::{ErrorKind, Value}; /// - Booleans coerced: TRUE=1, FALSE=0. /// - Text in direct args → `#VALUE!`. /// - Empty → skip. +/// - Empty array argument → `#REF!`. /// - No args → `#N/A`. pub fn maxa_fn(args: &[Value]) -> Value { if args.is_empty() { @@ -26,6 +27,14 @@ pub fn maxa_fn(args: &[Value]) -> Value { Value::Text(_) => return Value::Error(ErrorKind::Value), Value::Empty => {} Value::Array(inner) => { + // An empty argument is #REF!, as it is for MIN and MAX + // (`=MAXA({})`). Note MAXA reaches the *other* answers by a + // different route: text folds in as 0 rather than being + // skipped, so `=MAXA({"a","b"})` is already 0 without any + // "populated but numberless" rule. + if inner.is_empty() { + return Value::Error(ErrorKind::Ref); + } // In array context: Numbers included, Bool→1/0, Text→0, Empty→skip. // Recurses into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays). diff --git a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs index 894de41d1..0bc0be92e 100644 --- a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs @@ -1,5 +1,5 @@ use super::super::maxa_fn; -use crate::types::Value; +use crate::types::{ErrorKind, Value}; #[test] fn empty_values_skipped() { @@ -37,3 +37,28 @@ fn bool_and_number_mixed() { Value::Number(10.0) ); } + +#[test] +fn maxa_empty_array_is_ref_error() { + // `=MAXA({})` is #REF!, as it is for MIN and MAX. Reached by the + // empty-argument check alone: text folds in as 0 here, so a populated + // array is never numberless in the first place. + assert_eq!(maxa_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); + assert_eq!( + maxa_fn(&[Value::Number(1.0), Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); +} + +#[test] +fn maxa_text_only_array_is_zero() { + // `=MAXA({"a","b"})` is 0 — text counts as zero rather than being + // skipped, so this needs no separate rule. + assert_eq!( + maxa_fn(&[Value::Array(vec![ + Value::Text("a".to_string()), + Value::Text("b".to_string()), + ])]), + Value::Number(0.0) + ); +} diff --git a/crates/core/src/eval/functions/statistical/min/mod.rs b/crates/core/src/eval/functions/statistical/min/mod.rs index f9d87e96b..c17b12ee1 100644 --- a/crates/core/src/eval/functions/statistical/min/mod.rs +++ b/crates/core/src/eval/functions/statistical/min/mod.rs @@ -3,13 +3,16 @@ use crate::types::{ErrorKind, Value}; /// `MIN(value1, ...)` — smallest numeric value in the arguments. /// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. /// Array elements: Numbers only; text/Bool → skip; errors propagate. -/// Empty array arg → `#REF!` (matching MAX). No numbers → 0.0. /// -/// Note the two rules are distinct: an *empty* array is `#REF!`, but a -/// non-empty array holding nothing numeric still answers 0 — the fixtures -/// pin `=IFERROR(MIN({"a","b","c"}),"no numbers")` to the number 0, so MIN -/// deliberately does not carry MAX's broader "saw an array, found no -/// numbers → #REF!" rule. +/// "No numbers" is *two* rules, not one — the fixtures separate them: +/// +/// - an **absent** argument is `#REF!`: `=MIN({})` and `=MIN()`; +/// - a **populated** argument holding nothing numeric is 0: +/// `=MIN({"a","b"})`, and `=IFERROR(MIN({"a","b","c"}),"no numbers")` is +/// pinned to the number 0. +/// +/// So blanks read as absent while text reads as present-but-unusable, which +/// is why `array_had_content` ignores `Empty` but counts everything else. pub fn min_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -20,6 +23,8 @@ pub fn min_fn(args: &[Value]) -> Value { return r; } let mut result: Option = None; + let mut had_array = false; + let mut array_had_content = false; for arg in args { match arg { Value::Number(n) => { @@ -40,13 +45,18 @@ pub fn min_fn(args: &[Value]) -> Value { } Value::Empty => {} Value::Array(elems) => { + had_array = true; + // An explicitly empty argument is fatal on the spot, even if a + // number was already in hand — matching MAX, whose + // `=MAX(SPARKLINE({1,2,3}),{})` row is #REF! despite the + // sparkline that would otherwise answer 0. if elems.is_empty() { return Value::Error(ErrorKind::Ref); } // Recurse into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays) so every // cell is visited. - if let Err(e) = min_array_into(elems, &mut result) { + if let Err(e) = min_array_into(elems, &mut result, &mut array_had_content) { return e; } } @@ -55,21 +65,37 @@ pub fn min_fn(args: &[Value]) -> Value { _ => {} } } + // Arrays that held only blanks are as absent as `{}` is: `=MIN()` is #REF!, while `=MIN({"a","b"})` is 0. + if had_array && !array_had_content && result.is_none() { + return Value::Error(ErrorKind::Ref); + } Value::Number(result.unwrap_or(0.0)) } /// Recursively fold a nested array's numbers into `result` for MIN's /// array-context rules (Bool/Text/Empty skipped, errors propagate). -fn min_array_into(elems: &[Value], result: &mut Option) -> Result<(), Value> { +/// +/// `had_content` records whether the array held *anything* other than a +/// blank. Text and booleans do not contribute a number here, but they do make +/// the array present rather than absent, which is what separates +/// `=MIN({"a","b"})` (0) from `=MIN()` (#REF!). +fn min_array_into( + elems: &[Value], + result: &mut Option, + had_content: &mut bool, +) -> Result<(), Value> { for elem in elems { match elem { Value::Number(n) => { + *had_content = true; *result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); } Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - Value::Array(inner) => min_array_into(inner, result)?, - _ => {} + Value::Array(inner) => min_array_into(inner, result, had_content)?, + Value::Empty => {} + _ => *had_content = true, } } Ok(()) diff --git a/crates/core/src/eval/functions/statistical/min/tests/edge.rs b/crates/core/src/eval/functions/statistical/min/tests/edge.rs index 11ab9cf9a..0f770fb45 100644 --- a/crates/core/src/eval/functions/statistical/min/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/min/tests/edge.rs @@ -35,9 +35,9 @@ fn min_empty_array_beside_a_number_is_ref_error() { #[test] fn min_non_empty_array_without_numbers_still_returns_zero() { - // Distinct from the empty-array rule: the fixtures pin - // `=IFERROR(MIN({"a","b","c"}),"no numbers")` to the number 0, so a - // populated-but-numberless array must not become #REF!. + // Distinct from the absent-argument rule: the fixtures pin + // `=MIN({"a","b"})` and `=IFERROR(MIN({"a","b","c"}),"no numbers")` to + // the number 0, so a populated-but-numberless array must not become #REF!. assert_eq!( min_fn(&[Value::Array(vec![ Value::Text("a".to_string()), @@ -45,12 +45,48 @@ fn min_non_empty_array_without_numbers_still_returns_zero() { ])]), Value::Number(0.0) ); + // Booleans are skipped in array context but still make the array present. assert_eq!( - min_fn(&[Value::Array(vec![Value::Empty, Value::Empty])]), + min_fn(&[Value::Array(vec![Value::Bool(true), Value::Bool(false)])]), Value::Number(0.0) ); } +#[test] +fn min_array_of_only_blanks_is_ref_error() { + // `=MIN()` is #REF!: blanks read as absent, not as + // present-but-unusable. Both the flat and the nested-row materializations + // of such a range must agree. + assert_eq!( + min_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), + Value::Error(ErrorKind::Ref) + ); + assert_eq!( + min_fn(&[Value::Array(vec![ + Value::Array(vec![Value::Empty]), + Value::Array(vec![Value::Empty]), + ])]), + Value::Error(ErrorKind::Ref) + ); +} + +#[test] +fn min_blanks_beside_content_are_not_absent() { + // One non-blank cell is enough to make the whole argument present. + assert_eq!( + min_fn(&[Value::Array(vec![ + Value::Empty, + Value::Text("z".to_string()), + ])]), + Value::Number(0.0) + ); + // And a number anywhere wins outright. + assert_eq!( + min_fn(&[Value::Array(vec![Value::Empty, Value::Number(4.0)])]), + Value::Number(4.0) + ); +} + #[test] fn min_negative_numbers() { assert_eq!( diff --git a/crates/core/src/eval/functions/statistical/mina/mod.rs b/crates/core/src/eval/functions/statistical/mina/mod.rs index 1d4ab3680..63449e413 100644 --- a/crates/core/src/eval/functions/statistical/mina/mod.rs +++ b/crates/core/src/eval/functions/statistical/mina/mod.rs @@ -5,6 +5,7 @@ use crate::types::{ErrorKind, Value}; /// - Booleans coerced: TRUE=1, FALSE=0. /// - Text in direct args → `#VALUE!`. /// - Empty → skip. +/// - Empty array argument → `#REF!`. /// - No args → `#N/A`. pub fn mina_fn(args: &[Value]) -> Value { if args.is_empty() { @@ -26,6 +27,14 @@ pub fn mina_fn(args: &[Value]) -> Value { Value::Text(_) => return Value::Error(ErrorKind::Value), Value::Empty => {} Value::Array(inner) => { + // An empty argument is #REF!, as it is for MIN and MAX + // (`=MINA({})`). Note MINA reaches the *other* answers by a + // different route: text folds in as 0 rather than being + // skipped, so `=MINA({"a","b"})` is already 0 without any + // "populated but numberless" rule. + if inner.is_empty() { + return Value::Error(ErrorKind::Ref); + } // In array context: Numbers included, Bool→1/0, Text→0, Empty→skip. // Recurses into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays). diff --git a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs index cf7c44d39..87fffa6a2 100644 --- a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs @@ -1,5 +1,5 @@ use super::super::mina_fn; -use crate::types::Value; +use crate::types::{ErrorKind, Value}; #[test] fn empty_values_skipped() { @@ -37,3 +37,28 @@ fn bool_and_number_mixed() { Value::Number(0.0) ); } + +#[test] +fn mina_empty_array_is_ref_error() { + // `=MINA({})` is #REF!, as it is for MIN and MAX. Reached by the + // empty-argument check alone: text folds in as 0 here, so a populated + // array is never numberless in the first place. + assert_eq!(mina_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); + assert_eq!( + mina_fn(&[Value::Number(1.0), Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); +} + +#[test] +fn mina_text_only_array_is_zero() { + // `=MINA({"a","b"})` is 0 — text counts as zero rather than being + // skipped, so this needs no separate rule. + assert_eq!( + mina_fn(&[Value::Array(vec![ + Value::Text("a".to_string()), + Value::Text("b".to_string()), + ])]), + Value::Number(0.0) + ); +} From 8952f1ef7d088ced62379a0c7285fc79c0f584d2 Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:59:14 +1200 Subject: [PATCH 3/4] fix(statistical): withdraw the blank-array rule, keep the empty-array one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The blank-range evidence behind the previous commit was a probe artifact, not a Sheets answer. Controls added alongside it show the harness never resolved `Data!` ranges for these functions at all: =COUNTA(Data!M1:M4) 1 the sheet exists and M4 holds 1 =MAX(Data!M4) #REF! but MAX cannot see it =MAX(Data!M1:M3) #REF! so this was never about blanks MAX over a single cell holding 1 cannot really be #REF!, so every `Data!` row was measuring the harness. Those rows have been withdrawn from the capture, which now contains only array literals — which need no setup: {} #REF! MAX, MIN, MAXA, MINA {"a","b"} 0 all four {TRUE,FALSE} 0 MAX Both oracle-backed halves stay. What goes is the claim about blanks. MIN loses the `array_had_content` rule entirely; its empty-array check is enough, because a populated-but-numberless array already answered 0. An all-blank array is back to 0. MAX keeps `array_had_content`, but only as the thing that carves text and booleans *out* of the old blanket rule. An all-blank array sets no content and so keeps the #REF! MAX has always given it. That is not a claim about blanks — it is the absence of one. MINA and MAXA were never touched on this axis and stay at #N/A. Blank-only arrays are therefore unprobed for all four, and the four do not agree with each other: MAX #REF! MIN 0 MAXA #N/A MINA #N/A That disagreement predates this branch and is left exactly as found. It needs a capture, not a guess. Differential re-run over 16,320 cases against origin/main. 1,829 changed: MIN 574 → #REF!, every one carrying an empty array MAX 199 #REF! → 0, every one carrying a numberless array MINA 528 → #REF!, every one carrying an empty array MAXA 528 → #REF!, every one carrying an empty array Machine-checked assertions, all clean. The one that matters most here: 688 cases have a blank-only array as their only array argument, and **zero** of them changed. Blank-only shapes were probed flat, as nested one-element rows, and via a defined name, for all four functions; every one is byte-identical to main. bugs.tsv unchanged at 2056 passing / 181 open, with the failing-row sets diffed rather than the counts compared: one row flipped to passing, none regressed. --- .../src/eval/functions/statistical/max/mod.rs | 38 ++++++++++-------- .../functions/statistical/max/tests/edge.rs | 9 +++-- .../src/eval/functions/statistical/min/mod.rs | 39 +++++-------------- .../functions/statistical/min/tests/edge.rs | 27 +++---------- 4 files changed, 41 insertions(+), 72 deletions(-) diff --git a/crates/core/src/eval/functions/statistical/max/mod.rs b/crates/core/src/eval/functions/statistical/max/mod.rs index e771f38e3..c7dc816a7 100644 --- a/crates/core/src/eval/functions/statistical/max/mod.rs +++ b/crates/core/src/eval/functions/statistical/max/mod.rs @@ -6,14 +6,16 @@ use crate::types::{ErrorKind, Value}; /// /// "No numbers" is *two* rules, not one — the fixtures separate them: /// -/// - an **absent** argument is `#REF!`: `=MAX({})` and `=MAX()`; -/// - a **populated** argument holding nothing numeric is 0: `=MAX({"a","b"})` +/// - an **empty** array argument is `#REF!`: `=MAX({})`; +/// - a **populated** array holding nothing numeric is 0: `=MAX({"a","b"})` /// and `=MAX({TRUE,FALSE})` are both 0, even though neither text nor /// booleans contribute a number in array context. /// -/// So blanks read as absent while text and booleans read as -/// present-but-unusable, which is why `array_had_content` ignores `Empty` but -/// counts everything else. +/// An array holding only *blanks* is a third case, and it is unprobed — no +/// captured row covers it. `array_had_content` ignores `Empty` precisely so +/// that case keeps the `#REF!` MAX has always given it; the flag exists to +/// carve text and booleans *out* of the old rule, not because blanks were +/// measured. pub fn max_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -70,18 +72,19 @@ pub fn max_fn(args: &[Value]) -> Value { } } // A skipped sparkline is not "nothing usable": the aggregate had something - // in scope, so it answers 0 rather than falling into the absent-argument - // rule below. A sparkline *inside* an array now counts as content on its - // own, so this only still decides a direct sparkline argument sitting - // beside an otherwise-blank array — a shape no fixture row covers, left - // answering 0 as it always has. + // in scope, so it answers 0 rather than falling into the rule below. A + // sparkline *inside* an array now counts as content on its own, so this + // only still decides a direct sparkline argument sitting beside an + // otherwise-blank array — a shape no fixture row covers, left answering 0 + // as it always has. if skipped_sparkline && result.is_none() { return Value::Number(0.0); } - // Absent argument → #REF!. An array that held only blanks is as absent as - // `{}` is (`=MAX()` is #REF!), but one holding text - // or booleans is present-but-unusable and answers 0 (`=MAX({"a","b"})` - // and `=MAX({TRUE,FALSE})` are both 0). + // An array holding text or booleans is present-but-unusable and answers 0 + // (`=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` are both 0). Anything the + // fold saw nothing in at all — an array of blanks — keeps the long-standing + // #REF!. That half is unprobed; `array_had_content` is here to exempt + // text and booleans, not to make a claim about blanks. if had_array && !array_had_content && result.is_none() { return Value::Error(ErrorKind::Ref); } @@ -98,9 +101,10 @@ pub fn max_fn(args: &[Value]) -> Value { /// /// `had_content` records whether the array held *anything* other than a /// blank. Text and booleans do not contribute a number here, but they do make -/// the array present rather than absent, which is what separates -/// `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` (both 0) from -/// `=MAX()` (#REF!). +/// the array usable enough to answer 0, which is what lifts +/// `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` out of the `#REF!` rule. An +/// all-blank array sets nothing and so keeps that `#REF!` — unprobed, and +/// unchanged from what MAX has always done. fn max_array_into( elems: &[Value], result: &mut Option, diff --git a/crates/core/src/eval/functions/statistical/max/tests/edge.rs b/crates/core/src/eval/functions/statistical/max/tests/edge.rs index 29fdd29bf..0088d2dd2 100644 --- a/crates/core/src/eval/functions/statistical/max/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/max/tests/edge.rs @@ -38,9 +38,10 @@ fn max_non_empty_array_without_numbers_returns_zero() { } #[test] -fn max_array_of_only_blanks_is_ref_error() { - // `=MAX()` stays #REF! — this is the half the - // narrowing above must not disturb. +fn max_array_of_only_blanks_is_unchanged_at_ref_error() { + // No captured row covers an all-blank array, so MAX keeps the #REF! it + // has always given. Pinned here because narrowing the rule for text and + // booleans must not disturb this case. assert_eq!( max_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), Value::Error(ErrorKind::Ref) @@ -55,7 +56,7 @@ fn max_array_of_only_blanks_is_ref_error() { } #[test] -fn max_blanks_beside_content_are_not_absent() { +fn max_one_non_blank_lifts_the_array_out_of_the_ref_rule() { assert_eq!( max_fn(&[Value::Array(vec![ Value::Empty, diff --git a/crates/core/src/eval/functions/statistical/min/mod.rs b/crates/core/src/eval/functions/statistical/min/mod.rs index c17b12ee1..68bca0911 100644 --- a/crates/core/src/eval/functions/statistical/min/mod.rs +++ b/crates/core/src/eval/functions/statistical/min/mod.rs @@ -6,13 +6,13 @@ use crate::types::{ErrorKind, Value}; /// /// "No numbers" is *two* rules, not one — the fixtures separate them: /// -/// - an **absent** argument is `#REF!`: `=MIN({})` and `=MIN()`; -/// - a **populated** argument holding nothing numeric is 0: -/// `=MIN({"a","b"})`, and `=IFERROR(MIN({"a","b","c"}),"no numbers")` is -/// pinned to the number 0. +/// - an **empty** array argument is `#REF!`: `=MIN({})`; +/// - a **populated** array holding nothing numeric is 0: `=MIN({"a","b"})`, +/// and `=IFERROR(MIN({"a","b","c"}),"no numbers")` is pinned to the number +/// 0. /// -/// So blanks read as absent while text reads as present-but-unusable, which -/// is why `array_had_content` ignores `Empty` but counts everything else. +/// An array holding only *blanks* is a third case, and it is unprobed — no +/// captured row covers it. MIN leaves it answering 0 as it always has. pub fn min_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -23,8 +23,6 @@ pub fn min_fn(args: &[Value]) -> Value { return r; } let mut result: Option = None; - let mut had_array = false; - let mut array_had_content = false; for arg in args { match arg { Value::Number(n) => { @@ -45,7 +43,6 @@ pub fn min_fn(args: &[Value]) -> Value { } Value::Empty => {} Value::Array(elems) => { - had_array = true; // An explicitly empty argument is fatal on the spot, even if a // number was already in hand — matching MAX, whose // `=MAX(SPARKLINE({1,2,3}),{})` row is #REF! despite the @@ -56,7 +53,7 @@ pub fn min_fn(args: &[Value]) -> Value { // Recurse into nested arrays (e.g. a vertical range // materializes as nested one-element row arrays) so every // cell is visited. - if let Err(e) = min_array_into(elems, &mut result, &mut array_had_content) { + if let Err(e) = min_array_into(elems, &mut result) { return e; } } @@ -65,37 +62,21 @@ pub fn min_fn(args: &[Value]) -> Value { _ => {} } } - // Arrays that held only blanks are as absent as `{}` is: `=MIN()` is #REF!, while `=MIN({"a","b"})` is 0. - if had_array && !array_had_content && result.is_none() { - return Value::Error(ErrorKind::Ref); - } Value::Number(result.unwrap_or(0.0)) } /// Recursively fold a nested array's numbers into `result` for MIN's /// array-context rules (Bool/Text/Empty skipped, errors propagate). -/// -/// `had_content` records whether the array held *anything* other than a -/// blank. Text and booleans do not contribute a number here, but they do make -/// the array present rather than absent, which is what separates -/// `=MIN({"a","b"})` (0) from `=MIN()` (#REF!). -fn min_array_into( - elems: &[Value], - result: &mut Option, - had_content: &mut bool, -) -> Result<(), Value> { +fn min_array_into(elems: &[Value], result: &mut Option) -> Result<(), Value> { for elem in elems { match elem { Value::Number(n) => { - *had_content = true; *result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); } Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - Value::Array(inner) => min_array_into(inner, result, had_content)?, - Value::Empty => {} - _ => *had_content = true, + Value::Array(inner) => min_array_into(inner, result)?, + _ => {} } } Ok(()) diff --git a/crates/core/src/eval/functions/statistical/min/tests/edge.rs b/crates/core/src/eval/functions/statistical/min/tests/edge.rs index 0f770fb45..bcf260338 100644 --- a/crates/core/src/eval/functions/statistical/min/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/min/tests/edge.rs @@ -53,38 +53,21 @@ fn min_non_empty_array_without_numbers_still_returns_zero() { } #[test] -fn min_array_of_only_blanks_is_ref_error() { - // `=MIN()` is #REF!: blanks read as absent, not as - // present-but-unusable. Both the flat and the nested-row materializations - // of such a range must agree. +fn min_array_of_only_blanks_is_unchanged_at_zero() { + // No captured row covers an all-blank array, so MIN keeps the 0 it has + // always given. Pinned here so a future change to it has to be deliberate + // rather than a side effect of the empty-array rule above. assert_eq!( min_fn(&[Value::Array(vec![Value::Empty, Value::Empty, Value::Empty])]), - Value::Error(ErrorKind::Ref) + Value::Number(0.0) ); assert_eq!( min_fn(&[Value::Array(vec![ Value::Array(vec![Value::Empty]), Value::Array(vec![Value::Empty]), ])]), - Value::Error(ErrorKind::Ref) - ); -} - -#[test] -fn min_blanks_beside_content_are_not_absent() { - // One non-blank cell is enough to make the whole argument present. - assert_eq!( - min_fn(&[Value::Array(vec![ - Value::Empty, - Value::Text("z".to_string()), - ])]), Value::Number(0.0) ); - // And a number anywhere wins outright. - assert_eq!( - min_fn(&[Value::Array(vec![Value::Empty, Value::Number(4.0)])]), - Value::Number(4.0) - ); } #[test] From 38f64176c1216cd4322b9a4b531707a3d0d76950 Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:25:35 +1200 Subject: [PATCH 4/4] fix(statistical): stop a date-only array answering 0 in MAX MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review blocker. `array_had_content` was set by a catch-all `_ => true`, so every non-numeric variant inherited content-hood — including `Value::Date`, which `max_array_into` does not fold into a result. =MAX({DATE(2020,1,1),DATE(2021,1,1)}) main #REF! branch 0 =MAX() main #REF! branch 0 A workbook date cell maps to `Value::Date`, so `=MAX(A1:A10)` over a date column is the everyday form of this. Both answers are wrong against Sheets, which returns the latest date — but turning a visible error into a plausible-looking 0 is precisely the hazard this branch exists to remove, and it did so in the one shape no fixture covers. The flag now matches `Value::Text(_) | Value::Bool(_)` explicitly, which is exactly what the capture backs. Everything else numberless — dates, zoned instants, and any variant added later — keeps whatever main answered. A regression test pins the date case so it cannot drift again. Also in this commit, all from the same review: - Dropped the dead `had_content` write in the `Sparkline` arm. The `skipped_sparkline` early return always precedes the `had_content` test, so the write could never be read; the doc comment described an effect that did not exist. With it gone the sparkline flag is load-bearing again, as originally designed. - Removed a self-confirmed assertion from tests/sparkline.rs, whose header promises every expectation is a captured row. `=MIN(SPARKLINE({1,2,3}), {"a"})` was not one. The remaining MIN assertion there cites its bugs.tsv row. - Corrected min/mod.rs, which claimed the fixtures pin `=MIN({"a","b"})`. They do not — only the IFERROR row exists in-repo. The doc now says which evidence is in this repo and which lands separately. - Formatted the five new assertions rustfmt objected to. Pre-existing violations left alone. Nested empty arrays (`{{}}`) are left as found: MAX #REF!, MIN 0, MAXA and MINA #N/A. Unreachable from a resolver-delivered range, unprobed for all four, and the same open question as blank-only arrays — one capture would settle both. Making three of them agree with MAX on a guess is the move that produced the original defect. The date behaviour is filed separately as #776 rather than fixed here; it needs an oracle this repo does not have. Differential re-run over 20,128 cases against origin/main, now with date atoms in six materializations. 2,077 changed: MIN 606 → #REF!, every one carrying an empty array MAX 351 #REF! → 0, every one carrying a numberless array MINA 560 → #REF!, every one carrying an empty array MAXA 560 → #REF!, every one carrying an empty array Machine-checked, no violations: date-only arguments 920 cases, 0 changed blank-only arguments 720 cases, 0 changed All six date shapes — inline literal, cell range, named array, nested rows, date beside a blank, date beside a number — are identical to main for all four functions. Every oracle row still holds. --- .../src/eval/functions/statistical/max/mod.rs | 62 +++++++++---------- .../functions/statistical/max/tests/edge.rs | 32 +++++++++- .../functions/statistical/maxa/tests/edge.rs | 5 +- .../src/eval/functions/statistical/min/mod.rs | 18 +++--- .../functions/statistical/min/tests/edge.rs | 5 +- .../functions/statistical/mina/tests/edge.rs | 5 +- crates/core/tests/sparkline.rs | 9 +-- 7 files changed, 83 insertions(+), 53 deletions(-) diff --git a/crates/core/src/eval/functions/statistical/max/mod.rs b/crates/core/src/eval/functions/statistical/max/mod.rs index c7dc816a7..87e1ad249 100644 --- a/crates/core/src/eval/functions/statistical/max/mod.rs +++ b/crates/core/src/eval/functions/statistical/max/mod.rs @@ -4,18 +4,19 @@ use crate::types::{ErrorKind, Value}; /// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. /// Array elements: Numbers only; text/Bool → skip; errors propagate. /// -/// "No numbers" is *two* rules, not one — the fixtures separate them: +/// "No numbers" is *two* rules, not one: /// -/// - an **empty** array argument is `#REF!`: `=MAX({})`; +/// - an **empty** array argument is `#REF!`: `=MAX({})` — the one rule with +/// an in-repo row (statistical.tsv); /// - a **populated** array holding nothing numeric is 0: `=MAX({"a","b"})` /// and `=MAX({TRUE,FALSE})` are both 0, even though neither text nor -/// booleans contribute a number in array context. +/// booleans contribute a number in array context. Captured in Google +/// Sheets; the rows land separately, since they fail until this code exists. /// -/// An array holding only *blanks* is a third case, and it is unprobed — no -/// captured row covers it. `array_had_content` ignores `Empty` precisely so -/// that case keeps the `#REF!` MAX has always given it; the flag exists to -/// carve text and booleans *out* of the old rule, not because blanks were -/// measured. +/// Everything else numberless — blanks, dates, zoned instants — is unprobed +/// and keeps the `#REF!` MAX has always given it. `array_had_content` is set +/// by text and booleans alone, so it exempts exactly what was captured and +/// makes no claim past it. pub fn max_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); @@ -72,19 +73,18 @@ pub fn max_fn(args: &[Value]) -> Value { } } // A skipped sparkline is not "nothing usable": the aggregate had something - // in scope, so it answers 0 rather than falling into the rule below. A - // sparkline *inside* an array now counts as content on its own, so this - // only still decides a direct sparkline argument sitting beside an - // otherwise-blank array — a shape no fixture row covers, left answering 0 - // as it always has. + // in scope, so it answers 0 rather than falling into the rule below + // (google.tsv: `=MAX(Data!K1:K1)` is 0). This runs first, so it decides + // every sparkline case before `array_had_content` is consulted at all. if skipped_sparkline && result.is_none() { return Value::Number(0.0); } - // An array holding text or booleans is present-but-unusable and answers 0 - // (`=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` are both 0). Anything the - // fold saw nothing in at all — an array of blanks — keeps the long-standing - // #REF!. That half is unprobed; `array_had_content` is here to exempt - // text and booleans, not to make a claim about blanks. + // An array holding text or booleans answers 0 (`=MAX({"a","b"})` and + // `=MAX({TRUE,FALSE})` are both 0). `array_had_content` is set by exactly + // those two variants and nothing else, so every other numberless array — + // blanks, dates, zoned instants — keeps the long-standing #REF!. Those are + // unprobed; the flag exempts what the capture covers and makes no claim + // beyond it. if had_array && !array_had_content && result.is_none() { return Value::Error(ErrorKind::Ref); } @@ -99,12 +99,12 @@ pub fn max_fn(args: &[Value]) -> Value { /// and `=MAX(Data!K1:K1)` are both 0). The flag is what distinguishes "skipped a /// sparkline" from "saw nothing usable at all", which stay different answers. /// -/// `had_content` records whether the array held *anything* other than a -/// blank. Text and booleans do not contribute a number here, but they do make -/// the array usable enough to answer 0, which is what lifts -/// `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` out of the `#REF!` rule. An -/// all-blank array sets nothing and so keeps that `#REF!` — unprobed, and -/// unchanged from what MAX has always done. +/// `had_content` is set by text and booleans *only* — the two variants the +/// capture covers (`=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` are both 0). +/// It is deliberately not a catch-all: every other non-numeric variant, most +/// notably `Date`, leaves it alone and so keeps the `#REF!` MAX has always +/// answered. Listing the variants rather than falling through also stops a +/// future `Value` kind inheriting content-hood by accident. fn max_array_into( elems: &[Value], result: &mut Option, @@ -114,20 +114,14 @@ fn max_array_into( for elem in elems { match elem { Value::Number(n) => { - *had_content = true; *result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } - Value::Sparkline(_) => { - *had_content = true; - *skipped_sparkline = true; - } + Value::Text(_) | Value::Bool(_) => *had_content = true, + Value::Sparkline(_) => *skipped_sparkline = true, Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), - Value::Array(inner) => { - max_array_into(inner, result, skipped_sparkline, had_content)? - } - Value::Empty => {} - _ => *had_content = true, + Value::Array(inner) => max_array_into(inner, result, skipped_sparkline, had_content)?, + _ => {} } } Ok(()) diff --git a/crates/core/src/eval/functions/statistical/max/tests/edge.rs b/crates/core/src/eval/functions/statistical/max/tests/edge.rs index 0088d2dd2..739698faf 100644 --- a/crates/core/src/eval/functions/statistical/max/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/max/tests/edge.rs @@ -16,14 +16,18 @@ fn max_text_in_args_returns_value_error() { #[test] fn max_empty_array_is_ref_error() { - assert_eq!(max_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); + assert_eq!( + max_fn(&[Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); } #[test] fn max_non_empty_array_without_numbers_returns_zero() { // `=MAX({"a","b"})` and `=MAX({TRUE,FALSE})` are both 0: neither text nor - // booleans contribute a number in array context, but they do make the - // argument present rather than absent. + // booleans contribute a number in array context, but both are enough to + // lift the array out of the #REF! rule. These two variants are the whole + // of the carve-out — nothing else sets `had_content`. assert_eq!( max_fn(&[Value::Array(vec![ Value::Text("a".to_string()), @@ -55,6 +59,28 @@ fn max_array_of_only_blanks_is_unchanged_at_ref_error() { ); } +#[test] +fn max_array_of_only_dates_is_unchanged_at_ref_error() { + // `max_array_into` folds only `Value::Number`, so a date-only array has + // never produced a result and has always answered #REF!. That is almost + // certainly wrong against Sheets — but it is pre-existing, unprobed, and + // must not be quietly turned into a plausible-looking 0 by the + // text-and-boolean carve-out. `had_content` is set by text and booleans + // only, never by a catch-all, and this pins that. + assert_eq!( + max_fn(&[Value::Array(vec![ + Value::Date(43831.0), + Value::Date(44197.0) + ])]), + Value::Error(ErrorKind::Ref) + ); + // Same for a date arriving through a nested-row range materialization. + assert_eq!( + max_fn(&[Value::Array(vec![Value::Array(vec![Value::Date(43831.0)])])]), + Value::Error(ErrorKind::Ref) + ); +} + #[test] fn max_one_non_blank_lifts_the_array_out_of_the_ref_rule() { assert_eq!( diff --git a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs index 0bc0be92e..a1634bcef 100644 --- a/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/maxa/tests/edge.rs @@ -43,7 +43,10 @@ fn maxa_empty_array_is_ref_error() { // `=MAXA({})` is #REF!, as it is for MIN and MAX. Reached by the // empty-argument check alone: text folds in as 0 here, so a populated // array is never numberless in the first place. - assert_eq!(maxa_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); + assert_eq!( + maxa_fn(&[Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); assert_eq!( maxa_fn(&[Value::Number(1.0), Value::Array(vec![])]), Value::Error(ErrorKind::Ref) diff --git a/crates/core/src/eval/functions/statistical/min/mod.rs b/crates/core/src/eval/functions/statistical/min/mod.rs index 68bca0911..6122d9fab 100644 --- a/crates/core/src/eval/functions/statistical/min/mod.rs +++ b/crates/core/src/eval/functions/statistical/min/mod.rs @@ -4,15 +4,19 @@ use crate::types::{ErrorKind, Value}; /// Direct args: Numbers, Bool (TRUE=1, FALSE=0), parseable text coerced to number. /// Array elements: Numbers only; text/Bool → skip; errors propagate. /// -/// "No numbers" is *two* rules, not one — the fixtures separate them: +/// "No numbers" is *two* rules, not one: /// -/// - an **empty** array argument is `#REF!`: `=MIN({})`; -/// - a **populated** array holding nothing numeric is 0: `=MIN({"a","b"})`, -/// and `=IFERROR(MIN({"a","b","c"}),"no numbers")` is pinned to the number -/// 0. +/// - an **empty** array argument is `#REF!`: `=MIN({})`. Captured in Google +/// Sheets; the row lands separately, since it fails until this code exists. +/// - a **populated** array holding nothing numeric is 0. The in-repo evidence +/// is indirect but sufficient: statistical.tsv pins +/// `=IFERROR(MIN({"a","b","c"}),"no numbers")` to the *number* 0, so MIN +/// cannot have errored. A direct `=MIN({"a","b"})` row is captured and +/// lands with the others. /// -/// An array holding only *blanks* is a third case, and it is unprobed — no -/// captured row covers it. MIN leaves it answering 0 as it always has. +/// MIN needs no code for the second rule — it already falls through to 0. +/// An array holding only *blanks* is a third case, unprobed, left at the 0 +/// MIN has always given it. pub fn min_fn(args: &[Value]) -> Value { if args.is_empty() { return Value::Error(ErrorKind::NA); diff --git a/crates/core/src/eval/functions/statistical/min/tests/edge.rs b/crates/core/src/eval/functions/statistical/min/tests/edge.rs index bcf260338..9852387be 100644 --- a/crates/core/src/eval/functions/statistical/min/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/min/tests/edge.rs @@ -18,7 +18,10 @@ fn min_text_in_args_returns_value_error() { fn min_empty_array_is_ref_error() { // Matches MAX and Google Sheets: an empty array argument is #REF!, // not a silent 0. - assert_eq!(min_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); + assert_eq!( + min_fn(&[Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); } #[test] diff --git a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs index 87fffa6a2..09b256883 100644 --- a/crates/core/src/eval/functions/statistical/mina/tests/edge.rs +++ b/crates/core/src/eval/functions/statistical/mina/tests/edge.rs @@ -43,7 +43,10 @@ fn mina_empty_array_is_ref_error() { // `=MINA({})` is #REF!, as it is for MIN and MAX. Reached by the // empty-argument check alone: text folds in as 0 here, so a populated // array is never numberless in the first place. - assert_eq!(mina_fn(&[Value::Array(vec![])]), Value::Error(ErrorKind::Ref)); + assert_eq!( + mina_fn(&[Value::Array(vec![])]), + Value::Error(ErrorKind::Ref) + ); assert_eq!( mina_fn(&[Value::Number(1.0), Value::Array(vec![])]), Value::Error(ErrorKind::Ref) diff --git a/crates/core/tests/sparkline.rs b/crates/core/tests/sparkline.rs index f1353e416..98782ba17 100644 --- a/crates/core/tests/sparkline.rs +++ b/crates/core/tests/sparkline.rs @@ -738,16 +738,13 @@ fn an_empty_array_argument_outranks_the_sparkline_skip() { eval("=MAX(SPARKLINE({1,2,3}),{\"a\"})"), Value::Number(0.0) ); - // MIN now carries the same empty-array rule, so its counterpart row - // agrees with MAX's. + // bugs.tsv: `=MIN(SPARKLINE({1,2,3}),{})` is #REF! — a live Google Sheets + // value this engine used to miss. MIN now carries the same empty-array + // rule, so its counterpart row agrees with MAX's. assert_eq!( eval("=MIN(SPARKLINE({1,2,3}),{})"), Value::Error(ErrorKind::Ref) ); - assert_eq!(eval("=MIN({})"), Value::Error(ErrorKind::Ref)); - // MIN keeps answering 0 for a populated array holding nothing numeric — - // the empty-array rule is narrower than MAX's. - assert_eq!(eval("=MIN(SPARKLINE({1,2,3}),{\"a\"})"), Value::Number(0.0)); } // ── Registry surface ────────────────────────────────────────────────────────