From ecb0dfe92d61ca0f8208f04a596b521a2956c713 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 31 Aug 2026 12:44:27 +0100 Subject: [PATCH 1/9] feat: evaluate flags that depend on other flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds support for segments conditioned on another flag's result, via a `$.flags.` condition property, as the evaluation half of dependent flags. Segment conditions are already JSONPath, so a dependency needs no new operator or condition type. What it does need is for the flag to be resolved by the time the condition reads it, and evaluation resolved every segment before any flag. Rather than establish up front which segments depend on which flags, `$.flags` is a mapping that resolves a flag when a condition first reads it. Resolving a flag evaluates the segments overriding it, which in turn resolves whatever flags their conditions read, memoised throughout. The existing single pass is otherwise untouched: a context whose segments read no flag never enters the resolver, and pays only for one empty mapping. Resolving on read rather than in advance means there is no need to recognise a dependency in a condition property, and so no need to reimplement enough of the JSONPath grammar to tell that `$.flags.a['enabled']` and `$.flags.a.enabled` are the same query. Every spelling works because resolution is triggered by the read itself. It also costs only what is read: rule short-circuiting means a condition that is never evaluated resolves nothing. Measured against main across the benchmark contexts, interleaved to cancel drift: +3.7%, from the mapping and a shallow copy of the context to hold it. Establishing up front that an environment has no dependencies would remove that residual entirely, but needs a field on the context to carry it. A flag whose dependencies form a cycle is not resolvable. It serves its environment default and reports `ERROR; code=CIRCULAR_DEPENDENCY`, so that a flag which could not be resolved is distinguishable from one that was never gated, and only flags in the cycle are reported that way. Crucially the value it falls back to is never published to `$.flags`, so no other condition can match on a value that exists only because the cycle was cut — otherwise a segment gated on a flag that defaults to enabled would be reported as matched while the flag it overrides stayed at its default. Cycles are still expected to be rejected where dependencies are written. Behaviour is covered by test cases in Flagsmith/engine-test-data#59 rather than by unit tests here, so that every engine is held to it. --- flag_engine/context/types.py | 2 + flag_engine/segments/evaluator.py | 183 +++++++++++++++++++++++++++++- 2 files changed, 183 insertions(+), 2 deletions(-) diff --git a/flag_engine/context/types.py b/flag_engine/context/types.py index 328face..9e23dcc 100644 --- a/flag_engine/context/types.py +++ b/flag_engine/context/types.py @@ -8,6 +8,7 @@ from typing_extensions import NotRequired, TypedDict +from flag_engine.result.types import FlagResult from flag_engine.segments.types import ( ConditionOperator, ContextValue, @@ -79,3 +80,4 @@ class EvaluationContext(TypedDict, Generic[SegmentMetadataT, FeatureMetadataT]): identity: NotRequired[Optional[IdentityContext]] segments: NotRequired[Dict[str, SegmentContext[SegmentMetadataT, FeatureMetadataT]]] features: NotRequired[Dict[str, FeatureContext[FeatureMetadataT]]] + flags: NotRequired[Dict[str, FlagResult[FeatureMetadataT]]] diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 149e02e..daca2b5 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -6,7 +6,7 @@ import typing import warnings from contextlib import suppress -from functools import lru_cache, partial, wraps +from functools import cached_property, lru_cache, partial, wraps import jsonpath_rfc9535 import semver @@ -58,15 +58,55 @@ def get_evaluation_result( :return: EvaluationResult containing the context, flags, and segments """ context = get_enriched_context(context) + + resolved: _LazyFlags = _LazyFlags() + context = {**context, "flags": resolved} + resolved.bind(context) + segments, segment_overrides = evaluate_segments(context) flags = evaluate_features(context, segment_overrides) + if (resolver := resolved.__dict__.get("_resolver")) is not None: + # Only reached when a segment condition read a flag. Those results take + # precedence: they were resolved with the cycle guard, unlike the + # single-pass recomputation above. + flags.update(resolved) + for feature_name in resolver.cyclic: + if (flag := flags.get(feature_name)) is not None: + flag["reason"] = CIRCULAR_DEPENDENCY_REASON + return { "flags": flags, "segments": segments, } +class _LazyFlags(dict[str, FlagResult[typing.Any]]): + """ + The `$.flags` mapping, resolving a flag when a condition first reads it. + + A dict subclass rather than a Mapping, because the JSONPath implementation + only traverses real dicts; `__missing__` is what makes the read lazy. The + resolver is only ever entered by a read, so a context whose segments read + no flag never leaves the single-pass path. + """ + + _context: _EvaluationContextAnyMeta + + def bind(self, context: _EvaluationContextAnyMeta) -> None: + self._context = context + + @cached_property + def _resolver(self) -> _DependencyResolver[typing.Any, typing.Any]: + # Built on the first read of a flag, so a context whose segments read + # none never pays for it. + return _DependencyResolver(self._context, self) + + def __missing__(self, key: str) -> typing.Optional[FlagResult[typing.Any]]: + self._resolver.resolve_feature(key) + return self.get(key) + + def get_enriched_context( context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], ) -> EvaluationContext[SegmentMetadataT, FeatureMetadataT]: @@ -162,6 +202,145 @@ def evaluate_features( return flags +# A condition property is only treated as a JSONPath query when it carries this +# prefix; anything else is a trait key. +_JSONPATH_PREFIX = "$." + +# Reported in place of `DEFAULT` for a flag that could not be resolved because +# its dependencies form a cycle, so that the flag serving its environment +# default is distinguishable from one that was never gated at all. +CIRCULAR_DEPENDENCY_REASON = "ERROR; code=CIRCULAR_DEPENDENCY" + + +class _DependencyResolver(typing.Generic[SegmentMetadataT, FeatureMetadataT]): + """ + Resolves flags and segment membership for a context with flag dependencies, + memoising both. + + A flag is resolved by first evaluating every segment that overrides it, + which in turn resolves any flag those segments are conditioned on. A flag + involved in a dependency cycle is left unresolved rather than raising, so + that a cycle degrades to a non-matching condition instead of breaking + evaluation for the whole context. Cycles are expected to be rejected when + dependencies are written, not here. + """ + + def __init__( + self, + context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], + flags: dict[str, FlagResult[FeatureMetadataT]], + ) -> None: + self._context = context + self._flags = flags + self._cycle_hits = 0 + self.cyclic: set[str] = set() + self._segment_matches: dict[str, bool] = {} + self._resolving: list[str] = [] + # Feature name to the keys of the segments overriding it, in context + # order, so that override precedence doesn't depend on resolution order. + self._segment_keys_by_feature_name: dict[str, list[str]] = {} + for segment_key, segment_context in (context.get("segments") or {}).items(): + for override in segment_context.get("overrides") or (): + self._segment_keys_by_feature_name.setdefault( + override["name"], [] + ).append(segment_key) + + def resolve_feature(self, feature_name: str) -> None: + # Only ever reached from `_LazyFlags.__missing__`, so the flag is known + # not to be resolved yet; a second read of a resolved flag is a plain + # dict hit and never arrives here. + if feature_name in self._resolving: + # Cyclic dependency. Leave the flag unresolved so that the + # condition that led here sees no value, and so doesn't match. + # Every flag from the re-entered one upwards is part of the cycle, + # and is reported as such; anything below merely depends on it. + cycle_start = self._resolving.index(feature_name) + self.cyclic.update(self._resolving[cycle_start:]) + self._cycle_hits += 1 + return + if not ( + feature_context := (self._context.get("features") or {}).get(feature_name) + ): + # Depending on a feature absent from the context is not an error; + # it resolves to no value, as an unset property would. + return + + self._resolving.append(feature_name) + try: + segment_override = self._get_segment_override(feature_name) + finally: + # A `KeyError` raised under here is swallowed by the JSONPath + # implementation and evaluation carries on, so a name left on the + # stack would silently look like a cycle to a later read. + self._resolving.pop() + + if feature_name in self.cyclic: + # Resolved only by cutting a cycle, so the result is not something + # another condition may match on. Leaving it unpublished keeps + # every read of it empty, and the reason is applied to the + # single-pass result at the end. + return + + if segment_override is not None: + segment_name = segment_override["segment_name"] + self._flags[feature_name] = get_flag_result_from_context( + context=self._context, + feature_context=segment_override["feature_context"], + reason=f"TARGETING_MATCH; segment={segment_name}", + ) + else: + self._flags[feature_name] = get_flag_result_from_context( + context=self._context, + feature_context=feature_context, + reason="DEFAULT", + ) + + def matches_segment(self, segment_key: str) -> bool: + if (matches := self._segment_matches.get(segment_key)) is not None: + return matches + + cycle_hits = self._cycle_hits + matches = is_context_in_segment( + self._context, + (self._context.get("segments") or {})[segment_key], + ) + + if self._cycle_hits == cycle_hits: + # Only memoise a verdict reached without breaking a cycle. In a + # cycle a segment can be evaluated against an unresolved flag, and + # that verdict mustn't be reused afterwards. + self._segment_matches[segment_key] = matches + + return matches + + def _get_segment_override( + self, + feature_name: str, + ) -> typing.Optional[SegmentOverride[FeatureMetadataT]]: + segment_override: typing.Optional[SegmentOverride[FeatureMetadataT]] = None + override_priority = constants.DEFAULT_PRIORITY + + for segment_key in self._segment_keys_by_feature_name.get(feature_name) or (): + if not self.matches_segment(segment_key): + continue + segment_context = (self._context.get("segments") or {})[segment_key] + for override_feature_context in segment_context.get("overrides") or (): + if override_feature_context["name"] != feature_name: + continue + priority = override_feature_context.get( + "priority", + constants.DEFAULT_PRIORITY, + ) + if segment_override is None or priority < override_priority: + segment_override = SegmentOverride( + feature_context=override_feature_context, + segment_name=segment_context["name"], + ) + override_priority = priority + + return segment_override + + def get_flag_result_from_context( context: _EvaluationContextAnyMeta, feature_context: FeatureContext[FeatureMetadataT], @@ -321,7 +500,7 @@ def get_context_value( property: str, ) -> ContextValue: value = None - if property.startswith("$."): + if property.startswith(_JSONPATH_PREFIX): value = _get_context_value_getter(property)(context) else: value = _get_trait_value(context, property) From 99dea3ee453a31985a388842456d3a95dc0fda1c Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 31 Aug 2026 12:44:30 +0100 Subject: [PATCH 2/9] chore: point engine-test-data at the flag dependencies branch REVERT BEFORE MERGE. Dependent flags behaviour is covered by test cases in Flagsmith/engine-test-data#59 rather than by unit tests here, so that every engine is held to it. Those cases aren't in a release yet, so without this the new code is exercised by nothing and CI's 100% coverage gate fails. Once #59 is merged and tagged, this goes back to a semver tag, which Renovate now tracks as of #337. --- .gitmodules | 2 +- tests/engine_tests/engine-test-data | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index 606c611..8e1f9fd 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/engine_tests/engine-test-data"] path = tests/engine_tests/engine-test-data url = https://github.com/flagsmith/engine-test-data.git - branch = v3.9.0 + branch = test/flag-dependencies diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 5031065..805b353 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 5031065965d5ddbb499e1f5430657b80a609337c +Subproject commit 805b3537d8f68f235fa3d5450c3606d345ba1551 From 419d6861b88f4b9322d59739935d8172df90aa68 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 31 Aug 2026 12:54:09 +0100 Subject: [PATCH 3/9] improve docs --- flag_engine/segments/evaluator.py | 44 ++++--------------------------- 1 file changed, 5 insertions(+), 39 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index daca2b5..71a1610 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -84,11 +84,6 @@ def get_evaluation_result( class _LazyFlags(dict[str, FlagResult[typing.Any]]): """ The `$.flags` mapping, resolving a flag when a condition first reads it. - - A dict subclass rather than a Mapping, because the JSONPath implementation - only traverses real dicts; `__missing__` is what makes the read lazy. The - resolver is only ever entered by a read, so a context whose segments read - no flag never leaves the single-pass path. """ _context: _EvaluationContextAnyMeta @@ -98,8 +93,6 @@ def bind(self, context: _EvaluationContextAnyMeta) -> None: @cached_property def _resolver(self) -> _DependencyResolver[typing.Any, typing.Any]: - # Built on the first read of a flag, so a context whose segments read - # none never pays for it. return _DependencyResolver(self._context, self) def __missing__(self, key: str) -> typing.Optional[FlagResult[typing.Any]]: @@ -202,27 +195,14 @@ def evaluate_features( return flags -# A condition property is only treated as a JSONPath query when it carries this -# prefix; anything else is a trait key. _JSONPATH_PREFIX = "$." -# Reported in place of `DEFAULT` for a flag that could not be resolved because -# its dependencies form a cycle, so that the flag serving its environment -# default is distinguishable from one that was never gated at all. CIRCULAR_DEPENDENCY_REASON = "ERROR; code=CIRCULAR_DEPENDENCY" class _DependencyResolver(typing.Generic[SegmentMetadataT, FeatureMetadataT]): """ - Resolves flags and segment membership for a context with flag dependencies, - memoising both. - - A flag is resolved by first evaluating every segment that overrides it, - which in turn resolves any flag those segments are conditioned on. A flag - involved in a dependency cycle is left unresolved rather than raising, so - that a cycle degrades to a non-matching condition instead of breaking - evaluation for the whole context. Cycles are expected to be rejected when - dependencies are written, not here. + Resolves overrides for a context with flag dependencies, memoising the results. """ def __init__( @@ -236,8 +216,6 @@ def __init__( self.cyclic: set[str] = set() self._segment_matches: dict[str, bool] = {} self._resolving: list[str] = [] - # Feature name to the keys of the segments overriding it, in context - # order, so that override precedence doesn't depend on resolution order. self._segment_keys_by_feature_name: dict[str, list[str]] = {} for segment_key, segment_context in (context.get("segments") or {}).items(): for override in segment_context.get("overrides") or (): @@ -246,14 +224,8 @@ def __init__( ).append(segment_key) def resolve_feature(self, feature_name: str) -> None: - # Only ever reached from `_LazyFlags.__missing__`, so the flag is known - # not to be resolved yet; a second read of a resolved flag is a plain - # dict hit and never arrives here. if feature_name in self._resolving: - # Cyclic dependency. Leave the flag unresolved so that the - # condition that led here sees no value, and so doesn't match. - # Every flag from the re-entered one upwards is part of the cycle, - # and is reported as such; anything below merely depends on it. + # Cyclic dependency. Leave the flag unresolved. cycle_start = self._resolving.index(feature_name) self.cyclic.update(self._resolving[cycle_start:]) self._cycle_hits += 1 @@ -261,8 +233,7 @@ def resolve_feature(self, feature_name: str) -> None: if not ( feature_context := (self._context.get("features") or {}).get(feature_name) ): - # Depending on a feature absent from the context is not an error; - # it resolves to no value, as an unset property would. + # Depending on a feature absent from the context. return self._resolving.append(feature_name) @@ -275,10 +246,7 @@ def resolve_feature(self, feature_name: str) -> None: self._resolving.pop() if feature_name in self.cyclic: - # Resolved only by cutting a cycle, so the result is not something - # another condition may match on. Leaving it unpublished keeps - # every read of it empty, and the reason is applied to the - # single-pass result at the end. + # The result is not something another condition may match on. return if segment_override is not None: @@ -306,9 +274,7 @@ def matches_segment(self, segment_key: str) -> bool: ) if self._cycle_hits == cycle_hits: - # Only memoise a verdict reached without breaking a cycle. In a - # cycle a segment can be evaluated against an unresolved flag, and - # that verdict mustn't be reused afterwards. + # Only memoise a verdict reached without breaking a cycle. self._segment_matches[segment_key] = matches return matches From bf7ef5176f2a02613b1642494c320dda71842edc Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 1 Sep 2026 12:09:46 +0100 Subject: [PATCH 4/9] chore: bump engine-test-data to the reviewed case set Follows Flagsmith/engine-test-data#59, where two cases were dropped as redundant and one gained a clarifying note. Coverage is unaffected. --- tests/engine_tests/engine-test-data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 805b353..4299e54 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 805b3537d8f68f235fa3d5450c3606d345ba1551 +Subproject commit 4299e542c5bac70540649071903dc6487efdfc07 From 2f07f941b47d535b141692d8f95cfdef31729f47 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 1 Sep 2026 12:19:52 +0100 Subject: [PATCH 5/9] chore: bump engine-test-data for the cycle case rename --- tests/engine_tests/engine-test-data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 4299e54..d75acfa 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 4299e542c5bac70540649071903dc6487efdfc07 +Subproject commit d75acfa658a02a67f444de197b25216696645bc5 From 3c33febcd322fbafd6f009b4e0bef0208633de36 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 1 Sep 2026 12:40:29 +0100 Subject: [PATCH 6/9] fix: do not apply overrides from a segment whose evaluation cut a cycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `IS_NOT_SET` matches when a property has no value, and a flag left unresolved by a cycle has none. A segment overriding the very flag it tests for absence therefore had its condition made true by the cycle, and its override applied — reported with the circular dependency reason but carrying the override's value rather than the environment default the reason claims. A segment whose own evaluation cut a cycle read a flag that could not be resolved, so neither its match nor its failure to match rests on anything, and it must not be applied either way. The cycle counter moves onto the flags mapping so that both the resolver and the single pass over segments can see it, and the mapping is now passed to `evaluate_segments` explicitly rather than read back out of the context. Covered by `cyclic_is_not_set__segment_should_not_match` in Flagsmith/engine-test-data#59, which fails without this. --- flag_engine/segments/evaluator.py | 34 +++++++++++++++++++++-------- tests/engine_tests/engine-test-data | 2 +- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 71a1610..398a1c1 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -63,7 +63,7 @@ def get_evaluation_result( context = {**context, "flags": resolved} resolved.bind(context) - segments, segment_overrides = evaluate_segments(context) + segments, segment_overrides = evaluate_segments(context, resolved) flags = evaluate_features(context, segment_overrides) if (resolver := resolved.__dict__.get("_resolver")) is not None: @@ -88,6 +88,11 @@ class _LazyFlags(dict[str, FlagResult[typing.Any]]): _context: _EvaluationContextAnyMeta + #: Incremented whenever a cycle is cut. A segment whose own evaluation + #: increments it read a flag that could not be resolved, so its verdict + #: rests on that flag's absence and it must not match. + cycle_hits: int = 0 + def bind(self, context: _EvaluationContextAnyMeta) -> None: self._context = context @@ -125,6 +130,7 @@ def get_enriched_context( def evaluate_segments( context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], + flags: "_LazyFlags", ) -> typing.Tuple[ list[SegmentResult[SegmentMetadataT]], SegmentOverrides[FeatureMetadataT], @@ -134,9 +140,18 @@ def evaluate_segments( segment_results: list[SegmentResult[SegmentMetadataT]] = [] segment_overrides: SegmentOverrides[FeatureMetadataT] = {} + cycle_hits = flags.cycle_hits for segment_context in segment_contexts.values(): - if not is_context_in_segment(context, segment_context): + matches = is_context_in_segment(context, segment_context) + + if (hits := flags.cycle_hits) != cycle_hits: + # Evaluating this segment cut a cycle, so it matched, or failed to, + # on a flag that could not be resolved. Neither verdict is sound. + cycle_hits = hits + continue + + if not matches: continue segment_result: SegmentResult[SegmentMetadataT] = { @@ -208,11 +223,10 @@ class _DependencyResolver(typing.Generic[SegmentMetadataT, FeatureMetadataT]): def __init__( self, context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], - flags: dict[str, FlagResult[FeatureMetadataT]], + flags: "_LazyFlags", ) -> None: self._context = context self._flags = flags - self._cycle_hits = 0 self.cyclic: set[str] = set() self._segment_matches: dict[str, bool] = {} self._resolving: list[str] = [] @@ -228,7 +242,7 @@ def resolve_feature(self, feature_name: str) -> None: # Cyclic dependency. Leave the flag unresolved. cycle_start = self._resolving.index(feature_name) self.cyclic.update(self._resolving[cycle_start:]) - self._cycle_hits += 1 + self._flags.cycle_hits += 1 return if not ( feature_context := (self._context.get("features") or {}).get(feature_name) @@ -267,16 +281,18 @@ def matches_segment(self, segment_key: str) -> bool: if (matches := self._segment_matches.get(segment_key)) is not None: return matches - cycle_hits = self._cycle_hits + cycle_hits = self._flags.cycle_hits matches = is_context_in_segment( self._context, (self._context.get("segments") or {})[segment_key], ) - if self._cycle_hits == cycle_hits: - # Only memoise a verdict reached without breaking a cycle. - self._segment_matches[segment_key] = matches + if self._flags.cycle_hits != cycle_hits: + # Reached by cutting a cycle, so the verdict rests on a flag that + # could not be resolved. Not a match, and not worth memoising. + return False + self._segment_matches[segment_key] = matches return matches def _get_segment_override( diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index d75acfa..c7a9173 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit d75acfa658a02a67f444de197b25216696645bc5 +Subproject commit c7a9173a56874dcf53359b5ef8bee1e863520b94 From 1b25a0765771f6ab127d160cf385f7c7e1885a1d Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 1 Sep 2026 12:41:30 +0100 Subject: [PATCH 7/9] refactor: track dependency resolution on the flags mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Detecting whether a segment consulted a flag was done by reaching into `__dict__` to see whether a cached property had been built, which is both obscure and an access to a private member from outside. An empty mapping cannot answer the question on its own, because a flag in a cycle resolves to nothing and is deliberately not published. The mapping now records it directly. `used` says a condition read a flag, and `cyclic` — which the resolver already maintained — moves alongside it, so nothing outside the resolver reaches into it. Both are created on the first read rather than up front, so a context whose segments read no flag allocates neither. --- flag_engine/segments/evaluator.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 398a1c1..98242c3 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -66,12 +66,12 @@ def get_evaluation_result( segments, segment_overrides = evaluate_segments(context, resolved) flags = evaluate_features(context, segment_overrides) - if (resolver := resolved.__dict__.get("_resolver")) is not None: + if resolved.used: # Only reached when a segment condition read a flag. Those results take # precedence: they were resolved with the cycle guard, unlike the # single-pass recomputation above. flags.update(resolved) - for feature_name in resolver.cyclic: + for feature_name in resolved.cyclic: if (flag := flags.get(feature_name)) is not None: flag["reason"] = CIRCULAR_DEPENDENCY_REASON @@ -88,6 +88,14 @@ class _LazyFlags(dict[str, FlagResult[typing.Any]]): _context: _EvaluationContextAnyMeta + #: Names of the flags found to be in a dependency cycle. + cyclic: set[str] + + #: Set the first time a condition reads a flag. Distinguishes "no + #: dependency was ever consulted" from "one was, and resolved to nothing", + #: which an empty mapping cannot. + used: bool = False + #: Incremented whenever a cycle is cut. A segment whose own evaluation #: increments it read a flag that could not be resolved, so its verdict #: rests on that flag's absence and it must not match. @@ -101,6 +109,11 @@ def _resolver(self) -> _DependencyResolver[typing.Any, typing.Any]: return _DependencyResolver(self._context, self) def __missing__(self, key: str) -> typing.Optional[FlagResult[typing.Any]]: + if not self.used: + # Both are only needed once a condition has read a flag, so a + # context whose segments read none allocates neither. + self.used = True + self.cyclic = set() self._resolver.resolve_feature(key) return self.get(key) @@ -227,7 +240,6 @@ def __init__( ) -> None: self._context = context self._flags = flags - self.cyclic: set[str] = set() self._segment_matches: dict[str, bool] = {} self._resolving: list[str] = [] self._segment_keys_by_feature_name: dict[str, list[str]] = {} @@ -241,7 +253,7 @@ def resolve_feature(self, feature_name: str) -> None: if feature_name in self._resolving: # Cyclic dependency. Leave the flag unresolved. cycle_start = self._resolving.index(feature_name) - self.cyclic.update(self._resolving[cycle_start:]) + self._flags.cyclic.update(self._resolving[cycle_start:]) self._flags.cycle_hits += 1 return if not ( @@ -259,7 +271,7 @@ def resolve_feature(self, feature_name: str) -> None: # stack would silently look like a cycle to a later read. self._resolving.pop() - if feature_name in self.cyclic: + if feature_name in self._flags.cyclic: # The result is not something another condition may match on. return From 3ec060bbf2b21904a2f9f7d45d5da35919fd28c7 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 1 Sep 2026 12:42:13 +0100 Subject: [PATCH 8/9] refactor: share override precedence between the two callers Which of two competing segment overrides wins was implemented twice, once while collecting overrides across segments and once while resolving a single feature's dependencies. Two copies of a precedence rule is the kind of thing that drifts, and the second copy tracked the winning priority in a separate variable to do it. Both now call `_wins_over`, which states the rule once: lower priority wins, and the first seen wins a tie. --- flag_engine/segments/evaluator.py | 56 +++++++++++++++++-------------- 1 file changed, 31 insertions(+), 25 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 98242c3..9b96efa 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -141,6 +141,22 @@ def get_enriched_context( return context +def _wins_over( + candidate: FeatureContext[FeatureMetadataT], + incumbent: typing.Optional[FeatureContext[FeatureMetadataT]], +) -> bool: + """ + Whether a segment override takes precedence over the one held so far. + + Lower priority wins, and the first seen wins a tie, so that precedence + follows context order rather than the order segments happen to be + evaluated in. + """ + return incumbent is None or candidate.get( + "priority", constants.DEFAULT_PRIORITY + ) < incumbent.get("priority", constants.DEFAULT_PRIORITY) + + def evaluate_segments( context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], flags: "_LazyFlags", @@ -174,24 +190,17 @@ def evaluate_segments( segment_result["metadata"] = segment_metadata segment_results.append(segment_result) - if overrides := segment_context.get("overrides"): - for override_feature_context in overrides: - feature_name = override_feature_context["name"] - if ( - feature_name not in segment_overrides - or override_feature_context.get( - "priority", - constants.DEFAULT_PRIORITY, - ) - < (segment_overrides[feature_name]["feature_context"]).get( - "priority", - constants.DEFAULT_PRIORITY, - ) - ): - segment_overrides[feature_name] = SegmentOverride( - feature_context=override_feature_context, - segment_name=segment_context["name"], - ) + for override_feature_context in segment_context.get("overrides") or (): + feature_name = override_feature_context["name"] + incumbent = segment_overrides.get(feature_name) + if _wins_over( + override_feature_context, + incumbent["feature_context"] if incumbent else None, + ): + segment_overrides[feature_name] = SegmentOverride( + feature_context=override_feature_context, + segment_name=segment_context["name"], + ) return segment_results, segment_overrides @@ -312,7 +321,6 @@ def _get_segment_override( feature_name: str, ) -> typing.Optional[SegmentOverride[FeatureMetadataT]]: segment_override: typing.Optional[SegmentOverride[FeatureMetadataT]] = None - override_priority = constants.DEFAULT_PRIORITY for segment_key in self._segment_keys_by_feature_name.get(feature_name) or (): if not self.matches_segment(segment_key): @@ -321,16 +329,14 @@ def _get_segment_override( for override_feature_context in segment_context.get("overrides") or (): if override_feature_context["name"] != feature_name: continue - priority = override_feature_context.get( - "priority", - constants.DEFAULT_PRIORITY, - ) - if segment_override is None or priority < override_priority: + if _wins_over( + override_feature_context, + segment_override["feature_context"] if segment_override else None, + ): segment_override = SegmentOverride( feature_context=override_feature_context, segment_name=segment_context["name"], ) - override_priority = priority return segment_override From aac80cbceb732c96a4d904a7abc8ea03ad80271b Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 1 Sep 2026 17:05:18 +0100 Subject: [PATCH 9/9] chore: point engine-test-data at v3.11.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discharges the REVERT BEFORE MERGE on 99dea3e. Flagsmith/engine-test-data#59 is merged and released, so the submodule goes back to a semver tag — the form Renovate tracks as of #337 — rather than a branch. --- .gitmodules | 2 +- tests/engine_tests/engine-test-data | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index 8e1f9fd..935bed8 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/engine_tests/engine-test-data"] path = tests/engine_tests/engine-test-data url = https://github.com/flagsmith/engine-test-data.git - branch = test/flag-dependencies + branch = v3.11.0 diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index c7a9173..28363b3 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit c7a9173a56874dcf53359b5ef8bee1e863520b94 +Subproject commit 28363b388de2850c7c3f1b6e7a8bdd41d8da433c