Address CI review: fix logit coef expansion, reject string masks

P1 fixes from CI AI review (PR #238):
- solve_logit(): track original column count and expand returned beta
  back to p+1 length after effective-sample column dropping. Previously
  returned a shortened vector breaking the solver contract.
- subpopulation(): reject string/object masks that would silently coerce
  non-empty strings to True, defining the wrong domain.
- REGISTRY.md: add Note entries for estimator-level replicate limitations
  (SunAbraham rejection, CS/ContinuousDiD/EfficientDiD bootstrap rejection)

Tests: assert beta length p+1 after zero-weight rank-deficient solve,
assert string mask raises ValueError.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: igerber

diff_diff/linalg.py

@@ -1186,6 +1186,10 @@ def solve_logit(
             f"got '{rank_deficient_action}'"
         )
 
+    # Track original column count for coefficient expansion at the end
+    k_original = X_with_intercept.shape[1]
+    eff_dropped_original: list = []  # indices in original column space
+
     # Validate effective weighted sample when weights have zeros
     if weights is not None and np.any(weights == 0):
         pos_mask = weights > 0
@@ -1207,8 +1211,7 @@ def solve_logit(
                 f"{X_eff.shape[1]} parameters. Cannot identify logistic model."
             )
         # Check rank deficiency on positive-weight rows — full design may
-        # be full rank due to zero-weight padding. Use effective-sample rank
-        # result to drive column dropping (same flow as full-sample check).
+        # be full rank due to zero-weight padding.
         eff_rank_info = _detect_rank_deficiency(X_eff)
         if len(eff_rank_info[1]) > 0:
             n_dropped_eff = len(eff_rank_info[1])
@@ -1226,13 +1229,12 @@ def solve_logit(
                     UserWarning,
                     stacklevel=2,
                 )
-            # Use the effective-sample rank info for column dropping
-            # (overrides the full-sample check below which may show no deficiency)
-            _eff_rank, _eff_dropped, _eff_pivot = eff_rank_info
-            X_with_intercept = np.delete(X_with_intercept, _eff_dropped, axis=1)
+            # Drop columns and track original indices for final expansion
+            eff_dropped_original = list(eff_rank_info[1])
+            X_with_intercept = np.delete(X_with_intercept, eff_rank_info[1], axis=1)
             k = X_with_intercept.shape[1]
 
-    # Check rank deficiency once before iterating
+    # Check rank deficiency once before iterating (on possibly-shrunk matrix)
     rank_info = _detect_rank_deficiency(X_with_intercept)
     rank, dropped_cols, _ = rank_info
     if len(dropped_cols) > 0:
@@ -1328,10 +1330,20 @@ def solve_logit(
                 stacklevel=2,
             )
 
-    # Expand beta back to full size if columns were dropped
-    if len(dropped_cols) > 0:
-        beta_full = np.zeros(k)
-        beta_full[kept_cols] = beta_solve
+    # Expand beta back to original column count, accounting for columns
+    # dropped in both the effective-sample check and the full-sample check
+    if len(dropped_cols) > 0 or len(eff_dropped_original) > 0:
+        # First expand from X_solve columns back to post-eff-drop columns
+        beta_post_eff = np.zeros(k)
+        beta_post_eff[kept_cols] = beta_solve
+
+        # Then expand from post-eff-drop columns back to original columns
+        if len(eff_dropped_original) > 0:
+            beta_full = np.zeros(k_original)
+            kept_original = [i for i in range(k_original) if i not in eff_dropped_original]
+            beta_full[kept_original] = beta_post_eff
+        else:
+            beta_full = beta_post_eff
     else:
         beta_full = beta_solve
 

diff_diff/survey.py

@@ -432,6 +432,18 @@ def subpopulation(
                     "Subpopulation mask contains None/NA values. "
                     "Provide a boolean mask with no missing values."
                 )
+            # Reject string/object masks — non-empty strings coerce to True
+            # which silently defines the wrong domain
+            if any(isinstance(v, str) for v in raw_mask):
+                raise ValueError(
+                    "Subpopulation mask has object dtype with string values. "
+                    "Provide a boolean or numeric (0/1) mask, not strings."
+                )
+        if hasattr(raw_mask, 'dtype') and raw_mask.dtype.kind in ('U', 'S'):
+            raise ValueError(
+                "Subpopulation mask contains string values. "
+                "Provide a boolean or numeric (0/1) mask."
+            )
         mask_arr = raw_mask.astype(bool)
 
         if len(mask_arr) != len(data):

docs/methodology/REGISTRY.md

@@ -2021,7 +2021,14 @@ variance from the distribution of replicate estimates.
 - **Note:** JKn requires explicit `replicate_strata` (per-replicate stratum
   assignment). Auto-derivation from weight patterns is not supported.
 - **Note:** Invalid replicate solves (singular/degenerate) are dropped with
-  a warning. Variance is computed from valid replicates only.
+  a warning. Variance is computed from valid replicates only. Fewer than 2
+  valid replicates returns NaN variance.
+- **Note:** SunAbraham rejects replicate-weight designs with
+  `NotImplementedError` because the weighted within-transformation must be
+  recomputed per replicate (not yet implemented).
+- **Note:** CallawaySantAnna, ContinuousDiD, and EfficientDiD reject
+  replicate weights with `n_bootstrap > 0`. Replicate weights provide
+  analytical variance; bootstrap is a separate inference mechanism.
 
 ### DEFF Diagnostics (Phase 6)
 

tests/test_survey_phase6.py

@@ -744,7 +744,20 @@ def test_solve_logit_rank_deficient_positive_weight_subset_warn_mode(self):
             w[i] = 1.0
             X[i, 1] = 5.0
         with pytest.warns(UserWarning, match="rank-deficient"):
-            solve_logit(X, y, weights=w, rank_deficient_action="warn")
+            beta, probs = solve_logit(X, y, weights=w, rank_deficient_action="warn")
+        # Key assertion: beta must have original p+1 length (intercept + 2 covariates)
+        assert len(beta) == 3, (
+            f"Expected beta length 3 (p+1), got {len(beta)}. "
+            f"Column dropping broke the coefficient vector shape."
+        )
+
+    def test_subpopulation_string_mask_rejected(self, basic_did_data):
+        """Subpopulation mask with string values should be rejected."""
+        sd = SurveyDesign(weights="weight")
+        mask = ["yes"] * len(basic_did_data)
+        mask[0] = "no"
+        with pytest.raises(ValueError, match="string"):
+            sd.subpopulation(basic_did_data, mask)
 
     def test_replicate_if_no_divide_by_zero_warning(self):
         """compute_replicate_if_variance should not warn on zero weights."""