Add JSON Logic comparison operators (<, <=, >, >=)#3484
Draft
ajpallares wants to merge 4 commits into
Draft
Conversation
Extends the JSON Logic operator set with the comparison operators rule
authors need to express numeric thresholds and ranges.
What's new:
- `<`, `<=`, `>`, `>=` per the JSON Logic spec.
- `<` and `<=` accept the 3-arg between form (`{"<=": [1, x, 10]}`
reads as `1 <= x <= 10`). `>` and `>=` are binary only, matching
the JS reference.
- All operators coerce operands through `Value.toNumberOrNull` and
compare as `Double`. Non-numeric operands (`ObjectValue`,
`ArrayValue`, unparseable strings) become `Double.NaN`; per
IEEE 754 every comparison against NaN is `false`, so a malformed
operand makes the predicate fail closed.
String semantics deviate from the JS reference (which compares two
strings lexicographically, e.g. `"10" < "9"` is true). We always
coerce numerically — `"10" < "9"` is false. Documented in the type's
KDoc.
Tests:
- `ComparisonOperatorsTest` covers each operator (basic, between
form, coercion, NaN propagation, no-between for `>` / `>=`, arity
errors).
- `EvaluatorTest` adds two integration tests through dispatch:
`var >= 3` and the 3-arg `1 <= var <= 10` between form.
Co-authored-by: Cursor <cursoragent@cursor.com>
…nto pallares/json-logic-comparison-operators
Mirrors the iOS counterpart `a60dac602`. The original implementation documented a deliberate deviation from the JSON Logic / JS spec and always coerced operands to `Double` for `<`, `<=`, `>`, `>=`. The spec (ECMAScript Abstract Relational Comparison) actually splits on operand type: - **Both operands are strings** → lexicographic comparison (`"10" < "9"` is `true` because `'1'` < `'9'` byte-wise). - **Otherwise** → numeric coercion (with `Double.NaN` fall-back for non-coercible operands so comparisons fail closed per IEEE 754). Refactor: - `ComparisonOperators` gains a private `Comparator` enum that dispatches to two explicit `apply` overloads (one `Double`, one `String`) — Kotlin's primitive `<` on `Double` is IEEE-conformant while a generic `Comparable<Double>.compareTo` route would silently flip to total-order semantics (NaN becomes greater than +Infinity). Pinned in a doc comment on the enum. - `compare(lhs, rhs, cmp)` picks the lex path when both operands are `Value.StringValue` and the numeric path otherwise. - `evalChain` and `evalBinary` now take a `Comparator` (not a `(Double, Double) -> Boolean` lambda) and route through `compare`. Tests: - `lt string compared numerically not lexicographically` is replaced by `lt compares two strings lexicographically` (the new pin) and `lt mixed string and number coerces numerically` (mixed-type fall through to numeric coercion). - New `le compares two strings lexicographically inclusive` and `gt compares two strings lexicographically` round out the cross-operator coverage. Drive-by, from the merge of `pallares/json-logic-evaluator`: drop the now-unused `RulesEngineLogger` parameter (and the `PrintlnLogger` import) from `ComparisonOperators` / `ComparisonOperatorsTest`, and fix the stale `, logger` references the auto-merge left in `Operators.dispatch` for the comparison cases. Engine internals route warnings through the module-level `Rules.logger` now, so threading the logger through every operator call is no longer needed. Verified: `:rules-engine:testDefaultsDebugUnitTest`, `detektAll`, and `scripts/check-rules-engine-internal-only.sh` all green. Co-authored-by: Cursor <cursoragent@cursor.com>
…nto pallares/json-logic-comparison-operators
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.
Resolves SDK-4328
Motivation
Extends the JSON Logic operator set with the comparison operators rule authors need to express numeric thresholds and ranges.
Summary
<,<=,>,>=per the JSON Logic spec.<and<=accept the 3-arg between form ({"<=": [1, x, 10]}reads as1 <= x <= 10).>and>=are binary only, matching the JS reference.Value.toNumberOrNulland compare asDouble. Non-numeric operands (ObjectValue,ArrayValue, unparseable strings) becomeDouble.NaN; per IEEE 754 every comparison against NaN isfalse, so a malformed operand makes the predicate fail closed.Tests
ComparisonOperatorsTestcovers each operator (basic, between form, coercion, NaN propagation, no-between for>/>=, arity errors).EvaluatorTestadds two integration tests through dispatch:var >= 3and the 3-arg1 <= var <= 10between form.Notes
"10" < "9"is true) and only coerces when types mix. We always coerce numerically, which gives the more intuitive"10" < "9"is false. Documented in the type's KDoc.mainonce that lands.Made with Cursor