mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
feat: improve error display with custom titles and expandable UI
- Add metadata support to ClineMessage type for storing custom error titles - Update ChatRow component to display errors with expandable UI similar to diff_error - Add sayError helper method to Task class for convenient error reporting - Update existing error calls to use custom titles for better context - Fix test to match new error format with metadata
This commit is contained in:
parent
0ce4e891fd
commit
47c21fb10b
7 changed files with 249 additions and 11 deletions
113
implementation-plan.md
Normal file
113
implementation-plan.md
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
# Implementation Plan: Improve Error Display in Webview
|
||||
|
||||
## Overview
|
||||
|
||||
Currently, `cline.say("error", ...)` displays error text in red. We want to improve this to look more like the "Edit Unsuccessful" display with:
|
||||
|
||||
1. A custom title that can be passed as metadata
|
||||
2. An expandable/collapsible UI pattern
|
||||
3. The error text shown when expanded
|
||||
|
||||
## Current Implementation Analysis
|
||||
|
||||
### 1. Error Display Flow
|
||||
|
||||
- **Backend (Task.ts)**: `say("error", text)` method sends error messages
|
||||
- **Frontend (ChatRow.tsx)**: Renders error messages with red text styling
|
||||
- **Diff Error Pattern**: Already implements the expandable UI pattern we want to replicate
|
||||
|
||||
### 2. Diff Error Implementation (lines 960-1048 in ChatRow.tsx)
|
||||
|
||||
The diff_error display has:
|
||||
|
||||
- Warning icon with yellow/orange color
|
||||
- Bold title ("Edit Unsuccessful")
|
||||
- Copy button
|
||||
- Expand/collapse chevron
|
||||
- Expandable content area showing the error details
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
### Step 1: Extend the say method signature
|
||||
|
||||
**File**: `src/core/task/Task.ts`
|
||||
|
||||
The say method already accepts an `options` parameter with metadata support. We need to:
|
||||
|
||||
- Document that error messages can include a `title` in metadata
|
||||
- No changes needed to the method signature itself
|
||||
|
||||
### Step 2: Update ChatRow.tsx to handle enhanced error display
|
||||
|
||||
**File**: `webview-ui/src/components/chat/ChatRow.tsx`
|
||||
|
||||
Changes needed:
|
||||
|
||||
1. Add state for error expansion (similar to `isDiffErrorExpanded`)
|
||||
2. Extract metadata from error messages
|
||||
3. Render errors with the expandable UI pattern
|
||||
4. Use custom title from metadata or default to "Error"
|
||||
|
||||
### Step 3: Update translation files
|
||||
|
||||
**Files**: All files in `webview-ui/src/i18n/locales/*/chat.json`
|
||||
|
||||
Add new translation keys:
|
||||
|
||||
- `error.defaultTitle`: Default title when no custom title is provided
|
||||
- Keep existing `error` key for backward compatibility
|
||||
|
||||
### Step 4: Update existing error calls
|
||||
|
||||
Search for all `say("error", ...)` calls and optionally add metadata with custom titles where appropriate.
|
||||
|
||||
## Technical Details
|
||||
|
||||
### Message Structure
|
||||
|
||||
```typescript
|
||||
// When calling say with error and custom title:
|
||||
await this.say(
|
||||
"error",
|
||||
"Detailed error message here",
|
||||
undefined, // images
|
||||
false, // partial
|
||||
undefined, // checkpoint
|
||||
undefined, // progressStatus
|
||||
{
|
||||
metadata: {
|
||||
title: "Custom Error Title",
|
||||
},
|
||||
},
|
||||
)
|
||||
```
|
||||
|
||||
### Frontend Rendering Logic
|
||||
|
||||
```typescript
|
||||
// In ChatRow.tsx, for case "error":
|
||||
// 1. Extract title from metadata or use default
|
||||
// 2. Render expandable UI similar to diff_error
|
||||
// 3. Show error text in expanded section
|
||||
```
|
||||
|
||||
## Benefits
|
||||
|
||||
1. **Consistency**: Error display matches the existing "Edit Unsuccessful" pattern
|
||||
2. **Clarity**: Custom titles provide immediate context about the error type
|
||||
3. **User Experience**: Collapsible errors reduce visual clutter
|
||||
4. **Flexibility**: Backward compatible - existing error calls continue to work
|
||||
|
||||
## Testing Considerations
|
||||
|
||||
1. Test with errors that have custom titles
|
||||
2. Test with errors without custom titles (should use default)
|
||||
3. Test expand/collapse functionality
|
||||
4. Test copy button functionality
|
||||
5. Verify all translations work correctly
|
||||
|
||||
## Migration Strategy
|
||||
|
||||
- The implementation is backward compatible
|
||||
- Existing `say("error", ...)` calls will continue to work
|
||||
- We can gradually update error calls to include custom titles where beneficial
|
||||
|
|
@ -224,6 +224,7 @@ export const clineMessageSchema = z.object({
|
|||
reasoning_summary: z.string().optional(),
|
||||
})
|
||||
.optional(),
|
||||
title: z.string().optional(), // Custom title for error messages
|
||||
})
|
||||
.optional(),
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1169,10 +1169,33 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
`Roo tried to use ${toolName}${
|
||||
relPath ? ` for '${relPath.toPosix()}'` : ""
|
||||
} without value for required parameter '${paramName}'. Retrying...`,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
{
|
||||
metadata: {
|
||||
title: "Missing Parameter Error",
|
||||
},
|
||||
},
|
||||
)
|
||||
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper method to say an error with a custom title
|
||||
* @param title - The title to display for the error
|
||||
* @param text - The error message text
|
||||
* @param images - Optional images to include
|
||||
*/
|
||||
async sayError(title: string, text: string, images?: string[]) {
|
||||
await this.say("error", text, images, undefined, undefined, undefined, {
|
||||
metadata: {
|
||||
title,
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
// Lifecycle
|
||||
// Start / Resume / Abort / Dispose
|
||||
|
||||
|
|
|
|||
|
|
@ -226,7 +226,19 @@ describe("insertContentTool", () => {
|
|||
expect(mockedFsReadFile).not.toHaveBeenCalled()
|
||||
expect(mockCline.consecutiveMistakeCount).toBe(1)
|
||||
expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content")
|
||||
expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file"))
|
||||
expect(mockCline.say).toHaveBeenCalledWith(
|
||||
"error",
|
||||
expect.stringContaining("non-existent file"),
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
expect.objectContaining({
|
||||
metadata: expect.objectContaining({
|
||||
title: "Invalid Line Number",
|
||||
}),
|
||||
}),
|
||||
)
|
||||
expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled()
|
||||
expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled()
|
||||
})
|
||||
|
|
|
|||
|
|
@ -83,7 +83,9 @@ export async function applyDiffToolLegacy(
|
|||
cline.consecutiveMistakeCount++
|
||||
cline.recordToolError("apply_diff")
|
||||
const formattedError = `File does not exist at path: ${absolutePath}\n\n<error_details>\nThe specified file could not be found. Please verify the file path and try again.\n</error_details>`
|
||||
await cline.say("error", formattedError)
|
||||
await cline.say("error", formattedError, undefined, undefined, undefined, undefined, {
|
||||
metadata: { title: "File Not Found" },
|
||||
})
|
||||
pushToolResult(formattedError)
|
||||
return
|
||||
}
|
||||
|
|
|
|||
|
|
@ -87,7 +87,9 @@ export async function insertContentTool(
|
|||
cline.consecutiveMistakeCount++
|
||||
cline.recordToolError("insert_content")
|
||||
const formattedError = `Cannot insert content at line ${lineNumber} into a non-existent file. For new files, 'line' must be 0 (to append) or 1 (to insert at the beginning).`
|
||||
await cline.say("error", formattedError)
|
||||
await cline.say("error", formattedError, undefined, undefined, undefined, undefined, {
|
||||
metadata: { title: "Invalid Line Number" },
|
||||
})
|
||||
pushToolResult(formattedError)
|
||||
return
|
||||
}
|
||||
|
|
|
|||
|
|
@ -120,7 +120,9 @@ export const ChatRowContent = ({
|
|||
const { info: model } = useSelectedModel(apiConfiguration)
|
||||
const [reasoningCollapsed, setReasoningCollapsed] = useState(true)
|
||||
const [isDiffErrorExpanded, setIsDiffErrorExpanded] = useState(false)
|
||||
const [isErrorExpanded, setIsErrorExpanded] = useState(false)
|
||||
const [showCopySuccess, setShowCopySuccess] = useState(false)
|
||||
const [showErrorCopySuccess, setShowErrorCopySuccess] = useState(false)
|
||||
const [isEditing, setIsEditing] = useState(false)
|
||||
const [editedContent, setEditedContent] = useState("")
|
||||
const [editMode, setEditMode] = useState<Mode>(mode || "code")
|
||||
|
|
@ -1243,16 +1245,99 @@ export const ChatRowContent = ({
|
|||
</div>
|
||||
)
|
||||
case "error":
|
||||
// Extract custom title from metadata if available
|
||||
const errorTitle = (message as any).metadata?.title || t("chat:error")
|
||||
|
||||
return (
|
||||
<>
|
||||
{title && (
|
||||
<div style={headerStyle}>
|
||||
{icon}
|
||||
{title}
|
||||
<div>
|
||||
<div
|
||||
style={{
|
||||
marginTop: "0px",
|
||||
overflow: "hidden",
|
||||
marginBottom: "8px",
|
||||
}}>
|
||||
<div
|
||||
style={{
|
||||
borderBottom: isErrorExpanded
|
||||
? "1px solid var(--vscode-editorGroup-border)"
|
||||
: "none",
|
||||
fontWeight: "normal",
|
||||
fontSize: "var(--vscode-font-size)",
|
||||
color: "var(--vscode-editor-foreground)",
|
||||
display: "flex",
|
||||
alignItems: "center",
|
||||
justifyContent: "space-between",
|
||||
cursor: "pointer",
|
||||
}}
|
||||
onClick={() => setIsErrorExpanded(!isErrorExpanded)}>
|
||||
<div
|
||||
style={{
|
||||
display: "flex",
|
||||
alignItems: "center",
|
||||
gap: "10px",
|
||||
flexGrow: 1,
|
||||
}}>
|
||||
<span
|
||||
className="codicon codicon-error"
|
||||
style={{
|
||||
color: "var(--vscode-errorForeground)",
|
||||
fontSize: 16,
|
||||
marginBottom: "-1.5px",
|
||||
}}></span>
|
||||
<span style={{ fontWeight: "bold", color: "var(--vscode-errorForeground)" }}>
|
||||
{errorTitle}
|
||||
</span>
|
||||
</div>
|
||||
<div style={{ display: "flex", alignItems: "center" }}>
|
||||
<VSCodeButton
|
||||
appearance="icon"
|
||||
style={{
|
||||
padding: "3px",
|
||||
height: "24px",
|
||||
marginRight: "4px",
|
||||
color: "var(--vscode-editor-foreground)",
|
||||
display: "flex",
|
||||
alignItems: "center",
|
||||
justifyContent: "center",
|
||||
background: "transparent",
|
||||
}}
|
||||
onClick={(e) => {
|
||||
e.stopPropagation()
|
||||
|
||||
// Call copyWithFeedback and handle the Promise
|
||||
copyWithFeedback(message.text || "").then((success) => {
|
||||
if (success) {
|
||||
// Show checkmark
|
||||
setShowErrorCopySuccess(true)
|
||||
|
||||
// Reset after a brief delay
|
||||
setTimeout(() => {
|
||||
setShowErrorCopySuccess(false)
|
||||
}, 1000)
|
||||
}
|
||||
})
|
||||
}}>
|
||||
<span
|
||||
className={`codicon codicon-${showErrorCopySuccess ? "check" : "copy"}`}></span>
|
||||
</VSCodeButton>
|
||||
<span
|
||||
className={`codicon codicon-chevron-${isErrorExpanded ? "up" : "down"}`}></span>
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
<p style={{ ...pStyle, color: "var(--vscode-errorForeground)" }}>{message.text}</p>
|
||||
</>
|
||||
{isErrorExpanded && (
|
||||
<div
|
||||
style={{
|
||||
padding: "8px",
|
||||
backgroundColor: "var(--vscode-editor-background)",
|
||||
borderTop: "none",
|
||||
}}>
|
||||
<p style={{ ...pStyle, color: "var(--vscode-errorForeground)", margin: 0 }}>
|
||||
{message.text}
|
||||
</p>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
case "completion_result":
|
||||
return (
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue