Skip to content

Commit 42f6ba7

Browse files
phodalphodal
committed
feat(plan): improve PlanSummaryBar UX and fix parameter passing
- 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
1 parent 0e8a480 commit 42f6ba7

File tree

6 files changed

+237
-35
lines changed

6 files changed

+237
-35
lines changed

‎mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/executor/CodingAgentExecutor.kt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,10 @@ class CodingAgentExecutor(
223223
for (toolCall in toolsToExecute) {
224224
val toolName = toolCall.toolName
225225
val params = toolCall.params.mapValues { it.value as Any }
226-
val paramsStr = params.entries.joinToString(" ") { (key, value) ->
227-
"$key=\"$value\""
228-
}
229226

230-
renderer.renderToolCall(toolName, paramsStr)
227+
// Use renderToolCallWithParams to pass parsed params directly
228+
// This avoids string parsing issues with complex values like planMarkdown
229+
renderer.renderToolCallWithParams(toolName, params)
231230

232231
val executionContext = OrchestratorContext(
233232
workingDirectory = projectPath,

‎mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/CodingAgentRenderer.kt‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,22 @@ interface CodingAgentRenderer {
1111
fun renderLLMResponseEnd()
1212

1313
fun renderToolCall(toolName: String, paramsStr: String)
14+
15+
/**
16+
* Render a tool call with parsed parameters.
17+
* This is the preferred method as it avoids string parsing issues with complex values.
18+
*
19+
* @param toolName The name of the tool being called
20+
* @param params The parsed parameters map
21+
*/
22+
fun renderToolCallWithParams(toolName: String, params: Map<String, Any>) {
23+
// Default implementation: convert to string format for backward compatibility
24+
val paramsStr = params.entries.joinToString(" ") { (key, value) ->
25+
"$key=\"$value\""
26+
}
27+
renderToolCall(toolName, paramsStr)
28+
}
29+
1430
fun renderToolResult(
1531
toolName: String,
1632
success: Boolean,

‎mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/RendererUtils.kt‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,38 @@ object RendererUtils {
8787

8888
/**
8989
* Parse parameter string into a map.
90-
* Handles both quoted and unquoted values.
90+
* Handles both JSON format and key=value format.
9191
*/
9292
fun parseParamsString(paramsStr: String): Map<String, String> {
9393
val params = mutableMapOf<String, String>()
94+
val trimmed = paramsStr.trim()
95+
96+
// Try JSON format first (starts with { and ends with })
97+
if (trimmed.startsWith("{") && trimmed.endsWith("}")) {
98+
try {
99+
// Simple JSON parsing for flat objects with string values
100+
val jsonContent = trimmed.substring(1, trimmed.length - 1)
101+
// Match "key": "value" or "key": number patterns
102+
val jsonRegex = Regex(""""(\w+)"\s*:\s*(?:"([^"\\]*(?:\\.[^"\\]*)*)"|(\d+(?:\.\d+)?)|(\w+))""")
103+
jsonRegex.findAll(jsonContent).forEach { match ->
104+
val key = match.groups[1]?.value
105+
// Value can be: quoted string, number, or unquoted word (like true/false)
106+
val value = match.groups[2]?.value
107+
?: match.groups[3]?.value
108+
?: match.groups[4]?.value
109+
if (key != null && value != null) {
110+
params[key] = value
111+
}
112+
}
113+
if (params.isNotEmpty()) {
114+
return params
115+
}
116+
} catch (_: Exception) {
117+
// Fall through to key=value parsing
118+
}
119+
}
120+
121+
// Fallback to key=value format
94122
val regex = Regex("""(\w+)="([^"]*)"|\s*(\w+)=([^\s]+)""")
95123
regex.findAll(paramsStr).forEach { match ->
96124
val key = match.groups[1]?.value ?: match.groups[3]?.value

‎mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt‎

Lines changed: 79 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import cc.unitmesh.agent.render.RendererUtils
99
import cc.unitmesh.agent.render.TaskInfo
1010
import cc.unitmesh.agent.render.TaskStatus
1111
import cc.unitmesh.agent.render.TimelineItem
12+
import cc.unitmesh.agent.render.ToolCallDisplayInfo
1213
import cc.unitmesh.agent.render.ToolCallInfo
1314
import cc.unitmesh.agent.tool.ToolType
1415
import cc.unitmesh.agent.tool.toToolType
@@ -148,8 +149,53 @@ class JewelRenderer : BaseRenderer() {
148149
// Handle plan management tool - update plan state
149150
if (toolName == "plan") {
150151
updatePlanFromToolCall(params)
152+
// Skip rendering plan tool to timeline - it's shown in PlanSummaryBar
153+
return
154+
}
155+
156+
renderToolCallInternal(toolName, toolInfo, params, paramsStr, toolType)
157+
}
158+
159+
/**
160+
* Render a tool call with parsed parameters.
161+
* This is the preferred method as it avoids string parsing issues with complex values.
162+
*/
163+
override fun renderToolCallWithParams(toolName: String, params: Map<String, Any>) {
164+
// Convert params to string format for display
165+
val paramsStr = params.entries.joinToString(" ") { (key, value) ->
166+
"$key=\"$value\""
151167
}
168+
val toolInfo = formatToolCallDisplay(toolName, paramsStr)
169+
val toolType = toolName.toToolType()
170+
171+
// Convert Map<String, Any> to Map<String, String> for internal use
172+
val stringParams = params.mapValues { it.value.toString() }
152173

174+
// Handle task-boundary tool - update task list
175+
if (toolName == "task-boundary") {
176+
updateTaskFromToolCall(stringParams)
177+
}
178+
179+
// Handle plan management tool - update plan state with original params
180+
if (toolName == "plan") {
181+
updatePlanFromToolCallWithAnyParams(params)
182+
// Skip rendering plan tool to timeline - it's shown in PlanSummaryBar
183+
return
184+
}
185+
186+
renderToolCallInternal(toolName, toolInfo, stringParams, paramsStr, toolType)
187+
}
188+
189+
/**
190+
* Internal method to render tool call UI elements
191+
*/
192+
private fun renderToolCallInternal(
193+
toolName: String,
194+
toolInfo: ToolCallDisplayInfo,
195+
params: Map<String, String>,
196+
paramsStr: String,
197+
toolType: ToolType?
198+
) {
153199
// Skip adding ToolCallItem for Shell - will be replaced by LiveTerminalItem
154200
// This prevents the "two bubbles" problem
155201
if (toolType == ToolType.Shell) {
@@ -220,21 +266,50 @@ class JewelRenderer : BaseRenderer() {
220266
}
221267

222268
/**
223-
* Update plan state from plan management tool call
269+
* Update plan state from plan management tool call (string params version)
224270
*/
225271
private fun updatePlanFromToolCall(params: Map<String, String>) {
226272
val action = params["action"]?.uppercase() ?: return
227273
val planMarkdown = params["planMarkdown"] ?: ""
274+
val taskIndex = params["taskIndex"]?.toIntOrNull()
275+
val stepIndex = params["stepIndex"]?.toIntOrNull()
276+
277+
updatePlanState(action, planMarkdown, taskIndex, stepIndex)
278+
}
279+
280+
/**
281+
* Update plan state from plan management tool call with Any params.
282+
* This is the preferred method as it handles complex values correctly.
283+
*/
284+
private fun updatePlanFromToolCallWithAnyParams(params: Map<String, Any>) {
285+
val action = (params["action"] as? String)?.uppercase() ?: return
286+
val planMarkdown = params["planMarkdown"] as? String ?: ""
287+
val taskIndex = when (val v = params["taskIndex"]) {
288+
is Number -> v.toInt()
289+
is String -> v.toIntOrNull()
290+
else -> null
291+
}
292+
val stepIndex = when (val v = params["stepIndex"]) {
293+
is Number -> v.toInt()
294+
is String -> v.toIntOrNull()
295+
else -> null
296+
}
228297

298+
updatePlanState(action, planMarkdown, taskIndex, stepIndex)
299+
}
300+
301+
/**
302+
* Internal method to update plan state
303+
*/
304+
private fun updatePlanState(action: String, planMarkdown: String, taskIndex: Int?, stepIndex: Int?) {
229305
when (action) {
230306
"CREATE", "UPDATE" -> {
231307
if (planMarkdown.isNotBlank()) {
232308
_currentPlan.value = MarkdownPlanParser.parseToPlan(planMarkdown)
233309
}
234310
}
235311
"COMPLETE_STEP" -> {
236-
val taskIndex = params["taskIndex"]?.toIntOrNull() ?: return
237-
val stepIndex = params["stepIndex"]?.toIntOrNull() ?: return
312+
if (taskIndex == null || stepIndex == null) return
238313
_currentPlan.value?.let { plan ->
239314
if (taskIndex in 1..plan.tasks.size) {
240315
val task = plan.tasks[taskIndex - 1]
@@ -249,8 +324,7 @@ class JewelRenderer : BaseRenderer() {
249324
}
250325
}
251326
"FAIL_STEP" -> {
252-
val taskIndex = params["taskIndex"]?.toIntOrNull() ?: return
253-
val stepIndex = params["stepIndex"]?.toIntOrNull() ?: return
327+
if (taskIndex == null || stepIndex == null) return
254328
_currentPlan.value?.let { plan ->
255329
if (taskIndex in 1..plan.tasks.size) {
256330
val task = plan.tasks[taskIndex - 1]

‎mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt‎

Lines changed: 98 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,17 @@ class ComposeRenderer : BaseRenderer() {
123123
override fun renderLLMResponseEnd() {
124124
super.renderLLMResponseEnd()
125125

126-
// Add the completed reasoning as a message to timeline
126+
// Save content and token info before clearing
127127
val finalContent = _currentStreamingOutput.trim()
128+
val tokenInfo = _lastMessageTokenInfo
129+
130+
// IMPORTANT: Clear streaming output FIRST to avoid showing both
131+
// StreamingMessageItem and MessageItem simultaneously (double progress bar issue)
132+
_currentStreamingOutput = ""
133+
_isProcessing = false
134+
_lastMessageTokenInfo = null
135+
136+
// Then add the completed message to timeline
128137
if (finalContent.isNotEmpty()) {
129138
_timeline.add(
130139
TimelineItem.MessageItem(
@@ -133,14 +142,10 @@ class ComposeRenderer : BaseRenderer() {
133142
role = MessageRole.ASSISTANT,
134143
content = finalContent
135144
),
136-
tokenInfo = _lastMessageTokenInfo
145+
tokenInfo = tokenInfo
137146
)
138147
)
139148
}
140-
141-
_currentStreamingOutput = ""
142-
_isProcessing = false
143-
_lastMessageTokenInfo = null // Reset after use
144149
}
145150

146151
override fun renderToolCall(
@@ -159,8 +164,54 @@ class ComposeRenderer : BaseRenderer() {
159164
// Handle plan management tool - update plan state
160165
if (toolName == "plan") {
161166
updatePlanFromToolCall(params)
167+
// Skip rendering plan tool to timeline - it's shown in PlanSummaryBar
168+
return
169+
}
170+
171+
renderToolCallInternal(toolName, toolInfo, params, paramsStr, toolType)
172+
}
173+
174+
/**
175+
* Render a tool call with parsed parameters.
176+
* This is the preferred method as it avoids string parsing issues with complex values.
177+
*/
178+
override fun renderToolCallWithParams(toolName: String, params: Map<String, Any>) {
179+
// Convert params to string format for display
180+
val paramsStr = params.entries.joinToString(" ") { (key, value) ->
181+
"$key=\"$value\""
182+
}
183+
val toolInfo = formatToolCallDisplay(toolName, paramsStr)
184+
val toolType = toolName.toToolType()
185+
186+
// Convert Map<String, Any> to Map<String, String> for internal use
187+
val stringParams = params.mapValues { it.value.toString() }
188+
189+
// Handle task-boundary tool - update task list
190+
if (toolName == "task-boundary") {
191+
updateTaskFromToolCall(stringParams)
192+
}
193+
194+
// Handle plan management tool - update plan state with original params
195+
if (toolName == "plan") {
196+
updatePlanFromToolCallWithAnyParams(params)
162197
}
163198

199+
// Skip rendering plan tool to timeline - it's shown in PlanSummaryBar
200+
if (toolName != "plan") {
201+
renderToolCallInternal(toolName, toolInfo, stringParams, paramsStr, toolType)
202+
}
203+
}
204+
205+
/**
206+
* Internal method to render tool call UI elements
207+
*/
208+
private fun renderToolCallInternal(
209+
toolName: String,
210+
toolInfo: ToolCallInfo,
211+
params: Map<String, String>,
212+
paramsStr: String,
213+
toolType: ToolType?
214+
) {
164215
// Extract file path for read/write operations
165216
val filePath =
166217
when (toolType) {
@@ -232,21 +283,50 @@ class ComposeRenderer : BaseRenderer() {
232283
}
233284

234285
/**
235-
* Update plan state from plan management tool call
286+
* Update plan state from plan management tool call (string params version)
236287
*/
237288
private fun updatePlanFromToolCall(params: Map<String, String>) {
238289
val action = params["action"]?.uppercase() ?: return
239290
val planMarkdown = params["planMarkdown"] ?: ""
291+
val taskIndex = params["taskIndex"]?.toIntOrNull()
292+
val stepIndex = params["stepIndex"]?.toIntOrNull()
293+
294+
updatePlanState(action, planMarkdown, taskIndex, stepIndex)
295+
}
296+
297+
/**
298+
* Update plan state from plan management tool call with Any params.
299+
* This is the preferred method as it handles complex values correctly.
300+
*/
301+
private fun updatePlanFromToolCallWithAnyParams(params: Map<String, Any>) {
302+
val action = (params["action"] as? String)?.uppercase() ?: return
303+
val planMarkdown = params["planMarkdown"] as? String ?: ""
304+
val taskIndex = when (val v = params["taskIndex"]) {
305+
is Number -> v.toInt()
306+
is String -> v.toIntOrNull()
307+
else -> null
308+
}
309+
val stepIndex = when (val v = params["stepIndex"]) {
310+
is Number -> v.toInt()
311+
is String -> v.toIntOrNull()
312+
else -> null
313+
}
314+
315+
updatePlanState(action, planMarkdown, taskIndex, stepIndex)
316+
}
240317

318+
/**
319+
* Internal method to update plan state
320+
*/
321+
private fun updatePlanState(action: String, planMarkdown: String, taskIndex: Int?, stepIndex: Int?) {
241322
when (action) {
242323
"CREATE", "UPDATE" -> {
243324
if (planMarkdown.isNotBlank()) {
244325
_currentPlan = MarkdownPlanParser.parseToPlan(planMarkdown)
245326
}
246327
}
247328
"COMPLETE_STEP" -> {
248-
val taskIndex = params["taskIndex"]?.toIntOrNull() ?: return
249-
val stepIndex = params["stepIndex"]?.toIntOrNull() ?: return
329+
if (taskIndex == null || stepIndex == null) return
250330
_currentPlan?.let { plan ->
251331
if (taskIndex in 1..plan.tasks.size) {
252332
val task = plan.tasks[taskIndex - 1]
@@ -261,8 +341,7 @@ class ComposeRenderer : BaseRenderer() {
261341
}
262342
}
263343
"FAIL_STEP" -> {
264-
val taskIndex = params["taskIndex"]?.toIntOrNull() ?: return
265-
val stepIndex = params["stepIndex"]?.toIntOrNull() ?: return
344+
if (taskIndex == null || stepIndex == null) return
266345
_currentPlan?.let { plan ->
267346
if (taskIndex in 1..plan.tasks.size) {
268347
val task = plan.tasks[taskIndex - 1]
@@ -302,6 +381,10 @@ class ComposeRenderer : BaseRenderer() {
302381
// Extract command from the last tool call if available
303382
val command = _currentToolCall?.details?.removePrefix("Executing: ") ?: "unknown"
304383

384+
// IMPORTANT: Clear currentToolCall FIRST to avoid showing both
385+
// CurrentToolCallItem and the result item simultaneously (double progress bar issue)
386+
_currentToolCall = null
387+
305388
// For Live sessions, we show both the terminal widget and the result summary
306389
// Don't remove anything, just add a result item after the live terminal
307390
if (isLiveSession) {
@@ -331,6 +414,10 @@ class ComposeRenderer : BaseRenderer() {
331414
)
332415
}
333416
} else {
417+
// IMPORTANT: Clear currentToolCall FIRST to avoid showing both
418+
// CurrentToolCallItem and the result item simultaneously (double progress bar issue)
419+
_currentToolCall = null
420+
334421
// Update the last ToolCallItem with result information
335422
val lastItem = _timeline.lastOrNull()
336423
if (lastItem is ToolCallItem && lastItem.success == null) {
@@ -376,8 +463,6 @@ class ComposeRenderer : BaseRenderer() {
376463
)
377464
}
378465
}
379-
380-
_currentToolCall = null
381466
}
382467

383468
override fun renderTaskComplete() {

0 commit comments

Comments
 (0)