From 475d932af7b96d4a0728000639c3b883ad2611cd Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Tue, 21 Jul 2026 12:22:15 -0500 Subject: [PATCH 1/2] [AI-FSSDK] [FSSDK-12735] Add holdout exclusion logic for Targeted Delivery rules --- lib/core/decision_service/index.spec.ts | 151 ++++++++++++++++++++++++ lib/core/decision_service/index.ts | 78 ++++++++---- lib/shared_types.ts | 1 + 3 files changed, 210 insertions(+), 20 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 2334a386b..944fe27c8 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -2196,6 +2196,157 @@ describe('DecisionService', () => { decisionSource: DECISION_SOURCES.HOLDOUT, }); }); + + describe('excludeTargetedDeliveries', () => { + const getExcludeTDDatafile = (excludeTargetedDeliveries?: boolean) => { + const datafile = getHoldoutTestDatafile(); + datafile.holdouts = datafile.holdouts.map((holdout: any) => { + if (holdout.id === 'holdout_running_id') { + return { + ...holdout, + ...(excludeTargetedDeliveries !== undefined ? { excludeTargetedDeliveries } : {}), + }; + } + return holdout; + }); + return datafile; + }; + + it('should return holdout variation immediately when excludeTargetedDeliveries is false', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(false))); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should return holdout variation immediately when excludeTargetedDeliveries is undefined', async () => { + const { decisionService } = getDecisionService(); + const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(undefined))); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should return delivery variation when excludeTargetedDeliveries is true and delivery matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'delivery_1') { + return { result: '5004', reasons: [] }; + } + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.experimentKeyMap['delivery_1'], + variation: config.variationIdMap['5004'], + decisionSource: DECISION_SOURCES.ROLLOUT, + }); + }); + + it('should return holdout variation when excludeTargetedDeliveries is true but no delivery matches', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + + it('should still block experiment rules when excludeTargetedDeliveries is true', async () => { + const { decisionService } = getDecisionService(); + const datafile = getExcludeTDDatafile(true); + + const config = createProjectConfig(JSON.stringify(datafile)); + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'tester', + attributes: { age: 20 }, + }); + + mockBucket.mockImplementation((param: BucketerParams) => { + if (param.experimentKey === 'holdout_running') { + return { result: 'holdout_variation_running_id', reasons: [] }; + } + if (param.experimentKey === 'exp_1') { + return { result: '5001', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const feature = config.featureKeyMap['flag_1']; + const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + const variation = (await value)[0]; + + // Should NOT get exp_1 — holdout blocks experiments even with excludeTargetedDeliveries + // Should get holdout since no delivery matched + expect(variation.result).toEqual({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + decisionSource: DECISION_SOURCES.HOLDOUT, + }); + }); + }); }); }); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 2064ec2a2..14213e5b3 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -119,6 +119,7 @@ export const USER_MEETS_CONDITIONS_FOR_HOLDOUT = 'User %s meets conditions for h export const USER_DOESNT_MEET_CONDITIONS_FOR_HOLDOUT = 'User %s does not meet conditions for holdout %s.'; export const USER_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in variation %s of holdout %s.'; export const USER_NOT_BUCKETED_INTO_HOLDOUT_VARIATION = 'User %s is in no holdout variation.'; +export const TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT = 'User %s is in holdout %s but targeted deliveries are excluded from holdout.'; export interface DecisionObj { experiment: Experiment | Holdout | null; @@ -969,11 +970,23 @@ export class DecisionService { // getGlobalHoldouts() returns holdouts with includedRules == null/undefined. const globalHoldouts = getGlobalHoldouts(configObj); + let activeHoldoutDecision: DecisionObj | null = null; + let activeHoldout: Holdout | null = null; + for (const holdout of globalHoldouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); if (holdoutDecision.result.variation) { + if (holdout.excludeTargetedDeliveries) { + const userId = user.getUserId(); + this.logger?.info(TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, userId, holdout.key); + decideReasons.push([TARGETED_DELIVERY_EXCLUDED_FROM_HOLDOUT, userId, holdout.key]); + activeHoldoutDecision = holdoutDecision.result; + activeHoldout = holdout; + break; + } + return Value.of(op, { result: holdoutDecision.result, reasons: decideReasons, @@ -981,33 +994,58 @@ export class DecisionService { } } - return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { - if (experimentDecision.error || experimentDecision.result.variation !== null) { + if (!activeHoldoutDecision) { + return this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker).then((experimentDecision) => { + if (experimentDecision.error || experimentDecision.result.variation !== null) { + return Value.of(op, { + ...experimentDecision, + reasons: [...decideReasons, ...experimentDecision.reasons], + }); + } + + decideReasons.push(...experimentDecision.reasons); + + const rolloutDecision = this.getVariationForRollout(configObj, feature, user); + decideReasons.push(...rolloutDecision.reasons); + const rolloutDecisionResult = rolloutDecision.result; + const userId = user.getUserId(); + + if (rolloutDecisionResult.variation) { + this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); + } else { + this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); + } + return Value.of(op, { - ...experimentDecision, - reasons: [...decideReasons, ...experimentDecision.reasons], + result: rolloutDecisionResult, + reasons: decideReasons, }); - } + }); + } - decideReasons.push(...experimentDecision.reasons); - - const rolloutDecision = this.getVariationForRollout(configObj, feature, user); - decideReasons.push(...rolloutDecision.reasons); - const rolloutDecisionResult = rolloutDecision.result; - const userId = user.getUserId(); - - if (rolloutDecisionResult.variation) { - this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); - decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); - } else { - this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); - decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); - } - + // User is in holdout with excludeTargetedDeliveries — skip experiments, evaluate delivery rules + const rolloutDecision = this.getVariationForRollout(configObj, feature, user); + decideReasons.push(...rolloutDecision.reasons); + const rolloutDecisionResult = rolloutDecision.result; + const userId = user.getUserId(); + + if (rolloutDecisionResult.variation) { + this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); return Value.of(op, { result: rolloutDecisionResult, reasons: decideReasons, }); + } + + this.logger?.debug(USER_NOT_IN_ROLLOUT, userId, feature.key); + decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); + + return Value.of(op, { + result: activeHoldoutDecision, + reasons: decideReasons, }); } diff --git a/lib/shared_types.ts b/lib/shared_types.ts index 6640622ab..4cc772019 100644 --- a/lib/shared_types.ts +++ b/lib/shared_types.ts @@ -190,6 +190,7 @@ export interface Holdout extends ExperimentCore { * membership. */ isGlobal: boolean; + excludeTargetedDeliveries?: boolean; } export function isHoldout(obj: Experiment | Holdout): obj is Holdout { From 354d2bc692167982d232d4bfb77029fb09108cff Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Wed, 22 Jul 2026 14:31:36 -0500 Subject: [PATCH 2/2] [AI-FSSDK] [FSSDK-12735] Add holdout event dispatch, local holdout guard, and null TD fallback --- lib/core/decision_service/index.spec.ts | 58 ++++++++++++++++++++----- lib/core/decision_service/index.ts | 22 +++++++++- lib/optimizely/index.spec.ts | 46 ++++++++++++++++++++ lib/optimizely/index.ts | 9 ++++ 4 files changed, 123 insertions(+), 12 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 944fe27c8..ca25019a4 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -2252,7 +2252,7 @@ describe('DecisionService', () => { }); }); - it('should return delivery variation when excludeTargetedDeliveries is true and delivery matches', async () => { + it('should return delivery variation with holdout info when excludeTargetedDeliveries is true and delivery matches', async () => { const { decisionService } = getDecisionService(); const datafile = getExcludeTDDatafile(true); @@ -2281,10 +2281,14 @@ describe('DecisionService', () => { experiment: config.experimentKeyMap['delivery_1'], variation: config.variationIdMap['5004'], decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }, }); }); - it('should return holdout variation when excludeTargetedDeliveries is true but no delivery matches', async () => { + it('should return null decision with holdout info when excludeTargetedDeliveries is true but no delivery matches', async () => { const { decisionService } = getDecisionService(); const datafile = getExcludeTDDatafile(true); @@ -2307,9 +2311,13 @@ describe('DecisionService', () => { const variation = (await value)[0]; expect(variation.result).toEqual({ - experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], - variation: config.variationIdMap['holdout_variation_running_id'], - decisionSource: DECISION_SOURCES.HOLDOUT, + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }, }); }); @@ -2338,12 +2346,14 @@ describe('DecisionService', () => { const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); const variation = (await value)[0]; - // Should NOT get exp_1 — holdout blocks experiments even with excludeTargetedDeliveries - // Should get holdout since no delivery matched expect(variation.result).toEqual({ - experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], - variation: config.variationIdMap['holdout_variation_running_id'], - decisionSource: DECISION_SOURCES.HOLDOUT, + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }, }); }); }); @@ -3252,6 +3262,34 @@ describe('DecisionService', () => { expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.FEATURE_TEST); expect(value[0].result.variation?.key).toBe('variation_1'); }); + + it('local holdout ignores excludeTargetedDeliveries and applies normally', async () => { + const datafile = makeLocalHoldoutDatafile('2001'); + (datafile as any).localHoldouts[0].excludeTargetedDeliveries = true; + const config = createProjectConfig(JSON.stringify(datafile)); + const { decisionService } = getDecisionService(); + + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const user = new OptimizelyUserContext({ + optimizely: {} as any, + userId: 'user1', + attributes: { age: 15 }, + }); + + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + // Local holdout applies normally even with excludeTargetedDeliveries set + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT); + expect(value[0].result.experiment?.id).toBe('local_holdout_id'); + expect(value[0].result.variation?.id).toBe('local_holdout_variation_id'); + }); }); }); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index 14213e5b3..88cf62a58 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -126,6 +126,10 @@ export interface DecisionObj { variation: Variation | null; decisionSource: DecisionSource; cmabUuid?: string; + holdout?: { + experiment: Holdout; + variation: Variation; + }; } interface DecisionServiceOptions { @@ -1035,7 +1039,13 @@ export class DecisionService { this.logger?.debug(USER_IN_ROLLOUT, userId, feature.key); decideReasons.push([USER_IN_ROLLOUT, userId, feature.key]); return Value.of(op, { - result: rolloutDecisionResult, + result: { + ...rolloutDecisionResult, + holdout: { + experiment: activeHoldout!, + variation: activeHoldoutDecision!.variation!, + }, + }, reasons: decideReasons, }); } @@ -1044,7 +1054,15 @@ export class DecisionService { decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); return Value.of(op, { - result: activeHoldoutDecision, + result: { + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: activeHoldout!, + variation: activeHoldoutDecision!.variation!, + }, + }, reasons: decideReasons, }); } diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index d891e283a..7563d41a5 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -872,6 +872,52 @@ describe('Optimizely', () => { }), }); }); + + it('should dispatch separate holdout impression when decisionObj.holdout is populated', async () => { + const processSpy = vi.spyOn(eventProcessor, 'process'); + + const holdoutExperiment = projectConfig.holdouts[0]; + const holdoutVariation = projectConfig.holdouts[0].variations[0]; + const rolloutExperiment = projectConfig.experimentKeyMap['delivery_1'] || Object.values(projectConfig.experimentIdMap)[0]; + const rolloutVariation = rolloutExperiment?.variations?.[0] || { id: 'test_var_id', key: 'test_var_key', variables: [] }; + + vi.spyOn(decisionService, 'resolveVariationsForFeatureList').mockImplementation(() => { + return Value.of('async', [{ + error: false, + result: { + variation: rolloutVariation, + experiment: rolloutExperiment, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: { + experiment: holdoutExperiment, + variation: holdoutVariation, + }, + }, + reasons: [], + }]); + }); + + const user = new OptimizelyUserContext({ + optimizely, + userId: 'test_user', + attributes: {}, + }); + + await optimizely.decideAsync(user, 'flag_1', []); + + // The holdout impression is dispatched even when the main rollout impression is not + // (sendFlagDecisions not set). Verify at least one impression is a holdout event. + expect(processSpy).toHaveBeenCalled(); + + const holdoutEvent = processSpy.mock.calls.find( + (call: any) => (call[0] as ImpressionEvent).ruleType === 'holdout' + ); + expect(holdoutEvent).toBeDefined(); + const holdoutImpression = holdoutEvent![0] as ImpressionEvent; + expect(holdoutImpression.ruleKey).toBe('holdout_test_key'); + expect(holdoutImpression.ruleType).toBe('holdout'); + expect(holdoutImpression.enabled).toBe(false); + }); }); it('should flush eventProcessor and odpManager on flushImmediately()', async () => { diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index 251c08b0e..35c6c1111 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -1542,6 +1542,15 @@ export default class Optimizely extends BaseService implements Client { decisionEventDispatched = true; } + if (decisionObj.holdout && !options[OptimizelyDecideOption.DISABLE_DECISION_EVENT]) { + const holdoutDecisionObj: DecisionObj = { + experiment: decisionObj.holdout.experiment, + variation: decisionObj.holdout.variation, + decisionSource: DECISION_SOURCES.HOLDOUT, + }; + this.sendImpressionEvent(holdoutDecisionObj, key, userId, false, attributes); + } + const shouldIncludeReasons = options[OptimizelyDecideOption.INCLUDE_REASONS]; let reportedReasons: string[] = [];