Skip to content

feat: add get_feature_flags for bulk flag evaluation with events#534

Closed
dmarticus wants to merge 1 commit intomainfrom
posthog-code/get-feature-flags-bulk-v2
Closed

feat: add get_feature_flags for bulk flag evaluation with events#534
dmarticus wants to merge 1 commit intomainfrom
posthog-code/get-feature-flags-bulk-v2

Conversation

@dmarticus
Copy link
Copy Markdown
Contributor

Summary

Add a new get_feature_flags(keys, distinct_id, ...) method for evaluating a known subset of feature flags in one bulk pass while still emitting $feature_flag_called events per resolved flag. Also adds a send_feature_flag_events option to the existing get_all_flags / get_all_flags_and_payloads methods for opt-in per-flag event emission.

The motivation (per the original PR): customers loading a page server-side often need a specific ~10 flags. get_feature_flag per key is N network calls; get_all_flags is one call but emits no $feature_flag_called events and evaluates flags the caller doesn't care about. The new method is the sweet spot: at most one network round trip, per-flag usage events.

Ports PostHog/posthog-js#3447 from posthog-node to posthog-python. Supersedes #532 (closed because its branch was based on a stale commit that predated several unsigned release commits on main, which blocked a clean rebase-and-push under the repo's signature policy).

Changes

  • New get_feature_flags method (posthog/client.py): evaluates a subset of flags in bulk. Locally-evaluated flags reuse the poller's cached definitions; keys that can't be resolved locally fall through to a single remote /flags call with flag_keys_to_evaluate. Threads device_id and participates in the same error handling (quota, timeout, connection, api-error, unknown) + stale-cache fallback as _get_feature_flag_result.
  • Event deduplication: reuses the existing distinct_ids_feature_flags_reported cache so single-flag and bulk paths share one source of truth.
  • New send_feature_flag_events option on get_all_flags / get_all_flags_and_payloads. Defaults to False to preserve existing behavior.
  • Top-level proxy (posthog/__init__.py): adds posthog.get_feature_flags(...) and threads send_feature_flag_events through the module-level get_all_flags / get_all_flags_and_payloads wrappers.
  • Tests (posthog/test/test_feature_flags.py): new TestGetFeatureFlagsBulk class with 9 tests covering remote-only evaluation, local-only evaluation, hybrid scenarios, event deduplication, send_feature_flag_events=False suppression, missing-key handling (with $feature_flag_error=flag_missing), and the new get_all_flags option.

Test plan

  • uv run pytest posthog/test/test_feature_flags.py::TestGetFeatureFlagsBulk — 9/9 pass
  • uv run pytest posthog/test/ --ignore=posthog/test/ai — 459/459 pass (no regressions; send_feature_flag_events=False default preserves historical behavior)
  • uv run mypy posthog/client.py posthog/__init__.py — no new errors vs. baseline
  • uvx ruff format — clean

Created with PostHog Code

Add a new get_feature_flags(keys, distinct_id, ...) method for evaluating
a known subset of feature flags in one bulk pass while still emitting
$feature_flag_called events per resolved flag. Locally-evaluated flags
reuse the poller's cached definitions; keys that can't be resolved
locally fall through to a single remote /flags call with
flag_keys_to_evaluate. Event dedup uses the existing
distinct_ids_feature_flags_reported cache so the single-flag and bulk
paths share one source of truth.

Also adds a send_feature_flag_events option to get_all_flags and
get_all_flags_and_payloads for opt-in per-flag event emission. Defaults
to False to preserve existing behavior.

Ports PostHog/posthog-js#3447 to posthog-python.

Generated-By: PostHog Code
Task-Id: b54693f6-498d-4193-bbc2-b9a97e07e69d
@dmarticus dmarticus requested a review from a team as a code owner April 23, 2026 22:20
@github-actions
Copy link
Copy Markdown
Contributor

posthog-python Compliance Report

Date: 2026-04-23 22:23:46 UTC
Duration: 160032ms

✅ All Tests Passed!

30/30 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 519ms
Format Validation.Event Has Uuid 1508ms
Format Validation.Event Has Lib Properties 1507ms
Format Validation.Distinct Id Is String 1507ms
Format Validation.Token Is Present 1507ms
Format Validation.Custom Properties Preserved 1507ms
Format Validation.Event Has Timestamp 1507ms
Retry Behavior.Retries On 503 9519ms
Retry Behavior.Does Not Retry On 400 3505ms
Retry Behavior.Does Not Retry On 401 3509ms
Retry Behavior.Respects Retry After Header 9511ms
Retry Behavior.Implements Backoff 23533ms
Retry Behavior.Retries On 500 7505ms
Retry Behavior.Retries On 502 7512ms
Retry Behavior.Retries On 504 7513ms
Retry Behavior.Max Retries Respected 23529ms
Deduplication.Generates Unique Uuids 1497ms
Deduplication.Preserves Uuid On Retry 7512ms
Deduplication.Preserves Uuid And Timestamp On Retry 14522ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7510ms
Deduplication.No Duplicate Events In Batch 1505ms
Deduplication.Different Events Have Different Uuids 1507ms
Compression.Sends Gzip When Enabled 1507ms
Batch Format.Uses Proper Batch Structure 1507ms
Batch Format.Flush With No Events Sends Nothing 1006ms
Batch Format.Multiple Events Batched Together 1505ms
Error Handling.Does Not Retry On 403 3508ms
Error Handling.Does Not Retry On 413 3508ms
Error Handling.Retries On 408 7510ms

Feature_Flags Tests

1/1 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 519ms

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 23, 2026

Comments Outside Diff (3)

  1. posthog/test/test_feature_flags.py, line 718-722 (link)

    P1 Wrong mock format makes the test trivially pass

    The mock here returns {"featureFlags": ...} (the old response shape), but to_flags_and_payloads requires a {"flags": ...} key. Because to_values short-circuits on "flags" not in response, it returns None, so flag_values becomes {} and no events could ever be emitted — regardless of the send_feature_flag_events setting. The assertion patch_capture.call_count == 0 passes for the wrong reason, and the test would also pass even if event-suppression logic were removed entirely.

    To actually verify that send_feature_flag_events=False suppresses emission, use the same flags-keyed response as test_get_all_flags_emits_events_when_send_feature_flag_events_enabled:

    patch_flags.return_value = {
        "flags": {
            "bulk-a": {
                "key": "bulk-a",
                "enabled": True,
                "variant": None,
                "reason": {"description": "Matched"},
                "metadata": {"id": 1, "version": 1},
            },
            "bulk-b": {
                "key": "bulk-b",
                "enabled": True,
                "variant": "b-variant",
                "reason": {"description": "Matched"},
                "metadata": {"id": 2, "version": 1},
            },
        },
        "requestId": "req-all",
    }
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: posthog/test/test_feature_flags.py
    Line: 718-722
    
    Comment:
    **Wrong mock format makes the test trivially pass**
    
    The mock here returns `{"featureFlags": ...}` (the old response shape), but `to_flags_and_payloads` requires a `{"flags": ...}` key. Because `to_values` short-circuits on `"flags" not in response`, it returns `None`, so `flag_values` becomes `{}` and no events could ever be emitted — regardless of the `send_feature_flag_events` setting. The assertion `patch_capture.call_count == 0` passes for the wrong reason, and the test would also pass even if event-suppression logic were removed entirely.
    
    To actually verify that `send_feature_flag_events=False` suppresses emission, use the same `flags`-keyed response as `test_get_all_flags_emits_events_when_send_feature_flag_events_enabled`:
    
    ```python
    patch_flags.return_value = {
        "flags": {
            "bulk-a": {
                "key": "bulk-a",
                "enabled": True,
                "variant": None,
                "reason": {"description": "Matched"},
                "metadata": {"id": 1, "version": 1},
            },
            "bulk-b": {
                "key": "bulk-b",
                "enabled": True,
                "variant": "b-variant",
                "reason": {"description": "Matched"},
                "metadata": {"id": 2, "version": 1},
            },
        },
        "requestId": "req-all",
    }
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.
  2. posthog/client.py, line 115-125 (link)

    P2 Superfluous property reassignments after _add_local_person_and_group_properties

    _add_local_person_and_group_properties always returns plain dict objects (never None), so the two lines that follow overwrite the enriched results with themselves — they can't change anything. This violates the "no superfluous parts" principle and could mislead future readers into thinking these vars might still be None at this point.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: posthog/client.py
    Line: 115-125
    
    Comment:
    **Superfluous property reassignments after `_add_local_person_and_group_properties`**
    
    `_add_local_person_and_group_properties` always returns plain `dict` objects (never `None`), so the two lines that follow overwrite the enriched results with themselves — they can't change anything. This violates the "no superfluous parts" principle and could mislead future readers into thinking these vars might still be `None` at this point.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
  3. posthog/test/test_feature_flags.py, line 452-460 (link)

    P2 Prefer parameterised tests over repeated helper-based tests

    The instructions for this repo say "We always prefer parameterised tests." Several of the new tests in TestGetFeatureFlagsBulk follow the same structure (set up a mock response, call get_feature_flags, assert on capture count and/or properties) while varying only one axis at a time — for example, test_no_events_when_send_feature_flag_events_is_false vs test_returns_feature_flag_result_per_key_from_single_remote_call. These would read more concisely and be easier to extend as a @parameterized.expand suite (or with subTest).

    Context Used: Do not attempt to comment on incorrect alphabetica... (source)

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: posthog/test/test_feature_flags.py
    Line: 452-460
    
    Comment:
    **Prefer parameterised tests over repeated helper-based tests**
    
    The instructions for this repo say "We always prefer parameterised tests." Several of the new tests in `TestGetFeatureFlagsBulk` follow the same structure (set up a mock response, call `get_feature_flags`, assert on capture count and/or properties) while varying only one axis at a time — for example, `test_no_events_when_send_feature_flag_events_is_false` vs `test_returns_feature_flag_result_per_key_from_single_remote_call`. These would read more concisely and be easier to extend as a `@parameterized.expand` suite (or with `subTest`).
    
    **Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
    
    How can I resolve this? If you propose a fix, please make it concise.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
This is a comment left during a code review.
Path: posthog/test/test_feature_flags.py
Line: 718-722

Comment:
**Wrong mock format makes the test trivially pass**

The mock here returns `{"featureFlags": ...}` (the old response shape), but `to_flags_and_payloads` requires a `{"flags": ...}` key. Because `to_values` short-circuits on `"flags" not in response`, it returns `None`, so `flag_values` becomes `{}` and no events could ever be emitted — regardless of the `send_feature_flag_events` setting. The assertion `patch_capture.call_count == 0` passes for the wrong reason, and the test would also pass even if event-suppression logic were removed entirely.

To actually verify that `send_feature_flag_events=False` suppresses emission, use the same `flags`-keyed response as `test_get_all_flags_emits_events_when_send_feature_flag_events_enabled`:

```python
patch_flags.return_value = {
    "flags": {
        "bulk-a": {
            "key": "bulk-a",
            "enabled": True,
            "variant": None,
            "reason": {"description": "Matched"},
            "metadata": {"id": 1, "version": 1},
        },
        "bulk-b": {
            "key": "bulk-b",
            "enabled": True,
            "variant": "b-variant",
            "reason": {"description": "Matched"},
            "metadata": {"id": 2, "version": 1},
        },
    },
    "requestId": "req-all",
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: posthog/client.py
Line: 115-125

Comment:
**Superfluous property reassignments after `_add_local_person_and_group_properties`**

`_add_local_person_and_group_properties` always returns plain `dict` objects (never `None`), so the two lines that follow overwrite the enriched results with themselves — they can't change anything. This violates the "no superfluous parts" principle and could mislead future readers into thinking these vars might still be `None` at this point.

```suggestion
        person_properties, group_properties = (
            self._add_local_person_and_group_properties(
                distinct_id,
                groups or {},
                person_properties or {},
                group_properties or {},
            )
        )
        groups = groups or {}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: posthog/test/test_feature_flags.py
Line: 452-460

Comment:
**Prefer parameterised tests over repeated helper-based tests**

The instructions for this repo say "We always prefer parameterised tests." Several of the new tests in `TestGetFeatureFlagsBulk` follow the same structure (set up a mock response, call `get_feature_flags`, assert on capture count and/or properties) while varying only one axis at a time — for example, `test_no_events_when_send_feature_flag_events_is_false` vs `test_returns_feature_flag_result_per_key_from_single_remote_call`. These would read more concisely and be easier to extend as a `@parameterized.expand` suite (or with `subTest`).

**Context Used:** Do not attempt to comment on incorrect alphabetica... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "feat: add get_feature_flags for bulk fla..." | Re-trigger Greptile

@marandaneto marandaneto requested a review from a team April 24, 2026 05:39
@posthog-project-board-bot posthog-project-board-bot Bot moved this to In Review in Feature Flags Apr 24, 2026
Comment thread posthog/__init__.py
disable_geoip=None, # type: Optional[bool]
device_id=None, # type: Optional[str]
flag_keys_to_evaluate=None, # type: Optional[list[str]]
send_feature_flag_events=False, # type: bool
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iirc there was rfc to kill this flag due to its side effect
https://github.com/PostHog/requests-for-comments-internal/pull/1020 i think
so before adding this flag to more sdks, should we double check if we actually want to do this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so my interpretation of that SDK is to remove it from methods like e.g. capture, since we really don't need to send flag data for non-flag calls. Me adding to to get_all_flags here was to make it so that people could still use get_all_flags and send the flag values as events. what I noticed this week when talking to a customer was that they basically call get_all_flags once as part of the initial page load, save those values into their own app memory, and then use them throughout. This would make it so that they don't have to refactor their code to use my proposed new get_feature_flags() method, and could instead just add a field to their existing code.

but I read that RFC and it's a much saner approach IMO, so maybe rather than me adding this new method that follows the existing patterns, I should do the heavier lift and just implement the RFC. The reason I'm hesitant is because it turns this into a heavier lift for the customer – rather than them just bumping the SDK and adding this new method, they have to bump to a bigger version and deprecate their other methods everywhere. Maybe in LLM world, that's just as easy, but I wanted to be cognizant of that.

@dmarticus
Copy link
Copy Markdown
Contributor Author

closing in favor of a more principled approach, see https://posthog.slack.com/archives/C07Q2U4BH4L/p1777054243923129

@dmarticus dmarticus closed this Apr 25, 2026
@github-project-automation github-project-automation Bot moved this from In Review to Done in Feature Flags Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants