From f571af04d73966d656b5d336c0d64a1c62e7edf3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 19:28:33 +0000 Subject: [PATCH 1/5] Initial plan From 13904f6e735b6c4a80d0f9abf2c759852d510284 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 19:42:53 +0000 Subject: [PATCH 2/5] fix: omit segments from evaluation context for get_environment_flags Co-authored-by: khvn26 <979078+khvn26@users.noreply.github.com> --- flagsmith/flagsmith.py | 9 ++++- tests/test_flagsmith.py | 75 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index 8eaf847..7c18e13 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -332,7 +332,14 @@ def _get_environment_flags_from_document(self) -> Flags: if self._evaluation_context is None: raise TypeError("No environment present") - evaluation_result = engine.get_evaluation_result(self._evaluation_context) + # Omit segments from evaluation context for environment flags + # as they are only relevant for identity-specific evaluations + context_without_segments: SDKEvaluationContext = { + "environment": self._evaluation_context["environment"], + "features": self._evaluation_context.get("features", {}), + } + + evaluation_result = engine.get_evaluation_result(context_without_segments) return Flags.from_evaluation_result( evaluation_result=evaluation_result, diff --git a/tests/test_flagsmith.py b/tests/test_flagsmith.py index cf1367a..a3fe329 100644 --- a/tests/test_flagsmith.py +++ b/tests/test_flagsmith.py @@ -94,6 +94,41 @@ 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( + mocker: MockerFixture, + flagsmith: Flagsmith, + evaluation_context: SDKEvaluationContext, +) -> None: + # Given + flagsmith._evaluation_context = evaluation_context + flagsmith.enable_local_evaluation = True + mock_engine = mocker.patch("flagsmith.flagsmith.engine") + + expected_evaluation_result = { + "flags": { + "some_feature": { + "name": "some_feature", + "enabled": True, + "value": "some-feature-state-value", + "metadata": {"id": 1}, + } + }, + "segments": [], + } + + mock_engine.get_evaluation_result.return_value = expected_evaluation_result + + # When + flagsmith.get_environment_flags() + + # Then + mock_engine.get_evaluation_result.assert_called_once() + call_args = mock_engine.get_evaluation_result.call_args + context = call_args[0][0] # First positional argument + # Segments should not be present in the context passed to the engine + assert "segments" not in context + + @responses.activate() def test_get_identity_flags_calls_api_when_no_local_environment_no_traits( flagsmith: Flagsmith, identities_json: str @@ -191,6 +226,46 @@ def test_get_identity_flags_uses_local_environment_when_available( assert identity_flags[0].value == "some-feature-state-value" +def test_get_identity_flags_includes_segments_in_evaluation_context( + mocker: MockerFixture, + flagsmith: Flagsmith, + evaluation_context: SDKEvaluationContext, +) -> None: + # Given + flagsmith._evaluation_context = evaluation_context + flagsmith.enable_local_evaluation = True + mock_engine = mocker.patch("flagsmith.flagsmith.engine") + + expected_evaluation_result = { + "flags": { + "some_feature": { + "name": "some_feature", + "enabled": True, + "value": "some-feature-state-value", + "metadata": {"id": 1}, + } + }, + "segments": [], + } + + identifier = "identifier" + traits = {"some_trait": "some_value"} + + mock_engine.get_evaluation_result.return_value = expected_evaluation_result + + # When + flagsmith.get_identity_flags(identifier, traits) + + # Then + mock_engine.get_evaluation_result.assert_called_once() + call_args = mock_engine.get_evaluation_result.call_args + context = call_args[1]["context"] + # Segments should be present in the context passed to the engine for identity flags + assert "segments" in context + # Verify segments from evaluation_context are preserved + assert context["segments"] == evaluation_context["segments"] + + @responses.activate() def test_get_identity_flags__transient_identity__calls_expected( flagsmith: Flagsmith, From 8f10c4241527d0f7c2f8314cc6480f56d5e9528d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 19:48:00 +0000 Subject: [PATCH 3/5] fix: improve context filtering to be more robust and type-safe Co-authored-by: khvn26 <979078+khvn26@users.noreply.github.com> --- flagsmith/flagsmith.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index 7c18e13..a5e3f33 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -334,10 +334,14 @@ def _get_environment_flags_from_document(self) -> Flags: # Omit segments from evaluation context for environment flags # as they are only relevant for identity-specific evaluations - context_without_segments: SDKEvaluationContext = { - "environment": self._evaluation_context["environment"], - "features": self._evaluation_context.get("features", {}), - } + context_without_segments = typing.cast( + SDKEvaluationContext, + { + key: value + for key, value in self._evaluation_context.items() + if key != "segments" + }, + ) evaluation_result = engine.get_evaluation_result(context_without_segments) From 3bda6aa6d50dd1f58e5f898f6736465b80d4d68a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 19:53:22 +0000 Subject: [PATCH 4/5] fix: ensure consistent API usage for engine.get_evaluation_result Co-authored-by: khvn26 <979078+khvn26@users.noreply.github.com> --- flagsmith/flagsmith.py | 4 +++- tests/test_flagsmith.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index a5e3f33..19b241e 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -343,7 +343,9 @@ def _get_environment_flags_from_document(self) -> Flags: }, ) - evaluation_result = engine.get_evaluation_result(context_without_segments) + evaluation_result = engine.get_evaluation_result( + context=context_without_segments, + ) return Flags.from_evaluation_result( evaluation_result=evaluation_result, diff --git a/tests/test_flagsmith.py b/tests/test_flagsmith.py index a3fe329..da00fa6 100644 --- a/tests/test_flagsmith.py +++ b/tests/test_flagsmith.py @@ -124,7 +124,7 @@ def test_get_environment_flags_omits_segments_from_evaluation_context( # Then mock_engine.get_evaluation_result.assert_called_once() call_args = mock_engine.get_evaluation_result.call_args - context = call_args[0][0] # First positional argument + context = call_args[1]["context"] # Keyword argument 'context' # Segments should not be present in the context passed to the engine assert "segments" not in context From 10242fbea1f1f3d2fc2e9183c76dc49648ea1e90 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 31 Oct 2025 22:31:12 +0000 Subject: [PATCH 5/5] refactor: address code review feedback - Use local_eval_flagsmith fixture in tests - Mock specific function with autospec - Use assert_called_once_with for cleaner assertions - Simplify context filtering with copy() and pop() Co-authored-by: khvn26 <979078+khvn26@users.noreply.github.com> --- flagsmith/flagsmith.py | 10 ++-------- tests/test_flagsmith.py | 43 +++++++++++++++++++---------------------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/flagsmith/flagsmith.py b/flagsmith/flagsmith.py index 19b241e..15da5ec 100644 --- a/flagsmith/flagsmith.py +++ b/flagsmith/flagsmith.py @@ -334,14 +334,8 @@ def _get_environment_flags_from_document(self) -> Flags: # Omit segments from evaluation context for environment flags # as they are only relevant for identity-specific evaluations - context_without_segments = typing.cast( - SDKEvaluationContext, - { - key: value - for key, value in self._evaluation_context.items() - if key != "segments" - }, - ) + context_without_segments = self._evaluation_context.copy() + context_without_segments.pop("segments", None) evaluation_result = engine.get_evaluation_result( context=context_without_segments, diff --git a/tests/test_flagsmith.py b/tests/test_flagsmith.py index da00fa6..494e854 100644 --- a/tests/test_flagsmith.py +++ b/tests/test_flagsmith.py @@ -96,13 +96,14 @@ def test_get_environment_flags_uses_local_environment_when_available( def test_get_environment_flags_omits_segments_from_evaluation_context( mocker: MockerFixture, - flagsmith: Flagsmith, + local_eval_flagsmith: Flagsmith, evaluation_context: SDKEvaluationContext, ) -> None: # Given - flagsmith._evaluation_context = evaluation_context - flagsmith.enable_local_evaluation = True - mock_engine = mocker.patch("flagsmith.flagsmith.engine") + mock_get_evaluation_result = mocker.patch( + "flagsmith.flagsmith.engine.get_evaluation_result", + autospec=True, + ) expected_evaluation_result = { "flags": { @@ -116,17 +117,16 @@ def test_get_environment_flags_omits_segments_from_evaluation_context( "segments": [], } - mock_engine.get_evaluation_result.return_value = expected_evaluation_result + mock_get_evaluation_result.return_value = expected_evaluation_result # When - flagsmith.get_environment_flags() + local_eval_flagsmith.get_environment_flags() # Then - mock_engine.get_evaluation_result.assert_called_once() - call_args = mock_engine.get_evaluation_result.call_args - context = call_args[1]["context"] # Keyword argument 'context' - # Segments should not be present in the context passed to the engine - assert "segments" not in context + # 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) @responses.activate() @@ -228,13 +228,13 @@ def test_get_identity_flags_uses_local_environment_when_available( def test_get_identity_flags_includes_segments_in_evaluation_context( mocker: MockerFixture, - flagsmith: Flagsmith, - evaluation_context: SDKEvaluationContext, + local_eval_flagsmith: Flagsmith, ) -> None: # Given - flagsmith._evaluation_context = evaluation_context - flagsmith.enable_local_evaluation = True - mock_engine = mocker.patch("flagsmith.flagsmith.engine") + mock_get_evaluation_result = mocker.patch( + "flagsmith.flagsmith.engine.get_evaluation_result", + autospec=True, + ) expected_evaluation_result = { "flags": { @@ -251,19 +251,16 @@ def test_get_identity_flags_includes_segments_in_evaluation_context( identifier = "identifier" traits = {"some_trait": "some_value"} - mock_engine.get_evaluation_result.return_value = expected_evaluation_result + mock_get_evaluation_result.return_value = expected_evaluation_result # When - flagsmith.get_identity_flags(identifier, traits) + local_eval_flagsmith.get_identity_flags(identifier, traits) # Then - mock_engine.get_evaluation_result.assert_called_once() - call_args = mock_engine.get_evaluation_result.call_args + # Verify segments are present in the context passed to the engine for identity flags + call_args = mock_get_evaluation_result.call_args context = call_args[1]["context"] - # Segments should be present in the context passed to the engine for identity flags assert "segments" in context - # Verify segments from evaluation_context are preserved - assert context["segments"] == evaluation_context["segments"] @responses.activate()