From a3c670f417ba9a94eac1dbc1830f676477986082 Mon Sep 17 00:00:00 2001 From: MO Thibault <103271673+MO-Thibault@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:54:16 -0400 Subject: [PATCH] setupAB: correct isControl and carry edge fields on multi-arm tests setupAB could not express a split test with more than two arms, in three separate ways. All three surface together the moment a third arm exists. isControl was `selected.id !== treatmentId`, so any arm that is neither the named treatment nor the control was reported as a holdout, and had OPTABLE_RESOLVED and the SDK targeting cache cleared. For an arm that still resolves EIDs that is wrong twice over: it corrupts the measurement, and because the session guard survives the wipe the user serves an empty cache for the rest of the session. It is now `selected.id === controlId`, which is the same predicate for two arms and the intended one for more. fillTrafficPercentages rebuilt each variant as {id, trafficPercentage}, dropping skipMatchers, skipResolvers and matcher_override. Those fields are the reason an arm would differ at the edge at all, and ABTestConfig already declares them. Variants are now spread through, so the selected variant can be handed straight to InitConfig.abTests. The sticky read returned the object stored in localStorage. Editing an arm's skipMatchers or its weight then applied only to newly assigned users. It now resolves the stored id against the current variant list and returns that entry, so config is the single source of truth for everything but the id. Adds optableSplitTest, which forces a variant by id. optableControlGroup only reaches the two arms named by controlId and treatmentId, so it cannot hold a middle arm for QA. An unknown id falls through to normal assignment. Behaviour is unchanged for every two-arm caller: with variants production and test, `id === 'test'` and `id !== 'production'` are the same test. Tests: 12 new cases covering three-arm assignment, cache retention for a middle arm, skipMatchers passthrough, config re-read over the stored copy, and the optableSplitTest precedence chain. 387 pass. --- lib/addons/abTestAssignment.md | 63 +++++++++++++++---- lib/addons/abTestAssignment.test.ts | 96 +++++++++++++++++++++++++++++ lib/addons/abTestAssignment.ts | 47 ++++++++++---- lib/core/flags.md | 1 + lib/core/flags.ts | 1 + 5 files changed, 183 insertions(+), 25 deletions(-) diff --git a/lib/addons/abTestAssignment.md b/lib/addons/abTestAssignment.md index fd33813b..90e15faf 100644 --- a/lib/addons/abTestAssignment.md +++ b/lib/addons/abTestAssignment.md @@ -75,33 +75,72 @@ const ab = setupAB({ }); ``` +`isControl` is true only for `controlId`. Every other arm — including ones that are +neither the named treatment nor the control — is treated as enabled and keeps its +targeting cache. Set `controlId` explicitly whenever you use more than two variants. + +### Variants that change what the edge resolves + +A variant may carry any `ABTestConfig` field, not just `id` and `trafficPercentage`. +`skipMatchers`, `skipResolvers` and `matcher_override` are passed through untouched, +so the assigned variant can go straight into `abTests` on the SDK constructor — which +is what puts `skip_matchers` on the targeting request. Setting `skipMatchers` at the +top level of `InitConfig` does nothing; only the selected `abTests` entry is read. + +```js +const ab = setupAB({ + variants: [ + { id: "production" }, // 50% + { id: "skip1p", trafficPercentage: 45, skipMatchers: ["1p"] }, + { id: "test", trafficPercentage: 5 }, // holdout + ], +}); + +new OptableSDK({ + ..., + abTests: [{ ...ab.variant, trafficPercentage: 100 }], +}); +``` + +Only the `id` is read back from `localStorage`, so editing an arm's `skipMatchers` or +its weight applies to users already assigned to it, not just to new ones. + ## API ### `setupAB(config)` **Config options** -| Option | Type | Default | Description | -| ------------- | ----------------- | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | -| `variants` | `ABTestVariant[]` | required | List of variants. Each has an `id` and an optional `trafficPercentage`. Variants without `trafficPercentage` share the remaining traffic equally. | -| `storageKey` | `string` | `"OPTABLE_SPLIT_TEST"` | `localStorage` key used to persist the assignment across sessions. | -| `controlId` | `string` | `"test"` | The variant `id` considered the control group. Used to resolve `isControl` and the `optableControlGroup` flag override. | -| `treatmentId` | `string` | `"production"` | The variant `id` considered the treatment group. Used to resolve `isControl` and the `optableControlGroup` flag override. | -| `sdk` | `OptableSDK` | — | When provided, uses `sdk.targetingClearCache()` for precise control-group cache clearing instead of a key-prefix scan. | -| `pbjs` | `object` | — | When provided, bid-stamping hooks are registered on `pbjs` automatically at setup time. | +| Option | Type | Default | Description | +| ------------- | ----------------- | ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `variants` | `ABTestVariant[]` | required | List of variants. Each has an `id`, an optional `trafficPercentage`, and optionally any other `ABTestConfig` field (`skipMatchers`, `skipResolvers`, `matcher_override`). Variants without `trafficPercentage` share the remaining traffic equally. | +| `storageKey` | `string` | `"OPTABLE_SPLIT_TEST"` | `localStorage` key used to persist the assignment across sessions. | +| `controlId` | `string` | `"test"` | The variant `id` considered the control group. Used to resolve `isControl` and the `optableControlGroup` flag override. | +| `treatmentId` | `string` | `"production"` | The variant `id` considered the treatment group. Used to resolve `isControl` and the `optableControlGroup` flag override. | +| `sdk` | `OptableSDK` | — | When provided, uses `sdk.targetingClearCache()` for precise control-group cache clearing instead of a key-prefix scan. | +| `pbjs` | `object` | — | When provided, bid-stamping hooks are registered on `pbjs` automatically at setup time. | **Returned object** | Property | Type | Description | | --------------------- | ---------------- | -------------------------------------------------------------------------------------------------- | -| `variant` | `ABTestConfig` | The assigned variant (`id` + `trafficPercentage`). | -| `isControl` | `boolean` | `true` when the assigned variant is not the treatment. | +| `variant` | `ABTestConfig` | The assigned variant, as configured, with `trafficPercentage` resolved. | +| `isControl` | `boolean` | `true` when the assigned variant is `controlId`. | | `splitTestAssignment` | `string` | The assigned variant id. | | `setHooks` | `(pbjs) => void` | Registers bid-stamping hooks on a Prebid instance. Use when `pbjs` is not available at setup time. | ## Overriding the assignment for testing -Add `optableControlGroup` to the page URL: +Add `optableSplitTest` to the page URL to force a variant by id. This is the only +override that reaches an arm which is neither `controlId` nor `treatmentId`: + +``` +https://example.com/page?optableSplitTest=skip1p +``` + +An id that is not in `variants` is ignored, and assignment proceeds normally. + +Or add `optableControlGroup`: ``` https://example.com/page?optableControlGroup=1 # force control @@ -115,4 +154,4 @@ sessionStorage.setItem("optableControlGroup", "1"); // force control sessionStorage.setItem("optableControlGroup", "0"); // force treatment ``` -URL params take precedence over `sessionStorage`. Clear `localStorage.OPTABLE_SPLIT_TEST` to reset a sticky assignment. +URL params take precedence over `sessionStorage`, and `optableSplitTest` takes precedence over `optableControlGroup`. Clear `localStorage.OPTABLE_SPLIT_TEST` to reset a sticky assignment. diff --git a/lib/addons/abTestAssignment.test.ts b/lib/addons/abTestAssignment.test.ts index dd6d0f29..353f8ec2 100644 --- a/lib/addons/abTestAssignment.test.ts +++ b/lib/addons/abTestAssignment.test.ts @@ -99,6 +99,102 @@ describe("setupAB - custom variant ids", () => { }); }); +describe("setupAB - three arms", () => { + const threeArms = [ + { id: "production" }, + { id: "skip1p", trafficPercentage: 45, skipMatchers: ["1p"] }, + { id: "test", trafficPercentage: 5 }, + ]; + + it("treats a middle arm as treatment, not control", () => { + jest.spyOn(Math, "random").mockReturnValue(0.6); // bucket 60 → skip1p + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("skip1p"); + expect(result.isControl).toBe(false); + }); + + it("keeps the targeting cache for a middle arm", () => { + localStorage.setItem("OPTABLE_RESOLVED", "valid"); + localStorage.setItem("OPTABLE_TARGETING_abc123", "valid"); + jest.spyOn(Math, "random").mockReturnValue(0.6); + setupAB({ variants: threeArms }); + expect(localStorage.getItem("OPTABLE_RESOLVED")).toBe("valid"); + expect(localStorage.getItem("OPTABLE_TARGETING_abc123")).toBe("valid"); + }); + + it("still treats controlId as control and clears its cache", () => { + localStorage.setItem("OPTABLE_RESOLVED", "stale"); + jest.spyOn(Math, "random").mockReturnValue(0.97); // bucket 97 → test + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("test"); + expect(result.isControl).toBe(true); + expect(localStorage.getItem("OPTABLE_RESOLVED")).toBeNull(); + }); + + it("carries skipMatchers through to the selected variant", () => { + jest.spyOn(Math, "random").mockReturnValue(0.6); + const result = setupAB({ variants: threeArms }); + expect(result.variant.skipMatchers).toEqual(["1p"]); + }); + + it("does not attach skipMatchers to an arm that has none", () => { + jest.spyOn(Math, "random").mockReturnValue(0.0); + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("production"); + expect(result.variant.skipMatchers).toBeUndefined(); + }); + + it("re-reads variant config from the current list, not the stored copy", () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ id: "skip1p", trafficPercentage: 45 })); + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("skip1p"); + expect(result.variant.skipMatchers).toEqual(["1p"]); + }); +}); + +describe("setupAB - optableSplitTest override", () => { + const threeArms = [ + { id: "production" }, + { id: "skip1p", trafficPercentage: 45, skipMatchers: ["1p"] }, + { id: "test", trafficPercentage: 5 }, + ]; + + it("forces an arm that optableControlGroup cannot name", () => { + sessionStorage.setItem("optableSplitTest", "skip1p"); + resetFlags(); + jest.spyOn(Math, "random").mockReturnValue(0.0); // would otherwise be production + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("skip1p"); + expect(result.variant.skipMatchers).toEqual(["1p"]); + expect(result.isControl).toBe(false); + }); + + it("beats a sticky assignment", () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ id: "production", trafficPercentage: 50 })); + sessionStorage.setItem("optableSplitTest", "test"); + resetFlags(); + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("test"); + expect(result.isControl).toBe(true); + }); + + it("beats optableControlGroup", () => { + sessionStorage.setItem("optableSplitTest", "skip1p"); + sessionStorage.setItem("optableControlGroup", "1"); + resetFlags(); + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("skip1p"); + }); + + it("ignores an id that is not in the variants list", () => { + sessionStorage.setItem("optableSplitTest", "nonexistent"); + resetFlags(); + jest.spyOn(Math, "random").mockReturnValue(0.0); + const result = setupAB({ variants: threeArms }); + expect(result.variant.id).toBe("production"); + }); +}); + describe("setupAB - control group cache clearing", () => { it("clears OPTABLE_RESOLVED and OPTABLE_TARGETING_* when assigned to control", () => { localStorage.setItem("OPTABLE_RESOLVED", "stale"); diff --git a/lib/addons/abTestAssignment.ts b/lib/addons/abTestAssignment.ts index 1a4080b0..a429ccb0 100644 --- a/lib/addons/abTestAssignment.ts +++ b/lib/addons/abTestAssignment.ts @@ -4,10 +4,13 @@ import { getFlags } from "../core/flags"; const DEFAULT_STORAGE_KEY = "OPTABLE_SPLIT_TEST"; -export interface ABTestVariant { - id: string; +// A variant is an ABTestConfig whose trafficPercentage may be omitted and +// inferred. The edge-facing fields (skipMatchers, skipResolvers, +// matcher_override) are carried through untouched, so a caller can attach them +// to one arm and hand the selected variant straight to InitConfig.abTests. +export type ABTestVariant = Omit & { trafficPercentage?: number; -} +}; export interface SetupABConfig { variants: ABTestVariant[]; @@ -36,7 +39,7 @@ function fillTrafficPercentages(variants: ABTestVariant[]): ABTestConfig[] { const unassigned = variants.filter((v) => v.trafficPercentage === undefined); const each = unassigned.length > 0 ? (100 - allocated) / unassigned.length : 0; return variants.map((v) => ({ - id: v.id, + ...v, trafficPercentage: v.trafficPercentage ?? each, })); } @@ -57,36 +60,48 @@ export function setupAB(config: SetupABConfig): ABTestSetupResult { let selected: ABTestConfig | null = null; - // Priority 1 — QA/debug override via URL param or sessionStorage flag. + // Priority 1 — QA/debug override naming a variant directly. + // ?optableSplitTest= forces that arm. optableControlGroup only reaches the + // two arms named by controlId and treatmentId, so this is the only way to hold + // a third arm in a multi-variant test. Unknown ids fall through to the normal + // resolution rather than inventing a variant. + const splitTestFlag = getFlags().optableSplitTest; + if (splitTestFlag) { + selected = filled.find((v) => v.id === splitTestFlag) ?? null; + } + + // Priority 2 — QA/debug override via URL param or sessionStorage flag. // ?optableControlGroup=1 forces the control variant; =0 forces treatment. // This lets QA verify both branches without clearing localStorage. const controlGroupFlag = getFlags().optableControlGroup; - if (controlGroupFlag === "1") { + if (!selected && controlGroupFlag === "1") { selected = filled.find((v) => v.id === controlId) ?? { id: controlId, trafficPercentage: 0 }; - } else if (controlGroupFlag === "0") { + } else if (!selected && controlGroupFlag === "0") { selected = filled.find((v) => v.id === treatmentId) ?? { id: treatmentId, trafficPercentage: 0 }; } - // Priority 2 — sticky assignment from a previous visit. + // Priority 3 — sticky assignment from a previous visit. // Once a user is assigned a variant it must not change across page loads or // sessions, otherwise the same user could appear in both groups. We validate // the cached id against the current variant list so a stale cache from an // old experiment config is silently discarded. + // + // Only the id is taken from storage. Everything else comes from the current + // variant config, so editing an arm's skipMatchers (or its weight) applies to + // users who were already assigned to it instead of only to new ones. if (!selected) { try { const cached = localStorage.getItem(storageKey); if (cached) { const parsed = JSON.parse(cached); - if (parsed?.id && filled.some((v) => v.id === parsed.id)) { - selected = parsed as ABTestConfig; - } + selected = filled.find((v) => v.id === parsed?.id) ?? null; } } catch { // localStorage unavailable or invalid JSON } } - // Priority 3 — first visit: randomly assign based on traffic weights. + // Priority 4 — first visit: randomly assign based on traffic weights. // determineABTest returns null when the random bucket falls outside all // defined ranges (i.e. weights sum to less than 100). filled[0] is the // fallback so selected is always non-null after this point. @@ -101,7 +116,13 @@ export function setupAB(config: SetupABConfig): ABTestSetupResult { // localStorage unavailable } - const isControl = selected.id !== treatmentId; + // Control is the arm named by controlId, not "anything that is not treatment". + // With two variants the two readings agree. With three or more they do not: a + // middle arm that still resolves EIDs — say one carrying skipMatchers — would + // be classified as a holdout and have its cache cleared below, which both + // corrupts the measurement and strands the user on an empty cache for the rest + // of the session. + const isControl = selected.id === controlId; const assignment = selected.id; // Control group: clear cached targeting data so RTD, PPID and TargetingFromCache diff --git a/lib/core/flags.md b/lib/core/flags.md index bed9d923..6b47a16a 100644 --- a/lib/core/flags.md +++ b/lib/core/flags.md @@ -59,6 +59,7 @@ if (controlGroup === "1") { | `optableDebug` | `debugLog`, RTD module | Verbose logging. | | `optableDisableConsent` | `getConsent` | Bypass the CMP and treat all permissions as granted. | | `optableControlGroup` | `setupAB` | `1` forces the control variant, `0` forces treatment. Two-state — read the raw value. | +| `optableSplitTest` | `setupAB` | Forces the variant with this id. Reaches arms `optableControlGroup` cannot name. | | `optableForceTargeting` | wrapper code | Re-run targeting even when a session guard says it already ran. | | `optableForceTokenize` | wrapper code | Re-run tokenize even when a session guard says it already ran. | | `optableForceGlobalRouting` | `buildRTD` | Route every EID to `global` instead of per-bidder. | diff --git a/lib/core/flags.ts b/lib/core/flags.ts index f0964172..f2db4b36 100644 --- a/lib/core/flags.ts +++ b/lib/core/flags.ts @@ -5,6 +5,7 @@ const FLAG_KEYS = [ "optableResolve3P", "optableEnableAnalytics", "optableControlGroup", + "optableSplitTest", "optableForceTargeting", "optableForceGlobalRouting", "optableForceSkipMerge",