Conversation
There was a problem hiding this comment.
Pull request overview
Adds preliminary support for rendering bedj tracks from a client-supplied in-memory list of BED-like items (instead of reading a BED/bigBed file), with an integration test demonstrating the new behavior.
Changes:
- Server: refactors bedj item acquisition into
getBEDitems()and adds areq.query.bedItemscode path. - Client: includes
bedItemsin the POST payload for bedj tracks when present. - Tests: adds an integration test that renders a bedj track from
bedItems.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| server/src/bedj.js | Adds bedItems support and factors item loading/filtering into getBEDitems() |
| client/src/block.js | Sends bedItems in bedj request payload when provided on the track config |
| client/src/test/block.integration.spec.ts | Adds integration coverage for rendering a bedj track from a custom bedItems list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async function getBEDitems(req, genomeobj, flag_gm, gmisoform) { | ||
| if (req.query.bedItems) { | ||
| // client supplies list of "bed" items to render. no file to read | ||
| if (!Array.isArray(req.query.bedItems)) throw 'bedItems not array' | ||
| const lst = [] | ||
| for (const j of req.query.bedItems) { | ||
| if (typeof j != 'object') throw 'one of bedItems[] not obj' | ||
| if (!j.chr) throw 'bedItems[].chr missing' | ||
| if (!Number.isInteger(j.start)) throw 'bedItems[].start not integer' | ||
| if (!Number.isInteger(j.stop)) throw 'bedItems[].stop not integer' | ||
| j.rglst = [] | ||
| // duplicate following lines with elsewhere as chr equivalency check is needed here but not there | ||
| for (let i = 0; i < req.query.rglst.length; i++) { | ||
| const r = req.query.rglst[i] | ||
| if (r.chr != j.chr) continue | ||
| if (Math.max(j.start, r.start) < Math.min(j.stop, r.stop)) { | ||
| j.rglst.push({ idx: i }) | ||
| } | ||
| } | ||
| if (j.rglst.length == 0) continue | ||
| lst.push(j) | ||
| } | ||
| return lst | ||
| } |
There was a problem hiding this comment.
When bedItems is provided, the function returns early and bypasses the existing filtering logic (usevalue, bplengthUpperLimit, filterByName). If these query params are meant to apply uniformly to all bedj data sources, the same filters should be applied to bedItems as well (or the API should explicitly reject/ignore them to avoid inconsistent behavior).
| const lst = [] | ||
| for (const j of req.query.bedItems) { | ||
| if (typeof j != 'object') throw 'one of bedItems[] not obj' | ||
| if (!j.chr) throw 'bedItems[].chr missing' | ||
| if (!Number.isInteger(j.start)) throw 'bedItems[].start not integer' | ||
| if (!Number.isInteger(j.stop)) throw 'bedItems[].stop not integer' | ||
| j.rglst = [] | ||
| // duplicate following lines with elsewhere as chr equivalency check is needed here but not there | ||
| for (let i = 0; i < req.query.rglst.length; i++) { | ||
| const r = req.query.rglst[i] | ||
| if (r.chr != j.chr) continue | ||
| if (Math.max(j.start, r.start) < Math.min(j.stop, r.stop)) { | ||
| j.rglst.push({ idx: i }) | ||
| } |
There was a problem hiding this comment.
bedItems processing is O(bedItems × rglst) and there is no server-side cap on the number of items beyond the global 5MB JSON body limit. A large bedItems payload can still cause significant CPU/memory load during stacking/rendering; consider enforcing a reasonable max item count (and/or pre-indexing by chr) and returning a clear error when exceeded.
| if (!Array.isArray(req.query.bedItems)) throw 'bedItems not array' | ||
| const lst = [] | ||
| for (const j of req.query.bedItems) { | ||
| if (typeof j != 'object') throw 'one of bedItems[] not obj' |
There was a problem hiding this comment.
typeof j != 'object' will not reject null (and will also allow arrays), which can lead to a runtime TypeError at j.chr rather than the intended validation error. Consider explicitly checking j && !Array.isArray(j) && typeof j === 'object' (or similar) before accessing fields.
| if (typeof j != 'object') throw 'one of bedItems[] not obj' | |
| if (!j || Array.isArray(j) || typeof j !== 'object') throw 'one of bedItems[] not obj' |
Description
see example in newly added test in test/block.integration.spec.ts, which renders as below
TODO
Checklist
Check each task that has been performed or verified to be not applicable.