From bc916dbb02dabcff9c3ed8e7135dbc2bffdb6d8e Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:23:34 +1200 Subject: [PATCH 1/2] =?UTF-8?q?feat(google)!:=20SPARKLINE=20=E2=80=94=20pa?= =?UTF-8?q?rse=20and=20validate=20the=20in-cell=20chart=20(#766)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SPARKLINE(data, [options]) now parses and validates its arguments and produces a render spec; drawing stays with the consumer. Every behaviour below is a row of the Google Sheets conformance fixtures, not a guess. Value model: a new `Value::Sparkline(Box)` variant, because Sheets models the result as a value kind of its own — `TYPE()` reports the undocumented code 128 (outside the documented 1/2/4/16/64 set) and `ISERROR()` is FALSE. A sparkline read back out of a cell is still a sparkline: `=TYPE(Data!K1)` is 128, so referencing is not a coercion point. Sheets keeps two notions of sameness, and the engine reproduces both: - `=` reports ANY two sparklines equal, whatever they plot, whatever their charttype or options. So `PartialEq` on the core value ignores the payload, like `ErrorMsg` does, and a sparkline outranks every scalar in ordering (`>1`, `>"zzzz"`, `>TRUE` are all TRUE) while two sparklines are mutually equal (`<`/`>` FALSE, `>=` TRUE). - `COUNTUNIQUE` nonetheless counts two different sparklines as 2 and two identical ones as 1. The spec is therefore retained, keyed on in COUNTUNIQUE, used as storage identity in the workbook layer (recalc both writes a cell back only when the value differs and tests convergence by value equality, so a coarser identity would strand a changed chart's stale spec), and carried in full by every serialization — including back *in* through the WASM and MCP variable decoders, so an emitted sparkline handed back as a variable is still a sparkline rather than silently becoming empty. All three decoders accept exactly what the engine can emit and nothing wider. Every guard in all three is covered by a test verified to fail when that guard is removed — 21 guards individually knocked out and restored, including the two `pair.len() != 2` checks whose absence would index out of bounds on caller-supplied JSON rather than reject it. The comparison-alias functions EQ/NE/GT/GTE/LT/LTE are backed by a second comparison path in the operator module; both paths now answer identically, pinned by a test that compares each alias against its operator. Coercion runs through three seams — `to_number` rejects with #VALUE!, `to_string_val` reads "" and `to_bool` reads false — but individual functions carry hand-carved arms that override their seam (`N`, `TEXT`, the `TO_*` family, the `&` operator, the aggregates). The module doc does not try to enumerate them: google.tsv is the record and the only authority, and the doc says so. `DOLLAR` is #VALUE! while `TO_DOLLARS` is "" — that pair is why nothing here is inferred by analogy. Aggregates skip a sparkline as if the argument were absent, and answer the same however it arrived — directly or from a cell in a range. SUM, PRODUCT, MAX, MIN, MAXA, MINA, COUNT and SUMSQ all answer 0 when a sparkline is the only thing in scope; AVERAGE and AVERAGEA answer #DIV/0!, which is the row proving the argument list genuinely empties rather than gaining a zero. An explicitly empty array argument outranks that: `=MAX(SPARKLINE(...),{})` is #REF! while `=MAX(SPARKLINE(...),{"a"})` is 0. MAX's numberless-array `#REF!` and MAXA/MINA's `#N/A` are still reached by every input that does not involve a sparkline: a 360-case differential against origin/main over text, boolean, empty, mixed and nested arrays for MAX/MIN/MAXA/MINA/SUM/PRODUCT/AVERAGE/COUNT is byte-identical. Validation splits into three error classes: #N/A arity/shape of `data` — no arguments, a scalar instead of a range, a single value #REF! structural malformation — an empty array, options that are not key/value pairs #VALUE! a bad option *value* — an unrecognised charttype An unrecognised option *key* is not an error: Sheets ignores it, so it is kept in the spec rather than rejected, which is what lets a workbook written against a newer option set still evaluate. Kept keys participate in COUNTUNIQUE's key exactly as recognised ones do. Option keys and charttype values match case-insensitively, a non-text option key is accepted, 2-D data flattens row-major, a genuine blank cell inside the source range is a data point, and `bar` given a third value renders — all recorded. Carried across all four value representations: the core enum, the workbook wire value (canonical JSON `{"type":"sparkline","value":{charttype,data, options}}`), the WASM `EvalResult` (a typed `SparklineSpecResult`) and the MCP JSON surfaces, each with a decode path back. The shared TSV runner also learns to skip rows that read sheet-qualified references (`=SUM(Data!K1:K2)`). It evaluates each row standalone with no workbook behind it, so such a reference resolves to empty — which does not only fail the row, it makes one *pass for the wrong reason* whenever an empty read happens to match the recorded value (`=SUM(Data!K1:K1)` is 0 either way). In the two runners that affects only google.tsv; the per-function coverage scan applies the same guard, where it also drops workbook.tsv's 24 sheet-qualified rows from the credit scan — harmless (every function they mention is credited by many other rows) and deliberate, since an accidental empty-vs-empty match is not evidence of coverage. Because that adds a second silent skip category to a runner whose silent skipping is already tracked as a defect, both runners now print a per-file breakdown — total rows, enforced, and skipped by reason: google.tsv: 128 rows — 83 enforced, 29 skipped (no recorded expected value), 16 skipped (reads authored cells) nextest captures a passing test's stdout, so the `ci` profile carries a `success-output` override for the conformance binary — without it the accounting would be invisible under the exact command CI runs, which is the only place it matters. It is scoped by `binary_id`, not binary name, so a future crate adding a `tests/conformance.rs` does not inherit it silently. The reason labels describe the whole bucket rather than SPARKLINE's case: text.tsv's 46 rows there are 41 with a genuinely empty recorded value plus 5 whose recorded value is whitespace and is skipped by a pre-existing `trim()`. This is a narrow mitigation, not the fix core#767 asks for — the rows are still skipped, they merely announce themselves. Tests live in separate files per repo convention: crates/core/tests/ sparkline.rs (47 engine-level cases, each citing its fixture row), crates/workbook/tests/sparkline_value_tests.rs (serialization, storage identity, decode rejection), crates/wasm/tests/sparkline_round_trip.rs and crates/mcp/tests/sparkline_variables.rs (emit-then-read-back, plus decoder rejection on both surfaces). google.tsv is a blocking conformance gate, covering 69 of the function's 114 rows there: 29 record an empty expected value and 16 read authored input cells. A 115th recorded row is a known engine divergence and sits in bugs.tsv instead — `=MIN(SPARKLINE({1,2,3}),{})` is #REF! in Sheets and 0 here, because MIN has no empty-array rule at all (`=MIN({})` is 0 on main, `=MAX({})` is #REF!). That gap predates sparklines and fixing it would move MIN for inputs unrelated to this work. The skipped set is exactly the rows where a sparkline renders, projects to empty text, or arrives through a range — so the conformance suite alone would not catch a regression of the text seam or of range delivery. crates/core/tests/sparkline.rs is what covers them, deliberately, the range rows against a seeded resolver. BREAKING CHANGE: `truecalc_core::Value` and `truecalc_workbook::Value` gain a `Sparkline` variant. Neither enum is `#[non_exhaustive]`, so any downstream exhaustive `match` on a cell value must add an arm. Refs #766, #767 --- .config/nextest.toml | 13 + crates/core/src/eval/coercion/mod.rs | 12 + crates/core/src/eval/functions/array/mod.rs | 5 + crates/core/src/eval/functions/google/mod.rs | 229 ++++++ .../src/eval/functions/logical/info/mod.rs | 8 +- .../src/eval/functions/math/average/mod.rs | 2 + .../eval/functions/math/countunique/mod.rs | 10 + .../src/eval/functions/math/product/mod.rs | 54 +- .../core/src/eval/functions/math/sum/mod.rs | 7 +- .../core/src/eval/functions/math/sumsq/mod.rs | 3 + crates/core/src/eval/functions/mod.rs | 2 + .../core/src/eval/functions/operator/mod.rs | 9 + .../src/eval/functions/parser/to_date/mod.rs | 2 + .../eval/functions/parser/to_dollars/mod.rs | 2 + .../eval/functions/parser/to_percent/mod.rs | 2 + .../functions/parser/to_pure_number/mod.rs | 2 + .../src/eval/functions/parser/to_text/mod.rs | 8 + .../statistical/distributions_impl.rs | 4 +- .../src/eval/functions/statistical/max/mod.rs | 25 +- .../eval/functions/statistical/maxa/mod.rs | 23 +- .../eval/functions/statistical/mina/mod.rs | 23 +- .../functions/statistical/stat_helpers.rs | 15 +- .../src/eval/functions/text/text_fn/mod.rs | 14 + crates/core/src/eval/mod.rs | 17 + crates/core/src/types/mod.rs | 2 + crates/core/src/types/sparkline.rs | 98 +++ crates/core/src/types/value.rs | 19 + crates/core/tests/conformance.rs | 173 +++- crates/core/tests/conformance_reporter.rs | 3 + crates/core/tests/sparkline.rs | 761 ++++++++++++++++++ crates/mcp/src/main.rs | 86 ++ crates/mcp/tests/sparkline_variables.rs | 136 ++++ crates/wasm-workbook/src/lib.rs | 25 + crates/wasm/src/lib.rs | 103 ++- crates/wasm/tests/sparkline_round_trip.rs | 99 +++ crates/workbook/src/recalc.rs | 2 + crates/workbook/src/value.rs | 170 +++- .../workbook/tests/sparkline_value_tests.rs | 140 ++++ 38 files changed, 2271 insertions(+), 37 deletions(-) create mode 100644 crates/core/src/eval/functions/google/mod.rs create mode 100644 crates/core/src/types/sparkline.rs create mode 100644 crates/core/tests/sparkline.rs create mode 100644 crates/mcp/tests/sparkline_variables.rs create mode 100644 crates/wasm/tests/sparkline_round_trip.rs create mode 100644 crates/workbook/tests/sparkline_value_tests.rs diff --git a/.config/nextest.toml b/.config/nextest.toml index 02ad533dc..8df0537f6 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -4,3 +4,16 @@ fail-fast = false [profile.ci.junit] path = "junit.xml" + +# Surface the conformance runners' stdout even when they pass. +# +# Each TSV runner prints how many of its rows it actually enforced and how many +# it skipped, and why (core#767: a row the runner skips looks enforced and +# asserts nothing). nextest captures a passing test's output by default, so +# without this the accounting is invisible in CI — the only place it matters. +# Scoped by binary *id*, not name: `binary(conformance)` matches that binary +# name in every package, so any crate adding a `tests/conformance.rs` would +# silently inherit this. +[[profile.ci.overrides]] +filter = 'binary_id(truecalc-core::conformance)' +success-output = 'final' diff --git a/crates/core/src/eval/coercion/mod.rs b/crates/core/src/eval/coercion/mod.rs index e915417a1..dc70caa02 100644 --- a/crates/core/src/eval/coercion/mod.rs +++ b/crates/core/src/eval/coercion/mod.rs @@ -28,6 +28,10 @@ pub fn to_number(v: Value) -> Result { // Zoned instants have no naive numeric value; force an explicit TZSERIAL // downcast rather than silently mixing naive/aware time. Value::Zoned(_) => Err(Value::Error(ErrorKind::Value)), + // Arithmetic rejects a sparkline: `=SPARKLINE({1,2,3})+1` is `#VALUE!` + // (google.tsv). `N()` (0) and the aggregates (which skip it) do not come + // through here. + Value::Sparkline(_) => Err(Value::Error(ErrorKind::Value)), } } @@ -49,6 +53,12 @@ pub fn to_string_val(v: Value) -> Result { Value::Array(_) => Err(Value::Error(ErrorKind::Value)), // Self-describing canonical RFC-9557 form so concatenation is lossless. Value::Zoned(z) => Ok(z.to_rfc9557()), + // Text contexts are permissive: a sparkline reads as empty text + // (google.tsv: `LEN` is `0`, `LEFT` and `TEXT` and `TEXTJOIN` are `""`, + // `CONCATENATE(sparkline,"x")` is `"x"`, `EXACT(sparkline,"")` is + // `TRUE`). The `&` *operator* is the one carved-out exception and + // rejects it before reaching here — see `eval_binary`. + Value::Sparkline(_) => Ok(String::new()), } } @@ -74,6 +84,8 @@ pub fn to_bool(v: Value) -> Result { Value::Empty => Err(Value::Error(ErrorKind::Value)), // A zoned instant has no truthiness. Value::Zoned(_) => Err(Value::Error(ErrorKind::Value)), + // A sparkline is falsy (google.tsv: `=IF(SPARKLINE({1,2,3}),1,2)` is 2). + Value::Sparkline(_) => Ok(false), // Array condition: use the top-left (anchor) element — same as the // unspilled-array collapse the workbook layer applies. Value::Array(mut elems) => { diff --git a/crates/core/src/eval/functions/array/mod.rs b/crates/core/src/eval/functions/array/mod.rs index b5e995dc8..c1375f881 100644 --- a/crates/core/src/eval/functions/array/mod.rs +++ b/crates/core/src/eval/functions/array/mod.rs @@ -490,6 +490,11 @@ fn compare_values_sort(a: &Value, b: &Value) -> std::cmp::Ordering { (Value::Bool(x), Value::Bool(y)) => x.cmp(y), // Zone-aware instants sort by the absolute instant. (Value::Zoned(x), Value::Zoned(y)) => x.utc_nanos.cmp(&y.utc_nanos), + // Sparklines: only equality is observed (google.tsv), and no ordering + // between two different ones is — matching the `=`/`<`/`>` operators and + // their EQ/LT/GT aliases, which report no ordering relation either. The + // sort is stable, so sparklines keep their input order. + (Value::Sparkline(_), Value::Sparkline(_)) => std::cmp::Ordering::Equal, _ => std::cmp::Ordering::Equal, } } diff --git a/crates/core/src/eval/functions/google/mod.rs b/crates/core/src/eval/functions/google/mod.rs new file mode 100644 index 000000000..d982fcd6f --- /dev/null +++ b/crates/core/src/eval/functions/google/mod.rs @@ -0,0 +1,229 @@ +//! `SPARKLINE(data, [options])` — the in-cell chart function. +//! +//! The engine's job here is parsing and validation, not drawing: a successful +//! call produces a [`Value::Sparkline`] carrying the parsed, validated +//! [`SparklineSpec`], and the consumer renders it. +//! +//! ## What Google Sheets does (conformance fixtures, `google.tsv`) +//! +//! The result is a value kind of its own — `TYPE()` reports `128` (outside +//! `TYPE`'s documented set) and `ISERROR()` is `FALSE`. +//! +//! ## Coercion — where the answers live +//! +//! Three coercion seams carry most contexts, each a single blanket arm in +//! [`crate::eval::coercion`] that every caller of that seam inherits: +//! +//! | seam | a sparkline reads as | +//! |---|---| +//! | `to_number` | `#VALUE!` | +//! | `to_string_val` | `""` | +//! | `to_bool` | `false` | +//! +//! **That is not enough to predict any particular function.** Individual +//! functions carry hand-carved arms that override their seam — `N`, `TEXT`, the +//! `TO_*` family, the `&` operator, and the aggregates — and they are spread +//! across the codebase, so no list here can stay complete. (The aggregates are +//! uniform *as of the rows below*: every one of them skips a sparkline and +//! answers 0 when nothing else is in scope, `AVERAGE` alone answering `#DIV/0!`. +//! That uniformity is a fact about the probed set, not a rule to extend from — +//! `MINA` had no arm at all until a row asked for one.) +//! +//! The record is `tests/fixtures/google_sheets/google.tsv`, every row of it +//! observed in live Google Sheets and the only authority here. This function +//! contributes **114** of that file's **128** data rows (the other 14 are +//! ARRAYFORMULA's, and a 115th recorded row sits in `bugs.tsv` — see below). +//! `tests/sparkline.rs` asserts the rows the TSV runner skips. +//! +//! One recorded row is a known engine divergence rather than a passing case: +//! `=MIN(SPARKLINE({1,2,3}),{})` is `#REF!` in Sheets and `0` here, because +//! `MIN` has no empty-array rule at all (`=MIN({})` is `0` on main, while +//! `=MAX({})` is `#REF!`). That gap predates sparklines and is unrelated to +//! them, so it lives in `bugs.tsv` awaiting its own fix rather than being +//! patched from here. +//! +//! Why not reason it out instead: `DOLLAR(sparkline)` is `#VALUE!` while +//! `TO_DOLLARS(sparkline)` is `""`. Near-identical names, both format a number +//! as currency text, opposite answers. +//! +//! So: **if you need the answer for a function, find its row. If it has no row, +//! probe it.** Do not infer one from its seam or from a function that resembles +//! it. +//! +//! Validation splits into three distinct error classes: +//! +//! - `#N/A` — the arity/shape of `data`: no arguments, a scalar instead of a +//! range, or a single value. +//! - `#REF!` — structural malformation: an empty array, or an `options` array +//! that is not key/value pairs. +//! - `#VALUE!` — a bad option *value* (an unrecognised `charttype`). +//! +//! An unrecognised option **key** is *not* an error — Sheets ignores it, which +//! is what lets a workbook written against a newer option set still evaluate. +//! A genuine blank cell inside the source range is likewise fine: it renders. + +use crate::display::display_number; +use crate::eval::functions::{check_arity, FunctionMeta, Registry}; +use crate::types::{ErrorKind, SparklineChartType, SparklineSpec, SparklineValue, Value}; + +/// The `charttype` option key, lifted out of the generic option list. +const CHART_TYPE_KEY: &str = "charttype"; + +/// Convert one evaluated cell into a plotted point / option value. +/// +/// `Zoned` and a nested sparkline have no Google Sheets analogue, so there is no +/// ground truth for them; they are rejected with `#VALUE!` rather than given an +/// invented projection. Those two arms are reachable only for a cell *inside* +/// an array — a sparkline handed straight to `data` never gets here, because +/// [`parse_data`]'s non-array check answers `#N/A` first (the same answer as any +/// other scalar `data` argument). +fn to_sparkline_value(v: &Value) -> Result { + match v { + Value::Number(n) | Value::Date(n) => Ok(SparklineValue::number(*n)), + Value::Text(s) => Ok(SparklineValue::Text(s.clone())), + Value::Bool(b) => Ok(SparklineValue::Bool(*b)), + Value::Empty => Ok(SparklineValue::Blank), + Value::Error(_) | Value::ErrorMsg(_, _) => Err(v.clone()), + Value::Array(_) | Value::Zoned(_) | Value::Sparkline(_) => { + Err(Value::Error(ErrorKind::Value)) + } + } +} + +/// Flatten a (possibly row-nested) array into its cells, row-major. +fn flatten<'a>(v: &'a Value, out: &mut Vec<&'a Value>) { + match v { + Value::Array(elems) => elems.iter().for_each(|e| flatten(e, out)), + other => out.push(other), + } +} + +/// Parse and validate the `data` argument. +/// +/// `#N/A` for a scalar or a single value, `#REF!` for an empty array. +fn parse_data(v: &Value) -> Result, Value> { + if !matches!(v, Value::Array(_)) { + return Err(Value::Error(ErrorKind::NA)); + } + let mut cells = Vec::new(); + flatten(v, &mut cells); + match cells.len() { + 0 => return Err(Value::Error(ErrorKind::Ref)), + 1 => return Err(Value::Error(ErrorKind::NA)), + _ => {} + } + cells.iter().map(|c| to_sparkline_value(c)).collect() +} + +/// The option-key projection of a cell. Keys are matched case-insensitively, +/// so they are stored ASCII-lower-cased. +fn option_key(v: &Value) -> Result { + let key = match v { + Value::Text(s) => s.clone(), + Value::Number(n) | Value::Date(n) => display_number(*n), + Value::Bool(b) => (if *b { "TRUE" } else { "FALSE" }).to_string(), + Value::Empty => String::new(), + Value::Error(_) | Value::ErrorMsg(_, _) => return Err(v.clone()), + Value::Array(_) | Value::Zoned(_) | Value::Sparkline(_) => { + return Err(Value::Error(ErrorKind::Ref)) + } + }; + Ok(key.to_ascii_lowercase()) +} + +/// Split the `options` argument into key/value pairs. +/// +/// Anything that is not a well-formed pair list — a scalar, an empty array, a +/// row that is not two cells wide, an odd-length flat array — is `#REF!`. +fn option_pairs(v: &Value) -> Result, Value> { + let Value::Array(elems) = v else { + return Err(Value::Error(ErrorKind::Ref)); + }; + if elems.is_empty() { + return Err(Value::Error(ErrorKind::Ref)); + } + let rows = elems.iter().filter(|e| matches!(e, Value::Array(_))).count(); + if rows == elems.len() { + // `{"charttype","line";"color","red"}` — one key/value pair per row. + let mut pairs = Vec::with_capacity(elems.len()); + for row in elems { + let Value::Array(cells) = row else { unreachable!() }; + if cells.len() != 2 { + return Err(Value::Error(ErrorKind::Ref)); + } + pairs.push((&cells[0], &cells[1])); + } + Ok(pairs) + } else if rows == 0 { + // `{"bogus","x"}` — a single flat row of key/value cells. + if elems.len() % 2 != 0 { + return Err(Value::Error(ErrorKind::Ref)); + } + Ok(elems.chunks(2).map(|p| (&p[0], &p[1])).collect()) + } else { + // Rows mixed with bare cells is not a pair list. + Err(Value::Error(ErrorKind::Ref)) + } +} + +/// `SPARKLINE(data, [options])` — build the render spec for an in-cell chart. +pub fn sparkline_fn(args: &[Value]) -> Value { + if let Some(err) = check_arity(args, 1, 2) { + return err; + } + + let data = match parse_data(&args[0]) { + Ok(data) => data, + Err(e) => return e, + }; + + let mut chart_type = SparklineChartType::Line; + let mut options = Vec::new(); + if let Some(raw_options) = args.get(1) { + let pairs = match option_pairs(raw_options) { + Ok(pairs) => pairs, + Err(e) => return e, + }; + for (raw_key, raw_value) in pairs { + let key = match option_key(raw_key) { + Ok(key) => key, + Err(e) => return e, + }; + let value = match to_sparkline_value(raw_value) { + Ok(value) => value, + Err(e) => return e, + }; + if key == CHART_TYPE_KEY { + // A bad option *value* is `#VALUE!` (a bad option *key* is not + // an error at all — it lands in `options` untouched). + let SparklineValue::Text(ref name) = value else { + return Value::Error(ErrorKind::Value); + }; + match SparklineChartType::parse(name) { + Some(t) => chart_type = t, + None => return Value::Error(ErrorKind::Value), + } + } else { + options.push((key, value)); + } + } + } + + Value::Sparkline(Box::new(SparklineSpec { + chart_type, + data, + options, + })) +} + +pub fn register_google(registry: &mut Registry) { + registry.register_eager( + "SPARKLINE", + sparkline_fn, + FunctionMeta { + category: "google", + signature: "SPARKLINE(data, [options])", + description: "Miniature in-cell chart over a range, as a render spec", + }, + ); +} diff --git a/crates/core/src/eval/functions/logical/info/mod.rs b/crates/core/src/eval/functions/logical/info/mod.rs index 9bc5fa0fa..536feafa6 100644 --- a/crates/core/src/eval/functions/logical/info/mod.rs +++ b/crates/core/src/eval/functions/logical/info/mod.rs @@ -52,6 +52,9 @@ pub fn n_fn(args: &[Expr], ctx: &mut EvalCtx<'_>) -> Value { Value::Number(n) | Value::Date(n) => Value::Number(n), Value::Bool(b) => Value::Number(if b { 1.0 } else { 0.0 }), Value::Empty | Value::Text(_) | Value::Array(_) => Value::Number(0.0), + // `=N(SPARKLINE({1,2,3}))` is 0 (google.tsv) even though arithmetic on + // a sparkline is `#VALUE!` — the asymmetry is Sheets', not ours. + Value::Sparkline(_) => Value::Number(0.0), Value::Zoned(_) => Value::Error(ErrorKind::Value), Value::Error(_) | Value::ErrorMsg(_, _) => val, } @@ -72,6 +75,9 @@ pub fn type_fn(args: &[Expr], ctx: &mut EvalCtx<'_>) -> Value { Value::Array(_) => 64.0, Value::Empty => 1.0, // Excel treats empty as number Value::Zoned(_) => 1.0, // classify like Number/Date for the TYPE code + // 128 is outside TYPE's documented set (1/2/4/16/64): Sheets reports a + // sparkline as a value kind of its own (google.tsv). + Value::Sparkline(_) => 128.0, }; Value::Number(code) } @@ -152,7 +158,7 @@ pub fn cell_fn(args: &[Expr], ctx: &mut EvalCtx<'_>) -> Value { Value::Zoned(_) => Value::Text("n".to_string()), Value::Error(e) => Value::Error(e), Value::ErrorMsg(e, m) => Value::ErrorMsg(e, m), - Value::Array(_) => Value::Error(ErrorKind::NA), + Value::Array(_) | Value::Sparkline(_) => Value::Error(ErrorKind::NA), } } "col" => { diff --git a/crates/core/src/eval/functions/math/average/mod.rs b/crates/core/src/eval/functions/math/average/mod.rs index 37625804b..c3de22450 100644 --- a/crates/core/src/eval/functions/math/average/mod.rs +++ b/crates/core/src/eval/functions/math/average/mod.rs @@ -24,6 +24,8 @@ pub fn average_fn(args: &[Value]) -> Value { } } Value::Empty => {} // skip + // Aggregates skip a sparkline (google.tsv: SUM/MAX skip it). + Value::Sparkline(_) => {} Value::Zoned(_) => return Value::Error(ErrorKind::Value), Value::Error(_) | Value::ErrorMsg(_, _) => return arg.clone(), Value::Array(elems) => { diff --git a/crates/core/src/eval/functions/math/countunique/mod.rs b/crates/core/src/eval/functions/math/countunique/mod.rs index c96989b91..ac14b2da0 100644 --- a/crates/core/src/eval/functions/math/countunique/mod.rs +++ b/crates/core/src/eval/functions/math/countunique/mod.rs @@ -10,6 +10,15 @@ enum UniqueKey { Text(String), // case-sensitive per GS Bool(bool), ErrorVal(String), // GS: errors are counted as unique, not propagated + /// A sparkline, keyed by its whole parsed spec. `=` reports any two + /// sparklines equal, but COUNTUNIQUE does not — google.tsv records + /// `=COUNTUNIQUE(SPARKLINE({1,2,3}),SPARKLINE({9,9,9}))` as 2 and the same + /// call on two identical sparklines as 1, so uniqueness keys off something + /// deeper than the comparison operator does. Every option the spec kept + /// participates, recognised or not: an unrecognised key is not dropped, so + /// `{"bogus","x"}` and `{"bogus","y"}` count as two, exactly like two + /// different values of a recognised key would. + SparklineVal(String), } fn to_unique_key(v: &Value) -> Option { @@ -22,6 +31,7 @@ fn to_unique_key(v: &Value) -> Option { Value::ErrorMsg(e, _) => Some(UniqueKey::ErrorVal(format!("{e:?}"))), Value::Date(_) | Value::Array(_) => None, Value::Zoned(_) => None, + Value::Sparkline(spec) => Some(UniqueKey::SparklineVal(format!("{spec:?}"))), } } diff --git a/crates/core/src/eval/functions/math/product/mod.rs b/crates/core/src/eval/functions/math/product/mod.rs index 5e33d73b0..ba48e8d0a 100644 --- a/crates/core/src/eval/functions/math/product/mod.rs +++ b/crates/core/src/eval/functions/math/product/mod.rs @@ -7,12 +7,24 @@ pub fn product_fn(args: &[Value]) -> Value { return err; } let mut product = 1.0_f64; + let mut contributed = false; for arg in args { match product_top_level(arg) { Err(e) => return e, - Ok(n) => product *= n, + // A skipped argument is *absent*, not a factor of 1: with nothing + // left to multiply, PRODUCT is 0, not the multiplicative identity + // (google.tsv: `=PRODUCT(SPARKLINE({1,2,3}))` is 0, matching SUM, + // MAX and COUNT of a lone sparkline, while `=PRODUCT(S,3)` is 3). + Ok(None) => {} + Ok(Some(n)) => { + product *= n; + contributed = true; + } } } + if !contributed { + return Value::Number(0.0); + } if !product.is_finite() { return Value::Error(ErrorKind::Num); } @@ -21,27 +33,51 @@ pub fn product_fn(args: &[Value]) -> Value { /// Top-level: arrays use array-context (booleans/text skipped). /// Direct scalars use full coercion. -fn product_top_level(v: &Value) -> Result { +/// +/// `Ok(None)` means the argument contributed no factor at all — only a +/// sparkline does that, and only as a direct argument. An array still yields +/// `Ok(Some(_))` even when array-context rules skipped everything inside it, +/// which is pre-existing behaviour this does not disturb. +fn product_top_level(v: &Value) -> Result, Value> { match v { Value::Array(_) => product_array_value(v), - other => to_number(other.clone()), + // Aggregates skip a sparkline in any position, direct argument or not + // (google.tsv: `=PRODUCT(SPARKLINE({1,2,3}),3)` is 3 and + // `=PRODUCT(SPARKLINE({1,2,3}))` is 0). + Value::Sparkline(_) => Ok(None), + other => to_number(other.clone()).map(Some), } } /// Array-context: booleans, text, empty silently skipped (contribute 1). -fn product_array_value(v: &Value) -> Result { +/// +/// `Ok(None)` means "nothing here contributed a factor", which only a sparkline +/// produces — an aggregate's answer must not depend on whether the sparkline +/// arrived directly or through a range (google.tsv: `=PRODUCT(K1:K1)` over a +/// cell holding a sparkline is 0, the same as `=PRODUCT(SPARKLINE({1,2,3}))`). +/// Booleans, text and blanks keep contributing the identity factor 1 as before, +/// and an empty array is not a skip. +fn product_array_value(v: &Value) -> Result, Value> { match v { Value::Array(elems) => { + if elems.is_empty() { + return Ok(Some(1.0)); + } let mut p = 1.0_f64; + let mut contributed = false; for elem in elems { - p *= product_array_value(elem)?; + if let Some(n) = product_array_value(elem)? { + p *= n; + contributed = true; + } } - Ok(p) + Ok(if contributed { Some(p) } else { None }) } - Value::Bool(_) | Value::Text(_) | Value::Empty => Ok(1.0), - Value::Zoned(_) => Ok(1.0), + Value::Bool(_) | Value::Text(_) | Value::Empty => Ok(Some(1.0)), + Value::Zoned(_) => Ok(Some(1.0)), + Value::Sparkline(_) => Ok(None), Value::Error(_) | Value::ErrorMsg(_, _) => Err(v.clone()), - Value::Number(n) | Value::Date(n) => Ok(*n), + Value::Number(n) | Value::Date(n) => Ok(Some(*n)), } } diff --git a/crates/core/src/eval/functions/math/sum/mod.rs b/crates/core/src/eval/functions/math/sum/mod.rs index 0e3456aa9..dcdd6f39b 100644 --- a/crates/core/src/eval/functions/math/sum/mod.rs +++ b/crates/core/src/eval/functions/math/sum/mod.rs @@ -37,6 +37,9 @@ fn sum_top_level(v: &Value) -> Result { s.trim().parse::() .map_err(|_| Value::Error(crate::types::ErrorKind::Value)) } + // Aggregates skip a sparkline rather than erroring on it, direct + // argument or not (google.tsv: `=SUM(SPARKLINE({1,2,3}),1)` is 1). + Value::Sparkline(_) => Ok(0.0), // Direct non-array arg: full to_number coercion (Bool -> 0/1) other => to_number(other.clone()), } @@ -55,8 +58,8 @@ fn sum_array_value(v: &Value) -> Result { } // In array context: booleans and text are silently skipped Value::Bool(_) | Value::Text(_) | Value::Empty => Ok(0.0), - // In array context: zoned instants are silently skipped - Value::Zoned(_) => Ok(0.0), + // In array context: zoned instants and sparklines are silently skipped + Value::Zoned(_) | Value::Sparkline(_) => Ok(0.0), // Errors propagate Value::Error(_) | Value::ErrorMsg(_, _) => Err(v.clone()), // Numbers and Dates contribute their value diff --git a/crates/core/src/eval/functions/math/sumsq/mod.rs b/crates/core/src/eval/functions/math/sumsq/mod.rs index 4f5c7fd3e..363ea81cb 100644 --- a/crates/core/src/eval/functions/math/sumsq/mod.rs +++ b/crates/core/src/eval/functions/math/sumsq/mod.rs @@ -44,6 +44,9 @@ fn sumsq_value(v: &Value, in_array: bool) -> Result { if let Ok(n) = s.trim().parse::() { Ok(n * n) } else { Err(Value::Error(crate::types::ErrorKind::Value)) } } + // Aggregates skip a sparkline in any position (google.tsv: SUM and MAX + // both skip a direct sparkline argument). + Value::Sparkline(_) => Ok(0.0), Value::Zoned(_) if in_array => Ok(0.0), // skipped in array context Value::Zoned(_) => Err(Value::Error(crate::types::ErrorKind::Value)), Value::Error(_) | Value::ErrorMsg(_, _) => Err(v.clone()), diff --git a/crates/core/src/eval/functions/mod.rs b/crates/core/src/eval/functions/mod.rs index dcaf2dc12..8d29151f0 100644 --- a/crates/core/src/eval/functions/mod.rs +++ b/crates/core/src/eval/functions/mod.rs @@ -4,6 +4,7 @@ pub mod date; pub mod engineering; pub mod filter; pub mod financial; +pub mod google; pub mod logical; pub mod lookup; pub mod math; @@ -248,6 +249,7 @@ impl Registry { database::register_database(&mut r); lookup::register_lookup(&mut r); query::register_query(&mut r); + google::register_google(&mut r); web::register_web(&mut r); timezone::register_timezone(&mut r); r diff --git a/crates/core/src/eval/functions/operator/mod.rs b/crates/core/src/eval/functions/operator/mod.rs index 2341ce355..d121d1d07 100644 --- a/crates/core/src/eval/functions/operator/mod.rs +++ b/crates/core/src/eval/functions/operator/mod.rs @@ -117,6 +117,9 @@ fn type_rank(v: &Value) -> u8 { Value::Text(_) => 1, Value::Bool(_) => 2, Value::Zoned(_) => 3, + // A sparkline outranks every scalar, matching `crate::eval::type_rank` + // (google.tsv: `>1`, `>"zzzz"` and `>TRUE` are all TRUE). + Value::Sparkline(_) => 4, _ => 255, } } @@ -132,6 +135,11 @@ fn compare_values(a: &Value, b: &Value) -> std::cmp::Ordering { // Zoned instants order on the absolute instant only (same moment in a // different zone compares equal). Cross-type Zoned is rejected upstream. (Value::Zoned(x), Value::Zoned(y)) => x.utc_nanos.cmp(&y.utc_nanos), + // Any two sparklines compare equal, whatever they plot — the same answer + // `crate::eval::compare_values` gives the `=`/`<`/`>` operators these + // functions are Sheets' aliases for (google.tsv records `EQ` TRUE, `NE` + // FALSE and `GTE` TRUE across two *different* sparklines). + (Value::Sparkline(_), Value::Sparkline(_)) => std::cmp::Ordering::Equal, _ => type_rank(a).cmp(&type_rank(b)), } } @@ -145,6 +153,7 @@ fn same_type(a: &Value, b: &Value) -> bool { | (Value::Bool(_), Value::Bool(_)) | (Value::Empty, Value::Empty) | (Value::Zoned(_), Value::Zoned(_)) + | (Value::Sparkline(_), Value::Sparkline(_)) ) } diff --git a/crates/core/src/eval/functions/parser/to_date/mod.rs b/crates/core/src/eval/functions/parser/to_date/mod.rs index c57500757..6b69b5181 100644 --- a/crates/core/src/eval/functions/parser/to_date/mod.rs +++ b/crates/core/src/eval/functions/parser/to_date/mod.rs @@ -10,6 +10,8 @@ pub fn to_date_fn(args: &[Value]) -> Value { Value::Date(n) => Value::Date(*n), Value::Text(s) => Value::Text(s.clone()), Value::Error(_) | Value::ErrorMsg(_, _) => args[0].clone(), + // `TO_*` family behaviour — see `super::to_text::to_text_fn`. + Value::Sparkline(_) => Value::Text(String::new()), _ => Value::Error(ErrorKind::Value), } } diff --git a/crates/core/src/eval/functions/parser/to_dollars/mod.rs b/crates/core/src/eval/functions/parser/to_dollars/mod.rs index 93ee34c65..26b0b2eae 100644 --- a/crates/core/src/eval/functions/parser/to_dollars/mod.rs +++ b/crates/core/src/eval/functions/parser/to_dollars/mod.rs @@ -10,6 +10,8 @@ pub fn to_dollars_fn(args: &[Value]) -> Value { Value::Bool(b) => Value::Bool(*b), Value::Text(s) => Value::Text(s.clone()), Value::Error(_) | Value::ErrorMsg(_, _) => args[0].clone(), + // `TO_*` family behaviour — see `super::to_text::to_text_fn`. + Value::Sparkline(_) => Value::Text(String::new()), _ => Value::Error(ErrorKind::Value), } } diff --git a/crates/core/src/eval/functions/parser/to_percent/mod.rs b/crates/core/src/eval/functions/parser/to_percent/mod.rs index 8e30b26c0..0e942271e 100644 --- a/crates/core/src/eval/functions/parser/to_percent/mod.rs +++ b/crates/core/src/eval/functions/parser/to_percent/mod.rs @@ -10,6 +10,8 @@ pub fn to_percent_fn(args: &[Value]) -> Value { Value::Bool(b) => Value::Bool(*b), Value::Text(s) => Value::Text(s.clone()), Value::Error(_) | Value::ErrorMsg(_, _) => args[0].clone(), + // `TO_*` family behaviour — see `super::to_text::to_text_fn`. + Value::Sparkline(_) => Value::Text(String::new()), _ => Value::Error(ErrorKind::Value), } } diff --git a/crates/core/src/eval/functions/parser/to_pure_number/mod.rs b/crates/core/src/eval/functions/parser/to_pure_number/mod.rs index 48b150145..0a2056e54 100644 --- a/crates/core/src/eval/functions/parser/to_pure_number/mod.rs +++ b/crates/core/src/eval/functions/parser/to_pure_number/mod.rs @@ -11,6 +11,8 @@ pub fn to_pure_number_fn(args: &[Value]) -> Value { Value::Bool(b) => Value::Bool(*b), Value::Text(s) => Value::Text(s.clone()), Value::Error(_) | Value::ErrorMsg(_, _) => args[0].clone(), + // `TO_*` family behaviour — see `super::to_text::to_text_fn`. + Value::Sparkline(_) => Value::Text(String::new()), _ => Value::Error(ErrorKind::Value), } } diff --git a/crates/core/src/eval/functions/parser/to_text/mod.rs b/crates/core/src/eval/functions/parser/to_text/mod.rs index ac3f8fbd0..d77dfa957 100644 --- a/crates/core/src/eval/functions/parser/to_text/mod.rs +++ b/crates/core/src/eval/functions/parser/to_text/mod.rs @@ -40,6 +40,14 @@ pub fn to_text_fn(args: &[Value]) -> Value { Value::Bool(b) => Value::Text(if *b { "TRUE".to_string() } else { "FALSE".to_string() }), Value::Text(s) => Value::Text(s.clone()), Value::Error(_) | Value::ErrorMsg(_, _) => args[0].clone(), + // Canonical statement for the whole `TO_*` family (`TO_TEXT`, + // `TO_PERCENT`, `TO_DOLLARS`, `TO_PURE_NUMBER`, `TO_DATE`): every member + // reads a sparkline as the empty string (google.tsv). The family is + // uniform, but nothing *outside* it can be inferred from that — + // `DOLLAR` rejects a sparkline while `TO_DOLLARS` does not. The other + // members point here rather than restating it; see also the coercion + // section of `crate::eval::functions::google`. + Value::Sparkline(_) => Value::Text(String::new()), _ => Value::Error(ErrorKind::Value), } } diff --git a/crates/core/src/eval/functions/statistical/distributions_impl.rs b/crates/core/src/eval/functions/statistical/distributions_impl.rs index 740d73aae..8a0ba822a 100644 --- a/crates/core/src/eval/functions/statistical/distributions_impl.rs +++ b/crates/core/src/eval/functions/statistical/distributions_impl.rs @@ -96,6 +96,8 @@ fn collect_weights_arg(arg: &Value) -> Result, Value> { _ => Err(Value::Error(ErrorKind::Value)), }, Value::Empty => Ok(vec![]), + // Skipped like every other aggregate input (google.tsv: SUM/MAX skip it). + Value::Sparkline(_) => Ok(vec![]), Value::Zoned(_) => Err(Value::Error(ErrorKind::Value)), Value::Error(_) | Value::ErrorMsg(_, _) => Err(arg.clone()), Value::Array(inner) => { @@ -106,7 +108,7 @@ fn collect_weights_arg(arg: &Value) -> Result, Value> { Value::Date(n) => out.push(*n), Value::Bool(_) => return Err(Value::Error(ErrorKind::Value)), Value::Text(_) | Value::Empty => {} - Value::Zoned(_) => {} + Value::Zoned(_) | Value::Sparkline(_) => {} Value::Error(_) | Value::ErrorMsg(_, _) => return Err(item.clone()), Value::Array(_) => {} } diff --git a/crates/core/src/eval/functions/statistical/max/mod.rs b/crates/core/src/eval/functions/statistical/max/mod.rs index ed5ede774..6318ef65b 100644 --- a/crates/core/src/eval/functions/statistical/max/mod.rs +++ b/crates/core/src/eval/functions/statistical/max/mod.rs @@ -15,8 +15,10 @@ pub fn max_fn(args: &[Value]) -> Value { } let mut result: Option = None; let mut had_array = false; + let mut skipped_sparkline = false; for arg in args { match arg { + Value::Sparkline(_) => skipped_sparkline = true, Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } @@ -42,7 +44,7 @@ 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) { + if let Err(e) = max_array_into(elems, &mut result, &mut skipped_sparkline) { return e; } } @@ -51,6 +53,13 @@ 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!`. + if skipped_sparkline && result.is_none() { + return Value::Number(0.0); + } // Empty array with no numbers → Ref if had_array && result.is_none() { return Value::Error(ErrorKind::Ref); @@ -60,15 +69,25 @@ pub fn max_fn(args: &[Value]) -> Value { /// Recursively fold a nested array's numbers into `result` for MAX's /// array-context rules (Bool/Text/Empty skipped, errors propagate). -fn max_array_into(elems: &[Value], result: &mut Option) -> Result<(), Value> { +/// A sparkline is skipped wherever it appears, and an aggregate whose scope +/// holds nothing else answers 0 — the same answer whether it arrived as a +/// 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. +fn max_array_into( + elems: &[Value], + result: &mut Option, + skipped_sparkline: &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::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)?, + Value::Array(inner) => max_array_into(inner, result, skipped_sparkline)?, _ => {} } } diff --git a/crates/core/src/eval/functions/statistical/maxa/mod.rs b/crates/core/src/eval/functions/statistical/maxa/mod.rs index 43d0cdfc7..9756b7f3c 100644 --- a/crates/core/src/eval/functions/statistical/maxa/mod.rs +++ b/crates/core/src/eval/functions/statistical/maxa/mod.rs @@ -11,8 +11,11 @@ pub fn maxa_fn(args: &[Value]) -> Value { return Value::Error(ErrorKind::NA); } let mut result: Option = None; + // See `fold_array_max` for why this flag exists. + let mut skipped_sparkline = false; for arg in args { match arg { + Value::Sparkline(_) => skipped_sparkline = true, Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.max(*n))); } @@ -26,7 +29,7 @@ pub fn maxa_fn(args: &[Value]) -> Value { // 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). - if let Err(e) = fold_array_max(inner, &mut result) { + if let Err(e) = fold_array_max(inner, &mut result, &mut skipped_sparkline) { return e; } } @@ -37,6 +40,7 @@ pub fn maxa_fn(args: &[Value]) -> Value { } match result { Some(n) => Value::Number(n), + None if skipped_sparkline => Value::Number(0.0), None => Value::Error(ErrorKind::NA), } } @@ -44,15 +48,28 @@ pub fn maxa_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, folding into `result` /// with MAXA's array-context coercion rules. -fn fold_array_max(arr: &[Value], result: &mut Option) -> Result<(), Value> { +/// A sparkline is skipped wherever it appears, and an aggregate whose scope +/// holds nothing else answers 0 — the same answer whether it arrived as a +/// direct argument or through a range (google.tsv: `=MAXA(SPARKLINE({1,2,3}))` +/// and `=MAXA(Data!K1:K1)` are both 0). The flag is what distinguishes "skipped a +/// sparkline" from "saw nothing usable at all", which stay different answers. +fn fold_array_max( + arr: &[Value], + result: &mut Option, + skipped_sparkline: &mut bool, +) -> Result<(), Value> { for v in arr { let n = match v { + Value::Sparkline(_) => { + *skipped_sparkline = true; + continue; + } Value::Number(n) => *n, Value::Bool(b) => if *b { 1.0 } else { 0.0 }, Value::Text(_) => 0.0, Value::Empty => continue, Value::Array(inner) => { - fold_array_max(inner, result)?; + fold_array_max(inner, result, skipped_sparkline)?; continue; } Value::Error(e) => return Err(Value::Error(e.clone())), diff --git a/crates/core/src/eval/functions/statistical/mina/mod.rs b/crates/core/src/eval/functions/statistical/mina/mod.rs index a414aa317..1d4ab3680 100644 --- a/crates/core/src/eval/functions/statistical/mina/mod.rs +++ b/crates/core/src/eval/functions/statistical/mina/mod.rs @@ -11,8 +11,11 @@ pub fn mina_fn(args: &[Value]) -> Value { return Value::Error(ErrorKind::NA); } let mut result: Option = None; + // See `fold_array_min` for why this flag exists. + let mut skipped_sparkline = false; for arg in args { match arg { + Value::Sparkline(_) => skipped_sparkline = true, Value::Number(n) => { result = Some(result.map_or(*n, |cur: f64| cur.min(*n))); } @@ -26,7 +29,7 @@ pub fn mina_fn(args: &[Value]) -> Value { // 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). - if let Err(e) = fold_array_min(inner, &mut result) { + if let Err(e) = fold_array_min(inner, &mut result, &mut skipped_sparkline) { return e; } } @@ -37,6 +40,7 @@ pub fn mina_fn(args: &[Value]) -> Value { } match result { Some(n) => Value::Number(n), + None if skipped_sparkline => Value::Number(0.0), None => Value::Error(ErrorKind::NA), } } @@ -44,15 +48,28 @@ pub fn mina_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, folding into `result` /// with MINA's array-context coercion rules. -fn fold_array_min(arr: &[Value], result: &mut Option) -> Result<(), Value> { +/// A sparkline is skipped wherever it appears, and an aggregate whose scope +/// holds nothing else answers 0 — the same answer whether it arrived as a +/// direct argument or through a range (google.tsv: `=MINA(SPARKLINE({1,2,3}))` +/// and `=MINA(Data!K1:K1)` are both 0). The flag is what distinguishes "skipped a +/// sparkline" from "saw nothing usable at all", which stay different answers. +fn fold_array_min( + arr: &[Value], + result: &mut Option, + skipped_sparkline: &mut bool, +) -> Result<(), Value> { for v in arr { let n = match v { + Value::Sparkline(_) => { + *skipped_sparkline = true; + continue; + } Value::Number(n) => *n, Value::Bool(b) => if *b { 1.0 } else { 0.0 }, Value::Text(_) => 0.0, Value::Empty => continue, Value::Array(inner) => { - fold_array_min(inner, result)?; + fold_array_min(inner, result, skipped_sparkline)?; continue; } Value::Error(e) => return Err(Value::Error(e.clone())), diff --git a/crates/core/src/eval/functions/statistical/stat_helpers.rs b/crates/core/src/eval/functions/statistical/stat_helpers.rs index e91bec1e8..c8d3040e4 100644 --- a/crates/core/src/eval/functions/statistical/stat_helpers.rs +++ b/crates/core/src/eval/functions/statistical/stat_helpers.rs @@ -39,7 +39,7 @@ pub fn zoned_extreme(args: &[Value], want_min: bool) -> Option { *error = Some(v.clone()); } } - Value::Empty => {} + Value::Empty | Value::Sparkline(_) => {} Value::Array(elems) => { for e in elems { walk(e, best, saw_zoned, saw_numeric, error, want_min); @@ -122,7 +122,7 @@ fn collect_nums_a_into_checked(args: &[Value], out: &mut Vec) -> Option return Some(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Some(Value::ErrorMsg(e.clone(), m.clone())), Value::Empty => {} - Value::Zoned(_) => {} + Value::Zoned(_) | Value::Sparkline(_) => {} } } None @@ -171,6 +171,9 @@ pub fn collect_nums_a_checked(args: &[Value]) -> Result, Value> { return Err(err); } } + // Aggregates skip a sparkline (google.tsv: + // `=MAX(SPARKLINE({1,2,3}),1)` and `=SUM(...)` are both 1). + Value::Sparkline(_) => {} Value::Zoned(_) => return Err(Value::Error(ErrorKind::Value)), Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), @@ -219,6 +222,9 @@ pub fn collect_nums_direct(args: &[Value]) -> Result, Value> { return Err(err); } } + // Aggregates skip a sparkline (google.tsv: + // `=MAX(SPARKLINE({1,2,3}),1)` and `=SUM(...)` are both 1). + Value::Sparkline(_) => {} Value::Zoned(_) => return Err(Value::Error(ErrorKind::Value)), Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), @@ -268,6 +274,9 @@ pub fn collect_nums_a_direct(args: &[Value]) -> Result, Value> { // Array context: bools coerce, text→0 collect_nums_a_into(inner, &mut nums); } + // Aggregates skip a sparkline (google.tsv: + // `=MAX(SPARKLINE({1,2,3}),1)` and `=SUM(...)` are both 1). + Value::Sparkline(_) => {} Value::Zoned(_) => return Err(Value::Error(ErrorKind::Value)), Value::Error(e) => return Err(Value::Error(e.clone())), Value::ErrorMsg(e, m) => return Err(Value::ErrorMsg(e.clone(), m.clone())), @@ -294,7 +303,7 @@ pub fn collect_nums_a_into(args: &[Value], out: &mut Vec) { Value::Array(inner) => collect_nums_a_into(inner, out), Value::Empty => {} Value::Error(_) | Value::ErrorMsg(_, _) => {} - Value::Zoned(_) => {} + Value::Zoned(_) | Value::Sparkline(_) => {} } } } diff --git a/crates/core/src/eval/functions/text/text_fn/mod.rs b/crates/core/src/eval/functions/text/text_fn/mod.rs index f0484f7ac..07ba3b3e0 100644 --- a/crates/core/src/eval/functions/text/text_fn/mod.rs +++ b/crates/core/src/eval/functions/text/text_fn/mod.rs @@ -299,6 +299,20 @@ pub fn text_fn(args: &[Value]) -> Value { if let Value::Bool(b) = raw { return Value::Text(if b { "TRUE".to_string() } else { "FALSE".to_string() }); } + // google.tsv: `=TEXT(SPARKLINE({1,2,3}),"0")` is the empty string, so TEXT + // needs its own answer — it reads its value as a number, which a sparkline + // is not. Note this is *not* "as if empty text" either: `TEXT("","0")` + // formats as "0". + // + // There is no rule to derive this from: the whole `TO_*` family also answers + // "" while `DOLLAR` and `FIXED` answer `#VALUE!` — `DOLLAR` and + // `TO_DOLLARS` differ despite the names. The set of exceptions is + // empirical: see the coercion map in `crate::eval::functions::google` for + // which functions were probed in which direction, and extend that list from + // the oracle rather than by analogy. + if matches!(raw, Value::Sparkline(_)) { + return Value::Text(String::new()); + } let n = match &raw { Value::Date(d) => *d, Value::Number(n) => *n, diff --git a/crates/core/src/eval/mod.rs b/crates/core/src/eval/mod.rs index 321332565..f979f9e83 100644 --- a/crates/core/src/eval/mod.rs +++ b/crates/core/src/eval/mod.rs @@ -267,6 +267,9 @@ fn type_rank(v: &Value) -> u8 { // Error and Array cannot reach compare_values through the normal eval path // (eval_binary guards against errors before calling compare_values). Value::Error(_) | Value::ErrorMsg(_, _) | Value::Array(_) => 3, + // A sparkline outranks every scalar (google.tsv: `>1`, `>"zzzz"` and + // `>TRUE` are all TRUE, and `=SPARKLINE(...)=""` is FALSE). + Value::Sparkline(_) => 4, } } @@ -351,6 +354,14 @@ fn eval_binary(op: &BinaryOp, lv: Value, rv: Value) -> Value { // ── Concatenation ─────────────────────────────────────────────────── BinaryOp::Concat => { + // The `&` *operator* rejects a sparkline (google.tsv: + // `="x"&SPARKLINE({1,2,3})` is `#VALUE!`) even though `CONCATENATE` + // of the same value concatenates it as empty text. That asymmetry + // is Sheets'; it lives here because every other text context goes + // through the permissive `to_string_val`. + if matches!(lv, Value::Sparkline(_)) || matches!(rv, Value::Sparkline(_)) { + return Value::Error(ErrorKind::Value); + } let ls = match to_string_val(lv) { Ok(s) => s, Err(e) => return e }; let rs = match to_string_val(rv) { Ok(s) => s, Err(e) => return e }; Value::Text(ls + &rs) @@ -385,6 +396,12 @@ fn compare_values(op: &BinaryOp, lv: &Value, rv: &Value) -> bool { // Zoned instants compare on the absolute instant only (same moment in a // different zone compares equal). Cross-type Zoned is rejected in eval_binary. (Value::Zoned(a), Value::Zoned(b)) => apply_cmp(op, Some(a.utc_nanos.cmp(&b.utc_nanos))), + // Any two sparklines compare equal, whatever they plot (google.tsv: + // `=SPARKLINE({1,2,3})=SPARKLINE({9,9,9})` is TRUE, `<>` is FALSE, and + // `<`/`>` between two sparklines are both FALSE while `>=` is TRUE). + (Value::Sparkline(_), Value::Sparkline(_)) => { + apply_cmp(op, Some(std::cmp::Ordering::Equal)) + } (Value::Text(a), Value::Text(b)) => apply_cmp(op, Some(a.cmp(b))), (Value::Bool(a), Value::Bool(b)) => apply_cmp(op, Some(a.cmp(b))), (Value::Empty, Value::Empty) => apply_cmp(op, Some(std::cmp::Ordering::Equal)), diff --git a/crates/core/src/types/mod.rs b/crates/core/src/types/mod.rs index c9bb2be20..50ec6b706 100644 --- a/crates/core/src/types/mod.rs +++ b/crates/core/src/types/mod.rs @@ -1,7 +1,9 @@ pub mod error; +pub mod sparkline; pub mod value; pub mod zoned; pub use error::{ErrorKind, ParseError}; +pub use sparkline::{SparklineChartType, SparklineSpec, SparklineValue}; pub use value::Value; pub use zoned::{AmbiguousPolicy, ZoneId, ZonedInstant}; diff --git a/crates/core/src/types/sparkline.rs b/crates/core/src/types/sparkline.rs new file mode 100644 index 000000000..896d26eb0 --- /dev/null +++ b/crates/core/src/types/sparkline.rs @@ -0,0 +1,98 @@ +//! The parsed, validated render spec produced by `SPARKLINE`. +//! +//! Google Sheets models a sparkline as a **distinct value kind**, not as a +//! specially-formatted string: `=TYPE(SPARKLINE({1,2,3}))` returns `128`, +//! which is outside `TYPE`'s documented set (1 number / 2 text / 4 boolean / +//! 16 error / 64 array), and `=ISERROR(SPARKLINE({1,2,3}))` is `FALSE`. That +//! is why [`crate::types::Value`] gains its own variant rather than folding +//! the result into `Text` or an error — see the Google Sheets conformance +//! fixtures (`tests/fixtures/google_sheets/google.tsv`). +//! +//! The spec is **not** what the `=` operator compares: Sheets reports *any* two +//! sparklines equal, whatever they plot (`=SPARKLINE({1,2,3})=SPARKLINE({9,9,9})` +//! is `TRUE`, and so is the same row across differing charttypes and options). +//! In that respect it follows the `ErrorMsg` precedent, whose payload is also +//! excluded from equality. +//! +//! The spec is still carried in full by every surface, because `COUNTUNIQUE` +//! *does* distinguish two different sparklines (2 for different, 1 for +//! identical) — Sheets keys uniqueness off something deeper than `=` compares, +//! so a lossy serialization would break that instead. + +/// The `charttype` of a sparkline. `line` is the default when the option is +/// omitted. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum SparklineChartType { + Line, + Bar, + Column, + Winloss, +} + +impl SparklineChartType { + /// The lower-case wire name, as written in the `charttype` option. + pub fn as_str(self) -> &'static str { + match self { + SparklineChartType::Line => "line", + SparklineChartType::Bar => "bar", + SparklineChartType::Column => "column", + SparklineChartType::Winloss => "winloss", + } + } + + /// Parse a `charttype` option value. Matching is ASCII case-insensitive. + /// An unrecognised value is an error in Sheets (`#VALUE!`), unlike an + /// unrecognised option *key*, which is silently ignored. + pub fn parse(s: &str) -> Option { + match s.to_ascii_lowercase().as_str() { + "line" => Some(SparklineChartType::Line), + "bar" => Some(SparklineChartType::Bar), + "column" => Some(SparklineChartType::Column), + "winloss" => Some(SparklineChartType::Winloss), + _ => None, + } + } +} + +/// One plotted data point, or one option value. +/// +/// A blank cell inside the source range is a legitimate data point (it renders +/// normally — the fixtures probe a real range with a deliberately empty cell), +/// so `Blank` is a value here, not an error. Text inside the data likewise +/// renders. +#[derive(Debug, Clone, PartialEq)] +pub enum SparklineValue { + /// A finite number. `-0.0` is normalized to `0.0` on construction so that + /// structural equality and any hash of the spec agree. + Number(f64), + Text(String), + Bool(bool), + /// An empty cell. + Blank, +} + +impl SparklineValue { + /// Build a numeric point, normalizing `-0.0` to `0.0`. + pub fn number(n: f64) -> Self { + SparklineValue::Number(if n == 0.0 { 0.0 } else { n }) + } +} + +/// A parsed, validated sparkline render spec: what to plot and how. +/// +/// Drawing is the consumer's job; the engine's job is to parse, validate and +/// carry this faithfully across every surface. +#[derive(Debug, Clone, PartialEq)] +pub struct SparklineSpec { + /// The chart type (`line` when the option is omitted). + pub chart_type: SparklineChartType, + /// The points to plot, flattened row-major from the `data` argument. + pub data: Vec, + /// The remaining option key/value pairs in the order given, keys + /// ASCII-lower-cased. `charttype` is lifted into [`Self::chart_type`] and + /// is not repeated here. Keys the engine does not recognise are kept + /// rather than rejected: Sheets ignores an unknown option key instead of + /// erroring, which is what lets a workbook written against a newer option + /// set still evaluate. + pub options: Vec<(String, SparklineValue)>, +} diff --git a/crates/core/src/types/value.rs b/crates/core/src/types/value.rs index 405286737..3cc8b6f35 100644 --- a/crates/core/src/types/value.rs +++ b/crates/core/src/types/value.rs @@ -1,4 +1,5 @@ use super::error::ErrorKind; +use super::sparkline::SparklineSpec; use super::zoned::ZonedInstant; #[derive(Debug, Clone)] @@ -27,6 +28,18 @@ pub enum Value { /// `Value` is cloned heavily on the hot numeric path. Equality/ordering for /// the engine's comparison operators is defined on the instant only. Zoned(Box), + /// A sparkline: the parsed, validated render spec produced by `SPARKLINE`. + /// Google Sheets models this as a value kind of its own — `TYPE()` reports + /// the undocumented code `128` and `ISERROR()` is `FALSE` — so it is + /// neither text nor an error here either. Boxed for the same reason as + /// `Zoned`: the payload is large and `Value` is cloned heavily. + /// + /// Like [`Value::ErrorMsg`], the payload is excluded from equality: in + /// Sheets **any** two sparklines compare equal under `=`, whatever they + /// plot. The spec is still carried in full by every surface, because + /// `COUNTUNIQUE` does distinguish two different sparklines — Sheets keys + /// uniqueness off something deeper than `=` compares. + Sparkline(Box), } impl Value { @@ -69,6 +82,12 @@ impl PartialEq for Value { (Value::Array(a), Value::Array(b)) => a == b, (Value::Date(a), Value::Date(b)) => a == b, (Value::Zoned(a), Value::Zoned(b)) => a == b, + // Any two sparklines are equal, whatever they plot: google.tsv + // records `=SPARKLINE({1,2,3})=SPARKLINE({9,9,9})` as TRUE, as well + // as the same row for differing charttypes and options. The spec is + // still carried (COUNTUNIQUE distinguishes two sparklines with a + // deeper key than `=` uses) — it just is not what `==` compares. + (Value::Sparkline(_), Value::Sparkline(_)) => true, // Both error variants: equal iff same kind (message ignored). _ => match (self.error_kind(), other.error_kind()) { (Some(ka), Some(kb)) => ka == kb, diff --git a/crates/core/tests/conformance.rs b/crates/core/tests/conformance.rs index a829da2d1..d72c2482e 100644 --- a/crates/core/tests/conformance.rs +++ b/crates/core/tests/conformance.rs @@ -277,9 +277,46 @@ fn infer_type(v: &Value) -> &'static str { Value::Error(_) | Value::ErrorMsg(_, _) => "error", Value::Array(_) => "array", Value::Empty => "string", + // Sheets reports a sparkline as its own kind (TYPE code 128); the + // fixtures record its *displayed* value, which is always empty. + Value::Sparkline(_) => "sparkline", } } +/// True when a formula reads a *sheet-qualified* reference, e.g. +/// `=SUM(Data!K1:K2)`. +/// +/// This runner evaluates each row standalone, with no workbook behind it, so +/// every such reference resolves to empty. That does not merely fail the row — +/// it can also make one **pass for the wrong reason** whenever the recorded +/// value happens to be what an empty read produces (`=SUM(Data!K1:K1)` is 0 +/// either way). Both outcomes are noise, so these rows are skipped here; they +/// are canonical ground truth and are enforced against a seeded resolver +/// instead (see `tests/sparkline.rs`, and `tests/workbook_inputs_conformance.rs` +/// for `workbook.tsv`'s equivalent rows). +/// +/// Scope: in the two runners this only affects `google.tsv`, the only category +/// file with such rows. The per-function coverage scan below also applies it, +/// where it additionally drops `workbook.tsv`'s 24 sheet-qualified rows from +/// the credit scan — harmless today (every function they mention is credited by +/// many other rows) but not a no-op, and it is deliberate: a row that matches +/// its recorded value only because both sides are empty is not evidence of +/// coverage. +fn needs_authored_input_cells(formula: &str) -> bool { + // Engine-explicit: the free `parse` is deprecated in favor of + // `Engine::sheets().parse` (ADR 2026-04-27), same as `evaluate` below. + let Ok(expr) = truecalc_core::Engine::sheets().parse(formula) else { + return false; + }; + truecalc_core::extract_refs(&expr).iter().any(|r| { + matches!( + r, + truecalc_core::Ref::Cell { sheet: Some(_), .. } + | truecalc_core::Ref::Range { sheet: Some(_), .. } + ) + }) +} + /// Returns true if a formula contains volatile functions. fn is_volatile_formula(formula: &str) -> bool { let upper = formula.to_uppercase(); @@ -306,13 +343,65 @@ fn pinned_now_serial(path: &Path) -> Option { // TSV runner // --------------------------------------------------------------------------- +/// Per-file accounting of what the runner actually checked. +/// +/// Both runners skip rows silently, for several unrelated reasons, and a green +/// test says nothing about how many rows were inert — that is the defect +/// core#767 tracks (rows that look enforced and assert nothing). This does not +/// fix it: the rows are still skipped. It is a narrow mitigation that makes the +/// skipping *visible*, so someone adding a row does not get a silent pass where +/// they expected a check. The real fix is to evaluate these rows, which needs a +/// display-value projection and a seeded resolver in this harness. +#[derive(Default)] +struct RowTally { + rows: usize, + enforced: usize, + /// The expected-value column is blank *after trimming*. The TSV format + /// cannot say whether that means "observed to be empty" or "never probed", + /// so this bucket holds both — that conflation is core#767's first point. + /// + /// It is *not* a "no text projection" bucket. `text.tsv`'s 46 rows here are + /// 41 with a genuinely empty recorded value (`=LEFT("hello",0)`) plus 5 + /// whose recorded value is whitespace (`=CHAR(32)`, `=UNICHAR(32)`, + /// `=LEFT(" ",2)`, `=RIGHT(" ",1)`, `=CONCATENATE(" "," ")`) — those 5 + /// *do* have a recorded value and are skipped only because of the `trim()` + /// noted at the predicate below. Counts measured over the fixture files, + /// not estimated. + no_expected_value: usize, + no_formula: usize, + authored_cells: usize, + volatile: usize, + unparseable_expected: usize, + not_a_formula: usize, +} + +impl RowTally { + fn summary(&self, path: &Path) -> String { + let name = path.file_name().unwrap_or_default().to_string_lossy(); + let mut parts = vec![format!("{} enforced", self.enforced)]; + for (count, reason) in [ + (self.no_expected_value, "no recorded expected value"), + (self.authored_cells, "reads authored cells"), + (self.volatile, "volatile"), + (self.unparseable_expected, "unparseable expected value"), + (self.no_formula, "no formula"), + (self.not_a_formula, "not a formula"), + ] { + if count > 0 { + parts.push(format!("{count} skipped ({reason})")); + } + } + format!("{name}: {} rows — {}", self.rows, parts.join(", ")) + } +} + fn run_tsv_fixture(path: &Path) { assert!(path.exists(), "fixture not found: {:?}", path); let pinned_now = pinned_now_serial(path); let vars: HashMap = HashMap::new(); let mut failures: Vec = Vec::new(); - let mut total = 0usize; + let mut tally = RowTally::default(); let mut rdr = csv::ReaderBuilder::new() .delimiter(b'\t') @@ -326,6 +415,7 @@ fn run_tsv_fixture(path: &Path) { if record.len() < 5 { continue; } + tally.rows += 1; let desc = record[0].trim(); let formula = record[1].trim(); @@ -335,25 +425,46 @@ fn run_tsv_fixture(path: &Path) { let _test_category = record[3].trim(); let expected_type = record[4].trim(); - if formula.is_empty() || expected_str.trim().is_empty() { + if formula.is_empty() { + tally.no_formula += 1; + continue; + } + // NOTE the `trim()` here contradicts the comment above about preserving + // leading whitespace: a recorded value of `" "` is treated as absent and + // skipped. That is 5 rows in text.tsv today (`=CHAR(32)` and the four + // listed on `RowTally::no_expected_value`), which is why that field's + // name says "no *recorded* expected value" rather than "empty result" — + // the bucket mixes both. Pre-existing, and part of what core#767 has to + // untangle; the tally at least stops it being silent. + if expected_str.trim().is_empty() { + tally.no_expected_value += 1; continue; } // Skip malformed rows where formula column doesn't contain a formula if !formula.starts_with('=') { + tally.not_a_formula += 1; continue; } if is_volatile_formula(formula) { + tally.volatile += 1; + continue; + } + if needs_authored_input_cells(formula) { + tally.authored_cells += 1; continue; } let expected = match parse_expected(expected_str, expected_type) { Some(v) => v, - None => continue, + None => { + tally.unparseable_expected += 1; + continue; + } }; - total += 1; + tally.enforced += 1; let actual = match pinned_now { Some(now) => truecalc_core::Engine::sheets().evaluate_at(formula, &vars, now), None => evaluate(formula, &vars), @@ -367,13 +478,21 @@ fn run_tsv_fixture(path: &Path) { } } + // A bare green must not hide how many rows were skipped, or why (core#767). + // libtest and nextest both capture a *passing* test's stdout, so this line + // reaches a reader through: `--nocapture`, any failure (captured output is + // replayed), and CI, whose nextest `ci` profile carries a + // `success-output = 'final'` override for this binary (.config/nextest.toml). + println!("{}", tally.summary(path)); + if !failures.is_empty() { panic!( - "\n{}/{} conformance failures in {}:\n\n{}\n", + "\n{}/{} conformance failures in {}:\n\n{}\n\n{}\n", failures.len(), - total, + tally.enforced, path.file_name().unwrap().to_string_lossy(), failures.join("\n\n"), + tally.summary(path), ); } } @@ -387,6 +506,7 @@ fn run_tsv_fixture_report(path: &Path) { let vars: HashMap = HashMap::new(); let mut pass = 0usize; let mut fail = 0usize; + let mut tally = RowTally::default(); let mut rdr = csv::ReaderBuilder::new() .delimiter(b'\t') @@ -400,6 +520,7 @@ fn run_tsv_fixture_report(path: &Path) { if record.len() < 5 { continue; } + tally.rows += 1; let desc = record[0].trim(); let formula = record[1].trim(); @@ -407,24 +528,47 @@ fn run_tsv_fixture_report(path: &Path) { let _test_category = record[3].trim(); let expected_type = record[4].trim(); - if formula.is_empty() || expected_str.trim().is_empty() { + if formula.is_empty() { + tally.no_formula += 1; + continue; + } + // NOTE the `trim()` here contradicts the comment above about preserving + // leading whitespace: a recorded value of `" "` is treated as absent and + // skipped. That is 5 rows in text.tsv today (`=CHAR(32)` and the four + // listed on `RowTally::no_expected_value`), which is why that field's + // name says "no *recorded* expected value" rather than "empty result" — + // the bucket mixes both. Pre-existing, and part of what core#767 has to + // untangle; the tally at least stops it being silent. + if expected_str.trim().is_empty() { + tally.no_expected_value += 1; continue; } // Skip malformed rows where formula column doesn't contain a formula if !formula.starts_with('=') { + tally.not_a_formula += 1; continue; } if is_volatile_formula(formula) { + tally.volatile += 1; + continue; + } + if needs_authored_input_cells(formula) { + tally.authored_cells += 1; continue; } let expected = match parse_expected(expected_str, expected_type) { Some(v) => v, - None => continue, + None => { + tally.unparseable_expected += 1; + continue; + } }; + tally.enforced += 1; + let actual = match pinned_now { Some(now) => truecalc_core::Engine::sheets().evaluate_at(formula, &vars, now), None => evaluate(formula, &vars), @@ -443,6 +587,10 @@ fn run_tsv_fixture_report(path: &Path) { let name = path.file_name().unwrap_or_default().to_string_lossy(); println!("{name}: {pass} passed, {fail} open"); + // Same accounting as the blocking runner: a skipped row is not an open one, + // and neither number is visible without it (core#767). Surfaced in CI by + // the `success-output` override in .config/nextest.toml. + println!("{}", tally.summary(path)); } // --------------------------------------------------------------------------- @@ -486,6 +634,7 @@ conformance_tsv_test!(array_conformance, "array.tsv"); conformance_tsv_test!(filter_conformance, "filter.tsv"); conformance_tsv_test!(web_conformance, "web.tsv"); conformance_tsv_test!(financial_conformance, "financial.tsv"); +conformance_tsv_test!(google_conformance, "google.tsv"); // workbook.tsv is fully covered by the blocking `workbook_conformance` test in // `tests/workbook_inputs_conformance.rs` (core#575): cross-sheet/named-range @@ -682,7 +831,13 @@ fn every_registered_function_has_conformance_coverage() { } let expected_str = record[2].trim(); let expected_type = record[4].trim(); - if expected_str.is_empty() || is_volatile_formula(formula) { + // Same guard as the runners: an unresolvable sheet-qualified read + // can *match* its recorded value by accident, which would credit a + // function with coverage it does not have. + if expected_str.is_empty() + || is_volatile_formula(formula) + || needs_authored_input_cells(formula) + { continue; } let expected = match parse_expected(expected_str, expected_type) { diff --git a/crates/core/tests/conformance_reporter.rs b/crates/core/tests/conformance_reporter.rs index fb74f0125..8ba2fe879 100644 --- a/crates/core/tests/conformance_reporter.rs +++ b/crates/core/tests/conformance_reporter.rs @@ -92,6 +92,9 @@ fn infer_type(v: &Value) -> &'static str { Value::Error(_) | Value::ErrorMsg(_, _) => "error", Value::Array(_) => "array", Value::Empty => "string", + // Sheets reports a sparkline as its own kind (TYPE code 128); the + // fixtures record its *displayed* value, which is always empty. + Value::Sparkline(_) => "sparkline", } } diff --git a/crates/core/tests/sparkline.rs b/crates/core/tests/sparkline.rs new file mode 100644 index 000000000..916423f3a --- /dev/null +++ b/crates/core/tests/sparkline.rs @@ -0,0 +1,761 @@ +//! `SPARKLINE` — behaviour pinned to the Google Sheets conformance fixtures. +//! +//! Every expectation below is a row of `tests/fixtures/google_sheets/google.tsv`, +//! observed in live Google Sheets. Nothing here is self-confirmed. The +//! fixture rows whose recorded value is the empty string (a rendered chart has +//! no text projection) are skipped by the TSV runner, so they are asserted +//! here instead — as "this renders, it is not an error", plus the parsed spec +//! the engine built. + +mod helpers; +use helpers::{eval, eval_with}; + +use truecalc_core::types::{SparklineChartType, SparklineValue}; +use truecalc_core::{CellAddr, Engine, ErrorKind, Ref, Resolver, Value}; + +/// The parsed spec of a formula that must evaluate to a sparkline. +fn spec(formula: &str) -> (SparklineChartType, Vec, Vec<(String, SparklineValue)>) { + match eval(formula) { + Value::Sparkline(s) => (s.chart_type, s.data.clone(), s.options.clone()), + other => panic!("{formula} should render a sparkline, got {other:?}"), + } +} + +fn num(n: f64) -> SparklineValue { + SparklineValue::Number(n) +} + +// ── The result is a value kind of its own ─────────────────────────────────── + +#[test] +fn type_of_a_sparkline_is_128() { + // google.tsv: =TYPE(SPARKLINE({1,2,3})) → 128, outside TYPE's documented + // set (1 number / 2 text / 4 boolean / 16 error / 64 array). + assert_eq!(eval("=TYPE(SPARKLINE({1,2,3}))"), Value::Number(128.0)); +} + +#[test] +fn a_sparkline_is_not_an_error() { + // google.tsv: =ISERROR(SPARKLINE({1,2,3})) → FALSE, =ISNA(...) → FALSE. + assert_eq!(eval("=ISERROR(SPARKLINE({1,2,3}))"), Value::Bool(false)); + assert_eq!(eval("=ISNA(SPARKLINE({1,2,3}))"), Value::Bool(false)); +} + +#[test] +fn every_sparkline_is_equal_to_every_other_sparkline() { + // google.tsv — the row that reads like spec identity, plus the controls + // that disprove it: `=` reports TRUE for two sparklines whatever they plot, + // whatever their charttype, and whatever options they carry. + for formula in [ + "=SPARKLINE({1,2,3})=SPARKLINE({1,2,3})", + "=SPARKLINE({1,2,3})=SPARKLINE({9,9,9})", + "=SPARKLINE({1,2,3},{\"charttype\",\"column\"})=SPARKLINE({1,2,3},{\"charttype\",\"line\"})", + "=SPARKLINE({1,2,3},{\"charttype\",\"line\"})=SPARKLINE({1,2,3})", + "=SPARKLINE({1,2,3},{\"bogus\",\"x\"})=SPARKLINE({1,2,3})", + "=SPARKLINE({1,2,3},{\"bogus\",\"x\"})=SPARKLINE({1,2,3},{\"bogus\",\"y\"})", + ] { + assert_eq!(eval(formula), Value::Bool(true), "{formula}"); + } + // …and `<>` is the negation of that, not of a spec comparison. + assert_eq!( + eval("=SPARKLINE({1,2,3})<>SPARKLINE({1,2,3})"), + Value::Bool(false) + ); +} + +#[test] +fn a_sparkline_is_not_equal_to_empty_text() { + // google.tsv: `=SPARKLINE({1,2,3})=""` is FALSE — even though every text + // projection of a sparkline is the empty string, and `EXACT(…,"")` is TRUE. + assert_eq!(eval("=SPARKLINE({1,2,3})=\"\""), Value::Bool(false)); +} + +#[test] +fn a_sparkline_outranks_every_scalar_in_ordering() { + // google.tsv: `>1`, `>"zzzz"` and `>TRUE` are all TRUE. + for formula in [ + "=SPARKLINE({1,2,3})>1", + "=SPARKLINE({1,2,3})>\"zzzz\"", + "=SPARKLINE({1,2,3})>TRUE", + ] { + assert_eq!(eval(formula), Value::Bool(true), "{formula}"); + } +} + +/// `EQ`/`NE`/`GT`/`GTE`/`LT`/`LTE` are Google Sheets' own function names for +/// `=`/`<>`/`>`/`>=`/`<`/`<=`, and the engine backs them with a *separate* +/// comparison path. google.tsv records both paths agreeing (EQ TRUE, NE FALSE, +/// `<>` FALSE, GTE TRUE), so any divergence is an implementation defect. +#[test] +fn comparison_alias_functions_agree_with_their_operators() { + let a = "SPARKLINE({1,2,3})"; + let b = "SPARKLINE({9,9,9})"; + let cases = [ + ("EQ", "="), + ("NE", "<>"), + ("GT", ">"), + ("GTE", ">="), + ("LT", "<"), + ("LTE", "<="), + ]; + for (func, op) in cases { + for (left, right) in [(a, a), (a, b), (b, a)] { + let via_function = eval(&format!("={func}({left},{right})")); + let via_operator = eval(&format!("={left}{op}{right}")); + assert_eq!( + via_function, via_operator, + "{func}({left},{right}) must match {left}{op}{right}" + ); + } + } +} + +#[test] +fn eq_and_ne_agree_with_the_equality_operator() { + // google.tsv records EQ TRUE and NE FALSE directly, reached through the + // function alias rather than the operator. + assert_eq!( + eval("=EQ(SPARKLINE({1,2,3}),SPARKLINE({1,2,3}))"), + Value::Bool(true) + ); + assert_eq!( + eval("=NE(SPARKLINE({1,2,3}),SPARKLINE({1,2,3}))"), + Value::Bool(false) + ); + // …and the same answers for two *different* sparklines, because `=` does + // not look at what they plot. + assert_eq!( + eval("=EQ(SPARKLINE({1,2,3}),SPARKLINE({9,9,9}))"), + Value::Bool(true) + ); + assert_eq!( + eval("=NE(SPARKLINE({1,2,3}),SPARKLINE({9,9,9}))"), + Value::Bool(false) + ); +} + +#[test] +fn two_sparklines_are_mutually_equal_in_ordering() { + // google.tsv: `>` and `<` between two sparklines are both FALSE, while `>=` + // (and its GTE alias, on two *different* sparklines) is TRUE — the ordering + // reading of "all sparklines are equal". + for formula in [ + "=SPARKLINE({1,2,3})>SPARKLINE({9,9,9})", + "=SPARKLINE({1,2,3})=SPARKLINE({1,2,3})", + "=SPARKLINE({1,2,3})>=SPARKLINE({9,9,9})", + "=SPARKLINE({1,2,3})<=SPARKLINE({9,9,9})", + "=GTE(SPARKLINE({1,2,3}),SPARKLINE({9,9,9}))", + ] { + assert_eq!(eval(formula), Value::Bool(true), "{formula}"); + } +} + +// ── Coercion ──────────────────────────────────────────────────────────────── +// +// Text and boolean contexts are permissive — a sparkline reads as empty text +// and as falsy — and aggregates skip it. Exactly two contexts reject it: the +// arithmetic operators and the `&` concatenation *operator*. Note that `&` +// errors while `CONCATENATE` of the same value succeeds; that asymmetry is +// recorded, not a modelling choice. + +#[test] +fn coercion_arithmetic_on_a_sparkline_is_value_error() { + // google.tsv: =SPARKLINE({1,2,3})+1 → #VALUE! + assert_eq!( + eval("=SPARKLINE({1,2,3})+1"), + Value::Error(ErrorKind::Value) + ); +} + +#[test] +fn coercion_concatenating_a_sparkline_is_value_error() { + // google.tsv: ="x"&SPARKLINE({1,2,3}) → #VALUE! (empty text would have + // concatenated instead.) + assert_eq!( + eval("=\"x\"&SPARKLINE({1,2,3})"), + Value::Error(ErrorKind::Value) + ); +} + +#[test] +fn coercion_len_of_a_sparkline_is_zero() { + // google.tsv: =LEN(SPARKLINE({1,2,3})) → 0 + assert_eq!(eval("=LEN(SPARKLINE({1,2,3}))"), Value::Number(0.0)); +} + +#[test] +fn coercion_n_of_a_sparkline_is_zero() { + // google.tsv: =N(SPARKLINE({1,2,3})) → 0 + assert_eq!(eval("=N(SPARKLINE({1,2,3}))"), Value::Number(0.0)); +} + +#[test] +fn coercion_to_text_of_a_sparkline_is_the_empty_string() { + // google.tsv: =TO_TEXT(SPARKLINE({1,2,3})) → "" (an empty recorded value, + // so the TSV runner skips the row; asserted here instead). + assert_eq!( + eval("=TO_TEXT(SPARKLINE({1,2,3}))"), + Value::Text(String::new()) + ); +} + +#[test] +fn coercion_text_functions_read_a_sparkline_as_empty_text() { + // google.tsv: LEFT → "", TEXT → "", TEXTJOIN → "", CONCATENATE(…,"x") → "x", + // EXACT(…,"") → TRUE. The `&` operator above is the carve-out, not these. + assert_eq!( + eval("=LEFT(SPARKLINE({1,2,3}),1)"), + Value::Text(String::new()) + ); + assert_eq!( + eval("=TEXT(SPARKLINE({1,2,3}),\"0\")"), + Value::Text(String::new()) + ); + assert_eq!( + eval("=TEXTJOIN(\",\",TRUE,SPARKLINE({1,2,3}))"), + Value::Text(String::new()) + ); + assert_eq!( + eval("=CONCATENATE(SPARKLINE({1,2,3}),\"x\")"), + Value::Text("x".to_owned()) + ); + assert_eq!( + eval("=EXACT(SPARKLINE({1,2,3}),\"\")"), + Value::Bool(true) + ); +} + +#[test] +fn coercion_currency_and_conversion_functions_split_two_ways() { + // google.tsv, and the reason the coercion map lists exceptions by name: + // DOLLAR and FIXED reject a sparkline, while TEXT and the whole TO_* family + // answer "". DOLLAR vs TO_DOLLARS is the sharpest pair — near-identical + // names, opposite answers. (VALUE is not in that argument: it reads + // through the permissive text seam, so its 0 comes free from LEN's route, + // not from a number-reading carve-out.) + assert_eq!( + eval("=TO_PERCENT(SPARKLINE({1,2,3}))"), + Value::Text(String::new()) + ); + assert_eq!(eval("=VALUE(SPARKLINE({1,2,3}))"), Value::Number(0.0)); + assert_eq!( + eval("=DOLLAR(SPARKLINE({1,2,3}))"), + Value::Error(ErrorKind::Value) + ); + assert_eq!( + eval("=FIXED(SPARKLINE({1,2,3}))"), + Value::Error(ErrorKind::Value) + ); +} + +#[test] +fn coercion_the_whole_to_family_reads_a_sparkline_as_empty_text() { + // google.tsv: TO_TEXT, TO_PERCENT, TO_DOLLARS, TO_PURE_NUMBER and TO_DATE + // all answer "" — the family is uniform, which is exactly why TO_DOLLARS + // parting company with DOLLAR is not derivable from either name. + for formula in [ + "=TO_TEXT(SPARKLINE({1,2,3}))", + "=TO_PERCENT(SPARKLINE({1,2,3}))", + "=TO_DOLLARS(SPARKLINE({1,2,3}))", + "=TO_PURE_NUMBER(SPARKLINE({1,2,3}))", + "=TO_DATE(SPARKLINE({1,2,3}))", + ] { + assert_eq!(eval(formula), Value::Text(String::new()), "{formula}"); + } +} + +#[test] +fn coercion_the_number_seam_rejects_a_sparkline_wholesale() { + // `to_number` has one blanket arm, so this is not a list of four functions: + // every caller of that seam rejects. google.tsv pins the operators; these + // are the probed function-level confirmations. + for formula in [ + "=SPARKLINE({1,2,3})+1", + "=SPARKLINE({1,2,3})-1", + "=SPARKLINE({1,2,3})*2", + "=SPARKLINE({1,2,3})/2", + "=-SPARKLINE({1,2,3})", + "=SPARKLINE({1,2,3})%", + "=ROUND(SPARKLINE({1,2,3}))", + "=ABS(SPARKLINE({1,2,3}))", + "=INT(SPARKLINE({1,2,3}))", + ] { + assert_eq!(eval(formula), Value::Error(ErrorKind::Value), "{formula}"); + } +} + +#[test] +fn coercion_more_text_functions_read_a_sparkline_as_empty_text() { + // google.tsv: TRIM → "", UPPER → "". + assert_eq!( + eval("=TRIM(SPARKLINE({1,2,3}))"), + Value::Text(String::new()) + ); + assert_eq!( + eval("=UPPER(SPARKLINE({1,2,3}))"), + Value::Text(String::new()) + ); +} + +#[test] +fn coercion_a_sparkline_is_falsy() { + // google.tsv: =IF(SPARKLINE({1,2,3}),1,2) → 2. + assert_eq!(eval("=IF(SPARKLINE({1,2,3}),1,2)"), Value::Number(2.0)); +} + +#[test] +fn coercion_aggregates_skip_a_sparkline_rather_than_erroring() { + // google.tsv: SUM → 1, MAX → 1, PRODUCT(…,3) → 3, COUNT(…,1) → 1. A + // sparkline is skipped wherever it appears, direct argument or array + // element — PRODUCT is the one that had a direct-argument path of its own. + assert_eq!(eval("=SUM(SPARKLINE({1,2,3}),1)"), Value::Number(1.0)); + assert_eq!(eval("=MAX(SPARKLINE({1,2,3}),1)"), Value::Number(1.0)); + assert_eq!(eval("=PRODUCT(SPARKLINE({1,2,3}),3)"), Value::Number(3.0)); + assert_eq!(eval("=PRODUCT({SPARKLINE({1,2,3}),3})"), Value::Number(3.0)); + assert_eq!(eval("=COUNT(SPARKLINE({1,2,3}),1)"), Value::Number(1.0)); +} + +#[test] +fn the_statistical_family_skips_a_sparkline_too() { + // google.tsv, recorded rather than assumed — MINA had no arm at all until a + // row asked for one, and matching MAXA was a coin-flip until then. + assert_eq!(eval("=MINA(SPARKLINE({1,2,3}))"), Value::Number(0.0)); + assert_eq!(eval("=MAXA(SPARKLINE({1,2,3}))"), Value::Number(0.0)); + assert_eq!( + eval("=AVERAGEA(SPARKLINE({1,2,3}))"), + Value::Error(ErrorKind::DivByZero) + ); + assert_eq!(eval("=MEDIAN(SPARKLINE({1,2,3}),1)"), Value::Number(1.0)); + assert_eq!( + eval("=STDEV(SPARKLINE({1,2,3}),1,2)"), + Value::Number(0.7071067811865476) + ); + assert_eq!(eval("=SUMSQ(SPARKLINE({1,2,3}))"), Value::Number(0.0)); + assert_eq!(eval("=SUMPRODUCT(SPARKLINE({1,2,3}))"), Value::Number(0.0)); + assert_eq!( + eval("=CELL(\"type\",SPARKLINE({1,2,3}))"), + Value::Error(ErrorKind::NA) + ); +} + +#[test] +fn a_lone_sparkline_leaves_an_aggregate_with_no_arguments_at_all() { + // google.tsv: SUM, PRODUCT, MAX, MIN, MAXA, MINA and COUNT of nothing but a + // sparkline are all 0 — PRODUCT included, so a skipped argument is *absent*, + // not a factor of 1 (which two-argument calls cannot distinguish), and MIN + // and the A-variants each have their own recorded direct-form row rather + // than being inferred from the range form. AVERAGE is #DIV/0!, the row that + // proves the list is empty rather than holding a zero. + for formula in [ + "=SUM(SPARKLINE({1,2,3}))", + "=PRODUCT(SPARKLINE({1,2,3}))", + "=MAX(SPARKLINE({1,2,3}))", + "=MIN(SPARKLINE({1,2,3}))", + "=MAXA(SPARKLINE({1,2,3}))", + "=MINA(SPARKLINE({1,2,3}))", + "=COUNT(SPARKLINE({1,2,3}))", + ] { + assert_eq!(eval(formula), Value::Number(0.0), "{formula}"); + } + assert_eq!( + eval("=AVERAGE(SPARKLINE({1,2,3}))"), + Value::Error(ErrorKind::DivByZero) + ); +} + +#[test] +fn a_sparkline_counts_as_a_present_non_blank_value() { + // google.tsv: =COUNTA(SPARKLINE({1,2,3})) → 1, =ISBLANK(…) → FALSE. + assert_eq!(eval("=COUNTA(SPARKLINE({1,2,3}))"), Value::Number(1.0)); + assert_eq!(eval("=ISBLANK(SPARKLINE({1,2,3}))"), Value::Bool(false)); +} + +#[test] +fn countunique_distinguishes_sparklines_that_the_equality_operator_does_not() { + // google.tsv: 2 for two different sparklines, 1 for two identical ones — + // even though `=` reports both pairs equal. This is the row that requires + // the parsed spec to be retained and serialized in full. + assert_eq!( + eval("=COUNTUNIQUE(SPARKLINE({1,2,3}),SPARKLINE({9,9,9}))"), + Value::Number(2.0) + ); + assert_eq!( + eval("=COUNTUNIQUE(SPARKLINE({1,2,3}),SPARKLINE({1,2,3}))"), + Value::Number(1.0) + ); +} + +// ── Error class 1: arity / shape of `data` → #N/A ─────────────────────────── + +#[test] +fn error_na_when_called_with_no_arguments() { + // google.tsv: =SPARKLINE() → #N/A + assert_eq!(eval("=SPARKLINE()"), Value::Error(ErrorKind::NA)); +} + +#[test] +fn error_na_when_data_is_a_scalar_instead_of_a_range() { + // google.tsv: =SPARKLINE(5) → #N/A + assert_eq!(eval("=SPARKLINE(5)"), Value::Error(ErrorKind::NA)); +} + +#[test] +fn error_na_when_data_holds_a_single_value() { + // google.tsv: =SPARKLINE({5}) → #N/A + assert_eq!(eval("=SPARKLINE({5})"), Value::Error(ErrorKind::NA)); +} + +// ── Error class 2: structural malformation → #REF! ────────────────────────── + +#[test] +fn error_ref_when_data_is_an_empty_array() { + // google.tsv: =SPARKLINE({}) → #REF! (Note this is *not* #N/A: an empty + // array is malformed, a one-point array is merely too short.) + assert_eq!(eval("=SPARKLINE({})"), Value::Error(ErrorKind::Ref)); +} + +#[test] +fn error_ref_when_options_are_not_key_value_pairs() { + // google.tsv: =SPARKLINE({1,2,3},{"charttype"}) → #REF! + assert_eq!( + eval("=SPARKLINE({1,2,3},{\"charttype\"})"), + Value::Error(ErrorKind::Ref) + ); +} + +// ── Error class 3: a bad option *value* → #VALUE! ─────────────────────────── + +#[test] +fn error_value_when_the_charttype_value_is_unknown() { + // google.tsv: =SPARKLINE({1,2,3},{"charttype","bogus"}) → #VALUE! + assert_eq!( + eval("=SPARKLINE({1,2,3},{\"charttype\",\"bogus\"})"), + Value::Error(ErrorKind::Value) + ); +} + +#[test] +fn an_unknown_option_key_is_kept_not_rejected() { + // google.tsv: =SPARKLINE({1,2,3},{"bogus","x"}) renders — an unrecognised + // *key* is not an error, unlike an unrecognised charttype *value*. Sheets + // "ignores" it; the engine keeps it in the parsed spec, which `=` cannot + // observe (all sparklines are equal) but COUNTUNIQUE's deeper key can — so + // it is kept, treated exactly like a recognised option. + let (chart_type, data, options) = spec("=SPARKLINE({1,2,3},{\"bogus\",\"x\"})"); + assert_eq!(chart_type, SparklineChartType::Line); + assert_eq!(data, vec![num(1.0), num(2.0), num(3.0)]); + assert_eq!( + options, + vec![("bogus".to_owned(), SparklineValue::Text("x".to_owned()))] + ); +} + +// ── Cases that render ─────────────────────────────────────────────────────── + +#[test] +fn charttype_defaults_to_line_when_omitted() { + // google.tsv: =SPARKLINE({1,2,3}) renders. + let (chart_type, data, options) = spec("=SPARKLINE({1,2,3})"); + assert_eq!(chart_type, SparklineChartType::Line); + assert_eq!(data, vec![num(1.0), num(2.0), num(3.0)]); + assert!(options.is_empty()); +} + +#[test] +fn every_charttype_renders() { + // google.tsv: column, bar (two values), winloss and line all render. + let cases = [ + ( + "=SPARKLINE({1,2,3},{\"charttype\",\"column\"})", + SparklineChartType::Column, + ), + ( + "=SPARKLINE({1,2},{\"charttype\",\"bar\"})", + SparklineChartType::Bar, + ), + ( + "=SPARKLINE({1,-1,1},{\"charttype\",\"winloss\"})", + SparklineChartType::Winloss, + ), + ( + "=SPARKLINE({1,2,3},{\"charttype\",\"line\";\"color\",\"red\"})", + SparklineChartType::Line, + ), + ]; + for (formula, expected) in cases { + let (chart_type, _, _) = spec(formula); + assert_eq!(chart_type, expected, "{formula}"); + } +} + +#[test] +fn bar_charttype_with_three_values_renders() { + // google.tsv: =SPARKLINE({1,2,3},{"charttype","bar"}) renders, even though + // `bar` is documented as two-valued. A wrong-arity check here would be a + // divergence, not a validation. + let (chart_type, data, _) = spec("=SPARKLINE({1,2,3},{\"charttype\",\"bar\"})"); + assert_eq!(chart_type, SparklineChartType::Bar); + assert_eq!(data.len(), 3); +} + +#[test] +fn color_option_is_kept_and_charttype_is_lifted_out() { + // google.tsv: =SPARKLINE({1,2,3},{"charttype","line";"color","red"}) renders. + let (chart_type, _, options) = + spec("=SPARKLINE({1,2,3},{\"charttype\",\"line\";\"color\",\"red\"})"); + assert_eq!(chart_type, SparklineChartType::Line); + assert_eq!( + options, + vec![("color".to_owned(), SparklineValue::Text("red".to_owned()))] + ); +} + +#[test] +fn ymin_and_ymax_options_render() { + // google.tsv: =SPARKLINE({1,2,3},{"ymin",0;"ymax",10}) renders. + let (_, _, options) = spec("=SPARKLINE({1,2,3},{\"ymin\",0;\"ymax\",10})"); + assert_eq!( + options, + vec![ + ("ymin".to_owned(), num(0.0)), + ("ymax".to_owned(), num(10.0)), + ] + ); +} + +#[test] +fn text_inside_the_data_renders() { + // google.tsv: =SPARKLINE({1,"a",3}) renders — text is a data point, not an + // error. + let (_, data, _) = spec("=SPARKLINE({1,\"a\",3})"); + assert_eq!( + data, + vec![num(1.0), SparklineValue::Text("a".to_owned()), num(3.0)] + ); +} + +#[test] +fn all_negative_data_renders() { + // google.tsv: =SPARKLINE({-1,-2,-3}) renders. + let (_, data, _) = spec("=SPARKLINE({-1,-2,-3})"); + assert_eq!(data, vec![num(-1.0), num(-2.0), num(-3.0)]); +} + +#[test] +fn a_genuine_blank_inside_the_range_renders() { + // google.tsv: =SPARKLINE(Data!H1:H3) with H2 deliberately empty renders, + // exactly like the all-present control =SPARKLINE(Data!I1:I3). (The + // neighbouring `{1,,3}` → #ERROR! row is Sheets rejecting the array-literal + // syntax before SPARKLINE runs, not a blank-cell rule.) + let blank = Value::Array(vec![Value::Number(1.0), Value::Empty, Value::Number(3.0)]); + match eval_with("=SPARKLINE(RANGE)", [("RANGE", blank)]) { + Value::Sparkline(s) => assert_eq!( + s.data, + vec![num(1.0), SparklineValue::Blank, num(3.0)], + "a blank cell is a data point" + ), + other => panic!("a blank inside the range must still render, got {other:?}"), + } +} + +#[test] +fn an_invalid_array_literal_fails_before_sparkline_runs() { + // google.tsv: =SPARKLINE({1,,3}) → #ERROR! in Sheets — an array-literal + // parse failure, which this engine reports as #VALUE! (the code the + // fixtures map #ERROR! onto). Recorded here so the row is not mistaken + // for a blank-cell rule; the row above is the trustworthy blank probe. + assert_eq!(eval("=SPARKLINE({1,,3})"), Value::Error(ErrorKind::Value)); +} + +#[test] +fn two_dimensional_and_single_row_data_render() { + // google.tsv: =SPARKLINE({1,2;3,4}) and =SPARKLINE({1,2}) both render. + let (_, data, _) = spec("=SPARKLINE({1,2;3,4})"); + assert_eq!(data, vec![num(1.0), num(2.0), num(3.0), num(4.0)]); + let (_, data, _) = spec("=SPARKLINE({1,2})"); + assert_eq!(data, vec![num(1.0), num(2.0)]); +} + +#[test] +fn option_keys_and_charttype_values_are_case_insensitive() { + // google.tsv: {"CHARTTYPE","column"} and {"charttype","COLUMN"} both render. + let (chart_type, _, options) = spec("=SPARKLINE({1,2,3},{\"CHARTTYPE\",\"column\"})"); + assert_eq!(chart_type, SparklineChartType::Column); + assert!(options.is_empty(), "an upper-case charttype key is still charttype"); + let (chart_type, _, _) = spec("=SPARKLINE({1,2,3},{\"charttype\",\"COLUMN\"})"); + assert_eq!(chart_type, SparklineChartType::Column); +} + +#[test] +fn a_non_text_option_key_is_accepted() { + // google.tsv: {TRUE,"x"} and {0,"x"} both render — a non-text key is not an + // error. (What the coerced key *string* is has not been probed; nothing + // observable depends on it, since neither key is recognised.) + for formula in [ + "=SPARKLINE({1,2,3},{TRUE,\"x\"})", + "=SPARKLINE({1,2,3},{0,\"x\"})", + ] { + let (chart_type, _, options) = spec(formula); + assert_eq!(chart_type, SparklineChartType::Line, "{formula}"); + assert_eq!(options.len(), 1, "{formula}"); + } +} + +// ── Delivery through a range ──────────────────────────────────────────────── +// +// A real workbook delivers a sparkline from a *cell* holding `=SPARKLINE(...)`, +// not as a literal argument. google.tsv probes that with `Data!K1` (a +// sparkline cell) and `Data!K2` (the number 5). The shared TSV runner +// evaluates rows standalone with no workbook behind it, so those rows are +// skipped there — and would silently pass for the wrong reason where an empty +// read happens to match — which is why they are enforced here against a +// resolver seeded with exactly those two cells. + +/// The `Data` sheet google.tsv's `Data!K…` rows read: K1 holds a sparkline, +/// K2 holds 5. +struct SparklineCellResolver; + +impl SparklineCellResolver { + fn cell(addr: &CellAddr) -> Value { + match (addr.col, addr.row) { + // K1 = `=SPARKLINE({1,2,3})`, pre-resolved as a workbook would. + (11, 1) => eval("=SPARKLINE({1,2,3})"), + (11, 2) => Value::Number(5.0), + _ => Value::Empty, + } + } +} + +impl Resolver for SparklineCellResolver { + fn resolve(&mut self, r: &Ref) -> Value { + match r { + Ref::Cell { sheet: Some(s), addr } if s.eq_ignore_ascii_case("data") => { + Self::cell(addr) + } + Ref::Range { sheet: Some(s), start, end } if s.eq_ignore_ascii_case("data") => { + let mut cells = Vec::new(); + for row in start.row..=end.row { + for col in start.col..=end.col { + cells.push(Self::cell(&CellAddr { col, row, ..*start })); + } + } + Value::Array(cells) + } + _ => Value::Error(ErrorKind::Ref), + } + } +} + +fn eval_over_cells(formula: &str) -> Value { + Engine::sheets().evaluate_with_resolver(formula, &mut SparklineCellResolver) +} + +#[test] +fn a_referenced_sparkline_cell_is_still_a_sparkline() { + // google.tsv: =TYPE(Data!K1) → 128. Reading a cell is not a coercion + // point, even though the cell displays as empty. + assert_eq!(eval_over_cells("=TYPE(Data!K1)"), Value::Number(128.0)); + assert!(matches!( + eval_over_cells("=Data!K1"), + Value::Sparkline(_) + )); +} + +#[test] +fn aggregates_answer_the_same_whether_a_sparkline_arrives_directly_or_by_range() { + // google.tsv, K1:K1 (a lone sparkline cell) and K1:K2 (sparkline + 5). + // The lone-cell rows are the evidence: with a second value present, + // "skipped" and "contributes the identity" are indistinguishable for + // PRODUCT (1 × 5 = 5 either way). + for (formula, expected) in [ + // A lone sparkline cell: every aggregate answers 0, whichever way it + // arrived. MAX and MAXA/MINA are the ones that had to change — MAX fell + // into its numberless-array `#REF!` rule, and the A-variants into their + // own `#N/A`. + ("=SUM(Data!K1:K1)", 0.0), + ("=PRODUCT(Data!K1:K1)", 0.0), + ("=MAX(Data!K1:K1)", 0.0), + ("=MIN(Data!K1:K1)", 0.0), + ("=MAXA(Data!K1:K1)", 0.0), + ("=MINA(Data!K1:K1)", 0.0), + // …and with a real value alongside it, the sparkline is simply absent. + ("=SUM(Data!K1:K2)", 5.0), + ("=PRODUCT(Data!K1:K2)", 5.0), + ("=MAX(Data!K1:K2)", 5.0), + ("=MIN(Data!K1:K2)", 5.0), + ("=MAXA(Data!K1:K2)", 5.0), + ("=COUNT(Data!K1:K2)", 1.0), + ("=COUNTA(Data!K1:K2)", 2.0), + ("=COUNTUNIQUE(Data!K1:K2)", 2.0), + ] { + assert_eq!( + eval_over_cells(formula), + Value::Number(expected), + "{formula}" + ); + } + // AVERAGE is the one exception, and a confirming one: an argument list that + // empties rather than gaining a zero. + assert_eq!( + eval_over_cells("=AVERAGE(Data!K1:K1)"), + Value::Error(ErrorKind::DivByZero) + ); + // …and the direct forms agree with the range forms. + assert_eq!(eval("=SUM(SPARKLINE({1,2,3}))"), eval_over_cells("=SUM(Data!K1:K1)")); + assert_eq!( + eval("=PRODUCT(SPARKLINE({1,2,3}))"), + eval_over_cells("=PRODUCT(Data!K1:K1)") + ); + for (direct, ranged) in [ + ("=MAX(SPARKLINE({1,2,3}))", "=MAX(Data!K1:K1)"), + ("=MIN(SPARKLINE({1,2,3}))", "=MIN(Data!K1:K1)"), + ("=MAXA(SPARKLINE({1,2,3}))", "=MAXA(Data!K1:K1)"), + ("=MINA(SPARKLINE({1,2,3}))", "=MINA(Data!K1:K1)"), + ("=AVERAGE(SPARKLINE({1,2,3}))", "=AVERAGE(Data!K1:K1)"), + ] { + assert_eq!(eval(direct), eval_over_cells(ranged), "{direct} vs {ranged}"); + } +} + +#[test] +fn an_empty_array_argument_outranks_the_sparkline_skip() { + // google.tsv: `=MAX(SPARKLINE({1,2,3}),{})` is #REF! while + // `=MAX(SPARKLINE({1,2,3}),{"a"})` is 0. So "a skipped sparkline leaves an + // empty argument list ⇒ 0" holds against a text-only array but not against + // an explicitly empty one, which raises MAX's own error first. + assert_eq!( + eval("=MAX(SPARKLINE({1,2,3}),{})"), + Value::Error(ErrorKind::Ref) + ); + assert_eq!( + 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)); +} + +// ── Registry surface ──────────────────────────────────────────────────────── + +#[test] +fn sparkline_is_listed_by_the_function_registry() { + let registry = truecalc_core::Registry::new(); + let meta = registry + .get_metadata() + .into_iter() + .find(|e| e.name == "SPARKLINE") + .expect("SPARKLINE should be listed by the registry"); + assert_eq!(meta.meta.category, "google"); + assert_eq!(meta.meta.signature, "SPARKLINE(data, [options])"); +} diff --git a/crates/mcp/src/main.rs b/crates/mcp/src/main.rs index e3a96c07c..c30a9c72c 100644 --- a/crates/mcp/src/main.rs +++ b/crates/mcp/src/main.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use std::io::{self, BufRead, Write}; +use truecalc_core::types::{SparklineChartType, SparklineSpec, SparklineValue}; use truecalc_core::{Engine, Expr, Registry, Value}; use truecalc_workbook::{Address, CellInput, EngineFlavor as WbEngine, RecalcContext, Value as WbValue, Workbook}; use serde_json::{json, Value as JsonValue}; @@ -492,6 +493,7 @@ fn wb_value_to_json(v: &WbValue) -> JsonValue { WbValue::Empty => json!({ "type": "empty", "value": null }), WbValue::Date(d) => json!({ "type": "date", "value": d }), WbValue::Zoned(z) => json!({ "type": "zoned", "value": z.to_rfc9557() }), + WbValue::Sparkline(spec) => json!({ "type": "sparkline", "value": sparkline_to_json(spec) }), WbValue::Array(rows) => { let arr: Vec> = rows.iter() .map(|r| r.iter().map(wb_value_to_json).collect()) @@ -501,6 +503,57 @@ fn wb_value_to_json(v: &WbValue) -> JsonValue { } } +/// One sparkline data point / option value, read back from the shape +/// [`sparkline_to_json`] emits for it. +fn json_to_sparkline_value(v: &JsonValue) -> Option { + let obj = v.as_object()?; + match obj.get("type")?.as_str()? { + "number" => Some(SparklineValue::number(obj.get("value")?.as_f64()?)), + "text" => Some(SparklineValue::Text(obj.get("value")?.as_str()?.to_owned())), + "bool" => Some(SparklineValue::Bool(obj.get("value")?.as_bool()?)), + "empty" => Some(SparklineValue::Blank), + _ => None, + } +} + +/// Read the payload of a `{ "type": "sparkline", "value": {...} }` object back +/// into a spec, so a sparkline this server emitted can be handed back as a +/// variable unchanged. Without it the object would silently arrive as `empty`. +fn json_to_sparkline(spec: &JsonValue) -> Option { + let obj = spec.as_object()?; + let chart_type = SparklineChartType::parse(obj.get("charttype")?.as_str()?)?; + let raw_data = obj.get("data")?.as_array()?; + // The evaluator answers `#N/A` for a `data` argument with fewer than two + // points, so a shorter spec is not something it can emit — reject it here + // too, exactly as the workbook decoder does. + if raw_data.len() < 2 { + return None; + } + let mut data = Vec::new(); + for raw in raw_data { + data.push(json_to_sparkline_value(raw)?); + } + let mut options = Vec::new(); + for raw in obj.get("options")?.as_array()? { + let pair = raw.as_array()?; + if pair.len() != 2 { + return None; + } + let key = pair[0].as_str()?.to_ascii_lowercase(); + // `charttype` is lifted into the spec's own field, never left in the + // option list — so a payload carrying it there was not emitted by us. + if key == "charttype" { + return None; + } + options.push((key, json_to_sparkline_value(&pair[1])?)); + } + Some(SparklineSpec { + chart_type, + data, + options, + }) +} + fn parse_variables(vars_json: &JsonValue) -> HashMap { let mut map = HashMap::new(); if let Some(obj) = vars_json.as_object() { @@ -528,6 +581,15 @@ fn parse_variables(vars_json: &JsonValue) -> HashMap { None => continue, } } + // Self-describing sparkline: { "type": "sparkline", "value": {...} }. + JsonValue::Object(o) + if o.get("type").and_then(|t| t.as_str()) == Some("sparkline") => + { + match o.get("value").and_then(json_to_sparkline) { + Some(spec) => Value::Sparkline(Box::new(spec)), + None => continue, + } + } _ => continue, }; map.insert(k.clone(), val); @@ -536,6 +598,29 @@ fn parse_variables(vars_json: &JsonValue) -> HashMap { map } +/// The plain-value projection of a sparkline data point / option value, so a +/// spec is emitted in the same vocabulary as any other value. +fn sparkline_cell(v: &SparklineValue) -> Value { + match v { + SparklineValue::Number(n) => Value::Number(*n), + SparklineValue::Text(s) => Value::Text(s.clone()), + SparklineValue::Bool(b) => Value::Bool(*b), + SparklineValue::Blank => Value::Empty, + } +} + +/// A sparkline's parsed spec, carried in full: it is the value's identity, so +/// no surface projects it to text (every text projection of it is empty). +fn sparkline_to_json(spec: &SparklineSpec) -> JsonValue { + let data: Vec = spec.data.iter().map(|d| value_to_json(&sparkline_cell(d))).collect(); + let options: Vec = spec + .options + .iter() + .map(|(k, v)| json!([k, value_to_json(&sparkline_cell(v))])) + .collect(); + json!({ "charttype": spec.chart_type.as_str(), "data": data, "options": options }) +} + fn value_to_json(v: &Value) -> JsonValue { match v { Value::Number(n) | Value::Date(n) => json!({ "value": n, "type": "number" }), @@ -550,6 +635,7 @@ fn value_to_json(v: &Value) -> JsonValue { let items: Vec = arr.iter().map(value_to_json).collect(); json!({ "value": items, "type": "array" }) } + Value::Sparkline(spec) => json!({ "value": sparkline_to_json(spec), "type": "sparkline" }), } } diff --git a/crates/mcp/tests/sparkline_variables.rs b/crates/mcp/tests/sparkline_variables.rs new file mode 100644 index 000000000..5aeab1656 --- /dev/null +++ b/crates/mcp/tests/sparkline_variables.rs @@ -0,0 +1,136 @@ +//! A sparkline emitted by the MCP `evaluate` tool must be usable as a variable +//! on the way back in. +//! +//! `value_to_json` emits `{ "type": "sparkline", "value": {...} }`. Without the +//! matching decode in `parse_variables` that object matches no branch and the +//! binding is silently dropped to `empty`, so `TYPE(x)` answers 1 instead of +//! 128 and `ISBLANK(x)` answers TRUE — a wrong answer, not an error. + +use serde_json::{json, Value as JsonValue}; +use std::io::Write; + +/// Run one `evaluate` call against the binary and return its inner JSON result. +fn evaluate(formula: &str, variables: JsonValue) -> JsonValue { + let mut child = std::process::Command::new(env!("CARGO_BIN_EXE_truecalc-mcp")) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn() + .expect("failed to start truecalc-mcp"); + + let request = json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": { + "name": "evaluate", + "arguments": { "formula": formula, "variables": variables } + } + }); + + let stdin = child.stdin.as_mut().expect("stdin"); + writeln!(stdin, "{}", serde_json::to_string(&request).unwrap()).unwrap(); + drop(child.stdin.take()); + + let output = child.wait_with_output().expect("wait"); + let stdout = String::from_utf8_lossy(&output.stdout); + let response: JsonValue = serde_json::from_str(stdout.trim()).expect("json"); + let text = response["result"]["content"][0]["text"] + .as_str() + .unwrap_or_default(); + serde_json::from_str(text).expect("inner json") +} + +/// The emitted form of a sparkline, taken from the server itself rather than +/// hand-written, so the test breaks if either direction drifts. +fn emitted_sparkline() -> JsonValue { + let value = evaluate("=SPARKLINE({1,2,3},{\"color\",\"red\"})", json!({})); + assert_eq!(value["type"], json!("sparkline"), "emitted {value}"); + value +} + +#[test] +fn a_sparkline_variable_is_still_a_sparkline() { + let result = evaluate("=TYPE(x)", json!({ "x": emitted_sparkline() })); + assert_eq!( + result["value"], + json!(128.0), + "a sparkline variable must keep its own value kind, got {result}" + ); +} + +#[test] +fn a_sparkline_variable_is_not_blank() { + let result = evaluate("=ISBLANK(x)", json!({ "x": emitted_sparkline() })); + assert_eq!(result["value"], json!(false), "got {result}"); +} + +#[test] +fn a_malformed_sparkline_variable_is_dropped_rather_than_decoded() { + // The decoder accepts exactly what this server can emit and nothing wider: + // an unknown charttype, a `data` array shorter than two points (the + // evaluator answers `#N/A` for such a call, so it cannot emit one), and + // `charttype` left in the option list (it is always lifted into its own + // field). A rejected payload drops the binding — `TYPE(x)` then reports an + // unbound name's kind, never 128. + let point = json!({ "value": 1.0, "type": "number" }); + let unbound = evaluate("=TYPE(x)", json!({})); + for bad in [ + // Two valid points, so the unknown charttype is what rejects this — + // with an empty `data` the length guard would fire first and this case + // would pass even with the charttype check deleted. + json!({ "type": "sparkline", "value": { + "charttype": "bogus", + "data": [point.clone(), point.clone()], + "options": [] } }), + json!({ "type": "sparkline", "value": { + "charttype": "line", "data": [], "options": [] } }), + json!({ "type": "sparkline", "value": { + "charttype": "line", "data": [point.clone()], "options": [] } }), + json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), point.clone()], + "options": [["charttype", { "value": "bar", "type": "text" }]] } }), + // An option that is not a [key, value] pair. Without the length guard + // this indexes out of bounds — a panic in a server decoding + // caller-supplied JSON, not a rejection. + json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), point.clone()], + "options": [["color"]] } }), + json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), point.clone()], + "options": [["color", { "value": "red", "type": "text" }, "extra"]] } }), + // A data point whose payload does not match its own tag. + json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), { "value": "not a number", "type": "number" }], + "options": [] } }), + json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), { "value": 1.0, "type": "unknown" }], + "options": [] } }), + ] { + let result = evaluate("=TYPE(x)", json!({ "x": bad.clone() })); + assert_ne!(result["value"], json!(128.0), "decoded {bad}"); + assert_eq!(result, unbound, "should have been dropped: {bad}"); + } +} + +#[test] +fn a_sparkline_variable_round_trips_its_spec() { + // COUNTUNIQUE is the only surface that can see the spec, so it is what + // proves the payload survived rather than being rebuilt as some default. + let spark = emitted_sparkline(); + let same = evaluate( + "=COUNTUNIQUE(x,SPARKLINE({1,2,3},{\"color\",\"red\"}))", + json!({ "x": spark.clone() }), + ); + assert_eq!(same["value"], json!(1.0), "got {same}"); + + let different = evaluate( + "=COUNTUNIQUE(x,SPARKLINE({9,9,9}))", + json!({ "x": spark }), + ); + assert_eq!(different["value"], json!(2.0), "got {different}"); +} diff --git a/crates/wasm-workbook/src/lib.rs b/crates/wasm-workbook/src/lib.rs index 531cf8db0..3b0375b27 100644 --- a/crates/wasm-workbook/src/lib.rs +++ b/crates/wasm-workbook/src/lib.rs @@ -2,6 +2,7 @@ use serde::Serialize; use tsify_next::Tsify; use wasm_bindgen::prelude::*; +use truecalc_core::types::SparklineValue; use truecalc_core::Engine; use truecalc_workbook::{ Address, CellInput, Change, EngineFlavor, RecalcContext, Resolved, Value, Workbook, Worksheet, @@ -62,6 +63,30 @@ fn value_to_json(v: &Value) -> serde_json::Value { .collect(); serde_json::json!({"type": "array", "value": arr}) } + // The sparkline's parsed spec, carried in full: it is the value's + // identity, and every text projection of it is empty. + Value::Sparkline(spec) => { + let cell = |v: &SparklineValue| match v { + SparklineValue::Number(n) => value_to_json(&Value::Number(*n)), + SparklineValue::Text(s) => value_to_json(&Value::Text(s.clone())), + SparklineValue::Bool(b) => value_to_json(&Value::Boolean(*b)), + SparklineValue::Blank => value_to_json(&Value::Empty), + }; + let data: Vec = spec.data.iter().map(&cell).collect(); + let options: Vec = spec + .options + .iter() + .map(|(k, v)| serde_json::json!([k, cell(v)])) + .collect(); + serde_json::json!({ + "type": "sparkline", + "value": { + "charttype": spec.chart_type.as_str(), + "data": data, + "options": options, + } + }) + } } } diff --git a/crates/wasm/src/lib.rs b/crates/wasm/src/lib.rs index 3128b39ff..b7bec0926 100644 --- a/crates/wasm/src/lib.rs +++ b/crates/wasm/src/lib.rs @@ -9,14 +9,68 @@ use tsify_next::Tsify; use wasm_bindgen::prelude::*; use truecalc_core::types::zoned::parse_rfc9557; +use truecalc_core::types::{SparklineChartType, SparklineSpec, SparklineValue}; use truecalc_core::Value; +/// One sparkline data point / option value, read back from the shape +/// [`value_to_result`] emits for it. +fn json_to_sparkline_value(v: &serde_json::Value) -> Option { + let obj = v.as_object()?; + match obj.get("type")?.as_str()? { + "number" => Some(SparklineValue::number(obj.get("value")?.as_f64()?)), + "text" => Some(SparklineValue::Text(obj.get("value")?.as_str()?.to_owned())), + "bool" => Some(SparklineValue::Bool(obj.get("value")?.as_bool()?)), + "empty" => Some(SparklineValue::Blank), + _ => None, + } +} + +/// Read a `{ type: "sparkline", value: SparklineSpecResult }` object back into a +/// spec, so an emitted sparkline can be fed back in as a variable unchanged. +fn json_to_sparkline(spec: &serde_json::Value) -> Option { + let obj = spec.as_object()?; + let chart_type = SparklineChartType::parse(obj.get("charttype")?.as_str()?)?; + let raw_data = obj.get("data")?.as_array()?; + // The evaluator answers `#N/A` for a `data` argument with fewer than two + // points, so a shorter spec is not something it can emit — reject it here + // too, exactly as the workbook decoder does. + if raw_data.len() < 2 { + return None; + } + let mut data = Vec::new(); + for raw in raw_data { + data.push(json_to_sparkline_value(raw)?); + } + let mut options = Vec::new(); + for raw in obj.get("options")?.as_array()? { + let pair = raw.as_array()?; + if pair.len() != 2 { + return None; + } + let key = pair[0].as_str()?.to_ascii_lowercase(); + // `charttype` is lifted into the spec's own field, never left in the + // option list — so a payload carrying it there was not emitted by us. + if key == "charttype" { + return None; + } + options.push((key, json_to_sparkline_value(&pair[1])?)); + } + Some(SparklineSpec { + chart_type, + data, + options, + }) +} + /// Convert a JSON value (from JS) into a truecalc-core Value. /// /// A zoned instant round-trips in via the self-describing object /// `{ "type": "zoned", "value": "" }` (the same shape `value_to_result` -/// emits), so an emitted `Zoned` can be fed back as a variable. -fn json_to_value(v: &serde_json::Value) -> Value { +/// emits), so an emitted `Zoned` can be fed back as a variable. A sparkline +/// round-trips the same way, through `{ "type": "sparkline", "value": {...} }`: +/// without it an emitted sparkline would silently read back as `empty`, and +/// `TYPE(x)` would answer 1 instead of 128. +pub fn json_to_value(v: &serde_json::Value) -> Value { match v { serde_json::Value::Number(n) => n .as_f64() @@ -35,6 +89,11 @@ fn json_to_value(v: &serde_json::Value) -> Value { return Value::Zoned(Box::new(zi)); } } + if map.get("type").and_then(|t| t.as_str()) == Some("sparkline") { + if let Some(spec) = map.get("value").and_then(json_to_sparkline) { + return Value::Sparkline(Box::new(spec)); + } + } Value::Empty } _ => Value::Empty, @@ -91,6 +150,35 @@ pub enum EvalResult { /// An (unspilled) array result. Recursive: 2-D arrays are arrays of `array` /// rows. Cells carry their own type, including nested `date`/`error`/`empty`. Array { value: Vec }, + /// A sparkline: the parsed, validated render spec produced by `SPARKLINE`. + /// Google Sheets models this as a value kind of its own (`TYPE()` reports + /// the undocumented code `128`), and the spec is the value's identity, so + /// it is carried in full rather than projected to text. + Sparkline { value: SparklineSpecResult }, +} + +/// A parsed sparkline render spec on the WASM surface. `data` points and +/// option values are ordinary [`EvalResult`] cells (a blank cell inside the +/// source range is `empty`). +#[derive(Tsify, Serialize, Debug)] +pub struct SparklineSpecResult { + /// `line` (the default), `bar`, `column` or `winloss`. + pub charttype: String, + /// The points to plot, row-major. + pub data: Vec, + /// The remaining option key/value pairs, in the order given, keys + /// lower-cased. Keys the engine does not recognise are kept, not rejected: + /// Sheets ignores an unknown option key rather than erroring. + pub options: Vec<(String, EvalResult)>, +} + +fn sparkline_value_to_result(v: &SparklineValue) -> EvalResult { + match v { + SparklineValue::Number(n) => EvalResult::Number { value: *n }, + SparklineValue::Text(s) => EvalResult::Text { value: s.clone() }, + SparklineValue::Bool(b) => EvalResult::Bool { value: *b }, + SparklineValue::Blank => EvalResult::Empty, + } } /// Map a `truecalc-core` `Value` onto the WASM `EvalResult` surface shape. @@ -110,6 +198,17 @@ pub fn value_to_result(value: Value) -> EvalResult { Value::Array(items) => EvalResult::Array { value: items.into_iter().map(value_to_result).collect(), }, + Value::Sparkline(spec) => EvalResult::Sparkline { + value: SparklineSpecResult { + charttype: spec.chart_type.as_str().to_string(), + data: spec.data.iter().map(sparkline_value_to_result).collect(), + options: spec + .options + .iter() + .map(|(k, v)| (k.clone(), sparkline_value_to_result(v))) + .collect(), + }, + }, } } diff --git a/crates/wasm/tests/sparkline_round_trip.rs b/crates/wasm/tests/sparkline_round_trip.rs new file mode 100644 index 000000000..07b423e4c --- /dev/null +++ b/crates/wasm/tests/sparkline_round_trip.rs @@ -0,0 +1,99 @@ +//! A sparkline handed back in as a variable must arrive as the same value. +//! +//! `value_to_result` emits `{ type: "sparkline", value: {...} }`; without the +//! matching decode in `json_to_value` that object matches no branch and falls +//! through to `empty` *silently*, so `TYPE(x)` would answer 1 instead of 128 +//! and `ISBLANK(x)` would answer TRUE. This pins the contract `json_to_value`'s +//! own doc comment states. + +use truecalc_core::types::{SparklineChartType, SparklineSpec, SparklineValue}; +use truecalc_core::Value; +use truecalc_wasm::{json_to_value, value_to_result}; + +fn round_trip(value: Value) -> Value { + let emitted = serde_json::to_value(value_to_result(value)).expect("EvalResult serializes"); + json_to_value(&emitted) +} + +fn sparkline() -> Value { + Value::Sparkline(Box::new(SparklineSpec { + chart_type: SparklineChartType::Column, + data: vec![ + SparklineValue::number(1.0), + SparklineValue::Text("a".to_owned()), + SparklineValue::Blank, + SparklineValue::Bool(true), + ], + options: vec![ + ("color".to_owned(), SparklineValue::Text("red".to_owned())), + ("ymin".to_owned(), SparklineValue::number(0.0)), + ], + })) +} + +#[test] +fn a_sparkline_survives_the_emit_then_read_back_round_trip() { + let original = sparkline(); + let back = round_trip(original.clone()); + match (&original, &back) { + (Value::Sparkline(a), Value::Sparkline(b)) => assert_eq!(a, b, "the spec must survive"), + _ => panic!("a sparkline must not read back as {back:?}"), + } +} + +#[test] +fn a_read_back_sparkline_is_still_a_sparkline_to_the_engine() { + // The failure this guards is silent: `empty` evaluates fine, it just lies. + let back = round_trip(sparkline()); + assert!( + matches!(back, Value::Sparkline(_)), + "read back as {back:?}, so TYPE() would answer 1 instead of 128" + ); +} + +#[test] +fn a_malformed_sparkline_object_still_falls_back_to_empty() { + // Unchanged contract for anything that is not a well-formed spec — and the + // decoder accepts exactly what the engine can emit, nothing wider: a `data` + // array shorter than two points is `#N/A` from the evaluator, and + // `charttype` is always lifted out of the option list. + let point = serde_json::json!({ "type": "number", "value": 1.0 }); + for bad in [ + // Two valid points, so the unknown charttype is what rejects this — + // with an empty `data` the length guard would fire first and this row + // would pass with the charttype check deleted. + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "bogus", + "data": [point.clone(), point.clone()], + "options": [] } }), + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", "data": [], "options": [] } }), + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", "data": [point.clone()], "options": [] } }), + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), point.clone()], + "options": [["charttype", { "type": "text", "value": "bar" }]] } }), + // An option that is not a [key, value] pair. Without the length guard + // this indexes out of bounds — a panic, not a rejection. + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), point.clone()], + "options": [["color"]] } }), + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), point], + "options": [["color", { "type": "text", "value": "red" }, "extra"]] } }), + // A data point whose payload does not match its own tag. + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), { "type": "number", "value": "not a number" }], + "options": [] } }), + serde_json::json!({ "type": "sparkline", "value": { + "charttype": "line", + "data": [point.clone(), { "type": "unknown", "value": 1.0 }], + "options": [] } }), + ] { + assert_eq!(json_to_value(&bad), Value::Empty, "{bad}"); + } +} diff --git a/crates/workbook/src/recalc.rs b/crates/workbook/src/recalc.rs index f7b61a256..de14dc24a 100644 --- a/crates/workbook/src/recalc.rs +++ b/crates/workbook/src/recalc.rs @@ -1291,6 +1291,7 @@ fn core_to_workbook(v: CoreValue) -> Value { CoreValue::Empty => Value::Empty, CoreValue::Date(n) => Value::Date(n), CoreValue::Zoned(z) => Value::Zoned(z), + CoreValue::Sparkline(spec) => Value::Sparkline(spec), CoreValue::Array(elems) => core_array_to_workbook(elems), } } @@ -1334,6 +1335,7 @@ fn workbook_to_core(v: &Value) -> CoreValue { Value::Empty => CoreValue::Empty, Value::Date(n) => CoreValue::Date(*n), Value::Zoned(z) => CoreValue::Zoned(z.clone()), + Value::Sparkline(spec) => CoreValue::Sparkline(spec.clone()), Value::Array(rows) => CoreValue::Array( rows.iter() .map(|row| CoreValue::Array(row.iter().map(workbook_to_core).collect())) diff --git a/crates/workbook/src/value.rs b/crates/workbook/src/value.rs index cefd3233b..4603c3ce0 100644 --- a/crates/workbook/src/value.rs +++ b/crates/workbook/src/value.rs @@ -4,7 +4,7 @@ use serde::de::Error as _; use serde::ser::{Error as _, SerializeMap}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use truecalc_core::types::zoned::parse_rfc9557; -use truecalc_core::types::ZonedInstant; +use truecalc_core::types::{SparklineChartType, SparklineSpec, SparklineValue, ZonedInstant}; /// An evaluated cell value — one of the seven types of schema spec §6. /// @@ -54,6 +54,23 @@ pub enum Value { /// A zone-aware instant (Model B). Serialized as its canonical, self- /// describing RFC-9557 string, e.g. `2026-07-14T11:00:00+02:00[Europe/Berlin]`. Zoned(Box), + /// A sparkline: the parsed, validated render spec produced by `SPARKLINE` + /// (Google Sheets models it as a value kind of its own — `TYPE()` reports + /// the undocumented code `128`). + /// + /// Sheets keeps *two* notions of sameness for a sparkline, and this type + /// carries the deeper one. The `=` operator reports any two sparklines + /// equal, whatever they plot (that is the engine's + /// [`truecalc_core::Value`] equality); `COUNTUNIQUE` nonetheless counts two + /// different sparklines as 2 and two identical ones as 1. Storage needs the + /// deeper notion: recalc writes a recomputed cell back only when the new + /// value differs from the old, so if every sparkline compared equal here a + /// changed chart would silently keep its stale spec. Equality and hashing + /// therefore compare the whole spec, and canonical JSON carries it in + /// full — serializing it lossily (as `""`, or by dropping it and + /// recomputing from the formula) would collapse two genuinely different + /// sparklines into one canonical form. + Sparkline(Box), } /// Bit pattern of a finite f64 with `-0.0` normalized to `0.0`, so that @@ -102,6 +119,9 @@ impl PartialEq for Value { (Value::Array(a), Value::Array(b)) => a == b, (Value::Date(a), Value::Date(b)) => a == b, (Value::Zoned(a), Value::Zoned(b)) => a == b, + // Storage identity is the deep (COUNTUNIQUE-grade) one, not the + // `=` operator's — see the variant's doc comment. + (Value::Sparkline(a), Value::Sparkline(b)) => a == b, _ => match (self.error_code(), other.error_code()) { (Some(a), Some(b)) => a == b, _ => false, @@ -138,12 +158,84 @@ impl Hash for Value { } } } + // Hash the whole spec: it is this value's identity, so two + // sparklines that compare equal must hash equal. + Value::Sparkline(spec) => { + spec.chart_type.as_str().hash(state); + spec.data.len().hash(state); + for point in &spec.data { + hash_sparkline_value(point, state); + } + spec.options.len().hash(state); + for (key, value) in &spec.options { + key.hash(state); + hash_sparkline_value(value, state); + } + } // Handled above via `error_code()`. Value::Error(_) | Value::ErrorMsg(_, _) => unreachable!(), } } } +/// A sparkline data point / option value as an ordinary scalar cell value, so +/// a spec serializes in the same vocabulary as every other value on the wire. +fn sparkline_value_to_value(v: &SparklineValue) -> Value { + match v { + SparklineValue::Number(n) => Value::Number(if *n == 0.0 { 0.0 } else { *n }), + SparklineValue::Text(s) => Value::Text(s.clone()), + SparklineValue::Bool(b) => Value::Boolean(*b), + SparklineValue::Blank => Value::Empty, + } +} + +/// The inverse of [`sparkline_value_to_value`]; only scalar cell values can be +/// a data point or an option value. +fn value_to_sparkline_value(v: &Value) -> Result { + match v { + Value::Number(n) => Ok(SparklineValue::number(*n)), + Value::Text(s) => Ok(SparklineValue::Text(s.clone())), + Value::Boolean(b) => Ok(SparklineValue::Bool(*b)), + Value::Empty => Ok(SparklineValue::Blank), + _ => Err( + "a sparkline data point or option value must be a number, text, boolean or empty" + .to_string(), + ), + } +} + +fn hash_sparkline_value(v: &SparklineValue, state: &mut H) { + std::mem::discriminant(v).hash(state); + match v { + SparklineValue::Number(n) => normalized_bits(*n).hash(state), + SparklineValue::Text(s) => s.hash(state), + SparklineValue::Bool(b) => b.hash(state), + SparklineValue::Blank => {} + } +} + +/// Canonical wire form of a parsed sparkline spec. Keys are emitted in +/// lexicographic order (`charttype` < `data` < `options`) so the encoding is +/// canonical (JCS) like every other value in this module. +struct SparklineSpecWire<'a>(&'a SparklineSpec); + +impl Serialize for SparklineSpecWire<'_> { + fn serialize(&self, serializer: S) -> Result { + let data: Vec = self.0.data.iter().map(sparkline_value_to_value).collect(); + let options: Vec<(&str, Value)> = self + .0 + .options + .iter() + .map(|(k, v)| (k.as_str(), sparkline_value_to_value(v))) + .collect(); + let mut map = serializer.serialize_map(Some(3))?; + map.serialize_entry("charttype", self.0.chart_type.as_str())?; + map.serialize_entry("data", &data)?; + map.serialize_entry("options", &options)?; + map.end() + } +} + fn serialize_tagged_number( kind: &'static str, n: f64, @@ -178,6 +270,15 @@ impl Serialize for Value { map.serialize_entry("value", s)?; map.end() } + // The full parsed spec, never a lossy projection: it is the value's + // identity, so a canonical form that dropped it would make two + // different sparklines indistinguishable. + Value::Sparkline(spec) => { + let mut map = serializer.serialize_map(Some(2))?; + map.serialize_entry("type", "sparkline")?; + map.serialize_entry("value", &SparklineSpecWire(spec))?; + map.end() + } Value::Boolean(b) => { let mut map = serializer.serialize_map(Some(2))?; map.serialize_entry("type", "boolean")?; @@ -289,6 +390,7 @@ fn parse_value(raw: &serde_json::Value) -> Result { } } "array" => parse_array(payload), + "sparkline" => parse_sparkline(payload), other => Err(format!("unknown value type {other:?}")), } } @@ -312,6 +414,72 @@ fn parse_finite_f64(payload: &serde_json::Value, kind: &str) -> Result Result { + let obj = payload + .as_object() + .ok_or_else(|| "a sparkline value must be a JSON object".to_string())?; + if obj.len() != 3 + || !obj.contains_key("charttype") + || !obj.contains_key("data") + || !obj.contains_key("options") + { + return Err( + "a sparkline value must have exactly the fields \"charttype\", \"data\" and \"options\"" + .to_string(), + ); + } + + let raw_chart_type = obj["charttype"] + .as_str() + .ok_or_else(|| "a sparkline charttype must be a JSON string".to_string())?; + let chart_type = SparklineChartType::parse(raw_chart_type) + .ok_or_else(|| format!("unknown sparkline charttype {raw_chart_type:?}"))?; + + let raw_data = obj["data"] + .as_array() + .ok_or_else(|| "sparkline data must be a JSON array".to_string())?; + // The evaluator rejects a single-point `data` argument with `#N/A`, so a + // shorter spec is unrepresentable and must not round-trip in. + if raw_data.len() < 2 { + return Err("sparkline data must hold at least two points".to_string()); + } + let mut data = Vec::with_capacity(raw_data.len()); + for raw in raw_data { + data.push(value_to_sparkline_value(&parse_value(raw)?)?); + } + + let raw_options = obj["options"] + .as_array() + .ok_or_else(|| "sparkline options must be a JSON array".to_string())?; + let mut options = Vec::with_capacity(raw_options.len()); + for raw in raw_options { + let pair = raw + .as_array() + .filter(|p| p.len() == 2) + .ok_or_else(|| "a sparkline option must be a [key, value] pair".to_string())?; + let key = pair[0] + .as_str() + .ok_or_else(|| "a sparkline option key must be a JSON string".to_string())?; + if key != key.to_ascii_lowercase() { + return Err(format!("a sparkline option key must be lower-case, got {key:?}")); + } + if key == "charttype" { + return Err( + "charttype is carried by the sparkline's own field, not in options".to_string(), + ); + } + options.push((key.to_owned(), value_to_sparkline_value(&parse_value(&pair[1])?)?)); + } + + Ok(Value::Sparkline(Box::new(SparklineSpec { + chart_type, + data, + options, + }))) +} + fn parse_array(payload: &serde_json::Value) -> Result { let raw_rows = match payload.as_array() { Some(rows) => rows, diff --git a/crates/workbook/tests/sparkline_value_tests.rs b/crates/workbook/tests/sparkline_value_tests.rs new file mode 100644 index 000000000..8719836d8 --- /dev/null +++ b/crates/workbook/tests/sparkline_value_tests.rs @@ -0,0 +1,140 @@ +//! The sparkline value on the workbook wire: the parsed spec is carried in +//! full, and it is part of *storage* identity. +//! +//! Sheets keeps two notions of sameness. The `=` operator reports any two +//! sparklines equal whatever they plot — that is the engine's +//! `truecalc_core::Value` equality, pinned in `crates/core/tests/sparkline.rs`. +//! `COUNTUNIQUE` nonetheless counts two different sparklines as 2, so the spec +//! is retained and distinguishable, and the storage layer uses that deeper +//! notion: recalc only writes a recomputed cell back when it differs from the +//! stored one. + +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; + +use truecalc_core::types::{SparklineChartType, SparklineSpec, SparklineValue}; +use truecalc_workbook::Value; + +fn spec( + chart_type: SparklineChartType, + data: Vec, + options: Vec<(String, SparklineValue)>, +) -> Value { + Value::Sparkline(Box::new(SparklineSpec { + chart_type, + data, + options, + })) +} + +fn line(data: Vec) -> Value { + spec(SparklineChartType::Line, data, Vec::new()) +} + +fn nums(ns: &[f64]) -> Vec { + ns.iter().copied().map(SparklineValue::number).collect() +} + +fn hash_of(v: &Value) -> u64 { + let mut hasher = DefaultHasher::new(); + v.hash(&mut hasher); + hasher.finish() +} + +fn round_trip(v: &Value) -> Value { + serde_json::from_str(&serde_json::to_string(v).unwrap()).unwrap() +} + +#[test] +fn encoding_carries_the_whole_spec_with_canonical_key_order() { + let value = spec( + SparklineChartType::Column, + vec![ + SparklineValue::number(1.0), + SparklineValue::Blank, + SparklineValue::Text("a".to_owned()), + ], + vec![("color".to_owned(), SparklineValue::Text("red".to_owned()))], + ); + assert_eq!( + serde_json::to_string(&value).unwrap(), + r#"{"type":"sparkline","value":{"charttype":"column","data":[{"type":"number","value":1.0},{"type":"empty","value":null},{"type":"text","value":"a"}],"options":[["color",{"type":"text","value":"red"}]]}}"# + ); +} + +#[test] +fn spec_round_trips_through_json() { + let value = spec( + SparklineChartType::Winloss, + vec![ + SparklineValue::number(1.0), + SparklineValue::number(-1.0), + SparklineValue::Bool(true), + SparklineValue::Blank, + ], + vec![ + ("ymin".to_owned(), SparklineValue::number(0.0)), + ("bogus".to_owned(), SparklineValue::Text("x".to_owned())), + ], + ); + assert_eq!(round_trip(&value), value); +} + +#[test] +fn storage_equality_and_hashing_compare_the_spec() { + assert_eq!(line(nums(&[1.0, 2.0, 3.0])), line(nums(&[1.0, 2.0, 3.0]))); + assert_eq!( + hash_of(&line(nums(&[1.0, 2.0, 3.0]))), + hash_of(&line(nums(&[1.0, 2.0, 3.0]))) + ); + + // Different data, different chart type and different options are all + // different stored values — the payload is never ignored here, even though + // the `=` operator cannot see it. + assert_ne!(line(nums(&[1.0, 2.0, 3.0])), line(nums(&[1.0, 2.0, 4.0]))); + assert_ne!( + line(nums(&[1.0, 2.0])), + spec(SparklineChartType::Bar, nums(&[1.0, 2.0]), Vec::new()) + ); + assert_ne!( + line(nums(&[1.0, 2.0])), + spec( + SparklineChartType::Line, + nums(&[1.0, 2.0]), + vec![("color".to_owned(), SparklineValue::Text("red".to_owned()))] + ) + ); +} + +#[test] +fn a_sparkline_is_not_equal_to_any_scalar() { + assert_ne!(line(nums(&[1.0, 2.0])), Value::Text(String::new())); + assert_ne!(line(nums(&[1.0, 2.0])), Value::Empty); + assert_ne!(line(nums(&[1.0, 2.0])), Value::Number(0.0)); +} + +#[test] +fn malformed_specs_are_rejected_on_decode() { + let bad = [ + // Unknown charttype. + r#"{"type":"sparkline","value":{"charttype":"bogus","data":[{"type":"number","value":1.0},{"type":"number","value":2.0}],"options":[]}}"#, + // A single data point is unrepresentable (the evaluator answers #N/A). + r#"{"type":"sparkline","value":{"charttype":"line","data":[{"type":"number","value":1.0}],"options":[]}}"#, + // Missing field. + r#"{"type":"sparkline","value":{"charttype":"line","data":[{"type":"number","value":1.0},{"type":"number","value":2.0}]}}"#, + // An option must be a [key, value] pair. + r#"{"type":"sparkline","value":{"charttype":"line","data":[{"type":"number","value":1.0},{"type":"number","value":2.0}],"options":[["color"]]}}"#, + // charttype belongs to its own field, not to options. + r#"{"type":"sparkline","value":{"charttype":"line","data":[{"type":"number","value":1.0},{"type":"number","value":2.0}],"options":[["charttype",{"type":"text","value":"bar"}]]}}"#, + // Option keys are stored lower-cased. + r#"{"type":"sparkline","value":{"charttype":"line","data":[{"type":"number","value":1.0},{"type":"number","value":2.0}],"options":[["COLOR",{"type":"text","value":"red"}]]}}"#, + // A data point must be a scalar cell value. + r#"{"type":"sparkline","value":{"charttype":"line","data":[{"type":"number","value":1.0},{"type":"array","value":[[{"type":"number","value":1.0},{"type":"number","value":2.0}]]}],"options":[]}}"#, + ]; + for json in bad { + assert!( + serde_json::from_str::(json).is_err(), + "should have been rejected: {json}" + ); + } +} From 085b015346d84217f0e6ca3be25169bf821bd7a1 Mon Sep 17 00:00:00 2001 From: hhimanshu <6589036+hhimanshu@users.noreply.github.com> Date: Tue, 28 Jul 2026 14:22:35 +1200 Subject: [PATCH 2/2] test(conformance): let SPARKLINE land before its fixture rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage guard requires every registered function to have fixture rows, and the "Check fixture / code separation" CI job rejects any PR touching both `fixtures/google_sheets/*.tsv` and code. Those two rules deadlock for a new function: code first leaves SPARKLINE registered with no rows and fails this test; fixtures first leaves 103 rows for a function the engine does not have and fails the conformance run. Add SPARKLINE to the existing pending-fixture-verification set to break the tie for exactly one merge. Unlike QUERY, which is there because its rows do not exist yet, SPARKLINE's rows are pipeline-verified and land in the immediately following fixtures-only PR — the entry is removed there, so the guard enforces SPARKLINE from that point on. Refs #766 --- crates/core/tests/conformance.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/core/tests/conformance.rs b/crates/core/tests/conformance.rs index d72c2482e..c62065cec 100644 --- a/crates/core/tests/conformance.rs +++ b/crates/core/tests/conformance.rs @@ -765,7 +765,17 @@ fn every_registered_function_has_conformance_coverage() { // fixtures pipeline this repo's CI does not have access to — self-verified // fixture values are forbidden. Remove from this set once QUERY has // pipeline-verified fixture rows. - let pending_fixture_verification: std::collections::HashSet<&str> = ["QUERY"].iter().copied().collect(); + // + // SPARKLINE (issue #766) is here for a different, purely mechanical reason: + // its 103 pipeline-verified rows exist and land in the immediately + // following fixtures-only PR. The "Check fixture / code separation" CI job + // rejects any PR touching both `fixtures/google_sheets/*.tsv` and code, and + // the two orderings deadlock — code first leaves a registered function with + // no rows (this test), fixtures first leaves rows for a function the engine + // does not have. This entry breaks that tie for exactly one merge, and is + // removed in the follow-up that lands the rows. + let pending_fixture_verification: std::collections::HashSet<&str> = + ["QUERY", "SPARKLINE"].iter().copied().collect(); let gdir = fixture_dir(); let vars: HashMap = HashMap::new();