diff --git a/implementation-plan.md b/implementation-plan.md new file mode 100644 index 0000000000..f78085621e --- /dev/null +++ b/implementation-plan.md @@ -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 diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index b6eb67e171..b3a1cd8111 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -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(), }) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index c5be865731..7a66647cc8 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1169,10 +1169,33 @@ export class Task extends EventEmitter 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 diff --git a/src/core/tools/__tests__/insertContentTool.spec.ts b/src/core/tools/__tests__/insertContentTool.spec.ts index 5f055fb29a..ae2ad6a9e3 100644 --- a/src/core/tools/__tests__/insertContentTool.spec.ts +++ b/src/core/tools/__tests__/insertContentTool.spec.ts @@ -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() }) diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts index 903e3c846e..583d39e734 100644 --- a/src/core/tools/applyDiffTool.ts +++ b/src/core/tools/applyDiffTool.ts @@ -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\nThe specified file could not be found. Please verify the file path and try again.\n` - await cline.say("error", formattedError) + await cline.say("error", formattedError, undefined, undefined, undefined, undefined, { + metadata: { title: "File Not Found" }, + }) pushToolResult(formattedError) return } diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts index e22a368167..7b2f417cbb 100644 --- a/src/core/tools/insertContentTool.ts +++ b/src/core/tools/insertContentTool.ts @@ -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 } diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 7b3107a2be..d96168b533 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -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 || "code") @@ -1243,16 +1245,99 @@ export const ChatRowContent = ({ ) case "error": + // Extract custom title from metadata if available + const errorTitle = (message as any).metadata?.title || t("chat:error") + return ( - <> - {title && ( -
- {icon} - {title} +
+
+
setIsErrorExpanded(!isErrorExpanded)}> +
+ + + {errorTitle} + +
+
+ { + 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) + } + }) + }}> + + + +
- )} -

{message.text}

- + {isErrorExpanded && ( +
+

+ {message.text} +

+
+ )} +
+
) case "completion_result": return (