Skip to content

feat: Make A/B tests independent #7511

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

Jump to bottom
Merged
MarioAslau merged 30 commits into from
Dec 19, 2025
Merged

feat: Make A/B tests independent #7511

MarioAslau merged 30 commits into from
Dec 19, 2025
+952 −134

Conversation

@MarioAslau
Copy link
Contributor

@MarioAslau MarioAslau commented Dec 16, 2025 •
edited by cursor bot
Loading

Explanation

Right now, only the ID is used as a seed to determine what bucket the user falls into in an A/B test.

Due to this, if there’s more than 1 A/B test configured, they will never be independent of each other

Solution:
Optimizes threshold-based feature flag processing to ensure independent user assignment across different flags. Define the distribution using profile/metametrics ID + A/B test ID as a seed instead of just profile/metametrics ID

Key changes:

  • Use sha256 from @metamask/[email protected] to generate unique threshold per flag (native crypto.subtle.digest - 10-100x faster)
  • Persist threshold values in controller state to avoid recalculating on app restart
  • Skip cryptographic operations for non-threshold arrays
  • Batch cache updates and cleanup into single state change
  • Automatically remove stale cache entries when flags are deleted
  • Remove @noble/hashes dependency (now available in @metamask/utils)

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Implements per-flag deterministic A/B assignment with cached thresholds in the Remote Feature Flag Controller and upgrades @metamask/utils to ^11.9.0 repo-wide.

  • Remote Feature Flag Controller:
    • Implement per-flag threshold selection using sha256(metaMetricsId + flagName) via @metamask/utils@^11.9.0.
    • Add thresholdCache to state; batch cache updates and clean up stale entries; skip processing for non-threshold arrays.
    • Update tests and expectations; remove reliance on external hashing utility.
  • Dependencies:
    • Upgrade @metamask/utils to ^11.9.0 across packages (yarn.lock updated).

Written by Cursor Bugbot for commit 297369a. This will update automatically on new commits. Configure here.

@MarioAslau MarioAslau requested review from a team as code owners December 16, 2025 03:47
@MarioAslau MarioAslau requested a review from salimtb December 16, 2025 03:48
@MarioAslau MarioAslau self-assigned this Dec 16, 2025
asalsys
asalsys requested changes Dec 16, 2025
View reviewed changes
@MarioAslau MarioAslau linked an issue Dec 17, 2025 that may be closed by this pull request
Enable A/B Testing in RemoteFeatureFlag - Make a/b tests independent MetaMask/metamask-mobile#24029
Closed
9 tasks
@MarioAslau MarioAslau removed the request for review from salimtb December 17, 2025 00:05
@MarioAslau MarioAslau requested a review from asalsys December 17, 2025 00:05
@asalsys
Copy link
Contributor

asalsys commented Dec 17, 2025

LGTM!

@Gudahtt
Copy link
Member

Gudahtt commented Dec 17, 2025

Why was no-changelog applied?

@MarioAslau
Copy link
Contributor Author

MarioAslau commented Dec 17, 2025

Why was no-changelog applied?

Sorry @Gudahtt, reflex from the mobile repo

Gudahtt
Gudahtt reviewed Dec 19, 2025
View reviewed changes
packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts

if (thresholdValue === undefined) {
// eslint-disable-next-line require-atomic-updates
thresholdValue = await calculateThresholdForFlag(
Copy link
Member

@Gudahtt Gudahtt Dec 19, 2025

Choose a reason for hiding this comment

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

Nit: We should be able to calculate these thresholds in parallel, rather than one-at-a-time. Given that there is this async step, it's possible this could significantly speed up the operation

@MarioAslau MarioAslau changed the title feat: Make a/b tests independent feat: Make A/B tests independent Dec 19, 2025
@MarioAslau MarioAslau requested a review from Gudahtt December 19, 2025 21:22
Gudahtt
Gudahtt reviewed Dec 19, 2025
View reviewed changes
Gudahtt
Gudahtt approved these changes Dec 19, 2025
View reviewed changes
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@MarioAslau MarioAslau enabled auto-merge December 19, 2025 21:44
@MarioAslau MarioAslau added this pull request to the merge queue Dec 19, 2025
Merged via the queue into main with commit da6abc0 Dec 19, 2025
286 checks passed
@MarioAslau MarioAslau deleted the feat/ab-independent branch December 19, 2025 21:45
@MarioAslau
Copy link
Contributor Author

MarioAslau commented Dec 19, 2025

@metamaskbot publish-preview

@MarioAslau MarioAslau mentioned this pull request Dec 19, 2025
Merged
7 tasks
github-merge-queue bot pushed a commit to MetaMask/metamask-mobile that referenced this pull request Dec 20, 2025
@MarioAslau
feat: increase version of remote-feature-flag-controller to 4.0.0 (#2…
e03727e 
…4211)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

Bumping version of remote-feature-flag-controller to support independent
A/B tests introduced in:
- PR: MetaMask/core#7511
- Core release: MetaMask/core#7546

## **Changelog**

<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`

If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`

(This helps the Release Engineer do their job more quickly and
accurately)
-->

CHANGELOG entry:

## **Related issues**

Fixes:

## **Manual testing steps**

```gherkin
Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]
```

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> Bumps `@metamask/remote-feature-flag-controller` to 4.0.0 and
refreshes lockfile, updating related `@metamask/utils` and
`@metamask/controller-utils` versions.
> 
> - **Dependencies**:
> - Upgrade `@metamask/remote-feature-flag-controller` from `^3.1.0` to
`^4.0.0` in `package.json`.
> - Update `yarn.lock` resolutions accordingly, including
`@metamask/utils` to `11.9.0` and `@metamask/controller-utils` to
`11.17.0`.
> 
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
8a5dc27. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FrederikBolding FrederikBolding FrederikBolding left review comments

@cursor cursor[bot] cursor[bot] left review comments

@Gudahtt Gudahtt Gudahtt approved these changes

@asalsys asalsys asalsys approved these changes

Assignees

@MarioAslau MarioAslau

Labels

team-mobile-platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable A/B Testing in RemoteFeatureFlag - Make a/b tests independent

5 participants

@MarioAslau @asalsys @Gudahtt @FrederikBolding