[AI-FSSDK] [FSSDK-12760] Add localHoldouts datafile section for backward compat

optimizely/entities.py

@@ -233,7 +233,8 @@ def __init__(
         self.trafficAllocation = trafficAllocation
         self.audienceIds = audienceIds
         self.audienceConditions = audienceConditions
-        # None = global holdout (applies to all rules); list of rule IDs = local holdout
+        # Per-rule targeting for local holdouts. Scope comes from the datafile
+        # section, not this field; ProjectConfig strips it on 'holdouts' entries.
         self.included_rules: Optional[list[str]] = includedRules
 
     def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
@@ -246,13 +247,11 @@ def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
 
     @property
     def is_global(self) -> bool:
-        """Check if this is a global holdout (applies to all rules across all flags).
+        """True if this is a global holdout.
 
-        A holdout is global when includedRules is None (absent from datafile).
-        An empty list [] is a local holdout that targets no rules (different from global).
-
-        Returns:
-            True if included_rules is None (global), False otherwise (local).
+        Scope is set by the datafile section ('holdouts' vs 'localHoldouts').
+        ProjectConfig strips 'includedRules' on 'holdouts' entries, so this
+        property stays consistent with section membership.
         """
         return self.included_rules is None
 

optimizely/helpers/types.py

@@ -130,4 +130,6 @@ class HoldoutDict(ExperimentDict):
     Extends ExperimentDict with holdout-specific properties.
     """
     holdoutStatus: HoldoutStatus
-    includedRules: Optional[list[str]]  # None = global holdout; list of rule IDs = local holdout
+    # Per-rule targeting for local holdouts. Scope comes from the datafile section,
+    # not this field; required on 'localHoldouts' entries, stripped on 'holdouts'.
+    includedRules: Optional[list[str]]

optimizely/project_config.py

@@ -89,18 +89,58 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
         region_value = config.get('region')
         self.region: str = region_value or 'US'
 
-        # Parse holdouts from datafile and convert to Holdout entities
-        holdouts_data: list[types.HoldoutDict] = config.get('holdouts', [])
+        # Parse holdouts from datafile and convert to Holdout entities.
+        #
+        # Two top-level datafile sections drive holdout scoping (Gen 3+):
+        #   - 'holdouts'      → ALL entries are global holdouts (applied to every flag).
+        #                       Any 'includedRules' field on these entries is IGNORED;
+        #                       section membership alone determines scope.
+        #   - 'localHoldouts' → ALL entries are local holdouts (rule-scoped via
+        #                       'includedRules'). Entries missing 'includedRules' are
+        #                       invalid and skipped with an error log.
+        #
+        # Backward compatibility: older datafiles that only emit the 'holdouts' section
+        # continue to work — every entry is treated as global, matching pre-localHoldouts
+        # behavior. The 'localHoldouts' key is simply absent and parsed as an empty list.
+        global_holdouts_data: list[types.HoldoutDict] = config.get('holdouts', [])
+        local_holdouts_data: list[types.HoldoutDict] = config.get('localHoldouts', [])
         self.holdouts: list[entities.Holdout] = []
         self.holdout_id_map: dict[str, entities.Holdout] = {}
-        # Global holdouts (includedRules is None) — evaluated at flag level before any rule
+        # Global holdouts — evaluated at flag level before any rule
         self.global_holdouts: list[entities.Holdout] = []
         # Rule-level holdouts — map from rule ID to holdouts targeting that rule
         self.rule_holdouts_map: dict[str, list[entities.Holdout]] = {}
 
-        # Convert holdout dicts to Holdout entities
-        for holdout_data in holdouts_data:
-            # Create Holdout entity
+        # Process global holdouts: section membership is the sole signal for scope.
+        # Drop any 'includedRules' field on entries here so the entity is unambiguously
+        # global (is_global → True), even if the datafile incorrectly includes one.
+        for holdout_data in global_holdouts_data:
+            # Copy the typed dict and remove includedRules without losing its shape.
+            sanitized = cast(types.HoldoutDict, dict(holdout_data))
+            sanitized.pop('includedRules', None)  # type: ignore[misc]
+            holdout = entities.Holdout(**sanitized)
+            self.holdouts.append(holdout)
+
+            # Only process Running holdouts
+            if not holdout.is_activated:
+                continue
+
+            # Map by ID for quick lookup
+            self.holdout_id_map[holdout.id] = holdout
+            self.global_holdouts.append(holdout)
+
+        # Process local holdouts: every entry must carry 'includedRules' (list of rule IDs).
+        # Entries without 'includedRules' are invalid per spec — log an error and exclude
+        # them from evaluation (do NOT fall back to global application).
+        for holdout_data in local_holdouts_data:
+            if 'includedRules' not in holdout_data or holdout_data.get('includedRules') is None:
+                holdout_id = holdout_data.get('id', '<unknown>')
+                self.logger.error(
+                    f'Local holdout with ID "{holdout_id}" is missing required "includedRules" '
+                    f'field and will be excluded from evaluation.'
+                )
+                continue
+
             holdout = entities.Holdout(**holdout_data)
             self.holdouts.append(holdout)
 
@@ -111,15 +151,11 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
             # Map by ID for quick lookup
             self.holdout_id_map[holdout.id] = holdout
 
-            # Classify holdout as global or local based on includedRules
-            if holdout.is_global:
-                self.global_holdouts.append(holdout)
-            else:
-                # Local holdout — register for each targeted rule ID
-                for rule_id in (holdout.included_rules or []):
-                    if rule_id not in self.rule_holdouts_map:
-                        self.rule_holdouts_map[rule_id] = []
-                    self.rule_holdouts_map[rule_id].append(holdout)
+            # Register for each targeted rule ID
+            for rule_id in (holdout.included_rules or []):
+                if rule_id not in self.rule_holdouts_map:
+                    self.rule_holdouts_map[rule_id] = []
+                self.rule_holdouts_map[rule_id].append(holdout)
 
         # Utility maps for quick lookup
         self.group_id_map: dict[str, entities.Group] = self._generate_key_map(self.groups, 'id', entities.Group)
@@ -887,9 +923,11 @@ def get_flag_variation(
         return None
 
     def get_global_holdouts(self) -> list[entities.Holdout]:
-        """Return all global holdouts (includedRules is None).
+        """Return all global holdouts (parsed from the top-level 'holdouts' section).
 
         Global holdouts are evaluated at flag level before any rule is checked.
+        Section membership in 'holdouts' is the sole signal for global scope —
+        any 'includedRules' field on these entries is ignored.
 
         Returns:
             List of global Holdout entities that are currently running.
@@ -899,9 +937,11 @@ def get_global_holdouts(self) -> list[entities.Holdout]:
     def get_holdouts_for_rule(self, rule_id: str) -> list[entities.Holdout]:
         """Return local holdouts that target a specific rule.
 
-        Local holdouts are evaluated per-rule, before the rule's audience and
-        traffic allocation checks. A rule ID not present in any holdout's
-        includedRules simply returns an empty list — silently skipped.
+        Local holdouts come from the top-level 'localHoldouts' datafile section and
+        are scoped per-rule via their 'includedRules' field. They are evaluated
+        before the rule's audience and traffic allocation checks. A rule ID not
+        present in any holdout's includedRules simply returns an empty list —
+        silently skipped.
 
         Args:
             rule_id: The experiment or delivery rule ID to look up.

tests/test_decision_service_holdout.py

@@ -1380,9 +1380,33 @@ def tearDown(self):
         if hasattr(self, 'opt_obj'):
             self.opt_obj.close()
 
-    def _make_opt(self, holdouts):
+    def _make_opt(self, holdouts, local_holdouts=None):
+        """Build an Optimizely instance from holdout entries.
+
+        Per FSSDK-12760, the datafile partitions holdouts into two top-level sections:
+        - 'holdouts' → global holdouts (every entry is global, regardless of includedRules)
+        - 'localHoldouts' → local holdouts (rule-scoped via includedRules)
+
+        For convenience, this helper auto-routes entries: any entry in `holdouts`
+        whose `includedRules` is a non-None list is moved to the `localHoldouts`
+        section. Tests that explicitly want to control sectioning can pass both
+        `holdouts` and `local_holdouts` lists.
+        """
         cfg = self.config_dict_with_features.copy()
-        cfg['holdouts'] = holdouts
+        if local_holdouts is None:
+            globals_list: list = []
+            locals_list: list = []
+            for h in holdouts:
+                if h.get('includedRules') is not None:
+                    locals_list.append(h)
+                else:
+                    globals_list.append(h)
+            cfg['holdouts'] = globals_list
+            if locals_list:
+                cfg['localHoldouts'] = locals_list
+        else:
+            cfg['holdouts'] = holdouts
+            cfg['localHoldouts'] = local_holdouts
         self.opt_obj = optimizely_module.Optimizely(json.dumps(cfg))
         return self.opt_obj