Skip to content

fix(3742): improve user segmentation with BigInt-based random generation #5110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DDDDDanica merged 6 commits into from
Jan 10, 2025
Merged

fix(3742): improve user segmentation with BigInt-based random generation #5110

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c650ada
fix(3742): improve user segmentation with BigInt-based random generation
DDDDDanica Jan 7, 2025
3e155c1
fix(3742): remove unnecessary normalization of bit
DDDDDanica Jan 8, 2025
87cd8f5
fix(3742): add readbility to large number and fix the logic to remove…
DDDDDanica Jan 8, 2025
8724cc3
fix(3742): improve UUID random number distribution
DDDDDanica Jan 9, 2025
3ae4dd8
fix(3742): add validation to metametricsId
DDDDDanica Jan 9, 2025
b3a2a69
Merge branch 'main' into fix/3742-generateDeterministicRandomNumber
DDDDDanica Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 94 additions & 18 deletions packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import { v4 as uuidV4 } from 'uuid';

import {
generateDeterministicRandomNumber,
isFeatureFlagWithScopeValue,
} from './user-segmentation-utils';

const MOCK_METRICS_IDS = [
'123e4567-e89b-4456-a456-426614174000',
'987fcdeb-51a2-4c4b-9876-543210fedcba',
'a1b2c3d4-e5f6-4890-abcd-ef1234567890',
'f9e8d7c6-b5a4-4210-9876-543210fedcba',
];
const MOCK_METRICS_IDS = {
MOBILE_VALID: '123e4567-e89b-4456-a456-426614174000',
EXTENSION_VALID:
'0x86bacb9b2bf9a7e8d2b147eadb95ac9aaa26842327cd24afc8bd4b3c1d136420',
MOBILE_MIN: '00000000-0000-4000-8000-000000000000',
MOBILE_MAX: 'ffffffff-ffff-4fff-bfff-ffffffffffff',
EXTENSION_MIN: `0x${'0'.repeat(64) as string}`,
EXTENSION_MAX: `0x${'f'.repeat(64) as string}`,
};

const MOCK_FEATURE_FLAGS = {
VALID: {
Expand All @@ -28,26 +33,97 @@ const MOCK_FEATURE_FLAGS = {

describe('user-segmentation-utils', () => {
describe('generateDeterministicRandomNumber', () => {
it('generates consistent numbers for the same input', () => {
const result1 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]);
const result2 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]);
describe('Mobile client new implementation (uuidv4)', () => {
it('generates consistent results for same uuidv4', () => {
const result1 = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.MOBILE_VALID,
);
const result2 = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.MOBILE_VALID,
);
expect(result1).toBe(result2);
});

expect(result1).toBe(result2);
});
it('handles minimum uuidv4 value', () => {
const result = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.MOBILE_MIN,
);
expect(result).toBe(0);
});

it('generates numbers between 0 and 1', () => {
MOCK_METRICS_IDS.forEach((id) => {
const result = generateDeterministicRandomNumber(id);
it('handles maximum uuidv4 value', () => {
const result = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.MOBILE_MAX,
);
// For practical purposes, 0.999999 is functionally equivalent to 1 in this context
// the small deviation from exactly 1.0 is a limitation of floating-point arithmetic, not a bug in the logic.
expect(result).toBeCloseTo(1, 5);
});

it('results a random number between 0 and 1', () => {
const result = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.MOBILE_VALID,
);
expect(result).toBeGreaterThanOrEqual(0);
expect(result).toBeLessThanOrEqual(1);
});
});

it('generates different numbers for different inputs', () => {
const result1 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[0]);
const result2 = generateDeterministicRandomNumber(MOCK_METRICS_IDS[1]);
describe('Mobile client old implementation and Extension client (hex string)', () => {
it('generates consistent results for same hex', () => {
const result1 = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.EXTENSION_VALID,
);
const result2 = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.EXTENSION_VALID,
);
expect(result1).toBe(result2);
});

expect(result1).not.toBe(result2);
it('handles minimum hex value', () => {
const result = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.EXTENSION_MIN,
);
expect(result).toBe(0);
});

it('handles maximum hex value', () => {
const result = generateDeterministicRandomNumber(
MOCK_METRICS_IDS.EXTENSION_MAX,
);
expect(result).toBe(1);
});
});

describe('Distribution validation', () => {
it('produces uniform distribution across 1000 samples', () => {
const samples = 1000;
const buckets = 10;
const tolerance = 0.3;
const distribution = new Array(buckets).fill(0);

// Generate samples using valid UUIDs
Array.from({ length: samples }).forEach(() => {
const uuid = uuidV4();
const value = generateDeterministicRandomNumber(uuid);
const bucketIndex = Math.floor(value * buckets);
// Handle edge case where value === 1
distribution[
bucketIndex === buckets ? buckets - 1 : bucketIndex
] += 1;
});

// Check distribution
const expectedPerBucket = samples / buckets;
const allowedDeviation = expectedPerBucket * tolerance;

distribution.forEach((count) => {
const minExpected = Math.floor(expectedPerBucket - allowedDeviation);
const maxExpected = Math.ceil(expectedPerBucket + allowedDeviation);
expect(count).toBeGreaterThanOrEqual(minExpected);
expect(count).toBeLessThanOrEqual(maxExpected);
});
});
});
});

Expand Down
36 changes: 28 additions & 8 deletions packages/remote-feature-flag-controller/src/utils/user-segmentation-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,45 @@
import type { Json } from '@metamask/utils';
import { validate as uuidValidate, version as uuidVersion } from 'uuid';

import type { FeatureFlagScopeValue } from '../remote-feature-flag-controller-types';

/* eslint-disable no-bitwise */
/**
* Generates a deterministic random number between 0 and 1 based on a metaMetricsId.
* This is useful for A/B testing and feature flag rollouts where we want
* consistent group assignment for the same user.
*
* @param metaMetricsId - The unique identifier used to generate the deterministic random number
* Supports two metaMetricsId formats:
* - UUIDv4 format (new mobile implementation)
* - Hex format with 0x prefix (extension or old mobile implementation)
*
* For UUIDv4 format, the following normalizations are applied:
* - Removes all dashes from the UUID
* - Remove version (4) bits and replace with 'f'
Copy link
Member

@Gudahtt Gudahtt Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, I think we need to remove it rather than replace it with f. If we replace it with f, we're skewing the result that amount away from zero. But if we remove it completely, it won't impact the result.

Suggested change
* - Remove version (4) bits and replace with 'f'
* - Remove version (4) bits

Copy link
Member

@Gudahtt Gudahtt Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, though I notice that the test does show that the minimum UUID does result in zero. Odd. It looks like the result would be skewed to me though.

Copy link
Member

@Gudahtt Gudahtt Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I tested it and it does skew the results, but by too small an amount. You need to use a precision of 10^15 to see a non-zero result from the minimum input, but we're only using precision of 10^6.

Copy link
Member

@Gudahtt Gudahtt Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using an implicit minimum of zero, and a maximum of 'f' * length, perhaps we can achieve perfect distribution by updating the final calculation to something like this:

function uuidToBigInt(id: string) {
  return BigInt(`0x${uuid.replace(/-/gu, '')}`);
}

const MIN_UUID_V4 = '00000000-0000-4000-8000-000000000000';
const MAX_UUID_V4 = 'ffffffff-ffff-4fff-bfff-ffffffffffff';
const MIN_UUID_V4_BIGINT = uuidToBigInt(MIN_UUI_V4);
const MAX_UUID_V4_BIGINT = uuidToBigInt(MAX_UUI_V4);

...

export function generateDeterministicRandomNumber(
  metaMetricsId: string,
): number {
  let idValue: BigInt;
  let maxValue: BigInt;
  // uuidv4 format
  if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) {
    // Adjust both values by subtracting the minimum value, so that the result isn't biased away from zero
    idValue = uuidToBigInt(metaMetricsId) - MIN_UUID_V4_BIGINT;
    maxId = MAX_UUID_V4_BIGINT - MIN_UUID_V4_BIGINT;
  } else {
    // hex format with 0x prefix
    idValue = BigInt(`0x${metaMetricsId.slice(2)}`;
    maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`);
  }
  // Use BigInt division first, then convert to number to maintain precision
  return Number((value * BigInt(1_000_000)) / maxValue) / 1_000_000;
}

By adjusting the minimum and maximum for the UUIDv4 case, it ensures we use the entire range of 0-1.

Copy link
Member

@Gudahtt Gudahtt Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified the max, and it's also skewed for the same reason as the minimum, but only if you increase precision to 10^15. Not bad! Close enough that we won't notice, it's well under a percentage.

The suggestion I left here would give us a perfect distribution (JS rounding aside; it could get slightly better with bignumber.js and/or more precision). But what you have here is good enough I think.

Copy link
Contributor Author

@DDDDDanica DDDDDanica Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion ! Converting uuid to hex value and then normalizing the UUID range to start from 0 instead of removing the bits within is a much better solution indeed! Adapted in 8724cc3

* - Converts the remaining hex string to a BigInt for calculation
*
* For hex format:
* - Expects a hex string with '0x' prefix (e.g., '0x1234abcd')
* - Removes the '0x' prefix before conversion
* - Converts the remaining hex string to a BigInt for calculation
*
* @param metaMetricsId - The unique identifier used to generate the deterministic random number, can be a UUIDv4 or hex string
* @returns A number between 0 and 1 that is deterministic for the given metaMetricsId
*/
export function generateDeterministicRandomNumber(
metaMetricsId: string,
): number {
const hash = [...metaMetricsId].reduce((acc, char) => {
const chr = char.charCodeAt(0);
return ((acc << 5) - acc + chr) | 0;
}, 0);

return (hash >>> 0) / 0xffffffff;
let cleanId: string;
// uuidv4 format
if (uuidValidate(metaMetricsId) && uuidVersion(metaMetricsId) === 4) {
cleanId = metaMetricsId.replace(/-/gu, '').replace(/^(.{12})4/u, '$1f');
} else {
// hex format with 0x prefix
cleanId = metaMetricsId.slice(2);
}
const value = BigInt(`0x${cleanId}`);
const maxValue = BigInt(`0x${'f'.repeat(cleanId.length)}`);
// Use BigInt division first, then convert to number to maintain precision
return Number((value * BigInt(1_000_000)) / maxValue) / 1_000_000;
}

/**
Expand Down
Loading