Skip to content

chore(analytics): replace json-logic-rb with a bundled evaluator (SDK-162) - #174

Closed
tylerjroach wants to merge 1 commit into
masterfrom
chore/sdk-162-drop-json-logic-rb
Closed

tylerjroach wants to merge 1 commit into
masterfrom
chore/sdk-162-drop-json-logic-rb

Conversation

@tylerjroach

Copy link
Copy Markdown
Contributor

Closes SDK-162.

Why

Local flag evaluation resolves runtime_evaluation_rule trees with JsonLogic, which came from the json-logic-rb gem. Two problems with that:

  • It is a runtime dependency in every host application's graph, for ~400 lines of tree walking over a fixed operator set that we control end to end.
  • The gem exposes one process-wide engine, and custom_operators.rb called JsonLogic.add_operation on it. Loading this SDK changed how an unrelated JsonLogic rule elsewhere in the host application evaluated.

What

Mixpanel::Flags::JsonLogic (lib/mixpanel-ruby/flags/json_logic.rb) replaces it, with the operator registry private to the SDK. Everything else is a call-site swap: custom_operators.rb registers against the new module, local_flags_provider.rb applies against it, and the gemspec dependency is gone.

Behavior at the provider boundary is unchanged — is_runtime_evaluation_satisfied? still requires a literal true and still fails closed on anything it cannot evaluate. Unknown operators raise UnknownOperatorError, which subclasses ArgumentError, so the existing rescue StandardError catches it exactly as before.

This also makes #140 (dependabot bumping json-logic-rb) moot; it can be closed once this merges.

How it was verified

Two independent checks, beyond the existing flags specs:

1. The official JsonLogic suite. spec/fixtures/jsonlogic_tests.json is vendored verbatim from https://jsonlogic.com/tests.json (byte-identical to the copy json-logic-rb ships). All 278 cases run individually in json_logic_spec.rb, and all 278 pass.

2. A differential run against the gem. ~120k (rule, data) pairs across every operator, comparing old and new side by side. There is no input the gem evaluated successfully and this does not. 49 distinct behavioral differences turned up, and every one is a place the gem departed from the reference implementation:

Rule Data json-logic-rb This PR Reference
{"var": "beta"} {"beta": false} nil false false
{"var": ["beta", "fb"]} {"beta": false} "fb" false false
{"var": ["beta", "fb"]} {"beta": null} "fb" nil nil
{"==": [true, 1]} — false true true
{"+": [1, "apple"]} — 1.0 NaN NaN
{"missing": ["x"]} {"x": ""} [] ["x"] ["x"]
{"missing": ["x"]} {"x": false} ["x"] [] []

The first three are the ones that matter for targeting: the gem could not distinguish a property set to false from an absent one, so {"==": [{"var": "beta"}, false]} never matched, and a false property silently fell through to a var default. These are now covered by specs, and aligning on the reference is also what keeps this SDK's rules evaluating the same way as the other Mixpanel SDKs.

Testing

  • bundle exec rspec — 721 examples, 0 failures on the flags and analytics suites.
  • One unrelated pre-existing failure shows up locally only: tracker_spec.rb:23 calls CGI.parse, which Ruby 4.0 removed. CI runs 3.0–3.4, where it passes; it is untouched by this PR.
  • No Gemfile.lock change to review — it is gitignored in this repo.

🤖 Generated with Claude Code

…-162)

Local flag evaluation resolves `runtime_evaluation_rule` trees with JsonLogic,
which came from the `json-logic-rb` gem. That put a third-party gem in every
host application's dependency graph for ~400 lines of tree walking, and the gem
exposes a single process-wide operator registry that we mutated to install
`semver_compare` and `datetime_compare` -- so loading this SDK changed how an
unrelated JsonLogic rule in the host application evaluated.

Mixpanel::Flags::JsonLogic replaces it. The registry is private to the SDK, and
semantics follow the JsonLogic reference implementation, verified two ways: the
official suite from jsonlogic.com/tests.json is vendored and run case by case
(278/278), and a differential run against json-logic-rb over ~120k (rule, data)
pairs found no input the gem evaluated and this does not. The 49 distinct
differences it did find are all places the gem departed from the reference:

- `var` treated a stored `false` as an absent key, returning null (or the
  default) instead. A boolean custom property is an ordinary way to target a
  flag, so `{"==": [{"var": "beta"}, false]}` did not match.
- `var` returned the default for a key present and holding null, where the
  reference returns null.
- `==` compared a boolean against a number as unequal (`true == 1` was false).
- arithmetic on an unparseable string yielded 0 rather than NaN, so a garbage
  property read as zero instead of failing the comparison.
- `missing` counted a blank string as present and a `false` as missing.

Unknown operators raise UnknownOperatorError, an ArgumentError, which the
caller's existing rescue already turns into an unsatisfied rollout.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tylerjroach
tylerjroach requested review from a team and ketanmixpanel September 21, 2026 16:23
@linear-code

linear-code Bot commented Sep 21, 2026

Copy link
Copy Markdown

SDK-162

@greptile-apps

greptile-apps Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

The PR is not yet safe to merge because collection coercion and reduce scoping can produce incorrect runtime flag decisions.

Findings

  1. P1 Non-arrays become collections ▶
  2. P1 Reduce exposes outer properties ▶

Reviews (1) · Last reviewed commit: "chore(analytics): replace json-logic-rb ..."

# one, with the item -- not the outer scope -- as the data the inner rule reads.
def each_item(arguments, data)
items_rule, item_rule = arguments
items = Array(evaluate(items_rule, data))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Non-arrays become collections

Array(...) wraps scalar and hash custom-property values in a one-element collection, while JsonLogic collection operators treat non-arrays as empty. For example, all over a scalar with an always-true predicate returns true here instead of false, potentially enabling a rollout that should not match.

Comment on lines +203 to +205
scope = (data.is_a?(Hash) ? data : {}).merge(
'' => item, 'current' => item, 'accumulator' => accumulator
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Reduce exposes outer properties

This merges the outer custom-property hash into every reduce step, although JsonLogic reduce steps may access only current and accumulator. A step such as {"var":"enabled"} can therefore resolve an outer property and produce true here instead of an absent value, changing the final rollout decision.

@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.42857% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.10%. Comparing base (ff81cce) to head (be6a8a7).

Files with missing lines Patch % Lines
lib/mixpanel-ruby/flags/json_logic.rb 96.34% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #174      +/-   ##
==========================================
- Coverage   97.28%   97.10%   -0.19%     
==========================================
  Files          16       17       +1     
  Lines         885     1104     +219     
==========================================
+ Hits          861     1072     +211     
- Misses         24       32       +8     
Flag Coverage Δ
openfeature 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant