Skip to content

Conversation

@paula-stacho
Copy link
Collaborator

@paula-stacho paula-stacho commented Dec 5, 2025

Description

Note: I've tested a basic react antipattern and the freeStorage tag, but mostly I think we'll just need to see how it performs on real PRs and keep adjusting for a good signal-to-noise ratio.

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
  • If this change could impact the load on the MongoDB cluster, please describe the expected and worst case impact
  • 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)

@paula-stacho paula-stacho requested a review from Copilot December 5, 2025 14:25
@paula-stacho paula-stacho marked this pull request as ready for review December 5, 2025 14:25
@paula-stacho paula-stacho requested a review from a team as a code owner December 5, 2025 14:25
@paula-stacho paula-stacho requested a review from nbbeeken December 5, 2025 14:25
Copy link
Contributor

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 adds custom Copilot instructions for code reviews and includes test changes to validate the reviewer's detection capabilities.

  • Adds a new .github/copilot-instructions.md file with guidelines for code reviews
  • Introduces intentional anti-pattern code in diagram-editor.tsx to test Copilot's detection

Reviewed changes

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

File Description
.github/copilot-instructions.md New file defining custom instructions for Copilot code reviews, covering performance, code quality, best practices, and MongoDB-specific guidelines
packages/compass-data-modeling/src/components/diagram-editor.tsx Adds unused import and intentional React anti-pattern (direct state mutation in useEffect) for testing purposes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@paula-stacho paula-stacho requested a review from Copilot December 5, 2025 14:39
Copy link
Contributor

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

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 239 to 240
obj.a = 'abc';
}, [obj]);
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

This code directly mutates the state object instead of using the setObj setter function. Direct state mutation bypasses React's state management and will not trigger re-renders. Replace obj.a = 'abc' with setObj({ a: 'abc' }). Additionally, the dependency array [obj] will cause an infinite loop since obj is recreated on every render. See React documentation on state updates.

Suggested change
obj.a = 'abc';
}, [obj]);
setObj({ a: 'abc' });
}, []);

Copilot uses AI. Check for mistakes.
@paula-stacho paula-stacho requested a review from Copilot December 5, 2025 14:44
Copy link
Contributor

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

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@paula-stacho paula-stacho requested a review from Copilot December 5, 2025 15:52
Copy link
Contributor

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

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const stats = await runCommand(
db,
{ dbStats: 1 },
{ dbStats: 1, freeStorage: 1 },
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The freeStorage: 1 option can have significant performance implications on large deployments as it requires scanning all storage metadata. Consider documenting this performance impact and evaluating if this metric is necessary for all use cases. See https://www.mongodb.com/docs/manual/reference/command/dbStats/#performance-considerations

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

Does this only configure the review bot or will it also change the local copilot experience. I think my comment may only be relevant to the local exp, generally I think we all do use the modern practices but I've seen copilot output a healthy amount of legacy stuff, it would be nice to steer it away


## Best practices

- Refer to official documentation and best practices for React.js, Redux and Node.js. If you see anti-patterns, highlight them and provide links to the relevant official documentation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess I don't want to give it too much of an opinion but either here or above I would add a mention of using modern JS practices since its been trained on a lot of legacy stuff, like I see copilot not using for-of when it should etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants