Conversation
13-task plan covering robustness fixes, DDA support, new DIA-NN params, InfinDIA groundwork, comprehensive documentation, and issue cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without pipefail, if the command before tee fails, tee returns 0 and the Nextflow task appears to succeed. This masked failures in generate_cfg, diann_msstats, samplesheet_check, and sdrf_parsing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These are the longest-running tasks and most susceptible to transient failures (OOM, I/O timeouts). The error_retry label enables automatic retry on signal exits (130-145, 104, 175). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Guard ch_searchdb and ch_experiment_meta with ifEmpty to fail fast with clear error messages instead of hanging indefinitely. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds conf/diann_versions/v2_3_2.config with ghcr.io/bigbio/diann:2.3.2 container. Use -profile diann_v2_3_2 to opt in. Default stays 1.8.1. Enables DDA support and InfinDIA features. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- New param diann_dda (boolean, default: false) - Version guard: requires DIA-NN >= 2.3.2 - Passes --dda to all 5 DIA-NN modules when enabled - Accepts DDA acquisition method in SDRF when diann_dda=true - Added --dda to blocked lists in all modules Closes #5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- test_dda: BSA dataset with diann_dda=true on DIA-NN 2.3.2 - test_dia_skip_preanalysis: tests previously untested skip path Both added to extended_ci.yml stage 2a. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- diann_light_models: 10x faster in-silico library generation - diann_export_quant: fragment-level parquet export - diann_site_ms1_quant: MS1 apex intensities for PTM quantification All require DIA-NN >= 2.0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Experimental support for InfinDIA (DIA-NN 2.3.0+). Passes --infin-dia to library generation when enabled. Version guard enforces >= 2.3.0. No test config — InfinDIA requires large databases. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Complete reference for all ~70 pipeline parameters grouped by category with types, defaults, descriptions, and version requirements. Closes #1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- DDA mode documentation with limitations - Missing param sections (preprocessing, extra_args scope, verbose output) - DIA-NN version selection guide - Parquet vs TSV output explanation - MSstats format section - pmultiqc citation added - README updated with version table and parameter reference link Closes #3, #9, #15 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by Qodov1.0.0 release — robustness fixes, DDA support, new params, InfinDIA groundwork, comprehensive docs
WalkthroughsDescription• Robustness fixes: pipefail guards, error_retry labels, empty input guards • DDA support via --diann_dda flag (requires DIA-NN >= 2.3.2) • New DIA-NN feature parameters: light-models, export-quant, site-ms1-quant • InfinDIA groundwork with enable_infin_dia and diann_pre_select params • DIA-NN 2.3.2 version config and two new test configs (test_dda, test_dia_skip_preanalysis) • Comprehensive documentation: parameters.md, expanded usage.md, output.md updates, README version table Diagramflowchart LR
A["Robustness Fixes<br/>pipefail, error_retry,<br/>empty guards"] --> B["DDA Support<br/>--diann_dda flag<br/>v2.3.2 required"]
A --> C["New Features<br/>light-models<br/>export-quant<br/>site-ms1-quant"]
A --> D["InfinDIA<br/>Groundwork<br/>enable_infin_dia"]
B --> E["Test Configs<br/>test_dda<br/>test_dia_skip_preanalysis"]
C --> F["Documentation<br/>parameters.md<br/>usage.md<br/>output.md"]
D --> F
E --> G["v1.0.0 Release"]
F --> G
File Changes1. .github/workflows/extended_ci.yml
|
Code Review by Qodo
1. diann_pre_select default is null
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+63
to
+65
| // DIA-NN: InfinDIA (experimental, v2.3.0+) | ||
| enable_infin_dia = false // Enable InfinDIA for ultra-large search spaces | ||
| diann_pre_select = null // --pre-select N precursor limit for InfinDIA |
There was a problem hiding this comment.
1. diann_pre_select default is null 📎 Requirement gap ≡ Correctness
The PR introduces diann_pre_select but leaves its default as null and does not expose the required immunopeptidomics/InfinDIA control parameters (e.g. diann_immunopeptidomics, diann_pre_select_force, diann_ref_library). This violates the requirement to expose and wire the specified immunopeptidomics/InfinDIA parameters with correct defaults/behavior.
Agent Prompt
## Issue description
Compliance requires immunopeptidomics/InfinDIA parameters to be exposed and wired with specific defaults/behavior. The PR adds `diann_pre_select` but leaves its default as `null` and does not expose the required companion parameters (`diann_immunopeptidomics`, `diann_pre_select_force`, `diann_ref_library`).
## Issue Context
Per the checklist, these parameters must exist with the specified defaults and be propagated to the correct DIA-NN flags when immunopeptidomics/InfinDIA mode is active.
## Fix Focus Areas
- nextflow.config[53-66]
- nextflow_schema.json[472-506]
- docs/parameters.md[135-143]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add version guard for DIA-NN 2.0+ params (--light-models, --export-quant, --site-ms1-quant) to prevent crashes with 1.8.1 - Add *.site_report.parquet as optional output in FINAL_QUANTIFICATION for site-level PTM quantification Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prepares the workflows/dia.nf-based pipeline for a v1.0.0 release by improving failure robustness, adding DIA-NN v2.3.2-based DDA support and new DIA-NN feature parameters, and expanding user documentation/test coverage.
Changes:
- Add DDA mode support (
--diann_dda) with DIA-NN version/profile wiring and pass-through of--ddato DIA-NN steps. - Improve robustness via
error_retrylabeling,pipefailin relevant scripts, and.ifEmpty { error(...) }guards to avoid silent hangs. - Add/expand docs and CI test profiles (including new DDA and skip-preanalysis test configs) and add a DIA-NN 2.3.2 version profile.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| workflows/dia.nf | Adds version guards and empty-channel guards for DB/meta channels. |
| subworkflows/local/create_input_channel/main.nf | Extends acquisition-method validation to allow DDA when --diann_dda is enabled. |
| README.md | Adds supported DIA-NN versions table and link to parameter docs. |
| nextflow.config | Adds new params, new test profiles, and DIA-NN 2.3.2 profile include. |
| nextflow_schema.json | Documents new params (diann_dda, InfinDIA, new DIA-NN flags) in the schema. |
| modules/local/sdrf_parsing/main.nf | Adds set -o pipefail in the process script. |
| modules/local/samplesheet_check/main.nf | Adds set -o pipefail in the process script. |
| modules/local/diann/preliminary_analysis/main.nf | Adds error_retry label, blocks/strips --dda from extra args, and passes --dda when enabled. |
| modules/local/diann/insilico_library_generation/main.nf | Adds error_retry, supports --dda, --light-models, and InfinDIA flags. |
| modules/local/diann/individual_analysis/main.nf | Adds error_retry, blocks/strips --dda from extra args, and passes --dda when enabled. |
| modules/local/diann/generate_cfg/main.nf | Adds set -o pipefail in the process script. |
| modules/local/diann/final_quantification/main.nf | Adds error_retry, supports --dda, --export-quant, and --site-ms1-quant. |
| modules/local/diann/diann_msstats/main.nf | Adds set -o pipefail in the process script. |
| modules/local/diann/assemble_empirical_library/main.nf | Adds error_retry, blocks/strips --dda from extra args, and passes --dda when enabled. |
| docs/v1-release-roadmap.md | Removes the old roadmap/design-spec document. |
| docs/usage.md | Adds DDA mode usage/limitations, preprocessing options, extra args guidance, version selection, and verbose outputs info. |
| docs/plans/2026-04-03-v1-release-implementation.md | Adds a detailed internal implementation plan document. |
| docs/parameters.md | Adds a comprehensive parameter reference. |
| docs/output.md | Documents parquet vs TSV output and MSstats conversion output. |
| conf/tests/test_dia_skip_preanalysis.config | Adds a new CI test profile for the skip-preanalysis code path. |
| conf/tests/test_dda.config | Adds a new CI test profile for DDA mode (DIA-NN 2.3.2 container). |
| conf/diann_versions/v2_3_2.config | Adds DIA-NN 2.3.2 version/profile configuration. |
| CITATIONS.md | Adds a pmultiqc citation entry. |
| .github/workflows/extended_ci.yml | Adds test_dda and test_dia_skip_preanalysis to extended CI matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+80
to
+82
| meta.acquisition_method = "dda" | ||
| } else if (acqMethod.isEmpty()) { | ||
| meta.acquisition_method = params.diann_dda ? "dda" : "dia" |
There was a problem hiding this comment.
Setting meta.acquisition_method = "dda" will cause DDA files to be dropped by the downstream branching in workflows/quantmsdiann.nf (it currently only routes items where acquisition_method contains "dia" into the DIA workflow). As a result, --diann_dda true can lead to an empty input channel for the main workflow. Either keep acquisition_method as "dia" for routing purposes (and rely on params.diann_dda for mode), or update the branching logic to include DDA when --diann_dda is enabled.
Suggested change
| meta.acquisition_method = "dda" | |
| } else if (acqMethod.isEmpty()) { | |
| meta.acquisition_method = params.diann_dda ? "dda" : "dia" | |
| // Preserve DIA-NN routing compatibility; DDA mode is controlled by params.diann_dda downstream. | |
| meta.acquisition_method = "dia" | |
| } else if (acqMethod.isEmpty()) { | |
| // Default to the routing value expected by downstream DIA-NN branching. | |
| meta.acquisition_method = "dia" |
Comment on lines
+22
to
+37
| params { | ||
| config_profile_name = 'Test profile for DDA analysis' | ||
| config_profile_description = 'DDA test using BSA dataset with DIA-NN 2.3.2.' | ||
|
|
||
| outdir = './results_dda' | ||
|
|
||
| // Input data - BSA DDA dataset | ||
| input = 'https://raw.githubusercontent.com/bigbio/quantms-test-datasets/quantms/testdata/lfq_ci/BSA/BSA_design.sdrf.tsv' | ||
| database = 'https://raw.githubusercontent.com/bigbio/quantms-test-datasets/quantms/testdata/lfq_ci/BSA/18Protein_SoCe_Tr_detergents_trace.fasta' | ||
|
|
||
| // DDA mode | ||
| diann_dda = true | ||
|
|
||
| // Search parameters matching BSA dataset | ||
| min_peptide_length = 7 | ||
| max_peptide_length = 30 |
There was a problem hiding this comment.
test_dda.config enables diann_dda = true and pins the DIA-NN container to 2.3.2, but it does not set params.diann_version. With the new version guard in workflows/dia.nf, this profile will fail immediately because the default diann_version remains 1.8.1. Set params.diann_version = '2.3.2' here (or ensure CI runs with -profile diann_v2_3_2 in addition to test_dda).
Member
Author
Code reviewFound 2 issues:
quantmsdiann/conf/tests/test_dda.config Lines 22 to 43 in 046c7a1
quantmsdiann/workflows/quantmsdiann.nf Lines 65 to 70 in 046c7a1 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
1. test_dda.config: Add diann_version = '2.3.2' so the version guard
doesn't reject DDA mode (default is 1.8.1, guard requires >= 2.3.2)
2. quantmsdiann.nf: Update branch condition to also match "dda"
acquisition method. Previously "dda".contains("dia") was false,
causing all DDA files to be silently dropped from processing.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These flags exist in DIA-NN 1.8.x but were removed in 2.3.x, causing 'unrecognised option' warnings. Only pass them for versions < 2.3. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Merge dev branch (version bump, tdf2mzml removal, lint fixes, DOI update) - Update test_dda.config to use PXD022287 HeLa DDA dataset with subset FASTA - Add test_dda profile to CI matrix in ci.yml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test_dda profile uses ghcr.io/bigbio/diann:2.3.2 which is a private container requiring authentication. Add Docker login step (matching merge_ci.yml) conditioned on test_dda profile. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove implementation plan from repo, add docs/plans/ to .gitignore - Add lib/VersionUtils.groovy for semantic version comparison (prevents string comparison bugs like '2.10.0' < '2.3') - Update all version guards in dia.nf and module scripts to use VersionUtils.versionLessThan/versionAtLeast Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
DDA analysis support is a major feature warranting a major version bump. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rename output version→versions in sdrf_parsing/meta.yml - Add ch_ prefix to input_file→ch_input_file in input_check/meta.yml - Fix grammar in pmultiqc and diann_msstats meta.yml descriptions - Fix glob pattern in decompress_dotd/meta.yml (double-dot expansion) - Update CITATIONS.md to link published Nature Methods article - Fix schema_input.json error messages (source name, whitespace) - Standardize quantmsdiann keyword in utils meta.yml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
DDA mode is now automatically detected from the SDRF `comment[proteomics data acquisition method]` column when it contains `data-dependent acquisition`. The `--diann_dda` flag is kept as a fallback for SDRFs that lack this column. - Modules now read acquisition method from meta instead of params - INSILICO_LIBRARY_GENERATION receives is_dda as input from workflow - Version guard triggers for both param and SDRF-detected DDA - Updated docs, parameters, and schema to reflect auto-detection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Revert meta.yml input_file naming to match workflow interface - Gate --pre-select behind enable_infin_dia (InfinDIA-only flag) - Tighten --direct-quant version check from 1.9 to 1.9.2 - Improve GHCR login: guard both token and username, skip test_dda gracefully on fork PRs where credentials are unavailable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
Comprehensive pre-release work for quantmsdiann covering robustness fixes, DDA support, new DIA-NN parameters, and documentation.
Robustness (Week 1)
set -o pipefailto 4 modules with tee pipes masking failureserror_retrylabel to all 5 DIA-NN analysis modulesch_searchdbandch_experiment_metawithifEmptyto prevent silent hangsDDA Support (Week 2)
diann_ddaparam — passes--ddato all DIA-NN steps-profile diann_v2_3_2)diann_dda=truetest_ddausing BSA datasetNew Features (Week 3)
--diann_light_models: 10x faster library generation--diann_export_quant: fragment-level parquet export--diann_site_ms1_quant: MS1 apex PTM quantificationenable_infin_dia,diann_pre_select)test_dia_skip_preanalysistest config for previously untested code pathDocumentation (Week 4)
docs/parameters.md: comprehensive reference for all ~70 parametersdocs/usage.md: DDA mode, preprocessing, extra_args scope, version selection, verbose outputdocs/output.md: Parquet vs TSV, MSstats formatIssues
External PRs Required
bigbio/quantms-containers— Build and pushghcr.io/bigbio/diann:2.3.2bigbio/quantms-test-datasets— Add acquisition method column to BSA SDRFTest plan