feat(flags): surface payloads from local flag evaluation by slshults · Pull Request #199 · PostHog/posthog-rs

slshults · 2026-08-07T22:03:13Z

Problem

Local flag evaluation never surfaced feature flag payloads, even though the payloads ship in the definitions manifest and were already deserialized into the cache (FeatureFlagFilters.payloads). local_record() hardcoded payload: None, so FeatureFlagEvaluations::get_flag_payload() returned None for every locally-evaluated flag, silently.

It is worse than a missing feature: turning local evaluation on removes payload access that remote-only callers have. In evaluate_flags, the /flags round trip is skipped entirely when the caller passed flag_keys and local evaluation covered all of them, and even when /flags is called, locally-evaluated keys are skipped when merging the remote records. Either path loses the payload. The only thing that still worked was the deprecated get_feature_flag_payload(), which posts straight to /flags and costs a billed request per call.

Rust was the only PostHog server SDK with local evaluation that did not resolve payloads locally. Python, Go, and Node all do.

Reported by a customer using server-side local evaluation.

What changed

(See also #200 )

Wiring only. No public API change, no new types, no signature changes to the public evaluator methods (api/public-api.txt regenerates clean).

FlagCache::flag_payload(key, value) (src/local_evaluation.rs) looks the payload up by the matched value: "true" for a boolean match, the variant key for a multivariate one. It mirrors the existing has_experiment accessor, so it reads one field under the read lock instead of cloning the whole FeatureFlag.
local_record() (src/client/common.rs) takes the resolved payload and runs it through the pre-existing normalize_payload(), the same helper the remote path already uses.
Both callers (blocking.rs and async_client.rs) pass it through.

On the double-encoding trap

Payloads arrive JSON-encoded from both endpoints: filters.payloads[key] in the definitions manifest and metadata.payload in /flags?v=2 are literally the same stored value, and the flags service does not decode either one. So the correctness requirement is to apply the same decode on both paths, which normalize_payload already does for remote (parse a Value::String, fall back to the raw string when it does not parse). Reusing it means the same flag returns the same payload whichever path evaluated it. Tests cover all four shapes: JSON-encoded object, already-parsed object, double-encoded string, and an undecodable string that falls back raw.

A flag that evaluated false gets no payload

Deliberate, for parity with /flags: the flags service only computes get_matching_payload on a matching flag, so a disabled flag has no payload remotely either. Worth flagging that posthog-go and posthog-python do an unconditional "false" key lookup here, while posthog-js guards it out the way this PR does. Since PostHog never stores a "false" payload and the server never returns one, matching our own remote path seemed more important than matching Go's lookup shape. Happy to flip it if you disagree.

Data shape change, please read

Once the payload is populated, build_called_event_properties starts attaching $feature_flag_payload to $feature_flag_called events for locally-evaluated flags, where today it does not. That is the intended parity with remote evaluation, and the event-minimization allowlist strips the property when the gate is on (existing tests in v0_capture.rs / v1_capture.rs assert this and still pass). But the gate defaults to off, so for customers who have not opted into minimization this is an observable change to their own event data with a small ingestion cost attached. Calling it out rather than leaving it to be discovered.

A security review specifically checked where a payload can now travel: only into $feature_flag_called properties, which is the customer's own project. It does not reach logs, error hooks, panic messages, or exception capture, and the per-variant lookup cannot return another flag's or another variant's payload.

Tests

The bug shipped because of a test blind spot: every fixture in the repo built payloads: HashMap::new(), so the non-empty payload path had never been exercised (the same shape of gap that hid the cohort bug in #187, where every fixture used "cohorts": {}). New fixtures use a real non-empty payloads map:

boolean flag, payload under the "true" key
multivariate flag, payload keyed by variant, with a different payload on the other variant so reading the wrong one fails
JSON-encoded-string payload, asserting the decoded value
undecodable string payload, asserting the raw-string fallback
already-parsed payload, asserting passthrough
no payload at all, asserting None
a flag that evaluates false while carrying a "true" payload, asserting both that the flag is present as Boolean(false) and that its payload is None
end to end through evaluate_flags with flag_keys set so local_covers_request is true, asserting /flags is never called

Unit tests run against both the blocking and async clients, since the two duplicate this call site.

Not in this PR

The deprecated get_feature_flag_payload() still posts to /flags unconditionally instead of trying local first (which is what Go does). It is a real win, since it would stop charging people for payload lookups they could resolve locally, but it touches deprecated surface and is a separate behavior change. Happy to do it as a follow-up.

Base

Branched from #187 (cohort deserialization), which is open and touches the same files. Review that one first. This PR does not touch the cohort work.

Created with PostHog Code

greptile-apps · 2026-08-07T22:15:32Z

Prompt To Fix All With AI

### Issue 1
src/feature_flags.rs:1068
**Cyclic cohort recursion is unbounded**

When definitions contain cyclic cohort references, `match_nested_cohort` repeatedly re-enters `match_cohort_by_id` without tracking active IDs or limiting depth, causing local flag evaluation to exhaust the thread stack and potentially abort the process.

### Issue 2
src/client/async_client.rs:1067
**Payload crosses definitions snapshots**

If the definitions poller updates a flag after local evaluation but before this lookup, `flag_payload` reads the replacement cache and pairs the previously evaluated value with a new, changed, or absent payload, returning an internally inconsistent flag record. The blocking client has the same split lookup.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "feat(flags): surface payloads from local..." | Re-trigger Greptile}

greptile-apps · 2026-08-07T22:15:35Z

+        .and_then(|n| n.as_bool())
+        .unwrap_or(false);
+
+    let is_member = match_cohort_by_id(&cohort_id, properties, ctx)?;


Cyclic cohort recursion is unbounded

When definitions contain cyclic cohort references, match_nested_cohort repeatedly re-enters match_cohort_by_id without tracking active IDs or limiting depth, causing local flag evaluation to exhaust the thread stack and potentially abort the process.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/feature_flags.rs Line: 1068 Comment: **Cyclic cohort recursion is unbounded** When definitions contain cyclic cohort references, `match_nested_cohort` repeatedly re-enters `match_cohort_by_id` without tracking active IDs or limiting depth, causing local flag evaluation to exhaust the thread stack and potentially abort the process. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

The definitions manifest carries each flag's payloads, but locally evaluated flags always reported `payload: None`, so `get_flag_payload` returned nothing for them. Worse, turning local evaluation on removed payload access that remote-only callers had: when `flag_keys` is fully covered locally there is no `/flags` round trip left to recover the payload from, and locally evaluated keys are skipped when merging remote records. Local evaluation now resolves the payload for the matched value ("true" for a boolean match, the variant key for a multivariate one) and decodes it through the same `normalize_payload` the remote path uses, so a flag returns the same payload whichever path evaluated it. A flag that evaluated false gets no payload, matching `/flags`, which only attaches one to a matching flag. Generated-By: PostHog Code Task-Id: 52072a7c-7431-4b59-8de1-90d9ababf0db

slshults · 2026-08-07T22:25:22Z

Heads up for anyone mid-review: I force-pushed a rebase just now, sorry for the churn.

#187 was squash-merged, so the five commits this branch inherited from its branch were no longer in main's history. Even after GitHub retargeted the base to main, the diff was re-showing all of #187's changes as if they were part of this PR, including src/feature_flags.rs and api/public-api.txt. Greptile reviewed that duplicated code and left a comment on the cohort recursion, which is #187's code and already merged.

The branch is now a single commit replayed onto current main, and the diff is the 8 files that actually belong to this change. Nothing about the change itself moved. cargo fmt, cargo clippy -- -D warnings, and the test suite are green on the rebased tree, on both the default and blocking feature configurations.

Separately: the cohort recursion Greptile flagged looks like a genuine bug in the merged code, not a false positive. match_cohort_by_id -> match_property_group -> match_property_group_values -> match_nested_cohort -> back to match_cohort_by_id has no visited set and no depth cap, so a cohort that references itself, directly or transitively, recurses until the stack is exhausted. A Rust stack overflow aborts the process rather than panicking catchably, so that would take down a customer's server. I'm opening a separate PR for it rather than piling it in here.

(Claude in PostHog Code here, replying via Steven's account.)

github-actions · 2026-08-07T22:28:46Z

posthog-rs-v0 Compliance Report

Date: 2026-08-10 08:17:07 UTC
Duration: 15539ms

✅ All Tests Passed!

46/46 tests passed

Capture Tests

✅ 29/29 tests passed

View Details

Test	Status	Duration
Format Validation.Event Has Required Fields	✅	139ms
Format Validation.Event Has Uuid	✅	150ms
Format Validation.Event Has Lib Properties	✅	142ms
Format Validation.Distinct Id Is String	✅	148ms
Format Validation.Token Is Present	✅	153ms
Format Validation.Custom Properties Preserved	✅	139ms
Format Validation.Event Has Timestamp	✅	149ms
Retry Behavior.Retries On 503	✅	5143ms
Retry Behavior.Does Not Retry On 400	✅	2151ms
Retry Behavior.Does Not Retry On 401	✅	2151ms
Retry Behavior.Respects Retry After Header	✅	5092ms
Retry Behavior.Implements Backoff	✅	15100ms
Retry Behavior.Retries On 500	✅	5096ms
Retry Behavior.Retries On 502	✅	5092ms
Retry Behavior.Retries On 504	✅	5088ms
Retry Behavior.Max Retries Respected	✅	15090ms
Deduplication.Generates Unique Uuids	✅	92ms
Deduplication.Preserves Uuid On Retry	✅	5022ms
Deduplication.Preserves Uuid And Timestamp On Retry	✅	10040ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry	✅	5035ms
Deduplication.No Duplicate Events In Batch	✅	23ms
Deduplication.Different Events Have Different Uuids	✅	23ms
Compression.Sends Gzip When Enabled	✅	23ms
Batch Format.Uses Proper Batch Structure	✅	22ms
Batch Format.Flush With No Events Sends Nothing	✅	22ms
Batch Format.Multiple Events Batched Together	✅	101ms
Error Handling.Does Not Retry On 403	✅	2060ms
Error Handling.Does Not Retry On 413	✅	2078ms
Error Handling.Retries On 408	✅	5081ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details

Test	Status	Duration
Request Payload.Request With Person Properties Device Id	✅	70ms
Request Payload.Flags Request Uses V2 Query Param	✅	49ms
Request Payload.Flags Request Hits Flags Path Not Decide	✅	20ms
Request Payload.Flags Request Omits Authorization Header	✅	38ms
Request Payload.Token In Flags Body Matches Init	✅	38ms
Request Payload.Groups Round Trip	✅	36ms
Request Payload.Groups Default To Empty Object	✅	30ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False	✅	50ms
Request Payload.Disable Geoip Omitted Defaults To False	✅	35ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key	✅	35ms
Request Lifecycle.No Flags Request On Init Alone	✅	26ms
Request Lifecycle.No Flags Request On Normal Capture	✅	60ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests	✅	57ms
Request Lifecycle.Mock Response Value Is Returned To Caller	✅	41ms
Retry Behavior.Retries Flags On 502	✅	247ms
Retry Behavior.Retries Flags On 504	✅	244ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event	✅	43ms

github-actions · 2026-08-07T22:29:12Z

posthog-rs-v1 Compliance Report

Date: 2026-08-10 08:18:14 UTC
Duration: 23069ms

✅ All Tests Passed!

111/111 tests passed

Capture_V1 Tests

✅ 94/94 tests passed

View Details

Test	Status	Duration
Endpoint And Method.Targets V1 Endpoint	✅	152ms
Endpoint And Method.Does Not Use Legacy Endpoints	✅	153ms
Required Headers.Has Authorization Bearer Header	✅	153ms
Required Headers.Has Content Type Json	✅	154ms
Required Headers.Has Posthog Sdk Info Format	✅	151ms
Required Headers.Has Posthog Attempt Header	✅	152ms
Required Headers.Has Posthog Request Id	✅	155ms
Required Headers.Has Posthog Request Timestamp	✅	152ms
Required Headers.Has User Agent	✅	151ms
Body Format.Body Has Created At And Batch	✅	151ms
Body Format.No Api Key In Body	✅	129ms
Body Format.No Sent At In Body	✅	128ms
Event Format.Event Has Required Root Fields	✅	131ms
Event Format.Event Uuid Is Valid	✅	129ms
Event Format.Event Timestamp Is Rfc3339	✅	131ms
Event Format.Distinct Id Is String	✅	131ms
Event Format.Distinct Id At Root Not Properties	✅	132ms
Event Format.Custom Properties Preserved	✅	135ms
Event Format.Set Properties Preserved	✅	131ms
Event Format.Set Once Properties Preserved	✅	126ms
Event Format.Groups Properties Preserved	✅	135ms
Event Format.Sdk Generates Uuid If Not Provided	✅	136ms
Event Format.Event Has Required Root Fields Batch	✅	173ms
Event Format.Event Uuid Is Valid Batch	✅	171ms
Event Format.Event Timestamp Is Rfc3339 Batch	✅	170ms
Event Format.Distinct Id Is String Batch	✅	169ms
Event Format.Distinct Id At Root Not Properties Batch	✅	180ms
Event Format.Custom Properties Preserved Batch	✅	180ms
Event Format.Set Properties Preserved Batch	✅	178ms
Event Format.Set Once Properties Preserved Batch	✅	177ms
Event Format.Groups Properties Preserved Batch	✅	154ms
Event Format.Sdk Generates Uuid If Not Provided Batch	✅	153ms
Batch Behavior.Multiple Events In Single Batch	✅	180ms
Batch Behavior.Batch Envelope Smoke	✅	167ms
Batch Behavior.Flush With No Events Sends Nothing	✅	103ms
Batch Behavior.Flush At Triggers Batch	✅	1128ms
Batch Behavior.Created At Reflects Batch Creation Time	✅	118ms
Deduplication.Generates Unique Uuids	✅	218ms
Deduplication.Different Events Same Content Different Uuids	✅	157ms
Deduplication.Preserves Uuid On Retry	✅	5123ms
Deduplication.Preserves Timestamp On Retry	✅	5082ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry	✅	5119ms
Deduplication.No Duplicate Events In Batch	✅	135ms
Header Behavior On Retry.Attempt Header Starts At One	✅	91ms
Header Behavior On Retry.Attempt Header Increments On Retry	✅	10070ms
Header Behavior On Retry.Request Id Preserved On Retry	✅	5060ms
Header Behavior On Retry.Different Requests Have Different Request Ids	✅	2066ms
Header Behavior On Retry.Request Timestamp Changes On Retry	✅	5044ms
Response Format Validation.Success Response Has Uuid Keyed Results	✅	48ms
Response Format Validation.Success Response Has Ok For Each Event	✅	47ms
Response Format Validation.Success No Retry After When All Ok	✅	32ms
Response Format Validation.Success Retry After Present When Retry Events	✅	33ms
Response Format Validation.Success No Retry After When Drop Only	✅	33ms
Response Format Validation.Response Echoes Request Id	✅	26ms
Retry Behavior.Retries On 408	✅	5033ms
Retry Behavior.Retries On 500	✅	5026ms
Retry Behavior.Retries On 503	✅	5026ms
Retry Behavior.Retries On 504	✅	5025ms
Retry Behavior.Retryable Errors Have Retry After	✅	2025ms
Retry Behavior.Respects Retry After On Retryable Error	✅	8025ms
Retry Behavior.Does Not Retry On 400	✅	2032ms
Retry Behavior.Does Not Retry On 401	✅	2033ms
Retry Behavior.Does Not Retry On 402	✅	2024ms
Retry Behavior.Does Not Retry On 413	✅	2032ms
Retry Behavior.Does Not Retry On 415	✅	2030ms
Retry Behavior.Non Retryable Errors Have No Retry After	✅	2026ms
Retry Behavior.Implements Backoff	✅	15030ms
Retry Behavior.Max Retries Respected	✅	15055ms
Partial Batch Handling.Handles 200 Full Success	✅	2047ms
Partial Batch Handling.Handles 200 With All Ok	✅	3050ms
Partial Batch Handling.Does Not Retry Dropped Events	✅	3036ms
Partial Batch Handling.Does Not Retry Limited Events	✅	3030ms
Partial Batch Handling.Prunes Ok Events On Partial Retry	✅	5031ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry	✅	5027ms
Partial Batch Handling.Retries Only Retry Events From Partial	✅	5028ms
Partial Batch Handling.Partial Retry Preserves Uuids	✅	5024ms
Partial Batch Handling.Partial Retry Attempt Header Increments	✅	5031ms
Partial Batch Handling.Partial Retry Request Id Preserved	✅	5032ms
Partial Batch Handling.Respects Retry After On Partial	✅	5026ms
Partial Batch Handling.Unknown Result Treated As Terminal	✅	3026ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry	✅	3033ms
Compression.Sends Gzip Content Encoding	✅	35ms
Compression.No Content Encoding When Disabled	✅	22ms
Compression.Compressed Body Is Decompressible	✅	23ms
Error Handling.Does Not Retry On Unknown 4Xx	✅	2024ms
Event Options.Cookieless Mode Override	✅	24ms
Event Options.Disable Skew Correction Override	✅	23ms
Event Options.Process Person Profile Override	✅	21ms
Event Options.Product Tour Id Override	✅	21ms
Event Options.Unset Options Omitted	✅	24ms
Event Options.Options Override In Batch	✅	22ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties	✅	23ms
Geoip And Historical Migration.Historical Migration Set In Body	✅	22ms
Geoip And Historical Migration.Historical Migration Absent By Default	✅	23ms

Feature_Flags Tests

✅ 17/17 tests passed

View Details

Test	Status	Duration
Request Payload.Request With Person Properties Device Id	✅	15ms
Request Payload.Flags Request Uses V2 Query Param	✅	15ms
Request Payload.Flags Request Hits Flags Path Not Decide	✅	15ms
Request Payload.Flags Request Omits Authorization Header	✅	16ms
Request Payload.Token In Flags Body Matches Init	✅	15ms
Request Payload.Groups Round Trip	✅	15ms
Request Payload.Groups Default To Empty Object	✅	17ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False	✅	15ms
Request Payload.Disable Geoip Omitted Defaults To False	✅	17ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key	✅	15ms
Request Lifecycle.No Flags Request On Init Alone	✅	11ms
Request Lifecycle.No Flags Request On Normal Capture	✅	26ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests	✅	18ms
Request Lifecycle.Mock Response Value Is Returned To Caller	✅	16ms
Retry Behavior.Retries Flags On 502	✅	219ms
Retry Behavior.Retries Flags On 504	✅	219ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event	✅	24ms

marandaneto · 2026-08-10T08:14:46Z

pushed a fix

Base automatically changed from posthog-self-driving/fixflags-deserialize-cohorts-correctly-ff65ce to main August 7, 2026 22:12

slshults added feature/feature-flags team/feature-flags labels Aug 7, 2026

slshults requested review from a team and marandaneto August 7, 2026 22:12

slshults marked this pull request as ready for review August 7, 2026 22:13

slshults requested a review from a team as a code owner August 7, 2026 22:13

greptile-apps Bot reviewed Aug 7, 2026

View reviewed changes

slshults force-pushed the posthog-code/local-eval-flag-payloads branch from f99eafc to 533e153 Compare August 7, 2026 22:25

slshults mentioned this pull request Aug 7, 2026

fix(flags): bound cohort recursion in local evaluation #200

Merged

marandaneto reviewed Aug 8, 2026

View reviewed changes

Comment thread src/client/async_client.rs Outdated

marandaneto requested a review from a team August 8, 2026 07:17

marandaneto and others added 2 commits August 10, 2026 09:59

fix(flags): keep local payloads snapshot-consistent

346fa59

Merge branch 'main' into posthog-code/local-eval-flag-payloads

ecdcc89

marandaneto approved these changes Aug 10, 2026

View reviewed changes

slshults merged commit ea92c93 into main Aug 10, 2026
25 checks passed

slshults deleted the posthog-code/local-eval-flag-payloads branch August 10, 2026 15:39

Navigation Menu

Uh oh!

feat(flags): surface payloads from local flag evaluation - #199

feat(flags): surface payloads from local flag evaluation#199
slshults merged 3 commits into
mainfrom
posthog-code/local-eval-flag-payloads

slshults commented Aug 7, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 7, 2026

Uh oh!

greptile-apps Bot Aug 7, 2026

Uh oh!

Uh oh!

slshults commented Aug 7, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

marandaneto commented Aug 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

slshults commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

What changed

On the double-encoding trap

A flag that evaluated false gets no payload

Data shape change, please read

Tests

Not in this PR

Base

Uh oh!

greptile-apps Bot commented Aug 7, 2026

Uh oh!

greptile-apps Bot Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

slshults commented Aug 7, 2026

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

posthog-rs-v0 Compliance Report

✅ All Tests Passed!

Capture Tests

Feature_Flags Tests

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

posthog-rs-v1 Compliance Report

✅ All Tests Passed!

Capture_V1 Tests

Feature_Flags Tests

Uh oh!

Uh oh!

marandaneto commented Aug 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

slshults commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading

github-actions Bot commented Aug 7, 2026 •

edited

Loading