Repository navigation
feat(flags): read the /flags?v=4 response - #1031
Conversation
Request /flags?v=3 from the sync and async clients and keep each flag's typed value, structured reason and metadata on the parsed record. - A record with a value member is a v3 record. A record without one comes from an older server: its value is variant, or enabled without one, and its config version is 1. - A known field with the wrong JSON type fails only that record with PARSE_ERROR; unknown fields are ignored. - Legacy getters, callbacks and $feature_flag_called keep reading the legacy rendering, now derived from value in one place: a string is the variant, null is disabled, and a number or object is enabled with the value as a JSON-encoded payload. - evaluate_flags() snapshots gain get_boolean/string/number/object_value and _details accessors. They never coerce and return the caller default for missing, failed, null and mismatched values.
posthog-python Compliance ReportDate: 2026-11-02T19:40:56.890595+00:00 ✅ All Tests Passed!121/121 tests passed Capture_V1 Tests✅ 95/95 tests passed View Details
Capture_Ai Tests✅ 5/5 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
A v3 record for a flag in the new format carries no experiment ID: an experiment is linked to its rule on the server, by rule ID. A complete experiment split is config version 2, reason experiment_split, rule type experiment, a rule ID and a variant key. - Remove experiment_id from FlagMetadata. An experiment_id key in a record is now ignored like any other unknown metadata key, whatever its type. - The vendored harness 1.13.1 fixtures still carry the field. Two reader cases that depend on it are overridden in the test until the fixtures are re-pinned to a contract without it.
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 811c6c2: 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
A v3 record carries no holdout ID: holdouts in the new format are local to their rule and have no ID, and older records never sent one. A holdout result is identified by its rule ID and the reason holdout. - Remove holdout_id from FlagMetadata. A holdout_id key in a record is now ignored like any other unknown metadata key, whatever its type. - The vendored harness 1.13.1 fixtures still send the field. No reader case depends on it; the producer-response test skips it, like experiment_id, until the fixtures are re-pinned to a contract without it.
|
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
Clients already send v=3 and arbitrary v values, so the server selects the typed record only for an exact v=4. Request v=4 and name the record after it in tests, docstrings and the changeset. A server that answers v=4 with the older record (enabled/variant, no value) is still read the same way.
Align the evaluate_flags() typed accessors with the typed flag values API proposed in PostHog/sdk-specs#104. - FlagEvaluationDetails has an OpenFeature reason (FlagEvaluationReason) and a flag_metadata dict of the record's id, version, config_version, reason_code, condition_index, rule_type, rule_id and variant_key. It no longer carries the structured reason or the metadata object, so the payload stays on the payload accessors. - variant is set only when the accessor returns the flag's value. - Locally evaluated flags read with config_version 1 and a reason derived from the value, or DISABLED for an inactive flag. - Each client logs once when /flags sends records without typed values, and a TYPE_MISMATCH message says why the value was not returned. - Add get_integer_value/details and get_float_value/details. Object values are returned as copies. - Re-vendor the rules v2 contract fixtures from posthog-sdk-test-harness 1.22.0 (contract 7.0.0). A config version 1 record may carry an integer metadata.experiment_id, so its type is checked; it is not kept.
The adapter sends /flags?v=4, so it declares flags_response_v4. A harness that knows the capability runs flags_request_uses_v4_query_param in place of the ?v=2 check; older harness releases ignore it.
# Conflicts: # posthog/client.py # posthog/feature_flag_evaluations.py
Keep reading the /flags?v=4 response and retaining each flag's typed value, reason and metadata, with the existing getters derived from the value. Drop the public typed accessors (get_boolean_value/_details and the string, number, integer, float and object forms), FlagEvaluationDetails, FlagEvaluationReason and FlagEvaluationErrorCode, and the error code and message fields on FeatureFlag that only they read. They return in a separate change once the cross-SDK accessor API is agreed.
A record without a typed value now has its known fields type-checked like a /flags?v=4 record, so a wrong-typed field fails only that flag instead of the whole response. A key member of the wrong type fails its record; the map key stays the flag key. An unreadable record keeps the field that failed it, apart from a record the server marks failed.
A record without metadata.id or metadata.version no longer reads them as 0. Events already left out a falsy ID and version, so their properties do not change.
A /flags?v=4 record with a number or object value exposes that value as its payload, even when the record also sends a payload. Other payloads are kept as sent.
A value nested too deeply for json.dumps used to abort the whole response. Also test an empty-string reason, and the async client with unreadable and sparse records beside a healthy one. The changeset now says which results can differ from before.
get_flags_decision() returns FeatureFlag and FlagMetadata, so public fields could be adopted before the typed flag API is settled. The typed accessors make them public.
#1031 keeps the v4 value and rule metadata fields private. The typed accessors make them public, so this branch keeps them public and the changeset lists them.
…ields The typed value, failure flags and rule metadata were declared as dataclass fields, so repr, equality, hashing, dataclasses.asdict and the generated SDK reference showed them. Keep them as instance attributes with class-level defaults instead, import FeatureFlag privately in the snapshot module, and say in the changeset that the details stay internal.
A /flags record with a known field of the wrong type reads as disabled. The single-flag getter cached that result, replacing the last good value, so a later failed request fell back to False. An unreadable record now leaves the fallback cache alone and falls back to its value, like a failed request does; without a cached value it still reads as disabled.
FeatureFlag.from_json delegated to record parsers that always built a FeatureFlag, so a subclass calling it got a base instance. The parsers now take the class to build, for v4, older and unreadable records alike.
A snapshot kept each remotely evaluated flag twice: a flattened record and the parsed /flags record, with the payload both as its JSON text and decoded. The snapshot record now reads its fields from the parsed record and decodes the payload on its first read, then keeps it. Private record details that equal their defaults are no longer stored on each instance. Retained memory of evaluate_flags() for a /flags?v=4 response of 5,000 boolean flags (tracemalloc, Python 3.14, including JSON decoding): no payloads: 3.64 MiB -> 2.87 MiB 1 KiB JSON payload each: 28.10 MiB -> 7.95 MiB A payload that is read is decoded and kept as before, so reading every payload retains about what it did before this change.
The parity tests compare this SDK's v2 and v4 readers with each other, so a change shared by both passes. These tests compare both with results recorded once with posthog 7.82.1 for the same flags: the legacy getters, get_flags_decision, snapshot reads, $feature_flag_called properties and fallback-cache histories. Where a result differs from 7.82.1 by decision, the test says why: an absent flag ID or version reads as None, not 0, and a record with a field of the wrong type fails only itself.
A single-flag read of a record that cannot be read used to fail the whole request, so its $feature_flag_called event carried $feature_flag_error: unknown_error and no request id or evaluated-at. The read now succeeds for the other records, but this one still reports the same error and no request details, so a value served from the fallback cache stays distinguishable from a fresh one.
marandaneto
left a comment
There was a problem hiding this comment.
The code looks good; no qualifying findings. Verdict: correct.
(cherry picked from commit e094dc5)
* feat(flags): read the /flags?v=4 response (#1031) (cherry picked from commit e094dc5) * chore: Release v7.83.0 [skip ci] (cherry picked from commit 43c7545) * Update generated references (cherry picked from commit 314fad0) * fix(mcp): inject context into mcp 2.x tool objects and test without fastmcp (#1166) * test(mcp): share the dual-major structured content reader * test(mcp): read structured content without the fastmcp camelcase shim * fix(mcp): inject context into mcp 2.x tool objects * ci: run the mcp 2.x suite without fastmcp (cherry picked from commit 0c12ca1) * chore: Release v7.83.1 [skip ci] (cherry picked from commit 9f5d458) * Update generated references (cherry picked from commit 06e5611) * test(flags): read flag scenario events in before_send on v1 v1 passes flag properties to _enqueue separately and layers them while building the event, so a stubbed _enqueue never saw them. Reading the built event in before_send compares v1's real output with 7.82.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Anders <754494+andehen@users.noreply.github.com> Co-authored-by: releaser-posthog-python[bot] <262414804+releaser-posthog-python[bot]@users.noreply.github.com> Co-authored-by: Connor Grady <50903811+connor-grady@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
💡 Motivation and Context
The flags service can answer with a typed record (PostHog/posthog#107732), selected by
/flags?v=4(PostHog/posthog#123366). Clients already sendv=3and arbitraryvvalues, so only an exactv=4selects the new record. A v4 record carries one typedvalue(boolean, string, number, object or null) instead ofenabledandvariant, plus a structuredreasonand rule metadata.This PR moves the SDK to that response and keeps the full record for each flag, while every existing getter and event returns the same values as before for valid flag records. It adds no new public methods and does not change event emission. The typed accessor API (
get_boolean_value/_detailsand the string, number, integer, float and object forms) is split out into #1147, which stacks on this PR and waits for the cross-SDK accessor spec in PostHog/sdk-specs#104.The reader follows the wire contract published in posthog-sdk-test-harness 1.22.0 (
contracts/feature_flag_rules_v2, contract 7.0.0):/flags/?v=4. A record withvalueis a v4 record. A record without it comes from a server that does not serve v4, which answersv=4with the older record. Its value isvariant, orenabledwhen there is no variant, its config version is 1, and any rule context in it is ignored. No probing or retry. Each client logs one diagnostic (at info level) the first time it reads such a response. There is deliberately no retry withv=2: the flags service answers avit does not serve with an older record rather than an error, so a fallback request would only repeat requests that failed for other reasons (an outage, a quota limit), doubling their load and hiding the error.keymember of the wrong type is such a field; a stringkeythat differs from the map key is ignored, and the map key stays the flag key. An unreadable record:$feature_flag_error: "unknown_error", with no request ID or evaluated-at. So a stale cached value is told apart from a fresh one.valuein one place, when the record is parsed: a boolean as is, a string as the variant,nullasfalse, and a number or object astruewith the value as its payload. v4 sendsmetadata.payload: nullfor number and object flags, so the SDK supplies the payload from the value (the same JSON value, not necessarily the same text), and ignores a payload a record sends with a number or object value. Payloads for boolean and string values are kept as sent.get_feature_flag,get_feature_flag_result,get_feature_flag_payload,get_all_flags(_and_payloads), the snapshot getters,$feature/<key>,$active_feature_flagsand$feature_flag_calledreturn the same values for both responses.evaluate_flags()snapshot keeps its parsed record internally (typed value, failure state, reason and metadata), for the follow-up accessors. The snapshot keeps one copy per flag and decodes payloads only when they are read.Notes for reviewers
FeatureFlagkeeps the typed value and failure state in private attributes (_value,_failed,_malformed_field), andFlagMetadatakeeps the v4 rule metadata in private attributes (_config_version,_rule_type,_rule_id,_variant_key,_forced_variant). They are instance attributes with class-level defaults, not dataclass fields, sorepr, equality, hashing,dataclasses.asdict()and the generated SDK reference show only the existing fields (a test pins this). They are private on purpose:get_flags_decision()returns these types, so public fields could be adopted before the typed flag API settles. feat(flags): typed flag value accessors #1147 makes them public together with the typed accessors. No new public fields, types, exports or methods are added.FlagMetadata.idandFlagMetadata.versionare nowNonewhen a record has none, instead of0; events already left a falsy ID and version out, so event properties do not change.FeatureFlag.from_json(),FlagMetadata.from_json()andFlagReason.from_json()build the subclass they are called on, as in 7.82.1.v=2; only float exponent spelling can differ (1e-07here,1e-7from the server).FlagMetadata.idandFlagMetadata.versionwiden frominttoOptional[int](absent staysNoneinstead of0), and a record with a known field of the wrong JSON type, such as a stringreason, an empty-stringreasonor a non-booleanhas_experiment, now reads as described in point 2. Before, such a record was read anyway or failed the whole response. A value nested too deeply to encode also fails only its own record. When nothing is cached, two results for the unreadable flag itself differ from 7.82.1, where the whole response failed: a single-flag getter returnsFalseinstead ofNone, and a snapshot read of it sends its event withoutflag_missing. Its sibling flags now read normally instead of failing with it.reason.description("Matched rule 2" instead of "Matched condition set 2") andreason.code(no_rule_matchinstead ofno_condition_match).FeatureFlagResult.reasonand$feature_flag_reasonshow the v4 text once a server sends v4. Values do not change, and v1 flags are identical in every field.FlagMetadatakeeps noexperiment_idorholdout_id. Contract 7.0.0 lets only config version 1 records carrymetadata.experiment_id, so the reader checks that it is an integer (a wrong type fails the record) and does not keep it.evaluate_flags()snapshot of 5,000 flags: 2.87 MiB without payloads (7.82.1: 1.50 MiB) and 7.95 MiB with 1 KiB payloads (7.82.1: 20.9 MiB). The extra cost without payloads is the parsed record each flag keeps.flags_response_v4capability, and CI runs harness 1.23.0, which checksflags_request_uses_v4_query_paraminstead of the?v=2check.minor.💚 How did you test it?
posthog/test/test_flags_released_behaviour.pycompares this SDK with results recorded by posthog 7.82.1.posthog/test/released_flags_scenarios.pyuses only the public client API, so the same scenarios run on 7.82.1 (which recordedposthog/test/fixtures/released_flags_7_82_1.json; the module says how to regenerate it) and on this SDK, from both thev2and thev4response shapes. They cover every legacy getter,get_flags_decision, the snapshot getters,capture(flags=...),capture(send_feature_flags=True), every$feature_flag_calledevent, responses with an unreadable record, and fallback cache histories (good, then unreadable, then a timeout or error). The only expected differences are the absent ID and version and the unreadable flag's own value, and the test says which decision causes each one.posthog/test/test_flags_v4_response.py(153 tests), built on fixtures copied unchanged from harness release 1.22.0.posthog/test/fixtures/feature_flag_rules_v2/SOURCE.jsonrecords the release, revision and file digests, and a test checks them. It covers:from_json()on subclasses;ruff format --check,ruff check, mypy with the baseline filter, and the public API snapshot check.flags_request_uses_v4_query_param.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code, an AI agent, from a brief by the assignee. The agent extended the existing parsing path rather than adding a parallel one: the parsed
FeatureFlagcarries the typed value, and the legacy fields are derived from it once. An earlier revision moved the typed accessor API to #1147, so this change ships without new public methods. A later review round kept unreadable records out of the fallback cache, kept theunknown_error7.82.1 reported for them, builtfrom_json()subclasses again, reduced snapshot memory, and pinned legacy results to a 7.82.1 recording instead of comparing the new SDK with itself. Choices the reviewer may want to revisit are listed under "Notes for reviewers".🤖 Generated with Claude Code