Skip to content

Conversation

@gagik
Copy link
Contributor

@gagik gagik commented Aug 18, 2025

Adds the explain plan entry point.
Screenshot 2025-08-18 at 3 12 32 PM
Screenshot 2025-08-18 at 3 12 53 PM

Copilot AI review requested due to automatic review settings August 18, 2025 13:13
@gagik gagik requested a review from a team as a code owner August 18, 2025 13:13
@gagik gagik force-pushed the gagik/ai-entry-point branch 2 times, most recently from a0b2adf to 58624e8 Compare August 18, 2025 13:18
"mocha": "^10.2.0"
}
},
"packages/bson-transpilers/node_modules/@typescript-eslint/eslint-plugin": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is weird... I tried checking out main and re-running from a clean slate and this still happens... is main package-lock in a bad state?

Copy link
Contributor

Choose a reason for hiding this comment

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

It could be, but do double-check which version of npm you're running. I've seen it behave completely differently between versions recently giving me exactly this kind of behaviour.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Someone recently merged a pretty stale PR and this got in main, it's not great that these were duplicated, but also is not an issue really, this is a harmless cleanup, we're running checks in CI to make sure that package-lock in a good shape and doesn't have issues

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 an AI Assistant integration to the Explain Plan modal, allowing users to interpret explain plan output through natural language. The feature includes a new "Interpret for me" button that opens the AI Assistant drawer and sends the explain plan data for interpretation.

  • Adds AI Assistant integration to the Explain Plan modal with conditional rendering based on preferences
  • Implements explain plan interpretation functionality that converts JSON data to natural language
  • Updates type definitions to support custom display text for AI Assistant messages

Reviewed Changes

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

Show a summary per file
File Description
packages/compass-explain-plan/src/components/explain-plan-modal.tsx Adds "Interpret for me" button and AI Assistant integration to the modal header
packages/compass-explain-plan/src/components/explain-plan-modal.spec.tsx Adds tests for AI Assistant button visibility based on preferences
packages/compass-explain-plan/package.json Adds compass-assistant dependency
packages/compass-assistant/src/prompts.ts Creates explain plan prompt template for AI interpretation
packages/compass-assistant/src/index.tsx Exports useAssistantActions hook
packages/compass-assistant/src/compass-assistant-provider.tsx Implements interpretExplainPlan action and extends message type with metadata
packages/compass-assistant/test/utils.tsx Updates mock chat utility to use new AssistantMessage type
packages/compass-assistant/src/compass-assistant-provider.spec.tsx Updates tests to use new AssistantMessage type
packages/compass-assistant/src/assistant-chat.tsx Adds support for displaying custom text via message metadata
packages/compass-assistant/src/assistant-chat.spec.tsx Adds tests for custom display text functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

?.filter((part) => part.type === 'text')
.map((part) => part.text)
.join('') ||
''}
Copy link

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

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

[nitpick] The display logic uses a complex ternary expression that could be simplified for better readability. Consider extracting this into a helper function or variable to make the intent clearer.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Personally I would do this, but it is indeed nitpicky.

interpretExplainPlan: (explainPlan: string) => {
openDrawer(ASSISTANT_DRAWER_ID);
const { prompt, displayText } = buildExplainPlanPrompt(explainPlan);
void chat.sendMessage(
Copy link

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

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

[nitpick] Using 'void' to ignore the promise is acceptable here, but consider adding error handling for the sendMessage call to provide better user feedback if the operation fails.

Suggested change
void chat.sendMessage(
chat.sendMessage(

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will be followed up later and handled with onError on useChat hook

Explain output:
${explainPlan}`,
displayText:
'Given this MongoDB explain plan, provide a concise human readable explanation that explains the query execution plan and highlights aspects of the plan that might impact query performance.',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if for displaying purposes it's worth including something relevant to specific explain that was run, like a namespace or something?

Copy link
Contributor Author

@gagik gagik Aug 19, 2025

Choose a reason for hiding this comment

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

we could if we get designs to use them but from my understanding for v1 we're just going to hide i.e. the explain plan and let the response include relevant enough context.

Ah, it could also be in the text, right, good idea. I will ask this to Misba and add it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The explain might be very verbose to be converted to text, but I think even just adding a namespace to display text will go a long way here. I was playing around with this feature across multiple tabs and at some point it was not easy to navigate. To be very fair though this usage pattern is probably a corner case that doesn't matter that much 🙂

Comment on lines 108 to 109
const isAiAssistantEnabled = usePreference('enableAIAssistant');
const { interpretExplainPlan } = useAssistantActions();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be a silly idea, but I wonder if we will almost always have these two together, maybe worth "re-exporting" the enabled prop as part of the useAssistant hook return value

Comment on lines 139 to 152
<Button
size="small"
variant="default"
leftGlyph={
// TODO(COMPASS-9384): Will be replaced with Sparkle gradient icon once Leafygreen components are updated.
<Icon glyph="Sparkle" style={{ color: palette.green.dark1 }} />
}
data-testid="interpret-for-me-button"
onClick={() => {
onModalClose();
interpretExplainPlan(JSON.stringify(explainPlan));
}}
>
Interpret for me
</Button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This button should be disabled / hidden while we're still running the explain, otherwise you're going to be explaining no plan

@gagik gagik force-pushed the gagik/ai-entry-point branch from 87fd0ae to 0e82ba8 Compare August 19, 2025 08:35
}) => {
return {
prompt: `Given the MongoDB explain plan output below, provide a concise human readable explanation that explains the query execution plan and highlights aspects of the plan that might impact query performance. Respond with as much concision and clarity as possible.
If a clear optimization should be made, please suggest the optimization and describe how it can be accomplished in MongoDB Compass. Do not advise users to create indexes without weighing the pros and cons.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm so skeptical about this 😆

@lerouxb
Copy link
Contributor

lerouxb commented Aug 20, 2025

Tested it locally and it works great!

@gagik gagik merged commit dc1c477 into main Aug 20, 2025
54 of 58 checks passed
@gagik gagik deleted the gagik/ai-entry-point branch August 20, 2025 13:04
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.

4 participants