Skip to content

fix: bound ParserWithCaching cache growth (HF-105) - #1760

Open
Tobiadefami wants to merge 3 commits into
developfrom
fix/hf-105-parser-cache
Open

fix: bound ParserWithCaching cache growth (HF-105)#1760
Tobiadefami wants to merge 3 commits into
developfrom
fix/hf-105-parser-cache

Conversation

@Tobiadefami

@Tobiadefami Tobiadefami commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Context

Addresses HF-105: unbounded growth of the ParserWithCaching cache.

Repeatedly replacing a cell with distinct formulas retained every parsed formula for the lifetime of the engine, even with undo disabled. This change bounds the parser cache with least-recently-used eviction and adds maxParserCacheSize (default 10,000; 0 disables caching).

Undo and clipboard snapshots now retain parsed formula (AST) references directly. Restoration rebuilds parsing metadata when an entry has been evicted, preserving undo/redo, paste, and structural operations. A Map plus a doubly linked list keeps promotion and eviction efficient. The limit controls cache entries, not formulas still needed by the workbook, undo history, or clipboard.

Updates include API documentation, the performance guide, developer notes on formula ownership, and changelog entries linked to this PR.

How did you test your changes?

Validation on Node 22:

  • Reproduced the bug on the original code: 10,001 replacements of one cell retained 10,002 cache entries with undo disabled.
  • Added 34 regression cases covering capacity limits, LRU ordering, disabled caching, metadata recovery, undo/redo, clipboard, structural changes, arrays, named expressions, and configuration validation.
  • npm run test:jest -- --maxWorkers=2: 505 suites and 6,267 tests passed; 3 existing skips.
  • npm run test:browser: Chrome and Firefox each passed 6,267 tests, with 3 existing skips.
  • npm run compile, npm run verify:typings, and npm run lint passed; lint reported warnings but no errors.
  • npm run test:performance passed all five build and five CRUD benchmarks.
  • An 80,000-edit probe kept the fixed cache at 10,000 entries with approximately flat retained heap; the original cache grew to 80,000 entries. A five-trial comparison showed no meaningful slowdown for repeated formulas.

After removing the redundant existing-entry branch in Cache.set(), all 36 tests in four focused cache suites passed, along with production type checking, lint for Cache.ts, and git diff --check. The focused coverage run covered every executable line in the Cache class.

Types of changes

  • Breaking change (a fix or a feature because of which an existing functionality doesn't work as expected anymore)
  • New feature or improvement (a non-breaking change that adds functionality)
  • Bug fix (a non-breaking change that fixes an issue)
  • Additional language file, or a change to an existing language file (translations)
  • Change to the documentation

Related issues:

Fixes #1635.

Checklist:

  • I have reviewed the guidelines about Contributing to HyperFormula and I confirm that my code follows the code style of this project.
  • I have signed the Contributor License Agreement.
  • My change is compliant with the OpenDocument standard.
  • My change is compatible with Microsoft Excel.
  • My change is compatible with Google Sheets.
  • I described my changes in the CHANGELOG.md file.
  • My changes require a documentation update.
  • My changes require a migration guide.

Note

Medium Risk
Touches parser caching, undo/redo snapshots, and clipboard restore; ClipboardCellFormula now exposes ast instead of hash, which may affect TypeScript integrators.

Overview
Adds maxParserCacheSize (default 10,000, 0 disables caching) so the parser AST cache is LRU-bounded instead of growing without limit in long-running workloads.

Because cache entries can be evicted, undo snapshots, clipboard formula cells, and formula restore paths now retain Ast references (not parser hashes). Restoration goes through fetchCachedResultForAst / setFormulaToCellFromAst, which rebuild parsing metadata when the cache no longer has the entry. Docs and changelog describe the new option and parser-cache ownership.

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

@cla-external-contractor-signup

Copy link
Copy Markdown

@Tobiadefami thanks for the pull request. No CLA step needed here — our records show you signed the Contributor License Agreement on 2026-07-31. That signature came from our previous signing form and has been carried over, so there is nothing for you to re-sign.

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

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 28cf3ce. Configure here.

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

cloudflare-workers-and-pages Bot commented Sep 9, 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 5aaeae0 Commit Preview URL

Branch Preview URL
Sep 10 2026, 09:40 AM

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Performance comparison of head (5aaeae0) vs base (c920375)

                                     testName |    base |    head |  change
---------------------------------------------------------------------------
                                      Sheet A |  367.77 |  374.86 |  +1.93%
                                      Sheet B |  113.93 |  112.66 |  -1.11%
                                      Sheet T |   99.51 |  100.35 |  +0.84%
                                Column ranges |   492.9 |  487.57 |  -1.08%
                                Sorted lookup | 15819.5 | 14896.4 |  -5.84%
Sheet A:  change value, add/remove row/column |    9.91 |      10 |  +0.91%
 Sheet B: change value, add/remove row/column |   96.87 |   87.14 | -10.04%
                   Column ranges - add column |  131.04 |  132.44 |  +1.07%
                Column ranges - without batch |  406.16 |  391.76 |  -3.55%
                        Column ranges - batch |  100.81 |   94.02 |  -6.74%

@Tobiadefami Tobiadefami changed the title fix: bound parser cache growth while preserving formula restoration fix: bound ParserWithCaching cache growth (HF-105) Sep 10, 2026
@qunabu

qunabu commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.33%. Comparing base (c920375) to head (5aaeae0).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1760      +/-   ##
===========================================
+ Coverage    97.32%   97.33%   +0.01%     
===========================================
  Files          195      195              
  Lines        15739    15762      +23     
  Branches      3390     3471      +81     
===========================================
+ Hits         15318    15342      +24     
+ Misses         421      412       -9     
- Partials         0        8       +8     
Files with missing lines Coverage Δ
src/ClipboardOperations.ts 96.55% <ø> (ø)
src/Config.ts 94.23% <100.00%> (+0.11%) ⬆️
src/LazilyTransformingAstService.ts 97.87% <100.00%> (-0.05%) ⬇️
src/Operations.ts 98.93% <100.00%> (-0.01%) ⬇️
src/UndoRedo.ts 100.00% <100.00%> (ø)
src/parser/Cache.ts 100.00% <100.00%> (ø)
src/parser/ParserWithCaching.ts 95.41% <100.00%> (+0.34%) ⬆️

... and 4 files 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.

3 participants