Skip to content

feat(flags): report unresolved local flags on evaluateFlags snapshots - #281

Merged
gustavohstrassburger merged 7 commits into
mainfrom
posthog/unresolved-local-flags
Oct 28, 2026
Merged

gustavohstrassburger merged 7 commits into
mainfrom
posthog/unresolved-local-flags

Conversation

@gustavohstrassburger

@gustavohstrassburger gustavohstrassburger commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

💡 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_called reports flag_missing for both. With onlyEvaluateLocally this is permanent. posthog-node already implements this (PostHog/posthog-js#5234).

Changes:

  • FeatureFlagEvaluations::getUnresolvedFlags() maps each unresolved flag to an UnresolvedFlagReason value (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.
  • A snapshot read of an unresolved flag reports local_evaluation_inconclusive instead of flag_missing. The legacy getFeatureFlag() / getAllFlags() paths are unchanged, as in posthog-node.
  • InconclusiveMatchException takes an optional reason, and both it and RequiresServerEvaluationException expose getReason(). A server-evaluation error is unsupported_definition, or unresolved_dependency when a flag dependency needs server data. Every InconclusiveMatchException throw site sets its reason, and dependency errors carry the typed reason instead of being matched by message text. A bad person value is missing_context; a bad filter value is unsupported_definition.
  • When a fresh definitions response (HTTP 200) loads, the SDK logs a warning, also with debug off, 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 a flag_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):

  • A condition no longer stops at the first filter it cannot check. If another filter does not match, the flag resolves locally. A static cohort still forces the fallback only when it is the first filter in its condition that cannot be checked, as in posthog-node.
  • An inconclusive dependency no longer stops a later condition from resolving the flag. A dependency that needs server data, such as a static cohort, still sends the flag to the server.
  • An inactive flag with experience continuity resolves to false locally.
  • A flag that depends on a flag with experience continuity no longer resolves locally.
  • A malformed date makes its condition inconclusive instead of failing local evaluation of the whole flag.

Divergences from the spec or posthog-node:

  • No device id and a missing group key still resolves to false, so the device-id acceptance row does not apply.
  • The warning acceptance rows are adapted to the 200-only rule: a 200 load followed by an unchanged, 304 or cache reload logs one warning in total, and a client that only reads the cache logs none.
  • A dependency whose holdout uses a different aggregation reports unsupported_definition but is not in the load-time warning.

api/public-api.json was regenerated. main currently fails composer api:check because of the stale SpanContext entry, 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_capable and @unresolvable_flag_warning_capable in acceptance/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 test
  • composer api:check and composer validate --no-check-version --strict
  • python3 -m unittest discover -s sdk_compliance_adapter

No manual testing in an app.

📝 Checklist

  • All commits are signed (see Commit signing)
  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm change to generate a change intent file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Written with Claude Code in PostHog Desktop, from a cross-SDK plan to apply sdk-specs fix: don't sort condition sets with variant overrides to the top #85 to all server SDKs. It follows posthog-node and the posthog-python (#1133), posthog-server (#896) and posthog-ruby (#319) PRs from the same plan.
  • $evaluationCache is 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

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
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-fork_curl Compliance Report

Date: 2026-10-28T20:02:01.311154+00:00
Duration: 116118ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
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

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-lib_curl Compliance Report

Date: 2026-10-28T20:01:38.706910+00:00
Duration: 116752ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
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

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread lib/FeatureFlag.php Outdated
Comment thread lib/FeatureFlag.php Outdated
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

posthog-php-socket Compliance Report

Date: 2026-10-28T20:01:51.431447+00:00
Duration: 115912ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
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

@gustavohstrassburger
gustavohstrassburger marked this pull request as ready for review October 28, 2026 16:18
@gustavohstrassburger
gustavohstrassburger requested a review from a team as a code owner October 28, 2026 16:18

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 20c3b45: no findings. This pull request is large, so the review ran in 4 parts with less depth than usual.

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 4277da1: 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.

Comment thread lib/FeatureFlag.php Outdated
@gustavohstrassburger
gustavohstrassburger merged commit dc9650f into main Oct 28, 2026
27 checks passed
@gustavohstrassburger
gustavohstrassburger deleted the posthog/unresolved-local-flags branch October 28, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants