feat(frontend): add HuggingFace audio upload component#5567
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #5567 +/- ##
============================================
+ Coverage 56.74% 56.80% +0.05%
Complexity 3024 3024
============================================
Files 1124 1126 +2
Lines 43593 43702 +109
Branches 4712 4733 +21
============================================
+ Hits 24739 24826 +87
- Misses 17386 17405 +19
- Partials 1468 1471 +3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Ma77Ball
left a comment
There was a problem hiding this comment.
Left Comments, please address or give reply.
d8cb4ef to
df965e5
Compare
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 382 | 0.233 | 23,714/41,418/41,418 us | 🔴 +32.6% / 🔴 +174.8% |
| ⚪ | bs=100 sw=10 sl=64 | 803 | 0.49 | 122,895/141,268/141,268 us | ⚪ within ±5% / 🔴 +29.8% |
| ⚪ | bs=1000 sw=10 sl=64 | 924 | 0.564 | 1,083,581/1,129,657/1,129,657 us | ⚪ within ±5% / 🔴 +8.3% |
Baseline details
Latest main 613b76e from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 382 tuples/sec | 450 tuples/sec | 770.82 tuples/sec | -15.1% | -50.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.233 MB/s | 0.274 MB/s | 0.47 MB/s | -15.0% | -50.5% |
| bs=10 sw=10 sl=64 | p50 | 23,714 us | 21,305 us | 12,723 us | +11.3% | +86.4% |
| bs=10 sw=10 sl=64 | p95 | 41,418 us | 31,243 us | 15,070 us | +32.6% | +174.8% |
| bs=10 sw=10 sl=64 | p99 | 41,418 us | 31,243 us | 18,429 us | +32.6% | +124.7% |
| bs=100 sw=10 sl=64 | throughput | 803 tuples/sec | 810 tuples/sec | 973.75 tuples/sec | -0.9% | -17.5% |
| bs=100 sw=10 sl=64 | MB/s | 0.49 MB/s | 0.494 MB/s | 0.594 MB/s | -0.8% | -17.6% |
| bs=100 sw=10 sl=64 | p50 | 122,895 us | 122,118 us | 102,519 us | +0.6% | +19.9% |
| bs=100 sw=10 sl=64 | p95 | 141,268 us | 138,939 us | 108,855 us | +1.7% | +29.8% |
| bs=100 sw=10 sl=64 | p99 | 141,268 us | 138,939 us | 117,788 us | +1.7% | +19.9% |
| bs=1000 sw=10 sl=64 | throughput | 924 tuples/sec | 914 tuples/sec | 1,004 tuples/sec | +1.1% | -8.0% |
| bs=1000 sw=10 sl=64 | MB/s | 0.564 MB/s | 0.558 MB/s | 0.613 MB/s | +1.1% | -8.0% |
| bs=1000 sw=10 sl=64 | p50 | 1,083,581 us | 1,093,513 us | 1,001,930 us | -0.9% | +8.1% |
| bs=1000 sw=10 sl=64 | p95 | 1,129,657 us | 1,139,585 us | 1,042,923 us | -0.9% | +8.3% |
| bs=1000 sw=10 sl=64 | p99 | 1,129,657 us | 1,139,585 us | 1,074,893 us | -0.9% | +5.1% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,522.95,200,128000,382,0.233,23714.14,41418.29,41418.29
1,100,10,64,20,2491.64,2000,1280000,803,0.490,122894.97,141267.54,141267.54
2,1000,10,64,20,21634.43,20000,12800000,924,0.564,1083580.70,1129656.68,1129656.68e5ac4ef to
e3a3335
Compare
Ma77Ball
left a comment
There was a problem hiding this comment.
LGTM. All prior comments resolved:
- re-entry guard (if (this.isUploading) return;) added to onFileSelected
- spec now real TestBed coverage (rejects non-audio, uploads + sets formControl, guards concurrent uploads); dead metadata variable removed
No other issues. Static review only, not built or run.
@xuang7 can you review.
fa99eb7 to
a61374f
Compare
a61374f to
fa99eb7
Compare
Automated Reviewer SuggestionsBased on the
|
2a7ac1d to
7aa553a
Compare
xuang7
left a comment
There was a problem hiding this comment.
Left one comment. Please update the target branch to the latest main and rebase your branch.
aebc137 to
db598da
Compare
xuang7
left a comment
There was a problem hiding this comment.
LGTM overall. One minor non-blocking comment: the two catch blocks currently swallow the error entirely without any logging. Also, please address the merge conflict when you get a chance.
6054455 to
77a99d6
Compare
Register the huggingface-audio-upload formly field type and declare HuggingFaceAudioUploadComponent in AppModule. Handles server-side audio storage via the /huggingface/upload-audio endpoint with local preview. Co-Authored-By: Anish Shivamurthy <anish@uci.edu>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… upload component Add early return in onFileSelected when isUploading is true to prevent race conditions from concurrent file selections. Remove the second spec test that had a dead metadata variable and duplicated the first test's assertion. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ponent Add meaningful tests: reject non-audio files, successful upload sets formControl value, and concurrent upload guard prevents race conditions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use preview URL as a token to detect if the user cleared during an in-flight upload and discard the stale response. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ponent Add tests for ngOnInit, previewSrc, clearAudio, upload error handling, model updates, file validation, and Content-Type verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The native <audio> tag doesn't send the JWT, so reopening a saved workflow would 401 on the /audio-preview endpoint. Fetch the audio as a blob via HttpClient (which carries auth) and use a blob URL for the <audio> element instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ocks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
77a99d6 to
978cada
Compare
|
@xuang7 done and merge conflicts have been resolved |
xuang7
left a comment
There was a problem hiding this comment.
Generally LGTM! The test coverage looks solid overall. One gap worth filling is loadServerAudioPreview. Could you also add tests for uncovered part?
…guards, and edge cases Add tests for loadServerAudioPreview (blob fetch success/error/stale guards), previewSrc with localPreviewUrl, stale upload discard on clear, ngOnDestroy URL revocation, and getDisplayName edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@xuang7 New tests including loadServerAudioPreview test has been written! |
### What changes were proposed in this PR? Add `HuggingFaceAudioUploadComponent`, a custom formly field type (`huggingface-audio-upload`) that provides: - An audio file picker that uploads to the Texera backend's `/huggingface/upload-audio` endpoint for server-side storage - Authenticated audio preview/playback — fetches server-stored audio via `HttpClient` (which carries the JWT) and creates a blob URL for the `<audio>` element, avoiding 401s on workflow reload - Local audio preview using a browser Object URL while upload is in progress - Stale response guards — discards upload results and preview fetches if the user clears the field while a request is in flight - Concurrent upload protection — disables the file input and Clear button during upload - Storage of the returned server path in the formly form control - A guidance message explaining why audio is uploaded to backend storage rather than embedded in the workflow JSON This PR also registers `HuggingFaceComponent` and `HuggingFaceAudioUploadComponent` in `formly-config.ts` and declares them in `AppModule`. The `jsonSchemaMapIntercept` mapping that routes the `audioInput` field to this component is added in the follow-up property-editor PR (PR 7). ### Any related issues, documentation, discussions? - Tracking issue: apache#5314 - Closes: apache#5314 - Stacked on: apache#5566 - Parent issue: apache#5041 ### How was this PR tested? 38 unit tests in `hugging-face-audio-upload.component.spec.ts` covering: - `ngOnInit` — filename extraction from server paths, data URLs, empty/whitespace values - `previewSrc` — returns data URLs as-is, empty for server paths (loaded async), empty for blank values - `onFileSelected` — successful upload, non-audio rejection, concurrent upload guard, no-file guard, upload-in-progress state, error handling, filename fallback, model updates, Content-Type/URL encoding - `loadServerAudioPreview` — successful blob fetch, fetch failure error message, stale response discard on both success and error, no-fetch for data URLs, URL encoding - `clearAudio` — full state reset, error preservation, model clearing, dirty/touched marking - Stale upload guards — discard success/error when cleared during flight - `ngOnDestroy` — blob URL revocation - `getDisplayName` — forward/backslash paths, trailing separator, flat filename Run with `ng test`. ### Was this PR authored or co-authored using generative AI tooling? Co-authored with Claude Opus 4.6 --------- Co-authored-by: Elliot <36275109+Falcons-Royale@users.noreply.github.com> Co-authored-by: Anish Shivamurthy <anish@uci.edu> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
What changes were proposed in this PR?
Add
HuggingFaceAudioUploadComponent, a custom formly field type (huggingface-audio-upload) that provides:/huggingface/upload-audioendpoint for server-side storageHttpClient(which carries the JWT) and creates a blob URL for the<audio>element, avoiding 401s on workflow reloadThis PR also registers
HuggingFaceComponentandHuggingFaceAudioUploadComponentinformly-config.tsand declares them inAppModule. ThejsonSchemaMapInterceptmapping that routes theaudioInputfield to this component is added in the follow-up property-editor PR (PR 7).Any related issues, documentation, discussions?
How was this PR tested?
38 unit tests in
hugging-face-audio-upload.component.spec.tscovering:ngOnInit— filename extraction from server paths, data URLs, empty/whitespace valuespreviewSrc— returns data URLs as-is, empty for server paths (loaded async), empty for blank valuesonFileSelected— successful upload, non-audio rejection, concurrent upload guard, no-file guard, upload-in-progress state, error handling, filename fallback, model updates, Content-Type/URL encodingloadServerAudioPreview— successful blob fetch, fetch failure error message, stale response discard on both success and error, no-fetch for data URLs, URL encodingclearAudio— full state reset, error preservation, model clearing, dirty/touched markingngOnDestroy— blob URL revocationgetDisplayName— forward/backslash paths, trailing separator, flat filenameRun with
ng test.Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.6