Address PR #346 CI review round 2: 2 P1s + 1 P2

**P1 #1 (Methodology): continuous_near_d_lower on mass-point samples**

When a user explicitly forced design="continuous_near_d_lower" on a
sample that actually satisfies the >2% modal-fraction mass-point
criterion, the downstream regressor shift (D - d_lower) would move the
support minimum to zero on the shifted scale. Phase 1c's mass-point
rejection guard only fires when d.min() > 0 (_validate_had_inputs), so
the silent coercion ran the nonparametric local-linear estimator on a
sample the paper (Section 3.2.4) requires to use the 2SLS branch,
producing the wrong estimand.

Fix: `HeterogeneousAdoptionDiD.fit()` now runs the modal-fraction
check on the ORIGINAL (unshifted) d_arr when the user explicitly
selects design="continuous_near_d_lower". If the fraction at d.min()
exceeds 2%, the fit raises ValueError pointing to design="mass_point"
or design="auto". design="auto" is unaffected (_detect_design already
correctly resolves such samples to mass_point).

**P1 #2 (Code Quality): first_treat_col validator not dtype-agnostic**

The previous validator called `.astype(np.float64)` and `int(v)` on
grouped first_treat values, which crashed on otherwise-supported
string-labelled two-period panels (period in {"A","B"}, first_treat
in {0, "B"}). Rewrote using `pd.isna()` for missingness and raw-value
set-membership against `{0, t_post}` with no numeric coercion.

**P2 (Maintainability): cluster-applied mass-point stored wrong vcov_type**

When cluster was supplied, `_fit_mass_point_2sls` unconditionally
switches to the CR1 cluster-robust sandwich, but the result object
stored the REQUESTED family ("hc1" or "classical") as `vcov_type`.
`summary()` rendered correctly via the cluster_name branch, but
`to_dict()` and downstream programmatic consumers saw the stale
requested label. Fixed: when cluster is supplied, `vcov_type` is
stored as `"cr1"` regardless of the requested family. Renamed the
local variable from `vcov_effective` to `vcov_requested` to separate
the input from the effective family. Updated the
`HeterogeneousAdoptionDiDResults.summary()` branch so the cluster
rendering still works with the new stored value.

**Tests added (+8 regression):**
- TestValidateHadPanel.test_first_treat_col_with_string_periods
- TestValidateHadPanel.test_first_treat_col_dtype_agnostic_rejects_invalid_string
- TestContinuousPathRejectsMassPoint (2 tests)
- TestMassPointClusterLabel (4 tests: cr1 stored when clustered, base
  family when unclustered, classical+cluster collapses to cr1,
  to_dict shows effective family)

Targeted regression: 126 HAD tests + 505 total across Phase 1 and
adjacent surfaces, all green.

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

Author: igerber

diff_diff/had.py

@@ -189,10 +189,15 @@ class HeterogeneousAdoptionDiDResults:
         ``"analytical_nonparametric"`` (continuous designs) or
         ``"analytical_2sls"`` (mass-point).
     vcov_type : str or None
-        Variance-covariance family used. ``None`` on continuous paths
-        (they use the CCT-2014 robust SE from Phase 1c, not the library's
-        ``vcov_type`` enum). Mass-point: one of ``"classical"`` or
-        ``"hc1"`` (CR1 when ``cluster`` is supplied).
+        Effective variance-covariance family used. ``None`` on continuous
+        paths (they use the CCT-2014 robust SE from Phase 1c, not the
+        library's ``vcov_type`` enum). Mass-point: ``"classical"`` or
+        ``"hc1"`` when ``cluster`` is not supplied, and ``"cr1"``
+        whenever ``cluster`` is supplied (cluster-robust CR1 is computed
+        regardless of the requested ``vcov_type`` because
+        classical/hc1 + cluster collapses to the same CR1 sandwich).
+        Downstream consumers reading ``result.to_dict()`` can inspect
+        this field directly to determine the effective SE family.
     cluster_name : str or None
         Column name of the cluster variable on the mass-point path when
         cluster-robust SE is requested. ``None`` otherwise.
@@ -269,9 +274,12 @@ def summary(self) -> str:
             lines.append(f"{'Strictly above d_lower:':<30} {self.n_above_d_lower:>20}")
         lines.append(f"{'Inference method:':<30} {self.inference_method:>20}")
         if self.vcov_type is not None:
-            label = self.vcov_type
             if self.cluster_name is not None:
+                # Cluster-robust (CR1): the stored vcov_type is already "cr1",
+                # but render with the cluster column for clarity.
                 label = f"CR1 at {self.cluster_name}"
+            else:
+                label = self.vcov_type
             lines.append(f"{'Variance:':<30} {label:>20}")
         if self.bandwidth_diagnostics is not None:
             bw = self.bandwidth_diagnostics
@@ -476,20 +484,24 @@ def _validate_had_panel(
     # domain check that catches typos and staggered-timing mix-ups; it
     # does NOT cross-validate first_treat against post-period dose
     # (D_{g, t_post} remains the primary signal). Extended cross-checks
-    # are queued for a follow-up PR.
+    # are queued for a follow-up PR. The check is DTYPE-AGNOSTIC: it uses
+    # pd.isna() for missingness and raw-value membership against
+    # {0, t_post} so that string-labelled periods (e.g., ("A", "B")) with
+    # first_treat in {0, "B"} are supported.
     if first_treat_col is not None:
-        ft = data.groupby(unit_col)[first_treat_col].first().to_numpy()
-        if np.any(np.isnan(ft.astype(np.float64, copy=False))):
+        ft_series = data.groupby(unit_col)[first_treat_col].first()
+        if bool(ft_series.isna().any()):
             raise ValueError(
                 f"first_treat_col={first_treat_col!r} contains NaN. "
                 f"Use 0 for never-treated units and t_post for treated."
             )
         valid_values = {0, t_post}
-        bad = [v for v in np.unique(ft) if int(v) not in valid_values]
+        observed = set(ft_series.unique().tolist())
+        bad = sorted(observed - valid_values, key=lambda x: str(x))
         if bad:
             raise ValueError(
                 f"first_treat_col={first_treat_col!r} contains value(s) "
-                f"{bad} outside the allowed set {{0, {t_post}}} for a "
+                f"{bad} outside the allowed set {{0, {t_post!r}}} for a "
                 f"two-period HAD panel. Staggered timing with multiple "
                 f"cohorts is Phase 2b."
             )
@@ -1042,6 +1054,34 @@ def fit(
         else:
             d_lower_val = float(d_lower_arg)
 
+        # ---- Original-scale mass-point check before the regressor shift ----
+        # When a user explicitly forces design="continuous_near_d_lower"
+        # on a sample that is actually a mass-point sample (modal fraction
+        # at d.min() > 2% per paper Section 3.2.4), the downstream code
+        # would shift D - d_lower, making the support minimum zero on the
+        # shifted scale and suppressing the Phase 1c mass-point rejection
+        # guard (_validate_had_inputs only checks modal fraction when
+        # d.min() > 0). That silent coercion produces wrong CIs for a
+        # mass-point sample by running the nonparametric estimator where
+        # the paper's 2SLS branch is required. Front-door reject.
+        if resolved_design == "continuous_near_d_lower":
+            d_min_orig = float(d_arr.min())
+            if d_min_orig > 0:
+                eps_mp = 1e-12 * max(1.0, abs(d_min_orig))
+                at_d_min_mask_orig = np.abs(d_arr - d_min_orig) <= eps_mp
+                modal_fraction_orig = float(np.mean(at_d_min_mask_orig))
+                if modal_fraction_orig > _MASS_POINT_THRESHOLD:
+                    raise ValueError(
+                        f"design='continuous_near_d_lower' cannot be used on a "
+                        f"mass-point sample (modal fraction {modal_fraction_orig:.4f} "
+                        f"at d.min()={d_min_orig!r} exceeds the "
+                        f"{_MASS_POINT_THRESHOLD:.2f} threshold from paper Section "
+                        f"3.2.4). Use design='mass_point' (Wald-IV / 2SLS) or "
+                        f"design='auto' which will auto-detect. Forcing the "
+                        f"continuous path on a mass-point sample would produce "
+                        f"the wrong estimand."
+                    )
+
         # ---- d_lower contract for Design 1 paths ----
         # Paper Sections 3.2.2-3.2.4 define the Design 1 estimators at
         # d_lower = support infimum of D_{g,2}. For the mass-point path,
@@ -1148,21 +1188,25 @@ def fit(
         elif resolved_design == "mass_point":
             if vcov_type_arg is None:
                 # Backward-compat: robust=True -> hc1, robust=False -> classical.
-                vcov_effective = "hc1" if robust_arg else "classical"
+                vcov_requested = "hc1" if robust_arg else "classical"
             else:
-                vcov_effective = vcov_type_arg.lower()
-            # Explicit cluster + hc1 becomes CR1 inside _fit_mass_point_2sls.
+                vcov_requested = vcov_type_arg.lower()
             att, se = _fit_mass_point_2sls(
                 d_arr,
                 dy_arr,
                 d_lower_val,
                 cluster_arr,
-                vcov_effective,
+                vcov_requested,
             )
             bc_fit = None
             bw_diag = None
             inference_method = "analytical_2sls"
-            vcov_label = vcov_effective
+            # Store the EFFECTIVE variance family so downstream consumers
+            # (to_dict, to_dataframe, summary) see the actual SE type that
+            # was computed. When cluster is supplied, _fit_mass_point_2sls
+            # unconditionally computes CR1 regardless of vcov_requested
+            # (e.g. classical+cluster -> CR1), so we surface that here.
+            vcov_label = "cr1" if cluster_arg is not None else vcov_requested
             cluster_label = cluster_arg if cluster_arg is not None else None
         else:
             raise ValueError(f"Internal error: unhandled design={resolved_design!r}.")

tests/test_had.py

@@ -1404,3 +1404,118 @@ def test_rejects_string_periods_gracefully(self):
         t_pre, t_post = _validate_had_panel(panel, "outcome", "dose", "period", "unit", None)
         assert t_pre == "A"
         assert t_post == "B"
+
+    def test_first_treat_col_with_string_periods(self):
+        """Review P1: first_treat_col validator must be dtype-agnostic.
+
+        With string periods ("A", "B") and first_treat_col values in
+        {0, "B"}, the validator must not attempt numeric coercion.
+        """
+        d, dy = _dgp_continuous_at_zero(100, seed=0)
+        panel = _make_panel(d, dy, periods=("A", "B"))
+        # 50 units never-treated (first_treat=0), 50 treated (first_treat="B")
+        ft_unit = np.array([0 if i % 2 == 0 else "B" for i in range(100)], dtype=object)
+        panel["ft"] = np.repeat(ft_unit, 2)
+        t_pre, t_post = _validate_had_panel(panel, "outcome", "dose", "period", "unit", "ft")
+        assert t_pre == "A"
+        assert t_post == "B"
+
+    def test_first_treat_col_dtype_agnostic_rejects_invalid_string(self):
+        """Mix string periods + invalid first_treat_col string -> ValueError."""
+        d, dy = _dgp_continuous_at_zero(100, seed=0)
+        panel = _make_panel(d, dy, periods=("A", "B"))
+        # Invalid: "Z" is neither 0 nor "B"
+        ft_unit = np.array([0 if i % 2 == 0 else "Z" for i in range(100)], dtype=object)
+        panel["ft"] = np.repeat(ft_unit, 2)
+        with pytest.raises(ValueError, match="first_treat_col"):
+            _validate_had_panel(panel, "outcome", "dose", "period", "unit", "ft")
+
+
+# =============================================================================
+# Review P1: continuous_near_d_lower on a true mass-point sample rejects
+# =============================================================================
+
+
+class TestContinuousPathRejectsMassPoint:
+    """Explicit override to continuous_near_d_lower on a mass-point sample
+    must raise before the regressor shift, otherwise the Phase 1c
+    mass-point guard (which fires only on d.min() > 0) is bypassed.
+    """
+
+    def test_continuous_near_on_mass_point_sample_raises(self):
+        d, dy = _dgp_mass_point(500, seed=0)
+        panel = _make_panel(d, dy)
+        est = HeterogeneousAdoptionDiD(design="continuous_near_d_lower")
+        with pytest.raises(ValueError, match=r"mass-point sample|mass_point"):
+            est.fit(panel, "outcome", "dose", "period", "unit")
+
+    def test_continuous_near_on_continuous_sample_runs(self):
+        """Sanity: the pre-shift check does NOT reject valid continuous samples."""
+        d, dy = _dgp_continuous_near_d_lower(500, seed=0)
+        panel = _make_panel(d, dy)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", UserWarning)
+            r = HeterogeneousAdoptionDiD(design="continuous_near_d_lower").fit(
+                panel, "outcome", "dose", "period", "unit"
+            )
+        assert np.isfinite(r.att)
+
+
+# =============================================================================
+# Review P2: cluster-applied mass-point stores vcov_type="cr1"
+# =============================================================================
+
+
+class TestMassPointClusterLabel:
+    def test_cluster_stores_cr1(self):
+        """to_dict() / downstream consumers see 'cr1' not 'hc1' when clustered."""
+        d, dy = _dgp_mass_point(200, seed=0)
+        cluster_unit = np.repeat(np.arange(50), 4)
+        panel = _make_panel(d, dy, extra_cols={"state": cluster_unit})
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", UserWarning)
+            r = HeterogeneousAdoptionDiD(design="mass_point", cluster="state").fit(
+                panel, "outcome", "dose", "period", "unit"
+            )
+        assert r.vcov_type == "cr1"
+        assert r.cluster_name == "state"
+
+    def test_no_cluster_stores_base_family(self):
+        """Unclustered mass-point keeps 'hc1' or 'classical' label."""
+        d, dy = _dgp_mass_point(200, seed=0)
+        panel = _make_panel(d, dy)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", UserWarning)
+            r_hc1 = HeterogeneousAdoptionDiD(design="mass_point", vcov_type="hc1").fit(
+                panel, "outcome", "dose", "period", "unit"
+            )
+            r_cl = HeterogeneousAdoptionDiD(design="mass_point", vcov_type="classical").fit(
+                panel, "outcome", "dose", "period", "unit"
+            )
+        assert r_hc1.vcov_type == "hc1"
+        assert r_cl.vcov_type == "classical"
+
+    def test_cluster_with_classical_collapses_to_cr1(self):
+        """classical + cluster is CR1 in practice; label reflects that."""
+        d, dy = _dgp_mass_point(200, seed=0)
+        cluster_unit = np.repeat(np.arange(50), 4)
+        panel = _make_panel(d, dy, extra_cols={"state": cluster_unit})
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", UserWarning)
+            r = HeterogeneousAdoptionDiD(
+                design="mass_point", vcov_type="classical", cluster="state"
+            ).fit(panel, "outcome", "dose", "period", "unit")
+        assert r.vcov_type == "cr1"
+
+    def test_to_dict_shows_effective_family(self):
+        d, dy = _dgp_mass_point(200, seed=0)
+        cluster_unit = np.repeat(np.arange(50), 4)
+        panel = _make_panel(d, dy, extra_cols={"state": cluster_unit})
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", UserWarning)
+            r = HeterogeneousAdoptionDiD(design="mass_point", cluster="state").fit(
+                panel, "outcome", "dose", "period", "unit"
+            )
+        result_dict = r.to_dict()
+        assert result_dict["vcov_type"] == "cr1"
+        assert result_dict["cluster_name"] == "state"