Skip to content

Commit d0a4a0b

Browse files
rubychildsclaude
andcommitted
fix: honor filters.holdout in local flag evaluation
Local evaluation ignored the holdout on a flag's filters, so a user in an experiment holdout was bucketed into a regular variant instead of being excluded. The server resolves the holdout before release conditions and returns holdout-<id>; local evaluation now does the same. The existing _hash helper cannot be reused: it joins key and bucketing value with a dot, while the server hashes "holdout-<value>". Reusing it would look uniform and deterministic while selecting a different population. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent dd61434 commit d0a4a0b

3 files changed

Lines changed: 144 additions & 0 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
pypi/posthog: patch
3+
---
4+
5+
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‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,42 @@ def _hash(key: str, bucketing_value: str, salt: str = "") -> float:
119119
return hash_val / __LONG_SCALE__
120120

121121

122+
def _holdout_hash(bucketing_value: str) -> float:
123+
"""Hash a bucketing value for holdout membership, matching the server.
124+
125+
`_hash` can't be reused: it joins key and value with a dot, while the server hashes
126+
`holdout-<value>`. Reusing it would still look uniform and deterministic while holding
127+
out a different set of people than the server does.
128+
"""
129+
hash_key = f"holdout-{bucketing_value}"
130+
hash_val = int(hashlib.sha1(hash_key.encode("utf-8")).hexdigest()[:15], 16)
131+
return hash_val / __LONG_SCALE__
132+
133+
134+
def _get_holdout_variant(flag, bucketing_value) -> Optional[str]:
135+
"""The `holdout-<id>` variant this value is held out into, or None.
136+
137+
Mirrors the server's evaluation order: a held-out value never reaches the flag's
138+
release conditions, so callers check this before matching any condition.
139+
"""
140+
holdout = (flag.get("filters") or {}).get("holdout")
141+
if not holdout:
142+
return None
143+
144+
exclusion_percentage = holdout.get("exclusion_percentage")
145+
holdout_id = holdout.get("id")
146+
if exclusion_percentage is None or holdout_id is None:
147+
return None
148+
149+
# The server clamps out-of-range percentages rather than rejecting them, and treats
150+
# 100 as "everyone" without hashing, so a 100% holdout can't miss on a hash boundary.
151+
percentage = min(max(float(exclusion_percentage), 0.0), 100.0)
152+
if percentage != 100.0 and _holdout_hash(bucketing_value) > percentage / 100:
153+
return None
154+
155+
return f"holdout-{holdout_id}"
156+
157+
122158
def get_matching_variant(flag, bucketing_value):
123159
hash_value = _hash(flag["key"], bucketing_value, salt="variant")
124160
for variant in variant_lookup_table(flag):
@@ -364,6 +400,13 @@ def match_feature_flag_properties(
364400
bucketing_value = resolve_bucketing_value(flag, distinct_id, device_id)
365401

366402
flag_filters = flag.get("filters") or {}
403+
404+
# Holdouts are evaluated before release conditions, so a held-out value is excluded
405+
# from the flag's targeting entirely rather than being bucketed into a variant.
406+
holdout_variant = _get_holdout_variant(flag, bucketing_value)
407+
if holdout_variant is not None:
408+
return holdout_variant
409+
367410
flag_conditions = flag_filters.get("groups") or []
368411
flag_aggregation = flag_filters.get("aggregation_group_type_index")
369412
early_exit_enabled = flag_filters.get("early_exit")

‎posthog/test/test_feature_flags.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import datetime
2+
import hashlib
23
import threading
34
import unittest
45

@@ -462,6 +463,101 @@ def test_early_exit_does_not_trigger_on_property_mismatch(self):
462463
)
463464
)
464465

466+
def _holdout_flag(self, exclusion_percentage, rollout_percentage=100):
467+
return [
468+
{
469+
"id": 1,
470+
"name": "Experiment Flag",
471+
"key": "experiment-flag",
472+
"active": True,
473+
"filters": {
474+
"multivariate": {
475+
"variants": [
476+
{"key": "control", "rollout_percentage": 50},
477+
{"key": "test", "rollout_percentage": 50},
478+
]
479+
},
480+
"groups": [
481+
{"properties": [], "rollout_percentage": rollout_percentage}
482+
],
483+
"holdout": {
484+
"id": 727,
485+
"exclusion_percentage": exclusion_percentage,
486+
},
487+
},
488+
}
489+
]
490+
491+
def test_holdout_at_100_percent_excludes_every_distinct_id(self):
492+
self.client.feature_flags = self._holdout_flag(100)
493+
494+
for distinct_id in ["user_1", "user_2", "user_3", "user_4", "user_5"]:
495+
self.assertEqual(
496+
self.client.get_feature_flag(
497+
"experiment-flag", distinct_id, only_evaluate_locally=True
498+
),
499+
"holdout-727",
500+
)
501+
502+
def test_holdout_at_0_percent_excludes_nobody(self):
503+
self.client.feature_flags = self._holdout_flag(0)
504+
505+
for distinct_id in ["user_1", "user_2", "user_3", "user_4", "user_5"]:
506+
self.assertIn(
507+
self.client.get_feature_flag(
508+
"experiment-flag", distinct_id, only_evaluate_locally=True
509+
),
510+
["control", "test"],
511+
)
512+
513+
def test_holdout_is_evaluated_before_release_conditions(self):
514+
# The flag releases to nobody, but a held-out user is excluded before targeting
515+
# is consulted, so they still get the holdout variant rather than False.
516+
self.client.feature_flags = self._holdout_flag(100, rollout_percentage=0)
517+
518+
self.assertEqual(
519+
self.client.get_feature_flag(
520+
"experiment-flag", "user_1", only_evaluate_locally=True
521+
),
522+
"holdout-727",
523+
)
524+
525+
def test_holdout_membership_matches_server_bucketing(self):
526+
# The server hashes "holdout-<distinct_id>". Pinning the exact membership set
527+
# guards the string construction: reusing the flag hash helper, which joins with
528+
# a dot, still looks uniform and deterministic but holds out different people.
529+
distinct_ids = [f"user_{n}" for n in range(1, 21)]
530+
exclusion_percentage = 20
531+
532+
def server_hash(prefix, distinct_id):
533+
digest = hashlib.sha1(f"{prefix}{distinct_id}".encode("utf-8")).hexdigest()
534+
return int(digest[:15], 16) / float(0xFFFFFFFFFFFFFFF)
535+
536+
expected = {
537+
distinct_id
538+
for distinct_id in distinct_ids
539+
if server_hash("holdout-", distinct_id) <= exclusion_percentage / 100
540+
}
541+
dot_joined = {
542+
distinct_id
543+
for distinct_id in distinct_ids
544+
if server_hash("holdout.", distinct_id) <= exclusion_percentage / 100
545+
}
546+
# Guard the guard: if these ever coincide the test would pass with the bug present.
547+
self.assertNotEqual(expected, dot_joined)
548+
549+
self.client.feature_flags = self._holdout_flag(exclusion_percentage)
550+
held_out = {
551+
distinct_id
552+
for distinct_id in distinct_ids
553+
if self.client.get_feature_flag(
554+
"experiment-flag", distinct_id, only_evaluate_locally=True
555+
)
556+
== "holdout-727"
557+
}
558+
559+
self.assertEqual(held_out, expected)
560+
465561
def test_early_exit_on_multivariate_flag(self):
466562
self.client.feature_flags = [
467563
{

0 commit comments

Comments
 (0)