Fix input to allow empty fixed modifications from SDRF

[Shen-YuFei]

Fix input validation incorrectly rejecting valid SDRF inputs with no fixed modifications.

• Preserve an empty fixed-modification set without adding default Carbamidomethyl (C).
• After applying the variable-modification fallback, reject MS-GF+ searches if both modification sets remain empty, preventing OpenMS MSGFPlusAdapter from silently enabling fixed CAM.
• Add regression tests and update the usage documentation and changelog.
• Restore the executable bit on the existing pipeline-output commit guard; its logic is unchanged.

Validation

Tested with Nextflow 25.10.4, nf-test 0.9.5, and nf-core 4.1.0:

• Regression tests passed.
• All pre-commit checks passed.
• nf-core lint: 0 failures, with the same 23 warnings as upstream.
• Full debug,test,docker integration tests completed 30/30 tasks for both the candidate and upstream, producing quantification tables, QPX, and MuData outputs. No new runtime warning categories were observed.

The initial remote-input run encountered a GitHub TLS handshake failure. Successful runs used local copies of the official test inputs, verified against their Git blob hashes. SDRF contents and analysis parameters were unchanged.

PR checklist

• This description explains the changes and their rationale.
• Regression tests have been added.
• Code linting has been completed.
• The integration test passes.
• Debug-mode warnings have been reviewed against upstream.
• docs/usage.md is updated.
• CHANGELOG.md is updated.

No new tools or output formats are introduced; README, output documentation, citations, and test-datasets updates are not applicable.

[coderabbitai]

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.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 79962c85-927f-4698-b774-7f5ce0cabfd3

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

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Allow empty SDRF fixed modifications with safe MS-GF+ validation

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Preserve empty SDRF fixed-modification sets without injecting Carbamidomethyl.
• Reject MS-GF+ only when fixed and fallback-resolved variable modifications are both empty.
• Add regression coverage, user guidance, changelog notes, and restore hook executability.

Diagram

graph TD
    A["SDRF row"] --> B["Validate fields"] --> C["Normalize fixed"] --> D["Variable fallback"] --> E{"MS-GF+ empty?"}
    E -- "Yes" --> F["Reject search"]
    E -- "No" --> G["Search metadata"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Patch or upgrade MSGFPlusAdapter

• ➕ Would permit truly modification-free MS-GF+ searches
• ➕ Could remove the pipeline-specific compatibility check
• ➖ Depends on an upstream OpenMS behavior change or maintained patch
• ➖ Introduces dependency and version-compatibility risk

2. Validate inside search-engine modules

• ➕ Would colocate the restriction with the affected MS-GF+ adapter
• ➕ Could isolate engine-specific rules from input parsing
• ➖ Would detect invalid input later in pipeline execution
• ➖ May duplicate validation across downstream module paths

Recommendation: Keep the PR's centralized, post-fallback validation because it fails early and preserves valid empty-fixed inputs for every engine. An upstream MSGFPlusAdapter fix would be preferable long-term, but relying on it now would not protect current OpenMS versions.

Files changed (6) +81 / -5

Bug fix (1) +7 / -4

main.nfPreserve empty fixed modifications and guard MS-GF+ +7/-4

Preserve empty fixed modifications and guard MS-GF+

• Removes FixedModifications from nonempty required-field validation and normalizes absent or whitespace values to an empty string. After variable fallback resolution, rejects MS-GF+ searches only when both modification sets remain empty.

subworkflows/local/create_input_channel/main.nf

Tests (1) +59 / -0

main.nf.testCover SDRF modification normalization and engine safeguards +59/-0

Cover SDRF modification normalization and engine safeguards

• Adds parameterized nf-test cases for empty, missing, whitespace, and populated fixed modifications; variable fallback behavior; modification-free Comet and Sage searches; MS-GF+ rejection; and retained label and enzyme validation.

subworkflows/local/create_input_channel/tests/main.nf.test

Documentation (3) +15 / -1

CHANGELOG.mdRecord empty fixed-modification handling +6/-0

Record empty fixed-modification handling

• Adds an Unreleased fix entry describing supported empty fixed modifications and the MS-GF+ safeguard against implicit Carbamidomethyl.

CHANGELOG.md

usage.mdDocument SDRF modification resolution rules +8/-0

Document SDRF modification resolution rules

• Explains fixed and variable modification sourcing, fallback behavior, and modification-free search support. Documents why fully empty sets are rejected for MS-GF+ but accepted for Comet and Sage.

docs/usage.md

nextflow_schema.jsonClarify SDRF modification requirements in input help +1/-1

Clarify SDRF modification requirements in input help

• Updates schema help text so fixed modifications are no longer presented as nonempty required values. Describes empty-set support and the MS-GF+ compatibility restriction.

nextflow_schema.json

Other (1) +0 / -0

block_pipeline_outdir.shRestore pipeline-output guard executability +0/-0

Restore pipeline-output guard executability

• Restores the executable file mode for the existing commit guard. The script logic is unchanged.

.hooks/block_pipeline_outdir.sh

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Three test cases exceed line limit ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The reject MSGF implicit CAM, require label, and require enzyme test maps each place all
fields on one line beyond the 120-character target. Adding another input or assertion attribute to
these regression cases will further obscure which values distinguish each scenario.

Code

subworkflows/local/create_input_channel/tests/main.nf.test[R16-18]

+        [name: 'reject MSGF implicit CAM', fixed: '', variable: '', fallback: '', error: 'Both modification lists are empty'],
+        [name: 'require label', fixed: '', variable: 'Oxidation (M)', label: '', error: 'Missing or empty required SDRF columns'],
+        [name: 'require enzyme', fixed: '', variable: 'Oxidation (M)', enzyme: '', error: 'Missing or empty required SDRF columns'],

Evidence

Compliance rule 10 applies the 120-character target to changed Nextflow and Groovy code. The three
added test definitions each combine enough map fields on one line to exceed that target.

AGENTS.md: Nextflow and Groovy Code Must Follow Project Style
subworkflows/local/create_input_channel/tests/main.nf.test[16-18]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Three newly added test-case definitions avoidably exceed the project's 120-character line-length target.

## Fix Focus Areas
- subworkflows/local/create_input_channel/tests/main.nf.test[16-18]

## Recommended Fix
Expand each long test map across multiple indented lines while preserving every field and expected value.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Validation text exceeds line limit ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new exit call places the entire interpolated validation message on a single line that exceeds
the project's 120-character target. Editing this message requires scanning a dense line, and future
additions can make the validation branch still harder to review.

Code

subworkflows/local/create_input_channel/main.nf[186]

+        exit(1, "ERROR: Both modification lists are empty for '${filestr}', but OpenMS MSGFPlusAdapter would enable fixed Carbamidomethyl (C) in this case. Use Comet or Sage for a search without modifications.")

Evidence

Compliance rule 10 requires changed Nextflow and Groovy code to generally remain within 120
characters and identifies avoidable excess as a failure. The added validation message is written
entirely on one substantially longer line.

AGENTS.md: Nextflow and Groovy Code Must Follow Project Style
subworkflows/local/create_input_channel/main.nf[186-186]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new MS-GF+ validation message avoidably exceeds the project's 120-character line-length target.

## Fix Focus Areas
- subworkflows/local/create_input_channel/main.nf[186-186]

## Recommended Fix
Build the message from wrapped interpolated and single-quoted string segments, preserving its exact rendered text, and pass the resulting value to `exit`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Whitespace-only searches reach MS-GF+ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new guard tests meta.variablemodifications by Groovy truthiness even though the fallback
assigns params.variable_mods without trimming or normalizing it. When fixed modifications are
empty and --variable_mods contains only whitespace, the guard accepts the row and the MS-GF+
command receives that whitespace as a modification token.

Code

subworkflows/local/create_input_channel/main.nf[185]

+    if (params.search_engines.contains('msgf') && !meta.fixedmodifications && !meta.variablemodifications) {

Evidence

The fallback is stored without normalization, while the added guard only checks truthiness; the
schema permits arbitrary strings, and the downstream adapter tokenizes the stored value directly
into command arguments.

subworkflows/local/create_input_channel/main.nf[178-186]
nextflow_schema.json[302-307]
modules/local/openms/msgf/main.nf[90-91]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The MS-GF+ empty-modification guard treats a whitespace-only `params.variable_mods` value as nonempty, allowing a semantically empty search configuration to reach the adapter.

## Fix Focus Areas
- subworkflows/local/create_input_channel/main.nf[178-186]
- subworkflows/local/create_input_channel/tests/main.nf.test[8-18]

## Recommended Fix
Normalize the selected SDRF or fallback variable-modification value with null-safe string conversion and `trim()`, producing an empty string when appropriate, before applying the MS-GF+ guard. Add a regression case using an empty fixed set and a whitespace-only fallback with MS-GF+ and assert that validation rejects it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: ⚖️ Balanced: This changes runtime SDRF validation and MS-GF+ behavior, including fallback ordering and adapter safety, so it carries meaningful correctness risk despite the localized scope.

Tip of the day

💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

🟡 Changes recommended

Trim or normalize whitespace-only variable modifications and add regression coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes SDRF handling so empty fixed modifications remain valid while preventing unsafe empty MS-GF+ modification configurations.

Changes:

• Updates modification validation and fallback behavior.
• Adds regression tests and documentation/changelog updates.
• Restores the pipeline-output hook executable bit.

Review note: whitespace-only variable modifications can bypass the MS-GF+ validation and require normalization.

File summaries

File	Summary
subworkflows/local/create_input_channel/tests/main.nf.test	Adds regression coverage for SDRF modifications.
subworkflows/local/create_input_channel/main.nf	Adjusts modification validation and metadata handling.
nextflow_schema.json	Documents updated input behavior.
docs/usage.md	Explains modification handling and MS-GF+ limitations.
CHANGELOG.md	Records the fix.
.hooks/block_pipeline_outdir.sh	Restores the executable bit without logic changes.

Review details

• Files reviewed: 5/6 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.