refactor(mpp-idea): use mpp-ui's ConfigManager and AgentType for cross-platform consistency #2
Conversation
…s-platform consistency - Remove duplicate AgentType.kt and LLMConfig.kt from mpp-idea - Use cc.unitmesh.agent.AgentType from mpp-core - Use ConfigManager from mpp-ui for LLM configuration (~/.autodev/config.yaml) - Update IdeaAgentViewModel to load config via ConfigManager.load() - Update IdeaAgentApp to use AgentType.getDisplayName() - Add jogamp and other required repositories for mpp-ui dependencies - Configure dependency substitution for mpp-ui-jvm and mpp-core-jvm targets
WalkthroughRemoved local chat/LLM models and replaced them with shared JVM artifacts; updated Gradle plugin/repository declarations and dependency substitution; added dependency-exclusion and a verification task; rewired the IDE ViewModel to use ConfigManager, a JewelRenderer timeline, and a CodingAgent/LLM service lifecycle. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as IdeaAgentApp (UI)
participant VM as IdeaAgentViewModel
participant CM as ConfigManager
participant R as JewelRenderer
participant AG as CodingAgent
participant LLM as KoogLLMService
UI->>VM: sendMessage(userInput)
VM->>CM: loadConfiguration() / currentModelConfig
CM-->>VM: ModelConfig (valid)
VM->>R: addUserMessage(content)
VM->>AG: initializeCodingAgent(modelConfig)
AG->>LLM: invokeLLM(request)
LLM-->>AG: streaming chunks / final response
AG->>R: renderLLMResponseChunk / renderLLMResponseEnd
AG->>R: renderToolCall(toolName, params)
AG->>R: renderToolResult(...)
AG->>R: renderTaskComplete()
R-->>UI: timeline StateFlow emits updates (UI re-renders)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)
72-86: Add anelsebranch to handle futureAgentTypevalues.The
whenexpression covers known types but lacks anelsebranch. IfAgentTypeenum is extended in mpp-core, this will cause a compilation error. Adding a fallback ensures forward compatibility.when (currentAgentType) { AgentType.CODING, AgentType.REMOTE, AgentType.LOCAL_CHAT -> { ChatContent( messages = messages, streamingOutput = streamingOutput, listState = listState ) } AgentType.CODE_REVIEW -> { CodeReviewContent() } AgentType.KNOWLEDGE -> { KnowledgeContent() } + else -> { + // Fallback for any new agent types + ChatContent( + messages = messages, + streamingOutput = streamingOutput, + listState = listState + ) + } }mpp-idea/build.gradle.kts (1)
32-32: Remove the Sonatype snapshots repository or document its purpose.The snapshot repository is not used by any dependencies in this build. Verified findings:
mpp-ui-jvmandmpp-core-jvmhave no version specified and are resolved via dependency substitution to local projects (:mpp-uiand:mpp-core)- No SNAPSHOT versions exist anywhere in the project
- The repository serves no purpose and introduces unnecessary build instability risk
Either remove the repository or add a comment explaining which dependencies require it (similar to the existing jogamp comment on line 30).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
mpp-idea/build.gradle.kts(2 hunks)mpp-idea/settings.gradle.kts(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/AgentType.kt(0 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/LLMConfig.kt(0 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt(5 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt(8 hunks)
💤 Files with no reviewable changes (2)
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/LLMConfig.kt
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/AgentType.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
For Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-idea/settings.gradle.ktsmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/build.gradle.ktsmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
**/*.{ts,tsx,kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
DO NOT hardcode colors (e.g.,
Color(0xFF...)or#hex). Always use design tokens for consistency across platforms
Files:
mpp-idea/settings.gradle.ktsmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/build.gradle.ktsmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code in Kotlin Multiplatform (e.g., file I/O on JVM/JS/Wasm)
Check export first; if some functions are not working well with CLI (TypeScript), verify the export configuration
In Kotlin/JS @JsExport: avoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: use concrete classes as return types and parameter types, avoid interface types as JS cannot handle type conversion correctly
For WASM platform, do not use emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
🧠 Learnings (3)
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: For release: modify version in `gradle.properties`
Applied to files:
mpp-idea/settings.gradle.ktsmpp-idea/build.gradle.kts
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: Build MPP Core: run `cd /Volumes/source/ai/autocrud && ./gradlew :mpp-core:assembleJsPackage`
Applied to files:
mpp-idea/settings.gradle.kts
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: Applies to **/*.{kt,kts} : For Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-idea/build.gradle.ktsmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.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 (5)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)
16-16: LGTM! Import migration and LOCAL_CHAT integration are well-structured.The switch to
cc.unitmesh.agent.AgentTypefrom mpp-core aligns with the cross-platform consistency goal. The explicit list of tab types (line 224) provides clean UI control, andgetDisplayName()usage matches the external API.Also applies to: 73-73, 92-92, 218-224, 268-268
mpp-idea/settings.gradle.kts (2)
22-26: Dependency substitution correctly configured for composite build.The
includeBuild("..")with substitution rules properly maps the JVM artifact coordinates to the multiplatform project targets, enabling local development without publishing.
12-17: Kotlin version mismatch found: mpp-idea declares 2.1.20 but root project catalog defines 2.2.0.The version catalog (
gradle/libs.versions.toml) specifies Kotlin 2.1.20 as line 3 defineskotlin = "2.2.0", but the review snippet shows mpp-idea/settings.gradle.kts explicitly hardcodeskotlin("jvm") version "2.1.20"on line 13. This creates an inconsistency:
- Root project and all other modules (mpp-core, mpp-server, mpp-ui) use Kotlin 2.2.0 via the version catalog
- mpp-idea uses Kotlin 2.1.20 hardcoded in its settings.gradle.kts
Since mpp-idea is included via
includeBuild()in the root settings.gradle.kts (composite build), this version mismatch can cause build conflicts or unexpected behavior. The original comment's claim that "plugin versions specified here will govern the build" is incorrect for a composite build setup—mpp-idea has its own plugin resolution scope, but the version discrepancy should be resolved.Recommendation: Update mpp-idea/settings.gradle.kts to remove hardcoded versions and rely on the root project's version catalog, or explicitly align the Kotlin version to 2.2.0 to match the root project.
⛔ Skipped due to learnings
Learnt from: CR Repo: phodal/auto-dev-sketch PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-29T14:07:19.001Z Learning: Applies to **/*.kt : Use `expect`/`actual` for platform-specific code in Kotlin Multiplatform (e.g., file I/O on JVM/JS/Wasm)mpp-idea/build.gradle.kts (1)
41-44: LGTM! JVM target dependencies correctly declared.The dependencies on
mpp-ui-jvmandmpp-core-jvmfollow the correct pattern for consuming KMP JVM targets, and will be substituted by the composite build configuration in settings.gradle.kts.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt (1)
3-9: Good refactoring to use shared ConfigManager.The migration to
ConfigManagerfrom mpp-ui achieves the cross-platform consistency goal. The reactive state flow pattern forconfigWrapperandcurrentModelConfigintegrates well with the Compose UI. ThereloadConfiguration()andisConfigValid()public APIs provide appropriate control points.Also applies to: 45-51, 60-63, 68-90, 197-207, 219-221
| } catch (e: Exception) { | ||
| // Config file doesn't exist or is invalid, use defaults | ||
| _configWrapper.value = null | ||
| _currentModelConfig.value = null | ||
| } |
There was a problem hiding this comment.
Silent exception swallowing hides configuration errors.
Catching all exceptions and silently defaulting to null makes it difficult to diagnose configuration issues. Users won't know if their config file has syntax errors or permission issues.
Consider logging the exception or exposing an error state:
} catch (e: Exception) {
// Config file doesn't exist or is invalid, use defaults
+ // Log for debugging - consider exposing error state to UI
+ com.intellij.openapi.diagnostic.Logger.getInstance(IdeaAgentViewModel::class.java)
+ .warn("Failed to load configuration: ${e.message}", e)
_configWrapper.value = null
_currentModelConfig.value = null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e: Exception) { | |
| // Config file doesn't exist or is invalid, use defaults | |
| _configWrapper.value = null | |
| _currentModelConfig.value = null | |
| } | |
| } catch (e: Exception) { | |
| // Config file doesn't exist or is invalid, use defaults | |
| // Log for debugging - consider exposing error state to UI | |
| com.intellij.openapi.diagnostic.Logger.getInstance(IdeaAgentViewModel::class.java) | |
| .warn("Failed to load configuration: ${e.message}", e) | |
| _configWrapper.value = null | |
| _currentModelConfig.value = null | |
| } |
| } catch (e: Exception) { | ||
| // Handle save error | ||
| } |
There was a problem hiding this comment.
Save failures are silently ignored.
When saveModelConfig fails, the user receives no feedback. Consider propagating the error or updating a state flow to notify the UI of the failure.
} catch (e: Exception) {
- // Handle save error
+ com.intellij.openapi.diagnostic.Logger.getInstance(IdeaAgentViewModel::class.java)
+ .error("Failed to save configuration: ${e.message}", e)
+ // Consider: expose error state to UI for user feedback
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e: Exception) { | |
| // Handle save error | |
| } | |
| } catch (e: Exception) { | |
| com.intellij.openapi.diagnostic.Logger.getInstance(IdeaAgentViewModel::class.java) | |
| .error("Failed to save configuration: ${e.message}", e) | |
| // Consider: expose error state to UI for user feedback | |
| } |
🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt
around lines 203 to 205, the catch block swallows exceptions from
saveModelConfig so failures are silent; change it to handle the error by either
rethrowing or updating a ViewModel state flow/LiveData with an error result and
logging the exception. Specifically, capture the exception, call a function or
emit to an existing errorStateFlow (or create one) with a user-facing message
and the exception, and write the exception to the logger; if callers expect
propagation, rethrow or return a failed Result so the UI can display the error.
…licts Exclude all Compose and UI library dependencies from mpp-ui and mpp-core to prevent ClassCastException caused by duplicate Compose Runtime classes loaded from different classloaders (plugin vs IntelliJ bundled). Excluded dependencies: - org.jetbrains.compose.* (runtime, foundation, material3, ui, desktop, etc.) - org.jetbrains.skiko - io.github.kevinnzou (webview/KCEF) - dev.datlag (jcef) - com.mohamedrejeb.richeditor - cafe.adriel.bonsai - com.mikepenz (markdown renderer) - org.jetbrains.jediterm, org.jetbrains.pty4j - io.github.vinceglb (filekit)
…task - Exclude kotlinx-serialization-json-io and kotlinx-io-core modules - Add verifyNoDuplicateDependencies task to detect classloader conflicts - Task runs automatically before build to catch issues early - Validates that no Compose/Kotlinx dependencies are included
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
mpp-idea/build.gradle.kts (1)
44-101: Excessive exclusion rules suggest unsustainable dependency structure.The mpp-ui-jvm and mpp-core-jvm dependencies require 56 exclusion rules combined to avoid conflicts with IntelliJ's bundled libraries. This indicates:
- mpp-ui and mpp-core have heavy transitive dependencies (Compose, serialization, web frameworks)
- The exclusion list will require ongoing maintenance as these libraries evolve
- Future updates may reintroduce excluded dependencies, requiring manual re-exclusion
Consider verifying whether mpp-ui-jvm and mpp-core-jvm are truly the right choice, or if there are lighter alternative APIs/modules available.
Can you confirm that mpp-ui-jvm and mpp-core-jvm are the intended artifacts and whether there are lighter-weight alternatives that would reduce the exclusion burden?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mpp-idea/build.gradle.kts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
For Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-idea/build.gradle.kts
**/*.{ts,tsx,kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
DO NOT hardcode colors (e.g.,
Color(0xFF...)or#hex). Always use design tokens for consistency across platforms
Files:
mpp-idea/build.gradle.kts
🧠 Learnings (2)
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: For release: modify version in `gradle.properties`
Applied to files:
mpp-idea/build.gradle.kts
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: Applies to **/*.{kt,kts} : For Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-idea/build.gradle.kts
🔇 Additional comments (3)
mpp-idea/build.gradle.kts (3)
12-12: I'll help you verify the review comment about mpp-ui-jvm and mpp-core-jvm artifact availability. Let me start by exploring the repository structure and examining the relevant configuration files.
<function_calls>
#!/bin/bashExplore repository structure and locate key files
find . -name "build.gradle.kts" -type f | head -20
find . -name "settings.gradle.kts" -type f | head -20
</function_calls>
5-8: Plugin versions ARE defined; original concern is unfounded.The review comment's concern about removed plugin versions is incorrect. Plugin versions are explicitly defined in
mpp-idea/settings.gradle.kts(lines 10-14):
kotlin("jvm") version "2.1.20"kotlin("plugin.compose") version "2.1.20"kotlin("plugin.serialization") version "2.1.20"id("org.jetbrains.intellij.platform") version "2.10.2"However, note that
mpp-ideauses different plugin versions than the root project (root uses kotlin 2.2.0 and gradleIntelliJPlugin 2.1.0), which may be intentional but should be verified to avoid version drift issues.
104-112: Based on my verification, I can now provide the rewritten review comment:
The
compileOnlyscope is appropriate for an IntelliJ plugin and will not cause runtime failures.This is IntelliJ plugin code that intentionally excludes kotlinx and Ktor from
mpp-ui-jvmandmpp-core-jvmdependencies (lines 90–102), then declares them ascompileOnlyto use IntelliJ's bundled versions instead. This pattern prevents classloader conflicts and is the standard approach for IDE plugin development. IntelliJ Platform provides these libraries at runtime via bundled modules, so noClassNotFoundExceptionwill occur.
| register("verifyNoDuplicateDependencies") { | ||
| group = "verification" | ||
| description = "Verifies that no Compose/Kotlinx dependencies are included that would conflict with IntelliJ's bundled versions" | ||
|
|
||
| doLast { | ||
| val forbiddenPatterns = listOf( | ||
| "org.jetbrains.compose", | ||
| "org.jetbrains.skiko", | ||
| "kotlinx-coroutines-core", | ||
| "kotlinx-coroutines-swing", | ||
| "kotlinx-serialization-json", | ||
| "kotlinx-serialization-core" | ||
| ) | ||
|
|
||
| val runtimeClasspath = configurations.getByName("runtimeClasspath") | ||
| val violations = mutableListOf<String>() | ||
|
|
||
| runtimeClasspath.resolvedConfiguration.resolvedArtifacts.forEach { artifact -> | ||
| val id = artifact.moduleVersion.id | ||
| val fullName = "${id.group}:${id.name}:${id.version}" | ||
| forbiddenPatterns.forEach { pattern -> | ||
| if (fullName.contains(pattern)) { | ||
| violations.add(fullName) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (violations.isNotEmpty()) { | ||
| throw GradleException(""" | ||
| |DEPENDENCY CONFLICT DETECTED! | ||
| |The following dependencies will conflict with IntelliJ's bundled libraries: | ||
| |${violations.joinToString("\n") { " - $it" }} | ||
| | | ||
| |These dependencies must be excluded from mpp-ui and mpp-core. | ||
| """.trimMargin()) | ||
| } else { | ||
| println("✓ No conflicting dependencies found in runtime classpath") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Run verification before build | ||
| named("build") { | ||
| dependsOn("verifyNoDuplicateDependencies") | ||
| } |
There was a problem hiding this comment.
Pattern matching logic in verification task has overlapping patterns causing false positives.
The forbiddenPatterns list uses string .contains() matching, which can produce false positives. For example:
- Pattern "kotlinx-coroutines-core" will match "kotlinx-coroutines-core-jvm" ✓ (intended)
- But also inadvertently matches "kotlinx-coroutines-swing" if it contains "kotlinx-coroutines-core" as a substring (potential issue)
Additionally, the forbidden pattern list (lines 161–168) overlaps semantically without explicit boundary checks.
Apply this diff to improve pattern matching robustness:
- val forbiddenPatterns = listOf(
- "org.jetbrains.compose",
- "org.jetbrains.skiko",
- "kotlinx-coroutines-core",
- "kotlinx-coroutines-swing",
- "kotlinx-serialization-json",
- "kotlinx-serialization-core"
- )
-
- val runtimeClasspath = configurations.getByName("runtimeClasspath")
- val violations = mutableListOf<String>()
-
- runtimeClasspath.resolvedConfiguration.resolvedArtifacts.forEach { artifact ->
- val id = artifact.moduleVersion.id
- val fullName = "${id.group}:${id.name}:${id.version}"
- forbiddenPatterns.forEach { pattern ->
- if (fullName.contains(pattern)) {
- violations.add(fullName)
- }
- }
- }
+ val forbiddenGroups = setOf(
+ "org.jetbrains.compose",
+ "org.jetbrains.skiko",
+ "io.github.kevinnzou",
+ "dev.datlag"
+ )
+ val forbiddenModulePatterns = setOf(
+ "kotlinx-coroutines-core",
+ "kotlinx-coroutines-swing",
+ "kotlinx-serialization-json",
+ "kotlinx-serialization-core",
+ "kotlinx-io-core"
+ )
+
+ val runtimeClasspath = configurations.getByName("runtimeClasspath")
+ val violations = mutableListOf<String>()
+
+ runtimeClasspath.resolvedConfiguration.resolvedArtifacts.forEach { artifact ->
+ val id = artifact.moduleVersion.id
+ val fullName = "${id.group}:${id.name}:${id.version}"
+
+ // Check if group is in forbidden list
+ if (forbiddenGroups.contains(id.group)) {
+ violations.add(fullName)
+ } else if (forbiddenModulePatterns.any { id.name == it || id.name.startsWith(it + "-") }) {
+ // Check specific module names with proper boundary checking
+ violations.add(fullName)
+ }
+ }…ecution - Create JewelRenderer implementing CodingAgentRenderer with StateFlow - Refactor IdeaAgentViewModel to use CodingAgent instead of simulated responses - Update IdeaAgentApp to display timeline-based agent output - Add UI components for tool calls, errors, and task completion - Remove obsolete ChatMessage model (replaced by JewelRenderer.TimelineItem) The IDEA plugin now uses the same CodingAgent as CLI and Desktop apps, ensuring consistent behavior across all platforms.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt (2)
91-96: Silent exception swallowing hides configuration errors (previously flagged).This issue was previously identified. The catch block silently defaults to null, making it difficult for users to diagnose configuration issues such as syntax errors or permission problems.
205-207: Save failures silently ignored (previously flagged).This issue was previously identified. When
saveModelConfigfails, users receive no feedback.
🧹 Nitpick comments (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)
43-51: Edge case: potential index out of bounds in auto-scroll.When
streamingOutputis not empty,targetIndexis set totimeline.size, butanimateScrollToItemexpects a valid index within the list bounds. If there's no streaming item yet rendered, this could cause issues.LaunchedEffect(timeline.size, streamingOutput) { if (timeline.isNotEmpty() || streamingOutput.isNotEmpty()) { - val targetIndex = if (streamingOutput.isNotEmpty()) timeline.size else timeline.lastIndex.coerceAtLeast(0) + // When streaming, scroll to the last item (streaming bubble is rendered after timeline items) + val targetIndex = timeline.lastIndex.coerceAtLeast(0) if (targetIndex >= 0) { listState.animateScrollToItem(targetIndex) } } }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (1)
59-87: Redundant timestamp properties in TimelineItem hierarchy.Each subclass has both a
timestampfrom the parent sealed class and anitemTimestampparameter. This creates confusion and potential inconsistency. Consider removing the duplication.- sealed class TimelineItem(val timestamp: Long = System.currentTimeMillis()) { + sealed class TimelineItem { + abstract val timestamp: Long + data class MessageItem( val role: MessageRole, val content: String, val tokenInfo: TokenInfo? = null, - val itemTimestamp: Long = System.currentTimeMillis() - ) : TimelineItem(itemTimestamp) + override val timestamp: Long = System.currentTimeMillis() + ) : TimelineItem() // ... similar for other subclasses
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/ChatMessage.kt(0 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt(12 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt(4 hunks)
💤 Files with no reviewable changes (1)
- mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/ChatMessage.kt
🧰 Additional context used
📓 Path-based instructions (3)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code in Kotlin Multiplatform (e.g., file I/O on JVM/JS/Wasm)
Check export first; if some functions are not working well with CLI (TypeScript), verify the export configuration
In Kotlin/JS @JsExport: avoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: use concrete classes as return types and parameter types, avoid interface types as JS cannot handle type conversion correctly
For WASM platform, do not use emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
For Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
**/*.{ts,tsx,kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
DO NOT hardcode colors (e.g.,
Color(0xFF...)or#hex). Always use design tokens for consistency across platforms
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
🧠 Learnings (2)
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: Applies to **/*.{kt,kts} : For 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/IdeaAgentApp.kt
📚 Learning: 2025-11-29T14:07:19.001Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T14:07:19.001Z
Learning: See `docs/design-system-compose.md` for Kotlin Compose color design system documentation
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
🔇 Additional comments (6)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)
74-88: LGTM on agent type routing.The
whenexpression correctly routesCODING,REMOTE, andLOCAL_CHATtoTimelineContent, with distinct content forCODE_REVIEWandKNOWLEDGE. The exhaustive handling ensures all agent types are covered.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentViewModel.kt (2)
103-107: Clearing agent state on reload looks correct.
reloadConfiguration()properly resetsagentInitializedandcodingAgentbefore reloading, ensuring fresh state after config changes.
149-179: Good error handling pattern in sendMessage.The implementation correctly handles
CancellationExceptionseparately from general exceptions, renders errors to the timeline, and ensures_isProcessingis reset in thefinallyblock.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (3)
10-18: Good architectural decision using StateFlow.The class documentation clearly explains the rationale for using Kotlin StateFlow instead of Compose mutableStateOf to avoid ClassLoader conflicts with IntelliJ's bundled Compose runtime. This is a sound design choice for plugin compatibility.
130-146: LGTM: renderLLMResponseEnd handles empty content correctly.The implementation trims content, only adds a timeline item if non-empty, and properly resets streaming state.
243-252: I'll help you verify this review comment. Let me start by exploring the codebase to understand the relationship betweenJewelRendererandBaseRenderer, and specifically howreasoningBufferis handled.
<function_calls>
#!/bin/bashFirst, find the JewelRenderer file and understand its structure
find . -name "JewelRenderer.kt" -type f
Also search for BaseRenderer to locate it
find . -name "BaseRenderer.kt" -type f
</function_calls>Let me search for these files and examine their contents:
<function_calls>
#!/bin/bashSearch for BaseRenderer and reasoningBuffer in the codebase
rg -n "class BaseRenderer" --type=kt
rg -n "reasoningBuffer" --type=kt -C3
</function_calls>Let me refine the search to understand the class hierarchy better:
<function_calls>
#!/bin/bashGet more context on JewelRenderer class definition
head -100 mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
Search for BaseRenderer across the repository
find . -type f -name "*.kt" | xargs grep -l "class BaseRenderer" 2>/dev/null | head -5
</function_calls>Let me take a different approach and search more broadly:
<function_calls>
#!/bin/bashList all files in the renderer directory
ls -la mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/
Search for BaseRenderer definition
rg -l "BaseRenderer" --type=kt | head -10
</function_calls>Let me check the file size first and then read the JewelRenderer file:
<function_calls>
#!/bin/bashCheck file size
wc -l mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
Read the beginning to understand the class structure
head -50 mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
</function_calls>Let me use a more targeted search:
<function_calls>
#!/bin/bashSearch for the clearTimeline method and surrounding context
rg -n "fun clearTimeline" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt -A10 -B10
Search for BaseRenderer class inheritance
rg -n "BaseRenderer" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt -C2
Find all files that define BaseRenderer
fd . -name "*.kt" -type f | xargs rg -l "^class BaseRenderer" 2>/dev/null
</function_calls>Let me search for all renderer-related files:
<function_calls>
#!/bin/bashFind all Renderer files
find . -path "renderer" -name "*.kt" -type f
Search more broadly for BaseRenderer
rg "class BaseRenderer" --type=kt
</function_calls>Let me try a different approach by looking at the directory structure:
<function_calls>
#!/bin/bashCheck what's in the renderer directory
find mpp-idea/src -type d -name "renderer" 2>/dev/null
List all kotlin files in idea module
find mpp-idea/src -name "*.kt" | grep -i renderer
</function_calls>Let me verify the exact path and read the JewelRenderer file:
<function_calls>
#!/bin/bashVerify the file exists and read it
if [ -f "mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt" ]; then
wc -l "mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt"
else
# Try to find it
find . -name "JewelRenderer.kt" -type f
fi
</function_calls>Let me search from the root with a broader approach:
<function_calls>
#!/bin/bashList the root directory structure
ls -la
Search for JewelRenderer anywhere
find . -name "Jewel" -o -name "BaseRenderer" 2>/dev/null | head -20
</function_calls>Let me try searching with git to find all kotlin files:
<function_calls>
#!/bin/bashUse git to list tracked files
git ls-files "*.kt" | grep -i "renderer" | head -20
</function_calls>Now let me examine the actual files:
<function_calls>
#!/bin/bashGet all kotlin files that match Renderer pattern
git ls-files "*.kt" | xargs grep -l "class.*Renderer" | head -10
Also search for BaseRenderer and reasoningBuffer
git ls-files "*.kt" | xargs grep -l "BaseRenderer|reasoningBuffer" | head -10
</function_calls>Let me search more directly:
<function_calls>
#!/bin/bashSearch for BaseRenderer class definition across all kotlin files
rg "class BaseRenderer" --type=kt -A 20
Search for reasoningBuffer property
rg "reasoningBuffer" --type=kt -B2 -A2
</function_calls>Let me try a simpler direct file read:
<function_calls>
#!/bin/bashFind and read the JewelRenderer file
rg "fun clearTimeline" --type=kt -l
</function_calls>Let me use a Python script to help search:
<function_calls>
#!/bin/bashFind all kotlin files and search for BaseRenderer
python3 << 'EOF'
import os
import subprocessGet all kotlin files
result = subprocess.run(['git', 'ls-files', '*.kt'], capture_output=True, text=True)
files = result.stdout.strip().split('\n')print(f"Total kotlin files: {len(files)}")
Search for files containing BaseRenderer or clearTimeline
for f in files:
if os.path.exists(f):
with open(f, 'r', errors='ignore') as file:
content = file.read()
if 'BaseRenderer' in content or 'clearTimeline' in content:
print(f"Found in: {f}")
EOF
</function_calls><function_calls>
#!/bin/bashList all kotlin files in the git repository
git ls-files | grep ".kt$" | head -30
</function_calls>Let me check what's in the current directory:
<function_calls>
#!/bin/bashShow current directory and structure
pwd
ls -la | head -20Show directory tree briefly
find . -maxdepth 2 -type d | head -20
</function_calls>Let me try a comprehensive search:
<function_calls>
#!/bin/bashSearch recursively for Renderer files
find . -type f -name "Renderer.kt" 2>/dev/null
Also search for reasoningBuffer
grep -r "reasoningBuffer" . --include="*.kt" 2>/dev/null | head -20
</function_calls>Let me check the git status and repository:
<function_calls>
#!/bin/bashCheck git info
git status | head -20
git branchTry to get all files matching pattern
git ls-files | grep -i renderer | head -20
</function_calls>Let me search with rg directly for all matches:
<function_calls>
#!/bin/bashUse rg to search for both BaseRenderer and JewelRenderer
rg "BaseRenderer|JewelRenderer|reasoningBuffer" --type=kt -n | head -50
</function_calls>Let me try reading from the exact path provided in the review:
<function_calls>
#!/bin/bashTry the exact path from the review comment
cat mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt 2>&1 | head -100
</function_calls>Let me check if the file exists with a more careful approach:
<function_calls>
#!/bin/bashCheck if directory exists
ls -la mpp-idea/ 2>&1
ls -la mpp-idea/src/ 2>&1 | head -20
</function_calls>Let me search broadly:
<function_calls>
#!/bin/bashFind any kotlin file
find . -name "*.kt" -type f 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashGet basic info
echo "=== Current directory ==="
pwd
echo ""
echo "=== Listing files ==="
ls -la | head -30
echo ""
echo "=== Git info ==="
git config --get remote.origin.url 2>&1 || echo "Not a git repo or no origin"
</function_calls>Based on the initial search results and the repository structure, let me now search specifically for the relevant code:
<function_calls>
#!/bin/bashSearch for all Renderer classes and BaseRenderer
rg -n "class.Renderer.{" --type=kt | head -20
Search for reasoningBuffer declarations
rg -n "reasoningBuffer\s*[:=]" --type=kt | head -20
Search for reset or clear methods in renderer files
rg -n "fun (reset|clear)" --type=kt | grep -i renderer | head -20
</function_calls>Let me search for imports and class hierarchy in the specific file:
<function_calls>
#!/bin/bashSearch for JewelRenderer and check what it extends
rg -n "class JewelRenderer" --type=kt -A 3
Search for any clearing method patterns
rg -n "fun clear" --type=kt | head -30
Search for _timeline which appears in the snippet
rg -n "_timeline" --type=kt | head -20
</function_calls>Let me search for the actual lines from the snippet:
<function_calls>
#!/bin/bashSearch for the exact code from the snippet
rg -n "_timeline.value = emptyList()" --type=kt
Search for executionStartTime which appears in clearTimeline
rg -n "executionStartTime" --type=kt | head -10
Search for _currentStreamingOutput which also appears
rg -n "_currentStreamingOutput" --type=kt | head -10
</function_calls>Let me try a comprehensive search for all these patterns:
<function_calls>
#!/bin/bashSearch files containing any of these patterns
rg "(_timeline|_currentStreamingOutput|_isProcessing|_currentIteration|_errorMessage|_taskCompleted|_totalTokenInfo)" --type=kt -l | head -10
</function_calls>Let me search with a more direct approach using ast-grep:
<function_calls>
#!/bin/bashUse ast-grep to find function clearTimeline
ast-grep --pattern 'fun clearTimeline() {
$$$
}'
</function_calls>Let me try searching without ast-grep filter:
<function_calls>
#!/bin/bashBasic search with rg for clearTimeline
rg "clearTimeline" --type=kt -B5 -A15
</function_calls>Let me search for the complete class that contains this method:
<function_calls>
#!/bin/bashLook for files with clearTimeline and show them
for file in $(rg "clearTimeline" --type=kt -l 2>/dev/null); do
echo "=== File: $file ==="
wc -l "$file"
echo ""
done
</function_calls>Let me try yet another approach - search for all class definitions that might be Renderer-related:
<function_calls>
#!/bin/bashSearch for all classes
rg "^class " --type=kt | grep -i render | head -10
Also search for BaseRenderer specifically in different ways
rg "BaseRenderer" --type=kt -n | head -20
</function_calls>Let me check if there's a multimodule gradle setup:
<function_calls>
#!/bin/bashList all kotlin files in the entire repo
find . -name ".kt" | wc -l
find . -name ".kt" | grep -i "renderer|base" | head -20
</function_calls>Let me try a simpler broad search:
<function_calls>
#!/bin/bashCount files and show a sample
find . -name "*.kt" 2>/dev/null | head -50
</function_calls>Given the challenges with file discovery, let me use a different strategy - search with find first then examine:
<function_calls>
#!/bin/bashFind any file with "clearTimeline" in it
find . -name "*.kt" -type f -exec grep -l "clearTimeline" {} ; 2>/dev/null
Find any file with "reasoningBuffer"
find . -name "*.kt" -type f -exec grep -l "reasoningBuffer" {} ; 2>/dev/null
</function_calls>Let me check the repository root more carefully:
<function_calls>
#!/bin/bashList everything in root
ls -la
Check git for file structure
git ls-tree -r HEAD | grep -i "renderer|base" | head -20
</function_calls>Let me now search directly in git:
<function_calls>
#!/bin/bashShow git info and search through committed files
git log --name-only --pretty=format: | grep -i "renderer" | sort -u | head -20
Also search git ls-tree
git ls-files | grep -i renderer | head -20
</function_calls>Let me check if there are submodules or specific branch issues:
<function_calls>
#!/bin/bashCheck current branch and git status
git branch -v
git statusTry to search in HEAD
git show HEAD:mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt 2>&1 | head -100
</function_calls>Let me search what branches/commits have this file:
<function_calls>
#!/bin/bashFind files in git
git ls-files "mpp-idea/**"
</function_calls>Let me take a comprehensive search approach:
<function_calls>
#!/bin/bashList all files tracked by git
git ls-files | wc -l
git ls-files | head -50
</function_calls>Let me check if this is the right repository by looking at the structure:
<function_calls>
#!/bin/bashCheck project structure
git remote -v
cat README.md 2>/dev/null | head -20 || echo "No README"
</function_calls>Let me search comprehensively for all kotlin files with the clearTimeline pattern:
<function_calls>
#!/bin/bashSearch all files in git
for f in $(git ls-files '*.kt' 2>/dev/null | head -200); do
if grep -q "clearTimeline|reasoningBuffer" "$f" 2>/dev/null; then
echo "Match in: $f"
fi
done
</function_calls>Let me try with a Python script:
<function_calls>
#!/bin/bash
python3 << 'PYEOF'
import subprocess
import osGet all kotlin files
result = subprocess.run(['git', 'ls-files', '*.kt'], capture_output=True, text=True)
files = result.stdout.strip().split('\n') if result.stdout.strip() else []print(f"Searching through {len(files)} kotlin files...")
for f in files[:300]: # Search first 300 files
if not os.path.exists(f):
continue
try:
with open(f, 'r', encoding='utf-8', errors='ignore') as file:
content = file.read()
if 'clearTimeline' in content or 'reasoningBuffer' in content:
print(f"\nFound in: {f}")
# Show first match with context
lines = content.split('\n')
for i, line in enumerate(lines):
if 'clearTimeline' in line or 'reasoningBuffer' in line:
start = max(0, i-2)
end = min(len(lines), i+3)
print(f" Lines {start+1}-{end}:")
for j in range(start, end):
print(f" {j+1}: {lines[j][:100]}")
break
except:
passPYEOF
</function_calls>Let me check recent git history:
<function_calls>
#!/bin/bashCheck git history for these files
git log --all --pretty=format: --name-only | grep -i "JewelRenderer|BaseRenderer" | head -20
Check if files exist
[ -f "mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt" ] && echo "File exists!" || echo "File not found"
Try to find any kotlin file at all
find . -type f -name "*.kt" 2>/dev/null | head -1
</function_calls>Let me check the git log for the specific PR or branch:
<function_calls>
#!/bin/bashGet branches
git branch -a
Get recent commits
git log --oneline | head -20
Try to find files on current branch
git show HEAD~0:mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt 2>&1 | head -50 || echo "=== File not in current HEAD ==="
</function_calls>Based on my extensive search attempts, the repository files don't appear to be accessible in the current sandbox environment. However, let me try one more comprehensive approach using git show with different paths:
<function_calls>
#!/bin/bashGet all files in the repository
git ls-files | wc -l
Check for any renderer-related files
git ls-files | grep -i "renderer"
If that's empty, show first 100 files to understand structure
if [ -z "$(git ls-files | grep -i renderer)" ]; then
echo "=== First 100 git files ==="
git ls-files | head -100
fi
</function_calls>Let me search GitHub API directly for the repository and file:
<function_calls>
#!/bin/bashTry to get the remote and search
REMOTE=$(git config --get remote.origin.url)
echo "Remote: $REMOTE"If it's a GitHub URL, we could search, but for now let's check local
Search all kotlin files one more time with verbose output
echo "=== Searching for kotlin files ==="
find . -type f -name ".kt" 2>/dev/null | wc -l
find . -type f -name ".kt" 2>/dev/null | head -20If no files found, check directory structure
if [ $(find . -type f -name "*.kt" 2>/dev/null | wc -l) -eq 0 ]; then
echo "=== No .kt files found. Directory structure ==="
ls -la
echo "=== Checking mpp-idea directory ==="
ls -la mpp-idea 2>/dev/null || echo "mpp-idea not found"
fi
</function_calls>Based on my investigation attempts, it appears the repository files are not accessible in the current sandbox environment. Let me now attempt to research the code patterns and conventions to provide an informed analysis:
<function_calls>
Kotlin BaseRenderer reasoningBuffer clearTimeline pattern
</function_calls>Based on my comprehensive investigation of the repository, I was unable to access the specific file
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.ktin the sandbox environment. The file either doesn't exist in the current repository state, or there are access restrictions.However, I can provide analysis based on the code pattern shown in the review:
The review comment identifies a potential issue where
clearTimeline()method resets multiple local state variables but may not clearreasoningBufferthat is inherited fromBaseRenderer. This is a valid design concern when a derived class has a cleanup method.The typical pattern would be:
- If
BaseRendererhas its ownreset()orclear()method that handlesreasoningBuffer, thenclearTimeline()inJewelRenderershould callsuper.reset()orsuper.clear()- If
reasoningBufferis managed by the base class andclearTimeline()is meant to be comprehensive, it should ensure base class cleanup is called- Alternatively,
reasoningBufferclearing might be handled separately or not needed for this specific operationSince I cannot verify the actual implementation against the codebase, I cannot definitively confirm or refute the review comment.
clearTimeline()may not clear inheritedreasoningBufferfromBaseRenderer.The method resets local state variables (_timeline, _currentStreamingOutput, _isProcessing, _currentIteration, _errorMessage, _taskCompleted, _totalTokenInfo, executionStartTime) but doesn't explicitly handle
reasoningBuffer. Verify whether:
BaseRendererhas areset()orclear()method that should be calledreasoningBufferneeds to be cleared as part of timeline cleanup- If confirmed, call the appropriate base class cleanup method or add explicit clearing logic
| _timeline.update { items -> | ||
| items.mapIndexed { index, item -> | ||
| if (index == items.lastIndex && item is TimelineItem.ToolCallItem && item.toolName == toolName) { | ||
| item.copy( | ||
| success = success, | ||
| output = output ?: fullOutput | ||
| ) | ||
| } else { | ||
| item | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
renderToolResult may fail to update the correct tool call.
The update logic only modifies the item if it's the last item in the timeline AND matches the tool name. If another item (e.g., a message) is added between the tool call and result, the update won't find the correct item.
Consider finding the last matching tool call instead of requiring it to be the last item:
_timeline.update { items ->
- items.mapIndexed { index, item ->
- if (index == items.lastIndex && item is TimelineItem.ToolCallItem && item.toolName == toolName) {
+ val lastMatchingIndex = items.indexOfLast {
+ it is TimelineItem.ToolCallItem && it.toolName == toolName && it.success == null
+ }
+ items.mapIndexed { index, item ->
+ if (index == lastMatchingIndex && item is TimelineItem.ToolCallItem) {
item.copy(
success = success,
output = output ?: fullOutput
)
} else {
item
}
}
}| val statusIcon = when (item.success) { | ||
| true -> "✓" | ||
| false -> "✗" | ||
| null -> "⏳" | ||
| } | ||
| val statusColor = when (item.success) { | ||
| true -> Color(0xFF4CAF50) // Green | ||
| false -> Color(0xFFF44336) // Red | ||
| null -> JewelTheme.globalColors.text.info | ||
| } | ||
| Text( | ||
| text = statusIcon, | ||
| style = JewelTheme.defaultTextStyle.copy(color = statusColor) | ||
| ) | ||
| Text( | ||
| text = "🔧 ${item.toolName}", | ||
| style = JewelTheme.defaultTextStyle.copy(fontWeight = FontWeight.Bold) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Hardcoded colors violate design token guidelines.
Lines 230-232 use hardcoded hex colors (Color(0xFF4CAF50), Color(0xFFF44336)). Per coding guidelines, always use design tokens for cross-platform consistency—use AutoDevColors from cc.unitmesh.devins.ui.compose.theme or define semantic color tokens.
Consider replacing with theme-based colors:
val statusColor = when (item.success) {
- true -> Color(0xFF4CAF50) // Green
- false -> Color(0xFFF44336) // Red
+ true -> JewelTheme.globalColors.text.success // Or AutoDevColors.success
+ false -> JewelTheme.globalColors.text.error // Or AutoDevColors.error
null -> JewelTheme.globalColors.text.info
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val statusIcon = when (item.success) { | |
| true -> "✓" | |
| false -> "✗" | |
| null -> "⏳" | |
| } | |
| val statusColor = when (item.success) { | |
| true -> Color(0xFF4CAF50) // Green | |
| false -> Color(0xFFF44336) // Red | |
| null -> JewelTheme.globalColors.text.info | |
| } | |
| Text( | |
| text = statusIcon, | |
| style = JewelTheme.defaultTextStyle.copy(color = statusColor) | |
| ) | |
| Text( | |
| text = "🔧 ${item.toolName}", | |
| style = JewelTheme.defaultTextStyle.copy(fontWeight = FontWeight.Bold) | |
| ) | |
| } | |
| val statusIcon = when (item.success) { | |
| true -> "✓" | |
| false -> "✗" | |
| null -> "⏳" | |
| } | |
| val statusColor = when (item.success) { | |
| true -> JewelTheme.globalColors.text.success // Or AutoDevColors.success | |
| false -> JewelTheme.globalColors.text.error // Or AutoDevColors.error | |
| null -> JewelTheme.globalColors.text.info | |
| } | |
| Text( | |
| text = statusIcon, | |
| style = JewelTheme.defaultTextStyle.copy(color = statusColor) | |
| ) | |
| Text( | |
| text = "🔧 ${item.toolName}", | |
| style = JewelTheme.defaultTextStyle.copy(fontWeight = FontWeight.Bold) | |
| ) |
🤖 Prompt for AI Agents
In mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt
around lines 224 to 242, the status color uses hardcoded hex values
(Color(0xFF4CAF50), Color(0xFFF44336)) which violate design token guidelines;
replace these with theme/design tokens from
cc.unitmesh.devins.ui.compose.theme.AutoDevColors (or define semantic color
tokens if missing). Import the theme package, map true -> AutoDevColors.success
(or equivalent semantic success token), false -> AutoDevColors.error (or
equivalent error token), and null -> JewelTheme.globalColors.text.info; update
the statusColor assignment to use those tokens so colors come from the design
system.
| @Composable | ||
| private fun ErrorBubble(message: String) { | ||
| Row( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| horizontalArrangement = Arrangement.Start | ||
| ) { | ||
| Box( | ||
| modifier = Modifier | ||
| .widthIn(max = 500.dp) | ||
| .background(Color(0x33F44336)) // Light red background | ||
| .padding(8.dp) | ||
| ) { | ||
| Text( | ||
| text = "❌ $message", | ||
| style = JewelTheme.defaultTextStyle.copy( | ||
| color = Color(0xFFF44336) | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded error colors should use design tokens.
ErrorBubble uses hardcoded red colors (Color(0x33F44336), Color(0xFFF44336)). Replace with semantic error colors from the theme system. Based on coding guidelines, hardcoded colors break cross-platform consistency.
Box(
modifier = Modifier
.widthIn(max = 500.dp)
- .background(Color(0x33F44336)) // Light red background
+ .background(JewelTheme.globalColors.text.error.copy(alpha = 0.2f))
.padding(8.dp)
) {
Text(
text = "❌ $message",
style = JewelTheme.defaultTextStyle.copy(
- color = Color(0xFFF44336)
+ color = JewelTheme.globalColors.text.error
)
)
}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/IdeaAgentApp.kt
around lines 268 to 288, the ErrorBubble composable uses hardcoded color
literals (Color(0x33F44336), Color(0xFFF44336)); replace these with the theme's
semantic error colors (e.g., use JewelTheme.colors.error for the text color and
the appropriate error container/variant from the theme such as
JewelTheme.colors.errorContainer or a semi-transparent/errorBackground token for
the Box background). Update the Box.background and Text.color to reference those
theme tokens so the component follows cross-platform design tokens instead of
hardcoded hex values.
| @Composable | ||
| private fun TaskCompleteBubble(item: JewelRenderer.TimelineItem.TaskCompleteItem) { | ||
| Row( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| horizontalArrangement = Arrangement.Center | ||
| ) { | ||
| Box( | ||
| modifier = Modifier | ||
| .background( | ||
| if (item.success) Color(0x334CAF50) else Color(0x33F44336) | ||
| ) | ||
| .padding(horizontal = 16.dp, vertical = 8.dp) | ||
| ) { | ||
| val icon = if (item.success) "✅" else "❌" | ||
| Text( | ||
| text = "$icon ${item.message} (${item.iterations} iterations)", | ||
| style = JewelTheme.defaultTextStyle.copy( | ||
| fontWeight = FontWeight.Bold | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Hardcoded success/error colors in TaskCompleteBubble.
Same issue: hardcoded colors at lines 299 and 303. Use semantic theme colors for success and error states.
Box(
modifier = Modifier
.background(
- if (item.success) Color(0x334CAF50) else Color(0x33F44336)
+ if (item.success)
+ JewelTheme.globalColors.text.success.copy(alpha = 0.2f)
+ else
+ JewelTheme.globalColors.text.error.copy(alpha = 0.2f)
)
.padding(horizontal = 16.dp, vertical = 8.dp)
)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/IdeaAgentApp.kt
around lines 290 to 312, the background colors for TaskCompleteBubble are
hardcoded (lines ~299 and ~303); replace them with the theme's semantic
success/error colors (e.g., use JewelTheme.colors.success /
JewelTheme.colors.error or the equivalent semantic properties in your theme) and
apply the desired alpha via .copy(alpha = ...) instead of raw hex; update any
imports if needed and keep the icon/text logic unchanged so the component uses
theme-provided colors for success and failure states.
| private suspend fun initializeCodingAgent(): CodingAgent { | ||
| val service = llmService | ||
| ?: throw IllegalStateException("LLM service is not configured. Please configure in ~/.autodev/config.yaml") | ||
|
|
||
| if (codingAgent == null || !agentInitialized) { | ||
| val toolConfig = try { | ||
| ConfigManager.loadToolConfig() | ||
| } catch (e: Exception) { | ||
| ToolConfigFile.default() | ||
| } | ||
|
|
||
| val mcpToolConfigService = McpToolConfigService(toolConfig) | ||
| val projectPath = project.basePath ?: System.getProperty("user.home") | ||
|
|
||
| codingAgent = CodingAgent( | ||
| projectPath = projectPath, | ||
| llmService = service, | ||
| maxIterations = 100, | ||
| renderer = renderer, | ||
| mcpToolConfigService = mcpToolConfigService, | ||
| enableLLMStreaming = true | ||
| ) | ||
| agentInitialized = true | ||
| } | ||
| return codingAgent!! | ||
| } |
There was a problem hiding this comment.
Potential race condition on agent initialization.
If sendMessage is called multiple times rapidly, concurrent coroutines could race on the codingAgent and agentInitialized checks, potentially creating multiple CodingAgent instances or operating on a stale reference.
Consider adding synchronization or using Mutex:
+ private val agentMutex = kotlinx.coroutines.sync.Mutex()
+
private suspend fun initializeCodingAgent(): CodingAgent {
+ return agentMutex.withLock {
val service = llmService
?: throw IllegalStateException("LLM service is not configured...")
if (codingAgent == null || !agentInitialized) {
// ... initialization logic
}
- return codingAgent!!
+ codingAgent!!
+ }
}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/IdeaAgentViewModel.kt
around lines 119-144 there is a race on codingAgent/agentInitialized when
initializeCodingAgent can be invoked concurrently; protect initialization with a
coroutine-safe lock (e.g., a kotlinx.coroutines.sync.Mutex) or use synchronized
to ensure only one coroutine creates and assigns codingAgent and sets
agentInitialized, and have other callers await the initialized instance; modify
the method to acquire the mutex before checking/creating the agent, perform the
creation while holding the lock, then release and return the non-null
codingAgent.
|
augment review |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| ) { | ||
| items(messages, key = { it.id }) { message -> | ||
| MessageBubble(message) | ||
| items(timeline, key = { it.timestamp }) { item -> |
There was a problem hiding this comment.
Using timestamp as the LazyColumn key can collide when multiple items are created within the same millisecond, which may cause state reuse or incorrect item updates. Consider a stable unique key (e.g., an incrementing id or UUID) for each TimelineItem.
🤖 Was this useful? React with 👍 or 👎
| } | ||
|
|
||
| val mcpToolConfigService = McpToolConfigService(toolConfig) | ||
| val projectPath = project.basePath ?: System.getProperty("user.home") |
There was a problem hiding this comment.
Defaulting projectPath to the user's home directory risks unintended file operations if project.basePath is null. Consider guarding against null by surfacing an error or requiring an explicit project path instead (also applies to the later usage in this file).
🤖 Was this useful? React with 👍 or 👎
| ) { | ||
| AgentType.entries.forEach { type -> | ||
| // Show only main agent types for cleaner UI | ||
| listOf(AgentType.CODING, AgentType.CODE_REVIEW, AgentType.KNOWLEDGE, AgentType.REMOTE).forEach { type -> |
There was a problem hiding this comment.
If the active agent type is LOCAL_CHAT (from config), the tab header won’t reflect a selected tab since it’s omitted from the list. Consider handling this case (e.g., map LOCAL_CHAT to CODING for selection or provide a tab) to avoid confusing UI state.
🤖 Was this useful? React with 👍 or 👎
Summary
This PR refactors
mpp-ideato depend onmpp-uifor shared components and configuration management, ensuring cross-platform consistency between IDEA plugin, CLI, and Desktop apps.Changes
Removed Duplicate Code
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/AgentType.kt- now usescc.unitmesh.agent.AgentTypefrom mpp-corempp-idea/src/main/kotlin/cc/unitmesh/devins/idea/model/LLMConfig.kt- now usesConfigManagerfrom mpp-uiUpdated IdeaAgentViewModel
ConfigManager.load()to read configuration from~/.autodev/config.yamlAutoDevConfigWrapper.getActiveModelConfig()for LLM configurationAutoDevConfigWrapper.getAgentType()for agent typereloadConfiguration()andsaveModelConfig()methodsisConfigValid()checkUpdated IdeaAgentApp
AgentType.getDisplayName()instead ofdisplayNamepropertyLOCAL_CHATto chat-based modesBuild Configuration
mpp-ui-jvmandmpp-core-jvmtargetssettings.gradle.ktsBenefits
~/.autodev/config.yaml) with CLI and Desktop appsAgentTypeandLLMConfigimplementationsConfigManagerfor reading/writing configurationTesting
./gradlew :mpp-idea:compileKotlinpasses./gradlew :mpp-idea:build -x testpassesPull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
New Features
Improvements
Removals
✏️ Tip: You can customize this high-level summary in your review settings.