fix(flags): honor versioned local property matching - #200
Conversation
posthog-elixir Compliance ReportDate: 2026-09-19T06:21:49.539914+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 610ms |
| Format Validation.Event Has Uuid | ✅ | 610ms |
| Format Validation.Event Has Lib Properties | ✅ | 609ms |
| Format Validation.Distinct Id Is String | ✅ | 610ms |
| Format Validation.Token Is Present | ✅ | 609ms |
| Format Validation.Custom Properties Preserved | ✅ | 610ms |
| Format Validation.Event Has Timestamp | ✅ | 610ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ❌ | 608ms |
| Retry Behavior.Retries On 503 | ✅ | 5615ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 2612ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 2611ms |
| Retry Behavior.Respects Retry After Header | ✅ | 5615ms |
| Retry Behavior.Implements Backoff | ✅ | 15620ms |
| Retry Behavior.Retries On 500 | ✅ | 5616ms |
| Retry Behavior.Retries On 502 | ✅ | 5615ms |
| Retry Behavior.Retries On 504 | ✅ | 5615ms |
| Retry Behavior.Max Retries Respected | ✅ | 15626ms |
| Deduplication.Generates Unique Uuids | ✅ | 620ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5614ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 10620ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 5617ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 615ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 612ms |
| Compression.Sends Gzip When Enabled | ✅ | 609ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 609ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 607ms |
| Batch Format.Multiple Events Batched Together | ✅ | 614ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2611ms |
| Error Handling.Does Not Retry On 413 | ✅ | 2612ms |
| Error Handling.Retries On 408 | ✅ | 5615ms |
Failures
format_validation.non_utc_event_timestamp_is_converted_to_utc
Event 0 field 'timestamp' instant '2026-09-19T06:20:05.494447Z' != expected '2025-01-02T03:04:05Z'
Feature_Flags Tests
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 8ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 7ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 6ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 7ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 6ms |
| Request Payload.Groups Round Trip | ✅ | 6ms |
| Request Payload.Groups Default To Empty Object | ❌ | 6ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 6ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ❌ | 6ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 6ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 2ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 607ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 10ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ❌ | 6ms |
| Retry Behavior.Retries Flags On 502 | ❌ | 310ms |
| Retry Behavior.Retries Flags On 504 | ❌ | 311ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ❌ | 610ms |
Failures
request_payload.groups_default_to_empty_object
Field 'groups' not found in /flags request body at path 'groups'. Available keys: ['api_key', 'distinct_id', 'flag_keys_to_evaluate']
request_payload.disable_geoip_omitted_defaults_to_false
Field 'geoip_disable' not found in /flags request body at path 'geoip_disable'. Available keys: ['api_key', 'distinct_id', 'flag_keys_to_evaluate']
request_lifecycle.mock_response_value_is_returned_to_caller
Last action result missing field 'value'. Keys: ['error', 'success']
retry_behavior.retries_flags_on_502
Last action result missing field 'value'. Keys: ['error', 'success']
retry_behavior.retries_flags_on_504
Last action result missing field 'value'. Keys: ['error', 'success']
side_effect_events.get_feature_flag_captures_feature_flag_called_event
Expected 1 events with name '$feature_flag_called', got 0
|
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
|
The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified. Reviews (1) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." |
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
💡 Motivation and Context
Local flag evaluation needs to follow the
property_matching_versionreturned with flag definitions. This implements the behavior in the backend change and the shared SDK contract.This intentionally corrects a compatibility mismatch in the old Elixir implementation. Missing metadata and version 1 now use released service legacy matching, not the SDK's former array-membership behavior. For example, a
falsefilter matches"banana"under service v1 truthiness, but not under v2. Existing applications can therefore see different local results even without version 2 metadata.Only numeric version 2 selects explicit scalar equality and equality against individual members of a nonempty filter array. Empty filters retain recursive truthiness in both modes. For known properties,
is_notcomplementsexact. Missing properties and ambiguous numeric comparisons remain inconclusive.Composite equality uses recursively sorted JSON object keys. Composite values containing floats or integers outside the service's i64/u64 range remain inconclusive because Jason and the service can serialize those numbers differently. For example, Jason writes
{"n":0.00001}as{"n":1.0e-5}, which must not produce a definitive local mismatch. These cases can fall back to/flags, while local-only evaluation omits the unresolved flag.Composite comparisons also stay inconclusive when either serialized side contains uppercase Greek sigma, whose contextual lowercase differs between Elixir and Rust, or when atom and string object keys collide after Jason-compatible conversion. Nested maps and arrays use the same safeguards. Atom object keys use their names, matching Jason, so
nilbecomes"nil", not an empty key. A map with bothniland"nil"stays inconclusive, whileniland""remain distinct keys. Ordinary composite integers, noncolliding object keys and scalar Date/Time string conversion are preserved. Pre-existing scalar-string Unicode behavior is unchanged.Composite normalization also leaves nested structs inconclusive, including
Jason.OrderedObjectand custom or derived Jason encoders. Their encoders can hide unsorted keys, duplicate keys or ambiguous numbers from recursive validation. This deliberately reduces local coverage even for nested structs that happen to encode safely, including nested Date/Time values. Plain maps and lists still normalize locally, and top-level scalar Date/Time string comparisons are unchanged. The evaluator does not serialize and decode arbitrary structs to guess their meaning.When boolean truthiness reaches an opaque property struct, directly or inside an array, the result also stays inconclusive rather than assuming false. Native non-boolean property atoms use their JSON string names, so
:TRUEbehaves like"TRUE". Ordinary JSON truthiness andis_notcomplements remain unchanged. These fallback cases omit the flag in local-only mode and can use the server result otherwise.The definition loader keeps the version in the existing immutable snapshot and complete cache-provider envelope. Person, group, recursive cohort and flag-dependency evaluation use that snapshot. HTTP and provider refreshes apply version-only changes even when flags are unchanged. Older envelopes without metadata default to version 1.
The existing Sampo patch changeset documents the service-v1 correction. No dependencies or harness adapters change. Optional harness coverage is separate.
💚 How did you test it?
mix test: 507 passed, 18 live-service integration tests excluded by repository defaults. Live-service tests were not run. Existing deprecated getter and Req test-adapter warnings remain.serde_jsonprobe confirmed the decimal/exponent and out-of-range integer differences from Jason.exactandis_not, nested maps and arrays on either side of equality, numeric range boundaries, mocked/flagsfallback and local-only omission without affecting unrelated local flags./flagsfallback and local-only omission.serde_jsonoracle confirmed duplicate-key parsing and the service result. The repair leaves these comparisons inconclusive instead of guessing which key survives./flagsfallback."true"and"TRUE"for a custom scalar encoder and a native atom, with correct merge-base results and incorrect new v1 results. After approval, a narrow truthiness repair made 18 additional failing tests pass. They cover nested arrays, v2 empty filters, both operators, atom/string equivalence, nil/boolean/map controls and server fallback.mix format --check-formatted,mix compile --warnings-as-errors,mix posthog.public_api --check,mix credo --strictandgit diff --check: passed.origin/maincompleted without actionable findings onb306825798e3607a2f062c3e7574bb9307351c78. Validation used Elixir 1.20.4 / OTP 29, not the exact 1.20.0-otp-29 / 29.0.1 pins. Local results do not establish CI status.CI formatting repair
The CI Format job uses Elixir 1.18.3 / OTP 27.3. Its formatter wrapped one long two-argument assertion differently from local Elixir 1.20.4 / OTP 29. The assertion now uses explicit parentheses and multiline arguments. Its parsed AST is unchanged, including the pattern match and diagnostic message. No matcher, dependency or workflow changes were needed.
git diff --check: passed. An AST comparison confirmed the edit changes layout only.origin/maincompleted without actionable findings ond3bc85f555655e3e6439100535c77b04394f8b31.d3bc85f555655e3e6439100535c77b04394f8b31: all 28 active checks passed, with no failures or pending checks. The CI run passed Format on Elixir 1.18.3 / OTP 27.3, all 12 build/test combinations, Credo, warnings-as-errors compilation, public API checks and package build. SDK compliance also passed. One superseded PR-title run was cancelled when this description was updated, and its replacement passed. Local checks used Elixir 1.20.4 / OTP 29.0.5.📝 Checklist
The changelog entry is supplied through the Sampo changeset. The intentional behavior correction is described above.
If releasing new changes
sampo addto generate a changeset fileThe previously generated
.sampo/changesets/roguish-lady-louhi.mdis retained unchanged.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Pi implementation agents prepared and reviewed the changes under human direction. The publication and fresh-review fix sessions used Pi, Git, GitHub CLI, Mix, offline Rust serialization and lowercase probes and the isolated autoreview helper. Local session identifiers:
sdk-pr-publication/posthog-elixirandpi-review-all/posthog-elixir/fix-1andelixir-final-pass/fixandelixir-opaque-pass/fixandci-fixes/posthog-elixir(no public session links).The implementation keeps the existing provider envelope and immutable snapshot model. The numeric, Unicode, duplicate-key and opaque-struct repairs use the existing inconclusive fallback rather than adding a serializer, Unicode implementation or dependency. Human review is required before merge.