Skip to content

Commit 1608870

Browse files
esrakartalOptFarhanAnjum-opticlaude
authored
[AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules (#522)
* [AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules * [AI-FSSDK] [FSSDK-12735] Add holdout event dispatch, local holdout flag isolation, and TD null passthrough * [AI-FSSDK] [FSSDK-12735] Fix holdout impression event tracking and add bypass decision reason * [FSSDK-12735] Re-trigger FSC tests after datafile updates * Fix message * [FSSDK-12735] Trigger FSC tests * Fix mypy no-redef error for result variable in decision_service Remove duplicate type annotation on second assignment to `result`, which was already typed on line 805. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Farhan Anjum <Farhan.Anjum@optimizely.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3eaa6cb commit 1608870

5 files changed

Lines changed: 489 additions & 22 deletions

File tree

‎optimizely/decision_service.py‎

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,11 @@ class VariationResult(TypedDict):
6464
variation: Optional[Union[entities.Variation, VariationDict]]
6565

6666

67-
class DecisionResult(TypedDict):
68-
"""
69-
A TypedDict representing the result of a decision process.
67+
class _DecisionResultOptional(TypedDict, total=False):
68+
holdout_decision: Decision
7069

71-
Attributes:
72-
decision (Decision): The decision object containing the outcome of the evaluation.
73-
error (bool): Indicates whether an error occurred during the decision process.
74-
reasons (List[str]): A list of reasons explaining the decision or any errors encountered.
75-
"""
70+
71+
class DecisionResult(_DecisionResultOptional):
7672
decision: Decision
7773
error: bool
7874
reasons: List[str]
@@ -610,7 +606,6 @@ def get_variation_for_rollout(
610606
return Decision(experiment=rule, variation=forced_decision_variation,
611607
source=enums.DecisionSources.ROLLOUT, cmab_uuid=None), decide_reasons
612608

613-
# Check local holdouts targeting this specific delivery rule (FSSDK-12369)
614609
local_holdouts = project_config.get_holdouts_for_rule(rule.id)
615610
for holdout in local_holdouts:
616611
local_holdout_decision = self.get_variation_for_holdout(
@@ -751,14 +746,16 @@ def get_decision_for_flag(
751746
reasons = decide_reasons.copy() if decide_reasons else []
752747
user_id = user_context.user_id
753748

749+
global_holdout_result: DecisionResult | None = None
750+
global_holdout_key: str | None = None
751+
754752
# Check global holdouts (flag level — before any rules are evaluated)
755753
global_holdouts = project_config.get_global_holdouts()
756754
for holdout in global_holdouts:
757755
holdout_decision = self.get_variation_for_holdout(holdout, user_context, project_config)
758756
reasons.extend(holdout_decision['reasons'])
759757

760758
decision = holdout_decision['decision']
761-
# Check if user was bucketed into holdout (has a variation)
762759
if decision.variation is None:
763760
continue
764761

@@ -768,18 +765,33 @@ def get_decision_for_flag(
768765
)
769766
self.logger.info(message)
770767
reasons.append(message)
771-
return {
772-
'decision': holdout_decision['decision'],
773-
'error': False,
774-
'reasons': reasons
775-
}
776768

777-
# If no global holdout decision, check experiments then rollouts
769+
if not holdout.exclude_targeted_deliveries:
770+
return {
771+
'decision': holdout_decision['decision'],
772+
'error': False,
773+
'reasons': reasons
774+
}
775+
776+
message = (
777+
f"Holdout '{holdout.key}' excludes targeted deliveries. "
778+
f"Targeted delivery rules will be evaluated."
779+
)
780+
self.logger.info(message)
781+
reasons.append(message)
782+
global_holdout_result = holdout_decision
783+
global_holdout_key = holdout.key
784+
break
785+
786+
# Check experiments then rollouts
778787
if feature_flag.experimentIds:
779788
for experiment_id in feature_flag.experimentIds:
780789
experiment = project_config.get_experiment_from_id(experiment_id)
781790

782791
if experiment:
792+
if global_holdout_result is not None and experiment.type != enums.ExperimentTypes.td:
793+
continue
794+
783795
# Check for forced decision
784796
optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(
785797
feature_flag.key, experiment.key)
@@ -790,13 +802,15 @@ def get_decision_for_flag(
790802
if forced_decision_variation:
791803
decision = Decision(experiment, forced_decision_variation,
792804
enums.DecisionSources.FEATURE_TEST, None)
793-
return {
805+
result: DecisionResult = {
794806
'decision': decision,
795807
'error': False,
796808
'reasons': reasons
797809
}
810+
if global_holdout_result is not None:
811+
result['holdout_decision'] = global_holdout_result['decision']
812+
return result
798813

799-
# Check local holdouts targeting this specific experiment rule (FSSDK-12369)
800814
local_holdouts = project_config.get_holdouts_for_rule(experiment.id)
801815
for holdout in local_holdouts:
802816
local_holdout_decision = self.get_variation_for_holdout(
@@ -837,11 +851,22 @@ def get_decision_for_flag(
837851
decision = Decision(experiment, variation_result['variation'],
838852
enums.DecisionSources.FEATURE_TEST,
839853
variation_result['cmab_uuid'])
840-
return {
854+
result = {
841855
'decision': decision,
842856
'error': False,
843857
'reasons': reasons
844858
}
859+
if global_holdout_result is not None:
860+
result['holdout_decision'] = global_holdout_result['decision']
861+
return result
862+
863+
if global_holdout_result is not None:
864+
message = (
865+
f"Holdout \"{global_holdout_key}\" has excludeTargetedDeliveries enabled, "
866+
f"continuing to rollout evaluation."
867+
)
868+
self.logger.info(message)
869+
reasons.append(message)
845870

846871
# If no experiment decision, check rollouts
847872
rollout_decision, rollout_reasons = self.get_variation_for_rollout(
@@ -863,11 +888,14 @@ def get_decision_for_flag(
863888
else:
864889
self.logger.debug(f'User "{user_id}" not bucketed into any rollout for feature "{feature_flag.key}".')
865890

866-
return {
891+
final_result: DecisionResult = {
867892
'decision': rollout_decision,
868893
'error': False,
869894
'reasons': reasons
870895
}
896+
if global_holdout_result is not None:
897+
final_result['holdout_decision'] = global_holdout_result['decision']
898+
return final_result
871899

872900
def get_variation_for_holdout(
873901
self,

‎optimizely/entities.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ def __init__(
224224
audienceIds: list[str],
225225
audienceConditions: Optional[Sequence[str | list[str]]] = None,
226226
includedRules: Optional[list[str]] = None,
227+
excludeTargetedDeliveries: bool = False,
227228
**kwargs: Any
228229
):
229230
self.id = id
@@ -236,6 +237,7 @@ def __init__(
236237
# Per-rule targeting for local holdouts. Scope comes from the datafile
237238
# section, not this field; ProjectConfig strips it on 'holdouts' entries.
238239
self.included_rules: Optional[list[str]] = includedRules
240+
self.exclude_targeted_deliveries: bool = excludeTargetedDeliveries
239241

240242
def get_audience_conditions_or_ids(self) -> Sequence[str | list[str]]:
241243
"""Returns audienceConditions if present, otherwise audienceIds.

‎optimizely/optimizely.py‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,8 @@ def _create_optimizely_decision(
12451245
flag_decision: Decision,
12461246
decision_reasons: Optional[list[str]],
12471247
decide_options: list[str],
1248-
project_config: ProjectConfig
1248+
project_config: ProjectConfig,
1249+
holdout_decision: Optional[Decision] = None
12491250
) -> OptimizelyDecision:
12501251
user_id = user_context.user_id
12511252
feature_enabled = False
@@ -1264,6 +1265,26 @@ def _create_optimizely_decision(
12641265

12651266
feature_flag = project_config.feature_key_map.get(flag_key)
12661267

1268+
# Send holdout impression when user was bucketed into a holdout bypassed due to exclude_targeted_deliveries
1269+
if (holdout_decision is not None
1270+
and decision_source != DecisionSources.HOLDOUT
1271+
and OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options):
1272+
holdout_enabled = self._get_feature_enabled(holdout_decision.variation)
1273+
holdout_rule_key = holdout_decision.experiment.key if holdout_decision.experiment else ''
1274+
self._send_impression_event(
1275+
project_config,
1276+
holdout_decision.experiment,
1277+
holdout_decision.variation,
1278+
flag_key,
1279+
holdout_rule_key,
1280+
str(DecisionSources.HOLDOUT),
1281+
holdout_enabled,
1282+
user_id,
1283+
attributes,
1284+
holdout_decision.cmab_uuid
1285+
)
1286+
decision_event_dispatched = True
1287+
12671288
# Send impression event if Decision came from a feature
12681289
if OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options:
12691290
if (decision_source == DecisionSources.FEATURE_TEST or
@@ -1448,11 +1469,13 @@ def _decide_for_keys(
14481469
user_context,
14491470
merged_decide_options
14501471
)
1472+
holdout_decisions: dict[str, Optional[Decision]] = {}
14511473
for i in range(0, len(flags_without_forced_decision)):
14521474
decision = decision_list[i]['decision']
14531475
reasons = decision_list[i]['reasons']
14541476
error = decision_list[i]['error']
14551477
flag_key = flags_without_forced_decision[i].key
1478+
holdout_decisions[flag_key] = decision_list[i].get('holdout_decision')
14561479
# store error decision against key and remove key from valid keys
14571480
if error:
14581481
optimizely_decision = OptimizelyDecision.new_error_decision(flags_without_forced_decision[i].key,
@@ -1472,7 +1495,8 @@ def _decide_for_keys(
14721495
flag_decision,
14731496
decision_reasons,
14741497
merged_decide_options,
1475-
project_config
1498+
project_config,
1499+
holdout_decision=holdout_decisions.get(key)
14761500
)
14771501
enabled_flags_only_missing = OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options
14781502
is_enabled = optimizely_decision.enabled

0 commit comments

Comments
 (0)