Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ne discoverability Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When quantms runs with `mzml_statistics = false`, no mzML or ms_info
parquet files are produced, so `mzml_table` is empty when idXML parsing
begins. Two crashes resulted:
- `KeyError` in `IdXMLReader._parse_search_file` when writing search
engine counts into `mzml_table[ms_name]` for a key that was never
initialized.
- Potential `KeyError` in `QuantMSModule.parse_idxml` when iterating
`exp_design_runs` and accessing `mzml_table[spectrum_name]`.
Fix by calling `mzml_table.setdefault(ms_name, {})` before any write in
`_parse_search_file`, and guarding the `exp_design_runs` loop with an
`if spectrum_name in mzml_table` check.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Handle the case where exp_design_runs is None (neither experimental design nor SDRF file found): fall back to iterating mzml_table.keys() to preserve run order instead of crashing with TypeError. - Add regression tests for IdXMLReader with an initially-empty mzml_table (covers both Comet and MSGF idXML fixtures) to prevent the KeyError from reappearing. Addresses Copilot review comments on PR #664. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…zml-files fix: handle empty mzml_table when mzml_statistics is disabled
Contributor
📝 WalkthroughWalkthroughPR adds LLM-accessible documentation files (llms.txt, llms-full.txt, robots.txt), updates site metadata in HTML template and MkDocs configuration with Google Analytics, enhances project metadata in pyproject.toml, and fixes idXML parsing logic to properly initialize mzml_table and handle optional experimental design run ordering. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Documentation | 4 minor |
| Security | 4 high |
🟢 Metrics 0 complexity
Metric Results Complexity 0
TIP This summary will be updated as you push new changes. Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_quantms.py (1)
41-62: Consider adding one module-level regression for run ordering fallback.A focused test on
QuantMSModule.parse_idxml()withexp_design_runs=Noneand with explicit run ordering would lock in the behavior introduced in this PR at the module integration level.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_quantms.py` around lines 41 - 62, Add a module-level regression test that exercises QuantMSModule.parse_idxml() when exp_design_runs is None but an explicit run ordering is provided, to ensure the fallback ordering behavior is preserved; create a new test (similar location to test_idxml_reader_empty_mzml_table) that constructs a QuantMSModule instance with exp_design_runs=None and a controlled set of input idXML/paths or fixtures that specify run order, call parse_idxml() (or the public entry that triggers it), and assert that the resulting run list/order matches the explicit ordering and that required keys (e.g., expected_engine_key, "num_quant_psms", "num_quant_peps") are present for each run; reference the QuantMSModule.parse_idxml symbol to locate the implementation and reuse or adapt the idxml fixtures used by test_idxml_reader_empty_mzml_table.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/overrides/main.html`:
- Around line 3-29: The overridden Jinja block extrahead is replacing the parent
theme's head content; update the template to call {{ super() }} inside the {%
block extrahead %} so inherited analytics/metadata and other head scripts are
preserved (locate the {% block extrahead %} in the overrides/main.html and
insert a call to super() before or after your custom meta/script content).
In `@pmultiqc/modules/quantms/quantms.py`:
- Around line 1338-1343: The loop that reassigns keys (using spectrum_names from
self.exp_design_runs or mzml_table.keys()) doesn't reorder an OrderedDict when
it's the same instance; instead, create a new OrderedDict, iterate
spectrum_names and, for each name present in the incoming mzml_table, copy the
entry into the new OrderedDict, then set self.mzml_table to that new OrderedDict
(ensuring you reference mzml_table, self.mzml_table, and self.exp_design_runs in
the change).
---
Nitpick comments:
In `@tests/test_quantms.py`:
- Around line 41-62: Add a module-level regression test that exercises
QuantMSModule.parse_idxml() when exp_design_runs is None but an explicit run
ordering is provided, to ensure the fallback ordering behavior is preserved;
create a new test (similar location to test_idxml_reader_empty_mzml_table) that
constructs a QuantMSModule instance with exp_design_runs=None and a controlled
set of input idXML/paths or fixtures that specify run order, call parse_idxml()
(or the public entry that triggers it), and assert that the resulting run
list/order matches the explicit ordering and that required keys (e.g.,
expected_engine_key, "num_quant_psms", "num_quant_peps") are present for each
run; reference the QuantMSModule.parse_idxml symbol to locate the implementation
and reuse or adapt the idxml fixtures used by
test_idxml_reader_empty_mzml_table.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25df8771-f781-486e-8a10-df009e306573
⛔ Files ignored due to path filters (1)
docs/images/social-card.pngis excluded by!**/*.png
📒 Files selected for processing (9)
docs/llms-full.txtdocs/llms.txtdocs/overrides/main.htmldocs/robots.txtmkdocs.ymlpmultiqc/modules/common/ms/idxml.pypmultiqc/modules/quantms/quantms.pypyproject.tomltests/test_quantms.py
Comment on lines
+3
to
+29
| {% block extrahead %} | ||
| <meta property="og:image" content="https://pmultiqc.quantms.org/images/social-card.png"> | ||
| <script type="application/ld+json"> | ||
| { | ||
| "@context": "https://schema.org", | ||
| "@type": "SoftwareApplication", | ||
| "name": "pmultiqc", | ||
| "description": "MultiQC plugin for proteomics quality control. Generates interactive HTML reports from quantms and quantmsdiann pipeline output.", | ||
| "applicationCategory": "Scientific Software", | ||
| "applicationSubCategory": "Proteomics", | ||
| "operatingSystem": "Linux, macOS, Windows", | ||
| "url": "https://pmultiqc.quantms.org", | ||
| "downloadUrl": "https://pypi.org/project/pmultiqc/", | ||
| "license": "https://opensource.org/licenses/MIT", | ||
| "author": { | ||
| "@type": "Organization", | ||
| "name": "BigBio", | ||
| "url": "https://quantms.org" | ||
| }, | ||
| "citation": { | ||
| "@type": "ScholarlyArticle", | ||
| "name": "pmultiqc: An open-source, lightweight, and metadata-oriented QC reporting library for MS proteomics", | ||
| "url": "https://doi.org/10.1016/j.mcpro.2026.101530" | ||
| } | ||
| } | ||
| </script> | ||
| {% endblock %} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For MkDocs Material template overrides, should {% block extrahead %}include{{ super() }} to preserve parent-provided head metadata/scripts (including analytics integrations)?
💡 Result:
Yes. When overriding Material for MkDocs template blocks (including extrahead), include {{ super }} inside your {% block extrahead %} if you want to preserve any parent-provided head metadata/scripts; otherwise you may replace the block content entirely and accidentally drop integrations. Material’s own customization docs explicitly recommend using {{ super }} when you “intend to add something to a block rather than to replace it altogether,” and they list extrahead as an overrideable block. MkDocs’ theme customization docs also show that overriding a block without calling super replaces the parent block content, so super is the mechanism to retain it (e.g., for additional scripts).
Citations:
- 1: https://squidfunk.github.io/mkdocs-material/customization/
- 2: https://www.mkdocs.org/user-guide/customizing-your-theme/
Add {{ super() }} to preserve parent theme integrations.
Overriding {% block extrahead %} at line 3 without {{ super() }} will drop parent theme integrations (analytics, metadata scripts, and other head content). Material for MkDocs requires {{ super() }} to retain inherited head content.
🔧 Proposed fix
{% block extrahead %}
+{{ super() }}
<meta property="og:image" content="https://pmultiqc.quantms.org/images/social-card.png">
<script type="application/ld+json">
{📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {% block extrahead %} | |
| <meta property="og:image" content="https://pmultiqc.quantms.org/images/social-card.png"> | |
| <script type="application/ld+json"> | |
| { | |
| "@context": "https://schema.org", | |
| "@type": "SoftwareApplication", | |
| "name": "pmultiqc", | |
| "description": "MultiQC plugin for proteomics quality control. Generates interactive HTML reports from quantms and quantmsdiann pipeline output.", | |
| "applicationCategory": "Scientific Software", | |
| "applicationSubCategory": "Proteomics", | |
| "operatingSystem": "Linux, macOS, Windows", | |
| "url": "https://pmultiqc.quantms.org", | |
| "downloadUrl": "https://pypi.org/project/pmultiqc/", | |
| "license": "https://opensource.org/licenses/MIT", | |
| "author": { | |
| "@type": "Organization", | |
| "name": "BigBio", | |
| "url": "https://quantms.org" | |
| }, | |
| "citation": { | |
| "@type": "ScholarlyArticle", | |
| "name": "pmultiqc: An open-source, lightweight, and metadata-oriented QC reporting library for MS proteomics", | |
| "url": "https://doi.org/10.1016/j.mcpro.2026.101530" | |
| } | |
| } | |
| </script> | |
| {% endblock %} | |
| {% block extrahead %} | |
| {{ super() }} | |
| <meta property="og:image" content="https://pmultiqc.quantms.org/images/social-card.png"> | |
| <script type="application/ld+json"> | |
| { | |
| "@context": "https://schema.org", | |
| "@type": "SoftwareApplication", | |
| "name": "pmultiqc", | |
| "description": "MultiQC plugin for proteomics quality control. Generates interactive HTML reports from quantms and quantmsdiann pipeline output.", | |
| "applicationCategory": "Scientific Software", | |
| "applicationSubCategory": "Proteomics", | |
| "operatingSystem": "Linux, macOS, Windows", | |
| "url": "https://pmultiqc.quantms.org", | |
| "downloadUrl": "https://pypi.org/project/pmultiqc/", | |
| "license": "https://opensource.org/licenses/MIT", | |
| "author": { | |
| "@type": "Organization", | |
| "name": "BigBio", | |
| "url": "https://quantms.org" | |
| }, | |
| "citation": { | |
| "@type": "ScholarlyArticle", | |
| "name": "pmultiqc: An open-source, lightweight, and metadata-oriented QC reporting library for MS proteomics", | |
| "url": "https://doi.org/10.1016/j.mcpro.2026.101530" | |
| } | |
| } | |
| </script> | |
| {% endblock %} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/overrides/main.html` around lines 3 - 29, The overridden Jinja block
extrahead is replacing the parent theme's head content; update the template to
call {{ super() }} inside the {% block extrahead %} so inherited
analytics/metadata and other head scripts are preserved (locate the {% block
extrahead %} in the overrides/main.html and insert a call to super() before or
after your custom meta/script content).
Comment on lines
+1338
to
+1343
| # mass spectrum files sorted based on experimental file when available; | ||
| # otherwise preserve the mzml_table iteration order. | ||
| spectrum_names = self.exp_design_runs if self.exp_design_runs is not None else mzml_table.keys() | ||
| for spectrum_name in spectrum_names: | ||
| if spectrum_name in mzml_table: | ||
| self.mzml_table[spectrum_name] = mzml_table[spectrum_name] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Verify call site passes self.mzml_table into parse_idxml:"
rg -n -C2 'parse_idxml\(self\.mzml_table\)' pmultiqc/modules/quantms/quantms.py
echo
echo "2) Verify current reindex block:"
rg -n -C4 'spectrum_names = self\.exp_design_runs if self\.exp_design_runs is not None else mzml_table\.keys\(\)' pmultiqc/modules/quantms/quantms.py
echo
echo "3) Demonstrate reassignment does not reorder keys:"
python - <<'PY'
from collections import OrderedDict
od = OrderedDict([("run_b", {}), ("run_a", {})])
for k in ["run_a", "run_b"]:
od[k] = od[k]
print("Observed order:", list(od.keys()))
print("Expected if reordered:", ["run_a", "run_b"])
PYRepository: bigbio/pmultiqc
Length of output: 963
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Find method containing lines 1338-1343 ==="
sed -n '1320,1345p' pmultiqc/modules/quantms/quantms.py | head -30
echo
echo "=== 2) Find method definition context ==="
sed -n '1300,1350p' pmultiqc/modules/quantms/quantms.py | grep -E '^\s*(def |class )' -A1
echo
echo "=== 3) Check what data type mzml_table is initialized as ==="
rg -n 'self\.mzml_table\s*=' pmultiqc/modules/quantms/quantms.py | head -5
echo
echo "=== 4) Check imports to see if OrderedDict is used ==="
head -50 pmultiqc/modules/quantms/quantms.py | grep -i 'import\|from'
echo
echo "=== 5) Full context of lines 1330-1350 ==="
sed -n '1330,1350p' pmultiqc/modules/quantms/quantms.pyRepository: bigbio/pmultiqc
Length of output: 3308
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if mzml_table is populated before parse_idxml call ==="
grep -n 'self\.mzml_table\[' pmultiqc/modules/quantms/quantms.py | head -10
echo
echo "=== Confirm alias issue: parameter receives self.mzml_table ==="
sed -n '395,410p' pmultiqc/modules/quantms/quantms.pyRepository: bigbio/pmultiqc
Length of output: 814
Reordering via key reassignment does not work on OrderedDict.
At line 1343, reassigning existing keys on self.mzml_table will not reorder them in insertion order. Since mzml_table is the same OrderedDict instance passed as a parameter, the intended reordering based on exp_design_runs is lost. This leaves the runs in their original order regardless of the loop.
🔧 Proposed fix (build a new OrderedDict in desired order)
- # mass spectrum files sorted based on experimental file when available;
- # otherwise preserve the mzml_table iteration order.
- spectrum_names = self.exp_design_runs if self.exp_design_runs is not None else mzml_table.keys()
- for spectrum_name in spectrum_names:
- if spectrum_name in mzml_table:
- self.mzml_table[spectrum_name] = mzml_table[spectrum_name]
+ # mass spectrum files sorted based on experimental file when available;
+ # otherwise preserve the mzml_table iteration order.
+ spectrum_names = self.exp_design_runs if self.exp_design_runs is not None else mzml_table.keys()
+ reordered = OrderedDict()
+ for spectrum_name in spectrum_names:
+ if spectrum_name in mzml_table:
+ reordered[spectrum_name] = mzml_table[spectrum_name]
+ # keep any runs not present in exp_design_runs at the end
+ for spectrum_name, run_data in mzml_table.items():
+ reordered.setdefault(spectrum_name, run_data)
+ self.mzml_table = reordered🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmultiqc/modules/quantms/quantms.py` around lines 1338 - 1343, The loop that
reassigns keys (using spectrum_names from self.exp_design_runs or
mzml_table.keys()) doesn't reorder an OrderedDict when it's the same instance;
instead, create a new OrderedDict, iterate spectrum_names and, for each name
present in the incoming mzml_table, copy the entry into the new OrderedDict,
then set self.mzml_table to that new OrderedDict (ensuring you reference
mzml_table, self.mzml_table, and self.exp_design_runs in the change).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
Brief description of the changes made in this PR.
Type of Change
Summary by CodeRabbit
Release Notes
Documentation
Bug Fixes
SEO & Analytics
Tests