Skip to content

Commit 68c7b39

Browse files
committed
Fix multi-condition flags with static cohorts returning wrong variants
When a feature flag has multiple conditions and one contains a static cohort, the SDK now correctly falls back to the API instead of evaluating subsequent conditions locally and returning incorrect variants. Introduce RequiresServerEvaluation exception to distinguish between: - Missing server-side data (static cohorts) → immediate API fallback - Evaluation errors (bad regex, missing properties) → try next condition Changes: - Add RequiresServerEvaluation exception class - Update match_cohort() to throw RequiresServerEvaluation for static cohorts - Update match_property_group() to propagate RequiresServerEvaluation - Update match_feature_flag_properties() to handle both exception types - Update client.py to catch both exceptions for API fallback - Export RequiresServerEvaluation in __init__.py - Add test for multi-condition static cohort scenario All 84 feature flag tests pass.
1 parent e06830e commit 68c7b39

File tree

4 files changed

+101
-4
lines changed

4 files changed

+101
-4
lines changed

posthog/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
set_context_session as inner_set_context_session,
1212
identify_context as inner_identify_context,
1313
)
14+
from posthog.feature_flags import InconclusiveMatchError, RequiresServerEvaluation
1415
from posthog.types import FeatureFlag, FlagsAndPayloads, FeatureFlagResult
1516
from posthog.version import VERSION
1617

posthog/client.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,11 @@
2020
exception_is_already_captured,
2121
mark_exception_as_captured,
2222
)
23-
from posthog.feature_flags import InconclusiveMatchError, match_feature_flag_properties
23+
from posthog.feature_flags import (
24+
InconclusiveMatchError,
25+
RequiresServerEvaluation,
26+
match_feature_flag_properties,
27+
)
2428
from posthog.poller import Poller
2529
from posthog.request import (
2630
DEFAULT_HOST,
@@ -1583,7 +1587,7 @@ def _locally_evaluate_flag(
15831587
self.log.debug(
15841588
f"Successfully computed flag locally: {key} -> {response}"
15851589
)
1586-
except InconclusiveMatchError as e:
1590+
except (RequiresServerEvaluation, InconclusiveMatchError) as e:
15871591
self.log.debug(f"Failed to compute flag {key} locally: {e}")
15881592
except Exception as e:
15891593
self.log.exception(

posthog/feature_flags.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ class InconclusiveMatchError(Exception):
2222
pass
2323

2424

25+
class RequiresServerEvaluation(Exception):
26+
"""
27+
Raised when feature flag evaluation requires server-side data that is not
28+
available locally (e.g., static cohorts, experience continuity).
29+
30+
This error should propagate immediately to trigger API fallback, unlike
31+
InconclusiveMatchError which allows trying other conditions.
32+
"""
33+
34+
pass
35+
36+
2537
# This function takes a distinct_id and a feature flag key and returns a float between 0 and 1.
2638
# Given the same distinct_id and key, it'll always return the same float. These floats are
2739
# uniformly distributed between 0 and 1, so if we want to show this feature to 20% of traffic
@@ -239,7 +251,12 @@ def match_feature_flag_properties(
239251
else:
240252
variant = get_matching_variant(flag, distinct_id)
241253
return variant or True
254+
except RequiresServerEvaluation:
255+
# Static cohort or other missing server-side data - must fallback to API
256+
raise
242257
except InconclusiveMatchError:
258+
# Evaluation error (bad regex, invalid date, missing property, etc.)
259+
# Track that we had an inconclusive match, but try other conditions
243260
is_inconclusive = True
244261

245262
if is_inconclusive:
@@ -449,8 +466,8 @@ def match_cohort(
449466
# }
450467
cohort_id = str(property.get("value"))
451468
if cohort_id not in cohort_properties:
452-
raise InconclusiveMatchError(
453-
"can't match cohort without a given cohort property value"
469+
raise RequiresServerEvaluation(
470+
f"cohort {cohort_id} not found in local cohorts - likely a static cohort that requires server evaluation"
454471
)
455472

456473
property_group = cohort_properties[cohort_id]
@@ -503,6 +520,9 @@ def match_property_group(
503520
# OR group
504521
if matches:
505522
return True
523+
except RequiresServerEvaluation:
524+
# Immediately propagate - this condition requires server-side data
525+
raise
506526
except InconclusiveMatchError as e:
507527
log.debug(f"Failed to compute property {prop} locally: {e}")
508528
error_matching_locally = True
@@ -552,6 +572,9 @@ def match_property_group(
552572
return True
553573
if not matches and negation:
554574
return True
575+
except RequiresServerEvaluation:
576+
# Immediately propagate - this condition requires server-side data
577+
raise
555578
except InconclusiveMatchError as e:
556579
log.debug(f"Failed to compute property {prop} locally: {e}")
557580
error_matching_locally = True

posthog/test/test_feature_flags.py

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3013,6 +3013,75 @@ def test_multivariate_feature_flag_payloads(self, patch_flags):
30133013
)
30143014
self.assertEqual(patch_flags.call_count, 0)
30153015

3016+
@mock.patch("posthog.client.flags")
3017+
@mock.patch("posthog.client.get")
3018+
def test_fallback_to_api_when_flag_has_static_cohort_in_multi_condition(
3019+
self, patch_get, patch_flags
3020+
):
3021+
"""
3022+
When a flag has multiple conditions and one contains a static cohort,
3023+
the SDK should fallback to API for the entire flag, not just skip that
3024+
condition and evaluate the next one locally.
3025+
3026+
This prevents returning wrong variants when later conditions could match
3027+
locally but the user is actually in the static cohort.
3028+
"""
3029+
client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
3030+
3031+
# Mock the local flags response - cohort 999 is NOT in cohorts map (static cohort)
3032+
client.feature_flags = [
3033+
{
3034+
"id": 1,
3035+
"key": "multi-condition-flag",
3036+
"active": True,
3037+
"filters": {
3038+
"groups": [
3039+
{
3040+
"properties": [
3041+
{"key": "id", "value": 999, "type": "cohort"}
3042+
],
3043+
"rollout_percentage": 100,
3044+
"variant": "set-1",
3045+
},
3046+
{
3047+
"properties": [
3048+
{
3049+
"key": "$geoip_country_code",
3050+
"operator": "exact",
3051+
"value": ["DE"],
3052+
"type": "person",
3053+
}
3054+
],
3055+
"rollout_percentage": 100,
3056+
"variant": "set-8",
3057+
},
3058+
],
3059+
"multivariate": {
3060+
"variants": [
3061+
{"key": "set-1", "rollout_percentage": 50},
3062+
{"key": "set-8", "rollout_percentage": 50},
3063+
]
3064+
},
3065+
},
3066+
}
3067+
]
3068+
client.cohorts = {} # Note: cohort 999 is NOT here - it's a static cohort
3069+
3070+
# Mock the API response - user is in the static cohort
3071+
patch_flags.return_value = {"featureFlags": {"multi-condition-flag": "set-1"}}
3072+
3073+
result = client.get_feature_flag(
3074+
"multi-condition-flag",
3075+
"test-distinct-id",
3076+
person_properties={"$geoip_country_code": "DE"},
3077+
)
3078+
3079+
# Should return the API result (set-1), not local evaluation (set-8)
3080+
self.assertEqual(result, "set-1")
3081+
3082+
# Verify API was called (fallback occurred)
3083+
self.assertEqual(patch_flags.call_count, 1)
3084+
30163085

30173086
class TestMatchProperties(unittest.TestCase):
30183087
def property(self, key, value, operator=None):

0 commit comments

Comments
 (0)