fix(flags): evict $feature_flag_called dedupe tracker incrementally instead of clearing it (#821)

fix(flags): evict flag-called tracker entries incrementally

`SizeLimitedDict.__setitem__` cleared the entire mapping when it reached
`max_size`. The only user is `Client.distinct_ids_feature_flags_reported`, the
`$feature_flag_called` dedupe tracker, so every 50,000th new distinct ID wiped
dedupe state for all previously seen distinct IDs and their next flag read
re-emitted an event they had already deduped.

Evict only the oldest (least-recently-inserted) key instead, which is what the
sdk-specs feature-flag-called-tracker contract requires: capacity pressure must
not clear the whole tracker, and full clears are reserved for lifecycle
boundaries such as shutdown.

Generated-By: PostHog Code
Task-Id: af4d7563-5297-47da-95f8-75da9a218fde

Co-authored-by: posthog[bot] <206114724+posthog[bot]@users.noreply.github.com>

Author: posthog[bot]

.sampo/changesets/flag-called-tracker-incremental-eviction.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+The `$feature_flag_called` dedupe tracker now evicts its oldest entry when it reaches capacity instead of clearing every entry. Previously, each time a client accumulated 50,000 distinct IDs the whole tracker was wiped, so the next flag read for every previously seen distinct ID re-emitted a `$feature_flag_called` event it had already deduped.

posthog/test/test_feature_flags.py

@@ -6436,9 +6436,57 @@ def test_capture_multiple_users_doesnt_out_of_memory(
                 )
 
                 self.assertEqual(
-                    len(client.distinct_ids_feature_flags_reported), i % 100 + 1
+                    len(client.distinct_ids_feature_flags_reported), min(i + 1, 100)
                 )
 
+    @mock.patch.object(Client, "capture")
+    @mock.patch("posthog.client.flags")
+    def test_flag_called_dedupe_survives_tracker_capacity_pressure(
+        self, patch_flags, patch_capture
+    ):
+        """Hitting the tracker's capacity must not re-open dedupe for every user.
+
+        Evicting only the oldest entry keeps the still-tracked users deduped; clearing
+        the whole tracker would make each of them emit a duplicate
+        `$feature_flag_called` on their next read.
+        """
+        client = Client(FAKE_TEST_API_KEY, secret_key=FAKE_TEST_API_KEY)
+        client.distinct_ids_feature_flags_reported.max_size = 3
+        client.feature_flags = [
+            {
+                "id": 1,
+                "name": "Beta Feature",
+                "key": "complex-flag",
+                "active": True,
+                "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]},
+            }
+        ]
+
+        def flag_called_count_for(distinct_id):
+            return sum(
+                1
+                for call in patch_capture.call_args_list
+                if call.args
+                and call.args[0] == "$feature_flag_called"
+                and call.kwargs.get("distinct_id") == distinct_id
+            )
+
+        # Fill the tracker, then push one more user through to force an eviction.
+        for i in range(4):
+            client.get_feature_flag_result("complex-flag", f"user-{i}")
+
+        self.assertEqual(len(client.distinct_ids_feature_flags_reported), 3)
+        self.assertNotIn("user-0", client.distinct_ids_feature_flags_reported)
+
+        # Users still in the tracker stay deduped across the eviction.
+        for i in range(1, 4):
+            client.get_feature_flag_result("complex-flag", f"user-{i}")
+            self.assertEqual(flag_called_count_for(f"user-{i}"), 1)
+
+        # The evicted user is treated as unseen again, which is the expected cost.
+        client.get_feature_flag_result("complex-flag", "user-0")
+        self.assertEqual(flag_called_count_for("user-0"), 2)
+
 
 class TestConsistency(unittest.TestCase):
     @classmethod

posthog/test/test_size_limited_dict.py

@@ -14,14 +14,46 @@ def test_size_limited_dict(self, size: int, iterations: int) -> None:
             values[i] = i
 
             assert values[i] == i
-            assert len(values) == i % size + 1
-
-            if i % size == 0:
-                # old numbers should've been removed
-                self.assertIsNone(values.get(i - 1))
-                self.assertIsNone(values.get(i - 3))
-                self.assertIsNone(values.get(i - 5))
-                self.assertIsNone(values.get(i - 9))
+            # Capacity is a ceiling, not a reset point: the dict fills up and then
+            # stays full instead of collapsing back to one entry.
+            assert len(values) == min(i + 1, size)
+
+            # The most recent `size` keys survive; only what fell off the front is gone.
+            for recent in range(max(0, i - size + 1), i + 1):
+                assert values.get(recent) == recent
+            if i >= size:
+                self.assertIsNone(values.get(i - size))
+
+    @parameterized.expand([(10, 100), (5, 20), (20, 200)])
+    def test_size_limited_dict_evicts_incrementally(
+        self, size: int, iterations: int
+    ) -> None:
+        """Passing capacity must drop one oldest entry, not clear the whole mapping."""
+        values = utils.SizeLimitedDict(size, lambda _: -1)
+
+        for i in range(size):
+            values[i] = i
+
+        for i in range(size, iterations):
+            values[i] = i
+
+            assert len(values) == size
+            # Exactly one entry was evicted per insertion, so everything but the
+            # oldest key is still deduped.
+            self.assertIsNone(values.get(i - size))
+            assert values.get(i - size + 1) == i - size + 1
+
+    def test_size_limited_dict_overwrite_at_capacity_does_not_evict(self) -> None:
+        values = utils.SizeLimitedDict(3, lambda _: -1)
+        for i in range(3):
+            values[i] = i
+
+        values[0] = "updated"
+
+        assert len(values) == 3
+        assert values[0] == "updated"
+        assert values[1] == 1
+        assert values[2] == 2
 
     def test_size_limited_dict_forwards_defaultdict_args_and_kwargs(self) -> None:
         values = utils.SizeLimitedDict(

posthog/utils.py

@@ -152,13 +152,24 @@ def is_valid_regex(value) -> bool:
 
 
 class SizeLimitedDict(defaultdict):
+    """A ``defaultdict`` that evicts its oldest entries once it reaches ``max_size``.
+
+    Eviction is incremental: inserting a new key past capacity removes only the
+    least-recently-inserted key, never the whole mapping. This matters for the
+    ``$feature_flag_called`` dedupe tracker, whose only bound this is — wiping every
+    entry under capacity pressure would let the next flag read for every previously
+    seen ``distinct_id`` re-emit an event it had already deduped.
+    """
+
     def __init__(self, max_size, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.max_size = max_size
 
     def __setitem__(self, key, value):
-        if len(self) >= self.max_size:
-            self.clear()
+        if key not in self:
+            # dicts iterate in insertion order, so the first key is the oldest one.
+            while self and len(self) >= self.max_size:
+                del self[next(iter(self))]
 
         super().__setitem__(key, value)