mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
chore: remove accidentally committed temp files
This commit is contained in:
parent
b9ccbe5d24
commit
fb0296c4e7
5 changed files with 0 additions and 1975 deletions
|
|
@ -1 +0,0 @@
|
|||
[]
|
||||
|
|
@ -1,73 +0,0 @@
|
|||
author: copilot-pull-request-reviewer
|
||||
association: none
|
||||
edited: false
|
||||
status: commented
|
||||
--
|
||||
## Pull Request Overview
|
||||
|
||||
This PR enhances error display in the chat interface by introducing collapsible UI components and contextual error titles for improved user experience and clarity.
|
||||
|
||||
- Unified error display UX with collapsible headers, copy-to-clipboard functionality, and visual consistency between regular errors and diff errors
|
||||
- Added optional title field to messages for displaying contextual error information like "File Not Found" or "Tool Call Error: <tool_name>"
|
||||
- Updated all tool implementations to provide tool-specific error titles and propagate contextual information
|
||||
|
||||
### Reviewed Changes
|
||||
|
||||
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
|
||||
|
||||
<details>
|
||||
<summary>Show a summary per file</summary>
|
||||
|
||||
| File | Description |
|
||||
| ---- | ----------- |
|
||||
| webview-ui/src/components/chat/ChatRow.tsx | Implements collapsible error UI with copy functionality matching diff_error design |
|
||||
| webview-ui/src/components/chat/__tests__/ChatRow.spec.tsx | Comprehensive test suite for error UI behaviors and custom titles |
|
||||
| packages/types/src/message.ts | Adds optional title field to message schema |
|
||||
| src/core/task/Task.ts | Updates say() method to support custom error titles |
|
||||
| src/core/prompts/responses.ts | Enhances toolError formatter to include tool names |
|
||||
| src/core/prompts/__tests__/responses-tool-error.spec.ts | Tests for tool error formatting with/without tool names |
|
||||
| src/core/assistant-message/presentAssistantMessage.ts | Integrates tool names into error display |
|
||||
| src/core/tools/*.ts | Updates all tool files to pass tool names and contextual titles |
|
||||
| src/i18n/locales/en/tools.json | Adds common error title translations |
|
||||
</details>
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
|
||||
---
|
||||
|
||||
<sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/RooCodeInc/Roo-Code/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>
|
||||
--
|
||||
author: roomote
|
||||
association: none
|
||||
edited: false
|
||||
status: commented
|
||||
--
|
||||
Thank you for this excellent enhancement to error display! The implementation of collapsible UI and contextual titles is well done with great backward compatibility.
|
||||
|
||||
## Review Summary
|
||||
|
||||
**Positive Aspects:**
|
||||
- ✅ Excellent backward compatibility maintained with optional `message.title` field
|
||||
- ✅ Comprehensive test coverage added
|
||||
- ✅ Good UX parity between error and diff_error displays
|
||||
- ✅ Clean implementation of contextual tool error titles
|
||||
|
||||
**Suggestions for Improvement:**
|
||||
|
||||
1. **Accessibility improvements needed** - The collapsible error UI in `ChatRow.tsx` (lines 1109-1158) lacks proper accessibility attributes. Consider adding `role="button"`, `tabIndex={0}`, `aria-expanded`, and keyboard handlers for Enter/Space keys to match the diff_error implementation which already has these.
|
||||
|
||||
2. **Style consistency** - The error UI implementation uses inline style objects while the project guidelines specify using Tailwind CSS classes. The diff_error section already uses Tailwind classes correctly.
|
||||
|
||||
3. **i18n consistency issue** - The test file `ChatRow.spec.tsx` expects "Diff Error" as the title, but the actual locale file has "Edit Unsuccessful". This mismatch could cause test failures.
|
||||
|
||||
4. **Code duplication** - The collapsible header logic is duplicated between error and diff_error implementations. Consider extracting a shared `CollapsibleHeader` component.
|
||||
|
||||
5. **Language detection for CodeBlock** - Generic errors are rendered with `language="xml"`, but not all errors are XML. Consider using "text" by default or auto-detecting when `<error>` tags are present.
|
||||
|
||||
6. **Consider centralizing error titles** - Common error titles like "File Not Found", "Parse Error" etc. are currently hardcoded. Consider moving them to i18n files for better maintainability.
|
||||
|
||||
Overall, this is a solid implementation that significantly improves the error display UX. The suggestions above are mostly about code consistency and maintainability rather than functionality issues.
|
||||
--
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because it is too large
Load diff
|
|
@ -1,146 +0,0 @@
|
|||
[
|
||||
{
|
||||
"id": 3153264713,
|
||||
"node_id": "PRR_kwDONIq5lM678wBJ",
|
||||
"user": {
|
||||
"login": "copilot-pull-request-reviewer[bot]",
|
||||
"id": 175728472,
|
||||
"node_id": "BOT_kgDOCnlnWA",
|
||||
"avatar_url": "https://avatars.githubusercontent.com/in/946600?v=4",
|
||||
"gravatar_id": "",
|
||||
"url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D",
|
||||
"html_url": "https://github.com/apps/copilot-pull-request-reviewer",
|
||||
"followers_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/followers",
|
||||
"following_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/following{/other_user}",
|
||||
"gists_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/gists{/gist_id}",
|
||||
"starred_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/starred{/owner}{/repo}",
|
||||
"subscriptions_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/subscriptions",
|
||||
"organizations_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/orgs",
|
||||
"repos_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/repos",
|
||||
"events_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/events{/privacy}",
|
||||
"received_events_url": "https://api.github.com/users/copilot-pull-request-reviewer%5Bbot%5D/received_events",
|
||||
"type": "Bot",
|
||||
"user_view_type": "public",
|
||||
"site_admin": false
|
||||
},
|
||||
"body": "## Pull Request Overview\n\nThis PR enhances error display in the chat interface by introducing collapsible UI components and contextual error titles for improved user experience and clarity.\n\n- Unified error display UX with collapsible headers, copy-to-clipboard functionality, and visual consistency between regular errors and diff errors\n- Added optional title field to messages for displaying contextual error information like \"File Not Found\" or \"Tool Call Error: <tool_name>\"\n- Updated all tool implementations to provide tool-specific error titles and propagate contextual information\n\n### Reviewed Changes\n\nCopilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.\n\n<details>\n<summary>Show a summary per file</summary>\n\n| File | Description |\r\n| ---- | ----------- |\r\n| webview-ui/src/components/chat/ChatRow.tsx | Implements collapsible error UI with copy functionality matching diff_error design |\r\n| webview-ui/src/components/chat/__tests__/ChatRow.spec.tsx | Comprehensive test suite for error UI behaviors and custom titles |\r\n| packages/types/src/message.ts | Adds optional title field to message schema |\r\n| src/core/task/Task.ts | Updates say() method to support custom error titles |\r\n| src/core/prompts/responses.ts | Enhances toolError formatter to include tool names |\r\n| src/core/prompts/__tests__/responses-tool-error.spec.ts | Tests for tool error formatting with/without tool names |\r\n| src/core/assistant-message/presentAssistantMessage.ts | Integrates tool names into error display |\r\n| src/core/tools/*.ts | Updates all tool files to pass tool names and contextual titles |\r\n| src/i18n/locales/en/tools.json | Adds common error title translations |\n</details>\n\n\n\n\n\n\n---\n\n<sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href=\"/RooCodeInc/Roo-Code/new/main/.github?filename=copilot-instructions.md\" class=\"Link--inTextBlock\" target=\"_blank\" rel=\"noopener noreferrer\">Create the file</a> or <a href=\"https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot\" class=\"Link--inTextBlock\" target=\"_blank\" rel=\"noopener noreferrer\">learn how to get started</a>.</sub>",
|
||||
"state": "COMMENTED",
|
||||
"html_url": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153264713",
|
||||
"pull_request_url": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401",
|
||||
"author_association": "NONE",
|
||||
"_links": {
|
||||
"html": { "href": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153264713" },
|
||||
"pull_request": { "href": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401" }
|
||||
},
|
||||
"submitted_at": "2025-08-25T22:52:53Z",
|
||||
"commit_id": "f4238a37135f1fd3af2a35f5dbce5c124116eb64"
|
||||
},
|
||||
{
|
||||
"id": 3153265715,
|
||||
"node_id": "PRR_kwDONIq5lM678wQz",
|
||||
"user": {
|
||||
"login": "ellipsis-dev[bot]",
|
||||
"id": 65095814,
|
||||
"node_id": "MDM6Qm90NjUwOTU4MTQ=",
|
||||
"avatar_url": "https://avatars.githubusercontent.com/in/64358?v=4",
|
||||
"gravatar_id": "",
|
||||
"url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D",
|
||||
"html_url": "https://github.com/apps/ellipsis-dev",
|
||||
"followers_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/followers",
|
||||
"following_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/following{/other_user}",
|
||||
"gists_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/gists{/gist_id}",
|
||||
"starred_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/starred{/owner}{/repo}",
|
||||
"subscriptions_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/subscriptions",
|
||||
"organizations_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/orgs",
|
||||
"repos_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/repos",
|
||||
"events_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/events{/privacy}",
|
||||
"received_events_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/received_events",
|
||||
"type": "Bot",
|
||||
"user_view_type": "public",
|
||||
"site_admin": false
|
||||
},
|
||||
"body": "",
|
||||
"state": "COMMENTED",
|
||||
"html_url": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153265715",
|
||||
"pull_request_url": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401",
|
||||
"author_association": "NONE",
|
||||
"_links": {
|
||||
"html": { "href": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153265715" },
|
||||
"pull_request": { "href": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401" }
|
||||
},
|
||||
"submitted_at": "2025-08-25T22:53:41Z",
|
||||
"commit_id": "f4238a37135f1fd3af2a35f5dbce5c124116eb64"
|
||||
},
|
||||
{
|
||||
"id": 3153272897,
|
||||
"node_id": "PRR_kwDONIq5lM678yBB",
|
||||
"user": {
|
||||
"login": "roomote[bot]",
|
||||
"id": 219738659,
|
||||
"node_id": "BOT_kgDODRjyIw",
|
||||
"avatar_url": "https://avatars.githubusercontent.com/in/1546624?v=4",
|
||||
"gravatar_id": "",
|
||||
"url": "https://api.github.com/users/roomote%5Bbot%5D",
|
||||
"html_url": "https://github.com/apps/roomote",
|
||||
"followers_url": "https://api.github.com/users/roomote%5Bbot%5D/followers",
|
||||
"following_url": "https://api.github.com/users/roomote%5Bbot%5D/following{/other_user}",
|
||||
"gists_url": "https://api.github.com/users/roomote%5Bbot%5D/gists{/gist_id}",
|
||||
"starred_url": "https://api.github.com/users/roomote%5Bbot%5D/starred{/owner}{/repo}",
|
||||
"subscriptions_url": "https://api.github.com/users/roomote%5Bbot%5D/subscriptions",
|
||||
"organizations_url": "https://api.github.com/users/roomote%5Bbot%5D/orgs",
|
||||
"repos_url": "https://api.github.com/users/roomote%5Bbot%5D/repos",
|
||||
"events_url": "https://api.github.com/users/roomote%5Bbot%5D/events{/privacy}",
|
||||
"received_events_url": "https://api.github.com/users/roomote%5Bbot%5D/received_events",
|
||||
"type": "Bot",
|
||||
"user_view_type": "public",
|
||||
"site_admin": false
|
||||
},
|
||||
"body": "Thank you for this excellent enhancement to error display! The implementation of collapsible UI and contextual titles is well done with great backward compatibility.\n\n## Review Summary\n\n**Positive Aspects:**\n- ✅ Excellent backward compatibility maintained with optional `message.title` field\n- ✅ Comprehensive test coverage added\n- ✅ Good UX parity between error and diff_error displays\n- ✅ Clean implementation of contextual tool error titles\n\n**Suggestions for Improvement:**\n\n1. **Accessibility improvements needed** - The collapsible error UI in `ChatRow.tsx` (lines 1109-1158) lacks proper accessibility attributes. Consider adding `role=\"button\"`, `tabIndex={0}`, `aria-expanded`, and keyboard handlers for Enter/Space keys to match the diff_error implementation which already has these.\n\n2. **Style consistency** - The error UI implementation uses inline style objects while the project guidelines specify using Tailwind CSS classes. The diff_error section already uses Tailwind classes correctly.\n\n3. **i18n consistency issue** - The test file `ChatRow.spec.tsx` expects \"Diff Error\" as the title, but the actual locale file has \"Edit Unsuccessful\". This mismatch could cause test failures.\n\n4. **Code duplication** - The collapsible header logic is duplicated between error and diff_error implementations. Consider extracting a shared `CollapsibleHeader` component.\n\n5. **Language detection for CodeBlock** - Generic errors are rendered with `language=\"xml\"`, but not all errors are XML. Consider using \"text\" by default or auto-detecting when `<error>` tags are present.\n\n6. **Consider centralizing error titles** - Common error titles like \"File Not Found\", \"Parse Error\" etc. are currently hardcoded. Consider moving them to i18n files for better maintainability.\n\nOverall, this is a solid implementation that significantly improves the error display UX. The suggestions above are mostly about code consistency and maintainability rather than functionality issues.",
|
||||
"state": "COMMENTED",
|
||||
"html_url": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153272897",
|
||||
"pull_request_url": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401",
|
||||
"author_association": "NONE",
|
||||
"_links": {
|
||||
"html": { "href": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153272897" },
|
||||
"pull_request": { "href": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401" }
|
||||
},
|
||||
"submitted_at": "2025-08-25T22:58:37Z",
|
||||
"commit_id": "f4238a37135f1fd3af2a35f5dbce5c124116eb64"
|
||||
},
|
||||
{
|
||||
"id": 3153309836,
|
||||
"node_id": "PRR_kwDONIq5lM6787CM",
|
||||
"user": {
|
||||
"login": "ellipsis-dev[bot]",
|
||||
"id": 65095814,
|
||||
"node_id": "MDM6Qm90NjUwOTU4MTQ=",
|
||||
"avatar_url": "https://avatars.githubusercontent.com/in/64358?v=4",
|
||||
"gravatar_id": "",
|
||||
"url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D",
|
||||
"html_url": "https://github.com/apps/ellipsis-dev",
|
||||
"followers_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/followers",
|
||||
"following_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/following{/other_user}",
|
||||
"gists_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/gists{/gist_id}",
|
||||
"starred_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/starred{/owner}{/repo}",
|
||||
"subscriptions_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/subscriptions",
|
||||
"organizations_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/orgs",
|
||||
"repos_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/repos",
|
||||
"events_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/events{/privacy}",
|
||||
"received_events_url": "https://api.github.com/users/ellipsis-dev%5Bbot%5D/received_events",
|
||||
"type": "Bot",
|
||||
"user_view_type": "public",
|
||||
"site_admin": false
|
||||
},
|
||||
"body": "",
|
||||
"state": "COMMENTED",
|
||||
"html_url": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153309836",
|
||||
"pull_request_url": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401",
|
||||
"author_association": "NONE",
|
||||
"_links": {
|
||||
"html": { "href": "https://github.com/RooCodeInc/Roo-Code/pull/7401#pullrequestreview-3153309836" },
|
||||
"pull_request": { "href": "https://api.github.com/repos/RooCodeInc/Roo-Code/pulls/7401" }
|
||||
},
|
||||
"submitted_at": "2025-08-25T23:22:19Z",
|
||||
"commit_id": "91c53b99ac197709539fc40e27b8c9aa765982b6"
|
||||
}
|
||||
]
|
||||
Loading…
Add table
Reference in a new issue