fix(metadata): mark the SHEET/SHEETS argument as optional - #1717
Draft
sequba wants to merge 1 commit into
Draft
Conversation
`SHEET` and `SHEETS` declared their single argument as required in `implementedFunctions`, although both accept a zero-argument call: `runFunctionWithReferenceArgument` serves `args.length === 0` itself, before `isNumberOfArgumentValuesValid` is consulted. The change is metadata-only — `optionalArg` has no other consumer in `src/`, and evaluation is unaffected (covered by the existing function-sheet/function-sheets specs). It is split out of #1692, whose function-metadata API is the first consumer that can observe it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ Deploy Preview for hyperformula-dev-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Performance comparison of head (8eded2f) vs base (ad142ba) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1717 +/- ##
========================================
Coverage 97.22% 97.22%
========================================
Files 178 178
Lines 15612 15612
Branches 3430 3430
========================================
Hits 15178 15178
Misses 426 426
Partials 8 8
🚀 New features to boost your workflow:
|
sequba
added a commit
that referenced
this pull request
Jul 30, 2026
…F-249)
Addresses the review of this branch. Every factual claim below was verified
against a live engine built from src/, not against Excel's documentation.
Correctness
-----------
- GAUSS stated the inverse of what it computes ("falling more than this many
times standard deviation from mean"). GAUSS(2) is 0.47725, i.e. the
probability of falling BETWEEN the mean and z; "more than" is 0.0228.
- SHEET/SHEETS declared their single argument as required while accepting a
zero-argument call, so getFunctionDetails contradicted the entry's own
"An optional sheet name..." and its first example, =SHEET(). Restores
optionalArg: true. This is metadata-only: optionalArg has exactly two
readers, and isNumberOfArgumentValuesValid is unreachable from
runFunctionWithReferenceArgument, which serves args.length === 0 itself.
The docs page prints SHEET([value]) again, as it did before this branch.
Supersedes #1717, which can be closed.
- A function id colliding with an Object.prototype member (`toString`,
`valueOf`, `__proto__`) reported as a built-in id, because the catalogue
doubles as the built-in id set and is indexed with a caller-supplied id.
A custom function registered under such a name leaked into the static list
and came back with localizedName as a function rather than a string, which
then vanished through JSON.stringify - breaking this API's own round-trip
invariant. FUNCTION_DOCS is now prototype-less, which fixes every lookup
site at once, plus a typeof guard in resolveName for the translation
package, which is indexed the same way.
Prose describes HyperFormula, not Excel
---------------------------------------
The catalogue was seeded from a page documenting Excel. INT and BASE were
corrected earlier as one-offs; these are the rest of that population:
- CEILING/FLOOR named #NUM! for significance 0; it is #DIV/0!. #NUM! occurs
only for a positive number with a negative significance, and number 0
yields 0 whatever the sign.
- CEILING.MATH/FLOOR.MATH said "when non-zero"; only mode 1 switches the
direction - mode 2 behaves like the default 0.
- nine engineering `places` descriptions were BASE's semantics: a minimum
width. They are an exact width, #NUM! when the result needs more digits,
and ignored for negative numbers.
- DECIMAL said "positive integer"; DECIMAL("0", 2) is 0.
- MOD said "one integer"; MOD(10.5, 3) is 1.5.
- MROUND omitted that base 0 returns 0.
- GCD, LCM, MULTINOMIAL, COMBIN, COMBINA and ROMAN truncate to an integer,
documented for FACT/FACTDOUBLE but not for these.
- ISEVEN/ISODD short descriptions implied a binary split; both return FALSE
for a fractional value.
- the seven *A variants claimed text counts as 0. AVERAGEA(1, "3") is 2:
numeric text coerces, non-numeric text passed directly gives #VALUE!, and
only text inside a range counts as 0.
Six of these are genuine deviations from Excel and Google Sheets and are now
rows in the list of differences, with a note on the shared root causes.
Naming and categories, before they become API surface
-----------------------------------------------------
getFunctionDetails makes ~900 parameter names public, so these are cheap now
and breaking later. Parameter counts are unchanged throughout.
- one `cumulative` flag across all 14 distributions, replacing a 6/8 split
between `boolean` (which named the type, not the role) and `mode` (which
was also carrying four unrelated meanings). TDIST's third argument is a
tail count, so it becomes `tails`.
- positional number1..number5 replaced by the statistical roles they play in
11 functions (BETA.DIST, BINOM.DIST, HYPGEOM.DIST, WEIBULL.DIST, ...).
- Excel's criteria_range1/criteria1 everywhere; COUNTIFS used `range1`,
which meant "criteria range" there and "the x values" in SUMX2MY2.
- FIND takes SEARCH's parameter names; they are the same operation.
- DAYS(end_date, start_date) and DAYS360(start_date, end_date, format) no
longer share date1/date2 for opposite roles.
- SUMX2MY2/SUMX2PY2/SUMXMY2 get array_x/array_y and three descriptions that
can actually be told apart.
- BESSELI/J/K/Y move to Engineering, N to Information, COUNTUNIQUE to
Statistical, matching where Excel and Google Sheets classify them.
- FUNCTION_CATEGORIES is alphabetical, so the generated page's sections are.
- no two of the 423 ids now share a shortDescription (was 14 groups over 32
functions, including the standard-normal pair reusing the general normal
pair's wording verbatim).
Structure
---------
- FunctionRegistry.isListableFunctionId is now the single listability gate,
applied by both getListableFunctionIds and getFunctionDetails. The design
rests on the two tiers agreeing; that was upheld by two copies of the rule
in two files.
- getAvailableFunctions' JSDoc called custom functions "instance-scoped".
registerFunctionPlugin registers globally; the static methods omit them by
policy. Reworded to say that.
- DEV_DOCS promised drift detection on "optionality that does not match
optionalArg/defaultValue". No such check exists - ParameterDoc has no
optionality field - and its absence is why the SHEET/SHEETS contradiction
shipped. Documents the real gap and the rule for zero-argument calls.
- script/*.ts is now linted (it was covered by the blanket `script` ignore,
so the branch's three new files were neither linted nor type-checked by
any npm script). They pass clean. The legacy CommonJS helpers stay ignored.
Verified: 502/502 suites, 6172 passed, 0 failed (was 2 failed of 6164);
tsc --noEmit clean; lint 0 errors; docs page still 423 rows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
SHEETandSHEETSdeclared their single argument as required inimplementedFunctions, although both accept a zero-argument call:runFunctionWithReferenceArgumentservesargs.length === 0itself, beforeisNumberOfArgumentValuesValidis consulted.The change is metadata-only —
optionalArghas no other consumer insrc/, and evaluation is unaffected (covered by the existing function-sheet/function-sheets specs). It is split out of #1692, whose function-metadata API is the first consumer that can observe it.Context
How did you test your changes?
Types of changes
Related issues:
Checklist: