Conversation
Shehrozkashif
left a comment
Shehrozkashif
left a comment
There was a problem hiding this comment.
I’d suggest moving these changes into a separate JS file at frontend/script and removing the <script> tag from upload.html.
frontend/html/upload.html
Outdated
| const arrayBuffer = await chunk.arrayBuffer(); | ||
|
|
||
| const base64Chunk = await arrayBufferToBase64(arrayBuffer); | ||
| const result = await window.pywebview.api.save_file_chunk( |
There was a problem hiding this comment.
Please add a guard to ensure the pywebview API exists before calling it. If the bridge fails to initialize, this will crash the upload instead of showing a helpful error.
frontend/html/upload.html
Outdated
| offset += CHUNK_SIZE; | ||
| chunkIndex++; | ||
|
|
||
| const progress = Math.min((offset/file.size)*100, 100); |
There was a problem hiding this comment.
This can produce floating-point percentages like 37.333333%. Please round to integers for cleaner UI.
frontend/main.py
Outdated
| except Exception as e: | ||
| return f"error: {str(e)}" | ||
|
|
||
| def save_file_chunk(self, filename, chunk_data, chunk_index, is_last): |
There was a problem hiding this comment.
Add a full docstring explaining purpose, parameters, side-effects, and return type
frontend/script/upload.js
Outdated
| //Define Maximum File Size as 50MB | ||
| const MAX_FILE_SIZE = 50 * 1024 * 1024; | ||
|
|
||
| //Define Upload Chunk Size as 1MB | ||
| const CHUNK_SIZE = 1024 * 1024; | ||
|
|
There was a problem hiding this comment.
correct the indentation plz!
frontend/script/upload.js
Outdated
| if (!window.pywebview || !window.pywebview.api || typeof window.pywebview.api.save_file_chunk !== "function") { | ||
| alert("Upload service is not available. Please try again."); | ||
| resetUploadUI(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
correct intendation !
|
@r1thk4 I’d encourage you to add test coverage for your changes and extend the existing unit tests where applicable. Also, as a best practice, try to squash this into a single commit with a clear, detailed commit message describing the changes you made. |
Shehrozkashif
force-pushed
the
main
branch
2 times, most recently
from
9858304 to
7f6e102
Compare
|
Thanks for the suggestion! I’ve updated the PR by fixing the formatting issues and adding unit tests for the changes. |
|
@r1thk4 Have you pulled the changes from main? |
- Normalize indentation and formatting in upload.js - Add unit tests for Api.save_file_chunk - Mark frontend as a package to resolve mypy module ambiguity
r1thk4
force-pushed
the
fix-chunked-file-upload
branch
from
c797aab to
fff4928
Compare
|
After rebasing this branch onto the latest It looks similar to an issue that was previously addressed, but the empty-input guard doesn’t seem to be present in the current main. Could you please advise on the best way to proceed? I’m happy to either open a separate PR to address it or include the fix in this branch to keep CI passing. |
|
@r1thk4 The CI failure you’re seeing isn’t caused by your upload changes. It’s happening because To keep tests independent from GUI dependencies, please make the webview import optional in try:
import webview
except ImportError:
webview = NoneThis allows CI to run without requiring GUI libraries while keeping the app functional locally. Regarding the empty-input issue you noticed after rebasing: good catch. Let’s keep this PR focused on the upload feature. If the classifier bug still exists on Once the optional import change is in, CI should pass and we can move forward with merging. |
2 participants
Due to PyWebView GUI dependencies in WSL, I was unable to run the full UI locally. The changes were validated by tracing the upload flow from upload.html - uploadFile() - pywebview.api.save_file and implementing chunked handling accordingly. Please review and let me know if any adjustments are needed after testing.