-
Notifications
You must be signed in to change notification settings - Fork 50
Implement file attachment feature #8
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
Conversation
Co-authored-by: pushpak1300 <[email protected]>
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.
Bug: Inconsistent Attachment Data Types
The attachments field has an inconsistent data type: user messages now store attachments as an array, while assistant messages still store them as a JSON string ('[]'). This type mismatch violates data consistency, causing potential runtime errors and data handling issues when processing messages. Existing tests confirm this discrepancy, expecting different types for user versus assistant message attachments.
app/Http/Controllers/ChatStreamController.php#L26-L62
ai-chat/app/Http/Controllers/ChatStreamController.php
Lines 26 to 62 in cba6d34
| $model = $request->enum('model', ModelName::class, ModelName::GPT_4_1_NANO); | |
| $attachments = $request->input('attachments', []); | |
| $chat->messages()->create([ | |
| 'role' => 'user', | |
| 'parts' => [ | |
| ChunkType::Text->value => $userMessage, | |
| ], | |
| 'attachments' => $attachments, | |
| ]); | |
| $messages = $this->buildConversationHistory($chat); | |
| return Response::stream(function () use ($chat, $messages, $model): Generator { | |
| $parts = []; | |
| try { | |
| $response = Prism::text() | |
| ->withSystemPrompt(view('prompts.system')) | |
| ->using($model->getProvider(), $model->value) | |
| ->withMessages($messages) | |
| ->asStream(); | |
| foreach ($response as $chunk) { | |
| $chunkData = [ | |
| 'chunkType' => $chunk->chunkType->value, | |
| 'content' => $chunk->text, | |
| ]; | |
| if (! isset($parts[$chunk->chunkType->value])) { | |
| $parts[$chunk->chunkType->value] = ''; | |
| } | |
| $parts[$chunk->chunkType->value] .= $chunk->text; | |
| yield json_encode($chunkData)."\n"; | |
| } |
Bug: Concurrent Uploads Reset Flag Prematurely
The isUploading flag is prematurely reset to false when multiple files are uploaded concurrently, as it's cleared in the finally block of each individual upload, even if other uploads are still in progress. This leads to an inaccurate overall upload status. Furthermore, the uploadQueue removal logic using indexOf(file.name) is flawed; it can incorrectly remove or leave entries in the queue if multiple files with identical names are uploaded.
resources/js/composables/useFileUpload.ts#L22-L55
ai-chat/resources/js/composables/useFileUpload.ts
Lines 22 to 55 in cba6d34
| try { | |
| isUploading.value = true | |
| uploadQueue.value.push(file.name) | |
| const response = await fetch('/api/file/upload', { | |
| method: 'POST', | |
| body: formData, | |
| headers: { | |
| 'X-CSRF-TOKEN': document.querySelector('meta[name="csrf-token"]')?.getAttribute('content') || '', | |
| 'Accept': 'application/json', | |
| }, | |
| }) | |
| const result: UploadResponse = await response.json() | |
| if (result.success) { | |
| uploadedFiles.value.push(result) | |
| return result | |
| } else { | |
| console.error('Upload failed:', result.message) | |
| return null | |
| } | |
| } catch (error) { | |
| console.error('Upload error:', error) | |
| return null | |
| } finally { | |
| // Remove from upload queue | |
| const index = uploadQueue.value.indexOf(file.name) | |
| if (index > -1) { | |
| uploadQueue.value.splice(index, 1) | |
| } | |
| isUploading.value = false | |
| } |
BugBot free trial expires on July 22, 2025
You have used $0.00 of your $20.00 spend limit so far. Manage your spend limit in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Implement file attachment feature to allow users to upload and send files in chat.