fix: validate presence and shape at the HSJ/XFORM bridge

NEWS.md

@@ -88,6 +88,15 @@
   known rooting-sensitivity of HSJ scoring, which remains a separate, open
   issue.
 
+- Zero-length-branch collapse (`collapse = TRUE`) no longer disables itself
+  for an `inapplicable = "hsj"`/`"xform"` search whenever *no* hierarchy
+  block actually exists in that replicate -- previously it keyed on the
+  scoring mode alone.  This only affects `Resample()`, whose bootstrap and
+  jackknife replicates can drop every hierarchy block from a unit while
+  still passing a (now-empty) hierarchy config through; those replicates are
+  ordinary Fitch data and now collapse like any other.  A replicate that
+  retains any hierarchy block is unaffected.
+
 - `MaximizeParsimony(effort = )` replaces `strategy = `, which is removed (it
   was never released).  `effort` is a **relative** offset, not an absolute
   level: `0` (the default) accepts the amount of search the dataset's size and

src/ts_collapsed.cpp

@@ -21,8 +21,14 @@ void compute_collapsed_flags(
   // (all-zero flags == nothing collapses — the safe conservative outcome).
   // Falling back to the conservative flags is NOT sufficient: it is equally
   // blind to hierarchy/Sankoff support.  See red-team T-330.
-  if (ds.scoring_mode == ScoringMode::HSJ ||
-      ds.scoring_mode == ScoringMode::XFORM) return;
+  //
+  // Gate on whether hierarchy data actually EXISTS (T-408), not merely on
+  // scoring_mode: an HSJ/XFORM config with no hierarchy_blocks / sankoff_n_chars
+  // carries no topology-dependent support this kernel is blind to, so collapse
+  // is safe. Matches DataSet::topology_independent()'s predicate.
+  if ((ds.scoring_mode == ScoringMode::HSJ ||
+       ds.scoring_mode == ScoringMode::XFORM) &&
+      (!ds.hierarchy_blocks.empty() || ds.sankoff_n_chars > 0)) return;
 
   // If all characters were simplified away (total_words == 0), every binary
   // resolution ties at the same score: no internal branch carries support,
@@ -150,8 +156,12 @@ void compute_collapsed_flags_aggressive(
   // modes (all-zero flags).  Guarded independently of compute_collapsed_flags:
   // the has_na delegation at the bottom of this block only reaches it on NA
   // data, not the general HSJ/XFORM case.  See red-team T-330.
-  if (ds.scoring_mode == ScoringMode::HSJ ||
-      ds.scoring_mode == ScoringMode::XFORM) {
+  //
+  // Gate on hierarchy data presence, not scoring_mode alone (T-408); see
+  // compute_collapsed_flags() above for the rationale.
+  if ((ds.scoring_mode == ScoringMode::HSJ ||
+       ds.scoring_mode == ScoringMode::XFORM) &&
+      (!ds.hierarchy_blocks.empty() || ds.sankoff_n_chars > 0)) {
     collapsed.assign(tree.n_node, 0);
     return;
   }

src/ts_rcpp.cpp

@@ -1850,14 +1850,44 @@ static void unpack_hsj(Nullable<List> hsjConfig, ts::DataSet& ds) {
     ds.hsj_alpha = as<double>(hc["hsjAlpha"]);
     ds.scoring_mode = ts::ScoringMode::HSJ;
 
-    if (hc.containsElementNamed("hsjTipLabels") &&
-        !Rf_isNull(hc["hsjTipLabels"])) {
+    // hsjTipLabels must be present and non-NULL whenever HSJ is enabled:
+    // score_hierarchy_block() reads ds.tip_labels unconditionally once
+    // scoring_mode == HSJ, and that field is only populated inside this
+    // branch. `list(hsjTipLabels = NULL)` keeps the element name, so
+    // containsElementNamed() alone does not catch an empty tip_labels (T-398).
+    if (!hc.containsElementNamed("hsjTipLabels") ||
+        Rf_isNull(hc["hsjTipLabels"])) {
+      Rcpp::stop("hsjConfig$hsjTipLabels must be provided (non-NULL) whenever "
+                 "hsjConfig is supplied (which enables HSJ scoring).");
+    }
+
+    {
       IntegerMatrix tl = as<IntegerMatrix>(hc["hsjTipLabels"]);
       validate_hsj_tip_labels(tl, hsjAbsentState,
                               static_cast<int>(ds.token_states.size()),
                               ds.n_levels);
       int n_t = tl.nrow();
       int n_c = tl.ncol();
+      // hsjTipLabels must cover every block's primary/secondary character
+      // index: score_hierarchy_block() reads
+      // tip_labels[t * n_orig_chars + block.primary_char] (and likewise for
+      // secondaries) unconditionally once scoring_mode == HSJ, so a
+      // non-NULL but too-narrow matrix reads past ds.tip_labels the same
+      // way a NULL one did (T-398).
+      for (const ts::HierarchyBlock& block : ds.hierarchy_blocks) {
+        if (block.primary_char < 0 || block.primary_char >= n_c) {
+          Rcpp::stop("hsjConfig$hsjTipLabels has %d columns, but a hierarchy "
+                     "block's primary character index is %d",
+                     n_c, block.primary_char);
+        }
+        for (int sec : block.secondary_chars) {
+          if (sec < 0 || sec >= n_c) {
+            Rcpp::stop("hsjConfig$hsjTipLabels has %d columns, but a "
+                       "hierarchy block's secondary character index is %d",
+                       n_c, sec);
+          }
+        }
+      }
       ds.n_orig_chars = n_c;
       ds.tip_labels.resize(n_t * n_c);
       for (int t = 0; t < n_t; ++t) {
@@ -1900,6 +1930,16 @@ static void unpack_xform(Nullable<List> xformConfig,
       List rc = xf_list[ch];
       NumericMatrix cm = as<NumericMatrix>(rc["cost_matrix"]);
       int ns = ns_vec[ch];
+      // Validate cost matrix dimensions match the character's state count
+      // (mirrors the check ts_sankoff_test() already performs; T-397 —
+      // Rcpp's Matrix indexing never bounds-checks a mis-shaped-but-
+      // same-length matrix, so an unguarded read here silently scores
+      // garbage instead of erroring).
+      if (cm.nrow() != ns || cm.ncol() != ns) {
+        Rcpp::stop("xformChars[[%d]]$cost_matrix has dimensions %d x %d, but "
+                   "character %d has %d states (expected %d x %d)",
+                   ch + 1, cm.nrow(), cm.ncol(), ch + 1, ns, ns, ns);
+      }
       double* dst = ds.sankoff_cost_matrices.data() +
           static_cast<size_t>(ch) * max_ns * max_ns;
       for (int r = 0; r < ns; ++r)
@@ -1926,6 +1966,22 @@ static void unpack_xform(Nullable<List> xformConfig,
       IntegerMatrix combo_grid = as<IntegerMatrix>(rc["combo_grid"]);
       IntegerMatrix tip_sec = as<IntegerMatrix>(rc["tip_sec_known"]);
       int n_sec = combo_grid.ncol();
+      // combo_grid must carry one row per present state (states 1..ns-1);
+      // state == -2 below indexes it at (s - 1) for s up to ns - 1, so
+      // fewer rows than that reads out of bounds (T-397).
+      if (combo_grid.nrow() != ns - 1) {
+        Rcpp::stop("xformChars[[%d]]$combo_grid has %d rows, but character "
+                   "%d has %d states (expected %d rows)",
+                   ch + 1, combo_grid.nrow(), ch + 1, ns, ns - 1);
+      }
+      // tip_sec_known is read at (t, d) for t in [0, n_t), d in
+      // [0, n_sec) in the state == -2 branch below; a truncated matrix
+      // reads past the SEXP the same way an unguarded combo_grid would.
+      if (tip_sec.nrow() != n_t || tip_sec.ncol() != n_sec) {
+        Rcpp::stop("xformChars[[%d]]$tip_sec_known has dimensions %d x %d, "
+                   "but expected %d x %d (n_tips x n_secondaries)",
+                   ch + 1, tip_sec.nrow(), tip_sec.ncol(), n_t, n_sec);
+      }
       for (int t = 0; t < n_t; ++t) {
         int state = ts_r[t];
         double* tip_ptr = ds.sankoff_tip_costs.data() +
@@ -1950,6 +2006,15 @@ static void unpack_xform(Nullable<List> xformConfig,
           }
         } else if (state >= 0 && state < ns) {
           tip_ptr[state] = 0.0;
+        } else {
+          // Any other value (e.g. state >= ns) falls through every branch
+          // above, leaving tip_ptr all-INF; that INF then propagates through
+          // the pool sentinel (1e18) rather than a true Inf and passes
+          // is.finite(), silently corrupting the score instead of erroring
+          // (T-397).
+          Rcpp::stop("xformChars[[%d]]$tip_states[%d] = %d is out of range; "
+                     "must be -1, -2, or in [0, %d)",
+                     ch + 1, t + 1, state, ns);
         }
       }
     }