fix(flags): parse locally-evaluated flag payloads in evaluate_flags() (sdk-specs local-feature-flag-evaluator) - #828
Draft
posthog[bot] wants to merge 1 commit into
Conversation
The evaluate_flags() snapshot JSON-decoded payloads coming back from /flags but stored locally-evaluated payloads verbatim, so get_flag_payload() returned a dict for remotely-resolved flags and the raw JSON string for locally-resolved ones. The sdk-specs local-feature-flag-evaluator and get-feature-flag-payload contracts both expect the decoded value. Both branches now go through a shared _parse_flag_payload() helper; strings that aren't valid JSON are still passed through unchanged. Generated-By: PostHog Code Task-Id: 9b7661cd-0281-4019-b9ee-516e13639286
Contributor
posthog-python Compliance ReportDate: 2026-08-04 08:04:12 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
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
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.
💡 Motivation and Context
Compliance gap against the cross-SDK contracts in PostHog/sdk-specs.
openspec/specs/local-feature-flag-evaluator/spec.md— Behavior #8 "Resolve payload from the chosen value", Scenario "Evaluator resolves payload from the matched value":openspec/specs/get-feature-flag-payload/spec.mdsays the same thing about the read path:What was out of compliance. In
Client.evaluate_flags(), the remote branch JSON-decodesmetadata.payloadbefore building the_EvaluatedFlagRecord, but the local branch stored the value straight from the flag definition — where payloads live as JSON strings. So the payload type of the same flag depended on where it happened to resolve:The rest of the SDK already treats "payload" as the decoded value —
FeatureFlagResult.from_value_and_payload/from_flag_detailsbothjson.loads, soget_feature_flag_result(...).payloadreturns a dict for the very same locally-evaluated flag thatevaluate_flags(...).get_flag_payload(...)returned a string for. The same raw string also leaked into$feature_flag_payloadon$feature_flag_calledevents for locally-evaluated flags, while the remote path sent the decoded value (the existing test atposthog/test/test_feature_flag_called_minimization.pynotes "The evaluate_flags path JSON-parses the payload before attaching it" — which held only for remote).How this fixes it. Both branches now route through one
_parse_flag_payload()helper, so a decoded payload is what callers see regardless of evaluation source. The helper preserves the remote branch's existing semantics exactly: an empty string becomesNone, a string that isn't valid JSON is passed through unchanged, and non-string values (a number or an already-structured payload) are untouched.Behavior change / compatibility risk.
evaluate_flags(...).get_flag_payload(key)and$feature_flag_payloadnow return the decoded payload for locally-evaluated flags instead of a JSON string. Anyone who calledjson.loads()on that result will need to drop it — but only for flags that resolved locally, since the same code already received a decoded value on the remote path, so no caller could have relied on the string shape reliably. No public API, signature, config, or event-name changes.Deliberately scoped to the
evaluate_flags()snapshot.get_all_flags_and_payloads()returns JSON strings on both its local and remote paths, so it is internally consistent today; changing it is a wider, unambiguously breaking change and belongs in its own discussion.💚 How did you test it?
Added
TestEvaluateFlagsLocalPayloadsinposthog/test/test_evaluate_flags.py, covering local evaluation of a multivariate and a boolean flag (decoded payload, no/flagsrequest), a non-JSON payload string (passed through), and the$feature_flag_payloadproperty on$feature_flag_called.Verified the two payload assertions fail without the fix (
AssertionError: '{"copy": "new"}' != {'copy': 'new'}) and pass with it. Also rantest_evaluate_flags.py,test_feature_flags.py,test_feature_flag.py,test_feature_flag_result.py,test_feature_flag_called_minimization.py,test_feature_flag_has_experiment.py,test_types.py,test_client.py,test_module.py,test_contexts.pyandposthog/test/features— all passing.ruff format --check,ruff check,mypy, andcheck_public_api.pyare clean (no public API surface change).📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Fully autonomous
Opened by the scheduled "SDK Spec Compliance Enforcer" loop, which audits posthog-python against the contracts in
PostHog/sdk-specsand files one focused fix per run. This run checked the server-SDK-applicable requirements across ~40 specs (capture/exception, batching + HTTP/retry, flag getters, flag internals, identity, logs/traces/tracing-headers) using Claude Code with read-only search plus targeted runs of the SDK against mocked transports.This finding was chosen over the other candidates because it is narrow, verifiable against two spec files, and internally corroborated: the SDK's own
FeatureFlagResultand the remote branch of the same function already decode payloads, so this is the recommended non-deprecated API regressing against a convention the rest of the codebase keeps. Candidates deliberately not filed: the group-flag-without-group-context "conclusiveFalse" behavior (the code comment marks it intentional and it is shared with other server SDKs — needs a cross-SDK decision),get_all_flags_and_payloads()payload strings (wider breaking change), and missing input validation onalias()/group_identify()(real but separate; would drop events users send today).Agent-authored — please review rather than rubber-stamp, especially the compatibility call on returning decoded payloads.
Created with PostHog Code