feat(compass-collection): CLOUDP-347048 LLM Output Validation Followup - Mock Data Generator #7365

ncarbon · 2025-09-23T16:42:18Z

Description

This pull request introduces more validation and rendering improvements for handling faker method arguments in the mock data generator modal. The main changes include adding comprehensive validation logic for faker arguments and method names, updating UI components to display faker arguments, and refactoring validation logic out of the module to a shared utility.

Validation and Utility Enhancements:

Added areFakerArgsValid and improved isValidFakerMethod utility functions in utils.ts to thoroughly validate faker arguments (including nested and object types) and method names, enforcing limits on argument length, string size, and value types.
Refactored the schema validation logic in collection-tab.ts to use the new isValidFakerMethod utility, ensuring faker method and argument validation is consistent and centralized. [1] [2] [3]

Type and Interface Updates:

Extended the FakerArg type to support nested arrays, enabling more flexible representation of faker arguments.

UI and Rendering Improvements:

Updated FakerMappingSelector to render TextInput components for each faker argument, supporting various types (string, number, boolean, arrays, and objects) and displaying them in a read-only format. [1] [2] [3]
Updated FakerSchemaEditorContent to pass the active faker arguments to the mapping selector for display. [1] [2]

Testing:

Added comprehensive unit tests for areFakerArgsValid and isValidFakerMethod in utils.spec.ts, covering edge cases such as deeply nested structures, invalid argument types, and method existence.
Added an integration test to ensure that excessively large faker arguments are not displayed in the modal.

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)

…p - Mock Data Generator

ncarbon · 2025-09-23T16:42:42Z

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

      },
    };

+    // @ts-expect-error // TODO: fix ts error


Fixing this

Copilot

Pull Request Overview

This PR enhances the mock data generator modal with comprehensive validation and rendering improvements for faker method arguments. The changes focus on improving argument validation, centralizing validation logic, and enabling UI display of faker arguments.

Adds comprehensive validation for faker arguments including nested structures, type checking, and size limits
Refactors validation logic from collection-tab.ts to a shared utility module for better maintainability
Updates UI components to display faker arguments as read-only text inputs with proper type handling

Reviewed Changes

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

Show a summary per file

File	Description
collection-tab.ts	Refactors validation to use centralized utility function
utils.ts	Introduces comprehensive faker argument and method validation utilities
utils.spec.ts	Adds extensive unit tests for validation functions
script-generation-utils.ts	Extends FakerArg type to support nested arrays
mock-data-generator-modal.spec.tsx	Adds integration test for oversized argument handling
faker-schema-editor-screen.tsx	Passes faker arguments to mapping selector
faker-mapping-selector.tsx	Implements UI rendering for faker arguments

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

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

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

codeowners-service-app · 2025-09-23T17:20:33Z

Assigned gagik for team compass-developers because mabaasit is out of office.

jcobis · 2025-09-23T19:16:38Z

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

+        expect(result.fakerArgs).to.deep.equal([]);
+      });
+
+      it('returns false for valid method with invalid arguments and fallback fails', () => {


Shouldn't this be invalid arguments with valid method falls back to true and strips args? That's what's happening I think, which is why this test is failing right now

Yep, forgot to update it.

jcobis · 2025-09-23T19:17:41Z

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

+        expect(result.fakerArgs).to.deep.equal([]);
+      });
+
+      it('returns false for method with invalid fakerArgs', () => {


My comment applies here too I believe. Both tests should expect isValid: true and fakerArgs: []

jcobis · 2025-09-23T19:28:13Z

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

+  try {
+    if (areFakerArgsValid(args)) {
+      callable(...args);
+      return { isValid: true, fakerArgs: args };
+    } else {
+      callable();
+      return { isValid: true, fakerArgs: [] };
+    }
+  } catch {
+    // Intentionally ignored: error may be due to invalid arguments, will retry without args.
+    if (args.length > 0) {
+      try {
+        callable();
+        return { isValid: true, fakerArgs: [] };
+      } catch {
+        // Intentionally ignored: method is invalid without arguments as well.
+        return { isValid: false, fakerArgs: [] };
+      }
+    }
+  }
+  return { isValid: false, fakerArgs: [] };


What if we refactored a bit for clarity:

Suggested change

try {

if (areFakerArgsValid(args)) {

callable(...args);

return { isValid: true, fakerArgs: args };

} else {

callable();

return { isValid: true, fakerArgs: [] };

}

} catch {

// Intentionally ignored: error may be due to invalid arguments, will retry without args.

if (args.length > 0) {

try {

callable();

return { isValid: true, fakerArgs: [] };

} catch {

// Intentionally ignored: method is invalid without arguments as well.

return { isValid: false, fakerArgs: [] };

}

}

}

return { isValid: false, fakerArgs: [] };

// If args are present and safe, try calling with args

if (args.length > 0 && areFakerArgsValid(args)) {

try {

callable(...args);

return { isValid: true, fakerArgs: args };

} catch {

// Call with args failed. Fall through to trying without args

}

}

// Try without args (either because args were invalid or args failed)

try {

callable();

return { isValid: true, fakerArgs: [] };

} catch {

// Method doesn't work even without args

return { isValid: false, fakerArgs: [] };

}

jcobis · 2025-09-23T19:30:49Z

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

+    } else if (typeof arg === 'boolean') {
+      // booleans are always valid, continue
+    } else if (Array.isArray(arg)) {
+      if (!areFakerArgsValid(arg)) {


Should we limit the depth of this to prevent potential infinite recursion? We could pass a depth param and limit to say, 3 levels

kpamaran · 2025-09-23T20:54:16Z

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

+      callable();
+      return { isValid: true, fakerArgs: [] };
+    }
+  } catch {


A warning when there's an exception or the args are invalid would be valuable for fine tuning the prompt

kpamaran · 2025-09-23T21:03:11Z

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

+
+      // advance to the schema editor step
+      userEvent.click(screen.getByText('Confirm'));
+      await waitFor(() => {


seems to be an assertion missing? I'm not connecting how "faker args that are too large" are not shown

kpamaran · 2025-09-23T21:15:47Z

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

+/**
+ * Renders read-only TextInput components for each key-value pair in a faker arguments object.
+ */
+const getFakerArgsInputFromObject = (fakerArgsObject: Record<string, any>) => {


nit: use render in name to clarify React Elements are returned, something like renderFakerArgsInput

kpamaran · 2025-09-23T21:18:22Z

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

+        <TextInput
+          key={`faker-arg-${idx}`}
+          type="text"
+          label="Faker Arg"


labels should be unique across the form's inputs

kpamaran · 2025-09-23T21:23:29Z

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

+ */
+const getFakerArgsInputFromObject = (fakerArgsObject: Record<string, any>) => {
+  return Object.entries(fakerArgsObject).map(([key, item]: [string, any]) => {
+    if (typeof item === 'string' || typeof item === 'boolean') {


shouldn't boolean use something like a radio option instead of arbitrary text?

kpamaran · 2025-09-23T21:27:59Z

The faker arg form experience itself is interesting, I wonder how well users can intuit that the args are positional. Could be a follow-up. Maybe the user can be shown an inline preview of the faker call (cc @jcobis), right now they'd need to make the connection by reading the Script output

ncarbon · 2025-09-23T21:33:17Z

The faker arg form experience itself is interesting, I wonder how well users can intuit that the args are positional. Could be a follow-up. Maybe the user can be shown an inline preview of the faker call (cc @jcobis), right now they'd need to make the connection by reading the Script output

Yeah, good question. If we can't show a specific label for each faker argument, an inline preview of the faker call would be good UI to show.

feat(compass-collection): CLOUDP-347048 LLM Output Validation Followu…

d9f6a23

…p - Mock Data Generator

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

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

ncarbon commented Sep 23, 2025

View reviewed changes

ncarbon marked this pull request as ready for review September 23, 2025 17:15

ncarbon requested a review from a team as a code owner September 23, 2025 17:15

ncarbon requested review from mabaasit and Copilot September 23, 2025 17:15

Copilot AI reviewed Sep 23, 2025

View reviewed changes

codeowners-service-app bot requested a review from gagik September 23, 2025 17:20

call faker method without args if invalid

406aa2b

jcobis reviewed Sep 23, 2025

View reviewed changes

kpamaran reviewed Sep 23, 2025

View reviewed changes

update faker argument handling and improve validation logging

0b079fb

kpamaran reviewed Sep 23, 2025

View reviewed changes

resolve TypeScript error by casting initialState to any

df49911

feat(compass-collection): CLOUDP-347048 LLM Output Validation Followup - Mock Data Generator #7365

Are you sure you want to change the base?

feat(compass-collection): CLOUDP-347048 LLM Output Validation Followup - Mock Data Generator #7365

Uh oh!

Conversation

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

ncarbon Sep 23, 2025

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codeowners-service-app bot commented Sep 23, 2025

Uh oh!

jcobis Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

ncarbon Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

jcobis Sep 23, 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 Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

jcobis Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

kpamaran Sep 23, 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 Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

kpamaran Sep 23, 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 Sep 23, 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 Sep 23, 2025

Choose a reason for hiding this comment

Uh oh!

kpamaran commented Sep 23, 2025

Uh oh!

ncarbon commented Sep 23, 2025

Uh oh!

Uh oh!

ncarbon commented Sep 23, 2025 •

edited

Loading

jcobis Sep 23, 2025 •

edited

Loading

kpamaran Sep 23, 2025 •

edited

Loading

kpamaran Sep 23, 2025 •

edited

Loading

kpamaran Sep 23, 2025 •

edited

Loading