Skip to content

Commit 6953d4e

Browse files
haackedhaacked
committed
fix(flags): flag dependency evaluation for multivariate flags
When evaluating multivariate flags, an expected value of `true` means match any variant. An expected value of a string value "some-variant" means we must match the variant exactly.
1 parent 10472e7 commit 6953d4e

File tree

2 files changed

+324
-0
lines changed

2 files changed

+324
-0
lines changed

‎posthog/feature_flags.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,69 @@ def evaluate_flag_dependency(
139139
# Definitive False result - dependency failed
140140
return False
141141

142+
# All dependencies in the chain have been evaluated successfully
143+
# Now check if the final flag value matches the expected value in the property
144+
flag_key = property.get("key")
145+
expected_value = property.get("value")
146+
operator = property.get("operator", "exact")
147+
148+
if flag_key and expected_value is not None:
149+
# Get the actual value of the flag we're checking
150+
actual_value = evaluation_cache.get(flag_key)
151+
152+
if actual_value is None:
153+
# Flag wasn't evaluated - this shouldn't happen if dependency chain is correct
154+
raise InconclusiveMatchError(
155+
f"Flag '{flag_key}' was not evaluated despite being in dependency chain"
156+
)
157+
158+
# For flag dependencies, we need to compare the actual flag result with expected value
159+
# using the flag_evaluates_to operator logic
160+
if operator == "flag_evaluates_to":
161+
return matches_dependency_value(actual_value, expected_value)
162+
else:
163+
# For backwards compatibility, treat other operators as exact comparison
164+
# but flag dependencies should use flag_evaluates_to
165+
return actual_value == expected_value
166+
167+
# If no value check needed, return True (all dependencies passed)
142168
return True
143169

144170

171+
def matches_dependency_value(actual_value, expected_value):
172+
"""
173+
Check if the actual flag value matches the expected dependency value.
174+
175+
This follows the same logic as the C# MatchesDependencyValue function:
176+
- String variant case: check for exact match or boolean true
177+
- Boolean case: must match expected boolean value
178+
179+
Args:
180+
actual_value: The actual value returned by the flag evaluation
181+
expected_value: The expected value from the property
182+
183+
Returns:
184+
bool: True if the values match according to flag dependency rules
185+
"""
186+
# String variant case - check for exact match or boolean true
187+
if isinstance(actual_value, str) and len(actual_value) > 0:
188+
if isinstance(expected_value, bool):
189+
# Any variant matches boolean true
190+
return expected_value
191+
elif isinstance(expected_value, str):
192+
# String comparison (case-insensitive to match C# StringComparison.OrdinalIgnoreCase)
193+
return actual_value.lower() == expected_value.lower()
194+
else:
195+
return False
196+
197+
# Boolean case - must match expected boolean value
198+
elif isinstance(actual_value, bool) and isinstance(expected_value, bool):
199+
return actual_value == expected_value
200+
201+
# Default case
202+
return False
203+
204+
145205
def match_feature_flag_properties(
146206
flag,
147207
distinct_id,

‎posthog/test/test_feature_flags.py‎

Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1835,6 +1835,270 @@ def test_flag_dependencies_without_context_raises_inconclusive(self):
18351835
self.assertIn("Cannot evaluate flag dependency", str(cm.exception))
18361836
self.assertIn("some-flag", str(cm.exception))
18371837

1838+
@mock.patch("posthog.client.flags")
1839+
@mock.patch("posthog.client.get")
1840+
def test_multi_level_multivariate_dependency_chain(self, patch_get, patch_flags):
1841+
"""Test multi-level multivariate dependency chain: dependent-flag -> intermediate-flag -> leaf-flag"""
1842+
client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
1843+
client.feature_flags = [
1844+
# Leaf flag: multivariate with "control" and "test" variants using person property overrides
1845+
{
1846+
"id": 1,
1847+
"name": "Leaf Flag",
1848+
"key": "leaf-flag",
1849+
"active": True,
1850+
"rollout_percentage": 100,
1851+
"filters": {
1852+
"groups": [
1853+
{
1854+
"properties": [
1855+
{
1856+
"key": "email",
1857+
"type": "person",
1858+
"value": "[email protected]",
1859+
"operator": "exact",
1860+
}
1861+
],
1862+
"rollout_percentage": 100,
1863+
"variant": "control",
1864+
},
1865+
{
1866+
"properties": [
1867+
{
1868+
"key": "email",
1869+
"type": "person",
1870+
"value": "[email protected]",
1871+
"operator": "exact",
1872+
}
1873+
],
1874+
"rollout_percentage": 100,
1875+
"variant": "test",
1876+
},
1877+
{"rollout_percentage": 50, "variant": "control"}, # Default fallback
1878+
],
1879+
"multivariate": {
1880+
"variants": [
1881+
{
1882+
"key": "control",
1883+
"name": "Control",
1884+
"rollout_percentage": 50,
1885+
},
1886+
{"key": "test", "name": "Test", "rollout_percentage": 50},
1887+
]
1888+
},
1889+
},
1890+
},
1891+
# Intermediate flag: multivariate with "blue" and "green" variants, depends on leaf-flag="control"
1892+
{
1893+
"id": 2,
1894+
"name": "Intermediate Flag",
1895+
"key": "intermediate-flag",
1896+
"active": True,
1897+
"rollout_percentage": 100,
1898+
"filters": {
1899+
"groups": [
1900+
{
1901+
"properties": [
1902+
{
1903+
"key": "leaf-flag",
1904+
"operator": "flag_evaluates_to",
1905+
"value": "control",
1906+
"type": "flag",
1907+
"dependency_chain": ["leaf-flag"],
1908+
},
1909+
{
1910+
"key": "variant_type",
1911+
"type": "person",
1912+
"value": "blue",
1913+
"operator": "exact",
1914+
}
1915+
],
1916+
"rollout_percentage": 100,
1917+
"variant": "blue",
1918+
},
1919+
{
1920+
"properties": [
1921+
{
1922+
"key": "leaf-flag",
1923+
"operator": "flag_evaluates_to",
1924+
"value": "control",
1925+
"type": "flag",
1926+
"dependency_chain": ["leaf-flag"],
1927+
},
1928+
{
1929+
"key": "variant_type",
1930+
"type": "person",
1931+
"value": "green",
1932+
"operator": "exact",
1933+
}
1934+
],
1935+
"rollout_percentage": 100,
1936+
"variant": "green",
1937+
}
1938+
],
1939+
"multivariate": {
1940+
"variants": [
1941+
{"key": "blue", "name": "Blue", "rollout_percentage": 50},
1942+
{"key": "green", "name": "Green", "rollout_percentage": 50},
1943+
]
1944+
},
1945+
},
1946+
},
1947+
# Dependent flag: boolean flag that depends on intermediate-flag="blue"
1948+
{
1949+
"id": 3,
1950+
"name": "Dependent Flag",
1951+
"key": "dependent-flag",
1952+
"active": True,
1953+
"rollout_percentage": 100,
1954+
"filters": {
1955+
"groups": [
1956+
{
1957+
"properties": [
1958+
{
1959+
"key": "intermediate-flag",
1960+
"operator": "flag_evaluates_to",
1961+
"value": "blue",
1962+
"type": "flag",
1963+
"dependency_chain": [
1964+
"leaf-flag",
1965+
"intermediate-flag",
1966+
],
1967+
}
1968+
],
1969+
"rollout_percentage": 100,
1970+
}
1971+
],
1972+
},
1973+
},
1974+
]
1975+
1976+
# Test using person properties and variant overrides to ensure predictable variants
1977+
1978+
# Test 1: Make sure the leaf flag evaluates to the variant we expect using email overrides
1979+
self.assertEqual(
1980+
"control",
1981+
client.get_feature_flag(
1982+
"leaf-flag",
1983+
"any-user",
1984+
person_properties={"email": "[email protected]"},
1985+
),
1986+
)
1987+
self.assertEqual(
1988+
"test",
1989+
client.get_feature_flag(
1990+
"leaf-flag",
1991+
"any-user",
1992+
person_properties={"email": "[email protected]"},
1993+
),
1994+
)
1995+
1996+
# Test 2: Make sure the intermediate flag evaluates to the expected variants when dependency is satisfied
1997+
self.assertEqual(
1998+
"blue",
1999+
client.get_feature_flag(
2000+
"intermediate-flag",
2001+
"any-user",
2002+
person_properties={
2003+
"email": "[email protected]",
2004+
"variant_type": "blue"
2005+
},
2006+
),
2007+
)
2008+
2009+
self.assertEqual(
2010+
"green",
2011+
client.get_feature_flag(
2012+
"intermediate-flag",
2013+
"any-user",
2014+
person_properties={
2015+
"email": "[email protected]",
2016+
"variant_type": "green"
2017+
},
2018+
),
2019+
)
2020+
2021+
# Test 3: Make sure the intermediate flag evaluates to false when leaf dependency fails
2022+
self.assertEqual(
2023+
False,
2024+
client.get_feature_flag(
2025+
"intermediate-flag",
2026+
"any-user",
2027+
person_properties={
2028+
"email": "[email protected]", # This makes leaf-flag="test", breaking dependency
2029+
"variant_type": "blue"
2030+
},
2031+
),
2032+
)
2033+
2034+
# Test 4: When leaf-flag="control", intermediate="blue", dependent should be true
2035+
self.assertEqual(
2036+
True,
2037+
client.get_feature_flag(
2038+
"dependent-flag",
2039+
"any-user",
2040+
person_properties={
2041+
"email": "[email protected]",
2042+
"variant_type": "blue"
2043+
},
2044+
),
2045+
)
2046+
2047+
# Test 5: When leaf-flag="control", intermediate="green", dependent should be false
2048+
self.assertEqual(
2049+
False,
2050+
client.get_feature_flag(
2051+
"dependent-flag",
2052+
"any-user",
2053+
person_properties={
2054+
"email": "[email protected]",
2055+
"variant_type": "green"
2056+
},
2057+
),
2058+
)
2059+
2060+
# Test 6: When leaf-flag="test", intermediate is False, dependent should be false
2061+
self.assertEqual(
2062+
False,
2063+
client.get_feature_flag(
2064+
"dependent-flag",
2065+
"any-user",
2066+
person_properties={
2067+
"email": "[email protected]",
2068+
"variant_type": "blue"
2069+
},
2070+
),
2071+
)
2072+
2073+
def test_matches_dependency_value(self):
2074+
"""Test the matches_dependency_value function logic"""
2075+
from posthog.feature_flags import matches_dependency_value
2076+
2077+
# String variant matches string exactly (case-insensitive)
2078+
self.assertTrue(matches_dependency_value("control", "control"))
2079+
self.assertTrue(matches_dependency_value("Control", "control"))
2080+
self.assertTrue(matches_dependency_value("CONTROL", "control"))
2081+
self.assertFalse(matches_dependency_value("test", "control"))
2082+
2083+
# String variant matches boolean true (any variant is truthy)
2084+
self.assertTrue(matches_dependency_value("control", True))
2085+
self.assertTrue(matches_dependency_value("test", True))
2086+
self.assertFalse(matches_dependency_value("control", False))
2087+
2088+
# Boolean matches boolean exactly
2089+
self.assertTrue(matches_dependency_value(True, True))
2090+
self.assertTrue(matches_dependency_value(False, False))
2091+
self.assertFalse(matches_dependency_value(True, False))
2092+
self.assertFalse(matches_dependency_value(False, True))
2093+
2094+
# Empty string doesn't match
2095+
self.assertFalse(matches_dependency_value("", True))
2096+
self.assertFalse(matches_dependency_value("", "control"))
2097+
2098+
# Type mismatches
2099+
self.assertFalse(matches_dependency_value("control", 123))
2100+
self.assertFalse(matches_dependency_value(True, "control"))
2101+
18382102
@mock.patch("posthog.client.Poller")
18392103
@mock.patch("posthog.client.get")
18402104
def test_load_feature_flags(self, patch_get, patch_poll):

0 commit comments

Comments
 (0)