From 4807e66e734fdd457be20267d0b3c53cbc1d00ae Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 16 Oct 2026 19:18:01 +0200 Subject: [PATCH 1/2] feat: Support `$.flags` context values in segment conditions Environment flags are now evaluated with segments, so identity-less `$.flags` and `$.environment` conditions apply to them. Lazily resolved identity flags overridden by a segment reading `$.flags` are evaluated against the full context, so their dependencies resolve. Closes #245 --- flagsmith/flagsmith.py | 26 +++++--- flagsmith/models.py | 78 ++++++++++++++++++++--- tests/test_flagsmith.py | 137 ++++++++++++++++++++++++++++++++++++++-- tests/test_models.py | 87 +++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 25 deletions(-) diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index d8f34b2..9639d06 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -28,6 +28,7 @@ Flags, Segment, SegmentOverridesIndex, + build_flag_dependent_feature_names, build_segment_overrides_index, ) from flagsmith.offline_handlers import OfflineHandler @@ -137,6 +138,7 @@ def __init__( self._event_processor: typing.Optional[EventProcessor] = None self.__evaluation_context: typing.Optional[SDKEvaluationContext] = None self._segment_overrides_index: SegmentOverridesIndex = {} + self._flag_dependent_features: typing.FrozenSet[str] = frozenset() self._environment_updated_at: typing.Optional[datetime] = None # argument validation @@ -491,15 +493,21 @@ def _evaluation_context( ) -> None: """Swap in a new evaluation context and rebuild the overrides index. - The index maps feature_name -> segments that override it. Built once - per refresh and reused across every subsequent per-identity lazy - resolution; rebuilding here keeps it in sync with the current doc - without any hot-path cost. + The index maps feature_name -> segments that override it, and the + flag-dependent set names features whose overriding segments read + `$.flags`. Both are built once per refresh and reused across every + subsequent per-identity lazy resolution; rebuilding here keeps them + in sync with the current doc without any hot-path cost. """ self.__evaluation_context = context self._segment_overrides_index = ( build_segment_overrides_index(context) if context is not None else {} ) + self._flag_dependent_features = ( + build_flag_dependent_feature_names(context) + if context is not None + else frozenset() + ) def _get_headers( self, @@ -523,13 +531,10 @@ def _get_environment_flags_from_document(self) -> Flags: if self._evaluation_context is None: raise TypeError("No environment present") - # Omit segments from evaluation context for environment flags - # as they are only relevant for identity-specific evaluations - context_without_segments = self._evaluation_context.copy() - context_without_segments.pop("segments", None) - + # Segments are included so that identity-less conditions, e.g. on + # `$.environment` or `$.flags`, apply to environment flags too. evaluation_result = engine.get_evaluation_result( - context=context_without_segments, + context=self._evaluation_context, ) return Flags.from_evaluation_result( @@ -557,6 +562,7 @@ def _get_identity_flags_from_document( return Flags.from_evaluation_context( context=context, overrides_index=self._segment_overrides_index, + flag_dependent_features=self._flag_dependent_features, analytics_processor=self._analytics_processor, default_flag_handler=self.default_flag_handler, ) diff --git a/flagsmith/models.py b/flagsmith/models.py index d29f86c..2caf067 100644 --- a/flagsmith/models.py +++ b/flagsmith/models.py @@ -1,10 +1,11 @@ from __future__ import annotations +import re import typing from dataclasses import dataclass, field from flag_engine import engine -from flag_engine.context.types import SegmentContext +from flag_engine.context.types import SegmentContext, SegmentRule from flagsmith.analytics import AnalyticsProcessor from flagsmith.exceptions import FlagsmithFeatureDoesNotExistError @@ -41,6 +42,43 @@ def build_segment_overrides_index( return index +# Matches JSONPath condition properties that can read `$.flags`: the +# dot (`$.flags`) and bracket (`$['flags']`) forms, plus descendant +# segments (`$..`), which can reach it too. +_FLAGS_PROPERTY_RE = re.compile( + r"^\$\s*(?:\.\.|\.\s*flags\b|\[\s*['\"]flags['\"]\s*\])" +) + + +def _rules_read_flags(rules: typing.Iterable[SegmentRule]) -> bool: + return any( + any( + _FLAGS_PROPERTY_RE.match(condition.get("property") or "") + for condition in rule.get("conditions") or () + ) + or _rules_read_flags(rule.get("rules") or ()) + for rule in rules + ) + + +def build_flag_dependent_feature_names( + context: SDKEvaluationContext, +) -> typing.FrozenSet[str]: + """Names of features overridden by a segment with a `$.flags` condition. + + Such a feature depends on other flags, which the engine resolves + from the context's features and segments. The trimmed contexts used + by `Flags._resolve_flag` leave those out, so these features are + resolved against the full context instead. + """ + return frozenset( + override["name"] + for segment_context in (context.get("segments") or {}).values() + if _rules_read_flags(segment_context["rules"]) + for override in segment_context.get("overrides") or () + ) + + @dataclass(frozen=True) class ExperimentMetadata: """The running experiment a flag was evaluated under; identity evaluations only.""" @@ -127,6 +165,7 @@ class Flags: # paths (`from_evaluation_result` / `from_api_flags`). _context: typing.Optional[SDKEvaluationContext] = None _overrides_index: typing.Optional[SegmentOverridesIndex] = None + _flag_dependent_features: typing.FrozenSet[str] = frozenset() _fully_materialised: bool = False @classmethod @@ -153,20 +192,26 @@ def from_evaluation_context( overrides_index: SegmentOverridesIndex, analytics_processor: typing.Optional[AnalyticsProcessor], default_flag_handler: typing.Optional[typing.Callable[[str], DefaultFlag]], + flag_dependent_features: typing.Optional[typing.FrozenSet[str]] = None, ) -> Flags: """Build a lazy `Flags` backed by an evaluation context. No engine work is done here — flags are resolved on first access via :meth:`_resolve_flag`. Reusing the same `overrides_index` - across calls amortises its construction cost (it's rebuilt only - when the environment doc refreshes, not per identity). + and `flag_dependent_features` across calls amortises their + construction cost (they're rebuilt only when the environment doc + refreshes, not per identity). When `flag_dependent_features` is + omitted, it is derived from `context`. """ + if flag_dependent_features is None: + flag_dependent_features = build_flag_dependent_feature_names(context) return cls( flags={}, default_flag_handler=default_flag_handler, _analytics_processor=analytics_processor, _context=context, _overrides_index=overrides_index, + _flag_dependent_features=flag_dependent_features, ) @classmethod @@ -199,15 +244,19 @@ def all_flags(self) -> typing.List[Flag]: :return: list of Flag objects. """ if self._context is not None and not self._fully_materialised: - result = engine.get_evaluation_result(self._context) - for feature_name, flag_result in result["flags"].items(): - if feature_name not in self.flags: - self.flags[feature_name] = Flag.from_evaluation_result( - flag_result, - ) - self._fully_materialised = True + self._materialise_all() return list(self.flags.values()) + def _materialise_all(self) -> None: + assert self._context is not None + result = engine.get_evaluation_result(self._context) + for feature_name, flag_result in result["flags"].items(): + if feature_name not in self.flags: + self.flags[feature_name] = Flag.from_evaluation_result( + flag_result, + ) + self._fully_materialised = True + def is_feature_enabled(self, feature_name: str) -> bool: """ Check whether a given feature is enabled. @@ -273,6 +322,11 @@ def _resolve_flag(self, feature_name: str) -> Flag: that could override it, looked up in O(1) via the precomputed reverse index — so the engine's full pipeline runs against an input small enough to evaluate in ~1 µs. + + Features overridden by a segment with a `$.flags` condition + depend on other flags, which a trimmed context would leave out, + so they are resolved with the full context instead. Every flag + that evaluation yields is memoised. """ context = self._context overrides_index = self._overrides_index @@ -280,6 +334,10 @@ def _resolve_flag(self, feature_name: str) -> Flag: # non-None checks; assert here so type checkers can narrow. assert context is not None and overrides_index is not None + if feature_name in self._flag_dependent_features: + self._materialise_all() + return self.flags[feature_name] + trimmed: SDKEvaluationContext = { **context, "features": {feature_name: context["features"][feature_name]}, diff --git a/tests/test_flagsmith.py b/tests/test_flagsmith.py index 9296feb..ec4d810 100644 --- a/tests/test_flagsmith.py +++ b/tests/test_flagsmith.py @@ -98,7 +98,7 @@ def test_get_environment_flags_uses_local_environment_when_available( assert all_flags[0].value == "some-value" -def test_get_environment_flags_omits_segments_from_evaluation_context( +def test_get_environment_flags_includes_segments_in_evaluation_context( mocker: MockerFixture, local_eval_flagsmith: Flagsmith, evaluation_context: SDKEvaluationContext, @@ -128,10 +128,137 @@ def test_get_environment_flags_omits_segments_from_evaluation_context( local_eval_flagsmith.get_environment_flags() # Then - # Verify segments are not present in the context passed to the engine - context_without_segments = evaluation_context.copy() - context_without_segments.pop("segments", None) - mock_get_evaluation_result.assert_called_once_with(context=context_without_segments) + # The full context, segments included, is passed to the engine + assert evaluation_context["segments"] + mock_get_evaluation_result.assert_called_once_with(context=evaluation_context) + + +def _feature(name: str, feature_id: int, enabled: bool, value: str) -> typing.Any: + return { + "key": str(feature_id), + "name": name, + "enabled": enabled, + "value": value, + "metadata": {"id": feature_id}, + } + + +def _segment(key: str, prop: str, value: str, override: typing.Any) -> typing.Any: + return { + "key": key, + "name": key, + "rules": [ + { + "type": "ALL", + "conditions": [{"property": prop, "operator": "EQUAL", "value": value}], + } + ], + "overrides": [{**override, "priority": 0}], + } + + +@pytest.fixture() +def context_values_flagsmith(flagsmith: Flagsmith) -> Flagsmith: + """Local-eval client whose segments use `$.flags` and `$.environment`. + + - `dependent` is overridden when `$.flags.dependency.enabled` is true. + - `env_targeted` is overridden when `$.environment.name` matches. + - `dependency` is disabled for identities with the `premium` tier. + """ + flagsmith._evaluation_context = { + "environment": {"key": "env-key", "name": "Test Environment"}, + "features": { + "dependency": _feature("dependency", 1, True, "dependency"), + "dependent": _feature("dependent", 2, False, "base"), + "env_targeted": _feature("env_targeted", 3, False, "base"), + }, + "segments": { + "flags_segment": _segment( + "flags_segment", + "$.flags.dependency.enabled", + "true", + _feature("dependent", 2, True, "overridden"), + ), + "environment_segment": _segment( + "environment_segment", + "$.environment.name", + "Test Environment", + _feature("env_targeted", 3, True, "overridden"), + ), + "premium_segment": _segment( + "premium_segment", + "tier", + "premium", + _feature("dependency", 1, False, "dependency"), + ), + }, + } + flagsmith.enable_local_evaluation = True + return flagsmith + + +@pytest.mark.parametrize("feature_name", ["dependent", "env_targeted"]) +def test_get_environment_flags__identityless_context_value_segment__applies_override( + context_values_flagsmith: Flagsmith, + feature_name: str, +) -> None: + # When + flag = context_values_flagsmith.get_environment_flags().get_flag(feature_name) + + # Then + assert isinstance(flag, Flag) + assert flag.enabled is True + assert flag.value == "overridden" + assert flag.reason is not None + assert flag.reason.startswith("TARGETING_MATCH") + + +def test_get_environment_flags__trait_segment__does_not_apply_override( + context_values_flagsmith: Flagsmith, +) -> None: + # When + flag = context_values_flagsmith.get_environment_flags().get_flag("dependency") + + # Then + assert isinstance(flag, Flag) + assert flag.enabled is True + assert flag.reason == "DEFAULT" + + +@pytest.mark.parametrize( + "traits, expected_enabled, expected_value", + [ + ({}, True, "overridden"), + ({"tier": "premium"}, False, "base"), + ], +) +def test_get_identity_flags__flags_segment__resolves_dependency( + context_values_flagsmith: Flagsmith, + traits: typing.Dict[str, typing.Any], + expected_enabled: bool, + expected_value: str, +) -> None: + # When + # the dependent flag is read on its own, through the lazy path + flag = context_values_flagsmith.get_identity_flags("user", traits).get_flag( + "dependent" + ) + + # Then + # the dependency is resolved for the identity, its overrides included + assert flag.enabled is expected_enabled + assert flag.value == expected_value + + +def test_get_identity_flags__environment_segment__applies_override( + context_values_flagsmith: Flagsmith, +) -> None: + # When + flag = context_values_flagsmith.get_identity_flags("user").get_flag("env_targeted") + + # Then + assert flag.enabled is True + assert flag.value == "overridden" @responses.activate() diff --git a/tests/test_models.py b/tests/test_models.py index af1274b..b3e2790 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,12 +1,15 @@ import typing import pytest +from flag_engine import engine +from pytest_mock import MockerFixture from flagsmith.models import ( DefaultFlag, ExperimentMetadata, Flag, Flags, + build_flag_dependent_feature_names, build_segment_overrides_index, ) from flagsmith.types import ( @@ -561,3 +564,87 @@ def test_build_segment_overrides_index__duplicate_segment_key_fields__keeps_both # Then # both segments are indexed, neither collapses into the other assert set(index["target"]) == {"premium_segment", "enterprise_segment"} + + +@pytest.mark.parametrize( + "condition_property, nested, expected", + [ + ("$.flags.noise_0.enabled", False, {"target"}), + ("$.flags.noise_0.enabled", True, {"target"}), + ("$['flags']['noise 0'].value", False, {"target"}), + ('$["flags"].noise_0.variant', False, {"target"}), + ("$..enabled", False, {"target"}), + ("$.environment.name", False, set()), + ("$.identity.traits.flags", False, set()), + ("$.flagship", False, set()), + ("tier", False, set()), + ], +) +def test_build_flag_dependent_feature_names__returns_features_overridden_by_flags_segments( + lazy_context: SDKEvaluationContext, + condition_property: str, + nested: bool, + expected: typing.Set[str], +) -> None: + # Given + assert lazy_context["segments"] is not None + condition: typing.Any = { + "property": condition_property, + "operator": "EQUAL", + "value": "true", + } + rule: typing.Any = {"type": "ALL", "conditions": [condition]} + if nested: + rule = {"type": "ALL", "rules": [rule]} + lazy_context["segments"]["premium_segment"]["rules"] = [rule] + + # When + names = build_flag_dependent_feature_names(lazy_context) + + # Then + assert names == expected + + +def test_lazy_flags__get_flag__flag_dependent_feature__resolves_with_full_context( + lazy_context: SDKEvaluationContext, + mocker: MockerFixture, +) -> None: + # Given + # the target is overridden when `noise_0` is enabled, which it is + assert lazy_context["segments"] is not None + lazy_context["segments"]["premium_segment"]["rules"] = [ + { + "type": "ALL", + "conditions": [ + { + "property": "$.flags.noise_0.enabled", + "operator": "EQUAL", + "value": "true", + }, + ], + } + ] + flags = Flags.from_evaluation_context( + context=lazy_context, + overrides_index=build_segment_overrides_index(lazy_context), + analytics_processor=None, + default_flag_handler=None, + ) + spy = mocker.spy(engine, "get_evaluation_result") + + # When + target = flags.get_flag("target") + + # Then + # the dependency is resolved, so the override applies + assert isinstance(target, Flag) + assert target.value == "premium-value" + assert target.reason == "TARGETING_MATCH; segment=premium_segment" + + # And + # the full context is evaluated once, memoising every flag + spy.assert_called_once_with(lazy_context) + assert set(flags.flags) == {"target", "noise_0", "noise_1"} + flags.get_flag("noise_1") + flags.all_flags() + spy.assert_called_once() From 35891e01617b75f61439b8ed2904593af7fe1081 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 16 Oct 2026 19:23:07 +0200 Subject: [PATCH 2/2] refactor: Detect `$.flags` conditions by parsing JSONPath Replace the regex with the JSONPath parser the engine uses, mirroring how the engine reads condition properties. A property is ruled out only when its first segment selects top-level keys by name, none of them `flags`, so ambiguous selectors fall back to full-context evaluation. --- flagsmith/models.py | 45 ++++++++++++++++++++++++++++++++++++-------- poetry.lock | 2 +- pyproject.toml | 1 + tests/test_models.py | 13 +++++++++++-- 4 files changed, 50 insertions(+), 11 deletions(-) diff --git a/flagsmith/models.py b/flagsmith/models.py index 2caf067..051287b 100644 --- a/flagsmith/models.py +++ b/flagsmith/models.py @@ -1,11 +1,14 @@ from __future__ import annotations -import re import typing from dataclasses import dataclass, field +from functools import lru_cache +import jsonpath_rfc9535 from flag_engine import engine from flag_engine.context.types import SegmentContext, SegmentRule +from jsonpath_rfc9535.segments import JSONPathChildSegment +from jsonpath_rfc9535.selectors import NameSelector from flagsmith.analytics import AnalyticsProcessor from flagsmith.exceptions import FlagsmithFeatureDoesNotExistError @@ -42,18 +45,44 @@ def build_segment_overrides_index( return index -# Matches JSONPath condition properties that can read `$.flags`: the -# dot (`$.flags`) and bracket (`$['flags']`) forms, plus descendant -# segments (`$..`), which can reach it too. -_FLAGS_PROPERTY_RE = re.compile( - r"^\$\s*(?:\.\.|\.\s*flags\b|\[\s*['\"]flags['\"]\s*\])" -) +@lru_cache(maxsize=1024) +def _property_may_read_flags(condition_property: str) -> bool: + """Whether a condition property may read the engine's `$.flags`. + + Mirrors how the engine reads properties: only those starting with + `$.` that compile as JSONPath are queried against the context; + anything else is a trait key. The compiled query is then checked + the way the engine would run it, rather than by pattern matching. + + Errs on the side of `True`: a false positive only costs a + full-context evaluation, whereas a false negative would evaluate a + flag without its dependencies. So the property is ruled out only + when its first segment selects top-level keys by name, none of them + `flags`. Wildcards, filters, indices and descendant segments + (`$..`) are all assumed to reach `flags`. + """ + if not condition_property.startswith("$."): + return False + try: + query = jsonpath_rfc9535.compile(condition_property) + except jsonpath_rfc9535.JSONPathSyntaxError: + return False + if not query.segments: + return False + first_segment = query.segments[0] + return not ( + type(first_segment) is JSONPathChildSegment + and all( + isinstance(selector, NameSelector) and selector.name != "flags" + for selector in first_segment.selectors + ) + ) def _rules_read_flags(rules: typing.Iterable[SegmentRule]) -> bool: return any( any( - _FLAGS_PROPERTY_RE.match(condition.get("property") or "") + _property_may_read_flags(condition.get("property") or "") for condition in rule.get("conditions") or () ) or _rules_read_flags(rule.get("rules") or ()) diff --git a/poetry.lock b/poetry.lock index 325dd62..6fda9a1 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1235,4 +1235,4 @@ typing-extensions = {version = ">=4.13.2", markers = "python_version < \"3.11\"" [metadata] lock-version = "2.1" python-versions = ">=3.10,<4" -content-hash = "079238a62ea14b7bbf4852682e950390da201cd67fecc602c16144faef8b8ad2" +content-hash = "5487684fa9dea088c60c88bbac7d652d36ac063cdee821575f776216e83ac33b" diff --git a/pyproject.toml b/pyproject.toml index 4a24f3f..6ff17f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,6 +12,7 @@ packages = [{ include = "flagsmith" }] [tool.poetry.dependencies] flagsmith-flag-engine = "^11.1.1" iso8601 = { version = "^2.1.0", python = "<3.11" } +jsonpath-rfc9535 = ">=0.1.5,<1" python = ">=3.10,<4" requests = "^2.32.3" requests-futures = "^1.0.1" diff --git a/tests/test_models.py b/tests/test_models.py index b3e2790..5737467 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -571,13 +571,22 @@ def test_build_segment_overrides_index__duplicate_segment_key_fields__keeps_both [ ("$.flags.noise_0.enabled", False, {"target"}), ("$.flags.noise_0.enabled", True, {"target"}), - ("$['flags']['noise 0'].value", False, {"target"}), - ('$["flags"].noise_0.variant', False, {"target"}), + ("$.flags['noise 0'].value", False, {"target"}), + ("$.flags", False, {"target"}), + # Selectors that may reach `$.flags` are assumed to ("$..enabled", False, {"target"}), + ("$.*.noise_0.enabled", False, {"target"}), + # Properties reading elsewhere ("$.environment.name", False, set()), + ("$.environment['name']", False, set()), ("$.identity.traits.flags", False, set()), ("$.flagship", False, set()), + # Trait keys, as the engine only queries properties starting + # with `$.` that compile as JSONPath ("tier", False, set()), + ("flags", False, set()), + ("$['flags'].noise_0.enabled", False, set()), + ("$.flags.", False, set()), ], ) def test_build_flag_dependent_feature_names__returns_features_overridden_by_flags_segments(