Refactor: Align Holdout implementation with Swift SDK

This commit refactors the holdout implementation to use proper entity
classes aligned with the Swift SDK architecture, improving type safety
and consistency across the codebase.

## Changes Made

### 1. entities.py
- Added Holdout entity class with proper type safety
- Implemented Holdout.Status constants matching Swift enum
- Added is_activated property (matches Swift's isActivated)
- Added get_audience_conditions_or_ids() method for consistent audience handling

### 2. project_config.py
- Converted holdout storage from dicts to Holdout entities
- Changed global_holdouts from dict to list (matching Swift SDK)
- Convert holdout variations to Variation entities during initialization
- Updated get_holdouts_for_flag() to return list[entities.Holdout]
- Updated get_holdout() to return Optional[entities.Holdout]

### 3. decision_service.py
- Updated get_variation_for_holdout() to accept Holdout entities
- Use is_activated property instead of dict status check
- Use get_audience_conditions_or_ids() for audience evaluation
- CRITICAL FIX: Return holdout as experiment field in Decision (was None)
- Removed HoldoutDict import, now uses entities.Holdout

### 4. bucketer.py
- Unified handling of Experiment and Holdout entities
- Removed dict-based branching for holdouts
- Use entity properties directly (key, id, trafficAllocation)
- Simplified variation lookup (now consistent for all entity types)

## Benefits

- **Type Safety**: Holdouts are now proper entity classes, not plain dicts
- **Consistency**: Holdouts work exactly like Experiments throughout codebase
- **Swift Alignment**: Python SDK matches Swift SDK's holdout architecture
- **Bug Fix**: Decision.experiment now returns Holdout entity (enables metadata access)
- **Maintainability**: Unified code paths reduce complexity

## Alignment with Swift SDK

This implementation now matches Swift SDK's approach where:
- Holdout is a proper struct/class conforming to ExperimentCore
- isActivated is a computed property
- Variations are proper Variation entities
- Decision returns the holdout as the experiment field
- Global holdouts stored as arrays/lists

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <[email protected]>

Author: muzahidul-opti

optimizely/bucketer.py

@@ -125,14 +125,10 @@ def bucket(
             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
+        # Handle both Experiment and Holdout entities (aligned with Swift SDK)
+        # Both have key and id attributes
+        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.'
@@ -141,13 +137,10 @@ def bucket(
 
         variation_id, decide_reasons = self.bucket_to_entity_id(project_config, experiment, user_id, bucketing_id)
         if 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)
+            # Use the same variation lookup method for both experiments and holdouts
+            # This works because we now convert holdout variations to Variation entities
+            # in project_config.py (matching Swift SDK approach)
+            variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)
             return variation, decide_reasons
 
         # No variation found - log message for empty traffic range
@@ -176,21 +169,19 @@ 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)
+        # Handle both Experiment and Holdout entities (aligned with Swift SDK)
+        # Both entities have key, id, and trafficAllocation attributes
+        from . import entities
+
+        experiment_key = experiment.key
+        experiment_id = experiment.id
+        traffic_allocations = experiment.trafficAllocation
+
+        # Check if this is a Holdout entity
+        # Holdouts don't have groups or CMAB
+        is_holdout = isinstance(experiment, entities.Holdout)
+        has_cmab = False if is_holdout else bool(getattr(experiment, 'cmab', None))
+        group_policy = None if is_holdout else getattr(experiment, 'groupPolicy', None)
 
         # Determine if experiment is in a mutually exclusive group.
         # This will not affect evaluation of rollout rules or holdouts.

optimizely/decision_service.py

@@ -14,7 +14,6 @@
 from __future__ import annotations
 from typing import TYPE_CHECKING, NamedTuple, Optional, Sequence, List, TypedDict, Union
 
-from optimizely.helpers.types import HoldoutDict, VariationDict
 
 from . import bucketer
 from . import entities
@@ -719,7 +718,7 @@ def get_decision_for_flag(
                 continue
 
             message = (
-                f"The user '{user_id}' is bucketed into holdout '{holdout['key']}' "
+                f"The user '{user_id}' is bucketed into holdout '{holdout.key}' "
                 f"for feature flag '{feature_flag.key}'."
             )
             self.logger.info(message)
@@ -748,15 +747,17 @@ def get_decision_for_flag(
 
     def get_variation_for_holdout(
         self,
-        holdout: HoldoutDict,
+        holdout: entities.Holdout,
         user_context: OptimizelyUserContext,
         project_config: ProjectConfig
     ) -> DecisionResult:
         """
         Get the variation for holdout.
 
+        Aligned with Swift SDK's getVariationForHoldout which returns DecisionResponse<Variation>.
+
         Args:
-            holdout: The holdout configuration (HoldoutDict).
+            holdout: The holdout configuration (Holdout entity).
             user_context: The user context.
             project_config: The project config.
 
@@ -769,9 +770,10 @@ def get_variation_for_holdout(
         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."
+        # Check if holdout is activated (Running status)
+        # Aligned with Swift's: guard holdout.isActivated else { return nil }
+        if not holdout.is_activated:
+            message = f"Holdout '{holdout.key}' is not running."
             self.logger.info(message)
             decide_reasons.append(message)
             return {
@@ -783,13 +785,14 @@ def get_variation_for_holdout(
         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')
+        # Check audience conditions using the same method as experiments
+        # Holdout now has get_audience_conditions_or_ids() just like Experiment
+        audience_conditions = holdout.get_audience_conditions_or_ids()
         user_meets_audience_conditions, reasons_received = audience_helper.does_user_meet_audience_conditions(
             project_config,
             audience_conditions,
             ExperimentAudienceEvaluationLogs,
-            holdout.get('key', 'unknown'),
+            holdout.key,
             user_context,
             self.logger
         )
@@ -798,7 +801,7 @@ def get_variation_for_holdout(
         if not user_meets_audience_conditions:
             message = (
                 f"User '{user_id}' does not meet the conditions for holdout "
-                f"'{holdout['key']}'."
+                f"'{holdout.key}'."
             )
             self.logger.debug(message)
             decide_reasons.append(message)
@@ -810,23 +813,23 @@ def get_variation_for_holdout(
 
         # Bucket user into holdout variation
         variation, bucket_reasons = self.bucketer.bucket(
-            project_config, holdout, user_id, bucketing_id  # type: ignore[arg-type]
+            project_config, holdout, user_id, bucketing_id
         )
         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']}'."
+                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
+            # CRITICAL FIX: Return holdout as the experiment field (matching Swift SDK)
+            # Swift: return FeatureDecision(experiment: holdout, variation: variation, source: .holdout)
+            # Previously Python returned experiment=None which was inconsistent
             holdout_decision: Decision = Decision(
-                experiment=None,
+                experiment=holdout,  # Changed from None to holdout
                 variation=variation,
                 source=enums.DecisionSources.HOLDOUT,
                 cmab_uuid=None
@@ -837,7 +840,7 @@ def get_variation_for_holdout(
                 'reasons': decide_reasons
             }
 
-        message = f"User '{user_id}' is not bucketed into any variation for holdout '{holdout['key']}'."
+        message = f"User '{user_id}' is not bucketed into any variation for holdout '{holdout.key}'."
         self.logger.info(message)
         decide_reasons.append(message)
         return {

optimizely/entities.py

@@ -194,3 +194,67 @@ def __init__(self, key: str, host: Optional[str] = None, publicKey: Optional[str
         self.key = key
         self.host = host
         self.publicKey = publicKey
+
+
+class Holdout(BaseEntity):
+    """Holdout entity representing a holdout experiment for measuring incremental impact.
+
+    Aligned with Swift SDK implementation where Holdout is a proper entity class
+    that conforms to ExperimentCore protocol.
+    """
+
+    class Status:
+        """Holdout status constants matching Swift enum Status."""
+        DRAFT: Final = 'Draft'
+        RUNNING: Final = 'Running'
+        CONCLUDED: Final = 'Concluded'
+        ARCHIVED: Final = 'Archived'
+
+    def __init__(
+        self,
+        id: str,
+        key: str,
+        status: str,
+        variations: list[VariationDict],
+        trafficAllocation: list[TrafficAllocation],
+        audienceIds: list[str],
+        includedFlags: list[str],
+        excludedFlags: list[str],
+        audienceConditions: Optional[Sequence[str | list[str]]] = None,
+        **kwargs: Any
+    ):
+        self.id = id
+        self.key = key
+        self.status = status
+        self.variations = variations
+        self.trafficAllocation = trafficAllocation
+        self.audienceIds = audienceIds
+        self.audienceConditions = audienceConditions
+        self.includedFlags = includedFlags or []
+        self.excludedFlags = excludedFlags or []
+
+        # Not necessary for holdouts but included for compatibility with ExperimentCore pattern
+        self.layerId = ''
+
+    def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
+        """Returns audienceConditions if present, otherwise audienceIds.
+
+        This matches the Experiment.get_audience_conditions_or_ids() method
+        and enables holdouts to work with the same audience evaluation logic.
+        """
+        return self.audienceConditions if self.audienceConditions is not None else self.audienceIds
+
+    @property
+    def is_activated(self) -> bool:
+        """Check if the holdout is activated (running).
+
+        Matches Swift's isActivated computed property:
+        var isActivated: Bool { return status == .running }
+
+        Returns:
+            True if status is 'Running', False otherwise.
+        """
+        return self.status == self.Status.RUNNING
+
+    def __str__(self) -> str:
+        return self.key