feat(chat): add image attachment support to composer#2676
Conversation
Users can now attach images (PNG, JPEG, WebP, GIF, BMP) to chat messages using the paperclip button in the text composer. Attachments are previewed as thumbnail chips above the textarea and removed individually before send. On send, images are encoded as `[IMAGE:<data-uri>]` markers appended to the message text. The Rust agent harness already parses these markers in `agent/multimodal.rs` and routes them to the inference provider — no backend changes required. Limits match the existing backend `MultimodalConfig` defaults: up to 4 images per message, 8 MB each. Validation errors (unsupported type, oversized file, count exceeded) surface in the existing composer error banner. - `app/src/lib/attachments.ts` — validation, FileReader util, marker composer - `app/src/components/chat/AttachmentPreview.tsx` — thumbnail chip strip - `app/src/pages/Conversations.tsx` — file input, attachment state, composer wiring - i18n keys added to `en.ts` and all 12 locale chunk files - 15 Vitest unit tests covering validation, composition, and formatting
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds image attachment support: utilities to validate/convert files, AttachmentPreview UI, i18n strings, and composer integration to select, validate, preview, compose, send, and render image attachments with metadata. ChangesImage Attachments in Chat
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
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. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/pages/Conversations.tsx`:
- Around line 617-649: The batch attach logic in handleAttachFiles uses the
stale attachments.length when calling validateAndReadFile, causing over-limit
selections to be silently dropped; fix by tracking a local counter (e.g., let
currentCount = attachments.length) and pass currentCount to validateAndReadFile
for each file, increment currentCount whenever a file is accepted, and keep
using setAttachments(prev => ...) to append files (while still guarding against
ATTACHMENT_MAX_IMAGES). This ensures validateAndReadFile sees the cumulative
count for the current selection and still enforces the limit and error path
(too_many) correctly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c2e1e963-88a8-4aa1-99b8-2865db1c6a61
📒 Files selected for processing (18)
app/src/components/chat/AttachmentPreview.tsxapp/src/lib/attachments.test.tsapp/src/lib/attachments.tsapp/src/lib/i18n/chunks/ar-2.tsapp/src/lib/i18n/chunks/bn-2.tsapp/src/lib/i18n/chunks/de-2.tsapp/src/lib/i18n/chunks/en-2.tsapp/src/lib/i18n/chunks/es-2.tsapp/src/lib/i18n/chunks/fr-2.tsapp/src/lib/i18n/chunks/hi-2.tsapp/src/lib/i18n/chunks/id-2.tsapp/src/lib/i18n/chunks/it-2.tsapp/src/lib/i18n/chunks/ko-2.tsapp/src/lib/i18n/chunks/pt-2.tsapp/src/lib/i18n/chunks/ru-2.tsapp/src/lib/i18n/chunks/zh-CN-2.tsapp/src/lib/i18n/en.tsapp/src/pages/Conversations.tsx
…flow - Track accepted count locally in handleAttachFiles loop so the too-many error fires correctly when selecting more than 4 at once - Use separate attachError state so attachment errors are not cleared when the user types in the composer - Store attachment data URIs in extraMetadata so the user message bubble can render image thumbnails without embedding markers in content - Render attachment images above the text in the user bubble using a clean image-only layout (no blue background behind images) - Allow attachment-only sends by bypassing the empty_input block when attachments are present - Add parseMessageImages utility and extend test suite to cover all fixed paths (19 tests passing)
graycyrus
left a comment
There was a problem hiding this comment.
Good work — clean module decomposition, solid test coverage on the utility layer, and the [IMAGE:] marker protocol meshes well with the existing Rust multimodal parser. Two things to address before this can land:
| File | Change |
|---|---|
attachments.ts |
New — validation, FileReader, marker composition/parsing, formatting |
AttachmentPreview.tsx |
New — thumbnail chip strip component |
Conversations.tsx |
Attachment state, file input, composer button, send wiring, bubble rendering |
| 13 i18n files | 8 new attachment-related keys each |
attachments.test.ts |
19 unit tests |
[major] Conversations.tsx bubble rendering — Images render from msg.extraMetadata.attachmentDataUris only. This is a client-side-only field. When messages are rehydrated from the thread API (page reload, different device, history scroll), extraMetadata may not survive the round-trip, and attached images silently vanish.
You already wrote and tested parseMessageImages for exactly this scenario, but it's never called in the rendering path. Wire it up as a fallback:
const dataUris = Array.isArray(msg.extraMetadata?.attachmentDataUris)
? (msg.extraMetadata.attachmentDataUris as string[])
: parseMessageImages(msg.content).dataUris;This way local sends use the fast path, and persisted messages still render their images from the [IMAGE:] markers in content.
[minor] All 12 non-English locale chunk files (ar-2.ts, bn-2.ts, de-2.ts, etc.) have the new chat.attachment.* keys in English only. Fine as a placeholder but open a tracking issue so they get translated before release.
When messages are rehydrated from the thread API (page reload, history scroll), extraMetadata.attachmentDataUris may not survive the round-trip. Use the client-side field as the fast path and parseMessageImages as the fallback so persisted messages still render their images from the [IMAGE:] markers embedded in content.
|
Hey @graycyrus — both points addressed: [major] Wired [minor] Opened #2725 to track translations for the 8 Let me know if anything else needs attention. |
Add unit tests for AttachmentPreview component, attachment validation utilities (including read_failed path and parseMessageImages), and Conversations attachment integration to meet the ≥80% diff-cover gate.
Closes #2662
Summary
[IMAGE:<data-uri>]markers in the message string — the existing Rust backend (agent/multimodal.rs) already parses these and routes them to the inference provider, so no backend changes were neededMultimodalConfigdefaults: up to 4 images per message, 8 MB eachFiles changed
app/src/lib/attachments.tsFileReaderutil,[IMAGE:]marker composition,parseMessageImagesutilapp/src/components/chat/AttachmentPreview.tsxapp/src/pages/Conversations.tsxapp/src/lib/i18n/en.ts+ 12 locale chunk filesapp/src/lib/attachments.test.tsTest plan
Notes
Pre-push hook fails on
cargo fmt --checkdue tocargonot being in PATH in this dev environment. This is a pre-existing environment issue unrelated to the changes here (no Rust files were modified). TypeScript typecheck, ESLint, Prettier, and Vitest all pass.Summary by CodeRabbit
New Features
Tests
Documentation