fix: predict the array size of a named expression from what it stands for (HF-166, partial) - #1772
Draft
marcin-kordas-hoc wants to merge 2 commits into
Draft
marcin-kordas-hoc wants to merge 2 commits into
marcin-kordas-hoc wants to merge 2 commits into
Conversation
…s for A named expression had no case in the array-size predictor, so it fell to the default and was predicted as a scalar error. The referring cell was therefore never turned into an array vertex, and an array-shaped result reached the exporter, which rejects it as a #VALUE! error. A named range passed to an array-returning function failed this way while the same call on a range literal worked. The prediction now recurses into the expression the name is bound to, isRef included, so a name bound to a range predicts like the range literal it stands for and a name bound to an array-returning formula predicts like that formula. Two details worth stating. Names may refer to one another, so a set of names currently being predicted guards against a reference cycle recursing until the stack overflows; a name already on it is treated as unpredictable, which leaves the referring cell a scalar formula and lets the evaluator report the cycle. And the prediction deliberately uses the engine's own array-arithmetic setting rather than the calling state's, because the named expression has a cell of its own that is always computed with the engine setting, no matter where the name is used. Adds DependencyGraph#getFormulaAst, which the predictor needs to reach the expression behind a name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
hyperformula-docs | 134d2b7 | Commit Preview URL Branch Preview URL |
Sep 12 2026, 02:47 PM |
The named-expressions guide stated that "a bare =myRange + 1 does not spill" in array arithmetic mode. That sentence described the defect, so it is replaced by what the engine now does, and the function-argument bullet gains the array-returning case that was the reported symptom. The neighbouring claim about SUM(myRange + 1) returning 20 in array mode was re-measured and still holds, as does every default-mode result on the page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Performance comparison of head (134d2b7) vs base (c920375) |
marcin-kordas-hoc
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1772 +/- ##
========================================
Coverage 97.32% 97.32%
========================================
Files 195 195
Lines 15739 15758 +19
Branches 3390 3465 +75
========================================
+ Hits 15318 15337 +19
+ Misses 421 413 -8
- Partials 0 8 +8
🚀 New features to boost your workflow:
|
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.
Context
A named range passed to an array-returning function evaluated to
#VALUE!("Cell range not allowed.") while the same call on the range literal the name stands for worked:=TRANSPOSE(MyRange)failed where=TRANSPOSE(Sheet1!A1:C1)spilled normally. Aggregations over the same name —SUM,COUNT,INDEX— were unaffected throughout.The cause is in
ArraySizePredictor.checkArraySizeForAsthad noNAMED_EXPRESSIONcase, so a named expression fell through to thedefaultbranch and was predicted asArraySize.error(), a 1×1 scalar. The referring cell was therefore never turned into an array vertex, the array-shaped result reachedExporter.exportValue, and that rejects aSimpleRangeValuein a scalar cell with#VALUE!. The prediction now recurses into the expression the name is bound to,isRefincluded, so a name behaves like what it stands for.DependencyGraph#getFormulaAstis added for the predictor to reach the expression behind a name, andArraySizePredictortakes the dependency graph as a constructor argument.Scope of the behaviour change
Two things change, both in the direction of making a name and the range it stands for agree.
useArrayArithmetic: true), a named range used as an operand now spills rather than returning#VALUE!.=myRange+1,=myRange>2,=myRange&"x"and their sheet-scoped and horizontal equivalents now produce exactly what=A1:A5+1and friends produce in the same engine. This was characterized in the test suite as an accepted limitation of the array-size predictor, with a note that no engine follow-up ticket existed yet; this is that follow-up, and the five cases that pinned the old behaviour are updated to assert the two now agree.Not changed: a bare
=MyRangestill returns#VALUE!, exactly as a bare=A1:A5does. Excel spills both. Making bare names spill while bare range literals do not would trade one inconsistency for another, so that difference is left alone and is worth deciding separately.Two details in the implementation worth calling out. Names may refer to one another, so a set of names currently being predicted guards against a reference cycle recursing until the stack overflows; a name already on it is treated as unpredictable, which leaves the referring cell a scalar formula and lets the evaluator report the cycle as it always did. And the prediction deliberately uses the engine's own
useArrayArithmeticsetting rather than the calling state's, because a named expression has a cell of its own that is always computed with the engine setting, no matter where the name is used.Known limitation — this does not yet cover the reported case
A formula that is already present when the engine is built is still not fixed. The array
size of a formula is predicted while the sheet is parsed, and named expressions passed to
buildFromArray/buildFromSheetsare registered after that(
src/BuildEngineFactory.ts:graphBuilder.buildGraph(...)runs beforeinputNamedExpressions.forEach(...)). At prediction time the name does not resolve, so thecell is sized as a scalar and never becomes an array vertex, whatever this change does later.
Measured, same engine, same definitions:
=TRANSPOSE(myName)buildFromArray, names passed alongside#VALUE!— unchangedsetCellContentsafter the engine is builtaddNamedExpressionafter the build, then the formulaSo this change helps a sheet that is edited, and not a sheet that is loaded in one go — which
is the more common path and the one in the original report. Reordering the two steps in
BuildEngineFactoryis not the fix: tried and measured, it turns every name-referencingformula into
#REF!, because the sheets are still placeholders at that point. Closing the gapneeds a re-sizing pass over name-referencing formulas after the names are registered, which is
a separate, designed change rather than an improvisation on top of this one. The same gap
affects
addNamedExpressioncalled after a formula that already refers to the name.How did you test your changes?
test/fetch-tests.shpicks it up.unit/named-expression-array-size.spec.ts, 8 cases. Negative control: 3 of the 8 fail without the engine change; the other 5 are regression guards that pass either way.unit/named-expressions.spec.tsupdated to the new behaviour. Each asserted value was measured against the patched engine, and each compares the named result against the direct range in the same engine rather than only pinning a literal.npm run lint: 0 errors.Documentation
docs/guide/named-expressions.mdstated that "a bare=myRange + 1does not spill" in array arithmetic mode. That sentence described the defect, so it now says what the engine does; the function-argument bullet gains the array-returning case. The neighbouring claim that=SUM(myRange + 1)returns20in array mode was re-measured and still holds, as does every default-mode result on the page.Status
Opened as a draft: the implementation is complete and green, but release scheduling for this
fix is not decided, so this is not yet a request to merge and no reviewer is requested.
Related issues
Types of changes
Checklist
🤖 Generated with Claude Code