Repository navigation
Image attachment support in chat + pop-out state transfer fix - #63
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe PR adds image attachments to chat input, message storage, and chat streaming, renders attached images in message bubbles, and wraps chat pop-out IPC responses in ChangesChat image attachments
Chat pop-out IPC response wrapping
Sequence Diagram(s)sequenceDiagram
participant ChatInput
participant ChatPanel
participant chatStream as chat:stream
participant ChatMessageBubble
ChatInput->>ChatPanel: onAttachImages(files)
ChatInput->>ChatPanel: onSend(text, pendingImages)
ChatPanel->>ChatPanel: addMessage(message, images)
ChatPanel->>chatStream: sendStream(message, images)
chatStream->>chatStream: build multimodal userMessage
ChatPanel->>ChatMessageBubble: render message.images
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
- Paste/attach images in chat input with thumbnail preview strip
- Images kept inline as base64 data URLs (no persistent disk storage)
- Rendered as inline thumbnails in chat bubbles
- Sent to LLM as base64 multi-part content (OpenAI vision API format)
- Supported models: GPT-4o, Claude Sonnet 4.6, Gemini 3.5 Flash
- Fixed chat-popout handlers returning plain objects instead of { data: T }
ddutchie
force-pushed
the
ddutchie/imagesinchat
branch
from
June 24, 2026 20:51
8185484 to
c595b2b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/components/chat/ChatInput.tsx (1)
287-292: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a color token instead of raw
text-white.The remove button uses the raw Tailwind color
text-white. As per coding guidelines, all colours must use CSS variables (e.g.var(--text-on-accent)/ a danger-foreground token) and never raw Tailwind colour names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/chat/ChatInput.tsx` around lines 287 - 292, The remove button in ChatInput uses a raw Tailwind text color, which needs to be replaced with a CSS variable token. Update the button class in ChatInput’s image remove control (the onClick/remove-image button near the X icon) to use the appropriate foreground token for the danger background instead of text-white, keeping the existing danger background styling intact.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@changelogs/v2.2.4.md`:
- Line 5: Update the changelog entry for image attachments so it matches the
actual behavior implemented in the chat attachment flow. The current description
incorrectly says images remain only as base64 data URLs with no disk
persistence; revise it to reflect that attachments are stored in the attachments
storage, persisted on ChatMessage, and served via asset:// while still
mentioning inline thumbnail rendering and vision-capable model support.
In `@electron/chat-popout.ts`:
- Around line 158-162: The chat pop-in handler currently relays
`chat:requestPopIn` from any renderer, so update
`ipcMain.handle("chat:requestPopIn", ...)` to validate the caller first. Use the
handler’s `event.sender.id` and compare it against `mainWindowWebContentsId`,
only sending `popoutWindow.webContents.send("chat:requestPopIn")` when the
request comes from the main window; otherwise ignore or reject the request while
keeping the existing return shape.
In `@electron/ipc/chat.ts`:
- Around line 389-397: The user message assembly in chat.ts currently appends
image_url content whenever req.images is present, regardless of whether the
selected provider/model supports multimodal input. Update the logic around the
userMessage construction to check the active provider/model capability before
including images, and only build the image parts when vision is supported. If
images are not supported, either omit them and notify the caller or surface a
clear notice before sending the request.
In `@src/components/chat/chat-panel/index.tsx`:
- Around line 339-350: The FileReader flow in handleAttachImages only resolves
on reader.onload, so a failed or aborted read can leave the promise pending and
stall the loop. Update the FileReader promise to also handle reader.onerror and
reader.onabort by rejecting or otherwise settling with an error path, and make
handleAttachImages recover per file so one bad image does not block later files
or setPendingImages.
In `@src/types/index.ts`:
- Around line 203-204: The `images` field in `chatMessages` is currently
persisting inline `dataUrl` payloads via `persist`, while the database write
path omits `images`, so update the message model and storage flow to avoid
keeping base64 data in client state. In the relevant message/image handling code
that sets `images[].url` from `img.dataUrl` and the persistence logic around
`get().persist()`, convert images to workspace attachment references (such as an
`asset://` or attachment path) before storing them, or, if inline data URLs are
intentional, update the `images` JSDoc in `Message` to match the actual
behavior. Use the `images` property in `src/types/index.ts` and the chat message
persistence flow as the anchor points for the fix.
---
Nitpick comments:
In `@src/components/chat/ChatInput.tsx`:
- Around line 287-292: The remove button in ChatInput uses a raw Tailwind text
color, which needs to be replaced with a CSS variable token. Update the button
class in ChatInput’s image remove control (the onClick/remove-image button near
the X icon) to use the appropriate foreground token for the danger background
instead of text-white, keeping the existing danger background styling intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f0b23e9-cdd5-4880-bba7-31373e8227f2
📒 Files selected for processing (10)
changelogs/v2.2.4.mdelectron/chat-popout.tselectron/ipc/chat.tselectron/lib/tools.tssrc/components/chat/ChatInput.tsxsrc/components/chat/chat-panel/ChatMessageBubble.tsxsrc/components/chat/chat-panel/index.tsxsrc/hooks/useChatStream.tssrc/store/slices/chat.tssrc/types/index.ts
13 of 18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds image attachment support to chat — paste clipboard images or use the attachment button, preview as thumbnails, send as base64 inline data URLs to vision-capable LLMs (GPT-4o, Claude Sonnet 4.6, Gemini 3.5 Flash). Also fixes the pop-out state transfer bug where
{ data: T }unwrapping prevented pop-out from working, and validates sender identity on thechat:requestPopInhandler.Images are ephemeral — kept as inline base64 data URLs on
ChatMessage.images, no persistent disk storage. Only the current message's images are sent to the LLM (not full history). When the model doesn't support vision, images are silently omitted with a text notice prepended to the message.Type of change
Screenshots / recording
N/A
Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpassesnpm run test:e2epassestext-[Npx]pixel font classes — rem equivalents onlyhandle()and returnIpcResult<T>schema.tselectron/db/queries.tsdependenciesordevDependenciesadded toROLE_MAPelectron/mcp/tools/index.tsdispatch--external:<pkg>flagNotes for reviewer
Vision API tested end-to-end against three endpoints: localhost:3042 (GPT-4o, Claude Sonnet 4.6) and localhost:3420 (Gemini 3.5 Flash). Uses standard OpenAI multi-part
contentarray format — any vision-capable OpenAI-compatible endpoint should work. Image attach/remove via paste handler and attachment button in ChatInput, thumbnail preview strip with per-image remove. FileReader handles error/abort per file so one bad image doesn't block others.chat:requestPopInhandler now rejects non-main-window callers.