fix(flags): honor versioned local property matching - #238
Merged
Merged
Conversation
Contributor
posthog-php Compliance ReportDate: 2026-09-05 11:56:30 UTC ✅ All Tests Passed!46/46 tests passed Capture Tests✅ 29/29 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
marandaneto
marked this pull request as ready for review
September 5, 2026 17:29
|
Reviews (1): Last reviewed commit: "fix(flags): honor versioned local proper..." | Re-trigger Greptile |
marandaneto
marked this pull request as draft
September 6, 2026 07:00
Contributor
|
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 |
…-local-evaluation
Contributor
posthog-php-fork_curl Compliance ReportDate: 2026-09-18T10:25:12.576899+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 35ms |
| Format Validation.Event Has Uuid | ✅ | 529ms |
| Format Validation.Event Has Lib Properties | ✅ | 530ms |
| Format Validation.Distinct Id Is String | ✅ | 532ms |
| Format Validation.Token Is Present | ✅ | 530ms |
| Format Validation.Custom Properties Preserved | ✅ | 529ms |
| Format Validation.Event Has Timestamp | ✅ | 530ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 530ms |
| Retry Behavior.Retries On 503 | ❌ | 5533ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2533ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2534ms |
| Retry Behavior.Respects Retry After Header | ❌ | 5534ms |
| Retry Behavior.Implements Backoff | ❌ | 15550ms |
| Retry Behavior.Retries On 500 | ❌ | 5536ms |
| Retry Behavior.Retries On 502 | ❌ | 5537ms |
| Retry Behavior.Retries On 504 | ❌ | 5535ms |
| Retry Behavior.Max Retries Respected | ❌ | 15547ms |
| Deduplication.Generates Unique Uuids | ✅ | 538ms |
| Deduplication.Preserves Uuid On Retry | ❌ | 5533ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ❌ | 10543ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ❌ | 5537ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 539ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 534ms |
| Compression.Sends Gzip When Enabled | ✅ | 533ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 530ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 519ms |
| Batch Format.Multiple Events Batched Together | ✅ | 519ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2533ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2532ms |
| Error Handling.Retries On 408 | ❌ | 5536ms |
Failures
retry_behavior.retries_on_503
Expected at least 3 requests, got 1
retry_behavior.respects_retry_after_header
Expected at least 2 requests, got 1
retry_behavior.implements_backoff
Expected at least 3 requests, got 1
retry_behavior.retries_on_500
Expected at least 2 requests, got 1
retry_behavior.retries_on_502
Expected at least 2 requests, got 1
retry_behavior.retries_on_504
Expected at least 2 requests, got 1
retry_behavior.max_retries_respected
Expected 4 requests, got 1
deduplication.preserves_uuid_on_retry
Need at least 2 requests to check retry
deduplication.preserves_uuid_and_timestamp_on_retry
Expected at least 3 requests, got 1
deduplication.preserves_uuid_and_timestamp_on_batch_retry
Expected at least 2 requests, got 1
error_handling.retries_on_408
Expected at least 2 requests, got 1
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 522ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 521ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 521ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 522ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 521ms |
| Request Payload.Groups Round Trip | ✅ | 521ms |
| Request Payload.Groups Default To Empty Object | ✅ | 521ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 523ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 520ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 523ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 517ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 516ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 524ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 523ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 625ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 624ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 531ms |
Contributor
posthog-php-lib_curl Compliance ReportDate: 2026-09-18T10:25:21.556854+00:00 ✅ All Tests Passed!47/47 tests passed Capture Tests✅ 30/30 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
Contributor
posthog-php-socket Compliance ReportDate: 2026-09-18T10:25:42.890615+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 26ms |
| Format Validation.Event Has Uuid | ✅ | 522ms |
| Format Validation.Event Has Lib Properties | ✅ | 524ms |
| Format Validation.Distinct Id Is String | ✅ | 523ms |
| Format Validation.Token Is Present | ✅ | 524ms |
| Format Validation.Custom Properties Preserved | ✅ | 523ms |
| Format Validation.Event Has Timestamp | ✅ | 524ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 524ms |
| Retry Behavior.Retries On 503 | ❌ | 9231ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2527ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2524ms |
| Retry Behavior.Respects Retry After Header | ❌ | 9235ms |
| Retry Behavior.Implements Backoff | ❌ | 19242ms |
| Retry Behavior.Retries On 500 | ❌ | 8736ms |
| Retry Behavior.Retries On 502 | ❌ | 9238ms |
| Retry Behavior.Retries On 504 | ❌ | 9234ms |
| Retry Behavior.Max Retries Respected | ❌ | 19247ms |
| Deduplication.Generates Unique Uuids | ✅ | 536ms |
| Deduplication.Preserves Uuid On Retry | ❌ | 9236ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ❌ | 14240ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ❌ | 9239ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 531ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 526ms |
| Compression.Sends Gzip When Enabled | ✅ | 524ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 523ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 520ms |
| Batch Format.Multiple Events Batched Together | ✅ | 512ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2525ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2526ms |
| Error Handling.Retries On 408 | ❌ | 5530ms |
Failures
retry_behavior.retries_on_503
Expected at least 3 requests, got 1
retry_behavior.respects_retry_after_header
Expected at least 2 requests, got 1
retry_behavior.implements_backoff
Expected at least 3 requests, got 1
retry_behavior.retries_on_500
Expected at least 2 requests, got 1
retry_behavior.retries_on_502
Expected at least 2 requests, got 1
retry_behavior.retries_on_504
Expected at least 2 requests, got 1
retry_behavior.max_retries_respected
Expected 4 requests, got 1
deduplication.preserves_uuid_on_retry
Need at least 2 requests to check retry
deduplication.preserves_uuid_and_timestamp_on_retry
Expected at least 3 requests, got 1
deduplication.preserves_uuid_and_timestamp_on_batch_retry
Expected at least 2 requests, got 1
error_handling.retries_on_408
Expected at least 2 requests, got 1
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 524ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 521ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 522ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 521ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 522ms |
| Request Payload.Groups Round Trip | ✅ | 522ms |
| Request Payload.Groups Default To Empty Object | ✅ | 521ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 522ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 521ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 522ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 517ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 508ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 525ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 523ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 625ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 624ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 524ms |
 The PR appears safe to merge with no actionable correctness, security, or repository-rule violations identified. Reviews (2) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." |
turnipdabeets
approved these changes
Sep 18, 2026
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
2 participants
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
Local feature flag evaluation needs to honor the matching version returned with flag definitions. This follows the backend behavior in PostHog/posthog#90694 and shared contract in PostHog/sdk-specs#59.
is_notcomplementsexact. Missing properties still allow remote fallback.property_matching_version. Providers receive the complete normalized snapshot for storage. Cache reads without the selector reset to legacy matching. The existinggroupTypeMappingalias remains supported for group mapping, not for version metadata.FeatureFlagevaluation helpers gain a trailing optionalpropertyMatchingVersionparameter defaulting to 1. Existing calls remain compatible, and the public API snapshot records the additions. The external cache provider contract documents preserving the metadata. The existing patch change intent is included.Related optional harness coverage is PostHog/posthog-sdk-test-harness#53. This PR does not opt a PHP adapter into that coverage.
💚 How did you test it?
Reran these checks on the source tree committed as
65ca15245e9c4d19b5311e285a3833fec6c0d2adwith PHP 8.5.10 and PHPUnit 11.5.55:./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml --no-coverage --filter 'VersionedPropertyMatchingTest|testMatchingVersionSurvivesApiAndProviderRoundTrip|testVersionOnly|testReloadDuringEvaluation|testVersionedMissing' php scripts/check-public-api.php ./vendor/bin/phpcs --standard=phpcs.xml -n lib/FeatureFlag.php lib/Client.php lib/FlagDefinitionCacheProvider.php test/VersionedPropertyMatchingTest.php test/FlagDefinitionCacheProviderTest.php git diff --checkAll exited successfully. The focused run passed 195 tests and 613 assertions, with 42 existing PHPUnit metadata deprecations. Tests exercise the real Client and evaluator with mocked HTTP/provider boundaries, including cache round-trips, version-only transitions, nested conditions, reentrant reloads, and remote fallback. Positive local cases permit fallback and assert that no remote flag request occurs.
Earlier implementation validation passed the broader flag suite with 471 tests and 3819 assertions. That earlier run reported four existing SDK deprecated-method notices as well as the metadata deprecations. PHPCS without
-npreviously reported 13 unchanged-line length warnings and no errors. The complete unrelated suite, coverage, and live-service integration were not run in this publication pass. Local checks are not a CI result.The required isolated committed-branch autoreview passed for this exact commit against
origin/mainat6da44cd256743e9ba08c14a685dc8e8c5ac1e561, with no actionable findings.📝 Checklist
If releasing new changes
pnpm changeto generate a change intent fileThe existing
.changeset/versioned-property-matching.mdrecords a patch intent. It was preserved without rerunning the generator or adding a duplicate.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi agents assisted implementation and publication under human direction. Tools used included Git, PHP, PHPUnit, PHP_CodeSniffer, GitHub CLI, and the isolated Pi autoreview helper with TruffleHog. The local agent session is not publicly shared.
The implementation preserves legacy defaults and optional helper arguments while keeping matching metadata with each definition snapshot. Publication retained the approved changes and used a new signed commit without rewriting history. Human review is required before merging.