From fff3c8b3a181e2304e5b27060a240ec8dc277104 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 7 Aug 2026 09:34:20 +0200 Subject: [PATCH 1/3] fix(flags): match backend cohort definitions --- .sampo/changesets/grim-iceseeker-akka.md | 5 + posthog/feature_flags.py | 152 +++++++++++------------ posthog/test/test_feature_flags.py | 70 +++++++++++ 3 files changed, 146 insertions(+), 81 deletions(-) create mode 100644 .sampo/changesets/grim-iceseeker-akka.md diff --git a/.sampo/changesets/grim-iceseeker-akka.md b/.sampo/changesets/grim-iceseeker-akka.md new file mode 100644 index 00000000..3df3b457 --- /dev/null +++ b/.sampo/changesets/grim-iceseeker-akka.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Fix local evaluation for negated, missing, and malformed cohort definitions diff --git a/posthog/feature_flags.py b/posthog/feature_flags.py index 87fbbb6f..7b4b09b7 100644 --- a/posthog/feature_flags.py +++ b/posthog/feature_flags.py @@ -735,7 +735,7 @@ def match_cohort( ) property_group = cohort_properties[cohort_id] - return match_property_group( + matches = match_property_group( property_group, property_values, cohort_properties, @@ -745,6 +745,13 @@ def match_cohort( device_id=device_id, ) + operator = property.get("operator") or "exact" + if operator in ("exact", "in"): + return matches + if operator == "not_in": + return not matches + raise InconclusiveMatchError(f"Unsupported cohort operator: {operator}") + def match_property_group( property_group, @@ -755,22 +762,31 @@ def match_property_group( distinct_id=None, device_id=None, ) -> bool: - if not property_group: + # The backend serializes its canonical empty PropertyGroup as {}. + if property_group == {}: return True + if not isinstance(property_group, dict): + raise RequiresServerEvaluation("Cohort property group must be an object") property_group_type = property_group.get("type") + is_and = property_group_type != "OR" properties = property_group.get("values") - - if not properties or len(properties) == 0: - # empty groups are no-ops, always match + if not isinstance(properties, list): + raise RequiresServerEvaluation("Cohort property group values must be a list") + if not properties: return True + decisive_result = None error_matching_locally = False - if "values" in properties[0]: - # a nested property group - for prop in properties: - try: + for prop in properties: + try: + if not isinstance(prop, dict): + raise RequiresServerEvaluation( + "Cohort property group entry must be an object" + ) + + if "values" in prop or prop.get("type") in ("AND", "OR"): matches = match_property_group( prop, property_values, @@ -780,81 +796,55 @@ def match_property_group( distinct_id, device_id=device_id, ) - if property_group_type == "AND": - if not matches: - return False - else: - # OR group - if matches: - return True - except RequiresServerEvaluation: - # Immediately propagate - this condition requires server-side data - raise - except InconclusiveMatchError as e: - log.debug(f"Failed to compute property {prop} locally: {e}") - error_matching_locally = True - - if error_matching_locally: - raise InconclusiveMatchError( - "Can't match cohort without a given cohort property value" - ) - # if we get here, all matched in AND case, or none matched in OR case - return property_group_type == "AND" - - else: - for prop in properties: - try: - if prop.get("type") == "cohort": - matches = match_cohort( - prop, - property_values, - cohort_properties, - flags_by_key, - evaluation_cache, - distinct_id, - device_id=device_id, - ) - elif prop.get("type") == "flag": - matches = evaluate_flag_dependency( - prop, - flags_by_key, - evaluation_cache, - distinct_id, - property_values, - cohort_properties, - device_id=device_id, - ) - else: - matches = match_property(prop, property_values) - + negation = False + elif prop.get("type") == "cohort": + matches = match_cohort( + prop, + property_values, + cohort_properties, + flags_by_key, + evaluation_cache, + distinct_id, + device_id=device_id, + ) + negation = prop.get("negation", False) + elif prop.get("type") == "flag": + matches = evaluate_flag_dependency( + prop, + flags_by_key, + evaluation_cache, + distinct_id, + property_values, + cohort_properties, + device_id=device_id, + ) + negation = prop.get("negation", False) + else: + matches = match_property(prop, property_values) negation = prop.get("negation", False) - if property_group_type == "AND": - # if negated property, do the inverse - if not matches and not negation: - return False - if matches and negation: - return False - else: - # OR group - if matches and not negation: - return True - if not matches and negation: - return True - except RequiresServerEvaluation: - # Immediately propagate - this condition requires server-side data - raise - except InconclusiveMatchError as e: - log.debug(f"Failed to compute property {prop} locally: {e}") - error_matching_locally = True - - if error_matching_locally: - raise InconclusiveMatchError( - "can't match cohort without a given cohort property value" - ) + effective_match = matches != bool(negation) + if is_and and not effective_match: + decisive_result = False + elif not is_and and effective_match: + decisive_result = True + except RequiresServerEvaluation: + # Static/missing cohorts and malformed definitions always require server evaluation, + # even when another branch would otherwise resolve the group locally. + raise + except InconclusiveMatchError as e: + log.debug(f"Failed to compute property {prop} locally: {e}") + error_matching_locally = True + + if decisive_result is not None: + return decisive_result + if error_matching_locally: + raise InconclusiveMatchError( + "Can't match cohort without a given cohort property value" + ) - # if we get here, all matched in AND case, or none matched in OR case - return property_group_type == "AND" + # AND: every entry matched. OR: none matched. + return is_and def parse_datetime(value: str) -> datetime.datetime: diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index c14c4188..29db9677 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -16,7 +16,10 @@ PROPERTY_OPERATORS, _UNHANDLED_OPERATOR_MESSAGE, InconclusiveMatchError, + RequiresServerEvaluation, + match_cohort, match_property, + match_property_group, parse_datetime, relative_date_parse_for_feature_flag_matching, ) @@ -37,6 +40,73 @@ ] +class TestCohortMatching(unittest.TestCase): + def setUp(self): + self.cohorts = { + "1": { + "type": "AND", + "values": [ + { + "key": "country", + "value": "US", + "operator": "exact", + "type": "person", + } + ], + } + } + + def test_cohort_membership_operators(self): + for operator in (None, "exact", "in"): + with self.subTest(operator=operator): + prop = {"value": 1, "operator": operator, "type": "cohort"} + self.assertTrue(match_cohort(prop, {"country": "US"}, self.cohorts)) + self.assertFalse(match_cohort(prop, {"country": "UK"}, self.cohorts)) + + not_in = {"value": 1, "operator": "not_in", "type": "cohort"} + self.assertFalse(match_cohort(not_in, {"country": "US"}, self.cohorts)) + self.assertTrue(match_cohort(not_in, {"country": "UK"}, self.cohorts)) + + def test_only_canonical_empty_groups_match(self): + self.assertTrue(match_property_group({}, {}, {})) + self.assertTrue(match_property_group({"type": "AND", "values": []}, {}, {})) + + malformed_groups = [ + None, + "invalid", + {"type": "AND"}, + {"type": "AND", "values": {}}, + ] + for group in malformed_groups: + with self.subTest(group=group): + with self.assertRaises(RequiresServerEvaluation): + match_property_group(group, {}, {}) + + def test_missing_nested_cohort_always_requires_server_evaluation(self): + matching_leaf = { + "key": "country", + "value": "US", + "operator": "exact", + "type": "person", + } + missing_cohort = {"key": "id", "value": 999, "type": "cohort"} + + cases = [ + ("OR", "US", [matching_leaf, missing_cohort]), + ("OR", "US", [missing_cohort, matching_leaf]), + ("AND", "UK", [matching_leaf, missing_cohort]), + ("AND", "UK", [missing_cohort, matching_leaf]), + ] + for group_type, country, values in cases: + with self.subTest(group_type=group_type, values=values): + with self.assertRaises(RequiresServerEvaluation): + match_property_group( + {"type": group_type, "values": values}, + {"country": country}, + {}, + ) + + class TestLocalEvaluation(unittest.TestCase): @classmethod def setUpClass(cls): From 4606cf943864b479c583e86bb001436f7752fe25 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 7 Aug 2026 09:36:26 +0200 Subject: [PATCH 2/3] fix(flags): reject invalid cohort group types --- posthog/feature_flags.py | 4 +++- posthog/test/test_feature_flags.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/posthog/feature_flags.py b/posthog/feature_flags.py index 7b4b09b7..ed56f952 100644 --- a/posthog/feature_flags.py +++ b/posthog/feature_flags.py @@ -769,7 +769,9 @@ def match_property_group( raise RequiresServerEvaluation("Cohort property group must be an object") property_group_type = property_group.get("type") - is_and = property_group_type != "OR" + if property_group_type not in ("AND", "OR"): + raise RequiresServerEvaluation("Cohort property group type must be AND or OR") + is_and = property_group_type == "AND" properties = property_group.get("values") if not isinstance(properties, list): raise RequiresServerEvaluation("Cohort property group values must be a list") diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index 29db9677..b2ce31e8 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -76,6 +76,8 @@ def test_only_canonical_empty_groups_match(self): "invalid", {"type": "AND"}, {"type": "AND", "values": {}}, + {"values": []}, + {"type": "INVALID", "values": []}, ] for group in malformed_groups: with self.subTest(group=group): From bfd1838713602f670bfe5156d65afe8e7572c7aa Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Fri, 7 Aug 2026 09:38:36 +0200 Subject: [PATCH 3/3] fix(flags): accept nested empty cohort groups --- posthog/feature_flags.py | 2 +- posthog/test/test_feature_flags.py | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/posthog/feature_flags.py b/posthog/feature_flags.py index ed56f952..26612f8d 100644 --- a/posthog/feature_flags.py +++ b/posthog/feature_flags.py @@ -788,7 +788,7 @@ def match_property_group( "Cohort property group entry must be an object" ) - if "values" in prop or prop.get("type") in ("AND", "OR"): + if prop == {} or "values" in prop or prop.get("type") in ("AND", "OR"): matches = match_property_group( prop, property_values, diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index b2ce31e8..6dd2b540 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -70,6 +70,13 @@ def test_cohort_membership_operators(self): def test_only_canonical_empty_groups_match(self): self.assertTrue(match_property_group({}, {}, {})) self.assertTrue(match_property_group({"type": "AND", "values": []}, {}, {})) + self.assertTrue( + match_property_group( + {"type": "AND", "values": [{"type": "AND", "values": []}, {}]}, + {}, + {}, + ) + ) malformed_groups = [ None,