feat(compass-collection): add Schema Editor UI - Mock Data Generator CLOUDP-333854 #7295

ncarbon · 2025-09-08T15:15:38Z

Description

This pull request introduces the main UI and logic for the "Faker Schema Editor" step in the Mock Data Generator modal. It implements a new editor interface for mapping collection schema fields to faker.js functions, including validation of the generated mappings, and updates the modal's flow to handle loading and confirmation states.

schema_editor.mov

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

Bugfix
New feature
Dependency update
Misc

Open Questions

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)

packages/compass-collection/src/components/mock-data-generator-modal/validate-faker-schema.ts

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx

gribnoysup · 2025-09-09T16:14:27Z

@ncarbon I just checked out the latest version of your branch, running it locally, and nothing shows up after schema analysis

Does the latest changes in this branch work for you fine? Is there any extra setup I need to do to make it work?

packages/compass-collection/src/modules/collection-tab.ts

gribnoysup · 2025-09-09T16:32:52Z

packages/compass-collection/src/modules/collection-tab.ts

+      const fakerMethodWithArgs = eval(
+        `(faker, ...fakerArgs) => faker["${first}"]["${second}"](...fakerArgs)`
+      );
+      fakerMethodWithArgs(faker, ...fakerArgs);


This code is passing unsanitized generated output from LLM to the eval, this is a pretty big security risk, you shouldn't do it and you don't need to eval if you're just trying to check if the keys exist on the object, you can do this via dynamic property accessors: faker[first][second]

Good point. We were considering using eval to ensure that the faker args are correctly formed. I think we can take the Copilot suggestion below and first verify that fakerMethod is a valid faker function, then try to execute that method with the args (without using eval)

Calling the function also doesn't sound safe as this is also generated output / unvalidated user input we're passing to the call methods here, we shouldn't do that. There's no guarantees that this will not run something like faker.string.alpha(1000000000) and lock the main thread completely / crash the browser with OOM

Would adding a timeout alleviate this issue?

Or do you think it's best to just rely on the function name validation and trust the args are valid

In that case, I think it's best to validate the faker functions by checking that the function name exists in the faker object, and do without the args (let faker use default values).

There might be ways to "sandbox" the function execution, but doesn't look straightforward or worth it if we can just check the function IS a faker function.

@jcobis timeout doesn't change anything, it would just delay the execution in the same thread, all faker functions are sync as far as I can see and can easily lead to issues when misused (like most of these that accept length values for generated output can be easily made to generate too much and exhaust the memory).

@ncarbon exactly, there are ways you can try to do this in a way that doesn't affect the thread you are on, "sandbox" it, but all of them will be adding a significant chunk of complexity that I don't think this validation is worth. If y'all think that arguments can be omitted, that's probably the easiest way to move forward here. You can probably just not validate them and lease as-is, but this has a risk of suggesting destructive scripts to the users. You can also consider adding a full proper validation to the options that actually inspects the values, but I'm not very familiar with faker so hard for me to estimate the effort here

To be clear also, these arguments will not be subject to user input, it will only be output from the LLM

Created a thread here. To summarize: We can use .apply, and testing shows faker.js has built-in argument validation that prevents breaking operations

Rough demo draft PR I mocked up with AI, branching off Nataly’s, to demonstrate

packages/compass-collection/src/modules/collection-tab.ts

packages/compass-components/src/index.ts

Copilot

Pull Request Overview

This PR introduces the main UI and logic for the Faker Schema Editor step in the Mock Data Generator modal. It implements a new editor interface that allows users to review and modify mappings between collection schema fields and faker.js functions, including validation of faker methods and updates to handle loading and confirmation states.

Adds faker schema validation that verifies faker.js methods are valid and accessible
Creates new UI components for field selection and faker function mapping
Updates the modal flow to handle loading states and requires user confirmation before proceeding

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/compass-web/webpack.config.js	Adds @faker-js/faker as a bundled dependency
packages/compass-collection/src/modules/collection-tab.ts	Adds faker schema validation logic and updates type definitions
packages/compass-collection/src/components/mock-data-generator-modal/types.ts	Defines FakerSchemaMapping type and updates state types
packages/compass-collection/src/components/mock-data-generator-modal/schema-field-selector.tsx	New component for selecting schema fields in the editor
packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx	Updates modal to handle loading states and schema confirmation
packages/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx	Adds tests for new functionality
packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor.tsx	Main editor component for reviewing faker mappings
packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx	Component for displaying and editing faker function mappings
packages/compass-collection/package.json	Adds @faker-js/faker dependency

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

packages/compass-collection/src/modules/collection-tab.ts

Copilot · 2025-09-10T14:24:04Z

packages/compass-collection/src/modules/collection-tab.ts

+    try {
+      // Try with arguments first
+      const method = (faker as any)?.[first]?.[second];
+      if (typeof method !== 'function') {
+        throw new Error(`Faker method ${fakerMethod} is not a function`);
+      }
+      method(...fakerArgs);
+      return field;
+    } catch (error) {
+      // If that fails and there are arguments, try without arguments
+      if (fakerArgs.length > 0) {
+        try {
+          const method = (faker as any)?.[first]?.[second];
+          if (typeof method !== 'function') {
+            throw new Error(`Faker method ${fakerMethod} is not a function`);
+          }
+          method();
+          return field;
+        } catch (error) {


The method lookup logic (faker as any)?.[first]?.[second] is duplicated. Extract this into a helper function to reduce code duplication and improve maintainability.

Copilot · 2025-09-10T14:24:04Z

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx

      case MockDataGeneratorStep.DOCUMENT_COUNT:
        return <></>; // TODO: CLOUDP-333856
      case MockDataGeneratorStep.PREVIEW_DATA:
        return <></>; // TODO: CLOUDP-333857
      case MockDataGeneratorStep.GENERATE_DATA:
        return <ScriptScreen />;
    }
-  }, [currentStep]);
+  }, [currentStep, fakerSchemaGenerationState]);


The useMemo dependency array includes the entire fakerSchemaGenerationState object. Consider extracting only the specific properties needed (like fakerSchemaGenerationState.status and fakerSchemaGenerationState.fakerSchema) to prevent unnecessary re-renders when other properties change.

Suggested change

}, [currentStep, fakerSchemaGenerationState]);

}, [

currentStep,

fakerSchemaGenerationState.status,

fakerSchemaGenerationState.fakerSchema,

]);

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx

gribnoysup · 2025-09-10T14:27:08Z

packages/compass-collection/src/modules/collection-tab.ts

+/* eslint-disable @typescript-eslint/no-require-imports */
+const faker = require('@faker-js/faker/locale/en');


The root cause of the error you are seeing is that tsc for this package is targeting commonjs, on practice this doesn't really matter, so you can continue using import instead of switching it to require and add a expect-error notation instead of disabling an eslint rule

Suggested change

/* eslint-disable @typescript-eslint/no-require-imports */

const faker = require('@faker-js/faker/locale/en');

// @ts-expect-error TypeScript warns us about importing esm when output is cjs,

// but in reality this code will be just consumed by webpack, so it doesn't

// really matter

import { faker } from '@faker-js/faker/locale/en';

The benefits of that are that you still have proper types for the package (using require just completely bails out of the module resolution and so you just get any instead) and when compass repo fully supports esm and we are not targeting commonjs anymore, compiler will highlight this code (as it won't be an error any more) and we can clean it up.

Also generally speaking, I would strongly suggest to document why rules are being broken when they are broken 🙂 It is somewhat unavoidable sometimes to do so, indeed, but leaving the context is a good practice for a maintainable codebase

This was helpful. Thank you!

gribnoysup · 2025-09-11T12:33:08Z

packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx

+  return (
+    <div className={fieldMappingSelectorsStyles}>
+      <Body className={labelStyles}>Mapping</Body>
+      <Select


These select boxes allow to deselect the value, and as soon as I deselect it, the whole input control disappears and I can't select anything anymore, that's probably not an expected behavior?

gribnoysup · 2025-09-11T12:53:03Z

packages/compass-collection/src/components/mock-data-generator-modal/faker-mapping-selector.tsx

+        value={activeJsonType}
+        onChange={onJsonTypeSelect}
+      >
+        {[activeJsonType].map((type) => (


It's a bit hard to understand the scope of work from the ticket. Is that correct that there's actually no choice here? Is this work planned for later? If there's a follow-up work planned, we usually use // TODO(ticket number): description comments in the code to annotate that, it's low effort and should make reviewing for us easier

Added a //TODO comment with a description. We're planning to show dropdown options in a follow up ticket - CLOUDP-344400. They will be derived from a hard-coded map of JSON types to faker methods.

gribnoysup · 2025-09-11T13:00:57Z

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx

      case MockDataGeneratorStep.DOCUMENT_COUNT:
        return <></>; // TODO: CLOUDP-333856
      case MockDataGeneratorStep.PREVIEW_DATA:
        return <></>; // TODO: CLOUDP-333857
      case MockDataGeneratorStep.GENERATE_DATA:
        return <ScriptScreen />;
    }
-  }, [currentStep]);
+  }, [currentStep, fakerSchemaGenerationState]);


Missing a dependency on isSchemaConfirmed

Suggested change

}, [currentStep, fakerSchemaGenerationState]);

}, [currentStep, fakerSchemaGenerationState, isSchemaConfirmed]);

I'm seeing these warnings in VSCode coming from ESLint:

just wondering are they not showing up for you?

Good catch! I didn't have ESLint set up properly. Addressed.

gribnoysup · 2025-09-11T13:07:46Z

packages/compass-collection/src/components/mock-data-generator-modal/faker-schema-editor.tsx

+      <Button
+        size={ButtonSize.Small}
+        className={confirmMappingsButtonStyles}
+        variant={ButtonVariant.Primary}
+        onClick={onConfirmMappings}
+      >
+        Confirm mappings
+      </Button>


Changing the faker mappings or navigating to the previous step and then going back to the form doesn't change the confirmation status, it's a bit unexpected to me: if this is confirming currently selected values, then surely changing them should reset this 🙂 So just wanted to check: is this an expected behavior for this confirm button?

You're right. I addressed and changed the confirmation state logic to the following:

Reset the state when navigating to the previous state.

Reset the state when updating any of the inputs.

Preserve the state when navigating to the next state.

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx

packages/compass-collection/src/components/mock-data-generator-modal/types.ts

packages/compass-collection/package.json

packages/compass-collection/src/modules/collection-tab.ts

...mpass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx

gribnoysup · 2025-09-15T09:06:25Z

packages/compass-collection/src/modules/collection-tab.ts

+    const [moduleName, methodName] = fakerMethod.split('.');
+    if (typeof (faker as any)[moduleName]?.[methodName] !== 'function') {


Nit: probably better to make sure that llm output didn't include more parts than needed just in case

Suggested change

const [moduleName, methodName] = fakerMethod.split('.');

if (typeof (faker as any)[moduleName]?.[methodName] !== 'function') {

const [moduleName, methodName, ...rest] = fakerMethod.split('.');

if (rest.length === 0 && typeof (faker as any)[moduleName]?.[methodName] !== 'function') {

Good idea, we can include in Nataly's followup validation ticket

gribnoysup · 2025-09-15T09:07:06Z

packages/compass-collection/src/components/mock-data-generator-modal/types.ts

@@ -19,7 +19,7 @@ type MockDataGeneratorInProgressState = {

 type MockDataGeneratorCompletedState = {
  status: 'completed';
-  fakerSchema: MockDataSchemaResponse;
+  fakerSchema: Array<FakerSchemaMapping>;


Very small nit: we prefer compact array syntax in compass codebase

Suggested change

fakerSchema: Array<FakerSchemaMapping>;

fakerSchema: FakerSchemaMapping[];

jcobis

Nice work Nataly!

… update tests

… to accept a boolean parameter, and implemented state reset logic for schema confirmation. Enhanced tests to verify schema mapping state behavior during navigation.

…emaEditor to handle loading state, and improve type definitions for FakerSchemaMapping.

…kerSchema function

ncarbon changed the title ~~CLOUDP-333854: WIP~~ feat(compass-collection): add Schema Editor UI - Mock Data Generator CLOUDP-333854 Sep 8, 2025

github-actions bot added the feat label Sep 8, 2025

jcobis reviewed Sep 8, 2025

View reviewed changes

packages/compass-collection/src/components/mock-data-generator-modal/validate-faker-schema.ts Outdated Show resolved Hide resolved

jcobis reviewed Sep 8, 2025

View reviewed changes

packages/compass-collection/src/components/mock-data-generator-modal/validate-faker-schema.ts Outdated Show resolved Hide resolved

jcobis reviewed Sep 8, 2025

View reviewed changes

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx Outdated Show resolved Hide resolved

ncarbon added the no release notes Fix or feature not for release notes label Sep 9, 2025

ncarbon force-pushed the CLOUDP-333854 branch from 6e82d90 to 81c62ce Compare September 9, 2025 05:09

ncarbon marked this pull request as ready for review September 9, 2025 05:10

Copilot AI review requested due to automatic review settings September 9, 2025 05:10

ncarbon requested a review from a team as a code owner September 9, 2025 05:10

This comment was marked as outdated.

Sign in to view

gribnoysup reviewed Sep 9, 2025

View reviewed changes

packages/compass-collection/src/modules/collection-tab.ts Outdated Show resolved Hide resolved

gribnoysup reviewed Sep 9, 2025

View reviewed changes

packages/compass-collection/src/modules/collection-tab.ts Outdated Show resolved Hide resolved

gribnoysup reviewed Sep 9, 2025

View reviewed changes

packages/compass-components/src/index.ts Outdated Show resolved Hide resolved

ncarbon requested review from gribnoysup, Copilot, jcobis and kpamaran September 10, 2025 14:21

Copilot AI reviewed Sep 10, 2025

View reviewed changes

gribnoysup reviewed Sep 10, 2025

View reviewed changes

ncarbon force-pushed the CLOUDP-333854 branch from 475bc8d to 9357ca7 Compare September 10, 2025 20:48

gribnoysup reviewed Sep 11, 2025

View reviewed changes

jcobis mentioned this pull request Sep 11, 2025

Demo Rough Draft PR for Arg Validation #7315

Draft

kpamaran reviewed Sep 11, 2025

View reviewed changes

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx Outdated Show resolved Hide resolved

kpamaran reviewed Sep 11, 2025

View reviewed changes

...es/compass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.tsx Outdated Show resolved Hide resolved

kpamaran reviewed Sep 11, 2025

View reviewed changes

packages/compass-collection/src/components/mock-data-generator-modal/types.ts Outdated Show resolved Hide resolved

kpamaran reviewed Sep 11, 2025

View reviewed changes

packages/compass-collection/package.json Outdated Show resolved Hide resolved

kpamaran reviewed Sep 11, 2025

View reviewed changes

packages/compass-collection/src/modules/collection-tab.ts Show resolved Hide resolved

kpamaran reviewed Sep 11, 2025

View reviewed changes

packages/compass-collection/src/modules/collection-tab.ts Outdated Show resolved Hide resolved

kpamaran reviewed Sep 11, 2025

View reviewed changes

...mpass-collection/src/components/mock-data-generator-modal/mock-data-generator-modal.spec.tsx Show resolved Hide resolved

ncarbon mentioned this pull request Sep 12, 2025

feat(compass-collection): Mongosh Script Generation Engine - CLOUDP-333860 #7313

Merged

8 tasks

ncarbon force-pushed the CLOUDP-333854 branch from 23e57fa to 08b758f Compare September 12, 2025 20:43

gribnoysup approved these changes Sep 15, 2025

View reviewed changes

jcobis approved these changes Sep 15, 2025

View reviewed changes

ncarbon added 13 commits September 15, 2025 14:22

CLOUDP-333854: WIP

bd24caa

reorganize faker schema validation and improve component structure

64ac4b0

remove duplicate return

24b1b1b

address comments - safely call faker methods

f8d3723

install fakerjs as a prod dep

0269dbc

add fakerSchemaState to memo dependency list

b73eeb2

fix unit test

fc8713f

remove fakerArgs, fix faker import, adjust next button disable logic,…

37fda7f

… update tests

remove faker arg validation

cbe46b8

remove duplicate dependency - fakerjs

505627b

Added allowDeselect prop to select inputs, modified onSchemaConfirmed…

1258704

… to accept a boolean parameter, and implemented state reset logic for schema confirmation. Enhanced tests to verify schema mapping state behavior during navigation.

Update @faker-js/faker dependency to version 9.0.0, refactor FakerSch…

c9f3a29

…emaEditor to handle loading state, and improve type definitions for FakerSchemaMapping.

use shorthand array syntax and enhance validation logic in validateFa…

a43ebf2

…kerSchema function

ncarbon force-pushed the CLOUDP-333854 branch from 08b758f to a43ebf2 Compare September 15, 2025 18:51

kpamaran approved these changes Sep 15, 2025

View reviewed changes

ncarbon merged commit 8f62a7e into main Sep 15, 2025
57 of 58 checks passed

ncarbon deleted the CLOUDP-333854 branch September 15, 2025 20:26

		/* eslint-disable @typescript-eslint/no-require-imports */
		const faker = require('@faker-js/faker/locale/en');

-/* eslint-disable @typescript-eslint/no-require-imports */
-const faker = require('@faker-js/faker/locale/en');
+// @ts-expect-error TypeScript warns us about importing esm when output is cjs,
+// but in reality this code will be just consumed by webpack, so it doesn't
+// really matter
+import { faker } from '@faker-js/faker/locale/en';

	}, [currentStep, fakerSchemaGenerationState]);
	}, [currentStep, fakerSchemaGenerationState, isSchemaConfirmed]);

		const [moduleName, methodName] = fakerMethod.split('.');
		if (typeof (faker as any)[moduleName]?.[methodName] !== 'function') {

	fakerSchema: Array<FakerSchemaMapping>;
	fakerSchema: FakerSchemaMapping[];

feat(compass-collection): add Schema Editor UI - Mock Data Generator CLOUDP-333854 #7295

feat(compass-collection): add Schema Editor UI - Mock Data Generator CLOUDP-333854 #7295

Uh oh!

Conversation

ncarbon commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Motivation and Context

Open Questions

Dependents

Types of changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

gribnoysup commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcobis Sep 9, 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!

ncarbon Sep 10, 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!

gribnoysup Sep 11, 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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gribnoysup Sep 11, 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!

gribnoysup Sep 11, 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

ncarbon commented Sep 8, 2025 •

edited

Loading

gribnoysup commented Sep 9, 2025 •

edited

Loading

jcobis Sep 9, 2025 •

edited

Loading

ncarbon Sep 10, 2025 •

edited

Loading

gribnoysup Sep 11, 2025 •

edited

Loading

gribnoysup Sep 11, 2025 •

edited

Loading

gribnoysup Sep 11, 2025 •

edited

Loading

gribnoysup Sep 11, 2025 •

edited

Loading