Skip to content

bump quantms-rescoring version - fix bug with quantms-rescoring container permissions#681

Merged
ypriverol merged 6 commits intobigbio:devfrom
daichengxin:dev
Mar 3, 2026
Merged

bump quantms-rescoring version - fix bug with quantms-rescoring container permissions#681
ypriverol merged 6 commits intobigbio:devfrom
daichengxin:dev

Conversation

@daichengxin
Copy link
Collaborator

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).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 3, 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.

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
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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
Contributor

Review Summary by Qodo

Upgrade quantms-rescoring to 0.0.15 and remove environment setup code

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Upgrade quantms-rescoring container images to version 0.0.15
• Remove redundant UID/environment variable setup code from processes
• Simplify container initialization across four rescoring modules
Diagram
flowchart LR
  A["Four Rescoring Modules"] -->|"Update container version"| B["0.0.15 Images"]
  A -->|"Remove UID setup code"| C["Simplified Scripts"]
  B --> D["Enhanced Processes"]
  C --> D
Loading

Grey Divider

File Changes

1. modules/local/utils/msrescore_features/main.nf ✨ Enhancement +2/-16

Update msrescore_features container and remove setup code

• Updated container image version from 0.0.14 to 0.0.15
• Removed 15-line UID-less environment setup block
• Simplified script initialization for feature extraction

modules/local/utils/msrescore_features/main.nf


2. modules/local/utils/msrescore_fine_tuning/main.nf ✨ Enhancement +2/-13

Update msrescore_fine_tuning container and remove setup code

• Updated container image version from 0.0.14 to 0.0.15
• Removed 14-line UID-less container environment setup block
• Streamlined transfer learning process initialization

modules/local/utils/msrescore_fine_tuning/main.nf


3. modules/local/utils/psm_clean/main.nf ✨ Enhancement +2/-12

Update psm_clean container and remove setup code

• Updated container image version from 0.0.13 to 0.0.15
• Removed 13-line UID-less container environment setup block
• Simplified PSM cleaning process initialization

modules/local/utils/psm_clean/main.nf


View more (1)
4. modules/local/utils/spectrum_features/main.nf ✨ Enhancement +2/-12

Update spectrum_features container and remove setup code

• Updated container image version from 0.0.13 to 0.0.15
• Removed 13-line UID-less container environment setup block
• Simplified spectrum feature extraction initialization

modules/local/utils/spectrum_features/main.nf


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Mar 3, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Removed UID/HOME hardening 🐞 Bug ⛯ Reliability
Description
The PR removes per-process environment setup (USER/LOGNAME/HOME and cache dir prep) from several
quantms-rescoring steps. With the pipeline’s Docker profile forcing containers to run as the host
UID/GID, this can break in arbitrary-UID environments (no passwd entry, non-existent or non-writable
$HOME), causing rescoring steps to fail at runtime.
Code

modules/local/utils/msrescore_fine_tuning/main.nf[L51-61]

-    # Fix for UID-less container environments (e.g. GitHub Actions uid 1001)
-    # Python's getpass.getuser() checks USER env var before calling getpwuid()
-    if [ -w /etc/passwd ]; then
-        echo "\${USER:-quantms}:x:\$(id -u):\$(id -g):\${USER:-quantms} user:/tmp:/bin/bash" >> /etc/passwd
-    fi
-    export USER="\${USER:-quantms}"
-    export LOGNAME="\${LOGNAME:-\${USER:-quantms}}"
-    export HOME="\${HOME:-\$PWD}"
-    export TORCHINDUCTOR_CACHE_DIR="\${TORCHINDUCTOR_CACHE_DIR:-\$PWD/.torchinductor_cache}"
-    mkdir -p "\$TORCHINDUCTOR_CACHE_DIR"
-
Evidence
The pipeline explicitly runs Docker containers as the host UID/GID, which commonly results in a UID
that is not present in the container’s /etc/passwd and may not have a valid/writable home directory.
After this PR, the rescoring processes execute rescoring ... directly without setting USER/HOME
(or a writable cache location). These processes are part of the main peptide DB search subworkflow
when rescoring/MS2 features are enabled, so failures would block that workflow branch.

nextflow.config[302-311]
modules/local/utils/msrescore_fine_tuning/main.nf[50-66]
modules/local/utils/msrescore_features/main.nf[93-108]
modules/local/utils/psm_clean/main.nf[24-31]
modules/local/utils/spectrum_features/main.nf[21-28]
subworkflows/local/peptide_database_search/main.nf[6-12]
subworkflows/local/peptide_database_search/main.nf[77-169]
modules/local/utils/msrescore_fine_tuning/meta.yml[1-3]

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 rescoring-related modules no longer set `USER`/`LOGNAME`/`HOME` (and previously prepared a writable cache dir). This is risky because the default `docker` profile runs containers with `-u $(id -u):$(id -g)`, which can yield an arbitrary UID inside the container with no passwd entry and/or no valid writable home directory.

### Issue Context
These processes (`MSRESCORE_FEATURES`, `MSRESCORE_FINE_TUNING`, `PSM_CLEAN`, `SPECTRUM_FEATURES`) are invoked from the peptide database search subworkflow when MS2 features/rescoring is enabled.

### Fix Focus Areas
- modules/local/utils/msrescore_features/main.nf[93-108]
- modules/local/utils/msrescore_fine_tuning/main.nf[50-66]
- modules/local/utils/psm_clean/main.nf[24-31]
- modules/local/utils/spectrum_features/main.nf[21-28]

Suggested implementation pattern (apply at the top of each script block, before calling `rescoring ...`):
- `export USER=&quot;${USER:-quantms}&quot;`
- `export LOGNAME=&quot;${LOGNAME:-$USER}&quot;`
- `export HOME=&quot;${HOME:-$PWD}&quot;`
- (optional) `export XDG_CACHE_HOME=&quot;${XDG_CACHE_HOME:-$PWD/.cache}&quot;; mkdir -p &quot;$XDG_CACHE_HOME&quot;`
- (optional, only if needed) `export TORCHINDUCTOR_CACHE_DIR=&quot;${TORCHINDUCTOR_CACHE_DIR:-$PWD/.torchinductor_cache}&quot;; mkdir -p &quot;$TORCHINDUCTOR_CACHE_DIR&quot;`

If the container image v0.0.15 now guarantees correct behavior without this bootstrap, add/ensure a CI test profile that runs these steps under arbitrary UID and document that assumption in-code to prevent future regressions.

ⓘ 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

@ypriverol ypriverol requested review from jpfeuffer and ypriverol March 3, 2026 07:39
@jpfeuffer
Copy link
Collaborator

Much better! Can you link the upstream fix PR please?

@daichengxin
Copy link
Collaborator Author

Fix PR bigbio/quantms-rescoring#64

@ypriverol
Copy link
Member

Also for tracing @daichengxin and @jpfeuffer, this PR tackles the following issue: #678

@ypriverol ypriverol linked an issue Mar 3, 2026 that may be closed by this pull request
@ypriverol ypriverol changed the title bump quantms-rescoring version bump quantms-rescoring version - fix bug with quantms-rescoring container permissions Mar 3, 2026
@ypriverol ypriverol merged commit 87d51ac into bigbio:dev Mar 3, 2026
56 of 57 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.

ms2rescore container and fine-tunning not working properly

3 participants