Conversation
📝 WalkthroughWalkthroughAdds an attachments system to the chat UI (pick/remove attachments, context-picker integration), wires attachments through the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ChatApp
participant MessageInput
participant useChat
participant API
participant VSCode
User->>MessageInput: Click "pick files" / trigger context picker
MessageInput->>useChat: pickFiles()
useChat->>User: Open file picker (UI / external)
User->>useChat: Select file(s)
useChat->>useChat: Add attachment(s) to state
useChat->>ChatApp: Update attachments prop
ChatApp->>MessageInput: Pass attachments, pickFiles, removeAttachment
alt Context picker flow
VSCode->>MessageInput: contextPickerResult
MessageInput->>useChat: notify context result
useChat->>useChat: Add context as attachment
end
User->>MessageInput: Send message (with attachments)
MessageInput->>useChat: sendMessage(text, attachments)
useChat->>API: POST message payload with attachments
API-->>useChat: message accepted / response
useChat->>ChatApp: Append message to list
ChatApp->>MessageInput: Render updated UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/hooks/useChat.ts (1)
85-90: Clear attachments when history is cleared to prevent stale context leakage.
Right nowhistoryClearedresets messages/feedback but leavesattachmentsintact, which can unintentionally carry files into a fresh conversation.🐛 Proposed fix
case 'historyCleared': setMessages([]); setFeedbackState(new Map()); setPendingFeedback(new Map()); pendingFeedbackRef.current = new Map(); + setAttachments([]); break;
🤖 Fix all issues with AI agents
In `@ui/.env.template`:
- Around line 1-2: Remove the temporary placeholder comment and add a trailing
newline to satisfy the linter: delete the "# test" line and ensure the file
ends with a single blank line after the VITE_API_BASE= entry (leave the
VITE_API_BASE variable intact).
In `@ui/src/components/MessageInput.tsx`:
- Around line 98-115: When in the branch handling contextPickerActive, handle
the case where the '#' was deleted by checking hashIndex after computing it; if
hashIndex is -1, call setContextPickerActive(false) and reset
hashPositionRef.current = -1 to fully deactivate the picker and avoid leaving
the UI stuck, otherwise continue extracting textAfterHash and posting the
openContextPicker message via vscodeApi.postMessage as before.
- Around line 29-61: The effect leaks message listeners and can insert
"#undefined"; update setupMessageListener to return a cleanup function that
removes the added window message handler, then call that cleanup from the
useEffect return (where handleMessage is passed) so the listener is removed on
unmount/re-render; also guard the reference construction in handleMessage (where
you build reference from msg.result.reference || `#${msg.result.file ||
msg.result.name}`) by falling back to a safe value or skipping insertion when
neither msg.result.file nor msg.result.name exists (use textareaRef,
hashPositionRef and setMessage logic already present).
🧹 Nitpick comments (3)
api/__init__.py (1)
28-28: Remove the stray test comment before release.
This looks like a leftover placeholder and adds noise to production config.♻️ Proposed cleanup
- `#this` is a testui/src/components/MessageInput.tsx (2)
63-69: Clarify or remove the misleading comment.The comment "Don't clear attachments - they should persist for visibility" is confusing because
attachmentsis a prop managed by the parent component, not local state. This component cannot clear them regardless. If the intent is to document expected parent behavior, consider rewording or removing to avoid confusion.✏️ Suggested fix
const handleSend = () => { if (message.trim() && !disabled) { onSend(message, attachments); setMessage(''); - // Don't clear attachments - they should persist for visibility + // Note: Parent component manages attachment state and should clear after send } };
157-163: Redundant null check on required prop.
pickFilesis a required prop in the interface, so theif (pickFiles)check is unnecessary. TypeScript guarantees it will be provided.✏️ Suggested simplification
onClick={(e) => { e.preventDefault(); e.stopPropagation(); - if (pickFiles) { - pickFiles(); - } + pickFiles(); }}
| VITE_API_BASE= | ||
| # test No newline at end of file |
There was a problem hiding this comment.
Clean up placeholder comment and add trailing newline (linter warning).
The dotenv linter flags the missing ending blank line, and the # test comment looks like a temporary marker.
✅ Suggested fix
-VITE_API_BASE=
-# test
+VITE_API_BASE=
+📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| VITE_API_BASE= | |
| # test | |
| VITE_API_BASE= | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
In `@ui/.env.template` around lines 1 - 2, Remove the temporary placeholder
comment and add a trailing newline to satisfy the linter: delete the "# test"
line and ensure the file ends with a single blank line after the VITE_API_BASE=
entry (leave the VITE_API_BASE variable intact).
| useEffect(() => { | ||
| const handleMessage = (msg: VSCodeMessage) => { | ||
| if (msg.command === 'contextPickerResult') { | ||
| setContextPickerActive(false); | ||
|
|
||
| if (msg.result && textareaRef.current) { | ||
| // Insert the reference into the input at the hash position | ||
| const textarea = textareaRef.current; | ||
| const currentValue = textarea.value; | ||
| const cursorPos = textarea.selectionStart; | ||
|
|
||
| // Find the hash position | ||
| if (hashPositionRef.current >= 0) { | ||
| const beforeHash = currentValue.substring(0, hashPositionRef.current); | ||
| const afterCursor = currentValue.substring(cursorPos); | ||
| const reference = msg.result.reference || `#${msg.result.file || msg.result.name}`; | ||
|
|
||
| const newValue = beforeHash + reference + ' ' + afterCursor; | ||
| setMessage(newValue); | ||
|
|
||
| // Set cursor position after the inserted reference | ||
| setTimeout(() => { | ||
| const newCursorPos = hashPositionRef.current + reference.length + 1; | ||
| textarea.setSelectionRange(newCursorPos, newCursorPos); | ||
| }, 0); | ||
| } | ||
|
|
||
| hashPositionRef.current = -1; | ||
| } | ||
| } | ||
| }; | ||
| setupMessageListener(handleMessage); | ||
| }, []); |
There was a problem hiding this comment.
Memory leak: event listener is never cleaned up.
setupMessageListener adds a window.addEventListener but provides no way to remove it. Without a cleanup function, listeners accumulate on each component mount/remount, causing memory leaks and potentially triggering stale handlers.
Additionally, line 44 can produce #undefined if neither msg.result.file nor msg.result.name exists.
🐛 Proposed fix
First, update vscodeApi.ts to return a cleanup function:
export function setupMessageListener(callback: (message: VSCodeMessage) => void): () => void {
if (typeof window !== 'undefined') {
const handler = (event: MessageEvent) => callback(event.data);
window.addEventListener('message', handler);
return () => window.removeEventListener('message', handler);
}
return () => {};
}Then use the cleanup in the effect:
useEffect(() => {
const handleMessage = (msg: VSCodeMessage) => {
if (msg.command === 'contextPickerResult') {
setContextPickerActive(false);
if (msg.result && textareaRef.current) {
// Insert the reference into the input at the hash position
const textarea = textareaRef.current;
const currentValue = textarea.value;
const cursorPos = textarea.selectionStart;
// Find the hash position
if (hashPositionRef.current >= 0) {
const beforeHash = currentValue.substring(0, hashPositionRef.current);
const afterCursor = currentValue.substring(cursorPos);
- const reference = msg.result.reference || `#${msg.result.file || msg.result.name}`;
+ const reference = msg.result.reference || (msg.result.file || msg.result.name ? `#${msg.result.file || msg.result.name}` : '');
+ if (!reference) return;
+
const newValue = beforeHash + reference + ' ' + afterCursor;
setMessage(newValue);
// Set cursor position after the inserted reference
setTimeout(() => {
const newCursorPos = hashPositionRef.current + reference.length + 1;
textarea.setSelectionRange(newCursorPos, newCursorPos);
}, 0);
}
hashPositionRef.current = -1;
}
}
};
- setupMessageListener(handleMessage);
+ const cleanup = setupMessageListener(handleMessage);
+ return cleanup;
}, []);🤖 Prompt for AI Agents
In `@ui/src/components/MessageInput.tsx` around lines 29 - 61, The effect leaks
message listeners and can insert "#undefined"; update setupMessageListener to
return a cleanup function that removes the added window message handler, then
call that cleanup from the useEffect return (where handleMessage is passed) so
the listener is removed on unmount/re-render; also guard the reference
construction in handleMessage (where you build reference from
msg.result.reference || `#${msg.result.file || msg.result.name}`) by falling
back to a safe value or skipping insertion when neither msg.result.file nor
msg.result.name exists (use textareaRef, hashPositionRef and setMessage logic
already present).
| } else if (contextPickerActive && value.length > 0) { | ||
| // Update query as user types after "#" | ||
| const hashIndex = value.lastIndexOf('#', cursorPos - 1); | ||
| if (hashIndex >= 0) { | ||
| const textAfterHash = value.substring(hashIndex + 1, cursorPos); | ||
| const spaceIndex = textAfterHash.indexOf(' '); | ||
| const query = spaceIndex > 0 ? textAfterHash.substring(0, spaceIndex) : textAfterHash; | ||
|
|
||
| // Update context picker query | ||
| vscodeApi.postMessage({ | ||
| command: 'openContextPicker', | ||
| query: query | ||
| }); | ||
| } | ||
| } else { | ||
| setContextPickerActive(false); | ||
| hashPositionRef.current = -1; | ||
| } |
There was a problem hiding this comment.
Context picker may not deactivate when "#" is deleted.
If the user is in contextPickerActive mode and deletes the # character while other text remains, the code enters the else if branch (line 98) but hashIndex will be -1. The condition on line 101 prevents entering the inner block, but contextPickerActive remains true, leaving the UI stuck in search mode.
🐛 Proposed fix
} else if (contextPickerActive && value.length > 0) {
// Update query as user types after "#"
const hashIndex = value.lastIndexOf('#', cursorPos - 1);
if (hashIndex >= 0) {
const textAfterHash = value.substring(hashIndex + 1, cursorPos);
const spaceIndex = textAfterHash.indexOf(' ');
const query = spaceIndex > 0 ? textAfterHash.substring(0, spaceIndex) : textAfterHash;
// Update context picker query
vscodeApi.postMessage({
command: 'openContextPicker',
query: query
});
+ } else {
+ // Hash was deleted, deactivate context picker
+ setContextPickerActive(false);
+ hashPositionRef.current = -1;
}
} else {
setContextPickerActive(false);
hashPositionRef.current = -1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if (contextPickerActive && value.length > 0) { | |
| // Update query as user types after "#" | |
| const hashIndex = value.lastIndexOf('#', cursorPos - 1); | |
| if (hashIndex >= 0) { | |
| const textAfterHash = value.substring(hashIndex + 1, cursorPos); | |
| const spaceIndex = textAfterHash.indexOf(' '); | |
| const query = spaceIndex > 0 ? textAfterHash.substring(0, spaceIndex) : textAfterHash; | |
| // Update context picker query | |
| vscodeApi.postMessage({ | |
| command: 'openContextPicker', | |
| query: query | |
| }); | |
| } | |
| } else { | |
| setContextPickerActive(false); | |
| hashPositionRef.current = -1; | |
| } | |
| } else if (contextPickerActive && value.length > 0) { | |
| // Update query as user types after "#" | |
| const hashIndex = value.lastIndexOf('#', cursorPos - 1); | |
| if (hashIndex >= 0) { | |
| const textAfterHash = value.substring(hashIndex + 1, cursorPos); | |
| const spaceIndex = textAfterHash.indexOf(' '); | |
| const query = spaceIndex > 0 ? textAfterHash.substring(0, spaceIndex) : textAfterHash; | |
| // Update context picker query | |
| vscodeApi.postMessage({ | |
| command: 'openContextPicker', | |
| query: query | |
| }); | |
| } else { | |
| // Hash was deleted, deactivate context picker | |
| setContextPickerActive(false); | |
| hashPositionRef.current = -1; | |
| } | |
| } else { | |
| setContextPickerActive(false); | |
| hashPositionRef.current = -1; | |
| } |
🤖 Prompt for AI Agents
In `@ui/src/components/MessageInput.tsx` around lines 98 - 115, When in the branch
handling contextPickerActive, handle the case where the '#' was deleted by
checking hashIndex after computing it; if hashIndex is -1, call
setContextPickerActive(false) and reset hashPositionRef.current = -1 to fully
deactivate the picker and avoid leaving the UI stuck, otherwise continue
extracting textAfterHash and posting the openContextPicker message via
vscodeApi.postMessage as before.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/evaluation.md`:
- Line 144: Replace the generic markdown image reference "! [alt
text](image.png)" with a descriptive alt text and a clear path; specifically
update the alt text to briefly describe the image (e.g., "Evaluation output
structure diagram") and rename the file/path to a meaningful name (e.g.,
assets/images/output-structure.png) or, if the content is better represented as
text, convert the image to a code block or inline markdown diagram; locate the
markdown image line in the document and make the replacement so screen readers
and future maintainers understand the image's purpose.
| The output structure is similar to: | ||
|
|
||
|  No newline at end of file | ||
|  |
There was a problem hiding this comment.
Improve image reference for accessibility and clarity.
The image reference uses generic placeholder text "alt text" and an unclear path "image.png". This impacts both accessibility (screen readers need descriptive alt text) and maintainability (unclear where the image is located or what it shows).
📝 Suggested improvement
Replace with a descriptive reference, for example:
-
+Or if the image shows the output structure, consider whether a code block might be clearer than an image.
🤖 Prompt for AI Agents
In `@docs/evaluation.md` at line 144, Replace the generic markdown image reference
"! [alt text](image.png)" with a descriptive alt text and a clear path;
specifically update the alt text to briefly describe the image (e.g.,
"Evaluation output structure diagram") and rename the file/path to a meaningful
name (e.g., assets/images/output-structure.png) or, if the content is better
represented as text, convert the image to a code block or inline markdown
diagram; locate the markdown image line in the document and make the replacement
so screen readers and future maintainers understand the image's purpose.
1 participant
This pull request introduces a new file attachment feature to the chat UI, allowing users to attach files and context references to their messages. It also improves the error display and integrates context picking (e.g., referencing files or symbols with "#") into the message input. The changes span the frontend React components, chat state management, and styling.
New file attachment and context picking functionality:
ui/src/hooks/useChat.ts[1] [2] [3] [4] [5] [6];ui/src/components/MessageInput.tsx[7] [8] [9] [10];ui/src/components/MessageList.tsx[11];ui/src/ChatApp.tsx[12] [13]ui/src/components/MessageInput.tsx[1] [2];ui/src/hooks/useChat.ts[3]UI/UX improvements:
ui/src/ChatApp.tsxui/src/ChatApp.tsxL63-R76)ui/src/components/MessageInput.tsx[1] [2];ui/src/components/MessageList.tsx[3]Styling and configuration:
ui/src/index.cssui/src/index.cssR1)ui/.env.template[1];api/__init__.py[2]Summary by CodeRabbit
New Features
UI/UX Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.