feat: match array properties element-wise for non-set operators#91
Closed
feat: match array properties element-wise for non-set operators#91
Conversation
Collaborator
Author
|
Superseded by #92, which is cut cleanly from latest main without unrelated stale commits. |
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
Adds element-wise (any-match) evaluation for multi-valued user properties when paired with non-set operators (
is,contains,greater, etc.), aligning the local evaluation engine with Amplitude analytics/charts behavior.Previously, non-set operators always stringified the whole property value via
coerce_string— so an array property like["a","b"]would never satisfyis "a". Now:match_conditioncallscoerce_string_arrayfor every non-null value. If the value is multi-valued (native array or JSON array string) and the operator is non-set, it delegates to a newmatch_strings_non_sethelper that returns true if any element satisfies the operator.is not,does not contain) also use any-match semantics —is not "A"on["A","B"]istruebecause"B"is not"A".coerce_string→match_stringpath unchanged.Also fixes two issues in
coerce_string_array:start_with?('[')pre-check so scalar strings skipJSON.parseand avoid exception-driven control flow on every evaluation.[value]instead ofnil, which caused set operators to spuriously match single-string properties.Tests
spec/experiment/evaluation/evaluation_spec.rbwith an inline test harness (flag_with_condition,context_with_prop,evaluate,assert_match,assert_no_match) and 14 unit cases covering scalars, native arrays, JSON array strings, malformed JSON, and leading-whitespace edge cases.operator testsblock; the deployment key is consolidated toserver-VVhLULXCxxY0xqmszXouXxiEzoeJWmSh(a superset of the prior key, containing all existing flags plus the new multi-value flags).Full suite: 271 examples, 0 failures. Rubocop: clean.
Checklist
Note
Medium Risk
Changes the local flag evaluation logic for non-set operators when targeting multi-valued properties, which can alter rollout/targeting results for existing flags. Also refactors evaluation classes into the
AmplitudeExperiment::Evaluationnamespace, so any missed references could cause runtime load/name errors.Overview
Local flag evaluation now supports element-wise (any-match) targeting for multi-valued user properties when using non-set operators (e.g.
is,contains, comparisons), including handling JSON array strings; this also tightenscoerce_string_arrayto avoid parsing non-array scalars and to stop treating malformed/non-array JSON as a singleton array.The evaluation subsystem is namespaced and consolidated under
AmplitudeExperiment::Evaluationvia a newlib/experiment/evaluation.rbentrypoint, with updates toLocalEvaluationClientand specs, plus expanded test coverage (new unit spec and added integration cases) for the new array-matching behavior.Reviewed by Cursor Bugbot for commit 1c89f46. Bugbot is set up for automated code reviews on this repo. Configure here.