feat: Compass Experimentation Provider CLOUDP-333843 #7151

jcobis · 2025-07-29T17:24:47Z

Description

Compass defines the CompassExperimentationProvider component and context interface
Provider will accept useAssignment and assignExperiment utils as props from MMS

Motivation and Context

To re-use experimentation logic built on top of the Experiment SDK in MMS, we will allow mms to provide required hooks and callback methods to compass through a React-context-based dependency-injection pattern.

Compass will define an CompassExperimentationProvider interface, which MMS will provide the implementation functions for. This follows conventional React dependency injection patterns and allows Compass to define what it needs from MMS for its experimentation architecture.

Compass will define the experimentation provider in the compass-telemetry package and export it through compass-web. MMS will then wrap the Compass application with this provider while supplying the necessary experiment functions.

(We have already started to accumulate experiment-related code in the compass-telemetry package; this integration creates a dedicated, shared place for experimentation code in the compass repository.)

Open Questions

Do we like defining these experiment types in Compass? Or should we import from MMS?

Dependents

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

ncarbon · 2025-07-29T19:38:37Z

packages/compass-telemetry/src/experimentation-provider.tsx

+}
+
+interface ExperimentData {
+  assignmentDate: Date;


Are these types supposed to identically match the types in the experiment sdk?
Some of these values are typed slightly differently. For example assignmentDate is typed as a string.

A few others including testGroupId are nullable in that types.ts file too

[nit] Alternatively, maybe the mdb-experiment-js-sdk can be imported into compass's dev deps and then the types imported/derived from there to here

Good idea @kpamaran! If it's a heavy-lift, we should consider it as a wrap-up item.

We can add the package as a dependency for the purposes of sharing types, this makes sense, but to make sure it's not being used for anything else, let's add a eslint rule to only allow type imports from it (we do this for a couple of other packages, so you can look at those rules as an example)

ncarbon · 2025-07-29T19:45:54Z

packages/compass-telemetry/src/experimentation-provider.tsx

+    useAssignment() {
+      return {
+        assignment: null,
+        asyncStatus: 'SUCCESS' as const,
+      };
+    },
+    assignExperiment() {
+      return Promise.resolve(null);
+    },


[nit] Consider making this its own const e.g. initialContext or so.

gribnoysup · 2025-07-30T07:36:40Z

packages/compass-telemetry/src/experimentation-provider.tsx

+  const contextValue = useMemo(
+    () => ({ useAssignment, assignExperiment }),
+    [useAssignment, assignExperiment]
+  );


This shouldn't be a memo, memo is too unstable for something that can cause erroneous re-renders across the whole application and this can be easily broken by someone changing code outside of compass-web control. We know that these functions are not changed through the application lifecycle, so this can be a simple ref that's initialized once and never changes while component is mounted:

Suggested change

const contextValue = useMemo(

() => ({ useAssignment, assignExperiment }),

[useAssignment, assignExperiment]

);

const { current: contextValue } = useRef({ useAssignment, assignExperiment });

If you want to make sure that the latest function is passed (somewhat wrong in this case because hook implementation should never change like that), you can keep the current value updated on renders, but still the value that gets passed to provider should be as stable as possible:

Suggested change

const contextValue = useMemo(

() => ({ useAssignment, assignExperiment }),

[useAssignment, assignExperiment]

);

const { current: contextValue } = useRef({ useAssignment, assignExperiment });

contextValue.useAssignment = useAssignment;

contextValue.assignExperiment = assignExperiment;

Ok! Makes sense to me. I was responding to this Copilot Review comment. I will revert.

Copilot

Pull Request Overview

This PR introduces a React-based experimentation provider that enables Compass to receive experimentation utilities from MMS through dependency injection. The implementation follows conventional React patterns for sharing experiment assignment functionality across the Compass application.

Key changes:

Created CompassExperimentationProvider React context component for dependency injection
Added type definitions for experiment assignment hooks and functions
Exported the provider through the compass-web package for MMS integration

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/compass-web/src/index.tsx	Exports the CompassExperimentationProvider for external consumption
packages/compass-telemetry/src/index.ts	Re-exports the experimentation provider from the telemetry package
packages/compass-telemetry/src/experimentation-provider.tsx	Implements the React context provider with experiment assignment functionality
packages/compass-telemetry/package.json	Adds dependency on @mongodb-js/mdb-experiment-js library
packages/compass-telemetry/.eslintrc.js	Configures ESLint to only allow type imports from the experiment library

Comments suppressed due to low confidence (1)

packages/compass-telemetry/package.json:59

Please verify that version 1.9.0 of @mongodb-js/mdb-experiment-js exists. Given that my knowledge cutoff is January 2025 and it's currently July 2025, this version may have been released after my training data, but it's worth confirming the version exists in the registry.

    "@mongodb-js/mdb-experiment-js": "1.9.0"

Copilot · 2025-07-30T18:57:25Z

packages/compass-telemetry/src/experimentation-provider.tsx

+  const { current: contextValue } = useRef({ useAssignment, assignExperiment });
+  contextValue.useAssignment = useAssignment;
+  contextValue.assignExperiment = assignExperiment;


The useRef pattern here won't prevent re-renders as intended. The context value object is recreated on every render because it captures the current prop values. Consider using useMemo with proper dependencies instead: const contextValue = useMemo(() => ({ useAssignment, assignExperiment }), [useAssignment, assignExperiment]);

Suggested change

const { current: contextValue } = useRef({ useAssignment, assignExperiment });

contextValue.useAssignment = useAssignment;

contextValue.assignExperiment = assignExperiment;

const contextValue = React.useMemo(

() => ({ useAssignment, assignExperiment }),

[useAssignment, assignExperiment]

);

Ignoring this, see above discussion

mabaasit · 2025-07-31T09:52:02Z

packages/compass-telemetry/.eslintrc.js

@@ -5,4 +5,26 @@ module.exports = {
    tsconfigRootDir: __dirname,
    project: ['./tsconfig-lint.json'],
  },
+  overrides: [


[nit]: if this package is intended to be used within compass-telemetry only then it makes sense to add it here, or else you can add it to base eslint config file: https://github.com/mongodb-js/compass/blob/main/configs/eslint-config-compass/index.js

I don't think we'll need these types outside this package! We'll import useAssignment or assignExperiment, but I don't think we'll have to define any other objects with the types.

diiiefiend · 2025-07-31T15:44:47Z

packages/compass-telemetry/src/experimentation-provider.tsx

+type UseAssignmentHook = (
+  experimentName: string,
+  trackIsInSample: boolean,
+  options?: typesReact.UseAssignmentOptions<types.TypeData>
+) => typesReact.UseAssignmentResponse<types.TypeData>;
+
+type AssignExperimentFn = (
+  experimentName: string,
+  options?: types.AssignOptions<string>
+) => Promise<types.AsyncStatus | null>;


Do we want to note that these should be kept in sync with the signatures in the SDK (with a link to where the SDK defines these)? I assume so, since these values are coming from MMS, which will be following the SDK conventions.

This does feel a little brittle, but it'll prob do for now til if/when we make the SDK a first-order dep in this repo.

My understanding is that the parent type signatures (UseAssignmentHook and AssignExperimentFn) are actually defined in MMS to type the useAssignment and assign functions respectively (to match the SDK functions), and that MMS uses the SDK types (typesReact.UseAssignmentOptions, typesReact.UseAssignmentResponse, etc.) as elements in those parent type signatures, but the parent type signatures are not exported by the SDK or MMS. Is that right?

So should we link to the MMS or SDK useAssignment?

Going to merge this in to unblock tickets and add the comment on next compass PR

diiiefiend

LGTM, just had the 1 nit about adding a comment 🧪

WIP

d1ff4f7

Copilot AI review requested due to automatic review settings July 29, 2025 17:24

jcobis requested a review from a team as a code owner July 29, 2025 17:24

github-actions bot added the feat label Jul 29, 2025

This comment was marked as outdated.

Sign in to view

Copilot review comment

8a3704e

jcobis changed the title ~~feat: CLOUDP-333843: Compass Experimentation Provider~~ feat: Compass Experimentation Provider CLOUDP-333843 Jul 29, 2025

jcobis added the no release notes Fix or feature not for release notes label Jul 29, 2025

jcobis requested a review from Copilot July 29, 2025 17:40

This comment was marked as outdated.

Sign in to view

jcobis requested a review from diiiefiend July 29, 2025 17:43

mongodb-js deleted a comment from Copilot AI Jul 29, 2025

jcobis requested a review from gribnoysup July 29, 2025 17:53

Rename provider to CompassExperimentationProvider

34155ed

ncarbon reviewed Jul 29, 2025

View reviewed changes

gribnoysup reviewed Jul 30, 2025

View reviewed changes

jcobis added 2 commits July 30, 2025 11:02

useMemo -> useRef

87680b7

Import SDK

19b0953

jcobis requested review from gribnoysup, ncarbon, Copilot and kpamaran July 30, 2025 18:56

Copilot AI reviewed Jul 30, 2025

View reviewed changes

jcobis added 4 commits July 30, 2025 14:58

initialContext nit comment

533719e

Package Lock

4614599

Tweak

7c72f4b

WIP

9f9b0ac

mabaasit reviewed Jul 31, 2025

View reviewed changes

mabaasit approved these changes Jul 31, 2025

View reviewed changes

diiiefiend reviewed Jul 31, 2025

View reviewed changes

diiiefiend approved these changes Jul 31, 2025

View reviewed changes

jcobis merged commit 7705104 into main Jul 31, 2025
59 checks passed

jcobis deleted the CLOUDP-333843 branch July 31, 2025 17:01

feat: Compass Experimentation Provider CLOUDP-333843 #7151

feat: Compass Experimentation Provider CLOUDP-333843 #7151

Uh oh!

Conversation

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

Uh oh!

Description

Motivation and Context

Open Questions

Dependents

Types of changes

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kpamaran 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!

kpamaran 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diiiefiend Jul 31, 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!

jcobis Jul 31, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

diiiefiend left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jcobis commented Jul 29, 2025 •

edited

Loading

kpamaran Jul 29, 2025 •

edited

Loading

kpamaran Jul 29, 2025 •

edited

Loading

diiiefiend Jul 31, 2025 •

edited

Loading

jcobis Jul 31, 2025 •

edited

Loading