Closes #1061 - Increase code coverage to 90%; GEMINI did the work. 10% mor…

[jimrothstein]

Issue

Closes #1061

Description

Add tests to increase code coverage to 100% for PKNCA.R

Change description.

Definition of Done

Definition of done, preferably copied from the issue.

How to test

Run tests in Rstudio or IDE.

How to test features not covered by unit tests.

Contributor checklist

• Code passes lintr checks
• Code passes all unit tests
• New logic covered by unit tests
• New logic is documented
• App or package changes are reflected in NEWS
• Package version is incremented
• R script works with the new implementation (if applicable)
• Settings upload works with the new implementation (if applicable)

Notes to reviewer

Anything that the reviewer should know before tacking the pull request?

[jimrothstein]

@Gero1999

UPDATE

Read through vignette of package PKNCA::
This will take me awhile, but seems promising route to understand more of aNCA code.

original

• Still DRAFT
• Gemini did the work, but also true I do not understand the code.
• At times feels like wrestling with Gemini: it wants to make extra changes | wants to fix code done by others (not related to increasing coverage |
• As result, may need to remove or redo or simply trim Gemini code.
• And finally, not sure I am really helping. Please see note in Slack.

[Gero1999]

At times feels like wrestling with Gemini

Yes I can feel sometimes Gemini is doing its own thing here. I would recommend you to if possible understand first understand the code piece you wanna make a test for before asking Gemini so you can provide better instructions to it. The key is to know the inputs you wanna test and how deep to check the output (as a regular standard, to the value level)

And finally, not sure I am really helping. Please see note in Slack.

Hey, I saw the Slack message. I just want you to know that you are helping a lot, and your efforts here are incredibly valuable! It's completely natural to feel confused or overwhelmed at times, especially when you're delving into the most complex functions of the App!

This is all voluntary and you are already doing a lot. If you ever feel like you would prefer to take on different tasks that are not tests we can talk about it through Slack as well

---

With that said, even if it is a DRAFT I will provide some guidance comments just for the future PR development. But please don't feel forced to do them if you don't want to continue or are not having fun with it. Someone else can always take care of it! Let's keep the chat through Slack

Priority	Action
High	Move new describe() blocks out of add_exclusion_reasons to top level
High	Remove 2 of the 3 duplicate BLQ test blocks
High	Remove if (FALSE) dead code — convert to real test or delete
Medium	Remove all AI/attribution/line-number comments
Medium	Rename test descriptions to describe behavior, not implementation
Medium	Add meaningful assertions beyond expect_s3_class
Low	Remove unused blq_data variable on line 345
Low	Reuse simple_data instead of get_test_data()
Low	Make ---- section markers consistent or remove them

[jimrothstein]

@Gero1999

• Working through PKNCA vignettes. Can see what functions do to raw data. (ex: dose vs conc). Will take time.

[jimrothstein]

@Gero1999
Please do not comment yet. LLM did this and I want to go through it myself.
I just wanted to show I am back.

[jimrothstein]

@Gero1999
As I go through individual test sections beginning with describe

describe( .... , { }    )

I notice that some describe are not independent. A describe chunk may depend on earlier results.
It could be environment changed or, especially when I debug, an important earlier piece was not run.

Also, I always forget to run source("setup.R")

Thoughts on making each decribe more self-contained?
For example, each could begin with a function to run any needed prior code?

[jimrothstein]

@jimrothstein
note to SELF: remember:

1. see this comment (https://github.com/pharmaverse/aNCA/pull/1080/changes#r2941821439)
2. repeated check of BLQ ?

[jimrothstein]

@Gero1999
Stuck, still at 98%

• Working on testing this function in R/PKNCA.R:

PKNCA_hl_rules_exclusion <- function(res, rules) { # nolint
  for (param in names(rules)) {
    if (startsWith(param, "AUCPE")) {

• Note param must start with "AUCPE" (all caps)
• However, the current testing code uses "aucpext.obs" (lower case) and so the if returns FALSE
• If I change "aucpext.obs" to upper case, errors are thrown and R stops.

From code coverage report

697		
#' Exclude NCA results based on user-defined rules over the half-life related parameters
698		
#' This function applies exclusion rules to the NCA results based on user-defined parameters.
699		
#' @param res A PKNCAresults object containing the NCA results.
700		
#' @param rules A list of exclusion rules where each rule is a named vector.
701		
#' @returns A PKNCAresults object with the exclusions applied.
702		
#' @details
703		
#' The function iterates over the rules and applies the exclusion criteria to the NCA results.
704		
#' For any parameter that is not aucpext.obs or aucpext.pred it applies a minimum threshold,
705		
#' and for aucpext.obs and aucpext.pred it applies a maximum threshold.
706		
#' @importFrom PKNCA exclude
707		
#' @export
708		
PKNCA_hl_rules_exclusion <- function(res, rules) { # nolint
709	6x	
  for (param in names(rules)) {
710	6x	
    if (startsWith(param, "AUCPE")) {   <<--------------------------------GOAL:  run lines 711-718
711	!	
      exc_fun <- PKNCA::exclude_nca_by_param(
712	!	
        param,
713	!	
        max_thr = rules[[param]],
714	!	
        affected_parameters = translate_terms(
715	!	
          PKNCA::get.parameter.deps(
716	!	
            translate_terms(gsub("PE", "IF", param), "PPTESTCD", "PKNCA")
717		
          )
718		
        )
719		
      )
720		
    } else {
721	6x	
      exc_fun <- PKNCA::exclude_nca_by_param(

LLM says:

1. Modify the R/PKNCA.R code to use case-insensitive comparison (e.g., startsWith(toupper(param), "AUCPE")) to make the test pass
2. Try a different approach to trigger the branch without causing errors
3. Leave as-is (not 100% coverage)

[jimrothstein]

@Gero1999
Removed some duplicate code. Still at 98.05%
More cleanup to do, but mostly stuck at "exclusions - acupext.obs"