test(conformance): silently skipped rows assert nothing — 46 in text.tsv alone, and one pair passed for the wrong reason

[hhimanshu]

What

The conformance TSV runner skips any row whose expected value is empty, and has no comparison arm for a result that is not a number/text/boolean/error/array. Together these mean a whole class of fixture rows is recorded, looks enforced, and asserts nothing.

Why this matters now

SPARKLINE (#766) is the first function whose successful result has no text projection — Sheets displays it as empty. Every "this renders fine" row therefore has an empty expected value, and:

• The runner skips them. crates/core/tests/conformance.rs (the formula.is_empty() || expected_str.trim().is_empty() guard) drops the row before evaluation. Of the first 28 SPARKLINE rows added, 13 are inert — every charttype row, the colour and ymin/ymax rows, unknown-option-key, bar-with-three-values, text-in-data, all-negative, and both real-range rows.
• Including one that matters. TO_TEXT(SPARKLINE(...)) → "" is one of three deliberately pinned coercion exceptions, and the conformance gate does not check it. Only a hand-written test does.
• The new infer_type arm is dead code. No enforced row produces a non-scalar value, so the arm added to conformance.rs and conformance_reporter.rs is never reached.
• A filled-in row would fail. values_match has no arm pairing a non-scalar actual against a text expected. If someone later gives one of these rows a non-empty expected value, it fails for a harness reason rather than a conformance reason — which is the worst possible failure, because it looks like a real divergence.

The empty-expected skip is right for genuinely unknown rows. It is wrong when empty is the observed value.

The distinction the format cannot currently express

Three different things collapse into an empty cell today:

1. not yet probed
2. probed, and the result is the empty string
3. probed, and the result is a value with no text projection at all

Only (1) should be skipped. Today all three are.

Sketch of a fix

Not prescriptive — whoever picks this up should weigh these:

• Use the existing expected_type column, which already carries a per-row type hint, to distinguish "no text projection" from "not probed" without a format change.
• Or add an explicit sentinel for "observed empty" so the skip rule can be narrowed to genuinely-unknown rows.
• Either way, values_match needs an arm for a non-scalar actual, and the reporter should count skipped rows visibly so a silently-inert block cannot grow again unnoticed.

Whatever is chosen must not disturb the existing skip behaviour for rows that really are unprobed.

Acceptance criteria

• A fixture row can assert "this evaluates successfully and has no text projection" and actually enforce it
• values_match handles a non-scalar actual rather than failing for a harness reason
• The runner reports how many rows it skipped, so inert blocks are visible
• The previously-inert SPARKLINE rows become enforced
• Genuinely-unprobed rows still skip as before
• CI green

Related

• SPARKLINE: implement the Google Sheets in-cell chart function #766 — SPARKLINE, which surfaced this

[hhimanshu]

A skipped row does not only fail to assert — it can pass for the wrong reason

Update from #766, where the fixture set has since grown to 90 SPARKLINE rows. Two things worth recording here, because they strengthen the case beyond "some rows are inert".

The stronger evidence: a false green

The Google Sheets conformance fixtures now include rows that read authored input cells, so that a sparkline reaches an aggregate the way a real workbook delivers one — from a cell holding =SPARKLINE(...) rather than as a literal argument:

=SUM(Data!K1:K1)       0     (K1 holds a sparkline)
=PRODUCT(Data!K1:K1)   0
=SUM(Data!K1:K2)       5     (K2 = 5)
=PRODUCT(Data!K1:K2)   5
=TYPE(Data!K1)         128

The TSV runner evaluates every row standalone, with no workbook behind it, so each of those references resolves to empty. Seven of the rows failed outright. Two passed — =SUM(Data!K1:K1) and =PRODUCT(Data!K1:K1), because an unresolved read produces 0 and the recorded value is also 0.

Those two rows would have gone green whether or not the engine handled a range-delivered sparkline at all. That is worse than an inert row: an inert row is visibly doing nothing once you look, whereas this one reports success while testing nothing. It is also the exact pair that caught a real divergence in #766 — the direct and array aggregate paths disagreed, and only the single-cell rows could tell them apart (with a second value present, "skipped" and "contributes the identity element" both give the same answer).

Second silent skip category, and a partial mitigation

#766 adds a skip for rows reading a sheet-qualified reference, so those rows neither fail nor pass by accident. That is a second silent skip in the same runner, which makes this issue worse rather than better — so the same change makes the skipping visible. Both runners now print a per-file breakdown on every run:

google.tsv: 104 rows — 69 enforced, 26 skipped (no text projection), 9 skipped (authored cells)
text.tsv: 779 rows — 732 enforced, 46 skipped (no text projection), 1 skipped (not a formula)
array.tsv: 552 rows — 547 enforced, 5 skipped (no text projection)
lookup.tsv: 397 rows — 392 enforced, 5 skipped (no text projection)
logical.tsv: 267 rows — 263 enforced, 4 skipped (no text projection)
web.tsv: 59 rows — 56 enforced, 3 skipped (no text projection)
filter.tsv: 81 rows — 80 enforced, 1 skipped (no text projection)
operator.tsv: 251 rows — 250 enforced, 1 skipped (no text projection)
parser.tsv: 93 rows — 92 enforced, 1 skipped (no text projection)

text.tsv is the one to note: 46 inert rows, unrelated to any non-scalar value. This is not a SPARKLINE-only problem.

This is a mitigation, not the fix. The rows are still skipped and still assert nothing; they merely announce themselves now. The acceptance criteria above are unchanged, with one addition suggested by the false-green pair:

• rows that read authored input cells need a seeded resolver in this harness (the workbook conformance test already has one), not just a skip — otherwise the accidental-match failure mode returns the moment someone adds such a row to another category file.

For #766 the affected rows are enforced in a dedicated test file against a hand-seeded resolver, which is a per-function workaround rather than something the harness offers.

Current counts for the SPARKLINE block

90 rows: 55 enforced, 26 skipped for having no text projection, 9 skipped for reading authored cells. Every one of the 35 is asserted in a hand-written test instead.

[hhimanshu]

Reframing this issue — it is not about non-scalar results

I filed this. The title was wrong, and it undersold the problem badly enough to change how it should be prioritised, so I have retitled it.

What I filed: "a new function's rows are inert because its results have no text projection." That reads as a niche problem confined to one unusual function, worth fixing eventually.

What it actually is, now that skipped rows are counted rather than invisible:

• text.tsv has 46 rows that assert nothing. Nothing to do with non-scalar values — that is a heavily-used file, and nobody knew.
• Across the suite: array.tsv 5, lookup.tsv 5, logical.tsv 4, web.tsv 3, filter.tsv 1, operator.tsv 1, parser.tsv 1.
• And a pair of rows passed for the wrong reason — =SUM(Data!K1:K1) and =PRODUCT(Data!K1:K1) both expect 0, and an unresolved cross-sheet read also yields 0. Green whether the engine works or not.

That last one is the real severity. An inert row is a gap. A row that passes for the wrong reason is misinformation — it reports coverage that does not exist, and it survives the behaviour it claims to check being broken outright. It was caught only because the same pair was needed to settle a genuine implementation divergence.

The dependency worth stating before anyone starts

An empty expected value collapses two different cases:

1. Nobody probed it → probing yields a real value, the row becomes enforced, no harness work needed.
2. The observed answer genuinely is empty → the format cannot express it, and that is this issue.

You cannot tell which without asking the oracle. So probing the empty-valued rows should come first — it fixes case 1 outright and, more importantly, measures case 2. Building harness support before knowing how many rows actually need it is building for an unmeasured population.

The probe work for those rows is tracked in the conformance-fixtures pipeline. This issue should be scoped to whatever remains after that count is known.

Suggested acceptance-criteria addition

Rows that read authored cells need a seeded resolver, not merely a skip. A skip prevents the false green for those specific rows; it does not prevent the next person adding such a row elsewhere and hitting the same accidental match.