fix(flags): parse locally-evaluated flag payloads in evaluate_flags() (sdk-specs local-feature-flag-evaluator) (#828)

fix(flags): parse locally-evaluated flag payloads in evaluate_flags()

The evaluate_flags() snapshot JSON-decoded payloads coming back from /flags
but stored locally-evaluated payloads verbatim, so get_flag_payload() returned
a dict for remotely-resolved flags and the raw JSON string for locally-resolved
ones. The sdk-specs local-feature-flag-evaluator and get-feature-flag-payload
contracts both expect the decoded value.

Both branches now go through a shared _parse_flag_payload() helper; strings that
aren't valid JSON are still passed through unchanged.

Generated-By: PostHog Code
Task-Id: 9b7661cd-0281-4019-b9ee-516e13639286

Co-authored-by: posthog[bot] <206114724+posthog[bot]@users.noreply.github.com>
Co-authored-by: Manoel Aranda Neto <marandaneto@gmail.com>

Author: posthog[bot]

.sampo/changesets/parse-locally-evaluated-flag-payloads.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+`evaluate_flags()` now JSON-decodes payloads for locally-evaluated flags, the same way it already did for flags resolved remotely. Previously `get_flag_payload()` returned a parsed value (`{"copy": "new"}`) when the flag came back from `/flags` but the raw JSON string (`'{"copy": "new"}'`) when the poller evaluated it locally, so the payload's type depended on where the flag happened to resolve. The `$feature_flag_payload` property on `$feature_flag_called` events is decoded for locally-evaluated flags too. Payload strings that aren't valid JSON are still passed through unchanged.

posthog/client.py

@@ -244,6 +244,20 @@ def _parse_has_experiment(value: Any) -> Optional[bool]:
     return value if isinstance(value, bool) else None
 
 
+def _parse_flag_payload(raw_payload: Any) -> Optional[Any]:
+    """Flag payloads are stored as JSON strings, both in the ``/flags`` response
+    metadata and in the local-evaluation flag definitions, so decode them before
+    handing them to callers. A string that isn't valid JSON is passed through as-is."""
+    if isinstance(raw_payload, str):
+        if not raw_payload:
+            return None
+        try:
+            return json.loads(raw_payload)
+        except (json.JSONDecodeError, TypeError):
+            return raw_payload
+    return raw_payload
+
+
 def _metadata_has_experiment(metadata: Any) -> Optional[bool]:
     """Server-reported experiment linkage from flag metadata; ``None`` when absent
     (e.g. ``LegacyFlagMetadata``, which doesn't carry the field)."""
@@ -3558,7 +3572,7 @@ def evaluate_flags(
                 key=key,
                 enabled=value is not False,
                 variant=value if isinstance(value, str) else None,
-                payload=local_payloads.get(key),
+                payload=_parse_flag_payload(local_payloads.get(key)),
                 id=flag_def.get("id"),
                 # The local-evaluation flag definition does not carry a version field;
                 # only the remote ``/flags`` response does via ``metadata.version``.
@@ -3597,19 +3611,11 @@ def evaluate_flags(
                 for key, detail in response.get("flags", {}).items():
                     if key in locally_evaluated_keys:
                         continue
-                    payload: Optional[Any] = None
-                    raw_payload = (
+                    payload = _parse_flag_payload(
                         detail.metadata.payload
                         if isinstance(detail.metadata, FlagMetadata)
                         else getattr(detail.metadata, "payload", None)
                     )
-                    if isinstance(raw_payload, str) and raw_payload:
-                        try:
-                            payload = json.loads(raw_payload)
-                        except (json.JSONDecodeError, TypeError):
-                            payload = raw_payload
-                    elif raw_payload is not None:
-                        payload = raw_payload
                     records[key] = _EvaluatedFlagRecord(
                         key=key,
                         enabled=detail.enabled,

posthog/test/test_evaluate_flags.py

@@ -271,6 +271,85 @@ def test_empty_distinct_id_returns_empty_snapshot_without_events(
         self.assertEqual(len(feature_flag_called), 0)
 
 
+class TestEvaluateFlagsLocalPayloads(unittest.TestCase):
+    """Locally-evaluated payloads must be decoded the same way remote ones are,
+    so a flag's payload type doesn't depend on where it happened to resolve."""
+
+    def setUp(self):
+        self.client = Client(FAKE_TEST_API_KEY, secret_key="test")
+        self.client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Checkout",
+                "key": "checkout",
+                "active": True,
+                "filters": {
+                    "groups": [{"properties": [], "rollout_percentage": 100}],
+                    "multivariate": {
+                        "variants": [{"key": "blue", "rollout_percentage": 100}]
+                    },
+                    "payloads": {"blue": '{"copy": "new"}'},
+                },
+            },
+            {
+                "id": 2,
+                "name": "Beta UI",
+                "key": "beta-ui",
+                "active": True,
+                "filters": {
+                    "groups": [{"properties": [], "rollout_percentage": 100}],
+                    "payloads": {"true": '{"color": "green"}'},
+                },
+            },
+            {
+                "id": 3,
+                "name": "Plain",
+                "key": "plain-payload",
+                "active": True,
+                "filters": {
+                    "groups": [{"properties": [], "rollout_percentage": 100}],
+                    "payloads": {"true": "not json"},
+                },
+            },
+        ]
+
+    def tearDown(self):
+        self.client.shutdown()
+
+    @mock.patch("posthog.client.flags")
+    def test_local_payloads_are_parsed(self, patch_flags):
+        flags = self.client.evaluate_flags("user-1")
+
+        self.assertEqual(flags.get_flag_payload("checkout"), {"copy": "new"})
+        self.assertEqual(flags.get_flag_payload("beta-ui"), {"color": "green"})
+        # /flags is not called because everything evaluated locally
+        self.assertEqual(patch_flags.call_count, 0)
+
+    @mock.patch("posthog.client.flags")
+    def test_non_json_local_payload_is_passed_through(self, patch_flags):
+        flags = self.client.evaluate_flags("user-1")
+
+        self.assertEqual(flags.get_flag_payload("plain-payload"), "not json")
+        self.assertEqual(patch_flags.call_count, 0)
+
+    @mock.patch("posthog.client.flags")
+    @mock.patch.object(Client, "capture")
+    def test_flag_called_event_carries_parsed_local_payload(
+        self, patch_capture, patch_flags
+    ):
+        flags = self.client.evaluate_flags("user-1")
+        flags.get_flag("checkout")
+
+        feature_flag_called = [
+            c
+            for c in patch_capture.call_args_list
+            if c[0] and c[0][0] == "$feature_flag_called"
+        ]
+        self.assertEqual(len(feature_flag_called), 1)
+        properties = feature_flag_called[0][1]["properties"]
+        self.assertEqual(properties["$feature_flag_payload"], {"copy": "new"})
+
+
 class TestEvaluateFlagsLocalDeviceBucketing(unittest.TestCase):
     def setUp(self):
         self.client = Client(FAKE_TEST_API_KEY)