feat(google)!: SPARKLINE — parse and validate the in-cell chart by hhimanshu · Pull Request #770 · truecalc/core

hhimanshu · 2026-07-28T02:15:38Z

Refs #766 (closed by the follow-up fixtures PR, which adds the conformance rows)

Summary

SPARKLINE(data, [options]) — Google Sheets' in-cell chart function. The engine parses and validates its arguments and produces a render spec; drawing stays with the consumer.

⚠️ BREAKING — adds a variant to the public Value enums in truecalc-core and truecalc-workbook. Neither is #[non_exhaustive], so downstream exhaustive matches will not compile. The commit carries a ! and a BREAKING CHANGE: footer, so release-plz should compute 7.0.0 from the just-released 6.1.0.

The value model, and why it is not the obvious one

SPARKLINE returns something that is not a number, string, bool, date or error. TYPE() reports 128 — outside the documented 1/2/4/16/64 set — so Sheets models it as a distinct value kind, not a specially-formatted string.

The variant follows ErrorMsg's shape (a variant carrying a validated payload) but not its identity treatment, and the two crates deliberately disagree:

	equality
`truecalc_core::Value`	all sparklines compare equal — matches Sheets' `=`
`truecalc_workbook::Value`	spec-deep — matches the `COUNTUNIQUE`-grade notion

That split is deliberate and load-bearing. recalc.rs::apply_changes writes a recomputed cell back only if old != new; under coarse equality, a cell whose sparkline changed would compare equal to its stale stored value and silently keep — and export — the old spec. A reviewer independently found a second site with the same exposure: the fixed-point loop's convergence check would declare premature convergence.

This was not the first design. The original built identity over the spec, on the strength of =SPARKLINE({1,2,3})=SPARKLINE({1,2,3}) → TRUE. That row was missing its control. =SPARKLINE({1,2,3})=SPARKLINE({9,9,9}) is also TRUE — every sparkline equals every other. COUNTUNIQUE is what actually distinguishes them (2 for different, 1 for identical), so the spec must still be carried, for a completely different reason than the one first recorded.

Coercion is asymmetric, and the asymmetry is not derivable

SPARKLINE(...) + 1     #VALUE!      LEN / N            0
"x" & SPARKLINE(...)   #VALUE!      TO_TEXT / LEFT     ""
CONCATENATE(...,"x")   "x"          IF(...,1,2)        2   (falsy)
SUM(..., 1)            1            EXACT(...,"")      TRUE

Note & errors while CONCATENATE succeeds, and =SPARKLINE(...)="" is FALSE while EXACT(...,"") is TRUE. Both are genuine Sheets asymmetries, reproduced rather than smoothed over.

The sharpest pair:

DOLLAR(SPARKLINE(...))      #VALUE!
TO_DOLLARS(SPARKLINE(...))  ""

Near-identical names, opposite answers. No reading of either implementation produces that — which is why the module carries a deliberately short coercion note: the three coercion seams and what each yields, a pointer to google.tsv as the only authority, and one rule — find the row; if there is no row, probe it. Every predictive claim was removed after three successive attempts to summarise engine behaviour in a comment turned out wrong.

Errors

class	code
arity/shape of `data` (no args, scalar, single value)	`#N/A`
structural malformation (empty array, non-pair options)	`#REF!`
bad option value (unknown charttype)	`#VALUE!`
unknown option key	silently ignored — deliberate forward compatibility

Also in this change

Round-trip on all four surfaces. WASM and MCP previously emitted a sparkline they could not read back — it fell to Value::Empty silently, so TYPE(x) returned 1 instead of 128. Both now decode, with guards matching the workbook decoder exactly, so all three decoders share one contract.
Conformance skips are now visible. The TSV runner prints a per-file tally on every run. This immediately surfaced that text.tsv has 46 rows asserting nothing — pre-existing, unrelated, previously uncounted. See test(conformance): silently skipped rows assert nothing — 46 in text.tsv alone, and one pair passed for the wrong reason #767.
A narrow harness skip for rows reading authored cells, because two such rows were passing for the wrong reason: they expect 0, and an unresolved cross-sheet read also yields 0. Documented as a mitigation, not test(conformance): silently skipped rows assert nothing — 46 in text.tsv alone, and one pair passed for the wrong reason #767's fix.

How to test

Fastest repro — the pair that decided the value model:

cargo run -p truecalc-mcp   # then evaluate:
=SPARKLINE({1,2,3})=SPARKLINE({9,9,9})     # TRUE  — all sparklines are equal
=COUNTUNIQUE(SPARKLINE({1,2,3}),SPARKLINE({9,9,9}))   # 2 — but the spec survives
=TYPE(SPARKLINE({1,2,3}))                  # 128
=DOLLAR(SPARKLINE({1,2,3}))                # #VALUE!
=TO_DOLLARS(SPARKLINE({1,2,3}))            # "" — the sibling disagrees

Or run the suite: cargo test -p truecalc-core --test sparkline (46 tests, each citing its fixture row), and cargo test -p truecalc-core --test conformance -- --nocapture to see the new per-file tally.

Review

Five independent review rounds, each by a reviewer that did not write the code. Every finding fixed:

1 blocker — EQ() returned FALSE where = returned TRUE. Sheets' comparison-alias functions run through a separate comparison implementation that had no sparkline arm. The contract every serialization decision rested on held through one path and was inverted through the other.
The value model itself was overturned by an oracle probe after round 2, as described above.
PRODUCT skipped only in array position, not as a direct argument — inconsistent with every other aggregate and with itself.
TO_DOLLARS/TO_PURE_NUMBER/TO_DATE kept the opposite answer from their probed siblings, in the same module, with character-identical match shapes.
MAXA returned #N/A where Sheets returns 0.
Verified by evaluation rather than by reading tests: that PRODUCT changed no non-sparkline behaviour (30 inputs diffed against origin/main via purpose-built probe binaries), that the decoders survive malformed input including a 10,000-element array and 2,000-deep nesting, and that Hash/PartialEq agree within each crate.

⚠️ A fifth review round on the harness change is still in flight. Everything else is reviewed and clean. I will report its findings here and push any fixes before asking for merge.

Ground truth

103 rows, every value observed in live Google Sheets — grown from 26 across five probe batches, each triggered by a review finding that could not be settled by reasoning. The rows arrive in the follow-up fixtures PR (CI hard-fails a PR touching both fixtures and code).

Of the 103: 65 enforced, 29 skipped (no text projection), 9 skipped (authored cells) — stated because an earlier version of this work overstated the gate's coverage.

Test plan

cargo test --workspace — 88 suites, 0 failures
cargo clippy --workspace -- -D warnings — clean (CI's exact form)
Both wasm-pack builds green
cargo fmt --check / clippy --all-targets dirty on main already — confirmed pre-existing, no new dirt from this diff
CI green
Fifth review round reported

SPARKLINE: implement the Google Sheets in-cell chart function #766 — the issue
test(conformance): silently skipped rows assert nothing — 46 in text.tsv alone, and one pair passed for the wrong reason #767 — conformance rows that assert nothing (this change makes them visible; it does not fix them)
fix(workbook): the JSON schema is missing branches for non-scalar value variants #768 — the workbook JSON schema is missing zoned and sparkline branches

🤖 Generated with Claude Code

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

github-actions · 2026-07-28T02:18:23Z

Test Coverage by Category

Category	Unit Tests	Google Sheets Conformance	Property Cases	Total
Array	42	547/547 ✓	1,000 (2×500)	1,589
Database	35	182/182 ✓	3,500 (7×500)	3,717
Date	369	418/418 ✓	2,500 (5×500)	3,287
Engineering	245	886/888 ⚠	5,500 (11×500)	6,633
Filter	11	80/80 ✓	4,500 (9×500)	4,591
Financial	149	1,208/1,208 ✓	2,000 (4×500)	3,357
Info	0	256/256 ✓	4,500 (9×500)	4,756
Logical	121	263/263 ✓	3,500 (7×500)	3,884
Lookup	69	392/392 ✓	1,000 (2×500)	1,461
Math	536	2,006/2,006 ✓	8,000 (16×500)	10,542
Operator	87	250/250 ✓	7,500 (15×500)	7,837
Parser	83	92/92 ✓	4,000 (8×500)	4,175
Query	37	—	—	37
Statistical	483	3,156/3,156 ✓	5,000 (10×500)	8,639
Text	298	729/733 ⚠	4,000 (8×500)	5,031
Timezone	47	—	—	47
Volatile	0	—	3,500 (7×500)	3,500
Web	29	56/56 ✓	6,000 (12×500)	6,085
Total	2,897	10,521/10,527	66,000 (132×500)	~79,424

_{✓ = 100% passing · ⚠ = known deviation · The ~79,424 total counts formula evaluations (each conformance row and each property case = 1). GitHub Checks reports 3,731 Rust test functions: 2,897 unit + 159 property functions (shown as cases above) + 675 conformance/integration.}

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<SparklineSpec>)` 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

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

github-actions · 2026-07-28T03:31:39Z

Test Coverage by Category

Category	Unit Tests	Google Sheets Conformance	Property Cases	Total
Array	42	547/547 ✓	1,000 (2×500)	1,589
Database	35	182/182 ✓	3,500 (7×500)	3,717
Date	369	418/418 ✓	2,500 (5×500)	3,287
Engineering	245	886/888 ⚠	5,500 (11×500)	6,633
Filter	11	80/80 ✓	4,500 (9×500)	4,591
Financial	149	1,208/1,208 ✓	2,000 (4×500)	3,357
Info	0	256/256 ✓	4,500 (9×500)	4,756
Logical	121	263/263 ✓	3,500 (7×500)	3,884
Lookup	69	392/392 ✓	1,000 (2×500)	1,461
Math	536	2,006/2,006 ✓	8,000 (16×500)	10,542
Operator	87	250/250 ✓	7,500 (15×500)	7,837
Parser	83	92/92 ✓	4,000 (8×500)	4,175
Query	37	—	—	37
Statistical	483	3,156/3,156 ✓	5,000 (10×500)	8,639
Text	298	729/733 ⚠	4,000 (8×500)	5,031
Timezone	47	—	—	47
Volatile	0	—	3,500 (7×500)	3,500
Web	29	56/56 ✓	6,000 (12×500)	6,085
Total	2,897	10,521/10,527	66,000 (132×500)	~79,424

_{✓ = 100% passing · ⚠ = known deviation · The ~79,424 total counts formula evaluations (each conformance row and each property case = 1). GitHub Checks reports 3,732 Rust test functions: 2,897 unit + 159 property functions (shown as cases above) + 676 conformance/integration.}

hhimanshu self-assigned this Jul 28, 2026

hhimanshu force-pushed the feat/766-sparkline-code branch from 4161cec to 1fe5112 Compare July 28, 2026 02:48

hhimanshu added 2 commits July 28, 2026 15:24

hhimanshu force-pushed the feat/766-sparkline-code branch from 1fe5112 to 085b015 Compare July 28, 2026 03:28

hhimanshu merged commit 2d5eeb2 into main Jul 28, 2026
7 checks passed

hhimanshu deleted the feat/766-sparkline-code branch July 28, 2026 03:40

github-actions Bot locked and limited conversation to collaborators Jul 28, 2026

Navigation Menu

Uh oh!

feat(google)!: SPARKLINE — parse and validate the in-cell chart - #770

feat(google)!: SPARKLINE — parse and validate the in-cell chart#770
hhimanshu merged 2 commits into
mainfrom
feat/766-sparkline-code

hhimanshu commented Jul 28, 2026 •

edited by blacksmith-sh Bot

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hhimanshu commented Jul 28, 2026 • edited by blacksmith-sh Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The value model, and why it is not the obvious one

Coercion is asymmetric, and the asymmetry is not derivable

Errors

Also in this change

How to test

Review

Ground truth

Test plan

Related

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Coverage by Category

Uh oh!

github-actions Bot commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Coverage by Category

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hhimanshu commented Jul 28, 2026 •

edited by blacksmith-sh Bot

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading

github-actions Bot commented Jul 28, 2026 •

edited

Loading