Force drug#54
Conversation
WalkthroughThe updates introduce a new parameter, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant getSubnetworkFromIndra
participant .validateGetSubnetworkFromIndraInput
participant .callIndraCogexApi
participant INDRA_API
User->>getSubnetworkFromIndra: Call with input, force_include_other
getSubnetworkFromIndra->>.validateGetSubnetworkFromIndraInput: Validate input, force_include_other
getSubnetworkFromIndra->>.callIndraCogexApi: Pass hgncIds, force_include_other
.callIndraCogexApi->>INDRA_API: Query with hgncIds + force_include_other
INDRA_API-->>.callIndraCogexApi: Return subnetwork data
.callIndraCogexApi-->>getSubnetworkFromIndra: Return edges/nodes
getSubnetworkFromIndra-->>User: Return subnetwork
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #54 +/- ##
==========================================
+ Coverage 32.08% 32.23% +0.14%
==========================================
Files 8 8
Lines 1206 1213 +7
==========================================
+ Hits 387 391 +4
- Misses 819 822 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
R/utils_getSubnetworkFromIndra.R (2)
13-16: Duplicate-count risk in protein total calculation
num_proteinsblindly addslength(force_include_other)tonrow(input).
If any identifier inforce_include_othercorresponds to an HGNC ID already present ininput, the same entity is double-counted and can spuriously trip the<400limit.- num_proteins = nrow(input) + ifelse(!is.null(force_include_other), length(force_include_other), 0) + unique_ids <- unique(c(input$HgncId, + sub("^.+:", "", force_include_other %||% character(0)))) + num_proteins <- length(unique_ids)Using
unique()avoids false “too many proteins” errors.
265-283: Minor: unnecessary temporaryhgnc_valueThe local assignment isn’t reused outside the one-liner; you can inline it to avoid the extra object:
nodes$hgncName <- if ("HgncName" %in% colnames(input) && is.character(input$HgncName)) { ifelse(is.na(input$HgncName[match(nodes$id, input$Protein)]), nodes$id, input$HgncName[match(nodes$id, input$Protein)]) } else { nodes$id }tests/testthat/test-getSubnetworkFromIndra.R (1)
5-6: Mock signature is brittleHard-coding two positional arguments ties the tests to the exact arity of
.callIndraCogexApi.
Using...makes the mock future-proof:local_mocked_bindings(.callIndraCogexApi = function(...) { readRDS(system.file("extdata/indraResponse.rds", package = "MSstatsBioNet")) })Also applies to: 17-18
R/getPathwaysFromIndra.R (1)
110-112: Stale variable after signature change
namespaceis now unused after removing the third argument to.addAdditionalMetadataToIndraEdge.
Consider deleting the earlier calculation (Lines 41–50) or repurposing it, to avoid dead code and reader confusion.man/getSubnetworkFromIndra.Rd (1)
59-61: Doc tweak – clarify accepted namespacesAdd an explicit note that only namespaces accepted by the INDRA CogEx API are valid; otherwise the call will error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
R/getPathwaysFromIndra.R(1 hunks)R/getSubnetworkFromIndra.R(2 hunks)R/utils_getSubnetworkFromIndra.R(4 hunks)man/getSubnetworkFromIndra.Rd(2 hunks)tests/testthat/test-getSubnetworkFromIndra.R(2 hunks)
🔇 Additional comments (2)
R/utils_getSubnetworkFromIndra.R (1)
162-173:edge$*_namemay be undefinedWhen no matching HGNC row exists you fall back to
edge$source_name/edge$target_name, but these fields are not guaranteed to be present in every INDRA statement object.
Accessing a missing element yieldsNULL, breaking laterdata.frameconstruction that expects character scalars.Safest option is to default to the raw HGNC ID:
edge$source_uniprot_id <- matched_rows_source$Protein %||% edge$source_name %||% edge$source_idR/getSubnetworkFromIndra.R (1)
62-64: Call chain still omitsforce_include_proteinsduplicates
force_include_proteinscan re-introduce proteins filtered out earlier, yet you only passinput$HgncIdto the API call.
If any forced protein was excluded for missingHgncId, it never reaches INDRA.
Verify that all intended proteins are represented ininput$HgncIdpost-filter or append them explicitly before the API request.
| .callIndraCogexApi <- function(hgncIds, force_include_other) { | ||
| indraCogexUrl <- | ||
| "https://discovery.indra.bio/api/indra_subnetwork_relations" | ||
|
|
||
| groundings <- lapply(hgncIds, function(x) list("HGNC", x)) | ||
| if (!is.null(force_include_other)) { | ||
| groundings <- c(groundings, lapply(force_include_other, function(x) { | ||
| parts <- unlist(strsplit(x, ":")) | ||
| if (length(parts) != 2) { | ||
| stop(paste0("Invalid identifier format: ", x, ". Expected format is 'namespace:identifier', e.g. 'HGNC:1234' or 'CHEBI:4911'.")) | ||
| } | ||
| list(parts[1], parts[2]) | ||
| })) | ||
| } |
There was a problem hiding this comment.
Missing HTTP-error handling & fragile string-split
POST()result is consumed without checkingstatus_code, so 4xx/5xx responses silently become cryptic list structures.strsplit(x, ":")fails onNAor strings containing additional ‘:’, and uses regex.
Usefixed = TRUEandtrimws()for robustness.
res <- POST(...)
if (httr::http_error(res))
stop("INDRA API request failed: ", httr::status_code(res), " – ", httr::http_status(res)$message)
...
parts <- strsplit(trimws(x), ":", fixed = TRUE)[[1]]🤖 Prompt for AI Agents
In R/utils_getSubnetworkFromIndra.R around lines 40 to 53, the POST request
result is used without checking for HTTP errors, which can cause silent failures
on 4xx/5xx responses. Add a check after the POST call using httr::http_error to
detect errors and stop execution with a clear message including the status code
and status message. Also, improve the string splitting by using strsplit with
fixed = TRUE and apply trimws to the input string before splitting to handle NA
and extra colons robustly.
2 participants
User description
Checklist Before Requesting a Review
PR Type
Enhancement
Description
Add external node inclusion via
force_include_otherUpdate INDRA API payload and validation
Relax edge metadata mapping with namespaces
Revise node construction from edges
Diagram Walkthrough
File Walkthrough
getPathwaysFromIndra.R
Switch to internal edge metadata helperR/getPathwaysFromIndra.R
.addAdditionalMetadataToIndraEdgenamespaceparametergetSubnetworkFromIndra.R
Support adding non-input nodes to networkR/getSubnetworkFromIndra.R
force_include_otherparameter to APIutils_getSubnetworkFromIndra.R
Validation, API call, metadata, and node construction updatesR/utils_getSubnetworkFromIndra.R
getSubnetworkFromIndra.Rd
Rd docs updated for external nodes parameterman/getSubnetworkFromIndra.Rd
force_include_otherparametertest-getSubnetworkFromIndra.R
Update tests for API signature changetests/testthat/test-getSubnetworkFromIndra.R