Skip to content

some updates related with diann, ptms and pmultiqc small issues#614

Merged
ypriverol merged 12 commits intobigbio:devfrom
ypriverol:dev
Nov 27, 2025
Merged

some updates related with diann, ptms and pmultiqc small issues#614
ypriverol merged 12 commits intobigbio:devfrom
ypriverol:dev

Conversation

@ypriverol
Copy link
Member

@ypriverol ypriverol commented Nov 26, 2025

User description

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the bigbio/quantms branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

PR Type

Enhancement, Bug fix


Description

  • Add contaminant string parameter support for pmultiqc report generation

  • Update SDRF parsing container to newer sdrf-pipelines version

  • Change DIA-NN mzTab export default from true to false

  • Add contaminant_string parameter to nextflow configuration


Diagram Walkthrough

flowchart LR
  A["Configuration Updates"] --> B["Add contaminant_string param"]
  A --> C["Disable DIA-NN mzTab export"]
  B --> D["pmultiqc module"]
  D --> E["Pass contaminant_affix flag"]
  F["Container Updates"] --> G["SDRF parsing module"]
  G --> H["sdrf-pipelines:0.0.33"]
Loading

File Walkthrough

Relevant files
Enhancement
main.nf
Add contaminant affix parameter to pmultiqc                           

modules/local/pmultiqc/main.nf

  • Add contaminant_affix variable that conditionally passes
    --contaminant_affix flag to multiqc command
  • Pass the new contaminant_affix variable to the multiqc command
    execution
+2/-0     
Dependencies
main.nf
Update SDRF parsing container version                                       

modules/local/sdrf_parsing/main.nf

  • Update Singularity container from quantms-utils:0.0.23 to
    sdrf-pipelines:0.0.33
  • Update Biocontainers image from quantms-utils:0.0.23 to
    sdrf-pipelines:0.0.33
+2/-2     
Configuration changes
nextflow.config
Update DIA-NN and pmultiqc configuration defaults               

nextflow.config

  • Change enable_diann_mztab default value from true to false
  • Add new parameter contaminant_string with default value 'CONT'
+2/-1     
nextflow_schema.json
Update schema with new contaminant parameter                         

nextflow_schema.json

  • Update enable_diann_mztab default from true to false in schema
  • Add new contaminant_string parameter definition with type string and
    default value "CONT"
  • Add description for contaminant_string parameter mapping to pmultiqc
    --contaminant_affix flag
+6/-1     

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 26, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review qodo-code-review bot changed the title some updates related with diann, ptms and pmultiqc small issues some updates related with diann, ptms and pmultiqc small issues Nov 26, 2025
This was linked to issues Nov 26, 2025
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 26, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing Auditing: The new parameter handling and MultiQC invocation add functionality but do not include any
explicit audit logging of critical actions in the changed lines.

Referred Code
def args = task.ext.args ?: ''
def disable_pmultiqc = (params.enable_pmultiqc) && (params.export_mztab) ? "--quantms_plugin" : ""
def disable_table_plots = (params.enable_pmultiqc) && (params.skip_table_plots) ? "--disable_table" : ""
def disable_idxml_index = (params.enable_pmultiqc) && (params.pmultiqc_idxml_skip) ? "--ignored_idxml" : ""
def contaminant_affix = params.contaminant_string ? "--contaminant_affix ${params.contaminant_string}" : ""

"""
set -x
set -e

# leaving here to ease debugging
ls -lcth *

cat results/*openms_design.tsv

multiqc \\
    -f \\
    ${disable_pmultiqc} \\
    --config ./results/multiqc_config.yml \\
    ${args} \\
    ${disable_table_plots} \\


 ... (clipped 4 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Edge Cases Unhandled: The new conditional flag construction and command invocation lack explicit validation or
error handling for empty or malformed parameters (e.g., contaminant string), relying on
shell exit only.

Referred Code
def args = task.ext.args ?: ''
def disable_pmultiqc = (params.enable_pmultiqc) && (params.export_mztab) ? "--quantms_plugin" : ""
def disable_table_plots = (params.enable_pmultiqc) && (params.skip_table_plots) ? "--disable_table" : ""
def disable_idxml_index = (params.enable_pmultiqc) && (params.pmultiqc_idxml_skip) ? "--ignored_idxml" : ""
def contaminant_affix = params.contaminant_string ? "--contaminant_affix ${params.contaminant_string}" : ""

"""
set -x
set -e

# leaving here to ease debugging
ls -lcth *

cat results/*openms_design.tsv

multiqc \\
    -f \\
    ${disable_pmultiqc} \\
    --config ./results/multiqc_config.yml \\
    ${args} \\
    ${disable_table_plots} \\


 ... (clipped 4 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Debug Trace Enabled: The script enables shell tracing with 'set -x', which may echo command arguments
(including parameter values) to logs, potentially exposing sensitive data.

Referred Code
set -x
set -e

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Not Validated: The new 'contaminant_affix' parameter is passed directly to the command without
visible sanitization or quoting beyond string interpolation, which could allow unexpected
values if misconfigured.

Referred Code
def contaminant_affix = params.contaminant_string ? "--contaminant_affix ${params.contaminant_string}" : ""

"""
set -x
set -e

# leaving here to ease debugging
ls -lcth *

cat results/*openms_design.tsv

multiqc \\
    -f \\
    ${disable_pmultiqc} \\
    --config ./results/multiqc_config.yml \\
    ${args} \\
    ${disable_table_plots} \\
    ${disable_idxml_index} \\
    ${contaminant_affix} \\

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@ypriverol
Copy link
Member Author

Superseed PR #612

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Nov 26, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Prevent a potential command injection

Wrap the params.contaminant_string value in single quotes to prevent a potential
command injection vulnerability.

modules/local/pmultiqc/main.nf [23]

-def contaminant_affix = params.contaminant_string ? "--contaminant_affix ${params.contaminant_string}" : ""
+def contaminant_affix = params.contaminant_string ? "--contaminant_affix '${params.contaminant_string}'" : ""
  • Apply / Chat
Suggestion importance[1-10]: 10

__

Why: The suggestion correctly identifies a critical command injection vulnerability and provides a valid fix, significantly improving the security of the script.

High
  • Update

# Conflicts:
#	.github/workflows/awsfulltest.yml
#	.github/workflows/awstest.yml
#	.github/workflows/download_pipeline.yml
#	.github/workflows/nf-test.yml
#	.github/workflows/release-announcements.yml
#	README.md
#	ro-crate-metadata.json
#	subworkflows/local/utils_nfcore_quantms_pipeline/main.nf
#	workflows/quantms.nf
@ypriverol ypriverol merged commit 447d8f1 into bigbio:dev Nov 27, 2025
37 checks passed
@qodo-code-review qodo-code-review bot mentioned this pull request Jan 8, 2026
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

pmultiqc Contaminant prefix parameter Met-loss+Acetyl parsing

2 participants