[FSSDK-11571] Python: Add holdout support and refactor decision logic in DefaultDecisionService (#467)

* [FSSDK-11571] Python: Add holdout support and refactor decision logic in DefaultDecisionService

* Fix all lint issues

* Fix type issues

* Fix type check error

* Implement suggested comments

* Implement suggested comments

* Fix lint

Author: esrakartalOpt

optimizely/bucketer.py

@@ -119,16 +119,41 @@ def bucket(
             and array of log messages representing decision making.
      */.
         """
+        # Check if experiment is None first
+        if not experiment:
+            message = 'Invalid entity key provided for bucketing. Returning nil.'
+            project_config.logger.debug(message)
+            return None, []
+
+        if isinstance(experiment, dict):
+            # This is a holdout dictionary
+            experiment_key = experiment.get('key', '')
+            experiment_id = experiment.get('id', '')
+        else:
+            # This is an Experiment object
+            experiment_key = experiment.key
+            experiment_id = experiment.id
+
+        if not experiment_key or not experiment_key.strip():
+            message = 'Invalid entity key provided for bucketing. Returning nil.'
+            project_config.logger.debug(message)
+            return None, []
+
         variation_id, decide_reasons = self.bucket_to_entity_id(project_config, experiment, user_id, bucketing_id)
         if variation_id:
-            variation = project_config.get_variation_from_id_by_experiment_id(experiment.id, variation_id)
+            if isinstance(experiment, dict):
+                # For holdouts, find the variation in the holdout's variations array
+                variations = experiment.get('variations', [])
+                variation = next((v for v in variations if v.get('id') == variation_id), None)
+            else:
+                # For experiments, use the existing method
+                variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)
             return variation, decide_reasons
 
-        else:
-            message = 'Bucketed into an empty traffic range. Returning nil.'
-            project_config.logger.info(message)
-            decide_reasons.append(message)
-
+        # No variation found - log message for empty traffic range
+        message = 'Bucketed into an empty traffic range. Returning nil.'
+        project_config.logger.info(message)
+        decide_reasons.append(message)
         return None, decide_reasons
 
     def bucket_to_entity_id(
@@ -151,9 +176,25 @@ def bucket_to_entity_id(
         if not experiment:
             return None, decide_reasons
 
+        # Handle both Experiment objects and holdout dictionaries
+        if isinstance(experiment, dict):
+            # This is a holdout dictionary - holdouts don't have groups
+            experiment_key = experiment.get('key', '')
+            experiment_id = experiment.get('id', '')
+            traffic_allocations = experiment.get('trafficAllocation', [])
+            has_cmab = False
+            group_policy = None
+        else:
+            # This is an Experiment object
+            experiment_key = experiment.key
+            experiment_id = experiment.id
+            traffic_allocations = experiment.trafficAllocation
+            has_cmab = bool(experiment.cmab)
+            group_policy = getattr(experiment, 'groupPolicy', None)
+
         # Determine if experiment is in a mutually exclusive group.
-        # This will not affect evaluation of rollout rules.
-        if experiment.groupPolicy in GROUP_POLICIES:
+        # This will not affect evaluation of rollout rules or holdouts.
+        if group_policy and group_policy in GROUP_POLICIES:
             group = project_config.get_group(experiment.groupId)
 
             if not group:
@@ -169,26 +210,27 @@ def bucket_to_entity_id(
                 decide_reasons.append(message)
                 return None, decide_reasons
 
-            if user_experiment_id != experiment.id:
-                message = f'User "{user_id}" is not in experiment "{experiment.key}" of group {experiment.groupId}.'
+            if user_experiment_id != experiment_id:
+                message = f'User "{user_id}" is not in experiment "{experiment_key}" of group {experiment.groupId}.'
                 project_config.logger.info(message)
                 decide_reasons.append(message)
                 return None, decide_reasons
 
-            message = f'User "{user_id}" is in experiment {experiment.key} of group {experiment.groupId}.'
+            message = f'User "{user_id}" is in experiment {experiment_key} of group {experiment.groupId}.'
             project_config.logger.info(message)
             decide_reasons.append(message)
 
-        traffic_allocations: list[TrafficAllocation] = experiment.trafficAllocation
-        if experiment.cmab:
-            traffic_allocations = [
-                {
-                    "entityId": "$",
-                    "endOfRange": experiment.cmab['trafficAllocation']
-                }
-            ]
+        if has_cmab:
+            if experiment.cmab:
+                traffic_allocations = [
+                    {
+                        "entityId": "$",
+                        "endOfRange": experiment.cmab['trafficAllocation']
+                    }
+                ]
+
         # Bucket user if not in white-list and in group (if any)
         variation_id = self.find_bucket(project_config, bucketing_id,
-                                        experiment.id, traffic_allocations)
+                                        experiment_id, traffic_allocations)
 
         return variation_id, decide_reasons

optimizely/decision_service.py

@@ -12,7 +12,9 @@
 # limitations under the License.
 
 from __future__ import annotations
-from typing import TYPE_CHECKING, NamedTuple, Optional, Sequence, List, TypedDict
+from typing import TYPE_CHECKING, NamedTuple, Optional, Sequence, List, TypedDict, Union
+
+from optimizely.helpers.types import HoldoutDict, VariationDict
 
 from . import bucketer
 from . import entities
@@ -59,7 +61,7 @@ class VariationResult(TypedDict):
     cmab_uuid: Optional[str]
     error: bool
     reasons: List[str]
-    variation: Optional[entities.Variation]
+    variation: Optional[Union[entities.Variation, VariationDict]]
 
 
 class DecisionResult(TypedDict):
@@ -80,7 +82,7 @@ class Decision(NamedTuple):
     """Named tuple containing selected experiment, variation, source and cmab_uuid.
     None if no experiment/variation was selected."""
     experiment: Optional[entities.Experiment]
-    variation: Optional[entities.Variation]
+    variation: Optional[Union[entities.Variation, VariationDict]]
     source: Optional[str]
     cmab_uuid: Optional[str]
 
@@ -670,7 +672,179 @@ def get_variation_for_feature(
             - 'error': Boolean indicating if an error occurred during the decision process.
             - 'reasons': List of log messages representing decision making for the feature.
         """
-        return self.get_variations_for_feature_list(project_config, [feature], user_context, options)[0]
+        holdouts = project_config.get_holdouts_for_flag(feature.key)
+
+        if holdouts:
+            # Has holdouts - use get_decision_for_flag which checks holdouts first
+            return self.get_decision_for_flag(feature, user_context, project_config, options)
+        else:
+            return self.get_variations_for_feature_list(project_config, [feature], user_context, options)[0]
+
+    def get_decision_for_flag(
+        self,
+        feature_flag: entities.FeatureFlag,
+        user_context: OptimizelyUserContext,
+        project_config: ProjectConfig,
+        decide_options: Optional[Sequence[str]] = None,
+        user_profile_tracker: Optional[UserProfileTracker] = None,
+        decide_reasons: Optional[list[str]] = None
+    ) -> DecisionResult:
+        """
+        Get the decision for a single feature flag.
+        Processes holdouts, experiments, and rollouts in that order.
+
+        Args:
+            feature_flag: The feature flag to get a decision for.
+            user_context: The user context.
+            project_config: The project config.
+            decide_options: Sequence of decide options.
+            user_profile_tracker: The user profile tracker.
+            decide_reasons: List of decision reasons to merge.
+
+        Returns:
+            A DecisionResult for the feature flag.
+        """
+        reasons = decide_reasons.copy() if decide_reasons else []
+        user_id = user_context.user_id
+
+        # Check holdouts
+        holdouts = project_config.get_holdouts_for_flag(feature_flag.key)
+        for holdout in holdouts:
+            holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
+            reasons.extend(holdout_decision['reasons'])
+
+            decision = holdout_decision['decision']
+            # Check if user was bucketed into holdout (has a variation)
+            if decision.variation is None:
+                continue
+
+            message = (
+                f"The user '{user_id}' is bucketed into holdout '{holdout['key']}' "
+                f"for feature flag '{feature_flag.key}'."
+            )
+            self.logger.info(message)
+            reasons.append(message)
+            return {
+                'decision': holdout_decision['decision'],
+                'error': False,
+                'reasons': reasons
+            }
+
+        # If no holdout decision, fall back to existing experiment/rollout logic
+        # Use get_variations_for_feature_list which handles experiments and rollouts
+        fallback_result = self.get_variations_for_feature_list(
+            project_config, [feature_flag], user_context, decide_options
+        )[0]
+
+        # Merge reasons
+        if fallback_result.get('reasons'):
+            reasons.extend(fallback_result['reasons'])
+
+        return {
+            'decision': fallback_result['decision'],
+            'error': fallback_result.get('error', False),
+            'reasons': reasons
+        }
+
+    def get_variation_for_holdout(
+        self,
+        holdout: HoldoutDict,
+        user_context: OptimizelyUserContext,
+        project_config: ProjectConfig
+    ) -> DecisionResult:
+        """
+        Get the variation for holdout.
+
+        Args:
+            holdout: The holdout configuration (HoldoutDict).
+            user_context: The user context.
+            project_config: The project config.
+
+        Returns:
+            A DecisionResult for the holdout.
+        """
+        from optimizely.helpers.enums import ExperimentAudienceEvaluationLogs
+
+        decide_reasons: list[str] = []
+        user_id = user_context.user_id
+        attributes = user_context.get_user_attributes()
+
+        if not holdout or not holdout.get('status') or holdout.get('status') != 'Running':
+            key = holdout.get('key') if holdout else 'unknown'
+            message = f"Holdout '{key}' is not running."
+            self.logger.info(message)
+            decide_reasons.append(message)
+            return {
+                'decision': Decision(None, None, enums.DecisionSources.HOLDOUT, None),
+                'error': False,
+                'reasons': decide_reasons
+            }
+
+        bucketing_id, bucketing_id_reasons = self._get_bucketing_id(user_id, attributes)
+        decide_reasons.extend(bucketing_id_reasons)
+
+        # Check audience conditions
+        audience_conditions = holdout.get('audienceIds')
+        user_meets_audience_conditions, reasons_received = audience_helper.does_user_meet_audience_conditions(
+            project_config,
+            audience_conditions,
+            ExperimentAudienceEvaluationLogs,
+            holdout.get('key', 'unknown'),
+            user_context,
+            self.logger
+        )
+        decide_reasons.extend(reasons_received)
+
+        if not user_meets_audience_conditions:
+            message = (
+                f"User '{user_id}' does not meet the conditions for holdout "
+                f"'{holdout['key']}'."
+            )
+            self.logger.debug(message)
+            decide_reasons.append(message)
+            return {
+                'decision': Decision(None, None, enums.DecisionSources.HOLDOUT, None),
+                'error': False,
+                'reasons': decide_reasons
+            }
+
+        # Bucket user into holdout variation
+        variation, bucket_reasons = self.bucketer.bucket(
+            project_config, holdout, user_id, bucketing_id  # type: ignore[arg-type]
+        )
+        decide_reasons.extend(bucket_reasons)
+
+        if variation:
+            # For holdouts, variation is a dict, not a Variation entity
+            variation_key = variation['key'] if isinstance(variation, dict) else variation.key
+            message = (
+                f"The user '{user_id}' is bucketed into variation '{variation_key}' "
+                f"of holdout '{holdout['key']}'."
+            )
+            self.logger.info(message)
+            decide_reasons.append(message)
+
+            # Create Decision for holdout - experiment is None, source is HOLDOUT
+            holdout_decision: Decision = Decision(
+                experiment=None,
+                variation=variation,
+                source=enums.DecisionSources.HOLDOUT,
+                cmab_uuid=None
+            )
+            return {
+                'decision': holdout_decision,
+                'error': False,
+                'reasons': decide_reasons
+            }
+
+        message = f"User '{user_id}' is not bucketed into any variation for holdout '{holdout['key']}'."
+        self.logger.info(message)
+        decide_reasons.append(message)
+        return {
+            'decision': Decision(None, None, enums.DecisionSources.HOLDOUT, None),
+            'error': False,
+            'reasons': decide_reasons
+        }
 
     def validated_forced_decision(
         self,
@@ -779,7 +953,7 @@ def get_variations_for_feature_list(
             if feature.experimentIds:
                 for experiment_id in feature.experimentIds:
                     experiment = project_config.get_experiment_from_id(experiment_id)
-                    decision_variation = None
+                    decision_variation: Optional[Union[entities.Variation, VariationDict]] = None
 
                     if experiment:
                         optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(

optimizely/helpers/enums.py

@@ -99,6 +99,7 @@ class DecisionSources:
     EXPERIMENT: Final = 'experiment'
     FEATURE_TEST: Final = 'feature-test'
     ROLLOUT: Final = 'rollout'
+    HOLDOUT: Final = 'holdout'
 
 
 class Errors:

optimizely/helpers/types.py

@@ -12,7 +12,7 @@
 # limitations under the License.
 from __future__ import annotations
 
-from typing import Optional, Any
+from typing import Literal, Optional, Any
 from sys import version_info
 
 
@@ -115,3 +115,16 @@ class CmabDict(BaseEntity):
     """Cmab dict from parsed datafile json."""
     attributeIds: list[str]
     trafficAllocation: int
+
+
+HoldoutStatus = Literal['Draft', 'Running', 'Concluded', 'Archived']
+
+
+class HoldoutDict(ExperimentDict):
+    """Holdout dict from parsed datafile json.
+
+    Extends ExperimentDict with holdout-specific properties.
+    """
+    holdoutStatus: HoldoutStatus
+    includedFlags: list[str]
+    excludedFlags: list[str]