Skip to content

Commit 5a4e3f6

Browse files
jonn-smithclaude
andauthored
Refactor HiFi QC pipeline + add Dockstore manifest (#8)
* refactor(wdl): split hifi_qc_pipeline_3 into per-category modules Reorganize monolithic wdl/hifi_qc_pipeline_3.wdl into LRP-style layout (structs/, tasks/<Category>/, pipelines/<Tech>/<Category>/) per docs/WDL_STYLE_RULES.md. Every task now carries the canonical boilerplate: meta + outputs sub-block, tool citation where applicable, parameter_meta for every input, extra_args passthrough, 5x input-based disk sizing, resource-detection preamble, RuntimeAttr override hook, and column-aligned runtime block. Workflow calls aliased t_01..t_07 in lexical invocation order. Also fix two shellcheck lints surfaced by miniwdl --strict (SC2002 useless-cat in the preamble, SC2086 unquoted \${NUM_CPUS}); update the preamble in WDL_STYLE_RULES.md to match. miniwdl check --strict wdl/pipelines/PacBio/QC/HifiPipeline.wdl passes clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): take sample-sheet columns as direct workflow inputs Replace the single File input_tsv with five parallel Array[String] inputs (bams, genera, species, strains, barcodes). The caller now hands every column in directly; the workflow no longer reads or parses a TSV. Consequences: - Drop ParseSampleSheet task from tasks/Utility/SampleSheetUtils.wdl (no longer used). MergeSampleStats stays. - Renumber workflow call aliases from t_01..t_07 down to t_01..t_06 in lexical invocation order: t_01_BatchGetTaxIdAndGenomeSize t_02_BamToFastqAndStats (scatter) t_03_HifiSeqkitStats t_04_HifiKraken2 t_05_HifiReadStats t_06_MergeSampleStats - Barcodes were previously derived from BAM basenames inside ParseSampleSheet; they are now explicit inputs. miniwdl check --strict wdl/pipelines/PacBio/QC/HifiPipeline.wdl passes clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): collapse HifiPipeline to a single-sample workflow Strip the multi-sample scatter and replace its inputs (parallel arrays of bams/genera/species/strains/barcodes) with scalar inputs for one sample: input_bam, sample_name, genus, species. - Drop tasks/Utility/SampleSheetUtils.wdl entirely; MergeSampleStats no longer has a caller. - Replace BatchGetTaxIdAndGenomeSize with GetTaxIdAndGenomeSize in tasks/Utility/Taxonomy.wdl: scalar (genus, species) inputs, single TSV plus tax_id (Int) and expected_genome_size (String) outputs. - HifiPipeline now has 5 calls, no scatter, aliased t_01..t_05 in lexical order: t_01_GetTaxIdAndGenomeSize t_02_BamToFastqAndStats t_03_HifiSeqkitStats t_04_HifiKraken2 t_05_HifiReadStats - Workflow outputs renamed per_sample_* -> sample_* and now File (not Array[File]); added sample_taxid_tsv and sample_bam_stats pass-throughs. - strain column dropped; its only consumer was the (removed) merge task. miniwdl check --strict wdl/pipelines/PacBio/QC/HifiPipeline.wdl passes clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): expose per-metric scalars instead of stats TSV files Replace the bundled-TSV outputs (sample_stats_tsv, sample_bam_stats, sample_taxid_tsv, sample_stats_txt) with one workflow output per metric, typed Int / Float / String as appropriate. HifiReadStats: - Add a small bash splitter after the awk that fans the single-row hifi_read_stats.tsv out into stat.<key>.txt files (one per column), then bind each file in the output block via read_int/read_float/ read_string. - Drop the hifi_read_stats_tsv and hifi_read_stats_txt outputs; the report (formatted ASCII table) is still exposed. - Add the 27 new outputs to meta.outputs; update the stale parameter_meta reference (BatchGetTaxIdAndGenomeSize -> the new GetTaxIdAndGenomeSize). HifiPipeline: - Drop sample_stats_tsv, sample_stats_txt, sample_bam_stats, sample_taxid_tsv. - Keep sample_fastq, sample_kraken_report, sample_stats_report as file outputs. - Pass through tax_id (Int) and expected_genome_size (String) from t_01, and the 27 metric scalars from t_05. - Update meta.outputs to match. miniwdl check --strict wdl/pipelines/PacBio/QC/HifiPipeline.wdl passes clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): decouple BamToFastqAndStats from HifiReadStats HifiReadStats no longer consumes bam_stats_tsv — the 3 bam-derived metrics (mean_read_accuracy, mean_qual_score, mean_passes) were pure passthrough with no computation. Drop the input, the awk column lookups, the report/tsv/txt emissions, and the 3 output bindings. HifiPipeline now exposes those scalars directly from t_02_BamToFastqAndStats under the task's native names (mean_read_accuracy, mean_qual_score, mean_passes), dropping the cosmetic HifiReadStats renames (mean_read_base_qual, mean_num_passes). BamToFastqAndStats drops the bam_stats_tsv File output — no remaining consumer; the per-scalar txt files driving the typed outputs are kept. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): expose scalars from originating tasks; shrink HifiReadStats HifiSeqkitStats and HifiKraken2 now emit per-metric scalar outputs (read counts, length quantiles, Q20/Q30, GC%, bases over thresholds, kraken classification percentages, top genus/species). HifiReadStats no longer parses upstream TSVs — it takes the scalars directly, computes the 3 coverage estimates (the only derived metric), and writes the boxed ASCII report. Workflow output names are unchanged; bindings now point at the originating tasks (t_03_HifiSeqkitStats for length/quality/GC metrics, t_04_HifiKraken2 for classification metrics, t_05_HifiReadStats for the coverage estimates and the human-readable report). Dead avg_len lookup is dropped. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): rename HifiReadStats -> HifiCoverageAndReport Name now reflects what the task actually does: estimate coverage (overall + reads >10kb / >20kb) and render the per-sample boxed ASCII report. Artifact filename and output binding follow: hifi_read_stats.report.txt -> hifi_stats.report.txt, hifi_read_stats_report -> hifi_stats_report. Workflow call alias updated to t_05_HifiCoverageAndReport. Workflow output names unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): rename HifiCoverageAndReport -> CreateHifiQCReport Task name and workflow call alias updated; artifact filename and output binding (hifi_stats_report) unchanged. Workflow output names unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(wdl): port LRP SummarizePBI into HVP-LR HifiPipeline Adds wdl/tasks/Utility/PacBioPbi.wdl with task SummarizeHifiPbi, a verbatim port of broadinstitute/long-read-pipelines' SummarizePBI (wdl/tasks/Utility/PBUtils.wdl) and its compute_pbi_stats.py (docker/lr-pb/). Attribution preserved in task header; original author Kiran V Garimella. Rather than depending on the LRP-internal lr-pb container, the Python script is embedded via heredoc and runs under python:3.11-slim with numpy<2 and construct<2.10 pip-installed at task start. HifiPipeline now takes File input_pbi and surfaces the 12 PBI-derived metrics (pbi_reads, pbi_bases, pbi_mean_qual, pbi_median_qual, polymerase_{mean,median,stdev,n50}, subread_{mean,median,stdev,n50}) plus the raw key/value map. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Revert "feat(wdl): port LRP SummarizePBI into HVP-LR HifiPipeline" This reverts commit 9257209. * chore(dockstore): add .dockstore.yml registering HifiPipeline Initial Dockstore manifest (schema v1.2) listing the PacBio HiFi QC workflow. Filters track main and jts_* branches plus all tags so in-progress refactor branches publish alongside releases. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(dockstore): add Terrance Shea as co-author; record ORCID + affiliation Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(wdl): rename HifiPipeline -> HifiReadQCPipeline Workflow file and entry point renamed so the name itself signals "QC pipeline for PacBio HiFi reads". Dockstore manifest updated to match the new path and workflow name. No behavioral change; all inputs, outputs, and call structure unchanged. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(dockstore): add Terrance Shea ORCID Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: link docs/WDL_STYLE_RULES.md from CLAUDE.md and AGENTS.md The style rules already exist under docs/ but were not referenced from the top-level agent guidance, so a fresh contributor or agent had no way to discover them. Point at them from CLAUDE.md's preamble and from AGENTS.md's WDL validation section plus the new-WDL checklist. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(readme): refresh layout, paths, and current workflows - Expand wdl/ subtree to show structs/tasks/pipelines split. - Add docs/ and .dockstore.yml to the layout block. - Replace the flat 'wdl/<workflow>.wdl' rows in the 'where to put new things' table with the actual tasks/<Category>/ and pipelines/<Tech>/<Category>/ paths used in this repo. - Link docs/WDL_STYLE_RULES.md from 'Where to read next'. - Add a 'Current workflows' table covering HifiReadQCPipeline and point at the Dockstore manifest. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Jonn Smith <11667487+jonn-smith@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 112b423 commit 5a4e3f6

11 files changed

Lines changed: 1054 additions & 839 deletions

File tree

.dockstore.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
version: 1.2
2+
3+
# Dockstore registration for HVP-LR workflows.
4+
# Reference: https://docs.dockstore.org/en/stable/getting-started/dockstore-yml.html
5+
6+
workflows:
7+
- name: HifiReadQCPipeline
8+
subclass: WDL
9+
primaryDescriptorPath: /wdl/pipelines/PacBio/QC/HifiReadQCPipeline.wdl
10+
readMePath: /README.md
11+
authors:
12+
- name: Jonn Smith
13+
orcid: 0000-0002-4279-8220
14+
affiliation: Broad Institute
15+
- name: Terrance Shea
16+
orcid: 0000-0002-6586-9298
17+
affiliation: Broad Institute
18+
filters:
19+
branches:
20+
- main
21+
- /jts_.*/
22+
tags:
23+
- /.*/

AGENTS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ Trivy runs twice per image: once as a build gate (table format, `exit-code: 1`),
180180

181181
Override the threshold per repo via the `TRIVY_LARGE_IMAGE_BYTES` GitHub Actions repository variable.
182182

183-
## WDL validation
183+
## WDL style + validation
184+
185+
Style rules (file layout, boilerplate, task skeleton, naming, output ordering, `t_NN_` call aliasing, etc.) live in [docs/WDL_STYLE_RULES.md](docs/WDL_STYLE_RULES.md). Read it before writing or reformatting any `.wdl` file.
184186

185187
The `miniwdl check --strict` pre-commit hook is the **only** WDL gate. There is no CI safety net. If a contributor commits without running pre-commit (`git commit -n`), broken WDLs land on `main`.
186188

@@ -233,6 +235,7 @@ Concurrency: `release-main` group, no cancel-in-progress.
233235

234236
## Adding a new WDL — checklist
235237

238+
0. Read [docs/WDL_STYLE_RULES.md](docs/WDL_STYLE_RULES.md) — file layout, task skeleton, naming, call aliasing.
236239
1. Write under `wdl/<workflow>.wdl`.
237240
2. (Optional) Inputs JSON alongside as `wdl/<workflow>.inputs.json`.
238241
3. Run `miniwdl check --strict wdl/<workflow>.wdl` locally. The pre-commit hook runs the same.

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# CLAUDE.md
22

3-
**Before doing any work in this repo, read [AGENTS.md](AGENTS.md) in its entirety.** Then read [CONTRIBUTING.md](CONTRIBUTING.md).
3+
**Before doing any work in this repo, read [AGENTS.md](AGENTS.md) in its entirety.** Then read [CONTRIBUTING.md](CONTRIBUTING.md). **Before touching any `.wdl` file, also read [docs/WDL_STYLE_RULES.md](docs/WDL_STYLE_RULES.md).**
44

55
This is not optional and not skippable for "small" or "obvious" changes. Many file edits in this repo look trivial but have non-obvious CI consequences:
66

README.md

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,14 @@ HVP-LR/
1010
│ └── <image>/ # (Dockerfile, Makefile, env.yaml, .dockerignore,
1111
│ # .trivyignore, .trivy-ignore-policy.rego)
1212
├── wdl/ # WDL workflow definitions
13+
│ ├── structs/Structs.wdl # Shared RuntimeAttr struct
14+
│ ├── tasks/<Category>/ # Task modules (Preprocessing, QC, Utility, ...)
15+
│ └── pipelines/<Tech>/<Cat>/ # End-to-end workflows (<Tech> = PacBio | ONT | ILMN | TechAgnostic)
16+
├── docs/ # Project docs (WDL style rules, etc.)
1317
├── scripts/ # CI helper scripts
1418
├── .github/workflows/ # docker.yml (build/push/scan), cd.yml (release)
1519
├── .pre-commit-config.yaml # WDL validation hook
20+
├── .dockstore.yml # Dockstore workflow registration manifest
1621
├── dev-requirements.txt # Local dev tooling
1722
├── VERSION # Repo-level version
1823
├── LICENSE # MIT License
@@ -27,14 +32,16 @@ HVP-LR/
2732
| What | Where |
2833
|------|-------|
2934
| A new Docker image | `docker/<image-name>/` |
30-
| A new WDL workflow or task | `wdl/<workflow>.wdl` |
31-
| A workflow's inputs JSON | `wdl/<workflow>.inputs.json` (alongside the WDL) |
35+
| A new task module | `wdl/tasks/<Category>/<Name>.wdl` |
36+
| A new pipeline / workflow | `wdl/pipelines/<Tech>/<Category>/<Name>.wdl` |
37+
| A workflow's inputs JSON | Alongside the WDL as `<Name>.inputs.json` |
3238
| Helper / one-off scripts | `scripts/` (avoid if not CI-related; otherwise put inside the relevant `docker/<image>/`) |
3339

3440
## Where to read next
3541

3642
- **Setting up locally / opening a PR**[CONTRIBUTING.md](CONTRIBUTING.md)
3743
- **CI mechanics, Docker conventions, releases, Trivy filtering, hard rules**[AGENTS.md](AGENTS.md)
44+
- **WDL style rules** (file layout, task skeleton, naming, call aliasing) → [docs/WDL_STYLE_RULES.md](docs/WDL_STYLE_RULES.md)
3845
- **License terms**[LICENSE](LICENSE)
3946

4047
If you're an AI agent (Claude or otherwise), start with [CLAUDE.md](CLAUDE.md) / [AGENTS.md](AGENTS.md).
@@ -47,6 +54,14 @@ If you're an AI agent (Claude or otherwise), start with [CLAUDE.md](CLAUDE.md) /
4754

4855
Images are published to `ghcr.io/broadinstitute/hvp-lr/<image>:<version>` and (when GCR is configured) `us.gcr.io/broadinstitute/<image>:<version>`. GHCR packages are private by default.
4956

57+
## Current workflows
58+
59+
| Workflow | Path | Purpose |
60+
|----------|------|---------|
61+
| `HifiReadQCPipeline` | `wdl/pipelines/PacBio/QC/HifiReadQCPipeline.wdl` | Single-sample PacBio HiFi read QC: taxonomy lookup, BAM→FASTQ, seqkit + kraken2 stats, per-sample human-readable report |
62+
63+
Workflows are registered with Dockstore via [.dockstore.yml](.dockstore.yml).
64+
5065
## License
5166

5267
MIT — see [LICENSE](LICENSE). The LICENSE file is bundled into every built container at `/opt/hvp-lr/LICENSE`. If you redistribute the source or an image, MIT requires the copyright notice and license text to travel with it.

docs/WDL_STYLE_RULES.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ task ExampleTask {
8181
set -euxo pipefail
8282
8383
# ---- Resource detection (required preamble) ----
84-
NUM_CPUS=$(cat /proc/cpuinfo | grep '^processor' | tail -n1 | awk '{print $NF+1}')
84+
NUM_CPUS=$(grep '^processor' /proc/cpuinfo | tail -n1 | awk '{print $NF+1}')
8585
RAM_IN_GB=$(free -g | grep "^Mem" | awk '{print $2}')
8686
8787
# Reserve 1 GB for OS + container overhead.
@@ -386,7 +386,7 @@ Insert immediately after `set -euxo pipefail`:
386386
set -euxo pipefail
387387
388388
# ---- Resource detection (required preamble) ----
389-
NUM_CPUS=$(cat /proc/cpuinfo | grep '^processor' | tail -n1 | awk '{print $NF+1}')
389+
NUM_CPUS=$(grep '^processor' /proc/cpuinfo | tail -n1 | awk '{print $NF+1}')
390390
RAM_IN_GB=$(free -g | grep "^Mem" | awk '{print $2}')
391391
392392
# Reserve 1 GB for OS + container overhead.

0 commit comments

Comments
 (0)