Skip to content

HF-131 (4/7): expose which function produced an error, and which argument - #1764

Open
marcin-kordas-hoc wants to merge 10 commits into
feat/hf-131-has-messagefrom
feat/hf-131-origin-function
Open

marcin-kordas-hoc wants to merge 10 commits into
feat/hf-131-has-messagefrom
feat/hf-131-origin-function

Conversation

@marcin-kordas-hoc

@marcin-kordas-hoc marcin-kordas-hoc commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

What and why

Stacked on #1763. Exposes on the public DetailedCellError type the two fields the
origin/propagation mechanism (introduced in #1762) already computes internally:
originFunction?: string and argumentIndex?: number — which function or operator produced a
cell error, and, when it failed coercing one of its own arguments, which argument that was.

Revision note: an earlier version of this description said this PR built the mechanism
itself (the two choke points, Cell.ts's builders, etc.). That was wrong — the mechanism landed
in #1762 (see its revised description). This PR only does the public-facing exposure step the
mechanism commit's own message called out as a separate, later concern:

"originFunction and argumentIndex are carried on CellError and stamped in evaluateAst, but
DetailedCellError does not expose them yet... That split is the natural PR boundary here."

This PR is that boundary: expose the two fields, and wire the one direct caller outside the two
choke points.

What actually changes here (4 files, +32/-3):

  • src/CellValue.tsDetailedCellError gains originFunction/argumentIndex, copied from the
    underlying CellError in the constructor. Additive only: enumerable property count on
    DetailedCellError grows from 5 (type/message/value/address/hasMessage from HF-131 (3/7): a consumer can tell 'no message' from 'empty message' #1763) to 7.
  • src/interpreter/plugin/FunctionPlugin.tscoerceArgumentsToRequiredTypes's two error-return
    paths (wrong-type, non-scalar) now call .withArgumentIndex(i) — the only direct callers of
    that builder outside the two choke points HF-131 (2/7): the origin/propagation mechanism, plus the same guarantee outside the interpreter #1762 introduced.
  • src/Cell.tswithArgumentIndex's no-op gate also checks originFunction !== undefined, not
    just argumentIndex: needed so a nested call's own error (SQRT(-1) inside
    =DATE(1,1,SQRT(-1))) doesn't get DATE's argument index attached after already claiming its
    own origin.
  • CHANGELOG.md — documents the two new public fields.

Verified against nested/propagated cases directly: =SUM(SQRT(-1)) reports SQRT, not SUM;
=1/0 then =SUM(A1) reports divide, propagated: true, not re-claimed by SUM;
=DATE(1,1,SQRT(-1)) reports {originFunction: 'SQRT', argumentIndex: undefined}, not a leaked
index on DATE's argument slot.

Known, deliberate limit: a binary operator's error names the operator (addWithEpsilon,
divide, etc.), not which operand failed — argumentIndex is only set for a function's own
coercion loop; the two operands of a binary op have no equivalent loop. Not solved here.

Verified

Stack

4 of 7 — stacked on #1763. Next: fix/hf-131-rootless-origin.

🤖 Generated with Claude Code


Note

Low Risk
Additive public fields on error export objects and small stamping logic; no changes to successful evaluation paths.

Overview
Exposes error provenance on the public API. DetailedCellError now includes originFunction and argumentIndex, copied from the internal CellError that #1762 already populated — so consumers can see which function/operator produced an error and, when applicable, which argument failed coercion.

Wires the last coercion stamping site. FunctionPlugin.coerceArgumentsToRequiredTypes attaches .withArgumentIndex(i) on wrong-type and non-scalar coercion failures.

Tightens withArgumentIndex. It becomes a no-op once originFunction is set (not only when argumentIndex or propagated is set), so nested errors like SQRT(-1) inside DATE(1,1,...) keep SQRT as origin without DATE's argument index incorrectly attached.

CHANGELOG documents the new fields alongside the existing hasMessage work.

Reviewed by Cursor Bugbot for commit 624a8e4. Bugbot is set up for automated code reviews on this repo. Configure here.

Performance

The hot-path change lives in #1762 and is measured there: total median −0.24% over three alternating
base/head rounds against origin/develop, within the run-to-run spread, with one CRUD benchmark
("Column ranges - add column") consistently about 3% slower. This PR adds fields to an object that
is only built when an error is exported, so it carries no measurable cost of its own.

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>
@qunabu

qunabu commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread CHANGELOG.md Outdated
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 10, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
hyperformula-docs 624a8e4 Commit Preview URL

Branch Preview URL
Sep 15 2026, 07:29 AM

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Performance comparison of head (624a8e4) vs base (915ca17)

                                     testName |    base |    head | change
--------------------------------------------------------------------------
                                      Sheet A |  446.12 |  445.24 | -0.20%
                                      Sheet B |   141.9 |  138.82 | -2.17%
                                      Sheet T |  125.58 |   122.8 | -2.21%
                                Column ranges |  584.23 |  581.39 | -0.49%
                                Sorted lookup | 17372.7 | 16735.2 | -3.67%
Sheet A:  change value, add/remove row/column |   12.95 |   12.86 | -0.69%
 Sheet B: change value, add/remove row/column |  121.91 |  120.68 | -1.01%
                   Column ranges - add column |  163.01 |  162.98 | -0.02%
                Column ranges - without batch |  510.21 |  499.41 | -2.12%
                        Column ranges - batch |  129.91 |   123.6 | -4.86%

@marcin-kordas-hoc marcin-kordas-hoc changed the title HF-131 (4/7): which function produced an error, and which argument HF-131 (4/7): expose which function produced an error, and which argument Sep 11, 2026
marcin-kordas-hoc and others added 7 commits September 14, 2026 09:48
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>
GitHub #131 is an unrelated, closed issue; this work is tracked by #1547.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.40%. Comparing base (915ca17) to head (624a8e4).

Additional details and impacted files

Impacted file tree graph

@@                     Coverage Diff                     @@
##           feat/hf-131-has-message    #1764      +/-   ##
===========================================================
+ Coverage                    97.39%   97.40%   +0.01%     
===========================================================
  Files                          195      195              
  Lines                        15844    15846       +2     
  Branches                      3494     3494              
===========================================================
+ Hits                         15431    15435       +4     
+ Misses                         405      403       -2     
  Partials                         8        8              
Files with missing lines Coverage Δ
src/Cell.ts 95.31% <100.00%> (ø)
src/CellValue.ts 83.33% <100.00%> (+3.33%) ⬆️
src/interpreter/plugin/FunctionPlugin.ts 99.06% <100.00%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants