Repository navigation
fix: an empty string is no longer coerced to 0 in numeric contexts (HF-361) - #1768
marcin-kordas-hoc wants to merge 9 commits into
Conversation
`ArithmeticHelper#coerceNonDateScalarToMaybeNumber` special-cased an
empty string ('') to coerce to 0, the same as a truly blank cell
(EmptyValue). This is a different case from EmptyValue -- a cell or
literal holding '' is text, not blank -- and it made HyperFormula
diverge from Excel in two ways:
- Arithmetic on '' returned 0 instead of #VALUE! (e.g. `=""+0`).
- COUNT("") returned 1 instead of 0, since the literal argument was
coerced to 0 (an ExtendedNumber) before COUNT's own
isExtendedNumber predicate ever saw it.
Root cause:
- Interpreter.ts `plusOp`/`minusOp`/etc. call
`ArithmeticHelper#coerceScalarToNumberOrError`, which returned 0
for '' before this fix.
- NumericAggregationPlugin#reduce (used by COUNT, SUM, AVERAGE,
PRODUCT, MIN, MAX) calls the same coercion for a literal/expression
argument (as opposed to a direct cell reference, which uses the
aggregation's own predicate without numeric coercion) -- so
COUNT("") went through the same 0-producing bug before
isExtendedNumber(0) counted it.
This removes the `arg === ''` special case, letting '' fall through
to the normal string-to-number parsing (which correctly fails for
non-numeric text), matching how HyperFormula already treats any
other non-numeric string literal (e.g. `SUM("abc")` already returned
#VALUE!; `SUM("")` was the sole inconsistent exception).
Not affected (verified with tests): blank cells (EmptyValue) still
coerce to 0 in arithmetic and are still governed separately by
`evaluateNullToZero`; a '' criterion in COUNTIF/SUMIF-style functions
still matches only genuinely blank cells (Criterion.ts resolves that
case before any numeric coercion runs).
This is a breaking change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
hyperformula-docs | cc66c2a | Commit Preview URL Branch Preview URL |
Oct 23 2026, 08:24 AM |
The fix itself was already in place; this adds what a breaking change owes
the reader, and moves the tests where this repo keeps them.
- Changelog: moved the entry from Fixed to Changed and rewrote it in the
"A **breaking change**:" form the other 4.0 entries use, with a migration
guide link and the PR number instead of an internal task id.
- Migration guide: new docs/guide/migration-from-3.x-to-4.0.md (plus its
sidebar entry) with a section on empty-string coercion. Every before/after
value in it is measured, not assumed -- two rounds of probes across the
unpatched and patched engine, plus the assertions the private suite already
pinned. That caught two wrong claims: DATE("","","") returned #NUM! and not
a computed date, and ACOT("") returned 1.5707963267949 -- a plausible number
from an argument that was never numeric, which is the most useful example
this change has.
- Removed test/hf-361-empty-string-coercion.spec.ts. DEV_DOCS.md:129 is
explicit that interpreter specs do not live in the public repo, because a
copy here runs twice and leaves develop red if only one of the paired PRs
lands. Those tests move to the paired hyperformula-tests branch of the same
name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Performance comparison of head (cc66c2a) vs base (d143456) |
…n guide
Two independent reviews converged on the same defect: both documents implied
that a cell holding an empty string breaks any numeric formula. It does not.
Measured on both engines: SUM, AVERAGE, MIN, MAX and COUNT over a reference
or a range are byte-identical before and after -- they have always ignored
text found there, matching Excel -- because the aggregation path filters
strings out before this coercion is reached. Only arithmetic, number-typed
arguments, and an empty string written directly into an aggregation change.
A reader with =SUM(A1:A100) would have been told to expect a break that
cannot happen, in a document whose whole job is to predict breaks.
Also corrected or added, all measured rather than read off a test:
- =ACOT("") returned 1.5707963268, not 1.5707963267949. I had taken the
digits from the private suite's toBeCloseTo(..., 10) assertion, which
passes for both. Default config rounds to 10 significant digits.
- =DATE(A1, 2, 3) over a text empty string silently returned 35 -- a date in
1900 -- which is the same "answered instead of failing" class as ACOT and a
better example, because it is the reference form users actually write.
- Added the contexts the table omitted while claiming to be exhaustive:
=""%, =SUM(1, ""), =ABS, =INT, =SUMSQ, =MEDIAN, =STDEV.
- Added the nested-expression class (=SUM(IF(TRUE,"",1)), =SUM(LEFT("abc",0))),
where 3.x was wrong and 4.0 matches Excel, and the one class that moves away
from Excel (an argument Excel evaluates as an array of only text), with the
pre-existing non-array-aware IF that actually causes it.
- Fixed the code blocks against DOCS_CONTENT_GUIDE: the mandated
{ sheet, row, col } key order, a snippet that actually runs as written
(verified), and an untagged fence for the before/after pair that is not
JavaScript. Pointed N() and IFERROR at their real category anchors.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c99427b. Configure here.
Tobiadefami
left a comment
Tobiadefami
left a comment
There was a problem hiding this comment.
Could we preserve reference handling through parentheses? With B1 containing ="", =SUM((B1)) returns 0 in Excel Online and on base, but #VALUE! on this head.
A possible fix is to recursively look through parentheses when checking for CELL_REFERENCE in NumericAggregationPlugin.reduce(). I tested this temporarily: it fixes the regression while preserving rejection of literal empty strings. All 195 tests across 16 checked suites passed.
…eric aggregations Reported in review on this PR: with B1 holding ="", SUM(B1) correctly returns 0 (aggregation over a reference ignores non-numeric text, unchanged by this PR), but SUM((B1)) returned #VALUE!. Root cause: NumericAggregationPlugin#reduce decides how to treat an argument by checking arg.type against CELL_REFERENCE (and separately against CELL_RANGE/COLUMN_RANGE/ROW_RANGE) to tell "this is a reference, ignore its non-numeric text" apart from "this is a literal/computed value, coerce it to a number." A parenthesized argument reaches the reducer as a PARENTHESIS node wrapping the real expression, so neither check matched and (B1) fell through to the same coercion "" now correctly fails for as a literal. reduce() is the single shared implementation behind SUM, SUMSQ, COUNT, COUNTA, AVERAGE, AVERAGEA, MIN, MAX, MINA, MAXA, PRODUCT, VAR.S/P, VARA/VARPA and STDEV.S/P/A/PA (via doAverage/doCount/reduceAggregate(A)/doMax/doMin/ doSum/doProduct), so this was not SUM-specific: every one of those reproduced the same #VALUE! regression through a single extra pair of parentheses. Fix: unwrap PARENTHESIS nodes before either arg.type check, the same idiom already used in FunctionPlugin#runFunctionWithReferenceArgument and InformationPlugin. PARENTHESIS is a pure passthrough in Interpreter#evaluateAst, so this only changes which branch classifies the argument, not what evaluateAst(arg, state) itself returns. Verified against Excel Online via MS Graph: a parenthesized reference or range behaves identically to the bare one in every case tested (single and double parentheses, SUM/COUNT/AVERAGE/MIN/MAX/PRODUCT/SUMSQ/STDEV, a range containing an embedded empty string). Ranges wrapped in parentheses already worked before this fix -- they take a different path in reduce() that checks the evaluated value's runtime type (SimpleRangeValue) rather than the AST node type -- so this fix does not change range handling, only single-cell (and column/row) references. Paired failing-then-passing tests: handsontable/hyperformula-tests, same branch, "parentheses around a reference do not change numeric-aggregation behavior" in empty-string-coercion.spec.ts. Negative control: reverting this commit's source change while keeping those tests fails 5 of the 8 new specs (the 3 that also pass without the fix are pins on already-correct behavior: COUNT((A1)) and the two parenthesized-range cases). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@Tobiadefami reproduced and fixed — thanks for catching it, and sorry it sat this long.
That choke point ( Verified against Excel Online (MS Graph, live session): parentheses are fully transparent to reference handling in every case I measured — single and double parens, the whole aggregation family, and a parenthesized range containing an embedded Added 8 specs to the paired repo ( One caveat I want to be explicit about: I could not run the Karma/Jasmine browser leg locally — no headless browser in my environment — so I pushed rather than claim it green. CI is running it now on
|
|
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
|
Fixed in a24a523, along the lines you suggested: The branch is now up to date with |
…and the migration guide Drop two search tags that repeat the page's own heading, and shorten the comment on the parenthesis unwrap to the behaviour it implements. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rFormula needs `=SUM(IF(TRUE,"",1))` returns #NAME? in HyperFormula, because TRUE is a function there, not a literal; the table's values hold for `TRUE()`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1768 +/- ##
========================================
Coverage 97.28% 97.28%
========================================
Files 207 207
Lines 16218 16218
Branches 3556 3587 +31
========================================
Hits 15777 15777
Misses 433 433
Partials 8 8
🚀 New features to boost your workflow:
|
Context
HyperFormula read an empty string (
"") as0wherever it expected a number:=""+0returned0and=COUNT("")returned1. In Excel""is text, so the first formula returns#VALUE!and the second returns0.ArithmeticHelper.coerceNonDateScalarToMaybeNumberreturned0for''as a special case. This PR removes the special case, so''goes through the normal text-to-number parsing, which finds no number. A blank cell is a different value (EmptyValue) and still reads as0, soevaluateNullToZerois unaffected.Aggregations ignore text found through a reference or a range, so
=SUM(A1), withA1holding"", stays0. That check recognized only a bare reference, so with the coercion fix alone,=SUM((A1))reached the coercion and became#VALUE!.NumericAggregationPluginnow looks through parentheses before it decides whether an argument is a reference. A parenthesized reference therefore behaves like the bare one whatever the cell holds, not only for"". WithA1holdingabc,=SUM((A1))changes from#VALUE!to0. WithA1empty,=COUNT((A1))changes from1to0.This is a breaking change for 4.0. The new migration guide,
docs/guide/migration-from-3.x-to-4.0.md, lists what changes and what does not.How did you test your changes?
fix/hf-361-empty-string-coercioninhyperformula-tests(hyperformula-tests#53). It changes 12 spec files: the newunit/interpreter/empty-string-coercion.spec.ts, the specs of the functions whose expectations changed, and parenthesized-reference tests in theSUM,COUNTandAVERAGEspecs.ArithmeticHelper.tsandNumericAggregationPlugin.tsreverted todevelop, 29 of them fail.develop.Compatibility with Excel and Google Sheets
Excel values were measured in Excel Online. Google Sheets was not checked.
A1holds an empty string;C1:C3holds1,"",2.=""+0#VALUE!#VALUE!(was0)=A1+1#VALUE!#VALUE!(was1)=ROUND(A1,0),=ABS(A1)#VALUE!#VALUE!(was0)=LN(A1)#VALUE!#VALUE!(was#NUM!)=COUNT("")00(was1)=SUM(""),=AVERAGE(""),=MIN(""),=MAX(""),=PRODUCT("")#VALUE!#VALUE!(was0)=SUM(C1:C3,"")#VALUE!#VALUE!(was3)=SUM(IF(TRUE(),"",1))#VALUE!#VALUE!(was0)=COUNT(IF(TRUE(),"",1))00(was1)=SUM(LEFT("abc",0))#VALUE!#VALUE!(was0)=SUM(A1),=COUNT(A1)00(unchanged)=AVERAGE(A1)#DIV/0!#DIV/0!(unchanged)=SUM(C1:C3)33(unchanged)=SUM({1,"",2})33(unchanged)One case moves away from Excel. Excel ignores text in an argument it evaluates as an array, and HyperFormula agreed only because it read
""as0:=SUM(IF(G1:G3>0,"",G1:G3)),G1:G3=1, 2, 30#VALUE!(was0)=COUNT(IF({1,0},"",5))10(was1)HyperFormula's
IFdoes not return an array here:=ROWS(IF(G1:G3>0,"",G1:G3))is1.=SUM(IF(G1:G3>5,"",G1:G3))is6in Excel and1in HyperFormula, before and after this PR. The migration guide documents this case.The parenthesized-reference results (
=SUM((A1))withA1holdingabc,=COUNT((A1))withA1empty) have not been measured in Excel yet.Public API changes
No method or option changes. Formula results change as listed above; the changelog marks the entry as a breaking change.
Out of scope and follow-ups
=PRODUCT(A1)withA1empty or holding""returns1; Excel returns0. Not changed here.=COUNTIF(C1:C3,"")counts only blank cells, so it returns0for the range above; Excel returns1. Not changed here.''into a cell create a blank cell instead of a text cell? That would also changeISTEXT,COUNTAandLENon such cells. This PR leaves cell parsing alone.docs/guide/migration-from-3.x-to-4.0.mdfile, so whichever PR merges second resolves a conflict there.Types of changes
Related issues
Checklist
dev-docs/pages this change affects – or none needed it.🤖 Generated with Claude Code
Note
High Risk
Breaking formula semantics across arithmetic, aggregations, and parenthesized references; existing sheets and integrations that rely on
""as zero or write''for blank cells may get new errors or different results.Overview
Breaking change for 4.0: HyperFormula no longer treats an empty string as
0in arithmetic and other numeric contexts, aligning with Excel (=""+0and cell""in=A1+1now yield#VALUE!instead of numeric results). Blank cells (EmptyValue/null) still coerce to zero; aggregations over bare references or ranges (=SUM(A1),=SUM(A1:C1)) are unchanged.ArithmeticHelper.coerceNonDateScalarToMaybeNumberdrops the'' → 0shortcut so literals and evaluated""flow through normal text parsing.NumericAggregationPlugin.reduceunwraps parenthesized AST nodes so(A1)is handled likeA1—text/empty values in referenced cells are ignored inSUM/COUNT/etc., fixing cases where=SUM((A1))incorrectly coerced or counted.Docs add a 3.x → 4.0 migration guide (tables for what changes vs. what does not, app guidance to use
nullinstead of''when clearing cells) and a changelog entry under Changed.Reviewed by Cursor Bugbot for commit cc66c2a. Bugbot is set up for automated code reviews on this repo. Configure here.