Repository navigation
fix: treat failed feature flag records as unevaluated - #1238
Merged
Merged
Conversation
Contributor
posthog-python Compliance ReportDate: 2026-11-13T14:12:34.661753+00:00 ✅ All Tests Passed!271/271 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_V4 Tests✅ 150/150 tests passed View Details
Feature_Flags_Local_Evaluation Tests✅ 4/4 tests passed View Details
|
Contributor
SDK performance: posthog-pythonBenchmarks completed. No changes flagged for review. Flags require at least a 5% change in the median and all six pairs agreeing on direction. This is a noise filter, not proof of a performance change or a CI gate. View all results
Values are medians of paired measurements. Not flagged does not mean unchanged. Unavailable means a measurement could not be provided, not zero. Estimated allocation measurements retain their instrumentation uncertainty. RSS measures the whole process, not just SDK allocations; observed peaks are sampled checkpoints. Paired ranges in the diagnostics describe variability, not confidence intervals. Each shard uses its own runner; compare Before PR against With PR within a row, not across shards. Compared 102 metric rows; 102 rows shown in all results. Omitted or shortened data remain in |
marandaneto
marked this pull request as ready for review
November 12, 2026 12:13
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 0f9c9cf: 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
haacked
approved these changes
Nov 12, 2026
haacked
left a comment
haacked
left a comment
Contributor
There was a problem hiding this comment.
Nice work! A few non-blocking suggestions inline.
# Conflicts: # posthog/client.py # posthog/feature_flag_evaluations.py
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
Implements sdk-specs #211: well-formed server-failed records have no evaluated value.
Public API addition:
FeatureFlagError.EVALUATION_FAILED = "evaluation_failed". The feature delta adds exactly this constant to the API snapshot; upstream main's other API additions are retained. Signatures and dataclass fields are unchanged by this feature.None, not a healthy disabledFalseresult or placeholder payload. Diagnostics retain independent response errors and config/reason provenance, without experiment exposures.only()andonly_accessed()preserve only selected/accessed private failure records. Filtered values/keys/enrichment, existing unevaluated-key warnings and empty filtered unresolved maps remain unchanged; private provenance is not evaluated membership.False/UNKNOWN_ERRORcompatibility remain unchanged.local_evaluation_inconclusivealongsideevaluation_failed. Healthy false resolves; remote-only failures invent no local reason. Response-wide errors compose independently.03174d0c29dc1d240001981c46f6f15a8ae4a6d1. Resolved two files by porting the approved behavior into main's_FlagReadResult, remote-result and snapshot/error helpers—not by reinstating the old inline implementations. All five feature test diffs are unchanged; incoming controls remain. No additional runtime policy or fix commit was needed.Core patch notes describe None/bulk/default behavior and filtered/unresolved diagnostics. The qualified OpenFeature provider patch intent documents existing None-to-default mapping with an upgraded core dependency; it does not force that upgrade. Main's version/editable-lock changes are inherited upstream. The latest follow-up upgrades only CI/local compliance tooling to harness 1.31.1, consuming merged harness #109; both reusable-workflow references pin release commit
003d4162421193b458bbaa93fbb7bb0d0104bc3b. Compose and contributor instructions match. No shipped dependency, SDK version, lockfile or release change. Frozen fixtures/schemas/digests and release workflows are untouched by this feature.How did you test it?
Latest harness upgrade
Signed HEAD
48307c0e4231ebe6201c02d7d629e4c6a231d61eupgrades both capture modes from harness 1.28.0 to the latest stable 1.31.1. The released OCI image (sha256:f84d5c70dd72ea4436957ea6dbbc1f6836b6f6cbb472c749711d0f3659d85377, version1.31.1/source revisiona169ec9) was tested against real SDK adapters built from this unchanged runtime source:Earlier main-sync validation
At signed merge HEAD
18f72826cc2a57654580a150259a0b8abd23b221, frozen existing Python 3.13.13 environment:autoreview --mode branch --base origin/main: clean; TruffleHog clean. Parent independently reviewed conflict owners/proof and verified unchanged feature-test hunks. No source edits after review.611a869: 68 passed, Ruff/Mypy clean in a frozen external environment. These mock core and validate default mapping, not live propagation; provider source is unchanged and was not rerun for this merge.No full-root pass, alternate interpreter/platform matrix, live-service or distribution/benchmark claim. The earlier harness1.28.0 false-versus-null failures are now addressed by the released1.31.1 consumer upgrade, without changing SDK no-result behavior. Full local classic-harness compliance results are above; no broader cross-SDK/v2 acceptance claim.
Checklist
Agent context
Autonomy: Human-driven (agent-assisted). Human DRI and assignee: @marandaneto. Pi's isolated worker performed the earlier Python merge while the parent handled JS separately. Parent directly implemented this harness upgrade, ran the released Docker compliance suites, independently accepted source/proof and owns final review/publication. Human review is required before merge; no auto-merge or release requested.