-
Notifications
You must be signed in to change notification settings - Fork 1
feat(plan): improve PlanSummaryBar UX and fix parameter passing #39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- PlanSummaryBar now expands upward instead of downward to avoid compressing content - Plan tool calls are no longer rendered in the message timeline (shown only in PlanSummaryBar) - Added renderToolCallWithParams method to pass parsed parameters directly - Fixed parameter parsing issues with complex values like planMarkdown containing JSON
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis change adds a parameter-map rendering path: the executor now passes parsed parameter maps to a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–70 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the PlanSummaryBar component UX and resolves parameter passing issues for plan tool calls by implementing direct parameter passing instead of string serialization/deserialization.
Key changes:
- Modified PlanSummaryBar to expand upward instead of downward to prevent layout compression
- Excluded plan tool calls from message timeline rendering to reduce visual duplication
- Introduced
renderToolCallWithParamsmethod to pass parsed parameters directly, avoiding serialization issues with complex values
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
PlanSummaryBar.kt |
Reordered UI components to enable upward expansion animation |
ComposeRenderer.kt |
Added parameter-based rendering methods and excluded plan tools from timeline |
JewelRenderer.kt |
Implemented parameter-based rendering and plan filtering for IDEA plugin |
RendererUtils.kt |
Extended parameter parsing to support JSON format |
CodingAgentRenderer.kt |
Added renderToolCallWithParams interface method with default implementation |
CodingAgentExecutor.kt |
Updated to use new parameter-based rendering method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| // Simple JSON parsing for flat objects with string values | ||
| val jsonContent = trimmed.substring(1, trimmed.length - 1) | ||
| // Match "key": "value" or "key": number patterns |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The complex regex pattern is difficult to understand and maintain. Consider adding an explanatory comment describing what each capture group matches (e.g., group 1: key, group 2: quoted string, group 3: number, group 4: unquoted word).
| // Match "key": "value" or "key": number patterns | |
| // Match "key": "value" or "key": number patterns | |
| // Regex capture groups: | |
| // group 1: key | |
| // group 2: quoted string value | |
| // group 3: number value | |
| // group 4: unquoted word value (e.g., true/false) |
| override fun renderToolCallWithParams(toolName: String, params: Map<String, Any>) { | ||
| // Convert params to string format for display | ||
| val paramsStr = params.entries.joinToString(" ") { (key, value) -> | ||
| "$key=\"$value\"" |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string conversion doesn't escape quotes in the value, which could break parameter parsing if a value contains quotes. This is the same issue the PR aims to fix. Consider either escaping quotes in the value or documenting that this display format is only for UI purposes and not for re-parsing.
| "$key=\"$value\"" | |
| val escapedValue = value.toString().replace("\"", "\\\"") | |
| "$key=\"$escapedValue\"" |
| override fun renderToolCallWithParams(toolName: String, params: Map<String, Any>) { | ||
| // Convert params to string format for display | ||
| val paramsStr = params.entries.joinToString(" ") { (key, value) -> | ||
| "$key=\"$value\"" |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The string conversion doesn't escape quotes in the value, which could break parameter parsing if a value contains quotes. Consider either escaping quotes in the value or documenting that this display format is only for UI purposes and not for re-parsing.
| "$key=\"$value\"" | |
| val escapedValue = value.toString().replace("\"", "\\\"") | |
| "$key=\"$escapedValue\"" |
| if (params.isNotEmpty()) { | ||
| return params | ||
| } | ||
| } catch (_: Exception) { |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Catching a broad Exception silently could hide unexpected errors. Consider catching more specific exceptions (e.g., parsing-related exceptions) or logging the exception for debugging purposes.
| } catch (_: Exception) { | |
| } catch (e: Exception) { | |
| println("Failed to parse params as JSON: ${e.message}") | |
| e.printStackTrace() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (9)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt (1)
96-119: JSON parsing has limited key support and silently swallows exceptions.
- The key pattern
\w+only matches[a-zA-Z0-9_], so keys like"plan-markdown"or"file.path"won't be parsed.- The
catch (_: Exception)silently discards parse errors. Consider logging at debug level for troubleshooting.nullJSON values are not handled and will be skipped.If the current key restrictions match your schema, this is acceptable. Otherwise:
- val jsonRegex = Regex(""""(\w+)"\s*:\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|(\d+(?:\.\d+)?)|(\w+))""") + // Support keys with hyphens and dots: [\w.-]+ + val jsonRegex = Regex(""""([\w.-]+)"\s*:\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|(\d+(?:\.\d+)?)|(\w+))""")mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/CodingAgentRenderer.kt (1)
22-28: Default implementation doesn't escape quotes in values.If a value contains quotes (e.g.,
"file with \"quotes\""), the generated string will be malformed:key="file with "quotes"".This is acceptable as a backward-compatibility fallback since implementations should override this method, but consider documenting this limitation.
fun renderToolCallWithParams(toolName: String, params: Map<String, Any>) { // Default implementation: convert to string format for backward compatibility val paramsStr = params.entries.joinToString(" ") { (key, value) -> - "$key=\"$value\"" + val escaped = value.toString().replace("\"", "\\\"") + "$key=\"$escaped\"" } renderToolCall(toolName, paramsStr) }mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/plan/PlanSummaryBar.kt (2)
192-223: Hardcoded colors violate coding guidelines.Per coding guidelines for
**/compose/**/*.kt: "DO NOT hardcode colors (e.g.,Color(0xFF...)or#hex). Always use design tokens for consistency across platforms."Multiple hardcoded colors are used:
Color(0xFF4CAF50),Color(0xFFF44336),Color(0xFF2196F3),Color(0xFFFF9800).Consider using
MaterialTheme.colorSchemeor defining semantic color tokens. As per coding guidelines.TaskStatus.COMPLETED -> Icon( imageVector = Icons.Default.CheckCircle, contentDescription = "Completed", modifier = Modifier.size(18.dp), - tint = Color(0xFF4CAF50) + tint = MaterialTheme.colorScheme.primary // or define a semantic success color ) TaskStatus.FAILED -> Icon( imageVector = Icons.Default.Error, contentDescription = "Failed", modifier = Modifier.size(18.dp), - tint = Color(0xFFF44336) + tint = MaterialTheme.colorScheme.error )
243-249: Additional hardcoded colors in progress indicator.Same issue:
Color(0xFF4CAF50),Color(0xFFF44336),Color(0xFF2196F3)should use design tokens. As per coding guidelines.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
139-187: Code duplication betweenrenderToolCallandrenderToolCallWithParams.Both methods have nearly identical logic for handling
task-boundaryandplantools, then delegating torenderToolCallInternal. Consider consolidating by havingrenderToolCalldelegate torenderToolCallWithParams:override fun renderToolCall(toolName: String, paramsStr: String) { - val toolInfo = formatToolCallDisplay(toolName, paramsStr) val params = parseParamsString(paramsStr) - val toolType = toolName.toToolType() - - // Handle task-boundary tool - update task list - if (toolName == "task-boundary") { - updateTaskFromToolCall(params) - } - - // Handle plan management tool - update plan state - if (toolName == "plan") { - updatePlanFromToolCall(params) - // Skip rendering plan tool to timeline - it's shown in PlanSummaryBar - return - } - - renderToolCallInternal(toolName, toolInfo, params, paramsStr, toolType) + // Delegate to params version, converting String values to Any + renderToolCallWithParams(toolName, params.mapValues { it.value as Any }) }This ensures consistent behavior and reduces maintenance burden when logic changes.
163-167: Same quote escaping issue as interface default.Values containing quotes will produce malformed output. Consider escaping:
val paramsStr = params.entries.joinToString(" ") { (key, value) -> - "$key=\"$value\"" + val escaped = value.toString().replace("\"", "\\\"") + "$key=\"$escaped\"" }mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (3)
165-203: Plan tool no longer in timeline – consider history implicationsThe early return for
toolName == "plan"inrenderToolCalland theif (toolName != "plan")guard inrenderToolCallWithParamssuccessfully keep plan tool calls out of the timeline, matching the goal of showing them only inPlanSummaryBar. However, this also means:
getTimelineSnapshot()will no longer contain any representation of plan tool calls.loadFromMessages()has no way to reconstruct_currentPlan, so reloaded sessions will lose plan context entirely.If you care about preserving plan context across reloads, consider persisting either the parsed
AgentPlan(or at least the latestplanMarkdown) intoMessageMetadataand restoring_currentPlaninloadFromMessages(), or introduce a non-user-visible metadata-only entry for plan actions.
174-214: Map-based tool-call path is sound; stringification could be made more robust laterThe new
renderToolCallWithParamsplusrenderToolCallInternalprovides a clean separation:
- Typed
Map<String, Any>is used for logic-sensitive paths (updatePlanFromToolCallWithAnyParams).- A
Map<String, String>andparamsStrare derived only for display and existing utilities (formatToolCallDisplay, timeline, etc.).- Task-boundary handling remains consistent with the string-based
renderToolCall.Two minor, non-blocking observations:
paramsStris built withvalue.toString()and quoted askey="value". For complex values (nested maps, long markdown, or values containing quotes/newlines), this is “best effort” and may not round-trip perfectly throughparseParamsString, though it’s only used for display at the moment.- There’s some duplication between the string and map code paths (task-boundary/plan branching). If this grows further, you might want a small shared helper that takes a normalized
Map<String, String>and an optional rawMap<String, Any>for plan/task updates.Functionally this is fine as-is.
285-316: Plan state helpers centralize logic correctly; numeric parsing is conservativeUnifying plan handling through
updatePlanFromToolCall(string params) andupdatePlanFromToolCallWithAnyParams(typed params) intoupdatePlanStateis a solid refactor:
actionis normalized with.uppercase()and handled in a singlewhen.taskIndex/stepIndexare nullable and explicitly checked before mutating the plan, preventing accidental out-of-bounds or NPEs.- The 1-based index checks and recomposition trigger via
plan.copy(updatedAt = ...)look consistent.Minor behavioral notes (not blockers):
- For
Map<String, Any>,taskIndex/stepIndexuseis Number -> v.toInt()andis String -> v.toIntOrNull(). Non-integerNumbers (e.g.,1.5) will be truncated; numeric strings like"1.0"will now fail to parse where they might previously have been accepted. If your tool protocol might send such values, you could add a small helper that handles decimal strings more gracefully or logs unexpected formats.- When
planMarkdownis missing or blank onCREATE/UPDATE, the call is a no-op, which seems safe but might be worth logging if this is unexpected.Overall, the centralization and null-guarding look good.
Also applies to: 321-359
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/executor/CodingAgentExecutor.kt(1 hunks)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/CodingAgentRenderer.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(4 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt(7 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/plan/PlanSummaryBar.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/CodingAgentRenderer.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/executor/CodingAgentExecutor.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/plan/PlanSummaryBar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
**/compose/**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
In Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/plan/PlanSummaryBar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt
{**/compose/**/*.kt,mpp-ui/src/jsMain/typescript/**/*.{ts,tsx}}
📄 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-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/plan/PlanSummaryBar.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt
**/idea/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/idea/**/*.kt: For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer:JewelFlags.useCustomPopupRenderer = trueinIdeaAgentToolWindowFactory
For popup/dropdown menus in IntelliJ plugins, use Jewel'sPopupMenuinstead ofandroidx.compose.ui.window.Popup
For dialogs in IntelliJ plugins, use IntelliJ'sDialogWrapperwithorg.jetbrains.jewel.bridge.composeinstead ofandroidx.compose.ui.window.Dialog
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
🧠 Learnings (2)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: For `mpp-idea` plugin tests, run with: `cd mpp-idea && ../gradlew test --tests "cc.unitmesh.devins.idea.renderer.JewelRendererTest"`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
🧬 Code graph analysis (3)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/CodingAgentRenderer.kt (6)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (1)
renderToolCall(139-157)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (1)
renderToolCall(151-172)mpp-core/src/jsMain/kotlin/cc/unitmesh/agent/RendererExports.kt (2)
renderToolCall(51-51)renderToolCall(100-102)mpp-ui/src/jsMain/typescript/agents/render/CliRenderer.ts (1)
renderToolCall(155-161)mpp-ui/src/jvmMain/kotlin/cc/unitmesh/server/cli/CodingCli.kt (1)
renderToolCall(190-198)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/DefaultCodingAgentRenderer.kt (1)
renderToolCall(45-48)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt (1)
formatToolCallDisplay(15-50)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (5)
renderToolCallInternal(208-245)formatToolCallDisplay(721-723)updateTaskFromToolCall(250-283)updatePlanFromToolCallWithAnyParams(301-316)updatePlanState(321-359)mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt (1)
formatToolCallDisplay(15-50)
⏰ 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). (4)
- GitHub Check: Agent
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (7)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/executor/CodingAgentExecutor.kt (1)
227-229: LGTM!The change correctly passes the parsed parameter map directly to the renderer, avoiding string assembly/parsing issues with complex values like
planMarkdown. The comment clearly documents the rationale.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/plan/PlanSummaryBar.kt (1)
73-94: LGTM on layout restructuring!The reordering correctly places expanded content above the header, and the animation parameters (
expandFrom = Alignment.Bottom,shrinkTowards = Alignment.Bottom) ensure smooth upward expansion. This achieves the PR objective of avoiding layout compression.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
284-299: LGTM onupdatePlanFromToolCallWithAnyParams.The type handling for
taskIndexandstepIndexcorrectly handles bothNumberandStringtypes, matching the implementation inComposeRenderer. This ensures consistent behavior across renderers.
304-342: LGTM onupdatePlanStaterefactoring.The method now accepts explicit
taskIndexandstepIndexparameters, eliminating redundant parsing and enabling both string-param and any-param callers to share the same logic.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (3)
123-147: Streaming end ordering and token attachment look correctSaving
finalContentand_lastMessageTokenInfobefore clearing streaming state, then zeroing_currentStreamingOutput,_isProcessing, and_lastMessageTokenInfoand finally appending aMessageItemwith the capturedtokenInfois a good fix for the “double progress bar” issue while preserving accurate token accounting. No issues spotted here.
384-387: Clearing_currentToolCallbefore results resolves double-progress without breaking shell behaviorIn both the shell and non-shell branches of
renderToolResult,_currentToolCallis now cleared before appending the result item, which prevents simultaneous display of a “current tool call” and its completed result (the double progress bar issue).In the shell branch, the command string is derived from
_currentToolCall?.detailsbefore clearing, so this ordering preserves existing behavior while fixing the overlap. The non-shell branch’s logic for updating the lastToolCallItemremains unchanged aside from the earlier clear, which is safe because it only uses the timeline state, not_currentToolCall.Also applies to: 417-420
701-719: Forced stop behavior remains consistent with new streaming end semantics
forceStop()still promotes any non-empty streaming buffer into a final assistant message (with an “[Interrupted]” suffix) before clearing_isProcessing,_currentStreamingOutput, and_currentToolCall. This is compatible with the updatedrenderLLMResponseEndbehavior and ensures partial outputs are not silently discarded when a run is cancelled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review completed. 1 suggestion posted.
Comment augment review to trigger a new review at any time.
|
|
||
| // IMPORTANT: Clear currentToolCall FIRST to avoid showing both | ||
| // CurrentToolCallItem and the result item simultaneously (double progress bar issue) | ||
| _currentToolCall = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small correctness issue nearby: in renderToolResult, we read metadata["live_exit_code"], but ToolOrchestrator populates "exit_code" (and JewelRenderer uses that key). Consider switching to metadata["exit_code"] to ensure accurate exit code handling (see around line ~372 in this file).
🤖 Was this useful? React with 👍 or 👎
Revise agent template to require codebase exploration before plan creation, add concrete examples, and update planning guidelines for both English and Chinese. Also update DevInEditorInput to use transparent text and default keyboard behavior.
Refine minHeight, maxHeight, and padding for iOS to improve input usability and avoid keyboard constraint issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mpp-ios/build-ios-app.sh (1)
425-426: Remove outdated reference toios-deploy.Line 426 suggests using
ios-deployas a next step, but this script replacedios-deploywithdevicectl. Update the messaging to reflect the new workflow or remove the outdated reference.if [ "$TARGET_TYPE" = "device" ]; then - echo -e " 运行应用: 打开 Xcode 或安装 ios-deploy" + echo -e " 运行应用: 再次运行脚本使用 --run 选项" else
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt(2 hunks)mpp-ios/build-ios-app.sh(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.ktmpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt
**/compose/**/*.{kt,kts}
📄 CodeRabbit inference engine (AGENTS.md)
In Compose (Desktop/Android), use
AutoDevColorsfromcc.unitmesh.devins.ui.compose.themeorMaterialTheme.colorScheme
Files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt
{**/compose/**/*.kt,mpp-ui/src/jsMain/typescript/**/*.{ts,tsx}}
📄 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-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt
🧠 Learnings (3)
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/compose/**/*.{kt,kts} : In Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to {**/compose/**/*.kt,mpp-ui/src/jsMain/typescript/**/*.{ts,tsx}} : DO NOT hardcode colors (e.g., `Color(0xFF...)` or `#hex`). Always use design tokens for consistency across platforms
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For dialogs in IntelliJ plugins, use IntelliJ's `DialogWrapper` with `org.jetbrains.jewel.bridge.compose` instead of `androidx.compose.ui.window.Dialog`
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.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 (223)
- GitHub Check: Build (241)
🔇 Additional comments (11)
mpp-ios/build-ios-app.sh (1)
375-380: Clarify error handling for app launch failure.When
devicectl device process launchfails (line 375), the script logs a warning about trusting the developer certificate but does not exit. In contrast, the app installation failure (line 364) callsexit 1. Confirm whether launch failure should be fatal or advisory—the current behavior allows the script to complete with partial success.mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/CodingAgentTemplate.kt (2)
39-108: Exploration-first + plan rules are clear and consistent with /plan usageThe new exploration-before-planning guidance, plan action list, update rules, and example
/planpayload are all coherent, consistent with the later “One Tool Per Response” section, and align with the intendedCREATE/COMPLETE_STEPbehavior. No changes needed.
227-296: ZH instructions accurately mirror EN behaviorThe Chinese “先探索,后计划” section correctly reflects the English semantics (two phases, minimum exploration, plan actions, update rules, example workflow, and
/planJSON example). EN/ZH remain in sync; nothing critical to adjust.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/editor/DevInEditorInput.kt (8)
15-15: LGTM!The import additions are necessary and properly used in the code (Color.Transparent at line 528 and LocalFocusManager.current at line 142).
Also applies to: 18-18
154-169: Verify iOS keyboard behavior with fixed heights.The change from flexible to fixed min/max heights is significant. While the iOS standard touch target (44dp) is correct, fixed height constraints might cause issues with iOS keyboard handling, especially on smaller devices or in landscape mode.
Test on various iOS devices to ensure:
- Keyboard doesn't overlap or compress the input field
- Input remains visible when keyboard is shown
- Scrolling behavior works correctly with fixed heights
171-175: LGTM!Platform-specific padding values are appropriate, with iOS using smaller padding (10dp) to match platform conventions and touch targets.
505-505: LGTM!The heightIn modifier correctly applies the platform-specific min/max height constraints calculated earlier.
527-528: Acceptable use of Color.Transparent for technical rendering.Using
Color.Transparentto prevent text ghosting is a valid technical approach, where the actual input is transparent and only the highlighted overlay is visible. While the coding guideline discourages hardcoded colors,Color.Transparentis a utility value rather than a design token, making this an acceptable exception.As per coding guidelines: Design tokens should be used for visual colors, but transparent is a rendering technique.
533-534: Verify mobile keyboard behavior after removing KeyboardOptions/KeyboardActions.Removing
KeyboardOptionsandKeyboardActionsis a significant behavioral change. The file documentation (line 62) mentions "IME-aware keyboard handling (ImeAction.Send on mobile)", but these options are now removed. Verify that:
- Mobile keyboard still displays the correct action button (e.g., "Send" instead of "Enter")
- Keyboard dismissal works correctly on mobile
- The change doesn't conflict with the documented mobile-friendly improvements
Test on Android and iOS devices to ensure the keyboard UX remains functional without explicit KeyboardOptions configuration.
570-571: LGTM!The decorationBox simplification correctly implements the transparent input approach. The actual input field is now transparent (line 528), rendering only the cursor and selection, while the highlighted text is displayed in a separate layer (lines 543-554). This prevents text ghosting from overlapping renders.
525-525: Remove or align font family usage across platforms.Replacing
getUtf8FontFamily()withFontFamily.Monospacebreaks UTF-8 text rendering on WASM. The custom function returnsNotoSansSC_Regularfont on WASM to support UTF-8 and CJK characters, while other platforms fall back toFontFamily.Default. On WASM,FontFamily.Monospacewill not have this font loaded, causing text input with UTF-8 characters to render incorrectly. Either restoregetUtf8FontFamily()or ensureFontFamily.Monospaceincludes UTF-8 support on WASM.⛔ Skipped due to learnings
Learnt from: CR Repo: phodal/auto-dev-sketch PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-02T00:20:34.480Z Learning: Applies to **/*.kt : For WASM platform, avoid using emoji and UTF-8 in code
|
|
||
| # 获取连接的设备 ID (使用 devicectl) | ||
| # UUID 格式: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX | ||
| DEVICE_UUID=$(xcrun devicectl list devices 2>&1 | grep "connected" | grep -oE "[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}" | head -n 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n mpp-ios/build-ios-app.sh | sed -n '340,380p'Repository: phodal/auto-dev-sketch
Length of output: 1946
🏁 Script executed:
cat -n mpp-ios/build-ios-app.sh | sed -n '1,50p'Repository: phodal/auto-dev-sketch
Length of output: 1566
🏁 Script executed:
grep -n "APP_PATH" mpp-ios/build-ios-app.sh | head -20Repository: phodal/auto-dev-sketch
Length of output: 505
🏁 Script executed:
cat -n mpp-ios/build-ios-app.sh | sed -n '420,430p'Repository: phodal/auto-dev-sketch
Length of output: 522
🌐 Web query:
xcrun devicectl device install app command syntax documentation
💡 Result:
Syntax (example)
xcrun devicectl device install app --device <path/to/App.ipa>
Notes
- Use the device UDID from xcrun devicectl list devices. [1]
- Accepts .ipa (or .app/.xcarchive as appropriate); requires Xcode 15+ / iOS 17+ device support. [1][2]
- Common companion commands: xcrun devicectl device process launch --device . [1][3]
Sources
[1] Lauerman — Hand-making an iOS App (devicectl usage).
[2] SweetPad docs (examples showing devicectl install syntax).
[3] react-native-community issue demonstrating typical xcrun devicectl args.
🌐 Web query:
xcrun devicectl list devices output format UUID uppercase lowercase
💡 Result:
Short answer: devicectl emits hyphenated UUIDs (e.g. 00001234-0001-234A-3C03-401E) in JSON but letter case is not guaranteed — you’ll see both uppercase and lowercase hex in examples. Treat UUIDs case‑insensitively (or normalize to lowercase) when matching. [1][2]
Sources:
- devicectl JSON examples (shows "udid"/"identifier" with uppercase hex). [1]
- simctl example showing hyphenated UDID with uppercase hex (UUID hex is case‑insensitive). [2]
References:
[1] Flutter tests using devicectl JSON examples. (flutter.googlesource.com)
[2] simctl sample output (hyphenated UDID). (lobehub.com)
Citations:
- 1: https://flutter.googlesource.com/mirrors/flutter/%2B/refs/tags/3.15.0-0.0.pre/packages/flutter_tools/test/general.shard/ios/core_devices_test.dart?utm_source=openai
- 2: https://lobehub.com/mcp/devyhan-xcode-mcp?utm_source=openai
Fix UUID extraction regex to handle mixed-case hex digits.
Line 349's UUID extraction regex [0-9A-F] only matches uppercase hex, but xcrun devicectl list devices can output UUIDs with mixed or lowercase hex characters. This will cause the device detection to fail silently when devicectl returns lowercase UUIDs.
Change the regex to [0-9A-Fa-f] to match both cases:
DEVICE_UUID=$(xcrun devicectl list devices 2>&1 | grep "connected" | grep -oE "[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12}" | head -n 1)
Also, line 426 still references ios-deploy in the device setup instructions, but this command has been replaced by devicectl in the script. Update the message to reflect the new workflow.
🤖 Prompt for AI Agents
In mpp-ios/build-ios-app.sh around line 349, the UUID extraction regex only
matches uppercase hex ([0-9A-F]) so it can miss mixed- or lowercase UUIDs;
update the regex to accept both cases (use [0-9A-Fa-f] for all five groups) so
DEVICE_UUID is extracted reliably; additionally, update the device setup /
troubleshooting message later in the script (around line 426) to replace
references to the deprecated `ios-deploy` command with the current `devicectl`
workflow so the instructions match the script behavior.
Removed the linear progress indicator when content is not complete in SketchRenderer.
Summary
This PR improves the PlanSummaryBar component UX and fixes parameter passing issues for plan tool calls.
Changes
1. PlanSummaryBar expands upward
expandFrom = Alignment.BottomandshrinkTowards = Alignment.Bottomfor smooth animation2. Plan tool calls not rendered in timeline
ComposeRendererandJewelRenderer3. Fixed parameter passing for plan tool
renderToolCallWithParams(toolName: String, params: Map<String, Any>)method toCodingAgentRendererinterfaceplanMarkdowncontaining JSON or special charactersupdatePlanFromToolCallWithAnyParamsmethod to handleMap<String, Any>with proper type conversionFiles Changed
mpp-core/.../CodingAgentExecutor.kt- UserenderToolCallWithParamsinstead ofrenderToolCallmpp-core/.../CodingAgentRenderer.kt- AddrenderToolCallWithParamsinterface methodmpp-core/.../RendererUtils.kt- Add JSON format support inparseParamsStringmpp-idea/.../JewelRenderer.kt- Implement new methods and skip plan renderingmpp-ui/.../ComposeRenderer.kt- Implement new methods and skip plan renderingmpp-ui/.../PlanSummaryBar.kt- Change expansion direction to upwardTesting
Pull Request opened by Augment Code with guidance from the PR author
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.