Skip to content

[AI-FSSDK] [FSSDK-12368] Remove legacy flag-level holdout fields #1146

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

Merged
raju-opti merged 4 commits into from
May 7, 2026
Merged

[AI-FSSDK] [FSSDK-12368] Remove legacy flag-level holdout fields #1146

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
168a07b
[FSSDK-12368] Remove legacy flag-level holdout fields
Mat001 Apr 21, 2026
bb87136
cleanup
raju-opti May 7, 2026
795ed3a
update
raju-opti May 7, 2026
54d08a7
up
raju-opti May 7, 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
90 changes: 3 additions & 87 deletions lib/core/decision_service/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1960,63 +1960,15 @@ describe('DecisionService', () => {
});
});

it("should consider global holdout even if local holdout is present", async () => {
const { decisionService } = getDecisionService();
const datafile = getHoldoutTestDatafile();
const newEntry = {
id: 'holdout_included_id',
key: 'holdout_included',
status: 'Running',
includedFlags: ['1001'],
excludedFlags: [],
audienceIds: ['4002'], // age_40 audience
audienceConditions: ['or', '4002'],
variations: [
{
id: 'holdout_variation_included_id',
key: 'holdout_variation_included',
variables: [],
},
],
trafficAllocation: [
{
entityId: 'holdout_variation_included_id',
endOfRange: 5000,
},
],
};
datafile.holdouts = [newEntry, ...datafile.holdouts];
const config = createProjectConfig(datafile);
const user = new OptimizelyUserContext({
optimizely: {} as any,
userId: 'tester',
attributes: {
age: 20, // satisfies both global holdout (age_22) and included holdout (age_40) audiences
},
});
const feature = config.featureKeyMap['flag_1'];
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();

expect(value).toBeInstanceOf(Promise);

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 consider local holdout if misses global holdout", async () => {
it("should consider next global holdout if misses previous holdouts", async () => {
const { decisionService } = getDecisionService();
const datafile = getHoldoutTestDatafile();

datafile.holdouts.push({
id: 'holdout_included_specific_id',
key: 'holdout_included_specific',
status: 'Running',
includedFlags: ['1001'],
includedFlags: [],
excludedFlags: [],
audienceIds: ['4002'], // age_60 audience (age <= 60)
audienceConditions: ['or', '4002'],
Expand All @@ -2039,7 +1991,7 @@ describe('DecisionService', () => {
optimizely: {} as any,
userId: 'test_holdout_user',
attributes: {
age: 50, // Does not satisfy global holdout (age_22, age <= 22) but satisfies included holdout (age_60, age <= 60)
age: 50, // Does not satisfy first global holdout (age_22, age <= 22) but satisfies newly added holdout (age_60, age <= 60)
},
});
const feature = config.featureKeyMap['flag_1'];
Expand Down Expand Up @@ -2195,42 +2147,6 @@ describe('DecisionService', () => {
});
});

it('should skip holdouts excluded for specific flags', async () => {
const { decisionService } = getDecisionService();
const datafile = getHoldoutTestDatafile();

datafile.holdouts = datafile.holdouts.map((holdout: any) => {
if(holdout.id === 'holdout_running_id') {
return {
...holdout,
excludedFlags: ['1001']
}
}
return holdout;
});

const config = createProjectConfig(datafile);
const user = new OptimizelyUserContext({
optimizely: {} as any,
userId: 'tester',
attributes: {
age: 15, // satisfies age_22 audience condition (age <= 22) for global holdout, but holdout excludes flag_1
},
});
const feature = config.featureKeyMap['flag_1'];
const value = decisionService.resolveVariationsForFeatureList('async', config, [feature], user, {}).get();

expect(value).toBeInstanceOf(Promise);

const variation = (await value)[0];

expect(variation.result).toEqual({
experiment: config.experimentKeyMap['exp_1'],
variation: config.variationIdMap['5001'],
decisionSource: DECISION_SOURCES.FEATURE_TEST,
});
});

it('should handle multiple holdouts and use first matching one', async () => {
const { decisionService } = getDecisionService();
const datafile = getHoldoutTestDatafile();
Expand Down
6 changes: 4 additions & 2 deletions lib/core/decision_service/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
getVariationKeyFromId,
isActive,
ProjectConfig,
getHoldoutsForFlag,
} from '../../project_config/project_config';
import { AudienceEvaluator, createAudienceEvaluator } from '../audience_evaluator';
import * as stringValidator from '../../utils/string_value_validator';
Expand Down Expand Up @@ -943,7 +942,10 @@ export class DecisionService {
reasons: decideReasons,
});
}
const holdouts = getHoldoutsForFlag(configObj, feature.key);

// all global holouts should be evaluated for all flags
// global holdouts are available in configObj.holdouts
const { holdouts } = configObj;

for (const holdout of holdouts) {
const holdoutDecision = this.getVariationForHoldout(configObj, holdout, user);
Expand Down
54 changes: 3 additions & 51 deletions lib/project_config/project_config.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import testDatafile from '../tests/test_data';
import configValidator from '../utils/config_validator';
import { FEATURE_VARIABLE_TYPES } from '../utils/enums';
import { keyBy, sprintf } from '../utils/fns';
import projectConfig, { ProjectConfig, getHoldoutsForFlag } from './project_config';
import projectConfig, { ProjectConfig } from './project_config';

const cloneDeep = (obj: any) => JSON.parse(JSON.stringify(obj));
const logger = getMockLogger();
Expand Down Expand Up @@ -329,7 +329,7 @@ const getHoldoutDatafile = () => {
key: 'holdout_2',
status: 'Running',
includedFlags: [],
excludedFlags: ['44829230000'],
excludedFlags: [],
audienceIds: [],
audienceConditions: [],
variations: [
Expand All @@ -350,7 +350,7 @@ const getHoldoutDatafile = () => {
id: 'holdout_id_3',
key: 'holdout_3',
status: 'Draft',
includedFlags: ['4482920077'],
includedFlags: [],
excludedFlags: [],
audienceIds: [],
audienceConditions: [],
Expand Down Expand Up @@ -392,22 +392,6 @@ describe('createProjectConfig - holdouts', () => {
holdout_id_2: configObj.holdouts[1],
holdout_id_3: configObj.holdouts[2],
});

expect(configObj.globalHoldouts).toHaveLength(2);
expect(configObj.globalHoldouts).toEqual([
configObj.holdouts[0], // holdout_1 has empty includedFlags
configObj.holdouts[1] // holdout_2 has empty includedFlags
]);

expect(configObj.includedHoldouts).toEqual({
feature_1: [configObj.holdouts[2]], // holdout_3 includes feature_1 (ID: 4482920077)
});

expect(configObj.excludedHoldouts).toEqual({
feature_3: [configObj.holdouts[1]] // holdout_2 excludes feature_3 (ID: 44829230000)
});

expect(configObj.flagHoldoutsMap).toEqual({});
});

it('should handle empty holdouts array', function() {
Expand All @@ -417,10 +401,6 @@ describe('createProjectConfig - holdouts', () => {

expect(configObj.holdouts).toEqual([]);
expect(configObj.holdoutIdMap).toEqual({});
expect(configObj.globalHoldouts).toEqual([]);
expect(configObj.includedHoldouts).toEqual({});
expect(configObj.excludedHoldouts).toEqual({});
expect(configObj.flagHoldoutsMap).toEqual({});
});

it('should handle undefined includedFlags and excludedFlags in holdout', function() {
Expand All @@ -436,34 +416,6 @@ describe('createProjectConfig - holdouts', () => {
});
});

describe('getHoldoutsForFlag', () => {
it('should return all applicable holdouts for a flag', () => {
const datafile = getHoldoutDatafile();
const configObj = projectConfig.createProjectConfig(JSON.parse(JSON.stringify(datafile)));

const feature1Holdouts = getHoldoutsForFlag(configObj, 'feature_1');
expect(feature1Holdouts).toHaveLength(3);
expect(feature1Holdouts).toEqual([
configObj.holdouts[0],
configObj.holdouts[1],
configObj.holdouts[2],
]);

const feature2Holdouts = getHoldoutsForFlag(configObj, 'feature_2');
expect(feature2Holdouts).toHaveLength(2);
expect(feature2Holdouts).toEqual([
configObj.holdouts[0],
configObj.holdouts[1],
]);

const feature3Holdouts = getHoldoutsForFlag(configObj, 'feature_3');
expect(feature3Holdouts).toHaveLength(1);
expect(feature3Holdouts).toEqual([
configObj.holdouts[0],
]);
});
});

describe('getExperimentId', () => {
let testData: Record<string, any>;
let configObj: ProjectConfig;
Expand Down
65 changes: 5 additions & 60 deletions lib/project_config/project_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,6 @@ export interface ProjectConfig {
odpIntegrationConfig: OdpIntegrationConfig;
holdouts: Holdout[];
holdoutIdMap?: { [id: string]: Holdout };
globalHoldouts: Holdout[];
includedHoldouts: { [key: string]: Holdout[]; }
excludedHoldouts: { [key: string]: Holdout[]; }
flagHoldoutsMap: { [key: string]: Holdout[]; }
}

const EXPERIMENT_RUNNING_STATUS = 'Running';
Expand Down Expand Up @@ -390,70 +386,19 @@ const getEveryoneElseVariation = function(
const parseHoldoutsConfig = (projectConfig: ProjectConfig): void => {
projectConfig.holdouts = projectConfig.holdouts || [];
projectConfig.holdoutIdMap = keyBy(projectConfig.holdouts, 'id');
projectConfig.globalHoldouts = [];
projectConfig.includedHoldouts = {};
projectConfig.excludedHoldouts = {};
projectConfig.flagHoldoutsMap = {};

const featureFlagIdMap = keyBy(projectConfig.featureFlags, 'id');

projectConfig.holdouts.forEach((holdout) => {
if (!holdout.includedFlags) {
holdout.includedFlags = [];
}

if (!holdout.excludedFlags) {
holdout.excludedFlags = [];
}

// Original design of holdouts made use of the includeFlags and excludeFlags fields to identify local holdouts.
// But this was never released. In the current design, these fields are no longer used. These fields are kept
// and assigned empty array to keep the published type `Holdout` unchanged.
holdout.includedFlags = [];
holdout.excludedFlags = [];
holdout.variationKeyMap = keyBy(holdout.variations, 'key');

assignBy(holdout.variations, 'id', projectConfig.variationIdMap);

if (holdout.includedFlags.length === 0) {
projectConfig.globalHoldouts.push(holdout);

holdout.excludedFlags.forEach((flagId: string) => {
const flag = featureFlagIdMap[flagId];
if (flag) {
const flagKey = flag.key;
if (!projectConfig.excludedHoldouts[flagKey]) {
projectConfig.excludedHoldouts[flagKey] = [];
}
projectConfig.excludedHoldouts[flagKey].push(holdout);
}
});
} else {
holdout.includedFlags.forEach((flagId: string) => {
const flag = featureFlagIdMap[flagId];
if (flag) {
const flagKey = flag.key;
if (!projectConfig.includedHoldouts[flagKey]) {
projectConfig.includedHoldouts[flagKey] = [];
}
projectConfig.includedHoldouts[flagKey].push(holdout);
}
})
}
});
}

export const getHoldoutsForFlag = (projectConfig: ProjectConfig, flagKey: string): Holdout[] => {
if (projectConfig.flagHoldoutsMap[flagKey]) {
return projectConfig.flagHoldoutsMap[flagKey];
}

const flagHoldouts: Holdout[] = [
...projectConfig.globalHoldouts.filter((holdout) => {
return !(projectConfig.excludedHoldouts[flagKey] || []).includes(holdout);
}),
...(projectConfig.includedHoldouts[flagKey] || []),
];

projectConfig.flagHoldoutsMap[flagKey] = flagHoldouts;
return flagHoldouts;
}

/**
* Extract all audience segments used in this audience's conditions
* @param {Audience} audience Object representing the audience being parsed
Expand Down
8 changes: 2 additions & 6 deletions lib/shared_types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,8 @@ export interface Holdout extends ExperimentCore {
}

export function isHoldout(obj: Experiment | Holdout): obj is Holdout {
// Holdout has 'status', 'includedFlags', and 'excludedFlags' properties
return (
(obj as Holdout).status !== undefined &&
Array.isArray((obj as Holdout).includedFlags) &&
Array.isArray((obj as Holdout).excludedFlags)
);
// Holdout doesn't have 'layerId' property, while Experiment does
return (obj as Experiment).layerId === undefined;
}

export enum VariableType {
Expand Down
Loading