Clean up and generate base docu with claude#6
Conversation
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (38)
💤 Files with no reviewable changes (6)
📝 WalkthroughWalkthroughThis PR transforms a generic ESQLabs R project template into a complete tuberculosis PBPK/PD modeling platform. It reorganizes utility functions into modular helpers, parameterizes two core simulation scripts via centralized configuration, adds drug parameter datasets, provides comprehensive technical and workflow documentation, and includes Quarto reports for scenario and dose-sensitivity analysis visualization. ChangesTB Platform Implementation
🎯 4 (Complex) | ⏱️ ~45 minutes 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.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes a comprehensive PBPK/PD simulation platform for anti-tuberculosis drugs, introducing automated workflows for parameter transformation, model generation, and clinical scenario reporting. The changes include refactored simulation scripts for improved maintainability and extensive documentation. Feedback identifies several logic errors in data filtering where numeric values are incorrectly compared to time-window strings, as well as fragile hardcoded plot layer indexing. Additionally, improvements are suggested for robust parameter parsing and ensuring correct data types during EBA calculations.
| observedData <- esqlabsR::loadObservedData( | ||
| projectConfiguration = projectConfiguration, | ||
| sheets = NULL | ||
| ) |
There was a problem hiding this comment.
The createResults function accepts dataSheets and customDataSets as arguments, but the implementation ignores them. loadObservedData is called with sheets = NULL, and customDataSets are never merged into the observedData list. This breaks the ability to load specific data sheets or provide custom datasets.
observedData <- esqlabsR::loadObservedData(
projectConfiguration = projectConfiguration,
sheets = dataSheets
)
if (!is.null(customDataSets)) {
observedData[names(customDataSets)] <- customDataSets
}| combined_noBDQ <- combined_cavg %>% filter(Drug != 'BDQ') | ||
| observedEBAs_long_noBDQ <- observedEBAs_long %>% | ||
| filter(Drug != 'BDQ') %>% | ||
| filter(EBA_value != '2-14') |
There was a problem hiding this comment.
The filter filter(EBA_value != '2-14') appears to be a logic error. EBA_value contains numeric log-reduction values, while '2-14' refers to a time window suffix. This filter was likely intended to exclude the EBA2-14 type by checking the suffix column.
filter(suffix != '2-14')| plots[[p]] <- plots[[p]] + my_theme(1) | ||
| plots[[p]]$layers[[7]]$aes_params$size <- 5 | ||
| if (length(plots[[p]]$layers) >= 7) { | ||
| plots[[p]]$layers[[7]]$aes_params$size <- 5 |
There was a problem hiding this comment.
Modifying plot layers using a hardcoded index (layers[[7]]) is fragile. If the plot structure changes (e.g., due to updates in createPlotsFromExcel or different data inputs), this will target the wrong layer or fail. It is safer to iterate through layers and check the geom type or use the format_plot helper function.
|
|
||
| combined_noBDQ <- combined_cavg %>% filter(Drug != 'BDQ') | ||
| observed_noBDQ <- observedEBAs_long %>% | ||
| filter(Drug != 'BDQ', EBA_value != '2-14') |
There was a problem hiding this comment.
The filter EBA_value != '2-14' is likely incorrect as EBA_value is numeric. It should probably filter on the suffix column to exclude the 2–14 day time window.
filter(Drug != 'BDQ', suffix != '2-14')| keys <- vapply(parts, function(x) x[2], "") | ||
| vals <- vapply(parts, function(x) x[3], "") | ||
|
|
||
| drug <- vals[keys == "name"] |
There was a problem hiding this comment.
The assignment drug <- vals[keys == "name"] assumes the key "name" exists exactly once in the input file. If it is missing or duplicated, the subsequent data.frame construction will fail due to a dimension mismatch. Consider using a more robust selection like vals[which(keys == "name")[1]].
| relConc <- dfRes %>% | ||
| filter(Time > ebaStartTimes[e], Time < ebaEndTimes[e]) %>% | ||
| select(all_of(concPaths)) | ||
| cmaxs[e] <- max(relConc) |
There was a problem hiding this comment.
Since relConc is a data frame (result of select()), calling max(relConc) may lead to unexpected results or errors depending on the R version and data structure. It is safer to use unlist() to ensure the maximum is calculated across all values in the selected columns, consistent with the mean(unlist(relConc)) call on the next line.
cmaxs[e] <- max(unlist(relConc))|
|
||
| combined_noBDQ <- combined_cavg %>% filter(Drug != 'BDQ') | ||
| observedEBAs_long_noBDQ <- observedEBAs_long %>% | ||
| filter(Drug != 'BDQ', EBA_value != '2-14') |
There was a problem hiding this comment.
The filter EBA_value != '2-14' is likely a typo for suffix != '2-14'. EBA_value contains the numeric results of the simulation, not the time window labels.
filter(Drug != 'BDQ', suffix != '2-14')
1 participant
Summary by CodeRabbit
Release Notes
New Features
Documentation
Refactor