Merge pull request #1387 from guardian/tf-handle-missing-variants

Handle deleted variants in bandit data

Author: tomrf1

src/server/selection/banditData.ts

@@ -77,7 +77,7 @@ async function getBanditSamplesForTest(
     return parsedResults.data;
 }
 
-interface BanditVariantData {
+export interface BanditVariantData {
     variantName: string;
     mean: number;
 }

src/server/selection/epsilonGreedySelection.test.ts

@@ -126,4 +126,18 @@ describe('selectVariantWithHighestMean', () => {
         jest.spyOn(global.Math, 'random').mockReturnValue(0.8);
         expect(selectVariantWithHighestMean(banditData, epicTest)?.name).toEqual('v3');
     });
+
+    it('should ignore variants in bandit data that are not in the test configuration', () => {
+        const banditData = {
+            testName: 'example-1',
+            sortedVariants: [
+                { variantName: 'ghost', mean: 2 }, // not in test.variants
+                { variantName: 'v1', mean: 1 },
+                { variantName: 'v2', mean: 0.5 },
+                { variantName: 'v3', mean: 0.3 },
+            ],
+        };
+        const result = selectVariantWithHighestMean(banditData, epicTest);
+        expect(result?.name).toEqual('v1');
+    });
 });

src/server/selection/epsilonGreedySelection.ts

@@ -2,7 +2,7 @@ import type { Test, Variant } from '../../shared/types';
 import { putMetric } from '../utils/cloudwatch';
 import { logError } from '../utils/logging';
 import type { BanditData } from './banditData';
-import { selectRandomVariant } from './helpers';
+import { filterValidVariants, selectRandomVariant } from './helpers';
 
 /**
  * In general we select the best known variant, except with probability 'epsilon' when we select at random.
@@ -13,7 +13,8 @@ export function selectVariantWithHighestMean<V extends Variant, T extends Test<V
     testBanditData: BanditData,
     test: T,
 ): V | undefined {
-    const { sortedVariants } = testBanditData;
+    const sortedVariants = filterValidVariants(testBanditData.sortedVariants, test);
+
     // variants array is sorted by mean, use just the best variants
     const bestMean = sortedVariants[0].mean;
     const bestVariants = sortedVariants.findIndex((variant) => variant.mean < bestMean);

src/server/selection/helpers.ts

@@ -2,6 +2,7 @@ import seedrandom from 'seedrandom';
 import type { Test, Variant } from '../../shared/types';
 import { putMetric } from '../utils/cloudwatch';
 import { logError } from '../utils/logging';
+import type { BanditVariantData } from './banditData';
 
 export function selectRandomVariant<V extends Variant, T extends Test<V>>(test: T): V | undefined {
     const randomVariantIndex = Math.floor(Math.random() * test.variants.length);
@@ -20,3 +21,16 @@ export const getRandomNumber = (seed: string, mvtId: number | string = ''): numb
     const rng = seedrandom(mvtId + seed);
     return Math.abs(rng.int32());
 };
+
+/**
+ * It's possible for the variants in the bandit data to temporarily not match the variants in a test configuration.
+ * This can happen if a variant is deleted in the RRCP, and the cached bandit data still references the deleted variant.
+ * This function filters out any invalid variants.
+ */
+export const filterValidVariants = <V extends Variant, T extends Test<V>>(
+    sortedVariantsData: BanditVariantData[],
+    test: T,
+): BanditVariantData[] => {
+    const validVariantNames = new Set(test.variants.map((v) => v.name));
+    return sortedVariantsData.filter((v) => validVariantNames.has(v.variantName));
+};

src/server/selection/rouletteSelection.ts

@@ -1,6 +1,6 @@
 import type { Test, Variant } from '../../shared/types';
 import type { BanditData } from './banditData';
-import { selectRandomVariant } from './helpers';
+import { filterValidVariants, selectRandomVariant } from './helpers';
 
 export function selectVariantUsingRoulette<V extends Variant, T extends Test<V>>(
     test: T,
@@ -11,20 +11,20 @@ export function selectVariantUsingRoulette<V extends Variant, T extends Test<V>>
         return selectRandomVariant(test);
     }
 
-    const sumOfMeans = testBanditData.sortedVariants.reduce((sum, v) => sum + v.mean, 0);
+    const sortedVariants = filterValidVariants(testBanditData.sortedVariants, test);
+    const sumOfMeans = sortedVariants.reduce((sum, v) => sum + v.mean, 0);
 
     if (sumOfMeans <= 0) {
         return selectRandomVariant(test);
     }
 
     const minWeight = 0.1; // Ensure no variant gets less than 10%
-    const variantsWithWeights: Array<{ weight: number; variantName: string }> =
-        testBanditData.sortedVariants
-            .map(({ variantName, mean }) => ({
-                variantName,
-                weight: Math.max(mean / sumOfMeans, minWeight),
-            }))
-            .sort((a, b) => a.weight - b.weight);
+    const variantsWithWeights: Array<{ weight: number; variantName: string }> = sortedVariants
+        .map(({ variantName, mean }) => ({
+            variantName,
+            weight: Math.max(mean / sumOfMeans, minWeight),
+        }))
+        .sort((a, b) => a.weight - b.weight);
 
     // The sum of the weights may be greater than 1, so we now need to normalise them
     const sumOfWeights = variantsWithWeights.reduce((sum, v) => sum + v.weight, 0);