Skip to content

Fix binary operator reduction & associativity handling in parser; add operator precedence tests#1

Merged
ChasLui merged 1 commit into
masterfrom
codex/find-and-fix-an-important-bug
May 14, 2026
Merged

Fix binary operator reduction & associativity handling in parser; add operator precedence tests#1
ChasLui merged 1 commit into
masterfrom
codex/find-and-fix-an-important-bug

Conversation

@ChasLui
Copy link
Copy Markdown
Owner

@ChasLui ChasLui commented May 14, 2026

Motivation

  • Make binary operator reduction respect operator associativity (notably right-associative exponentiation) and improve multi-percent handling in the formula parser.

Description

  • Replace the previous precedence-only list with BINARY_OPERATOR_GROUPS that pairs operator sets with an associativity flag and implement reduceBinaryOperators and applyInfixAt to correctly reduce operators by associativity.
  • Make percent postfix parsing repeatable by changing the percent handling from OPTION to MANY so multiple % tokens are applied in sequence.
  • Add @types/node to devDependencies and update the pnpm lockfile accordingly.
  • Extend test/operators/testcase.ts with many operator-precedence and associativity cases (exponentiation chaining, unary minus vs exponent, chained comparisons, percent combinations, concatenation order, and others).

Testing

  • Ran the unit test suite via npm run test, which includes the operator tests in test/operators/testcase.ts, and the tests passed.
  • The updated operator test cases covering exponentiation associativity, multiple percent postfixes, chaining, and concatenation were executed and succeeded.

Codex Task

@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 25848258237

Warning

No base build found for commit d8305a0 on master.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 95.048%

Details

  • Patch coverage: 31 of 31 lines across 1 file are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 5548
Covered Lines: 5284
Line Coverage: 95.24%
Relevant Branches: 2247
Covered Branches: 2125
Branch Coverage: 94.57%
Branches in Coverage %: Yes
Coverage Strength: 10537.48 hits per line

💛 - Coveralls

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a217cc24c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread grammar/parsing.ts
private utils: Utils;
private binaryOperatorsPrecedence: string[][];
private static readonly BINARY_OPERATOR_GROUPS: ReadonlyArray<BinaryOperatorGroup> = [
{ opSet: new Set(['^']), associativity: 'right' },
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep exponentiation left-associative for Excel formulas

For Excel-compatible formulas, chained operators at the same precedence are evaluated left-to-right; Microsoft’s Excel precedence docs state this for same-precedence operators, so 2^3^2 should be reduced as (2^3)^2 (64), not 2^(3^2) (512). Marking ^ as right-associative changes results for any chained exponent formula and diverges from Excel behavior.

Useful? React with 👍 / 👎.

@ChasLui ChasLui merged commit ab336c1 into master May 14, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants