Repository navigation
HF-131 (4/7): expose which function produced an error, and which argument - #1764
marcin-kordas-hoc wants to merge 17 commits into
Conversation
DetailedCellError.originFunction and .argumentIndex ride along from CellError, which already carried them internally since the origin/ propagation redesign -- this task is the public-exposure half. Two gaps found and closed while verifying, not assumed fixed by that earlier redesign: - FunctionPlugin.coerceArgumentsToRequiredTypes's two error-return sites never called withArgumentIndex, so a genuine per-argument coercion failure (e.g. `=DATE(2020,"x",1)`) reported argumentIndex: undefined instead of 1. Both sites now call it. - withArgumentIndex's gate checked only propagated/argumentIndex-already-set, missing the same originFunction check withOrigin already has. A nested call's own error (SQRT(-1) inside `=DATE(1,1,SQRT(-1))`) stamps its origin before DATE's own coercion loop ever sees it; without the extra guard that loop would still attach its own argument index to SQRT's error -- incoherent, since SQRT never had that argument. Verified via a same-formula, no-cell, no-propagation repro (the cross-cell version does not reproduce the bug, since the propagated gate alone already covers it). Every operator/function origin name asserted in the new specs was verified by direct probe against this worktree before being committed, including correcting one substituted-in test formula that turned out to exercise a different operator (multiply, not addWithEpsilon) than intended. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Task linked: HF-131 Verbose and actionable formula error messages |
![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 1bb9db5. Configure here.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
hyperformula-docs | e745be2 | Commit Preview URL Branch Preview URL |
Oct 23 2026, 07:17 AM |
Performance comparison of head (e745be2) vs base (9d436c4) |
marcin-kordas-hoc
changed the title
The doc predated the identities the later commits added, so it said a parsing error or a value typed into a cell reports undefined, when they report 'parser' and 'user input'. It now lists the vocabulary, and names the one case where an argument index can appear without an origin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tobiadefami
left a comment
Tobiadefami
left a comment
There was a problem hiding this comment.
Could we preserve the original argument index through range expansion? With A1:A3 containing 1, =AND(A1:A3,"x") reports argumentIndex: 3, but "x" is the second argument, so the index should be 1. The .withArgumentIndex(i) calls in FunctionPlugin.ts:488–492 use the flattened position.
Carrying the original index alongside each expanded value fixed this for AND, OR, and XOR in a temporary patch. All 164 tests in the selected suites passed, along with TypeScript and lint.
…nsion @Tobiadefami on PR #1764: with A1:A3 holding 1, =AND(A1:A3,"x") reported argumentIndex 3, but "x" is the second argument, so it should be 1. Reproduced first, before touching anything, and measured at 3. Cause: for a function with `expandRanges`, listOfScalarValues flattens every range into its cells before coercion runs, and coerceArgumentsToRequiredTypes then called withArgumentIndex with its position in that flattened list. The reported argument therefore depended on how many cells a preceding range happened to cover -- the same formula over A1:A2 and A1:A5 blamed arguments 2 and 5 for the same "x". Fix: listOfScalarValues now carries the originating argument index alongside each scalar, every value expanded out of one range keeping that range's own index, and runFunction threads the list down to the two withArgumentIndex call sites. The fourth tuple element is additive: a custom plugin that overrides listOfScalarValues with the old three-element shape yields undefined and falls back to the loop position, i.e. today's behaviour. Measured after the fix, on the same probes: =AND(A1:A3,"x") 1 (was 3) =AND(A1:A2,B1:B3,"x") 2 (was 5) =AND("x",A1:A2) 0 (unchanged -- already correct) =AND(TRUE(),"x") 1 (unchanged -- no range involved) Full suite 507 suites / 6297 tests green; eslint 0 errors and one warning fewer than before. Paired tests: hyperformula-tests 8604992. The changelog bullet is extended rather than given a Fixed entry, because argumentIndex has never shipped -- the guarantee it now states is that the index counts arguments as written, so it does not shift with the size of a range. The public JSDoc says the same, and also records the measured case where there is deliberately no index at all: a bad value INSIDE a range that the function ignores (=AND(A1:A2,TRUE()) over text returns TRUE). Worth naming, because it is the third time on this task: every worked example in the design, the ADR and this file's existing tests used scalar arguments. Nobody passed a range. Same shape as "first occurrence wins" (every example was a function inside a function) and originAddress (no CRUD probe was ever run against the claim). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the independent re-derivation. The fix also changed where a coercion failure INSIDE a range is attributed, and neither the JSDoc nor the changelog said so. The JSDoc's only inside-a-range sentence described the case where a function IGNORES the value (AND over text returns TRUE, so there is no error). For a function that coerces every value the error is real, and it is now attributed to the range's own argument slot rather than to a position derived from cell count -- which is the same contract, stated for the case the sentence did not cover. Also says plainly what the field is not: it names the argument to look at, not the offending cell, whose address this field never carried. Paired test: hyperformula-tests 275e955. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… stack
Two sentences described behaviour that arrives in 5/7 (fix/hf-131-rootless-origin),
not here. Measured at this branch's tip:
=DATE(2020,A99999999999,1) -> {originFunction:'DATE', argumentIndex:1}
while the JSDoc said an unresolvable reference "reports itself"
=DATE(2020,A1:A2,1) over text, array arithmetic
-> every element {argumentIndex:1, originFunction:undefined}
while the JSDoc said the element "carries the index alongside the
function's own name"
Both identical at 624a8e4, so neither is caused by this change -- but 624a8e4
is the commit that deleted the known-limitation wording for the second one, with
the message "now that it is fixed", and it was not fixed at that commit. So the
false sentences are this PR's, even though the behaviour is not.
Rewritten to state what the branch does and to name both cases as addressed later
in the change set. That way every sentence shipped is true at every tip of the
stack rather than only at the last one, which is what a per-PR reviewer needs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
marcin-kordas-hoc
left a comment
marcin-kordas-hoc
left a comment
There was a problem hiding this comment.
Fixed in 223711dd — thanks, the diagnosis was exact.
sourceArgumentIndexes now travels with the expanded values, and coerceArgumentsToRequiredTypes reports writtenArgumentIndex(i) instead of the loop position. On the current head =AND(A1:A3,"x") with A1:A3 filled reports argumentIndex: 1; same for OR and XOR. Pinned in the tests repo by "points at the argument the user wrote, not at its position after range expansion".
Could you re-review when you get a chance? The CHANGES_REQUESTED is the only thing holding this one.
|
Follow-up since A re-review found the first round of tests could not tell this fix from an implementation that merely clamps the flattened position to the argument count — it passed 16/16. Two things worth knowing: the fix also moved where a bad cell inside a range is attributed ( |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The whole stack is now merged with |
…hat the code does at this point The originFunction JSDoc listed origin names that the engine does not produce yet, and the argumentIndex JSDoc referred to a later change instead of stating the current behaviour. 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 @@
## feat/hf-131-has-message #1764 +/- ##
===========================================================
+ Coverage 97.34% 97.36% +0.01%
===========================================================
Files 207 207
Lines 16323 16327 +4
Branches 3621 3624 +3
===========================================================
+ Hits 15890 15896 +6
+ Misses 425 423 -2
Partials 8 8
🚀 New features to boost your workflow:
|
Context
The PR exposes on
DetailedCellErrorwhich function or operator produced a cell error, and which argument it rejected. It is part 4 of 7 of HF-131, stacked on #1763.originFunctionandargumentIndexon the internalCellError, but a consumer could not read them.hyperformula/src/CellValue.ts:DetailedCellError.originFunctionandDetailedCellError.argumentIndex, copied from theCellError, with JSDoc.hyperformula/src/interpreter/plugin/FunctionPlugin.ts: a failed argument coercion records the index of the argument as written. A range keeps its own index for every value expanded out of it.hyperformula/src/Cell.ts:withArgumentIndexdoes nothing onceoriginFunctionis set, so a nested call's error does not get the outer call's index.originFunctionargumentIndex=SUM(SQRT(-1))SQRT=1/0divide=DATE(2000, "x", 1)DATE1=AND(A1:A3, "x"),A1:A3hold1AND1=DATE(1, 1, SQRT(-1))SQRTAn operator is named after the arithmetic helper it dispatches to, for example
divideoraddWithEpsilon.How did you test your changes?
feat/hf-131-origin-function, withunit/error-argument-index.spec.tsandunit/error-origin-function.spec.ts.Compatibility with Excel and Google Sheets
Not applicable: no formula result changes.
Public API changes
DetailedCellError.originFunction?: stringandDetailedCellError.argumentIndex?: number(new, read-only).FunctionPlugin.listOfScalarValuesandFunctionPlugin.evaluateArguments(protected, used by custom function plugins) return tuples with a fourth element, the written argument index. A plugin that overrides either method with the three-element tuple still runs, but no longer type-checks.Out of scope and follow-ups
argumentIndex, because only a function's argument coercion sets it.Types of changes
Related issues
Checklist
dev-docs/pages this change affects – or none needed it.