fix(flags): malformed flag-dependency conditions no-match locally#740
Conversation
… instead of falling back to /flags on every evaluation
|
Reviews (1): Last reviewed commit: "fix(flags): malformed flag-dependency co..." | Re-trigger Greptile |
posthog-python Compliance ReportDate: 2026-07-14 18:37:43 UTC ✅ All Tests Passed!111/111 tests passed Capture_V1 Tests✅ 94/94 tests passed View Details
Feature_Flags Tests✅ 17/17 tests passed View Details
|
|
LGMT, but two questions before merging:
|
ioannisj
left a comment
There was a problem hiding this comment.
LG left a minor comment spotted by my agent
| # OR group | ||
| if matches and not negation: | ||
| return True | ||
| if not matches and negation: |
There was a problem hiding this comment.
if the server's cohort evaluator treats a malformed flag-dep condition as "never contributes to a match, negated or not," then the SDK now diverges?
There was a problem hiding this comment.
I checked the Rust side, and the server's cohort evaluator doesn't skip these, so no new
divergence. evaluate_cohort_filter routes any non-cohort filter (including flag-type)
through evaluate_property_with_negation
(rust/feature-flags/src/cohorts/cohort_operations.rs), where match_property returns an
error for FlagEvaluatesTo (rust/feature-flags/src/properties/property_matching.rs:532)
and .unwrap_or(false) turns that into false, and then negation is applied. So the
server's behavior for a malformed flag-dep inside a cohort is exactly false-then-negate,
which is what this code now does too.
Good questions @turnipdabeets
I agreed it's worth a follow-up though. |
…y condition during local evaluation
Problem
PR #734 (released as 7.22.1) added upfront validation to
evaluate_flag_dependencythat raisesInconclusiveMatchErrorwhen a flag-dependency condition is malformed (wrong operator, missingkey, orvalue: null).InconclusiveMatchErroris the signal that local evaluation cannot decide, so the client falls back to a network call to the billable/flagsendpoint — on every evaluation of the affected flag. Customers whose flag definitions contain a single malformed dependency condition (typically legacyvalue: nulldata) went from near-zero/flagstraffic to millions of requests per day.Fix
Mirror the server-side Rust evaluator (
match_flag_value_to_flag_filter), which never errors on malformed conditions — it treats them as no match:operatoris notflag_evaluates_to→return Falsekeyis missing/empty →return FalsevalueisNone/missing →return FalseThese are now local, definitive results: no
InconclusiveMatchError, no/flagsfallback. A non-matching condition just means the flag falls through to its other conditions (or evaluates to false if none match) — exactly what the server computes.Genuinely inconclusive cases still raise and fall back to the server: referenced flag not present locally, an inconclusive dependency in the chain, circular dependencies, and missing evaluation context.
Tests
False) with zero/flagscalls.value: None, empty/missingkey, and wrong operator, each asserting zero/flagscalls.Falselocally, while a flag with a second matching condition group still resolves via that group.value: falsetests.Full feature-flags suite passes (158 passed).
Changeset
Added
.sampo/changesets/malformed-flag-dependency-no-match.md(pypi/posthog: patch).