[AI-FSSDK] [FSSDK-12735] Fix holdout impression event tracking and add bypass decision reason

Author: esrakartalOpt

optimizely/decision_service.py

@@ -747,6 +747,7 @@ def get_decision_for_flag(
         user_id = user_context.user_id
 
         global_holdout_result: DecisionResult | None = None
+        global_holdout_key: str | None = None
 
         # Check global holdouts (flag level — before any rules are evaluated)
         global_holdouts = project_config.get_global_holdouts()
@@ -779,6 +780,7 @@ def get_decision_for_flag(
             self.logger.info(message)
             reasons.append(message)
             global_holdout_result = holdout_decision
+            global_holdout_key = holdout.key
             break
 
         # Check experiments then rollouts
@@ -858,6 +860,14 @@ def get_decision_for_flag(
                             result['holdout_decision'] = global_holdout_result['decision']
                         return result
 
+        if global_holdout_result is not None:
+            message = (
+                f"Holdout '{global_holdout_key}' has excludeTargetedDeliveries enabled, "
+                f"continuing to rollout evaluation."
+            )
+            self.logger.info(message)
+            reasons.append(message)
+
         # If no experiment decision, check rollouts
         rollout_decision, rollout_reasons = self.get_variation_for_rollout(
             project_config, feature_flag, user_context

optimizely/optimizely.py

@@ -1283,6 +1283,7 @@ def _create_optimizely_decision(
                 attributes,
                 holdout_decision.cmab_uuid
             )
+            decision_event_dispatched = True
 
         # Send impression event if Decision came from a feature
         if OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options:

tests/test_decision_service_holdout.py

@@ -1866,7 +1866,8 @@ def test_global_holdout_exclude_td_true_blocks_ab_experiment(self):
 
     def test_global_holdout_exclude_td_true_no_td_returns_non_holdout_decision(self):
         """When exclude_targeted_deliveries=True but no TD experiments match,
-        returns a non-holdout decision with the bypassed holdout attached."""
+        returns a non-holdout decision with the bypassed holdout attached
+        and includes the rollout evaluation reason."""
         opt = self._make_opt_with_td(
             [_holdout_with_etd('gh1', 'global_exclude_td', exclude_targeted_deliveries=True)],
         )
@@ -1881,6 +1882,12 @@ def test_global_holdout_exclude_td_true_no_td_returns_non_holdout_decision(self)
         self.assertNotEqual(decision.source, enums.DecisionSources.HOLDOUT)
         self.assertIsNotNone(result.get('holdout_decision'))
 
+        expected_reason = (
+            "Holdout 'global_exclude_td' has excludeTargetedDeliveries enabled, "
+            "continuing to rollout evaluation."
+        )
+        self.assertIn(expected_reason, result['reasons'])
+
     # ------------------------------------------------------------------
     # Test 5: Local holdout on delivery rule with exclude_targeted_deliveries=True
     # ------------------------------------------------------------------
@@ -2019,7 +2026,8 @@ def test_global_holdout_exclude_td_true_td_match_has_holdout_decision(self):
 
     def test_global_holdout_exclude_td_true_no_td_has_holdout_decision(self):
         """When exclude_targeted_deliveries=True and no TD matches,
-        result is non-holdout but holdout_decision key is present."""
+        result is non-holdout but holdout_decision key is present,
+        and reasons include the rollout evaluation continuation message."""
         opt = self._make_opt_with_td(
             [_holdout_with_etd('gh1', 'global_exclude_td', exclude_targeted_deliveries=True)],
         )
@@ -2036,18 +2044,38 @@ def test_global_holdout_exclude_td_true_no_td_has_holdout_decision(self):
         self.assertIsNotNone(holdout_dec)
         self.assertEqual(holdout_dec.source, enums.DecisionSources.HOLDOUT)
 
+        expected_reason = (
+            "Holdout 'global_exclude_td' has excludeTargetedDeliveries enabled, "
+            "continuing to rollout evaluation."
+        )
+        self.assertIn(expected_reason, result['reasons'])
+
     # ------------------------------------------------------------------
     # Test 11: Holdout impression event dispatched for bypassed holdout
     # ------------------------------------------------------------------
 
     def test_holdout_impression_sent_when_td_evaluated(self):
         """When exclude_targeted_deliveries=True and TD matches,
-        two impression events are sent: one for holdout, one for TD."""
+        two impression events are sent: one for holdout, one for TD,
+        and decision_event_dispatched is True in notification."""
         opt = self._make_opt_with_td(
             [_holdout_with_etd('gh1', 'global_exclude_td', exclude_targeted_deliveries=True)],
             experiment_type='td',
         )
-        with mock.patch.object(opt, '_send_impression_event') as mock_send:
+
+        captured_notifications: list[dict[str, object]] = []
+
+        def capture_notification(notification_type: str, user_id: str,
+                                 user_attributes: dict[str, object],
+                                 decision_info: dict[str, object]) -> None:
+            captured_notifications.append(decision_info.copy())
+
+        opt.notification_center.add_notification_listener(
+            enums.NotificationTypes.DECISION,
+            capture_notification
+        )
+
+        with mock.patch.object(opt, '_send_impression_event', wraps=opt._send_impression_event) as mock_send:
             user_ctx = opt.create_user_context('testUserId', {})
             user_ctx.decide('test_feature_in_experiment')
 
@@ -2056,3 +2084,9 @@ def test_holdout_impression_sent_when_td_evaluated(self):
                            for call in mock_send.call_args_list]
         self.assertIn(str(enums.DecisionSources.HOLDOUT), call_rule_types)
         self.assertIn(str(enums.DecisionSources.FEATURE_TEST), call_rule_types)
+
+        self.assertEqual(len(captured_notifications), 1)
+        self.assertTrue(
+            captured_notifications[0].get('decision_event_dispatched'),
+            'decision_event_dispatched should be True when holdout impression is sent'
+        )