Skip to content

Commit 8941c2c

Browse files
authored
[AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit (#1181)
* [AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit * [AI-FSSDK] [FSSDK-12958] Test decision error discards local holdout events * [AI-FSSDK] [FSSDK-12958] Drop deprecated DecisionObj.holdout; test legacy holdout events
1 parent 11bd171 commit 8941c2c

4 files changed

Lines changed: 448 additions & 87 deletions

File tree

‎lib/core/decision_service/index.spec.ts‎

Lines changed: 248 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2025, Optimizely
2+
* Copyright 2025-2026, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -2212,6 +2212,11 @@ describe('DecisionService', () => {
22122212
return datafile;
22132213
};
22142214

2215+
const globalHit = (config: ProjectConfig) => ({
2216+
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2217+
variation: config.variationIdMap['holdout_variation_running_id'],
2218+
});
2219+
22152220
it('should return holdout variation immediately when excludeTargetedDeliveries is false', async () => {
22162221
const { decisionService } = getDecisionService();
22172222
const config = createProjectConfig(JSON.stringify(getExcludeTDDatafile(false)));
@@ -2281,10 +2286,7 @@ describe('DecisionService', () => {
22812286
experiment: config.experimentKeyMap['delivery_1'],
22822287
variation: config.variationIdMap['5004'],
22832288
decisionSource: DECISION_SOURCES.ROLLOUT,
2284-
holdout: {
2285-
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2286-
variation: config.variationIdMap['holdout_variation_running_id'],
2287-
},
2289+
holdouts: [globalHit(config)],
22882290
});
22892291

22902292
const reasonStrings = variation.reasons.map((r: any) => typeof r === 'string' ? r : r[0]);
@@ -2317,10 +2319,7 @@ describe('DecisionService', () => {
23172319
experiment: null,
23182320
variation: null,
23192321
decisionSource: DECISION_SOURCES.ROLLOUT,
2320-
holdout: {
2321-
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2322-
variation: config.variationIdMap['holdout_variation_running_id'],
2323-
},
2322+
holdouts: [globalHit(config)],
23242323
});
23252324
});
23262325

@@ -2353,10 +2352,7 @@ describe('DecisionService', () => {
23532352
experiment: null,
23542353
variation: null,
23552354
decisionSource: DECISION_SOURCES.ROLLOUT,
2356-
holdout: {
2357-
experiment: config.holdoutIdMap && config.holdoutIdMap['holdout_running_id'],
2358-
variation: config.variationIdMap['holdout_variation_running_id'],
2359-
},
2355+
holdouts: [globalHit(config)],
23602356
});
23612357
});
23622358
});
@@ -3076,6 +3072,11 @@ describe('DecisionService', () => {
30763072
return datafile;
30773073
};
30783074

3075+
const localHit = (config: ProjectConfig) => ({
3076+
experiment: config.holdoutIdMap!['local_holdout_id'],
3077+
variation: config.variationIdMap['local_holdout_variation_id'],
3078+
});
3079+
30793080
beforeEach(() => {
30803081
mockBucket.mockReset();
30813082
});
@@ -3118,12 +3119,10 @@ describe('DecisionService', () => {
31183119
expect(value[0].result.experiment?.id).toBe('global_holdout_id');
31193120
});
31203121

3121-
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 () => {
3122-
// exp_1 has id '2001'
3122+
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 () => {
31233123
const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001')));
31243124
const { decisionService } = getDecisionService();
31253125

3126-
// bucket returns holdout variation when evaluating the local holdout
31273126
mockBucket.mockImplementation((params: BucketerParams) => {
31283127
if (params.experimentId === 'local_holdout_id') {
31293128
return { result: 'local_holdout_variation_id', reasons: [] };
@@ -3135,10 +3134,68 @@ describe('DecisionService', () => {
31353134
const feature = config.featureKeyMap['flag_1'];
31363135
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
31373136

3138-
// Should return holdout decision for the local holdout
3139-
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT);
3140-
expect(value[0].result.experiment?.id).toBe('local_holdout_id');
3141-
expect(value[0].result.variation?.id).toBe('local_holdout_variation_id');
3137+
const hit = localHit(config);
3138+
expect(value[0].result).toEqual({
3139+
experiment: null,
3140+
variation: null,
3141+
decisionSource: DECISION_SOURCES.ROLLOUT,
3142+
holdouts: [hit],
3143+
});
3144+
expect(mockBucket.mock.calls.some(([p]) => p.experimentId === '2001')).toBe(false);
3145+
});
3146+
3147+
it('local holdout hit on an experiment rule continues to the next experiment rule, which serves', async () => {
3148+
const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001')));
3149+
const { decisionService } = getDecisionService();
3150+
3151+
mockBucket.mockImplementation((params: BucketerParams) => {
3152+
if (params.experimentId === 'local_holdout_id') {
3153+
return { result: 'local_holdout_variation_id', reasons: [] };
3154+
}
3155+
if (params.experimentId === '2001') {
3156+
return { result: '5001', reasons: [] };
3157+
}
3158+
if (params.experimentId === '2002') {
3159+
return { result: '5002', reasons: [] };
3160+
}
3161+
return { result: null, reasons: [] };
3162+
});
3163+
3164+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
3165+
const feature = config.featureKeyMap['flag_1'];
3166+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3167+
3168+
const hit = localHit(config);
3169+
expect(value[0].result).toEqual({
3170+
experiment: config.experimentIdMap['2002'],
3171+
variation: config.variationIdMap['5002'],
3172+
decisionSource: DECISION_SOURCES.FEATURE_TEST,
3173+
holdouts: [hit],
3174+
});
3175+
});
3176+
3177+
it('local holdout hit on an experiment rule continues to a delivery rule when no later experiment serves', async () => {
3178+
const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001')));
3179+
const { decisionService } = getDecisionService();
3180+
3181+
mockBucket.mockImplementation((params: BucketerParams) => {
3182+
if (params.experimentId === 'local_holdout_id') {
3183+
return { result: 'local_holdout_variation_id', reasons: [] };
3184+
}
3185+
if (params.experimentId === '3001') {
3186+
return { result: '5004', reasons: [] };
3187+
}
3188+
return { result: null, reasons: [] };
3189+
});
3190+
3191+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
3192+
const feature = config.featureKeyMap['flag_1'];
3193+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3194+
3195+
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.ROLLOUT);
3196+
expect(value[0].result.experiment?.id).toBe('3001');
3197+
expect(value[0].result.variation?.id).toBe('5004');
3198+
expect(value[0].result.holdouts).toEqual([localHit(config)]);
31423199
});
31433200

31443201
it('local holdout miss branch: user not bucketed into local holdout falls through to regular rule evaluation', async () => {
@@ -3202,31 +3259,188 @@ describe('DecisionService', () => {
32023259
expect(value[0].result.experiment?.id).toBe('2001');
32033260
});
32043261

3205-
it('local holdout applies to delivery rules (rollouts) as well as experiment rules', async () => {
3206-
// delivery_1 has id '3001'
3262+
it('local holdout hit on a delivery rule continues to the next delivery rule, not straight to everyone else', async () => {
32073263
const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('3001')));
32083264
const { decisionService } = getDecisionService();
32093265

3210-
// bucket returns null for all experiments and the local holdout variation for delivery rule
32113266
mockBucket.mockImplementation((params: BucketerParams) => {
32123267
if (params.experimentId === 'local_holdout_id') {
32133268
return { result: 'local_holdout_variation_id', reasons: [] };
32143269
}
3270+
if (params.experimentId === '3002') {
3271+
return { result: '5005', reasons: [] };
3272+
}
3273+
if (params.experimentId === 'default-rollout-id') {
3274+
return { result: '5007', reasons: [] };
3275+
}
32153276
return { result: null, reasons: [] };
32163277
});
32173278

3218-
// No audience attributes → experiments won't match, falls through to rollout
3219-
const user = new OptimizelyUserContext({
3220-
optimizely: {} as any,
3221-
userId: 'user1',
3222-
attributes: { age: 15 }, // satisfies 4001 used by delivery_1
3279+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
3280+
const feature = config.featureKeyMap['flag_1'];
3281+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3282+
3283+
const hit = localHit(config);
3284+
expect(value[0].result).toEqual({
3285+
experiment: config.experimentIdMap['3002'],
3286+
variation: config.variationIdMap['5005'],
3287+
decisionSource: DECISION_SOURCES.ROLLOUT,
3288+
holdouts: [hit],
32233289
});
3290+
});
3291+
3292+
it('one local holdout targeting two rules records a hit for each rule', async () => {
3293+
const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001', ['2001', '3001'])));
3294+
const { decisionService } = getDecisionService();
3295+
3296+
mockBucket.mockImplementation((params: BucketerParams) => {
3297+
if (params.experimentId === 'local_holdout_id') {
3298+
return { result: 'local_holdout_variation_id', reasons: [] };
3299+
}
3300+
if (params.experimentId === '3002') {
3301+
return { result: '5005', reasons: [] };
3302+
}
3303+
return { result: null, reasons: [] };
3304+
});
3305+
3306+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
32243307
const feature = config.featureKeyMap['flag_1'];
32253308
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
32263309

3227-
// Should be a holdout decision from the local holdout targeting the delivery rule
3228-
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.HOLDOUT);
3229-
expect(value[0].result.experiment?.id).toBe('local_holdout_id');
3310+
expect(value[0].result.experiment?.id).toBe('3002');
3311+
expect(value[0].result.holdouts).toEqual([localHit(config), localHit(config)]);
3312+
});
3313+
3314+
it('holdouts list is ordered by evaluation: experiment-rule hits before delivery-rule hits, and holdout is the first entry', async () => {
3315+
const datafile = makeLocalHoldoutDatafile('2002');
3316+
(datafile as any).localHoldouts.push({
3317+
...(datafile as any).localHoldouts[0],
3318+
id: 'local_holdout_2_id',
3319+
key: 'local_holdout_2',
3320+
includedRules: ['3001'],
3321+
variations: [{ id: 'local_holdout_2_variation_id', key: 'local_holdout_2_variation', variables: [] }],
3322+
trafficAllocation: [{ entityId: 'local_holdout_2_variation_id', endOfRange: 10000 }],
3323+
});
3324+
const config = createProjectConfig(JSON.stringify(datafile));
3325+
const { decisionService } = getDecisionService();
3326+
3327+
mockBucket.mockImplementation((params: BucketerParams) => {
3328+
if (params.experimentId === 'local_holdout_id') {
3329+
return { result: 'local_holdout_variation_id', reasons: [] };
3330+
}
3331+
if (params.experimentId === 'local_holdout_2_id') {
3332+
return { result: 'local_holdout_2_variation_id', reasons: [] };
3333+
}
3334+
if (params.experimentId === '3002') {
3335+
return { result: '5005', reasons: [] };
3336+
}
3337+
return { result: null, reasons: [] };
3338+
});
3339+
3340+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
3341+
const feature = config.featureKeyMap['flag_1'];
3342+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3343+
3344+
const expHit = localHit(config);
3345+
const deliveryHit = {
3346+
experiment: config.holdoutIdMap!['local_holdout_2_id'],
3347+
variation: config.variationIdMap['local_holdout_2_variation_id'],
3348+
};
3349+
expect(value[0].result.experiment?.id).toBe('3002');
3350+
expect(value[0].result.holdouts).toEqual([expHit, deliveryHit]);
3351+
});
3352+
3353+
it('global excludeTargetedDeliveries holdout hit is listed before local delivery-rule hits', async () => {
3354+
const datafile = makeLocalHoldoutDatafile('3001');
3355+
(datafile as any).holdouts = [{
3356+
id: 'global_holdout_id',
3357+
key: 'global_holdout',
3358+
status: 'Running',
3359+
includedFlags: [],
3360+
excludedFlags: [],
3361+
excludeTargetedDeliveries: true,
3362+
audienceIds: [],
3363+
audienceConditions: [],
3364+
variations: [{ id: 'global_holdout_var_id', key: 'global_holdout_var', variables: [] }],
3365+
trafficAllocation: [{ entityId: 'global_holdout_var_id', endOfRange: 10000 }],
3366+
}];
3367+
const config = createProjectConfig(JSON.stringify(datafile));
3368+
const { decisionService } = getDecisionService();
3369+
3370+
mockBucket.mockImplementation((params: BucketerParams) => {
3371+
if (params.experimentId === 'global_holdout_id') {
3372+
return { result: 'global_holdout_var_id', reasons: [] };
3373+
}
3374+
if (params.experimentId === 'local_holdout_id') {
3375+
return { result: 'local_holdout_variation_id', reasons: [] };
3376+
}
3377+
if (params.experimentId === '3002') {
3378+
return { result: '5005', reasons: [] };
3379+
}
3380+
return { result: null, reasons: [] };
3381+
});
3382+
3383+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
3384+
const feature = config.featureKeyMap['flag_1'];
3385+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3386+
3387+
const globalHit = {
3388+
experiment: config.holdoutIdMap!['global_holdout_id'],
3389+
variation: config.variationIdMap['global_holdout_var_id'],
3390+
};
3391+
expect(value[0].result.experiment?.id).toBe('3002');
3392+
expect(value[0].result.holdouts).toEqual([globalHit, localHit(config)]);
3393+
});
3394+
3395+
it('when several local holdouts match one rule only the first is recorded', async () => {
3396+
const datafile = makeLocalHoldoutDatafile('2001');
3397+
(datafile as any).localHoldouts.push({
3398+
...(datafile as any).localHoldouts[0],
3399+
id: 'local_holdout_2_id',
3400+
key: 'local_holdout_2',
3401+
variations: [{ id: 'local_holdout_2_variation_id', key: 'local_holdout_2_variation', variables: [] }],
3402+
trafficAllocation: [{ entityId: 'local_holdout_2_variation_id', endOfRange: 10000 }],
3403+
});
3404+
const config = createProjectConfig(JSON.stringify(datafile));
3405+
const { decisionService } = getDecisionService();
3406+
3407+
mockBucket.mockImplementation((params: BucketerParams) => {
3408+
if (params.experimentId === 'local_holdout_id') {
3409+
return { result: 'local_holdout_variation_id', reasons: [] };
3410+
}
3411+
if (params.experimentId === 'local_holdout_2_id') {
3412+
return { result: 'local_holdout_2_variation_id', reasons: [] };
3413+
}
3414+
return { result: null, reasons: [] };
3415+
});
3416+
3417+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1' });
3418+
const feature = config.featureKeyMap['flag_1'];
3419+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3420+
3421+
expect(value[0].result.holdouts).toEqual([localHit(config)]);
3422+
});
3423+
3424+
it('user with no local holdout hit gets no holdout fields on the decision', async () => {
3425+
const config = createProjectConfig(JSON.stringify(makeLocalHoldoutDatafile('2001')));
3426+
const { decisionService } = getDecisionService();
3427+
3428+
mockBucket.mockImplementation((params: BucketerParams) => {
3429+
if (params.experimentId === '2001') {
3430+
return { result: '5001', reasons: [] };
3431+
}
3432+
return { result: null, reasons: [] };
3433+
});
3434+
3435+
const user = new OptimizelyUserContext({ optimizely: {} as any, userId: 'user1', attributes: { age: 15 } });
3436+
const feature = config.featureKeyMap['flag_1'];
3437+
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
3438+
3439+
expect(value[0].result).toEqual({
3440+
experiment: config.experimentIdMap['2001'],
3441+
variation: config.variationIdMap['5001'],
3442+
decisionSource: DECISION_SOURCES.FEATURE_TEST,
3443+
});
32303444
});
32313445

32323446
it('forced decision beats a 100% traffic local holdout: forced decision takes precedence over local holdout', async () => {
@@ -3288,10 +3502,9 @@ describe('DecisionService', () => {
32883502
const feature = config.featureKeyMap['flag_1'];
32893503
const value = await decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();
32903504

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');
3505+
expect(value[0].result.decisionSource).toBe(DECISION_SOURCES.ROLLOUT);
3506+
expect(value[0].result.variation).toBeNull();
3507+
expect(value[0].result.holdouts).toEqual([localHit(config)]);
32953508
});
32963509
});
32973510
});

0 commit comments

Comments
 (0)