Skip to content

fix(formula): register mixed double/int arithmetic overloads (#1928) - #1930

Merged
os-zhuang merged 1 commit into
mainfrom
fix/cel-mixed-numeric-arithmetic-overloads
Jun 16, 2026
Merged

fix(formula): register mixed double/int arithmetic overloads (#1928)#1930
os-zhuang merged 1 commit into
mainfrom
fix/cel-mixed-numeric-arithmetic-overloads

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Part 1 of 2 for #1928 (runtime fix). Part 2 adds the build-time typed check.

Problem

cel-js types a record field number as double and a bare integer literal as int, and ships arithmetic overloads only for matching numeric pairs. So an everyday Field.formula like record.amount / 100 or record.price * 2 faults at runtime — no such overload: dyn<double> / int — and the engine catches the fault and the formula silently evaluates to null. Build-green, empty at runtime (the class #1927 hit in expected_revenue).

Fix

registerNumericCoercions registers the missing double <op> int and int <op> double overloads for + - * / %, computing the result as a double (CEL's mixed-numeric promotion rule). Wired into the CEL engine's buildEnv.

  • Pure int op int is untouched → integer division 7 / 2 == 3 preserved. Overloads fire only on a genuine double × int pair.
  • Composes with the existing string-field hydration retry: "120000.00" + 1 == 120001.
  • Authors no longer need the / 100.0 ritual — reverts that workaround in example-crm's expected_revenue back to plain / 100.
record.amount / 100 → 1200 (was: null)
record.amount * record.probability/100 → 84000 (was: null)
7 / 2 → 3 (unchanged — integer divide)
record.a / record.b (10, 4) → 2.5 (unchanged)

Tests

7 new cases in cel-engine.test.ts (mixed double/int arithmetic overloads); full formula suite 102/102 green. example-crm builds clean and expected_revenue computes natively.

Refs #1928. Follow-up PR: build-time typed check() to turn the genuinely-wrong cases (field typos, type mismatches, bare identifiers) into actionable build errors.

🤖 Generated with Claude Code

cel-js types a record field number as `double` and a bare integer literal as
`int`, with overloads only for matching pairs. An everyday formula like
`record.amount / 100` or `record.price * 2` therefore faulted at runtime
(`no such overload: dyn<double> / int`) and the engine silently returned null —
build-green, empty at runtime.
Register the missing `double <op> int` / `int <op> double` overloads for
`+ - * / %` (result computed as double, per CEL mixed-numeric promotion).
Pure int/int is untouched, so integer division (`7 / 2 == 3`) is preserved;
overloads fire only on a genuine double×int pair. Composes with the existing
string-field hydration retry (`"120000.00" + 1 == 120001`).
Reverts the `/ 100.0` float-literal workaround in example-crm's
`expected_revenue` back to plain `/ 100` now that it computes natively.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercelBot commented Jun 15, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
specReadyReadyPreview, CommentJun 15, 2026 6:12pm

Request Review

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Jun 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/formula.

5 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/packages.mdx(via @objectstack/formula)
  • content/docs/guides/formula.mdx(via @objectstack/formula)
  • content/docs/guides/metadata/validation.mdx(via @objectstack/formula)
  • content/docs/guides/packages.mdx(via @objectstack/formula)
  • content/docs/protocol/objectui/record-alert.mdx(via @objectstack/formula)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@os-zhuang