Make bandit tests configurable #1236
Conversation
| @@ -1,6 +1,5 @@ | |||
| import { EpicTest, EpicVariant } from '../../shared/types'; | |||
There was a problem hiding this comment.
previously this file was specific to Epics, but I've made it generic
| @@ -126,7 +126,6 @@ export const epicTestFromToolSchema = testSchema.extend({ | |||
| articlesViewedSettings: articlesViewedSettingsSchema.optional(), | |||
| priority: z.number(), | |||
| variants: variantSchema.array(), | |||
| isBanditTest: z.boolean().optional(), | |||
There was a problem hiding this comment.
there are currently no tests using this field, so I'm replacing it with the methodologies field which better models the capabilities
| * If controlProportionSettings is set then we use this to define the range of mvt values for the control variant. | ||
| * Otherwise we evenly distribute all variants across maxMvt. | ||
| */ | ||
| export const selectVariant = <V extends Variant, T extends Test<V>>(test: T, mvtId: number): V => { | ||
| export const selectVariantUsingMVT = <V extends Variant, T extends Test<V>>( |
There was a problem hiding this comment.
banner + header selection is still done using this function (for AB testing only)
| }; | ||
| } | ||
| } else if (test.methodologies) { | ||
| // More than one methodology, pick one of them using the mvt value |
There was a problem hiding this comment.
How does this know to spilt it 50/50?
There was a problem hiding this comment.
The getRandomNumber function returns a value based on the user's mvtId.
The % test.methodologies.length part divides all values between the methodologies. So if there are 2 methodologies then they'll be split 50/50.
This is similar to the logic in selectWithSeed elsewhere in this file.
src/server/lib/ab.ts
Outdated
| } else { | ||
| // No configured methodology, default to AB test | ||
| const methodology: Methodology = { name: 'ABTest' }; | ||
| const variant = selectVariantWithMethodology<V, T>(test, mvtId, banditData, methodology); |
There was a problem hiding this comment.
Could call selectVariantUsingMVT instead? Had to double check but don't think specifying 'ABTest' is doing anything here
| name: z.literal('EpsilonGreedyBandit'), | ||
| epsilon: z.number(), | ||
| }); | ||
| const methodologySchema = z.discriminatedUnion('name', [ |
|
|
||
| const addMethodologyToTestName = (testName: string, methodology: Methodology): string => { | ||
| if (methodology.name === 'EpsilonGreedyBandit') { | ||
| return `${testName}_EpsilonGreedyBandit-${methodology.epsilon}`; |
There was a problem hiding this comment.
I just realised a flaw in cases where this is used (i.e. where we have more than 1 methodology).
In banditData.ts we don't currently append the methodology to the test name when looking up data for a given test.
This isn't currently an issue because nothing is using bandit tests yet
Note - epic only for now, support for banner tests will come soon.
Adds an optional
methodologiesarray field to theTestmodel. If missing or empty, we default to an AB test.With this change we can now:
Currently we support
ABTestorEpsilonGreedyBandit. For the latter, anepsilonvalue is required.If more than one methodology is configured then the the audience is split evenly between the methodologies. Each methodology is tracked separately with a different test name.
E.g. for test name
MY_TESTand the following config:The audience is split 50/50 between the two, and the test names become:
MY_TEST-ABTestMY_TEST-EpsilonGreedyBandit-0.5This means they will show up as separate tests in the Tableau dashboard.
Associated changes: