From 420c92f971227e8e16c9bda512628fdffe906498 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Mon, 29 Jun 2026 11:09:19 -0400 Subject: [PATCH 1/7] fix(flags): tag fallback_reason so OpenFeature can distinguish causes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SelectedVariant now carries two source fields: `variant_source` (local | remote | fallback) and `fallback_reason` (FLAG_NOT_FOUND | MISSING_CONTEXT_KEY | NO_ROLLOUT_MATCH | BACKEND_ERROR | NOT_READY, set only when source is fallback). Three behaviorally distinct outcomes — flag-not-found, no-rollout-match, and missing-context-key — previously all returned the bare fallback. The OpenFeature wrapper collapsed them to FLAG_NOT_FOUND, sending callers chasing the flag name when the real cause was usually a rule miss or absent context. The wrapper now dispatches on fallback_reason and maps each to the spec-correct OpenFeature response. Most notably, NO_ROLLOUT_MATCH becomes `reason: DEFAULT` with no error code instead of FLAG_NOT_FOUND. Constant names align with mixpanel-php for consistency across SDKs. Linear: SDK-79 Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/local_feature_flags.py | 10 ++- mixpanel/flags/remote_feature_flags.py | 20 +++-- mixpanel/flags/test_local_feature_flags.py | 39 +++++++++ mixpanel/flags/types.py | 46 ++++++++++ .../src/mixpanel_openfeature/provider.py | 38 +++++++- openfeature-provider/tests/test_provider.py | 87 +++++++++++++++++-- 6 files changed, 221 insertions(+), 19 deletions(-) diff --git a/mixpanel/flags/local_feature_flags.py b/mixpanel/flags/local_feature_flags.py index e180943..5b9adcb 100644 --- a/mixpanel/flags/local_feature_flags.py +++ b/mixpanel/flags/local_feature_flags.py @@ -15,9 +15,11 @@ from .types import ( ExperimentationFlag, ExperimentationFlags, + FallbackReason, LocalFlagsConfig, Rollout, SelectedVariant, + VariantSource, ) from .utils import ( EXPOSURE_EVENT, @@ -228,7 +230,7 @@ def get_variant( if not flag_definition: logger.warning("Cannot find flag definition for key: '%s'", flag_key) - return fallback_value + return fallback_value.as_fallback(FallbackReason.FLAG_NOT_FOUND) if not (context_value := context.get(flag_definition.context)): logger.warning( @@ -236,7 +238,7 @@ def get_variant( flag_definition.context, flag_key, ) - return fallback_value + return fallback_value.as_fallback(FallbackReason.MISSING_CONTEXT_KEY) selected_variant: SelectedVariant | None = None @@ -257,7 +259,7 @@ def get_variant( self._track_exposure( flag_key, selected_variant, context, end_time - start_time ) - return selected_variant + return selected_variant.with_source(VariantSource.LOCAL) logger.debug( "%s context %s not eligible for any rollout for flag: %s", @@ -265,7 +267,7 @@ def get_variant( context_value, flag_key, ) - return fallback_value + return fallback_value.as_fallback(FallbackReason.NO_ROLLOUT_MATCH) def track_exposure_event( self, flag_key: str, variant: SelectedVariant, context: dict[str, Any] diff --git a/mixpanel/flags/remote_feature_flags.py b/mixpanel/flags/remote_feature_flags.py index c47a20c..551715e 100644 --- a/mixpanel/flags/remote_feature_flags.py +++ b/mixpanel/flags/remote_feature_flags.py @@ -11,7 +11,13 @@ from mixpanel.credentials import ServiceAccountCredentials -from .types import RemoteFlagsConfig, RemoteFlagsResponse, SelectedVariant +from .types import ( + FallbackReason, + RemoteFlagsConfig, + RemoteFlagsResponse, + SelectedVariant, + VariantSource, +) from .utils import ( EXPOSURE_EVENT, REQUEST_HEADERS, @@ -153,7 +159,7 @@ async def aget_variant( ) except Exception: logger.exception("Failed to get remote variant for flag '%s'", flag_key) - return fallback_value + return fallback_value.as_fallback(FallbackReason.BACKEND_ERROR) else: return selected_variant @@ -267,7 +273,7 @@ def get_variant( except Exception: logger.exception("Failed to get remote variant for flag '%s'", flag_key) - return fallback_value + return fallback_value.as_fallback(FallbackReason.BACKEND_ERROR) else: return selected_variant @@ -362,13 +368,17 @@ def _lookup_flag_in_response( fallback_value: SelectedVariant, ) -> tuple[SelectedVariant, bool]: if flag_key in flags: - return flags[flag_key], False + return flags[flag_key].with_source(VariantSource.REMOTE), False logger.debug( "Flag '%s' not found in remote response. Returning fallback, '%s'", flag_key, fallback_value, ) - return fallback_value, True + # The /flags endpoint only returns variants the user is enrolled in, + # so a missing key could mean the flag doesn't exist OR the user + # isn't in any rollout. The remote SDK can't tell them apart without + # server-side help — surface as FLAG_NOT_FOUND for now. + return fallback_value.as_fallback(FallbackReason.FLAG_NOT_FOUND), True def shutdown(self): self._sync_client.close() diff --git a/mixpanel/flags/test_local_feature_flags.py b/mixpanel/flags/test_local_feature_flags.py index b39caa3..2b4366c 100644 --- a/mixpanel/flags/test_local_feature_flags.py +++ b/mixpanel/flags/test_local_feature_flags.py @@ -16,6 +16,7 @@ from .types import ( ExperimentationFlag, ExperimentationFlags, + FallbackReason, FlagTestUsers, LocalFlagsConfig, Rollout, @@ -23,6 +24,7 @@ SelectedVariant, Variant, VariantOverride, + VariantSource, ) TEST_FLAG_KEY = "test_flag" @@ -723,6 +725,43 @@ async def test_is_enabled_returns_true_for_true_variant_value(self): result = self._flags.is_enabled(TEST_FLAG_KEY, USER_CONTEXT) assert result is True + @respx.mock + async def test_get_variant_tags_match_as_local(self): + flag = create_test_flag(rollout_percentage=100.0) + await self.setup_flags([flag]) + fallback = SelectedVariant(variant_value="fb") + result = self._flags.get_variant(TEST_FLAG_KEY, fallback, USER_CONTEXT) + assert result.variant_source == VariantSource.LOCAL + assert result.fallback_reason is None + assert result.variant_key is not None + + @respx.mock + async def test_get_variant_tags_missing_flag(self): + await self.setup_flags([]) + fallback = SelectedVariant(variant_value="fb") + result = self._flags.get_variant("missing", fallback, USER_CONTEXT) + assert result.variant_source == VariantSource.FALLBACK + assert result.fallback_reason == FallbackReason.FLAG_NOT_FOUND + assert result.variant_value == "fb" + + @respx.mock + async def test_get_variant_tags_missing_context(self): + flag = create_test_flag(context="distinct_id") + await self.setup_flags([flag]) + fallback = SelectedVariant(variant_value="fb") + result = self._flags.get_variant(TEST_FLAG_KEY, fallback, {}) + assert result.variant_source == VariantSource.FALLBACK + assert result.fallback_reason == FallbackReason.MISSING_CONTEXT_KEY + + @respx.mock + async def test_get_variant_tags_no_rollout_match(self): + flag = create_test_flag(rollout_percentage=0.0) + await self.setup_flags([flag]) + fallback = SelectedVariant(variant_value="fb") + result = self._flags.get_variant(TEST_FLAG_KEY, fallback, USER_CONTEXT) + assert result.variant_source == VariantSource.FALLBACK + assert result.fallback_reason == FallbackReason.NO_ROLLOUT_MATCH + @respx.mock async def test_get_variant_value_uses_most_recent_polled_flag(self): polling_iterations = 0 diff --git a/mixpanel/flags/types.py b/mixpanel/flags/types.py index 8325439..5a77687 100644 --- a/mixpanel/flags/types.py +++ b/mixpanel/flags/types.py @@ -63,6 +63,32 @@ class ExperimentationFlag(BaseModel): hash_salt: Optional[str] = None +class VariantSource: + """Where a SelectedVariant came from. Set by the providers on every + returned variant — coarse-grained (local / remote / fallback). For the + specific reason behind a fallback, see FallbackReason. + """ + + LOCAL = "local" + REMOTE = "remote" + FALLBACK = "fallback" + + +class FallbackReason: + """Why the SDK returned the developer fallback. Only meaningful when + SelectedVariant.variant_source == VariantSource.FALLBACK. Matches the + constant set used by mixpanel-php so the OpenFeature wrapper can map to + the spec-correct error code instead of collapsing every fallback to + FLAG_NOT_FOUND. + """ + + FLAG_NOT_FOUND = "FLAG_NOT_FOUND" + MISSING_CONTEXT_KEY = "MISSING_CONTEXT_KEY" + NO_ROLLOUT_MATCH = "NO_ROLLOUT_MATCH" + BACKEND_ERROR = "BACKEND_ERROR" + NOT_READY = "NOT_READY" + + class SelectedVariant(BaseModel): # variant_key can be None if being used as a fallback variant_key: Optional[str] = None @@ -70,6 +96,26 @@ class SelectedVariant(BaseModel): experiment_id: Optional[str] = None is_experiment_active: Optional[bool] = None is_qa_tester: Optional[bool] = None + variant_source: Optional[str] = None + # None on success; one of FallbackReason.* when variant_source is FALLBACK + fallback_reason: Optional[str] = None + + def with_source(self, source: str) -> "SelectedVariant": + """Return a copy of this variant tagged with the given source. + Clears fallback_reason — use as_fallback() if returning a fallback. + """ + return self.model_copy( + update={"variant_source": source, "fallback_reason": None} + ) + + def as_fallback(self, reason: str) -> "SelectedVariant": + """Return a copy of this variant tagged as a fallback with the given reason.""" + return self.model_copy( + update={ + "variant_source": VariantSource.FALLBACK, + "fallback_reason": reason, + } + ) class ExperimentationFlags(BaseModel): diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index ec16d78..b295198 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -11,7 +11,12 @@ from openfeature.provider import AbstractProvider, Metadata from mixpanel import Mixpanel -from mixpanel.flags.types import LocalFlagsConfig, RemoteFlagsConfig, SelectedVariant +from mixpanel.flags.types import ( + FallbackReason, + LocalFlagsConfig, + RemoteFlagsConfig, + SelectedVariant, +) FlagValueType = Union[bool, str, int, float, list, dict, None] @@ -156,12 +161,41 @@ def _resolve( reason=Reason.ERROR, ) - if result is fallback: + # variant_source distinguishes local / remote / fallback. When fallback, + # fallback_reason carries the specific reason (PHP-aligned constants) + # so we can map each to the spec-correct OpenFeature response instead + # of collapsing every fallback to FLAG_NOT_FOUND. + if result.fallback_reason == FallbackReason.FLAG_NOT_FOUND: return FlagResolutionDetails( value=default_value, error_code=ErrorCode.FLAG_NOT_FOUND, reason=Reason.DEFAULT, ) + if result.fallback_reason == FallbackReason.MISSING_CONTEXT_KEY: + return FlagResolutionDetails( + value=default_value, + error_code=ErrorCode.TARGETING_KEY_MISSING, + reason=Reason.ERROR, + ) + if result.fallback_reason == FallbackReason.NO_ROLLOUT_MATCH: + # Flag exists, user just didn't match any rollout — per the + # OpenFeature spec this is `reason: DEFAULT` with no error. + return FlagResolutionDetails( + value=default_value, + reason=Reason.DEFAULT, + ) + if result.fallback_reason == FallbackReason.BACKEND_ERROR: + return FlagResolutionDetails( + value=default_value, + error_code=ErrorCode.GENERAL, + reason=Reason.ERROR, + ) + if result.fallback_reason == FallbackReason.NOT_READY: + return FlagResolutionDetails( + value=default_value, + error_code=ErrorCode.PROVIDER_NOT_READY, + reason=Reason.ERROR, + ) value = result.variant_value variant_key = result.variant_key diff --git a/openfeature-provider/tests/test_provider.py b/openfeature-provider/tests/test_provider.py index 723f9ce..f1f9a20 100644 --- a/openfeature-provider/tests/test_provider.py +++ b/openfeature-provider/tests/test_provider.py @@ -5,7 +5,7 @@ from openfeature.exception import ErrorCode from openfeature.flag_evaluation import Reason -from mixpanel.flags.types import SelectedVariant +from mixpanel.flags.types import FallbackReason, SelectedVariant, VariantSource from mixpanel_openfeature import MixpanelProvider @@ -22,17 +22,28 @@ def provider(mock_flags): def setup_flag(mock_flags, flag_key, value, variant_key="variant-key"): - """Configure mock to return a SelectedVariant with the given value.""" + """Configure mock to return a successfully-evaluated SelectedVariant.""" mock_flags.get_variant.side_effect = lambda key, fallback, ctx: ( - SelectedVariant(variant_key=variant_key, variant_value=value) + SelectedVariant( + variant_key=variant_key, + variant_value=value, + variant_source=VariantSource.LOCAL, + ) if key == flag_key - else fallback + else fallback.as_fallback(FallbackReason.FLAG_NOT_FOUND) + ) + + +def setup_fallback(mock_flags, reason): + """Configure mock to always return the caller's fallback tagged with `reason`.""" + mock_flags.get_variant.side_effect = ( + lambda key, fallback, ctx: fallback.as_fallback(reason) ) -def setup_flag_not_found(mock_flags, flag_key): - """Configure mock to return the fallback (identity check triggers FLAG_NOT_FOUND).""" - mock_flags.get_variant.side_effect = lambda key, fallback, ctx: fallback +def setup_flag_not_found(mock_flags, flag_key): # noqa: ARG001 - flag_key kept for call-site clarity + """Configure mock for the genuinely-missing-flag path.""" + setup_fallback(mock_flags, FallbackReason.FLAG_NOT_FOUND) # --- Metadata --- @@ -298,7 +309,7 @@ def test_remote_provider_always_ready(): remote_flags = MagicMock(spec=[]) # empty spec = no attributes remote_flags.get_variant = MagicMock( side_effect=lambda key, fallback, ctx: SelectedVariant( - variant_key="v1", variant_value=True + variant_key="v1", variant_value=True, variant_source=VariantSource.REMOTE ) ) provider = MixpanelProvider(remote_flags) @@ -307,6 +318,66 @@ def test_remote_provider_always_ready(): assert result.reason == Reason.TARGETING_MATCH +# --- No rollout matched (flag exists, no rollout matched) --- + + +def test_no_rollout_match_returns_default_reason_without_error(provider, mock_flags): + setup_fallback(mock_flags, FallbackReason.NO_ROLLOUT_MATCH) + result = provider.resolve_boolean_details("flag", True) + assert result.value is True + assert result.reason == Reason.DEFAULT + assert result.error_code is None + + +def test_no_rollout_match_for_string(provider, mock_flags): + setup_fallback(mock_flags, FallbackReason.NO_ROLLOUT_MATCH) + result = provider.resolve_string_details("flag", "default") + assert result.value == "default" + assert result.reason == Reason.DEFAULT + assert result.error_code is None + + +# --- Missing context key --- + + +def test_missing_context_key_returns_targeting_key_missing(provider, mock_flags): + setup_fallback(mock_flags, FallbackReason.MISSING_CONTEXT_KEY) + result = provider.resolve_boolean_details("flag", False) + assert result.value is False + assert result.error_code == ErrorCode.TARGETING_KEY_MISSING + assert result.reason == Reason.ERROR + + +def test_missing_context_key_for_string(provider, mock_flags): + setup_fallback(mock_flags, FallbackReason.MISSING_CONTEXT_KEY) + result = provider.resolve_string_details("flag", "default") + assert result.value == "default" + assert result.error_code == ErrorCode.TARGETING_KEY_MISSING + assert result.reason == Reason.ERROR + + +# --- Backend error --- + + +def test_backend_error_maps_to_general(provider, mock_flags): + setup_fallback(mock_flags, FallbackReason.BACKEND_ERROR) + result = provider.resolve_string_details("flag", "default") + assert result.value == "default" + assert result.error_code == ErrorCode.GENERAL + assert result.reason == Reason.ERROR + + +# --- Not ready --- + + +def test_not_ready_reason_maps_to_provider_not_ready(provider, mock_flags): + setup_fallback(mock_flags, FallbackReason.NOT_READY) + result = provider.resolve_boolean_details("flag", True) + assert result.value is True + assert result.error_code == ErrorCode.PROVIDER_NOT_READY + assert result.reason == Reason.ERROR + + # --- Lifecycle --- From 4d8ba39c1b27f46ada36775fda161ef2bbd3fa36 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Mon, 29 Jun 2026 11:38:04 -0400 Subject: [PATCH 2/7] fix(lint): satisfy ruff D205 docstrings and reduce _resolve complexity Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/types.py | 20 ++-- .../src/mixpanel_openfeature/provider.py | 106 +++++++++--------- openfeature-provider/tests/test_provider.py | 2 +- 3 files changed, 69 insertions(+), 59 deletions(-) diff --git a/mixpanel/flags/types.py b/mixpanel/flags/types.py index 5a77687..85ef8ea 100644 --- a/mixpanel/flags/types.py +++ b/mixpanel/flags/types.py @@ -64,9 +64,11 @@ class ExperimentationFlag(BaseModel): class VariantSource: - """Where a SelectedVariant came from. Set by the providers on every - returned variant — coarse-grained (local / remote / fallback). For the - specific reason behind a fallback, see FallbackReason. + """Where a SelectedVariant came from. + + Set by the providers on every returned variant — coarse-grained + (local / remote / fallback). For the specific reason behind a fallback, + see FallbackReason. """ LOCAL = "local" @@ -75,11 +77,12 @@ class VariantSource: class FallbackReason: - """Why the SDK returned the developer fallback. Only meaningful when - SelectedVariant.variant_source == VariantSource.FALLBACK. Matches the - constant set used by mixpanel-php so the OpenFeature wrapper can map to - the spec-correct error code instead of collapsing every fallback to - FLAG_NOT_FOUND. + """Why the SDK returned the developer fallback. + + Only meaningful when SelectedVariant.variant_source == VariantSource.FALLBACK. + Matches the constant set used by mixpanel-php so the OpenFeature wrapper + can map to the spec-correct error code instead of collapsing every + fallback to FLAG_NOT_FOUND. """ FLAG_NOT_FOUND = "FLAG_NOT_FOUND" @@ -102,6 +105,7 @@ class SelectedVariant(BaseModel): def with_source(self, source: str) -> "SelectedVariant": """Return a copy of this variant tagged with the given source. + Clears fallback_reason — use as_fallback() if returning a fallback. """ return self.model_copy( diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index b295198..3364ebc 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -2,10 +2,8 @@ import math import typing -from collections.abc import Mapping, Sequence -from typing import Optional, Union +from typing import Union -from openfeature.evaluation_context import EvaluationContext from openfeature.exception import ErrorCode from openfeature.flag_evaluation import FlagResolutionDetails, Reason from openfeature.provider import AbstractProvider, Metadata @@ -18,6 +16,11 @@ SelectedVariant, ) +if typing.TYPE_CHECKING: + from collections.abc import Mapping, Sequence + + from openfeature.evaluation_context import EvaluationContext + FlagValueType = Union[bool, str, int, float, list, dict, None] @@ -27,7 +30,7 @@ class MixpanelProvider(AbstractProvider): def __init__( self, flags_provider: typing.Any, - mixpanel_instance: Optional[Mixpanel] = None, + mixpanel_instance: Mixpanel | None = None, ) -> None: super().__init__() self._flags_provider = flags_provider @@ -61,7 +64,7 @@ def from_remote_config( return cls(remote_flags, mixpanel_instance=mp) @property - def mixpanel(self) -> Optional[Mixpanel]: + def mixpanel(self) -> Mixpanel | None: """The Mixpanel instance used by this provider, if created via a class method.""" return self._mixpanel @@ -75,7 +78,7 @@ def resolve_boolean_details( self, flag_key: str, default_value: bool, - evaluation_context: typing.Optional[EvaluationContext] = None, + evaluation_context: EvaluationContext | None = None, ) -> FlagResolutionDetails[bool]: return self._resolve(flag_key, default_value, bool, evaluation_context) @@ -83,7 +86,7 @@ def resolve_string_details( self, flag_key: str, default_value: str, - evaluation_context: typing.Optional[EvaluationContext] = None, + evaluation_context: EvaluationContext | None = None, ) -> FlagResolutionDetails[str]: return self._resolve(flag_key, default_value, str, evaluation_context) @@ -91,7 +94,7 @@ def resolve_integer_details( self, flag_key: str, default_value: int, - evaluation_context: typing.Optional[EvaluationContext] = None, + evaluation_context: EvaluationContext | None = None, ) -> FlagResolutionDetails[int]: return self._resolve(flag_key, default_value, int, evaluation_context) @@ -99,17 +102,17 @@ def resolve_float_details( self, flag_key: str, default_value: float, - evaluation_context: typing.Optional[EvaluationContext] = None, + evaluation_context: EvaluationContext | None = None, ) -> FlagResolutionDetails[float]: return self._resolve(flag_key, default_value, float, evaluation_context) def resolve_object_details( self, flag_key: str, - default_value: Union[Sequence[FlagValueType], Mapping[str, FlagValueType]], - evaluation_context: typing.Optional[EvaluationContext] = None, + default_value: Sequence[FlagValueType] | Mapping[str, FlagValueType], + evaluation_context: EvaluationContext | None = None, ) -> FlagResolutionDetails[ - Union[Sequence[FlagValueType], Mapping[str, FlagValueType]] + Sequence[FlagValueType] | Mapping[str, FlagValueType] ]: return self._resolve(flag_key, default_value, None, evaluation_context) @@ -125,7 +128,7 @@ def _unwrap_value(value: typing.Any) -> typing.Any: @staticmethod def _build_user_context( - evaluation_context: typing.Optional[EvaluationContext], + evaluation_context: EvaluationContext | None, ) -> dict: user_context: dict = {} if evaluation_context is not None: @@ -140,8 +143,8 @@ def _resolve( self, flag_key: str, default_value: typing.Any, - expected_type: typing.Optional[type], - evaluation_context: typing.Optional[EvaluationContext] = None, + expected_type: type | None, + evaluation_context: EvaluationContext | None = None, ) -> FlagResolutionDetails: if not self._are_flags_ready(): return FlagResolutionDetails( @@ -161,41 +164,11 @@ def _resolve( reason=Reason.ERROR, ) - # variant_source distinguishes local / remote / fallback. When fallback, - # fallback_reason carries the specific reason (PHP-aligned constants) - # so we can map each to the spec-correct OpenFeature response instead - # of collapsing every fallback to FLAG_NOT_FOUND. - if result.fallback_reason == FallbackReason.FLAG_NOT_FOUND: - return FlagResolutionDetails( - value=default_value, - error_code=ErrorCode.FLAG_NOT_FOUND, - reason=Reason.DEFAULT, - ) - if result.fallback_reason == FallbackReason.MISSING_CONTEXT_KEY: - return FlagResolutionDetails( - value=default_value, - error_code=ErrorCode.TARGETING_KEY_MISSING, - reason=Reason.ERROR, - ) - if result.fallback_reason == FallbackReason.NO_ROLLOUT_MATCH: - # Flag exists, user just didn't match any rollout — per the - # OpenFeature spec this is `reason: DEFAULT` with no error. - return FlagResolutionDetails( - value=default_value, - reason=Reason.DEFAULT, - ) - if result.fallback_reason == FallbackReason.BACKEND_ERROR: - return FlagResolutionDetails( - value=default_value, - error_code=ErrorCode.GENERAL, - reason=Reason.ERROR, - ) - if result.fallback_reason == FallbackReason.NOT_READY: - return FlagResolutionDetails( - value=default_value, - error_code=ErrorCode.PROVIDER_NOT_READY, - reason=Reason.ERROR, - ) + fallback_details = self._fallback_details( + result.fallback_reason, default_value + ) + if fallback_details is not None: + return fallback_details value = result.variant_value variant_key = result.variant_key @@ -244,6 +217,39 @@ def _resolve( value=value, variant=variant_key, reason=Reason.TARGETING_MATCH ) + @staticmethod + def _fallback_details( + fallback_reason: str | None, default_value: typing.Any + ) -> FlagResolutionDetails | None: + """Map a fallback reason to its OpenFeature response, or None if not a fallback. + + variant_source distinguishes local / remote / fallback. When fallback, + fallback_reason carries the specific reason (PHP-aligned constants) + so we map each to the spec-correct OpenFeature response instead of + collapsing every fallback to FLAG_NOT_FOUND. + """ + if fallback_reason is None: + return None + # Flag exists, user just didn't match any rollout — per the + # OpenFeature spec this is `reason: DEFAULT` with no error. + if fallback_reason == FallbackReason.NO_ROLLOUT_MATCH: + return FlagResolutionDetails(value=default_value, reason=Reason.DEFAULT) + error_mapping = { + FallbackReason.FLAG_NOT_FOUND: (ErrorCode.FLAG_NOT_FOUND, Reason.DEFAULT), + FallbackReason.MISSING_CONTEXT_KEY: ( + ErrorCode.TARGETING_KEY_MISSING, + Reason.ERROR, + ), + FallbackReason.BACKEND_ERROR: (ErrorCode.GENERAL, Reason.ERROR), + FallbackReason.NOT_READY: (ErrorCode.PROVIDER_NOT_READY, Reason.ERROR), + } + if fallback_reason in error_mapping: + error_code, reason = error_mapping[fallback_reason] + return FlagResolutionDetails( + value=default_value, error_code=error_code, reason=reason + ) + return None + def _are_flags_ready(self) -> bool: if hasattr(self._flags_provider, "are_flags_ready"): return self._flags_provider.are_flags_ready() diff --git a/openfeature-provider/tests/test_provider.py b/openfeature-provider/tests/test_provider.py index f1f9a20..6114469 100644 --- a/openfeature-provider/tests/test_provider.py +++ b/openfeature-provider/tests/test_provider.py @@ -41,7 +41,7 @@ def setup_fallback(mock_flags, reason): ) -def setup_flag_not_found(mock_flags, flag_key): # noqa: ARG001 - flag_key kept for call-site clarity +def setup_flag_not_found(mock_flags, flag_key): """Configure mock for the genuinely-missing-flag path.""" setup_fallback(mock_flags, FallbackReason.FLAG_NOT_FOUND) From 8137f0e7660eab38e6c0e7ee363c6d9a6156e834 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Mon, 29 Jun 2026 15:41:57 -0400 Subject: [PATCH 3/7] fix(flags): upgrade FallbackReason to a value object and forward backend message (SDK-79, SDK-83) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SDK-79 made fallback_reason a PHP-aligned string constant on every returned fallback variant. That covers the local-eval cases (flag not found, no rollout match, missing context key) but couldn't carry detail — when the remote /flags endpoint returned an error response, the SDK still had nowhere to attach the backend's message. Upgrade FallbackReason from a class of string constants to a frozen Pydantic value object with `kind` (the discriminator) and optional `message`. Frozen singletons for the no-detail reasons; factory methods for the ones that carry detail. SDK-83: RemoteFeatureFlagsProvider's except branches now tag the fallback with FallbackReason.backend_error(message). For httpx HTTPStatusError specifically, the response body is extracted via _describe_backend_error so the actionable detail ("distinct_id must be provided in evalContext as a string") reaches the wrapper — str(exc) alone only yields httpx's standardized status line. Wrapper dispatches on reason.kind, forwards reason.message into FlagResolutionDetails.error_message for backend_error and missing_context_key. Linear: SDK-79, SDK-83 Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/local_feature_flags.py | 8 ++- mixpanel/flags/remote_feature_flags.py | 34 +++++++++-- mixpanel/flags/test_local_feature_flags.py | 9 +-- mixpanel/flags/test_remote_feature_flags.py | 27 +++++++- mixpanel/flags/types.py | 61 +++++++++++++++---- .../src/mixpanel_openfeature/provider.py | 34 ++++++----- openfeature-provider/tests/test_provider.py | 31 ++++++---- 7 files changed, 151 insertions(+), 53 deletions(-) diff --git a/mixpanel/flags/local_feature_flags.py b/mixpanel/flags/local_feature_flags.py index 5b9adcb..8465e79 100644 --- a/mixpanel/flags/local_feature_flags.py +++ b/mixpanel/flags/local_feature_flags.py @@ -230,7 +230,7 @@ def get_variant( if not flag_definition: logger.warning("Cannot find flag definition for key: '%s'", flag_key) - return fallback_value.as_fallback(FallbackReason.FLAG_NOT_FOUND) + return fallback_value.as_fallback(FallbackReason.flag_not_found()) if not (context_value := context.get(flag_definition.context)): logger.warning( @@ -238,7 +238,9 @@ def get_variant( flag_definition.context, flag_key, ) - return fallback_value.as_fallback(FallbackReason.MISSING_CONTEXT_KEY) + return fallback_value.as_fallback( + FallbackReason.missing_context_key(flag_definition.context) + ) selected_variant: SelectedVariant | None = None @@ -267,7 +269,7 @@ def get_variant( context_value, flag_key, ) - return fallback_value.as_fallback(FallbackReason.NO_ROLLOUT_MATCH) + return fallback_value.as_fallback(FallbackReason.no_rollout_match()) def track_exposure_event( self, flag_key: str, variant: SelectedVariant, context: dict[str, Any] diff --git a/mixpanel/flags/remote_feature_flags.py b/mixpanel/flags/remote_feature_flags.py index 551715e..f573c72 100644 --- a/mixpanel/flags/remote_feature_flags.py +++ b/mixpanel/flags/remote_feature_flags.py @@ -157,9 +157,15 @@ async def aget_variant( distinct_id, EXPOSURE_EVENT, properties ) ) - except Exception: + except Exception as exc: logger.exception("Failed to get remote variant for flag '%s'", flag_key) - return fallback_value.as_fallback(FallbackReason.BACKEND_ERROR) + # SDK-83: attach the exception message so the OpenFeature wrapper + # can forward it as error_message. Without this the caller sees + # a bare GENERAL error and has to dig through logs to find out + # the backend rejected the request. + return fallback_value.as_fallback( + FallbackReason.backend_error(self._describe_backend_error(exc)) + ) else: return selected_variant @@ -271,9 +277,13 @@ def get_variant( ) self._tracker(distinct_id, EXPOSURE_EVENT, properties) - except Exception: + except Exception as exc: logger.exception("Failed to get remote variant for flag '%s'", flag_key) - return fallback_value.as_fallback(FallbackReason.BACKEND_ERROR) + # SDK-83: attach the exception message so the OpenFeature wrapper + # can forward it as error_message. + return fallback_value.as_fallback( + FallbackReason.backend_error(self._describe_backend_error(exc)) + ) else: return selected_variant @@ -361,6 +371,20 @@ def _handle_response(self, response: httpx.Response) -> dict[str, SelectedVarian flags_response = RemoteFlagsResponse.model_validate(response.json()) return flags_response.flags + @staticmethod + def _describe_backend_error(exc: Exception) -> str: + """Best-effort backend message for FallbackReason.backend_error. + + For HTTP errors the response body usually contains the actionable + detail (e.g. "distinct_id must be provided in evalContext as a + string") — httpx's default str(exc) only carries the status line, + so reach into exc.response.text when available. + """ + if isinstance(exc, httpx.HTTPStatusError): + body = exc.response.text.strip() if exc.response is not None else "" + return f"HTTP {exc.response.status_code}: {body}" if body else str(exc) + return str(exc) + def _lookup_flag_in_response( self, flag_key: str, @@ -378,7 +402,7 @@ def _lookup_flag_in_response( # so a missing key could mean the flag doesn't exist OR the user # isn't in any rollout. The remote SDK can't tell them apart without # server-side help — surface as FLAG_NOT_FOUND for now. - return fallback_value.as_fallback(FallbackReason.FLAG_NOT_FOUND), True + return fallback_value.as_fallback(FallbackReason.flag_not_found()), True def shutdown(self): self._sync_client.close() diff --git a/mixpanel/flags/test_local_feature_flags.py b/mixpanel/flags/test_local_feature_flags.py index 2b4366c..9839b1b 100644 --- a/mixpanel/flags/test_local_feature_flags.py +++ b/mixpanel/flags/test_local_feature_flags.py @@ -16,7 +16,6 @@ from .types import ( ExperimentationFlag, ExperimentationFlags, - FallbackReason, FlagTestUsers, LocalFlagsConfig, Rollout, @@ -741,7 +740,8 @@ async def test_get_variant_tags_missing_flag(self): fallback = SelectedVariant(variant_value="fb") result = self._flags.get_variant("missing", fallback, USER_CONTEXT) assert result.variant_source == VariantSource.FALLBACK - assert result.fallback_reason == FallbackReason.FLAG_NOT_FOUND + assert result.fallback_reason.kind == "FLAG_NOT_FOUND" + assert result.fallback_reason.message is None assert result.variant_value == "fb" @respx.mock @@ -751,7 +751,8 @@ async def test_get_variant_tags_missing_context(self): fallback = SelectedVariant(variant_value="fb") result = self._flags.get_variant(TEST_FLAG_KEY, fallback, {}) assert result.variant_source == VariantSource.FALLBACK - assert result.fallback_reason == FallbackReason.MISSING_CONTEXT_KEY + assert result.fallback_reason.kind == "MISSING_CONTEXT_KEY" + assert result.fallback_reason.message == "distinct_id" @respx.mock async def test_get_variant_tags_no_rollout_match(self): @@ -760,7 +761,7 @@ async def test_get_variant_tags_no_rollout_match(self): fallback = SelectedVariant(variant_value="fb") result = self._flags.get_variant(TEST_FLAG_KEY, fallback, USER_CONTEXT) assert result.variant_source == VariantSource.FALLBACK - assert result.fallback_reason == FallbackReason.NO_ROLLOUT_MATCH + assert result.fallback_reason.kind == "NO_ROLLOUT_MATCH" @respx.mock async def test_get_variant_value_uses_most_recent_polled_flag(self): diff --git a/mixpanel/flags/test_remote_feature_flags.py b/mixpanel/flags/test_remote_feature_flags.py index ec5f16d..a63966b 100644 --- a/mixpanel/flags/test_remote_feature_flags.py +++ b/mixpanel/flags/test_remote_feature_flags.py @@ -10,7 +10,12 @@ from mixpanel.credentials import ServiceAccountCredentials from .remote_feature_flags import RemoteFeatureFlagsProvider -from .types import RemoteFlagsConfig, RemoteFlagsResponse, SelectedVariant +from .types import ( + RemoteFlagsConfig, + RemoteFlagsResponse, + SelectedVariant, + VariantSource, +) ENDPOINT = "https://api.mixpanel.com/flags" @@ -265,6 +270,26 @@ def test_get_variant_value_is_fallback_if_call_fails(self): ) assert result == "control" + @respx.mock + def test_get_variant_tags_fallback_with_backend_message_on_http_error(self): + """SDK-83: the backend's response message must propagate through + FallbackReason.message so the OpenFeature wrapper can forward it + as error_message instead of swallowing it into a bare GENERAL.""" + respx.get(ENDPOINT).mock( + return_value=httpx.Response( + 400, text="distinct_id must be provided in evalContext as a string" + ) + ) + + fallback = SelectedVariant(variant_value="control") + result = self._flags.get_variant( + "test_flag", fallback, {"distinct_id": "user123"}, reportExposure=False + ) + + assert result.variant_source == VariantSource.FALLBACK + assert result.fallback_reason.kind == "BACKEND_ERROR" + assert "distinct_id must be provided" in result.fallback_reason.message + @respx.mock def test_get_variant_value_is_fallback_if_bad_response_format(self): respx.get(ENDPOINT).mock(return_value=httpx.Response(200, text="invalid json")) diff --git a/mixpanel/flags/types.py b/mixpanel/flags/types.py index 85ef8ea..002641d 100644 --- a/mixpanel/flags/types.py +++ b/mixpanel/flags/types.py @@ -1,4 +1,4 @@ -from typing import Any, Optional +from typing import Any, Literal, Optional from pydantic import BaseModel, ConfigDict @@ -76,20 +76,55 @@ class VariantSource: FALLBACK = "fallback" -class FallbackReason: +class FallbackReason(BaseModel): """Why the SDK returned the developer fallback. Only meaningful when SelectedVariant.variant_source == VariantSource.FALLBACK. - Matches the constant set used by mixpanel-php so the OpenFeature wrapper - can map to the spec-correct error code instead of collapsing every - fallback to FLAG_NOT_FOUND. + + `kind` is the discriminator (PHP-aligned). `message` is set on reasons + that carry useful detail (BACKEND_ERROR with the backend's response body, + MISSING_CONTEXT_KEY with the missing attribute name); None otherwise. + The OpenFeature wrapper dispatches on kind and forwards message into + FlagResolutionDetails.error_message. """ - FLAG_NOT_FOUND = "FLAG_NOT_FOUND" - MISSING_CONTEXT_KEY = "MISSING_CONTEXT_KEY" - NO_ROLLOUT_MATCH = "NO_ROLLOUT_MATCH" - BACKEND_ERROR = "BACKEND_ERROR" - NOT_READY = "NOT_READY" + model_config = ConfigDict(frozen=True) + + kind: Literal[ + "FLAG_NOT_FOUND", + "MISSING_CONTEXT_KEY", + "NO_ROLLOUT_MATCH", + "BACKEND_ERROR", + "NOT_READY", + ] + message: Optional[str] = None + + # Factory methods. Reasons without meaningful detail return a frozen + # singleton; reasons with detail allocate per call. + @classmethod + def flag_not_found(cls) -> "FallbackReason": + return _FLAG_NOT_FOUND + + @classmethod + def no_rollout_match(cls) -> "FallbackReason": + return _NO_ROLLOUT_MATCH + + @classmethod + def not_ready(cls) -> "FallbackReason": + return _NOT_READY + + @classmethod + def missing_context_key(cls, key: Optional[str] = None) -> "FallbackReason": + return cls(kind="MISSING_CONTEXT_KEY", message=key) + + @classmethod + def backend_error(cls, message: str) -> "FallbackReason": + return cls(kind="BACKEND_ERROR", message=message) + + +_FLAG_NOT_FOUND = FallbackReason(kind="FLAG_NOT_FOUND") +_NO_ROLLOUT_MATCH = FallbackReason(kind="NO_ROLLOUT_MATCH") +_NOT_READY = FallbackReason(kind="NOT_READY") class SelectedVariant(BaseModel): @@ -100,8 +135,8 @@ class SelectedVariant(BaseModel): is_experiment_active: Optional[bool] = None is_qa_tester: Optional[bool] = None variant_source: Optional[str] = None - # None on success; one of FallbackReason.* when variant_source is FALLBACK - fallback_reason: Optional[str] = None + # None on success; set when variant_source == FALLBACK + fallback_reason: Optional[FallbackReason] = None def with_source(self, source: str) -> "SelectedVariant": """Return a copy of this variant tagged with the given source. @@ -112,7 +147,7 @@ def with_source(self, source: str) -> "SelectedVariant": update={"variant_source": source, "fallback_reason": None} ) - def as_fallback(self, reason: str) -> "SelectedVariant": + def as_fallback(self, reason: FallbackReason) -> "SelectedVariant": """Return a copy of this variant tagged as a fallback with the given reason.""" return self.model_copy( update={ diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index 3364ebc..e3ba387 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -157,10 +157,11 @@ def _resolve( user_context = self._build_user_context(evaluation_context) try: result = self._flags_provider.get_variant(flag_key, fallback, user_context) - except Exception: + except Exception as exc: return FlagResolutionDetails( value=default_value, error_code=ErrorCode.GENERAL, + error_message=str(exc), reason=Reason.ERROR, ) @@ -219,34 +220,35 @@ def _resolve( @staticmethod def _fallback_details( - fallback_reason: str | None, default_value: typing.Any + fallback_reason: FallbackReason | None, default_value: typing.Any ) -> FlagResolutionDetails | None: """Map a fallback reason to its OpenFeature response, or None if not a fallback. variant_source distinguishes local / remote / fallback. When fallback, - fallback_reason carries the specific reason (PHP-aligned constants) - so we map each to the spec-correct OpenFeature response instead of - collapsing every fallback to FLAG_NOT_FOUND. + fallback_reason carries the discriminating kind (PHP-aligned) and an + optional message (BACKEND_ERROR's response body, MISSING_CONTEXT_KEY's + missing attribute) so we map each to the spec-correct OpenFeature + response and forward the message as error_message. """ if fallback_reason is None: return None # Flag exists, user just didn't match any rollout — per the # OpenFeature spec this is `reason: DEFAULT` with no error. - if fallback_reason == FallbackReason.NO_ROLLOUT_MATCH: + if fallback_reason.kind == "NO_ROLLOUT_MATCH": return FlagResolutionDetails(value=default_value, reason=Reason.DEFAULT) error_mapping = { - FallbackReason.FLAG_NOT_FOUND: (ErrorCode.FLAG_NOT_FOUND, Reason.DEFAULT), - FallbackReason.MISSING_CONTEXT_KEY: ( - ErrorCode.TARGETING_KEY_MISSING, - Reason.ERROR, - ), - FallbackReason.BACKEND_ERROR: (ErrorCode.GENERAL, Reason.ERROR), - FallbackReason.NOT_READY: (ErrorCode.PROVIDER_NOT_READY, Reason.ERROR), + "FLAG_NOT_FOUND": (ErrorCode.FLAG_NOT_FOUND, Reason.DEFAULT), + "MISSING_CONTEXT_KEY": (ErrorCode.TARGETING_KEY_MISSING, Reason.ERROR), + "BACKEND_ERROR": (ErrorCode.GENERAL, Reason.ERROR), + "NOT_READY": (ErrorCode.PROVIDER_NOT_READY, Reason.ERROR), } - if fallback_reason in error_mapping: - error_code, reason = error_mapping[fallback_reason] + if fallback_reason.kind in error_mapping: + error_code, reason = error_mapping[fallback_reason.kind] return FlagResolutionDetails( - value=default_value, error_code=error_code, reason=reason + value=default_value, + error_code=error_code, + error_message=fallback_reason.message, + reason=reason, ) return None diff --git a/openfeature-provider/tests/test_provider.py b/openfeature-provider/tests/test_provider.py index 6114469..62756ce 100644 --- a/openfeature-provider/tests/test_provider.py +++ b/openfeature-provider/tests/test_provider.py @@ -30,7 +30,7 @@ def setup_flag(mock_flags, flag_key, value, variant_key="variant-key"): variant_source=VariantSource.LOCAL, ) if key == flag_key - else fallback.as_fallback(FallbackReason.FLAG_NOT_FOUND) + else fallback.as_fallback(FallbackReason.flag_not_found()) ) @@ -43,7 +43,7 @@ def setup_fallback(mock_flags, reason): def setup_flag_not_found(mock_flags, flag_key): """Configure mock for the genuinely-missing-flag path.""" - setup_fallback(mock_flags, FallbackReason.FLAG_NOT_FOUND) + setup_fallback(mock_flags, FallbackReason.flag_not_found()) # --- Metadata --- @@ -322,7 +322,7 @@ def test_remote_provider_always_ready(): def test_no_rollout_match_returns_default_reason_without_error(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.NO_ROLLOUT_MATCH) + setup_fallback(mock_flags, FallbackReason.no_rollout_match()) result = provider.resolve_boolean_details("flag", True) assert result.value is True assert result.reason == Reason.DEFAULT @@ -330,7 +330,7 @@ def test_no_rollout_match_returns_default_reason_without_error(provider, mock_fl def test_no_rollout_match_for_string(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.NO_ROLLOUT_MATCH) + setup_fallback(mock_flags, FallbackReason.no_rollout_match()) result = provider.resolve_string_details("flag", "default") assert result.value == "default" assert result.reason == Reason.DEFAULT @@ -341,37 +341,46 @@ def test_no_rollout_match_for_string(provider, mock_flags): def test_missing_context_key_returns_targeting_key_missing(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.MISSING_CONTEXT_KEY) + setup_fallback(mock_flags, FallbackReason.missing_context_key("distinct_id")) result = provider.resolve_boolean_details("flag", False) assert result.value is False assert result.error_code == ErrorCode.TARGETING_KEY_MISSING assert result.reason == Reason.ERROR -def test_missing_context_key_for_string(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.MISSING_CONTEXT_KEY) +def test_missing_context_key_forwards_missing_attribute_as_error_message( + provider, mock_flags +): + setup_fallback(mock_flags, FallbackReason.missing_context_key("distinct_id")) result = provider.resolve_string_details("flag", "default") assert result.value == "default" assert result.error_code == ErrorCode.TARGETING_KEY_MISSING + assert result.error_message == "distinct_id" assert result.reason == Reason.ERROR -# --- Backend error --- +# --- Backend error (SDK-83: forwards the backend's response message) --- -def test_backend_error_maps_to_general(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.BACKEND_ERROR) +def test_backend_error_maps_to_general_and_forwards_message(provider, mock_flags): + setup_fallback( + mock_flags, + FallbackReason.backend_error( + "HTTP 400: distinct_id must be provided in evalContext as a string" + ), + ) result = provider.resolve_string_details("flag", "default") assert result.value == "default" assert result.error_code == ErrorCode.GENERAL assert result.reason == Reason.ERROR + assert "distinct_id must be provided" in result.error_message # --- Not ready --- def test_not_ready_reason_maps_to_provider_not_ready(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.NOT_READY) + setup_fallback(mock_flags, FallbackReason.not_ready()) result = provider.resolve_boolean_details("flag", True) assert result.value is True assert result.error_code == ErrorCode.PROVIDER_NOT_READY From 3f58c9480f8bd1ab69b4a09fd07b352255d7ea17 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Tue, 30 Jun 2026 10:54:24 -0400 Subject: [PATCH 4/7] fix(flags): drop unused NOT_READY FallbackReason kind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wrapper short-circuits to PROVIDER_NOT_READY at the top of resolve when areFlagsReady() is false (see _are_flags_ready), so no producer ever constructs a FallbackReason with kind NOT_READY — the case was dead, same pattern Swift PR #745 and Android PR #981 just cleaned up. Remove the kind from the Literal type, drop the not_ready() factory and _NOT_READY singleton, and drop the NOT_READY dispatch arm from the wrapper's error_mapping. The provider_not_ready short-circuit test path is unchanged. Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/types.py | 10 ++++------ .../src/mixpanel_openfeature/provider.py | 4 +++- openfeature-provider/tests/test_provider.py | 13 ++++--------- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/mixpanel/flags/types.py b/mixpanel/flags/types.py index 002641d..7ec9d6f 100644 --- a/mixpanel/flags/types.py +++ b/mixpanel/flags/types.py @@ -86,6 +86,10 @@ class FallbackReason(BaseModel): MISSING_CONTEXT_KEY with the missing attribute name); None otherwise. The OpenFeature wrapper dispatches on kind and forwards message into FlagResolutionDetails.error_message. + + Note: the wrapper handles PROVIDER_NOT_READY by short-circuiting before + invoking the provider (see MixpanelProvider._are_flags_ready), so there + is no NOT_READY kind here — no producer would ever construct it. """ model_config = ConfigDict(frozen=True) @@ -95,7 +99,6 @@ class FallbackReason(BaseModel): "MISSING_CONTEXT_KEY", "NO_ROLLOUT_MATCH", "BACKEND_ERROR", - "NOT_READY", ] message: Optional[str] = None @@ -109,10 +112,6 @@ def flag_not_found(cls) -> "FallbackReason": def no_rollout_match(cls) -> "FallbackReason": return _NO_ROLLOUT_MATCH - @classmethod - def not_ready(cls) -> "FallbackReason": - return _NOT_READY - @classmethod def missing_context_key(cls, key: Optional[str] = None) -> "FallbackReason": return cls(kind="MISSING_CONTEXT_KEY", message=key) @@ -124,7 +123,6 @@ def backend_error(cls, message: str) -> "FallbackReason": _FLAG_NOT_FOUND = FallbackReason(kind="FLAG_NOT_FOUND") _NO_ROLLOUT_MATCH = FallbackReason(kind="NO_ROLLOUT_MATCH") -_NOT_READY = FallbackReason(kind="NOT_READY") class SelectedVariant(BaseModel): diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index e3ba387..265a1c7 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -236,11 +236,13 @@ def _fallback_details( # OpenFeature spec this is `reason: DEFAULT` with no error. if fallback_reason.kind == "NO_ROLLOUT_MATCH": return FlagResolutionDetails(value=default_value, reason=Reason.DEFAULT) + # PROVIDER_NOT_READY is handled before invoking the provider + # (see _are_flags_ready short-circuit at the top of resolve), so + # there's no NOT_READY kind to dispatch on here. error_mapping = { "FLAG_NOT_FOUND": (ErrorCode.FLAG_NOT_FOUND, Reason.DEFAULT), "MISSING_CONTEXT_KEY": (ErrorCode.TARGETING_KEY_MISSING, Reason.ERROR), "BACKEND_ERROR": (ErrorCode.GENERAL, Reason.ERROR), - "NOT_READY": (ErrorCode.PROVIDER_NOT_READY, Reason.ERROR), } if fallback_reason.kind in error_mapping: error_code, reason = error_mapping[fallback_reason.kind] diff --git a/openfeature-provider/tests/test_provider.py b/openfeature-provider/tests/test_provider.py index 62756ce..9564780 100644 --- a/openfeature-provider/tests/test_provider.py +++ b/openfeature-provider/tests/test_provider.py @@ -376,15 +376,10 @@ def test_backend_error_maps_to_general_and_forwards_message(provider, mock_flags assert "distinct_id must be provided" in result.error_message -# --- Not ready --- - - -def test_not_ready_reason_maps_to_provider_not_ready(provider, mock_flags): - setup_fallback(mock_flags, FallbackReason.not_ready()) - result = provider.resolve_boolean_details("flag", True) - assert result.value is True - assert result.error_code == ErrorCode.PROVIDER_NOT_READY - assert result.reason == Reason.ERROR +# Not-ready handling is covered by the test_provider_not_ready_* tests above, +# which exercise the wrapper's are_flags_ready short-circuit. The provider +# never stamps a NOT_READY fallback reason, so there is no producer-side +# dispatch to test here. # --- Lifecycle --- From 54c8e5f583069ac909b602ea6ef313fa4b566194 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Tue, 30 Jun 2026 11:06:22 -0400 Subject: [PATCH 5/7] chore: drop verbose NOT_READY comments Comments were explaining the absence of a case to a hypothetical cross-SDK reader. The absence is self-explanatory. Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/types.py | 4 ---- openfeature-provider/src/mixpanel_openfeature/provider.py | 3 --- openfeature-provider/tests/test_provider.py | 6 ------ 3 files changed, 13 deletions(-) diff --git a/mixpanel/flags/types.py b/mixpanel/flags/types.py index 7ec9d6f..3426198 100644 --- a/mixpanel/flags/types.py +++ b/mixpanel/flags/types.py @@ -86,10 +86,6 @@ class FallbackReason(BaseModel): MISSING_CONTEXT_KEY with the missing attribute name); None otherwise. The OpenFeature wrapper dispatches on kind and forwards message into FlagResolutionDetails.error_message. - - Note: the wrapper handles PROVIDER_NOT_READY by short-circuiting before - invoking the provider (see MixpanelProvider._are_flags_ready), so there - is no NOT_READY kind here — no producer would ever construct it. """ model_config = ConfigDict(frozen=True) diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index 265a1c7..80cacf8 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -236,9 +236,6 @@ def _fallback_details( # OpenFeature spec this is `reason: DEFAULT` with no error. if fallback_reason.kind == "NO_ROLLOUT_MATCH": return FlagResolutionDetails(value=default_value, reason=Reason.DEFAULT) - # PROVIDER_NOT_READY is handled before invoking the provider - # (see _are_flags_ready short-circuit at the top of resolve), so - # there's no NOT_READY kind to dispatch on here. error_mapping = { "FLAG_NOT_FOUND": (ErrorCode.FLAG_NOT_FOUND, Reason.DEFAULT), "MISSING_CONTEXT_KEY": (ErrorCode.TARGETING_KEY_MISSING, Reason.ERROR), diff --git a/openfeature-provider/tests/test_provider.py b/openfeature-provider/tests/test_provider.py index 9564780..b2f12da 100644 --- a/openfeature-provider/tests/test_provider.py +++ b/openfeature-provider/tests/test_provider.py @@ -376,12 +376,6 @@ def test_backend_error_maps_to_general_and_forwards_message(provider, mock_flags assert "distinct_id must be provided" in result.error_message -# Not-ready handling is covered by the test_provider_not_ready_* tests above, -# which exercise the wrapper's are_flags_ready short-circuit. The provider -# never stamps a NOT_READY fallback reason, so there is no producer-side -# dispatch to test here. - - # --- Lifecycle --- From 6e0f514227deaf384c339246805a45da6df78b0c Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Tue, 30 Jun 2026 16:18:21 -0400 Subject: [PATCH 6/7] fix(flags): tag get_all_variants results + defensive bare-fallback handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review-driven fixes: 1. RemoteFeatureFlagsProvider.{get,aget}_all_variants now stamp variant_source=REMOTE on every returned variant, matching the docstring's promise that VariantSource is set on "every returned variant". Previously these methods returned the raw decoded SelectedVariants and undercut the wrapper's source-based dispatch. 2. The OpenFeature wrapper now treats `result is fallback && fallback_reason is None` as FLAG_NOT_FOUND. Defends against custom flags providers written against the pre-SDK-79 contract that return the fallback object unchanged — otherwise the wrapper would incorrectly report a successful targeting match. Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/remote_feature_flags.py | 4 ++++ mixpanel/flags/test_remote_feature_flags.py | 11 +++++++++-- .../src/mixpanel_openfeature/provider.py | 18 +++++++++++++++++- openfeature-provider/tests/test_provider.py | 14 ++++++++++++++ 4 files changed, 44 insertions(+), 3 deletions(-) diff --git a/mixpanel/flags/remote_feature_flags.py b/mixpanel/flags/remote_feature_flags.py index f573c72..caba858 100644 --- a/mixpanel/flags/remote_feature_flags.py +++ b/mixpanel/flags/remote_feature_flags.py @@ -100,6 +100,8 @@ async def aget_all_variants( except Exception: logger.exception("Failed to get remote variants") + if flags is not None: + flags = {k: v.with_source(VariantSource.REMOTE) for k, v in flags.items()} return flags async def aget_variant_value( @@ -222,6 +224,8 @@ def get_all_variants( except Exception: logger.exception("Failed to get remote variants") + if flags is not None: + flags = {k: v.with_source(VariantSource.REMOTE) for k, v in flags.items()} return flags def get_variant_value( diff --git a/mixpanel/flags/test_remote_feature_flags.py b/mixpanel/flags/test_remote_feature_flags.py index a63966b..b92d06e 100644 --- a/mixpanel/flags/test_remote_feature_flags.py +++ b/mixpanel/flags/test_remote_feature_flags.py @@ -159,7 +159,11 @@ async def test_aget_all_variants_returns_all_variants_from_api(self): result = await self._flags.aget_all_variants({"distinct_id": "user123"}) - assert result == variants + assert set(result.keys()) == {"flag1", "flag2"} + assert result["flag1"].variant_value == "value1" + assert result["flag2"].variant_value == "value2" + # Every returned variant must be tagged with variant_source=REMOTE. + assert all(v.variant_source == VariantSource.REMOTE for v in result.values()) @respx.mock async def test_aget_all_variants_returns_none_on_network_error(self): @@ -390,7 +394,10 @@ def test_get_all_variants_returns_all_variants_from_api(self): result = self._flags.get_all_variants({"distinct_id": "user123"}) - assert result == variants + assert set(result.keys()) == {"flag1", "flag2"} + assert result["flag1"].variant_value == "value1" + assert result["flag2"].variant_value == "value2" + assert all(v.variant_source == VariantSource.REMOTE for v in result.values()) @respx.mock def test_get_all_variants_returns_none_on_network_error(self): diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index 80cacf8..2b03d2d 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -139,7 +139,7 @@ def _build_user_context( user_context["targetingKey"] = evaluation_context.targeting_key return user_context - def _resolve( + def _resolve( # noqa: C901 — type-coercion branches are intentional self, flag_key: str, default_value: typing.Any, @@ -165,6 +165,16 @@ def _resolve( reason=Reason.ERROR, ) + # Defensive: a custom flags provider written against the pre-SDK-79 + # contract may return the fallback object unchanged (no .as_fallback + # tag). Treat that as FLAG_NOT_FOUND rather than a successful match. + if self._is_untagged_fallback(result, fallback): + return FlagResolutionDetails( + value=default_value, + error_code=ErrorCode.FLAG_NOT_FOUND, + reason=Reason.DEFAULT, + ) + fallback_details = self._fallback_details( result.fallback_reason, default_value ) @@ -218,6 +228,12 @@ def _resolve( value=value, variant=variant_key, reason=Reason.TARGETING_MATCH ) + @staticmethod + def _is_untagged_fallback( + result: SelectedVariant, fallback: SelectedVariant + ) -> bool: + return result is fallback and result.fallback_reason is None + @staticmethod def _fallback_details( fallback_reason: FallbackReason | None, default_value: typing.Any diff --git a/openfeature-provider/tests/test_provider.py b/openfeature-provider/tests/test_provider.py index b2f12da..6a5809c 100644 --- a/openfeature-provider/tests/test_provider.py +++ b/openfeature-provider/tests/test_provider.py @@ -163,6 +163,20 @@ def test_flag_not_found_boolean(provider, mock_flags): assert result.reason == Reason.DEFAULT +def test_bare_fallback_treated_as_flag_not_found(provider, mock_flags): + """A custom flags provider written against the pre-SDK-79 contract may + return the fallback object unchanged (no .as_fallback tag). Treat that + as FLAG_NOT_FOUND rather than a successful targeting match.""" + # Return the fallback object as-is with no fallback_reason set. + mock_flags.get_variant.side_effect = ( + lambda _key, fallback, _ctx, **_kwargs: fallback + ) + result = provider.resolve_string_details("any-flag", "default") + assert result.value == "default" + assert result.error_code == ErrorCode.FLAG_NOT_FOUND + assert result.reason == Reason.DEFAULT + + def test_flag_not_found_string(provider, mock_flags): setup_flag_not_found(mock_flags, "missing-flag") result = provider.resolve_string_details("missing-flag", "fallback") From c708c0aa31fc4da9bbb7ef6e6768b296d1310bc0 Mon Sep 17 00:00:00 2001 From: Tyler Roach Date: Wed, 1 Jul 2026 16:14:51 -0400 Subject: [PATCH 7/7] fix(flags): address Greptile P2 review nits (SDK-79) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two low-priority-but-legit review findings on #180: - FallbackReason.missing_context_key(key) — dropped the `Optional[str]` default. Passing None silently produced MISSING_CONTEXT_KEY with message=None, which the OpenFeature wrapper then forwarded as a null error_message — defeating SDK-79's whole point of telling callers *which* attribute was missing. All current callers already pass the key explicitly, so no behavior change; the signature just no longer invites misuse. - provider.py _fallback_details — replaced the terminal `return None` with an explicit GENERAL error for unrecognized FallbackReason.kind values. Every current Literal case is handled above, but a future kind added without updating the mapping would previously return None and be misinterpreted by _resolve as a successful targeting match. Defensive exhaustiveness: fail visibly rather than silently misreport. Co-Authored-By: Claude Opus 4.7 --- mixpanel/flags/types.py | 6 +++++- .../src/mixpanel_openfeature/provider.py | 12 +++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/mixpanel/flags/types.py b/mixpanel/flags/types.py index 3426198..4e02d8c 100644 --- a/mixpanel/flags/types.py +++ b/mixpanel/flags/types.py @@ -109,7 +109,11 @@ def no_rollout_match(cls) -> "FallbackReason": return _NO_ROLLOUT_MATCH @classmethod - def missing_context_key(cls, key: Optional[str] = None) -> "FallbackReason": + def missing_context_key(cls, key: str) -> "FallbackReason": + # The whole point of MISSING_CONTEXT_KEY is telling the caller *which* + # attribute is absent; a nullable default would leak `message=None` + # into the OpenFeature wrapper's error_message and defeat the SDK-79 + # richer-error-propagation goal. return cls(kind="MISSING_CONTEXT_KEY", message=key) @classmethod diff --git a/openfeature-provider/src/mixpanel_openfeature/provider.py b/openfeature-provider/src/mixpanel_openfeature/provider.py index 2b03d2d..6ec9cb3 100644 --- a/openfeature-provider/src/mixpanel_openfeature/provider.py +++ b/openfeature-provider/src/mixpanel_openfeature/provider.py @@ -265,7 +265,17 @@ def _fallback_details( error_message=fallback_reason.message, reason=reason, ) - return None + # Exhaustive: every FallbackReason.Kind Literal is handled above. If a + # new kind is added to the enum without also being wired into the + # mapping (or NO_ROLLOUT_MATCH branch above), fall back to GENERAL + # rather than returning None — which _resolve would silently + # misinterpret as a successful targeting match. + return FlagResolutionDetails( + value=default_value, + error_code=ErrorCode.GENERAL, + error_message=f"Unrecognized fallback_reason.kind: {fallback_reason.kind}", + reason=Reason.ERROR, + ) def _are_flags_ready(self) -> bool: if hasattr(self._flags_provider, "are_flags_ready"):