Repository navigation
docs(flags): align unsupported cohort groups with backend - #207
marandaneto wants to merge 2 commits into
Conversation
SDK cohort backend-parity audit: PR #207SummaryAll 19 listed canonical repositories were fetched from Contract: SDK-specs #207, head Seven SDK implementations diverge directly from the cohort-order contract. Go and Node pass the core cohort matrix, but an additional outer flag-condition boundary can discard the required hard-fallback signal. The other ten repositories have no independent applicable local cohort evaluator on their exposed paths. These are conformance results for synthetic unsupported definitions. No deployed XOR-emitting backend configuration was established, and no claim is made that ordinary valid definitions are currently wrong. Fix PR trackerThese are nine suggested SDK fix PRs/work items, not PRs created by this audit. Seven fix the cohort engine directly; Go and Node address the separate outer flag-condition boundary described below.
Node #5648 is also merged. Its nested- No current open PRs in the other repositories were nominated or reviewed as fix vehicles. “PR needed” means an implementation work item, not a claim that no other open PR could incorporate it. Applicable backend implementations
Required ordering and observed distinctionsLet X be a nested group with unsupported type XOR, T a matching supported group, F a mismatching supported group, and M ordinary missing-property context.
.NET, PHP, Ruby, JVM and Rust SDK fail unsupported-first cases. Python and Elixir preserve that final fallback but fail decisive-first short circuit. Rust additionally evaluates after decisive results; a scalar-descendant witness demonstrates the visit even when a simple XOR case happens to return the expected final boolean. Additional outer-boundary gap (Go/Node and others)Go #440 and Node #5649 are present on the fetched main branches. Their cohort-group loops follow the core matrix. This is not evidence that those fixes failed to merge. However, under #207's wording that a reached unsupported group must stop local evaluation, the outer flag-condition loops can still lose the hard failure:
Go's public GetFeatureFlag behavior was reproduced at its exact main SHA. Node has the same code path by source inspection; no Node runtime reproduction was run. Equivalent outer-boundary cases were also reproduced/source-traced in .NET, Elixir, Python, PHP and Ruby. The individual reports distinguish evaluator-level, public API and source-only proof. This is a separate ownership boundary from cohort sibling composition. A correction must preserve the hard unsupported-group signal there without indiscriminately changing existing missing-context/static-cohort handling. It does not justify whole-tree prevalidation: an earlier decisive cohort sibling must still short-circuit. Remaining repositories: applicability, not conformance passesThe Android mobile client itself is remote/cached-value based; its separate posthog-server JVM module is included in the applicable-backend table. Roblox is server-side at runtime, but flags are remote-evaluated. Pi, OpenClaw and OpenCode are integrations using posthog-node rather than independent evaluator implementations. Unreal delegates through posthog-rs and exposes no separate cohort engine; its current client construction does not supply local-evaluation configuration. Validation and limits
No retained SDK source edits, commits, SDK PR changes, issues, or fixes were made. This audit comment is the publication action. Owned disposable .NET/Go validation worktrees were removed; user changes in primary checkouts were preserved. Fetching the requested latest mains changed remote-tracking refs only. Reproduction commands and evidence boundariesValidation performed on the immutable main snapshotsThe temporary audit probes assert and record observed behavior, including incorrect outcomes. Their passing status does not mean the SDK conforms.
All validation that wrote tests or build files used owned disposable worktrees or unique external source exports. The .NET and Go validation worktrees were removed afterward. Dirty/outdated primary checkouts and their staged files were not changed. Minimal reusable inputFor {
"X": {"type":"XOR","values":[{"type":"person","key":"email","operator":"exact","value":"other@x"}]},
"T": {"type":"AND","values":[{"type":"person","key":"email","operator":"exact","value":"user@x"}]},
"F": {"type":"AND","values":[{"type":"person","key":"email","operator":"exact","value":"other@x"}]}
}Use each expression as a referenced dynamic cohort in one active flag with 100% rollout. Test For the separate outer-boundary probe, use cohort Audit action: report only. No SDK fixes, commits, PR creations, issues or review-state changes were made by this audit. |
haacked
left a comment
haacked
left a comment
There was a problem hiding this comment.
I'm not sure we should copy this behavior from the Rust /flags service.
OR is commutative, so A OR B should always equal B OR A. But in the Rust implementation, the result depends on child order: OR[true, XOR group] is true, while OR[XOR group, true] errors because the loop returns the parse error with ? before it reaches the true. That looks like an accident to me and not meant to be a contract.
What do you think about fixing the Rust service instead? It could treat an unsupported group as unknown, so another child in the same group can still decide the result, and fail the flag only when it can't. Then the SDKs could keep the spec as it was before this PR.
If you agree, I'd be happy to do it.
agree, i was unsure about the correct behaviour since its a product question, the sdk spec was just 'PRs over issues' in case that was the right approach but happy to pivot |
Problem
The cross-SDK comparison during review of posthog-go#431, followed by posthog-go#440's sibling-order discussion, exposed a contract mismatch.
The SDK spec allowed a later decisive sibling to override an unsupported cohort group. The Rust feature-flags backend instead stops with a parsing error when that group is reached. This PR makes backend parity the contract for this case. It concerns the Rust backend in
PostHog/posthog, not the Rust SDK.Changes
XOR. Do not evaluate later siblings after that failure.Source evidence: Rust cohort parsing and evaluation.
SDK impact
1f185fdstill visits later siblings after recording a decisive result. It needs a separate short-circuit follow-up for the decisive-first cases. This is a source comparison, not a Python runtime conformance result.Validation
The local harness checkout has neither the v2 Gherkin runner nor its step catalogue. The acceptance examples remain descriptive and unselected. Syntax validation is not a claim of harness readiness or cross-SDK conformance. No Rust backend runtime tests were run for this spec-only change.
Agent context
Autonomy: Human-driven (agent-assisted)
Prepared with Pi using the OpenSpec CLI, Git, GitHub CLI, and isolated autoreview. The user chose current backend behavior as the contract. Kept missing-context composition separate from unsupported-group failure and recorded the known SDK follow-ups. Human review is required before merge.