Skip to content

Conversation

@weizhongchun
Copy link

@weizhongchun weizhongchun commented Dec 22, 2025

Summary by CodeRabbit

  • New Features

    • Replaced PTM localization tool with onsite, providing enhanced configuration options including algorithm selection, customizable fragment tolerance, and advanced decoy handling for phosphorylation site identification.
  • Configuration

    • Updated parameters from luciphor_ to onsite_ prefix with new tuning options for fragment analysis, error units, and processing threads.
  • Documentation

    • Updated workflow documentation to reflect new PTM localization approach.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions
Copy link

This PR is against the master branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @weizhongchun,

It looks like this pull-request is has been made against the weizhongchun/quantms master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the weizhongchun/quantms dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 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.

Walkthrough

This PR replaces the LUCIPHOR tool with ONSITE for phosphorylation site localization across the entire workflow. Changes include adding a new ONSITE module, updating configuration parameters, modifying workflow subcomponents to reference ONSITE outputs, and revising documentation to reflect the new tool.

Changes

Cohort / File(s) Summary
Documentation
README.md, docs/output.md
Updated tool reference from "Luciphor" to "onsite" in modification localization step descriptions
Configuration Files
nextflow.config
Replaced luciphor_debug parameter with onsite_debug and added new onsite-specific parameters: onsite_algorithm, onsite_fragment_method, onsite_fragment_tolerance, onsite_fragment_error_units, onsite_add_decoys, onsite_neutral_losses, onsite_decoy_mass, onsite_decoy_neutral_losses, onsite_threads
Module Configuration
conf/modules/modules.config, conf/modules/verbose_modules.config
Updated withName patterns from "LUCIPHOR" to "ONSITE" and changed publishDir paths from ptm_localization/luciphor to ptm_localization/onsite
Schema Configuration
nextflow_schema.json
Replaced luciphor-prefixed parameters with comprehensive onsite parameter definitions including algorithm selection, fragment tolerance/units, and decoy handling options
New ONSITE Module
modules/local/openms/onsite/main.nf, modules/local/openms/onsite/meta.yml
Added new ONSITE process with support for lucxor, AScore, and PhosphoRS algorithms, configurable fragment tolerance/units, decoy handling, and thread management; includes metadata schema documentation
Workflow Updates
subworkflows/local/phospho_scoring/main.nf, subworkflows/local/phospho_scoring/meta.yml
Replaced LUCIPHOR module include/calls with ONSITE; updated output channel from id_luciphor to id_onsite and version references
Downstream Workflow Integration
subworkflows/local/id/main.nf, subworkflows/local/dda_id/main.nf
Updated channel references to use PHOSPHO_SCORING.out.id_onsite instead of PHOSPHO_SCORING.out.id_luciphor

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • ONSITE module implementation: Verify the command-line construction logic for all three supported algorithms (lucxor, AScore, PhosphoRS) including fragment tolerance/mass units, decoy modifications, and parameter passing
  • Parameter propagation: Ensure all new onsite parameters are correctly threaded through configuration files, schema, and module implementation
  • Workflow channel connectivity: Confirm output channel renaming (id_luciphorid_onsite) is correctly referenced across all subworkflows (phospho_scoring, id, dda_id)
  • Default values and algorithm specifics: Validate that algorithm-specific branching logic and default modifications list derivation from params.mod_localization work correctly

Possibly related PRs

Suggested labels

Review effort 4/5

Suggested reviewers

  • jpfeuffer
  • daichengxin
  • fabianegli

Poem

🐰 A hop, a skip—the scoring's switched with care,
From Luciphor to ONSITE, refined and rare!
New algorithms bloom with phospho-grace,
Parameters dance in their rightful place. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add onsite module' accurately reflects the primary change: introducing a new ONSITE module and replacing the LUCIPHOR module throughout the codebase. The title is concise and directly related to the main objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.

@weizhongchun weizhongchun changed the base branch from master to dev December 22, 2025 07:27
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e516ddc and 43d9cae.

📒 Files selected for processing (12)
  • README.md
  • conf/modules/modules.config
  • conf/modules/verbose_modules.config
  • docs/output.md
  • modules/local/openms/onsite/main.nf
  • modules/local/openms/onsite/meta.yml
  • nextflow.config
  • nextflow_schema.json
  • subworkflows/local/dda_id/main.nf
  • subworkflows/local/id/main.nf
  • subworkflows/local/phospho_scoring/main.nf
  • subworkflows/local/phospho_scoring/meta.yml
🔇 Additional comments (14)
docs/output.md (1)

23-23: LGTM!

Documentation correctly updated to reflect the new onsite tool for modification localization.

conf/modules/verbose_modules.config (1)

160-162: LGTM!

The verbose module configuration has been correctly updated to reference the ONSITE module and publish to the appropriate directory.

subworkflows/local/phospho_scoring/meta.yml (1)

11-11: LGTM!

The component reference has been correctly updated from luciphor to onsite in the phospho_scoring subworkflow metadata.

subworkflows/local/dda_id/main.nf (1)

171-171: LGTM!

The output reference has been consistently updated to use id_onsite, matching the change in subworkflows/local/id/main.nf. This ensures uniform handling of ONSITE outputs across both DDA_ID and ID workflows.

README.md (1)

39-39: The onsite repository exists and is publicly accessible.

The repository bigbio/onsite exists and was last updated August 27, 2025, confirming the URL referenced in the documentation is valid and publicly available.

subworkflows/local/id/main.nf (1)

67-67: PHOSPHO_SCORING subworkflow correctly emits id_onsite output.

Verified that the PHOSPHO_SCORING subworkflow (subworkflows/local/phospho_scoring/main.nf) emits the id_onsite output at line 28. The output reference in line 67 of the current file is valid.

conf/modules/modules.config (1)

112-125: Parameter definition verified and properly configured.

The params.onsite_debug parameter is defined in nextflow.config with default value 0 and fully documented in nextflow_schema.json as an integer type with description "Debug level for onsite step. Increase for verbose logging and keeping temp files." No action required.

subworkflows/local/phospho_scoring/main.nf (1)

6-28: LGTM! Clean migration from LUCIPHOR to ONSITE.

The workflow correctly replaces LUCIPHOR with ONSITE across both execution branches (multi-engine and single-engine paths). The channel naming, version tracking, and emit declarations are all consistently updated to reference ONSITE outputs.

modules/local/openms/onsite/main.nf (4)

54-67: Target modifications logic is well-structured.

The code correctly handles the target modifications for LucXor:

  • Parses comma-separated modifications from params.mod_localization
  • Adds decoy modification when enabled
  • Provides sensible defaults when no modifications are specified

13-16: Output declarations are correct.

The output patterns correctly use wildcards to match algorithm-specific filenames, and the emit name ptm_in_id_onsite aligns with the workflow's expectations.


95-98: Verify the version extraction command matches the onsite tool's output format.

The command at line 97 extracts version using grep -oP 'version \\K[0-9.]+'. This pattern assumes onsite --version outputs text containing the word "version" followed by dot-separated numbers. If the tool's output format differs (e.g., "onsite X.Y.Z" without the "version" prefix), the regex will fail and the fallback "0.0.1" will always be recorded.


32-32: Add validation to ensure meta.dissociationmethod is populated when using SDRF input.

When using SDRF, the DissociationMethod column is read without validation. If this column is missing, null, or contains an unrecognized value that doesn't match the known dissociation types (CID, HCD, ETD, ECD), meta.dissociationmethod could become null. Although params.onsite_fragment_method has a default fallback to 'CID' at line 32, other modules like luciphor and msgf use meta.dissociationmethod directly (see lines 44 and 80 in their respective main.nf files), making them susceptible to null values. Add a null-check or default value in the input channel creation to ensure this field is always populated.

nextflow.config (1)

147-156: Parameter configuration is well-structured.

The onsite parameters are properly defined with sensible defaults. The algorithm, fragmentation method, and tolerance settings align with the module implementation.

nextflow_schema.json (1)

433-504: Schema updates are comprehensive and consistent.

The JSON schema properly defines all onsite parameters with:

  • Correct types and constraints (enums, number ranges)
  • Default values matching nextflow.config
  • Appropriate help text and UI metadata
  • Proper field visibility settings

Comment on lines +6 to +8
container "${ workflow.containerEngine == 'singularity' && !task.ext.singularity_pull_docker_container ?
'https://depot.galaxyproject.org/singularity/pyonsite:0.0.1--pyhdfd78af_0' :
'quay.io/biocontainers/pyonsite:0.0.1--pyhdfd78af_0' }"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# Search for pyonsite references in the codebase
find . -type f -name "*.nf" | head -20

Repository: bigbio/quantms

Length of output: 956


🏁 Script executed:

# Check if the file exists and show its content
if [ -f "modules/local/openms/onsite/main.nf" ]; then
  cat -n "modules/local/openms/onsite/main.nf" | head -20
fi

# Also search for any pyonsite references in the repo
rg "pyonsite" -t nextflow 2>/dev/null || echo "No pyonsite references found with ripgrep"

Repository: bigbio/quantms

Length of output: 920


The pyonsite:0.0.1--pyhdfd78af_0 container does not appear to exist in public registries and will cause runtime failures.

Verify that this container is available in both Quay.io (quay.io/biocontainers/pyonsite:0.0.1--pyhdfd78af_0) and the Galaxy Singularity depot before deploying. If the package is in Bioconda, ensure it has been released and the BioContainers images have been built. If not yet released, this process module will fail at execution time.

Comment on lines +1 to +45
name: onsite
description: PTM site localization using onsite (AScore, PhosphoRS, or LucXor).
keywords:
- onsite
- AScore
- PhosphoRS
- LucXor
- modification
- PTM
tools:
- onsite:
description: |
Mass spectrometry post-translational modification localization tool
homepage: https://github.com/bigbio/onsite
documentation: https://github.com/bigbio/onsite
input:
- meta:
type: map
description: Groovy Map containing sample information
- mzml_file:
type: file
description: Input spectrum file.
pattern: "*.mzML"
- id_file:
type: file
description: Protein/peptide identifications file
pattern: "*.idXML"
output:
- meta:
type: map
description: Groovy Map containing sample information
- out_id_ptmscores:
type: file
description: Protein/peptide identifications file with scored PTM sites
pattern: "*.idXML"
- log:
type: file
description: log file
pattern: "*.log"
- version:
type: file
description: File containing software version
pattern: "versions.yml"
authors:
- "@weizhongchun"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify onsite module implementation exists
echo "=== Checking if onsite module main.nf exists ==="
fd -t f 'main.nf' modules/local/openms/onsite/

echo -e "\n=== Verifying output channel names in onsite module ==="
if [ -f modules/local/openms/onsite/main.nf ]; then
  rg -n 'emit:' modules/local/openms/onsite/main.nf
  echo -e "\n=== Full onsite main.nf content ==="
  cat modules/local/openms/onsite/main.nf
else
  echo "Module file not found"
fi

echo -e "\n=== Checking if phospho_scoring subworkflow exists ==="
fd -t f 'main.nf' subworkflows/local/phospho_scoring/

echo -e "\n=== Verifying phospho_scoring subworkflow uses correct output names ==="
if [ -f subworkflows/local/phospho_scoring/main.nf ]; then
  rg -n 'ONSITE' subworkflows/local/phospho_scoring/main.nf
else
  echo "Subworkflow file not found"
fi

Repository: bigbio/quantms

Length of output: 4894


Update meta.yml output names to match main.nf emit declarations.

The meta.yml file declares output out_id_ptmscores, but the corresponding main.nf module emits the channel as ptm_in_id_onsite. Update the meta.yml to declare:

  • ptm_in_id_onsite instead of out_id_ptmscores

The phospho_scoring subworkflow correctly uses ONSITE.out.ptm_in_id_onsite, confirming the main.nf implementation is correct and the meta.yml documentation is out of sync.

🤖 Prompt for AI Agents
modules/local/openms/onsite/meta.yml lines 1-45: the outputs section declares
out_id_ptmscores but the module actually emits ptm_in_id_onsite; update the
outputs key name to ptm_in_id_onsite (keep the same type, description and
pattern values), replacing the out_id_ptmscores entry so the meta.yml matches
main.nf and the phospho_scoring subworkflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant