Skip to content

feat(network-interpretation): Add button to download network analysis code#111

Merged
tonywu1999 merged 7 commits intodevelfrom
export-code
Aug 26, 2025
Merged

feat(network-interpretation): Add button to download network analysis code#111
tonywu1999 merged 7 commits intodevelfrom
export-code

Conversation

@tonywu1999
Copy link
Copy Markdown
Contributor

@tonywu1999 tonywu1999 commented Aug 12, 2025

PR Type

Enhancement


Description

  • Add code generation for network analysis

  • Provide downloadable R script via button

  • Include UI box for download control

  • Integrate display label options in exports


Diagram Walkthrough

flowchart LR
  UI["UI: Code Download Box"] -- triggers --> BTN["Download Button"]
  BTN -- click --> DH["downloadHandler: network_download_code"]
  OBS["Observer: showNetwork"] -- renders --> BTN
  GEN["generate_network_code()"] -- returns --> DH
  GEN -- uses --> PARAMS["getInputParameters + inputs"]
  GEN -- builds --> SCRIPT["R script for analysis"]
Loading

File Walkthrough

Relevant files
Enhancement
module-visualize-network-server.R
Server-side code generation and download hook                       

R/module-visualize-network-server.R

  • Generate R code based on current inputs
  • Add downloadHandler to export code file
  • Render download button after network shown
  • Include preview/export calls with label types
+87/-0   
module-visualize-network-ui.R
UI container for code download button                                       

R/module-visualize-network-ui.R

  • Add code download UI box
  • Place download control above visualization
  • Wire uiOutput for dynamic button
+8/-0     

Summary by CodeRabbit

  • New Features

    • Added a “Download analysis code” option producing a dated R script that reproduces the current network analysis, saves node/edge CSVs, and exports an HTML preview.
  • UI

    • Added a code-download box in Network Settings.
    • Clarified file-upload instructions and relabeled controls to reference INDRA (Evidence, Statement Types, Sources).
  • UX

    • Download button appears after processing; generated script reflects current UI selections.
  • Refactor

    • Network visualization now uses node/edge table inputs and supports a selectable display label type.
  • Tests / Documentation

    • Simplified tests and updated documentation to reflect the new display-label parameter and refactored visualization path.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 12, 2025

Walkthrough

Removes in-file Cytoscape helper/rendering functions and shifts to using package-level Cytoscape config generation; adds a display_label_type parameter for label selection; implements reproducible R script generation and download UI; updates related UI text and NAMESPACE imports; deletes some Rd docs and simplifies tests.

Changes

Cohort / File(s) Summary of Changes
Server: Cytoscape & code export
R/module-visualize-network-server.R
Removes local Cytoscape helper/rendering functions; updates generateCytoscapeJSForShiny() signature to accept display_label_type; changes internal call sites to pass node/edge tables and display_label_type; adds reactive generate_network_code, renders output$network.code.button, and output$network_download_code downloadHandler.
UI: settings, labels & code button
R/module-visualize-network-ui.R
Adds createCodeDownloadBox(ns) and integrates uiOutput(ns("network.code.button")) into createNetworkSettingsTab; updates upload label/tooltip and several labels to reference INDRA (evidence, statement types, sources).
Namespace imports
NAMESPACE
Adds importFrom(MSstatsBioNet, generateCytoscapeConfig); removes grDevices::colorRamp and grDevices::rgb imports.
Documentation: man pages removed/updated
man/generateCytoscapeConfig.Rd, man/generateJavaScriptCode.Rd, man/generateCytoscapeJSForShiny.Rd
Deletes Rd files for generateCytoscapeConfig and generateJavaScriptCode; updates generateCytoscapeJSForShiny.Rd to document new display_label_type argument.
Tests: network visualization
tests/testthat/test_network_visualization.R
Removes many unit tests tied to removed helper functions; updates tests to call generateCytoscapeJSForShiny(nodes, edges) (raw tables) and simplifies full-pipeline tests accordingly.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant UI as Network Settings UI
  participant S as visualizeNetworkServer
  participant MS as MSstatsBioNet (pkg)
  participant DL as downloadHandler

  U->>UI: Click "Show Network" (input$showNetwork)
  UI->>S: provide current inputs (filters, cutoffs, displayLabelType)
  S->>MS: getSubnetworkFromIndra(...) / annotate (uses inputs)
  MS-->>S: nodes_table, edges_table
  S->>S: generate_network_code() builds parameterized R script
  S->>UI: render download button (output$network.code.button)

  U->>UI: Click "Download analysis code"
  UI->>DL: request file
  DL->>S: fetch script string
  S-->>DL: return script
  DL-->>U: save network-analysis-code-YYYY-MM-DD.R
Loading
sequenceDiagram
  participant S as visualizeNetworkServer
  participant MS as MSstatsBioNet (pkg)
  participant JS as generateCytoscapeJSForShiny

  S->>MS: produce nodes_table, edges_table
  S->>JS: generateCytoscapeJSForShiny(nodes_table, edges_table, display_label_type)
  JS-->>S: js_code (Cytoscape init + Shiny handlers)
  S->>UI: inject js_code into UI container (session$ns("cy"))
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • Add INDRA integration to MSstatsShiny #110 — Prior refactor touching module-visualize-network-server.R and the same Cytoscape/generation functions; strongly related to the decoupling and API changes in this PR.

Poem

I nibble through inputs, stitch them neat,
A reproducible script hops out to greet.
Click my button, watch your analysis flee,
Nodes, edges, labels—bundled for thee.
Hop! Code delivered, carrot and feat. 🥕

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch export-code

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

Using input$displayLabelType directly in code generation while earlier parameters are sourced via getInputParameters may cause inconsistency or null issues. Consider sourcing display label type from the same params object for stability and reproducibility.

displayLabelTypeStr <- paste0("\"", paste(input$displayLabelType, collapse = "\", \""), "\"")
codes <- paste(codes, "previewNetworkInBrowser(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
codes <- paste(codes, "exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
Robustness

The generated script writes hard-coded read.csv('path/to/your/data.csv') which may confuse users. Consider parameterizing or commenting clearer placeholders and validating presence of required packages in the generated code.

codes <- paste(codes, "df <- read.csv(\"path/to/your/data.csv\")\n\n", sep = "")
Quoting/Sanitization

Directly interpolating params (labels, sources, protein IDs, etc.) into code strings without escaping quotes can break the generated R script when values contain quotes or special characters. Add proper escaping (e.g., use shQuote or gsub to escape) before paste.

if (params$selectedLabel != "" && !is.null(params$selectedLabel)) {
  codes <- paste(codes, "# Filter by selected comparison\n", sep = "")
  codes <- paste(codes, "filtered_df <- df[df$Label == \"", params$selectedLabel, "\" & !is.na(df$Label), ]\n\n", sep = "")
} else {
  codes <- paste(codes, "filtered_df <- df\n\n", sep = "")
}

codes <- paste(codes, "# Annotate protein information\n", sep = "")
codes <- paste(codes, "annotated_df <- annotateProteinInfoFromIndra(filtered_df, \"", params$proteinIdType, "\")\n\n", sep = "")

codes <- paste(codes, "# Extract subnetwork with filtering parameters\n", sep = "")
codes <- paste(codes, "subnetwork <- getSubnetworkFromIndra(\n", sep = "")
codes <- paste(codes, "  annotated_df,\n", sep = "")
codes <- paste(codes, "  pvalueCutoff = ", params$pValue, ",\n", sep = "")
codes <- paste(codes, "  evidence_count_cutoff = ", params$evidence, ",\n", sep = "")

# Handle statement types
if (is.null(params$statementTypes)) {
  codes <- paste(codes, "  statement_types = NULL,\n", sep = "")
} else {
  statement_types_str <- paste0("c(\"", paste(params$statementTypes, collapse = "\", \""), "\")")
  codes <- paste(codes, "  statement_types = ", statement_types_str, ",\n", sep = "")
}

# Handle sources
if (is.null(params$sources)) {
  codes <- paste(codes, "  sources_filter = NULL,\n", sep = "")
} else {
  sources_str <- paste0("c(\"", paste(params$sources, collapse = "\", \""), "\")")
  codes <- paste(codes, "  sources_filter = ", sources_str, ",\n", sep = "")
}

codes <- paste(codes, "  logfc_cutoff = ", params$absLogFC, sep = "")

# Handle selected proteins
if (!is.null(params$selectedProteins) && length(params$selectedProteins) > 0) {
  selected_proteins_str <- paste0("c(\"", paste(params$selectedProteins, collapse = "\", \""), "\")")
  codes <- paste(codes, ",\n  force_include_proteins = ", selected_proteins_str, "\n", sep = "")

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix vector argument construction

If input$displayLabelType is length > 1, the generated code becomes syntactically
invalid (multiple quoted values without c()). Wrap it as a vector literal when
needed. Also handle NULL to avoid "character(0)" constructing malformed code.

R/module-visualize-network-server.R [924-926]

-displayLabelTypeStr <- paste0("\"", paste(input$displayLabelType, collapse = "\", \""), "\"")
-codes <- paste(codes, "previewNetworkInBrowser(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
-codes <- paste(codes, "exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
+if (is.null(input$displayLabelType) || length(input$displayLabelType) == 0) {
+  displayLabelTypeStr <- "NULL"
+} else if (length(input$displayLabelType) == 1) {
+  displayLabelTypeStr <- paste0("\"", input$displayLabelType, "\"")
+} else {
+  displayLabelTypeStr <- paste0("c(\"", paste(input$displayLabelType, collapse = "\", \""), "\")")
+}
+codes <- paste(codes, "previewNetworkInBrowser(subnetwork$nodes, subnetwork$edges, displayLabelType = ", displayLabelTypeStr, ")\n", sep = "")
+codes <- paste(codes, "exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType = ", displayLabelTypeStr, ")\n", sep = "")
Suggestion importance[1-10]: 8

__

Why: The current construction produces invalid R code when multiple display labels are selected and doesn't handle NULL; the proposed fix correctly builds a scalar, vector, or NULL literal, improving robustness of generated code.

Medium
Prevent NULL check ordering bug

Accessing params$selectedLabel before checking for NULL can error if it is NULL due
to short-circuiting in R. Reorder the condition to check NULL first and ensure the
generated code quotes safely even when label contains quotes.

R/module-visualize-network-server.R [870-875]

-if (params$selectedLabel != "" && !is.null(params$selectedLabel)) {
+if (!is.null(params$selectedLabel) && params$selectedLabel != "") {
+  safe_label <- gsub("\"", "\\\\\"", params$selectedLabel)
   codes <- paste(codes, "# Filter by selected comparison\n", sep = "")
-  codes <- paste(codes, "filtered_df <- df[df$Label == \"", params$selectedLabel, "\" & !is.na(df$Label), ]\n\n", sep = "")
+  codes <- paste(codes, "filtered_df <- df[df$Label == \"", safe_label, "\" & !is.na(df$Label), ]\n\n", sep = "")
 } else {
   codes <- paste(codes, "filtered_df <- df\n\n", sep = "")
 }
Suggestion importance[1-10]: 5

__

Why: Reordering the NULL check is a safe improvement and escaping quotes in labels prevents malformed code; impact is moderate since many labels won't contain quotes.

Low
General
Qualify icon creation

downloadButton’s icon argument expects shiny’s icon object; without namespace
qualification it may resolve incorrectly within modules. Use shiny::icon(...) to
avoid masking or missing reference issues in modular contexts.

R/module-visualize-network-server.R [959-963]

 output$network.code.button <- renderUI({
   ns <- session$ns
-  downloadButton(ns("network_download_code"), "Download analysis code", icon("download"),
-                 style="color: #000000; background-color: #75ba82; border-color: #000000")
+  downloadButton(
+    ns("network_download_code"),
+    "Download analysis code",
+    shiny::icon("download"),
+    style = "color: #000000; background-color: #75ba82; border-color: #000000"
+  )
 })
Suggestion importance[1-10]: 4

__

Why: Namespacing icon to shiny::icon avoids potential masking issues in modules; it's a minor but correct improvement with limited impact.

Low

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
R/module-visualize-network-ui.R (1)

491-496: Consider adding error handling for missing UI output.

The function creates a download button container, but there's no guarantee that network.code.button will be populated if the network generation fails or is never triggered.

Consider adding a placeholder or conditional rendering to handle cases where the button is not yet available:

 createCodeDownloadBox <- function(ns) {
   div(
     style = "text-align: right; padding: 10px;",
-    uiOutput(ns("network.code.button"))
+    conditionalPanel(
+      condition = paste0("output['", ns("network.code.button"), "'] != null"),
+      uiOutput(ns("network.code.button"))
+    )
   )
 }
R/module-visualize-network-server.R (1)

889-892: Simplify vector construction for statement types.

The current approach creates the R vector literal correctly but could be more maintainable.

Consider using deparse() for safer R code generation:

   # Handle statement types
   if (is.null(params$statementTypes)) {
     codes <- paste(codes, "  statement_types = NULL,\n", sep = "")
   } else {
-    statement_types_str <- paste0("c(\"", paste(params$statementTypes, collapse = "\", \""), "\")")
+    statement_types_str <- deparse(params$statementTypes)
     codes <- paste(codes, "  statement_types = ", statement_types_str, ",\n", sep = "")
   }

This approach would automatically handle special characters and ensure valid R syntax.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be57bd5 and 8b99c01.

📒 Files selected for processing (2)
  • R/module-visualize-network-server.R (2 hunks)
  • R/module-visualize-network-ui.R (2 hunks)
🔇 Additional comments (2)
R/module-visualize-network-ui.R (1)

455-455: LGTM! Clean integration of the code download feature.

The placement of the code download box at the top of the right column provides good visibility and accessibility for users.

R/module-visualize-network-server.R (1)

959-963: LGTM! Well-styled download button with clear labeling.

The download button is properly namespaced and styled consistently with the application's theme.

Comment on lines +859 to +929
generate_network_code <- eventReactive(input$showNetwork, {
params <- getInputParameters(input)

codes <- ""
codes <- paste(codes, "\n# Load Required Packages\n", sep = "")
codes <- paste(codes, "library(MSstatsBioNet)\nlibrary(dplyr)\n\n", sep = "")

codes <- paste(codes, "# Read data\n", sep = "")
codes <- paste(codes, "df <- read.csv(\"path/to/your/data.csv\")\n\n", sep = "")

# Add label filtering if not default
if (params$selectedLabel != "" && !is.null(params$selectedLabel)) {
codes <- paste(codes, "# Filter by selected comparison\n", sep = "")
codes <- paste(codes, "filtered_df <- df[df$Label == \"", params$selectedLabel, "\" & !is.na(df$Label), ]\n\n", sep = "")
} else {
codes <- paste(codes, "filtered_df <- df\n\n", sep = "")
}

codes <- paste(codes, "# Annotate protein information\n", sep = "")
codes <- paste(codes, "annotated_df <- annotateProteinInfoFromIndra(filtered_df, \"", params$proteinIdType, "\")\n\n", sep = "")

codes <- paste(codes, "# Extract subnetwork with filtering parameters\n", sep = "")
codes <- paste(codes, "subnetwork <- getSubnetworkFromIndra(\n", sep = "")
codes <- paste(codes, " annotated_df,\n", sep = "")
codes <- paste(codes, " pvalueCutoff = ", params$pValue, ",\n", sep = "")
codes <- paste(codes, " evidence_count_cutoff = ", params$evidence, ",\n", sep = "")

# Handle statement types
if (is.null(params$statementTypes)) {
codes <- paste(codes, " statement_types = NULL,\n", sep = "")
} else {
statement_types_str <- paste0("c(\"", paste(params$statementTypes, collapse = "\", \""), "\")")
codes <- paste(codes, " statement_types = ", statement_types_str, ",\n", sep = "")
}

# Handle sources
if (is.null(params$sources)) {
codes <- paste(codes, " sources_filter = NULL,\n", sep = "")
} else {
sources_str <- paste0("c(\"", paste(params$sources, collapse = "\", \""), "\")")
codes <- paste(codes, " sources_filter = ", sources_str, ",\n", sep = "")
}

codes <- paste(codes, " logfc_cutoff = ", params$absLogFC, sep = "")

# Handle selected proteins
if (!is.null(params$selectedProteins) && length(params$selectedProteins) > 0) {
selected_proteins_str <- paste0("c(\"", paste(params$selectedProteins, collapse = "\", \""), "\")")
codes <- paste(codes, ",\n force_include_proteins = ", selected_proteins_str, "\n", sep = "")
} else {
codes <- paste(codes, ",\n force_include_proteins = NULL\n", sep = "")
}

codes <- paste(codes, ")\n\n", sep = "")

codes <- paste(codes, "# View network components\n", sep = "")
codes <- paste(codes, "print(\"Nodes in network:\")\n", sep = "")
codes <- paste(codes, "print(subnetwork$nodes)\n\n", sep = "")
codes <- paste(codes, "print(\"Edges in network:\")\n", sep = "")
codes <- paste(codes, "print(subnetwork$edges)\n\n", sep = "")

codes <- paste(codes, "# Save results\n", sep = "")
codes <- paste(codes, "write.csv(subnetwork$nodes, \"network_nodes.csv\", row.names = FALSE)\n", sep = "")
codes <- paste(codes, "write.csv(subnetwork$edges, \"network_edges.csv\", row.names = FALSE)\n", sep = "")
codes <- paste(codes, "# Visualize network on web browser and export as an HTML file\n", sep = "")
displayLabelTypeStr <- paste0("\"", paste(input$displayLabelType, collapse = "\", \""), "\"")
codes <- paste(codes, "previewNetworkInBrowser(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
codes <- paste(codes, "exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")

return(codes)
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add input validation and error handling for code generation.

The generate_network_code function concatenates strings without validating inputs that could contain special characters or quotes, which might break the generated R code.

Apply input sanitization for string parameters that will be embedded in the generated code:

 generate_network_code <- eventReactive(input$showNetwork, {
   params <- getInputParameters(input)
   
+  # Helper function to escape quotes in strings
+  escape_quotes <- function(x) {
+    if (is.null(x) || length(x) == 0) return(NULL)
+    gsub("\"", "\\\"", x, fixed = TRUE)
+  }
+  
   codes <- ""
   codes <- paste(codes, "\n# Load Required Packages\n", sep = "")
   codes <- paste(codes, "library(MSstatsBioNet)\nlibrary(dplyr)\n\n", sep = "")
   
   codes <- paste(codes, "# Read data\n", sep = "")
   codes <- paste(codes, "df <- read.csv(\"path/to/your/data.csv\")\n\n", sep = "")
   
   # Add label filtering if not default
   if (params$selectedLabel != "" && !is.null(params$selectedLabel)) {
     codes <- paste(codes, "# Filter by selected comparison\n", sep = "")
-    codes <- paste(codes, "filtered_df <- df[df$Label == \"", params$selectedLabel, "\" & !is.na(df$Label), ]\n\n", sep = "")
+    codes <- paste(codes, "filtered_df <- df[df$Label == \"", escape_quotes(params$selectedLabel), "\" & !is.na(df$Label), ]\n\n", sep = "")
   } else {
     codes <- paste(codes, "filtered_df <- df\n\n", sep = "")
   }

Also, escape quotes in other string parameters:

  • Line 878: params$proteinIdType
  • Line 890: params$statementTypes elements
  • Line 898: params$sources elements
  • Line 906: params$selectedProteins elements
🤖 Prompt for AI Agents
In R/module-visualize-network-server.R around lines 859-929, the generated code
embeds raw string inputs which can contain quotes or backslashes and break the
produced R code; sanitize all string inputs before embedding by replacing
backslashes and double quotes (e.g., sanitized <- gsub('(["\\\\])', '\\\\\\1',
as.character(x))) and handle NULL/NA by converting to NULL or "" as appropriate;
then use these sanitized values when building the code strings for
params$proteinIdType (line ~878), each element of params$statementTypes (~890),
params$sources (~898), and params$selectedProteins (~906) so the paste calls
insert the escaped strings (wrap in quotes after escaping) and avoid injecting
unescaped values into the generated R code.

Comment thread R/module-visualize-network-server.R
Comment thread R/module-visualize-network-server.R
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
R/module-visualize-network-ui.R (4)

140-149: Evidence should be an integer count; add step = 1 to the slider

Prevents fractional evidence values from entering the pipeline and exported code.

Apply this diff:

-      sliderInput(ns("evidence"), 
-                  label = NULL,  # Remove since we're handling it above
-                  min = 0, max = 50, value = 5)
+      sliderInput(ns("evidence"), 
+                  label = NULL,  # Remove since we're handling it above
+                  min = 0, max = 50, value = 5, step = 1)

157-197: Ensure values match INDRA statement types and handle 'all' sentinel consistently

UI labels look fine. Please verify the server maps these choice values as-is (case-sensitive) and that selecting "all" is handled unambiguously (e.g., ignored if any specific type is chosen, or treated as a true “select all”).

To reduce duplication and drift between UI/server/exported code, consider centralizing these choices:

-      selectInput(ns("statementTypes"),
-                  label = NULL,  # Remove since we're handling it above
-                  choices = list("All Types" = "all",
-                                 "Complex" = "Complex",
-                                 "Inhibition" = "Inhibition", 
-                                 "Activation" = "Activation",
-                                 "Increase Amount" = "IncreaseAmount",
-                                 "Decrease Amount" = "DecreaseAmount",
-                                 "Phosphorylation" = "Phosphorylation",
-                                 "Dephosphorylation" = "Dephosphorylation",
-                                 "Ubiquitination" = "Ubiquitination",
-                                 "Deubiquitination" = "Deubiquitination",
-                                 "Sumoylation" = "Sumoylation",
-                                 "Desumoylation" = "Desumoylation",
-                                 "Hydroxylation" = "Hydroxylation",
-                                 "Dehydroxylation" = "Dehydroxylation",
-                                 "Acetylation" = "Acetylation",
-                                 "Deacetylation" = "Deacetylation",
-                                 "Glycosylation" = "Glycosylation",
-                                 "Deglycosylation" = "Deglycosylation",
-                                 "Farnesylation" = "Farnesylation",
-                                 "Defarnesylation" = "Defarnesylation",
-                                 "Geranylgeranylation" = "Geranylgeranylation",
-                                 "Degeranylgeranylation" = "Degeranylgeranylation",
-                                 "Palmitoylation" = "Palmitoylation",
-                                 "Depalmitoylation" = "Depalmitoylation",
-                                 "Myristoylation" = "Myristoylation",
-                                 "Demyristoylation" = "Demyristoylation",
-                                 "Ribosylation" = "Ribosylation",
-                                 "Deribosylation" = "Deribosylation",
-                                 "Methylation" = "Methylation",
-                                 "Demethylation" = "Demethylation"),
-                  selected = "all",
-                  multiple = TRUE)
+      selectInput(ns("statementTypes"),
+                  label = NULL,  # Remove since we're handling it above
+                  choices = listIndraStatementTypes(),
+                  selected = "all",
+                  multiple = TRUE)

Place this helper alongside other UI helpers:

listIndraStatementTypes <- function() {
  list(
    "All Types" = "all",
    "Complex" = "Complex",
    "Inhibition" = "Inhibition",
    "Activation" = "Activation",
    "Increase Amount" = "IncreaseAmount",
    "Decrease Amount" = "DecreaseAmount",
    "Phosphorylation" = "Phosphorylation",
    "Dephosphorylation" = "Dephosphorylation",
    "Ubiquitination" = "Ubiquitination",
    "Deubiquitination" = "Deubiquitination",
    "Sumoylation" = "Sumoylation",
    "Desumoylation" = "Desumoylation",
    "Hydroxylation" = "Hydroxylation",
    "Dehydroxylation" = "Dehydroxylation",
    "Acetylation" = "Acetylation",
    "Deacetylation" = "Deacetylation",
    "Glycosylation" = "Glycosylation",
    "Deglycosylation" = "Deglycosylation",
    "Farnesylation" = "Farnesylation",
    "Defarnesylation" = "Defarnesylation",
    "Geranylgeranylation" = "Geranylgeranylation",
    "Degeranylgeranylation" = "Degeranylgeranylation",
    "Palmitoylation" = "Palmitoylation",
    "Depalmitoylation" = "Depalmitoylation",
    "Myristoylation" = "Myristoylation",
    "Demyristoylation" = "Demyristoylation",
    "Ribosylation" = "Ribosylation",
    "Deribosylation" = "Deribosylation",
    "Methylation" = "Methylation",
    "Demethylation" = "Demethylation"
  )
}

200-243: Source values look comprehensive; de-duplicate via a shared helper and confirm 'all' handling

Good coverage of text-mined and curated sources. To keep UI, server filters, and exported code in sync, centralize the choices. Also verify the server treats "all" as a sentinel consistently with statement types.

Replace the inline choices with a helper:

-      selectInput(ns("sources"),
-                  label = NULL,  # Remove since we're handling it above
-                  choices = list("All Sources" = "all",
-                                 "Text Mining Systems" = list(
-                                   "REACH" = "reach",
-                                   "TRIPS/DRUM" = "trips",
-                                   "Sparser" = "sparser",
-                                   "Eidos" = "eidos",
-                                   "TEES" = "tees",
-                                   "MedScan" = "medscan",
-                                   "RLIMS-P" = "rlimsp",
-                                   "ISI/AMR" = "isi",
-                                   "Geneways" = "geneways",
-                                   "GNBR" = "gnbr",
-                                   "SemRep" = "semrep"
-                                 ),
-                                 "Curated Databases" = list(
-                                   "BEL" = "bel",
-                                   "BioPAX" = "biopax",
-                                   "SIGNOR" = "signor",
-                                   "BioGRID" = "biogrid",
-                                   "HPRD" = "hprd",
-                                   "TRRUST" = "trrust",
-                                   "PhosphoELM" = "phosphoelm",
-                                   "VirHostNet" = "virhostnet",
-                                   "OmniPath" = "omnipath",
-                                   "UbiBrowser" = "ubibrowser",
-                                   "ACSN" = "acsn",
-                                   "WormBase" = "wormbase",
-                                   "CTD" = "ctd",
-                                   "DrugBank" = "drugbank",
-                                   "DGI" = "dgi",
-                                   "TAS" = "tas",
-                                   "CROG" = "crog",
-                                   "CREEDS" = "creeds"
-                                 )),
-                  selected = "all",
-                  multiple = TRUE)
+      selectInput(ns("sources"),
+                  label = NULL,  # Remove since we're handling it above
+                  choices = listIndraSources(),
+                  selected = "all",
+                  multiple = TRUE)

Add this helper near the other UI helpers:

listIndraSources <- function() {
  list(
    "All Sources" = "all",
    "Text Mining Systems" = list(
      "REACH" = "reach",
      "TRIPS/DRUM" = "trips",
      "Sparser" = "sparser",
      "Eidos" = "eidos",
      "TEES" = "tees",
      "MedScan" = "medscan",
      "RLIMS-P" = "rlimsp",
      "ISI/AMR" = "isi",
      "Geneways" = "geneways",
      "GNBR" = "gnbr",
      "SemRep" = "semrep"
    ),
    "Curated Databases" = list(
      "BEL" = "bel",
      "BioPAX" = "biopax",
      "SIGNOR" = "signor",
      "BioGRID" = "biogrid",
      "HPRD" = "hprd",
      "TRRUST" = "trrust",
      "PhosphoELM" = "phosphoelm",
      "VirHostNet" = "virhostnet",
      "OmniPath" = "omnipath",
      "UbiBrowser" = "ubibrowser",
      "ACSN" = "acsn",
      "WormBase" = "wormbase",
      "CTD" = "ctd",
      "DrugBank" = "drugbank",
      "DGI" = "dgi",
      "TAS" = "tas",
      "CROG" = "crog",
      "CREEDS" = "creeds"
    )
  )
}

498-503: Minor accessibility and layout polish for the download box

Add an explicit min-height to prevent layout shift when the button is first rendered, and ARIA attributes so screen readers announce the newly available control.

Apply this diff:

-createCodeDownloadBox <- function(ns) {
-  div(
-    style = "text-align: right; padding: 10px;",
-    uiOutput(ns("network.code.button"))
-  )
-}
+createCodeDownloadBox <- function(ns) {
+  div(
+    style = "text-align: right; padding: 10px; min-height: 42px;",  # reserve space for the button
+    role = "region",
+    `aria-live` = "polite",
+    uiOutput(ns("network.code.button"))
+  )
+}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a208747 and 9dead2b.

📒 Files selected for processing (1)
  • R/module-visualize-network-ui.R (6 hunks)
🔇 Additional comments (2)
R/module-visualize-network-ui.R (2)

455-463: Nice UX touch with the contextual paragraph and button placement

Centering the explainer text and placing the code download UI above the visualization reads well and matches the PR goals to expose reproducible code prominently.


33-37: CSV Column Names Verified

Confirmed that the server-side and plotting code consistently expect the columns named exactly Protein, log2FC, and adj.pvalue. No further changes needed.

• In R/module-visualize-network-server.R (lines 645–650), loadCsvData() checks for "Protein" in names(df) and uses df$Protein.
• In plotting_functions.R, various calls to colnames(data)[3] == "log2FC" and references to data$adj.pvalue enforce those exact names.
• Existing tests in tests/testthat/test_network_visualization.R construct mock data frames with Protein, log2FC, and adj.pvalue.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
R/module-visualize-network-server.R (1)

22-44: Fix Shiny input namespacing: avoid double dashes and mismatched observers

Passing module_id = session$ns(NULL) likely yields a trailing “-” (e.g., "ns-"), so concatenating -edgeClicked produces "ns--edgeClicked". Observers listen on input$edgeClicked (module-scoped), so this mismatch breaks click events.

Normalize the prefix once and always append the id without an extra dash.

Apply this diff:

 generateCytoscapeJSForShiny <- function(node_elements, edge_elements, 
                                         display_label_type = "id",
                                         container_id = "network-cy", module_id = "network") {
 
-  # Define Shiny-specific event handlers for both edges and nodes
+  # Normalize module prefix: ensure it ends with exactly one '-'
+  ns_prefix <- if (grepl("-$", module_id)) module_id else paste0(module_id, "-")
+
+  # Define Shiny-specific event handlers for both edges and nodes
   shiny_event_handlers <- list(
     edge_click = paste0("function(evt) {
         var edge = evt.target;
         const edgeId = edge.id();
-        Shiny.setInputValue('", module_id, "-edgeClicked', {
+        Shiny.setInputValue('", ns_prefix, "edgeClicked', {
             source: edge.data('source'),
             target: edge.data('target'),
             interaction: edge.data('interaction'),
             edge_type: edge.data('edge_type'),
             category: edge.data('category')
         });
     }"),
     node_click = paste0("function(evt) {
         var node = evt.target;
         const nodeId = node.id();
-        Shiny.setInputValue('", module_id, "-nodeClicked', { 
+        Shiny.setInputValue('", ns_prefix, "nodeClicked', { 
             id: node.data('id'),
             label: node.data('label'),
             color: node.data('color')
         });
     }")
   )
♻️ Duplicate comments (2)
R/module-visualize-network-server.R (2)

469-476: Add error handling to downloadHandler content

File writes can fail and generate_network_code() may be NULL before the button is used. Wrap in tryCatch and notify the user on error.

Apply this diff:

 output$network_download_code <- downloadHandler(
   filename = function() {
     paste("network-analysis-code-", Sys.Date(), ".R", sep="")
   },
   content = function(file) {
-    writeLines(generate_network_code(), file)
+    tryCatch({
+      code_content <- generate_network_code()
+      if (is.null(code_content) || length(code_content) == 0) {
+        stop("No code generated. Please click 'Show Network' first.")
+      }
+      writeLines(code_content, file)
+    }, error = function(e) {
+      showNotification(paste("Error downloading code:", e$message), type = "error")
+    })
   }
 )

362-432: Escape user-supplied strings in generated code + fix displayLabelType construction

Raw embedding of strings (labels, id types, vectors) can break the generated R script or lead to injection of malformed code. Also, displayLabelTypeStr treats a radio input as a vector.

Apply this diff:

   generate_network_code <- eventReactive(input$showNetwork, {
     params <- getInputParameters(input)
-    
+    # Helper to safely embed strings into generated R code
+    escape_r_string <- function(x) {
+      if (is.null(x)) return(NULL)
+      x <- as.character(x)
+      x <- gsub("\\\\", "\\\\\\\\", x)        # escape backslashes
+      x <- gsub("\"", "\\\\\"", x, fixed = TRUE) # escape double quotes
+      x
+    }
+
     codes <- ""
     codes <- paste(codes, "\n# Load Required Packages\n", sep = "")
     codes <- paste(codes, "library(MSstatsBioNet)\nlibrary(dplyr)\n\n", sep = "")
     
     codes <- paste(codes, "# Read data\n", sep = "")
     codes <- paste(codes, "df <- read.csv(\"path/to/your/data.csv\")\n\n", sep = "")
     
     # Add label filtering if not default
     if (params$selectedLabel != "" && !is.null(params$selectedLabel)) {
       codes <- paste(codes, "# Filter by selected comparison\n", sep = "")
-      codes <- paste(codes, "filtered_df <- df[df$Label == \"", params$selectedLabel, "\" & !is.na(df$Label), ]\n\n", sep = "")
+      codes <- paste(codes, "filtered_df <- df[df$Label == \"", escape_r_string(params$selectedLabel), "\" & !is.na(df$Label), ]\n\n", sep = "")
     } else {
       codes <- paste(codes, "filtered_df <- df\n\n", sep = "")
     }
     
     codes <- paste(codes, "# Annotate protein information\n", sep = "")
-    codes <- paste(codes, "annotated_df <- annotateProteinInfoFromIndra(filtered_df, \"", params$proteinIdType, "\")\n\n", sep = "")
+    codes <- paste(codes, "annotated_df <- annotateProteinInfoFromIndra(filtered_df, \"", escape_r_string(params$proteinIdType), "\")\n\n", sep = "")
     
     codes <- paste(codes, "# Extract subnetwork with filtering parameters\n", sep = "")
     codes <- paste(codes, "subnetwork <- getSubnetworkFromIndra(\n", sep = "")
     codes <- paste(codes, "  annotated_df,\n", sep = "")
     codes <- paste(codes, "  pvalueCutoff = ", params$pValue, ",\n", sep = "")
     codes <- paste(codes, "  evidence_count_cutoff = ", params$evidence, ",\n", sep = "")
     
     # Handle statement types
     if (is.null(params$statementTypes)) {
       codes <- paste(codes, "  statement_types = NULL,\n", sep = "")
     } else {
-      statement_types_str <- paste0("c(\"", paste(params$statementTypes, collapse = "\", \""), "\")")
+      sanitized_st <- vapply(params$statementTypes, escape_r_string, character(1))
+      statement_types_str <- paste0("c(\"", paste(sanitized_st, collapse = "\", \""), "\")")
       codes <- paste(codes, "  statement_types = ", statement_types_str, ",\n", sep = "")
     }
     
     # Handle sources
     if (is.null(params$sources)) {
       codes <- paste(codes, "  sources_filter = NULL,\n", sep = "")
     } else {
-      sources_str <- paste0("c(\"", paste(params$sources, collapse = "\", \""), "\")")
+      sanitized_src <- vapply(params$sources, escape_r_string, character(1))
+      sources_str <- paste0("c(\"", paste(sanitized_src, collapse = "\", \""), "\")")
       codes <- paste(codes, "  sources_filter = ", sources_str, ",\n", sep = "")
     }
     
     codes <- paste(codes, "  logfc_cutoff = ", params$absLogFC, sep = "")
     
     # Handle selected proteins
     if (!is.null(params$selectedProteins) && length(params$selectedProteins) > 0) {
-      selected_proteins_str <- paste0("c(\"", paste(params$selectedProteins, collapse = "\", \""), "\")")
+      sanitized_prots <- vapply(params$selectedProteins, escape_r_string, character(1))
+      selected_proteins_str <- paste0("c(\"", paste(sanitized_prots, collapse = "\", \""), "\")")
       codes <- paste(codes, ",\n  force_include_proteins = ", selected_proteins_str, "\n", sep = "")
     } else {
       codes <- paste(codes, ",\n  force_include_proteins = NULL\n", sep = "")
     }
     
     codes <- paste(codes, ")\n\n", sep = "")
@@
     codes <- paste(codes, "# Save results\n", sep = "")
     codes <- paste(codes, "write.csv(subnetwork$nodes, \"network_nodes.csv\", row.names = FALSE)\n", sep = "")
     codes <- paste(codes, "write.csv(subnetwork$edges, \"network_edges.csv\", row.names = FALSE)\n", sep = "")
     codes <- paste(codes, "# Visualize network on web browser and export as an HTML file\n", sep = "")
-    displayLabelTypeStr <- paste0("\"", paste(input$displayLabelType, collapse = "\", \""), "\"")
+    displayLabelTypeStr <- paste0("\"", escape_r_string(input$displayLabelType), "\"")
     codes <- paste(codes, "previewNetworkInBrowser(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
     codes <- paste(codes, "exportNetworkToHTML(subnetwork$nodes, subnetwork$edges, displayLabelType=", displayLabelTypeStr, ")\n", sep = "")
     
     return(codes)
   })
🧹 Nitpick comments (3)
man/generateCytoscapeJSForShiny.Rd (1)

10-21: Docs reflect new display_label_type parameter

The usage and arguments now include display_label_type = "id", which aligns with the server code. Consider clarifying accepted values (e.g., "id", "hgncName") in the roxygen source so Rd is regenerated with that detail.

tests/testthat/test_network_visualization.R (2)

62-66: Good coverage for JS handler wiring; consider adding label-type coverage

This validates Shiny event handler emission. Consider a follow-up test passing display_label_type = "hgncName" to ensure it propagates into the generated JS/config as expected.


240-244: Minimal end-to-end check looks good; add assertions on JS content

Asserting non-empty JS is useful. You could also assert that the emitted JS includes the container id ("network-cy" default) or input ids ("network-edgeClicked"/"network-nodeClicked") for stronger guarantees.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9dead2b and f033ab2.

📒 Files selected for processing (6)
  • NAMESPACE (1 hunks)
  • R/module-visualize-network-server.R (5 hunks)
  • man/generateCytoscapeConfig.Rd (0 hunks)
  • man/generateCytoscapeJSForShiny.Rd (2 hunks)
  • man/generateJavaScriptCode.Rd (0 hunks)
  • tests/testthat/test_network_visualization.R (2 hunks)
💤 Files with no reviewable changes (2)
  • man/generateJavaScriptCode.Rd
  • man/generateCytoscapeConfig.Rd
🔇 Additional comments (4)
NAMESPACE (1)

41-41: Import addition aligns with new dependency usage

Adding importFrom(MSstatsBioNet,generateCytoscapeConfig) matches the updated server usage and prevents namespace issues at runtime.

R/module-visualize-network-server.R (3)

13-17: Roxygen updates and import look correct

The added @param display_label_type and @importFrom MSstatsBioNet generateCytoscapeConfig match the NAMESPACE change and the function’s new signature.


48-53: Config generation call is consistent with new API

nodes, edges, and display_label_type are passed through to the package function; looks good.


462-466: Rendering the download button post-network render is a nice UX touch

Showing the button only after the network is displayed keeps the UI clean and avoids confusion. Style choice is consistent with app patterns.

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