feat(metamorpheus): Prepopulate modification ID as a dropdown for PTM conversion

[swaraj-neu]

Refactor Loadpage UI & Server to Read First 100 Rows Upon Upload and populate the modification ID dropdown

Motivation and Context

The Loadpage module required manual entry of Metamorpheus PTM modification-ID patterns, which is error-prone and reduces usability. This PR improves UX by reading the first 100 rows of an uploaded Metamorpheus ("meta") file when BIO == "PTM", extracting bracket-enclosed modification IDs from the sequence column, and prepopulating a modification-ID selector. If preview fails or no IDs are found, the UI falls back to a manual text input.

Detailed Changes

• R/module-loadpage-server.R

  • Added reactive state:
    • preview_data <- reactiveVal(NULL)
    • main_data_file <- reactive(...) to select the primary uploaded file input based on input$filetype and input$BIO (PTM-focused).
  • Added an observe() that, when input$filetype == "meta" and input$BIO == "PTM", reads up to the first 100 rows via data.table::fread(..., nrows = 100, header = TRUE). On read error, shows a Shiny warning notification and sets preview_data(NULL); otherwise stores the preview.
  • Introduced server-rendered UI outputs for modification-ID selection:
    • output$mod_id_meta_ui <- renderUI({ mods <- .extract_mod_ids_from_preview(preview_data()); create_meta_mod_id_selector(ns, mods) })
    • output$mod_id_meta_other_input <- renderUI({ textInput(ns("mod_id_meta_custom"), ...) }) shown when select == "other".
  • Replaced direct use of input$mod_id_meta in PTM processing with resolution via .resolve_mod_id(input$mod_id_meta_select, input$mod_id_meta_custom); added tryCatch to surface resolution errors and abort processing when needed.

• R/module-loadpage-ui.R

  • Replaced static textInput(ns("mod_id_meta"), ...) in create_ptm_uploads(ns) with uiOutput(ns("mod_id_meta_ui")), delegating rendering of the modification-ID field to server logic.
  • Added create_meta_mod_id_selector(ns, mod_choices = character(0)):
    • When mod_choices non-empty: renders selectizeInput(ns("mod_id_meta_select"), choices = c(mod_choices, "other")) plus uiOutput(ns("mod_id_meta_other_input")) for custom input when "other" selected.
    • When no mod_choices: renders fallback textInput(ns("mod_id_meta_custom"), ...).

• R/utils.R

  • Added .extract_mod_ids_from_preview(preview_df):
    • Detects a "Full Sequence"/"Full.Sequence"/"FullSequence" column (case-insensitive), extracts bracket-enclosed tokens (regex [[^\]]+]), unescapes and returns unique sorted tokens.
  • Added .resolve_mod_id(selected = NULL, custom = NULL):
    • If selected is present and not "other", returns selected with square brackets regex-escaped.
    • If selected == "other", uses custom (preserves already-escaped custom values or escapes brackets if needed).
    • Throws an error when neither selected nor custom provide a usable value.
  • Updated PTM getData and getDataCode paths to use .resolve_mod_id(...) for mod_ids embedding and added safer error handling and a placeholder fallback in emitted code generation.

• Other wiring

  • Removed client-side conditionalPanel for the "Other" custom input; server now renders that input via output$mod_id_meta_other_input.
  • main_data_file reactive currently targets Metamorpheus ("meta") PTM preview; notes remain for extending previews to other PTM sources.

Unit Tests Added or Modified

• tests/testthat/test-utils.R

  • Replaced mock_input$mod_id_meta with mock_input$mod_id_meta_select (unescaped bracketed ID) and mock_input$mod_id_meta_custom.
  • Added "MOD ID RESOLUTION TESTS" covering:
    • .resolve_mod_id() escapes dropdown-selected bracketed IDs.
    • Selecting "other" uses custom input and preserves or correctly escapes brackets.
    • Already-escaped custom values are preserved (no double-escape).
    • Partially-escaped custom values are normalized.
    • Errors when neither selected nor custom provided or when custom is empty with "other" selected.
  • Added tests for .extract_mod_ids_from_preview() to ensure extraction of multiple/consecutive bracketed tokens from preview sequences.

• tests/testthat/test-module-loadpage-ui.R

  • Updated tests to expect uiOutput(ns("mod_id_meta_ui")) instead of a static mod_id_meta input id.
  • Added tests verifying create_meta_mod_id_selector renders:
    • a selectizeInput (mod_id_meta_select) with provided mod labels plus "other" when mods available.
    • a custom text input (mod_id_meta_custom) when no mods are available.
  • Removed assertion for the literal "Modification IDs" label.

Coding Guidelines

• No coding guideline violations detected:
  • Reactive constructs (reactiveVal/reactive/observe) are used appropriately.
  • User-facing errors on preview read failure are surfaced via showNotification.
  • New helper functions are internal (dot-prefixed) and focused.
  • Tests were added/updated to cover extraction and resolution logic.

[coderabbitai]

📝 Walkthrough

Walkthrough

Reads first 100 rows of uploaded Metamorpheus files when filetype == "meta" and BIO == "PTM", extracts bracketed modification IDs, exposes a server-rendered mod-ID selector (dropdown + "Other" or free-text), and resolves/escapes the chosen mod ID for processing and generated code.

Changes

Cohort / File(s)	Summary
Server preview & UI binding R/module-loadpage-server.R	Adds preview_data and main_data_file reactives; observes input$filetype == "meta" & input$BIO == "PTM" to fread first 100 rows (handles errors), sets/clears preview_data, and provides output$mod_id_meta_ui and output$mod_id_meta_other_input for dynamic mod-ID inputs.
UI helper & placeholder R/module-loadpage-ui.R	Replaces static textInput(ns("mod_id_meta")) with uiOutput(ns("mod_id_meta_ui")); adds create_meta_mod_id_selector(ns, mod_choices = character(0)) that renders a selectizeInput(ns("mod_id_meta_select")) with an __other__ option and conditional custom text input, or a fallback free-text input when no choices exist.
Helpers & code generation R/utils.R	Adds .extract_mod_ids_from_preview(preview_df) to parse unescaped bracketed mod IDs from sequence columns and .resolve_mod_id(selected, custom) to select/normalize/escape the final mod ID; updates Metamorpheus PTM getData and getDataCode paths to use the resolved/escaped mod ID with error handling and fallback.
Tests tests/testthat/test-utils.R, tests/testthat/test-module-loadpage-ui.R	Replaces single mod_id_meta mock with mod_id_meta_select/mod_id_meta_custom; adds unit tests for .resolve_mod_id() and .extract_mod_ids_from_preview() (escaping, __other__ handling, errors, multiple bracketed tokens); updates UI tests to expect mod_id_meta_ui.

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as Shiny UI
    participant Server as Shiny Server
    participant Utils as Utils
    participant FS as File System

    User->>UI: set filetype="meta", BIO="PTM", choose file
    UI->>Server: input change triggers observer
    Server->>FS: read first 100 rows (data.table::fread)
    FS-->>Server: preview dataframe / error
    alt read success
        Server->>Utils: .extract_mod_ids_from_preview(preview_df)
        Utils-->>Server: mod ID list
        Server->>UI: render selectizeInput (IDs + "__other__") or textInput
        User->>UI: pick ID or choose "__other__" and enter custom
        UI->>Server: inputs selected/custom
        Server->>Utils: .resolve_mod_id(selected, custom)
        Utils-->>Server: escaped mod_id
        Server->>Server: embed escaped mod_id in generated code / processing
    else read failure
        Server->>UI: show warning notification, render fallback input
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Merge Metamorpheus PTM to MSstatsFormat Converter into Development #158: Modifies Metamorpheus PTM import/UI/code paths touching the same "meta" mod_id handling.
• Merge Metamorpheus to MSstatsFormat Converter into Development #154: Earlier Metamorpheus "meta" support that this change builds upon.

Suggested labels

Review effort 3/5

Suggested reviewers

• devonjkohler

Poem

🐇 I hopped through previews, counted brackets in rows,

Found mods in the sequences where curious code grows.
A dropdown or typing — whichever you pick,
I escape every bracket so regex stays slick.
Cheers from a rabbit for a tidy new trick! 🎉

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 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.
Title check	✅ Passed	The title clearly and specifically describes the main change: adding dropdown prepopulation of modification IDs for PTM conversion in Metamorpheus, which matches the core objective of reading uploaded files and prepopulating the modification ID selector.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/loadpage-enhancement

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

tests/testthat/test-utils.R (1)

1599-1630: Add one extractor-path regression case.

These cases cover .resolve_mod_id(), but the new dropdown population path still has no test. A Full.Sequence fixture with two bracketed annotations would protect the preview parser from returning combined spans.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/testthat/test-utils.R` around lines 1599 - 1630, Add a regression test
to exercises the extractor/dropdown population path (not just .resolve_mod_id):
create a Full.Sequence fixture containing two separate bracketed annotations
(e.g. "[A][B]"), call the same preview/extractor logic used to build the
dropdown (the preview parser path) and assert it returns two distinct bracketed
spans (not a merged span) and that selecting each yields the expected escaped
string via .resolve_mod_id; this ensures the preview parser and dropdown
population handle multiple bracketed annotations correctly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@R/module-loadpage-server.R`:
- Around line 53-92: The observer currently calls data.table::fread for every
uploaded file; restrict this so previews are only attempted for the Metamorpheus
PTM flow: check input$BIO and/or input$filetype before calling fread.
Concretely, inside the observe that uses main_data_file(), add a guard like
req(input$BIO == "PTM") or if (!(input$filetype %in% c(...ptm filetypes...))) {
preview_data(NULL); return() } so fread is only invoked when the PTM branch
applies; keep using preview_data(preview) and the existing error handler
unchanged. Ensure you reference main_data_file(), preview_data(), and the
observe block when making the change.

In `@R/utils.R`:
- Around line 15-16: The current extraction uses gregexpr with a lazy quantifier
which can collapse consecutive bracketed items; update the gregexpr pattern used
when building all_mods from sequences (the call paired with regmatches that
produces unique_mods) to use a bracket-aware pattern that matches an opening '['
then any number of characters that are not ']' and then a closing ']' (i.e.,
avoid ".*?" lazy quantifiers), leaving the regmatches/unlist/sort(unique(...))
pipeline intact so unique_mods is produced as before.

---

Nitpick comments:
In `@tests/testthat/test-utils.R`:
- Around line 1599-1630: Add a regression test to exercises the
extractor/dropdown population path (not just .resolve_mod_id): create a
Full.Sequence fixture containing two separate bracketed annotations (e.g.
"[A][B]"), call the same preview/extractor logic used to build the dropdown (the
preview parser path) and assert it returns two distinct bracketed spans (not a
merged span) and that selecting each yields the expected escaped string via
.resolve_mod_id; this ensures the preview parser and dropdown population handle
multiple bracketed annotations correctly.

🪄 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: 9d65ff4f-e41c-474b-a18b-8d623a3c7b1b

📥 Commits

Reviewing files that changed from the base of the PR and between 1c29944 and 904de5b.

📒 Files selected for processing (4)

• R/module-loadpage-server.R
• R/module-loadpage-ui.R
• R/utils.R
• tests/testthat/test-utils.R

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@R/module-loadpage-ui.R`:
- Around line 413-424: The UI currently auto-selects the first modification by
setting selected = choices[1] for the selectizeInput ns("mod_id_meta_select"),
which bypasses the "no selection" validation in R/utils.R:.resolve_mod_id();
change the selectizeInput to not preselect anything (set selected = NULL or
character(0)) and add a placeholder (via options = list(placeholder = "Select
modification or Other")) so users must explicitly choose and .resolve_mod_id()
can detect no-selection.

In `@R/utils.R`:
- Around line 31-35: The code incorrectly assumes both brackets are escaped by
only checking grepl("\\\\\\[", val), so partially escaped IDs like "\\[Phospho]"
slip through; update the branch in R/utils.R that handles 'custom' and 'val' to
verify escaping for both '[' and ']' (e.g., check both grepl("\\\\\\[", val) and
grepl("\\\\\\]", val)) or always run gsub("(\\[|\\])", "\\\\\\1", val) to
normalize both independently before returning; ensure you reference and modify
the existing use of grepl and gsub on 'val' so both brackets are consistently
escaped.
- Around line 985-987: getDataCode currently calls
.resolve_mod_id(input$mod_id_meta_select, input$mod_id_meta_custom) directly
which can throw when no mod is selected; wrap that call in a tryCatch (mirroring
getData's behavior) inside getDataCode so failures are caught, set a safe
fallback (e.g., NA or empty string) for code_mod_id and ensure the generated
code includes a validation-friendly comment or variable (so downstream
validation can show a message instead of crashing). Update references to
.resolve_mod_id and getDataCode to use the tryCatch wrapper and preserve
original behavior when resolution succeeds.

🪄 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: c8b52118-0064-4cf3-bef7-8afb84c255e2

📥 Commits

Reviewing files that changed from the base of the PR and between 87d2ae7 and 5f2faea.

📒 Files selected for processing (5)

• R/module-loadpage-server.R
• R/module-loadpage-ui.R
• R/utils.R
• tests/testthat/test-module-loadpage-ui.R
• tests/testthat/test-utils.R

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/testthat/test-module-loadpage-ui.R