fix: honor filters.holdout in local flag evaluation

.sampo/changesets/local-eval-holdouts.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+Honor `filters.holdout` during local feature flag evaluation. A user in an experiment holdout now receives the `holdout-<id>` variant instead of being bucketed into a regular variant, matching how the server evaluates the same flag. Holdout membership is resolved before release conditions, so a held-out user never reaches the flag's targeting.

posthog/feature_flags.py

@@ -113,12 +113,48 @@ class RequiresServerEvaluation(Exception):
 # Given the same bucketing value and key, it'll always return the same float. These floats are
 # uniformly distributed between 0 and 1, so if we want to show this feature to 20% of traffic
 # we can do _hash(key, bucketing_value) < 0.2
-def _hash(key: str, bucketing_value: str, salt: str = "") -> float:
-    hash_key = f"{key}.{bucketing_value}{salt}"
+def _hash(
+    key: str, bucketing_value: str, salt: str = "", separator: str = "."
+) -> float:
+    hash_key = f"{key}{separator}{bucketing_value}{salt}"
     hash_val = int(hashlib.sha1(hash_key.encode("utf-8")).hexdigest()[:15], 16)
     return hash_val / __LONG_SCALE__
 
 
+def _holdout_hash(bucketing_value: str) -> float:
+    """Hash a bucketing value for holdout membership, matching the server.
+
+    The separator is the whole point: the server hashes `holdout-<value>`, while flag
+    rollout hashing joins with a dot. Taking the default separator here would still look
+    uniform and deterministic while holding out a different set of people than the server.
+    """
+    return _hash("holdout", bucketing_value, separator="-")
+
+
+def _get_holdout_variant(flag, bucketing_value) -> Optional[str]:
+    """The `holdout-<id>` variant this value is held out into, or None.
+
+    Mirrors the server's evaluation order: a held-out value never reaches the flag's
+    release conditions, so callers check this before matching any condition.
+    """
+    holdout = (flag.get("filters") or {}).get("holdout")
+    if not holdout:
+        return None
+
+    exclusion_percentage = holdout.get("exclusion_percentage")
+    holdout_id = holdout.get("id")
+    if exclusion_percentage is None or holdout_id is None:
+        return None
+
+    # The server clamps out-of-range percentages rather than rejecting them, and treats
+    # 100 as "everyone" without hashing, so a 100% holdout can't miss on a hash boundary.
+    percentage = min(max(float(exclusion_percentage), 0.0), 100.0)
+    if percentage != 100.0 and _holdout_hash(bucketing_value) > percentage / 100:
+        return None
+
+    return f"holdout-{holdout_id}"
+
+
 def get_matching_variant(flag, bucketing_value):
     hash_value = _hash(flag["key"], bucketing_value, salt="variant")
     for variant in variant_lookup_table(flag):
@@ -364,6 +400,13 @@ def match_feature_flag_properties(
         bucketing_value = resolve_bucketing_value(flag, distinct_id, device_id)
 
     flag_filters = flag.get("filters") or {}
+
+    # Holdouts are evaluated before release conditions, so a held-out value is excluded
+    # from the flag's targeting entirely rather than being bucketed into a variant.
+    holdout_variant = _get_holdout_variant(flag, bucketing_value)
+    if holdout_variant is not None:
+        return holdout_variant
+
     flag_conditions = flag_filters.get("groups") or []
     flag_aggregation = flag_filters.get("aggregation_group_type_index")
     early_exit_enabled = flag_filters.get("early_exit")

posthog/test/test_feature_flags.py

@@ -1,4 +1,5 @@
 import datetime
+import hashlib
 import threading
 import unittest
 
@@ -462,6 +463,101 @@ def test_early_exit_does_not_trigger_on_property_mismatch(self):
             )
         )
 
+    def _holdout_flag(self, exclusion_percentage, rollout_percentage=100):
+        return [
+            {
+                "id": 1,
+                "name": "Experiment Flag",
+                "key": "experiment-flag",
+                "active": True,
+                "filters": {
+                    "multivariate": {
+                        "variants": [
+                            {"key": "control", "rollout_percentage": 50},
+                            {"key": "test", "rollout_percentage": 50},
+                        ]
+                    },
+                    "groups": [
+                        {"properties": [], "rollout_percentage": rollout_percentage}
+                    ],
+                    "holdout": {
+                        "id": 727,
+                        "exclusion_percentage": exclusion_percentage,
+                    },
+                },
+            }
+        ]
+
+    def test_holdout_at_100_percent_excludes_every_distinct_id(self):
+        self.client.feature_flags = self._holdout_flag(100)
+
+        for distinct_id in ["user_1", "user_2", "user_3", "user_4", "user_5"]:
+            self.assertEqual(
+                self.client.get_feature_flag(
+                    "experiment-flag", distinct_id, only_evaluate_locally=True
+                ),
+                "holdout-727",
+            )
+
+    def test_holdout_at_0_percent_excludes_nobody(self):
+        self.client.feature_flags = self._holdout_flag(0)
+
+        for distinct_id in ["user_1", "user_2", "user_3", "user_4", "user_5"]:
+            self.assertIn(
+                self.client.get_feature_flag(
+                    "experiment-flag", distinct_id, only_evaluate_locally=True
+                ),
+                ["control", "test"],
+            )
+
+    def test_holdout_is_evaluated_before_release_conditions(self):
+        # The flag releases to nobody, but a held-out user is excluded before targeting
+        # is consulted, so they still get the holdout variant rather than False.
+        self.client.feature_flags = self._holdout_flag(100, rollout_percentage=0)
+
+        self.assertEqual(
+            self.client.get_feature_flag(
+                "experiment-flag", "user_1", only_evaluate_locally=True
+            ),
+            "holdout-727",
+        )
+
+    def test_holdout_membership_matches_server_bucketing(self):
+        # The server hashes "holdout-<distinct_id>". Pinning the exact membership set
+        # guards the string construction: reusing the flag hash helper, which joins with
+        # a dot, still looks uniform and deterministic but holds out different people.
+        distinct_ids = [f"user_{n}" for n in range(1, 21)]
+        exclusion_percentage = 20
+
+        def server_hash(prefix, distinct_id):
+            digest = hashlib.sha1(f"{prefix}{distinct_id}".encode("utf-8")).hexdigest()
+            return int(digest[:15], 16) / float(0xFFFFFFFFFFFFFFF)
+
+        expected = {
+            distinct_id
+            for distinct_id in distinct_ids
+            if server_hash("holdout-", distinct_id) <= exclusion_percentage / 100
+        }
+        dot_joined = {
+            distinct_id
+            for distinct_id in distinct_ids
+            if server_hash("holdout.", distinct_id) <= exclusion_percentage / 100
+        }
+        # Guard the guard: if these ever coincide the test would pass with the bug present.
+        self.assertNotEqual(expected, dot_joined)
+
+        self.client.feature_flags = self._holdout_flag(exclusion_percentage)
+        held_out = {
+            distinct_id
+            for distinct_id in distinct_ids
+            if self.client.get_feature_flag(
+                "experiment-flag", distinct_id, only_evaluate_locally=True
+            )
+            == "holdout-727"
+        }
+
+        self.assertEqual(held_out, expected)
+
     def test_early_exit_on_multivariate_flag(self):
         self.client.feature_flags = [
             {