Skip to content

Conversation

@ironAiken2
Copy link
Contributor

@ironAiken2 ironAiken2 commented Oct 20, 2025

Resolves #4316 (FR-1501)

Changes

Fixes file upload status tracking and progress calculation issues, particularly for concurrent uploads. Refactored the upload state management to properly track upload progress using bytes instead of just file counts.

Key Changes

  1. Enhanced State Management Structure

  2. Fixed Progress Calculation

  • Problem: Progress was only tracked by file count, not actual bytes uploaded
  • Solution:
    • Added completedBytes to track actual upload progress
    • Added totalExpectedSize for accurate percentage calculation
    • Progress now shows: completedBytes / totalExpectedSize * 100
  1. Fixed Cumulative Bytes Issue
  • Problem: onProgress callback provides cumulative bytesUploaded, not incremental
  • Solution: Calculate delta bytes for each progress update
  let previousBytesUploaded = 0;
  const deltaBytes = bytesUploaded - previousBytesUploaded;
  previousBytesUploaded = bytesUploaded;
  1. Improved Concurrent Upload Handling
  • Problem: When starting new uploads while others are in progress, total size wasn't properly accumulated
  • Solution:
    • totalExpectedSize accumulates across all upload requests
    • All progress callbacks reference the same total from state
    • Proper reset when all uploads complete
  1. Better Upload Status Notifications
  • Progress notifications now show both file count and byte-based percentage
  • More accurate progress tracking during uploads
  • Proper cleanup of state after completion

Testing

  • Single file upload progress tracking
  • Multiple files upload (folder) progress tracking
  • Concurrent uploads (starting new upload while another
    is in progress)
  • Upload failure handling
  • Progress percentage accuracy

Technical Details

The main issue was that the original implementation only tracked file names without considering actual bytes uploaded. This led to inaccurate progress reporting, especially for files of varying sizes. The new implementation:

  1. Tracks both file counts and bytes for comprehensive progress reporting
  2. Properly handles cumulative vs incremental byte reporting from the TUS upload library
  3. Maintains a single source of truth (totalExpectedSize) for all concurrent uploads
  4. Correctly accumulates sizes when new uploads are added

Checklist:

  • Documentation
  • Minimum required manager version
  • Specific setting for review
  • Minimum requirements to check during review
  • Test case: Upload multiple files and verify status tracking works correctly

@github-actions github-actions bot added the size:L 100~500 LoC label Oct 20, 2025
Copy link
Contributor Author

ironAiken2 commented Oct 20, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ironAiken2 ironAiken2 marked this pull request as ready for review October 20, 2025 05:12
Copilot AI review requested due to automatic review settings October 20, 2025 05:12
@github-actions
Copy link

github-actions bot commented Oct 20, 2025

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements 4.66% 532/11416
🔴 Branches 3.77% 302/8008
🔴 Functions 2.88% 102/3544
🔴 Lines 4.61% 514/11157

Test suite run success

121 tests passing in 14 suites.

Report generated by 🧪jest coverage report action from d79efea

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes inaccurate and fragile file upload status tracking by restructuring status data and improving progress calculation and notifications.

  • Introduces structured UploadStatus objects with vFolderNames and totalSize.
  • Adds byte-based progress and concurrent uploads (PQueue concurrency 10).
  • Adjusts FolderExplorer to new status shape.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
react/src/components/FolderExplorer.tsx Updated empty-pending check to match new nested status structure.
react/src/components/FileUploadManager.tsx Refactored data structures, progress computation, notification messaging, and concurrency.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ironAiken2 ironAiken2 force-pushed the fix/FR-1501-file-upload-status-tracking branch from d5cfaff to 3f7e879 Compare October 20, 2025 07:23
Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please apply copilot's reviews and resolve conflicts

@ironAiken2 ironAiken2 force-pushed the fix/FR-1501-file-upload-status-tracking branch 3 times, most recently from bbb4912 to c4f7e84 Compare October 27, 2025 06:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ironAiken2 ironAiken2 force-pushed the fix/FR-1501-file-upload-status-tracking branch from c4f7e84 to 8a3b9bd Compare October 27, 2025 06:44
@ironAiken2 ironAiken2 marked this pull request as draft October 27, 2025 08:22
@ironAiken2 ironAiken2 force-pushed the fix/FR-1501-file-upload-status-tracking branch from 8a3b9bd to 443b82c Compare October 28, 2025 03:59
@github-actions github-actions bot added area:ux UI / UX issue. area:i18n Localization labels Oct 28, 2025
@ironAiken2 ironAiken2 marked this pull request as ready for review October 28, 2025 03:59
@ironAiken2 ironAiken2 force-pushed the fix/FR-1501-file-upload-status-tracking branch from 443b82c to a73aed7 Compare October 28, 2025 04:51
@ironAiken2 ironAiken2 force-pushed the fix/FR-1501-file-upload-status-tracking branch from a73aed7 to f2deff6 Compare October 28, 2025 08:17
@github-actions github-actions bot added size:XL 500~ LoC and removed size:L 100~500 LoC labels Oct 28, 2025
Copy link
Member

@yomybaby yomybaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@agatha197 agatha197 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yomybaby yomybaby force-pushed the fix/FR-1501-file-upload-status-tracking branch from f2deff6 to c08b298 Compare October 28, 2025 09:46
@graphite-app
Copy link

graphite-app bot commented Oct 28, 2025

Merge activity

Resolves #4316 ([FR-1501](https://lablup.atlassian.net/browse/FR-1501))

## Changes

Fixes file upload status tracking and progress  calculation issues, particularly for concurrent  uploads. Refactored the upload state management to  properly track upload progress using bytes instead of  just file counts.

### Key Changes

1. Enhanced State Management Structure

  2. Fixed Progress Calculation

  - Problem: Progress was only tracked by file count, not actual bytes uploaded
  - Solution:
    - Added completedBytes to track actual upload progress
    - Added totalExpectedSize for accurate percentage calculation
    - Progress now shows: completedBytes /  totalExpectedSize * 100

  3.  Fixed Cumulative Bytes Issue

  - Problem: onProgress callback provides cumulative bytesUploaded, not incremental
  - Solution: Calculate delta bytes for each progress update
```
  let previousBytesUploaded = 0;
  const deltaBytes = bytesUploaded - previousBytesUploaded;
  previousBytesUploaded = bytesUploaded;
```

4. Improved Concurrent Upload Handling

  - Problem: When starting new uploads while others are in progress, total size wasn't properly accumulated
  - Solution:
    - totalExpectedSize accumulates across all upload requests
    - All progress callbacks reference the same total from state
    - Proper reset when all uploads complete

  5. Better Upload Status Notifications

  - Progress notifications now show both file count and byte-based percentage
  - More accurate progress tracking during uploads
  - Proper cleanup of state after completion

  Testing

  - Single file upload progress tracking
  - Multiple files upload (folder) progress tracking
  - Concurrent uploads (starting new upload while another
   is in progress)
  - Upload failure handling
  - Progress percentage accuracy

  Technical Details

  The main issue was that the original implementation only tracked file names without considering actual bytes uploaded. This led to inaccurate progress reporting, especially for files of varying sizes. The new implementation:

  1. Tracks both file counts and bytes for comprehensive progress reporting
  2. Properly handles cumulative vs incremental byte reporting from the TUS upload library
  3. Maintains a single source of truth (totalExpectedSize) for all concurrent uploads
  4. Correctly accumulates sizes when new uploads are added

**Checklist:**

- [ ] Documentation
- [ ] Minimum required manager version
- [ ] Specific setting for review
- [ ] Minimum requirements to check during review
- [x] Test case: Upload multiple files and verify status tracking works correctly

[FR-1501]: https://lablup.atlassian.net/browse/FR-1501?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@graphite-app graphite-app bot force-pushed the fix/FR-1501-file-upload-status-tracking branch from c08b298 to d79efea Compare October 28, 2025 09:47
@graphite-app graphite-app bot merged commit d79efea into main Oct 28, 2025
11 checks passed
@graphite-app graphite-app bot deleted the fix/FR-1501-file-upload-status-tracking branch October 28, 2025 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n Localization area:ux UI / UX issue. size:XL 500~ LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhance the file upload logic to support multi-file uploads in the upload queue, and update the notification display accordingly

3 participants