[AI-FSSDK] [FSSDK-12369] Add local holdouts support with includedRules field (#398)

* [AI-FSSDK] [FSSDK-12369] Add local holdouts support with includedRules field

* Fix Rubocop offenses in local holdouts spec files

- Fix Layout/SpaceInsideHashLiteralBraces in datafile_project_config_spec.rb
- Fix Lint/UselessAssignment by removing unused feature_flag variable in decision_service_holdout_spec.rb

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* [AI-FSSDK] [FSSDK-12369] Fix rubocop offenses: rename get_global_holdouts, use next pattern

* [FSSDK-12369] Add mandatory enforcement test: forced decision beats 100% local holdout

* Empty commit to retrigger FSC

* Empty commit to trigger FSC

* Fix unit test issue

* Empty commit to trigger FSC

* Fix decision holdout test failures

* Update comment for holdout decision logic

Corrected comment to reflect global holdout decision handling.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: esrakartalOpt <esra.kartal@optimizely.com>
Co-authored-by: esrakartalOpt <102107327+esrakartalOpt@users.noreply.github.com>

Author: 3 people

lib/optimizely/config/datafile_project_config.rb

@@ -33,7 +33,8 @@ class DatafileProjectConfig < ProjectConfig
                 :group_id_map, :rollout_id_map, :rollout_experiment_id_map, :variation_id_map,
                 :variation_id_to_variable_usage_map, :variation_key_map, :variation_id_map_by_experiment_id,
                 :variation_key_map_by_experiment_id, :flag_variation_map, :integration_key_map, :integrations,
-                :public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map
+                :public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map,
+                :global_holdouts, :rule_holdouts_map
     # Boolean - denotes if Optimizely should remove the last block of visitors' IP address before storing event data
     attr_reader :anonymize_ip
 
@@ -114,6 +115,8 @@ def initialize(datafile, logger, error_handler)
       @variation_id_to_experiment_map = {}
       @flag_variation_map = {}
       @holdout_id_map = {}
+      @global_holdouts = []
+      @rule_holdouts_map = {}
 
       @holdouts.each do |holdout|
         next unless holdout['status'] == 'Running'
@@ -122,6 +125,19 @@ def initialize(datafile, logger, error_handler)
         holdout['layerId'] ||= ''
 
         @holdout_id_map[holdout['id']] = holdout
+
+        # Build global vs local holdout mappings
+        # A holdout is global when includedRules is nil/absent (applies to all rules)
+        # A holdout is local when includedRules is a non-nil array (applies only to specified rules)
+        if holdout_global?(holdout)
+          @global_holdouts << holdout
+        else
+          included_rules = holdout['includedRules'] || []
+          included_rules.each do |rule_id|
+            @rule_holdouts_map[rule_id] ||= []
+            @rule_holdouts_map[rule_id] << holdout
+          end
+        end
       end
 
       @experiment_id_map.each_value do |exp|
@@ -642,6 +658,27 @@ def get_holdout(holdout_id)
       nil
     end
 
+    def get_holdouts_for_rule(rule_id)
+      # Returns running local holdouts that target a specific rule ID.
+      # Local holdouts apply only to the rules listed in their includedRules array.
+      #
+      # rule_id - String ID of the experiment/delivery rule
+      #
+      # Returns Array of holdout hashes targeting the rule (empty array if none)
+      @rule_holdouts_map[rule_id] || []
+    end
+
+    def holdout_global?(holdout)
+      # Determines whether a holdout is global (applies to all rules) or local (applies to specific rules).
+      # A holdout is global when includedRules is nil or absent from the datafile.
+      # A holdout with an empty array [] is a local holdout with no matching rules (NOT global).
+      #
+      # holdout - Holdout hash from the datafile
+      #
+      # Returns true if the holdout is global, false if local
+      !holdout.key?('includedRules') || holdout['includedRules'].nil?
+    end
+
     private
 
     def get_everyone_else_variation(feature_flag)

lib/optimizely/decision_service.rb

@@ -40,7 +40,7 @@ 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)
+    VariationResult = Struct.new(:cmab_uuid, :error, :reasons, :variation_id, :holdout_decision)
     DecisionResult = Struct.new(:decision, :error, :reasons)
 
     DECISION_SOURCES = {
@@ -169,7 +169,7 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide
       # user_context - Optimizely user context instance
       #
       # Returns DecisionResult struct.
-      # Get running holdouts from the holdout_id_map (all holdouts are global now)
+      # Check for any running holdouts (global or local)
       running_holdouts = project_config.holdout_id_map.values
 
       if running_holdouts && !running_holdouts.empty?
@@ -196,8 +196,8 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
       reasons = decide_reasons ? decide_reasons.dup : []
       user_id = user_context.user_id
 
-      # Check holdouts (all holdouts are global now - apply to all flags)
-      holdouts = project_config.holdout_id_map.values
+      # Check global holdouts first (flag level) — these apply to all rules across all flags
+      holdouts = project_config.global_holdouts
 
       holdouts.each do |holdout|
         holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
@@ -365,6 +365,9 @@ def get_variation_for_feature_experiment(project_config, feature_flag, user_cont
         # If there's an error, return immediately instead of falling back to next experiment
         return DecisionResult.new(nil, error, decide_reasons) if error
 
+        # If a global holdout decision was made, return it directly
+        return DecisionResult.new(variation_result.holdout_decision, false, decide_reasons) if variation_result.holdout_decision
+
         next unless variation_id
 
         variation = project_config.get_variation_from_id_by_experiment_id(experiment_id, variation_id)
@@ -415,8 +418,11 @@ def get_variation_for_feature_rollout(project_config, feature_flag, user_context
       index = 0
       rollout_rules = rollout['experiments']
       while index < rollout_rules.length
-        variation, skip_to_everyone_else, reasons_received = get_variation_from_delivery_rule(project_config, feature_flag_key, rollout_rules, index, user_context)
+        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 variation
           rule = rollout_rules[index]
           feature_decision = Decision.new(rule, variation, DECISION_SOURCES['ROLLOUT'], nil)
@@ -441,11 +447,24 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use
       # Returns variation_id and reasons
       reasons = []
 
+      # Step 1: Forced decision check
       context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(flag_key, rule['key'])
       variation, forced_reasons = validated_forced_decision(project_config, context, user)
       reasons.push(*forced_reasons)
       return VariationResult.new(nil, false, reasons, variation['id']) if variation
 
+      # Step 2: Local holdout check
+      local_holdouts = project_config.get_holdouts_for_rule(rule['id'])
+      local_holdouts.each do |holdout|
+        holdout_decision = get_variation_for_holdout(holdout, user, project_config)
+        reasons.push(*holdout_decision.reasons)
+        next unless holdout_decision.decision
+
+        holdout_variation = holdout_decision.decision.variation
+        return VariationResult.new(nil, false, reasons, holdout_variation['id'], holdout_decision.decision)
+      end
+
+      # Step 3: Regular rule evaluation
       variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options)
       variation_result.reasons = reasons + variation_result.reasons
       variation_result
@@ -460,16 +479,26 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index
       # rule - An experiment rule key
       # user_context - Optimizely user context instance
       #
-      # Returns variation, boolean to skip for eveyone else rule and reasons
+      # Returns [holdout_decision, variation, skip_to_everyone_else, reasons]
       reasons = []
       skip_to_everyone_else = false
       rule = rules[rule_index]
+
+      # Step 1: Forced decision check
       context = Optimizely::OptimizelyUserContext::OptimizelyDecisionContext.new(flag_key, rule['key'])
       variation, forced_reasons = validated_forced_decision(project_config, context, user_context)
       reasons.push(*forced_reasons)
+      return [nil, variation, skip_to_everyone_else, reasons] if variation
 
-      return [variation, skip_to_everyone_else, reasons] if variation
+      # Step 2: Local holdout check
+      local_holdouts = project_config.get_holdouts_for_rule(rule['id'])
+      local_holdouts.each do |holdout|
+        holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
+        reasons.push(*holdout_decision.reasons)
+        return [holdout_decision.decision, nil, skip_to_everyone_else, reasons] if holdout_decision.decision
+      end
 
+      # Step 3: Regular rule evaluation
       user_id = user_context.user_id
       attributes = user_context.user_attributes
       bucketing_id, bucketing_id_reasons = get_bucketing_id(user_id, attributes)
@@ -485,7 +514,7 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index
         message = "User '#{user_id}' does not meet the conditions for targeting rule '#{logging_key}'."
         @logger.log(Logger::DEBUG, message)
         reasons.push(message)
-        return [nil, skip_to_everyone_else, reasons]
+        return [nil, nil, skip_to_everyone_else, reasons]
       end
 
       message = "User '#{user_id}' meets the audience conditions for targeting rule '#{logging_key}'."
@@ -505,7 +534,7 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index
         reasons.push(message)
         skip_to_everyone_else = true
       end
-      [bucket_variation, skip_to_everyone_else, reasons]
+      [nil, bucket_variation, skip_to_everyone_else, reasons]
     end
 
     def set_forced_variation(project_config, experiment_key, user_id, variation_key)

lib/optimizely/helpers/constants.rb

@@ -346,6 +346,9 @@ module Constants
                 },
                 'status' => {
                   'type' => 'string'
+                },
+                'includedRules' => {
+                  'type' => %w[array null]
                 }
               }
             }

spec/config/datafile_project_config_spec.rb

@@ -1935,4 +1935,163 @@ def build_datafile(experiments: [], rollouts: [], feature_flags: [])
       expect(experiment['trafficAllocation'].length).to eq(1)
     end
   end
+
+  # Level 1 — Local Holdouts config/parsing tests (FSSDK-12369)
+  describe 'local holdouts data model and config parsing' do
+    let(:config_with_local_holdouts) do
+      Optimizely::DatafileProjectConfig.new(
+        OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON,
+        logger,
+        error_handler
+      )
+    end
+
+    describe '#holdout_global?' do
+      it 'returns true for holdout with no includedRules key (old datafile format)' do
+        global_holdout = config_with_local_holdouts.get_holdout('holdout_1')
+        expect(global_holdout).not_to be_nil
+        expect(global_holdout.key?('includedRules')).to be false
+        expect(config_with_local_holdouts.holdout_global?(global_holdout)).to be true
+      end
+
+      it 'returns true for holdout with explicit nil includedRules' do
+        holdout = {'id' => 'test', 'key' => 'test', 'includedRules' => nil}
+        expect(config_with_local_holdouts.holdout_global?(holdout)).to be true
+      end
+
+      it 'returns false for holdout with non-nil includedRules array (local holdout)' do
+        local_holdout = config_with_local_holdouts.get_holdout('holdout_local_1')
+        expect(local_holdout).not_to be_nil
+        expect(config_with_local_holdouts.holdout_global?(local_holdout)).to be false
+      end
+
+      it 'returns false for holdout with empty includedRules array (local holdout with no matching rules)' do
+        local_holdout_empty = config_with_local_holdouts.get_holdout('holdout_local_empty_rules')
+        expect(local_holdout_empty).not_to be_nil
+        expect(local_holdout_empty['includedRules']).to eq([])
+        # Empty array is local holdout (not global) — DIFFERENT from nil
+        expect(config_with_local_holdouts.holdout_global?(local_holdout_empty)).to be false
+      end
+    end
+
+    describe '#global_holdouts' do
+      it 'returns only holdouts without includedRules (global holdouts)' do
+        global_holdouts = config_with_local_holdouts.global_holdouts
+
+        expect(global_holdouts).not_to be_nil
+        expect(global_holdouts).to be_an(Array)
+
+        # All returned holdouts must be global
+        global_holdouts.each do |holdout|
+          expect(config_with_local_holdouts.holdout_global?(holdout)).to be true
+        end
+      end
+
+      it 'does not include local holdouts (those with includedRules array)' do
+        global_holdouts = config_with_local_holdouts.global_holdouts
+        local_holdout = config_with_local_holdouts.get_holdout('holdout_local_1')
+
+        expect(global_holdouts).not_to include(local_holdout)
+      end
+
+      it 'does not include holdouts with empty includedRules array' do
+        global_holdouts = config_with_local_holdouts.global_holdouts
+        empty_local_holdout = config_with_local_holdouts.get_holdout('holdout_local_empty_rules')
+
+        # Empty [] is local, not global — must not appear in global_holdouts
+        expect(global_holdouts).not_to include(empty_local_holdout)
+      end
+
+      it 'returns empty array when no global holdouts are present' do
+        config_no_global = Optimizely::DatafileProjectConfig.new(
+          JSON.dump(
+            OptimizelySpec::VALID_CONFIG_BODY.merge(
+              'holdouts' => [
+                {
+                  'id' => 'only_local',
+                  'key' => 'only_local_holdout',
+                  'status' => 'Running',
+                  'audiences' => [],
+                  'includedRules' => ['some_rule_id'],
+                  'variations' => [{'id' => 'v1', 'key' => 'holdout', 'featureEnabled' => false}],
+                  'trafficAllocation' => [{'entityId' => 'v1', 'endOfRange' => 10_000}]
+                }
+              ]
+            )
+          ),
+          logger,
+          error_handler
+        )
+        expect(config_no_global.global_holdouts).to eq([])
+      end
+    end
+
+    describe '#get_holdouts_for_rule' do
+      it 'returns local holdouts targeting the specified rule ID' do
+        rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('122227')
+        local_holdout = config_with_local_holdouts.get_holdout('holdout_local_1')
+
+        expect(rule_holdouts).to include(local_holdout)
+      end
+
+      it 'returns empty array for a rule ID with no targeting holdouts' do
+        rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('unknown_rule_id_xyz')
+        expect(rule_holdouts).to eq([])
+      end
+
+      it 'does not return global holdouts (those without includedRules)' do
+        rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('122227')
+        global_holdout = config_with_local_holdouts.get_holdout('holdout_1')
+
+        # Global holdout must not appear in rule-specific list
+        expect(rule_holdouts).not_to include(global_holdout)
+      end
+
+      it 'does not return holdouts targeting other rules' do
+        # holdout_local_2 targets rule 122238, not 122227
+        holdouts_for_rule_a = config_with_local_holdouts.get_holdouts_for_rule('122227')
+        local_holdout_2 = config_with_local_holdouts.get_holdout('holdout_local_2')
+
+        expect(holdouts_for_rule_a).not_to include(local_holdout_2)
+      end
+
+      it 'returns empty array for holdout with empty includedRules — does not match any rule' do
+        # holdout_local_empty_rules has includedRules: [] — should not appear for any rule
+        rule_holdouts = config_with_local_holdouts.get_holdouts_for_rule('122227')
+        empty_rules_holdout = config_with_local_holdouts.get_holdout('holdout_local_empty_rules')
+
+        expect(rule_holdouts).not_to include(empty_rules_holdout)
+      end
+    end
+
+    describe 'backward compatibility — old datafiles without includedRules' do
+      it 'parses holdouts without includedRules field as global (backward compatible)' do
+        # Use the config with only global holdouts (no includedRules key)
+        config_global_only = Optimizely::DatafileProjectConfig.new(
+          OptimizelySpec::CONFIG_BODY_WITH_GLOBAL_HOLDOUTS_ONLY_JSON,
+          logger,
+          error_handler
+        )
+
+        holdout = config_global_only.get_holdout('global_holdout_only_1')
+        expect(holdout).not_to be_nil
+        expect(holdout.key?('includedRules')).to be false
+
+        # Must be classified as global
+        expect(config_global_only.holdout_global?(holdout)).to be true
+        expect(config_global_only.global_holdouts).to include(holdout)
+        expect(config_global_only.get_holdouts_for_rule('any_rule')).to eq([])
+      end
+
+      it 'does not raise errors when processing old datafiles' do
+        expect do
+          Optimizely::DatafileProjectConfig.new(
+            OptimizelySpec::CONFIG_BODY_WITH_GLOBAL_HOLDOUTS_ONLY_JSON,
+            logger,
+            error_handler
+          )
+        end.not_to raise_error
+      end
+    end
+  end
 end