From b07bfd4efb617770d63f823db371da08c3331e9c Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Wed, 22 Apr 2026 13:53:14 -0700 Subject: [PATCH 1/2] feat: match non-set operators against array-typed user properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Route non-set operators (is, contains, greater, etc.) through the same list-coercion path as set operators. When the property value is a list or a JSON array string, match against each element with any-match semantics — the condition is true if any single element satisfies the operator, including for negation operators like `is not` and `does not contain`. Scalar values continue to flow through the existing single-string match path. Add a `startswith("[")` guard to coerce_string_array so scalar strings skip JSON parsing (and its exception) on every evaluation. Fix two pre-existing bugs in coerce_string_array surfaced by the new path: the JSON-array branch iterated the original string value rather than the parsed list, and scalars were wrapped as `[s]` rather than returning None. Add engine-level unit tests covering scalar, list, and JSON-array-string property values across set and non-set operators. Update the integration test deployment key to the new superset deployment and add operator cases for arrays and JSON array strings. --- src/amplitude_experiment/evaluation/engine.py | 61 ++++++---- tests/evaluation/evaluation_engine_test.py | 109 ++++++++++++++++++ .../evaluation/evaluation_integration_test.py | 50 +++++++- 3 files changed, 197 insertions(+), 23 deletions(-) create mode 100644 tests/evaluation/evaluation_engine_test.py diff --git a/src/amplitude_experiment/evaluation/engine.py b/src/amplitude_experiment/evaluation/engine.py index 265b525..cb08ac0 100644 --- a/src/amplitude_experiment/evaluation/engine.py +++ b/src/amplitude_experiment/evaluation/engine.py @@ -110,25 +110,30 @@ def match_condition( """Match a single condition.""" prop_value = select(target, condition.selector) - # We need special matching for null properties and set type prop values - # and operators. All other values are matched as strings, since the - # filter values are always strings. + # Null values use dedicated null matching. For non-null values, we try + # to coerce to a string list first: set operators require a list, and + # non-set operators use any-match semantics over the elements when the + # value is multi-valued. Scalars fall through to single-string matching. if not prop_value: return self.match_null(condition.op, condition.values) - elif self.is_set_operator(condition.op): - prop_value_string_list = self.coerce_string_array(prop_value) + + prop_value_string_list = self.coerce_string_array(prop_value) + if self.is_set_operator(condition.op): if not prop_value_string_list: return False return self.match_set(prop_value_string_list, condition.op, condition.values) - else: - prop_value_string = self.coerce_string(prop_value) - if prop_value_string is not None: - return self.match_string( - prop_value_string, - condition.op, - condition.values - ) - return False + if prop_value_string_list is not None: + return self.match_strings_non_set( + prop_value_string_list, condition.op, condition.values + ) + prop_value_string = self.coerce_string(prop_value) + if prop_value_string is not None: + return self.match_string( + prop_value_string, + condition.op, + condition.values + ) + return False def get_hash(self, key: str) -> int: """Generate a hash value from a key.""" @@ -222,6 +227,18 @@ def match_set(self, prop_values: List[str], op: str, filter_values: List[str]) - return not self.matches_set_contains_any(prop_values, filter_values) return False + def match_strings_non_set( + self, + prop_values: List[str], + op: str, + filter_values: List[str] + ) -> bool: + """Match any element of a multi-valued property against a non-set operator.""" + return any( + self.match_string(prop_value, op, filter_values) + for prop_value in prop_values + ) + def match_string(self, prop_value: str, op: str, filter_values: List[str]) -> bool: """Match string values against filter values.""" if op == EvaluationOperator.IS: @@ -363,21 +380,21 @@ def coerce_string(self, value: Any) -> Optional[str]: return str(value) def coerce_string_array(self, value: Any) -> Optional[List[str]]: - """Coerce value to string array, handling special cases.""" + """Coerce a list-like value to a string list, else return None.""" if isinstance(value, list): return [s for s in map(self.coerce_string, value) if s is not None] string_value = str(value) + # Cheap guard so scalar strings skip JSON parsing (and its exception). + if not string_value.startswith("["): + return None try: parsed_value = json.loads(string_value) - if isinstance(parsed_value, list): - return [s for s in map(self.coerce_string, value) if s is not None] - - s = self.coerce_string(string_value) - return [s] if s is not None else None except json.JSONDecodeError: - s = self.coerce_string(string_value) - return [s] if s is not None else None + return None + if not isinstance(parsed_value, list): + return None + return [s for s in map(self.coerce_string, parsed_value) if s is not None] def is_set_operator(self, op: str) -> bool: """Check if operator is a set operator.""" diff --git a/tests/evaluation/evaluation_engine_test.py b/tests/evaluation/evaluation_engine_test.py new file mode 100644 index 0000000..97fb9be --- /dev/null +++ b/tests/evaluation/evaluation_engine_test.py @@ -0,0 +1,109 @@ +import unittest +from typing import Any, Dict, List, Optional + +from src.amplitude_experiment.evaluation.engine import EvaluationEngine +from src.amplitude_experiment.evaluation.types import ( + EvaluationCondition, + EvaluationFlag, + EvaluationOperator, + EvaluationSegment, + EvaluationVariant, +) + + +class EvaluationEngineTestCase(unittest.TestCase): + """Unit tests for EvaluationEngine covering non-set array matching.""" + + def setUp(self): + self.engine = EvaluationEngine() + + @staticmethod + def flag_with_condition(op: str, values: List[str]) -> EvaluationFlag: + return EvaluationFlag( + key="test-flag", + variants={"on": EvaluationVariant(key="on", value="on")}, + segments=[ + EvaluationSegment( + conditions=[[ + EvaluationCondition( + selector=["context", "user", "user_properties", "test_prop"], + op=op, + values=values, + ) + ]], + variant="on", + ) + ], + ) + + @staticmethod + def context_with_prop(value: Any) -> Dict[str, Any]: + return {"user": {"user_properties": {"test_prop": value}}} + + def evaluate(self, prop_value: Any, op: str, values: List[str]) -> Optional[EvaluationVariant]: + flag = self.flag_with_condition(op, values) + context = self.context_with_prop(prop_value) + return self.engine.evaluate(context, [flag]).get("test-flag") + + def assert_match(self, prop_value: Any, op: str, values: List[str]): + variant = self.evaluate(prop_value, op, values) + self.assertIsNotNone( + variant, f"Expected match for prop={prop_value!r} op={op!r} values={values!r}" + ) + self.assertEqual(variant.key, "on") + + def assert_no_match(self, prop_value: Any, op: str, values: List[str]): + variant = self.evaluate(prop_value, op, values) + self.assertIsNone( + variant, f"Expected no match for prop={prop_value!r} op={op!r} values={values!r}" + ) + + def test_scalar_string_is_match(self): + self.assert_match("hello", EvaluationOperator.IS, ["hello"]) + + def test_scalar_string_contains_match(self): + self.assert_match("hello", EvaluationOperator.CONTAINS, ["ell"]) + + def test_scalar_string_greater_than_match(self): + self.assert_match("2", EvaluationOperator.GREATER_THAN, ["1"]) + + def test_scalar_string_is_no_match(self): + self.assert_no_match("world", EvaluationOperator.IS, ["hello"]) + + def test_non_string_scalar_greater_than(self): + self.assert_match(42, EvaluationOperator.GREATER_THAN, ["1"]) + + def test_non_string_scalar_is_boolean(self): + self.assert_match(True, EvaluationOperator.IS, ["true"]) + + def test_json_array_string_set_operator(self): + self.assert_match('["a","b"]', EvaluationOperator.SET_CONTAINS, ["a"]) + + def test_json_array_string_non_set_operator(self): + self.assert_match('["a","b"]', EvaluationOperator.IS, ["a"]) + + def test_collection_set_operator(self): + self.assert_match(["a", "b"], EvaluationOperator.SET_CONTAINS, ["a"]) + + def test_collection_non_set_operator(self): + self.assert_match(["a", "b"], EvaluationOperator.IS, ["a"]) + + def test_malformed_json_array_falls_through(self): + self.assert_match("[broken", EvaluationOperator.IS, ["[broken"]) + + def test_empty_json_array_set_operator(self): + self.assert_no_match("[]", EvaluationOperator.SET_CONTAINS, ["a"]) + + def test_empty_json_array_non_set_operator(self): + # Not in the spec table; locks in any([]) -> False for the non-set path. + self.assert_no_match("[]", EvaluationOperator.IS, ["a"]) + + def test_leading_whitespace_not_parsed_non_set(self): + self.assert_match(' ["a"]', EvaluationOperator.IS, [' ["a"]']) + + def test_leading_whitespace_not_parsed_set(self): + self.assert_no_match(' ["a"]', EvaluationOperator.SET_CONTAINS, ["a"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/evaluation/evaluation_integration_test.py b/tests/evaluation/evaluation_integration_test.py index 0debc2d..5947278 100644 --- a/tests/evaluation/evaluation_integration_test.py +++ b/tests/evaluation/evaluation_integration_test.py @@ -12,7 +12,7 @@ class EvaluationIntegrationTestCase(unittest.TestCase): @classmethod def setUpClass(cls): """Set up test fixtures before running tests.""" - cls.deployment_key = "server-NgJxxvg8OGwwBsWVXqyxQbdiflbhvugy" + cls.deployment_key = "server-VVhLULXCxxY0xqmszXouXxiEzoeJWmSh" cls.engine = EvaluationEngine() cls.flags = cls.get_flags(cls.deployment_key) @@ -497,6 +497,54 @@ def test_is_with_booleans(self): result = self.engine.evaluate(user, self.flags)["test-is-with-booleans"] self.assertEqual(result.key, "on") + def test_set_is_with_json_array_string(self): + """Test 'set is' operator with a JSON array string property value.""" + user = self.user_context(None, None, None, {"key": '["1", "2", "3"]'}) + result = self.engine.evaluate(user, self.flags)["test-set-is"] + self.assertEqual(result.key, "on") + + def test_is_with_array_collection(self): + """Test 'is' operator with a list property value (any-match).""" + user = self.user_context(None, None, None, {"key": ["value1", "value2"]}) + result = self.engine.evaluate(user, self.flags)["test-is-array"] + self.assertEqual(result.key, "on") + + def test_is_not_with_array_collection(self): + """Test 'is not' operator with a list property value (any-match).""" + user = self.user_context(None, None, None, {"key": ["value3", "value4"]}) + result = self.engine.evaluate(user, self.flags)["test-is-not-array"] + self.assertEqual(result.key, "on") + + def test_contains_with_array_collection(self): + """Test 'contains' operator with a list property value (any-match).""" + user = self.user_context(None, None, None, { + "key": ["has-target-value", "has", "value"] + }) + result = self.engine.evaluate(user, self.flags)["test-contains-array"] + self.assertEqual(result.key, "on") + + def test_does_not_contain_with_array_collection(self): + """Test 'does not contain' operator with a list property value (any-match).""" + user = self.user_context(None, None, None, { + "key": ["has-value", "has", "value"] + }) + result = self.engine.evaluate(user, self.flags)["test-does-not-contain-array"] + self.assertEqual(result.key, "on") + + def test_is_with_json_array_string(self): + """Test 'is' operator with a JSON array string property value (any-match).""" + user = self.user_context(None, None, None, {"key": '["value1", "value2"]'}) + result = self.engine.evaluate(user, self.flags)["test-is-array"] + self.assertEqual(result.key, "on") + + def test_does_not_contain_with_json_array_string(self): + """Test 'does not contain' operator with a JSON array string property value.""" + user = self.user_context(None, None, None, { + "key": '["has-value", "has", "value"]' + }) + result = self.engine.evaluate(user, self.flags)["test-does-not-contain-array"] + self.assertEqual(result.key, "on") + @staticmethod def freeform_user_context(user: Dict[str, Any]) -> Dict[str, Any]: """Create a freeform user context dictionary.""" From 29dddfaab5891d76e47faaf59e5fd950c39cde1c Mon Sep 17 00:00:00 2001 From: Kenneth Yeh Date: Wed, 22 Apr 2026 14:27:06 -0700 Subject: [PATCH 2/2] docs: document any-match semantics for negation operators in match_strings_non_set --- src/amplitude_experiment/evaluation/engine.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/amplitude_experiment/evaluation/engine.py b/src/amplitude_experiment/evaluation/engine.py index cb08ac0..4044f27 100644 --- a/src/amplitude_experiment/evaluation/engine.py +++ b/src/amplitude_experiment/evaluation/engine.py @@ -233,7 +233,15 @@ def match_strings_non_set( op: str, filter_values: List[str] ) -> bool: - """Match any element of a multi-valued property against a non-set operator.""" + """Match any element of a multi-valued property against a non-set operator. + + Negation operators (is not, does not contain) also use any-match semantics + — the condition is true if any single element satisfies the negation, not + if all elements do. For example, `is not "A"` on `["A", "B"]` is true + because "B" is not "A", even though "A" is present. This matches + analytics/charts filtering behavior and the Kotlin evaluation-core + reference implementation. + """ return any( self.match_string(prop_value, op, filter_values) for prop_value in prop_values