Conversation
code-crusher
commented
Mar 12, 2026
- feat: add fade-up and shimmer animations to ChatRow, ChatView, and ReasoningBlock (feat: add fade-up and shimmer animations to ChatRow, ChatView, and ReasoningBlock #65)
- improve reasoning flow + time tracking (release/v5.6.2 #66)
- release/v5.6.3 (release/v5.6.3 #67)
- add upgrade button
- cleanup image attachements
- improve fuzzy search + chat text lock on image add
- cleanup settings
- cleanup settings
- cleanup settings ++
- add email in account settings
- uniform theme in settings
There was a problem hiding this comment.
🧪 PR Review is completed: The PR introduces solid improvements to file search ranking and webview UI components. However, there are a few performance optimizations and UI bugs that need to be addressed, particularly regarding redundant string operations during sorting and invalid Tailwind opacity modifiers on CSS variables.
Skipped files
webview-ui/src/i18n/locales/en/kilocode.json: Skipped file patternwebview-ui/src/i18n/locales/en/settings.json: Skipped file pattern
⬇️ Low Priority Suggestions (2)
webview-ui/src/components/settings/SettingsView.tsx (1 suggestion)
Location:
webview-ui/src/components/settings/SettingsView.tsx(Lines 702-702)🔵 UI/UX Improvement
Issue: When
profileEmailis in its initial state of"loading...", the avatar will display"L"which might look like a user's initial.Fix: Check if the email is
"loading..."before using its first character, or use a generic fallback like"U"or an icon until the actual email is loaded.Impact: Prevents confusing UI state during initial load.
- {profileEmail ? profileEmail.charAt(0).toUpperCase() : "S"} + {profileEmail && profileEmail !== "loading..." ? profileEmail.charAt(0).toUpperCase() : "U"}
webview-ui/src/components/chat/ChatTextArea.tsx (1 suggestion)
Location:
webview-ui/src/components/chat/ChatTextArea.tsx(Lines 1616-1624)🟡 UX Improvement
Issue: When clicking the "Add Context (@)" button, it always prepends a space (
" @"), even if the input is empty or already ends with a space. This can lead to leading spaces or double spaces.Fix: Conditionally add the space only if there is preceding text that doesn't end with a space.
Impact: Cleaner input formatting without unnecessary spaces.
- const nextValue = - currentValue.slice(0, currentCursorPosition) + - " @" + - currentValue.slice(currentCursorPosition) - const nextCursorPosition = currentCursorPosition + 2 - - setInputValue(nextValue) - setCursorPosition(nextCursorPosition) - intendedCursorPositionRef.current = nextCursorPosition + const prefix = currentValue.slice(0, currentCursorPosition) + const insertText = prefix.length > 0 && !prefix.endsWith(" ") ? " @" : "@" + const nextValue = + prefix + + insertText + + currentValue.slice(currentCursorPosition) + const nextCursorPosition = currentCursorPosition + insertText.length + + setInputValue(nextValue) + setCursorPosition(nextCursorPosition) + intendedCursorPositionRef.current = nextCursorPosition
| className="flex items-center gap-1 px-2.5 py-0.5 rounded-full bg-[var(--vscode-button-background)]/15 hover:bg-[var(--vscode-button-hoverBackground)] hover:text-[var(--vscode-button-foreground)] text-[var(--vscode-button-background)] text-xs font-medium transition-all duration-200 hover:scale-105" | ||
| onClick={() => openExternalLink("https://app.matterai.so/orbital")} |
There was a problem hiding this comment.
🔴 UI Bug
Issue: Tailwind's opacity modifier (/15) does not work with VS Code's CSS variables because they are defined as hex colors (e.g., #007acc), not RGB channels. This will result in invalid CSS (rgb(#007acc / 0.15)) and the background color will not be applied.
Fix: Use CSS color-mix to apply opacity to hex CSS variables, or use a solid color if transparency isn't strictly required.
Impact: Ensures the button renders with the correct background color.
| className="flex items-center gap-1 px-2.5 py-0.5 rounded-full bg-[var(--vscode-button-background)]/15 hover:bg-[var(--vscode-button-hoverBackground)] hover:text-[var(--vscode-button-foreground)] text-[var(--vscode-button-background)] text-xs font-medium transition-all duration-200 hover:scale-105" | |
| onClick={() => openExternalLink("https://app.matterai.so/orbital")} | |
| className="flex items-center gap-1 px-2.5 py-0.5 rounded-full hover:bg-[var(--vscode-button-hoverBackground)] hover:text-[var(--vscode-button-foreground)] text-[var(--vscode-button-background)] text-xs font-medium transition-all duration-200 hover:scale-105" | |
| style={{ backgroundColor: "color-mix(in srgb, var(--vscode-button-background) 15%, transparent)" }} | |
| onClick={() => openExternalLink("https://app.matterai.so/orbital")} |
| return [...results].sort((left, right) => { | ||
| const a = getRankSignals(left, normalizedQuery) | ||
| const b = getRankSignals(right, normalizedQuery) | ||
|
|
||
| return ( | ||
| b.exactBasename - a.exactBasename || | ||
| b.extensionExactness - a.extensionExactness || | ||
| b.exactPathSuffix - a.exactPathSuffix || | ||
| b.basenameStartsWith - a.basenameStartsWith || | ||
| b.pathSegmentMatch - a.pathSegmentMatch || | ||
| b.basenameIncludes - a.basenameIncludes || | ||
| b.isFile - a.isFile || | ||
| a.basenameLength - b.basenameLength || | ||
| a.pathLength - b.pathLength || | ||
| left.path.localeCompare(right.path) | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🟠 Performance
Issue: getRankSignals performs multiple string operations and is called inside the sort comparator. For an array of N items, the comparator runs O(N log N) times, leading to redundant calculations and potential performance bottlenecks during search.
Fix: Use the Schwartzian transform pattern to pre-calculate the rank signals once per item before sorting.
Impact: Significantly improves search ranking performance by reducing redundant string operations.
| return [...results].sort((left, right) => { | |
| const a = getRankSignals(left, normalizedQuery) | |
| const b = getRankSignals(right, normalizedQuery) | |
| return ( | |
| b.exactBasename - a.exactBasename || | |
| b.extensionExactness - a.extensionExactness || | |
| b.exactPathSuffix - a.exactPathSuffix || | |
| b.basenameStartsWith - a.basenameStartsWith || | |
| b.pathSegmentMatch - a.pathSegmentMatch || | |
| b.basenameIncludes - a.basenameIncludes || | |
| b.isFile - a.isFile || | |
| a.basenameLength - b.basenameLength || | |
| a.pathLength - b.pathLength || | |
| left.path.localeCompare(right.path) | |
| ) | |
| }) | |
| const withSignals = results.map((item) => ({ item, signals: getRankSignals(item, normalizedQuery) })) | |
| return withSignals | |
| .sort((left, right) => { | |
| const a = left.signals | |
| const b = right.signals | |
| return ( | |
| b.exactBasename - a.exactBasename || | |
| b.extensionExactness - a.extensionExactness || | |
| b.exactPathSuffix - a.exactPathSuffix || | |
| b.basenameStartsWith - a.basenameStartsWith || | |
| b.pathSegmentMatch - a.pathSegmentMatch || | |
| b.basenameIncludes - a.basenameIncludes || | |
| b.isFile - a.isFile || | |
| a.basenameLength - b.basenameLength || | |
| a.pathLength - b.pathLength || | |
| left.item.path.localeCompare(right.item.path) | |
| ) | |
| }) | |
| .map((obj) => obj.item) |
| return [...items].sort((left, right) => { | ||
| const a = getSignals(left) | ||
| const b = getSignals(right) | ||
|
|
||
| return ( | ||
| b.exactBasename - a.exactBasename || | ||
| b.extensionExactness - a.extensionExactness || | ||
| b.exactPathSuffix - a.exactPathSuffix || | ||
| b.basenameStartsWith - a.basenameStartsWith || | ||
| b.basenameIncludes - a.basenameIncludes || | ||
| b.isFile - a.isFile || | ||
| a.basenameLength - b.basenameLength || | ||
| a.pathLength - b.pathLength || | ||
| (left.value || "").localeCompare(right.value || "") | ||
| ) | ||
| }) |
There was a problem hiding this comment.
🟠 Performance
Issue: getSignals is called inside the sort comparator, which runs O(N log N) times. This causes redundant string operations for the same items during sorting.
Fix: Pre-calculate the signals before sorting (Schwartzian transform) to improve performance.
Impact: Improves context menu rendering performance, especially with a large number of file matches.
| return [...items].sort((left, right) => { | |
| const a = getSignals(left) | |
| const b = getSignals(right) | |
| return ( | |
| b.exactBasename - a.exactBasename || | |
| b.extensionExactness - a.extensionExactness || | |
| b.exactPathSuffix - a.exactPathSuffix || | |
| b.basenameStartsWith - a.basenameStartsWith || | |
| b.basenameIncludes - a.basenameIncludes || | |
| b.isFile - a.isFile || | |
| a.basenameLength - b.basenameLength || | |
| a.pathLength - b.pathLength || | |
| (left.value || "").localeCompare(right.value || "") | |
| ) | |
| }) | |
| const withSignals = items.map((item) => ({ item, signals: getSignals(item) })) | |
| return withSignals | |
| .sort((left, right) => { | |
| const a = left.signals | |
| const b = right.signals | |
| return ( | |
| b.exactBasename - a.exactBasename || | |
| b.extensionExactness - a.extensionExactness || | |
| b.exactPathSuffix - a.exactPathSuffix || | |
| b.basenameStartsWith - a.basenameStartsWith || | |
| b.basenameIncludes - a.basenameIncludes || | |
| b.isFile - a.isFile || | |
| a.basenameLength - b.basenameLength || | |
| a.pathLength - b.pathLength || | |
| (left.item.value || "").localeCompare(right.item.value || "") | |
| ) | |
| }) | |
| .map((obj) => obj.item) |