Skip to content

Commit 0f71efc

Browse files
[FSSDK-12368] Restore holdout evaluation in decide flow, remove only flag-level include/exclude fields
The previous cleanup removed the entire holdout evaluation from the decide flow. This restores holdout checking while keeping the removal of legacy includedFlags/excludedFlags fields — all running holdouts now apply to all flags uniformly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 77d0cd6 commit 0f71efc

3 files changed

Lines changed: 44 additions & 9 deletions

File tree

optimizely/decision_service.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,30 @@ def get_decision_for_flag(
733733
reasons = decide_reasons.copy() if decide_reasons else []
734734
user_id = user_context.user_id
735735

736-
# Check experiments then rollouts
736+
# Check holdouts
737+
holdouts = project_config.get_holdouts_for_flag(feature_flag.key)
738+
for holdout in holdouts:
739+
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
740+
reasons.extend(holdout_decision['reasons'])
741+
742+
decision = holdout_decision['decision']
743+
# Check if user was bucketed into holdout (has a variation)
744+
if decision.variation is None:
745+
continue
746+
747+
message = (
748+
f"The user '{user_id}' is bucketed into holdout '{holdout.key}' "
749+
f"for feature flag '{feature_flag.key}'."
750+
)
751+
self.logger.info(message)
752+
reasons.append(message)
753+
return {
754+
'decision': holdout_decision['decision'],
755+
'error': False,
756+
'reasons': reasons
757+
}
758+
759+
# If no holdout decision, check experiments then rollouts
737760
if feature_flag.experimentIds:
738761
for experiment_id in feature_flag.experimentIds:
739762
experiment = project_config.get_experiment_from_id(experiment_id)

optimizely/project_config.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
9393
holdouts_data: list[types.HoldoutDict] = config.get('holdouts', [])
9494
self.holdouts: list[entities.Holdout] = []
9595
self.holdout_id_map: dict[str, entities.Holdout] = {}
96+
self.flag_holdouts_map: dict[str, list[entities.Holdout]] = {}
9697

9798
# Convert holdout dicts to Holdout entities
9899
for holdout_data in holdouts_data:
@@ -239,6 +240,11 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any):
239240
everyone_else_variation.variables, 'id', entities.Variation.VariableUsage
240241
)
241242

243+
# Map all running holdouts to this flag
244+
applicable_holdouts = list(self.holdout_id_map.values())
245+
if applicable_holdouts:
246+
self.flag_holdouts_map[feature.key] = applicable_holdouts
247+
242248
rollout = None if len(feature.rolloutId) == 0 else self.rollout_id_map[feature.rolloutId]
243249
if rollout:
244250
for exp in rollout.experiments:
@@ -872,6 +878,20 @@ def get_flag_variation(
872878

873879
return None
874880

881+
def get_holdouts_for_flag(self, flag_key: str) -> list[entities.Holdout]:
882+
""" Helper method to get holdouts from an applied feature flag.
883+
884+
Args:
885+
flag_key: Key of the feature flag.
886+
887+
Returns:
888+
The holdouts that apply for a specific flag as Holdout entity objects.
889+
"""
890+
if not self.holdouts:
891+
return []
892+
893+
return self.flag_holdouts_map.get(flag_key, [])
894+
875895
def get_holdout(self, holdout_id: str) -> Optional[entities.Holdout]:
876896
""" Helper method to get holdout from holdout ID.
877897

tests/test_decision_service_holdout.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -863,9 +863,6 @@ def test_decide_all_with_global_holdout(self):
863863

864864
def test_decide_all_with_included_flags(self):
865865
"""Should apply holdout only to included flags in decide_all."""
866-
# Get feature flag IDs
867-
feature1_id = '91111' # test_feature_in_experiment
868-
869866
config_dict_with_holdouts = self.config_dict_with_features.copy()
870867
config_dict_with_holdouts['holdouts'] = [
871868
{
@@ -907,8 +904,6 @@ def test_decide_all_with_included_flags(self):
907904

908905
def test_decide_all_with_excluded_flags(self):
909906
"""Should exclude holdout from excluded flags in decide_all."""
910-
feature1_id = '91111' # test_feature_in_experiment
911-
912907
config_dict_with_holdouts = self.config_dict_with_features.copy()
913908
config_dict_with_holdouts['holdouts'] = [
914909
{
@@ -949,9 +944,6 @@ def test_decide_all_with_excluded_flags(self):
949944

950945
def test_decide_all_with_multiple_holdouts(self):
951946
"""Should handle multiple holdouts with correct priority."""
952-
feature1_id = '91111'
953-
feature2_id = '91112'
954-
955947
config_dict_with_holdouts = self.config_dict_with_features.copy()
956948
config_dict_with_holdouts['holdouts'] = [
957949
# Global holdout (applies to all)

0 commit comments

Comments
 (0)