Skip to content

feat(compass-collection): (re-merge) Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 #7203

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.

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
merged 8 commits into from
Aug 15, 2025

Conversation

ncarbon
Copy link
Collaborator

@ncarbon ncarbon commented Aug 15, 2025

Description

This was previously merged (#7177), then reverted due to failing unit tests.

This pull request introduces schema analysis functionality to the Compass Collection Tab, enabling the application to automatically analyze a collection's schema when it is loaded (unless the collection is read-only or time-series). The changes include new Redux state and actions for schema analysis, and a thunk to perform the analysis.

Motivation

Schema Analysis Feature:

  • Added a new SchemaAnalysis state to CollectionState, with status tracking (INITIAL, ANALYZING, COMPLETED, ERROR), schema data, sample document, schema metadata, and error information.
  • Implemented a thunk action analyzeCollectionSchema that samples documents, analyzes the schema, calculates metadata (like max nesting depth and validation rules), and updates the Redux state accordingly.
  • Updated the reducer to handle new schema analysis actions, updating the state based on progress and results.

Store Initialization and Integration:

  • Modified the store initialization to include the new schemaAnalysis state and inject required dependencies (logger, preferences, abort controller) for schema analysis.
  • On collection metadata load, the store now automatically dispatches schema analysis (unless the collection is read-only or time-series).

Testing Enhancements:

  • Updated and extended tests to mock and verify schema analysis behavior, ensuring it only runs for eligible collections and is skipped for read-only or time-series collections. [1] [2]

@github-actions github-actions bot added the feat label Aug 15, 2025
@ncarbon ncarbon added the no release notes Fix or feature not for release notes label Aug 15, 2025
@ncarbon ncarbon changed the title feat(compass-collection): Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 (Fixed unit tests) feat(compass-collection): Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 (re-merge) Aug 15, 2025
@ncarbon ncarbon marked this pull request as ready for review August 15, 2025 15:17
@Copilot Copilot AI review requested due to automatic review settings August 15, 2025 15:17
@ncarbon ncarbon requested a review from a team as a code owner August 15, 2025 15:17
@ncarbon ncarbon changed the title feat(compass-collection): Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 (re-merge) feat(compass-collection): (re-merge) Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 Aug 15, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reintroduces schema analysis functionality to the Compass Collection Tab, enabling automatic schema analysis when a collection loads (unless it's read-only or time-series). The implementation includes Redux state management, thunk actions for performing analysis, and comprehensive testing.

Reviewed Changes

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

Show a summary per file
File Description
packages/compass-collection/src/stores/collection-tab.ts Integrates schema analysis into store initialization and triggers analysis after metadata fetch
packages/compass-collection/src/stores/collection-tab.spec.ts Adds comprehensive tests for schema analysis behavior with different collection types
packages/compass-collection/src/schema-analysis-types.ts Defines TypeScript types for schema analysis state machine and data structures
packages/compass-collection/src/modules/collection-tab.ts Implements core schema analysis logic with Redux actions, reducer, and thunk
packages/compass-collection/src/calculate-schema-depth.ts Utility function to calculate maximum nesting depth of a schema with async processing
packages/compass-collection/src/calculate-schema-depth.spec.ts Tests for schema depth calculation with various nested structures
packages/compass-collection/package.json Adds dependencies for hadron-document, mongodb, and mongodb-schema packages

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

);
dispatch({
type: CollectionActions.SchemaAnalysisFailed,
error: err as Error,
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

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

Using 'any' type defeats TypeScript's type safety. Consider using 'unknown' instead and properly type-guard the error, or use a more specific error type like 'Error | MongoError'.

Suggested change
error: err as Error,
} catch (err: unknown) {
let errorMessage = 'Unknown error';
let errorObj: Error | MongoError;
if (
typeof err === 'object' &&
err !== null &&
'message' in err &&
typeof (err as { message?: unknown }).message === 'string'
) {
errorMessage = (err as { message: string }).message;
errorObj = err as Error | MongoError;
} else if (typeof err === 'string') {
errorMessage = err;
errorObj = new Error(errorMessage);
} else {
errorObj = new Error(errorMessage);
}
logger.log.error(
mongoLogId(1_001_000_363),
'Collection',
'Schema analysis failed',
{
namespace,
error: errorMessage,
}
);
dispatch({
type: CollectionActions.SchemaAnalysisFailed,
error: errorObj,

Copilot uses AI. Check for mistakes.

const unblockThread = async () =>
new Promise<void>((resolve) => setTimeout(resolve));

export async function calculateSchemaDepth(schema: Schema): Promise<number> {
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The magic number 1000 should be documented or made configurable. Consider adding a comment explaining why this specific interval was chosen for thread unblocking.

Suggested change
export async function calculateSchemaDepth(schema: Schema): Promise<number> {
/**
* Number of iterations after which to unblock the thread.
* The value 1000 was chosen as a balance between responsiveness and performance.
* Unblocking too frequently can slow down processing, while unblocking too rarely
* can make the UI unresponsive. Adjust as needed for your environment.
*/
const DEFAULT_UNBLOCK_INTERVAL_COUNT = 1000;
const unblockThread = async () =>
new Promise<void>((resolve) => setTimeout(resolve));
export async function calculateSchemaDepth(
schema: Schema,
unblockIntervalCount: number = DEFAULT_UNBLOCK_INTERVAL_COUNT
): Promise<number> {

Copilot uses AI. Check for mistakes.

schema.fields = schema.fields.filter(
({ path }) => !isInternalFieldPath(path[0])
);
// TODO: Transform schema to structure that will be used by the LLM.
Copy link
Preview

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

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

This TODO comment indicates incomplete functionality. Consider either implementing the transformation or creating a proper issue/ticket reference for tracking this work.

Suggested change
// TODO: Transform schema to structure that will be used by the LLM.
// Transform schema to structure that will be used by the LLM.
const llmSchema = transformSchemaForLLM(schema);

Copilot uses AI. Check for mistakes.

@ncarbon ncarbon changed the title feat(compass-collection): (re-merge) Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 feat(compass-collection): (re-merge) Schema Analysis Redux Integration for Collection Plugin CLOUDP-333846 Aug 15, 2025
@ncarbon ncarbon requested a review from gribnoysup August 15, 2025 15:31
@ncarbon ncarbon requested a review from jcobis August 15, 2025 15:49
@ncarbon ncarbon merged commit 78285b1 into main Aug 15, 2025
89 of 102 checks passed
@ncarbon ncarbon deleted the CLOUDP-333846 branch August 15, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat no release notes Fix or feature not for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants