-
Notifications
You must be signed in to change notification settings - Fork 1
feat(mpp-idea): improve file search popup and UI components #30
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
Conversation
- Restructure IdeaFileSearchPopup to use Box container with trigger button - Add IdeaContextManager for context state management - Simplify popup content with search field at top - Add recent files display and file/folder search - Add new icons: CheckBox, CheckBoxOutlineBlank, Remove - Update IdeaTopToolbar to use new popup API
- Add isDirectory field to SelectedFileItem - Use /dir: command for directories, /file: for files - Add Search icon to IdeaComposeIcons - Improve search field UI with icon and better padding - Update placeholder text to 'Search files and folders...'
- Use PsiCompiledFile interface instead of ClsFileImpl implementation class - This fixes NoClassDefFoundError in some IDE configurations - Remove unnecessary null check for content
Extract IdeaDevInInputArea composable from IdeaAgentApp.kt into its own file for better modularity and code organization. No functional changes.
- Add unified border around IdeaDevInInputArea for cohesive look - IdeaTopToolbar now supports horizontal scroll in collapsed mode - Add expand/collapse button to toggle between horizontal and vertical file list - FileChipExpanded shows full path in expanded mode - Use animateContentSize for smooth expand/collapse animation - Remove duplicate ExpandLess/ExpandMore icon definitions
…ement 1. Restyle IdeaMcpConfigDialog to match IdeaModelConfigDialog pattern: - Use styled Box container with rounded corners and proper background - Add proper tab selector with visual feedback - Improve spacing and visual hierarchy - Add Escape key handling for dialog dismissal - Extract IdeaMcpConfigDialogContent for reusability 2. Implement prompt enhancement feature: - Create IdeaPromptEnhancer service for AI-powered prompt optimization - Load domain dictionary from project's .autodev/domain.csv - Load README for project context - Use LLM to enhance prompts with domain-specific vocabulary - Add loading state indicator on enhancement button - Integrate with IdeaBottomToolbar and IdeaDevInInputArea
1. Add replaceText() method to IdeaDevInInput for setting text content - Renamed from setText() to avoid conflict with EditorTextField.setText() - Uses WriteCommandAction for proper document modification 2. Fix prompt enhancement in IdeaDevInInputArea: - Use IntelliJ Logger instead of KLogger (not available in mpp-idea) - Move logger to file-level to avoid Composable context issues - Use Dispatchers.IO for LLM calls - Use ApplicationManager.invokeLater for EDT updates - Add proper logging for debugging 3. Add logging to IdeaPromptEnhancer: - Log enhancement progress and results - Log model configuration and LLM response details - Log errors with stack traces
…n IDE - Create IdeaMcpConfigDialogWrapper using IntelliJ's DialogWrapper - Dialog now appears centered in the IDE window like IdeaModelConfigDialog - Proper z-index handling when used with SwingPanel components - Update IdeaBottomToolbar to use IdeaMcpConfigDialogWrapper.show() - Mark old IdeaMcpConfigDialog as deprecated
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds new project services (context manager, tool-config, prompt enhancer), several UI features (file search popup, top/bottom toolbars, MCP DialogWrapper, DevIn Compose input area, icons), switches tool-config usage in ViewModel, improves compiled-PSI file reading, changes runner message-bus connection scope, and adds a CI disk-space step. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review completed. 1 suggestions posted.
Comment augment review to trigger a new review at any time.
| */ | ||
| suspend fun enhance(input: String): String = withContext(Dispatchers.IO) { | ||
| try { | ||
| logger.info("Starting enhancement for input: ${input.take(50)}...") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider removing or masking logging of user prompt content (input.take(50)) to avoid leaking potentially sensitive information; log only metadata like length or a hash. This follows the repo guideline to avoid sensitive logging. (Guideline: no_sensitive_logging)
🤖 Was this useful? React with 👍 or 👎
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt (1)
524-538: Binary extension list differs fromIdeaContextManager.canBeAdded.This function is missing
rarand7zextensions that are present inIdeaContextManager.canBeAdded(per the relevant code snippet at lines 205-219). This inconsistency could cause files to appear in search results but behave differently when added via the context manager.Consider extracting a shared constant for binary extensions to ensure consistency.
+// Consider extracting to a shared location +private val BINARY_EXTENSIONS = setOf( + "jar", "class", "exe", "dll", "so", "dylib", + "png", "jpg", "jpeg", "gif", "ico", "pdf", + "zip", "tar", "gz", "rar", "7z" +) + private fun canBeAdded(project: Project, file: VirtualFile): Boolean { // ... - val binaryExtensions = setOf("jar", "class", "exe", "dll", "so", "dylib", "png", "jpg", "jpeg", "gif", "ico", "pdf", "zip", "tar", "gz") + if (extension in BINARY_EXTENSIONS) return falseexts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/compiler/exec/file/FileInsCommand.kt (1)
63-79: UsegetNavigationElement()orgetMirror()instead ofdecompiledPsiFilefor compiled file handlingThe IntelliJ Platform SDK recommends using
PsiCompiledElement.getNavigationElement()orgetMirror()to resolve compiled files to their decompiled/source counterparts—notdecompiledPsiFile, which can be null and is not the intended public API.Additionally, falling back to
virtualFile.readText()on a.classfile will produce binary bytecode, which contradicts the intended use of FileInsCommand (returning readable text for AI models). The project already handles binary files elsewhere (seeFileViewerPanel.jvm.ktcatchingMalformedInputExceptionand emitting an error message); apply a similar pattern here by either skipping content for undecompilable files or returning an explicit message.
🧹 Nitpick comments (17)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt (1)
1596-1606: Consider simplifying path configuration for consistency.The
CheckBox,CheckBoxOutlineBlank, andRemoveicons specify many explicit parameters (fillAlpha,stroke,strokeAlpha,strokeLineWidth,strokeLineCap,strokeLineJoin,strokeLineMiter) that are all defaults. Most other icons in this file only specifyfill = SolidColor(Color.Black).For consistency and brevity, consider simplifying to match the existing style:
path( - fill = SolidColor(Color.Black), - fillAlpha = 1f, - stroke = null, - strokeAlpha = 1f, - strokeLineWidth = 1f, - strokeLineCap = StrokeCap.Butt, - strokeLineJoin = StrokeJoin.Miter, - strokeLineMiter = 1f, - pathFillType = PathFillType.NonZero + fill = SolidColor(Color.Black) ) {Also applies to: 1640-1650, 1681-1691
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt (3)
100-118: Duplicate loading of recent files during search.When a search is triggered,
searchAllItems(line 106) internally callsloadRecentFilesagain at line 513, duplicating the work already done at line 101. Consider passing the already-loaded recent files tosearchAllItemsor filtering them in the caller instead.Additionally, the synchronous search in
remembermay cause UI jank on large projects. Consider usingLaunchedEffectwithkotlinx.coroutinesandwithContext(Dispatchers.Default)for the heavyiterateContentoperation, with debouncing on the search query.- // Search results - only computed when query is long enough - val searchResults = remember(searchQuery, project) { - if (searchQuery.length >= 2) { - searchAllItems(project, searchQuery) - } else { - null - } - } + // Search results - computed asynchronously when query is long enough + var searchResults by remember { mutableStateOf<SearchResults?>(null) } + + LaunchedEffect(searchQuery, project) { + if (searchQuery.length >= 2) { + // Debounce and offload search + delay(150) + searchResults = withContext(Dispatchers.Default) { + searchAllItems(project, searchQuery, recentFiles) + } + } else { + searchResults = null + } + }
404-423: Consider cachingtruncatedPathat construction time.The
truncatedPathproperty is recomputed on each access during recomposition. For large file lists, pre-computing this value in thefrom()factory method would be more efficient.data class IdeaFilePresentation( val virtualFile: VirtualFile, val name: String, val path: String, val presentablePath: String, val isRecentFile: Boolean = false, - val isDirectory: Boolean = false + val isDirectory: Boolean = false, + val truncatedPath: String = "" ) { - val truncatedPath: String - get() { - val parentPath = presentablePath.substringBeforeLast("/", "") - if (parentPath.isEmpty()) return "" - if (parentPath.length <= 40) return parentPath - val parts = parentPath.split("/") - if (parts.size <= 2) return "...$parentPath" - val keepParts = parts.takeLast(4) - return "...${keepParts.joinToString("/")}" - }Then compute it in
from():val truncatedPath = computeTruncatedPath(relativePath)
488-504: Fuzzy search viaiterateContentmay be slow on large projects.The
iterateContentiteration visits every file in the project content. For large codebases, this could be slow, especially since it runs synchronously. Consider:
- Limiting early with
ORcondition (stop when either limit is reached)- Using a background coroutine with cancellation support
fileIndex.iterateContent { file -> val nameLower = file.name.lowercase() if (nameLower.contains(lowerQuery)) { if (file.isDirectory) { if (folders.size < 20 && file.path !in existingFolderPaths) { folders.add(IdeaFilePresentation.from(project, file)) } } else if (canBeAdded(project, file) && files.size < 50 && file.path !in existingFilePaths) { files.add(IdeaFilePresentation.from(project, file)) } } - files.size < 50 && folders.size < 20 + files.size < 50 || folders.size < 20 // Stop when both limits are reached }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt (1)
13-13: Remove unused import.The
kotlinx.coroutines.launchimport is not used in this file.-import kotlinx.coroutines.launchmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (4)
120-132: Good deprecation pattern.The deprecation notice with migration guidance to
IdeaMcpConfigDialogWrapper.show()is helpful. Consider adding@Deprecatedannotation in addition to the KDoc to enable compiler warnings.+ @Deprecated("Use IdeaMcpConfigDialogWrapper.show() instead for proper z-index handling", + ReplaceWith("IdeaMcpConfigDialogWrapper.show(project)")) @Composable fun IdeaMcpConfigDialog( onDismiss: () -> Unit
428-447: Avoid hardcoded colors.Line 433 uses
androidx.compose.ui.graphics.Color.Transparentdirectly. Based on coding guidelines, hardcoded colors should be avoided in favor of design tokens for consistency across platforms.Consider using a theme-based approach:
.background( - if (selected) JewelTheme.globalColors.borders.normal.copy(alpha = 0.3f) - else androidx.compose.ui.graphics.Color.Transparent + if (selected) JewelTheme.globalColors.borders.normal.copy(alpha = 0.3f) + else JewelTheme.globalColors.panelBackground.copy(alpha = 0f) )Or simply omit the background when not selected since the parent already has a background.
156-184: Consider debug-level logging for auto-save failures.The silent failure in the catch block (lines 180-182) could mask configuration issues during development. Consider logging at debug level for troubleshooting while maintaining the silent UX.
Also,
kotlinx.coroutines.delayon line 161 is fully qualified—add an import for consistency.} catch (e: Exception) { - // Silent fail for auto-save + // Silent fail for auto-save - log for debugging + com.intellij.openapi.diagnostic.Logger.getInstance("IdeaMcpConfigDialog") + .debug("Auto-save failed", e) }
364-377: Consider extracting the duplicated callback logic.This callback implementation (lines 364-377) is nearly identical to the one defined on lines 195-210. Extract a helper function to reduce duplication.
// Define once at the top of IdeaMcpConfigDialogContent fun createMcpLoadingCallback( onLoadingState: (McpLoadingState) -> Unit, onToolsLoaded: (String, List<ToolItem>) -> Unit ) = object : McpLoadingStateCallback { override fun onServerStateChanged(serverName: String, state: McpServerState) { onLoadingState(mcpLoadingState.updateServerState(serverName, state)) if (state.isLoaded) onToolsLoaded(serverName, state.tools) } override fun onLoadingStateChanged(loadingState: McpLoadingState) { onLoadingState(loadingState) } override fun onBuiltinToolsLoaded(tools: List<ToolItem>) { onLoadingState(mcpLoadingState.copy(builtinToolsLoaded = true)) } }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.kt (2)
81-92: Domain dictionary path logic doesn't match documentation.The AI summary indicates the domain dictionary should be loaded from
.autodev/domain.csvorprompts/domain.csv, but the code looks for.autodevorpromptsdirectories first, thendomain.csvwithin them. This means it will look at.autodev/domain.csvorprompts/domain.csv, which matches. However, if.autodevexists but doesn't containdomain.csv, the fallback toprompts/domain.csvwon't happen.Consider improving fallback logic:
private fun loadDomainDict(): String { return try { runReadAction { val baseDir = project.guessProjectDir() ?: return@runReadAction "" - val promptsDir = baseDir.findChild(".autodev") ?: baseDir.findChild("prompts") - val dictFile = promptsDir?.findChild("domain.csv") + val dictFile = baseDir.findChild(".autodev")?.findChild("domain.csv") + ?: baseDir.findChild("prompts")?.findChild("domain.csv") dictFile?.contentsToByteArray()?.toString(Charsets.UTF_8) ?: "" } } catch (e: Exception) { "" } }
154-165: Regex may miss code blocks without trailing newline.The regex pattern
\n([\\s\\S]*?)\\n\``` requires a newline before the closing backticks. If the LLM response ends the code block immediately after content without a newline, extraction will fail.Consider making the trailing newline optional:
-val codeBlockRegex = Regex("```(?:\\w+)?\\s*\\n([\\s\\S]*?)\\n```") +val codeBlockRegex = Regex("```(?:\\w+)?\\s*\\n([\\s\\S]*?)\\n?```")mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt (2)
121-135: Duplicate send logic should be extracted to a shared function.The message sending logic (building fullText with file commands, calling onSend, clearing input/state) is duplicated between the
onSubmitlistener (lines 121-135) andonSendClick(lines 167-182). This violates DRY and makes it easy for the two paths to diverge.Extract the common logic:
// Add this helper inside the composable fun handleSend(text: String) { if (text.isNotBlank() && !isProcessing) { val filesText = selectedFiles.joinToString("\n") { it.toDevInsCommand() } val fullText = if (filesText.isNotEmpty()) "$text\n$filesText" else text onSend(fullText) devInInput?.clearInput() inputText = "" selectedFiles = emptyList() } } // Then use in both places: // onSubmit: handleSend(text) // onSendClick: handleSend(devInInput?.text?.trim() ?: inputText.trim())Also applies to: 167-182
200-208: Redundant EDT dispatching -invokeLaterinsideDispatchers.Mainis unnecessary.
Dispatchers.Mainin an IntelliJ plugin context already dispatches to the EDT. Wrapping withApplicationManager.getApplication().invokeLaterinsidewithContext(Dispatchers.Main)is redundant and could introduce unnecessary delays.Simplify to use only one dispatch mechanism:
if (enhanced != currentText && enhanced.isNotBlank()) { - // Update UI on EDT - withContext(Dispatchers.Main) { - ApplicationManager.getApplication().invokeLater { - devInInput?.replaceText(enhanced) - inputText = enhanced - inputAreaLogger.info("Text updated in input field") - } + // Update UI on EDT + ApplicationManager.getApplication().invokeLater { + devInInput?.replaceText(enhanced) + inputText = enhanced + inputAreaLogger.info("Text updated in input field") } }Or use only
withContext(Dispatchers.Main)withoutinvokeLater.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.kt (2)
57-64: Simplify nullable flow collection with helper function.The pattern of
contextManager?.flow?.collectAsState() ?: remember { mutableStateOf(default) }is repeated multiple times and is verbose. Also,relatedFilesis collected but never used in the UI.Consider a helper or removing unused state:
-val hasDefaultContext by contextManager?.defaultContextFiles?.collectAsState() - ?: remember { mutableStateOf(emptyList<VirtualFile>()) } -val rules by contextManager?.rules?.collectAsState() - ?: remember { mutableStateOf(emptyList<ContextRule>()) } -val relatedFiles by contextManager?.relatedFiles?.collectAsState() - ?: remember { mutableStateOf(emptyList<VirtualFile>()) } +val hasDefaultContext by contextManager?.defaultContextFiles?.collectAsState() + ?: remember { mutableStateOf(emptyList<VirtualFile>()) } +val rules by contextManager?.rules?.collectAsState() + ?: remember { mutableStateOf(emptyList<ContextRule>()) } +// Remove relatedFiles if not used, or add UI to display it
128-146: Simplify conditional logic for file display.Lines 142-146 have redundant branches - both the
else if (!isExpanded)andelsebranches produce the sameSpacer. This can be simplified.// Selected files - horizontal scrollable in collapsed mode -if (!isExpanded && selectedFiles.isNotEmpty()) { +if (!isExpanded && selectedFiles.isNotEmpty()) { val scrollState = rememberScrollState() Row( modifier = Modifier .weight(1f) .horizontalScroll(scrollState), horizontalArrangement = Arrangement.spacedBy(4.dp), verticalAlignment = Alignment.CenterVertically ) { selectedFiles.forEach { file -> FileChip(file = file, onRemove = { onRemoveFile(file) }) } } -} else if (!isExpanded) { - Spacer(Modifier.weight(1f)) } else { Spacer(Modifier.weight(1f)) }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.kt (2)
3-4: Remove unused imports.
LookupManagerListener(line 3) andPsiManager(line 14) are imported but not used. The class documentation mentions "Related classes suggestion via LookupManagerListener" but this functionality isn't implemented.Remove the unused imports or implement the mentioned functionality:
package cc.unitmesh.devins.idea.editor -import com.intellij.codeInsight.lookup.LookupManagerListener import com.intellij.openapi.Disposable import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.components.Service import com.intellij.openapi.components.service import com.intellij.openapi.fileEditor.FileEditorManager import com.intellij.openapi.fileEditor.FileEditorManagerEvent import com.intellij.openapi.fileEditor.FileEditorManagerListener import com.intellij.openapi.project.Project import com.intellij.openapi.util.Disposer import com.intellij.openapi.vfs.VirtualFile -import com.intellij.psi.PsiManagerIf
LookupManagerListenerintegration is planned, consider tracking this as a TODO.Do you want me to open an issue to track the LookupManagerListener integration mentioned in the class documentation?
Also applies to: 14-14
206-220: Consider extracting file validation to shared utility.This
canBeAddedlogic duplicates similar validation inIdeaFileSearchPopup.canBeAdded()(seen in relevant code snippets at lines 523-537). The binary extension lists also differ slightly between the two implementations.Consider extracting to a shared utility in the
idea.editorpackage to ensure consistent behavior:// In a shared location like IdeaFileUtils.kt object IdeaFileUtils { private val BINARY_EXTENSIONS = setOf( "jar", "class", "exe", "dll", "so", "dylib", "png", "jpg", "jpeg", "gif", "ico", "pdf", "zip", "tar", "gz", "rar", "7z" ) fun isBinaryFile(file: VirtualFile): Boolean { val ext = file.extension?.lowercase() ?: "" return ext in BINARY_EXTENSIONS } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
exts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/compiler/exec/file/FileInsCommand.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt(4 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInInput.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt(7 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt(9 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.kt(5 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt(0 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt(1 hunks)
💤 Files with no reviewable changes (1)
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInInput.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktexts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/compiler/exec/file/FileInsCommand.kt
**/idea/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/idea/**/*.kt: For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer:JewelFlags.useCustomPopupRenderer = trueinIdeaAgentToolWindowFactory
For popup/dropdown menus in IntelliJ plugins, use Jewel'sPopupMenuinstead ofandroidx.compose.ui.window.Popup
For dialogs in IntelliJ plugins, use IntelliJ'sDialogWrapperwithorg.jetbrains.jewel.bridge.composeinstead ofandroidx.compose.ui.window.Dialog
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInInput.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
🧠 Learnings (6)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/compose/**/*.{kt,kts} : In Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For dialogs in IntelliJ plugins, use IntelliJ's `DialogWrapper` with `org.jetbrains.jewel.bridge.compose` instead of `androidx.compose.ui.window.Dialog`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For popup/dropdown menus in IntelliJ plugins, use Jewel's `PopupMenu` instead of `androidx.compose.ui.window.Popup`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to {**/compose/**/*.kt,mpp-ui/src/jsMain/typescript/**/*.{ts,tsx}} : DO NOT hardcode colors (e.g., `Color(0xFF...)` or `#hex`). Always use design tokens for consistency across platforms
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Note that `IdeaAgentViewModelTest` requires IntelliJ Platform Test Framework; `JewelRendererTest` can run standalone with JUnit 5
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt
🧬 Code graph analysis (4)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.kt (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt (1)
IdeaFileSearchPopup(44-89)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.kt (1)
canBeAdded(206-220)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.kt (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt (1)
canBeAdded(524-538)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt (3)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.kt (1)
IdeaTopToolbar(43-178)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInInput.kt (3)
recreateDocument(189-210)addInputListener(223-225)clearInput(259-261)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt (1)
IdeaBottomToolbar(28-164)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (17)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaComposeIcons.kt (1)
1585-1739: LGTM! Icon additions align well with the PR objectives.The four new icons (CheckBox, CheckBoxOutlineBlank, Remove, Search) are correctly implemented following the established pattern in the file. They use proper lazy initialization, standard 24x24 viewports, and Material Design path data. The Search icon supports the file search popup enhancement, and the checkbox icons likely enable the context selection features mentioned in the PR objectives.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaFileSearchPopup.kt (4)
44-89: LGTM!The component correctly uses Jewel's
PopupMenufor native IntelliJ look and feel, aligning with the coding guidelines for popup/dropdown menus in IntelliJ plugins. The hover state handling and trigger button implementation are clean.
293-338: LGTM!The file menu item layout with bold filename, truncated path, and appropriate icon selection based on
isRecentFileflag is well-structured.
340-382: Consistent folder item styling.The
FolderMenuItemfollows the same pattern asFileMenuItem, maintaining UI consistency.
446-464: LGTM!Proper use of
runReadActionfor thread-safe access to editor history, with appropriate error handling and logging.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt (2)
83-94: Good adoption of DialogWrapper pattern.The use of
IdeaMcpConfigDialogWrapper.show(project)correctly follows the coding guideline for dialogs in IntelliJ plugins, ensuring proper z-index handling with SwingPanel components. Based on learnings, this is the recommended approach.
97-113: LGTM!The Tooltip and button state management provide good UX feedback. The button is correctly disabled during enhancement or execution, and the tint visually indicates the active state.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (3)
72-107: Excellent DialogWrapper implementation.This correctly follows the coding guideline: "For dialogs in IntelliJ plugins, use IntelliJ's
DialogWrapperwithorg.jetbrains.jewel.bridge.compose". The removal of the south panel (OK/Cancel) is appropriate since the dialog auto-saves changes. Based on coding guidelines, this is the recommended approach.
505-523: LGTM!The bidirectional sync between
textFieldStateandmcpConfigJsonis correctly implemented with guards on lines 512 and 520 to prevent infinite loops.
245-250: Escape handling may be redundant with DialogWrapper.The
onKeyEventescape handler works for the deprecatedIdeaMcpConfigDialog, but when used viaIdeaMcpConfigDialogWrapper, theDialogWrapperalready handles Escape key dismissal. This redundancy is harmless sinceDialogWrapperintercepts first.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInInput.kt (1)
245-254: LGTM!The
replaceTextmethod follows the same pattern as the existingappendTextmethod, properly usingWriteCommandActionfor thread-safe document modification with appropriate null-safety.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.kt (1)
33-75: Good error handling with graceful fallback.The
enhancefunction properly handles failures by returning the original input, ensuring the feature doesn't break the user workflow. The use ofDispatchers.IOis appropriate for the file I/O and network operations.Note: Line 35 logs the first 50 characters of user input. Ensure this doesn't inadvertently log sensitive information in production environments.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt (1)
148-149: Good disposal pattern.Proper use of
Disposer.register(parentDisposable, input)ensures the editor is disposed when the parent is disposed, preventing resource leaks.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.kt (2)
351-365: LGTM!The
SelectedFileItemdata class extension withisDirectoryandtoDevInsCommand()is clean and provides a good abstraction for generating DevIn commands based on file type.
296-349: LGTM!The
FileChipExpandedcomposable properly handles the expanded view with full path display, consistent hover styling, and text overflow handling.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaContextManager.kt (2)
29-89: Good reactive state management with StateFlow.The use of
MutableStateFlowwithasStateFlow()for public exposure is idiomatic Kotlin. The message bus connection correctly usesthis(theDisposableservice) for automatic cleanup.
231-250: Rule infrastructure in place, awaiting implementation.
ContextRuleandContextRuleTypeprovide the data model for context filtering rules, but the rules are not currently applied when determining which files to include. ThecanBeAddedmethod and file selection don't consult the rules.Is the rule application logic planned for a follow-up PR? Consider adding a TODO comment or tracking this as an issue.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
Show resolved
Hide resolved
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
Show resolved
Hide resolved
There was a problem hiding this 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 enhances the mpp-idea module's file search popup and UI components, introducing several significant improvements to the user experience and fixing a critical ClassNotFoundException issue.
Key Changes
- Fixed
ClsFileImplClassNotFoundException by replacing it with thePsiCompiledFileinterface for better compatibility with the IntelliJ Platform API - Redesigned the input area layout with a unified rounded border and removed internal component borders for a cohesive appearance
- Enhanced file list display with collapsible/expandable modes featuring horizontal scrolling in collapsed mode and vertical list view in expanded mode
- Implemented AI-powered prompt enhancement feature using domain dictionaries and project README files
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
IdeaDevInInputArea.kt |
New file extracting input area component with unified border design and prompt enhancement integration |
IdeaComposeIcons.kt |
Added new icon definitions (CheckBox, CheckBoxOutlineBlank, Remove, Search) for UI enhancements |
IdeaAgentApp.kt |
Refactored by extracting IdeaDevInInputArea into separate file, removing duplicate code |
IdeaTopToolbar.kt |
Enhanced with collapsible file list, context manager integration, and improved file chip display |
IdeaPromptEnhancer.kt |
New service implementing AI-powered prompt enhancement using LLM and domain context |
IdeaMcpConfigDialog.kt |
Redesigned using DialogWrapper for proper z-index handling and improved styling consistency |
IdeaFileSearchPopup.kt |
Improved with better layout, search-first UI, and optimized file presentation |
IdeaDevInInput.kt |
Added replaceText() method for programmatic text replacement |
IdeaContextManager.kt |
New service for managing context files, rules, and related files with auto-add functionality |
IdeaBottomToolbar.kt |
Updated to use DialogWrapper for MCP config and added prompt enhancement button with loading state |
FileInsCommand.kt |
Fixed ClsFileImpl ClassNotFoundException by using PsiCompiledFile interface |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val hasDefaultContext by contextManager?.defaultContextFiles?.collectAsState() | ||
| ?: remember { mutableStateOf(emptyList<VirtualFile>()) } | ||
| val rules by contextManager?.rules?.collectAsState() | ||
| ?: remember { mutableStateOf(emptyList<ContextRule>()) } | ||
| val relatedFiles by contextManager?.relatedFiles?.collectAsState() | ||
| ?: remember { mutableStateOf(emptyList<VirtualFile>()) } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The collectAsState() call on line 59 can throw IllegalStateException if called outside of a composable context. The Elvis operator provides a fallback, but consider adding a null check on contextManager before calling collectAsState() to avoid potential issues when the project is null.
| val hasDefaultContext by contextManager?.defaultContextFiles?.collectAsState() | |
| ?: remember { mutableStateOf(emptyList<VirtualFile>()) } | |
| val rules by contextManager?.rules?.collectAsState() | |
| ?: remember { mutableStateOf(emptyList<ContextRule>()) } | |
| val relatedFiles by contextManager?.relatedFiles?.collectAsState() | |
| ?: remember { mutableStateOf(emptyList<VirtualFile>()) } | |
| val hasDefaultContext by if (contextManager != null) { | |
| contextManager.defaultContextFiles.collectAsState() | |
| } else { | |
| remember { mutableStateOf(emptyList<VirtualFile>()) } | |
| } | |
| val rules by if (contextManager != null) { | |
| contextManager.rules.collectAsState() | |
| } else { | |
| remember { mutableStateOf(emptyList<ContextRule>()) } | |
| } | |
| val relatedFiles by if (contextManager != null) { | |
| contextManager.relatedFiles.collectAsState() | |
| } else { | |
| remember { mutableStateOf(emptyList<VirtualFile>()) } | |
| } |
| // Load recent files immediately (not in LaunchedEffect) | ||
| val recentFiles = remember(project) { loadRecentFiles(project) } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remember(project) block for recentFiles on line 101 can potentially block the UI thread as it calls loadRecentFiles(project) synchronously during composition. Consider using LaunchedEffect or moving this to a background thread to avoid EDT violations, especially since loadRecentFiles accesses the EditorHistoryManager.
| // Load recent files immediately (not in LaunchedEffect) | |
| val recentFiles = remember(project) { loadRecentFiles(project) } | |
| // Load recent files asynchronously to avoid blocking UI thread | |
| var recentFiles by remember { mutableStateOf(emptyList<VirtualFile>()) } | |
| LaunchedEffect(project) { | |
| recentFiles = loadRecentFiles(project) | |
| } |
| val searchResults = remember(searchQuery, project) { | ||
| if (searchQuery.length >= 2) { | ||
| searchAllItems(project, searchQuery) | ||
| } else { | ||
| null | ||
| } | ||
| files = results.files | ||
| folders = results.folders | ||
| recentFiles = results.recentFiles | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remember(searchQuery, project) block for searchResults on line 104 performs synchronous file system operations during composition, which can block the UI thread. The searchAllItems() function iterates through the project file index and can be slow for large projects. Consider using LaunchedEffect with a debounce to perform the search asynchronously.
| * Styled to match IdeaModelConfigDialog for consistency. | ||
| * | ||
| * @deprecated Use IdeaMcpConfigDialogWrapper.show() instead for proper z-index handling with SwingPanel. | ||
| */ |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The IdeaMcpConfigDialog composable is marked as @deprecated in the documentation (line 120) but should use the actual @Deprecated annotation with a proper message and ReplaceWith suggestion to help developers migrate to IdeaMcpConfigDialogWrapper.show(). This provides better IDE support and clearer deprecation guidance.
| */ | |
| */ | |
| @Deprecated( | |
| message = "Use IdeaMcpConfigDialogWrapper.show() instead for proper z-index handling with SwingPanel.", | |
| replaceWith = ReplaceWith("IdeaMcpConfigDialogWrapper.show(project)"), | |
| level = DeprecationLevel.WARNING | |
| ) |
| if (text.isNotBlank() && !isProcessing) { | ||
| // Append file references to the message (use /dir: for directories, /file: for files) | ||
| val filesText = selectedFiles.joinToString("\n") { it.toDevInsCommand() } | ||
| val fullText = if (filesText.isNotEmpty()) { | ||
| "$text\n$filesText" | ||
| } else { | ||
| text | ||
| } | ||
| onSend(fullText) | ||
| clearInput() | ||
| inputText = "" | ||
| // Clear selected files after sending | ||
| selectedFiles = emptyList() | ||
| } | ||
| } | ||
|
|
||
| override fun onStop() { | ||
| onAbort() | ||
| } | ||
|
|
||
| override fun onTextChanged(text: String) { | ||
| inputText = text | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| // Register for disposal | ||
| Disposer.register(parentDisposable, input) | ||
| devInInput = input | ||
|
|
||
| // Wrap in a JPanel to handle dynamic sizing | ||
| JPanel(BorderLayout()).apply { | ||
| add(input, BorderLayout.CENTER) | ||
| // Don't set fixed preferredSize - let it fill available space | ||
| minimumSize = Dimension(200, 60) | ||
| } | ||
| }, | ||
| update = { panel -> | ||
| // Update panel if needed | ||
| } | ||
| ) | ||
|
|
||
| // Bottom toolbar with Compose (no individual border) | ||
| IdeaBottomToolbar( | ||
| project = project, | ||
| onSendClick = { | ||
| val text = devInInput?.text?.trim() ?: inputText.trim() | ||
| if (text.isNotBlank() && !isProcessing) { | ||
| // Append file references to the message (use /dir: for directories, /file: for files) | ||
| val filesText = selectedFiles.joinToString("\n") { it.toDevInsCommand() } | ||
| val fullText = if (filesText.isNotEmpty()) { | ||
| "$text\n$filesText" | ||
| } else { | ||
| text | ||
| } | ||
| onSend(fullText) | ||
| devInInput?.clearInput() | ||
| inputText = "" | ||
| // Clear selected files after sending | ||
| selectedFiles = emptyList() | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The duplicate code on lines 122-135 and lines 167-182 should be extracted into a helper function. Both blocks perform the same logic: checking if text is not blank, appending file references using toDevInsCommand(), sending the message, clearing input, and clearing selected files. This violates the DRY principle and makes maintenance harder.
| * Represents a context rule for filtering files | ||
| */ | ||
| data class ContextRule( | ||
| val id: String = java.util.UUID.randomUUID().toString(), |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The id field in ContextRule uses java.util.UUID.randomUUID().toString() as a default value, which will generate a new UUID every time the data class is copied. This can lead to unexpected behavior when comparing or tracking rules. Consider generating the ID only once in the constructor or making it a required parameter.
| val id: String = java.util.UUID.randomUUID().toString(), | |
| val id: String, |
| /** | ||
| * CheckBox icon (checked checkbox) | ||
| */ | ||
| val CheckBox: ImageVector by lazy { | ||
| ImageVector.Builder( | ||
| name = "CheckBox", | ||
| defaultWidth = 24.dp, | ||
| defaultHeight = 24.dp, | ||
| viewportWidth = 24f, | ||
| viewportHeight = 24f | ||
| ).apply { | ||
| path( | ||
| fill = SolidColor(Color.Black), | ||
| fillAlpha = 1f, | ||
| stroke = null, | ||
| strokeAlpha = 1f, | ||
| strokeLineWidth = 1f, | ||
| strokeLineCap = StrokeCap.Butt, | ||
| strokeLineJoin = StrokeJoin.Miter, | ||
| strokeLineMiter = 1f, | ||
| pathFillType = PathFillType.NonZero | ||
| ) { | ||
| moveTo(19f, 3f) | ||
| horizontalLineTo(5f) | ||
| curveToRelative(-1.1f, 0f, -2f, 0.9f, -2f, 2f) | ||
| verticalLineToRelative(14f) | ||
| curveToRelative(0f, 1.1f, 0.9f, 2f, 2f, 2f) | ||
| horizontalLineToRelative(14f) | ||
| curveToRelative(1.1f, 0f, 2f, -0.9f, 2f, -2f) | ||
| verticalLineTo(5f) | ||
| curveToRelative(0f, -1.1f, -0.9f, -2f, -2f, -2f) | ||
| close() | ||
| moveTo(10f, 17f) | ||
| lineToRelative(-5f, -5f) | ||
| lineToRelative(1.41f, -1.41f) | ||
| lineTo(10f, 14.17f) | ||
| lineToRelative(7.59f, -7.59f) | ||
| lineTo(19f, 8f) | ||
| lineToRelative(-9f, 9f) | ||
| close() | ||
| } | ||
| }.build() | ||
| } | ||
|
|
||
| /** | ||
| * CheckBoxOutlineBlank icon (unchecked checkbox) | ||
| */ | ||
| val CheckBoxOutlineBlank: ImageVector by lazy { | ||
| ImageVector.Builder( | ||
| name = "CheckBoxOutlineBlank", | ||
| defaultWidth = 24.dp, | ||
| defaultHeight = 24.dp, | ||
| viewportWidth = 24f, | ||
| viewportHeight = 24f | ||
| ).apply { | ||
| path( | ||
| fill = SolidColor(Color.Black), | ||
| fillAlpha = 1f, | ||
| stroke = null, | ||
| strokeAlpha = 1f, | ||
| strokeLineWidth = 1f, | ||
| strokeLineCap = StrokeCap.Butt, | ||
| strokeLineJoin = StrokeJoin.Miter, | ||
| strokeLineMiter = 1f, | ||
| pathFillType = PathFillType.NonZero | ||
| ) { | ||
| moveTo(19f, 5f) | ||
| verticalLineToRelative(14f) | ||
| horizontalLineTo(5f) | ||
| verticalLineTo(5f) | ||
| horizontalLineToRelative(14f) | ||
| moveTo(19f, 3f) | ||
| horizontalLineTo(5f) | ||
| curveToRelative(-1.1f, 0f, -2f, 0.9f, -2f, 2f) | ||
| verticalLineToRelative(14f) | ||
| curveToRelative(0f, 1.1f, 0.9f, 2f, 2f, 2f) | ||
| horizontalLineToRelative(14f) | ||
| curveToRelative(1.1f, 0f, 2f, -0.9f, 2f, -2f) | ||
| verticalLineTo(5f) | ||
| curveToRelative(0f, -1.1f, -0.9f, -2f, -2f, -2f) | ||
| close() | ||
| } | ||
| }.build() | ||
| } | ||
|
|
||
| /** | ||
| * Remove icon (minus sign) | ||
| */ | ||
| val Remove: ImageVector by lazy { | ||
| ImageVector.Builder( | ||
| name = "Remove", | ||
| defaultWidth = 24.dp, | ||
| defaultHeight = 24.dp, | ||
| viewportWidth = 24f, | ||
| viewportHeight = 24f | ||
| ).apply { | ||
| path( | ||
| fill = SolidColor(Color.Black), | ||
| fillAlpha = 1f, | ||
| stroke = null, | ||
| strokeAlpha = 1f, | ||
| strokeLineWidth = 1f, | ||
| strokeLineCap = StrokeCap.Butt, | ||
| strokeLineJoin = StrokeJoin.Miter, | ||
| strokeLineMiter = 1f, | ||
| pathFillType = PathFillType.NonZero | ||
| ) { | ||
| moveTo(19f, 13f) | ||
| horizontalLineTo(5f) | ||
| verticalLineToRelative(-2f) | ||
| horizontalLineToRelative(14f) | ||
| verticalLineToRelative(2f) | ||
| close() | ||
| } | ||
| }.build() | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The CheckBox, CheckBoxOutlineBlank, and Remove icons are added to IdeaComposeIcons but are not used anywhere in this PR. Consider removing unused icon definitions to keep the codebase clean, or document where these icons will be used in future changes.
| } catch (e: Exception) { | ||
| "" | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Load README file from project root. | ||
| */ | ||
| private fun loadReadme(): String { | ||
| return try { | ||
| runReadAction { | ||
| val baseDir = project.guessProjectDir() ?: return@runReadAction "" | ||
| val readmeFile = baseDir.findChild("README.md") | ||
| ?: baseDir.findChild("README") | ||
| ?: baseDir.findChild("readme.md") | ||
|
|
||
| val content = readmeFile?.contentsToByteArray()?.toString(Charsets.UTF_8) ?: "" | ||
| // Limit README content to avoid token overflow | ||
| if (content.length > 2000) content.take(2000) + "\n..." else content | ||
| } | ||
| } catch (e: Exception) { | ||
| "" | ||
| } |
Copilot
AI
Dec 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The loadReadme() and loadDomainDict() functions catch all exceptions and silently return empty strings. Consider logging these exceptions for debugging purposes, similar to how other parts of the code use the logger for error tracking.
Add step to maximize available disk space in GitHub Actions by using jlumbroso/free-disk-space before fetching sources.
1. IdeaPromptEnhancer: - Log only metadata (length) instead of user prompt content to avoid sensitive info leakage - Fix domain dictionary fallback logic to properly check both paths - Add debug logging for file loading failures - Fix regex to handle code blocks without trailing newline 2. IdeaMcpConfigDialog: - Add proper @deprecated annotation with ReplaceWith suggestion 3. IdeaBottomToolbar: - Remove unused kotlinx.coroutines.launch import 4. IdeaDevInInputArea: - Extract duplicate send logic into buildAndSendMessage helper function - Use isProcessingRef to fix stale closure issue in SwingPanel listener - Remove redundant withContext(Dispatchers.Main) + invokeLater combination
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (1)
242-254: Height constraint mismatch persists.The Box uses
heightIn(max = 700.dp)butIdeaMcpConfigDialogWrapper(line 93) setspreferredSize = Dimension(600, 600). This 100dp mismatch can cause content clipping. Align both to 600 or make content responsive.Apply this diff to align the heights:
Box( modifier = Modifier .width(600.dp) - .heightIn(max = 700.dp) + .heightIn(max = 600.dp) .clip(RoundedCornerShape(12.dp))
🧹 Nitpick comments (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (1)
160-187: Auto-save silently ignores errors.The auto-save function silently catches exceptions (line 184) without user feedback. While this provides a non-blocking UX, configuration save failures go unnoticed. Consider logging errors or showing a subtle indicator.
Example:
} catch (e: Exception) { - // Silent fail for auto-save + mcpLoadError = "Auto-save failed: ${e.message}" }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt(4 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt(9 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptEnhancer.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
**/idea/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/idea/**/*.kt: For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer:JewelFlags.useCustomPopupRenderer = trueinIdeaAgentToolWindowFactory
For popup/dropdown menus in IntelliJ plugins, use Jewel'sPopupMenuinstead ofandroidx.compose.ui.window.Popup
For dialogs in IntelliJ plugins, use IntelliJ'sDialogWrapperwithorg.jetbrains.jewel.bridge.composeinstead ofandroidx.compose.ui.window.Dialog
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
🧠 Learnings (5)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For dialogs in IntelliJ plugins, use IntelliJ's `DialogWrapper` with `org.jetbrains.jewel.bridge.compose` instead of `androidx.compose.ui.window.Dialog`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For popup/dropdown menus in IntelliJ plugins, use Jewel's `PopupMenu` instead of `androidx.compose.ui.window.Popup`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/compose/**/*.{kt,kts} : In Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to {**/compose/**/*.kt,mpp-ui/src/jsMain/typescript/**/*.{ts,tsx}} : DO NOT hardcode colors (e.g., `Color(0xFF...)` or `#hex`). Always use design tokens for consistency across platforms
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
🧬 Code graph analysis (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt (3)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInInput.kt (3)
clearInput(259-261)recreateDocument(189-210)addInputListener(223-225)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaTopToolbar.kt (1)
IdeaTopToolbar(43-178)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt (1)
IdeaBottomToolbar(27-163)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (223)
- GitHub Check: Build (241)
🔇 Additional comments (12)
.github/workflows/build.yml (1)
120-126: Good consistency improvement across workflow jobs.Adding the disk space optimization step to the test job maintains parity with build and verify jobs and helps prevent disk-full failures during testing. The placement and configuration are correct.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (5)
3-47: Excellent migration to DialogWrapper!The imports correctly support the migration from Compose Dialog to IntelliJ's DialogWrapper. This addresses z-index issues with SwingPanel components as per the coding guidelines.
72-107: Proper DialogWrapper implementation.The wrapper correctly uses IntelliJ's DialogWrapper with Jewel's compose bridge, ensuring proper z-index handling and IDE-native dialog behavior. The companion
show()method provides a clean API for callers.
120-124: Proper deprecation annotation added.The Kotlin
@Deprecatedannotation withReplaceWithprovides clear migration guidance and IDE support, addressing the previous review feedback.
247-247: Proper design token usage throughout.All colors correctly use
JewelTheme.globalColorsdesign tokens, adhering to the coding guidelines against hardcoded colors.Also applies to: 270-279, 315-321, 408-414
425-450: Clean tab button implementation.The custom tab button properly uses JewelTheme design tokens for colors and provides clear visual feedback for the selected state.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt (6)
46-64: Good refactoring: DRY principle applied.The
buildAndSendMessagehelper effectively consolidates the duplicate message building logic that was previously flagged. This makes the code more maintainable.
86-88: Good fix: Stale closure issue addressed.The
isProcessingRefpattern withLaunchedEffectcorrectly ensures the SwingPanel listener sees the current processing state, addressing the previously flagged closure issue.
170-172: LGTM: Proper disposal handling.The
IdeaDevInInputis correctly registered with theDisposerusing the providedparentDisposable, ensuring cleanup when the parent is disposed.
213-237: LGTM: Proper threading for async enhancement.The prompt enhancement correctly uses
Dispatchers.IOfor the background work andApplicationManager.invokeLaterto update UI state on the EDT. Thefinallyblock ensuresisEnhancingis reset even on failure.
187-203: LGTM: Compose callbacks correctly see current state.Unlike the SwingPanel factory, the
onSendClicklambda is recomposed when state changes, so usingisProcessingandselectedFilesdirectly here is correct.
94-104: LGTM: Unified border styling.The unified border approach with
JewelTheme.globalColors.borders.normalprovides a cohesive look as intended by the PR objectives.
| override fun onSubmit(text: String, trigger: IdeaInputTrigger) { | ||
| // Use ref to get current processing state | ||
| if (text.isNotBlank() && !isProcessingRef.value) { | ||
| buildAndSendMessage( | ||
| text = text, | ||
| selectedFiles = selectedFiles, | ||
| onSend = onSend, | ||
| clearInput = { | ||
| clearInput() | ||
| inputText = "" | ||
| }, | ||
| clearFiles = { selectedFiles = emptyList() } | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale closure: selectedFiles captured at factory creation time.
Similar to the previously fixed isProcessing issue, selectedFiles is captured when the SwingPanel factory lambda is first invoked. Subsequent changes to selectedFiles state won't be reflected in the onSubmit callback, causing file references to be incorrect or missing.
Apply the same ref pattern used for isProcessingRef:
val isProcessingRef = remember { mutableStateOf(isProcessing) }
LaunchedEffect(isProcessing) { isProcessingRef.value = isProcessing }
+ val selectedFilesRef = remember { mutableStateOf(selectedFiles) }
+ LaunchedEffect(selectedFiles) { selectedFilesRef.value = selectedFiles }
// ... in onSubmit:
override fun onSubmit(text: String, trigger: IdeaInputTrigger) {
if (text.isNotBlank() && !isProcessingRef.value) {
buildAndSendMessage(
text = text,
- selectedFiles = selectedFiles,
+ selectedFiles = selectedFilesRef.value,
onSend = onSend,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaDevInInputArea.kt
around lines 144 to 157, the onSubmit lambda captures the mutable state
selectedFiles at factory creation time causing stale file lists — create a
mutable ref (e.g., selectedFilesRef) alongside selectedFiles (same pattern as
isProcessingRef), update selectedFilesRef.value whenever selectedFiles changes,
and replace references to selectedFiles inside onSubmit with
selectedFilesRef.value so the callback always sees the current file list.
1. McpServersTab improvements: - Add header row with title and real-time JSON validation status - Show validation status indicator (Loading/Invalid JSON/Valid JSON) - Add styled error container with icon for error details - Add border around JSON editor with error state styling - Add footer with example hint and Save & Reload button with icon - Use CircularProgressIndicator for loading states 2. Fix Icon component usage: - Change 'key =' to 'imageVector =' for all Icon components - Replace IdeaComposeIcons.Schedule with IdeaComposeIcons.History 3. UI consistency: - Match the design patterns from ToolConfigDialog.kt - Use consistent spacing, colors, and typography
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (1)
50-57: Consider explicit handling for empty configuration.The function returns the default template when the servers map is empty (line 51) or on serialization errors (line 55). While this provides helpful examples, it might be unexpected behavior—users might expect an empty JSON object
{}when no servers are configured.This is primarily a UX consideration. The current approach aids discoverability but could confuse users who want to start with a blank configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt(9 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
**/idea/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/idea/**/*.kt: For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer:JewelFlags.useCustomPopupRenderer = trueinIdeaAgentToolWindowFactory
For popup/dropdown menus in IntelliJ plugins, use Jewel'sPopupMenuinstead ofandroidx.compose.ui.window.Popup
For dialogs in IntelliJ plugins, use IntelliJ'sDialogWrapperwithorg.jetbrains.jewel.bridge.composeinstead ofandroidx.compose.ui.window.Dialog
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For dialogs in IntelliJ plugins, use IntelliJ's `DialogWrapper` with `org.jetbrains.jewel.bridge.compose` instead of `androidx.compose.ui.window.Dialog`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For popup/dropdown menus in IntelliJ plugins, use Jewel's `PopupMenu` instead of `androidx.compose.ui.window.Popup`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: For `mpp-idea` plugin development, use `cd mpp-idea && ../gradlew compileKotlin` instead of `./gradlew :mpp-idea:compileKotlin`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (223)
- GitHub Check: Build (241)
🔇 Additional comments (6)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (6)
96-127: Excellent use of DialogWrapper for proper z-index handling.The implementation correctly uses IntelliJ's
DialogWrapperwith Compose content, which ensures proper z-index handling and centers the dialog in the IDE window. The size is now consistent between the wrapper'spreferredSize(850×650) and the content'sheightIn(max = 650.dp).As per coding guidelines and learnings, this is the recommended approach for dialogs in IntelliJ plugins.
240-245: Good UX: Escape key handling improves dialog usability.The
onKeyEventhandler properly captures the Escape key to dismiss the dialog, which is a standard and expected behavior for modal dialogs.
152-179: Well-implemented auto-save with debouncing.The auto-save mechanism correctly:
- Debounces changes with a 2-second delay to avoid excessive saves
- Cancels previous jobs to prevent race conditions
- Silently fails for auto-save errors (acceptable since it's non-critical)
- Updates the UI state to reflect saved/unsaved status
538-575: Excellent UX: informative banner about built-in tools.The info banner clearly communicates that built-in tools are always enabled, which helps users understand the tool configuration model. The styling with
JewelThemecolors ensures consistency with the IDE's appearance.
838-882: Excellent real-time JSON validation with clear visual feedback.The validation status indicator provides immediate, clear feedback to users:
- Success state with green check icon (lines 867-879)
- Error state with red error icon (lines 852-865)
- Loading state with spinner (lines 843-851)
This helps users catch configuration errors early and understand the current state at a glance.
64-68: TheMcpServerConfig.validate()method exists in the codebase atmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/mcp/McpConfig.ktand is correctly implemented. It validates that the configuration has exactly one of either acommandorurlset (but not both). The call on line 65 is valid.
| var isChecked by remember { mutableStateOf(tool.enabled) } | ||
|
|
||
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(start = 8.dp) | ||
| .clip(RoundedCornerShape(4.dp)) | ||
| .background( | ||
| if (isChecked) JewelTheme.globalColors.borders.normal.copy(alpha = 0.08f) | ||
| else Color.Transparent | ||
| ) | ||
| .clickable { | ||
| isChecked = !isChecked | ||
| onToggle(isChecked) | ||
| } | ||
| .padding(horizontal = 8.dp, vertical = 6.dp), | ||
| verticalAlignment = Alignment.CenterVertically | ||
| ) { | ||
| Checkbox( | ||
| checked = isChecked, | ||
| onCheckedChange = { | ||
| isChecked = it | ||
| onToggle(it) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State synchronization issue: local checkbox state may drift from source.
The isChecked state is initialized from tool.enabled but won't re-synchronize if the parent updates tool.enabled (e.g., during configuration reload or external changes). This can cause the checkbox to show stale state.
Apply this diff to keep checkbox state synchronized with the source:
- var isChecked by remember { mutableStateOf(tool.enabled) }
+ var isChecked by remember(tool.enabled) { mutableStateOf(tool.enabled) }Or derive the state directly without local caching:
- var isChecked by remember { mutableStateOf(tool.enabled) }
+ val isChecked = tool.enabled
Row(
modifier = Modifier
@@ -722,8 +721,7 @@
else Color.Transparent
)
.clickable {
- isChecked = !isChecked
- onToggle(isChecked)
+ onToggle(!isChecked)
}
.padding(horizontal = 8.dp, vertical = 6.dp),
verticalAlignment = Alignment.CenterVertically
@@ -731,8 +729,7 @@
Checkbox(
checked = isChecked,
onCheckedChange = {
- isChecked = it
- onToggle(it)
+ onToggle(it)
}
)🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt
around lines 713–736, the local isChecked state is initialized from tool.enabled
and can drift when tool.enabled changes; remove the cached mutable state and
derive the checkbox state directly from tool.enabled (e.g., use val isChecked =
tool.enabled) or keep a local mutableState but add synchronization using
LaunchedEffect(tool.enabled) { isChecked = tool.enabled } so the UI always
reflects the current source state, and ensure onToggle still calls the parent
updater.
…ement 1. Create IdeaToolConfigService: - Project-level service for managing tool configuration state - Provides StateFlow for observing config changes - Uses configVersion counter to trigger UI recomposition - Centralized save/load with notification to listeners 2. Update IdeaToolLoadingStatusBar: - Add project parameter - Observe configVersion from IdeaToolConfigService - Recompute toolStatus when config version changes 3. Update IdeaAgentViewModel: - Use IdeaToolConfigService for loading tool config - Get fresh config from service in getToolLoadingStatus() 4. Update IdeaMcpConfigDialog: - Add project parameter to IdeaMcpConfigDialogContent - Use IdeaToolConfigService.saveAndUpdateConfig() for auto-save - Notify listeners when tools are toggled 5. Register service in plugin.xml This ensures the status bar updates when MCP tools are enabled/disabled in the configuration dialog.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (1)
417-424: Inconsistent save path bypasses listener notifications.The
onReloadhandler usesConfigManager.saveToolConfig()directly (line 424), while auto-save (lines 181-186) correctly usestoolConfigService.saveAndUpdateConfig()to notify listeners. This inconsistency can cause state desynchronization if other components depend on service notifications.Apply this diff to use the service consistently:
val newServers = result.getOrThrow() toolConfig = toolConfig.copy(mcpServers = newServers) - ConfigManager.saveToolConfig(toolConfig) + if (toolConfigService != null) { + toolConfigService.saveAndUpdateConfig(toolConfig) + } else { + ConfigManager.saveToolConfig(toolConfig) + } try {
♻️ Duplicate comments (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (1)
728-751: State synchronization issue still present (as flagged in previous review).The
isCheckedstate is initialized fromtool.enabledbut won't re-synchronize if the parent updates it. This was already flagged in a previous review.
🧹 Nitpick comments (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt (1)
160-165: CallingreloadConfig()inside a suspend function is inefficient.Since
startMcpPreloading()is already a suspend function, callingreloadConfig()(which internally usesrunBlocking) creates nested blocking. IfIdeaToolConfigServiceis refactored to use suspend functions as suggested, this would become:- val toolConfigService = IdeaToolConfigService.getInstance(project) - toolConfigService.reloadConfig() - val toolConfig = toolConfigService.getToolConfig() + val toolConfigService = IdeaToolConfigService.getInstance(project) + toolConfigService.reloadConfigAsync() + val toolConfig = toolConfigService.getToolConfig()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt(9 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/services/IdeaToolConfigService.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt(3 hunks)mpp-idea/src/main/resources/META-INF/plugin.xml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/services/IdeaToolConfigService.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
**/idea/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/idea/**/*.kt: For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer:JewelFlags.useCustomPopupRenderer = trueinIdeaAgentToolWindowFactory
For popup/dropdown menus in IntelliJ plugins, use Jewel'sPopupMenuinstead ofandroidx.compose.ui.window.Popup
For dialogs in IntelliJ plugins, use IntelliJ'sDialogWrapperwithorg.jetbrains.jewel.bridge.composeinstead ofandroidx.compose.ui.window.Dialog
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/services/IdeaToolConfigService.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
🧠 Learnings (7)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/compose/**/*.{kt,kts} : In Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/resources/META-INF/plugin.xmlmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For dialogs in IntelliJ plugins, use IntelliJ's `DialogWrapper` with `org.jetbrains.jewel.bridge.compose` instead of `androidx.compose.ui.window.Dialog`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For popup/dropdown menus in IntelliJ plugins, use Jewel's `PopupMenu` instead of `androidx.compose.ui.window.Popup`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to {**/compose/**/*.kt,mpp-ui/src/jsMain/typescript/**/*.{ts,tsx}} : DO NOT hardcode colors (e.g., `Color(0xFF...)` or `#hex`). Always use design tokens for consistency across platforms
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Note that `IdeaAgentViewModelTest` requires IntelliJ Platform Test Framework; `JewelRendererTest` can run standalone with JUnit 5
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: For `mpp-idea` plugin tests, run with: `cd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt
🔇 Additional comments (11)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (5)
44-91: LGTM!JSON serialization helpers are well-implemented with proper error handling using
Resulttype, server config validation, and a helpful default template for new users.
97-131: Good implementation following IntelliJ dialog best practices.The
DialogWrapperapproach withorg.jetbrains.jewel.bridge.composeis correct per the coding guidelines. The sizing is now properly aligned between the wrapper (Dimension(850, 650)) and the Compose content.
539-649: LGTM!Good implementation with proper use of
mutableStateMapOffor expanded state tracking, appropriate key usage inLazyColumnitems, and clear visual hierarchy for MCP servers and tools.
806-824: LGTM!Well-implemented bidirectional sync between
textFieldStateand the externalmcpConfigJsonvalue usingsnapshotFlowwithdistinctUntilChanged. The conditional check on line 813 correctly prevents infinite update loops.
970-986: LGTM!Reload button properly handles loading state with visual feedback and correct disabled condition.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/services/IdeaToolConfigService.kt (2)
62-77: Good reactive state management pattern.The
updateStatemethod correctly uses StateFlow for reactive updates and incrementsconfigVersionto trigger recomposition in Compose UI. This aligns well with the observable pattern used inIdeaToolLoadingStatusBar.
109-117: Well-structured state data class.The
ToolConfigStatedata class provides a clean immutable snapshot of configuration state with sensible defaults.mpp-idea/src/main/resources/META-INF/plugin.xml (1)
61-62: Service registration looks correct.The
IdeaToolConfigServiceis properly registered as a project-level service, consistent with the existingCoroutineScopeHolderpattern.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt (1)
473-479: Clean integration with the new service.Getting fresh config from the service ensures the status bar reflects the latest configuration changes. Since
getToolConfig()is a synchronous read from the StateFlow's current value, this won't block the UI.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/status/IdeaToolLoadingStatusBar.kt (1)
33-39: Well-implemented reactive pattern.The use of
rememberwithconfigVersionas a key correctly triggers recomputation when configuration changes. Observing the service'sconfigVersionStateFlow ensures the UI stays synchronized with config updates.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)
246-250: Correct parameter propagation.The
projectparameter is correctly passed toIdeaToolLoadingStatusBarto enable reactive config observation.
| mcpLoadError?.let { error -> | ||
| Text(error, color = org.jetbrains.jewel.foundation.theme.JewelTheme.globalColors.text.error) | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .clip(RoundedCornerShape(8.dp)) | ||
| .background(Color(0xFFFFEBEE)) | ||
| .padding(8.dp) | ||
| ) { | ||
| Row( | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.spacedBy(8.dp) | ||
| ) { | ||
| Icon( | ||
| imageVector = IdeaComposeIcons.Error, | ||
| contentDescription = "Error", | ||
| tint = Color(0xFFD32F2F), | ||
| modifier = Modifier.size(18.dp) | ||
| ) | ||
| Text( | ||
| text = error, | ||
| style = JewelTheme.defaultTextStyle.copy( | ||
| fontSize = 12.sp, | ||
| color = Color(0xFFD32F2F) | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| Spacer(modifier = Modifier.height(8.dp)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded colors won't adapt to dark theme.
Multiple places use hardcoded colors like Color(0xFFFFEBEE) and Color(0xFFD32F2F) that are designed for light theme. These will look incorrect in IntelliJ's dark theme.
Consider using semantic colors from JewelTheme or IntelliJ's JBUI colors:
Box(
modifier = Modifier
.fillMaxWidth()
.clip(RoundedCornerShape(8.dp))
- .background(Color(0xFFFFEBEE))
+ .background(JewelTheme.globalColors.outlines.error.copy(alpha = 0.1f))
.padding(8.dp)
) {
// ...
Icon(
imageVector = IdeaComposeIcons.Error,
contentDescription = "Error",
- tint = Color(0xFFD32F2F),
+ tint = JewelTheme.globalColors.outlines.error,
modifier = Modifier.size(18.dp)
)This pattern should be applied to all hardcoded colors in the file (lines 674-675, 871-892, 905-922, 937).
Committable suggestion skipped: line range outside the PR's diff.
| init { | ||
| // Load initial configuration | ||
| reloadConfig() | ||
| } | ||
|
|
||
| /** | ||
| * Reload configuration from disk and update state. | ||
| * Uses runBlocking since this is called from non-suspend context. | ||
| */ | ||
| fun reloadConfig() { | ||
| try { | ||
| val toolConfig = runBlocking { ConfigManager.loadToolConfig() } | ||
| updateState(toolConfig) | ||
| logger.debug("Tool configuration reloaded: ${toolConfig.enabledMcpTools.size} enabled tools") | ||
| } catch (e: Exception) { | ||
| logger.warn("Failed to reload tool configuration: ${e.message}") | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid runBlocking in service initialization and public methods.
Using runBlocking in the init block and reloadConfig() can block the EDT if the service is instantiated during UI operations. IntelliJ project services may be created on the EDT, causing UI freezes.
Consider making reloadConfig() a suspend function and using lazy initialization or background loading:
- init {
- // Load initial configuration
- reloadConfig()
- }
+ init {
+ // Schedule async loading to avoid blocking EDT
+ @OptIn(DelicateCoroutinesApi::class)
+ GlobalScope.launch(Dispatchers.IO) {
+ reloadConfigAsync()
+ }
+ }
- fun reloadConfig() {
+ suspend fun reloadConfigAsync() {
try {
- val toolConfig = runBlocking { ConfigManager.loadToolConfig() }
+ val toolConfig = ConfigManager.loadToolConfig()
updateState(toolConfig)
logger.debug("Tool configuration reloaded: ${toolConfig.enabledMcpTools.size} enabled tools")
} catch (e: Exception) {
logger.warn("Failed to reload tool configuration: ${e.message}")
}
}Alternatively, if synchronous loading is required, use Dispatchers.IO with proper scope management.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/services/IdeaToolConfigService.kt
around lines 39-56, remove the blocking runBlocking usage in init and
reloadConfig; make reloadConfig a suspend function (suspend fun reloadConfig())
and update callers to invoke it from a coroutine, or create/own a CoroutineScope
and launch a background job on Dispatchers.IO to load the config asynchronously
during initialization (e.g., launch(Dispatchers.IO) { runCatching {
ConfigManager.loadToolConfig() }.onSuccess { updateState(it); logger.debug(...)
}.onFailure { logger.warn(...) } }), and ensure exceptions are caught and
logged; if synchronous load is absolutely required, perform it on Dispatchers.IO
via a dedicated blocking call executed off the EDT (not runBlocking on the EDT)
with proper scope management.
| /** | ||
| * Save tool configuration and update state. | ||
| * Uses runBlocking since this is called from non-suspend context. | ||
| */ | ||
| fun saveAndUpdateConfig(toolConfig: ToolConfigFile) { | ||
| try { | ||
| runBlocking { ConfigManager.saveToolConfig(toolConfig) } | ||
| updateState(toolConfig) | ||
| logger.debug("Tool configuration saved and state updated") | ||
| } catch (e: Exception) { | ||
| logger.error("Failed to save tool configuration: ${e.message}") | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
runBlocking in saveAndUpdateConfig can also block the calling thread.
Same issue as reloadConfig(). If called from a UI callback or Compose event handler, this will freeze the UI during file I/O.
Consider accepting a CoroutineScope parameter or making this a suspend function:
- fun saveAndUpdateConfig(toolConfig: ToolConfigFile) {
+ suspend fun saveAndUpdateConfigAsync(toolConfig: ToolConfigFile) {
try {
- runBlocking { ConfigManager.saveToolConfig(toolConfig) }
+ ConfigManager.saveToolConfig(toolConfig)
updateState(toolConfig)
logger.debug("Tool configuration saved and state updated")
} catch (e: Exception) {
logger.error("Failed to save tool configuration: ${e.message}")
}
}🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/services/IdeaToolConfigService.kt
around lines 79 to 91, replace the blocking runBlocking call which can freeze
the UI with a non-blocking approach: either make saveAndUpdateConfig a suspend
function and call ConfigManager.saveToolConfig(toolConfig) directly, or add a
CoroutineScope parameter and launch a coroutine (scope.launch) to call
ConfigManager.saveToolConfig and then updateState(toolConfig) on success; ensure
exceptions are caught inside the coroutine and logged via logger.error, and keep
the debug log after successful update.
Problem: DevInsProgramRunner was implementing Disposable and registering a MessageBusConnection with 'this' as parent, but the runner itself was never properly disposed, causing memory leak warnings. Solution: - Remove Disposable interface from DevInsProgramRunner - Connect to project's message bus instead of application's - Register the connection with the project as parent disposable - This ensures proper cleanup when the project is closed The connection is now tied to the project lifecycle instead of the runner's lifecycle, which is the correct pattern for ProgramRunner implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
exts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/run/DevInsProgramRunner.kt (1)
46-50: Unusedconnectionfield assignment.The
connectionfield is assigned on line 47 but never read anywhere in the class. If it was intended for cleanup or tracking purposes, it's no longer serving that purpose. This appears to be dead code after the refactoring.If you adopt the per-project approach suggested above, this field can be removed entirely. Otherwise, clarify its intended purpose or remove it:
- private var connection: MessageBusConnection? = nullAnd remove line 47:
val projectConnection = environment.project.messageBus.connect() - connection = projectConnection
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
exts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/run/DevInsProgramRunner.kt(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
exts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/run/DevInsProgramRunner.kt
| // Use lazy initialization to avoid memory leak - connection is created per execution | ||
| // and tied to the project's lifecycle, not the runner's lifecycle | ||
| private var connection: MessageBusConnection? = null | ||
| private var isSubscribed = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical bug: isSubscribed flag is never reset when the connection is disposed.
DevInsProgramRunner is a singleton extension. When a project closes, the connection is disposed via Disposer.register, but isSubscribed remains true. Subsequent runs on a different project (or the same project after reopening) will skip subscription entirely because isSubscribed is still true, causing the DevInsRunListener to never be registered.
Consider using a per-project subscription map or resetting state when the connection is disposed:
- // Use lazy initialization to avoid memory leak - connection is created per execution
- // and tied to the project's lifecycle, not the runner's lifecycle
- private var connection: MessageBusConnection? = null
- private var isSubscribed = false
+ // Track subscriptions per project to handle multiple projects and project reopening
+ private val subscribedProjects = mutableSetOf<String>()Then in doExecute, use the project's unique identifier and add a dispose listener:
val projectId = environment.project.locationHash
if (projectId !in subscribedProjects) {
val projectConnection = environment.project.messageBus.connect()
Disposer.register(environment.project, projectConnection)
// Reset subscription state when project is disposed
Disposer.register(environment.project) {
subscribedProjects.remove(projectId)
}
projectConnection.subscribe(DevInsRunListener.TOPIC, object : DevInsRunListener {
// ... listener implementation
})
subscribedProjects.add(projectId)
}🤖 Prompt for AI Agents
In
exts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/run/DevInsProgramRunner.kt
around lines 25 to 28, the singleton runner uses a single boolean isSubscribed
that is never reset when a project's MessageBusConnection is disposed, causing
listeners to be skipped on subsequent project runs; change the approach to track
subscriptions per project (e.g., a Set or Map keyed by project.locationHash or
project) or ensure isSubscribed is cleared when the connection is disposed:
create the project-specific connection via project.messageBus.connect(),
register that connection with Disposer.register(project, connection), register a
Disposer callback to remove the project key (or set isSubscribed=false) when the
project is disposed, and only subscribe the DevInsRunListener when the project
key is not already tracked.
Summary
This PR improves the file search popup and related UI components in the mpp-idea module.
Changes
1. Fix
ClsFileImplClassNotFoundExceptionClsFileImplwithPsiCompiledFileinterfacedecompiledPsiFile?.textto get decompiled text2. Redesign
IdeaDevInInputAreaLayoutJewelTheme.globalColors.borders.normalfor border color3. Enhance
IdeaTopToolbarFile List DisplayCollapsed mode (default):
horizontalScrollfor horizontal scrollingExpanded mode:
animateContentSizefor smooth animation4. Restyle
IdeaMcpConfigDialogIdeaMcpConfigDialogWrapperusing IntelliJ'sDialogWrapper5. Implement Prompt Enhancement Feature
IdeaPromptEnhancerservice.autodev/domain.csvorprompts/domain.csvreplaceText()method toIdeaDevInInputRelated Issues
Closes #29
Testing
cd mpp-idea && ../gradlew compileKotlinPull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
New Features
Removed
✏️ Tip: You can customize this high-level summary in your review settings.