Skip to content

Commit 3cfb10c

Browse files
Mat001claude
Mat001
and
claude
committed
[AI-FSSDK] [FSSDK-12369] Add local holdouts support with includedRules field
- Add global_holdouts and rule_holdouts_map to DatafileProjectConfig initialization - Classify holdouts as global (includedRules nil) or local (includedRules array) during config parsing - Add get_global_holdouts and get_holdouts_for_rule methods to DatafileProjectConfig - Update get_decision_for_flag to use get_global_holdouts instead of holdout_id_map.values - Add local holdout checks in get_variation_from_experiment_rule (after forced decisions) - Add local holdout checks in get_variation_from_delivery_rule (after forced decisions) - Add CONFIG_BODY_WITH_LOCAL_HOLDOUTS test fixture with mixed global and local holdouts - Add local_holdouts_spec.rb with 10 tests covering config classification and helper methods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c44cf9c commit 3cfb10c

4 files changed

Lines changed: 249 additions & 7 deletions

File tree

‎lib/optimizely/config/datafile_project_config.rb‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ class DatafileProjectConfig < ProjectConfig
3333
:group_id_map, :rollout_id_map, :rollout_experiment_id_map, :variation_id_map,
3434
:variation_id_to_variable_usage_map, :variation_key_map, :variation_id_map_by_experiment_id,
3535
:variation_key_map_by_experiment_id, :flag_variation_map, :integration_key_map, :integrations,
36-
:public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map
36+
:public_key_for_odp, :host_for_odp, :all_segments, :region, :holdouts, :holdout_id_map,
37+
:global_holdouts, :rule_holdouts_map
3738
# Boolean - denotes if Optimizely should remove the last block of visitors' IP address before storing event data
3839
attr_reader :anonymize_ip
3940

@@ -114,6 +115,8 @@ def initialize(datafile, logger, error_handler)
114115
@variation_id_to_experiment_map = {}
115116
@flag_variation_map = {}
116117
@holdout_id_map = {}
118+
@global_holdouts = []
119+
@rule_holdouts_map = {}
117120

118121
@holdouts.each do |holdout|
119122
next unless holdout['status'] == 'Running'
@@ -122,6 +125,19 @@ def initialize(datafile, logger, error_handler)
122125
holdout['layerId'] ||= ''
123126

124127
@holdout_id_map[holdout['id']] = holdout
128+
129+
# Classify holdout as global or local based on includedRules field.
130+
# If includedRules is nil (absent from datafile), holdout is global and applies to all rules.
131+
# If includedRules is an array (even empty), holdout is local and targets specific rule IDs.
132+
included_rules = holdout['includedRules']
133+
if included_rules.nil?
134+
@global_holdouts << holdout
135+
else
136+
included_rules.each do |rule_id|
137+
@rule_holdouts_map[rule_id] ||= []
138+
@rule_holdouts_map[rule_id] << holdout
139+
end
140+
end
125141
end
126142

127143
@experiment_id_map.each_value do |exp|
@@ -642,6 +658,26 @@ def get_holdout(holdout_id)
642658
nil
643659
end
644660

661+
def get_global_holdouts
662+
# Returns all global holdouts (those with includedRules == nil).
663+
# Global holdouts apply to all rules and are evaluated at the flag level.
664+
#
665+
# Returns Array of holdout hashes
666+
667+
@global_holdouts
668+
end
669+
670+
def get_holdouts_for_rule(rule_id)
671+
# Returns local holdouts targeting a specific rule.
672+
# Local holdouts are holdouts where includedRules is a non-nil array containing rule_id.
673+
#
674+
# rule_id - String ID of the rule to look up
675+
#
676+
# Returns Array of holdout hashes (empty array if none found)
677+
678+
@rule_holdouts_map[rule_id] || []
679+
end
680+
645681
private
646682

647683
def get_everyone_else_variation(feature_flag)

‎lib/optimizely/decision_service.rb‎

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,14 @@ def get_variation_for_feature(project_config, feature_flag, user_context, decide
169169
# user_context - Optimizely user context instance
170170
#
171171
# Returns DecisionResult struct.
172-
# Get running holdouts from the holdout_id_map (all holdouts are global now)
172+
# Get running holdouts from the holdout_id_map (global and local).
173+
# If any holdouts exist, use get_decision_for_flag which checks global holdouts
174+
# at flag level and local holdouts per-rule.
173175
running_holdouts = project_config.holdout_id_map.values
174176

175177
if running_holdouts && !running_holdouts.empty?
176-
# Has holdouts - use get_decision_for_flag which checks holdouts first
178+
# Has holdouts - use get_decision_for_flag which checks global holdouts first,
179+
# then experiments/rollouts which check local holdouts per-rule
177180
get_decision_for_flag(feature_flag, user_context, project_config, decide_options)
178181
else
179182
get_variations_for_feature_list(project_config, [feature_flag], user_context, decide_options).first
@@ -196,16 +199,17 @@ def get_decision_for_flag(feature_flag, user_context, project_config, decide_opt
196199
reasons = decide_reasons ? decide_reasons.dup : []
197200
user_id = user_context.user_id
198201

199-
# Check holdouts (all holdouts are global now - apply to all flags)
200-
holdouts = project_config.holdout_id_map.values
202+
# Check global holdouts first (those with includedRules == nil).
203+
# Global holdouts apply to all rules and are evaluated at the flag level.
204+
global_holdouts = project_config.get_global_holdouts
201205

202-
holdouts.each do |holdout|
206+
global_holdouts.each do |holdout|
203207
holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
204208
reasons.push(*holdout_decision.reasons)
205209

206210
next unless holdout_decision.decision
207211

208-
message = "The user '#{user_id}' is bucketed into holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'."
212+
message = "The user '#{user_id}' is bucketed into global holdout '#{holdout['key']}' for feature flag '#{feature_flag['key']}'."
209213
@logger.log(Logger::INFO, message)
210214
reasons.push(message)
211215
return DecisionResult.new(holdout_decision.decision, false, reasons)
@@ -446,6 +450,21 @@ def get_variation_from_experiment_rule(project_config, flag_key, rule, user, use
446450
reasons.push(*forced_reasons)
447451
return VariationResult.new(nil, false, reasons, variation['id']) if variation
448452

453+
# Check local holdouts targeting this specific experiment rule.
454+
# Local holdouts are checked after forced decisions but before regular bucketing.
455+
local_holdouts = project_config.get_holdouts_for_rule(rule['id'])
456+
local_holdouts.each do |holdout|
457+
holdout_decision = get_variation_for_holdout(holdout, user, project_config)
458+
reasons.push(*holdout_decision.reasons)
459+
if holdout_decision.decision
460+
message = "The user '#{user.user_id}' is bucketed into local holdout '#{holdout['key']}' for experiment rule '#{rule['key']}' in flag '#{flag_key}'."
461+
@logger.log(Logger::INFO, message)
462+
reasons.push(message)
463+
variation_id = holdout_decision.decision.variation['id']
464+
return VariationResult.new(nil, false, reasons, variation_id)
465+
end
466+
end
467+
449468
variation_result = get_variation(project_config, rule['id'], user, user_profile_tracker, options)
450469
variation_result.reasons = reasons + variation_result.reasons
451470
variation_result
@@ -470,6 +489,20 @@ def get_variation_from_delivery_rule(project_config, flag_key, rules, rule_index
470489

471490
return [variation, skip_to_everyone_else, reasons] if variation
472491

492+
# Check local holdouts targeting this specific delivery rule.
493+
# Local holdouts are checked after forced decisions but before audience/bucketing evaluation.
494+
local_holdouts = project_config.get_holdouts_for_rule(rule['id'])
495+
local_holdouts.each do |holdout|
496+
holdout_decision = get_variation_for_holdout(holdout, user_context, project_config)
497+
reasons.push(*holdout_decision.reasons)
498+
if holdout_decision.decision
499+
message = "The user '#{user_context.user_id}' is bucketed into local holdout '#{holdout['key']}' for delivery rule '#{rule['key']}' in flag '#{flag_key}'."
500+
@logger.log(Logger::INFO, message)
501+
reasons.push(message)
502+
return [holdout_decision.decision.variation, skip_to_everyone_else, reasons]
503+
end
504+
end
505+
473506
user_id = user_context.user_id
474507
attributes = user_context.user_attributes
475508
bucketing_id, bucketing_id_reasons = get_bucketing_id(user_id, attributes)

‎spec/local_holdouts_spec.rb‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# frozen_string_literal: true
2+
3+
#
4+
# Copyright 2026, Optimizely and contributors
5+
#
6+
# Licensed under the Apache License, Version 2.0 (the "License");
7+
# you may not use this file except in compliance with the License.
8+
# You may obtain a copy of the License at
9+
#
10+
# http://www.apache.org/licenses/LICENSE-2.0
11+
#
12+
# Unless required by applicable law or agreed to in writing, software
13+
# distributed under the License is distributed on an "AS IS" BASIS,
14+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
# See the License for the specific language governing permissions and
16+
# limitations under the License.
17+
#
18+
require 'spec_helper'
19+
require 'optimizely/config/datafile_project_config'
20+
require 'optimizely/decision_service'
21+
require 'optimizely/error_handler'
22+
require 'optimizely/logger'
23+
24+
describe 'Local Holdouts' do
25+
let(:error_handler) { Optimizely::NoOpErrorHandler.new }
26+
let(:spy_logger) { spy('logger') }
27+
28+
describe 'DatafileProjectConfig local holdout classification' do
29+
let(:config_body_json) { OptimizelySpec::CONFIG_BODY_WITH_HOLDOUTS_JSON }
30+
let(:config) { Optimizely::DatafileProjectConfig.new(config_body_json, spy_logger, error_handler) }
31+
32+
context 'when holdouts have no includedRules field (old datafile format)' do
33+
it 'treats holdouts as global (backward compatibility)' do
34+
# All holdouts in CONFIG_BODY_WITH_HOLDOUTS have no includedRules = nil = global
35+
running_holdouts = config.holdout_id_map.values
36+
global_holdouts = config.get_global_holdouts
37+
38+
# Running holdouts: holdout_1, holdout_boolean_feature, holdout_empty_1, holdout_2 (holdout_3 is Inactive)
39+
expect(global_holdouts.length).to eq(running_holdouts.length)
40+
expect(config.rule_holdouts_map).to be_empty
41+
end
42+
end
43+
44+
context 'with local holdouts config' do
45+
let(:local_config_json) { OptimizelySpec::CONFIG_BODY_WITH_LOCAL_HOLDOUTS_JSON }
46+
let(:local_config) { Optimizely::DatafileProjectConfig.new(local_config_json, spy_logger, error_handler) }
47+
48+
it 'classifies holdout with nil includedRules as global' do
49+
global_holdouts = local_config.get_global_holdouts
50+
expect(global_holdouts.length).to eq(1)
51+
expect(global_holdouts.first['id']).to eq('global_holdout_1')
52+
end
53+
54+
it 'classifies holdout with non-nil includedRules as local' do
55+
global_holdouts = local_config.get_global_holdouts
56+
# Only global_holdout_1 should be in global list, not local_holdout_1
57+
expect(global_holdouts.none? { |h| h['id'] == 'local_holdout_1' }).to be(true)
58+
end
59+
60+
it 'populates rule_holdouts_map for local holdouts' do
61+
rule_holdouts_map = local_config.rule_holdouts_map
62+
expect(rule_holdouts_map).to have_key('111127')
63+
expect(rule_holdouts_map['111127'].length).to eq(1)
64+
expect(rule_holdouts_map['111127'].first['id']).to eq('local_holdout_1')
65+
end
66+
67+
it 'get_holdouts_for_rule returns local holdouts for a rule' do
68+
holdouts = local_config.get_holdouts_for_rule('111127')
69+
expect(holdouts.length).to eq(1)
70+
expect(holdouts.first['key']).to eq('local_holdout_exp')
71+
end
72+
73+
it 'get_holdouts_for_rule returns empty array for unknown rule' do
74+
holdouts = local_config.get_holdouts_for_rule('unknown_rule_id')
75+
expect(holdouts).to be_empty
76+
end
77+
78+
it 'get_global_holdouts returns empty array when no global holdouts' do
79+
# Build a config with only a local holdout
80+
only_local = OptimizelySpec::VALID_CONFIG_BODY.merge(
81+
'holdouts' => [
82+
{
83+
'id' => 'local_only',
84+
'key' => 'local_only_holdout',
85+
'status' => 'Running',
86+
'audiences' => [],
87+
'audienceIds' => [],
88+
'audienceConditions' => [],
89+
'includedRules' => ['some_rule'],
90+
'variations' => [{ 'id' => 'v1', 'key' => 'off', 'featureEnabled' => false }],
91+
'trafficAllocation' => [{ 'entityId' => 'v1', 'endOfRange' => 10_000 }]
92+
}
93+
]
94+
)
95+
only_local_config = Optimizely::DatafileProjectConfig.new(JSON.dump(only_local), spy_logger, error_handler)
96+
97+
expect(only_local_config.get_global_holdouts).to be_empty
98+
expect(only_local_config.rule_holdouts_map['some_rule'].length).to eq(1)
99+
end
100+
end
101+
end
102+
103+
describe 'is_global? semantics via includedRules' do
104+
it 'treats nil includedRules as global (no field in datafile)' do
105+
holdout = { 'id' => 'h1', 'key' => 'global', 'includedRules' => nil }
106+
expect(holdout['includedRules'].nil?).to be(true)
107+
end
108+
109+
it 'treats non-nil includedRules as local even if empty' do
110+
holdout = { 'id' => 'h2', 'key' => 'local_empty', 'includedRules' => [] }
111+
expect(holdout['includedRules'].nil?).to be(false)
112+
end
113+
114+
it 'treats non-nil includedRules with values as local' do
115+
holdout = { 'id' => 'h3', 'key' => 'local', 'includedRules' => ['rule1'] }
116+
expect(holdout['includedRules'].nil?).to be(false)
117+
end
118+
end
119+
end

‎spec/spec_params.rb‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2041,6 +2041,60 @@ module OptimizelySpec
20412041

20422042
CONFIG_BODY_WITH_HOLDOUTS_JSON = JSON.dump(CONFIG_BODY_WITH_HOLDOUTS).freeze
20432043

2044+
# Config with both global and local holdouts for testing local holdouts feature
2045+
CONFIG_BODY_WITH_LOCAL_HOLDOUTS = VALID_CONFIG_BODY.merge(
2046+
{
2047+
'holdouts' => [
2048+
{
2049+
'id' => 'global_holdout_1',
2050+
'key' => 'global_holdout',
2051+
'status' => 'Running',
2052+
'audiences' => [],
2053+
'audienceIds' => [],
2054+
'audienceConditions' => [],
2055+
'variations' => [
2056+
{
2057+
'id' => 'global_var_1',
2058+
'key' => 'holdout_variation',
2059+
'featureEnabled' => true
2060+
}
2061+
],
2062+
'trafficAllocation' => [
2063+
{
2064+
'entityId' => 'global_var_1',
2065+
'endOfRange' => 10_000
2066+
}
2067+
]
2068+
# No includedRules key => nil => global holdout
2069+
},
2070+
{
2071+
'id' => 'local_holdout_1',
2072+
'key' => 'local_holdout_exp',
2073+
'status' => 'Running',
2074+
'audiences' => [],
2075+
'audienceIds' => [],
2076+
'audienceConditions' => [],
2077+
'includedRules' => ['111127'], # targets experiment rule 111127
2078+
'variations' => [
2079+
{
2080+
'id' => 'local_var_1',
2081+
'key' => 'local_holdout_variation',
2082+
'featureEnabled' => true
2083+
}
2084+
],
2085+
'trafficAllocation' => [
2086+
{
2087+
'entityId' => 'local_var_1',
2088+
'endOfRange' => 10_000
2089+
}
2090+
]
2091+
}
2092+
]
2093+
}
2094+
).freeze
2095+
2096+
CONFIG_BODY_WITH_LOCAL_HOLDOUTS_JSON = JSON.dump(CONFIG_BODY_WITH_LOCAL_HOLDOUTS).freeze
2097+
20442098
def self.deep_clone(obj)
20452099
obj.dup.tap do |new_obj|
20462100
case new_obj

0 commit comments

Comments
 (0)