cleaner UI + fixes for fill-diff viewer + gemini models#16
cleaner UI + fixes for fill-diff viewer + gemini models#16code-crusher merged 2 commits intomainfrom
Conversation
ContextThis pull request, titled "cleaner UI + fixes for fill-diff viewer + gemini models", primarily focuses on commenting out the definitions for Gemini 3 Flash and Pro preview models within the application's model provider configurations and utility functions. While the title suggests broader UI and diff viewer improvements, the provided code changes are limited to these model definitions. ImplementationThe Gemini 3 Flash and Pro preview model definitions, including their IDs, names, descriptions, modalities, context lengths, sampling parameters, supported features, OpenRouter slugs, datacenters, creation timestamps, ownership, and pricing, have been commented out in Screenshots
How to Test
Get in TouchN/A Summary By MatterAI
🔄 What ChangedThis PR comments out the full definitions for Gemini 3 Flash and Pro preview models across 🔍 Impact of the ChangeUsers will temporarily lose access to Gemini 3 Flash and Pro preview models. This change likely serves as a temporary disablement or preparation for future model integration, ensuring these models are not inadvertently exposed or used in their current state. The impact of UI/diff viewer fixes mentioned in the PR title cannot be assessed from the provided code. 📁 Total Files Changed
Click to Expand
|
|
✅ Reviewed the changes: Review of UI improvements, diff viewer fixes, and Gemini model integration. Found several issues including potential XSS vulnerabilities, performance concerns with string operations, and React best practices violations. |
|
✅ Reviewed the changes: Reviewed the addition of commented-out Gemini model definitions across 3 files. The changes are structurally sound and follow existing patterns, but require attention to consistency and future activation readiness. |
|
/matter pls generate a crisp changelog for mintlify docs like |
## v4.202.0
Added
Changed
Fixed
|
|
/matter review-full |
![matter-ai-bot[bot]](https://avatars.githubusercontent.com/in/474549?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧪 PR Review is completed: Review of UI components and logic improvements. Identified issues with path handling for file operations, redundant text truncation logic conflicting with CSS, and opportunities for regex optimization.
Skipped files
webview-ui/src/i18n/locales/en/chat.json: Skipped file pattern
⬇️ Low Priority Suggestions (3)
webview-ui/src/components/chat/ErrorRow.tsx (1 suggestion)
Location:
webview-ui/src/components/chat/ErrorRow.tsx(Lines 112-116)🔵 Dead Code
Issue: The
truncateMessagehelper function is no longer needed if we rely on CSS truncation.Fix: Remove the unused function.
Impact: Cleans up unused code.
- // Truncate message for display - const truncateMessage = (msg: string, maxLength: number = 30) => { - if (msg.length <= maxLength) return msg - return msg.substring(0, maxLength) + "..." - } +
webview-ui/src/components/chat/BatchFilePermission.tsx (1 suggestion)
Location:
webview-ui/src/components/chat/BatchFilePermission.tsx(Lines 40-45)🔵 Code Quality
Issue: The string manipulation logic for
file.lineSnippetis redundant (calculated twice) and uses chainedreplaceAllcalls which can be simplified.Fix: Use a single regex replacement and simplify the conditional rendering.
Impact: Improves readability and performance.
- {file.lineSnippet - ?.replace("lines", "#L") - ?.replaceAll(" ", "") - .replaceAll("(", "") - .replaceAll(")", "") && - ` ${file.lineSnippet?.replace("lines", "#L")?.replaceAll(" ", "").replaceAll("(", "").replaceAll(")", "")}`} + {file.lineSnippet && + ` ${file.lineSnippet.replace("lines", "#L").replace(/[ ()]/g, "")}`}
webview-ui/src/components/chat/ChatRow.tsx (1 suggestion)
Location:
webview-ui/src/components/chat/ChatRow.tsx(Lines 769-773)🔵 Code Quality
Issue: Chained
replaceAllcalls for simple character removal are verbose and less efficient than a single regex replace.Fix: Use a regex to remove spaces and parentheses in one pass.
Impact: Improves code cleanliness.
- {tool.reason - ?.replace("lines", "#L") - ?.replaceAll(" ", "") - .replaceAll("(", "") - .replaceAll(")", "")} + {tool.reason?.replace("lines", "#L").replace(/[ ()]/g, "")}
| vscode.postMessage({ type: "openFile", text: "./" + tool.path }) | ||
| }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| vscode.postMessage({ type: "openFile", text: "./" + tool.path }) |
There was a problem hiding this comment.
🟠 Logic Error
Issue: Prepending ./ to tool.path is risky as tool.path might be an absolute path (e.g., from an LLM output) or already relative. This can result in invalid paths like .//usr/bin/file.
Fix: Use tool.path directly. The extension host or backend should handle path resolution (relative vs absolute).
Impact: Prevents file opening failures for absolute paths.
| vscode.postMessage({ type: "openFile", text: "./" + tool.path }) | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| vscode.postMessage({ type: "openFile", text: "./" + tool.path }) | |
| vscode.postMessage({ type: "openFile", text: tool.path }) | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter" || e.key === " ") { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| vscode.postMessage({ type: "openFile", text: tool.path }) |
| style={{ fontSize: 13.5 }} | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| vscode.postMessage({ type: "openFile", text: "./" + tool.path }) |
There was a problem hiding this comment.
🟠 Logic Error
Issue: Prepending ./ to tool.path is risky for absolute paths.
Fix: Use tool.path directly.
Impact: Prevents file opening failures.
| vscode.postMessage({ type: "openFile", text: "./" + tool.path }) | |
| vscode.postMessage({ type: "openFile", text: tool.path }) |
|
|
||
| {/* Truncated Message */} | ||
| <span className="text-vscode-descriptionForeground text-sm truncate flex-1 min-w-0"> | ||
| {truncateMessage(message)} |
There was a problem hiding this comment.
🟡 UX/Code Quality
Issue: The truncateMessage function hardcodes a 30-character limit, which conflicts with the truncate CSS class on the container span. The CSS class handles truncation responsively based on available width, whereas the JS function might cut off text prematurely or leave unused space.
Fix: Remove the truncateMessage usage and let CSS handle the truncation.
Impact: Improves responsiveness and utilizes available screen space better.
| {truncateMessage(message)} | |
| {message} |
1 participant
No description provided.