From 5db39a001c619caf736ebf5bce8d3907a2b33570 Mon Sep 17 00:00:00 2001
From: Daniel Nachun <daniel.nachun@gmail.com>
Date: Sun, 11 Oct 2026 17:09:44 -0500
Subject: [PATCH] fix mashr interface

---
 R/mashPipeline.R                   | 19 +++++++++++++++++--
 tests/testthat/test_mashPipeline.R | 23 +++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/R/mashPipeline.R b/R/mashPipeline.R
index 6a97821f..9c9301fe 100644
--- a/R/mashPipeline.R
+++ b/R/mashPipeline.R
@@ -570,6 +570,17 @@ mashResidualCorrelation <- function(
     .mashResidCorNullBased(sumStatsList, inputScale, method, corArgs)
 }
 
+# The component contract is a named LIST of covariance matrices. c() collapses
+# two NULLs to NULL, which is exactly what a requested-but-empty component set
+# produces (flashier finding no factors on weak data, say), so normalise that
+# one case rather than handing a NULL to callers who were promised a list --
+# length()/names()/c() all keep working on an empty list, and "this component
+# ran and yielded nothing" stays distinguishable from a structural failure.
+# @noRd
+.mashComponentList <- function(x) {
+    if (is.null(x)) list() else x
+}
+
 #' @title Build mash Data-Driven Covariance Components
 #' @description Build the raw per-method data-driven covariance components
 #'   (\code{cov_canonical} / \code{cov_pca} / \code{cov_flash} /
@@ -638,7 +649,7 @@ mashCovarianceComponents <- function(
         components = components,
         nPcs = nPcs
     )
-    c(built$canonical, built$dataDriven)
+    .mashComponentList(c(built$canonical, built$dataDriven))
 }
 
 # `components` and `engine` each take a name or that choice's own
@@ -1057,7 +1068,11 @@ MashComponentParam <- function(
         )
     }
     list(
-        U = c(refined$U, comps$canonical),
+        # Same list contract as mashCovarianceComponents(). Defensive here
+        # rather than load-bearing: the mashr fit above aborts on an empty
+        # Ulist, so this return is not reached with zero components -- but the
+        # two component-returning entry points should not differ in shape.
+        U = .mashComponentList(c(refined$U, comps$canonical)),
         w = refined$w,
         loglik = refined$loglik
     )
diff --git a/tests/testthat/test_mashPipeline.R b/tests/testthat/test_mashPipeline.R
index 36387d83..97953bb0 100644
--- a/tests/testthat/test_mashPipeline.R
+++ b/tests/testthat/test_mashPipeline.R
@@ -1189,6 +1189,29 @@ test_that("mashCovarianceComponents builds a single requested component", {
     )))
 })
 
+test_that("a requested component that yields nothing returns an empty list", {
+    skip_if_not_installed("mashr")
+    ss <- mashFixture()
+    # flashier legitimately finds no factors on weak data. c(NULL, NULL) is
+    # NULL, so the collapse used to leak a NULL out of a function documented
+    # to return a named list -- length()/names()/c() then behave differently
+    # for callers, and "ran, found nothing" became indistinguishable from
+    # "nothing was built at all".
+    local_mocked_bindings(cov_flash = function(...) NULL, .package = "mashr")
+    out <- suppressMessages(suppressWarnings(
+        mashCovarianceComponents(
+            list(strong = ss),
+            alpha = 0,
+            vhat = diag(3),
+            components = "flash",
+            setSeed = 1L
+        )
+    ))
+    expect_type(out, "list")
+    expect_length(out, 0L)
+    expect_false(is.null(out))
+})
+
 test_that("mashCovarianceComponents default builds all non-udr components", {
     skip_if_not_installed("mashr")
     skip_if_not_installed("flashier")