fix: an empty string is no longer coerced to 0 in numeric contexts (HF-361) - #1768
Open
marcin-kordas-hoc wants to merge 3 commits into
Open
fix: an empty string is no longer coerced to 0 in numeric contexts (HF-361)#1768marcin-kordas-hoc wants to merge 3 commits into
marcin-kordas-hoc wants to merge 3 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>
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
hyperformula-docs | c99427b | Commit Preview URL Branch Preview URL |
Sep 10 2026, 03:18 PM |
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 (c99427b) vs base (c920375) |
…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>
marcin-kordas-hoc
marked this pull request as ready for review
September 10, 2026 15:37
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1768 +/- ##
===========================================
- Coverage 97.32% 97.32% -0.01%
===========================================
Files 195 195
Lines 15739 15737 -2
Branches 3390 3460 +70
===========================================
- Hits 15318 15316 -2
+ Misses 421 413 -8
- Partials 0 8 +8
🚀 New features to boost your workflow:
|
![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.
| - v4.0 | ||
| - empty string | ||
| - coercion | ||
| --- |
There was a problem hiding this comment.
Redundant migration guide search tags
Low Severity
Tags empty string and coercion repeat the h2 Changes to empty string coercion, which search already indexes from headings. The multi-word empty string tag also matches empty and string separately, so generic queries can surface this migration page.
Triggered by learned rule: VuePress tags frontmatter invariants
Reviewed by Cursor Bugbot for commit c99427b. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
An empty string (
'') was unconditionally coerced to0in numeric contexts. This aligns HyperFormula with Excel, where""is text and is never a number.ArithmeticHelper.coerceNonDateScalarToMaybeNumbercarried anarg === '' → return 0early exit. Removing it lets''fall through to normal string→number parsing, which correctly finds no number. Blank cells are a different internal value (EmptyValue, handled one branch above) and are untouched —evaluateNullToZerokeeps working exactly as before.Reported by a customer via support:
COUNT("")returned1where Excel returns0, and=""+0returned0where Excel returns#VALUE!. The reporter had already verified in Excel Online that both cases error there, and had ruled out every config option as a workaround (there is none — the behaviour was hardcoded).What changes, and what does not
The distinction that matters for anyone reading this to estimate impact: aggregation over a cell reference or a range does not change.
SUM,AVERAGE,MIN,MAXandCOUNThave always ignored text found in a reference, and they still do, because the aggregation path filters strings out (strictlyNumbers) before this coercion is reached. Every value below is measured on both engines; the Excel column is measured live through the Microsoft Graph API against Excel Online.Changes — arithmetic and number-typed arguments, in both the literal and the reference form (
A1holds an empty string):=""+0/=A1+1#VALUE!0/1#VALUE!=A1-1,=1-A1,=A1*2,=A1/1,=A1^2,=-A1,=A1%#VALUE!#VALUE!=ROUND(A1, 0),=ABS(A1),=INT("")#VALUE!0#VALUE!=ACOT("")/=ACOT(A1)#VALUE!1.5707963268#VALUE!=DATE(A1, 2, 3)#VALUE!35#VALUE!=LN(""),=LOG10(""),=DATE("","","")#VALUE!#NUM!#VALUE!=COT(""),=COTH("")#VALUE!#DIV/0!#VALUE!Changes — an empty string written directly into an aggregation:
=COUNT("")010=SUM(""),=AVERAGE(""),=MIN(""),=MAX(""),=PRODUCT("")#VALUE!0#VALUE!=SUM(1, "")#VALUE!1#VALUE!=SUMSQ(""),=MEDIAN(""),=STDEV("")#VALUE!0/#DIV/0!#VALUE!Changes for the better — an empty string produced mid-formula. This is the most common real shape of the bug, and the strongest argument for the change: before, all of these were wrong; now all match Excel.
=SUM(IF(TRUE,"",1))#VALUE!0#VALUE!=COUNT(IF(TRUE,"",1))010=SUM(LEFT("abc",0)),=LEFT("abc",0)+1#VALUE!0/1#VALUE!=ROUND(IF(TRUE,"",1),0),=SUM(A1&""),=SUM(CONCATENATE("",""))#VALUE!#VALUE!Does not change — measured identical before and after, and matching Excel:
=SUM(A1),=MIN(A1),=MAX(A1),=COUNT(A1)00=AVERAGE(A1)#DIV/0!#DIV/0!=COUNTA(A1)11=SUM(A1:C1)over1,"",233=COUNT(A1:C1)over the same22=SUM({1,"",2}),=COUNT({1,"",2})3,23,2evaluateNullToZerosetting=COUNTIF(rng,""),=A1&"x",=A1="",=N(A1),=VALUE(A1)One class that moves away from Excel
Where Excel evaluates an argument as an array containing only text, it ignores the text; before, HyperFormula happened to agree by coercing
''to0:=SUM(IF(A1:A3>0,"",A1:A3))00#VALUE!=SUM({""})00#VALUE!=COUNT(IF({1,0},"",5))110That agreement was coincidence, not support: HyperFormula's
IFis not array-aware at all —=ROWS(IF(A1:A3>0,"",A1:A3))is1, so it collapses the argument to a single value regardless. The idiom is already broken independently of this change:=SUM(IF(A1:A3>5,"",A1:A3))over1, 2, 3is6in Excel and1here, before and after. What changes is only that the collapsed value stops being silently read as0. Worth its own ticket; not a reason to hold this one.Integration risk, and a design question worth answering first
setCellContents(address, '')produces a text cell holding''(STRING,ISBLANKfalse), not a blank one — onlynull/undefinedproduce a blank. Verified against the Handsontable Formulas plugin on the published package:core.js'semptySelectedCellspushesnull.SheetClip.parse('a\t\tc')yields["a","","c"],copyPaste.jsfeeds that topopulateFromArray, andformulas.js'ssyncChangeWithEngineforwards tosetCellContentswith only date-specific normalisation. Nothing maps''back tonull.Scope of that exposure, though, is narrower than it first looks: for a pasted
''cell,SUMandCOUNTare unaffected — only arithmetic and number-typed arguments break.The design question: should a written
''parse asEmptyValuerather than as text? That would fix the reported bug and remove the paste exposure at the same time, at the cost of a different divergence (ISTEXT,COUNTAandLENon such a cell). This PR takes the narrower route — fix the coercion, leave cell parsing alone — but the alternative deserves an explicit yes or no before a breaking change ships.Verification
coercions,function-ln,function-log10,function-cot,function-coth,function-acotandfunction-date(function-logchanges an input, not an expectation).browser-tests— the private suite runs under both Jest and Karma/Jasmine.Definition of Done
''is deliberately not special-casedChangedas a breaking changedocs/guide/migration-from-3.x-to-4.0.md)hyperformula-testsNote: this targets 4.0 and adds
docs/guide/migration-from-3.x-to-4.0.md, the same new file #1754 creates. Whichever lands first establishes it; the second resolves a mechanical conflict.🤖 Generated with Claude Code
Note
High Risk
Breaking formula semantics for any sheet or integration that relies on
""as zero in arithmetic or numeric function arguments; widespread recalculation impact though the code change is small.Overview
This is a breaking 4.0 change that stops treating an empty string (
"") as0in numeric contexts, matching Excel.The engine change removes the special-case in
ArithmeticHelper.coerceNonDateScalarToMaybeNumberthat mapped''to zero. Arithmetic and functions that expect numbers (e.g.=""+0,=ROUND(A1,0)whenA1is"") now yield#VALUE!instead of silent numeric results. Literals in aggregations change too (=COUNT("")→0,=SUM(1,"")→#VALUE!). Blank cells (EmptyValue) andevaluateNullToZeroare unchanged; SUM/COUNT over ranges that skip text still behave as before.Docs add a Changed changelog entry, a 3.x → 4.0 migration guide (including integration notes: prefer
nullover''when clearing cells), and a sidebar link in the docs config.Reviewed by Cursor Bugbot for commit c99427b. Bugbot is set up for automated code reviews on this repo. Configure here.