feat: implement file attachment system with session management and event handling #44
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const [isThinking, setIsThinking] = useState(false) | ||
| const [sessionId, setSessionId] = useState(null) | ||
| const [attachments, setAttachments] = useState(new Set()) | ||
| const [pendingFileEvents, setPendingFileEvents] = useState(new Map()) |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 25 days ago
The best way to fix this problem is to remove the unused state variable, pendingFileEvents, and its corresponding setter, setPendingFileEvents, from the state definition. This means eliminating the line that defines it (const [pendingFileEvents, setPendingFileEvents] = useState(new Map())) and ensuring that references to setPendingFileEvents in callback creators continue to function using only the setter (since the setter can be defined independently if needed, but the value itself does not need to be destructured or retained). All usages of setPendingFileEvents can remain as-is, as the issue solely concerns the unused value pendingFileEvents. This change should be made in frontend/src/contexts/ChatContext.jsx near line 41.
| @@ -38,7 +38,7 @@ | ||
| const [isThinking, setIsThinking] = useState(false) | ||
| const [sessionId, setSessionId] = useState(null) | ||
| const [attachments, setAttachments] = useState(new Set()) | ||
| const [pendingFileEvents, setPendingFileEvents] = useState(new Map()) | ||
| const [, setPendingFileEvents] = useState(new Map()) | ||
|
|
||
| // Method to add a file to attachments | ||
| const addAttachment = useCallback((fileId) => { |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a file attachment system that allows users to attach library files to chat sessions with real-time UI feedback. The implementation adds session management, attachment tracking, and proper handling of file attachment events.
- Session creation and tracking functionality to ensure files are attached to valid sessions
- File attachment state management with duplicate detection and pending event resolution
- WebSocket handler for file attachment responses with success/error handling
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
frontend/src/test/basic.test.js |
Added unit tests for session ID generation and file attachment event handling |
frontend/src/handlers/chat/websocketHandlers.js |
Added file_attach case handler and refined auto-canvas logic to skip user-uploaded files |
frontend/src/contexts/ChatContext.jsx |
Added session state, attachment tracking, and pending event management with helper methods |
frontend/src/components/Message.jsx |
Removed emoji from file reference display and added rendering for file attachment system events |
frontend/src/components/AllFilesView.jsx |
Refactored file loading to use new attachment system with duplicate checking and event tracking |
backend/tests/test_attach_file_flow.py |
Added comprehensive tests for file attachment, download, and authorization scenarios |
backend/application/chat/utilities/file_utils.py |
Enhanced file manifest description to mention attachments |
| mapMessages(messages => messages.map(msg => | ||
| msg.id === eventId | ||
| ? { ...msg, subtype: newSubtype, text: newText } | ||
| : msg | ||
| )) |
There was a problem hiding this comment.
The mapMessages call inside resolvePendingFileEvent doesn't use the result. mapMessages likely returns a new messages array, but it's being called without capturing or setting the return value. This means the message update will not persist in state.
| } | ||
|
|
||
| // Create a temporary session ID for frontend tracking | ||
| const tempSessionId = `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` |
There was a problem hiding this comment.
Using substr which is deprecated. Use substring or slice instead. The deprecated substr method may be removed in future JavaScript versions.
| const tempSessionId = `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` | |
| const tempSessionId = `session_${Date.now()}_${Math.random().toString(36).substring(2, 11)}` |
| // Simulate ensureSession logic | ||
| const ensureSession = () => { | ||
| return new Promise((resolve) => { | ||
| const tempSessionId = `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; |
There was a problem hiding this comment.
Using deprecated substr method. Replace with substring(2, 11) or slice(2, 11) to avoid using deprecated APIs.
| const tempSessionId = `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; | |
| const tempSessionId = `session_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`; |
| const [isThinking, setIsThinking] = useState(false) | ||
| const [sessionId, setSessionId] = useState(null) | ||
| const [attachments, setAttachments] = useState(new Set()) | ||
| const [pendingFileEvents, setPendingFileEvents] = useState(new Map()) |
There was a problem hiding this comment.
Unused variable pendingFileEvents.
| const [pendingFileEvents, setPendingFileEvents] = useState(new Map()) | |
| const [, setPendingFileEvents] = useState(new Map()) |
| triggerFileDownload | ||
| triggerFileDownload, | ||
| addAttachment, | ||
| addPendingFileEvent, |
There was a problem hiding this comment.
Unused variable addPendingFileEvent.
| addPendingFileEvent, |
| @@ -0,0 +1,210 @@ | |||
| import base64 | |||
| import uuid | |||
| import asyncio | |||
There was a problem hiding this comment.
Import of 'asyncio' is not used.
| import asyncio |
…ration with secure random string
Makes it more clear when a file is attached from the library