feat(vscode): improve renderer UI for better UX #41
Conversation
- Remove assistant message background for unified look - Add DevInRenderer to parse and display devin blocks as tool calls - Filter <devin> tags from markdown content - Add open file button for read-file tool - Hide Task completed message (implicit from response)
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a DevInRenderer to parse and display "devin" code blocks as tool-call items, strips tags from markdown, routes devin blocks from SketchRenderer to the new renderer, hides task_complete timeline items, and adjusts assistant message styling in the timeline. Changes
Sequence DiagramsequenceDiagram
participant Client as Webview Client
participant SR as SketchRenderer
participant MR as MarkdownRenderer
participant DVR as DevInRenderer
participant Parser as DevIn Parser
participant UI as DevIn UI
Client->>SR: render message with blocks
SR->>MR: provide markdown content
MR->>MR: filterDevinTags(content)
MR-->>SR: filtered markdown (no <devin> blocks)
SR->>DVR: devin code block (content, isComplete, onAction)
DVR->>Parser: parse lines → tool calls + params
Parser-->>DVR: tool call list
DVR->>UI: render DevInToolItem(s) (collapsed previews)
UI->>Client: onAction('openFile', {path}) when open clicked
UI->>UI: expand/collapse to show JSON param details
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 (4)
mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
17-35: Devin tag stripping matches intent; only minor nitsThe filter handles closed blocks, streaming tails, and stray closing tags correctly, and wiring
filteredContentthrough to the early return andReactMarkdownprevents devin-only messages from rendering as empty shells.Two small, optional tweaks:
- You already
trim()insidefilterDevinTags, so the extrafilteredContent.trim()in theifcheck is redundant.- This will also strip any literal
<devin>markup a model might emit as part of normal markdown; if that’s a realistic case, you may eventually want a more constrained pattern (e.g., only tags starting at column 0 or preceded by a newline).Also applies to: 127-127
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (2)
22-60: DevIn parsing and completed‑block gating look good; you can avoid work while streamingThe line‑wise parsing and
argsfallback cover the expected/tool key="val"shapes well, and the!isComplete || toolCalls.length === 0guard ensures we don’t render half‑baked blocks.To cut unnecessary work during streaming (when
isComplete === false), you can short‑circuitparseDevInContentviauseMemo:- const toolCalls = useMemo(() => parseDevInContent(content), [content]); + const toolCalls = useMemo( + () => (isComplete ? parseDevInContent(content) : []), + [content, isComplete] + );This keeps the behavior identical but skips parsing for partial DevIn blocks.
Also applies to: 74-83
106-175: Align DevInToolItem header behavior with pointer cursorCSS gives
.devin-tool-headera pointer cursor and hover state, but only the small toggle button actually expands/collapses. Making the whole header clickable better matches the affordance:- return ( - <div className="devin-tool-item"> - <div className="devin-tool-header"> + return ( + <div className="devin-tool-item"> + <div className="devin-tool-header" onClick={handleToggle}>Because
handleToggleno‑ops when there are no params and already callsstopPropagation, this shouldn’t change other behavior, and the dedicated toggle button can stay as‑is.mpp-vscode/webview/src/components/Timeline.tsx (1)
133-135: Hiding task_complete items via a no‑op renderer matches the UX goalReturning
nullfromTaskCompleteItemRenderercleanly removes these entries from the DOM while keeping the switch exhaustive. The only leftover is the now‑unused.timeline-item.task-completestyles; you can keep them for possible future reuse or trim them in a follow‑up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
mpp-vscode/webview/src/components/Timeline.css(1 hunks)mpp-vscode/webview/src/components/Timeline.tsx(1 hunks)mpp-vscode/webview/src/components/sketch/DevInRenderer.css(1 hunks)mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx(1 hunks)mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx(2 hunks)mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx(2 hunks)mpp-vscode/webview/src/components/sketch/index.ts(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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 mpp-ui/src/jsMain/typescript/**/*.{ts,tsx} : In CLI/TUI (TypeScript), use `mpp-ui/src/jsMain/typescript/design-system/` and import `semanticInk` / `semanticChalk`
Applied to files:
mpp-vscode/webview/src/components/sketch/index.tsmpp-vscode/webview/src/components/sketch/SketchRenderer.tsx
🧬 Code graph analysis (3)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/PlanManagementTool.kt (1)
action(25-37)
mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
mpp-vscode/webview/src/components/sketch/index.ts (1)
MarkdownRenderer(17-17)
mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx (2)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (1)
DevInRenderer(74-98)mpp-vscode/webview/src/components/sketch/index.ts (1)
DevInRenderer(23-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Build (223)
- GitHub Check: Build (241)
🔇 Additional comments (4)
mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx (1)
21-21: DevInRenderer wiring fordevinblocks looks correctThe
devinbranch now correctly delegates toDevInRenderer, withcontent,isComplete, andonActionall passed through andlanguageId.toLowerCase()ensuring case‑insensitive matching. This fits cleanly into the existing dispatch structure.Also applies to: 120-127
mpp-vscode/webview/src/components/Timeline.css (1)
25-30: Assistant message styling change is consistent and scopedMaking
.timeline-item.message.assistanttransparent with no border (and slightly reduced vertical padding) matches the goal of visually merging assistant replies, while leaving user and system message treatments intact.mpp-vscode/webview/src/components/sketch/index.ts (1)
3-14: Barrel export and docs updated correctly for DevInRendererThe index now documents and re‑exports
DevInRendereralongside the other sketch components, keeping the public surface aligned with the new DevIn rendering path.Also applies to: 23-23
mpp-vscode/webview/src/components/sketch/DevInRenderer.css (1)
1-95: DevInRenderer theming is coherent and matches component behaviorThe styles give DevIn tool calls a compact, readable card with clear affordances (success accent border, header hover state, compact action buttons) and rely on VSCode theme variables throughout. This pairs well with the React component structure and the header‑click behavior suggested in DevInRenderer.tsx.
There was a problem hiding this comment.
Pull request overview
This PR enhances the VSCode webview renderer with improved UI/UX for displaying AI assistant responses. It introduces a new DevInRenderer component to parse and display tool call blocks as interactive UI elements, removes visual clutter from assistant messages for a more unified look, and hides the redundant "Task completed" message.
Key changes:
- Added DevInRenderer component to parse
<devin>blocks and render them as collapsible tool call items with file-opening capabilities - Filtered
<devin>tags from markdown content to prevent duplicate display - Made assistant messages transparent for a more cohesive conversation flow
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| mpp-vscode/webview/src/components/sketch/index.ts | Adds DevInRenderer export and updates JSDoc to document new component |
| mpp-vscode/webview/src/components/sketch/SketchRenderer.tsx | Integrates DevInRenderer for handling 'devin' block types instead of generic code rendering |
| mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx | Filters out <devin> tags from markdown content to prevent duplication |
| mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx | New component that parses tool call syntax and renders interactive tool items with expand/collapse and file-opening features |
| mpp-vscode/webview/src/components/sketch/DevInRenderer.css | Styling for the DevInRenderer component with VSCode theme integration |
| mpp-vscode/webview/src/components/Timeline.tsx | Hides TaskCompleteItemRenderer output (returns null) as completion is implicit |
| mpp-vscode/webview/src/components/Timeline.css | Changes assistant messages to transparent background for unified appearance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Remove <devin>...</devin> blocks (including multiline) | ||
| let filtered = content.replace(/<devin>[\s\S]*?<\/devin>/gi, ''); | ||
| // Remove unclosed <devin> tags at the end (streaming) | ||
| filtered = filtered.replace(/<devin>[\s\S]*$/gi, ''); |
There was a problem hiding this comment.
The regex pattern on line 25 /<devin>[\s\S]*$/gi uses the global flag g with $ anchor, which may not work as expected. The $ anchor in global mode will match at the end of each line when using the multiline flag m, but without it, it only matches at the very end of the string. Since this regex is intended to match unclosed <devin> tags during streaming (which would only occur at the end of the entire content string), the g flag is unnecessary and could cause unexpected behavior. Consider using /<devin>[\s\S]*$/i instead.
| filtered = filtered.replace(/<devin>[\s\S]*$/gi, ''); | |
| filtered = filtered.replace(/<devin>[\s\S]*$/i, ''); |
| // Parse key="value" or key=value patterns | ||
| const paramRegex = /([a-zA-Z_][a-zA-Z0-9_]*)\s*=\s*(?:"([^"]*)"|'([^']*)'|(\S+))/g; | ||
| let match; | ||
| while ((match = paramRegex.exec(paramsStr)) !== null) { | ||
| const key = match[1]; | ||
| const value = match[2] ?? match[3] ?? match[4] ?? ''; |
There was a problem hiding this comment.
The parameter regex /([a-zA-Z_][a-zA-Z0-9_]*)\s*=\s*(?:"([^"]*)"|'([^']*)'|(\S+))/g doesn't handle escaped quotes within quoted values. For example, key="value with \"quote\"" would fail to parse correctly. Consider using a more robust parsing approach or documenting this limitation.
| // Parse key="value" or key=value patterns | |
| const paramRegex = /([a-zA-Z_][a-zA-Z0-9_]*)\s*=\s*(?:"([^"]*)"|'([^']*)'|(\S+))/g; | |
| let match; | |
| while ((match = paramRegex.exec(paramsStr)) !== null) { | |
| const key = match[1]; | |
| const value = match[2] ?? match[3] ?? match[4] ?? ''; | |
| // Parse key="value" or key=value patterns, handling escaped quotes | |
| let i = 0; | |
| const len = paramsStr.length; | |
| while (i < len) { | |
| // Skip whitespace | |
| while (i < len && /\s/.test(paramsStr[i])) i++; | |
| if (i >= len) break; | |
| // Parse key | |
| const keyMatch = paramsStr.slice(i).match(/^([a-zA-Z_][a-zA-Z0-9_]*)/); | |
| if (!keyMatch) break; | |
| const key = keyMatch[1]; | |
| i += key.length; | |
| // Skip whitespace and '=' | |
| while (i < len && /\s/.test(paramsStr[i])) i++; | |
| if (paramsStr[i] !== '=') break; | |
| i++; // skip '=' | |
| while (i < len && /\s/.test(paramsStr[i])) i++; | |
| if (i >= len) break; | |
| let value = ''; | |
| if (paramsStr[i] === '"' || paramsStr[i] === "'") { | |
| const quote = paramsStr[i]; | |
| i++; | |
| let val = ''; | |
| while (i < len) { | |
| if (paramsStr[i] === '\\' && i + 1 < len && paramsStr[i + 1] === quote) { | |
| val += quote; | |
| i += 2; | |
| } else if (paramsStr[i] === '\\' && i + 1 < len) { | |
| val += paramsStr[i + 1]; | |
| i += 2; | |
| } else if (paramsStr[i] === quote) { | |
| i++; | |
| break; | |
| } else { | |
| val += paramsStr[i]; | |
| i++; | |
| } | |
| } | |
| value = val; | |
| } else { | |
| // Unquoted value | |
| const valueMatch = paramsStr.slice(i).match(/^\S+/); | |
| if (valueMatch) { | |
| value = valueMatch[0]; | |
| i += value.length; | |
| } | |
| } |
| const handleToggle = (e: React.MouseEvent) => { | ||
| e.stopPropagation(); | ||
| if (hasParams) { | ||
| setIsExpanded(!isExpanded); | ||
| } | ||
| }; |
There was a problem hiding this comment.
The header div has cursor: pointer styling (line 20 in CSS), but the click behavior in handleToggle only triggers when hasParams is true. This could confuse users when clicking on tool items without parameters, as the cursor indicates clickability but nothing happens. Consider either removing the pointer cursor when hasParams is false, or making the entire header always clickable (even if it does nothing) for consistency.
| <span className="devin-tool-icon">✅</span> | ||
| <span className="devin-tool-name">{toolName}</span> | ||
| {!isExpanded && details && ( | ||
| <span className="devin-tool-details"> | ||
| {details.length > 60 ? details.slice(0, 60) + '...' : details} | ||
| </span> | ||
| )} | ||
| <div className="devin-tool-actions"> | ||
| {isFileTool && filePath && ( | ||
| <button | ||
| className="devin-action-btn" | ||
| onClick={handleOpenFile} | ||
| title="Open file in editor" | ||
| > | ||
| 📂 | ||
| </button> | ||
| )} | ||
| {hasParams && ( | ||
| <button | ||
| className="devin-action-btn toggle" | ||
| onClick={handleToggle} | ||
| title={isExpanded ? 'Collapse' : 'Expand'} | ||
| > | ||
| {isExpanded ? '▼' : '▶'} | ||
| </button> |
There was a problem hiding this comment.
The emoji icons (✅, 📂) and arrow symbols (▼, ▶) may not be accessible to screen readers. Consider adding aria-label attributes to provide text descriptions for assistive technologies. For example, the file open button could have aria-label="Open file in editor" (which you already have as title), but also consider using semantic HTML or ARIA roles for better accessibility.
| // Remove unclosed <devin> tags at the end (streaming) | ||
| filtered = filtered.replace(/<devin>[\s\S]*$/gi, ''); | ||
| // Remove standalone </devin> tags | ||
| filtered = filtered.replace(/<\/devin>/gi, ''); |
There was a problem hiding this comment.
The regex pattern /<\/devin>/gi uses the global flag to remove all standalone closing tags. However, this could unintentionally remove </devin> text that appears in code examples or other legitimate contexts within the markdown content. Consider being more conservative with this pattern, or ensure it only runs after the first two patterns have been applied to minimize false positives.
| filtered = filtered.replace(/<\/devin>/gi, ''); | |
| // Remove standalone </devin> tags that appear on their own line | |
| filtered = filtered.replace(/^\s*<\/devin>\s*$/gim, ''); |
|
augment review |
- Remove unnecessary 'g' flag from regex for unclosed devin tags - Make standalone </devin> removal more conservative (only on own line) - Add aria-label attributes for accessibility - Only show pointer cursor when header is clickable (has params) - Document limitation: parser does not handle escaped quotes
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (3)
45-53: Escaped quotes limitation is now documented.The previous concern about escaped quotes has been addressed with documentation notes on lines 26-27 and 46. This is acceptable for the expected use cases.
133-142: Cursor pointer issue addressed with conditional class.The previous concern about misleading cursor on non-clickable headers has been resolved. Line 142 now conditionally applies the
'clickable'class only whenhasParamsis true.
143-170: Accessibility concerns addressed with proper ARIA attributes.The previous concern about emoji accessibility has been resolved:
- Line 143:
✅hasrole="img"andaria-label="Completed"- Lines 155-158: File button has
aria-labeland the emoji hasaria-hidden="true"- Lines 165-169: Toggle button has
aria-label,aria-expanded, and the arrow hasaria-hidden="true"
🧹 Nitpick comments (6)
mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
33-36: Redundant trim call.Since
filterDevinTagsalready returns a trimmed string (line 29), the.trim()call on line 35 is unnecessary.Apply this diff:
const filteredContent = filterDevinTags(content); - if (!filteredContent.trim()) { + if (!filteredContent) { return null; }mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (2)
11-20: LGTM - Clean interface definitions.The interfaces are well-structured. Consider using a more specific type for the
dataparameter inonAction(e.g.,Record<string, unknown>or a union type) for improved type safety, but the currentanyis acceptable for a generic action handler.
110-111: Consider the specificity of the 'open' tool name.The
'open'entry is quite generic and might incorrectly trigger file-open behavior for unrelated tools. Verify this is intentional, or consider using more specific names like'open-file'.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt (1)
277-279: Consider notifying users when undo operations fail.Errors are logged but not displayed to users. When manual undo fails, consider showing a notification so users are aware the operation didn't succeed and can take corrective action.
You can use IntelliJ's notification system:
import com.intellij.notification.NotificationGroupManager import com.intellij.notification.NotificationType // In the catch block: catch (e: Exception) { logger.error("Failed to undo change for ${change.filePath}", e) NotificationGroupManager.getInstance() .getNotificationGroup("AutoDev") .createNotification( "Failed to undo changes to ${change.filePath}", e.message ?: "Unknown error", NotificationType.ERROR ) .notify(project) }mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt (2)
123-159: Extract repeated DiffContext implementation.The anonymous
DiffContextobject is duplicated three times (lines 127-132, 138-143, 149-154) with identical implementations. Extract this to a private property or helper method to improve maintainability.Apply this refactor:
+ private val diffContext = object : DiffContext() { + override fun getProject() = this@IdeaFileChangeDiffDialogWrapper.project + override fun isWindowFocused() = true + override fun isFocusedInWindow() = true + override fun requestFocusInWindow() = Unit + } + private fun createDiffViewer(): JComponent { return when (fileChange.changeType) { ChangeType.CREATE -> { val diffRequest = createOneSideDiffRequest() - val diffViewer = SimpleOnesideDiffViewer(object : DiffContext() { - override fun getProject() = this@IdeaFileChangeDiffDialogWrapper.project - override fun isWindowFocused() = true - override fun isFocusedInWindow() = true - override fun requestFocusInWindow() = Unit - }, diffRequest) + val diffViewer = SimpleOnesideDiffViewer(diffContext, diffRequest) diffViewer.init() diffViewer.component } ChangeType.DELETE -> { val diffRequest = createOneSideDiffRequestForDelete() - val diffViewer = SimpleOnesideDiffViewer(object : DiffContext() { - override fun getProject() = this@IdeaFileChangeDiffDialogWrapper.project - override fun isWindowFocused() = true - override fun isFocusedInWindow() = true - override fun requestFocusInWindow() = Unit - }, diffRequest) + val diffViewer = SimpleOnesideDiffViewer(diffContext, diffRequest) diffViewer.init() diffViewer.component } else -> { val diffRequest = createTwoSideDiffRequest() - val diffViewer = SimpleDiffViewer(object : DiffContext() { - override fun getProject() = this@IdeaFileChangeDiffDialogWrapper.project - override fun isWindowFocused() = true - override fun isFocusedInWindow() = true - override fun requestFocusInWindow() = Unit - }, diffRequest) + val diffViewer = SimpleDiffViewer(diffContext, diffRequest) diffViewer.init() diffViewer.component } } }
301-342: LGTM! Clean conversion utility.The
FileChangeConvertercorrectly mapsFileChangeto IntelliJ'sChangeAPI with proper handling of different change types and content revisions.Minor note: Line 311's
basePathvariable is declared but never used. You can remove it:fun toChange(project: Project, fileChange: FileChange): Change { - val basePath = project.basePath ?: System.getProperty("user.dir") val file = File(fileChange.filePath) val filePath: FilePath = VcsUtil.getFilePath(file, false)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt(3 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt(1 hunks)mpp-vscode/webview/src/components/sketch/DevInRenderer.css(1 hunks)mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx(1 hunks)mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mpp-vscode/webview/src/components/sketch/DevInRenderer.css
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code (e.g., file I/O on JVM/JS/Wasm) in Kotlin Multiplatform projects
Check export first if some functions are not working well with CLI (TypeScript)
In Kotlin/JS @JsExport: AvoidFlow, usePromiseinstead
In Kotlin/JS @JsExport: Use concrete classes as return types and parameter types; avoid interface types
For WASM platform, avoid using emoji and UTF-8 in code
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt
**/idea/**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/idea/**/*.kt: For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer:JewelFlags.useCustomPopupRenderer = trueinIdeaAgentToolWindowFactory
For popup/dropdown menus in IntelliJ plugins, use Jewel'sPopupMenuinstead ofandroidx.compose.ui.window.Popup
For dialogs in IntelliJ plugins, use IntelliJ'sDialogWrapperwithorg.jetbrains.jewel.bridge.composeinstead ofandroidx.compose.ui.window.Dialog
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.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 **/idea/**/*.kt : For dialogs in IntelliJ plugins, use IntelliJ's `DialogWrapper` with `org.jetbrains.jewel.bridge.compose` instead of `androidx.compose.ui.window.Dialog`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For SwingPanel z-index issues with Compose popups, enable Jewel's custom popup renderer: `JewelFlags.useCustomPopupRenderer = true` in `IdeaAgentToolWindowFactory`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt
📚 Learning: 2025-12-02T00:20:34.480Z
Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-02T00:20:34.480Z
Learning: Applies to **/idea/**/*.kt : For popup/dropdown menus in IntelliJ plugins, use Jewel's `PopupMenu` instead of `androidx.compose.ui.window.Popup`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt
🧬 Code graph analysis (4)
mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
mpp-vscode/webview/src/components/sketch/index.ts (1)
MarkdownRenderer(17-17)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt (1)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt (1)
performManualUndo(232-274)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (1)
mpp-vscode/webview/src/components/sketch/index.ts (1)
DevInRenderer(23-23)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt (2)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt (1)
performManualUndo(241-281)exts/devins-lang/src/main/kotlin/cc/unitmesh/devti/language/compiler/exec/file/WriteInsCommand.kt (1)
runWriteAction(66-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (223)
- GitHub Check: Build (241)
🔇 Additional comments (6)
mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
17-30: Past review comments have been addressed—implementation looks solid.The regex patterns correctly implement the suggestions from previous reviews:
- Line 26 now uses
/<devin>[\s\S]*$/iwithout the global flag, as recommended- Line 28 now uses line-anchored pattern
/^\s*<\/devin>\s*$/gimto be more conservativeThe three-step filtering approach is well-ordered: complete blocks → trailing unclosed tags → standalone closing tags.
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (1)
78-102: LGTM - Well-structured component with appropriate memoization.The use of
useMemofor parsing prevents unnecessary re-computation. The early return for incomplete or empty blocks is a clean approach.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt (4)
44-60: LGTM! Proper DialogWrapper usage.The constructor and initialization correctly follow IntelliJ's DialogWrapper pattern, as recommended in the coding guidelines for IntelliJ plugin dialogs.
62-121: LGTM! Clean Swing UI implementation.The header panel properly uses JBUI theming, appropriate icons, and displays relevant file change information clearly.
161-184: LGTM! Proper diff request construction.The three diff request helpers correctly handle different change types using IntelliJ's diff API.
186-217: LGTM! Action buttons properly wired.The south panel correctly creates action buttons with appropriate icons and callbacks.
| private fun performUndo() { | ||
| try { | ||
| // Try to use RollbackWorker for proper VCS integration | ||
| rollbackWorker.doRollback(listOf(change), false) | ||
| onUndoCallback() | ||
| close(OK_EXIT_CODE) | ||
| } catch (e: Exception) { | ||
| LOG.warn("RollbackWorker failed, falling back to manual revert", e) | ||
| // Fallback to manual revert | ||
| performManualUndo() | ||
| } | ||
| } |
There was a problem hiding this comment.
Add user-facing error notifications.
When RollbackWorker.doRollback fails (line 222), the code logs a warning and falls back to manual undo, but provides no visual feedback to the user. Similarly, if performManualUndo subsequently fails (line 271), the error is only logged. Users should be notified of undo failures.
Consider adding user notifications:
private fun performUndo() {
try {
// Try to use RollbackWorker for proper VCS integration
rollbackWorker.doRollback(listOf(change), false)
onUndoCallback()
close(OK_EXIT_CODE)
} catch (e: Exception) {
LOG.warn("RollbackWorker failed, falling back to manual revert", e)
+ // Notify user that VCS rollback failed
+ com.intellij.openapi.ui.Messages.showWarningDialog(
+ project,
+ "VCS rollback failed, attempting manual revert",
+ "Undo Warning"
+ )
// Fallback to manual revert
performManualUndo()
}
}And in performManualUndo:
} catch (e: Exception) {
LOG.error("Failed to undo change for ${fileChange.filePath}", e)
+ com.intellij.openapi.ui.Messages.showErrorDialog(
+ project,
+ "Failed to undo change: ${e.message}",
+ "Undo Failed"
+ )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private fun performUndo() { | |
| try { | |
| // Try to use RollbackWorker for proper VCS integration | |
| rollbackWorker.doRollback(listOf(change), false) | |
| onUndoCallback() | |
| close(OK_EXIT_CODE) | |
| } catch (e: Exception) { | |
| LOG.warn("RollbackWorker failed, falling back to manual revert", e) | |
| // Fallback to manual revert | |
| performManualUndo() | |
| } | |
| } | |
| private fun performUndo() { | |
| try { | |
| // Try to use RollbackWorker for proper VCS integration | |
| rollbackWorker.doRollback(listOf(change), false) | |
| onUndoCallback() | |
| close(OK_EXIT_CODE) | |
| } catch (e: Exception) { | |
| LOG.warn("RollbackWorker failed, falling back to manual revert", e) | |
| // Notify user that VCS rollback failed | |
| com.intellij.openapi.ui.Messages.showWarningDialog( | |
| project, | |
| "VCS rollback failed, attempting manual revert", | |
| "Undo Warning" | |
| ) | |
| // Fallback to manual revert | |
| performManualUndo() | |
| } | |
| } |
| private fun performManualUndo() { | ||
| runWriteAction { | ||
| try { | ||
| when (fileChange.changeType) { | ||
| ChangeType.CREATE -> { | ||
| // For created files, delete or clear the content | ||
| val virtualFile = LocalFileSystem.getInstance().findFileByPath(fileChange.filePath) | ||
| virtualFile?.let { vf -> | ||
| val document = FileDocumentManager.getInstance().getDocument(vf) | ||
| document?.setText("") | ||
| } | ||
| } | ||
| ChangeType.EDIT, ChangeType.RENAME -> { | ||
| // Restore original content | ||
| fileChange.originalContent?.let { original -> | ||
| val virtualFile = LocalFileSystem.getInstance().findFileByPath(fileChange.filePath) | ||
| virtualFile?.let { vf -> | ||
| val document = FileDocumentManager.getInstance().getDocument(vf) | ||
| document?.setText(original) | ||
| } | ||
| } | ||
| } | ||
| ChangeType.DELETE -> { | ||
| // For deleted files, recreate them | ||
| fileChange.originalContent?.let { original -> | ||
| val parentPath = fileChange.filePath.substringBeforeLast('/') | ||
| val fileName = fileChange.filePath.substringAfterLast('/') | ||
| val parentDir = LocalFileSystem.getInstance().findFileByPath(parentPath) | ||
| parentDir?.let { dir -> | ||
| val newFile = dir.createChildData(project, fileName) | ||
| val document = FileDocumentManager.getInstance().getDocument(newFile) | ||
| document?.setText(original) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ) | ||
| onUndoCallback() | ||
| close(OK_EXIT_CODE) | ||
| } catch (e: Exception) { | ||
| LOG.error("Failed to undo change for ${fileChange.filePath}", e) | ||
| } | ||
| } | ||
| dialogPanel.preferredSize = Dimension(800, 600) | ||
| return dialogPanel | ||
| } |
There was a problem hiding this comment.
Consolidate duplicated undo logic and fix path handling.
This method has two significant issues:
-
Code duplication: The
performManualUndoimplementation is nearly identical to the one inIdeaFileChangeSummary.kt(lines 240-280). This logic should be extracted to a shared utility to avoid maintenance burden. -
Platform-specific path handling: Line 257 uses
substringBeforeLast('/')which assumes Unix-style path separators and will fail on Windows where paths use backslashes.
Fix the path handling:
ChangeType.DELETE -> {
// For deleted files, recreate them
fileChange.originalContent?.let { original ->
- val parentPath = fileChange.filePath.substringBeforeLast('/')
- val fileName = fileChange.filePath.substringAfterLast('/')
+ val file = File(fileChange.filePath)
+ val parentPath = file.parent
+ val fileName = file.name
val parentDir = LocalFileSystem.getInstance().findFileByPath(parentPath)
parentDir?.let { dir ->
val newFile = dir.createChildData(project, fileName)
val document = FileDocumentManager.getInstance().getDocument(newFile)
document?.setText(original)
}
}
}Consolidate the duplication: Extract the manual undo logic to a shared utility class (e.g., FileChangeUndoUtil) that both IdeaFileChangeDiffDialogWrapper and IdeaFileChangeSummary can use.
| /** | ||
| * Undo a file change by restoring the original content | ||
| * Undo a file change using IntelliJ's RollbackWorker for proper VCS integration. | ||
| * Falls back to manual revert if RollbackWorker fails. | ||
| */ | ||
| private fun undoChange(project: Project, change: FileChange) { | ||
| private fun undoChange(project: Project, fileChange: FileChange) { | ||
| ApplicationManager.getApplication().invokeLater { | ||
| runWriteAction { | ||
| try { | ||
| when (change.changeType) { | ||
| ChangeType.CREATE -> { | ||
| // For created files, delete or clear the content | ||
| try { | ||
| // Convert FileChange to IntelliJ Change and use RollbackWorker | ||
| val change = FileChangeConverter.toChange(project, fileChange) | ||
| val rollbackWorker = com.intellij.openapi.vcs.changes.ui.RollbackWorker(project) | ||
| rollbackWorker.doRollback(listOf(change), false) | ||
| } catch (e: Exception) { | ||
| logger.warn("RollbackWorker failed, falling back to manual revert", e) | ||
| // Fallback to manual revert | ||
| performManualUndo(project, fileChange) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the actual imports in the file
echo "=== Checking all imports in IdeaFileChangeSummary.kt ==="
head -50 mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt | grep "^import"
echo -e "\n=== Checking for FileChangeConverter import ==="
rg "import.*FileChangeConverter" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt
echo -e "\n=== Finding FileChangeConverter definition in the codebase ==="
rg "class FileChangeConverter|object FileChangeConverter" --type=kt
echo -e "\n=== Checking if FileChangeConverter is defined in this file itself ==="
rg -n "FileChangeConverter" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt | head -20
echo -e "\n=== Checking RollbackWorker.doRollback signature ==="
rg -nA 5 "fun doRollback" --type=kt | head -30Repository: phodal/auto-dev-sketch
Length of output: 2014
🏁 Script executed:
#!/bin/bash
# Check the actual imports more thoroughly
echo "=== Full import section of IdeaFileChangeSummary.kt ==="
head -100 mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt | grep "^import"
echo -e "\n=== Searching for FileChangeConverter definition anywhere ==="
rg "class FileChangeConverter|object FileChangeConverter"
echo -e "\n=== Searching for FileChangeConverter usage in other files ==="
rg "FileChangeConverter" mpp-idea/src/main/kotlin/ | head -20
echo -e "\n=== Checking for RollbackWorker import ==="
rg "import.*RollbackWorker"
echo -e "\n=== Checking if RollbackWorker is available in IntelliJ API ==="
rg "com.intellij.openapi.vcs.changes" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt
echo -e "\n=== Looking at lines around 227-228 in context ==="
sed -n '220,240p' mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.ktRepository: phodal/auto-dev-sketch
Length of output: 3757
🏁 Script executed:
#!/bin/bash
# Find RollbackWorker.doRollback signature
echo "=== Searching for RollbackWorker.doRollback definition ==="
rg "fun doRollback" -A 5
echo -e "\n=== Checking IdeaFileChangeDiffDialog for comparison ==="
sed -n '1,50p' mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt | grep -E "^import|FileChangeConverter"
echo -e "\n=== Finding where FileChangeConverter is defined in IdeaFileChangeDiffDialog ==="
rg -n "object FileChangeConverter" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt -A 3
echo -e "\n=== Checking if FileChangeConverter needs to be public/exported ==="
grep -n "object FileChangeConverter\|fun toChange" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.ktRepository: phodal/auto-dev-sketch
Length of output: 2039
🏁 Script executed:
#!/bin/bash
# Verify the exact package structure
echo "=== Checking package declarations ==="
head -5 mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt
head -5 mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt
echo -e "\n=== Verify if FileChangeConverter is public ==="
grep -B2 "object FileChangeConverter" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeDiffDialog.kt
echo -e "\n=== Check all usages of RollbackWorker.doRollback parameters ==="
rg "\.doRollback" -B 2 -A 2Repository: phodal/auto-dev-sketch
Length of output: 3448
Add missing imports for FileChangeConverter and RollbackWorker.
Line 227 uses FileChangeConverter.toChange() but the import is missing. Add import cc.unitmesh.devins.idea.toolwindow.changes.FileChangeConverter at the top of the file. Additionally, add import com.intellij.openapi.vcs.changes.ui.RollbackWorker and replace the fully qualified name com.intellij.openapi.vcs.changes.ui.RollbackWorker(project) on line 228 with just RollbackWorker(project) for consistency with IdeaFileChangeDiffDialog.kt.
🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt
around lines 219 to 236, add the missing imports for FileChangeConverter and
RollbackWorker and simplify the instantiation: add import
cc.unitmesh.devins.idea.toolwindow.changes.FileChangeConverter and import
com.intellij.openapi.vcs.changes.ui.RollbackWorker at the top of the file, then
replace the fully-qualified
com.intellij.openapi.vcs.changes.ui.RollbackWorker(project) usage with
RollbackWorker(project) in the undoChange function for consistency.
| /** | ||
| * Manual undo fallback when RollbackWorker fails. | ||
| */ | ||
| private fun performManualUndo(project: Project, change: FileChange) { | ||
| runWriteAction { | ||
| try { | ||
| when (change.changeType) { | ||
| ChangeType.CREATE -> { | ||
| // For created files, delete or clear the content | ||
| val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath) | ||
| virtualFile?.let { vf -> | ||
| val document = FileDocumentManager.getInstance().getDocument(vf) | ||
| document?.setText("") | ||
| } | ||
| } | ||
| ChangeType.EDIT, ChangeType.RENAME -> { | ||
| // Restore original content | ||
| change.originalContent?.let { original -> | ||
| val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath) | ||
| virtualFile?.let { vf -> | ||
| val document = FileDocumentManager.getInstance().getDocument(vf) | ||
| document?.setText("") | ||
| } | ||
| } | ||
| ChangeType.EDIT, ChangeType.RENAME -> { | ||
| // Restore original content | ||
| change.originalContent?.let { original -> | ||
| val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath) | ||
| virtualFile?.let { vf -> | ||
| val document = FileDocumentManager.getInstance().getDocument(vf) | ||
| document?.setText(original) | ||
| } | ||
| document?.setText(original) | ||
| } | ||
| } | ||
| ChangeType.DELETE -> { | ||
| // For deleted files, we would need to recreate them | ||
| change.originalContent?.let { original -> | ||
| val parentPath = change.filePath.substringBeforeLast('/') | ||
| val fileName = change.filePath.substringAfterLast('/') | ||
| val parentDir = LocalFileSystem.getInstance().findFileByPath(parentPath) | ||
| parentDir?.let { dir -> | ||
| val newFile = dir.createChildData(project, fileName) | ||
| val document = FileDocumentManager.getInstance().getDocument(newFile) | ||
| document?.setText(original) | ||
| } | ||
| } | ||
| ChangeType.DELETE -> { | ||
| // For deleted files, we would need to recreate them | ||
| change.originalContent?.let { original -> | ||
| val parentPath = change.filePath.substringBeforeLast('/') | ||
| val fileName = change.filePath.substringAfterLast('/') | ||
| val parentDir = LocalFileSystem.getInstance().findFileByPath(parentPath) | ||
| parentDir?.let { dir -> | ||
| val newFile = dir.createChildData(project, fileName) | ||
| val document = FileDocumentManager.getInstance().getDocument(newFile) | ||
| document?.setText(original) | ||
| } | ||
| } | ||
| } | ||
| } catch (e: Exception) { | ||
| logger.error("Failed to undo change for ${change.filePath}", e) | ||
| } | ||
| } catch (e: Exception) { | ||
| logger.error("Failed to undo change for ${change.filePath}", e) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor duplicated performManualUndo logic into a shared utility.
This function duplicates nearly identical logic from IdeaFileChangeDiffDialog.kt (lines 231-273). Extract the common undo logic into a shared utility function to improve maintainability and ensure consistent behavior across both usages.
Consider creating a shared utility like this:
object FileChangeUndoUtil {
fun performManualUndo(project: Project, change: FileChange, onSuccess: (() -> Unit)? = null) {
runWriteAction {
try {
when (change.changeType) {
ChangeType.CREATE -> {
val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath)
virtualFile?.delete(project)
}
ChangeType.EDIT, ChangeType.RENAME -> {
change.originalContent?.let { original ->
val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath)
virtualFile?.let { vf ->
val document = FileDocumentManager.getInstance().getDocument(vf)
document?.setText(original)
}
}
}
ChangeType.DELETE -> {
change.originalContent?.let { original ->
val parentPath = change.filePath.substringBeforeLast('/')
val fileName = change.filePath.substringAfterLast('/')
val parentDir = LocalFileSystem.getInstance().findFileByPath(parentPath)
parentDir?.let { dir ->
val newFile = dir.createChildData(project, fileName)
val document = FileDocumentManager.getInstance().getDocument(newFile)
document?.setText(original)
}
}
}
}
onSuccess?.invoke()
} catch (e: Exception) {
logger.error("Failed to undo change for ${change.filePath}", e)
}
}
}
}🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt
around lines 238-281, the performManualUndo implementation duplicates logic
present in IdeaFileChangeDiffDialog.kt; extract the shared undo logic into a
single utility (e.g., object FileChangeUndoUtil with fun
performManualUndo(project: Project, change: FileChange, onSuccess: (() -> Unit)?
= null)) that implements the Create/Edit/Rename/Delete handling (use
VirtualFile.delete(project) for CREATE, restore original text for EDIT/RENAME,
recreate file and set text for DELETE), call runWriteAction inside the utility,
invoke onSuccess after success, log exceptions with the existing logger, replace
the duplicated implementations in both files with calls to this utility, and
update imports/removals accordingly.
| ChangeType.CREATE -> { | ||
| // For created files, delete or clear the content | ||
| val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath) | ||
| virtualFile?.let { vf -> | ||
| val document = FileDocumentManager.getInstance().getDocument(vf) | ||
| document?.setText("") | ||
| } | ||
| } |
There was a problem hiding this comment.
CREATE case should delete the file, not just clear content.
For ChangeType.CREATE, the current implementation clears the file content with setText(""), leaving an empty file on disk. To properly undo a file creation, the file should be deleted entirely using virtualFile.delete(project).
Apply this diff:
ChangeType.CREATE -> {
- // For created files, delete or clear the content
+ // For created files, delete them
val virtualFile = LocalFileSystem.getInstance().findFileByPath(change.filePath)
- virtualFile?.let { vf ->
- val document = FileDocumentManager.getInstance().getDocument(vf)
- document?.setText("")
- }
+ virtualFile?.delete(project)
}🤖 Prompt for AI Agents
In
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/changes/IdeaFileChangeSummary.kt
around lines 245-252, the CREATE branch currently clears file content instead of
removing the created file; replace the document.setText("") logic with a call to
virtualFile.delete(project) to delete the file from disk (use the surrounding
class's project reference or pass the appropriate Project instance), wrap the
delete in a null-check and try/catch to handle IO exceptions, and refresh the
VFS if needed so the IDE reflects the removed file.
- Remove redundant trim() call in MarkdownRenderer (filterDevinTags already trims) - Optimize useMemo in DevInRenderer to skip parsing during streaming
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (2)
66-76: Consider extracting the truncation length as a constant.The hardcoded
50on line 72 could be extracted as a named constant (e.g.,MAX_PARAM_DISPLAY_LENGTH) for easier maintenance.+const MAX_PARAM_DISPLAY_LENGTH = 50; + function formatToolCallDetails(params: Record<string, string>): string { return Object.entries(params) .map(([key, value]) => { - const truncated = value.length > 50 ? value.slice(0, 50) + '...' : value; + const truncated = value.length > MAX_PARAM_DISPLAY_LENGTH + ? value.slice(0, MAX_PARAM_DISPLAY_LENGTH) + '...' + : value; return `${key}="${truncated}"`; }) .join(' '); }
114-135: Consider making FILE_TOOLS configurable for extensibility.The hardcoded
FILE_TOOLSlist (line 115) works for current needs, but if tool extensibility is planned, consider accepting this as a prop or configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx(1 hunks)mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
mpp-vscode/webview/src/components/sketch/index.ts (1)
MarkdownRenderer(17-17)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (2)
mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/tool/impl/PlanManagementTool.kt (1)
action(25-37)mpp-vscode/webview/src/components/sketch/index.ts (1)
DevInRenderer(23-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (241)
- GitHub Check: Build (223)
🔇 Additional comments (4)
mpp-vscode/webview/src/components/sketch/DevInRenderer.tsx (3)
22-64: Parser limitation appropriately documented.The documented limitation regarding escaped quotes in parameter values (lines 26-28) addresses the previous review concern. For the current use case, this simple regex-based approach is sufficient.
78-106: LGTM: Efficient rendering with useMemo optimization.The
useMemohook appropriately skips parsing during streaming (line 84-87), and the early return for incomplete/empty blocks (line 90-92) prevents unnecessary rendering.
144-186: LGTM: Accessibility and UX concerns addressed.The conditional
clickableclass (line 146) addresses the previous cursor styling concern, and the comprehensive ARIA attributes (lines 147, 159-160, 169-171, 162, 173) properly support assistive technologies. Event propagation is correctly managed withstopPropagationcalls (lines 131, 138).mpp-vscode/webview/src/components/sketch/MarkdownRenderer.tsx (1)
17-30: LGTM: Previous review concerns resolved.The regex improvements address both prior comments:
- Line 26 correctly removes the
gflag since$only matches end of string- Line 28 now uses
^\s*<\/devin>\s*$/gimto match only standalone closing tags on their own lineNote: DevIn tags within code examples or documentation will still be filtered. This is likely acceptable for the current use case.
Changes
1. Remove assistant message background
2. Add DevInRenderer component
<devin>blocks and render as tool call items<devin>...</devin>tags from markdown contentread-filetool with open file button (📂)3. Hide Task completed message
Files Changed
Timeline.css- Remove assistant message backgroundTimeline.tsx- Hide TaskCompleteItemRendererMarkdownRenderer.tsx- Filter devin tagsSketchRenderer.tsx- Use DevInRenderer for devin blocksDevInRenderer.tsx- New component for tool call displayDevInRenderer.css- Styling for tool call itemsindex.ts- Export DevInRendererSummary by CodeRabbit
New Features
UI Updates
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.