Repository navigation
Conversation
A record the server marks failed reads as disabled, so it now yields no payload, even when it still carries metadata.payload. get_feature_flag_payload(), get_feature_flag_result().payload and a snapshot's get_flag_payload() return None for it, and its $feature_flag_called events leave out $feature_flag_payload. get_all_flags_and_payloads() and get_feature_payloads() already left it out, because they keep only enabled flags. This holds for /flags?v=4 records and for records from older servers. The 7.82.1 comparison's failed flag now carries a payload, and the fixture is re-recorded with posthog 7.82.1. Every other recorded result is unchanged. The test names the payload difference for the getters and for the server-failed cache history.
Contributor
posthog-python Compliance ReportDate: 2026-11-05T17:53:39.185695+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
|
5 tasks done
Co-Authored-By: Claude Opus 5.5 <[email protected]>
There was a problem hiding this comment.
Approved.
The disclosed contract and model changes have current-head independent assurance and a STRONG-familiarity author, with targeted regression coverage. No unresolved review concerns or other refusal grounds were found.
- Author wrote 100% of the modified lines and has 5 merged PRs in these paths (familiarity STRONG).
- PostHog Review (Standard) reviewed the current head: no findings.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 18L, 1F substantive, 186L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (186L, 6F, two-areas, fix) |
| stamphog 2.7.0 | .stamphog/policy.yml @ 2fb783e · reviewed head 2fb783e |
dustinbyrne
approved these changes
Nov 6, 2026
This branch has not been deployed
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
A
/flagsrecord the server marksfailedreads as disabled, but the legacy payload reads still returned the payload it carried. Thereaders.v1_failedcase in the feature flag rules v2 contract is such a record: a producer-valid, failed, config version 1 record that still carriesmetadata.payload{"columns":2}. Before this change,get_feature_flag_payload()returned{"columns": 2}for a flag that reads as disabled.The contract now makes this explicit: a failed record never yields a payload from the legacy payload accessor (PostHog/posthog-sdk-test-harness#101).
What changes for a failed record:
get_feature_flag_payload()Noneget_feature_flag_result().payloadNoneget_flag_payload(), sync and asyncNoneget_all_flags_and_payloads(),get_feature_payloads()$feature_flag_called$feature_flag_payloadThe fix is in the record parser,
posthog/types.py: a failed record getsmetadata.payload = None, so every read path follows from it. This covers/flags?v=4records and records from older servers. Values, reasons and the rest of the metadata are unchanged.A side effect is in the fallback cache, which stores what a read returned. After a failed record, a later read that falls back to the cache gets the disabled result without the payload, where 7.82.1 returned the payload.
💚 How did you test it?
test_a_failed_record_has_no_payload_on_any_read: uses the vendoredreaders.v1_failedcase and an older-server failed record. It checks the parsed record,get_feature_flag_result(),get_feature_flag_payload()with and without events,get_all_flags_and_payloads(),get_feature_payloads()and a snapshot read. A second test checks the async client's snapshot. All three tests fail without the fix.test_a_disabled_record_that_is_not_failed_keeps_its_payloadis the control: a disabled record withfailedfalse, null or absent, in both response shapes, keeps its payload on the parsed record,get_feature_flag_result(),get_feature_flag_payload()and a snapshot read, so a change that drops the payload from every disabled flag fails it.test_flags_released_behaviour.py: theserver-failedflag in the 7.82.1 scenarios now carries a payload. I re-recordedreleased_flags_7_82_1.jsonwith posthog 7.82.1, asreleased_flags_scenarios.pydescribes. Before changing the scenario, I checked that recording reproduces the committed file byte for byte. The only new differences from 7.82.1 are this payload, and the test names them for the getters and for thegood, server failed, timeoutcache history. Everything else still matches 7.82.1.uv run --frozen --all-extras pytest -q -p no:cacheprovider posthog/test. 6094 passed and 3 failed. The 3 failures are fork tests intest_metrics_autocapture.py. They also fail on an unchanged checkout when the whole suite runs on macOS, and they pass when that file runs alone.ruff format --check .,ruff check ., and mypy with the baseline filter all pass.📝 Checklist
If releasing new changes
sampo addto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
v2comparison shows it.🤖 Generated with Claude Code