diff --git a/.changeset/align-rollout-split-assignment.md b/.changeset/align-rollout-split-assignment.md new file mode 100644 index 00000000..59e220a7 --- /dev/null +++ b/.changeset/align-rollout-split-assignment.md @@ -0,0 +1,9 @@ +--- +"@vercel/flags-core": minor +--- + +Align rollout, split, and segment split user assignment onto a single hash bucketing scheme. + +Previously splits and rollouts bucketed users with opposite conventions, so switching a flag between a split and a rollout (or locking in a rollout as a split) could reassign users even when the effective distribution was unchanged. All three now derive their cut points from one shared boundary function over the full hash space, so a rollout at a given percentage is identical to the equivalent split. + +Split assignments are effectively unchanged. Rollouts and segment splits are re-bucketed once with this release; after that, converting a flag between outcome types never reassigns anyone. diff --git a/packages/vercel-flags-core/src/evaluate.test.ts b/packages/vercel-flags-core/src/evaluate.test.ts index 07fc77d0..8fcd0337 100644 --- a/packages/vercel-flags-core/src/evaluate.test.ts +++ b/packages/vercel-flags-core/src/evaluate.test.ts @@ -595,9 +595,9 @@ describe('evaluate', () => { const trueCount = results.filter((r) => r.value).length; const falseCount = results.filter((r) => !r.value).length; - // both should be close to 500 - expect(trueCount).toBe(5070); - expect(falseCount).toBe(4930); + // both should be close to 5000 (50% of 10k) + expect(trueCount).toBe(5023); + expect(falseCount).toBe(4977); }); it('should split roughly equally on a 50/50 split', () => { @@ -630,9 +630,9 @@ describe('evaluate', () => { const trueCount = results.filter((r) => r.value).length; const falseCount = results.filter((r) => !r.value).length; - // both should be close to 100 (1%) - expect(trueCount).toBe(102); - expect(falseCount).toBe(9898); + // should be close to 100 (1% of 10k) + expect(trueCount).toBe(118); + expect(falseCount).toBe(9882); }); it('should split roughly equally on a 50/50 split', () => { @@ -652,7 +652,7 @@ describe('evaluate', () => { outcome: { type: 'split', base: ['user', 'name'], - passPromille: 99_000, // pass 1% + passPromille: 99_000, // pass 99% }, }, ], @@ -665,9 +665,9 @@ describe('evaluate', () => { const trueCount = results.filter((r) => r.value).length; const falseCount = results.filter((r) => !r.value).length; - // both should be close to 9900 (99%) - expect(trueCount).toBe(9891); - expect(falseCount).toBe(109); + // both should be close to 9900 (99% of 10k) + expect(trueCount).toBe(9903); + expect(falseCount).toBe(97); }); }); @@ -2238,6 +2238,54 @@ describe('evaluate', () => { expect(getTotals([1, 1, 1, 1], 9)).toEqual(expectedTotals); expect(getTotals([1000, 1000, 1000, 1000], 9)).toEqual(expectedTotals); }); + + it.each([ + { weights: [30_000, 70_000] }, + { weights: [1, 99] }, + { weights: [10_000, 20_000, 30_000, 40_000] }, + { weights: [1, 1, 1, 1] }, + ])('assigns roughly proportional to weights $weights across many ids (statistical check)', ({ + weights, + }) => { + const N = 20_000; + const variants = weights.map((_, i) => `v${i}`); + const counts = new Array(weights.length).fill(0) as number[]; + + for (let i = 0; i < N; i++) { + const result = evaluate({ + definition: { + environments: { + production: { + fallthrough: { + type: 'split', + base: ['user', 'id'], + defaultVariant: 0, + weights, + }, + }, + }, + variants, + seed: 7, + } satisfies Packed.FlagDefinition, + environment: 'production', + entities: { user: { id: `uid${i}` } }, + }).value as string; + counts[variants.indexOf(result)]!++; + } + + // Expected count per variant under its configured weight, with a + // binomial standard error; assignment is hash-based (deterministic), + // so this is not flaky, but confirms the actual split tracks the + // configured weight ratios rather than some other distribution. + const totalWeight = weights.reduce((a, b) => a + b, 0); + weights.forEach((weight, index) => { + const p = weight / totalWeight; + const expected = N * p; + const stderr = Math.sqrt(N * p * (1 - p)); + expect(counts[index]).toBeGreaterThan(expected - 4 * stderr); + expect(counts[index]).toBeLessThan(expected + 4 * stderr); + }); + }); }); describe('rollouts', () => { @@ -2444,6 +2492,40 @@ describe('evaluate', () => { expect(trueCount).toBe(1000); }); + it.each([ + 1_000, 10_000, 25_000, 50_000, 75_000, 90_000, 99_000, + ])('assigns roughly %i/100000 of entities to rollToVariant across many ids (statistical check)', (promille) => { + const N = 20_000; + vi.setSystemTime(startTimestamp); + const rollout = makeRollout({ + slots: [[promille, 100 * HOUR]], + }); + + let rollToCount = 0; + for (let i = 0; i < N; i++) { + const result = evaluate({ + definition: { + environments: { production: { fallthrough: rollout } }, + seed: 7, + variants: [false, true], + }, + environment: 'production', + entities: { user: { id: `uid${i}` } }, + }); + if (result.value === true) rollToCount++; + } + + // Expected count under the configured promille, with a binomial + // standard error; assignment is hash-based (deterministic), so this + // is not flaky, but confirms the actual split tracks the configured + // percentage rather than some other distribution. + const p = promille / 100_000; + const expected = N * p; + const stderr = Math.sqrt(N * p * (1 - p)); + expect(rollToCount).toBeGreaterThan(expected - 2 * stderr); + expect(rollToCount).toBeLessThan(expected + 2 * stderr); + }); + it('works as a rule outcome', () => { vi.setSystemTime(startTimestamp + 12 * HOUR); expect( @@ -2473,6 +2555,98 @@ describe('evaluate', () => { outcomeType: OutcomeType.ROLLOUT, }); }); + + describe('assigns identically to the equivalent split (no reassignment on type change)', () => { + const SEED = 7; + const VARIANTS = [false, true] as const; + const USER_COUNT = 1000; + + // Evaluate a rollout pinned to `promille` (single slot active at t=0). + const rolloutValue = ( + rollFromVariant: number, + rollToVariant: number, + promille: number, + uid: string, + ): unknown => { + vi.setSystemTime(startTimestamp); + return evaluate({ + definition: { + environments: { + production: { + fallthrough: makeRollout({ + rollFromVariant, + rollToVariant, + defaultVariant: rollFromVariant, + slots: [[promille, 100 * HOUR]], + }), + }, + }, + seed: SEED, + variants: [...VARIANTS], + }, + environment: 'production', + entities: { user: { id: uid } }, + }).value; + }; + + // Evaluate the split that expresses the same instantaneous distribution: + // rollToVariant gets `promille`, rollFromVariant gets the remainder. + const splitValue = ( + rollFromVariant: number, + rollToVariant: number, + promille: number, + uid: string, + ): unknown => { + const weights = [0, 0]; + weights[rollFromVariant] = 100_000 - promille; + weights[rollToVariant] = promille; + return evaluate({ + definition: { + environments: { + production: { + fallthrough: { + type: 'split', + base: ['user', 'id'], + defaultVariant: rollFromVariant, + weights, + }, + }, + }, + seed: SEED, + variants: [...VARIANTS], + }, + environment: 'production', + entities: { user: { id: uid } }, + }).value; + }; + + // Both index orderings: rollTo after rollFrom, and rollTo before rollFrom. + const orderings = [ + { + rollFromVariant: 0, + rollToVariant: 1, + label: 'rollTo index > rollFrom', + }, + { + rollFromVariant: 1, + rollToVariant: 0, + label: 'rollTo index < rollFrom', + }, + ]; + + for (const { rollFromVariant, rollToVariant, label } of orderings) { + for (const promille of [1_000, 30_000, 50_000, 70_000, 99_000]) { + it(`matches split at ${promille / 1_000}% (${label})`, () => { + for (let i = 0; i < USER_COUNT; i++) { + const uid = `uid${i}`; + expect( + rolloutValue(rollFromVariant, rollToVariant, promille, uid), + ).toBe(splitValue(rollFromVariant, rollToVariant, promille, uid)); + } + }); + } + } + }); }); }); diff --git a/packages/vercel-flags-core/src/evaluate.ts b/packages/vercel-flags-core/src/evaluate.ts index 28d00e8a..90629896 100644 --- a/packages/vercel-flags-core/src/evaluate.ts +++ b/packages/vercel-flags-core/src/evaluate.ts @@ -13,23 +13,52 @@ type PathArray = (string | number)[]; const MAX_REGEX_INPUT_LENGTH = 10_000; -/** uint32 max — domain of xxHash32 output, used for split/rollout thresholds */ -const UINT32_MAX = 4_294_967_295; - -// Per-object memoization caches keyed by the outcome / rhs objects from the -// datafile. Using WeakMaps (instead of mutating the objects with symbol-keyed -// props) keeps datafile objects pristine so they serialize cleanly across the -// RSC server/client boundary. Entries are GC'd when the datafile is dropped. -const scaledWeightsCache = new WeakMap(); +/** + * Bucket space for all traffic splitting. `hashInput` (xxHash32) returns exactly + * `2**32` distinct values, so we use `2**32` as the space and compare the raw + * hash against boundaries in it — no modulo (which would bias low buckets), and + * the final variant's boundary is exactly `2**32` so nothing falls through. + */ +const HASH_SPACE = 2 ** 32; + +/** Denominator for promille values (rollout slots and segment `passPromille`). */ +const PROMILLE_SCALE = 100_000; + +/** + * Maps the fraction `numerator / denominator` to a cut point in `[0, HASH_SPACE]`; + * a hash is "below" the fraction iff `hash < boundaryFor(…)`. Splits, rollouts, + * and segments all cut with this one function, so a rollout at promille `p` is + * bit-for-bit identical to the split `{ rollFrom: PROMILLE_SCALE - p, rollTo: p }` + * — which is what lets a flag switch between split and rollout without reassigning + * anyone. `denominator === 0` yields `NaN`, so evaluation falls through to default. + */ +function boundaryFor(numerator: number, denominator: number): number { + return Math.floor((numerator / denominator) * HASH_SPACE); +} + +// WeakMaps keyed by datafile objects, so those objects stay pristine (no +// symbol-keyed props) and serialize cleanly across the RSC boundary; entries +// are GC'd with the datafile. Split boundaries are static per outcome, so the +// cumulative cut points are computed once and reused across evaluations. +const splitBoundariesCache = new WeakMap(); const compiledRegexCache = new WeakMap(); -function getScaledWeights(outcome: Packed.SplitOutcome): number[] { - const cached = scaledWeightsCache.get(outcome); +/** + * Cumulative hash boundaries for a split, one per variant in index order. + * Variant `i` is served for hashes in `[boundaries[i-1], boundaries[i])`. + */ +function getSplitBoundaries(outcome: Packed.SplitOutcome): number[] { + const cached = splitBoundariesCache.get(outcome); if (cached) return cached; const total = sum(outcome.weights); - const scaled = outcome.weights.map((w) => (w / total) * UINT32_MAX); - scaledWeightsCache.set(outcome, scaled); - return scaled; + const boundaries: number[] = []; + let cumulative = 0; + for (const weight of outcome.weights) { + cumulative += weight; + boundaries.push(boundaryFor(cumulative, total)); + } + splitBoundariesCache.set(outcome, boundaries); + return boundaries; } function getCompiledRegex(rhs: { pattern: string; flags: string }): RegExp { @@ -348,14 +377,12 @@ function handleSegmentOutcome( // exclude from segment if the lhs is not a string if (typeof lhs !== 'string') return false; - const maxValue = 100_000; - // bypass hashing for common values and edges if (outcome.passPromille <= 0) return false; - if (outcome.passPromille >= maxValue) return true; + if (outcome.passPromille >= PROMILLE_SCALE) return true; - const value = hashInput(lhs, params.definition.seed) % maxValue; - return value < outcome.passPromille; + const bucket = hashInput(lhs, params.definition.seed); + return bucket < boundaryFor(outcome.passPromille, PROMILLE_SCALE); } default: { const { type } = outcome; @@ -418,20 +445,22 @@ function handleOutcome( }; } - /** - * (xxHash32): turns the string into a number between 0 and 2^32-1 (max uint32 value) - * Since we know the range of the hash function, we don't use modulo here. If we change - * the hash function, or if the range changes, we should add a modulo here and/or adjust UINT32_MAX. - */ - const value = hashInput(lhs, params.definition.seed); - const scaledWeights = getScaledWeights(outcome); - const variantIndex = findWeightedIndex(scaledWeights, value, UINT32_MAX); - const variant = - variantIndex === -1 - ? defaultOutcome - : getVariant(params.definition, variantIndex); + const bucket = hashInput(lhs, params.definition.seed); + const boundaries = getSplitBoundaries(outcome); + + // Return the first variant whose cumulative boundary covers the bucket. + for (let index = 0; index < boundaries.length; index++) { + if (bucket < (boundaries[index] as number)) { + return { + ...getVariant(params.definition, index), + outcomeType: OutcomeType.SPLIT, + }; + } + } + + // Only reached when the weights sum to 0 (every boundary is NaN). return { - ...variant, + ...defaultOutcome, outcomeType: OutcomeType.SPLIT, }; } @@ -479,7 +508,7 @@ function handleOutcome( break; } } - if (exhausted) currentPromille = 100_000; + if (exhausted) currentPromille = PROMILLE_SCALE; // short-circuit common edges if (currentPromille <= 0) { @@ -492,20 +521,32 @@ function handleOutcome( params.definition, outcome.rollToVariant, ); - if (currentPromille >= 100_000) { + if (currentPromille >= PROMILLE_SCALE) { return { ...rollToVariant, outcomeType: OutcomeType.ROLLOUT, }; } - const value = hashInput(lhs, params.definition.seed); - const threshold = (currentPromille / 100_000) * UINT32_MAX; - - const variant = value < threshold ? rollToVariant : rollFromVariant; + const bucket = hashInput(lhs, params.definition.seed); + // Equivalent to the split { rollFrom: PROMILLE_SCALE - p, rollTo: p }: the + // lower-index variant takes the low buckets, matching where the split + // places it (see boundaryFor). So rollTo holds `[0, boundary(p))` when it + // has the lower index, otherwise rollFrom holds the low buckets. + if (outcome.rollToVariant < outcome.rollFromVariant) { + const rollToBoundary = boundaryFor(currentPromille, PROMILLE_SCALE); + return { + ...(bucket < rollToBoundary ? rollToVariant : rollFromVariant), + outcomeType: OutcomeType.ROLLOUT, + }; + } + const rollFromBoundary = boundaryFor( + PROMILLE_SCALE - currentPromille, + PROMILLE_SCALE, + ); return { - ...variant, + ...(bucket < rollFromBoundary ? rollFromVariant : rollToVariant), outcomeType: OutcomeType.ROLLOUT, }; } @@ -641,27 +682,3 @@ export function bulkEvaluate( } return results; } - -/** - * Find the weighted index that the given value falls into. - * - * Takes a set of weights that add up to maxValue, and returns the index - * that corresponds to the given value. - * - * @returns index or -1 - */ -export function findWeightedIndex( - weights: number[], - value: number, - maxValue: number, -): number { - if (value < 0 || value >= maxValue) return -1; - - let sum = 0; - for (let i = 0; i < weights.length; i++) { - sum += weights[i] as number; - if (value < sum) return i; - } - - return -1; -} diff --git a/packages/vercel-flags-core/src/integration.test.ts b/packages/vercel-flags-core/src/integration.test.ts index a1d9e968..fe79a4df 100644 --- a/packages/vercel-flags-core/src/integration.test.ts +++ b/packages/vercel-flags-core/src/integration.test.ts @@ -288,8 +288,8 @@ describe('integration evaluate', () => { const falseCount = results.filter((r) => !r.value).length; // both should be close to 500 - expect(trueCount).toBe(5070); - expect(falseCount).toBe(4930); + expect(trueCount).toBe(5023); + expect(falseCount).toBe(4977); }); it('should split roughly equally on a 50/50 split', () => { @@ -323,8 +323,8 @@ describe('integration evaluate', () => { const falseCount = results.filter((r) => !r.value).length; // both should be close to 100 (1%) - expect(trueCount).toBe(102); - expect(falseCount).toBe(9898); + expect(trueCount).toBe(118); + expect(falseCount).toBe(9882); }); it('should split roughly equally on a 50/50 split', () => { @@ -358,8 +358,8 @@ describe('integration evaluate', () => { const falseCount = results.filter((r) => !r.value).length; // both should be close to 9900 (99%) - expect(trueCount).toBe(9891); - expect(falseCount).toBe(109); + expect(trueCount).toBe(9903); + expect(falseCount).toBe(97); }); }); });