fix(flags): honor versioned local property matching - #200
Draft
marandaneto wants to merge 8 commits into
Draft
Conversation
Contributor
posthog-elixir Compliance ReportDate: 2026-09-05 17:39:44 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
|
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
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.