-
Notifications
You must be signed in to change notification settings - Fork 0
Release/v5.1.0 #32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release/v5.1.0 #32
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧪 PR Review is completed: The PR introduces diff viewing and line counting features. There is a critical logic issue in fileEditReviewAcceptAll where pending edits are cleared before being accessed for language detection. Also identified potential UI bugs in ReasoningBlock and robustness improvements for URI handling and network requests.
Skipped files
CHANGELOG.md: Skipped file patternwebview-ui/src/i18n/locales/ar/chat.json: Skipped file patternwebview-ui/src/i18n/locales/ca/chat.json: Skipped file patternwebview-ui/src/i18n/locales/cs/chat.json: Skipped file patternwebview-ui/src/i18n/locales/de/chat.json: Skipped file patternwebview-ui/src/i18n/locales/en/chat.json: Skipped file patternwebview-ui/src/i18n/locales/es/chat.json: Skipped file patternwebview-ui/src/i18n/locales/fr/chat.json: Skipped file patternwebview-ui/src/i18n/locales/hi/chat.json: Skipped file patternwebview-ui/src/i18n/locales/id/chat.json: Skipped file patternwebview-ui/src/i18n/locales/it/chat.json: Skipped file patternwebview-ui/src/i18n/locales/ja/chat.json: Skipped file patternwebview-ui/src/i18n/locales/ko/chat.json: Skipped file patternwebview-ui/src/i18n/locales/nl/chat.json: Skipped file patternwebview-ui/src/i18n/locales/pl/chat.json: Skipped file patternwebview-ui/src/i18n/locales/pt-BR/chat.json: Skipped file patternwebview-ui/src/i18n/locales/ru/chat.json: Skipped file patternwebview-ui/src/i18n/locales/th/chat.json: Skipped file patternwebview-ui/src/i18n/locales/tr/chat.json: Skipped file patternwebview-ui/src/i18n/locales/uk/chat.json: Skipped file patternwebview-ui/src/i18n/locales/vi/chat.json: Skipped file patternwebview-ui/src/i18n/locales/zh-CN/chat.json: Skipped file patternwebview-ui/src/i18n/locales/zh-TW/chat.json: Skipped file pattern
⬇️ Low Priority Suggestions (1)
src/core/webview/webviewMessageHandler.ts (1 suggestion)
Location:
src/core/webview/webviewMessageHandler.ts(Lines 980-981)🔵 Code Quality
Issue: Unnecessary
as anycast on the switch case label. TheWebviewMessagetype definition has been updated to includeviewPendingFileDiffs, so the cast suppresses valuable type checking.Fix: Remove
as any.Impact: Improves type safety.
- // kilocode_change start: View pending file diffs in VS Code diff view - case "viewPendingFileDiffs" as any: { + // kilocode_change start: View pending file diffs in VS Code diff view + case "viewPendingFileDiffs": {
| // Execute the accept all command and get line counters | ||
| const lineCounters = await vscode.commands.executeCommand< | ||
| { linesAdded: number; linesUpdated: number; linesDeleted: number } | undefined | ||
| >("axon-code.fileEdit.acceptAll") | ||
|
|
||
| // Send line counters to server if we have data | ||
| if ( | ||
| lineCounters && | ||
| (lineCounters.linesAdded > 0 || lineCounters.linesUpdated > 0 || lineCounters.linesDeleted > 0) | ||
| ) { | ||
| try { | ||
| // Get state to retrieve kilocodeToken | ||
| const state = await provider.getState() | ||
| const kilocodeToken = state.apiConfiguration?.kilocodeToken | ||
|
|
||
| if (!kilocodeToken) { | ||
| provider.log("KiloCode token not found, skipping line counter update") | ||
| break | ||
| } | ||
|
|
||
| // Get git repository info for x-axon-repo header | ||
| const { getGitRepositoryInfo } = await import("../../utils/git") | ||
| const gitInfo = await getGitRepositoryInfo(provider.cwd) | ||
| const repo = gitInfo.repositoryUrl || path.basename(provider.cwd) | ||
|
|
||
| // Get the primary language from the first edited file | ||
| const pendingEdits = currentTask.fileEditReviewController.getPendingEdits() | ||
| let language = "ts" // default | ||
| if (pendingEdits.size > 0) { | ||
| const firstFile = Array.from(pendingEdits.keys())[0] | ||
| const ext = path.extname(firstFile).toLowerCase() | ||
| const languageMap: Record<string, string> = { | ||
| ".ts": "ts", | ||
| ".tsx": "tsx", | ||
| ".js": "js", | ||
| ".jsx": "jsx", | ||
| ".py": "py", | ||
| ".java": "java", | ||
| ".go": "go", | ||
| ".rs": "rs", | ||
| ".cpp": "cpp", | ||
| ".c": "c", | ||
| ".cs": "cs", | ||
| ".php": "php", | ||
| ".rb": "rb", | ||
| ".swift": "swift", | ||
| ".kt": "kt", | ||
| ".dart": "dart", | ||
| ".vue": "vue", | ||
| ".svelte": "svelte", | ||
| } | ||
| language = languageMap[ext] || "ts" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Logic Error
Issue: pendingEdits are retrieved after axon-code.fileEdit.acceptAll command execution. Since the accept command clears the pending edits in FileEditReviewController, pendingEdits will always be empty here, causing the language detection to always default to "ts".
Fix: Retrieve the pending edits and determine the language before executing the accept command.
Impact: Incorrect language reporting in metrics.
| // Execute the accept all command and get line counters | |
| const lineCounters = await vscode.commands.executeCommand< | |
| { linesAdded: number; linesUpdated: number; linesDeleted: number } | undefined | |
| >("axon-code.fileEdit.acceptAll") | |
| // Send line counters to server if we have data | |
| if ( | |
| lineCounters && | |
| (lineCounters.linesAdded > 0 || lineCounters.linesUpdated > 0 || lineCounters.linesDeleted > 0) | |
| ) { | |
| try { | |
| // Get state to retrieve kilocodeToken | |
| const state = await provider.getState() | |
| const kilocodeToken = state.apiConfiguration?.kilocodeToken | |
| if (!kilocodeToken) { | |
| provider.log("KiloCode token not found, skipping line counter update") | |
| break | |
| } | |
| // Get git repository info for x-axon-repo header | |
| const { getGitRepositoryInfo } = await import("../../utils/git") | |
| const gitInfo = await getGitRepositoryInfo(provider.cwd) | |
| const repo = gitInfo.repositoryUrl || path.basename(provider.cwd) | |
| // Get the primary language from the first edited file | |
| const pendingEdits = currentTask.fileEditReviewController.getPendingEdits() | |
| let language = "ts" // default | |
| if (pendingEdits.size > 0) { | |
| const firstFile = Array.from(pendingEdits.keys())[0] | |
| const ext = path.extname(firstFile).toLowerCase() | |
| const languageMap: Record<string, string> = { | |
| ".ts": "ts", | |
| ".tsx": "tsx", | |
| ".js": "js", | |
| ".jsx": "jsx", | |
| ".py": "py", | |
| ".java": "java", | |
| ".go": "go", | |
| ".rs": "rs", | |
| ".cpp": "cpp", | |
| ".c": "c", | |
| ".cs": "cs", | |
| ".php": "php", | |
| ".rb": "rb", | |
| ".swift": "swift", | |
| ".kt": "kt", | |
| ".dart": "dart", | |
| ".vue": "vue", | |
| ".svelte": "svelte", | |
| } | |
| language = languageMap[ext] || "ts" | |
| } | |
| // Get the primary language from the first edited file before they are cleared | |
| const pendingEdits = currentTask.fileEditReviewController.getPendingEdits() | |
| let language = "ts" // default | |
| if (pendingEdits.size > 0) { | |
| const firstFile = Array.from(pendingEdits.keys())[0] | |
| const ext = path.extname(firstFile).toLowerCase() | |
| const languageMap: Record<string, string> = { | |
| ".ts": "ts", | |
| ".tsx": "tsx", | |
| ".js": "js", | |
| ".jsx": "jsx", | |
| ".py": "py", | |
| ".java": "java", | |
| ".go": "go", | |
| ".rs": "rs", | |
| ".cpp": "cpp", | |
| ".c": "c", | |
| ".cs": "cs", | |
| ".php": "php", | |
| ".rb": "rb", | |
| ".swift": "swift", | |
| ".kt": "kt", | |
| ".dart": "dart", | |
| ".vue": "vue", | |
| ".svelte": "svelte", | |
| } | |
| language = languageMap[ext] || "ts" | |
| } | |
| // Execute the accept all command and get line counters | |
| const lineCounters = await vscode.commands.executeCommand< | |
| { linesAdded: number; linesUpdated: number; linesDeleted: number } | undefined | |
| >("axon-code.fileEdit.acceptAll") | |
| // Send line counters to server if we have data | |
| if ( | |
| lineCounters && | |
| (lineCounters.linesAdded > 0 || lineCounters.linesUpdated > 0 || lineCounters.linesDeleted > 0) | |
| ) { | |
| try { | |
| // Get state to retrieve kilocodeToken | |
| const state = await provider.getState() | |
| const kilocodeToken = state.apiConfiguration?.kilocodeToken | |
| if (!kilocodeToken) { | |
| provider.log("KiloCode token not found, skipping line counter update") | |
| break | |
| } | |
| // Get git repository info for x-axon-repo header | |
| const { getGitRepositoryInfo } = await import("../../utils/git") | |
| const gitInfo = await getGitRepositoryInfo(provider.cwd) | |
| const repo = gitInfo.repositoryUrl || path.basename(provider.cwd) |
| const url = `https://api.matterai.so/axoncode/meta/${currentTask.taskId}/lines` | ||
| const response = await fetch(url, { | ||
| method: "POST", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Robustness
Issue: The fetch request lacks a timeout configuration. If the API endpoint hangs, this operation could remain pending indefinitely.
Fix: Add an AbortSignal with a reasonable timeout (e.g., 10 seconds).
Impact: Prevents potential resource leaks and hanging operations.
| const url = `https://api.matterai.so/axoncode/meta/${currentTask.taskId}/lines` | |
| const response = await fetch(url, { | |
| method: "POST", | |
| // Send POST request to update line counters | |
| const url = `https://api.matterai.so/axoncode/meta/${currentTask.taskId}/lines` | |
| const response = await fetch(url, { | |
| signal: AbortSignal.timeout(10000), | |
| method: "POST", |
| const originalUri = vscode.Uri.parse(`cline-diff:${fileName}`).with({ | ||
| query: Buffer.from(edit.originalContent).toString("base64"), | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Code Quality
Issue: Using vscode.Uri.parse with a constructed string cline-diff:${fileName} is unsafe if fileName contains spaces or special characters that are not URL-encoded.
Fix: Use vscode.Uri.from which handles path encoding correctly.
Impact: Ensures reliable URI creation for files with special characters.
| const originalUri = vscode.Uri.parse(`cline-diff:${fileName}`).with({ | |
| query: Buffer.from(edit.originalContent).toString("base64"), | |
| }) | |
| // Create a URI for the original content using the diff view scheme | |
| const originalUri = vscode.Uri.from({ | |
| scheme: "cline-diff", | |
| path: fileName, | |
| query: Buffer.from(edit.originalContent).toString("base64"), | |
| }) |
|
|
||
| const seconds = Math.floor(elapsed / 1000) | ||
| const secondsLabel = t("chat:reasoning.seconds", { count: seconds }) | ||
| const displayElapsed = isStreaming ? elapsed : finalElapsed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 UI Bug
Issue: displayElapsed uses isStreaming to decide whether to show elapsed or finalElapsed. If isStreaming is true (global chat state) but the current block is finished (partial is false), it incorrectly shows elapsed (which is reset to 0) instead of finalElapsed.
Fix: Use the partial prop to determine if the block is still active.
Impact: Correctly displays the final duration for completed reasoning blocks while the chat is still streaming.
| const displayElapsed = isStreaming ? elapsed : finalElapsed | |
| const displayElapsed = partial ? elapsed : finalElapsed |
|
/matter review-max |
|
I couldn't clone the repository. |
|
/matter review-max |
1 similar comment
|
/matter review-max |
No description provided.