fix(flags): fall back for missing local definitions (#878)

* fix(flags): fall back for missing local definitions

* docs(flags): document missing-key fallback cost

* docs(flags): clarify empty flag key scope

Author: marandaneto

.sampo/changesets/gallant-princess-kalma.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: minor
+---
+
+Fall back to remote evaluation when a requested flag is missing from local definitions. This changes the previous behavior where the key was omitted without a request.

posthog/__init__.py

@@ -1074,12 +1074,15 @@ def evaluate_flags(
         groups: Mapping of group type to group key.
         person_properties: Person properties to use for evaluation.
         group_properties: Group properties keyed by group type.
-        only_evaluate_locally: If ``True``, never fall back to remote evaluation.
+        only_evaluate_locally: If ``True``, never fall back to remote evaluation and
+            omit flags that cannot be evaluated locally.
         disable_geoip: Whether to disable GeoIP lookup.
-        flag_keys: Optional list of flag keys. When provided, only these flags are
-            evaluated — the underlying ``/flags`` request asks the server for just
-            this subset, which makes the response smaller and the request cheaper.
-            Use this when you only need a handful of flags out of many.
+        flag_keys: Optional non-empty list that scopes local evaluation, the underlying
+            ``/flags`` request, and the returned snapshot. An empty list is treated like ``None``
+            and evaluates all flags. A requested key absent from loaded local definitions is
+            included in one remote fallback per ``evaluate_flags`` call unless
+            ``only_evaluate_locally`` is ``True``. If the server also does not know the key, it is
+            omitted from the snapshot.
         device_id: Optional device ID override. If not provided, falls back to the
             context device_id (which may be set via tracing headers). Used by
             experience-continuity flags to match users across distinct_id changes.

posthog/client.py

@@ -4044,8 +4044,12 @@ def evaluate_flags(
             only_evaluate_locally: If True, never fall back to remote evaluation —
                 flags that can't be evaluated locally are simply omitted from the snapshot.
             disable_geoip: Whether to disable GeoIP lookup.
-            flag_keys: Optional list of flag keys to scope the underlying ``/flags``
-                request to a subset.
+            flag_keys: Optional non-empty list that scopes local evaluation, the underlying
+                ``/flags`` request, and the returned snapshot. An empty list is treated like
+                ``None`` and evaluates all flags. A requested key absent from loaded local
+                definitions is included in one remote fallback per ``evaluate_flags`` call unless
+                ``only_evaluate_locally`` is True. If the server also does not know the key, it is
+                omitted from the snapshot.
             device_id: Optional device ID override. If not provided, falls back to the
                 context device_id (which may be set via tracing headers). Used by
                 experience-continuity flags to match users across distinct_id changes.
@@ -4092,6 +4096,8 @@ def evaluate_flags(
             )
         )
         groups = groups or {}
+        # Keep the existing API convention that an empty list means no scope.
+        requested_keys = set(flag_keys) if flag_keys else None
 
         records: Dict[str, _EvaluatedFlagRecord] = {}
         request_id: Optional[str] = None
@@ -4121,6 +4127,11 @@ def evaluate_flags(
         feature_flags_by_key: Dict[str, Any] = self.feature_flags_by_key or {}
         local_flags = local_result.get("featureFlags") or {}
         local_payloads = local_result.get("featureFlagPayloads") or {}
+        if requested_keys and not requested_keys.issubset(local_flags):
+            # A requested flag may have been created since the last definitions poll.
+            # Ask /flags for the caller's original scope unless this is a local-only call.
+            fallback_to_server = True
+
         for key, value in local_flags.items():
             flag_def = feature_flags_by_key.get(key) or {}
             records[key] = _EvaluatedFlagRecord(
@@ -4164,6 +4175,8 @@ def evaluate_flags(
                     response.get("minimalFlagCalledEvents") is True
                 )
                 for key, detail in response.get("flags", {}).items():
+                    if requested_keys is not None and key not in requested_keys:
+                        continue
                     if key in locally_evaluated_keys:
                         continue
                     payload = _parse_flag_payload(

posthog/test/test_evaluate_flags.py

@@ -395,6 +395,81 @@ def test_flag_called_event_carries_parsed_local_payload(
         self.assertEqual(properties["$feature_flag_payload"], {"copy": "new"})
 
 
+class TestEvaluateFlagsMissingLocalDefinition(unittest.TestCase):
+    def setUp(self):
+        self.client = Client(FAKE_TEST_API_KEY, secret_key="test")
+        self.client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Local flag",
+                "key": "local-flag",
+                "active": True,
+                "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]},
+            }
+        ]
+
+    def tearDown(self):
+        self.client.shutdown()
+
+    @staticmethod
+    def _remote_response():
+        return {
+            "flags": {
+                "local-flag": {"enabled": False, "variant": None},
+                "remote-only": {"enabled": True, "variant": None},
+                "unrequested": {"enabled": True, "variant": None},
+            }
+        }
+
+    @mock.patch("posthog.client.flags")
+    def test_missing_requested_key_triggers_one_scoped_fallback(self, patch_flags):
+        patch_flags.return_value = self._remote_response()
+        requested_keys = ["local-flag", "remote-only"]
+
+        flags = self.client.evaluate_flags("user-1", flag_keys=requested_keys)
+
+        self.assertEqual(set(flags.keys), set(requested_keys))
+        self.assertTrue(flags.get_flag("local-flag"))
+        self.assertTrue(flags.get_flag("remote-only"))
+        patch_flags.assert_called_once()
+        self.assertEqual(
+            patch_flags.call_args.kwargs["flag_keys_to_evaluate"], requested_keys
+        )
+
+    @mock.patch("posthog.client.flags")
+    def test_missing_requested_key_is_omitted_for_local_only_evaluation(
+        self, patch_flags
+    ):
+        patch_flags.return_value = self._remote_response()
+
+        flags = self.client.evaluate_flags(
+            "user-1",
+            flag_keys=["local-flag", "remote-only"],
+            only_evaluate_locally=True,
+        )
+
+        self.assertEqual(flags.keys, ["local-flag"])
+        self.assertTrue(flags.get_flag("local-flag"))
+        self.assertIsNone(flags.get_flag("remote-only"))
+        patch_flags.assert_not_called()
+
+    @mock.patch("posthog.client.flags")
+    def test_server_missing_key_falls_back_once_per_evaluation_call(self, patch_flags):
+        patch_flags.return_value = {"flags": {}}
+        requested_keys = ["local-flag", "typo-flag"]
+
+        first = self.client.evaluate_flags("user-1", flag_keys=requested_keys)
+        second = self.client.evaluate_flags("user-1", flag_keys=requested_keys)
+
+        for flags in (first, second):
+            self.assertEqual(flags.keys, ["local-flag"])
+            self.assertTrue(flags.get_flag("local-flag"))
+            self.assertIsNone(flags.get_flag("typo-flag"))
+        self.assertEqual(patch_flags.call_count, 2)
+        for call in patch_flags.call_args_list:
+            self.assertEqual(call.kwargs["flag_keys_to_evaluate"], requested_keys)
+
+
 class TestEvaluateFlagsLocalDeviceBucketing(unittest.TestCase):
     def setUp(self):
         self.client = Client(FAKE_TEST_API_KEY)