From 342f3827428d96bec0aea56ad5eb1600730eecc6 Mon Sep 17 00:00:00 2001 From: Doug Manuel Date: Sun, 27 Sep 2026 16:29:19 -0400 Subject: [PATCH] Scope legacy variable selection by variable-level databaseStart Found in a second release review of 0.5.0. The legacy path chose which variables to generate from detail rows alone and ignored variable-level databaseStart, which the v0.4 adapter honours. So a formula or survival variable listed only for another database made the new guards stop generation for the requested one, and a plain variable listed only for another database was generated as random values, in every detail layout tested (no detail databaseStart, shared rows, other-only rows). The legacy path now filters enabled variables by variable-level databaseStart with the adapter's matcher and empty-means-all rule, before the guards run. Scoping only the guards' candidates would have removed the false stop but left the legacy path generating another database's formula variable as random values. This also corrects an earlier ruling that the survival guard's lack of databaseStart scoping could stand. --- NEWS.md | 7 +++ R/create_mock_data.R | 9 ++++ tests/testthat/test-create-mock-data-v04.R | 63 ++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/NEWS.md b/NEWS.md index 690e2e4..6775872 100644 --- a/NEWS.md +++ b/NEWS.md @@ -43,6 +43,13 @@ `create_mock_data()` stops and names the formula variables. Previously the legacy generator ignored the formula and returned unrelated random values, or dropped a `DerivedVar::` variable, without saying so. +- On the legacy path (`validate = FALSE` and the other legacy triggers), + `create_mock_data()` now generates only variables whose variable-level + `databaseStart` includes the requested database, as the v0.4 pipeline + does. Previously it chose variables from the detail rows alone, so a + variable listed only for another database was generated as random values, + and a formula or survival variable belonging to another database stopped + the run. ## New features diff --git a/R/create_mock_data.R b/R/create_mock_data.R index 0dd6284..eeb1cfd 100644 --- a/R/create_mock_data.R +++ b/R/create_mock_data.R @@ -395,6 +395,15 @@ create_mock_data <- function(databaseStart, enabled_vars <- variables } + # Variable-level databaseStart lists the databases a variable belongs to. + # The v0.4 adapter honours it; the legacy path used only detail rows, so a + # variable listed only for another database was generated here as random + # values, and the formula and survival guards below stopped on it. Same + # matcher and empty-means-all rule as the adapter. + if ("databaseStart" %in% names(enabled_vars)) { + enabled_vars <- .filter_recodeflow_by_database(enabled_vars, databaseStart, allow_empty = TRUE) + } + # Formula variables are computed only by the v0.4 pipeline; the legacy # dispatcher ignores mockFormula and would return unrelated random values, # or drop a DerivedVar:: variable, without saying so. Checked before the diff --git a/tests/testthat/test-create-mock-data-v04.R b/tests/testthat/test-create-mock-data-v04.R index 8a09c5f..e69dca8 100644 --- a/tests/testthat/test-create-mock-data-v04.R +++ b/tests/testthat/test-create-mock-data-v04.R @@ -277,3 +277,66 @@ test_that("create_mock_data v0.4 and legacy paths are distributionally aligned", expect_type(attr(v04, "mockdata_diagnostics"), "list") expect_null(attr(legacy, "mockdata_diagnostics")) }) + +test_that("the legacy path generates only variables that belong to the requested database", { + # Release review of 0.5.0: the legacy path chose variables by detail rows + # and ignored variable-level databaseStart, which the v0.4 adapter honours. + # A variable listed only for "other" was generated for "study" (as random + # values), and the formula and survival guards stopped "study" on variables + # that do not belong to it. + run <- function(v, d) { + suppressWarnings(suppressMessages( + create_mock_data("study", v, d, n = 20, seed = 1, validate = FALSE) + )) + } + details_for <- function(variables, other_only) { + data.frame( + variable = variables$variable, + recStart = c("[0,1]", "[0,1]"), + recEnd = "copy", + proportion = 1, + databaseStart = c("study, other", other_only), + stringsAsFactors = FALSE + ) + } + plain <- data.frame( + variable = c("x", "z"), + variableType = "Continuous", + rType = "double", + role = "enabled", + databaseStart = c("study, other", "other"), + distribution = "uniform", + stringsAsFactors = FALSE + ) + # Shared detail rows and "other"-only detail rows behave the same. + for (z_rows in c("study, other", "other")) { + expect_identical(names(run(plain, details_for(plain, z_rows))), "x", info = z_rows) + + formula_details <- details_for(plain, z_rows) + formula_details$mockFormula <- c("", "x * 2") + expect_identical(names(run(plain, formula_details)), "x", info = z_rows) + } + + survival <- data.frame( + variable = c("entry", "death"), + variableType = "Date", + rType = "date", + role = "enabled", + databaseStart = c("study, other", "other"), + distribution = "uniform", + anchor = c("", "entry"), + followup_min = c(NA, 0), + followup_max = c(NA, 10), + event_prop = c(NA, 1), + stringsAsFactors = FALSE + ) + survival_details <- data.frame( + variable = "entry", + recStart = "[2001-01-01,2001-12-31]", + recEnd = "copy", + proportion = 1, + databaseStart = "study, other", + stringsAsFactors = FALSE + ) + expect_identical(names(run(survival, survival_details)), "entry") +})