From ac1591844a759b7bd88e516f35debbf66e917b10 Mon Sep 17 00:00:00 2001 From: Dustin Byrne Date: Thu, 5 Nov 2026 17:18:26 -0500 Subject: [PATCH] fix(flags): evaluate person enrollment before group context --- .../person-enrollment-before-group-context.md | 5 + posthog/client.py | 4 + posthog/feature_flags.py | 20 +-- posthog/test/test_feature_flags.py | 118 +++++++++++++++++- 4 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 .sampo/changesets/person-enrollment-before-group-context.md diff --git a/.sampo/changesets/person-enrollment-before-group-context.md b/.sampo/changesets/person-enrollment-before-group-context.md new file mode 100644 index 000000000..7114e2583 --- /dev/null +++ b/.sampo/changesets/person-enrollment-before-group-context.md @@ -0,0 +1,5 @@ +--- +pypi/posthog: patch +--- + +Evaluate early-access enrollment from person properties before requiring group context. Group-based flags now honor explicit person enrollment locally and fall back to remote evaluation when enrollment is not supplied. diff --git a/posthog/client.py b/posthog/client.py index 87ba50705..1bd77be81 100644 --- a/posthog/client.py +++ b/posthog/client.py @@ -86,6 +86,7 @@ InconclusiveMatchError, RequiresServerEvaluation, _get_definition_unresolved_reason, + _match_feature_enrollment, match_feature_flag_properties, resolve_bucketing_value, ) @@ -3908,6 +3909,9 @@ def _compute_flag_locally( ) flag_filters = feature_flag.get("filters") or {} + if flag_filters.get("feature_enrollment") is True: + return _match_feature_enrollment(feature_flag, person_properties) + aggregation_group_type_index = flag_filters.get("aggregation_group_type_index") group_type_mapping = snapshot["group_type_mapping"] diff --git a/posthog/feature_flags.py b/posthog/feature_flags.py index 6a16ee319..9866fabc8 100644 --- a/posthog/feature_flags.py +++ b/posthog/feature_flags.py @@ -466,6 +466,17 @@ def resolve_bucketing_value(flag, distinct_id, device_id=None): return distinct_id +def _match_feature_enrollment(flag, person_properties) -> bool: + enrollment_key = f"$feature_enrollment/{flag.get('key')}" + if enrollment_key not in person_properties: + raise InconclusiveMatchError( + f"Flag has feature enrollment and '{enrollment_key}' was not passed", + reason=UnresolvedFlagReason.MISSING_CONTEXT, + ) + enrollment = person_properties[enrollment_key] + return enrollment is True or enrollment == "true" + + def match_feature_flag_properties( flag, distinct_id, @@ -493,19 +504,12 @@ def match_feature_flag_properties( flag_filters = flag.get("filters") or {} if flag_filters.get("feature_enrollment") is True: - enrollment_key = f"$feature_enrollment/{flag.get('key')}" if flag_filters.get("aggregation_group_type_index") is not None: raise InconclusiveMatchError( "Group flag has feature enrollment, which reads person properties", reason=UnresolvedFlagReason.UNSUPPORTED_DEFINITION, ) - if enrollment_key not in properties: - raise InconclusiveMatchError( - f"Flag has feature enrollment and '{enrollment_key}' was not passed", - reason=UnresolvedFlagReason.MISSING_CONTEXT, - ) - enrollment = properties[enrollment_key] - return enrollment is True or enrollment == "true" + return _match_feature_enrollment(flag, properties) # 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. diff --git a/posthog/test/test_feature_flags.py b/posthog/test/test_feature_flags.py index ff1d56593..466ad271e 100644 --- a/posthog/test/test_feature_flags.py +++ b/posthog/test/test_feature_flags.py @@ -1223,8 +1223,8 @@ def test_experience_continuity_flag_not_evaluated_locally(self, patch_flags): "group flag", {"$feature_enrollment/beta-feature": False}, 100, - "flags-fallback-value", - 1, + False, + 0, True, ), ] @@ -1264,6 +1264,120 @@ def test_feature_enrollment_flag( self.assertEqual((value, patch_flags.call_count), (expected, flags_calls)) + @parameterized.expand( + [ + ("enrolled without group", True, False, None, True), + ("string enrolled without group", "true", False, None, True), + ("opted out without group", False, False, None, False), + ("enrolled despite group opt-out", True, True, False, True), + ("opted out despite group enrollment", False, True, True, False), + ("missing enrollment without group", None, False, None, None), + ("missing enrollment with group enrollment", None, True, True, None), + ] + ) + @mock.patch("posthog.client.flags") + def test_group_feature_enrollment_uses_person_context( + self, _name, enrollment, has_group, group_enrollment, expected, patch_flags + ): + client = self.client + client.feature_flags = [ + { + "id": 1, + "key": "beta-feature", + "active": True, + "filters": { + "feature_enrollment": True, + "aggregation_group_type_index": 0, + "groups": [{"properties": [], "rollout_percentage": 0}], + }, + } + ] + client.group_type_mapping = {"0": "company"} + person_properties = ( + {"$feature_enrollment/beta-feature": enrollment} + if enrollment is not None + else {} + ) + groups = {"company": "acme"} if has_group else {} + group_properties = ( + {"company": {"$feature_enrollment/beta-feature": group_enrollment}} + if has_group + else {} + ) + patch_flags.return_value = { + "featureFlags": {"beta-feature": "flags-fallback-value"} + } + + value = client.get_feature_flag( + "beta-feature", + "distinct_id", + person_properties=person_properties, + groups=groups, + group_properties=group_properties, + ) + self.assertEqual( + value, expected if expected is not None else "flags-fallback-value" + ) + self.assertEqual(patch_flags.call_count, int(expected is None)) + patch_flags.reset_mock() + + snapshot = client.evaluate_flags( + "distinct_id", + person_properties=person_properties, + groups=groups, + group_properties=group_properties, + only_evaluate_locally=True, + ) + self.assertEqual(snapshot.get_flag("beta-feature"), expected) + self.assertEqual( + snapshot.unresolved_flags, + {"beta-feature": UnresolvedFlagReason.MISSING_CONTEXT} + if expected is None + else {}, + ) + patch_flags.assert_not_called() + + @parameterized.expand( + [ + ("inactive", False, False, {}, False, {}), + ( + "experience continuity", + True, + True, + {"$feature_enrollment/beta-feature": True}, + None, + {"beta-feature": UnresolvedFlagReason.EXPERIENCE_CONTINUITY}, + ), + ] + ) + @mock.patch("posthog.client.flags") + def test_group_feature_enrollment_preserves_evaluation_guards( + self, _name, active, continuity, properties, expected, unresolved, patch_flags + ): + client = self.client + client.feature_flags = [ + { + "id": 1, + "key": "beta-feature", + "active": active, + "ensure_experience_continuity": continuity, + "filters": { + "feature_enrollment": True, + "aggregation_group_type_index": 0, + "groups": [{"properties": [], "rollout_percentage": 100}], + }, + } + ] + client.group_type_mapping = {"0": "company"} + + snapshot = client.evaluate_flags( + "distinct_id", person_properties=properties, only_evaluate_locally=True + ) + + self.assertEqual(snapshot.get_flag("beta-feature"), expected) + self.assertEqual(snapshot.unresolved_flags, unresolved) + patch_flags.assert_not_called() + @mock.patch.object(Client, "capture") @mock.patch("posthog.client.flags") def test_get_all_flags_with_fallback(self, patch_flags, patch_capture):