HF-131 (2/7): the origin/propagation mechanism, plus the same guarantee outside the interpreter by marcin-kordas-hoc · Pull Request #1762 · handsontable/hyperformula

marcin-kordas-hoc · 2026-09-10T13:45:08Z

What and why

Stacked on #1761.

Revision note: an earlier version of this description covered only the second half below.
The first half — the origin/propagation mechanism — was always in this PR's actual diff; the
description just didn't say so. #1764's description has been corrected to match (it does not
build the mechanism, only exposes two of its fields publicly).

1. The origin/propagation mechanism. CellError needs to know, for any error value anywhere
in a sheet, which function or operator originally produced it, whether it's since been merely
propagated by a cell that read it, and (for static errors with no formula root) which address it
originated at. The hard problem isn't computing that — it's stamping it exactly once, at the
moment of truth, regardless of which of the ~200 call sites across the interpreter and dependency
graph produced or merely relayed the error.

Two choke points, first-wins immutability (the same shape attachRootVertex already used for
root):

Cell.ts gains withOrigin/withArgumentIndex/asPropagated/withOriginAddress — each a
no-op once the relevant field (or propagated) is already set, otherwise a fresh immutable copy.
Interpreter.ts's evaluateAst stamps originFunction in a single postprocessing tail
(stampOriginForAstNode) that runs after every AST node evaluates — including the NaN-overflow
guard's freshly-minted error, and including the fix that a propagated error no longer gets
attachRootVertex'd to whichever cell happened to read it.
DependencyGraph.ts's getCellValue — the one place every cross-cell read funnels through —
marks every error read from another cell as propagated, and additionally stamps the address
for a vertex holding a static error (no FormulaVertex to serve as a root).
Exporter.ts falls back to the stamped originAddress when there is no root.

Measured: A1='=1+' reports Sheet1!A1 where it reported no address; B1='=A1' reports
Sheet1!A1 where it reported its own address; the same holds two hops out, through a range, and
through a named expression; it follows the cell across addRows; =1/0 with a propagating SUM
still reports A1.

Also consolidates the toEqualError matcher: it existed twice, once per test runner, with
hand-copied logic and two strip lists that could drift — and had drifted in a way that made a
change appear inert under Jest. The decision now lives in one shared module both wrappers call
with their own equality function.

Not yet public: originFunction/argumentIndex are carried on CellError and stamped here,
but DetailedCellError does not expose them yet — that's #1764.

2. The same guarantee outside the interpreter. Extends #1761's guarantee to the 17
message-less sites (+2 with an explicit undefined) outside src/interpreter/: the parser's
#REF!/#NAME? branches, dependency transformers' 4 reference-removal sites, DependencyGraph's
array-source-removed site, FormulaVertex's outside-array-result site, and Evaluator's two
#CYCLE! sites.

Kuba's scope ruling was all places throwing a cell error, not only src/interpreter/ — these 19
sites are outside the spec's original 78-site counter but within that ruling.

Widens the ESLint rule from #1761 to all of src/, and documents the one call site it structurally
cannot catch: CellContentParser.ts's CellContent.Error constructor keeps message optional on
purpose (its one no-message caller — a user typing e.g. #REF! literally into a cell — has no
engine-side cause to state), and new CellError(errorType, message) is syntactically a two-argument
call that passes the rule regardless of what the caller actually passes.

One new ErrorMessage constant, for an error literal written into a formula, is deliberately honest
rather than invented: an error literal typed into a formula or a value typed directly into a cell
round-trips exactly what the user wrote, so the message says that rather than fabricating an
engine-side cause.

Verified

npx tsc --noEmit — clean
npx eslint src/ — 0 errors (same pre-existing warning count)
The widened rule was proven to reach files outside src/interpreter/: a #CYCLE! site in
src/Evaluator.ts was temporarily reverted to a bare new CellError(ErrorType.CYCLE), confirmed
to produce the expected lint error, then restored (git diff clean afterward)
Full private test suite reproduced against this branch's tip — no regressions beyond the same
pre-existing branch-pinning mismatch as HF-131 (1/7): every interpreter cell error carries a message #1761
Nested-attribution correctness re-confirmed by re-running the test suite live: =SUM(SQRT(-1))
reports SQRT not SUM; =1/0 then =SUM(A1) reports divide, propagated: true; no code
path exists to overwrite a propagated error's address; zero surviving message-less CellError
construction sites in src/ (338 call sites individually audited)

Stack

2 of 7 — stacked on #1761. Next: feat/hf-131-has-message.

🤖 Generated with Claude Code

Note

Medium Risk
Touches core evaluation, dependency reads, and export paths for every cell error; behavior changes for error origin addresses (especially static errors after sheet structure changes) are user-visible.

Overview
Extends HF-131 with an origin/propagation model for CellError and applies the mandatory cause message rule outside the interpreter.

Origin tracking: CellError gains immutable helpers (withOrigin, asPropagated, withOriginAddress, etc.) so errors record which function/operator produced them, whether they were only read from another cell, and—for static errors without a formula root—a snapshot address plus transformation version. The interpreter stamps originFunction in one evaluateAst post-pass and no longer attaches the reading cell as root for propagated errors. DependencyGraph.getCellValue marks cross-cell reads as propagated and stamps static-error origins; Exporter resolves reported addresses via root or by replaying structural transforms through LazilyTransformingAstService.applyTransformationsToAddress (omitting the address after compaction when history is gone).

Messages everywhere: Parser, dependency transformers, evaluator cycle handling, array cleanup, and related paths now pass ErrorMessage constants instead of bare CellError types. New constants cover unresolved/removed references, circular refs, array edge cases, and formula error literals. ESLint HF-131 applies to all src/**/*.ts, with a documented exception for user-typed error literals in CellContentParser.

Tests: Jest and Jasmine toEqualError matchers share one comparison module so structural ignore lists cannot drift between runners.

^{Reviewed by Cursor Bugbot for commit d8cd6cf. Bugbot is set up for automated code reviews on this repo. Configure here.}

Performance

Measured with the repo's own benchmark harness (npm run benchmark:write-to-file), origin/develop
c920375 vs this stack's tip, three alternating base/head rounds on a 4-core machine. Total median
−0.24% (per-round totals +3.1 / −1.6 / −4.4%, i.e. the sign flips and no direction is
measurable; base-vs-base spread per benchmark 1.7–18.7%). One benchmark is slower in all three
rounds: "Column ranges - add column", +2.8 / +3.3 / +4.2%, median +3.3% against a 1.9% base
spread. The candidates are the extra instanceof CellError test at the evaluateAst tail and the
error-only getCell lookup in DependencyGraph.getCellValue; the two were not separated.

Follow-up in this PR: a static error's address is now kept current

DependencyGraph.getCellValue records the address on the error value itself when a dependent reads
a cell that merely holds a static error. The reading cell caches that value and is not recomputed by
a row or column change that only shifts it, so the recorded address went stale and the dependent
kept naming the pre-shift cell — which could by then be empty or hold an unrelated value. The record
now carries the LazilyTransformingAstService version it was taken at, and Exporter replays the
intervening transformations over it, the same way an error produced by a formula already resolves
its address through its root vertex. Once a compaction has discarded those transformations the
address cannot be reconstructed and is omitted rather than guessed. Seven regression tests cover the
dependents, which the previous CRUD test did not: direct, two hops, through a range, on another
sheet, after an insert and after a removal, plus the compaction case.

Reading another cell's value funnels through DependencyGraph.getCellValue, so that is where an error now gets marked as propagated -- one choke point instead of a marking at each interpreter case, which is what let three read paths (named expressions, range reads via SimpleRangeValue, and getScalarValue's aggregation callers) slip through an earlier attempt. getScalarValue delegates to getCellValue rather than reading the address mapping itself, so its callers inherit the mark instead of bypassing it. wrapperForRootVertex then skips a propagated error, so the reading cell is no longer attached as its root. A vertex holding a STATIC error has no FormulaVertex to serve as a lazily-resolved root, so getCellValue stamps the address it already has; CellError is immutable, so this copies rather than mutating what is stored, which is what keeps the address correct across row and column changes. evaluateAst's overflow guard now assigns instead of returning early, so a NaN/Infinity result reaches the same postprocessing as every other value and gets a root like anything else -- previously =SQRT(-1) had neither. Measured on this branch: A1='=1+' reports Sheet1!A1 where it reported no address; B1='=A1' reports Sheet1!A1 where it reported its own address; the same holds two hops out, through a range, and through a named expression; it follows the cell across addRows; and =1/0 with a propagating SUM still reports A1. Full private suite: 5 failed / 6157 passed, byte-identical to this worktree's baseline (those 5 are a pre-existing branch-pinning mismatch in the test repo). tsc and eslint clean. Also consolidates the toEqualError matcher. It existed twice, once per runner, with hand-copied logic and two strip lists that could drift -- and had drifted in a way that made a change appear inert under Jest. The decision now lives in one shared module both wrappers call with their own equality function, so there is a single strip list to extend. Verified by short-circuiting the shared module: 30 tests went red and came back green, proving Jest really reads it. The Karma half could not be executed here (no browser in this environment). NOT yet observable through the public API: originFunction and argumentIndex are carried on CellError and stamped in evaluateAst, but DetailedCellError does not expose them yet. Exposing them is one line plus a test migration -- 10 assertions in error-address-preservation.spec.ts, arrays.spec.ts and matrix-plugin.spec.ts use plain toEqual, which no matcher edit reaches, and they must NOT be moved to toEqualError because it also strips address, which is exactly what those tests assert. That split is the natural PR boundary here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Task 2 of the HF-131 explainability plan: the 19 CellError construction sites outside src/interpreter/ (17) plus src/Evaluator.ts's two explicit-undefined CYCLE sites that passed no message at all now do, mirroring what Task 1 already did for src/interpreter/. Seven new catalogue constants cover semantically distinct groups rather than one generic REF/NAME sentence: a removed-reference in an existing formula (Transformer.ts, 4 sites), an array's source cell being overwritten (DependencyGraph.ts), a position outside an array's own computed bounds (FormulaVertex.ts), an unresolved parser reference (FormulaParser.ts, 6 REF sites), a reference exceeding sheet size limits (FormulaParser.ts, 4 NAME sites), an error value typed directly into a formula (FormulaParser.ts, 1 site), and a circular reference (Evaluator.ts, 2 CYCLE sites). No error type changed, no function signature changed, i18n/Cell.ts/ CellValue.ts/Exporter.ts/the DependencyGraph choke point untouched. Originally attempted via prep-ship's autonomous pipeline; the run crashed twice with an empty error at its own 'authoring AC specs' step (unrelated to this diff — see the private-test patch README for the diagnosis). The production src/ implementation it left behind was verified correct against every constraint in the task file and kept; the pre-existing private-test breakage it hadn't yet reached was fixed by hand. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Enforces the guarantee this PR's message-filling work establishes: a message-less new CellError(...) construction under src/interpreter/ is now a lint error. Two selectors — a bare one-argument call, and an explicit literal undefined as the second argument (the exact shape the two CYCLE sites in src/Evaluator.ts still use; Evaluator.ts is outside src/interpreter/ so this rule doesn't reach them yet, and is unaffected by this commit). Verified: 0 lint errors on the full src/ tree (same pre-existing warning count as before this commit — no new warnings). The rule was proven to actually fire, not just parse, by temporarily reverting one src/interpreter/ plugin site to a bare 'new CellError(ErrorType.NUM)', observing the expected lint error, then restoring it (git diff empty afterward). Widened to all of src/ in the next PR in this stack (feat/hf-131-error-messages-outside-interpreter). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Changes the override's files glob from src/interpreter/ to all of src/, so the guard this PR's own message-filling work (and the earlier commit's interpreter-only rule) establishes covers the whole engine, not just the interpreter plugins. Documents the one call site the rule structurally cannot catch: CellContentParser.ts's CellContent.Error constructor keeps message optional on purpose (its one no-message caller — a user typing e.g. #REF! literally into a cell — has no engine-side cause to state), and the call 'new CellError(errorType, message)' is syntactically a two-argument construction that passes the rule cleanly regardless of what the caller actually passes. Verified: 0 lint errors on the full src/ tree (same pre-existing warning count as before this commit). The widened rule was proven to actually reach files outside src/interpreter/ by temporarily reverting one CYCLE site in src/Evaluator.ts to a bare 'new CellError(ErrorType.CYCLE)', observing the expected lint error, then restoring it (git diff empty afterward). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

qunabu · 2026-09-10T13:45:14Z

Task linked: HF-131 Verbose and actionable formula error messages

cursor

Stale Bugbot comment from a previous run.

cloudflare-workers-and-pages · 2026-09-10T13:55:09Z

Deploying with 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	`d8cd6cf`	Commit Preview URL Branch Preview URL	Sep 14 2026, 10:36 AM

…at/hf-131-error-messages-outside-interpreter # Conflicts: # .eslintrc.js

cursor

Stale Bugbot comment from a previous run.

github-actions · 2026-09-11T03:43:22Z

Performance comparison of head (`d8cd6cf`) vs base (`c5b33cb`)

                                     testName |    base |    head | change
--------------------------------------------------------------------------
                                      Sheet A |  490.86 |  512.21 | +4.35%
                                      Sheet B |  155.89 |  161.13 | +3.36%
                                      Sheet T |  140.87 |  143.19 | +1.65%
                                Column ranges |  468.69 |  475.33 | +1.42%
                                Sorted lookup | 14124.6 | 14225.2 | +0.71%
Sheet A:  change value, add/remove row/column |   14.64 |   14.63 | -0.07%
 Sheet B: change value, add/remove row/column |  128.95 |  138.84 | +7.67%
                   Column ranges - add column |  147.05 |  150.22 | +2.16%
                Column ranges - without batch |  454.98 |  460.58 | +1.23%
                        Column ranges - batch |  111.83 |  114.18 | +2.10%

…es-outside-interpreter

cursor

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

There are 4 total unresolved issues (including 3 from previous reviews).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 749044d. Configure here.}

A cell that holds a static error - a formula the parser rejected, or an error value entered directly - has no formula vertex to resolve an address through, so DependencyGraph.getCellValue records the address on the error value itself when a dependent reads it. The reading cell caches that value and is not recomputed by a row or column change that merely shifts it, so the recorded address went stale: the dependent kept naming the pre-shift cell, which by then could be empty or hold an unrelated value. The record now carries the LazilyTransformingAstService version it was taken at, and Exporter replays the intervening transformations over it, the same way an error produced by a formula already resolves its address through its root vertex. The replay reuses each transformation's own transformSingleAst with an empty node rather than fixNodeAddress, because MoveCellsTransformer only moves an address that lies inside the range being moved and that decision lives in transformSingleAst. Once a compaction has discarded the transformations the record predates, the address cannot be reconstructed; it is omitted rather than guessed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ages-outside-interpreter # Conflicts: # CHANGELOG.md

The list's own note says to add every new CellError field here once. This commit adds originAddressVersion, which the address fix introduced, and the two fields that were already missing: propagated and originAddress. All three are recorded by the engine as it reads a value, so an expectation built by hand cannot know them — the same reason root and address are already ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

codecov · 2026-09-14T10:39:44Z

Codecov Report

❌ Patch coverage is 98.34711% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.39%. Comparing base (c5b33cb) to head (d8cd6cf).

Files with missing lines	Patch %	Lines
src/parser/FormulaParser.ts	81.81%	2 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                       @@
##           feat/hf-131-error-messages    #1762      +/-   ##
==============================================================
+ Coverage                       97.36%   97.39%   +0.02%     
==============================================================
  Files                             195      195              
  Lines                           15750    15843      +93     
  Branches                         3461     3494      +33     
==============================================================
+ Hits                            15335    15430      +95     
+ Misses                            407      405       -2     
  Partials                            8        8

Files with missing lines	Coverage Δ
src/Cell.ts	`95.31% <100.00%> (+0.91%)`	⬆️
src/CellContentParser.ts	`100.00% <ø> (ø)`
src/DependencyGraph/DependencyGraph.ts	`98.82% <100.00%> (+<0.01%)`	⬆️
src/DependencyGraph/FormulaVertex.ts	`84.25% <100.00%> (ø)`
src/Evaluator.ts	`100.00% <100.00%> (ø)`
src/Exporter.ts	`87.80% <100.00%> (+1.13%)`	⬆️
src/LazilyTransformingAstService.ts	`98.27% <100.00%> (+0.35%)`	⬆️
src/dependencyTransformers/Transformer.ts	`98.70% <100.00%> (+1.33%)`	⬆️
src/error-message.ts	`100.00% <100.00%> (ø)`
src/interpreter/Interpreter.ts	`95.91% <100.00%> (+0.55%)`	⬆️
... and 1 more

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

marcin-kordas-hoc and others added 4 commits September 8, 2026 02:25

This was referenced Sep 10, 2026

HF-131 (3/7): a consumer can tell 'no message' from 'empty message' #1763

Open

HF-131 (7/7): documentation #1767

Open

cursor Bot reviewed Sep 10, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Comment thread .eslintrc.js

marcin-kordas-hoc requested a review from Tobiadefami September 11, 2026 03:08

Merge remote-tracking branch 'pr1/feat-hf-131-error-messages' into fe…

f667a23

…at/hf-131-error-messages-outside-interpreter # Conflicts: # .eslintrc.js

cursor Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread src/Cell.ts Outdated

Comment thread test/_setupFiles/matchers/cellErrorComparison.ts

Merge remote-tracking branch 'pr1/tip2' into feat/hf-131-error-messag…

749044d

…es-outside-interpreter

cursor Bot reviewed Sep 11, 2026

View reviewed changes

Comment thread src/DependencyGraph/DependencyGraph.ts

marcin-kordas-hoc changed the title ~~HF-131 (2/7): the same guarantee outside the interpreter~~ HF-131 (2/7): the origin/propagation mechanism, plus the same guarantee outside the interpreter Sep 11, 2026

marcin-kordas-hoc mentioned this pull request Sep 11, 2026

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

Open

marcin-kordas-hoc and others added 2 commits September 14, 2026 09:10

Merge branch 'feat/hf-131-error-messages' into feat/hf-131-error-mess…

82c71d6

…ages-outside-interpreter # Conflicts: # CHANGELOG.md

marcin-kordas-hoc mentioned this pull request Sep 14, 2026

HF-131 (5/7): no error reports a cell that did not produce it #1765

Open

Navigation Menu

HF-131 (2/7): the origin/propagation mechanism, plus the same guarantee outside the interpreter - #1762

HF-131 (2/7): the origin/propagation mechanism, plus the same guarantee outside the interpreter#1762
marcin-kordas-hoc wants to merge 9 commits into
feat/hf-131-error-messagesfrom
feat/hf-131-error-messages-outside-interpreter

marcin-kordas-hoc commented Sep 10, 2026 •

edited by cursor Bot

Loading

Uh oh!

qunabu commented Sep 10, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

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

edited

Loading

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

marcin-kordas-hoc commented Sep 10, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What and why

Verified

Stack

Performance

Follow-up in this PR: a static error's address is now kept current

Uh oh!

qunabu commented Sep 10, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cloudflare-workers-and-pages Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance comparison of head (d8cd6cf) vs base (c5b33cb)

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Sep 14, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marcin-kordas-hoc commented Sep 10, 2026 •

edited by cursor Bot

Loading

cursor Bot left a comment •

edited

Loading

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

edited

Loading

cursor Bot left a comment •

edited

Loading

github-actions Bot commented Sep 11, 2026 •

edited

Loading

Performance comparison of head (`d8cd6cf`) vs base (`c5b33cb`)