Open
Conversation
…arDependencies, not hard-coded to the 99th iteration (which might not be hit)
… formula values as 0
…initialComputedValues Exporter.exportValue() now returns 0 instead of null for EmptyValue when evaluateNullToZero is true, making getAllSheetsValues() consistent with formula evaluation behavior. Also allows null cell values in the initialComputedValues validation so empty cells can be passed as null rather than empty strings.
![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 and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Publish | ||
| run: npm publish No newline at end of file |
There was a problem hiding this comment.
NODE_AUTH_TOKEN set on wrong workflow step
High Severity
The NODE_AUTH_TOKEN env var is set on the "Setup .npmrc" step but not on the "Publish" step. The setup-node action creates an .npmrc containing ${NODE_AUTH_TOKEN} as a runtime reference that npm resolves when executing. Since the env var is scoped to the setup step, npm publish won't have it in its environment, causing a 401 authentication failure. The env block with NODE_AUTH_TOKEN needs to be on the npm publish step instead.
Additional Locations (1)
| - name: Setup .npmrc | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ inputs.node_version }} |
There was a problem hiding this comment.
Workflow references undefined node_version input parameter
Medium Severity
The node-version parameter references ${{ inputs.node_version }} but no node_version input is defined in the workflow. The only defined inputs are tag and bun_js_version. This resolves to an empty string, causing setup-node to use an arbitrary default Node.js version instead of a deliberately chosen one.
There was a problem hiding this comment.
Pull request overview
This PR broadens a targeted MATCH/indexing fix into a larger fork-focused change set, adding circular-reference support (with iterative recalculation + optional initial seed values), adjusting EmptyValue export semantics, and updating packaging/CI to publish a scoped fork.
Changes:
- Adjust MATCH exact-match behavior to retry lookup with a coerced key type when the first lookup fails.
- Add
allowCircularReferences+initialComputedValuesconfig and implement iterative recalculation for cyclic SCCs. - Update export/evaluation behavior around
EmptyValue(optionally treating it as 0) and introduce fork-oriented packaging + publishing workflow changes.
Reviewed changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Enables JSON module imports (used by new tests/fixtures). |
| test/circular-dependencies.spec.ts | Adds test coverage for circular reference behavior and config validation. |
| test/CellValueExporter.spec.ts | Adds tests for evaluateNullToZero behavior in exported values. |
| src/Sheet.ts | Introduces InitialComputedValues type used by new config. |
| src/Lookup/AdvancedFind.ts | Changes exact-match search to include type coercion on lookup miss. |
| src/Exporter.ts | Exports EmptyValue as 0 when evaluateNullToZero is enabled. |
| src/Evaluator.ts | Implements iterative evaluation path for cyclic SCCs and initial seeding for cycles. |
| src/DependencyGraph/FormulaCellVertex.ts | Changes behavior for reading uncomputed formula values. |
| src/ConfigParams.ts | Adds config surface for circular references and initial computed values. |
| src/Config.ts | Wires new config params into defaults and validation. |
| src/BuildEngineFactory.ts | Adapts initialComputedValues mapping when building from a single unnamed sheet. |
| src/ArgumentSanitization.ts | Adds validation for initialComputedValues. |
| package.json | Renames package to scoped fork and updates build/publish tooling commands. |
| README.md | Replaces upstream README with fork-specific summary. |
| .github/workflows/test.yml | Removes upstream unit test workflow. |
| .github/workflows/publish.yml | Removes upstream docs publish workflow. |
| .github/workflows/publish.yaml | Adds fork package publishing workflow (Bun + GitHub Packages). |
| .github/workflows/performance.yml | Removes upstream performance comparison workflow. |
| .github/workflows/lint.yml | Removes upstream lint workflow. |
| .github/workflows/codeql.yml | Removes upstream CodeQL workflow. |
| .github/workflows/build.yml | Removes upstream build matrix workflow. |
| .github/workflows/build-docs.yml | Removes upstream docs build workflow. |
| .github/workflows/audit.yml | Removes upstream npm audit workflow. |
| .github/pull_request_template.md | Removes upstream PR template. |
| .github/codecov.yml | Removes upstream Codecov configuration. |
| .github/ISSUE_TEMPLATE/improve_docs.yaml | Removes upstream issue template. |
| .github/ISSUE_TEMPLATE/config.yml | Removes upstream issue template config. |
| .github/ISSUE_TEMPLATE/bug_report.yaml | Removes upstream bug report template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return index | ||
| } | ||
| const coercedKey = typeof searchKey === 'number' ? searchKey.toString() : typeof searchKey === 'string' ? Number(searchKey) : searchKey | ||
| console.log(`[MATCH COERCE] key: ${searchKey} (${typeof searchKey}) -> coercedKey: ${coercedKey} (${typeof coercedKey})`) |
Comment on lines
+152
to
+165
| if (isInitial){ | ||
| cycled.forEach((vertex: Vertex) => { | ||
| if (vertex instanceof FormulaVertex && !vertex.isComputed()) { | ||
| const address = vertex.getAddress(this.lazilyTransformingAstService) | ||
|
|
||
| const sheetName = this.dependencyGraph.sheetMapping.fetchDisplayName(address.sheet) | ||
| const sheetData = this.config.initialComputedValues?.[sheetName] || [] | ||
| const cellValue = (sheetData[address.row] || [])[address.col] | ||
|
|
||
| vertex.setCellValue(cellValue !== undefined ? cellValue : 0) | ||
| } | ||
| }) | ||
|
|
||
| return ContentChanges.empty() |
Comment on lines
+183
to
+192
| const address = vertex.getAddress(this.lazilyTransformingAstService) | ||
| const newCellValue = this.recomputeFormulaVertexValue(vertex) | ||
|
|
||
| if (i < cycles - 1) { | ||
| return | ||
| } | ||
|
|
||
| this.columnSearch.add(getRawValue(newCellValue), address) | ||
| changes.addChange(newCellValue, address) | ||
| }) |
Comment on lines
+227
to
+229
| const newCellValue = this.recomputeFormulaVertexValue(vertex) | ||
| const address = vertex.getAddress(this.lazilyTransformingAstService) | ||
| this.columnSearch.add(getRawValue(newCellValue), address) |
Comment on lines
255
to
261
| public getCellValue(): InterpreterValue { | ||
| if (this.cachedCellValue !== undefined) { | ||
| return this.cachedCellValue | ||
| } else { | ||
| throw Error('Value of the formula cell is not computed.') | ||
| } | ||
| } | ||
|
|
||
| return 0 | ||
| } |
Comment on lines
+146
to
+162
| it('should handle breaking cycles by setting constants', () => { | ||
| const engine = HyperFormula.buildFromArray([ | ||
| ['=B1+1', '=A1+1'] | ||
| ], { | ||
| allowCircularReferences: true, | ||
| initialComputedValues: {'Sheet1': [[51, 50]]} | ||
| }) | ||
|
|
||
| expect(engine.getCellValue(adr('A1'))).toBe(51) | ||
| expect(engine.getCellValue(adr('B1'))).toBe(50) | ||
|
|
||
| engine.setCellContents(adr('B1'), [['75']]) | ||
|
|
||
| expect(engine.getCellValue(adr('A1'))).toBe(76) | ||
| expect(engine.getCellValue(adr('B1'))).toBe(75) | ||
| }) | ||
|
|
Comment on lines
+258
to
+275
| sheetsWithCycles.forEach(sheet => { | ||
| for (const rangeVertex of this.dependencyGraph.rangeMapping.rangesInSheet(sheet)) { | ||
| const range = rangeVertex.range | ||
| let containsCyclicCell = false | ||
|
|
||
| for (const address of range.addresses(this.dependencyGraph)) { | ||
| const addressKey = `${address.sheet}:${address.col}:${address.row}` | ||
| if (cyclicAddresses.has(addressKey)) { | ||
| containsCyclicCell = true | ||
| break | ||
| } | ||
| } | ||
|
|
||
| if (containsCyclicCell) { | ||
| rangeVertex.clearCache() | ||
| } | ||
| } | ||
| }) |
Comment on lines
+45
to
+46
| allowCircularReferences: true | ||
| , initialComputedValues: {'Sheet1': [[300, 299, 298]]} |
Comment on lines
+4
to
+55
| workflow_call: | ||
| inputs: | ||
| tag: | ||
| description: 'Tag from which the package should be published' | ||
| default: '' | ||
| required: false | ||
| type: string | ||
| bun_js_version: | ||
| required: false | ||
| default: "1.1.26" | ||
| type: string | ||
| push: | ||
| branches: | ||
| - master | ||
|
|
||
| permissions: | ||
| packages: write | ||
| id-token: write | ||
| contents: read | ||
|
|
||
| jobs: | ||
| publish: | ||
| name: Publish artifacts | ||
| runs-on: ubuntu-22.04 | ||
| timeout-minutes: 5 | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ inputs.tag }} | ||
|
|
||
| - name: Setup Bun.js | ||
| uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: ${{ inputs.bun_js_version }} | ||
|
|
||
| - name: Install Dependencies | ||
| run: bun install --frozen-lockfile --ignore-scripts | ||
|
|
||
| - name: Build | ||
| run: bun run build | ||
|
|
||
| - name: Setup .npmrc | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: ${{ inputs.node_version }} | ||
| registry-url: "https://npm.pkg.github.com/" | ||
| scope: "@stellar-fusion" | ||
| always-auth: true | ||
| env: | ||
| NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
Comment on lines
+2
to
16
| "name": "@stellar-fusion/hyperformula", | ||
| "version": "0.0.6", | ||
| "author": "Stellar Fusion Tech <tech@stellarfusion.io>", | ||
| "contributors": [ | ||
| "Evan Hynes <evan.hynes@stellarfusion.io>" | ||
| ], | ||
| "browserslist": [ | ||
| "last 2 chrome versions", | ||
| "last 2 and_chr versions", | ||
| "last 2 firefox versions", | ||
| "last 2 and_ff versions", | ||
| "last 2 safari versions", | ||
| "last 2 ios_saf versions", | ||
| "last 2 edge versions", | ||
| "last 2 and_uc versions", | ||
| "last 2 and_qq versions", | ||
| "last 2 op_mob versions" | ||
| "publishConfig": { | ||
| "@stellar-fusion:registry": "https://npm.pkg.github.com/stellar-fusion" | ||
| }, | ||
| "files": [ | ||
| "commonjs", | ||
| "dist", | ||
| "es", | ||
| "typings" | ||
| ], |
Contributor
|
Hi @gracetravers, can you describe what change this PR introduces? |
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
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
How did you test your changes?
Types of changes
Related issues:
Checklist:
Note
Medium Risk
Removes most CI (tests/lint/security scanning) and issue/PR templates, which increases the risk of regressions and missed vulnerabilities despite no runtime code changes.
Overview
Repository automation is significantly pared down. This PR deletes the existing issue templates, PR template, Codecov config, and the full set of GitHub Actions workflows for build, lint, tests, docs, audit, CodeQL, and performance checks.
Publishing is reworked. It replaces the previous docs deployment workflow with a new
publish.yamlthat builds with Bun and publishes the package to GitHub Packages (also callable viaworkflow_callwith a tag input).Written by Cursor Bugbot for commit 3c28bd2. This will update automatically on new commits. Configure here.