feat(visualization-code): Add event handler for edge deletion#186
feat(visualization-code): Add event handler for edge deletion#186tonywu1999 merged 3 commits intodevelfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 25 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request extends the network visualization module to enable interactive edge deletion. The MSstatsBioNet's Changes
Sequence DiagramsequenceDiagram
actor User
participant Observer as Edge Deletion<br/>Observer
participant State as Reactive State<br/>(deletedEdges,<br/>currentEdgesTable)
participant Table as Edges Table<br/>Renderer
participant CodeGen as Code Download<br/>Generator
User->>Observer: Click delete edge<br/>(input$network_edge_deleted)
Observer->>State: Append deleted edge to<br/>deletedEdges
Observer->>Observer: Call deleteEdgeFromNetwork<br/>on currentEdgesTable
Observer->>State: Update currentEdgesTable
Observer->>Table: Trigger re-render with<br/>updated edges
Table-->>User: Display modified edges
User->>CodeGen: Request code download
CodeGen->>State: Query deletedEdges state
alt Edges were deleted
CodeGen->>CodeGen: Inject deleteEdgeFromNetwork<br/>calls into script
end
CodeGen-->>User: Provide updated R script
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
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)
655-688:⚠️ Potential issue | 🟠 MajorAvoid
stop()indownloadHandlercontent; write fallback output to file instead.Line 658 calls
stop()inside thecontentfunction, which can destabilize the Shiny session and leaves users with a broken download. Additionally, the error handler (lines 680–685) only shows a notification but does not write an error message to the file, leaving it empty on error.Replace
stop()with file output and add error recovery to the handler:Suggested fix
code_content <- generate_network_code() if (is.null(code_content) || length(code_content) == 0) { - stop("No code generated. Please ensure network is displayed first.") + writeLines( + "# Error: No code generated. Please ensure network is displayed first.", + file + ) + return(invisible(NULL)) } deleted <- deletedEdges() if (length(deleted) > 0) { deletion_lines <- vapply(deleted, function(e) { sprintf( 'subnetwork$edges <- MSstatsBioNet::deleteEdgeFromNetwork(subnetwork$edges, "%s", "%s", "%s")', e$source, e$target, e$interaction ) }, character(1)) edge_deletion_section <- paste0( "\n# Delete edges removed interactively\n", paste(deletion_lines, collapse = "\n"), "\n" ) code_content <- sub( "# Visualize network", paste0(edge_deletion_section, "# Visualize network"), code_content, fixed = TRUE ) } writeLines(code_content, file) }, error = function(e) { showNotification( paste("Error downloading code:", e$message), type = "error" ) + writeLines(paste0("# Error downloading code: ", e$message), file) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@R/module-visualize-network-server.R` around lines 655 - 688, The content function currently calls stop() when generate_network_code() returns NULL/empty and also doesn't write anything to the file on error; replace the stop() call with writing a clear fallback/error message into the download file (e.g. use writeLines("No code generated: ensure network is displayed first.", file) and return), and update the outer error handler (error = function(e)) to both showNotification(paste("Error downloading code:", e$message), type="error") and write a readable error message into the same file (writeLines(sprintf("Error generating download: %s", e$message), file)) so the downloaded file always contains a useful message; keep the existing edge deletion injection around generate_network_code()/deletedEdges() and then writeLines(code_content, file) when successful.
🤖 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-visualize-network-server.R`:
- Around line 663-667: The generated R code injects raw e$source, e$target and
e$interaction into string literals, which breaks the script when values contain
quotes or backslashes; update the deletion_lines construction to escape those
fields before interpolation (e.g., add a small helper like escape_r_string(x)
that replaces " and backslash with escaped versions via gsub('([\"\\\\])',
'\\\\\\1', x) or use an equivalent R escaping function) and use that helper on
e$source, e$target and e$interaction when building the sprintf passed to
deleteEdgeFromNetwork (references: deletion_lines,
MSstatsBioNet::deleteEdgeFromNetwork, e$source, e$target, e$interaction).
- Around line 707-716: The edge-deletion handler must guard against NULL inputs
and normalize the interaction string before calling the deletion helper: in the
observeEvent for input$network_edge_deleted, first return early if
input$network_edge_deleted or currentEdgesTable() is NULL, then strip the "
(bidirectional)" suffix from edge_data$interaction (same normalization used by
highlightEdgeInTable) and use that normalized value when calling
MSstatsBioNet::deleteEdgeFromNetwork; keep the existing deletedEdges(c(...))
update but ensure you operate on the validated/normalized edge_data.
---
Outside diff comments:
In `@R/module-visualize-network-server.R`:
- Around line 655-688: The content function currently calls stop() when
generate_network_code() returns NULL/empty and also doesn't write anything to
the file on error; replace the stop() call with writing a clear fallback/error
message into the download file (e.g. use writeLines("No code generated: ensure
network is displayed first.", file) and return), and update the outer error
handler (error = function(e)) to both showNotification(paste("Error downloading
code:", e$message), type="error") and write a readable error message into the
same file (writeLines(sprintf("Error generating download: %s", e$message),
file)) so the downloaded file always contains a useful message; keep the
existing edge deletion injection around generate_network_code()/deletedEdges()
and then writeLines(code_content, file) when successful.
🪄 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: 181a75a1-5226-4ffb-b513-915387c44c80
📒 Files selected for processing (2)
R/MSstatsShiny.RR/module-visualize-network-server.R
Motivation and Context
This PR implements interactive edge deletion functionality in the network visualization module. Previously, users could visualize network graphs and see edge information in tables, but could not dynamically remove edges from the visualization. The solution introduces reactive state management to track deleted edges, updates the live edges table as deletions occur, and ensures that exported R code reflects these interactive changes so users can reproduce their analysis in standalone scripts.
Detailed Changes
R/MSstatsShiny.R
deleteEdgeFromNetworkfunction from theMSstatsBioNetpackage to enable edge deletion operationsR/module-visualize-network-server.R
New Reactive State Management:
deletedEdges: Reactive values object that accumulates edges deleted interactively by the user throughout the sessioncurrentEdgesTable: Reactive expression maintaining the live edges table that reflects all deletions made during the current visualization sessionNetwork Display and Initialization:
input$showNetworktriggers:deletedEdgesis reset to an empty list to start fresh with each new network visualizationnodesTableandedgesTabledata tables are rendered with the freshly generated network datacurrentEdgesTableis seeded with the complete, unmodified edges tableEdge Deletion Observer (
input$network_edge_deleted):deletedEdgescurrentEdgesTableby callingMSstatsBioNet::deleteEdgeFromNetwork()to remove the edge from the tableoutput$edgesTablewith the updated edges, reflecting the deletion in real-timeEdge Click Handling Update:
currentEdgesTable()instead of reconstructing network data fromrenderNetwork(), ensuring that table highlighting correctly reflects the live (possibly edited) edges set with all interactive deletions appliedCode Download Feature Enhancement:
subnetwork$edges <- MSstatsBioNet::deleteEdgeFromNetwork(...)calls for each deleted edgeUnit Tests
No new tests were added or modified in this PR. The existing test file
tests/testthat/test_network_visualization.Rdoes not contain tests specific to the edge deletion functionality introduced in this change.Coding Guidelines
The following style guideline violations were identified in
R/module-visualize-network-server.R:These are minor whitespace issues that do not affect functionality but should be cleaned up in a future commit or addressed in a follow-up PR to maintain code cleanliness.