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

lib/optimizely.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 #
-#    Copyright 2016-2023, Optimizely and contributors
+#    Copyright 2016-2023, 2026, Optimizely and contributors
 #
 #    Licensed under the Apache License, Version 2.0 (the "License");
 #    you may not use this file except in compliance with the License.
@@ -225,21 +225,25 @@ def create_optimizely_decision(user_context, flag_key, decision, reasons, decide
         decision_event_dispatched = true
       end
 
-      if holdout_decision && !decide_options.include?(OptimizelyDecideOption::DISABLE_DECISION_EVENT) && decision_source != Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']
-        holdout_experiment = holdout_decision.experiment
-        holdout_variation = holdout_decision.variation
-        send_impression(
-          config,
-          holdout_experiment,
-          holdout_variation ? holdout_variation['key'] : '',
-          flag_key,
-          holdout_experiment ? holdout_experiment['key'] : '',
-          holdout_variation ? holdout_variation['featureEnabled'] : false,
-          Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'],
-          user_id,
-          attributes
-        )
-        decision_event_dispatched = true
+      # Accepts a single Decision or an Array; Kernel#Array would splat the Decision struct.
+      holdout_decisions = holdout_decision.is_a?(Array) ? holdout_decision : [holdout_decision].compact
+      if !decide_options.include?(OptimizelyDecideOption::DISABLE_DECISION_EVENT) && decision_source != Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT']
+        holdout_decisions.each do |local_holdout_decision|
+          holdout_experiment = local_holdout_decision.experiment
+          holdout_variation = local_holdout_decision.variation
+          send_impression(
+            config,
+            holdout_experiment,
+            holdout_variation ? holdout_variation['key'] : '',
+            flag_key,
+            holdout_experiment ? holdout_experiment['key'] : '',
+            holdout_variation ? holdout_variation['featureEnabled'] : false,
+            Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'],
+            user_id,
+            attributes
+          )
+          decision_event_dispatched = true
+        end
       end
 
       # Generate all variables map if decide options doesn't include excludeVariables
@@ -408,8 +412,7 @@ def decide_for_keys(user_context, keys, decide_options = [], ignore_default_opti
           next
         end
         flag_decisions[flag_key] = decision
-        holdout_decision = decision_list[i].holdout_decision
-        holdout_decisions[flag_key] = holdout_decision if holdout_decision
+        holdout_decisions[flag_key] = decision_list[i].holdout_decisions || [decision_list[i].holdout_decision].compact
         decision_reasons_dict[flag_key] ||= []
         decision_reasons_dict[flag_key].push(*reasons)
       end

lib/optimizely/decision_service.rb

@@ -41,7 +41,8 @@ class DecisionService
     Decision = Struct.new(:experiment, :variation, :source, :cmab_uuid)
     CmabDecisionResult = Struct.new(:error, :result, :reasons)
     VariationResult = Struct.new(:cmab_uuid, :error, :reasons, :variation_id, :holdout_decision)
-    DecisionResult = Struct.new(:decision, :error, :reasons, :holdout_decision)
+    # holdout_decision is deprecated; it holds the first entry of holdout_decisions.
+    DecisionResult = Struct.new(:decision, :error, :reasons, :holdout_decision, :holdout_decisions)
 
     DECISION_SOURCES = {
       'EXPERIMENT' => 'experiment',
@@ -199,6 +200,7 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
       # Check global holdouts first (flag level) — these apply to all rules across all flags
       holdouts = project_config.global_holdouts
       exclude_td_holdout_decision = nil
+      holdout_decisions = []
 
       holdouts.each do |holdout|
         holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
@@ -211,6 +213,7 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
           @logger.log(Logger::INFO, message)
           reasons.push(message)
           exclude_td_holdout_decision = holdout_decision.decision
+          holdout_decisions << exclude_td_holdout_decision
           break
         else
           message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'."
@@ -225,16 +228,18 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
       unless exclude_td_holdout_decision
         experiment_decision = get_variation_for_feature_experiment(project_config, feature_flag, user_context, user_profile_tracker, decide_options)
         reasons.push(*experiment_decision.reasons)
+        holdout_decisions.concat(experiment_decision.holdout_decisions || [])
 
-        return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons) if experiment_decision.decision
+        return DecisionResult.new(experiment_decision.decision, experiment_decision.error, reasons, holdout_decisions.first, holdout_decisions) if experiment_decision.decision
 
         # If there's an error (e.g., CMAB error), return immediately without falling back to rollout
-        return DecisionResult.new(nil, experiment_decision.error, reasons) if experiment_decision.error
+        return DecisionResult.new(nil, experiment_decision.error, reasons, holdout_decisions.first, holdout_decisions) if experiment_decision.error
       end
 
       # Check if the feature flag has a rollout and the user is bucketed into that rollout
       rollout_decision = get_variation_for_feature_rollout(project_config, feature_flag, user_context)
       reasons.push(*rollout_decision.reasons)
+      holdout_decisions.concat(rollout_decision.holdout_decisions || [])
 
       if rollout_decision.decision
         # Check if this was a forced decision (last reason contains "forced decision map")
@@ -247,11 +252,11 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
           reasons.push(message)
         end
 
-        DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons, exclude_td_holdout_decision)
+        DecisionResult.new(rollout_decision.decision, rollout_decision.error, reasons, holdout_decisions.first, holdout_decisions)
       else
         message = "The user '#{user_id}' is not bucketed into a rollout for feature flag '#{feature_flag['key']}'."
         @logger.log(Logger::INFO, message)
-        DecisionResult.new(nil, false, reasons, exclude_td_holdout_decision)
+        DecisionResult.new(nil, false, reasons, holdout_decisions.first, holdout_decisions)
       end
     end
 
@@ -358,6 +363,7 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont
         return DecisionResult.new(nil, false, decide_reasons)
       end
 
+      holdout_decisions = []
       # Evaluate each experiment and return the first bucketed experiment variation
       feature_flag['experimentIds'].each do |experiment_id|
         experiment = project_config.experiment_id_map[experiment_id]
@@ -377,25 +383,27 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont
         decide_reasons.push(*reasons_received)
 
         # If there's an error, return immediately instead of falling back to next experiment
-        return DecisionResult.new(nil, error, decide_reasons) if error
+        return DecisionResult.new(nil, error, decide_reasons, holdout_decisions.first, holdout_decisions) if error
 
-        # If a local holdout decision was made, return it directly
-        return DecisionResult.new(variation_result.holdout_decision, false, decide_reasons) if variation_result.holdout_decision
+        if variation_result.holdout_decision
+          holdout_decisions << variation_result.holdout_decision
+          next
+        end
 
         next unless variation_id
 
         variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)
         variation = project_config.get_variation_from_flag(feature_flag['key'], variation_id, 'id') if variation.nil?
 
         decision = Decision.new(experiment, variation, DECISION_SOURCES['FEATURE_TEST'], cmab_uuid)
-        return DecisionResult.new(decision, error, decide_reasons)
+        return DecisionResult.new(decision, error, decide_reasons, holdout_decisions.first, holdout_decisions)
       end
 
       message = "The user '#{user_id}' is not bucketed into any of the experiments on the feature '#{feature_flag_key}'."
       @logger.log(Logger::INFO, message)
       decide_reasons.push(message)
 
-      DecisionResult.new(nil, false, decide_reasons)
+      DecisionResult.new(nil, false, decide_reasons, holdout_decisions.first, holdout_decisions)
     end
 
     def get_variation_for_feature_rollout(project_config, feature_flag, user_context)
@@ -431,22 +439,27 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context
 
       index = 0
       rollout_rules = rollout['experiments']
+      holdout_decisions = []
       while index < rollout_rules.length
         holdout_decision, variation, skip_to_everyone_else, reasons_received = get_variation_from_delivery_rule(project_config, feature_flag_key, rollout_rules, index, user_context)
         decide_reasons.push(*reasons_received)
 
-        return DecisionResult.new(holdout_decision, false, decide_reasons) if holdout_decision
+        if holdout_decision
+          holdout_decisions << holdout_decision
+          index += 1
+          next
+        end
 
         if variation
           rule = rollout_rules[index]
           feature_decision = Decision.new(rule, variation, DECISION_SOURCES['ROLLOUT'], nil)
-          return DecisionResult.new(feature_decision, false, decide_reasons)
+          return DecisionResult.new(feature_decision, false, decide_reasons, holdout_decisions.first, holdout_decisions)
         end
 
         index = skip_to_everyone_else ? (rollout_rules.length - 1) : (index + 1)
       end
 
-      DecisionResult.new(nil, false, decide_reasons)
+      DecisionResult.new(nil, false, decide_reasons, holdout_decisions.first, holdout_decisions)
     end
 
     def get_variation_from_experiment_rule(project_config, flag_key, rule, user, user_profile_tracker, options = [])

spec/decision_service_holdout_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 #
-#    Copyright 2017-2020, 2023, Optimizely and contributors
+#    Copyright 2017-2020, 2023, 2026, Optimizely and contributors
 #
 #    Licensed under the Apache License, Version 2.0 (the "License");
 #    you may not use this file except in compliance with the License.
@@ -1386,4 +1386,189 @@
       end
     end
   end
+
+  describe 'local holdout hit continues rule evaluation' do
+    let(:spy_event_processor) { spy('event_processor') }
+    let(:holdout_source) { Optimizely::DecisionService::DECISION_SOURCES['HOLDOUT'] }
+    let(:chrome_user) { {'browser_type' => 'chrome'} }
+
+    def local_holdout(id, rule_ids)
+      {
+        'id' => id,
+        'key' => id,
+        'status' => 'Running',
+        'audiences' => [],
+        'includedRules' => rule_ids,
+        'variations' => [{'id' => "#{id}_var", 'key' => 'holdout', 'featureEnabled' => false}],
+        'trafficAllocation' => [{'entityId' => "#{id}_var", 'endOfRange' => 10_000}]
+      }
+    end
+
+    # Global holdouts in the shared fixture bucket every user, so they are removed here.
+    def datafile(local_holdouts: nil, send_flag_decisions: true)
+      body = OptimizelySpec.deep_clone(OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS)
+      body['holdouts'] = []
+      body['localHoldouts'] = local_holdouts if local_holdouts
+      body['sendFlagDecisions'] = send_flag_decisions
+      body
+    end
+
+    def build_project(body, user_profile_service: nil, cmab_service: nil)
+      project = Optimizely::Project.new(
+        datafile: JSON.dump(body),
+        logger: spy_logger,
+        error_handler: error_handler,
+        event_processor: spy_event_processor,
+        user_profile_service: user_profile_service,
+        cmab_service: cmab_service
+      )
+      allow(project).to receive(:send_impression).and_wrap_original do |original, *args|
+        @impressions << args
+        original.call(*args)
+      end
+      @projects << project
+      project
+    end
+
+    def holdout_impressions
+      @impressions.select { |args| args[6] == holdout_source }.map { |args| args[1]['id'] }
+    end
+
+    before do
+      @projects = []
+      @impressions = []
+    end
+    after { @projects.each(&:close) }
+
+    it 'serves a later delivery rule after a holdout hit without jumping to Everyone Else' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_a', ['177770'])]))
+      decision = project.create_user_context('user', chrome_user).decide('boolean_single_variable_feature')
+
+      expect(decision.rule_key).to eq('177772')
+      expect(decision.variation_key).to eq('177773')
+      expect(holdout_impressions).to eq(['lh_a'])
+    end
+
+    it 'sends one holdout event per held-out rule' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_a', ['177770']), local_holdout('lh_b', ['177772'])]))
+      decision = project.create_user_context('user', chrome_user).decide('boolean_single_variable_feature')
+
+      expect(decision.rule_key).to eq('177776')
+      expect(holdout_impressions).to eq(%w[lh_a lh_b])
+      expect(project).to have_received(:send_impression)
+        .with(anything, anything, '177778', 'boolean_single_variable_feature', '177776', true, 'rollout', 'user', anything, anything)
+    end
+
+    it 'sends holdout events when no rule serves' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_all', %w[177770 177772 177776])]))
+      decision = project.create_user_context('user', chrome_user).decide('boolean_single_variable_feature')
+
+      expect(decision.variation_key).to be_nil
+      expect(decision.enabled).to be false
+      expect(holdout_impressions).to eq(%w[lh_all lh_all lh_all])
+    end
+
+    it 'sends two holdout events when one local holdout targets two hit rules' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_two', %w[177770 177772])]))
+      decision = project.create_user_context('user', chrome_user).decide('boolean_single_variable_feature')
+
+      expect(decision.rule_key).to eq('177776')
+      expect(holdout_impressions).to eq(%w[lh_two lh_two])
+    end
+
+    it 'sends the holdout event for a delivery rule even when sendFlagDecisions is off' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_a', ['177770'])], send_flag_decisions: false))
+      decision = project.create_user_context('user', chrome_user).decide('boolean_single_variable_feature')
+
+      expect(decision.rule_key).to eq('177772')
+      expect(@impressions.length).to eq(1)
+      expect(holdout_impressions).to eq(['lh_a'])
+    end
+
+    it 'suppresses local holdout events with DISABLE_DECISION_EVENT' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_a', ['177770']), local_holdout('lh_b', ['177772'])]))
+      decision = project.create_user_context('user', chrome_user).decide(
+        'boolean_single_variable_feature',
+        [Optimizely::Decide::OptimizelyDecideOption::DISABLE_DECISION_EVENT]
+      )
+
+      expect(decision.rule_key).to eq('177776')
+      expect(@impressions).to be_empty
+    end
+
+    it 'serves the second experiment rule after being held out of the first' do
+      body = datafile
+      body['featureFlags'].find { |f| f['key'] == 'boolean_feature' }['experimentIds'] = %w[122227 111127]
+      project = build_project(body)
+
+      decision = project.create_user_context('user', {}).decide('boolean_feature')
+
+      expect(decision.rule_key).to eq('test_experiment')
+      expect(holdout_impressions).to eq(['holdout_local_1'])
+      expect(project).to have_received(:send_impression)
+        .with(anything, anything, decision.variation_key, 'boolean_feature', 'test_experiment', true, 'feature-test', 'user', anything, anything)
+
+      config = project.config_manager.config
+      result = Optimizely::DecisionService.new(spy_logger, spy_cmab_service).get_decision_for_flag(
+        config.feature_flag_key_map['boolean_feature'],
+        project.create_user_context('user', {}),
+        config
+      )
+      expect(result.decision.experiment['id']).to eq('111127')
+      expect(result.holdout_decisions.map { |d| d.experiment['id'] }).to eq(['holdout_local_1'])
+      expect(result.holdout_decision).to eq(result.holdout_decisions.first)
+    end
+
+    it 'discards earlier local holdout hits when a later rule ends in a decision error' do
+      body = datafile
+      body['featureFlags'].find { |f| f['key'] == 'boolean_feature' }['experimentIds'] = %w[122227 111127]
+      body['experiments'].find { |e| e['id'] == '111127' }['cmab'] = {'attributeIds' => [], 'trafficAllocation' => 10_000}
+      failing_cmab = double('cmab_service')
+      allow(failing_cmab).to receive(:get_decision).and_raise(StandardError, 'CMAB request failed')
+      project = build_project(body, cmab_service: failing_cmab)
+
+      decisions = project.create_user_context('user', chrome_user).decide_for_keys(%w[boolean_feature boolean_single_variable_feature])
+
+      config = project.config_manager.config
+      result = Optimizely::DecisionService.new(spy_logger, failing_cmab).get_decision_for_flag(
+        config.feature_flag_key_map['boolean_feature'],
+        project.create_user_context('user', chrome_user),
+        config
+      )
+      expect(result.error).to be true
+      expect(result.holdout_decisions.map { |d| d.experiment['id'] }).to eq(['holdout_local_1'])
+
+      expect(decisions['boolean_feature'].variation_key).to be_nil
+      expect(decisions['boolean_feature'].reasons).to include('Failed to fetch CMAB data for experiment test_experiment.')
+      expect(@impressions.map { |args| args[3] }).not_to include('boolean_feature')
+      expect(holdout_impressions).to be_empty
+      expect(decisions['boolean_single_variable_feature'].variation_key).not_to be_nil
+      expect(@impressions.map { |args| args[3] }).to include('boolean_single_variable_feature')
+    end
+
+    it 'applies the local holdout over a stored sticky variation' do
+      ups = double('user_profile_service', save: nil)
+      allow(ups).to receive(:lookup).with('user').and_return(
+        user_id: 'user',
+        experiment_bucket_map: {'122227' => {variation_id: '122228'}}
+      )
+      project = build_project(datafile, user_profile_service: ups)
+
+      decision = project.create_user_context('user', {}).decide('boolean_feature')
+
+      expect(decision.variation_key).not_to eq('control_with_audience')
+      expect(decision.rule_key).not_to eq('test_experiment_with_audience')
+      expect(decision.variation_key).to be_nil
+      expect(decision.rule_key).to be_nil
+      expect(decision.enabled).to be false
+      expect(holdout_impressions).to eq(['holdout_local_1'])
+    end
+
+    it 'skips a held-out rule without a holdout event in legacy APIs' do
+      project = build_project(datafile(local_holdouts: [local_holdout('lh_a', %w[177770 177772])]))
+
+      expect(project.is_feature_enabled('boolean_single_variable_feature', 'user', chrome_user)).to be true
+      expect(holdout_impressions).to eq([])
+    end
+  end
 end