Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .sampo/changesets/canonical-group-key-bucketing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
posthog: patch
---

Use canonical numeric group keys for local feature-flag bucketing and fall back to remote evaluation for unsupported key types.
3 changes: 2 additions & 1 deletion posthog/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
from posthog.feature_flags import (
InconclusiveMatchError,
RequiresServerEvaluation,
_group_key_to_string,
match_feature_flag_properties,
resolve_bucketing_value,
)
Expand Down Expand Up @@ -3582,7 +3583,7 @@ def _compute_flag_locally(
f"Flag has no group properties for group '{group_name}'"
)
focused_group_properties = group_properties[group_name]
group_key = groups[group_name]
group_key = _group_key_to_string(groups[group_name])
return match_feature_flag_properties(
feature_flag,
group_key,
Expand Down
10 changes: 9 additions & 1 deletion posthog/feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ def match_feature_flag_properties(
is_inconclusive = True
continue
effective_properties = group_properties[group_name]
effective_bucketing = groups[group_name]
effective_bucketing = _group_key_to_string(groups[group_name])
else:
effective_properties = properties
effective_bucketing = bucketing_value
Expand Down Expand Up @@ -613,6 +613,14 @@ def _json_value_to_string(value) -> str:
)


def _group_key_to_string(value) -> str:
if isinstance(value, str):
return value
if isinstance(value, (int, float)) and not isinstance(value, bool):
return _json_value_to_string(value)
raise InconclusiveMatchError("Group keys must be strings or finite numbers")


def _value_to_string(value) -> str:
if isinstance(value, str):
return value
Expand Down
80 changes: 80 additions & 0 deletions posthog/test/test_feature_flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,86 @@ def test_missing_nested_cohort_always_requires_server_evaluation(self):
)


class TestGroupKeyBucketing(unittest.TestCase):
def setUp(self):
self.client = Client(FAKE_TEST_API_KEY, send=False)
self.client.group_type_mapping = {"0": "company"}

def tearDown(self):
self.client.shutdown()

def set_group_flag(self, mixed=False, rollout=50):
condition = {"properties": [], "rollout_percentage": rollout}
filters = {"groups": [condition]}
if mixed:
condition["aggregation_group_type_index"] = 0
else:
filters["aggregation_group_type_index"] = 0
self.client.feature_flags = [
{"id": 1, "key": "group-beta", "active": True, "filters": filters}
]

@parameterized.expand([(False,), (True,)])
@mock.patch("posthog.client.flags")
def test_numeric_group_keys_use_server_representation(self, mixed, patch_flags):
self.set_group_flag(mixed)
# For this flag, "0.00001" hashes below 50%, but Python's "1e-05"
# hashes above it. Integer keys should keep their existing bucketing.
for group_key in (0.00001, "0.00001", 42, "42"):
with self.subTest(group_key=group_key):
self.assertTrue(
self.client.get_feature_flag(
"group-beta",
"user-1",
groups={"company": group_key},
only_evaluate_locally=True,
send_feature_flag_events=False,
)
)
flags = self.client.evaluate_flags(
"user-1",
groups={"company": group_key},
only_evaluate_locally=True,
)
self.assertTrue(flags.get_flag("group-beta"))
patch_flags.assert_not_called()

@parameterized.expand([(False,), (True,)])
@mock.patch("posthog.client.flags")
def test_unsupported_group_keys_require_remote_evaluation(self, mixed, patch_flags):
self.set_group_flag(mixed, rollout=100)
patch_flags.return_value = {"featureFlags": {"group-beta": "remote"}}
for group_key in (True, False, None, [], {}, float("inf"), float("nan")):
with self.subTest(group_key=group_key):
self.assertIsNone(
self.client.get_feature_flag(
"group-beta",
"user-1",
groups={"company": group_key},
only_evaluate_locally=True,
send_feature_flag_events=False,
)
)
flags = self.client.evaluate_flags(
"user-1",
groups={"company": group_key},
only_evaluate_locally=True,
)
self.assertNotIn("group-beta", flags.keys)
patch_flags.assert_not_called()
self.assertEqual(
self.client.get_feature_flag(
"group-beta",
"user-1",
groups={"company": group_key},
send_feature_flag_events=False,
),
"remote",
)
patch_flags.assert_called_once()
patch_flags.reset_mock()


class TestLocalEvaluation(unittest.TestCase):
@classmethod
def setUpClass(cls):
Expand Down