[AB testing PoC] calculate ab test MVTs without manual offset by Jakeii · Pull Request #14286 · guardian/dotcom-rendering

Jakeii · 2025-07-25T16:30:00Z

What does this change?

This does away with manual test offset, and now looks at the existing state of the tests in the mvt dictionary when building the key values for the current ab-tests.ts, ensuring any existing tests "stay where they are" within the test space.

It does the following

Work out which tests are using which MVT ids, and which MVT ids are available.
Free up space by removing any removed tests, and shrinking any down-sized tests - adding the freed mvt ids to the pool of available mvts.
Attempt to increase any up-sized tests using available mvts.
Attempt to create any new tests using available mvts.

This does means that tests will not use contiguous ranges of MVT IDs anymore, but MVTs are random so this doesn't affect the behaviour of test allocation.

Why?

At the moment tests need to specify their audienceOffset, which is where in the mvt space to position the test, this meant that developers wanting to add tests have to check all existing tests and work out what value they need to use for any new tests.

The reason for it's existence was to avoid tests "moving around" to different mvts, changing the users who are in the test while it's in progress.

github-actions · 2025-07-28T09:29:16Z

Deploy build 44237 of `Mobile::mobile-apps-rendering` to CODE

All deployment options

From guardian/actions-riff-raff.

github-actions · 2025-07-28T09:36:07Z

Deploy build 66 of `dotcom:rendering-all` to CODE

All deployment options

From guardian/actions-riff-raff.

Jakeii · 2025-07-28T10:34:48Z

ab-testing/deno.json

 		"validate": "deno run scripts/validation/index.ts",
 		"upload": "deno run --allow-read=dist --allow-env=FASTLY_AB_TESTING_API_TOKEN,FASTLY_AB_TESTING_SERVICE_ID,FASTLY_AB_TESTING_SERVICE_NAME,FASTLY_MVTS_DICTIONARY_ID,FASTLY_MVTS_DICTIONARY_NAME,FASTLY_AB_TESTS_DICTIONARY_ID,FASTLY_AB_TESTS_DICTIONARY_NAME --allow-net=api.fastly.com:443 scripts/deploy/index.ts --mvts=dist/mvts.json --ab-tests=dist/ab-tests.json",
-		"build": "deno run --allow-write scripts/build/index.ts --mvts=dist/mvts.json --ab-tests=dist/ab-tests.json"
+		"build": "deno run --allow-env=FASTLY_AB_TESTING_API_TOKEN,FASTLY_AB_TESTING_SERVICE_ID,FASTLY_AB_TESTING_SERVICE_NAME,FASTLY_MVTS_DICTIONARY_ID,FASTLY_MVTS_DICTIONARY_NAME,FASTLY_AB_TESTS_DICTIONARY_ID,FASTLY_AB_TESTS_DICTIONARY_NAME --allow-net=api.fastly.com:443 --allow-write scripts/build/index.ts --mvts=dist/mvts.json --ab-tests=dist/ab-tests.json"


Build step now needs to read the AB testing state from fastly

github-actions · 2025-07-28T10:44:44Z

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

Copilot

Pull Request Overview

This PR removes the manual audienceOffset requirement from A/B tests and implements an automatic MVT (multivariate test) allocation system that preserves existing test positions while optimizing space usage.

Removes audienceOffset field from ABTest type and replaces manual offset management with automatic allocation
Introduces a new TestGroupMVTManager class to handle MVT allocation, resizing, and deallocation
Refactors the MVT dictionary building process to use existing Fastly dictionary state as the source of truth

Reviewed Changes

Copilot reviewed 25 out of 28 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ab-testing/types.ts	Removes audienceOffset field from ABTest interface
ab-testing/scripts/validation/variantOverlap.ts	Removes entire overlap validation module (no longer needed)
ab-testing/scripts/validation/validSizeOffset.ts	Removes size/offset validation module
ab-testing/scripts/build/test-group-mvt-manager.ts	New class for managing MVT allocation and test group lifecycle
ab-testing/scripts/build/calculate-mvt-updates.ts	New module for calculating MVT updates across audience spaces
ab-testing/scripts/lib/	New shared utilities for constants, types, and Fastly subfield parsing
ab-testing/scripts/build/index.ts	Updates build process to fetch existing MVT state and use new allocation system
ab-testing/abTest.ts	Removes audienceOffset and highImpact fields from example tests

ab-testing/scripts/build/test-group-mvt-manager.ts

ab-testing/scripts/build/calculate-mvt-updates.ts

Copilot · 2025-07-29T15:04:38Z

ab-testing/scripts/build/calculate-mvt-updates.ts

+				string,
+				{ name: string; type: string; exp: number }
+			> = new Map(
+				mvtGroups.entries().map(([key, value]) => [key, value[i]]),


Map.entries() returns a MapIterator, not an array. This should be Array.from(mvtGroups.entries()).map(...) to properly convert to an array before calling map().

Suggested change

mvtGroups.entries().map(([key, value]) => [key, value[i]]),

Array.from(mvtGroups.entries()).map(([key, value]) => [key, value[i]]),

The AI is flagging that the result of mvtGroups.entries() cannot be iterated over using a .map, I asked it to generate a TypeScript playground snippet to demonstrate how to iterate over the result of .entries() which we can see here. So this snippet has a compiler error and fails to run, but this isn't an issue we've come across via our tests or with the compiler. 🤔

Copilot must think we're targeting an older ES version? If you change the target to ESNext the error goes away

Ok interesting, we should record this and provide feedback, all the issues it's flagged look to be the same kind of issue.

ab-testing/scripts/build/index.ts

cemms1

I got halfway through reviewing this but then stopped and forgot to continue. I might suggest this PR is slightly too large to give a good PR review here and in an ideal world, this would have been split into a couple of PRs to make it a bit easier for reviewing.

I'll finish my review soon but leaving the comments before I forget!

ab-testing/scripts/lib/fastly-subfield.ts

ab-testing/scripts/lib/constants.ts

ab-testing/scripts/build/calculate-mvt-updates.test.ts

ab-testing/scripts/build/test-group-mvt-manager.test.ts

Co-authored-by: Charlotte Emms <43961396+cemms1@users.noreply.github.com>

Jakeii · 2025-07-30T09:34:34Z

I got halfway through reviewing this but then stopped and forgot to continue. I might suggest this PR is slightly too large to give a good PR review here and in an ideal world, this would have been split into a couple of PRs to make it a bit easier for reviewing.

I'll finish my review soon but leaving the comments before I forget!

If you have a suggestion of how I could break it up, happy to do so! There were a couple of small things like removing the highImpact option that snuck in but I can't think how to break it down further.

…jlk/calculate-test-offsets

cemms1 · 2025-07-30T14:47:52Z

ab-testing/scripts/lib/fastly-subfield.ts

+const stringifyMVTValue = (array: FastlyTestParams[]): string => {
+	const subfield: Record<string, string> = {};
+	array.forEach((item, index) => {
+		subfield[`group:${index}`] = item.name;
+		subfield[`group:${index}:type`] = item.type;
+		subfield[`group:${index}:exp`] = String(item.exp);
+	});
+	return stringifyFastlySubfield(subfield);
+};


A test would help to better illustrate what is happening here

Yes good point!

Jakeii added 2 commits July 25, 2025 17:04

calculate ab test mvts without manual offset

2667d2b

remove offset validation

452d0b8

Jakeii added 2 commits July 28, 2025 10:45

also remove overlap validation

f5d3547

unit tests for calculating mvt updates

46a5cf9

Jakeii force-pushed the jlk/calculate-test-offsets branch from b4ad08f to 46a5cf9 Compare July 28, 2025 10:10

Jakeii commented Jul 28, 2025

View reviewed changes

Jakeii marked this pull request as ready for review July 28, 2025 10:44

use MVT_COUNT

9ff500f

GHaberis requested a review from Copilot July 29, 2025 15:03

Copilot AI reviewed Jul 29, 2025

View reviewed changes

cemms1 reviewed Jul 29, 2025

View reviewed changes

Jakeii and others added 2 commits July 30, 2025 10:20

Get rid of generic from parseFastlySubfield

a93e897

Update ab-testing/scripts/build/calculate-mvt-updates.test.ts

a4b6311

Co-authored-by: Charlotte Emms <43961396+cemms1@users.noreply.github.com>

Jakeii added 3 commits July 30, 2025 10:37

assert reallocation is as expected

5f654d4

extend tests

e2b31c9

Merge remote-tracking branch 'origin/commercial/ab-testing-poc' into …

a13a65b

…jlk/calculate-test-offsets

cemms1 approved these changes Jul 30, 2025

View reviewed changes

add tests and comments to fastly subfield

679c716

Jakeii merged commit 2953150 into commercial/ab-testing-poc Jul 30, 2025
25 checks passed

Jakeii deleted the jlk/calculate-test-offsets branch July 30, 2025 15:58

cemms1 mentioned this pull request Aug 1, 2025

[DO NOT MERGE] AB Testing POC #13798

Closed

Jakeii mentioned this pull request Aug 14, 2025

Fastly Dictionary AB testing configuration and associated scripts #14333

Merged

	mvtGroups.entries().map(([key, value]) => [key, value[i]]),
	Array.from(mvtGroups.entries()).map(([key, value]) => [key, value[i]]),

Conversation

Jakeii commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this change?

Why?

Uh oh!

github-actions bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploy build 44237 of Mobile::mobile-apps-rendering to CODE

Uh oh!

github-actions bot commented Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploy build 66 of dotcom:rendering-all to CODE

Uh oh!

Jakeii Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

Uh oh!

GHaberis Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jakeii Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

GHaberis Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cemms1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jakeii commented Jul 30, 2025

Uh oh!

cemms1 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Jakeii Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jakeii commented Jul 25, 2025 •

edited

Loading

github-actions bot commented Jul 28, 2025 •

edited

Loading

Deploy build 44237 of `Mobile::mobile-apps-rendering` to CODE

github-actions bot commented Jul 28, 2025 •

edited

Loading

Deploy build 66 of `dotcom:rendering-all` to CODE

GHaberis Jul 29, 2025 •

edited

Loading

GHaberis Jul 30, 2025 •

edited

Loading

Jakeii Jul 30, 2025 •

edited

Loading