Skip to content

Commit ee1ef19

Browse files
[AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules (#1171)
Implements holdout exclusion logic for Targeted Delivery rules. When a holdout's `excludeTargetedDeliveries` flag is true, users bucketed into that holdout will still be served Targeted Delivery (rollout) rules normally, while A/B Test and Multi-Armed Bandit experiment rules continue to be blocked by the holdout. ## Changes - Added `excludeTargetedDeliveries` field to the Holdout interface in shared types - Updated `resolveVariationForFlag` to evaluate delivery rules when holdout has exclusion enabled, falling back to holdout decision if no delivery rule matches - Added decision reason logging for when Targeted Delivery rules are excluded from holdout logic --------- Co-authored-by: Raju Ahmed <raju.ahmed@optimizely.com>
1 parent 6da7398 commit ee1ef19

5 files changed

Lines changed: 306 additions & 10 deletions

File tree

lib/core/decision_service/index.spec.ts

Lines changed: 193 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
USER_HAS_NO_FORCED_VARIATION
2020
} from 'log_message';
2121
import { beforeEach, describe, expect, it, MockInstance, vi } from 'vitest';
22-
import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService } from '.';
22+
import { CMAB_DUMMY_ENTITY_ID, CMAB_FETCH_FAILED, DecisionService, TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT } from '.';
2323
import OptimizelyUserContext from '../../optimizely_user_context';
2424
import { createProjectConfig, ProjectConfig } from '../../project_config/project_config';
2525
import { BucketerParams, Experiment, ExperimentBucketMap, Holdout, OptimizelyDecideOption, UserAttributes, UserProfile } from '../../shared_types';
@@ -2196,6 +2196,170 @@ describe('DecisionService', () => {
21962196
decisionSource: DECISION_SOURCES.HOLDOUT,
21972197
});
21982198
});
2199+
2200+
describe('excludeTargetedDeliveries', () => {
2201+
const getExcludeTDDatafile = (excludeTargetedDeliveries?: boolean) => {
2202+
const datafile = getHoldoutTestDatafile();
2203+
datafile.holdouts = datafile.holdouts.map((holdout: any) => {
2204+
if (holdout.id === 'holdout_running_id') {
2205+
return {
2206+
...holdout,
2207+
...(excludeTargetedDeliveries !== undefined ? { excludeTargetedDeliveries } : {}),
2208+
};
2209+
}
2210+
return holdout;
2211+
});
2212+
return datafile;
2213+
};
2214+
2215+
it('should return holdout variation immediately when excludeTargetedDeliveries is false', async () => {
2216+
const { decisionService } = getDecisionService();
2217+
const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(false)));
2218+
const user = new OptimizelyUserContext({
2219+
optimizely: {} as any,
2220+
userId: 'tester',
2221+
attributes: { age: 20 },
2222+
});
2223+
2224+
const feature = config.featureKeyMap['flag_1'];
2225+
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
2226+
const variation = (await value)[0];
2227+
2228+
expect(variation.result).toEqual({
2229+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2230+
variation: config.variationIdMap['holdout_variation_running_id'],
2231+
decisionSource: DECISION_SOURCES.HOLDOUT,
2232+
});
2233+
});
2234+
2235+
it('should return holdout variation immediately when excludeTargetedDeliveries is undefined', async () => {
2236+
const { decisionService } = getDecisionService();
2237+
const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(undefined)));
2238+
const user = new OptimizelyUserContext({
2239+
optimizely: {} as any,
2240+
userId: 'tester',
2241+
attributes: { age: 20 },
2242+
});
2243+
2244+
const feature = config.featureKeyMap['flag_1'];
2245+
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
2246+
const variation = (await value)[0];
2247+
2248+
expect(variation.result).toEqual({
2249+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2250+
variation: config.variationIdMap['holdout_variation_running_id'],
2251+
decisionSource: DECISION_SOURCES.HOLDOUT,
2252+
});
2253+
});
2254+
2255+
it('should return delivery variation with holdout info when excludeTargetedDeliveries is true and delivery matches', async () => {
2256+
const { decisionService } = getDecisionService();
2257+
const datafile = getExcludeTDDatafile(true);
2258+
2259+
const config = createProjectConfig(JSON.stringify(datafile));
2260+
const user = new OptimizelyUserContext({
2261+
optimizely: {} as any,
2262+
userId: 'tester',
2263+
attributes: { age: 20 },
2264+
});
2265+
2266+
mockBucket.mockImplementation((param: BucketerParams) => {
2267+
if (param.experimentKey === 'delivery_1') {
2268+
return { result: '5004', reasons: [] };
2269+
}
2270+
if (param.experimentKey === 'holdout_running') {
2271+
return { result: 'holdout_variation_running_id', reasons: [] };
2272+
}
2273+
return { result: null, reasons: [] };
2274+
});
2275+
2276+
const feature = config.featureKeyMap['flag_1'];
2277+
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
2278+
const variation = (await value)[0];
2279+
2280+
expect(variation.result).toEqual({
2281+
experiment: config.experimentKeyMap['delivery_1'],
2282+
variation: config.variationIdMap['5004'],
2283+
decisionSource: DECISION_SOURCES.ROLLOUT,
2284+
holdout: {
2285+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2286+
variation: config.variationIdMap['holdout_variation_running_id'],
2287+
},
2288+
});
2289+
2290+
const reasonStrings = variation.reasons.map((r: any) => typeof r === 'string' ? r : r[0]);
2291+
expect(reasonStrings).toContain(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT);
2292+
});
2293+
2294+
it('should return null decision with holdout info when excludeTargetedDeliveries is true but no delivery matches', async () => {
2295+
const { decisionService } = getDecisionService();
2296+
const datafile = getExcludeTDDatafile(true);
2297+
2298+
const config = createProjectConfig(JSON.stringify(datafile));
2299+
const user = new OptimizelyUserContext({
2300+
optimizely: {} as any,
2301+
userId: 'tester',
2302+
attributes: { age: 20 },
2303+
});
2304+
2305+
mockBucket.mockImplementation((param: BucketerParams) => {
2306+
if (param.experimentKey === 'holdout_running') {
2307+
return { result: 'holdout_variation_running_id', reasons: [] };
2308+
}
2309+
return { result: null, reasons: [] };
2310+
});
2311+
2312+
const feature = config.featureKeyMap['flag_1'];
2313+
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
2314+
const variation = (await value)[0];
2315+
2316+
expect(variation.result).toEqual({
2317+
experiment: null,
2318+
variation: null,
2319+
decisionSource: DECISION_SOURCES.ROLLOUT,
2320+
holdout: {
2321+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2322+
variation: config.variationIdMap['holdout_variation_running_id'],
2323+
},
2324+
});
2325+
});
2326+
2327+
it('should still block experiment rules when excludeTargetedDeliveries is true', async () => {
2328+
const { decisionService } = getDecisionService();
2329+
const datafile = getExcludeTDDatafile(true);
2330+
2331+
const config = createProjectConfig(JSON.stringify(datafile));
2332+
const user = new OptimizelyUserContext({
2333+
optimizely: {} as any,
2334+
userId: 'tester',
2335+
attributes: { age: 20 },
2336+
});
2337+
2338+
mockBucket.mockImplementation((param: BucketerParams) => {
2339+
if (param.experimentKey === 'holdout_running') {
2340+
return { result: 'holdout_variation_running_id', reasons: [] };
2341+
}
2342+
if (param.experimentKey === 'exp_1') {
2343+
return { result: '5001', reasons: [] };
2344+
}
2345+
return { result: null, reasons: [] };
2346+
});
2347+
2348+
const feature = config.featureKeyMap['flag_1'];
2349+
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
2350+
const variation = (await value)[0];
2351+
2352+
expect(variation.result).toEqual({
2353+
experiment: null,
2354+
variation: null,
2355+
decisionSource: DECISION_SOURCES.ROLLOUT,
2356+
holdout: {
2357+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2358+
variation: config.variationIdMap['holdout_variation_running_id'],
2359+
},
2360+
});
2361+
});
2362+
});
21992363
});
22002364
});
22012365

@@ -3101,6 +3265,34 @@ describe('DecisionService', () => {
31013265
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST);
31023266
expect(value[0].result.variation?.key).toBe('variation_1');
31033267
});
3268+
3269+
it('local holdout ignores excludeTargetedDeliveries and applies normally', async () => {
3270+
const datafile = makeLocalHoldoutDatafile('2001');
3271+
(datafile as any).localHoldouts[0].excludeTargetedDeliveries = true;
3272+
const config = createProjectConfig(JSON.stringify(datafile));
3273+
const { decisionService } = getDecisionService();
3274+
3275+
mockBucket.mockImplementation((params: BucketerParams) => {
3276+
if (params.experimentId === 'local_holdout_id') {
3277+
return { result: 'local_holdout_variation_id', reasons: [] };
3278+
}
3279+
return { result: null, reasons: [] };
3280+
});
3281+
3282+
const user = new OptimizelyUserContext({
3283+
optimizely: {} as any,
3284+
userId: 'user1',
3285+
attributes: { age: 15 },
3286+
});
3287+
3288+
const feature = config.featureKeyMap['flag_1'];
3289+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3290+
3291+
// Local holdout applies normally even with excludeTargetedDeliveries set
3292+
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT);
3293+
expect(value[0].result.experiment?.id).toBe('local_holdout_id');
3294+
expect(value[0].result.variation?.id).toBe('local_holdout_variation_id');
3295+
});
31043296
});
31053297
});
31063298

lib/core/decision_service/index.ts

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,17 @@ export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for h
119119
export const USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT = 'User %s does not meet conditions for holdout %s.';
120120
export const USER_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in variation %s of holdout %s.';
121121
export const USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in no holdout variation.';
122+
export const TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT = 'Holdout "%s" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.';
122123

123124
export interface DecisionObj {
124125
experiment: Experiment | Holdout | null;
125126
variation: Variation | null;
126127
decisionSource: DecisionSource;
127128
cmabUuid?: string;
129+
holdout?: {
130+
experiment: Holdout;
131+
variation: Variation;
132+
};
128133
}
129134

130135
interface DecisionServiceOptions {
@@ -913,11 +918,7 @@ export class DecisionService {
913918
options: DecideOptionsMap): Value<OP, DecisionResult[]> {
914919
const userId = user.getUserId();
915920
const attributes = user.getAttributes();
916-
const decisions: DecisionResponse<DecisionObj>[] = [];
917-
// const userProfileTracker : UserProfileTracker = {
918-
// isProfileUpdated: false,
919-
// userProfile: null,
920-
// }
921+
921922
const shouldIgnoreUPS = !!options[OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE];
922923

923924
const userProfileTrackerValue: Value<OP, Maybe<UserProfileTracker>> = shouldIgnoreUPS ? Value.of(op, undefined)
@@ -969,19 +970,41 @@ export class DecisionService {
969970
// getGlobalHoldouts() returns holdouts with includedRules == null/undefined.
970971
const globalHoldouts = getGlobalHoldouts(configObj);
971972

973+
let appliedHoldout : DecisionObj['holdout'] | undefined = undefined;
974+
972975
for (const holdout of globalHoldouts) {
973976
const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user);
974977
decideReasons.push(...holdoutDecision.reasons);
975978

976979
if (holdoutDecision.result.variation) {
980+
if (holdout.excludeTargetedDeliveries) {
981+
this.logger?.info(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, holdout.key);
982+
decideReasons.push([TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, holdout.key]);
983+
appliedHoldout = {
984+
experiment: holdout,
985+
variation: holdoutDecision.result.variation
986+
}
987+
break;
988+
}
989+
977990
return Value.of(op, {
978991
result: holdoutDecision.result,
979992
reasons: decideReasons,
980993
});
981994
}
982995
}
983996

984-
return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => {
997+
const experimentDecision: Value<OP, DecisionResult> = appliedHoldout ?
998+
Value.of(op, {
999+
reasons: decideReasons,
1000+
result: {
1001+
experiment: null,
1002+
variation: null,
1003+
decisionSource: DECISION_SOURCES.FEATURE_TEST,
1004+
}
1005+
}) : this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker);
1006+
1007+
return experimentDecision.then((experimentDecision) => {
9851008
if (experimentDecision.error || experimentDecision.result.variation !== null) {
9861009
return Value.of(op, {
9871010
...experimentDecision,
@@ -990,20 +1013,24 @@ export class DecisionService {
9901013
}
9911014

9921015
decideReasons.push(...experimentDecision.reasons);
993-
1016+
9941017
const rolloutDecision = this.getVariationForRollout(configObj, feature, user);
9951018
decideReasons.push(...rolloutDecision.reasons);
9961019
const rolloutDecisionResult = rolloutDecision.result;
9971020
const userId = user.getUserId();
998-
1021+
9991022
if (rolloutDecisionResult.variation) {
10001023
this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key);
10011024
decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]);
10021025
} else {
10031026
this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key);
10041027
decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]);
10051028
}
1006-
1029+
1030+
if (appliedHoldout) {
1031+
rolloutDecisionResult.holdout = appliedHoldout;
1032+
}
1033+
10071034
return Value.of(op, {
10081035
result: rolloutDecisionResult,
10091036
reasons: decideReasons,

lib/optimizely/index.spec.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,66 @@ describe('Optimizely', () => {
872872
}),
873873
});
874874
});
875+
876+
it('should dispatch separate holdout impression when decisionObj.holdout is populated', async () => {
877+
const processSpy = vi.spyOn(eventProcessor, 'process');
878+
const notificationSpyLocal = vi.fn();
879+
optimizely.notificationCenter.addNotificationListener(
880+
NOTIFICATION_TYPES.DECISION,
881+
notificationSpyLocal
882+
);
883+
884+
const holdoutExperiment = projectConfig.holdouts[0];
885+
const holdoutVariation = projectConfig.holdouts[0].variations[0];
886+
const rolloutExperiment = projectConfig.experimentKeyMap['delivery_1'] || Object.values(projectConfig.experimentIdMap)[0];
887+
const rolloutVariation = rolloutExperiment?.variations?.[0] || { id: 'test_var_id', key: 'test_var_key', variables: [] };
888+
889+
vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => {
890+
return Value.of('async', [{
891+
error: false,
892+
result: {
893+
variation: rolloutVariation,
894+
experiment: rolloutExperiment,
895+
decisionSource: DECISION_SOURCES.ROLLOUT,
896+
holdout: {
897+
experiment: holdoutExperiment,
898+
variation: holdoutVariation,
899+
},
900+
},
901+
reasons: [],
902+
}]);
903+
});
904+
905+
const user = new OptimizelyUserContext({
906+
optimizely,
907+
userId: 'test_user',
908+
attributes: {},
909+
});
910+
911+
await optimizely.decideAsync(user, 'flag_1', []);
912+
913+
// The holdout impression is dispatched even when the main rollout impression is not
914+
// (sendFlagDecisions not set). Verify at least one impression is a holdout event.
915+
expect(processSpy).toHaveBeenCalled();
916+
917+
const holdoutEvent = processSpy.mock.calls.find(
918+
(call: any) => (call[0] as ImpressionEvent).ruleType === 'holdout'
919+
);
920+
expect(holdoutEvent).toBeDefined();
921+
const holdoutImpression = holdoutEvent![0] as ImpressionEvent;
922+
expect(holdoutImpression.ruleKey).toBe('holdout_test_key');
923+
expect(holdoutImpression.ruleType).toBe('holdout');
924+
expect(holdoutImpression.enabled).toBe(false);
925+
926+
expect(notificationSpyLocal).toHaveBeenCalledWith(
927+
expect.objectContaining({
928+
type: DECISION_NOTIFICATION_TYPES.FLAG,
929+
decisionInfo: expect.objectContaining({
930+
decisionEventDispatched: true,
931+
}),
932+
})
933+
);
934+
});
875935
});
876936

877937
it('should flush eventProcessor and odpManager on flushImmediately()', async () => {

0 commit comments

Comments
 (0)