refactor: extract shared renderer models and utils to mpp-core#22
refactor: extract shared renderer models and utils to mpp-core#22
Conversation
- Create RendererModels.kt with shared data models: - ToolCallInfo, TaskInfo, TaskStatus, ToolCallDisplayInfo - Create RendererUtils.kt with shared utility methods: - formatToolCallDisplay(), formatToolResultSummary() - parseParamsString(), toToolCallInfo() - Update ComposeRenderer to use shared models and utils - Update JewelRenderer to use shared models and utils - Add UI extensions for TaskStatus (icon, color) in TaskPanel.kt - Update related imports in AgentChatInterface, ToolItem, tests
WalkthroughCentralizes renderer data models and formatting utilities into cc.unitmesh.agent.render and updates renderers, UI components, IDEA plugin, and tests to consume shared TimelineItem/ToolCall/Task types and RendererUtils; adds id/timestamp generation and parsing/formatting utilities. Changes
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: 0
🧹 Nitpick comments (7)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt (1)
29-36: Consider returning the composable content instead of Unit.The
icon()extension returnsUnitwhile invokingIcon()composables inline. While this works for rendering, it's unconventional for a function named like a property accessor. Consider either:
- Renaming to
RenderIcon()to clarify it's a side-effecting composable, or- Refactoring to render inline at call sites via a composable lambda.
Current usage works correctly, so this is optional.
-@Composable -fun TaskStatus.icon(): Unit = when (this) { - TaskStatus.PLANNING -> Icon(Icons.Default.Create, null) - TaskStatus.WORKING -> Icon(Icons.Default.Build, null) - TaskStatus.COMPLETED -> Icon(Icons.Default.CheckCircle, null) - TaskStatus.BLOCKED -> Icon(Icons.Default.Warning, null) - TaskStatus.CANCELLED -> Icon(Icons.Default.Cancel, null) -} +@Composable +fun TaskStatus.Icon() { + when (this) { + TaskStatus.PLANNING -> Icon(Icons.Default.Create, null) + TaskStatus.WORKING -> Icon(Icons.Default.Build, null) + TaskStatus.COMPLETED -> Icon(Icons.Default.CheckCircle, null) + TaskStatus.BLOCKED -> Icon(Icons.Default.Warning, null) + TaskStatus.CANCELLED -> Icon(Icons.Default.Cancel, null) + } +}mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt (2)
92-103: Regex pattern has a potential edge case with unquoted values containing equals signs.The regex
(\w+)="([^"]*)"|\s*(\w+)=([^\s]+)correctly handles quoted values and simple unquoted values. However, unquoted values likepath=foo=barwould only capturefoo(up to the first=in the value portion since[^\s]+is greedy but stops at the next key match).This may be acceptable depending on expected input formats. If parameters can contain
=in values, they should be quoted.
60-62: Minor: Line count includes trailing empty line if output ends with newline.
output?.lines()?.sizewill count a trailing empty string if the output ends with\n. For example,"line1\nline2\n".lines()returns["line1", "line2", ""]with size 3.This is likely acceptable for display purposes, but if exact line counts matter, consider filtering empty trailing elements.
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (2)
13-26: Consider consolidatingToolCallInfoandToolCallDisplayInfoif they serve the same purpose.Both data classes have identical fields (
toolName,description,details). The only difference is thatToolCallInfo.detailshas a default value ofnullwhileToolCallDisplayInfo.detailsdoesn't. ThetoToolCallInfoutility function inRendererUtilssimply copies fields between them.If these truly serve distinct purposes (one for display formatting, one for state management), consider adding documentation to clarify. Otherwise, consolidating to a single type would reduce complexity.
49-52: Silent fallback toWORKINGmay hide input errors.The
fromStringfunction silently defaults toWORKINGwhen an unrecognized status string is provided. This is defensive, but could mask bugs where an invalid status is passed.Consider logging a warning when falling back, or returning a nullable type if the caller should handle unknown statuses explicitly.
fun fromString(status: String): TaskStatus { - return entries.find { it.name.equals(status, ignoreCase = true) } ?: WORKING + return entries.find { it.name.equals(status, ignoreCase = true) } + ?: run { + // Consider logging: "Unknown task status: $status, defaulting to WORKING" + WORKING + } }mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (2)
536-538: Minor inconsistency in delegation pattern compared to JewelRenderer.
ComposeRenderer.formatToolCallDisplayreturnsToolCallInfoby chainingtoToolCallInfo(formatToolCallDisplay(...)), whileJewelRenderer.formatToolCallDisplayreturnsToolCallDisplayInfodirectly and constructsToolCallInfoat the call site (line 209-213 in JewelRenderer).Both approaches work, but the inconsistency adds slight cognitive overhead when comparing the two renderers. Consider aligning the patterns if the implementations should mirror each other closely.
270-274: Timestamp source differs from sharedTaskInfodefault.Here,
timestampis set usingClock.System.now().toEpochMilliseconds(), but the sharedTaskInfomodel usesPlatform.getCurrentTimestamp()as its default. While these likely produce equivalent values, using different timestamp sources across the codebase could lead to subtle inconsistencies.Consider using
Platform.getCurrentTimestamp()consistently, or document thatClock.Systemis the Compose-standard approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt(1 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt(2 hunks)mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt(2 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ToolItem.kt(2 hunks)mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/compose/agent/test/AgentMessageListPreviews.kt(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Use
expect/actualfor platform-specific code in KMP projects (e.g., file I/O on JVM/JS/Wasm)
Files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ToolItem.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.ktmpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/compose/agent/test/AgentMessageListPreviews.ktmpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
{mpp-core,mpp-ui}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify @JsExport usage
Files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ToolItem.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.ktmpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/compose/agent/test/AgentMessageListPreviews.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt
🧠 Learnings (2)
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to **/src/{androidMain,desktopMain}/**/*.kt : For Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/compose/agent/test/AgentMessageListPreviews.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Use `cd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"` for running standalone JUnit 5 tests that do not require IntelliJ Platform
Applied to files:
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.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). (3)
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (13)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentChatInterface.kt (1)
13-13: LGTM!The import correctly references the shared
TaskStatusenum frommpp-core, aligning with the PR's goal of centralizing renderer models. Usage at lines 128-129 and 288-290 for filtering active tasks is consistent with the new shared type.mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt (1)
3-3: LGTM!The test correctly imports
TaskStatusfrom the sharedmpp-coremodule and the assertion at line 214 properly referencesTaskStatus.WORKINGinstead of the previously nestedJewelRenderer.TaskStatus.WORKING. This validates thatJewelRenderernow correctly integrates with the centralized renderer models.Also applies to: 214-214
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/compose/agent/test/AgentMessageListPreviews.kt (1)
9-9: LGTM!The import correctly references
ToolCallInfofrom the sharedmpp-coremodule, and the preview instantiation at lines 105-109 properly uses the external type. This aligns with the refactoring to centralize renderer models.Also applies to: 105-109
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ToolItem.kt (1)
36-36: LGTM!The import and function signature correctly use
ToolCallInfofrom the sharedmpp-coremodule. The component's property access at lines 300-301 (toolCall.toolName,toolCall.description) aligns with the centralized type definition.Also applies to: 269-269
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/TaskPanel.kt (3)
21-23: LGTM!The imports correctly reference shared types (
Platform,TaskInfo,TaskStatus) from thempp-coremodule. This properly separates the core data models from UI-specific extensions.
38-45: LGTM!The
colorextension property cleanly maps eachTaskStatusto its corresponding display color. The explicit color values provide clear visual distinction between task states.
226-226: Code is correct — Platform.getCurrentTimestamp() consistently returns milliseconds across all platforms.Both
Platform.getCurrentTimestamp()andtask.startTimeuse the same source and unit (milliseconds), so the division by 1000 correctly converts elapsed time to seconds. Verified across all platform implementations: Android usesSystem.currentTimeMillis(), JS usesDate().getTime(), and iOS usesClock.System.now().toEpochMilliseconds().mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
4-8: LGTM!The imports are correctly updated to use the shared models from
mpp-core. The separation of concerns is well-maintained with UI-specific code remaining in this file while common rendering logic is centralized.
455-462: LGTM!The delegation to
RendererUtilsis clean and follows the single-line expression syntax appropriately. This eliminates code duplication while maintaining the same behavior.mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (1)
31-37: LGTM!Good use of
Platform.getCurrentTimestamp()for cross-platform compatibility instead of JVM-specificSystem.currentTimeMillis()or Compose-specificClock.System. This aligns with the design decision mentioned in the PR objectives.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (2)
5-8: LGTM!Imports are correctly updated to use the shared models and utilities from
mpp-core. The refactoring maintains separation of concerns.
540-546: LGTM!The delegation to
RendererUtils.formatToolResultSummaryandRendererUtils.parseParamsStringis clean and correctly eliminates code duplication.mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt (1)
10-10: This utility object is only consumed by Kotlin code (ComposeRenderer in commonMain) and does not require@JsExport. TypeScript consumers access rendering through theJsCodingAgentRendererinterface andRendererFactory, which are already properly exported. RendererUtils is an internal Kotlin utility and should not be exposed to JavaScript.Likely an incorrect or invalid review comment.
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
- Move TimelineItem sealed class from JewelRenderer to RendererModels.kt - Move MessageRole enum to shared module - Update all references in mpp-idea components and tests - Maintain compatibility with ComposeRenderer's TimelineItem structure
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaMessageBubble.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTaskCompleteBubble.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTerminalOutputBubble.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.kt(3 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaToolCallBubble.kt(3 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt(8 hunks)mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt(8 hunks)mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.kt(7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Use
expect/actualfor platform-specific code in KMP projects (e.g., file I/O on JVM/JS/Wasm)
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaMessageBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaToolCallBubble.ktmpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTaskCompleteBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTerminalOutputBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.ktmpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt
{mpp-core,mpp-ui}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify @JsExport usage
Files:
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt
🧠 Learnings (3)
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to **/src/{androidMain,desktopMain}/**/*.kt : 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/components/timeline/IdeaMessageBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaToolCallBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTaskCompleteBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTerminalOutputBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to {mpp-ui/src/jsMain/typescript/**/*.{ts,tsx},**/src/{androidMain,desktopMain}/**/*.kt} : 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/timeline/IdeaToolCallBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTerminalOutputBubble.kt
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Use `cd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"` for running standalone JUnit 5 tests that do not require IntelliJ Platform
Applied to files:
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.ktmpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt
🧬 Code graph analysis (1)
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (2)
assertTrue(29-33)assertEquals(45-49)
⏰ 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: Test mpp-core (KMP)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (13)
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.kt (1)
3-4: LGTM! Clean migration to shared types.The test updates correctly migrate from
JewelRenderer.TimelineItem.*to the new publicTimelineItem.*API, maintaining all test assertions and logic.Also applies to: 108-108, 131-131, 146-146, 158-158, 204-206, 230-231
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt (1)
3-5: LGTM! Test suite updated correctly.All type references have been properly migrated to use the shared
TimelineItem,MessageRole, andTaskStatustypes from the public API.Also applies to: 58-60, 96-96, 116-116, 130-130, 140-140, 205-206, 216-216
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTerminalOutputBubble.kt (1)
22-22: LGTM! UI component migrated to shared types.The component signature correctly uses
TimelineItem.TerminalOutputItemfrom the public API.Also applies to: 37-37
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTaskCompleteBubble.kt (1)
11-11: LGTM! UI component migrated to shared types.The component signature correctly uses
TimelineItem.TaskCompleteItemfrom the public API.Also applies to: 24-24
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaToolCallBubble.kt (1)
20-20: LGTM! UI component migrated to shared types.The component signature correctly uses
TimelineItem.ToolCallItemfrom the public API. The explicit type annotation on the lambda parameter is a good practice for clarity.Also applies to: 42-42, 126-127
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaMessageBubble.kt (1)
8-8: LGTM! UI component migrated to shared types.The component signature correctly uses
MessageRolefrom the public API.Also applies to: 19-19, 23-23
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.kt (1)
12-12: LGTM! Timeline container migrated to shared types.All function signatures and pattern matching correctly use the public
TimelineItemhierarchy. The exhaustivewhenexpression properly handles all timeline item variants.Also applies to: 22-22, 54-54, 56-73
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (3)
4-9: Clean import migration to shared module.The imports are correctly migrated from local definitions to the shared
cc.unitmesh.agent.renderpackage. This aligns well with the PR's goal of extracting duplicated models and utilities.
393-400: Good encapsulation with delegating wrapper methods.Retaining these private wrapper methods that delegate to
RendererUtilsis a good pattern — it keeps the internal API stable and allows for future customization if needed, while still benefiting from the shared implementation.
186-186: Remove unnecessary concern aboutTaskInfodefaults—they are properly implemented.The
TaskInfoconstructor inmpp-corealready provides sensible defaults fortimestampandstartTime, both usingPlatform.getCurrentTimestamp(). SinceJewelRenderer.ktexplicitly passesSystem.currentTimeMillis()in the JVM-onlympp-ideamodule, this is acceptable and does not create any compatibility issues with the shared code.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt (3)
17-18: Correct import migration to shared types.The imports are properly updated to reference
MessageRoleandTimelineItemfrom the sharedcc.unitmesh.agent.renderpackage, consistent with the rest of the PR's refactoring.
606-756: Well-structured exhaustivewhenexpression.The
whenexpression properly handles allTimelineItemvariants (MessageItem,ToolCallItem,ErrorItem,TaskCompleteItem,TerminalOutputItem). If new variants are added to the sealed class in the future, the compiler will flag this as non-exhaustive, ensuring maintainability.The null-safe
item.output?.let { output -> ... }pattern on line 669 is a good improvement for clarity.
447-447: Parameter type correctly updated.The
timelineparameter type is correctly changed fromList<JewelRenderer.TimelineItem>toList<TimelineItem>, aligning with the shared model migration.
| sealed class TimelineItem( | ||
| open val timestamp: Long = Platform.getCurrentTimestamp(), | ||
| open val id: String = generateId() | ||
| ) { | ||
| /** | ||
| * Message item for user/assistant/system messages. | ||
| */ | ||
| data class MessageItem( | ||
| val role: MessageRole, | ||
| val content: String, | ||
| val tokenInfo: TokenInfo? = null, | ||
| override val timestamp: Long = Platform.getCurrentTimestamp(), | ||
| override val id: String = generateId() | ||
| ) : TimelineItem(timestamp, id) |
There was a problem hiding this comment.
Default parameters calling generateId() may cause unexpected behavior.
The sealed class constructor has default parameters that call generateId(). When creating data class instances via copy(), these defaults are not re-evaluated, which means copied items will retain the original id and timestamp unless explicitly overridden. This could lead to duplicate IDs in the timeline.
Consider documenting this behavior or ensuring callers explicitly set id when copying:
// Example of the issue:
val item1 = TimelineItem.MessageItem(role = MessageRole.USER, content = "Hello")
val item2 = item1.copy(content = "World") // item2.id == item1.id (unexpected!)If unique IDs per instance are required, callers must explicitly override:
val item2 = item1.copy(content = "World", id = TimelineItem.generateId())🤖 Prompt for AI Agents
In mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt
around lines 69-82, the sealed class uses default parameters that call
generateId()/getCurrentTimestamp(), which are not re-evaluated on data class
copy() and will cause copied items to retain the original id/timestamp; either
(A) remove the defaults from the base-class constructor and require explicit
id/timestamp at construction while providing named factory functions (e.g.,
createMessageItem(...)) that set generateId()/timestamp for new instances, or
(B) keep the public constructors but add a clear helper on each data class
(e.g., copyWithNewId or newInstanceFrom(copy:Boolean) that returns a copy with
id=generateId() and timestamp=now), and update docs/comments to state that
copy() preserves id/timestamp so callers must use the factory or helper when
they need a fresh id.
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt
Show resolved
Hide resolved
…core - Remove local TimelineItem sealed class from ComposeRenderer - Update imports to use shared TimelineItem from cc.unitmesh.agent.render - Replace CombinedToolItem with ToolCallItem - Replace ToolErrorItem with ErrorItem - Update MessageItem to handle nullable message field - Update toMessageMetadata and fromMessageMetadata methods - Add LiveTerminalItem handling in mpp-idea components - Update MessageRole imports to use cc.unitmesh.devins.llm.MessageRole
|
augment review |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (1)
167-170: Thread-safety concern with shared mutable counter - previously flagged.The
idCounteris accessed without synchronization, which can cause race conditions in concurrent scenarios. This was already identified in a previous review.
🧹 Nitpick comments (7)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (2)
18-31: Consider consolidatingToolCallInfoandToolCallDisplayInfo.Both data classes have identical fields (
toolName,description,details). The only difference is thatToolCallDisplayInfo.detailsis non-nullable whileToolCallInfo.detailsis nullable with a default. This duplication may cause maintenance burden and confusion about when to use which type.Consider keeping only
ToolCallInfoand makingdetailsnon-nullable (with empty string default if needed), or document the distinct use cases clearly.
36-42: Potential timestamp inconsistency when using defaults.Both
timestampandstartTimedefault toPlatform.getCurrentTimestamp(). Since these are evaluated separately at construction time, they may have slightly different values when using the default parameters, which could be confusing semantically (a task starting withstartTime != timestamp).Consider using a single timestamp reference:
data class TaskInfo( val taskName: String, val status: TaskStatus, val summary: String = "", - val timestamp: Long = Platform.getCurrentTimestamp(), - val startTime: Long = Platform.getCurrentTimestamp() + val timestamp: Long = Platform.getCurrentTimestamp(), + val startTime: Long = timestamp )mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.kt (1)
74-84: Placeholder for unsupportedLiveTerminalItemis reasonable but could be improved.The placeholder approach is fine for now, but consider:
- Using a distinct visual indicator (e.g., different bubble style) to show this is a live session placeholder rather than mimicking completed terminal output.
- The hardcoded
exitCode = 0might be misleading if the session actually failed.This is acceptable for the current scope since the comment clearly indicates "Live terminal not supported in IDEA yet."
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt (1)
605-784: ChatMessageItem correctly updated to shared TimelineItem variants (plus LiveTerminal placeholder)The new
whenbranches overTimelineItem.MessageItem,ToolCallItem,ErrorItem,TaskCompleteItem,TerminalOutputItem, andLiveTerminalItemalign with the shared model, and the use ofitem.role/item.contentfor messages and nullableitem.outputfor tools is sound. The LiveTerminal branch’s placeholder text clearly communicates that live terminals aren’t supported in this view.If you later add interactive live terminals here, this branch would be a good place to factor out into a shared composable with the main terminal UI.
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (3)
140-183: ToolCallItem construction and TaskInfo/TaskStatus updates look correctUsing
parseParamsStringandtoToolType()to derivefilePathandtoolType, then pushing an initialToolCallItemwithsuccess = nullmatches the previous “call-then-result” pattern and works well with the later update inrenderToolResult. Mapping thetask-boundarytool intoTaskInfoviaTaskStatus.fromStringis also straightforward.You might consider passing a pre-built
ToolCallInfofromRendererUtilsdirectly into_currentToolCallinstead of re‑constructing it, to avoid duplication and keep any future fields in sync automatically.
188-221: TaskInfo timestamp source differs from core time abstraction
updateTaskFromToolCallusesClock.System.now().toEpochMilliseconds()directly to update task timestamps, whereas other parts of the refactor mention a platform abstraction (Platform.getCurrentTimestamp()). Functionally this is fine (kotlinx-datetime is multiplatform), but it slightly diverges from the “single time source” goal.You may want to route task timestamps through the same platform abstraction used in core so tasks appear with a consistent notion of “now” across renderers and platforms.
Please confirm whether the core module now standardizes on a helper like
Platform.getCurrentTimestamp(); if so, aligning this code path would keep behavior consistent.
329-342: Iterations argument is currently ignored for TaskCompleteItem
renderFinalResultreceives aniterations: Intbut doesn’t thread it intoTaskCompleteItemorMessageMetadata(onlysuccessandmessageare persisted). This means iteration count is effectively discarded in this renderer.If you intend to display or persist iteration counts (e.g., UIs reading
TaskCompleteItemor metadata), consider:
- Adding/plumbing an
iterationsfield onTaskCompleteItem(and corresponding metadata), and- Passing
iterationshere when constructing both the timeline item and its metadata.
If iteration count is no longer meaningful, you might want to document that decision and clean up any UI that still referencesiterations.Also applies to: 544-550, 659-665
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaMessageBubble.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.kt(3 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt(2 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt(9 hunks)mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt(8 hunks)mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.kt(7 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentMessageList.kt(4 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt(13 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/toolwindow/remote/IdeaRemoteAgentViewModelTest.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Use
expect/actualfor platform-specific code in KMP projects (e.g., file I/O on JVM/JS/Wasm)
Files:
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaMessageBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentMessageList.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt
{mpp-core,mpp-ui}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify @JsExport usage
Files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentMessageList.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt
🧠 Learnings (2)
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Use `cd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"` for running standalone JUnit 5 tests that do not require IntelliJ Platform
Applied to files:
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to **/src/{androidMain,desktopMain}/**/*.kt : 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/components/timeline/IdeaMessageBubble.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt
🧬 Code graph analysis (1)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (3)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentMessageList.kt (1)
MessageItem(244-289)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/chat/MessageItem.kt (1)
MessageItem(18-58)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ToolCallItem.kt (1)
TerminalOutputItem(451-528)
⏰ 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: Test mpp-core (KMP)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (15)
mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt (2)
58-216: LGTM! Test updates are consistent with the refactoring.All type references have been correctly updated to use the shared models from
cc.unitmesh.agent.render:
TimelineItem.*variants (MessageItem, ToolCallItem, ErrorItem, TaskCompleteItem)TaskStatus.WORKINGMessageRole.USERThe test logic remains unchanged, and the refactoring is mechanically correct throughout all test methods.
3-5: No action needed. The imports are correct.MessageRoleis intentionally defined incc.unitmesh.devins.llmand reused byRendererModels.kt. The test file's import ofcc.unitmesh.devins.llm.MessageRoleon line 3 is consistent with how the shared models import it.Likely an incorrect or invalid review comment.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaMessageBubble.kt (1)
8-8: Clean migration to sharedMessageRoletype.The import and usage updates correctly migrate from
JewelRenderer.MessageRoleto the sharedMessageRoletype fromcc.unitmesh.devins.llm. The comparison logic remains unchanged.Also applies to: 19-19, 23-23
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/AgentMessageList.kt (3)
23-23: Good migration to sharedTimelineItemtype.The import and parameter type updates correctly align with the new shared
TimelineItemhierarchy frommpp-core.Also applies to: 163-163
169-178: FallbackMessageconstruction handles backward compatibility.The fallback construction of
MessagewhentimelineItem.messageis null provides a reasonable bridge for cases where onlyroleandcontentare available. This maintains compatibility with existing callers.
181-227: Exhaustive handling of allTimelineItemvariants.The
whenexpression correctly handles all sealed class variants (ToolCallItem,ErrorItem,TaskCompleteItem,TerminalOutputItem,LiveTerminalItem), ensuring compile-time safety when new variants are added.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/components/timeline/IdeaTimelineContent.kt (1)
12-12: Proper migration to sharedTimelineItemtype.The import and parameter type changes correctly align with the shared model hierarchy.
Also applies to: 22-22, 54-54
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
4-9: Clean import migration to shared types.The imports correctly reference the new shared types from
cc.unitmesh.agent.render, consolidating the model definitions.
393-400: Good delegation toRendererUtilsfor formatting logic.The private wrapper methods cleanly delegate to the shared utility functions. This maintains the existing API surface while consolidating the implementation.
These could be inlined directly at call sites for simplicity, but keeping them as private methods maintains readability and provides a single point of modification if customization is needed later.
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (1)
1-171: No changes needed. The RendererModels inmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.ktare internal domain models for Compose UI rendering and should not have@JsExportannotations. The codebase uses a separate export layer injsMain(e.g.,RendererExports.kt) that exports wrapper interfaces and adapters, while TypeScript consumers interact through string-based parameters rather than direct object references. These models are not consumed by the CLI/TypeScript layer.Likely an incorrect or invalid review comment.
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/knowledge/IdeaKnowledgeContent.kt (1)
17-18: TimelineItem migration in AIChatPanel looks consistentSwitching AIChatPanel to consume
List<TimelineItem>and importing the sharedTimelineItem/MessageRoletypes is consistent with the new core renderer models; no behavioral issues stand out here.Also applies to: 446-454
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (4)
5-11: RendererUtils integration keeps renderer-specific helpers thinWrapping
RendererUtils.formatToolCallDisplay,formatToolResultSummary, andparseParamsStringbehind small private helpers preserves the existingToolCallInfo/Map<String, String>shapes while centralizing logic in core, which is good for consistency across renderers.Because the shell flow relies on
ToolCallInfo.detailsstarting with"Executing: ", please double-check thatRendererUtils.formatToolCallDisplay+toToolCallInfostill produce that prefix for Shell tools sorenderToolResultcan continue to extract the command correctly.Also applies to: 466-476
223-321: Shell tool and generic tool result handling remain consistent with prior behaviorThe updated
renderToolResultcorrectly:
- Differentiates Shell tools (including live vs non-live sessions) using metadata.
- Replaces the pending
ToolCallItemwith aTerminalOutputItemfor non-live shell calls.- Updates the last pending
ToolCallItemin-place for non-shell tools, including DocQL stats and a boundedoutputstring while preserving full details infullOutput.
This matches the previous design while leveraging the sharedToolCallItem/DocQL models.Given the reliance on metadata keys like
"isLiveSession","live_exit_code", and"execution_time_ms", please confirm that upstream tool runners still populate these keys with the same spelling and semantics after the refactor.
345-348: ErrorItem usage and metadata round‑trip look symmetricSwitching to
ErrorItem(message = message)and mapping it to/fromTimelineItemType.TOOL_ERRORviataskMessageinMessageMetadatapreserves the previous semantics while using the shared error model. The get/load snapshot paths appear symmetric.Also applies to: 507-541, 652-656
496-505: Message/Tool/Terminal timeline ↔ message metadata round‑trip is coherentThe conversions in
toMessageMetadata,fromMessageMetadata,loadFromMessages, andgetTimelineSnapshot:
- Preserve token info for
MessageItem.- Serialize/restore full tool context for
ToolCallItem, including DocQL stats.- Treat terminal outputs as assistant messages with command/output/exitCode metadata.
- Skip persisting
LiveTerminalItem, which is reasonable for runtime-only views.
Overall, the new mappings look logically consistent with the shared models.Since
MessageItemnow carries both amessageand separaterole/content/timestampfields, please verify that all creation sites either provide amessageor that the fallback usingrole/content/timestampingetTimelineSnapshot()matches your intended persisted content.Also applies to: 572-592, 621-636, 640-649, 668-675, 687-702, 709-771
…h Android-specific CodeParser - Remove incorrect import of IosCodeParser from androidMain - Create AndroidCodeParser class with regex-based parsing (similar to iOS implementation) - Support parsing for Java, Kotlin, JavaScript, TypeScript, and Python - Fix CI build failure in kmp-test.yml workflow
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt (1)
173-215: Consider consolidating duplicate parsing logic.
parseJSNodesandparsePythonNodesare identical except for thelanguageparameter. Extract a shared helper to reduce duplication.-private fun parseJSNodes( - sourceCode: String, - filePath: String, - language: Language -): List<CodeNode> { - val nodes = mutableListOf<CodeNode>() - val lines = sourceCode.lines() - - val classPattern = Regex("""class\s+(\w+)""") - - for ((index, line) in lines.withIndex()) { - val currentLine = index + 1 - - classPattern.find(line)?.let { match -> - val name = match.groupValues[1] - nodes.add(createCodeNode(name, CodeElementType.CLASS, "", filePath, currentLine, language)) - } - } - - return nodes -} - -private fun parsePythonNodes( - sourceCode: String, - filePath: String, - language: Language -): List<CodeNode> { - val nodes = mutableListOf<CodeNode>() - val lines = sourceCode.lines() - - val classPattern = Regex("""class\s+(\w+)""") - - for ((index, line) in lines.withIndex()) { - val currentLine = index + 1 - - classPattern.find(line)?.let { match -> - val name = match.groupValues[1] - nodes.add(createCodeNode(name, CodeElementType.CLASS, "", filePath, currentLine, language)) - } - } - - return nodes -} +private fun parseSimpleClassNodes( + sourceCode: String, + filePath: String, + language: Language +): List<CodeNode> { + val classPattern = Regex("""class\s+(\w+)""") + return sourceCode.lines().mapIndexedNotNull { index, line -> + classPattern.find(line)?.let { match -> + createCodeNode(match.groupValues[1], CodeElementType.CLASS, "", filePath, index + 1, language) + } + } +}Then update the
parseNodesdispatch:Language.JAVASCRIPT, Language.TYPESCRIPT -> parseSimpleClassNodes(sourceCode, filePath, language) Language.PYTHON -> parseSimpleClassNodes(sourceCode, filePath, language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Use
expect/actualfor platform-specific code in KMP projects (e.g., file I/O on JVM/JS/Wasm)
Files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
{mpp-core,mpp-ui}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify @JsExport usage
Files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
**/src/{androidMain,desktopMain}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
For Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
{mpp-ui/src/jsMain/typescript/**/*.{ts,tsx},**/src/{androidMain,desktopMain}/**/*.kt}
📄 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-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.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). (3)
- GitHub Check: Build (223)
- GitHub Check: Build (241)
- GitHub Check: Test mpp-core (KMP)
🔇 Additional comments (4)
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt (4)
7-16: LGTM!Good use of the
actualkeyword for the platform-specific implementation, and the documentation clearly explains the Android limitation and suggests server-side parsing as an alternative. This aligns with KMP best practices.
144-171: LGTM for simplified parsing.The OOP node extraction correctly handles basic Java/Kotlin constructs. The regex
(class|interface|enum|object)\s+(\w+)will match even with preceding modifiers likedata class,sealed class, or visibility keywords, which is sufficient for this simplified implementation.
24-45: LGTM!The parsing workflow is well-structured with appropriate language dispatching and the pair-based return for nodes and relationships follows the interface contract.
47-69: LGTM!The
parseCodeGraphimplementation correctly aggregates nodes and relationships across multiple files and includes useful metadata for debugging/tracing.
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
Show resolved
Hide resolved
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
Show resolved
Hide resolved
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
Show resolved
Hide resolved
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
Show resolved
Hide resolved
…cking RendererModels.kt: - Replace mutable idCounter with random-based ID generation for thread-safety - Add documentation explaining copy() behavior with default parameters AndroidCodeParserFactory.kt: - Fix line number tracking in extractJvmImports, extractPythonImports, extractJsImports to use actual line numbers from match positions instead of sequential counters - Replace hashCode-based ID with deterministic composite ID (filePath:startLine:qualifiedName) - Add comment explaining the approximate endLine calculation for regex-based parsing
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (1)
61-108: Nice documentation of copy() semantics; consider helpers to make “new ID” copies easierThe KDoc around
TimelineItemand the example withMessageItem.copy(...)clearly explain the default-ID/copy behavior and address the earlier confusion about duplicate IDs.If you find yourself frequently needing “same content, new identity” clones, you might still want lightweight helpers, e.g.:
fun TimelineItem.MessageItem.withNewId(): TimelineItem.MessageItem = copy(id = TimelineItem.generateId())That would make the intended pattern obvious at call sites and reduce the chance of accidentally reusing IDs, while keeping the current constructors and documentation.
🧹 Nitpick comments (4)
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt (1)
84-148: Excellent fix for line number tracking.All three import extractors now correctly compute actual line numbers from match positions, addressing the previous review comments. The
content.substring(0, match.range.first).count { it == '\n' } + 1approach produces accurate 1-based line numbers.For future optimization (not urgent), consider precomputing a line offset map to avoid repeated substring operations when processing files with many imports.
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (3)
18-31: Consider collapsing ToolCallInfo and ToolCallDisplayInfo to avoid driftThese two data classes have identical fields (modulo a default value on
details), which makes future changes easy to forget in one place.If the separation is only conceptual (public vs internal), consider either:
- Making
ToolCallDisplayInfoaninternal typealiastoToolCallInfo, or- Keeping a single
ToolCallInfoand documenting its dual role.This would reduce duplication and keep the API surface simpler.
36-59: Tighten TaskInfo timestamp defaults and TaskStatus parsing semanticsTwo small nits here:
TaskInfo.timestampandstartTimeboth default toPlatform.getCurrentTimestamp(), but they’re evaluated separately. If they’re conceptually “createdAt” and “startedAt” and should usually be equal on creation, consider a small factory to computenowonce and pass it into both fields, keeping constructors for more advanced cases.
TaskStatus.fromStringmatches only on enumnameand silently falls back toWORKINGfor unknown values. If inputs can be user- or tool-provided strings like"Planning"(displayName), you may want to also matchdisplayName, or introduce anUNKNOWNstatus instead of defaulting toWORKING, to avoid misrepresenting task state.
168-175: Re-evaluate keeping platform-specific handles on a commonMain model
LiveTerminalItem.ptyHandle: Any?is necessarily platform-specific (JVM-only), but placing it directly on a commonMain data model can complicate serialization or reuse in non-JVM contexts.If you ever need to persist/serialize
TimelineItems or share them across processes, consider:
- Moving
ptyHandleinto anactualplatform-specific wrapper (e.g., JVM-only extension data), or- Keeping
LiveTerminalItemas a purely logical descriptor (ids, command, cwd) and tracking the PTY handle in a separate, platform-scoped map keyed bysessionId.Not urgent, but it will keep the core model cleaner and easier to serialize.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt(1 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Use
expect/actualfor platform-specific code in KMP projects (e.g., file I/O on JVM/JS/Wasm)
Files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt
{mpp-core,mpp-ui}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify @JsExport usage
Files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt
**/src/{androidMain,desktopMain}/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
For Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
{mpp-ui/src/jsMain/typescript/**/*.{ts,tsx},**/src/{androidMain,desktopMain}/**/*.kt}
📄 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-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt
🧠 Learnings (1)
📚 Learning: 2025-11-30T02:30:49.805Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to {mpp-core,mpp-ui}/**/*.kt : Check export first in mpp-core and mpp-ui; if some functions are not working well with CLI (TypeScript), verify JsExport usage
Applied to files:
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.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). (3)
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (6)
mpp-core/src/androidMain/kotlin/cc/unitmesh/agent/tool/impl/AndroidCodeParserFactory.kt (5)
7-16: Well-documented platform implementation.The documentation clearly explains the limitations of this simplified regex-based parser and suggests server-side parsing as an alternative for full TreeSitter functionality. The
actual funimplementation correctly provides the Android-specific CodeParser.
24-82: Clean implementation of core parsing methods.The routing logic appropriately delegates to language-specific parsers, and the metadata enrichment in
parseCodeGraphprovides useful context about the parsed code structure.
150-221: Appropriate language-specific parsing implementations.The node parsers correctly handle different language constructs. The OOP parser appropriately distinguishes between class, interface, enum, and object types, while the JS and Python parsers focus on class declarations.
228-256: Well-addressed previous feedback on ID generation and magic numbers.The composite ID format (
"$filePath:$startLine:$qualifiedName") eliminates collision risks, and the inline comment clearly explains whyendLine = startLine + 10is an approximation inherent to regex-based parsing.
258-261: Acceptable simplification for basic parsing.Returning an empty relationship list is reasonable for this regex-based parser. More sophisticated relationship extraction would require AST-level parsing.
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.kt (1)
177-188: ID generation change looks good; minor readability tweak for RandomSwitching from a shared mutable counter to
Platform.getCurrentTimestamp()+kotlin.random.Randomremoves the previous thread-safety concern and should be more robust under concurrency for this use case.For readability, you might consider making the intent explicit:
private val random = kotlin.random.Random.Default fun generateId(): String = "${Platform.getCurrentTimestamp()}-${random.nextInt(0, Int.MAX_VALUE)}"Functionally this is the same, but it’s a bit clearer to readers that you’re using the default pseudo-random generator instance rather than the type itself.
If you want to double-check behavior across KMP targets, please confirm in your Kotlin stdlib docs that
Random.Defaultis appropriate for concurrent use in your supported platforms/versions.
1 participant
Summary
Extract duplicated code between
JewelRendererandComposeRendererinto shared modules inmpp-core.Changes
New Files
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererModels.ktToolCallInfo- Tool call informationTaskInfo- Task information from task-boundary toolTaskStatus- Task status enum (PLANNING, WORKING, COMPLETED, BLOCKED, CANCELLED)ToolCallDisplayInfo- Tool call display informationmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.ktformatToolCallDisplay()- Format tool call for displayformatToolResultSummary()- Format tool result summaryparseParamsString()- Parse parameter string to maptoToolCallInfo()- Convert ToolCallDisplayInfo to ToolCallInfoUpdated Files
Design Decisions
Platform.getCurrentTimestamp()instead ofClock.System.now()for cross-platform compatibilityTesting
./gradlew :mpp-core:compileKotlinJvm- Passed./gradlew :mpp-ui:compileKotlinJvm- Passedcd mpp-idea && ../gradlew compileKotlin- Passedcd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"- PassedPull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
Refactor
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.