Repository navigation
feat(flags): report unresolved local flags on evaluateFlags snapshots - #281
Conversation
Implements the "Snapshot unresolved flag reporting" and "Warning for flags local evaluation never resolves" requirements of the evaluate-flags spec (PostHog/sdk-specs#85). - Add `FeatureFlagEvaluations::getUnresolvedFlags()`, with an `UnresolvedFlagReason` for each flag and the spec's fixed precedence. - Report `local_evaluation_inconclusive` instead of `flag_missing` when a snapshot read hits an unresolved flag. - Do not stop a condition at the first inconclusive filter, so a filter that does not match resolves the condition locally. - Check `active` before experience continuity, also for dependencies. - Log one warning per flag that local evaluation can never resolve when a fresh definitions response loads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 39793b31-809e-4689-a5d8-b4359cbb22c1
posthog-php-fork_curl Compliance ReportDate: 2026-10-28T20:02:01.311154+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 38ms |
| Format Validation.Event Has Uuid | ✅ | 533ms |
| Format Validation.Event Has Lib Properties | ✅ | 532ms |
| Format Validation.Distinct Id Is String | ✅ | 533ms |
| Format Validation.Token Is Present | ✅ | 535ms |
| Format Validation.Custom Properties Preserved | ✅ | 534ms |
| Format Validation.Event Has Timestamp | ✅ | 532ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 534ms |
| Retry Behavior.Retries On 503 | ✅ | 5860ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2536ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2536ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6345ms |
| Retry Behavior.Implements Backoff | ✅ | 16278ms |
| Retry Behavior.Retries On 500 | ✅ | 5161ms |
| Retry Behavior.Retries On 502 | ✅ | 5649ms |
| Retry Behavior.Retries On 504 | ✅ | 5655ms |
| Retry Behavior.Max Retries Respected | ✅ | 16287ms |
| Deduplication.Generates Unique Uuids | ✅ | 541ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5649ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10866ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5654ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 539ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 533ms |
| Compression.Sends Gzip When Enabled | ✅ | 536ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 536ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 519ms |
| Batch Format.Multiple Events Batched Together | ✅ | 523ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2537ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2536ms |
| Error Handling.Retries On 408 | ✅ | 5651ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 811ms < 2500ms
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 | ✅ | 522ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 524ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 522ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 523ms |
| Request Payload.Groups Round Trip | ✅ | 523ms |
| Request Payload.Groups Default To Empty Object | ✅ | 524ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 523ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 524ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 523ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 518ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 519ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 526ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 524ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 627ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 625ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 534ms |
posthog-php-lib_curl Compliance ReportDate: 2026-10-28T20:01:38.706910+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 30ms |
| Format Validation.Event Has Uuid | ✅ | 524ms |
| Format Validation.Event Has Lib Properties | ✅ | 525ms |
| Format Validation.Distinct Id Is String | ✅ | 526ms |
| Format Validation.Token Is Present | ✅ | 525ms |
| Format Validation.Custom Properties Preserved | ✅ | 526ms |
| Format Validation.Event Has Timestamp | ✅ | 525ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 528ms |
| Retry Behavior.Retries On 503 | ✅ | 5836ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2529ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2530ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6336ms |
| Retry Behavior.Implements Backoff | ✅ | 16253ms |
| Retry Behavior.Retries On 500 | ✅ | 5636ms |
| Retry Behavior.Retries On 502 | ✅ | 5634ms |
| Retry Behavior.Retries On 504 | ✅ | 5634ms |
| Retry Behavior.Max Retries Respected | ✅ | 17041ms |
| Deduplication.Generates Unique Uuids | ✅ | 247ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5635ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10843ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5640ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 532ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 528ms |
| Compression.Sends Gzip When Enabled | ✅ | 527ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 526ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 522ms |
| Batch Format.Multiple Events Batched Together | ✅ | 513ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2529ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2529ms |
| Error Handling.Retries On 408 | ✅ | 5633ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 804ms < 2500ms
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 | ✅ | 522ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 525ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 523ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 525ms |
| Request Payload.Groups Round Trip | ✅ | 524ms |
| Request Payload.Groups Default To Empty Object | ✅ | 523ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 523ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 524ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 523ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 519ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 509ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 527ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 526ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 625ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 626ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 525ms |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 449375a: 2 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.
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.
posthog-php-socket Compliance ReportDate: 2026-10-28T20:01:51.431447+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 25ms |
| Format Validation.Event Has Uuid | ✅ | 519ms |
| Format Validation.Event Has Lib Properties | ✅ | 521ms |
| Format Validation.Distinct Id Is String | ✅ | 522ms |
| Format Validation.Token Is Present | ✅ | 521ms |
| Format Validation.Custom Properties Preserved | ✅ | 521ms |
| Format Validation.Event Has Timestamp | ✅ | 521ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 522ms |
| Retry Behavior.Retries On 503 | ✅ | 5828ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2525ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2522ms |
| Retry Behavior.Respects Retry After Header | ❌ | 6330ms |
| Retry Behavior.Implements Backoff | ✅ | 16245ms |
| Retry Behavior.Retries On 500 | ✅ | 5629ms |
| Retry Behavior.Retries On 502 | ✅ | 5629ms |
| Retry Behavior.Retries On 504 | ✅ | 5628ms |
| Retry Behavior.Max Retries Respected | ✅ | 16244ms |
| Deduplication.Generates Unique Uuids | ✅ | 526ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5629ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10835ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5633ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 525ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 522ms |
| Compression.Sends Gzip When Enabled | ✅ | 522ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 521ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 517ms |
| Batch Format.Multiple Events Batched Together | ✅ | 510ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2523ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2524ms |
| Error Handling.Retries On 408 | ✅ | 5629ms |
Failures
retry_behavior.respects_retry_after_header
Retry delay too short: 802ms < 2500ms
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 520ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 520ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 520ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 519ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 520ms |
| Request Payload.Groups Round Trip | ✅ | 520ms |
| Request Payload.Groups Default To Empty Object | ✅ | 519ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 520ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 520ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 520ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 515ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 507ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 523ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 521ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 623ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 621ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 521ms |
dustinbyrne
left a comment
There was a problem hiding this comment.
The unresolved-flag reporting, snapshot filtering, and access-error classification look consistent with the intended contract. No actionable findings on this head. Resolve the current merge conflicts and validate the resulting revision before merging.
AI-assisted review.
…cal-flags # Conflicts: # api/public-api.json
💡 Motivation and Context
Why: sdk-specs #85 defines how a server SDK reports flags that local evaluation could not resolve. Today such a flag is absent from the
evaluateFlags()snapshot, so it looks the same as a flag that does not exist, and$feature_flag_calledreportsflag_missingfor both. WithonlyEvaluateLocallythis is permanent. posthog-node already implements this (PostHog/posthog-js#5234).Changes:
FeatureFlagEvaluations::getUnresolvedFlags()maps each unresolved flag to anUnresolvedFlagReasonvalue (experience_continuity,unsupported_definition,unresolved_dependency,missing_context). When several causes apply, the spec's fixed precedence picks one.only()/onlyAccessed()snapshots have no entries, and reading the list is not an access.local_evaluation_inconclusiveinstead offlag_missing. The legacygetFeatureFlag()/getAllFlags()paths are unchanged, as in posthog-node.InconclusiveMatchExceptiontakes an optionalreason, and both it andRequiresServerEvaluationExceptionexposegetReason(). A server-evaluation error isunsupported_definition, orunresolved_dependencywhen a flag dependency needs server data. EveryInconclusiveMatchExceptionthrow site sets its reason, and dependency errors carry the typed reason instead of being matched by message text. A bad person value ismissing_context; a bad filter value isunsupported_definition.debugoff, for flags local evaluation can never resolve. PHP has no long-lived poller and PHP-FPM builds a client per request, so cache-provider loads and 304 responses do not log it. Without aflag_definition_cache_provider, each request fetches fresh definitions, so under PHP-FPM the warning is logged on every request. The changeset documents this and points to the cache provider.Behavior changes (all in the change intent):
falselocally.Divergences from the spec or posthog-node:
false, so the device-id acceptance row does not apply.unsupported_definitionbut is not in the load-time warning.api/public-api.jsonwas regenerated.maincurrently failscomposer api:checkbecause of the staleSpanContextentry, so this PR also drops it. That overlaps #280; whichever merges second needs a trivial rebase of that file.💚 How did you test it?
New PHPUnit tests mirror every scenario tagged
@unresolved_flags_capableand@unresolvable_flag_warning_capableinacceptance/public/evaluate-flags.feature, plus precedence, the condition loop, dependency reasons, warning dedup and unexpected errors. Nine existing tests were updated because they asserted changed error messages or no log output. Run locally with PHP 8.3:./vendor/bin/phpunit --bootstrap vendor/autoload.php --configuration phpunit.xml(2402 tests, 31 skipped)./vendor/bin/phpcs --standard=phpcs.xml --extensions=php -n lib testcomposer api:checkandcomposer validate --no-check-version --strictpython3 -m unittest discover -s sdk_compliance_adapterNo manual testing in an app.
📝 Checklist
If releasing new changes
pnpm changeto generate a change intent file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
$evaluationCacheis now passed by reference so dependency results and reasons are shared; a group-aggregated condition gets its own cache, so a group-key result is never reused for a person.Created with PostHog Desktop
🤖 Generated with Claude Code