Address PR #353 CI review round 4 (1 P1 + 2 P3)

P1 - stringified-label collision guard in stute_joint_pretest:
The core indexed residuals_arrays / fitted_arrays by `str(k)` with
no uniqueness check on the stringified keys. Two distinct raw keys
whose str() forms collide (e.g. {1: ..., "1": ...} both stringify
to "1", or custom objects with identical __str__) would silently
overwrite one entry and then be double-counted in S_joint =
sum(S_k) because the surviving horizon's statistic gets summed
twice while n_horizons still reports K=2. That produces wrong
methodology output with no diagnostic.

Fix: compute the stringified labels once up front and reject any
collision explicitly with a ValueError listing which raw keys
collide to which stringified form. Centralizes the check before
any residual/fitted array is dropped. Replaces the ad-hoc post-hoc
re-keying with a reuse of the pre-computed collision-free list.

P3 - dedupe staggered-filter UserWarning:
`_validate_had_panel_event_study` already warns on the staggered
auto-filter path; both joint-pretest wrappers and the event-study
workflow were re-emitting the same information with a
wrapper-prefixed message. Each staggered call therefore surfaced
two warnings to the user. Removes the secondary emissions;
wrappers now consume `_filter_info` silently. Existing tests still
pass because the validator's own `"Staggered-timing panel
detected"` message satisfies the regex matchers.

P3 - collision regression test: new
`TestStuteJointPretest::test_stringified_key_collision_raises`
exercises (a) the int 1 + str "1" case and (b) a pair of custom
objects with identical __str__ but distinct hash; both must raise
`ValueError` with "collision after str" in the message.

125 tests pass (124 + 1 new R4 collision regression); black/ruff/
mypy clean.

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

Author: igerber

diff_diff/had_pretests.py

@@ -1976,12 +1976,36 @@ def stute_joint_pretest(
     if not np.all(np.isfinite(X)):
         raise ValueError("design_matrix contains non-finite values (NaN/inf).")
 
-    horizon_labels = list(residuals_by_horizon.keys())
-    K = len(horizon_labels)
+    raw_horizon_labels = list(residuals_by_horizon.keys())
+    K = len(raw_horizon_labels)
+
+    # Stringified-label collision guard: distinct raw keys whose str()
+    # representations collide (e.g. {1: ..., "1": ..., 1.0: ...}) would
+    # overwrite each other in residuals_arrays / fitted_arrays, letting
+    # the surviving horizon be double-counted in S_joint = sum of S_k
+    # and leaving `n_horizons` inconsistent with the number of distinct
+    # diagnostic statistics. Reject explicitly rather than silently
+    # collapsing the test.
+    str_labels = [str(k) for k in raw_horizon_labels]
+    if len(set(str_labels)) != len(str_labels):
+        from collections import Counter
+
+        dup_strs = [s for s, c in Counter(str_labels).items() if c > 1]
+        collisions = {s: [k for k in raw_horizon_labels if str(k) == s] for s in dup_strs}
+        raise ValueError(
+            f"Horizon label collision after str() stringification: "
+            f"{collisions!r}. The joint Stute helpers index residuals "
+            f"and fitted values by str(label); distinct raw keys whose "
+            f"stringified form collides would silently overwrite each "
+            f"other and double-count the surviving horizon in S_joint. "
+            f"Use string-distinct horizon labels (e.g. 1997 and 1998 "
+            f'as int, or "1997" and "1998" as str; not both).'
+        )
+
     any_nan = False
     residuals_arrays: Dict[str, np.ndarray] = {}
     fitted_arrays: Dict[str, np.ndarray] = {}
-    for k in horizon_labels:
+    for k in raw_horizon_labels:
         eps_k = np.asarray(residuals_by_horizon[k], dtype=np.float64)
         fit_k = np.asarray(fitted_by_horizon[k], dtype=np.float64)
         if eps_k.shape != (G,) or fit_k.shape != (G,):
@@ -1997,8 +2021,9 @@ def stute_joint_pretest(
 
     # Re-key to str labels consistently (wrappers already pass str; direct
     # callers may pass int/object). String identity per the documented
-    # horizon_labels contract.
-    horizon_labels = [str(k) for k in horizon_labels]
+    # horizon_labels contract. The collision guard above ensures this
+    # stringification is injective on the provided keys.
+    horizon_labels = str_labels
 
     if any_nan:
         return StuteJointResult(
@@ -2242,7 +2267,7 @@ def joint_pretrends_test(
     n_periods = int(data[time_col].nunique())
     data_filtered: pd.DataFrame = data
     if n_periods >= 3:
-        F_val, t_pre_list, _t_post_list, data_filtered, filter_info = (
+        F_val, t_pre_list, _t_post_list, data_filtered, _filter_info = (
             _validate_had_panel_event_study(
                 data,
                 outcome_col=outcome_col,
@@ -2252,16 +2277,10 @@ def joint_pretrends_test(
                 first_treat_col=first_treat_col,
             )
         )
-        if filter_info is not None:
-            warnings.warn(
-                f"joint_pretrends_test: staggered panel auto-filtered to "
-                f"last cohort (F_last={filter_info['F_last']!r}, "
-                f"n_kept={filter_info['n_kept']}, "
-                f"n_dropped={filter_info['n_dropped']}). "
-                f"Paper Appendix B.2 prescription.",
-                UserWarning,
-                stacklevel=2,
-            )
+        # `_validate_had_panel_event_study` already emits its own
+        # `UserWarning` on the staggered-filter path; the wrapper
+        # consumes `_filter_info` silently to avoid duplicated console
+        # noise (R4 code-quality fix).
         # Subset invariants: the caller's base_period and pre_periods
         # must be pre-treatment periods under the validator's partition.
         if base_period not in t_pre_list:
@@ -2429,7 +2448,7 @@ def joint_homogeneity_test(
     n_periods = int(data[time_col].nunique())
     data_filtered: pd.DataFrame = data
     if n_periods >= 3:
-        F_val, t_pre_list, t_post_list, data_filtered, filter_info = (
+        F_val, t_pre_list, t_post_list, data_filtered, _filter_info = (
             _validate_had_panel_event_study(
                 data,
                 outcome_col=outcome_col,
@@ -2439,16 +2458,10 @@ def joint_homogeneity_test(
                 first_treat_col=first_treat_col,
             )
         )
-        if filter_info is not None:
-            warnings.warn(
-                f"joint_homogeneity_test: staggered panel auto-filtered "
-                f"to last cohort (F_last={filter_info['F_last']!r}, "
-                f"n_kept={filter_info['n_kept']}, "
-                f"n_dropped={filter_info['n_dropped']}). "
-                f"Paper Appendix B.2 prescription.",
-                UserWarning,
-                stacklevel=2,
-            )
+        # `_validate_had_panel_event_study` already emits its own
+        # `UserWarning` on the staggered-filter path; the wrapper
+        # consumes `_filter_info` silently to avoid duplicated console
+        # noise (R4 code-quality fix).
         if base_period not in t_pre_list:
             raise ValueError(
                 f"base_period={base_period!r} is not in the validated "
@@ -2615,26 +2628,18 @@ def did_had_pretest_workflow(
         )
 
     if aggregate == "event_study":
-        F, t_pre_list, t_post_list, data_filtered, filter_info = _validate_multi_period_panel(
+        F, t_pre_list, t_post_list, data_filtered, _filter_info = _validate_multi_period_panel(
             data,
             outcome_col=outcome_col,
             dose_col=dose_col,
             time_col=time_col,
             unit_col=unit_col,
             first_treat_col=first_treat_col,
         )
-        if filter_info is not None:
-            warnings.warn(
-                f"HAD event-study pre-test: staggered panel auto-"
-                f"filtered to last cohort "
-                f"(F_last={filter_info['F_last']!r}, "
-                f"n_kept={filter_info['n_kept']}, "
-                f"n_dropped={filter_info['n_dropped']}, "
-                f"dropped_cohorts={filter_info['dropped_cohorts']}). "
-                f"Paper Appendix B.2 prescription.",
-                UserWarning,
-                stacklevel=2,
-            )
+        # `_validate_multi_period_panel` delegates to
+        # `_validate_had_panel_event_study`, which already emits its own
+        # `UserWarning` on the staggered-filter path; we do NOT warn a
+        # second time here (R4 code-quality fix - single emission point).
 
         # Base period for both joint tests is the last pre-period
         # (paper convention: anchor at F-1 under natural time order).

tests/test_had_pretests.py

@@ -1575,6 +1575,64 @@ def test_constant_d_returns_nan_with_warning(self):
         assert set(result.per_horizon_stats.keys()) == set(resid.keys())
         assert all(np.isnan(v) for v in result.per_horizon_stats.values())
 
+    def test_stringified_key_collision_raises(self):
+        """R4 P1 regression: two raw keys whose str() representations
+        collide (e.g. int 1 and str '1', or int 1 and float 1.0) must
+        raise explicitly rather than silently overwrite one horizon in
+        the internal residuals_arrays map and double-count the survivor
+        in the sum-of-CvMs S_joint."""
+        G = 20
+        rng = np.random.default_rng(701)
+        d = rng.uniform(0.0, 1.0, G)
+        # int / str collision: str(1) == "1"
+        resid_int_str_collision = {
+            1: rng.normal(0.0, 1.0, G),
+            "1": rng.normal(0.0, 1.0, G),
+        }
+        fit_int_str_collision = {1: np.zeros(G), "1": np.zeros(G)}
+        with pytest.raises(ValueError, match="collision after str"):
+            stute_joint_pretest(
+                residuals_by_horizon=resid_int_str_collision,
+                fitted_by_horizon=fit_int_str_collision,
+                doses=d,
+                design_matrix=np.ones((G, 1)),
+                n_bootstrap=199,
+                seed=0,
+            )
+
+        # int / float collision: str(1) == "1" but str(1.0) == "1.0"
+        # so these actually don't collide. Test a real collision case:
+        # two different string representations of the same label.
+        # Python: str(True) == "True"; bool(1) == True but that's the
+        # same key. Use: str(None) == "None" collides if passed twice,
+        # but keys must be unique per dict. Safer: two equal-after-str
+        # object keys that were distinct before str conversion.
+        class _WeirdLabel:
+            def __init__(self, s):
+                self._s = s
+
+            def __str__(self):
+                return self._s
+
+            def __hash__(self):
+                return hash((id(self), self._s))
+
+        a = _WeirdLabel("horizon-1")
+        b = _WeirdLabel("horizon-1")  # same str, different object
+        assert a is not b
+        assert str(a) == str(b)
+        resid_obj_collision = {a: rng.normal(0.0, 1.0, G), b: rng.normal(0.0, 1.0, G)}
+        fit_obj_collision = {a: np.zeros(G), b: np.zeros(G)}
+        with pytest.raises(ValueError, match="collision after str"):
+            stute_joint_pretest(
+                residuals_by_horizon=resid_obj_collision,
+                fitted_by_horizon=fit_obj_collision,
+                doses=d,
+                design_matrix=np.ones((G, 1)),
+                n_bootstrap=199,
+                seed=0,
+            )
+
 
 class TestJointPretrendsTest:
     """Tests for :func:`joint_pretrends_test` data-in wrapper."""