Conversation
Deploying bedhost-ui with
|
| Latest commit: |
2294830
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Pull request overview
This PR moves BED parsing in the UI to a streaming Web Worker backed by the @databio/gtars WASM parser, enabling incremental progress reporting and avoiding loading full files into memory on the main thread.
Changes:
- Added a dedicated
bedAnalyzerWorkerthat streamsFile/fetch()bodies into the WASM parser and reports status/progress. - Updated the BED analytics page to use the worker for parsing and to render a progress indicator.
- Updated the UI’s
@databio/gtarsdependency source.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ui/src/workers/bedAnalyzerWorker.ts | New worker that streams BED data into the WASM parser and posts status/progress/result messages. |
| ui/src/pages/bed-analytics.tsx | Uses the worker for analysis, tracks progress/status, and updates loading UI. |
| ui/src/components/bed-analytics-components/chromosome-stats-panel.tsx | Formatting-only change. |
| ui/package.json | Changes the @databio/gtars dependency reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| bytesProcessed += value.length; | ||
| self.postMessage({ | ||
| type: 'progress', | ||
| bytesProcessed, | ||
| totalSize, | ||
| percent: totalSize > 0 ? Math.round((100 * bytesProcessed) / totalSize) : -1, | ||
| }); |
There was a problem hiding this comment.
Progress updates are posted to the main thread for every streamed chunk. For large files this can generate a very high message rate and noticeably slow parsing/UI updates. Consider throttling these messages (e.g., post at most every N ms or every M bytes, always sending a final 100% update at the end).
| useEffect(() => { | ||
| initializeRegionSet(); | ||
| initializeAnalysis(); | ||
| }, [triggerSearch]); |
There was a problem hiding this comment.
This effect calls initializeAnalysis but doesn’t include it in the dependency array. With plugin:react-hooks/recommended enabled and --max-warnings 0, this will fail lint due to react-hooks/exhaustive-deps. Refactor so the effect satisfies exhaustive-deps without changing the intended “only run when triggerSearch changes” behavior (e.g., make initializeAnalysis stable via refs, or restructure the effect logic accordingly).
|
|
||
| const contentLength = response.headers.get('content-length'); | ||
| const totalSize = contentLength ? parseInt(contentLength, 10) : 0; | ||
| const reader = response.body!.getReader(); |
There was a problem hiding this comment.
response.body can be null (e.g., for certain response types or environments), but this code uses a non-null assertion and will throw at runtime if it is. Handle the null case explicitly and emit a worker error message (or throw a clearer error) before calling getReader().
| const reader = response.body!.getReader(); | |
| const body = response.body; | |
| if (!body) { | |
| throw new Error('Failed to process BED file: response body is not available as a readable stream.'); | |
| } | |
| const reader = body.getReader(); |
1 participant
Changes:
TODO:
__version__.pyfile