Skip to content

Conversation

manavgup
Copy link
Owner

@manavgup manavgup commented Oct 8, 2025

Summary

Replaces simulated file upload with real file upload functionality in the collection creation modal. This enables users to actually upload documents when creating a new collection, with proper backend processing.

Changes

Frontend Upload Implementation

  • File Storage: Modified UploadedFile interface to store actual File objects
  • Removed Simulation: Deleted simulateUpload() function that was faking progress
  • Conditional API Calls:
    • Calls createCollectionWithFiles() when files are present
    • Calls createCollection() when creating empty collection
  • Drag-and-Drop Fix: Properly extracts File objects from drag events

User Experience Improvements

  • Double-Submission Prevention: Added useRef hook to prevent accidental double-clicks
  • Better Error Handling: Detects and displays specific message for duplicate collection names
  • Enhanced Notifications: Shows file count in success message when files are uploaded
  • Improved Modal Callback: Properly passes created collection back to parent component

Technical Details

Before: Files were added to UI with fake progress animation, but never actually uploaded

After: Files are:

  1. Stored as actual File objects in state
  2. Sent to backend via FormData when user clicks Create Collection
  3. Processed by backend document ingestion pipeline
  4. Success/error feedback shown based on actual API response

Code Changes

LightweightCreateCollectionModal.tsx

  • Added file: File to UploadedFile interface
  • Removed fake upload simulation
  • Added double-submission guard
  • Improved error messages for duplicate names
  • Fixed drag-and-drop file handling

LightweightCollections.tsx

  • Updated handleCollectionCreated() to accept collection object from modal
  • Removed duplicate API call (modal now handles creation)

apiClient.ts

  • Already had createCollectionWithFiles() method implemented

Testing

  • Tested creating collection with files (triggers document processing)
  • Tested creating collection without files (works as before)
  • Verified drag-and-drop file upload works correctly
  • Confirmed duplicate name error shows proper message
  • Validated double-click protection works

Dependencies

Signed-off-by: Manav Gup manavgup@gmail.com

- Store actual File objects in UploadedFile interface instead of simulating uploads
- Remove fake upload progress simulation (simulateUpload function)
- Add conditional logic to call createCollectionWithFiles when files are present
- Fix drag-and-drop handling to properly extract File objects
- Implement double-submission prevention with useRef hook
- Add better error handling with duplicate name detection
- Update success notifications to show file count when files are uploaded
- Fix modal callback to properly handle collection creation response

Signed-off-by: Manav Gupta <manavg@gmail.com>
Copy link
Contributor

github-actions bot commented Oct 8, 2025

🚀 Development Environment Options

This repository supports Dev Containers for a consistent development environment.

Option 1: GitHub Codespaces (Recommended)

Create a cloud-based development environment:

  1. Click the green Code button above
  2. Select the Codespaces tab
  3. Click Create codespace on pr-03-frontend-upload-pipeline
  4. Wait 2-3 minutes for environment setup
  5. Start coding with all tools pre-configured!

Option 2: VS Code Dev Containers (Local)

Use Dev Containers on your local machine:

  1. Install Docker Desktop
  2. Install VS Code
  3. Install the Dev Containers extension
  4. Clone this PR branch locally
  5. Open in VS Code and click "Reopen in Container" when prompted

Option 3: Traditional Local Setup

Set up the development environment manually:

# Clone the repository
git clone https://github.com/manavgup/rag_modulo.git
cd rag_modulo
git checkout pr-03-frontend-upload-pipeline

# Initialize development environment
make dev-init
make dev-build
make dev-up
make dev-validate

Available Commands

Once in your development environment:

make help           # Show all available commands
make dev-validate   # Validate environment setup
make test-atomic    # Run atomic tests
make test-unit      # Run unit tests
make lint          # Run linting

Services Available

When running make dev-up:


This automated message helps reviewers quickly set up the development environment.

Copy link

claude bot commented Oct 8, 2025

PR Review: Real File Upload Implementation

Overall Assessment

This PR successfully implements real file upload functionality. The implementation is solid with good error handling and UX improvements.


Strengths

  1. Clean Architecture - Proper separation of concerns, smart conditional routing
  2. Excellent Error Handling - Double-submission prevention, duplicate name detection, HTTP 409 responses
  3. Improved UX - File count in notifications, better drag-and-drop
  4. Backend Integration - New upload endpoint, consistent error handling

Issues and Recommendations

1. CRITICAL: Potential ID Collision (Lines 74, 246)

When multiple files are added rapidly, Date.now() could return same timestamp causing duplicate IDs.
Fix: Use crypto.randomUUID() or add a counter

2. Code Duplication (Lines 69-84 vs 241-256)

File object creation logic is duplicated. Extract into shared createUploadedFile() function.

3. Missing File Validation

UI shows accept and max 100MB but no actual client-side validation.
Fix: Add validation for file type and size

4. Backend: Accessing Private Method (Line 705)

Calling _upload_files_and_trigger_processing violates encapsulation.
Fix: Make it a public method

5. TypeScript Strictness (Lines 136, 138)

Using error: any instead of proper type guards.
Fix: Use AxiosError type and instanceof checks

6. Security: Missing File Content Validation

Backend should validate MIME type, not just extension.

7. Performance: Large File Handling

Storing large File objects in state could cause issues. Consider chunked uploads.

8. Missing Cleanup on Modal Close

Reset submittingRef in handleClose to prevent stuck state.


Test Coverage

No new tests added. Please add tests for upload logic, drag-and-drop, and error scenarios.


Approval Recommendation

APPROVE with minor changes

Priority:

  1. Must fix: ID collision
  2. Should fix: Code duplication, file validation
  3. Nice to have: Everything else

Great work! The approach is sound and error handling is well done.

Fix import ordering to pass ruff CI checks.

Signed-off-by: Manav Gupta <manavg@gmail.com>
@manavgup manavgup merged commit 7895fd3 into main Oct 9, 2025
24 checks passed
@manavgup manavgup deleted the pr-03-frontend-upload-pipeline branch October 9, 2025 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant