[AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit (#525)

* [AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit

* [AI-FSSDK] [FSSDK-12958] Add tests for disabled decision events and two experiment rules

* [AI-FSSDK] [FSSDK-12958] Add test for one local holdout hit on two rules

* [AI-FSSDK] [FSSDK-12958] Keep deprecated holdout_decision key alongside list

* [AI-FSSDK] [FSSDK-12958] Add test for holdout hit then CMAB decision error

* [AI-FSSDK] [FSSDK-12958] Re-run CI

---------

Co-authored-by: Farhan Anjum <Farhan.Anjum@optimizely.com>

Author: alexjoeyyong

optimizely/decision_service.py

@@ -1,4 +1,4 @@
-# Copyright 2017-2022, Optimizely
+# Copyright 2017-2022, 2026, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -65,7 +65,8 @@ class VariationResult(TypedDict):
 
 
 class _DecisionResultOptional(TypedDict, total=False):
-    holdout_decision: Decision
+    holdout_decision: Decision  # Deprecated: first entry of holdout_decisions.
+    holdout_decisions: List[Decision]
 
 
 class DecisionResult(_DecisionResultOptional):
@@ -552,7 +553,11 @@ def get_variation(
         }
 
     def get_variation_for_rollout(
-        self, project_config: ProjectConfig, feature: entities.FeatureFlag, user_context: OptimizelyUserContext
+        self,
+        project_config: ProjectConfig,
+        feature: entities.FeatureFlag,
+        user_context: OptimizelyUserContext,
+        holdout_decisions: Optional[list[Decision]] = None
     ) -> tuple[Decision, list[str]]:
         """ Determine which experiment/variation the user is in for a given rollout.
             Returns the variation of the first experiment the user qualifies for.
@@ -562,7 +567,7 @@ def get_variation_for_rollout(
           flagKey: Feature key.
           rollout: Rollout for which we are getting the variation.
           user: ID and attributes for user.
-          options: Decide options.
+          holdout_decisions: If provided, local holdout decisions for skipped rules are appended to it.
 
         Returns:
           Decision namedtuple consisting of experiment and variation for the user and
@@ -606,22 +611,14 @@ def get_variation_for_rollout(
                 return Decision(experiment=rule, variation=forced_decision_variation,
                                 source=enums.DecisionSources.ROLLOUT, cmab_uuid=None), decide_reasons
 
-            local_holdouts = project_config.get_holdouts_for_rule(rule.id)
-            for holdout in local_holdouts:
-                local_holdout_decision = self.get_variation_for_holdout(
-                    holdout, user_context, project_config
-                )
-                decide_reasons.extend(local_holdout_decision['reasons'])
-
-                local_decision = local_holdout_decision['decision']
-                if local_decision.variation is not None:
-                    message = (
-                        f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' "
-                        f"for delivery rule '{rule.key}'."
-                    )
-                    self.logger.info(message)
-                    decide_reasons.append(message)
-                    return local_decision, decide_reasons
+            local_holdout = self._get_local_holdout_decision(
+                project_config, rule, user_context, 'delivery', decide_reasons
+            )
+            if local_holdout is not None:
+                if holdout_decisions is not None:
+                    holdout_decisions.append(local_holdout)
+                index += 1
+                continue
 
             bucketing_id, bucket_reasons = self._get_bucketing_id(user_id, attributes)
             decide_reasons += bucket_reasons
@@ -748,6 +745,7 @@ def get_decision_for_flag(
 
         global_holdout_result: DecisionResult | None = None
         global_holdout_key: str | None = None
+        holdout_decisions: list[Decision] = []
 
         # Check global holdouts (flag level — before any rules are evaluated)
         global_holdouts = project_config.get_global_holdouts()
@@ -781,6 +779,7 @@ def get_decision_for_flag(
             reasons.append(message)
             global_holdout_result = holdout_decision
             global_holdout_key = holdout.key
+            holdout_decisions.append(holdout_decision['decision'])
             break
 
         # Check experiments then rollouts
@@ -807,30 +806,17 @@ def get_decision_for_flag(
                             'error': False,
                             'reasons': reasons
                         }
-                        if global_holdout_result is not None:
-                            result['holdout_decision'] = global_holdout_result['decision']
+                        if holdout_decisions:
+                            result['holdout_decisions'] = holdout_decisions
+                            result['holdout_decision'] = holdout_decisions[0]
                         return result
 
-                    local_holdouts = project_config.get_holdouts_for_rule(experiment.id)
-                    for holdout in local_holdouts:
-                        local_holdout_decision = self.get_variation_for_holdout(
-                            holdout, user_context, project_config
-                        )
-                        reasons.extend(local_holdout_decision['reasons'])
-
-                        local_decision = local_holdout_decision['decision']
-                        if local_decision.variation is not None:
-                            message = (
-                                f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' "
-                                f"for experiment rule '{experiment.key}'."
-                            )
-                            self.logger.info(message)
-                            reasons.append(message)
-                            return {
-                                'decision': local_holdout_decision['decision'],
-                                'error': False,
-                                'reasons': reasons
-                            }
+                    local_holdout = self._get_local_holdout_decision(
+                        project_config, experiment, user_context, 'experiment', reasons
+                    )
+                    if local_holdout is not None:
+                        holdout_decisions.append(local_holdout)
+                        continue
 
                     # Get variation for experiment
                     variation_result = self.get_variation(
@@ -856,8 +842,9 @@ def get_decision_for_flag(
                             'error': False,
                             'reasons': reasons
                         }
-                        if global_holdout_result is not None:
-                            result['holdout_decision'] = global_holdout_result['decision']
+                        if holdout_decisions:
+                            result['holdout_decisions'] = holdout_decisions
+                            result['holdout_decision'] = holdout_decisions[0]
                         return result
 
         if global_holdout_result is not None:
@@ -870,7 +857,7 @@ def get_decision_for_flag(
 
         # If no experiment decision, check rollouts
         rollout_decision, rollout_reasons = self.get_variation_for_rollout(
-            project_config, feature_flag, user_context
+            project_config, feature_flag, user_context, holdout_decisions
         )
         if rollout_reasons:
             reasons.extend(rollout_reasons)
@@ -893,10 +880,33 @@ def get_decision_for_flag(
             'error': False,
             'reasons': reasons
         }
-        if global_holdout_result is not None:
-            final_result['holdout_decision'] = global_holdout_result['decision']
+        if holdout_decisions:
+            final_result['holdout_decisions'] = holdout_decisions
+            final_result['holdout_decision'] = holdout_decisions[0]
         return final_result
 
+    def _get_local_holdout_decision(
+        self,
+        project_config: ProjectConfig,
+        rule: entities.Experiment,
+        user_context: OptimizelyUserContext,
+        rule_type: str,
+        reasons: list[str]
+    ) -> Optional[Decision]:
+        """Returns the first local holdout decision the user is bucketed into for the rule, if any."""
+        for holdout in project_config.get_holdouts_for_rule(rule.id):
+            holdout_result = self.get_variation_for_holdout(holdout, user_context, project_config)
+            reasons.extend(holdout_result['reasons'])
+            if holdout_result['decision'].variation is not None:
+                message = (
+                    f"The user '{user_context.user_id}' is bucketed into local holdout '{holdout.key}' "
+                    f"for {rule_type} rule '{rule.key}'. Skipping the rule."
+                )
+                self.logger.info(message)
+                reasons.append(message)
+                return holdout_result['decision']
+        return None
+
     def get_variation_for_holdout(
         self,
         holdout: entities.Holdout,

optimizely/optimizely.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2023, Optimizely
+# Copyright 2016-2023, 2026, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -1246,7 +1246,7 @@ def _create_optimizely_decision(
             decision_reasons: Optional[list[str]],
             decide_options: list[str],
             project_config: ProjectConfig,
-            holdout_decision: Optional[Decision] = None
+            holdout_decisions: Optional[list[Decision]] = None
     ) -> OptimizelyDecision:
         user_id = user_context.user_id
         feature_enabled = False
@@ -1265,24 +1265,25 @@ def _create_optimizely_decision(
 
         feature_flag = project_config.feature_key_map.get(flag_key)
 
-        # Send holdout impression when user was bucketed into a holdout bypassed due to exclude_targeted_deliveries
-        if (holdout_decision is not None
+        # Skipped local holdouts and global holdouts bypassed by exclude_targeted_deliveries
+        if (holdout_decisions
                 and decision_source != DecisionSources.HOLDOUT
                 and OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options):
-            holdout_enabled = self._get_feature_enabled(holdout_decision.variation)
-            holdout_rule_key = holdout_decision.experiment.key if holdout_decision.experiment else ''
-            self._send_impression_event(
-                project_config,
-                holdout_decision.experiment,
-                holdout_decision.variation,
-                flag_key,
-                holdout_rule_key,
-                str(DecisionSources.HOLDOUT),
-                holdout_enabled,
-                user_id,
-                attributes,
-                holdout_decision.cmab_uuid
-            )
+            for holdout_decision in holdout_decisions:
+                holdout_enabled = self._get_feature_enabled(holdout_decision.variation)
+                holdout_rule_key = holdout_decision.experiment.key if holdout_decision.experiment else ''
+                self._send_impression_event(
+                    project_config,
+                    holdout_decision.experiment,
+                    holdout_decision.variation,
+                    flag_key,
+                    holdout_rule_key,
+                    str(DecisionSources.HOLDOUT),
+                    holdout_enabled,
+                    user_id,
+                    attributes,
+                    holdout_decision.cmab_uuid
+                )
             decision_event_dispatched = True
 
         # Send impression event if Decision came from a feature
@@ -1469,13 +1470,13 @@ def _decide_for_keys(
             user_context,
             merged_decide_options
         )
-        holdout_decisions: dict[str, Optional[Decision]] = {}
+        holdout_decisions: dict[str, Optional[list[Decision]]] = {}
         for i in range(0, len(flags_without_forced_decision)):
             decision = decision_list[i]['decision']
             reasons = decision_list[i]['reasons']
             error = decision_list[i]['error']
             flag_key = flags_without_forced_decision[i].key
-            holdout_decisions[flag_key] = decision_list[i].get('holdout_decision')
+            holdout_decisions[flag_key] = decision_list[i].get('holdout_decisions')
             # store error decision against key and remove key from valid keys
             if error:
                 optimizely_decision = OptimizelyDecision.new_error_decision(flags_without_forced_decision[i].key,
@@ -1496,7 +1497,7 @@ def _decide_for_keys(
                 decision_reasons,
                 merged_decide_options,
                 project_config,
-                holdout_decision=holdout_decisions.get(key)
+                holdout_decisions=holdout_decisions.get(key)
             )
             enabled_flags_only_missing = OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options
             is_enabled = optimizely_decision.enabled

tests/test_decision_service.py

@@ -1,4 +1,4 @@
-# Copyright 2017-2021, Optimizely
+# Copyright 2017-2021, 2026, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -1510,7 +1510,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_rollout(sel
             )
 
         mock_get_variation_for_rollout.assert_called_once_with(
-            self.project_config, feature, user
+            self.project_config, feature, user, []
         )
 
         # Assert no log messages were generated