Skip to content

Enhancement/Expansion of TLG Catalog - #1356

Open
h5hoang wants to merge 72 commits into
mainfrom
1343-enhancement-expand-tlg-catalog
Open

Enhancement/Expansion of TLG Catalog#1356
h5hoang wants to merge 72 commits into
mainfrom
1343-enhancement-expand-tlg-catalog

Conversation

@h5hoang

@h5hoang h5hoang commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Issue

Closes #1343

Description

Expands the TLG catalog with new table, graph, and listing functions for PK reports, following the insightsengineering TLG catalog specifications.

New TLG functions (26 new entries in tlg.yaml):

  • Tables (9): t_pkct01 (summary concentration table with 4 variants: by TRT, by dose, TAD, dose+TAD), t_pkpt03_col (summary PK parameters), t_pkpt03_MP_col (metabolite/parent ratios), t_pkpt07_norm (dose-normalized parameters), t_pkpt08_uri (urine amount/percent recovered), t_pkpt11_gmr (geometric mean ratio with CIs)
  • Graphs (12): g_pkcg01/02/03_sbs (side-by-side concentration plots), p_pkcg03_lin/log/sbs_dose (mean concentration by dose), p_pkpg01_cum/per (urine recovery profiles), p_pkpg02_doseprop (dose-proportionality scatter with power-model regression), p_pkpg03_boxp / p_pkpg04_boxp (PK parameter boxplots), p_pkpg06_mp (metabolite/parent ratio boxplots)
  • Listings (5): l_pkcl01_tad (concentration listing by TAD), l_pkcl02_uri (urine concentration listing), l_pkpl01 (individual PK parameters), l_pkpl01_mp (metabolite ratios), l_pkpl04_mp (treatment comparison)

Shared helpers (R/utils-tlg.R):

  • split_and_apply — splits data by grouping variables and applies a function to each subset (used by all new TLG functions)
  • filter_metabolite_rows — three-tier metabolite detection fallback (METABFL → PPCAT → PARAM)
  • .summarise_adpp — descriptive statistics for PK parameter values (n, Mean, SD, CV%, GeoMean, GeoCV%, Median, Min, Max)
  • .build_pkpp_table — deduplicates ADPP rows and applies summary statistics per stratum/parameter
  • .get_var_label — label attribute lookup with column-name fallback

Shiny wiring improvements:

  • Added type = "table" support to tlg_module_server / tlg_module_ui (tables render via reactable)
  • ADPP data now flows from tab_nca_servertab_tlg_server → modules via a dedicated adpp reactive, avoiding recomputation
  • filter_tlg_excluded applied once at the tab_tlg boundary (not per-module), and extended to handle ADPP's PPSUMFL flag alongside ADNCA's PKSUM1F
  • Fixed NA handling in filter_tlg_excluded — rows with NA in exclusion flag columns were silently dropped; now correctly preserved
  • Replaced random module ID suffixes with deterministic IDs + a registration environment to prevent duplicate Shiny observer accumulation on re-submit
  • Extracted .build_tlg_panels helper to eliminate copy-paste across table/graph/listing renderUI blocks

Other changes:

  • export_cdisc: carries DOSEA/DOSEU columns through to ADPP for dose-proportionality TLGs
  • Fixed g_pkcg02_sbs YAML entry pointing to g_pkcg02_log instead of g_pkcg02_sbs
  • Removed stray print(tlg_order()) debug statement in tab_tlg.R

Definition of Done

  • List of proposed new TLGs discussed and agreed
  • New graph functions implemented and registered in tlg.yaml
  • New table functions implemented and registered in tlg.yaml
  • Each new function has unit tests
  • All new TLGs render correctly in the app

How to test

  1. Run NCA on a dataset that includes multiple treatment arms and at least one analyte
  2. Navigate to the TLG tab, the new Tables sub-tab should be functional (previously a placeholder)
  3. Submit the default TLG selection and verify tables render as reactable widgets with pagination
  4. Enable non-default entries (dose-proportionality, urine, GMR, boxplots) and verify they render without error
  5. For ADPP-based TLGs: confirm that data reflects the NCA results (not stale or recomputed)
  6. Re-submit the TLG order multiple times, verify no duplicate observer accumulation (pagination buttons should fire exactly once per click)
  7. For urine TLGs (t_pkpt08_uri, p_pkpg01_cum/per, l_pkcl02_uri): test with a dataset containing urine specimens; verify the condition: "URINE" auto-selects these entries when urine data is present (don't forget to click urine in NCA set-up tab like me :'))

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)
  • If any .scss change was done, run data-raw/compile_css.R
  • If a package dependency was added/changed, run data-raw/test_suggests_hidden.R

Notes to reviewer

  • All new graph functions use direct ggplot2 (no tern dependency), per Enhancement: Remove tern dependency from TLG plots #1319.
  • The t_pkct01 summary function has its own .summarise_group rather than reusing .summarise_adpp. This is intentional because concentration tables have BLQ-specific semantics (BLQ counting, different n definition that includes BLQ subjects) that don't map cleanly to the ADPP helper.
  • The condition auto-selection for ADPP-based TLGs (e.g. t_pkpt08_uri with condition: "URINE") evaluates against ADNCA's PCSPEC column, not ADPP's PPSPEC. This works in practice (if ADNCA has urine specimens, ADPP should too) but could be tightened in a follow-up if needed.
  • The DOSEA column is now carried through export_cdisc for dose-proportionality graphs (p_pkpg02_doseprop, p_pkcg03_*_dose). If DOSEA is absent from the source data, these TLGs will show an error message at render time (caught by tryCatch in tlg_module_server). They are all is_default: false so they won't be pre-selected.

h5hoang added 23 commits June 5, 2026 11:51
@h5hoang h5hoang linked an issue Jun 18, 2026 that may be closed by this pull request
5 tasks
@h5hoang

h5hoang commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

thanks so much for the review @Gero1999! 😄 ive gone through everything from the last round:

  • there's now a "Compare in columns" option that repeats the full stat block per group level (e.g. Male / Female) under a two-level Group × Statistic header. It's on the pkct01 and pkpt03/07/08 summary tables (left pkpt11 GMR out since it's already an arm-vs-reference comparison). I also restricted that dropdown to sensible categorical/low-cardinality columns so folks can't accidentally pick something like AVAL or a units column, and picking a variable that's already used for the rows/splits gives a clear inline message instead of erroring out

  • the new tables + listings now expose the same right-sidebar options that the original l_pkcl01 had (titles, grouping/displayed variables, formatting, etc.), so they all flow through the same way. i also added a test that checks every option in tlg.yaml actually maps to a real function argument so this can't silently drift again

  • and you were right, the PKSUM1F/PPSUMFL flags are meant for summaries only, i scoped them to the tables (and mean plots), and the listings now show all records including excluded subjects, which matches their "*: subjects excluded from the summary table" footnote

all of this has new unit tests so lmk if there's anything else, thanks again!! 🙏

@h5hoang
h5hoang requested a review from Gero1999 July 13, 2026 21:59

@Gero1999 Gero1999 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oleeeee this looks amazing 😍

Sorry to keep asking, but can we actually also included something for the user to choose which statistics columns to show? (i.e, "n", "Number BLQ", "Mean", "SD", "CV%"...). Ifyou think is easy/clean feel free to do it direclty here

Regarding the code I just catched 1 relevant thing so far:

Comment thread inst/shiny/modules/tab_tlg/tlg_module.R Outdated
Comment on lines +21 to +29
filter_tlg_excluded <- function(data) {
if ("PKSUM1F" %in% names(data)) {
data[data$PKSUM1F != "Y", , drop = FALSE]
} else {
data
data <- data[is.na(data$PKSUM1F) | data$PKSUM1F != "Y", , drop = FALSE]
}
if ("PPSUMFL" %in% names(data)) {
data <- data[is.na(data$PPSUMFL) | data$PPSUMFL != "Y", , drop = FALSE]
}
data
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: maybe is better to be more specific for each (ADPP, ADNCA)

Imagine that someone annotates in their ADNCA some records for profile N, because they consider for that profile you cannot run NCA (PPSUMFL = Y). But they still want to produce representations with those plots (PKSUMFL = Y). This process will filter them anyway...

Hope this makes sense!

@h5hoang
h5hoang requested a review from Gero1999 July 17, 2026 20:52

@Gero1999 Gero1999 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes it works as expected!

There is only one thing I am not sure if I am missing. But right now I can see that by default pkpt03 is not separating the statistics by profile. Statistics only separate by using TRT01A, but not considering PARAM (which is relevant if we have for the same treatment more than 1 analyte) or PCSPEC (specimen collected). The best would be to allow this input to be multiple choice and by default select all grouping variables but USUBJID in the PKNCA obj.

On the other hand, for the Tables, I think we should try to figure out a way to make everything have the same inputs structure... Something more homogenous. An example, in pkpt03 we have "Columns - Parameter variable: PARAM, Value variable: AVAL"... but is actually the same as doing Stratify by (rows): TRT01A PARAM.

We also need for Tables to figure out a way to filter Stratify by (rows) values in a clean way, specially for PARAM (pkct03) and NFRLT (pkct01)


But all these we could either keep working here or create an issue and think about it together. So far, great job! 😉

Comment thread tests/testthat/test-tlg_module.R
Comment thread tests/testthat/test-tlg_option_modules.R
@h5hoang

h5hoang commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

thanks @Gero1999! i went ahead and did all three of these in the PR, plus the sidebar consistency thing that was mentioned, but here's a quick rundown:

  • stratification is multi-select now and defaults to all the grouping variables in the PKNCA obj except USUBJID. so pkpt03 splits by PARAM and PCSPEC by default now, not just TRT01A

  • i made the table inputs homogenous like you suggested. you were right that "Parameter variable" was basically just stratification, so i dropped it and PARAM is now one of the "Stratify by (rows)" vars. every summary table has the same layout now (Filtering, Grouping, Value, Compare, Statistics). i also folded the concentration tables' Time/Visit inputs into "Stratify by (rows)" and gave them a "Value variable" so the pkct and pkpt sidebars are identical

  • i added value filters for the stratify vars, theres a "Parameters to show (PARAM)" on the pkpt tables and a "Timepoints to show" on the pkct tables so you can pick which rows show up

  • and finally i left pkpt11 (GMR) as an exception since its columns are fixed (GMR, CIs) so Statistics/Compare dont really apply there

one heads up: the "by Dose" concentration tables can only group by whats actually in the conc data, and DOSEA is dose-event level (mostly NA per sample), so those tables just warn and group by the rest instead of erroring. can make a follow up issue if you want a cleaner way to do dose grouping.

but thanks again this was very helpful for me!! 🙏 lmk if anything else needs tweaking ^_^

@Gero1999

Copy link
Copy Markdown
Collaborator

@h5hoang great job! Points to mention:

  • pkpt08 is supposed to be only for urine, but I see you allow selection of other PPSPEC. I personally would not do this, as we know only urine samples have urine parameter calcualtions. In addition, I noticed that when you run the NCA with only 1 specimen (i.e, SERUM) this input will only allow you to select PPSPEC (not its internal values, but the column).

  • Love the error added to the urine tables when no URINE is run on the NCA. We may need one for the urine Listings, perhaps in a new issue/PR? This is the error:

[1] "Error: \033[38;5;232m\033[36mℹ\033[38;5;232m In argument: `Label = parse_annotation(data, Label)`.\033[39m\n\033[1mCaused by error:\033[22m\n\033[33m!\033[39m object 'Label' not found"

Otherwise I don't find anything else to comment, we can proceed with a merge 😉

image

@Shaakon35

Copy link
Copy Markdown
Collaborator

@h5hoang

Tables:

  • Good, I guess we will use the gtsummary package soon @Gero1999 when do we switch? maybe now?
  • If you add a column in "Split tables by" and "Stratify by" then the column appears only in the title, by not as a column.
  • For the future, we will need to integrate the default title, subtitles, and footnotes (and the option to change it in the sidebar)
  • Summary M/P does not work, it does not divide the metabolite/parent. This is tricky to set up so that it works, we need to use Metabolite flag and DOSETRT, e.g.
    Param = Drug A, DoseTRT=Drug A
    Param = Metab Drug A, DoseTRT= Drug A, METABFL="Y"
    Param = PD parameter, Dose TRT= Drug A, METABFL = ""
    in this case it's easy because we have only two records for a single DOSETRT, but we need to make sure that you don't have two Metablf for a singl DOSETRT.

With dummy data:
Error in [pkpt03 - Summary PK Parameters (stats in cols)]
[pkpt08 - Mean Urine Amount and Percent]
-> for pkpt03. I think it is because it's selected by default but not selected in the object. When I add all the TLGs and submit, it works.

Listings:

All good :)

  • just the option "displayed variables" disappears for some TLGs
  • in pkcl02, select in the filtering : URINE by default. and rename the name of the button : "Specimen Material Type" (which is the label of PCSPEC) instead of "Urine specimen values (PCSPEC)"
    @Gero1999 we miss ATPTREF in the ADPP dataset because we need it to add it in the table grouping varaibles of pkpl01 - pk parameters listing, in case the scientist runs a NCA on Cycle 1 and cycle 10, we need to keep the info.

With dummy data:
Error in [pkcl02 - Urine Concentration Listing]
[1] "Error: \033[38;5;255m\033[36mℹ\033[38;5;255m In argument: Label = parse_annotation(data, Label).\033[39m\n\033[1mCaused by error in is.factor():\033[22m\n\033[33m!\033[39m objet 'Label' introuvable"

Plots:

  • By default, USUBJID should not be selected in Grouping variables when we select pkcg02 - combined plot linear or log.
  • The X ticks does not work
  • Titles are missing for Combined plots
  • I cannot see any plot pkcg03 pkpg01

General comment:

  • Could you switch all the buttons of the size bar to pickerInput? (like in the exploration tab)

I think we should merge this one and then create 3 PRs: one of listings, one plots, one tables :)

@Gero1999

Copy link
Copy Markdown
Collaborator

Good, I guess we will use the gtsummary package soon @Gero1999 when do we switch? maybe now?

As agreed let's for now not use it, if we see that the users are not satisfied with the current options then we can consider including a new dependency

Summary M/P does not work, it does not divide the metabolite/parent. This is tricky to set up so that it works, we need to use Metabolite flag and DOSETRT, e.g.
Param = Drug A, DoseTRT=Drug A
Param = Metab Drug A, DoseTRT= Drug A, METABFL="Y"
Param = PD parameter, Dose TRT= Drug A, METABFL = ""
in this case it's easy because we have only two records for a single DOSETRT, but we need to make sure that you don't have two Metablf for a singl DOSETRT.

Why not simply dividing by PARAM? All metabolites and drugs can be distinguished that way right?

@Gero1999 we miss ATPTREF in the ADPP dataset because we need it to add it in the table grouping varaibles of pkpl01 - pk parameters listing, in case the scientist runs a NCA on Cycle 1 and cycle 10, we need to keep the info.

Isn't in the ADPP? Maybe is because of AVISIT, which is the proper one for CDISC? Maybe we need to check that and add it indeed

By default, USUBJID should not be selected in Grouping variables when we select pkcg02 - combined plot linear or log.

@h5hoang USUBJID has its own column in the PKNCA obj in case you want a clean way to remove it

@Shaakon35

Shaakon35 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary M/P does not work, it does not divide the metabolite/parent. This is tricky to set up so that it works, we need to use Metabolite flag and DOSETRT, e.g.
Param = Drug A, DoseTRT=Drug A
Param = Metab Drug A, DoseTRT= Drug A, METABFL="Y"
Param = PD parameter, Dose TRT= Drug A, METABFL = ""
in this case it's easy because we have only two records for a single DOSETRT, but we need to make sure that you don't have two Metablf for a singl DOSETRT.

Why not simply dividing by PARAM? All metabolites and drugs can be distinguished that way right?

I don't understand; give me a proper example, please.

@Gero1999 we miss ATPTREF in the ADPP dataset because we need it to add it in the table grouping varaibles of pkpl01 - pk parameters listing, in case the scientist runs a NCA on Cycle 1 and cycle 10, we need to keep the info.

Isn't in the ADPP? Maybe is because of AVISIT, which is the proper one for CDISC? Maybe we need to check that and add it indeed

I didn't find it when I opened ADPP in the app, Indeed we need AVISIT or ATPTREF, we need to check what is standard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhancement: Expand TLG catalog with new graph and table functions

5 participants