fix(formula): hydrate string-serialized numeric fields in CEL comparisons (#1534)#1536
Merged
Conversation
…sons (#1534) Numeric fields that serialize as strings (Field.rating -> "5.0", Field.currency -> "250000.00", Field.percent) made predicates like `record.rating >= 4` fault under strict CEL (`no such overload: dyn >= int`). In automation flow decision/edge conditions this silently dead-ended the run (no edge matched); in objectql applyFormulaPlan it swallowed to null. The shared CEL engine now retries an evaluation once with purely-numeric strings hydrated to numbers, but only after a `no such overload` fault — so a comparison that already type-checks is never re-interpreted, and a genuinely non-numeric operand still faults loudly. Because both the automation condition path and the objectql formula path route through ExpressionEngine.evaluate, both are fixed consistently. https://claude.ai/code/session_01FjtHqsTWWPThCP6jsVdvPa
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
CodeQL flagged polynomial backtracking in NUMERIC_STRING_RE: the `\d+\.?\d*` form degrades to adjacent unbounded quantifiers (`\d+\d*`) when the dot is absent, backtracking on long digit runs. Anchor the fractional part as a single optional `(?:\.\d*)?` group — same matched language, no hazard. https://claude.ai/code/session_01FjtHqsTWWPThCP6jsVdvPa
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.
Summary
Fixes #1534. Record-change flow decision / edge conditions that compare a numeric field against a literal (
record.rating >= 4,record.amount > 100000) faulted at runtime whenever the field's value serialized as a string —Field.rating(allowHalf)→"5.0",Field.currency(scale)→"250000.00",Field.percentlikewise. Strict CEL raisedno such overload: dyn >= int, so the comparison could never be satisfied and the flow silently dead-ended at the decision node (no edge matched,success: true, nothing downstream ran).Root cause
The bound field value is the raw, string-serialized row value, while the build-time validator (ADR-0032 §1a) and schema treat the field as numeric — a build-vs-runtime mismatch. cel-js is strict:
string <op> numberhas no overload.Fix
The shared
@objectstack/formulaCEL engine (cel-engine.ts) now retries an evaluation once with purely-numeric strings hydrated to numbers — but only after ano such overloadfault. Key safety property:record.zip == "02134", string == string) never enters the retry path, so it is never re-interpreted.record.rating >= 4whereratingis"high") still faults loudly; if the retry can't type-check, the original error is surfaced (ADR-0032 §1c/§1d — fail loudly, never swallow).Because both projection paths the issue calls out route through
ExpressionEngine.evaluate—service-automation'sevaluateConditionandobjectql'sapplyFormulaPlan— fixing the shared engine resolves them consistently, as requested in the issue's relationship to #1530.The "silent dead-end" half is already covered on
main:evaluateConditionthrows an attributed error on a non-okCEL result (the #1491 fix), whichexecute()records as a loudfailedrun. With this change the common numeric-string case no longer faults at all.Tests
packages/formula/src/cel-engine.test.ts— newnumeric-string field hydrationsuite: rating"5.0" >= 4, currency"250000.00" > 100000, percent,previous.*transition gates, compound predicates, unmet compares returningfalse(not a fault), genuine string equality left untouched, and non-numeric strings still faulting.packages/services/service-automation/src/engine.test.ts— end-to-end: a record-change flow routes through adecisionnode'srecord.rating >= 4edge (rating"5.0") to the hot-lead branch instead of dead-ending.All formula (82) and automation (98) tests pass.
Scope note
The arithmetic case (
record.amount + 100on a hydrated number) hits a separate cel-js limitation —double + intliteral has no overload even for genuine numbers — and is out of scope for this comparison-focused issue.https://claude.ai/code/session_01FjtHqsTWWPThCP6jsVdvPa
Generated by Claude Code