Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alexjoeyyong wants to merge 4 commits into
base: master
Choose a base branch
from
+300 −96
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
45bf19c
[AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout…
alexjoeyyong Oct 13, 2026
532edfb
[AI-FSSDK] [FSSDK-12958] Add tests for disabled decision events and t…
alexjoeyyong Oct 15, 2026
2067caf
[AI-FSSDK] [FSSDK-12958] Add test for one local holdout hit on two rules
alexjoeyyong Oct 15, 2026
0a7dc91
[AI-FSSDK] [FSSDK-12958] Keep deprecated holdout_decision key alongsi…
alexjoeyyong Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 57 additions & 47 deletions optimizely/decision_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2017-2022, Optimizely
# Copyright 2017-2022, 2026, Optimizely
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand Down Expand Up @@ -65,7 +65,8 @@ class VariationResult(TypedDict):


class _DecisionResultOptional(TypedDict, total=False):
holdout_decision: Decision
holdout_decision: Decision # Deprecated: first entry of holdout_decisions.
holdout_decisions: List[Decision]


class DecisionResult(_DecisionResultOptional):
Expand Down Expand Up @@ -552,7 +553,11 @@ def get_variation(
}

def get_variation_for_rollout(
self, project_config: ProjectConfig, feature: entities.FeatureFlag, user_context: OptimizelyUserContext
self,
project_config: ProjectConfig,
feature: entities.FeatureFlag,
user_context: OptimizelyUserContext,
holdout_decisions: Optional[list[Decision]] = None
) -> tuple[Decision, list[str]]:
""" Determine which experiment/variation the user is in for a given rollout.
Returns the variation of the first experiment the user qualifies for.
Expand All @@ -562,7 +567,7 @@ def get_variation_for_rollout(
flagKey: Feature key.
rollout: Rollout for which we are getting the variation.
user: ID and attributes for user.
options: Decide options.
holdout_decisions: If provided, local holdout decisions for skipped rules are appended to it.

Returns:
Decision namedtuple consisting of experiment and variation for the user and
Expand Down Expand Up @@ -606,22 +611,14 @@ def get_variation_for_rollout(
return Decision(experiment=rule, variation=forced_decision_variation,
source=enums.DecisionSources.ROLLOUT, cmab_uuid=None), decide_reasons

local_holdouts = project_config.get_holdouts_for_rule(rule.id)
for holdout in local_holdouts:
local_holdout_decision = self.get_variation_for_holdout(
holdout, user_context, project_config
)
decide_reasons.extend(local_holdout_decision['reasons'])

local_decision = local_holdout_decision['decision']
if local_decision.variation is not None:
message = (
f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' "
f"for delivery rule '{rule.key}'."
)
self.logger.info(message)
decide_reasons.append(message)
return local_decision, decide_reasons
local_holdout = self._get_local_holdout_decision(
project_config, rule, user_context, 'delivery', decide_reasons
)
if local_holdout is not None:
if holdout_decisions is not None:
holdout_decisions.append(local_holdout)
index += 1
continue

bucketing_id, bucket_reasons = self._get_bucketing_id(user_id, attributes)
decide_reasons += bucket_reasons
Expand Down Expand Up @@ -748,6 +745,7 @@ def get_decision_for_flag(

global_holdout_result: DecisionResult | None = None
global_holdout_key: str | None = None
holdout_decisions: list[Decision] = []

# Check global holdouts (flag level — before any rules are evaluated)
global_holdouts = project_config.get_global_holdouts()
Expand Down Expand Up @@ -781,6 +779,7 @@ def get_decision_for_flag(
reasons.append(message)
global_holdout_result = holdout_decision
global_holdout_key = holdout.key
holdout_decisions.append(holdout_decision['decision'])
break

# Check experiments then rollouts
Expand All @@ -807,30 +806,17 @@ def get_decision_for_flag(
'error': False,
'reasons': reasons
}
if global_holdout_result is not None:
result['holdout_decision'] = global_holdout_result['decision']
if holdout_decisions:
result['holdout_decisions'] = holdout_decisions
result['holdout_decision'] = holdout_decisions[0]
return result

local_holdouts = project_config.get_holdouts_for_rule(experiment.id)
for holdout in local_holdouts:
local_holdout_decision = self.get_variation_for_holdout(
holdout, user_context, project_config
)
reasons.extend(local_holdout_decision['reasons'])

local_decision = local_holdout_decision['decision']
if local_decision.variation is not None:
message = (
f"The user '{user_id}' is bucketed into local holdout '{holdout.key}' "
f"for experiment rule '{experiment.key}'."
)
self.logger.info(message)
reasons.append(message)
return {
'decision': local_holdout_decision['decision'],
'error': False,
'reasons': reasons
}
local_holdout = self._get_local_holdout_decision(
project_config, experiment, user_context, 'experiment', reasons
)
if local_holdout is not None:
holdout_decisions.append(local_holdout)
continue

# Get variation for experiment
variation_result = self.get_variation(
Expand All @@ -856,8 +842,9 @@ def get_decision_for_flag(
'error': False,
'reasons': reasons
}
if global_holdout_result is not None:
result['holdout_decision'] = global_holdout_result['decision']
if holdout_decisions:
result['holdout_decisions'] = holdout_decisions
result['holdout_decision'] = holdout_decisions[0]
return result

if global_holdout_result is not None:
Expand All @@ -870,7 +857,7 @@ def get_decision_for_flag(

# If no experiment decision, check rollouts
rollout_decision, rollout_reasons = self.get_variation_for_rollout(
project_config, feature_flag, user_context
project_config, feature_flag, user_context, holdout_decisions
)
if rollout_reasons:
reasons.extend(rollout_reasons)
Expand All @@ -893,10 +880,33 @@ def get_decision_for_flag(
'error': False,
'reasons': reasons
}
if global_holdout_result is not None:
final_result['holdout_decision'] = global_holdout_result['decision']
if holdout_decisions:
final_result['holdout_decisions'] = holdout_decisions
final_result['holdout_decision'] = holdout_decisions[0]
return final_result

def _get_local_holdout_decision(
self,
project_config: ProjectConfig,
rule: entities.Experiment,
user_context: OptimizelyUserContext,
rule_type: str,
reasons: list[str]
) -> Optional[Decision]:
"""Returns the first local holdout decision the user is bucketed into for the rule, if any."""
for holdout in project_config.get_holdouts_for_rule(rule.id):
holdout_result = self.get_variation_for_holdout(holdout, user_context, project_config)
reasons.extend(holdout_result['reasons'])
if holdout_result['decision'].variation is not None:
message = (
f"The user '{user_context.user_id}' is bucketed into local holdout '{holdout.key}' "
f"for {rule_type} rule '{rule.key}'. Skipping the rule."
)
self.logger.info(message)
reasons.append(message)
return holdout_result['decision']
return None

def get_variation_for_holdout(
self,
holdout: entities.Holdout,
Expand Down
43 changes: 22 additions & 21 deletions optimizely/optimizely.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2016-2023, Optimizely
# Copyright 2016-2023, 2026, Optimizely
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand Down Expand Up @@ -1246,7 +1246,7 @@ def _create_optimizely_decision(
decision_reasons: Optional[list[str]],
decide_options: list[str],
project_config: ProjectConfig,
holdout_decision: Optional[Decision] = None
holdout_decisions: Optional[list[Decision]] = None
) -> OptimizelyDecision:
user_id = user_context.user_id
feature_enabled = False
Expand All @@ -1265,24 +1265,25 @@ def _create_optimizely_decision(

feature_flag = project_config.feature_key_map.get(flag_key)

# Send holdout impression when user was bucketed into a holdout bypassed due to exclude_targeted_deliveries
if (holdout_decision is not None
# Skipped local holdouts and global holdouts bypassed by exclude_targeted_deliveries
if (holdout_decisions
and decision_source != DecisionSources.HOLDOUT
and OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options):
holdout_enabled = self._get_feature_enabled(holdout_decision.variation)
holdout_rule_key = holdout_decision.experiment.key if holdout_decision.experiment else ''
self._send_impression_event(
project_config,
holdout_decision.experiment,
holdout_decision.variation,
flag_key,
holdout_rule_key,
str(DecisionSources.HOLDOUT),
holdout_enabled,
user_id,
attributes,
holdout_decision.cmab_uuid
)
for holdout_decision in holdout_decisions:
holdout_enabled = self._get_feature_enabled(holdout_decision.variation)
holdout_rule_key = holdout_decision.experiment.key if holdout_decision.experiment else ''
self._send_impression_event(
project_config,
holdout_decision.experiment,
holdout_decision.variation,
flag_key,
holdout_rule_key,
str(DecisionSources.HOLDOUT),
holdout_enabled,
user_id,
attributes,
holdout_decision.cmab_uuid
)
decision_event_dispatched = True

# Send impression event if Decision came from a feature
Expand Down Expand Up @@ -1469,13 +1470,13 @@ def _decide_for_keys(
user_context,
merged_decide_options
)
holdout_decisions: dict[str, Optional[Decision]] = {}
holdout_decisions: dict[str, Optional[list[Decision]]] = {}
for i in range(0, len(flags_without_forced_decision)):
decision = decision_list[i]['decision']
reasons = decision_list[i]['reasons']
error = decision_list[i]['error']
flag_key = flags_without_forced_decision[i].key
holdout_decisions[flag_key] = decision_list[i].get('holdout_decision')
holdout_decisions[flag_key] = decision_list[i].get('holdout_decisions')
# store error decision against key and remove key from valid keys
if error:
optimizely_decision = OptimizelyDecision.new_error_decision(flags_without_forced_decision[i].key,
Expand All @@ -1496,7 +1497,7 @@ def _decide_for_keys(
decision_reasons,
merged_decide_options,
project_config,
holdout_decision=holdout_decisions.get(key)
holdout_decisions=holdout_decisions.get(key)
)
enabled_flags_only_missing = OptimizelyDecideOption.ENABLED_FLAGS_ONLY not in merged_decide_options
is_enabled = optimizely_decision.enabled
Expand Down
4 changes: 2 additions & 2 deletions tests/test_decision_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2017-2021, Optimizely
# Copyright 2017-2021, 2026, Optimizely
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
Expand Down Expand Up @@ -1510,7 +1510,7 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_rollout(sel
)

mock_get_variation_for_rollout.assert_called_once_with(
self.project_config, feature, user
self.project_config, feature, user, []
)

# Assert no log messages were generated
Expand Down
Loading
Loading