Conversation
Merged
nrminor
marked this pull request as ready for review
August 5, 2026 15:07
nrminor
force-pushed
the
v3.4.0-rc
branch
4 times, most recently
from
August 14, 2026 21:11
aad2917 to
40117ef
Compare
Generate deterministic per-sample plain-text coverage histograms from filtered contig-mapback BAMs using samtools' native Unicode block rendering. Publish the terminal reporting artifacts under target query preparation without coupling them to alignment or mapped-count publication. Exercise the Nextflow process with a synthetic indexed BAM and require stable contig identifiers, UTF-8 block output, summary statistics, and the expected positional coverage pattern. Raise the declared samtools floor to cover histogram rendering and the pipeline's existing markdup options.
Generate self-contained Alignoth HTML reports for each covered target-filtered contig using the corresponding filtered mapback BAM. Group the high-cardinality outputs by sample under target query preparation while keeping the reporting branch terminal and best-effort. Add Alignoth to the locked cross-platform environment, exercise actual multi-contig HTML generation with mapped and unmapped synthetic references, and verify the release container exposes the renderer.
Add a curated TSV lookup asset under assets/ that maps ePathogen-style source labels onto human risk group classifications using exact-safe lookup names plus NCBI taxid remaps where available. The table was built in stages. First, a deterministic pass generated candidate lookup labels from the source names and alternate names, checked them against names.dmp and the NCBI Virus/WVDB sourmash lineage universe, and surfaced conflicts, misses, and grouped-name rows. From there, the long tail was curated manually by interpreting source intent rather than relying on raw token matching alone. That manual pass did three main kinds of work: - promote exact-safe aliases directly into the lookup corpus, - remap historical, common, disease-adjacent, and grouped-member labels onto the most plausible NCBI Virus taxids when the intended taxon concept was clear, - conservatively drop labels that were too broad, too disease-oriented, too stock-like, or otherwise unsafe for exact joins. Several grouped rows required intent recovery rather than one-to-one alias compilation. In particular, bundled Alphapapillomavirus labels were rewritten into explicit member-level papillomavirus rows before remapping, and influenza subtype rows were treated as subtype-level remaps where the subtype intent was clear. The remaining unresolved review residue was then conservatively excluded from the first finished asset. The final TSV contains only rows with non-empty human classification values and backfilled taxids for every retained row so it can support taxid joins against BLAST outputs as well as name joins against Sourmash/NCBI Virus results.
Resolution: Add a shared Polars annotator that maps canonical taxids to WHO risk groups. Resolve the nearest classified taxon along BLAST taxonomy and Sourmash BioBoxes paths so classifications flow from ancestors to descendants but never upward. Treat malformed or ambiguous data conservatively, and warn while selecting the highest RG1-RG4 classification for conflicts on one taxid. Integration: Annotate final BLAST results and experimental Sourmash summaries, publish the derived Sourmash report, and propagate who_risk_group into query and taxon Big Tables beside their taxid columns. Keep the existing LabKey schema isolated through an explicit projection. Verification: Update the mini viral taxonomy, Sourmash lineages, fixture generator, schema checks, and end-to-end expectations for modern Orf and monkeypox species taxids. Cover directional inheritance, overrides, malformed inputs, row preservation, exact risk groups, and output column placement.
Replace the LabKey-coupled plain-text notifier with a cacheable Block Kit completion lineage driven by explicit reporting and experimental completion channels. Summarize deduplicated final RG3/RG4 taxonomic assignments with complete paginated evidence and accessible fallbacks, while preserving best-effort stateless delivery. Add final BLAST risk-group source provenance for comparison with rapid screening, make LabKey links optional and experiment-filtered, and wait for successful LabKey registration only when integration is enabled. Cover message rendering, pagination, URL construction, provenance, and real Nextflow process contracts.
Preserve explicit read ordinals retained in SRA and ENA FASTQ representative labels before reducing them to compact source IDs. Keep the duplicate-source guard so genuinely ambiguous representative identities still fail instead of entering query lookups. Correct the integration contract so only standard skip-assembly runs expect no final BLAST output. Experimental skip-assembly runs intentionally query read-derived sequences and must satisfy the existing final-result, taxonomy, risk-group, and task assertions.
Toolchain: Pin FastQC 0.12.1 and MultiQC 1.35 as exact direct Pixi dependencies so reporting behavior changes only through deliberate upgrades. Isolate the resulting cross-platform solver expansion in one reviewable commit. Validation: Use disposable biologically neutral probes to verify native FastQC discovery, MultiQC Custom Content tables, a 90-sample heatmap and two PCoA scatter shapes, and the Nextflow collection and path-staging assumptions needed by later commits. Keep these as empirical design evidence rather than maintaining tests of documented third-party behavior. Boundaries: Add no production workflow modules, report compiler code, publication changes, fixtures, or dormant runtime scaffolding. NVD-owned aliases, configuration, channels, failure semantics, and normalized outputs begin with the durable C2 reporting spine and receive tests there. Verification: Confirm the lock is current, run the 604-test fast suite, and pass formatting, lint, schema, test, and all profile-rendering checks through the existing `just check` gate. Independent C1 reviews are waived explicitly because this commit contains only direct dependency declarations and their generated lock.
Input units: Expand each gathered sample bundle into deterministic physical-FASTQ units with collision-safe aliases while preserving sample identity, read end, input ordinal, and only the lane or chunk metadata that is actually available. Run FastQC over those raw units without inventing an expected-task ledger. Reporting spine: Add the durable terminal reporting architecture: NVD-owned status and manifest contracts, a pure compiler for the minimal roster/source-identity view, stable MultiQC configuration, native FastQC discovery, a best-effort terminal process, and publication of `multiqc_report.html` with complete `multiqc_data`. Feed every artifact through explicit channels and never read published results. Failure semantics: Retry and then ignore ancillary FastQC/compiler/renderer failures at production runtime without gating BLAST or LabKey. Keep development assertions strict about NVD-generated aliases, staged inputs, normalized data, report outputs, absence-without-inference, and poison-file exclusion. Boundaries: Install the final extensible reporting spine without placeholders or dormant future channels. Defer enrichment, depletion, FASTX, assembly, classification, CRUMBS, taxon evidence, rapid evaluation, and sample similarity sections to later vertical commits.
Split FASTX length distributions into single-read, overlap-merged-pair, and filtered-contig sections so unlike scales no longer obscure one another. Keep length and quality distributions as linear line graphs, preserve all points after bounded rebinning, and retain distinct semantic series labels through MultiQC name cleaning. Publish the standalone HTML report at the NVD root while placing its supporting multiqc_data directory with experiment summaries. Update user documentation and make end-to-end section expectations follow the enabled merge and assembly capabilities.
Generate ancillary per-sample query-batch summaries in standard and experimental runs, then carry them through an explicit query-preparation report package into the typed MultiQC compiler. Keep standalone summary publication experimental-only while deriving user-facing availability from the report plan. Render a long-form Prepared BLAST Query Batches table with plain-language class and sequence-type labels, numeric counts, configured-disabled states, hidden platform and validation details, and actionable malformed-summary guidance.
Resolve query identifiers from the named qseqid column in headered annotated BLAST TSVs rather than assuming a positional field. Add focused producer tests for empty, header-only, reordered, retained, removed, and malformed schemas without changing filtering or Nextflow behavior.
Package each observed MEGABLAST query partition by sample and query class, parse it through a dedicated typed BLAST report boundary, and render a compact MultiQC table of searched, MEGABLAST-matched, and BLASTN-forwarded query counts. Preserve configured BLAST disablement as a run-plan notice while leaving missing enabled outputs unexplained.
Add an experimental, bounded RG2–RG4 Taxon Big Table projection near the top of the MultiQC report. Preserve the producer's existing columns and salience semantics, prioritize higher WHO risk groups, and retain links to complete result tables rather than turning the static report into an exhaustive explorer. Wire the existing experimental Taxon Big Table into the typed report-package/compiler path, cover parsing and rendering at public boundaries, and keep standard-mode scientific outputs unchanged.
Add strongest- and faintest-signal Taxon Big Table projections alongside the higher-risk view, preserve the producer's established column order, and use readable finding labels within MultiQC's unique-row constraint. Accept blank optional risk annotations without invalidating a sample's table, derive the roster's displayed read structure from existing FastQC receipts, and place the findings views first while suppressing MultiQC's fixed General Statistics block.
Prepend the Pixi CLI during image construction and the locked default environment at runtime so env-based launchers resolve their matching interpreters. This fixes FastQC selecting Ubuntu Perl without mixing the system and Pixi Perl installations. Strengthen container CI around the runtime contract: assert Pixi-first PATH and Perl resolution, load FindBin, run Nextflow and FastQC, process a minimal FASTQ into validated ZIP and HTML outputs, and download an NVD checksum manifest over HTTPS with Pixi curl. Exercise every retained direct bioinformatics executable and the shadowed Pixi system tools while preserving the existing Python, NVD CLI, and environment smoke tests.
Add an explicit pipeline option for highly sharded runs where per-file raw FastQC would create excessive tasks and unusable reports. Gate only the ancillary FastQC branch; the remaining preprocessing and MultiQC report continue with FastQC outputs absent.
Extend MEGABLAST and BLASTN output with the subject accession, query and reference endpoints, reference length, and strand. Preserve those fields through reference retention, taxonomy annotation, LCA assignment, risk annotation, and final BLAST enrichment, including no-hit headers. Expose the selected best hit in the Query Big Table with its reference accession and title, alignment length, normalized 1-based inclusive query and reference intervals, reference length, and strand. Resolve fully tied HSPs and references deterministically so placement fields do not depend on input order. Keep the locked LabKey projection unchanged: the expanded final BLAST artifact carries the new fields, while LabKey preparation explicitly omits them. Cover the raw BLAST, annotation, LCA, finalization, Query Big Table, downstream taxon-table fixture, LabKey boundary, and experimental end-to-end seams.
Make query-level support notes identify the selected best-hit accession while leaving placement coordinates in structured columns. Summarize how contributing queries are positioned within their best-hit references in taxon-level support notes. Merge overlapping or exactly adjacent intervals independently per reference; report combined representative-interval coverage when all queries share one reference, and bounded region/reference counts when they do not. Avoid outer-span percentages that could make distant fragments resemble whole-genome coverage. Keep geometry independent of dominant/LCA assignment, support-tier rules, query classes, and taxonomic rank. Do not add Taxon Big Table columns or let placement geometry change support tiers.
Publish the query sequences represented in the Query Big Table, their distinct selected reference sequences, and BED6 best-hit placement intervals as a coherent evidence bundle. Keep FASTA sequences unwrapped and identifiers consistent across FASTA and BED artifacts. Publish the bundle under the dedicated 11_best_hit_sequences results directory without changing existing result paths.
Collaborator
|
Added to this release will be making query_class searching of everything as default behavior. Fixing LIMS side batching of the data so we don't overload an insert. |
params.preprocess was declared in nextflow.config, the schema, the model, and both CLIs, but no Nextflow, Groovy, or config code ever read it. The only matches in the pipeline are params.preprocessed_reads, an unrelated results path. The parameter therefore did nothing. It was also actively misleading: --trim-adapters and --filter-reads advertised their default as "follows --preprocess", describing an umbrella that was never implemented. Both help strings now state the real default. Removing it would surface as pydantic's bare "Extra inputs are not permitted" for anyone with an older preset, so add a REMOVED_PARAMS validator that names the parameter and the release instead. It runs before extra="forbid" and leaves genuine typos to the ordinary error path.
Overlap-merged pairs and single reads become BLAST query classes alongside assembly contigs on every run, instead of only under --experimental. A sample whose assembly yields no contigs now still gets its reads classified, and reads that fail to map back to contigs are no longer discarded before BLAST. The gate moves to a new skip_unassembled_read_queries parameter, declared to match the existing skip_assembly / skip_blast / skip_fastqc family, so contig-only runs remain available. requires_blast_db follows the same change: BLAST needs its database whenever either query source can run. MultiQC has to learn the difference. It derived read-query availability from experimental_enabled, and a batch carrying queries while its capability reads as disabled is treated as a run-plan conflict, which collapses the sample to a single "invalid" row and discards its per-class breakdown. Left alone, every sample on every default run would report that way. read_querying_enabled is now a separate required field; experimental_enabled keeps gating only the Experimental Capabilities invitation. The Nextflow stub tests bind to both subworkflow arity and the config surface, so they need the new param declared where they include conf/results.config, and the MULTIQC_BUNDLING helper needs the new channel. --skip_assembly alone now yields a working reads-only run rather than no BLAST output at all. That is allowed but deliberately undocumented; only the integration assertion changes, to stop expecting emptiness.
merge_pairs flips to true in nextflow.config, the model, and the schema. The CLI already exposed --merge-pairs/--no-merge-pairs, so the opt-out needs no new plumbing. This pairs with read querying becoming a default: overlap_merged_pair only carries its own signal when merging actually runs. No workflow logic changes; preprocess_reads.nf already branches on the parameter and simply takes the other path. Adds behavioral tests for adapter read-through, which default-on merging makes load-bearing for BLAST input. An earlier plan was to pass adapter flags to bbmerge, on the premise that merging ran before adapter trimming and would carry adapter into merged reads. That premise was wrong: bbmerge trims sequence to the right of the detected overlap by default, and read-through requires an insert shorter than the read, which guarantees the mates overlap. So the pairs at risk are exactly the ones bbmerge merges and trims, and read-through pairs cannot reach the single_read class either. The tests pin both properties against the real process so a future flag or version change cannot silently start feeding adapter sequence to BLAST.
insert_records sent every row in a single insert_rows call. Both call sites justified that explicitly: a (sample_id, query_class) unit was "small enough that chunked inserts only add partial-failure risk without a throughput benefit". Querying unassembled reads by default invalidated that. A unit now also covers read-scale hit tables, and one of those as a single request can hang the LabKey server. Records are now sent in batches of labkey_insert_batch_size rows (default 1000), configurable per run. Without an explicit batch size insert_records behaves exactly as before, so nothing else that calls it changes. Batching trades atomicity for a bounded request size, and that interacts badly with how uploads resume. The destination list is its own completion ledger keyed on row presence, and LABKEY_UPLOAD_BLAST retries twice, so rows left by a failed batch make the retry treat the unit as already uploaded and silently drop the rest. Rolling back was considered and declined, so instead a genuinely partial failure raises PartialInsertError and both scripts report how many rows were committed, that they were not rolled back, how many will be skipped, and which table to clear before re-running. A first-batch failure commits nothing, so the original LabKey error propagates unwrapped and the existing "no rows committed" message stays accurate. Wrapping that case too would have claimed rows were committed when none were, and would have hidden the underlying error from the operator.
SAMPLE_SIMILARITY_QC took query sketches from RAPID_SCREENING, which owned the sketch step only because it needed sketches for gather. Rapid screening is being removed and similarity QC is the remaining consumer, so it now takes post-QC read batches directly and sketches them itself. The filter and CONCAT_READS_AS_FASTA step move verbatim from rapid screening. Sketching needs no reference database, only sourmash_ksize and sourmash_scaled, which is why this capability survives the removal intact. Adds the first tests to cover this subworkflow at all: it previously ran only under --experimental inside a slow network test. sourmash, seqkit, and the downstream similarity scripts are faked so these stay wiring tests, pinning that the subworkflow accepts the profiled_batches_by_sample shape and still skips samples the profiler counted as empty.
Rapid k-mer screening has not performed well enough to keep, and a separate pipeline now covers the same goal, so maintaining a second implementation costs without returning anything. Removes the rapid-screening subworkflow, its evaluation harness, the sourmash gather/tax/reference processes, ANNOTATE_SOURMASH_RISK_GROUPS, NOTIFY_RAPID_SCREENING_SLACK, the three sourmash renderers, and the WVDB reference-building tooling that existed only to produce gather references. Sample-similarity QC survives; it shares only the sketch step, which needs no reference database. ANNOTATE_BLAST_RISK_GROUPS, the risk-group lookup, run completion Slack, and CRUMBS profiling are all untouched. sourmash_ksize and sourmash_scaled stay for sketching; the six reference and threshold params are removed and named in REMOVED_PARAMS so a stale preset explains itself. BREAKING CHANGE: results directories 11-14 are renumbered to 10-13 now that rapid screening no longer occupies 10. Unlike the parameter removals, nothing can warn about this: a downstream script globbing 13_experiment_summary will simply find nothing. Query sketches also move to 08_metagenomic_profiles/sourmash/query_sketches, which affects --experimental users only. Dropping sourmash-plugin-betterplot, which existed solely for the deleted Sankey renderer, also orphaned the pyqt pin that was carried to keep BetterPlot from building PyQt5 from source on headless platforms. Both are gone and the locks are regenerated. scripts/ held only the WVDB tool, so it disappears with it; PYTHON_CI_PATHS now points at lib/, matching what CI already lints.
Removing rapid screening left stale text in places nothing executes, so the test suite stayed green while the repo described capabilities that no longer exist. The user-visible one: the MultiQC Experimental Capabilities section still offered "rapid-screening evaluation" on every non-experimental run. It now lists what --experimental actually gates. The rest are comments and dead constants. results.config kept an orphaned "10 - Rapid-screening evaluation" banner, stranded directly above the unrelated "03 - Assembled contigs" banner, plus a "14 - LabKey uploads" heading left stale by the renumbering to 13 and an "08 - Sourmash metagenomic profiles" heading for a directory that now holds only query sketches. The justfile kept the WVDB download URLs its deleted recipes used, the fixture generator kept a SOURMASH_LINEAGES table whose writer was removed, .gitignore kept an allowlist entry for a deleted doc, and modules/sourmash.nf still described itself as the engine for a screening stage that no longer exists. Found by sweeping for the removed feature rather than for deleted symbols; the earlier symbol-level sweep could not see any of these.
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
2 participants
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.
We gotta stop making such big releases!
v3.4.0 is another big release. It introduces a significant amount of new machinery to generate a fully custom MultiQC report and final "big table" artifacts that are the intended review entrypoint for
nvdresults.MultiQC
After introducing query classes,
nvdhad significantly increased logic around which query sequences may end up moving forward into which steps. Users will reasonably ask questions like:Etc. The multiQC report is supposed to centralize answers to these questions so users don't need to dig down into our deeply nested results hierarchy to answer them. It's also a standalone artifact that can be sent between collaborators, whereas the results directory will be too unwieldly to be conveniently shared.
The "Big Tables"
All pipelines produce big tables at some point.
nvdis no exception. Building off our work in #32 introducing a de-biased abundance metric and in #31 introducing query classes, v3.3 introduces two new reporting tables, one focused on individual queries (the "query big table") and one aggregated table focused on taxa (the "taxon big table").We put significant thought into these tables, in particular how the table guide's the user's attention toward things that might be interesting, and how the table qualifies each entry to answer questions that the user my have progressively. These qualifications include ranking each hit into strong/moderate/weak bins based on BLAST statistics, explanatory entries based on each LCA resolution, and much, much more. We expect to tweak this table as it gets more use and more review, but suffice it to say that this is a step-change from the previous regime where users were directly reviewing overwhelming volumes of BLAST results.
Our attempt at sorting pathogens by risk
Version 3.4.0 also introduces a new column that notes whether each virus is considered part of a WHO Risk Group, which is one way of enabling users to sort results so that particularly dangerous detections bubble up. It's also a building block for rapid notification systems where users could be notified only on high-risk detections.
Building toward even better detection reporting
Additionally, v3.4.0 now tracks where BLAST placed queries on their best reference placement. These positions are reported in BED format alongside a FASTA of the query sequences themselves as well as a FASTA of the selected reference sequences. Note that in some cases, the LCA-selected taxon for a given query sequence may not be the same as the taxon for a selected reference; future versions may explore some creative solutions to this mismatch.
Query classes graduate from experimental
The query-class machinery introduced in #31 is no longer gated behind --experimental. Every default run now BLASTs unassembled evidence like single reads and merged read pairs, alongside assembled contigs. If you want the old contigs-only behavior, it's one flag away: --skip_unassembled_read_queries true. This is the machinery that motivated much of the MultiQC and big-table work above: once three query classes flow through the pipeline by default, users need the reporting to see what each class contributed.
Read-pair merging on by default
Overlapping read pairs are now merged by default (--merge_pairs true). Pass --merge_pairs false to opt out.
Farewell sourmash rapid screening
Breaking change: the sourmash-based rapid screening branch is gone, along with its taxon-abundance sunbursts, Sankey plots, and rapid-screening Slack alerts. We have moved to run that as a separate process alongside our NVD pipeline rather than include it within.
Sturdier LabKey uploads
Large hit tables were hanging the LabKey server when inserted in one shot. Inserts are now batched (tunable via --labkey_insert_batch_size), so big experiments upload reliably instead of timing out.
Other goodies
We also now provide interactive Alignoth alignment pileup visualizations for contigs to see how read mapback against assembled contigs went. We also fixed some issues in our container build, continued cleaning up our results directory structure, and more!
This is a foundational release the opens up many new, exciting improvements we can now make in the future.