Skip to content

fix(HF-268): support consecutive percent operators - #1773

Open
Tobiadefami wants to merge 4 commits into
developfrom
fix/HF-268
Open

Tobiadefami wants to merge 4 commits into
developfrom
fix/HF-268

Conversation

@Tobiadefami

@Tobiadefami Tobiadefami commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Formulas such as =5%% failed to parse even though =(5%)% worked. Consume consecutive postfix percent operators and wrap the preceding expression for each token, so =5%% returns 0.0005 and =5%%% returns 0.000005, matching Excel.

Preserve each operator's whitespace and the existing operator precedence. Document the behavior in the operator guide and Unreleased changelog.

Validated with the matching fix/HF-268 test branch: 502 Jest suites and 6,254 tests passed, and Chrome and Firefox each passed 6,254 tests. Compilation and performance benchmarks passed. Full lint passed with zero errors and existing warnings using the test TypeScript project.

Context

How did you test your changes?

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:

  1. Fixes #...

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

Low Risk
Localized parser change for unary % chaining with docs/changelog only; no auth, data, or API surface changes.

Overview
Fixes formula parsing so postfix % can repeat on the same operand (e.g. =5%%), aligning behavior with Excel where each % divides the current value by 100.

The parser rule rightUnaryOpAtomicExpression no longer accepts at most one PercentOp; it consumes any number of % tokens and nests buildPercentOpAst for each, preserving per-operator whitespace and existing precedence. =5%% evaluates to 0.0005 (same as =(5%)%).

The operator guide and Unreleased changelog document consecutive % and clarify that unary % divides by 100.

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

Formulas such as =5%% failed to parse even though =(5%)% worked.
Consume consecutive postfix percent operators and wrap the preceding
expression for each token, so =5%% returns 0.0005 and =5%%% returns
0.000005, matching Excel.

Preserve each operator's whitespace and the existing operator precedence.
Document the behavior in the operator guide and Unreleased changelog.

Validated with the matching fix/HF-268 test branch: 502 Jest suites and
6,254 tests passed, and Chrome and Firefox each passed 6,254 tests.
Compilation and performance benchmarks passed. Full lint passed with
zero errors and existing warnings using the test TypeScript project.
@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.

@qunabu

qunabu commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Task linked: HF-268 Promile operator (%%)

Add the pull request reference to the consecutive percent operators
changelog entry, matching the surrounding entries.

Validated the PR target and checked the Markdown diff for whitespace errors.

@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 5361c44. Configure here.

Comment thread CHANGELOG.md Outdated
@github-actions

github-actions Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Performance comparison of head (0fc1808) vs base (5abbbd9)

                                     testName |   base |   head |  change
-------------------------------------------------------------------------
                                      Sheet A | 272.57 | 290.48 |  +6.57%
                                      Sheet B |  91.35 | 103.34 | +13.13%
                                      Sheet T |  78.14 |  85.95 |  +9.99%
                                Column ranges | 302.79 | 311.37 |  +2.83%
                                Sorted lookup | 8992.3 | 9273.6 |  +3.13%
Sheet A:  change value, add/remove row/column |   9.22 |   8.78 |  -4.77%
 Sheet B: change value, add/remove row/column |  80.03 |  81.66 |  +2.04%
                   Column ranges - add column |  89.09 |  87.01 |  -2.33%
                Column ranges - without batch | 274.45 | 269.63 |  -1.76%
                        Column ranges - batch |  77.92 |   73.6 |  -5.54%

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 14, 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 Updated (UTC)
❌ Deployment failed
View logs
hyperformula-docs 0fc1808 Sep 30 2026, 09:28 AM

@marcin-kordas-hoc marcin-kordas-hoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Recommendation: Approve, blocked on a companion PR.

I reproduced the reported parsing defect on the current base: =1%%, =1%%%, =-1%%, =5%%
and =5%%% all fail to parse before this change ("Redundant input, expecting EOF but found: %"),
and all five parse and evaluate correctly after it (0.0001, 0.000001, -0.0001, 0.0005 and
0.000005 respectively). Measured via MS Graph against a live Excel Online session: Excel accepts
every one of these forms, plus a chained-percent-on-a-reference case and a percent/exponent
precedence case, and the fixed engine's values match Excel's, including the precedence case
(2^1%% → 1.0000693171 in the engine and 1.00006931712038 in Excel — agreement to 10 decimal
places).

I ran the full engine test suite together with the paired hyperformula-tests branch
(fix/HF-268) locally: 502 suites, 6254 tests passing, matching this PR's own reported numbers,
and confirmed the same result from this PR's CI log for the browser (Karma) run.

One blocker before merge: fix/HF-268 on hyperformula-tests has no open PR. Since our test repo
CI resolves the paired branch by name, it did gate this PR's own green status — but with no PR
open there, those test changes have no path into hyperformula-tests's develop. I confirmed
the consequence directly: running the four touched paired spec files against hyperformula-tests
develop (instead of fix/HF-268) while keeping this fix gives 2 failing suites / 3 failing
tests, because develop still encodes the pre-fix expectation that %% doesn't parse. Merging
this PR alone would redden our own develop the next time the paired suite runs against it.
Could you open a PR for fix/HF-268 so the two land together?

One clarification on intent, since the task title can mislead: HF-268's intended semantics are
Excel's chained percent — each % divides by 100, so 5%% = 0.0005 — not a promille (‰, ÷1000)
operator. Verified against live Excel Online and matched by this PR.

@Tobiadefami

Tobiadefami commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

@marcin-kordas-hoc Thanks for catching this. I opened the companion test PR (hyperformula-tests #65). It targets develop from fix/HF-268, so the tests can be reviewed and merged alongside engine #1773.

@Tobiadefami
Tobiadefami marked this pull request as draft September 30, 2026 08:48
@Tobiadefami
Tobiadefami marked this pull request as ready for review September 30, 2026 09:02
Tobiadefami and others added 2 commits September 30, 2026 10:04
Gather the consecutive PercentOp tokens in the MANY loop and build the
nested percent AST with a single reduce afterwards, keeping token
consumption separate from AST construction. Behavior is unchanged:
=5%% returns 0.0005 and each operator keeps its leading whitespace.

Validated with the matching fix/HF-268 test branch: 502 Jest suites and
6,254 tests passed (3 skipped). Type-check and lint passed with zero
errors.
@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.32%. Comparing base (5abbbd9) to head (0fc1808).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1773      +/-   ##
===========================================
- Coverage    97.32%   97.32%   -0.01%     
===========================================
  Files          195      195              
  Lines        15739    15738       -1     
  Branches      3461     3491      +30     
===========================================
- Hits         15318    15317       -1     
  Misses         413      413              
  Partials         8        8              
Files with missing lines Coverage Δ
src/parser/FormulaParser.ts 97.37% <100.00%> (-0.01%) ⬇️
🚀 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.

This branch has not been deployed

No deployments
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