fix(statistical): MAX/MIN/MAXA/MINA — an empty argument is an error, a numberless one is zero

crates/core/src/eval/functions/statistical/max/mod.rs

@@ -3,7 +3,20 @@ 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:
+///
+/// - 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. Captured in Google
+///   Sheets; the rows land separately, since they fail until this code exists.
+///
+/// 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);
@@ -15,6 +28,7 @@ pub fn max_fn(args: &[Value]) -> Value {
     }
     let mut result: Option<f64> = 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 +58,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 +73,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 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);
     }
-    // Empty array with no numbers → Ref
-    if had_array && result.is_none() {
+    // 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);
     }
     Value::Number(result.unwrap_or(0.0))
@@ -74,20 +98,29 @@ 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` 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<f64>,
     skipped_sparkline: &mut bool,
+    had_content: &mut bool,
 ) -> Result<(), Value> {
     for elem in elems {
         match elem {
             Value::Number(n) => {
                 *result = Some(result.map_or(*n, |cur: f64| cur.max(*n)));
             }
+            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)?,
+            Value::Array(inner) => max_array_into(inner, result, skipped_sparkline, had_content)?,
             _ => {}
         }
     }

crates/core/src/eval/functions/statistical/max/tests/edge.rs

@@ -14,6 +14,88 @@ 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 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()),
+            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_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)
+    );
+    assert_eq!(
+        max_fn(&[Value::Array(vec![
+            Value::Array(vec![Value::Empty]),
+            Value::Array(vec![Value::Empty]),
+        ])]),
+        Value::Error(ErrorKind::Ref)
+    );
+}
+
+#[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!(
+        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!(

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).

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,31 @@ 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)
+    );
+}

crates/core/src/eval/functions/statistical/min/mod.rs

@@ -3,7 +3,20 @@ 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.
+///
+/// "No numbers" is *two* rules, not one:
+///
+/// - 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.
+///
+/// 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);
@@ -34,6 +47,13 @@ pub fn min_fn(args: &[Value]) -> Value {
             }
             Value::Empty => {}
             Value::Array(elems) => {
+                // 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.

crates/core/src/eval/functions/statistical/min/tests/edge.rs

@@ -14,6 +14,65 @@ 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 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()),
+            Value::Text("b".to_string()),
+        ])]),
+        Value::Number(0.0)
+    );
+    // Booleans are skipped in array context but still make the array present.
+    assert_eq!(
+        min_fn(&[Value::Array(vec![Value::Bool(true), Value::Bool(false)])]),
+        Value::Number(0.0)
+    );
+}
+
+#[test]
+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::Number(0.0)
+    );
+    assert_eq!(
+        min_fn(&[Value::Array(vec![
+            Value::Array(vec![Value::Empty]),
+            Value::Array(vec![Value::Empty]),
+        ])]),
+        Value::Number(0.0)
+    );
+}
+
 #[test]
 fn min_negative_numbers() {
     assert_eq!(

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).