Skip to content

fix(MSstats+): Fix download analysis code#209

Merged
tonywu1999 merged 2 commits into
develfrom
fix-download-code
May 13, 2026
Merged

fix(MSstats+): Fix download analysis code#209
tonywu1999 merged 2 commits into
develfrom
fix-download-code

Conversation

@tonywu1999

@tonywu1999 tonywu1999 commented May 13, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

This PR fixes generation of the R code users download from MSstatsShiny so it correctly supports Spectronaut anomaly-score computation and makes the protein summarization method configurable. Generated reproducible code now conditionally includes anomaly-score processing (loading run-order, anomaly-model parameters, parallel/core settings) when enabled, and uses the UI-selected summarization method instead of a hardcoded "TMP".

Detailed Changes

  • R/utils.R — getDataCode()

    • For Spectronaut filetype (filetype == 'spec') the generated conversion call to SpectronauttoMSstatsFormat() is now conditional on calculate_anomaly_scores:
      • If true, generated code:
        • Loads a run-order CSV and assigns it to run_order_path.
        • Calls SpectronauttoMSstatsFormat(..., calculateAnomalyScores = TRUE, runOrder = run_order_path, anomalyModelFeatures = c("FG.ShapeQualityScore (MS2)", "FG.ShapeQualityScore (MS1)", "EGDeltaRT"), anomalyModelFeatureTemporal = c("mean_decrease", "mean_decrease", "dispersion_increase"), n_trees = 100, max_depth = "auto", numberOfCores = 1, ...)
      • If false, generated code retains the prior SpectronauttoMSstatsFormat() call without anomaly-score parameters.
  • R/utils.R — preprocessDataCode()

    • summaryMethod is now derived from qc_input$summaryMethod with fallback to "TMP" instead of being hardcoded.
    • When loadpage_input$calculate_anomaly_scores is true, appended generated plotting code calls MSstats::MSstatsQualityMetricsPlot() for metric = "AnomalyScores" with isPlotly = TRUE.
  • No exported function signatures were changed.

Unit Tests

  • tests/testthat/test-utils.R (updated/added):
    • Tests for getDataCode/getData generated output when anomaly scores enabled (mock_input_anomaly):
      • Asserts generated code includes calculateAnomalyScores = TRUE.
      • Asserts generated runOrder reference/path is present.
      • Asserts expected anomalyModelFeatures vector is present.
      • Asserts n_trees (100) is present.
    • Tests for getDataCode/getData when anomaly scores disabled (mock_input_no_anomaly):
      • Asserts anomaly-score-related parameters are not present in the generated output.
    • Note: preprocessDataCode() tests currently rely on stubbing getDataCode(); explicit assertions for the summaryMethod derivation and appended AnomalyScores plotting call are not deeply exercised in existing tests and may require additional unit tests to validate generated plotting code and summaryMethod propagation.

Coding Guidelines Observations

  • No public API or exported signatures changed.
  • Code follows existing project style and uses isTRUE() for boolean checks.
  • Generated-code construction uses nested paste()/string concatenation; readability could be improved by extracting helper builders or using templating, but this is a stylistic suggestion rather than a violation.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4c1eab22-6d47-4439-b810-8192e1bcfb78

📥 Commits

Reviewing files that changed from the base of the PR and between 56a039b and 93676a4.

📒 Files selected for processing (1)
  • R/utils.R
🚧 Files skipped from review as they are similar to previous changes (1)
  • R/utils.R

📝 Walkthrough

Walkthrough

The PR adds optional anomaly-score processing in Spectronaut conversion (run-order loading and anomaly parameters) and makes MSstats protein summarization method configurable; it also conditionally appends an AnomalyScores quality-metrics plot when enabled.

Changes

Anomaly-score integration across data conversion and preprocessing

Layer / File(s) Summary
Spectronaut anomaly-score processing in getDataCode
R/utils.R
When calculate_anomaly_scores is enabled, generated code loads run-order CSV and passes anomaly-score model, temporal, tree/depth, and numberOfCores settings into SpectronauttoMSstatsFormat. Without anomaly scoring, the prior conversion call is generated unchanged.
MSstats preprocessing with configurable summary and anomaly metrics
R/utils.R
preprocessDataCode() reads qc_input$summaryMethod (default "TMP") for the summaryMethod argument in MSstats::dataProcess. If loadpage_input$calculate_anomaly_scores is true, generated code appends MSstats::MSstatsQualityMetricsPlot(..., metric = "AnomalyScores", ...).

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit hops through code so spry,
Scores of anomalies now pass by,
Spectronaut hands data in tidy rows,
MSstats plots where the oddity shows,
Cheers from a bunny — swift and sly! 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'fix(MSstats+): Fix download analysis code' is vague and generic, using broad terms like 'download analysis code' that don't convey the specific nature of the changes (anomaly-score processing and summary method derivation). Revise the title to be more specific, such as 'fix: Handle anomaly-score processing and dynamic summary method in code generation' to clearly indicate the primary changes made.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-download-code

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@R/utils.R`:
- Around line 980-987: The else branch building the codes string uses hardcoded
SpectronauttoMSstatsFormat parameters (filter_with_Qvalue = TRUE, qvalue_cutoff
= 0.01, removeProtein_with1Feature = TRUE and fewMeasurements="remove") which
diverges from the if-branch and from getData() runtime behavior that use
input$q_val, input$q_cutoff, input$remove and do not include fewMeasurements;
update the else branch that appends to codes so it uses the same input variables
(input$q_val, input$q_cutoff, input$remove) and remove or align the
fewMeasurements argument to match the converter_args used in getData() and the
if branch, ensuring SpectronauttoMSstatsFormat invocations are consistent across
both branches.
- Around line 966-978: The anomalyModelFeatures vector passed into
SpectronauttoMSstatsFormat uses column names that violate MSstats+ requirements
(they start with "F." and include spaces/dots), so update the entries in
anomalyModelFeatures inside the call to SpectronauttoMSstatsFormat to use the
exact precursor-level column names present in your Spectronaut output and that
conform to MSstats+ naming (e.g., remove leading "F.", remove dots/spaces and
match parentheses style like FGShapeQualityScore(MS2) if that is the real
column), or replace them with other valid precursor-level feature column names;
verify the exact column names in your input data and ensure anomalyModelFeatures
contains those exact strings before calling SpectronauttoMSstatsFormat.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 237327e1-012d-4399-af22-b23eef618d72

📥 Commits

Reviewing files that changed from the base of the PR and between 2881aa4 and 56a039b.

📒 Files selected for processing (1)
  • R/utils.R

Comment thread R/utils.R
Comment thread R/utils.R
@tonywu1999 tonywu1999 merged commit ba5966d into devel May 13, 2026
2 checks passed
@tonywu1999 tonywu1999 deleted the fix-download-code branch May 13, 2026 20:46
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