fix(node): fire separate $feature_flag_called events per group context#3658
Merged
gustavohstrassburger merged 6 commits intoMay 25, 2026
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Contributor
|
Size Change: +368 B (0%) Total Size: 16.4 MB
ℹ️ View Unchanged
|
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/node/src/client.ts:1762
The custom sort comparator can be replaced with `localeCompare`, which is the idiomatic TypeScript approach for lexicographic string sorting and communicates intent more clearly.
```suggestion
? `_${JSON.stringify(Object.fromEntries(Object.entries(groups).sort(([a], [b]) => a.localeCompare(b))))}`
```
### Issue 2 of 2
packages/node/src/__tests__/evaluate-flags.spec.ts:195-219
**Prefer parameterised tests for related deduplication cases**
The two deduplication tests — "repeated access under the same group context" and "same groups in different key order" — both assert `toHaveLength(1)` and differ only in how the two evaluations are set up. Per the project's simplicity rules, grouping them with `it.each` would express the shared assertion once and make adding future deduplication scenarios straightforward without repeating the filter/expect boilerplate.
Reviews (1): Last reviewed commit: "fix(node): harden flag-called dedup key ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
dmarticus
approved these changes
May 22, 2026
Contributor
dmarticus
left a comment
dmarticus
left a comment
There was a problem hiding this comment.
Nice fix — the sorted JSON serialization makes the dedup key stable across insertion order, and the new tests cover the important cases (different groups fire separately, same groups dedup, key-order normalized).
Left a couple of optional style nits inline plus a flag on a pre-existing cache concern this PR amplifies (not a blocker). Reminder to add a patch changeset for posthog-node before merging — the changeset-hygiene bot already pinged on it.
| groups && Object.keys(groups).length > 0 | ||
| ? `_${JSON.stringify(Object.fromEntries(Object.entries(groups).sort(([a], [b]) => (a < b ? -1 : a > b ? 1 : 0))))}` | ||
| : '' | ||
| const featureFlagReportedKey = `${key}_${response}${groupSuffix}` |
Contributor
There was a problem hiding this comment.
Not a blocker for this PR, just flagging for follow-up: the existing eviction logic just below only fires on the number of distinct IDs — the inner string[] per distinctId has no cap, and each membership check is Array.includes (O(n)).
This PR amplifies the existing risk since dedup cardinality goes from flags × variants to flags × variants × group-combinations. For long-running Node processes that evaluate flags for one logical user across many tenants/groups, the inner array can grow and lookups slow down. Worth a follow-up to bound it (LRU) or switch to a Set for O(1) lookup.
| (m) => m.event === '$feature_flag_called' && m.properties.$feature_flag === 'boolean-flag' | ||
| ) | ||
| expect(flagCalled).toHaveLength(1) | ||
| }) |
Contributor
There was a problem hiding this comment.
Optional: consider locking down that groups: undefined and groups: {} dedup together. Both currently produce no suffix, so a single test asserting they collide would pin that contract — useful since users who omit groups on one call and pass {} on another would otherwise be surprised by either outcome changing.
…ined/empty contract
gustavohstrassburger
added a commit
to PostHog/posthog-python
that referenced
this pull request
May 25, 2026
In `_capture_feature_flag_called_if_needed`, the per-distinct_id dedupe key only included the flag key and response. For group-scoped flags, this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include the sorted `groups` map in the dedupe key so that the same (distinct_id, flag, response) combination fires once per distinct group context. Repeated calls under the same group context still dedupe, and calls that pass the same map in a different key order still dedupe (the groups are canonicalized via `sorted(groups.items())`). Mirrors the posthog-node fix in PostHog/posthog-js#3658. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
5 tasks
gustavohstrassburger
added a commit
to PostHog/posthog-ruby
that referenced
this pull request
May 25, 2026
In `_capture_feature_flag_called_if_needed`, the per-`distinct_id` dedupe key only included the flag key and response. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include the sorted `groups` hash in the dedupe key so that the same `(distinct_id, flag, response)` combination fires once per distinct group context. Repeated calls under the same group context still dedupe, and calls that pass the same map in a different key order still dedupe (the groups are canonicalized via `groups.sort.to_json` before being mixed into the dedup key). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
5 tasks
gustavohstrassburger
added a commit
to PostHog/posthog-go
that referenced
this pull request
May 25, 2026
In `captureFlagCalledIfNeeded`, the per-`distinct_id` LRU dedupe key only included `(distinct_id, flag_key, device_id)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Add the group context as a canonical JSON of the sorted group key/value pairs to the LRU cache key (new `groupsRepr` field on `flagUser`) so the same `(user, flag, device)` fires once per distinct group context. Repeated calls under the same group context still dedupe; calls with the same map but different Go map insertion order also dedupe (the JSON is built from a `sort.Strings`-sorted slice of keys). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
5 tasks
gustavohstrassburger
added a commit
to PostHog/posthog-php
that referenced
this pull request
May 25, 2026
In `Client::captureFlagCalledIfNeeded`, the per-`distinct_id` dedupe element only included the distinct ID under the flag key. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Append a canonical JSON of the sorted `groups` map to the dedup element (`$distinctId . canonicalGroupsRepr($groups)`) so the same `(distinct_id, flag)` fires once per distinct group context. Repeated calls under the same group context still dedupe; calls that pass the same array with keys inserted in a different order also dedupe (the array is canonicalized via `ksort` before being JSON-encoded). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
5 tasks
gustavohstrassburger
added a commit
to PostHog/posthog-dotnet
that referenced
this pull request
May 25, 2026
In `PostHogClient.TryCaptureDedupedFeatureFlagCalledEvent`, the per-`distinctId` `MemoryCache` key only included `(distinctId, featureKey, cacheKeyValue)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include a canonical representation of the `GroupCollection` in the cache key (new `CanonicalGroupsCacheKey` helper) so the same `(user, flag, response)` fires once per distinct group context. Repeated calls under the same group context still dedupe; group collections built in a different insertion order also dedupe (canonicalized via `OrderBy(GroupType, StringComparer.Ordinal)`). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
5 tasks
gustavohstrassburger
added a commit
to PostHog/posthog-android
that referenced
this pull request
May 25, 2026
In `PostHogFeatureFlagCalledCache.add`, the per-`distinctId` LRU dedupe key only included `(distinctId, flagKey, value)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Add a canonical groups representation (`canonicalGroupsRepr`) into the LRU cache key — built from the sorted group entries so two equal maps with keys inserted in a different order produce the same dedup key. The `groups` map is now threaded through `PostHogStateless.captureFeatureFlagCalledEvent` and the `posthog-server` snapshot accessors (`PostHogFeatureFlagEvaluations.isEnabled/getFlag`) so it can be funneled into the dedup cache and onto the captured `$feature_flag_called` event payload. This affects two modules: - `posthog/` (PostHogStateless / cache) — `captureFeatureFlagCalledEvent` now takes an optional `groups` parameter (added via `@JvmOverloads` so the existing Java-callable signature still exists). - `posthog-server/` — the snapshot carries the groups it was evaluated under and passes them to the host's dedup-aware capture call. Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
gustavohstrassburger
added a commit
to PostHog/posthog-rs
that referenced
this pull request
May 25, 2026
In `build_dedup_key` (in both `client/async_client.rs` and `client/blocking.rs`), the per-`distinct_id` dedup key only included `(flag_key, response)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. `build_dedup_key` now also takes the `groups` map; when non-empty it appends a canonical, sorted `"k1=v1;k2=v2;…"` suffix so the same `(user, flag, response)` fires once per distinct group context. Repeated calls under the same group context still dedupe; calls with the same group map in a different insertion order also dedupe (sorted via `sort_by(|a, b| a.0.cmp(b.0))`). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1
This was referenced May 25, 2026
gustavohstrassburger
added a commit
to PostHog/posthog-python
that referenced
this pull request
May 27, 2026
* fix: include group context in $feature_flag_called dedupe key In `_capture_feature_flag_called_if_needed`, the per-distinct_id dedupe key only included the flag key and response. For group-scoped flags, this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include the sorted `groups` map in the dedupe key so that the same (distinct_id, flag, response) combination fires once per distinct group context. Repeated calls under the same group context still dedupe, and calls that pass the same map in a different key order still dedupe (the groups are canonicalized via `sorted(groups.items())`). Mirrors the posthog-node fix in PostHog/posthog-js#3658. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * refactor: use tuple for feature_flag_reported_key and parameterize dedupe tests
gustavohstrassburger
added a commit
to PostHog/posthog-php
that referenced
this pull request
May 27, 2026
* fix: include group context in $feature_flag_called dedupe key In `Client::captureFlagCalledIfNeeded`, the per-`distinct_id` dedupe element only included the distinct ID under the flag key. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Append a canonical JSON of the sorted `groups` map to the dedup element (`$distinctId . canonicalGroupsRepr($groups)`) so the same `(distinct_id, flag)` fires once per distinct group context. Repeated calls under the same group context still dedupe; calls that pass the same array with keys inserted in a different order also dedupe (the array is canonicalized via `ksort` before being JSON-encoded). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * fix: add JSON_THROW_ON_ERROR to group dedup key encoding and combine dedup tests under dataProvider
gustavohstrassburger
added a commit
to PostHog/posthog-ruby
that referenced
this pull request
May 27, 2026
* fix: include group context in $feature_flag_called dedupe key In `_capture_feature_flag_called_if_needed`, the per-`distinct_id` dedupe key only included the flag key and response. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include the sorted `groups` hash in the dedupe key so that the same `(distinct_id, flag, response)` combination fires once per distinct group context. Repeated calls under the same group context still dedupe, and calls that pass the same map in a different key order still dedupe (the groups are canonicalized via `groups.sort.to_json` before being mixed into the dedup key). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * refactor(spec): extract shared setup for group-context dedup tests into context block * style: fix rubocop line length offenses in client_spec
gustavohstrassburger
added a commit
to PostHog/posthog-android
that referenced
this pull request
May 27, 2026
…#533) * fix(server): include group context in $feature_flag_called dedupe key In `PostHogFeatureFlagCalledCache.add`, the per-`distinctId` LRU dedupe key only included `(distinctId, flagKey, value)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Add a canonical groups representation (`canonicalGroupsRepr`) into the LRU cache key — built from the sorted group entries so two equal maps with keys inserted in a different order produce the same dedup key. The `groups` map is now threaded through `PostHogStateless.captureFeatureFlagCalledEvent` and the `posthog-server` snapshot accessors (`PostHogFeatureFlagEvaluations.isEnabled/getFlag`) so it can be funneled into the dedup cache and onto the captured `$feature_flag_called` event payload. This affects two modules: - `posthog/` (PostHogStateless / cache) — `captureFeatureFlagCalledEvent` now takes an optional `groups` parameter (added via `@JvmOverloads` so the existing Java-callable signature still exists). - `posthog-server/` — the snapshot carries the groups it was evaluated under and passes them to the host's dedup-aware capture call. Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * fix: escape entry separators in canonicalGroupsRepr Two fixes from Greptile review on this PR: 1. Escape `=`, `;`, and `\` in `canonicalGroupsRepr` so that values containing the entry separators can't collide with synthetic decompositions of a different map. Previously, `mapOf("a" to "b;c=d", "e" to "f")` and `mapOf("a" to "b", "c" to "d", "e" to "f")` both serialized to `a=b;c=d;e=f;` and would have silently dedup-ed to one event. 2. Split the "null vs empty vs unset" dedup test into three independent caches, plus a new test that verifies values containing the separators hit distinct cache entries. Three-call-on-shared-cache assertions could pass purely from key-identity dedup instead of canonicalization parity. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: appease spotless formatting spotlessKotlinCheck on CI flagged that `appendEscaped(sb: StringBuilder, value: String)` exceeds the project's per-line param width and needs to be broken across lines. Verified locally with `./gradlew spotlessKotlinCheck`. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * fix(server): defensively copy groups map on construction
gustavohstrassburger
added a commit
to PostHog/posthog-rs
that referenced
this pull request
May 28, 2026
* fix: include group context in $feature_flag_called dedupe key In `build_dedup_key` (in both `client/async_client.rs` and `client/blocking.rs`), the per-`distinct_id` dedup key only included `(flag_key, response)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. `build_dedup_key` now also takes the `groups` map; when non-empty it appends a canonical, sorted `"k1=v1;k2=v2;…"` suffix so the same `(user, flag, response)` fires once per distinct group context. Repeated calls under the same group context still dedupe; calls with the same group map in a different insertion order also dedupe (sorted via `sort_by(|a, b| a.0.cmp(b.0))`). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * fix: percent-encode group keys/values in dedup key to prevent collisions * chore: fix rustfmt formatting
gustavohstrassburger
added a commit
to PostHog/posthog-go
that referenced
this pull request
May 28, 2026
* fix: include group context in $feature_flag_called dedupe key In `captureFlagCalledIfNeeded`, the per-`distinct_id` LRU dedupe key only included `(distinct_id, flag_key, device_id)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Add the group context as a canonical JSON of the sorted group key/value pairs to the LRU cache key (new `groupsRepr` field on `flagUser`) so the same `(user, flag, device)` fires once per distinct group context. Repeated calls under the same group context still dedupe; calls with the same map but different Go map insertion order also dedupe (the JSON is built from a `sort.Strings`-sorted slice of keys). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * test: combine flag-called dedup tests into table-driven test Merge TestCaptureFlagCalled_DedupesAcrossRepeatedCallsUnderSameGroup and TestCaptureFlagCalled_DedupesAcrossGroupKeyOrder into a single parameterised test with a {name, calls []Groups} cases slice and t.Run subtests, so the shared setup/sleep/assert boilerplate is stated once. Generated-By: PostHog Code Task-Id: ad586036-ee97-4dc5-82e7-e96cb4fb9de7 * test: wait for async flush in dedup test to fix CI flake The combined TestCaptureFlagCalled_DedupesAcrossSameGroupContext relied on a 150ms sleep before asserting on captured events. On a slow CI runner with -race enabled, the batch flush sometimes didn't complete in time and the test saw zero events. Use the existing waitForEventCount helper (already used by the sibling TestCaptureFlagCalled_FiresPerGroupContext) to wait for the first event with a real timeout, then sleep briefly to catch any stragglers that would indicate broken deduplication, before counting. Generated-By: PostHog Code Task-Id: ad586036-ee97-4dc5-82e7-e96cb4fb9de7
gustavohstrassburger
added a commit
to PostHog/posthog-dotnet
that referenced
this pull request
May 28, 2026
* fix: include group context in $feature_flag_called dedupe key In `PostHogClient.TryCaptureDedupedFeatureFlagCalledEvent`, the per-`distinctId` `MemoryCache` key only included `(distinctId, featureKey, cacheKeyValue)`. For group-scoped flags this meant that when the same user was evaluated under a different group, no new `$feature_flag_called` event was fired — causing per-group exposure undercount for experiments scoped to a group key. Include a canonical representation of the `GroupCollection` in the cache key (new `CanonicalGroupsCacheKey` helper) so the same `(user, flag, response)` fires once per distinct group context. Repeated calls under the same group context still dedupe; group collections built in a different insertion order also dedupe (canonicalized via `OrderBy(GroupType, StringComparer.Ordinal)`). Mirrors the posthog-node fix in PostHog/posthog-js#3658 (which closes PostHog/posthog-js#3651). Both SDKs share the same dedupe shape, so backend evaluation needs the same change. Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * chore: add changeset Generated-By: PostHog Code Task-Id: d94308d9-7655-4bac-8f15-c61478b5fca1 * refactor: address review nits on flag-called dedupe-groups - Escape group type/key with Uri.EscapeDataString in CanonicalGroupsCacheKey to avoid `=`/`;` delimiter collisions. - Drop the intermediate tuple Select and order Groups directly. - Reword the dedupe-shape comment to not call the no-groups path "legacy". - Match the factory comment to the actual variable names in the cache tuple. - Collapse the three group-context dedupe Facts into one MemberData Theory. Generated-By: PostHog Code Task-Id: fe3e6da7-66a3-4fad-95bf-59d5df4557ee
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In the Node SDK,
$feature_flag_calledevents were not fired when the same flag+variant was evaluated under different group contexts. The deduplication key only included the flag key and variant, so only the first call would fire an event regardless of which group was used.Closes #3651
Changes
Include group context in the
$feature_flag_calleddeduplication key in_captureFlagCalledEventIfNeeded, so events fire independently per group combination.How did you test this code?
Added a test in
evaluate-flags.spec.tsverifying that two separate$feature_flag_calledevents fire for the same flag under different group contexts.👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file