refactor: extract shared TROP bootstrap resampling loop

[igerber]

Summary

• Deduplicate the byte-identical per-draw "pairs bootstrap" resampling loop shared by TROP._bootstrap_variance (trop_local.py) and TROP._bootstrap_variance_global (trop_global.py) into a new module-level helper _run_trop_bootstrap_loop in trop_local.py (imported by trop_global.py, mirroring the existing _setup_trop_data pattern). The two loops differed only in the per-draw refit call, now passed as a fit_callable closure.
• Remove the resolved TROP bootstrap-dedup row from TODO.md.

This is a pure, behavior-preserving refactor: RNG setup and the Rust-backend dispatch stay inline (the helper is RNG-free; the stratified bootstrap indices are pre-generated before dispatch, so the Rust/Python cross-backend parity is preserved); the post-loop warnings — whose wording legitimately differs between local and global — and the np.std(ddof=1) SE computation stay inline in each method; and the closures resolve self._fit_*_with_fixed_lambda at call time, so the existing monkeypatching tests still intercept.

Methodology references

• Method name(s): N/A — no methodology change. TROP variance remains bootstrap-based exactly as before (unit-level pairs resampling, fixed-lambda refits, failure filtering, proportional-failure warnings, NaN SE on zero successful draws). The REGISTRY TROP section is unchanged.
• Paper / source link(s): N/A (no algorithm change; the TROP / Algorithm 3 bootstrap description is unchanged).
• Intentional deviations from the source: None. One documented implementation micro-detail: in the degenerate empty-pool branch, the empty-array dtype unifies to control_units.dtype. This branch is unreachable in a real fit (_setup_trop_data rejects an empty treated/control pool before the bootstrap runs), and the resulting boot_data is byte-identical regardless since f"{u}_{idx}" coerces every unit id to str.

Validation

• Bit-identical verified: local + global bootstrap SE and the full bootstrap distributions match exactly pre/post on a fixed seed (Python-fallback path), confirmed by a before/after capture.
• Tests: full tests/test_trop.py + tests/test_rust_backend.py::TestTROPRustEdgeCaseParity = 119 passed (including the atol=1e-14 global / 1e-5 local bootstrap seed-reproducibility parity tests and the NaN-SE / proportional-failure-rate guards). Added 6 direct unit tests for the helper (tests/test_trop.py::TestRunTropBootstrapLoop): resampling + renamed ids, finite filter, exception skip, non-convergence tracker passthrough, and the empty-pool branches.
• black / ruff / mypy clean on the changed lines (no new errors).

Security / privacy

• Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

[github-actions]

Overall Assessment

✅ Looks good — no unmitigated P0/P1 findings.

Executive Summary

• Affected methods: TROP local _bootstrap_variance and global _bootstrap_variance_global.
• Methodology check passes: the refactor preserves Algorithm 3-style treatment-stratified unit/block bootstrap, fixed-lambda refits, finite-draw filtering, failure warnings, zero-success NaN SE behavior, and np.std(..., ddof=1).
• No new inline inference anti-patterns or partial NaN guards found in the changed TROP paths.
• Added helper tests cover resampling/renaming, finite filtering, exception skipping, non-convergence tracker passthrough, and empty-pool branches.
• git diff --check passed. I could not run pytest because this environment lacks pytest and pandas.

Methodology

Finding: P3 informational — behavior-preserving TROP bootstrap extraction

• Impact: No methodology defect found. The registry requires TROP standard errors to use block bootstrap preserving panel structure and notes zero-success bootstrap SE should be NaN with downstream safe_inference() propagation. The changed code still pre-generates treatment-stratified bootstrap indices in each caller, resamples whole unit time series with replacement, refits at fixed lambda through the closure, filters non-finite estimates, warns on high failure rates, returns NaN for zero successful draws, and computes SE with ddof=1. See docs/methodology/REGISTRY.md:L2582-L2611, docs/methodology/REGISTRY.md:L2740-L2741, diff_diff/trop_local.py:L1114-L1225, diff_diff/trop_global.py:L914-L1028, and diff_diff/trop_local.py:L197-L254.
• Concrete fix: None required.

Code Quality

Finding: None

• Severity: N/A
• Impact: The extraction is scoped and reuses the existing trop_global.py importing-from-trop_local.py pattern. The helper’s call contract is explicit and keeps method-specific warnings/SE handling at the caller.
• Concrete fix: None required.

Performance

Finding: None

• Severity: N/A
• Impact: The helper does not add RNG work or change Rust dispatch. Python fallback still performs the same per-draw pd.concat loop as before.
• Concrete fix: None required.

Maintainability

Finding: P3 informational — TODO cleanup matches implemented work

• Impact: Removing the TROP bootstrap-dedup TODO is appropriate because the shared loop now exists at diff_diff/trop_local.py:L197-L254 and is used by both local and global fallback paths.
• Concrete fix: None required.

Tech Debt

Finding: None

• Severity: N/A
• Impact: No new deferred correctness issue was introduced. Remaining TROP TODO rows are separate Rust solver/RNG parity follow-ups, not caused by this PR.
• Concrete fix: None required.

Security

Finding: None

• Severity: N/A
• Impact: No secrets, unsafe deserialization, shell execution, or external I/O changes found in the diff.
• Concrete fix: None required.

Documentation/Tests

Finding: P3 informational — targeted tests added; local execution unavailable here

• Impact: The new TestRunTropBootstrapLoop coverage exercises the helper’s key behavior directly at tests/test_trop.py:L2868-L3027. I could not independently run it because /usr/bin/python has no pytest or pandas installed in this review environment.
• Concrete fix: None required for the PR; run the targeted TROP tests in CI or a dev environment with project dependencies installed.