Repository navigation
feat(viewer): open any finding in the genome viewer from the live site or the row itself - #296
Merged
Merged
Conversation
The report linked only the ClinVar hits, the secondary findings and the mitochondrial variants to the genome viewer, as one line of links under each table. Every findings table now ends with a Viewer column, one link per row to that row's position: ClinVar hits, secondary findings, mito variants, deep intronic sites, the cDNA scan, AnnotSV class 4-5 and the calls moved to class 3, slivar's pairs in trans and mosaic alterations. A column of its own keeps every other cell's text as it was. Bead: pgp-awpe
The genome viewer ran only on 127.0.0.1 of the machine holding the data. bin/owngenome_live_viewer.py serves the same igv.js page for every run of the live site, behind its password, and points the served report's Viewer links at it. The igv.js pin moves to versions.env so both launchers share it. tests/test_live_viewer.py checks the login, the policy, the byte ranges, the refused files, the report links, the access log and the page in headless Chrome; tests/test_viewer.py checks every findings table and reads the pin from versions.env. Bead: pgp-5bja.8 Bead: pgp-awpe
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Owner
Author
|
@coderabbitai review |
Action not completed
|
…ps tracks with an index out of GENOME_DIR and serves quoted names The live viewer compared the still-quoted request name with the file name, so a reference named with a space, '#' or '?' answered 404. It now unquotes it. An index symlinked out of GENOME_DIR was served; now its track, file and index are left out. Each new check has a planted mistake that must fail it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
The genome viewer only ran on 127.0.0.1 of the machine that holds the data, with no login. From the live site on a phone or another laptop, the report's "see the reads" links pointed at a port that was not there. In the report itself, only three tables had viewer links, as one line of links under each table, so the Slivar pairs, the AnnotSV class 4-5 SVs, the deep intronic sites, the cDNA scan and the mosaic calls had no way to open the reads.
The fix
bin/owngenome_live_viewer.py, serves the same igv.js page for every run at/runs/<run>/viewer/, behind the site's password. It uses the same page, Content-Security-Policy and default files asscripts/owngenome-view.sh: the run folder's BAM or CRAM, VCF and SV VCF, each with its index, plusREF_FASTA. A track whose file or index is a symlink out ofGENOME_DIRis left out, with both files. A file name with a space,#or?is matched after unquoting the request path, as the standalone server does. The served report gets its viewer links pointed there.scripts/owngenome-live.shpasses the reference and igv.js, and downloads igv.js on the first start, checked by sha256. Without a reference index or igv.js the site starts without the viewer and says why.OWNGENOME_LIVE_VIEWER=offleaves the viewer out. Opening the viewer writes one access-log row. The byte ranges write none.IGV_JS_VERSIONandIGV_JS_SHA256move fromscripts/owngenome-view.shtoversions.env, so both launchers read the same pin.docs/versions.mdlists only images, so it is unchanged (gen-versions-doc.sh --checkpasses).bin/render_report.py.assets/viewer/serve.pygainssample_filesandsend_file(the byte-range sender, moved out of the handler), so the plugin reuses them instead of copying them.Output change, as the 1.0 contract asks: the HTML report gains a column on these tables. No cell is renamed or removed, and the text and JSON outputs are unchanged.
Privacy, said plainly in
docs/live-site.mdanddocs/viewer.md: from the live site the reads cross the network to the browser that logged in. Only the byte ranges igv.js asks for travel, and only after the login. Off loopback, use TLS. The standalone viewer keeps its "no login, do not expose the port" warning.Decisions taken in this PR
50-html-report-24.sh,annotsv-report-section-1-chain.sh,clinvar-combined-allele-1.sh,construct-scan-1-both.sh,report-followups-1-sections.sh). One of them is alackscheck that would have gone silently true. A column of its own keeps every existing cell's text and keeps main true. Each row still gets its own link.bin/pgp_live_viewer.pyandscripts/pgp-live.sh. Since the rename these arebin/owngenome_live_viewer.py(the live server loads onlyowngenome_live_*.pyplugins) andscripts/owngenome-live.sh. The test istests/test_live_viewer.py, as planned.assets/viewer/index.htmlsays "served from this computer only". The plugin swaps that line for "served by the run site, behind its login" when it serves the page, so the page stays true without editing the shared file.tests/test_isec_columns.shneeded no change: it pins the chromosome, gene and genotype cells, not the end of the row.How it was checked
CI on head f3189df. Every check except e2e is green. The Guard run's unit-tests job ran
tests/test_viewer.py,tests/test_live_viewer.pyandtests/test_collect_summary.pywith both headless Chrome checks: https://github.com/GeiserX/OwnGenome/actions/runs/41854962919Local runs on this branch (unit tests only, no containers):
python3 tests/test_viewer.py: ALL PASS. That covers the pin read fromversions.envwith its sha256, no pin left inowngenome-view.sh, a Viewer cell in every findings table, and headless Chrome on the standalone page.python3 tests/test_live_viewer.py: ALL PASS. That covers the login redirect and 401s, the viewer policy, BAM byte ranges (206, 416), refused files and runs (404), report links rewritten to/runs/S/viewer/, one access-log row, a symlinked BAM left out, a site without the flags, and igv.js ready with 3 tracks in headless Chrome.tests/test_collect_summary.py,tests/test_render_report.py,tests/test_owngenome_live.py,tests/test_live_post.shand the other report tests: ALL PASS.Red controls:
IGV_JS_SHA256inversions.envchanged to a wrong value,tests/test_viewer.pyexits 1:[FAIL] owngenome-view.sh: the server starts ... Download attempt 1/3 failed its check: sha256 checksum of igv.min.js is 87b991ae..., expected 000091ae....bin/render_report.pyof the previous head (no Viewer column on the Exomiser table), the table check names exactly that table:Exomiser ranking: the header does not end with the Viewer column,... a row whose last cell is not a viewer link to chr3:700,... no viewer note in the card under the table.tests/test_viewer.py, every CI run renders the report with the Viewer cell emptied, and the table check must then name all 10 tables (report: control: ...).tests/test_live_viewer.py, every CI run loads four planted copies of the plugin: one serves any file of the run folder, one drops the report filter, one compares the still-quoted file name, one serves an index symlinked out ofGENOME_DIR. The checks must fail on each (control: ...).Beads
pgp-5bja.8, pgp-awpe
Merge order: after paralog-rows (#276) and cross-engine-biallelic (#281), which touch the same report renderers.