-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Native images #6619
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
Native images #6619
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.
Pull request overview
This PR adds native image upload functionality to the desktop UI, allowing users to directly attach images via a file picker button in addition to the existing paste/drag-and-drop methods. Images are automatically compressed to a maximum of 1024px on the longest side at 0.85 JPEG quality before being sent.
Changes:
- Extended the message type system to support image content with base64 data and MIME types
- Added image compression logic that resizes and converts images to JPEG format
- Implemented file input-based image selection with the same compression pipeline as paste functionality
- Updated the chat submission flow to pass image data directly in message content
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui/desktop/src/types/message.ts | Adds ImageData interface and updates createUserMessage to accept optional image data, building message content array with both text and images |
| ui/desktop/src/hooks/useChatStream.ts | Updates handleSubmit signature to accept optional ImageData array parameter |
| ui/desktop/src/components/ChatInput.tsx | Adds compressImageDataUrl function, implements file input handler for image uploads, extracts base64 image data for direct message content, and updates tooltip text |
| ui/desktop/src/components/BaseChat.tsx | Extracts image data from form submission event and passes to handleSubmit |
| } else { | ||
| setPastedImages(prev => prev.map(img => | ||
| img.id === uniqueId | ||
| ? { ...img, dataUrl: compressedDataUrl, isLoading: false, error: undefined } |
Copilot
AI
Jan 21, 2026
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.
Missing filePath assignment when save succeeds. The success path should include filePath: saveResult.filePath to be consistent with the paste handling logic (line 775) and to properly populate the PastedImage type's optional filePath field.
| ? { ...img, dataUrl: compressedDataUrl, isLoading: false, error: undefined } | |
| ? { ...img, dataUrl: compressedDataUrl, filePath: saveResult.filePath, isLoading: false, error: undefined } |
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.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
| // Convert to JPEG with 0.85 quality | ||
| const compressedDataUrl = canvas.toDataURL('image/jpeg', 0.85); | ||
| resolve(compressedDataUrl); |
Copilot
AI
Jan 22, 2026
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.
The image compression always converts to JPEG format regardless of the original format. This means transparent PNGs will lose their transparency (converted to black or white background). Consider preserving PNG format for images that support transparency, or at least documenting this behavior change for users.
| </Button> | ||
| </TooltipTrigger> | ||
| <TooltipContent>Attach file or directory</TooltipContent> | ||
| <TooltipContent>Attach file</TooltipContent> |
Copilot
AI
Jan 22, 2026
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.
The tooltip text changed from "Attach file or directory" to "Attach file". This change aligns with the new implementation which uses a file input that doesn't support directory selection. However, verify this is intentional - if directory selection is still needed, the implementation should be updated to support it.
| interface ImagePreviewProps { | ||
| src: string; | ||
| alt?: string; | ||
| className?: string; | ||
| } |
Copilot
AI
Jan 22, 2026
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.
The alt and className props were removed from the ImagePreview component interface but the component now always uses a hardcoded "goose image" alt text. For accessibility, consider keeping the alt prop to allow meaningful descriptions of each image, especially for screen reader users.
ui/desktop/src/types/message.ts
Outdated
| export function createUserMessage(text: string, images?: ImageData[]): Message { | ||
| const content: Message['content'] = []; | ||
|
|
||
| // Add text content if present | ||
| if (text.trim()) { | ||
| content.push({ type: 'text', text }); | ||
| } | ||
|
|
||
| // Add image content if present | ||
| if (images && images.length > 0) { | ||
| images.forEach(img => { | ||
| content.push({ | ||
| type: 'image', | ||
| data: img.data, | ||
| mimeType: img.mimeType, | ||
| }); | ||
| }); | ||
| } | ||
|
|
Copilot
AI
Jan 22, 2026
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.
When creating a message with only images and no text, the content array will only contain image items. Verify the backend properly handles messages with no text content, as this is a new scenario enabled by this PR.
| interface PastedImage { | ||
| id: string; |
Copilot
AI
Jan 22, 2026
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.
The filePath property is no longer used since images are now handled as base64 data directly. This property should be removed from the interface. The old handlers that reference filePath (handleRetryImageSave and handleRemovePastedImage) also need to be updated or removed.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.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.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
| const interruptionMessage: QueuedMessage = { | ||
| id: Date.now().toString() + Math.random().toString(36).substr(2, 9), | ||
| content: contentToQueue, | ||
| timestamp: Date.now(), | ||
| images: [], | ||
| }; |
Copilot
AI
Jan 22, 2026
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.
Pasted images are being lost when messages are queued during interruptions. The images array should be populated with the current pastedImages state (converted to ImageData format) rather than an empty array, similar to how it's done in the performSubmit function.
| const newMessage: QueuedMessage = { | ||
| id: Date.now().toString() + Math.random().toString(36).substr(2, 9), | ||
| content: contentToQueue, | ||
| timestamp: Date.now(), | ||
| images: [], | ||
| }; |
Copilot
AI
Jan 22, 2026
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.
Pasted images are being lost when messages are queued while loading. The images array should be populated with the current pastedImages state (converted to ImageData format) rather than an empty array, similar to how it's done in the performSubmit function.
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.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
| reader.onload = async (evt) => { | ||
| const dataUrl = evt.target?.result as string; | ||
| if (dataUrl) { | ||
| const compressedDataUrl = await compressImageDataUrl(dataUrl); | ||
| setPastedImages((prev) => | ||
| prev.map((img) => | ||
| img.id === uniqueId | ||
| ? { ...img, dataUrl: compressedDataUrl, isLoading: false, error: undefined } | ||
| : img | ||
| ) | ||
| ); | ||
| } |
Copilot
AI
Jan 22, 2026
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.
Missing error handling for image compression. If compressImageDataUrl throws an error (e.g., unsupported image format, corrupted data), it will result in an unhandled promise rejection. Add a try-catch block similar to the paste handling and other compression calls.
| reader.onload = async (e) => { | ||
| const dataUrl = e.target?.result as string; | ||
| if (dataUrl) { | ||
| // Update the image with the data URL | ||
| const compressedDataUrl = await compressImageDataUrl(dataUrl); | ||
| setPastedImages((prev) => | ||
| prev.map((img) => (img.id === imageId ? { ...img, dataUrl, isLoading: true } : img)) | ||
| prev.map((img) => | ||
| img.id === imageId ? { ...img, dataUrl: compressedDataUrl, isLoading: false } : img | ||
| ) | ||
| ); | ||
|
|
||
| try { | ||
| const result = await window.electron.saveDataUrlToTemp(dataUrl, imageId); | ||
| setPastedImages((prev) => | ||
| prev.map((img) => | ||
| img.id === result.id | ||
| ? { ...img, filePath: result.filePath, error: result.error, isLoading: false } | ||
| : img | ||
| ) | ||
| ); | ||
| } catch (err) { | ||
| console.error('Error saving pasted image:', err); | ||
| setPastedImages((prev) => | ||
| prev.map((img) => | ||
| img.id === imageId | ||
| ? { ...img, error: 'Failed to save image via Electron.', isLoading: false } | ||
| : img | ||
| ) | ||
| ); | ||
| } | ||
| } |
Copilot
AI
Jan 22, 2026
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.
Missing error handling for image compression. If compressImageDataUrl throws an error, it will result in an unhandled promise rejection. Wrap the compression call in a try-catch block to handle failures gracefully, similar to the error handling in useFileDrop.ts lines 81-100.
| const customEvent = e as unknown as CustomEvent; | ||
| const textValue = customEvent.detail?.value || ''; | ||
|
|
||
| const chatInputSubmit = (textValue: string, images: ImageData[]) => { |
Copilot
AI
Jan 22, 2026
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.
Missing import for ImageData. The function chatInputSubmit uses ImageData[] but the type is not imported. Add ImageData to the imports from '../types/message' on line 35.
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.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
| const compressedDataUrl = await compressImageDataUrl(dataUrl); | ||
| setPastedImages((prev) => | ||
| prev.map((img) => | ||
| img.id === uniqueId | ||
| ? { ...img, dataUrl: compressedDataUrl, isLoading: false, error: undefined } | ||
| : img | ||
| ) | ||
| ); |
Copilot
AI
Jan 22, 2026
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.
Missing error handling for compressImageDataUrl. If compression fails, the promise will reject but there's no try-catch block to handle it, leaving the image in a loading state indefinitely. Wrap the compressImageDataUrl call in a try-catch and update the image state with an error message on failure.
| const compressedDataUrl = await compressImageDataUrl(dataUrl); | |
| setPastedImages((prev) => | |
| prev.map((img) => | |
| img.id === uniqueId | |
| ? { ...img, dataUrl: compressedDataUrl, isLoading: false, error: undefined } | |
| : img | |
| ) | |
| ); | |
| try { | |
| const compressedDataUrl = await compressImageDataUrl(dataUrl); | |
| setPastedImages((prev) => | |
| prev.map((img) => | |
| img.id === uniqueId | |
| ? { ...img, dataUrl: compressedDataUrl, isLoading: false, error: undefined } | |
| : img | |
| ) | |
| ); | |
| } catch (error) { | |
| console.error('Failed to compress image data URL', error); | |
| setPastedImages((prev) => | |
| prev.map((img) => | |
| img.id === uniqueId | |
| ? { ...img, isLoading: false, error: 'Failed to process image file' } | |
| : img | |
| ) | |
| ); | |
| } |
| ); | ||
| try { | ||
| // Compress the image | ||
| const compressedDataUrl = await compressImageDataUrl(dataUrl); |
Copilot
AI
Jan 22, 2026
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.
Missing error handling for compressImageDataUrl. If compression fails, the promise will reject but there's no try-catch block to handle it, leaving the image in a loading state indefinitely. Wrap the compressImageDataUrl call in a try-catch and update the image state with an error message on failure.
| const dataUrl = e.target?.result as string; | ||
| if (dataUrl) { | ||
| // Update the image with the data URL | ||
| const compressedDataUrl = await compressImageDataUrl(dataUrl); |
Copilot
AI
Jan 22, 2026
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.
Missing error handling for compressImageDataUrl. If compression fails, the promise will reject but there's no try-catch block to handle it, leaving the image in a loading state indefinitely. Wrap the compressImageDataUrl call in a try-catch and update the image state with an error message on failure.
zanesq
left a comment
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.
Nice good improvement! Code LGTM and verified its working as intended uploading and processing images in chat locally.
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.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Co-authored-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: fbalicchia <fbalicchia@cuebiq.com>
* origin/main: Fix GCP Vertex AI global endpoint support for Gemini 3 models (#6187) fix: macOS keychain infinite prompt loop (#6620) chore: reduce duplicate or unused cargo deps (#6630) feat: codex subscription support (#6600) smoke test allow pass for flaky providers (#6638) feat: Add built-in skill for goose documentation reference (#6534) Native images (#6619) docs: ml-based prompt injection detection (#6627) Strip the audience for compacting (#6646) chore(release): release version 1.21.0 (minor) (#6634) add collapsable chat nav (#6649) fix: capitalize Rust in CONTRIBUTING.md (#6640) chore(deps): bump lodash from 4.17.21 to 4.17.23 in /ui/desktop (#6623) Vibe mcp apps (#6569) Add session forking capability (#5882) chore(deps): bump lodash from 4.17.21 to 4.17.23 in /documentation (#6624) fix(docs): use named import for globby v13 (#6639) PR Code Review (#6043) fix(docs): use dynamic import for globby ESM module (#6636) # Conflicts: # Cargo.lock # crates/goose-server/src/routes/session.rs
Summary
Upload images directly. Also does a better job on pasting images in: