Skip to content

Remove unused logo#33

Merged
ypriverol merged 2 commits intobigbio:devfrom
yueqixuan:dev
Apr 5, 2026
Merged

Remove unused logo#33
ypriverol merged 2 commits intobigbio:devfrom
yueqixuan:dev

Conversation

@yueqixuan
Copy link
Copy Markdown
Collaborator

No description provided.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 5, 2026

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: Pro

Run ID: 09f6ed42-bbdb-4feb-95ea-5f3d7aaee81a

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
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
Copy link
Copy Markdown

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Remove unused logo and simplify MultiQC configuration

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Remove unused logo file reference from workflow
• Simplify MultiQC configuration by removing custom data sections
• Remove unused quantms_log input parameter from PMULTIQC process
• Clean up report section ordering and file search patterns
Diagram
flowchart LR
  A["MultiQC Config"] -->|Remove custom_data| B["Simplified config"]
  C["PMULTIQC Process"] -->|Remove quantms_log input| D["Streamlined process"]
  E["Workflow"] -->|Remove logo reference| F["Cleaned workflow"]
  B --> G["Cleaner configuration"]
  D --> G
  F --> G
Loading

Grey Divider

File Changes

1. assets/multiqc_config.yml ⚙️ Configuration changes +0/-65

Remove custom data sections from MultiQC config

• Removed report_section_order configuration block
• Removed entire custom_data section containing MS1 TIC, BPC, peaks, and general stats definitions
• Removed associated sp (search patterns) entries for the deleted custom data sections
• Retained core configuration for pmultiqc experiment design, SDRF, msstats, and DIANN report

assets/multiqc_config.yml


2. modules/local/pmultiqc/main.nf ✨ Enhancement +0/-1

Remove unused quantms_log input parameter

• Removed path quantms_log input parameter from PMULTIQC process
• Simplified process input to only accept multiqc_inputs parameter

modules/local/pmultiqc/main.nf


3. workflows/quantmsdiann.nf ✨ Enhancement +1/-5

Remove logo file reference from workflow

• Removed ch_multiqc_quantms_logo file reference assignment
• Updated SUMMARY_PIPELINE function call to remove logo parameter
• Simplified workflow by eliminating unused logo file handling

workflows/quantmsdiann.nf


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 5, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

Grey Divider


Remediation recommended

1. QC artefacts no longer discovered 🐞 Bug ≡ Correctness
Description
assets/multiqc_config.yml removes sp: search patterns (e.g., pmultiqc/ms_info) so
MultiQC/pmultiqc may stop discovering *_ms_info.parquet and other artefacts even though the
workflow still generates and stages them for reporting. This can silently drop QC content that the
docs say should appear when --mzml_statistics is enabled.
Code

assets/multiqc_config.yml[L65-76]

-  pmultiqc/mztab:
-    fn: "*.mzTab"
-    num_lines: 0
-  pmultiqc/mzML:
-    fn: "*.mzML"
-    num_lines: 0
-  pmultiqc/ms_info:
-    fn: "*_ms_info.parquet"
-    num_lines: 0
-  pmultiqc/idXML:
-    fn: "*.idXML"
-    num_lines: 0
Evidence
The workflow still mixes FILE_PREPARATION.out.statistics (documented as *_ms_info.parquet) into
the MultiQC input set, and docs state these parquet files are used in QC reporting; however the
current MultiQC config no longer includes an sp: mapping for pmultiqc/ms_info, which previously
would have allowed discovery/association of those files.

assets/multiqc_config.yml[7-22]
workflows/quantmsdiann.nf[102-125]
subworkflows/local/file_preparation/main.nf[104-120]
docs/usage.md[36-40]

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 MultiQC config cleanup removed `sp:` entries for artefacts that are still produced and staged for reporting (notably `*_ms_info.parquet`). This can cause pmultiqc/MultiQC to stop discovering those files and omit corresponding QC sections/downloads.

### Issue Context
- The workflow stages `FILE_PREPARATION.out.statistics` into MultiQC inputs.
- Docs state `*_ms_info.parquet` is used in QC reporting when `--mzml_statistics` is enabled.
- Current `assets/multiqc_config.yml` no longer includes an `sp:` mapping for `pmultiqc/ms_info` (and other previously mapped artefacts).

### Fix Focus Areas
- assets/multiqc_config.yml[7-22]

### What to change
- Re-add `sp:` entries for artefacts that should be discoverable (at least `pmultiqc/ms_info: fn: "*_ms_info.parquet"`).
- If removing these patterns was intentional, update docs to reflect that these artefacts are no longer used in QC reporting and/or stop staging them into MultiQC inputs.

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


2. Email template logo path broken 🐞 Bug ☼ Reliability
Description
The pipeline completion email template unconditionally reads
assets/bigbio-quantmsdiann_logo_light.png, but this repository does not appear to ship that asset
under assets/, so enabling email notifications can throw during template rendering (before the
sendmail try/catch). This PR’s “logo cleanup” likely remains incomplete for notifications.
Code

workflows/quantmsdiann.nf[117]

-    ch_multiqc_quantms_logo = file("${projectDir}/assets/nf-core-quantmsdiann_logo_light.png")
Evidence
sendmail_template.txt embeds a logo by reading a fixed path in assets/, and the email utility
renders this template outside the guarded try/catch block; if the file is absent, template rendering
fails and can break the completion notification path.

assets/sendmail_template.txt[12-18]
subworkflows/nf-core/utils_nfcore_pipeline/main.nf[296-301]

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

### Issue description
Email notifications render `assets/sendmail_template.txt`, which unconditionally reads a logo file from `assets/`. If that file is not shipped, rendering fails before the sendmail/mail fallback logic.

### Issue Context
- `sendmail_template.txt` reads `$projectDir/assets/bigbio-quantmsdiann_logo_light.png`.
- Template rendering happens before the `try { ... } catch { ... }` that wraps the sendmail invocation.

### Fix Focus Areas
- assets/sendmail_template.txt[12-18]
- subworkflows/nf-core/utils_nfcore_pipeline/main.nf[296-301]

### What to change
Implement one of:
1) Add the referenced logo file under `assets/` (and ensure it’s packaged/released).
2) Update the template to reference an existing shipped logo path.
3) Guard the template with an existence check and omit the image part if missing (preferred for robustness), e.g. conditional Groovy in the template.

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



Advisory comments

3. PMULTIQC meta.yml mismatch 🐞 Bug ⚙ Maintainability
Description
modules/local/pmultiqc/main.nf now takes a single multiqc_inputs path input, but
modules/local/pmultiqc/meta.yml still documents multiple distinct inputs (expdesign, mzmls,
quantms_results, raw_ids). This mismatch will mislead module consumers and causes incorrect
interface documentation.
Code

modules/local/pmultiqc/main.nf[R8-10]

    input:
    path multiqc_inputs, stageAs: 'results/*'
-    path quantms_log
Evidence
The process input signature is a single staged path list (multiqc_inputs), while the module
metadata describes a different multi-input interface, so the module contract/documentation is out of
sync with the actual code.

modules/local/pmultiqc/main.nf[8-10]
modules/local/pmultiqc/meta.yml[13-25]

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 pmultiqc module metadata (`meta.yml`) documents inputs that do not match the actual Nextflow process signature, which can confuse users and tooling that relies on `meta.yml`.

### Issue Context
The process currently accepts a single `path multiqc_inputs, stageAs: 'results/*'` input.

### Fix Focus Areas
- modules/local/pmultiqc/meta.yml[13-25]
- modules/local/pmultiqc/main.nf[8-10]

### What to change
- Update `modules/local/pmultiqc/meta.yml` to document the real input contract (e.g., a single `multiqc_inputs` collection/dir staged under `results/*`).
- If you want to preserve the older multi-input contract, revert the process interface or add a wrapper workflow that builds `multiqc_inputs` from typed inputs and keep meta.yml aligned.

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


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@yueqixuan yueqixuan requested a review from ypriverol April 5, 2026 13:11
@ypriverol ypriverol merged commit dd3dfbb into bigbio:dev Apr 5, 2026
13 of 16 checks passed
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.

2 participants