fix(flags): malformed flag-dependency conditions no-match locally

.sampo/changesets/malformed-flag-dependency-no-match.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Malformed flag-dependency conditions (missing key, null value, or wrong operator) now evaluate locally as no-match (false), matching the server, instead of falling back to the `/flags` endpoint on every evaluation. 7.22.1 made these conditions fall back to the server, which could massively increase billable `/flags` request volume for flag definitions containing legacy/malformed dependency conditions.

posthog/feature_flags.py

@@ -15,9 +15,34 @@
 
 log = logging.getLogger("posthog")
 
+# Tracks (flag_key, reason) pairs already warned about for malformed flag
+# dependency conditions, so the warning fires at most once per process instead
+# of on every get_feature_flag call. A plain set is safe under the GIL; a rare
+# duplicate warning from a race is acceptable.
+_warned_malformed_flag_dependencies: set = set()
+
 NONE_VALUES_ALLOWED_OPERATORS = ["is_not"]
 
 
+def _warn_malformed_flag_dependency_once(flag_key, reason):
+    """Emit a throttled warning for a malformed flag dependency condition.
+
+    Mirrors the server-side Rust evaluator, which logs before treating the
+    condition as not matching, so customers can discover broken flag
+    definitions locally. Deduplicated on (flag_key, reason) to avoid spamming
+    hot evaluation paths.
+    """
+    key = (flag_key, reason)
+    if key in _warned_malformed_flag_dependencies:
+        return
+    _warned_malformed_flag_dependencies.add(key)
+    log.warning(
+        f"Flag dependency condition on '{flag_key or 'unknown'}' is malformed "
+        f"({reason}); treating as not matching during local evaluation. "
+        f"Fix this condition in the PostHog UI."
+    )
+
+
 class ConditionMatch(Enum):
     """Outcome of evaluating a single condition group.
 
@@ -134,18 +159,38 @@ def evaluate_flag_dependency(
 
     Returns:
         bool: Whether the referenced flag's evaluated value matches the
-            condition's expected value.
+            condition's expected value. A malformed condition shape (wrong
+            operator, missing key, or missing value) is a definitive local
+            no-match and returns False, mirroring the server-side evaluator.
 
     Raises:
-        InconclusiveMatchError: If the condition is malformed or the chain
-            cannot be conclusively evaluated locally.
+        InconclusiveMatchError: If the chain cannot be conclusively evaluated
+            locally (referenced flag missing, an inconclusive dependency,
+            circular dependency, or missing evaluation context).
     """
     if flags_by_key is None or evaluation_cache is None:
         # Cannot evaluate flag dependencies without required context
         raise InconclusiveMatchError(
             f"Cannot evaluate flag dependency on '{property.get('key', 'unknown')}' without flags_by_key and evaluation_cache"
         )
 
+    # Validate the condition shape before walking the chain. A malformed shape
+    # is a definitive no-match (return False), mirroring the server-side Rust
+    # evaluator (match_flag_value_to_flag_filter). Raising InconclusiveMatchError
+    # here would force a billable /flags network fallback on every evaluation of
+    # the affected flag. Test `is None`, not truthiness: `False` is a valid
+    # expected value (the case flag dependencies exist for).
+    flag_key = property.get("key")
+    expected_value = property.get("value")
+    operator = property.get("operator", "exact")
+
+    if operator != "flag_evaluates_to":
+        _warn_malformed_flag_dependency_once(flag_key, f"invalid operator '{operator}'")
+        return False
+    if not flag_key or expected_value is None:
+        _warn_malformed_flag_dependency_once(flag_key, "missing key or value")
+        return False
+
     # Check if dependency_chain is present - it should always be provided for flag dependencies
     if "dependency_chain" not in property:
         # Missing dependency_chain indicates malformed server data
@@ -162,21 +207,6 @@ def evaluate_flag_dependency(
             f"Circular dependency detected for flag '{property.get('key', 'unknown')}'"
         )
 
-    # Validate the condition shape before walking the chain. Test `is None`, not
-    # truthiness: `False` is a valid expected value (the case this exists for).
-    flag_key = property.get("key")
-    expected_value = property.get("value")
-    operator = property.get("operator", "exact")
-
-    if operator != "flag_evaluates_to":
-        raise InconclusiveMatchError(
-            f"Flag dependency property for '{flag_key or 'unknown'}' has invalid operator '{operator}'"
-        )
-    if not flag_key or expected_value is None:
-        raise InconclusiveMatchError(
-            f"Flag dependency property for '{flag_key or 'unknown'}' is missing a key or value"
-        )
-
     # Evaluate and cache each flag in the chain; members already cached are
     # skipped. This does not decide the outcome — it only populates the cache.
     for dep_flag_key in dependency_chain:

posthog/test/test_feature_flags.py

@@ -9,6 +9,7 @@
 from parameterized import parameterized
 
 from posthog.client import Client
+import posthog.feature_flags
 from posthog.feature_flags import (
     InconclusiveMatchError,
     match_property,
@@ -38,6 +39,9 @@ def set_fail(self, e, batch):
     def setUp(self):
         self.failed = False
         self.client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail)
+        # Reset the process-wide malformed-dependency warning dedup set so tests
+        # asserting on the warning stay order-independent.
+        posthog.feature_flags._warned_malformed_flag_dependencies.clear()
 
     @mock.patch("posthog.client.get")
     def test_flag_person_properties(self, patch_get):
@@ -2310,8 +2314,10 @@ def test_flag_dependencies_inconclusive_base_falls_back(
 
     @mock.patch("posthog.client.flags")
     @mock.patch("posthog.client.get")
-    def test_flag_dependencies_null_value_falls_back(self, patch_get, patch_flags):
-        """A malformed `value: null` condition is inconclusive and falls back to /flags rather than silently matching."""
+    def test_flag_dependencies_null_value_no_match_locally(
+        self, patch_get, patch_flags
+    ):
+        """A malformed `value: null` condition is a definitive local no-match (False), never falling back to /flags."""
         patch_flags.return_value = {"featureFlags": {}}
         client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
         client.feature_flags = [
@@ -2346,9 +2352,228 @@ def test_flag_dependencies_null_value_falls_back(self, patch_get, patch_flags):
             },
         ]
 
+        # The malformed condition does not match, so the only group fails and the
+        # flag resolves to False locally - no billable /flags fallback.
         result = client.get_feature_flag("dependent-flag", "user-1")
-        self.assertIsNone(result)
-        self.assertEqual(patch_flags.call_count, 1)
+        self.assertFalse(result)
+        self.assertEqual(patch_flags.call_count, 0)
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch("posthog.client.get")
+    def test_flag_dependencies_missing_key_no_match_locally(
+        self, patch_get, patch_flags
+    ):
+        """A dependency condition with an empty/missing `key` is a definitive local no-match (False)."""
+        patch_flags.return_value = {"featureFlags": {}}
+        client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
+        client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Base Flag",
+                "key": "base-flag",
+                "active": True,
+                "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]},
+            },
+            {
+                "id": 2,
+                "name": "Dependent Flag",
+                "key": "dependent-flag",
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "properties": [
+                                {
+                                    "key": "",
+                                    "operator": "flag_evaluates_to",
+                                    "value": True,
+                                    "type": "flag",
+                                    "dependency_chain": ["base-flag"],
+                                }
+                            ],
+                            "rollout_percentage": 100,
+                        }
+                    ],
+                },
+            },
+        ]
+
+        result = client.get_feature_flag("dependent-flag", "user-1")
+        self.assertFalse(result)
+        self.assertEqual(patch_flags.call_count, 0)
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch("posthog.client.get")
+    def test_flag_dependencies_wrong_operator_no_match_locally(
+        self, patch_get, patch_flags
+    ):
+        """A dependency condition with the wrong operator is a definitive local no-match (False)."""
+        patch_flags.return_value = {"featureFlags": {}}
+        client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
+        client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Base Flag",
+                "key": "base-flag",
+                "active": True,
+                "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]},
+            },
+            {
+                "id": 2,
+                "name": "Dependent Flag",
+                "key": "dependent-flag",
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "properties": [
+                                {
+                                    "key": "base-flag",
+                                    "operator": "exact",
+                                    "value": True,
+                                    "type": "flag",
+                                    "dependency_chain": ["base-flag"],
+                                }
+                            ],
+                            "rollout_percentage": 100,
+                        }
+                    ],
+                },
+            },
+        ]
+
+        result = client.get_feature_flag("dependent-flag", "user-1")
+        self.assertFalse(result)
+        self.assertEqual(patch_flags.call_count, 0)
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch("posthog.client.get")
+    def test_flag_dependencies_malformed_condition_warns_once(
+        self, patch_get, patch_flags
+    ):
+        """A malformed dependency condition warns on first eval, then stays quiet (dedup)."""
+        patch_flags.return_value = {"featureFlags": {}}
+        client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
+        client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Base Flag",
+                "key": "base-flag",
+                "active": True,
+                "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]},
+            },
+            {
+                "id": 2,
+                "name": "Dependent Flag",
+                "key": "dependent-flag",
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "properties": [
+                                {
+                                    "key": "base-flag",
+                                    "operator": "flag_evaluates_to",
+                                    "value": None,
+                                    "type": "flag",
+                                    "dependency_chain": ["base-flag"],
+                                }
+                            ],
+                            "rollout_percentage": 100,
+                        }
+                    ],
+                },
+            },
+        ]
+
+        # First evaluation emits exactly one warning naming the malformed flag.
+        with self.assertLogs("posthog", level="WARNING") as logs:
+            self.assertFalse(client.get_feature_flag("dependent-flag", "user-1"))
+        self.assertEqual(len(logs.records), 1)
+        self.assertIn("base-flag", logs.output[0])
+        self.assertIn("malformed", logs.output[0])
+
+        # Second evaluation of the same malformed condition stays silent (dedup).
+        # assertLogs fails if nothing is logged, so log a sentinel to prove the
+        # malformed-dependency warning itself did not fire again.
+        with self.assertLogs("posthog", level="WARNING") as logs:
+            self.assertFalse(client.get_feature_flag("dependent-flag", "user-2"))
+            posthog.feature_flags.log.warning("sentinel")
+        self.assertEqual(len(logs.records), 1)
+        self.assertIn("sentinel", logs.output[0])
+
+        self.assertEqual(patch_flags.call_count, 0)
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch("posthog.client.get")
+    def test_flag_dependencies_malformed_condition_falls_through_to_other_group(
+        self, patch_get, patch_flags
+    ):
+        """A malformed dependency condition no-matches its group, but a second matching group still resolves the flag locally."""
+        patch_flags.return_value = {"featureFlags": {}}
+        client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
+        client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Base Flag",
+                "key": "base-flag",
+                "active": True,
+                "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]},
+            },
+            # Only condition is the malformed dependency -> resolves to False locally.
+            {
+                "id": 2,
+                "name": "Malformed Only Flag",
+                "key": "malformed-only-flag",
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "properties": [
+                                {
+                                    "key": "base-flag",
+                                    "operator": "flag_evaluates_to",
+                                    "value": None,
+                                    "type": "flag",
+                                    "dependency_chain": ["base-flag"],
+                                }
+                            ],
+                            "rollout_percentage": 100,
+                        }
+                    ],
+                },
+            },
+            # Malformed dependency group no-matches, but the second group matches.
+            {
+                "id": 3,
+                "name": "Malformed Plus Matching Flag",
+                "key": "malformed-plus-matching-flag",
+                "active": True,
+                "filters": {
+                    "groups": [
+                        {
+                            "properties": [
+                                {
+                                    "key": "base-flag",
+                                    "operator": "flag_evaluates_to",
+                                    "value": None,
+                                    "type": "flag",
+                                    "dependency_chain": ["base-flag"],
+                                }
+                            ],
+                            "rollout_percentage": 100,
+                        },
+                        {"properties": [], "rollout_percentage": 100},
+                    ],
+                },
+            },
+        ]
+
+        self.assertFalse(client.get_feature_flag("malformed-only-flag", "user-1"))
+        self.assertTrue(
+            client.get_feature_flag("malformed-plus-matching-flag", "user-1")
+        )
+        self.assertEqual(patch_flags.call_count, 0)
 
     @mock.patch("posthog.client.flags")
     @mock.patch("posthog.client.get")
@@ -5906,6 +6131,9 @@ def set_fail(self, e, batch):
     def setUp(self):
         self.failed = False
         self.client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail)
+        # Reset the process-wide malformed-dependency warning dedup set so tests
+        # asserting on the warning stay order-independent.
+        posthog.feature_flags._warned_malformed_flag_dependencies.clear()
 
     @mock.patch("posthog.client.get")
     def test_simple_flag_consistency(self, patch_get):