Optional whitelist Severus calling (--severus_whitelist), patched Severus 1.7 - #215
AmberVerhasselt wants to merge 5 commits into
Conversation
SEVERUS_WHITELIST runs a patched Severus 1.7 image (docker.io/amberverhasselt/severus:1.7-whitelist-6813dee) whose --whitelist reports every SV inside the given regions, past the read-quality, minimum-support and VNTR filters. Setting --severus_whitelist replaces SEVERUS with it; its VCFs feed SV VEP, Wakhan and the report. Extra flags go through --severus_whitelist_args. Conda is not supported for this step. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| severus_input, | ||
| [[:], params.bed_file, params.pon_file] | ||
| ) | ||
| if (params.severus_whitelist) { |
There was a problem hiding this comment.
No test covers this path. None of the tests/*.nf.test set severus_whitelist, and modules/local/severus/whitelist/ has no tests/ directory. So nothing in PR CI runs SEVERUS_WHITELIST, its stub, or the rewiring of ch_severus_all_vcf / ch_severus_somatic_vcf into SV VEP, Wakhan and the report.
Please add an nf-test tagged small (CI only runs small tests on PRs). It could be a stub pipeline test with severus_whitelist set to a small BED, or a module test. Also run the real Slurm test array, because the patched image (docker.io/amberverhasselt/severus:1.7-whitelist-6813dee and the -sif ORAS image) is not exercised anywhere.
There was a problem hiding this comment.
Added modules/local/severus/whitelist/tests/main.nf.test in 393e7ce: two small-tagged stub tests (tumour-only and paired). They pull the patched image under both the docker and singularity CI profiles (docker.io/amberverhasselt/severus / oras://docker.io/amberverhasselt/severus-sif, both public), assert the 1.7-whitelist-6813dee version from the versions topic, and snapshot every output.
The pipeline-level rewiring is not in CI: a stub pipeline test dies at CRAMINO_PRE first (its stub emits no *.arrow), and a real pipeline test with --severus_whitelist would roughly double the default test's CI time. It was checked end to end with real -profile test,singularity runs on Mindwell, with the option on and off: SEVERUS_WHITELIST ran on all 3 samples (--control-bam for paired, --PON for tumour-only), and SV VEP and the report consumed its VCFs. It was also checked on a CHM13 sample (FL3) with a CHM13 whitelist BED; results are in the PR description.
|
|
||
| ### `Added` | ||
|
|
||
| - Added optional whitelist SV calling: `--severus_whitelist <bed>` replaces `SEVERUS` with `SEVERUS_WHITELIST`, which runs a patched Severus 1.7 image (`docker.io/amberverhasselt/severus:1.7-whitelist-6813dee`) that reports every SV inside the whitelisted regions, past the read-quality, minimum-support and VNTR filters, with corroboration required for single-read junctions. Its VCFs feed SV VEP, Wakhan and the report in place of the standard Severus output. Extra flags go through `--severus_whitelist_args`. Conda is not supported for this step (@AmberVerhasselt). |
There was a problem hiding this comment.
This entry has no PR number. The other dev entries start with [#NNN](...) - . Suggested fix:
| - Added optional whitelist SV calling: `--severus_whitelist <bed>` replaces `SEVERUS` with `SEVERUS_WHITELIST`, which runs a patched Severus 1.7 image (`docker.io/amberverhasselt/severus:1.7-whitelist-6813dee`) that reports every SV inside the whitelisted regions, past the read-quality, minimum-support and VNTR filters, with corroboration required for single-read junctions. Its VCFs feed SV VEP, Wakhan and the report in place of the standard Severus output. Extra flags go through `--severus_whitelist_args`. Conda is not supported for this step (@AmberVerhasselt). | |
| - [#215](https://github.com/IntGenomicsLab/lrsomatic/pull/215) - Added optional whitelist SV calling: `--severus_whitelist <bed>` replaces `SEVERUS` with `SEVERUS_WHITELIST`, which runs a patched Severus 1.7 image (`docker.io/amberverhasselt/severus:1.7-whitelist-6813dee`) that reports every SV inside the whitelisted regions, past the read-quality, minimum-support and VNTR filters, with corroboration required for single-read junctions. Its VCFs feed SV VEP, Wakhan and the report in place of the standard Severus output. Extra flags go through `--severus_whitelist_args`. Conda is not supported for this step (@AmberVerhasselt). |
There was a problem hiding this comment.
Fixed in 393e7ce: the entry now starts with [#215](...) - .
| "help_text": "Inside the whitelisted regions the patched build skips the read-quality, minimum-support and VNTR filters, and keeps single-read junctions only when a reciprocal or independently supported junction corroborates them. Use it for loci such as the IG and TCR genes, where translocations are often carried by few, divergent reads.", | ||
| "fa_icon": "fas fa-file-alt" | ||
| }, | ||
| "severus_whitelist_args": { |
There was a problem hiding this comment.
severus_whitelist_args has no help_text, but the repo convention is that new parameters carry one. It should also say that these args are appended after --min-support ${severus_minsupport} --output-read-ids (see conf/modules.config), and that the parameter is ignored unless --severus_whitelist is set.
There was a problem hiding this comment.
Added in 393e7ce. The help_text says the args are appended after --min-support <severus_minsupport> --output-read-ids, that they are ignored unless --severus_whitelist is set, and what --whitelist-single-read and --write-alignments do.
| description: | | ||
| Groovy Map containing sample information | ||
| pattern: "${prefix}/severus.log" | ||
| - ${prefix}/all_SVs/severus_all.vcf: |
There was a problem hiding this comment.
The outputs listed in meta.yml don't match main.nf:
main.nfemitsall_SVs/severus_all.vcf.gzandsomatic_SVs/severus_somatic.vcf.gz, butmeta.ymllists.vcf.all_vcfis also typedmapinstead offile.main.nfemitsbreakpoint_clusters_list.tsv/breakpoint_clusters.tsv, butmeta.ymllistsbreakpoints_clusters_list.tsv/breakpoints_clusters.tsv(here and at lines 184–248).
This metadata was copied from the nf-core module, but because this is a new local module it should describe what the module actually emits.
There was a problem hiding this comment.
Fixed in 393e7ce: all_vcf / somatic_vcf now list severus_all.vcf.gz / severus_somatic.vcf.gz (all_vcf typed file), and the cluster outputs are breakpoint_clusters_list.tsv / breakpoint_clusters.tsv. All 15 output paths in meta.yml now match the path(...) emits in main.nf. The stub also writes its VCFs with gzip -n, so the new snapshot is reproducible.
|
Automated review: 4 findings (inline):
For a human reviewer to check by running things: run the Slurm test array with |
…uts, CHANGELOG, help_text - Add a small-tagged stub nf-test for SEVERUS_WHITELIST (tumour-only and paired), so PR CI pulls the patched image and checks the output declarations and the 1.7-whitelist-6813dee version - meta.yml now lists the outputs main.nf emits (.vcf.gz, breakpoint_clusters*) - Stub writes its gzipped VCFs with gzip -n so the snapshot is reproducible - CHANGELOG entry carries the PR number; severus_whitelist_args gets a help_text Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
SEVERUS_WHITELIST duplicated the patched nf-core SEVERUS module except for its container, a whitelist input, a conda guard and a version suffix. SEVERUS now always runs the patched Severus 1.7 image (oras://docker.io/amberverhasselt/severus-sif:1.7-whitelist-6813dee) and takes an optional third input [meta, whitelist] that adds --whitelist when set. - modules/nf-core/severus: whitelist input, patched container; severus.diff regenerated - workflow: single SEVERUS call; whitelist passed as [] when --severus_whitelist is unset - conf/modules.config: SEVERUS_WHITELIST block dropped; severus_whitelist_args appended to SEVERUS ext.args only with --severus_whitelist - --severus_whitelist under conda/mamba now errors at startup (validateSeverusWhitelist) - modules/local/severus/whitelist removed; docs, schema text and CHANGELOG updated Checked on FL3 (T2T, tumour-only): without a whitelist the patched image gives identical VCF records to stock 1.7 (header gains one WL_RESCUE INFO line); with TIER1_NHL_genes_T2T_v2.bed it calls the reciprocal IGH-BCL2 t(14;18) as PASS. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Update:
|
| Check | Result |
|---|---|
Patched image without whitelist vs stock 1.7 (main) |
VCF records identical (all 14,661 / somatic 251); breakpoints_double.csv, read_qual.txt, breakpoint_clusters_list.tsv, log identical |
| VCF header | one extra line, ##INFO=<ID=WL_RESCUE,...>, which the fork declares unconditionally |
read_ids.csv, breakpoint_clusters.tsv |
same contents, row/read order differs; a second stock run differs from the first the same way, so this is run-to-run noise, not the patch |
With TIER1_NHL_genes_T2T_v2.bed |
reciprocal IGH–BCL2 t(14;18) called PASS in both all and somatic VCFs: chr14:100134813 ↔ chr18:63329464 and chr14:100165576 ↔ chr18:63329474 (WL_RESCUE=WHITELIST); absent without the whitelist |
Next: a full pipeline run on FL3, with and without --severus_whitelist, to confirm the downstream steps (SV VEP, Wakhan, report) work end to end.
🤖 Generated with Claude Code
|
@claude review |
| 'https://community-cr-prod.seqera.io/docker/registry/v2/blobs/sha256/8f/8fd0858ee067f8b95246e57683a7431bc126929b4203fbda8e315cd94d2570ad/data': | ||
| 'community.wave.seqera.io/library/severus:1.7--d16d59609ef4ee7d' }" | ||
| 'oras://docker.io/amberverhasselt/severus-sif:1.7-whitelist-6813dee': | ||
| 'docker.io/amberverhasselt/severus:1.7-whitelist-6813dee' }" |
There was a problem hiding this comment.
The default path now uses the patched image, not just --severus_whitelist. The PR description says SEVERUS_WHITELIST replaces SEVERUS only when --severus_whitelist is set. In this diff, though, the shared SEVERUS container is swapped for every run, and no SEVERUS_WHITELIST process exists in the PR head. As a result:
- Every Docker/Singularity run, with or without a whitelist, now pulls a personal Docker Hub image (
amberverhasselt/severus) instead of the Seqera/biocontainer build. - Conda (
environment.yml, stock bioconda 1.7) and containers no longer run the same Severus build, even for runs without a whitelist. - According to the PR description,
severus --versionin this image reports1.7-whitelist-6813dee. Thesmallpipeline snapshot (tests/default.nf.test.snap:90,"severus": 1.7) has not been updated, so the default CI test should fail on the versions block. The same applies to thechm13andclair_onlysnapshots.
Either put the patched image behind a separate process that is only used with --severus_whitelist, as the description says, or update the snapshots and make the description match the code.
There was a problem hiding this comment.
This is intentional: the separate module was dropped in 38e5191, so the PR description was stale. It is now rewritten to match the code.
- Default output unchanged. On FL3 (CHM13, PacBio), the patched image without a whitelist gives the same VCF records as stock biocontainer 1.7 (all 14,661, somatic 251). The only difference is one extra header line,
##INFO=<ID=WL_RESCUE,...>.read_ids.csvandbreakpoint_clusters.tsvhave the same contents in a different row order. Two stock runs differ from each other in the same way, so that is run-to-run noise, not the patch. Details are in the update comment above. - Snapshots. The patched image reports
severus --version=1.7, not1.7-whitelist-6813dee(the old description was wrong here). So"severus": 1.7in thedefault,chm13andclair_onlysnapshots stays correct. The newtests/severus_whitelist.nf.testsnapshots the version from the real image. - Conda. Conda installs stock bioconda 1.7. Without a whitelist that matches the container output, as shown above. With
--severus_whitelistit would silently run the unfixed whitelist logic, so that combination errors at startup (validateSeverusWhitelist). - Image source. It follows REVIEW.md's custom-image convention (
oras://docker.io/...-sif/docker.io/..., pinned tag), like the wakhan, clairsto and sigprofiler images.
| input: | ||
| tuple val(meta), path(target_input), path(target_index), path(control_input), path(control_index), path(vcf), path(tbi) | ||
| tuple val(meta2), path(bed), path(pon_path) | ||
| tuple val(meta3), path(whitelist) |
There was a problem hiding this comment.
New third input, but the module's test and meta.yml are unchanged. modules/nf-core/severus/tests/main.nf.test still passes only input[0] and input[1], so every test case there now fails on an input-count mismatch. meta.yml doesn't document meta3/whitelist either; per REVIEW.md, meta.yml must be updated with new inputs.
Those module tests also lack the small tag, so CI never runs them. The description mentions "new small-tagged stub tests for SEVERUS_WHITELIST (tumour-only and paired)", but no test files are in this PR. Please add them, and add input[2] = [[:], []] to the existing cases.
There was a problem hiding this comment.
Fixed in 9016b74:
meta.ymlnow documents thetbi,pon_pathand optionalmeta3/whitelistinputs. The output paths now matchmain.nf(severus_*.vcf.gz,breakpoint_clusters*.tsv).severus.diffwas regenerated to covermeta.ymland the tests.modules/nf-core/severus/tests/main.nf.test: every case now passes thetbi, PON andinput[2] = [[:],[]]slots. Those cases had already drifted from the module with the earlier tbi/PON patch.nf-test.configignoresmodules/nf-core/**/tests/*, so nf-test refuses to run them here, even when named explicitly. Their snapshots are therefore not refreshed.- The
smallstub tests the old description mentioned belonged tomodules/local/severus/whitelist/, which was removed with that module. They are replaced bytests/severus_whitelist.nf.test; see the next thread.
| severus_input, | ||
| [[:], params.bed_file, params.pon_file] | ||
| [[:], params.bed_file, params.pon_file], | ||
| [[:], params.severus_whitelist ? file(params.severus_whitelist, checkIfExists: true) : []] |
There was a problem hiding this comment.
No CI test covers the whitelist path. No small test sets --severus_whitelist, so the following are never exercised in CI:
- this branch;
- the
--whitelistflag in the module; severus_whitelist_argsinconf/modules.config;- the Conda guard.
Please add a small-tagged test (a stub test is fine) that sets --severus_whitelist, and checks that --whitelist reaches the command and that the outputs are published to variants/severus/.
There was a problem hiding this comment.
Added tests/severus_whitelist.nf.test (small, 9016b74). It lives outside modules/nf-core/ so PR CI picks it up. It runs real Severus, not a stub, from the patched image on the nf-core nanopore test BAMs (paired), with and without genome.bed as the whitelist:
- asserts that
--whitelist genome.bedis in the SeverusCmd:line only when a whitelist is given; - snapshots the all and somatic VCF records, the
.vcf.gznames and the version (1.7); - with default filters it calls 0 records without the whitelist and 21 with it, so the snapshot also catches the whitelist rescue itself.
Both pass locally under singularity, and are stable on rerun.
Not covered in CI:
severus_whitelist_argsand the Conda guard. They sit at pipeline level, and a stub pipeline test dies atCRAMINO_PREfirst.- The rewiring into SV VEP, Wakhan and the report. No
smalltest runs it either, because a real pipeline test with--severus_whitelistwould roughly double thedefaulttest's CI time.
For both, full FL3 runs are going now on CHM13 and on GRCh38, each with and without --severus_whitelist.
|
Automated review: 3 findings (inline).
A human reviewer needs to run: the |
…small whitelist test - modules/nf-core/severus/meta.yml: document the tbi, pon_path and optional whitelist inputs; output paths now match main.nf (.vcf.gz, breakpoint_clusters*) - modules/nf-core/severus/tests: add the tbi, PON and whitelist slots so every case matches the module's inputs (these tests stay ignored by nf-test.config) - tests/severus_whitelist.nf.test: small-tagged, real Severus on the nf-core nanopore BAMs, paired, with and without --whitelist; asserts --whitelist reaches the command only when given, snapshots the VCF records and version (1.7). Default filters give 0 records; the whitelist rescues 21. - severus.diff regenerated to cover meta.yml and the module tests Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
AmberVerhasselt
changed the title
nf-core lint (nf_test_content) treats every tests/*.nf.test as a pipeline test and requires outdir and a versions.yml snapshot, which a process test does not have. It only globs tests/*.nf.test, while nf-test discovers tests recursively, so tests/modules/ keeps the test in PR CI (tag small) without a lint exemption. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Closes #173.
Adds
--severus_whitelist <bed>, passed to Severus as--whitelist. Inside the whitelisted regions, the read-quality, minimum-support and VNTR filters no longer drop events. A junction supported by a single read is kept only when a reciprocal or independently supported junction corroborates it. This is aimed at the IG/TCR loci, where translocations are often carried by few, divergent reads.The whitelist fixes are not in upstream Severus yet.
SEVERUStherefore runs Severus 1.7 from AmberVerhasselt/Severus@whitelist-reciprocal-corroboration (6813dee), with or without a whitelist:oras://docker.io/amberverhasselt/severus-sif:1.7-whitelist-6813deeunder Singularity/Apptainerdocker.io/amberverhasselt/severus:1.7-whitelist-6813deeunder DockerWithout a whitelist, its VCF records are identical to stock Severus 1.7 (see Testing). Revert to the biocontainer once KolmogorovLab/Severus carries the fixes.
Changes
SEVERUS(nf-core module, local patch) takes an optional third input,[ meta, whitelist ]. The workflow passes[]when--severus_whitelistis unset, and--whitelistis only added when a file is given. There is no separate process: the output publishes tovariants/severus/and feeds SV VEP, Wakhan and the report as before.--whitelist-single-read any,--whitelist-allow-intra-region,--write-alignments, ... go through--severus_whitelist_args. They are appended toSEVERUSext.argsonly when--severus_whitelistis set.--severus_whitelistunder-profile conda/mambaerrors at startup, because Conda installs stock Severus without the fixes. Without a whitelist, Conda still works.meta.ymldocuments the new inputs, the module tests pass the new input slots, andseverus.diffis regenerated.severus --version=1.7, so the versions snapshots are unchanged.Testing
--min-support 3 --output-read-ids, default PoN and VNTR BED). The patched image gives the same VCF records as the stock biocontainer (all 14,661 / somatic 251).breakpoints_double.csv,read_qual.txtand the log are identical. The VCF header adds one##INFO=<ID=WL_RESCUE>line.read_ids.csvandbreakpoint_clusters.tsvhave the same contents in a different order, which also happens between two stock runs.TIER1_NHL_genes_T2T_v2.bed). The reciprocal t(14;18) IGH–BCL2 is called PASS inall_SVsandsomatic_SVs, at chr14:100,134,813 / 100,165,576 ↔ chr18:63,329,464 / 63,329,474 (INSIDE_WHITELIST=TRUE;WL_RESCUE=WHITELIST). Without the whitelist, Severus does not call it.smalltesttests/modules/severus_whitelist.nf.testruns real Severus from the patched image (paired nf-core test BAMs), with and without a whitelist. It checks that--whitelistreaches the command only when given, and snapshots the VCF records (0 without, 21 with the whitelist) and the version.--severus_whitelist, are in progress.nextflow lint: no new errors. Whole-pipeline-stubis broken independently of this PR (theCRAMINOstub emits no*.arrow).PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).🤖 Generated with Claude Code