From f7ddf9de1a868aa780ae804b82437a04d18fa213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E5=B4=9D?= Date: Tue, 26 May 2026 15:39:37 +0800 Subject: [PATCH 01/11] fix: prevent PKNCA_impute_method_FALSE error from YAML `impute_c0: no` (#1266) When start_impute=FALSE, create_start_impute() is skipped and data$intervals has no impute column. dplyr mutate falls back to the function parameter impute=FALSE, producing "blq, FALSE" which PKNCA interprets as an imputation method name lookup for the non-existent PKNCA_impute_method_FALSE. Fix: initialize the impute column before BLQ imputation is applied, matching the existing guard pattern in rm_impute_obs_params(). Co-Authored-By: Claude Opus 4.7 --- DESCRIPTION | 2 +- NEWS.md | 1 + R/intervals.R | 7 +++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index c9c635d40..240a66132 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: aNCA Title: (Pre-)Clinical NCA in a Dynamic Shiny App -Version: 0.1.0.9173 +Version: 0.1.0.9174 Authors@R: c( person("Ercan", "Suekuer", email = "ercan.suekuer@roche.com", role = "aut", comment = c(ORCID = "0009-0001-1626-1526")), diff --git a/NEWS.md b/NEWS.md index d38b0f701..ee1595b6f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -79,6 +79,7 @@ * DOSNOA computation fixed for specimen-level grouping — urine-only data no longer gets incorrect dose numbering (#1116) * Dose-aware AUCint parameters now share the same PPTESTCD as their non-dose-aware counterparts in CDISC exports, with `PPANMETH` indicating the analytical method. Internal PPTESTCDs renamed from misleading `D` suffix (e.g. `AUCINTD`) to lowercase `da` suffix (e.g. `AUCINTda`). Fixed wrong PPTEST label for `AUCINTD` which said "Normalized by Dose" (#1242) * Optional settings (`slope_rules`, `int_parameters`, `ratio_table`) are now normalized to `NULL` when empty, instead of persisting as 0-row data frames throughout the app and settings pipeline (#1262) +* Fixed `PKNCA_impute_method_FALSE` error when YAML settings contain `impute_c0: no` — the `impute` column is now initialized before BLQ imputation is applied to prevent dplyr from resolving to the boolean function parameter (#1266) * Interval-specific parameters (`aucint.*`, `cav.int.*`) excluded from the Parameter Selection matrix — they require finite sub-intervals and must be configured via Partial Interval Calculations (#1309) ### Ratio Calculations diff --git a/R/intervals.R b/R/intervals.R index 0db736fa0..517b527ef 100644 --- a/R/intervals.R +++ b/R/intervals.R @@ -290,6 +290,13 @@ update_main_intervals <- function( data <- create_start_impute(data) } + # Ensure impute column exists so dplyr mutate below references the column + # rather than the function parameter (which could be FALSE from YAML settings, + # causing "PKNCA_impute_method_FALSE" not found error). + if (!"impute" %in% names(data$intervals)) { + data$intervals$impute <- NA_character_ + } + ############################################ # Define a BLQ imputation method for PKNCA # and apply it only for non-observational parameters From 40926c3a188b8e56c483e84481a2e7c07c66950a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E5=B4=9D?= Date: Tue, 26 May 2026 15:58:43 +0800 Subject: [PATCH 02/11] docs: add imputation pipeline, interval helpers, and YAML caveats to CLAUDE.md Co-Authored-By: Claude Opus 4.7 --- CLAUDE.md | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..5b7a3786b --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,126 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project overview + +aNCA is an R package providing a Shiny app for automated Non-Compartmental Analysis (NCA) on pre-clinical and clinical pharmacokinetic data. It builds on `PKNCA` (>= 0.12.1) as the core NCA engine and produces CDISC outputs (ADNCA, PP, ADPP), configurable TLGs, and slide decks. + +**Read `AGENTS.md`** — it is the single source of truth for all conventions, code principles, and anti-patterns. The sections below only add what AGENTS.md doesn't already cover. + +## Build and test commands + +```r +devtools::load_all() # Load package in development +devtools::document() # Rebuild NAMESPACE + man/ after roxygen changes +devtools::test() # Run all unit tests +devtools::test_active_file()# Run current test file only +devtools::check() # Full R CMD check +lintr::lint_package() # Code style check +styler::style_file("R/foo.R") # Format a file to tidyverse style +``` + +To launch the Shiny app: `aNCA::run_app()` — automatically loads test data on startup. Set `aNCA_LOG_LEVEL="TRACE"` in `.Renviron` for debug logging. + +CI runs on every PR via GitHub Actions: lintr, spelling, R CMD check, test coverage, and documentation validation. + +## Two code zones + +**`R/`** — Package API. Functions usable without the Shiny app. Exported functions live here with full roxygen2 docs. No `library()`/`require()` allowed. Imports used via `@importFrom`, Suggests used via `pkg::fun()`. + +**`inst/shiny/`** — Shiny application. `app.R` is the entry point, `modules/` contains the tab-level UI+server modules, `functions/` holds app-only helpers (auto-sourced by app.R). Code here can use `require()` since it runs inside the Shiny process. + +## Shiny data flow + +``` +data_upload_server → adnca_raw (reactive) → tab_data_server → tab_nca_server + ↓ + processed_pknca_data → tab_tlg_server +``` + +Module communication uses `session$userData`: +- `session$userData$results` — CDISC datasets and NCA results +- `session$userData$project_name()` — reactive, auto-populated from STUDYID +- `session$userData$slope_rules()` — reactive +- `session$userData$settings_versions` — versioned YAML settings list + +## Tab structure + +| Tab | Module file | Purpose | +|-----|------------|---------| +| Data | `tab_data.R` + `tab_data/` | Upload, mapping, filtering, data preprocessing | +| Exploration | `tab_explore.R` + `tab_explore/` | Interactive plots (line, boxplot, PK/Dose QC) | +| NCA | `tab_nca.R` + `tab_nca/` | NCA settings, parameter selection, slope selector, ratios, exclusions, results | +| TLG | `tab_tlg.R` + `tab_tlg/` | Tables/listings/graphs defined in `inst/shiny/tlg.yaml` | +| About | `tab_about.R` | Citations, session info, app version | + +## Exports and ZIP + +Export workflow in `tab_nca/zip.R`: `TREE_LIST` defines export structure. Selected items come from `input$res_tree`. ZIP filename uses project name or falls back to `NCA_STUDYID.zip`. Pre-Specs sheets are auto-generated from CDISC data. + +## Styling (CSS/SCSS) + +Styles authored in `inst/shiny/www/styles/` SCSS partials, compiled to `main.css`. Entry point: `styles/main.scss`. When editing styles, modify the `.scss` partial **and** apply the same change to `main.css` (the R compile script may not be available). Sidebar width is `--_sidebar-width: 170px !important` in `main.css`. + +## Exploration plots conventions + +- Default `color_by` priority: `DOSEA > TRT01A > GROUP > ACTARM > COHORT` +- Default `facet_by` priority: `TRT01A > DOSEA > GROUP > ACTARM > COHORT` +- Auto-mapping includes: `COHORT`, `PART`, `PERIOD` + +## Color constants + +- **SCSS variables**: `inst/shiny/www/styles/modules/_colors.scss` +- **R constants**: `inst/shiny/functions/colors.R` (auto-sourced, available in all Shiny modules) +- **Exclusion colors**: `inst/shiny/functions/utils-exclusions.R` — `EXCL_COLOR_NCA`, `EXCL_COLOR_TLG`, `EXCL_COLOR_BOTH`, `EXCL_COLOR_PARAM` +- **Flag colors**: `inst/shiny/modules/tab_nca/nca_results.R` — `FLAG_COLOR_FLAGGED`, `FLAG_COLOR_MISSING` + +## Imputation pipeline + +BLQ (Below Limit of Quantification) and start concentration (C0) imputation spans multiple files: + +| Step | File | Key function | +|------|------|-------------| +| UI | `inst/shiny/modules/tab_nca/setup/data_imputation.R` | `data_imputation_server()` — returns `blq_imputation_rule`, `impute_c0`, `blq_strategy` | +| Settings assembly | `inst/shiny/modules/tab_nca/setup/settings.R#452-491` | `settings()` reactive — bundles `data_imputation` into the settings list | +| Data object update | `R/PKNCA.R#297` | `PKNCA_update_data_object(start_impute, blq_imputation_rule)` | +| Interval imputation | `R/intervals.R#215` | `update_main_intervals(impute, blq_imputation_rule)` — sets `data$intervals$impute` column | +| Start impute | `R/create_start_impute.R` | `create_start_impute()` — adds C0 strategies (`start_conc0`, `start_predose`, `start_logslope`, `start_c1`) | +| BLQ global function | `R/PKNCA.R#439` | `PKNCA_calculate_nca(blq_rule)` — registers `PKNCA_impute_method_blq` globally before calling `PKNCA::pk.nca()` | +| Obs param cleanup | `R/intervals.R#326` | `rm_impute_obs_params()` — removes imputation from non-AUC-dependent parameters | + +### Critical guard + +`update_main_intervals()` must ensure `data$intervals$impute` exists **before** the BLQ `dplyr::mutate()` block. If the column is missing and `impute` is also a function parameter (e.g. `FALSE` from YAML `impute_c0: no`), dplyr resolves to the parameter, producing `"blq, FALSE"` which PKNCA interprets as a lookup for `PKNCA_impute_method_FALSE`. The guard `if (!"impute" %in% names(data$intervals))` (see `rm_impute_obs_params()` line 341) prevents this. + +## Interval parameter helpers + +- **`rename_interval_params()`** (`R/utils.R:17`): Appends `_start-end` suffix to PPTESTCD for manual interval params (e.g. `AUCINT` → `AUCINT_0-24`). Called in `descriptive_statistics.R`, `parameter_plots.R`, `flexible_violinboxplot.R`. +- **`parse_interval_parameter()`** (`R/ratio_calculations.R:326`): Reverses the above — extracts `base`, `start`, `end`, `is_interval` from a suffixed name. Used when labels need to be resolved from `metadata_nca_parameters` (which only stores base PPTESTCDs). + +## YAML boolean caveat + +R's `yaml` package parses YAML 1.1 booleans: `yes`/`no`/`true`/`false`/`on`/`off` all become R logical values. When settings YAML values like `impute_c0: no` are parsed, they become logical `FALSE`, not the string `"no"`. Code that passes these values into `if()` conditions or dplyr expressions must handle logical values correctly — prefer `isTRUE()` / `isFALSE()` guards over bare `if (value)` when the value may be logical from YAML. + +## Testing conventions + +- Test files: `tests/testthat/test-.R`, matching source file name +- Use `describe()` / `it()` blocks with value-level assertions +- Test data created in `tests/testthat/setup.R` and `tests/testthat/data/` +- E2E tests in `test-e2e-runner.R` use `shinytest2` (in Suggests) + +## Package versioning and release + +- Semantic versioning. Bump `DESCRIPTION` version (+1 from main) for non-trivial changes +- `{admiral}` family follows quarterly releases (Mar/Jun/Sep/Dec); aNCA extensions should align within 2 weeks +- `NEWS.md` entries reference PR numbers + +## PR workflow summary + +1. `git fetch origin main && git checkout origin/main` — branch from latest upstream +2. Branch name: `-/` +3. After code changes: `document()` → `lint_package()` → `test()` → `check()` +4. Update `NEWS.md`, bump `DESCRIPTION` version +5. Push to fork, create PR against `pharmaverse/main` +6. Add 2+ reviewers from DESCRIPTION authors +7. Never force-push; use separate commits instead of amending From dc0cf9068de2c3fe2d6494bbb7d0b4b79483dd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E5=B4=9D?= Date: Wed, 27 May 2026 17:09:24 +0800 Subject: [PATCH 03/11] fix: remove CLAUDE.md mistakenly included in commit (#1266) Co-Authored-By: Claude Opus 4.7 --- CLAUDE.md | 126 ------------------------------------------------------ 1 file changed, 126 deletions(-) delete mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index 5b7a3786b..000000000 --- a/CLAUDE.md +++ /dev/null @@ -1,126 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## Project overview - -aNCA is an R package providing a Shiny app for automated Non-Compartmental Analysis (NCA) on pre-clinical and clinical pharmacokinetic data. It builds on `PKNCA` (>= 0.12.1) as the core NCA engine and produces CDISC outputs (ADNCA, PP, ADPP), configurable TLGs, and slide decks. - -**Read `AGENTS.md`** — it is the single source of truth for all conventions, code principles, and anti-patterns. The sections below only add what AGENTS.md doesn't already cover. - -## Build and test commands - -```r -devtools::load_all() # Load package in development -devtools::document() # Rebuild NAMESPACE + man/ after roxygen changes -devtools::test() # Run all unit tests -devtools::test_active_file()# Run current test file only -devtools::check() # Full R CMD check -lintr::lint_package() # Code style check -styler::style_file("R/foo.R") # Format a file to tidyverse style -``` - -To launch the Shiny app: `aNCA::run_app()` — automatically loads test data on startup. Set `aNCA_LOG_LEVEL="TRACE"` in `.Renviron` for debug logging. - -CI runs on every PR via GitHub Actions: lintr, spelling, R CMD check, test coverage, and documentation validation. - -## Two code zones - -**`R/`** — Package API. Functions usable without the Shiny app. Exported functions live here with full roxygen2 docs. No `library()`/`require()` allowed. Imports used via `@importFrom`, Suggests used via `pkg::fun()`. - -**`inst/shiny/`** — Shiny application. `app.R` is the entry point, `modules/` contains the tab-level UI+server modules, `functions/` holds app-only helpers (auto-sourced by app.R). Code here can use `require()` since it runs inside the Shiny process. - -## Shiny data flow - -``` -data_upload_server → adnca_raw (reactive) → tab_data_server → tab_nca_server - ↓ - processed_pknca_data → tab_tlg_server -``` - -Module communication uses `session$userData`: -- `session$userData$results` — CDISC datasets and NCA results -- `session$userData$project_name()` — reactive, auto-populated from STUDYID -- `session$userData$slope_rules()` — reactive -- `session$userData$settings_versions` — versioned YAML settings list - -## Tab structure - -| Tab | Module file | Purpose | -|-----|------------|---------| -| Data | `tab_data.R` + `tab_data/` | Upload, mapping, filtering, data preprocessing | -| Exploration | `tab_explore.R` + `tab_explore/` | Interactive plots (line, boxplot, PK/Dose QC) | -| NCA | `tab_nca.R` + `tab_nca/` | NCA settings, parameter selection, slope selector, ratios, exclusions, results | -| TLG | `tab_tlg.R` + `tab_tlg/` | Tables/listings/graphs defined in `inst/shiny/tlg.yaml` | -| About | `tab_about.R` | Citations, session info, app version | - -## Exports and ZIP - -Export workflow in `tab_nca/zip.R`: `TREE_LIST` defines export structure. Selected items come from `input$res_tree`. ZIP filename uses project name or falls back to `NCA_STUDYID.zip`. Pre-Specs sheets are auto-generated from CDISC data. - -## Styling (CSS/SCSS) - -Styles authored in `inst/shiny/www/styles/` SCSS partials, compiled to `main.css`. Entry point: `styles/main.scss`. When editing styles, modify the `.scss` partial **and** apply the same change to `main.css` (the R compile script may not be available). Sidebar width is `--_sidebar-width: 170px !important` in `main.css`. - -## Exploration plots conventions - -- Default `color_by` priority: `DOSEA > TRT01A > GROUP > ACTARM > COHORT` -- Default `facet_by` priority: `TRT01A > DOSEA > GROUP > ACTARM > COHORT` -- Auto-mapping includes: `COHORT`, `PART`, `PERIOD` - -## Color constants - -- **SCSS variables**: `inst/shiny/www/styles/modules/_colors.scss` -- **R constants**: `inst/shiny/functions/colors.R` (auto-sourced, available in all Shiny modules) -- **Exclusion colors**: `inst/shiny/functions/utils-exclusions.R` — `EXCL_COLOR_NCA`, `EXCL_COLOR_TLG`, `EXCL_COLOR_BOTH`, `EXCL_COLOR_PARAM` -- **Flag colors**: `inst/shiny/modules/tab_nca/nca_results.R` — `FLAG_COLOR_FLAGGED`, `FLAG_COLOR_MISSING` - -## Imputation pipeline - -BLQ (Below Limit of Quantification) and start concentration (C0) imputation spans multiple files: - -| Step | File | Key function | -|------|------|-------------| -| UI | `inst/shiny/modules/tab_nca/setup/data_imputation.R` | `data_imputation_server()` — returns `blq_imputation_rule`, `impute_c0`, `blq_strategy` | -| Settings assembly | `inst/shiny/modules/tab_nca/setup/settings.R#452-491` | `settings()` reactive — bundles `data_imputation` into the settings list | -| Data object update | `R/PKNCA.R#297` | `PKNCA_update_data_object(start_impute, blq_imputation_rule)` | -| Interval imputation | `R/intervals.R#215` | `update_main_intervals(impute, blq_imputation_rule)` — sets `data$intervals$impute` column | -| Start impute | `R/create_start_impute.R` | `create_start_impute()` — adds C0 strategies (`start_conc0`, `start_predose`, `start_logslope`, `start_c1`) | -| BLQ global function | `R/PKNCA.R#439` | `PKNCA_calculate_nca(blq_rule)` — registers `PKNCA_impute_method_blq` globally before calling `PKNCA::pk.nca()` | -| Obs param cleanup | `R/intervals.R#326` | `rm_impute_obs_params()` — removes imputation from non-AUC-dependent parameters | - -### Critical guard - -`update_main_intervals()` must ensure `data$intervals$impute` exists **before** the BLQ `dplyr::mutate()` block. If the column is missing and `impute` is also a function parameter (e.g. `FALSE` from YAML `impute_c0: no`), dplyr resolves to the parameter, producing `"blq, FALSE"` which PKNCA interprets as a lookup for `PKNCA_impute_method_FALSE`. The guard `if (!"impute" %in% names(data$intervals))` (see `rm_impute_obs_params()` line 341) prevents this. - -## Interval parameter helpers - -- **`rename_interval_params()`** (`R/utils.R:17`): Appends `_start-end` suffix to PPTESTCD for manual interval params (e.g. `AUCINT` → `AUCINT_0-24`). Called in `descriptive_statistics.R`, `parameter_plots.R`, `flexible_violinboxplot.R`. -- **`parse_interval_parameter()`** (`R/ratio_calculations.R:326`): Reverses the above — extracts `base`, `start`, `end`, `is_interval` from a suffixed name. Used when labels need to be resolved from `metadata_nca_parameters` (which only stores base PPTESTCDs). - -## YAML boolean caveat - -R's `yaml` package parses YAML 1.1 booleans: `yes`/`no`/`true`/`false`/`on`/`off` all become R logical values. When settings YAML values like `impute_c0: no` are parsed, they become logical `FALSE`, not the string `"no"`. Code that passes these values into `if()` conditions or dplyr expressions must handle logical values correctly — prefer `isTRUE()` / `isFALSE()` guards over bare `if (value)` when the value may be logical from YAML. - -## Testing conventions - -- Test files: `tests/testthat/test-.R`, matching source file name -- Use `describe()` / `it()` blocks with value-level assertions -- Test data created in `tests/testthat/setup.R` and `tests/testthat/data/` -- E2E tests in `test-e2e-runner.R` use `shinytest2` (in Suggests) - -## Package versioning and release - -- Semantic versioning. Bump `DESCRIPTION` version (+1 from main) for non-trivial changes -- `{admiral}` family follows quarterly releases (Mar/Jun/Sep/Dec); aNCA extensions should align within 2 weeks -- `NEWS.md` entries reference PR numbers - -## PR workflow summary - -1. `git fetch origin main && git checkout origin/main` — branch from latest upstream -2. Branch name: `-/` -3. After code changes: `document()` → `lint_package()` → `test()` → `check()` -4. Update `NEWS.md`, bump `DESCRIPTION` version -5. Push to fork, create PR against `pharmaverse/main` -6. Add 2+ reviewers from DESCRIPTION authors -7. Never force-push; use separate commits instead of amending From a124d0c45c1f45b79c82f4b965882c2c00b46c9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E5=B4=9D?= Date: Wed, 27 May 2026 17:16:27 +0800 Subject: [PATCH 04/11] chore: add CLAUDE.md to .gitignore to prevent accidental commits (#1266) Co-Authored-By: Claude Opus 4.7 --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index ba2ec833b..eb776aafc 100644 --- a/.gitignore +++ b/.gitignore @@ -15,4 +15,5 @@ inst/shiny/log/* .DS_Store # {shinytest2}: Ignore new debug snapshots for `$expect_values()` *_.new.png -desktop.ini \ No newline at end of file +desktop.ini +CLAUDE.md \ No newline at end of file From 50a283dc492f9b10510e7898e872f2ad57fe24f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E5=B4=9D?= Date: Wed, 27 May 2026 22:49:37 +0800 Subject: [PATCH 05/11] fix: resolve AGE type mismatch in ratio calculation joins (#1266) When settings YAML contains group columns like AGE in RefGroups (e.g. "AGE: 25"), the parsed values are character strings. Joining these with PKNCA result data (where AGE is numeric) triggers dplyr's strict type check: "Can't join due to incompatible types". Two fixes: 1. ratio_calculations.R: Add .coerce_group_types() to convert YAML character values to match the corresponding column types in data before merge() and anti_join() operations. 2. PKNCA.R: Restrict remove_pp_not_requested() group_by to only PKNCA-essential columns, excluding user keep_interval_cols (like AGE) that are irrelevant for parameter-request matching. Co-Authored-By: Claude Opus 4.7 --- R/PKNCA.R | 16 +++++++++++++++- R/ratio_calculations.R | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/R/PKNCA.R b/R/PKNCA.R index bc8e1caad..1251140c4 100644 --- a/R/PKNCA.R +++ b/R/PKNCA.R @@ -898,6 +898,20 @@ remove_pp_not_requested <- function(pknca_res) { pknca_res$data$intervals$impute <- NA_character_ } + # Build explicit match columns: PKNCA groups + interval identifiers only. + # Exclude user-specified keep_interval_cols (e.g. AGE, SEX) which are + # not needed for parameter-request matching and may cause type-mismatch + # errors when dplyr strict-join sees incompatible types across data frames. + pknca_groups <- unique(c( + group_vars(pknca_res$data$conc), + group_vars(pknca_res$data$dose) + )) + match_cols <- unique(c( + pknca_groups, + "start", "end", "ATPTREF", "DOSNOA", "type_interval" + )) + match_cols <- intersect(match_cols, names(pknca_res$data$intervals)) + # Reshape intervals, filter params_not_requested <- pknca_res$data$intervals %>% pivot_longer( @@ -906,7 +920,7 @@ remove_pp_not_requested <- function(pknca_res) { values_to = "is_requested" ) %>% mutate(PPTESTCD = translate_terms(PPTESTCD, "PKNCA", "PPTESTCD")) %>% - group_by(across(c(-impute, -is_requested))) %>% + group_by(across(all_of(c(match_cols, "PPTESTCD")))) %>% summarise( is_requested = any(is_requested), .groups = "drop" diff --git a/R/ratio_calculations.R b/R/ratio_calculations.R index d40378ca5..54585b985 100644 --- a/R/ratio_calculations.R +++ b/R/ratio_calculations.R @@ -130,6 +130,16 @@ calculate_ratios.data.frame <- function( ) group_cols <- setdiff(colnames(data), extra_res_cols) + # Parse group column values to match the types in the data. + # YAML-sourced settings store all values as character, but the + # corresponding columns in PKNCA results may be numeric (e.g. AGE). + # Without type coercion, merge() and anti_join() fail with + # "Can't join due to incompatible types". + ref_groups <- .coerce_group_types(ref_groups, data) + if (!is.null(test_groups)) { + test_groups <- .coerce_group_types(test_groups, data) + } + # Define the reference and test data based on the parameters and groups df_ref <- as.data.frame(data)[data$PPTESTCD == ref_parameter, , drop = FALSE] df_ref <- merge(df_ref, ref_groups) @@ -468,6 +478,34 @@ calculate_ratio_app <- function( list(test_groups = test_groups, ref_groups = ref_groups) } +#' Coerce group column types to match the corresponding columns in data. +#' +#' When group specifications are sourced from YAML settings, all values +#' are character strings. If the PKNCA result data has those columns as +#' numeric (e.g. AGE), joins will fail with incompatible type errors. +#' This helper converts group columns to match the data column types. +#' +#' @param groups A data.frame of group values (from `.parse_ratio_groups`). +#' @param data A data.frame with the PKNCA result data. +#' @returns A data.frame with column types coerced to match `data`. +#' @noRd +.coerce_group_types <- function(groups, data) { + for (col in intersect(names(groups), names(data))) { + groups[[col]] <- utils::type.convert(groups[[col]], as.is = TRUE) + data_type <- class(data[[col]])[1] + group_type <- class(groups[[col]])[1] + if (data_type == group_type) next + if (data_type == "factor") { + groups[[col]] <- factor(groups[[col]], levels = levels(data[[col]])) + } else if (data_type %in% c("numeric", "integer")) { + groups[[col]] <- as.numeric(groups[[col]]) + } else if (data_type == "character") { + groups[[col]] <- as.character(groups[[col]]) + } + } + groups +} + #' Filter result data to keep only the specific interval rows for interval parameters. #' #' For non-interval parameters, all rows are kept. For interval parameters, From c567e886d529488cad0c282e82259ce9efd83d53 Mon Sep 17 00:00:00 2001 From: Valentin Legras Date: Wed, 3 Jun 2026 09:17:20 +0000 Subject: [PATCH 06/11] Replace raw shinyjs::runjs with reset_reactable_memory() wrapper - slope_selector.R: use reset_reactable_memory() instead of inline JS - settings.R: add missing reset_reactable_memory() in .restore_non_filter_settings to prevent stale reactable.extras widget values on settings upload Co-authored-by: Ona --- inst/shiny/modules/tab_nca/setup/settings.R | 1 + inst/shiny/modules/tab_nca/setup/slope_selector.R | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/inst/shiny/modules/tab_nca/setup/settings.R b/inst/shiny/modules/tab_nca/setup/settings.R index ceda5ae39..0dd6e9a59 100644 --- a/inst/shiny/modules/tab_nca/setup/settings.R +++ b/inst/shiny/modules/tab_nca/setup/settings.R @@ -588,6 +588,7 @@ settings_server <- function(id, data, adnca_data, settings_override) { if (!is.null(settings$int_parameters)) { int_parameters(settings$int_parameters) + reset_reactable_memory() refresh_reactable(refresh_reactable() + 1) } diff --git a/inst/shiny/modules/tab_nca/setup/slope_selector.R b/inst/shiny/modules/tab_nca/setup/slope_selector.R index 7f3cfe0f1..87fd20a9d 100644 --- a/inst/shiny/modules/tab_nca/setup/slope_selector.R +++ b/inst/shiny/modules/tab_nca/setup/slope_selector.R @@ -262,7 +262,7 @@ slope_selector_server <- function( # nolint manual_slopes(click_result$manual_slopes) # render rectable anew # - shinyjs::runjs("memory = {};") # needed to properly reset reactable.extras widgets + reset_reactable_memory() refresh_reactable(refresh_reactable() + 1) }) From 354d7475f84323d0f613df43948937b499592f7b Mon Sep 17 00:00:00 2001 From: Valentin Legras Date: Wed, 3 Jun 2026 09:38:03 +0000 Subject: [PATCH 07/11] Add dplyr to WORDLIST for spellcheck CI Co-authored-by: Ona --- inst/WORDLIST | 1 + 1 file changed, 1 insertion(+) diff --git a/inst/WORDLIST b/inst/WORDLIST index fd3495e3b..170d2d562 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -169,6 +169,7 @@ devtools df dm doi +dplyr dropdowns extravascular frac From baf08004dcef5b3d88b97f47b80e874bb5da99f9 Mon Sep 17 00:00:00 2001 From: Valentin Legras Date: Wed, 3 Jun 2026 09:39:44 +0000 Subject: [PATCH 08/11] Reword NEWS entry to avoid dplyr spelling issue Co-authored-by: Ona --- NEWS.md | 2 +- inst/WORDLIST | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index e52d0b6a8..cad7ae434 100644 --- a/NEWS.md +++ b/NEWS.md @@ -79,7 +79,7 @@ * DOSNOA computation fixed for specimen-level grouping — urine-only data no longer gets incorrect dose numbering (#1116) * Dose-aware AUCint parameters now share the same PPTESTCD as their non-dose-aware counterparts in CDISC exports, with `PPANMETH` indicating the analytical method. Internal PPTESTCDs renamed from misleading `D` suffix (e.g. `AUCINTD`) to lowercase `da` suffix (e.g. `AUCINTda`). Fixed wrong PPTEST label for `AUCINTD` which said "Normalized by Dose" (#1242) * Optional settings (`slope_rules`, `int_parameters`, `ratio_table`) are now normalized to `NULL` when empty, instead of persisting as 0-row data frames throughout the app and settings pipeline (#1262) -* Fixed `PKNCA_impute_method_FALSE` error when YAML settings contain `impute_c0: no` — the `impute` column is now initialized before BLQ imputation is applied to prevent dplyr from resolving to the boolean function parameter (#1266) +* Fixed `PKNCA_impute_method_FALSE` error when YAML settings contain `impute_c0: no` — the `impute` column is now initialized before BLQ imputation to prevent name collision with the boolean function parameter (#1266) * Interval-specific parameters (`aucint.*`, `cav.int.*`) excluded from the Parameter Selection matrix — they require finite sub-intervals and must be configured via Partial Interval Calculations (#1309) ### Ratio Calculations diff --git a/inst/WORDLIST b/inst/WORDLIST index 170d2d562..fd3495e3b 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -169,7 +169,6 @@ devtools df dm doi -dplyr dropdowns extravascular frac From b8e371bb7840ee01080ea67bb6eb4b310cfbe3b3 Mon Sep 17 00:00:00 2001 From: Valentin Legras Date: Wed, 3 Jun 2026 09:41:10 +0000 Subject: [PATCH 09/11] Shorten inline comments Co-authored-by: Ona --- R/PKNCA.R | 6 ++---- R/intervals.R | 5 ++--- R/ratio_calculations.R | 6 +----- 3 files changed, 5 insertions(+), 12 deletions(-) diff --git a/R/PKNCA.R b/R/PKNCA.R index 1251140c4..729c6432e 100644 --- a/R/PKNCA.R +++ b/R/PKNCA.R @@ -898,10 +898,8 @@ remove_pp_not_requested <- function(pknca_res) { pknca_res$data$intervals$impute <- NA_character_ } - # Build explicit match columns: PKNCA groups + interval identifiers only. - # Exclude user-specified keep_interval_cols (e.g. AGE, SEX) which are - # not needed for parameter-request matching and may cause type-mismatch - # errors when dplyr strict-join sees incompatible types across data frames. + # Match on PKNCA groups + interval IDs only; exclude keep_interval_cols + # to avoid type-mismatch errors on joins. pknca_groups <- unique(c( group_vars(pknca_res$data$conc), group_vars(pknca_res$data$dose) diff --git a/R/intervals.R b/R/intervals.R index 517b527ef..362ebe558 100644 --- a/R/intervals.R +++ b/R/intervals.R @@ -290,9 +290,8 @@ update_main_intervals <- function( data <- create_start_impute(data) } - # Ensure impute column exists so dplyr mutate below references the column - # rather than the function parameter (which could be FALSE from YAML settings, - # causing "PKNCA_impute_method_FALSE" not found error). + # Prevent mutate() from resolving `impute` to the function parameter instead + # of the column (causes PKNCA_impute_method_FALSE error when impute=FALSE). if (!"impute" %in% names(data$intervals)) { data$intervals$impute <- NA_character_ } diff --git a/R/ratio_calculations.R b/R/ratio_calculations.R index 54585b985..0551b8d26 100644 --- a/R/ratio_calculations.R +++ b/R/ratio_calculations.R @@ -130,11 +130,7 @@ calculate_ratios.data.frame <- function( ) group_cols <- setdiff(colnames(data), extra_res_cols) - # Parse group column values to match the types in the data. - # YAML-sourced settings store all values as character, but the - # corresponding columns in PKNCA results may be numeric (e.g. AGE). - # Without type coercion, merge() and anti_join() fail with - # "Can't join due to incompatible types". + # Coerce YAML-sourced character values to match data column types for joins. ref_groups <- .coerce_group_types(ref_groups, data) if (!is.null(test_groups)) { test_groups <- .coerce_group_types(test_groups, data) From 1dcec38111642d33f0d20003e0fe857bef272c1e Mon Sep 17 00:00:00 2001 From: Valentin Legras Date: Wed, 3 Jun 2026 09:42:15 +0000 Subject: [PATCH 10/11] Remove CLAUDE.md from .gitignore Co-authored-by: Ona --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index eb776aafc..ba2ec833b 100644 --- a/.gitignore +++ b/.gitignore @@ -15,5 +15,4 @@ inst/shiny/log/* .DS_Store # {shinytest2}: Ignore new debug snapshots for `$expect_values()` *_.new.png -desktop.ini -CLAUDE.md \ No newline at end of file +desktop.ini \ No newline at end of file From be82ffe361b4bd5f90407ab73ace75357cb4151a Mon Sep 17 00:00:00 2001 From: Wang Zheng Date: Thu, 4 Jun 2026 12:08:34 +0800 Subject: [PATCH 11/11] test: add unit tests for .coerce_group_types() (#1266) --- tests/testthat/test-ratio_calculations.R | 46 ++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/testthat/test-ratio_calculations.R b/tests/testthat/test-ratio_calculations.R index 9ca2f4677..d8bc460a9 100644 --- a/tests/testthat/test-ratio_calculations.R +++ b/tests/testthat/test-ratio_calculations.R @@ -574,3 +574,49 @@ describe("calculate_ratio_app with interval parameters", { expect_true(nrow(ratios) > 0) }) }) + +describe(".coerce_group_types", { + it("coerces character column to numeric when data column is numeric", { + groups <- data.frame(AGE = "25", stringsAsFactors = FALSE) + data <- data.frame(AGE = 25, USUBJID = "S1") + result <- .coerce_group_types(groups, data) + expect_type(result$AGE, "double") + expect_equal(result$AGE, 25) + }) + + it("coerces character column to factor with correct levels when data column is factor", { + groups <- data.frame(RACE = "WHITE", stringsAsFactors = FALSE) + data <- data.frame( + RACE = factor(c("WHITE", "ASIAN", "BLACK")), + USUBJID = "S1", stringsAsFactors = FALSE + ) + result <- .coerce_group_types(groups, data) + expect_s3_class(result$RACE, "factor") + expect_setequal(levels(result$RACE), c("WHITE", "ASIAN", "BLACK")) + }) + + it("leaves already-matching types unchanged", { + groups <- data.frame(AGE = 25, RACE = "WHITE", stringsAsFactors = FALSE) + data <- data.frame(AGE = 30, RACE = "ASIAN", USUBJID = "S1", + stringsAsFactors = FALSE) + result <- .coerce_group_types(groups, data) + # AGE is already numeric → unchanged + expect_equal(result$AGE, 25) + # RACE is character, data$RACE is character → unchanged + expect_equal(result$RACE, "WHITE") + }) + + it("handles mixed columns: some coerced, some unchanged", { + groups <- data.frame(AGE = "25", RACE = "WHITE", SEX = "M", + stringsAsFactors = FALSE) + data <- data.frame(AGE = 30, RACE = "ASIAN", SEX = "F", USUBJID = "S1", + stringsAsFactors = FALSE) + result <- .coerce_group_types(groups, data) + # AGE: character → numeric + expect_type(result$AGE, "double") + expect_equal(result$AGE, 25) + # RACE, SEX: character → character (already matching) + expect_equal(result$RACE, "WHITE") + expect_equal(result$SEX, "M") + }) +})