From b07d7b4dbf7948344c8c79bb1fb6b740519de2ff Mon Sep 17 00:00:00 2001 From: Alex Yong Date: Fri, 16 Oct 2026 13:02:18 -0400 Subject: [PATCH 1/2] [AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit --- lib/core/decision_service/index.spec.ts | 291 +++++++++++++++++++++--- lib/core/decision_service/index.ts | 69 +++--- lib/optimizely/index.spec.ts | 93 +++++++- lib/optimizely/index.ts | 31 +-- 4 files changed, 402 insertions(+), 82 deletions(-) diff --git a/lib/core/decision_service/index.spec.ts b/lib/core/decision_service/index.spec.ts index 77e942e0a..7f17aae94 100644 --- a/lib/core/decision_service/index.spec.ts +++ b/lib/core/decision_service/index.spec.ts @@ -1,5 +1,5 @@ /** - * Copyright 2025, Optimizely + * Copyright 2025-2026, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -2212,6 +2212,11 @@ describe('DecisionService', () => { return datafile; }; + const globalHit = (config: ProjectConfig) => ({ + experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], + variation: config.variationIdMap['holdout_variation_running_id'], + }); + it('should return holdout variation immediately when excludeTargetedDeliveries is false', async () => { const { decisionService } = getDecisionService(); const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(false))); @@ -2281,10 +2286,8 @@ 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'], - }, + holdout: globalHit(config), + holdouts: [globalHit(config)], }); const reasonStrings = variation.reasons.map((r: any) => typeof r === 'string' ? r : r[0]); @@ -2317,10 +2320,8 @@ describe('DecisionService', () => { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, - holdout: { - experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], - variation: config.variationIdMap['holdout_variation_running_id'], - }, + holdout: globalHit(config), + holdouts: [globalHit(config)], }); }); @@ -2353,10 +2354,8 @@ describe('DecisionService', () => { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, - holdout: { - experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'], - variation: config.variationIdMap['holdout_variation_running_id'], - }, + holdout: globalHit(config), + holdouts: [globalHit(config)], }); }); }); @@ -3076,6 +3075,11 @@ describe('DecisionService', () => { return datafile; }; + const localHit = (config: ProjectConfig) => ({ + experiment: config.holdoutIdMap!['local_holdout_id'], + variation: config.variationIdMap['local_holdout_variation_id'], + }); + beforeEach(() => { mockBucket.mockReset(); }); @@ -3118,12 +3122,10 @@ describe('DecisionService', () => { expect(value[0].result.experiment?.id).toBe('global_holdout_id'); }); - it('local holdout hit branch: user bucketed into local holdout for experiment rule returns holdout variation; audience and traffic not evaluated for that rule', async () => { - // exp_1 has id '2001' + it('local holdout hit on an experiment rule excludes the user from that rule only; with no other rule serving the decision is null with the hit recorded', async () => { const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001'))); const { decisionService } = getDecisionService(); - // bucket returns holdout variation when evaluating the local holdout mockBucket.mockImplementation((params: BucketerParams) => { if (params.experimentId === 'local_holdout_id') { return { result: 'local_holdout_variation_id', reasons: [] }; @@ -3135,10 +3137,70 @@ describe('DecisionService', () => { const feature = config.featureKeyMap['flag_1']; const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); - // Should return holdout decision for the local holdout - 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'); + const hit = localHit(config); + expect(value[0].result).toEqual({ + experiment: null, + variation: null, + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: hit, + holdouts: [hit], + }); + expect(mockBucket.mock.calls.some(([p]) => p.experimentId === '2001')).toBe(false); + }); + + it('local holdout hit on an experiment rule continues to the next experiment rule, which serves', async () => { + const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001'))); + const { decisionService } = getDecisionService(); + + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + if (params.experimentId === '2001') { + return { result: '5001', reasons: [] }; + } + if (params.experimentId === '2002') { + return { result: '5002', 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(); + + const hit = localHit(config); + expect(value[0].result).toEqual({ + experiment: config.experimentIdMap['2002'], + variation: config.variationIdMap['5002'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + holdout: hit, + holdouts: [hit], + }); + }); + + it('local holdout hit on an experiment rule continues to a delivery rule when no later experiment serves', async () => { + const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001'))); + const { decisionService } = getDecisionService(); + + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + if (params.experimentId === '3001') { + return { result: '5004', 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(); + + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.ROLLOUT); + expect(value[0].result.experiment?.id).toBe('3001'); + expect(value[0].result.variation?.id).toBe('5004'); + expect(value[0].result.holdouts).toEqual([localHit(config)]); }); it('local holdout miss branch: user not bucketed into local holdout falls through to regular rule evaluation', async () => { @@ -3202,31 +3264,191 @@ describe('DecisionService', () => { expect(value[0].result.experiment?.id).toBe('2001'); }); - it('local holdout applies to delivery rules (rollouts) as well as experiment rules', async () => { - // delivery_1 has id '3001' + it('local holdout hit on a delivery rule continues to the next delivery rule, not straight to everyone else', async () => { const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('3001'))); const { decisionService } = getDecisionService(); - // bucket returns null for all experiments and the local holdout variation for delivery rule mockBucket.mockImplementation((params: BucketerParams) => { if (params.experimentId === 'local_holdout_id') { return { result: 'local_holdout_variation_id', reasons: [] }; } + if (params.experimentId === '3002') { + return { result: '5005', reasons: [] }; + } + if (params.experimentId === 'default-rollout-id') { + return { result: '5007', reasons: [] }; + } return { result: null, reasons: [] }; }); - // No audience attributes → experiments won't match, falls through to rollout - const user = new OptimizelyUserContext({ - optimizely: {} as any, - userId: 'user1', - attributes: { age: 15 }, // satisfies 4001 used by delivery_1 + 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(); + + const hit = localHit(config); + expect(value[0].result).toEqual({ + experiment: config.experimentIdMap['3002'], + variation: config.variationIdMap['5005'], + decisionSource: DECISION_SOURCES.ROLLOUT, + holdout: hit, + holdouts: [hit], }); + }); + + it('one local holdout targeting two rules records a hit for each rule', async () => { + const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001', ['2001', '3001']))); + const { decisionService } = getDecisionService(); + + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + if (params.experimentId === '3002') { + return { result: '5005', 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(); - // Should be a holdout decision from the local holdout targeting the delivery rule - expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT); - expect(value[0].result.experiment?.id).toBe('local_holdout_id'); + expect(value[0].result.experiment?.id).toBe('3002'); + expect(value[0].result.holdouts).toEqual([localHit(config), localHit(config)]); + }); + + it('holdouts list is ordered by evaluation: experiment-rule hits before delivery-rule hits, and holdout is the first entry', async () => { + const datafile = makeLocalHoldoutDatafile('2002'); + (datafile as any).localHoldouts.push({ + ...(datafile as any).localHoldouts[0], + id: 'local_holdout_2_id', + key: 'local_holdout_2', + includedRules: ['3001'], + variations: [{ id: 'local_holdout_2_variation_id', key: 'local_holdout_2_variation', variables: [] }], + trafficAllocation: [{ entityId: 'local_holdout_2_variation_id', endOfRange: 10000 }], + }); + 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: [] }; + } + if (params.experimentId === 'local_holdout_2_id') { + return { result: 'local_holdout_2_variation_id', reasons: [] }; + } + if (params.experimentId === '3002') { + return { result: '5005', 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(); + + const expHit = localHit(config); + const deliveryHit = { + experiment: config.holdoutIdMap!['local_holdout_2_id'], + variation: config.variationIdMap['local_holdout_2_variation_id'], + }; + expect(value[0].result.experiment?.id).toBe('3002'); + expect(value[0].result.holdouts).toEqual([expHit, deliveryHit]); + expect(value[0].result.holdout).toEqual(expHit); + }); + + it('global excludeTargetedDeliveries holdout hit is listed before local delivery-rule hits', async () => { + const datafile = makeLocalHoldoutDatafile('3001'); + (datafile as any).holdouts = [{ + id: 'global_holdout_id', + key: 'global_holdout', + status: 'Running', + includedFlags: [], + excludedFlags: [], + excludeTargetedDeliveries: true, + audienceIds: [], + audienceConditions: [], + variations: [{ id: 'global_holdout_var_id', key: 'global_holdout_var', variables: [] }], + trafficAllocation: [{ entityId: 'global_holdout_var_id', endOfRange: 10000 }], + }]; + const config = createProjectConfig(JSON.stringify(datafile)); + const { decisionService } = getDecisionService(); + + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === 'global_holdout_id') { + return { result: 'global_holdout_var_id', reasons: [] }; + } + if (params.experimentId === 'local_holdout_id') { + return { result: 'local_holdout_variation_id', reasons: [] }; + } + if (params.experimentId === '3002') { + return { result: '5005', 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(); + + const globalHit = { + experiment: config.holdoutIdMap!['global_holdout_id'], + variation: config.variationIdMap['global_holdout_var_id'], + }; + expect(value[0].result.experiment?.id).toBe('3002'); + expect(value[0].result.holdouts).toEqual([globalHit, localHit(config)]); + expect(value[0].result.holdout).toEqual(globalHit); + }); + + it('when several local holdouts match one rule only the first is recorded', async () => { + const datafile = makeLocalHoldoutDatafile('2001'); + (datafile as any).localHoldouts.push({ + ...(datafile as any).localHoldouts[0], + id: 'local_holdout_2_id', + key: 'local_holdout_2', + variations: [{ id: 'local_holdout_2_variation_id', key: 'local_holdout_2_variation', variables: [] }], + trafficAllocation: [{ entityId: 'local_holdout_2_variation_id', endOfRange: 10000 }], + }); + 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: [] }; + } + if (params.experimentId === 'local_holdout_2_id') { + return { result: 'local_holdout_2_variation_id', reasons: [] }; + } + return { result: null, reasons: [] }; + }); + + const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1' }); + const feature = config.featureKeyMap['flag_1']; + const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get(); + + expect(value[0].result.holdouts).toEqual([localHit(config)]); + }); + + it('user with no local holdout hit gets no holdout fields on the decision', async () => { + const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001'))); + const { decisionService } = getDecisionService(); + + mockBucket.mockImplementation((params: BucketerParams) => { + if (params.experimentId === '2001') { + return { result: '5001', 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(); + + expect(value[0].result).toEqual({ + experiment: config.experimentIdMap['2001'], + variation: config.variationIdMap['5001'], + decisionSource: DECISION_SOURCES.FEATURE_TEST, + }); }); it('forced decision beats a 100% traffic local holdout: forced decision takes precedence over local holdout', async () => { @@ -3288,10 +3510,9 @@ describe('DecisionService', () => { 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'); + expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.ROLLOUT); + expect(value[0].result.variation).toBeNull(); + expect(value[0].result.holdouts).toEqual([localHit(config)]); }); }); }); diff --git a/lib/core/decision_service/index.ts b/lib/core/decision_service/index.ts index a7cfc3783..e57ec25c0 100644 --- a/lib/core/decision_service/index.ts +++ b/lib/core/decision_service/index.ts @@ -126,12 +126,16 @@ export interface DecisionObj { variation: Variation | null; decisionSource: DecisionSource; cmabUuid?: string; - holdout?: { - experiment: Holdout; - variation: Variation; - }; + /** @deprecated Use `holdouts`; holds the first entry. */ + holdout?: HoldoutHit; + holdouts?: HoldoutHit[]; } +export type HoldoutHit = { + experiment: Holdout; + variation: Variation; +}; + interface DecisionServiceOptions { userProfileService?: UserProfileService; userProfileServiceAsync?: UserProfileServiceAsync; @@ -970,7 +974,7 @@ export class DecisionService { // getGlobalHoldouts() returns holdouts with includedRules == null/undefined. const globalHoldouts = getGlobalHoldouts(configObj); - let appliedHoldout : DecisionObj['holdout'] | undefined = undefined; + let appliedHoldout: HoldoutHit | undefined = undefined; for (const holdout of globalHoldouts) { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); @@ -1004,10 +1008,18 @@ export class DecisionService { } }) : this.getVariationForFeatureExperiment(op, configObj, feature, user, decideOptions, userProfileTracker); + const withHoldouts = (result: DecisionObj, holdouts: HoldoutHit[]): DecisionObj => { + const all = appliedHoldout ? [appliedHoldout, ...holdouts] : holdouts; + return all.length ? { ...result, holdout: all[0], holdouts: all } : result; + }; + return experimentDecision.then((experimentDecision) => { + const experimentHoldouts = experimentDecision.result.holdouts ?? []; + if (experimentDecision.error || experimentDecision.result.variation !== null) { return Value.of(op, { ...experimentDecision, + result: withHoldouts(experimentDecision.result, experimentHoldouts), reasons: [...decideReasons, ...experimentDecision.reasons], }); } @@ -1027,12 +1039,8 @@ export class DecisionService { decideReasons.push([USER_NOT_IN_ROLLOUT, userId, feature.key]); } - if (appliedHoldout) { - rolloutDecisionResult.holdout = appliedHoldout; - } - return Value.of(op, { - result: rolloutDecisionResult, + result: withHoldouts(rolloutDecisionResult, [...experimentHoldouts, ...(rolloutDecisionResult.holdouts ?? [])]), reasons: decideReasons, }); }); @@ -1091,7 +1099,7 @@ export class DecisionService { }); } - return this.traverseFeatureExperimentList(op, configObj, feature, 0, user, [], decideOptions, userProfileTracker); + return this.traverseFeatureExperimentList(op, configObj, feature, 0, user, [], [], decideOptions, userProfileTracker); } private traverseFeatureExperimentList( @@ -1101,6 +1109,7 @@ export class DecisionService { fromIndex: number, user: OptimizelyUserContext, decideReasons: DecisionReason[], + holdouts: HoldoutHit[], decideOptions: DecideOptionsMap, userProfileTracker?: UserProfileTracker, ): Value { @@ -1111,6 +1120,7 @@ export class DecisionService { experiment: null, variation: null, decisionSource: DECISION_SOURCES.FEATURE_TEST, + ...(holdouts.length ? { holdouts } : {}), }, reasons: decideReasons, }); @@ -1119,7 +1129,7 @@ export class DecisionService { const experiment = getExperimentFromId(configObj, experimentIds[fromIndex], this.logger); if (!experiment) { return this.traverseFeatureExperimentList( - op, configObj, feature, fromIndex + 1, user, decideReasons, decideOptions, userProfileTracker); + op, configObj, feature, fromIndex + 1, user, decideReasons, holdouts, decideOptions, userProfileTracker); } const experimentResponse = this.getVariationFromExperimentRule( @@ -1129,12 +1139,11 @@ export class DecisionService { return experimentResponse.then((experimentResponse) => { decideReasons.push(...experimentResponse.reasons); - // If a local holdout was hit, return the holdout decision directly (preserves holdout as experiment). if (experimentResponse.holdoutDecision) { - return Value.of(op, { - result: experimentResponse.result, - reasons: decideReasons, - }); + const { experiment: holdout, variation } = experimentResponse.result; + holdouts.push({ experiment: holdout as Holdout, variation: variation as Variation }); + return this.traverseFeatureExperimentList( + op, configObj, feature, fromIndex + 1, user, decideReasons, holdouts, decideOptions, userProfileTracker); } if (experimentResponse.error) { @@ -1144,6 +1153,7 @@ export class DecisionService { experiment, variation: null, decisionSource: DECISION_SOURCES.FEATURE_TEST, + ...(holdouts.length ? { holdouts } : {}), }, reasons: decideReasons, }); @@ -1151,7 +1161,7 @@ export class DecisionService { if(!experimentResponse.result.variationKey) { return this.traverseFeatureExperimentList( - op, configObj, feature, fromIndex + 1, user, decideReasons, decideOptions, userProfileTracker); + op, configObj, feature, fromIndex + 1, user, decideReasons, holdouts, decideOptions, userProfileTracker); } const variationKey = experimentResponse.result.variationKey; @@ -1166,6 +1176,7 @@ export class DecisionService { experiment, variation, decisionSource: DECISION_SOURCES.FEATURE_TEST, + ...(holdouts.length ? { holdouts } : {}), }, reasons: decideReasons, }); @@ -1230,36 +1241,30 @@ export class DecisionService { }; } + const holdouts: HoldoutHit[] = []; let index = 0; while (index < rolloutRules.length) { const deliveryRuleResponse = this.getVariationFromDeliveryRule(configObj, feature.key, rolloutRules, index, user); decideReasons.push(...deliveryRuleResponse.reasons); - // If a local holdout was hit for this delivery rule, use its decision object directly. if (deliveryRuleResponse.holdoutDecision) { - return { - reasons: decideReasons, - result: deliveryRuleResponse.result, - } + const { experiment: holdout, variation } = deliveryRuleResponse.result; + holdouts.push({ experiment: holdout as Holdout, variation: variation as Variation }); + index++; + continue; } - const variation = deliveryRuleResponse.result; const skipToEveryoneElse = deliveryRuleResponse.skipToEveryoneElse; if (variation) { - // if (decisionVariation.localHoldoutDecision) { - // return { - // result: decisionVariation.localHoldoutDecision, - // reasons: decideReasons, - // }; - // } const rolloutRule = configObj.experimentIdMap[rolloutRules[index].id]; const decisionObj = { experiment: rolloutRule, variation: variation, - decisionSource: DECISION_SOURCES.ROLLOUT + decisionSource: DECISION_SOURCES.ROLLOUT, + ...(holdouts.length ? { holdouts } : {}), }; return { result: decisionObj, @@ -1274,6 +1279,7 @@ export class DecisionService { experiment: null, variation: null, decisionSource: DECISION_SOURCES.ROLLOUT, + ...(holdouts.length ? { holdouts } : {}), }; return { @@ -1641,7 +1647,6 @@ export class DecisionService { const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user); decideReasons.push(...holdoutDecision.reasons); if (holdoutDecision.result.variation) { - // Signal the caller to use the holdout decision directly, preserving the holdout as experiment. return Value.of(op, { result: holdoutDecision.result, reasons: decideReasons, diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 07f31dcc1..500178abe 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -1,5 +1,5 @@ /** - * Copyright 2024-2025, Optimizely + * Copyright 2024-2026, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -932,6 +932,97 @@ describe('Optimizely', () => { }) ); }); + + describe('local holdouts', () => { + const makeLocalHoldout = (id: string, includedRules: string[]) => ({ + ...JSON.parse(JSON.stringify(holdoutData[0])), + id, + key: `${id}_key`, + includedRules, + variations: [{ id: `${id}_var`, key: `${id}_var_key`, variables: [], featureEnabled: false }], + trafficAllocation: [{ entityId: `${id}_var`, endOfRange: 10000 }], + }); + + const setup = (localHoldouts: any[], sendFlagDecisions = true) => { + const datafile: any = getDecisionTestDatafile(); + datafile.holdouts = []; + datafile.localHoldouts = localHoldouts; + datafile.sendFlagDecisions = sendFlagDecisions; + const eventProcessor = getForwardingEventProcessor({ + dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), + }); + const processSpy = vi.spyOn(eventProcessor, 'process'); + const optimizely = new Optimizely({ + clientEngine: 'node-sdk', + projectConfigManager: getMockProjectConfigManager({ + initConfig: createProjectConfig(JSON.stringify(datafile)), + }), + eventProcessor, + jsonSchemaValidator, + logger, + odpManager, + disposable: true, + cmabService: {} as any + }); + const holdoutEvents = () => processSpy.mock.calls + .map((call: any) => call[0] as ImpressionEvent) + .filter((event) => event.ruleType === 'holdout'); + return { optimizely, processSpy, holdoutEvents }; + }; + + const makeUser = (optimizely: any, attributes = {}) => + new OptimizelyUserContext({ optimizely, userId: 'test_user', attributes }); + + it('should send one holdout impression per held-out rule', async () => { + const { optimizely, holdoutEvents } = setup([ + makeLocalHoldout('lh_exp', ['2001']), + makeLocalHoldout('lh_delivery', ['3001']), + ]); + + const decision = await optimizely.decideAsync(makeUser(optimizely), 'flag_1', []); + + expect(decision.ruleKey).toBe('default-rollout-key'); + expect(holdoutEvents().map((e) => e.ruleKey)).toEqual(['lh_exp_key', 'lh_delivery_key']); + expect(holdoutEvents().every((e) => e.enabled === false)).toBe(true); + }); + + it('should send two holdout impressions when one local holdout targets two held-out rules', async () => { + const { optimizely, holdoutEvents } = setup([makeLocalHoldout('lh', ['2001', '3001'])]); + + await optimizely.decideAsync(makeUser(optimizely), 'flag_1', []); + + expect(holdoutEvents().map((e) => e.ruleKey)).toEqual(['lh_key', 'lh_key']); + }); + + it('should send the holdout impression for a delivery-rule hit when sendFlagDecisions is false', async () => { + const { optimizely, processSpy, holdoutEvents } = setup([makeLocalHoldout('lh', ['3001'])], false); + + const decision = await optimizely.decideAsync(makeUser(optimizely), 'flag_1', []); + + expect(decision.ruleKey).toBe('default-rollout-key'); + expect(holdoutEvents().map((e) => e.ruleKey)).toEqual(['lh_key']); + expect(processSpy).toHaveBeenCalledOnce(); + }); + + it('should not send local holdout impressions when DISABLE_DECISION_EVENT is used', async () => { + const { optimizely, processSpy } = setup([makeLocalHoldout('lh', ['2001', '3001'])]); + + await optimizely.decideAsync(makeUser(optimizely), 'flag_1', [OptimizelyDecideOption.DISABLE_DECISION_EVENT]); + + expect(processSpy).not.toHaveBeenCalled(); + }); + + it('should skip the held-out rule and send no holdout impression for isFeatureEnabled', () => { + const { optimizely, processSpy, holdoutEvents } = setup([makeLocalHoldout('lh', ['2001'])]); + + const enabled = optimizely.isFeatureEnabled('flag_1', 'test_user', { age: 15 }); + + expect(enabled).toBe(true); + expect(holdoutEvents()).toEqual([]); + expect(processSpy).toHaveBeenCalledOnce(); + expect((processSpy.mock.calls[0][0] as ImpressionEvent).ruleKey).toBe('exp_2'); + }); + }); }); it('should flush eventProcessor and odpManager on flushImmediately()', async () => { diff --git a/lib/optimizely/index.ts b/lib/optimizely/index.ts index c7277ba65..ce1fa9451 100644 --- a/lib/optimizely/index.ts +++ b/lib/optimizely/index.ts @@ -1542,20 +1542,23 @@ 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, - decision.getFeatureEnabledFromVariation(holdoutDecisionObj), - attributes, - ); - decisionEventDispatched = true; + const holdoutHits = decisionObj.holdouts ?? (decisionObj.holdout ? [decisionObj.holdout] : []); + if (!options[OptimizelyDecideOption.DISABLE_DECISION_EVENT]) { + for (const hit of holdoutHits) { + const holdoutDecisionObj: DecisionObj = { + experiment: hit.experiment, + variation: hit.variation, + decisionSource: DECISION_SOURCES.HOLDOUT, + }; + this.sendImpressionEvent( + holdoutDecisionObj, + key, + userId, + decision.getFeatureEnabledFromVariation(holdoutDecisionObj), + attributes, + ); + decisionEventDispatched = true; + } } const shouldIncludeReasons = options[OptimizelyDecideOption.INCLUDE_REASONS]; From 6f26cb1a4006965b0b67584e5eee1efbc03d109f Mon Sep 17 00:00:00 2001 From: Alex Yong Date: Fri, 16 Oct 2026 17:57:15 -0400 Subject: [PATCH 2/2] [AI-FSSDK] [FSSDK-12958] Test decision error discards local holdout events --- lib/optimizely/index.spec.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/optimizely/index.spec.ts b/lib/optimizely/index.spec.ts index 500178abe..3aa13a11d 100644 --- a/lib/optimizely/index.spec.ts +++ b/lib/optimizely/index.spec.ts @@ -943,11 +943,12 @@ describe('Optimizely', () => { trafficAllocation: [{ entityId: `${id}_var`, endOfRange: 10000 }], }); - const setup = (localHoldouts: any[], sendFlagDecisions = true) => { + const setup = (localHoldouts: any[], sendFlagDecisions = true, cmabService: any = {}) => { const datafile: any = getDecisionTestDatafile(); datafile.holdouts = []; datafile.localHoldouts = localHoldouts; datafile.sendFlagDecisions = sendFlagDecisions; + datafile.experiments.find((e: any) => e.key === 'exp_3').cmab.trafficAllocation = 10000; const eventProcessor = getForwardingEventProcessor({ dispatchEvent: vi.fn(() => Promise.resolve({ statusCode: 200 })), }); @@ -962,7 +963,7 @@ describe('Optimizely', () => { logger, odpManager, disposable: true, - cmabService: {} as any + cmabService, }); const holdoutEvents = () => processSpy.mock.calls .map((call: any) => call[0] as ImpressionEvent) @@ -1012,6 +1013,22 @@ describe('Optimizely', () => { expect(processSpy).not.toHaveBeenCalled(); }); + it('should return an error decision and send no impressions when a later rule errors after a local holdout hit', async () => { + const cmabService = { getDecision: vi.fn().mockRejectedValue(new Error('cmab fetch failed')) }; + const { optimizely, processSpy } = setup([makeLocalHoldout('lh', ['2001'])], true, cmabService); + const user = new OptimizelyUserContext({ optimizely, userId: 'tester', attributes: { country: 'BD', age: 80 } }); + + const decision = await optimizely.decideAsync(user, 'flag_1', [OptimizelyDecideOption.INCLUDE_REASONS]); + + expect(cmabService.getDecision).toHaveBeenCalledOnce(); + expect(decision.variationKey).toBeNull(); + expect(decision.enabled).toBe(false); + expect(decision.ruleKey).toBeNull(); + expect(decision.reasons).toContain('Failed to fetch CMAB data for experiment exp_3.'); + expect(decision.reasons.some((r) => r.includes('lh_key'))).toBe(true); + expect(processSpy).not.toHaveBeenCalled(); + }); + it('should skip the held-out rule and send no holdout impression for isFeatureEnabled', () => { const { optimizely, processSpy, holdoutEvents } = setup([makeLocalHoldout('lh', ['2001'])]);