-
Notifications
You must be signed in to change notification settings - Fork 21
Daily branch 2026 01 20 #179
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
Conversation
Filter out messages with empty parts after file processing and add validation to show a user-friendly error when all messages are empty. This prevents the "must include at least one parts field" API error from Gemini when users send file-only messages that get removed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a unified sandboxed "file" AI tool (read/write/append/edit), a FileHandler UI with Monaco DiffEditor, sidebar and type updates to carry original/modified file contents (but strip them from model payloads), ESLint/Prettier ignore entries, and various formatting and test tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as "Client/UI"
participant MsgProc as "MessageProcessor"
participant FileTool as "AI Tool: file"
participant FS as "Sandbox FS"
participant Sidebar as "Sidebar / FileHandler"
participant Diff as "DiffEditor (Monaco)"
Client->>MsgProc: send tool-file request (action, path, text/edits)
MsgProc->>FileTool: invoke createFile.execute(...)
FileTool->>FS: perform read/write/append/edit
FS-->>FileTool: return result (originalContent?, modifiedContent?, output)
FileTool-->>MsgProc: return structured part (may include original/modified)
MsgProc->>MsgProc: strip originalContent/modifiedContent for model payload
MsgProc-->>Client: deliver message with reduced model payload + full part
Client->>Sidebar: render part → FileHandler detects action
Sidebar->>Diff: open diff view with original/modified (if present)
Diff-->>Sidebar: rendered diff (Monaco)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
…pend, and edit actions - Implemented a new `FileHandler` component to manage file operations in the sidebar. - Added a `createFile` tool for performing file actions in the sandbox, supporting reading, writing, appending, and editing. - Enhanced sidebar functionality to display real-time updates for file operations, including streaming content. - Updated `ComputerSidebar` and `DiffView` components to integrate with the new file handling logic. - Improved message processing to strip unnecessary content from tool outputs, optimizing payload size. This update enhances the user experience by providing a more cohesive and efficient way to interact with files within the application.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/ai/tools/file.ts`:
- Around line 44-46: The docs claim edits are atomic but the code silently skips
edits when an edit's find string isn't present; update the implementation that
processes the edits (the loop handling the edits array in the function that
applies edits, e.g. applyEdits/applyTextEdits) to throw a clear Error when an
edit's find string is not found instead of silently skipping it: detect when a
find match is missing for any edit in edits (the edit.find / find variable) and
raise an exception with context (which find string and which edit) so the caller
sees the failure and atomicity is preserved.
🧹 Nitpick comments (7)
app/components/DiffView.tsx (2)
41-57: Consider extractinggetMonacoLanguageoutside the component.This helper function is recreated on every render. While the performance impact is minimal, extracting it outside the component would be cleaner.
♻️ Suggested refactor
+// Map common language names to Monaco language IDs +const MONACO_LANGUAGE_MAP: Record<string, string> = { + js: "javascript", + ts: "typescript", + py: "python", + rb: "ruby", + yml: "yaml", + md: "markdown", + sh: "shell", + bash: "shell", + zsh: "shell", + txt: "plaintext", + text: "plaintext", +}; + +const getMonacoLanguage = (lang: string): string => { + return MONACO_LANGUAGE_MAP[lang.toLowerCase()] || lang.toLowerCase(); +}; + export const DiffView: React.FC<DiffViewProps> = ({ originalContent, modifiedContent, language, wrap = true, }) => { const [viewMode, setViewMode] = useState<ViewMode>("diff"); - - // Map common language names to Monaco language IDs - const getMonacoLanguage = (lang: string): string => { - const languageMap: Record<string, string> = { - js: "javascript", - ... - }; - return languageMap[lang.toLowerCase()] || lang.toLowerCase(); - };
84-96: Inline<style>tag relies on Monaco's internal CSS class names.This approach is fragile as Monaco's internal class names (
.original-in-monaco-diff-editor,.monaco-editor-background, etc.) are not part of the public API and could change in future versions. Consider documenting this coupling or exploring Monaco's official theming API.lib/ai/tools/file.ts (2)
99-101: Consider distinguishing empty file from non-existent file.Returning
{ error: "File is empty." }for both empty content and whitespace-only files may not always be the desired behavior. An empty file is valid in many contexts (e.g.,.gitkeep, initial placeholder files).
232-249: Silent skip of unmatched edits may cause confusion.When an edit's
findstring isn't found, the edit is silently skipped. Users might expect an error or at least a notification. The return message shows "X edits applied" but doesn't indicate if any were skipped.♻️ Suggested improvement to report skipped edits
let content = originalContent; let editsApplied = 0; let totalReplacements = 0; + let editsSkipped = 0; for (const edit of edits) { const { find, replace, all = false } = edit; if (!content.includes(find)) { + editsSkipped++; continue; } // ... rest of logic } // In return statement: return { - content: `Multi-edit completed: ${editsApplied} edits applied, ${totalReplacements} total replacements made\nLatest content with line numbers:\n${numberedLines}`, + content: `Multi-edit completed: ${editsApplied} edits applied, ${totalReplacements} total replacements made${editsSkipped > 0 ? `, ${editsSkipped} edits skipped (pattern not found)` : ''}\nLatest content with line numbers:\n${numberedLines}`,lib/ai/tools/index.ts (1)
10-12: Consider removing commented-out imports.The commented imports for
createReadFileandcreateWriteFileshould be removed once the new unifiedfiletool is confirmed stable. Leaving commented code can lead to confusion and code rot.app/components/tools/FileHandler.tsx (2)
17-20: Consider stronger typing forpartprop.Using
anyfor thepartprop loses type safety. Consider creating a proper type that matches the tool output structure.♻️ Suggested type definition
interface FileToolPart { type: "tool-file"; toolCallId: string; state: "input-streaming" | "input-available" | "output-available"; input?: FileInput; output?: string | { error: string } | { content: string; originalContent?: string; modifiedContent?: string }; } interface FileHandlerProps { part: FileToolPart; status: ChatStatus; }
130-166: Consider extracting error checking logic.The error checking pattern (
typeof output === "object" && output !== null && "error" in output) is repeated across all render functions. Extracting this to a helper would reduce duplication.♻️ Suggested helper function
const isErrorOutput = (output: unknown): output is { error: string } => typeof output === "object" && output !== null && "error" in output; const getOutputContent = <T extends string>( output: unknown, key: T ): string | undefined => { if (typeof output === "object" && output !== null && key in output) { return (output as Record<T, string>)[key]; } return undefined; };
…rge payloads Applies truncation to all content fields in read, append, and edit actions to avoid memory/performance issues with large files. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace bash globstar with POSIX-compliant find command (macOS ships with bash 3.2 which doesn't support globstar) - Try fd first if available for faster glob matching, fall back to find - Use find -path instead of -name to support full path patterns - Add match tool results to sidebar navigation - Show FolderSearch icon and "File Search" label for match results - Use plain text language for search results display Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, the edit action silently skipped edits when find strings weren't found, allowing partial application. This contradicted the documentation stating "all must succeed or none are applied". Now validates all find strings exist before applying any edits, returning a detailed error if any are missing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/components/ComputerSidebar.tsx`:
- Around line 427-432: The conditional render for DiffView incorrectly treats an
empty string as falsy; update the check in the component that decides to render
DiffView (the expression using sidebarContent.action,
sidebarContent.originalContent, and sidebarContent.modifiedContent) to test
modifiedContent explicitly for undefined (e.g., sidebarContent.modifiedContent
!== undefined) instead of a truthy check so that empty string values still
render the DiffView.
In `@lib/ai/tools/match.ts`:
- Around line 53-66: The recursive detection currently only checks for "**" via
isRecursive and thus incorrectly treats patterns like "src/*/index.js" as
non-recursive; update the logic after computing baseDir (from
extractBaseDir(scope)) to compute the remainder of the pattern (e.g., the part
after baseDir) and set isRecursive to true if scope.includes("**") OR if the
remainder contains path separators combined with glob metacharacters (i.e., a
"/" appears in the remainder and there is a wildcard like "*" or "?" or
character class "[" in that remainder); then use that isRecursive to choose the
appropriate findCommand so patterns with wildcards in path segments correctly
avoid -maxdepth 1 (affecting fdCommand and findCommand decisions).
In `@lib/utils/sidebar-utils.ts`:
- Around line 231-247: The read-path assumes part.output is a string and strips
line numbers using a tab, but createFile returns an object with a content
property and uses '|' as the delimiter; update the logic in sidebar-utils.ts
around the fileAction === "read" branch to: extract text from
part.output.content when part.output is an object (fall back to part.output when
string), split into lines, remove the header lines, and strip leading
line-number prefixes that use either '^\d+\|' or '^\d+\t' (handle both
delimiters) before joining back into content; keep the existing range extraction
(fileInput.range) unchanged.
♻️ Duplicate comments (1)
lib/ai/tools/file.ts (1)
241-257: Honor atomic edit semantics (currently skipped).On Line 244–246, missing
findstrings are silently ignored, but the tool contract says all edits must succeed or none apply. This still allows partial edits.🛠️ Suggested fix
- for (const edit of edits) { + for (const [index, edit] of edits.entries()) { const { find, replace, all = false } = edit; if (!content.includes(find)) { - continue; + throw new Error( + `Edit ${index + 1} failed: find string not found: "${find}"` + ); }
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@lib/ai/tools/file.ts`:
- Around line 236-271: The current atomic-edit logic only validates finds
against originalContent, so later edits can silently no-op after earlier
replacements; update the loop that applies edits (the for...of over edits using
variables find/replace/all) to first check the current content (not
originalContent) for the find string and fail fast if it's missing, returning
the same atomic error shape (use missingFinds or a similar message), and only
then perform the replacement and update totalReplacements (for all use
split/count on current content before replacing; for single replace ensure you
detect whether a replacement actually occurred and increment totalReplacements
accordingly).
- Around line 6-17: The current editSchema allows an empty find string which
leads to catastrophic replacements; update the editSchema's find field (the
z.string() for "The exact text string to find in the file") to enforce non-empty
input (e.g., use z.string().nonempty(...) or z.string().min(1, "...")) so blank
or empty-only values are rejected at validation time, preventing the
split("")/join behavior in the file edit logic.
- Around line 188-196: The catch-all in the sandbox.files.read error handler is
too broad and the proposed regex won't match ConvexSandbox's "No such file or
directory" message; update the catch to detect missing-file conditions
explicitly by checking error.name === "NotFoundError" OR testing the
error.message for both /not\s+found/i and /no such file or directory/i (and/or
the ConvexSandbox pattern like "Failed to read file: cat:"), and only treat
those as "file missing" (leave existingContent as empty); for any other error
rethrow it. Apply this logic where sandbox.files.read is used (see the
read/append block in lib/ai/tools/file.ts) following the same approach used in
sandbox.ts:87.
- ComputerSidebar: use explicit undefined check for modifiedContent to handle empty string case when clearing files - match.ts: fix recursive detection to handle wildcards in path segments (e.g., src/*/index.js) not just ** patterns - sidebar-utils: fix read parsing to handle object output format with originalContent property Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix fd to use relative pattern (fd -g matches against relative paths)
- Add brace expansion for find fallback (find -path doesn't support {a,b})
- Fix fallback logic to check fd output, not just exit code
- Handle wildcards in path segments for recursive detection
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…opped Removed stripping of originalContent during message normalization. Previously, file tool outputs were stripped of originalContent before being persisted, causing the sidebar to show empty content when a request was stopped. The backend already handles stripping via toModelOutput when sending to the model. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/ai/tools/match.ts`:
- Around line 118-120: The fd branch currently bypasses the MAX_FILES_GLOB limit
because the head is only applied to the find fallback; update the returned shell
command so the limit applies to both branches by grouping the alternation and
piping the whole result to head, e.g. wrap the `{ command -v fd ... } ||
${findCommand}` expression in a subshell/group and append `| head -n
${MAX_FILES_GLOB}`; locate the return that builds the command using fdCommand,
findCommand and MAX_FILES_GLOB and change it to pipe the grouped output through
head (or alternatively use fd's native max-results option inside the fdCommand
to avoid buffering large $out).
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.