Remove dead Rust compute_synthetic_weights (follow-up to PR #344)

[igerber]

Summary

• Deletes the orphaned Rust PyO3 compute_synthetic_weights binding and its internal PGD implementation from rust/src/weights.rs (86 lines).
• Unregisters it from rust/src/lib.rs and removes the obsolete constants (MAX_ITER, DEFAULT_TOL, DEFAULT_STEP_SIZE) that only serviced the deleted PGD code.
• Closes the Rust-side loop on silent-failures audit finding Implement CallawaySantAnna covariate adjustment for conditional parallel trends #22. The Python-side wrapper and its sole caller were removed in Delete compute_synthetic_weights shim; inline Frank-Wolfe in rank_control_units #344; this PR removes the dead Rust symbol it referenced.

Methodology references (required if estimator / math changes)

• Method name(s): N/A — dead-code removal only. The deleted PyO3 function had no Python callers after Delete compute_synthetic_weights shim; inline Frank-Wolfe in rank_control_units #344.
• Paper / source link(s): N/A
• Any intentional deviations from the source (and why): None.

Validation

• Tests added/updated: None (the deleted Rust symbol was only tested by one cargo test case — test_compute_weights_sum_to_one — deleted alongside the function it exercised; Python-side coverage of the replacement path is already in tests/test_prep.py::TestRankControlUnits).
• maturin develop --release --features accelerate: clean rebuild, no warnings.
• pytest tests/test_rust_backend.py tests/test_prep.py::TestRankControlUnits: 85 passed.
• Direct import verification: project_simplex / compute_sdid_unit_weights / compute_time_weights / sc_weight_fw still importable; compute_synthetic_weights correctly raises ImportError.
• cargo test --release on macOS hits the known PyO3 Python3.framework rpath issue unrelated to this change. cargo check is clean.

Security / privacy

• Confirm no secrets/PII in this PR: Yes.

---

Diff size: +1 / −126 across rust/src/weights.rs, rust/src/lib.rs, TODO.md.

[github-actions]

Overall Assessment

✅ Looks good

Executive Summary

• No supported estimator, weighting, or variance path changes are introduced. The registry-backed SDID implementation remains the Frank-Wolfe / two-pass synthdid match in docs/methodology/REGISTRY.md:L1438-L1566, and the retained Rust exports in rust/src/lib.rs:L31-L38 still align with that documented surface.
• The deletion is contained. The Python backend import layer already excludes compute_synthetic_weights in diff_diff/_backend.py:L18-L39, and remaining mentions are historical comments in diff_diff/utils.py:L1133-L1140 and diff_diff/prep.py:L1014-L1027.
• I do not see new empty-set, NaN/inference, control-group, parameter-propagation, or methodology-adherence risks in the changed code; this PR removes dead code rather than changing an active path.
• P3 informational: tests/test_rust_backend.py:L107-L110 still says the dead binding is tracked in TODO, which becomes stale once this PR lands.
• P3 informational: there is still no automated regression asserting that direct import of the removed private binding now fails.

Methodology

• No findings. Cross-check against docs/methodology/REGISTRY.md:L1438-L1566 shows the supported SDID method is the Frank-Wolfe / sparsified path, not the deleted legacy PGD helper.

Code Quality

• No findings. The cleanup is self-contained, and I did not find live Python callers or backend imports of the removed symbol beyond historical comments; see diff_diff/_backend.py:L18-L39.

Performance

• No findings. This is dead-code removal and is neutral-to-positive for binary size and maintenance cost.

Maintainability

• P3: tests/test_rust_backend.py:L107-L110 says the dead binding is still “tracked as a low-priority Rust cleanup TODO.” Impact: future readers will look for a tracker that this PR removes. Concrete fix: update or delete that comment so it says the cleanup is complete.

Tech Debt

• No findings. Removing the resolved TODO.md entry is appropriate and reduces tracked debt.

Security

• No findings. The PR only removes an orphaned private backend symbol and related constants/tests.

Documentation/Tests

• P3: There is no automated regression in tests/test_rust_backend.py:L103-L116 for the new intended behavior that from diff_diff._rust_backend import compute_synthetic_weights fails. Impact: a future accidental re-export would rely on manual validation to catch it. Concrete fix: add a negative import test asserting ImportError for that symbol.

[igerber]

/ai-review

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: ea381c83ea59659cd62735b704b6fa15bd4ea9f4

---

Overall Assessment

✅ Looks good

Re-review result: the prior informational issues are resolved, and I did not identify any new unmitigated P0/P1 findings in the changed code.

Executive Summary

• No supported estimator, weighting, variance, or identification path changes. The live SDID Rust surface still exposes the registry-backed Frank-Wolfe/two-pass implementation described in docs/methodology/REGISTRY.md:L1434-L1566 and exported in rust/src/lib.rs:L31-L38.
• The removed symbol was already off the Python package surface before this PR; the backend import layer only imports project_simplex, compute_sdid_unit_weights, compute_time_weights, compute_noise_level, and sc_weight_fw in diff_diff/_backend.py:L18-L39.
• Repo references to compute_synthetic_weights are now explanatory only, not live callers/imports, in diff_diff/utils.py:L1133-L1140, diff_diff/prep.py:L1014-L1027, tests/test_utils.py:L1127-L1129, and tests/test_estimators.py:L3127-L3129.
• The previous P3s are addressed: the stale comment is fixed and a negative regression test now guards against accidental re-export in tests/test_rust_backend.py:L107-L128.
• I do not see new empty-set, NaN/inference, control-group, parameter-propagation, or methodology-adherence risks because this PR removes a dead private binding rather than changing an active estimator path.

Methodology

• Severity: None. Impact: No live methodology path changed. Supported SDID unit/time weights still route through the Frank-Wolfe solver documented in docs/methodology/REGISTRY.md:L1434-L1566, implemented in rust/src/weights.rs:L349-L390 and rust/src/weights.rs:L507-L640, and consumed by the estimator in diff_diff/synthetic_did.py:L486-L505. The deleted PGD helper is not part of that registry-backed path. Concrete fix: None.

Code Quality

• Severity: None. Impact: The cleanup is internally consistent: the legacy binding is no longer registered in rust/src/lib.rs:L31-L38, the weights module now documents only the retained simplex/Frank-Wolfe/SDID functionality in rust/src/weights.rs:L1-L17, and the test suite now asserts the intended missing-symbol behavior in tests/test_rust_backend.py:L112-L128. Concrete fix: None.

Performance

• Severity: None. Impact: Dead-code removal is neutral-to-positive for compile/binary surface and does not touch any active hot path or numerical kernel used by supported estimators. Concrete fix: None.

Maintainability

• Severity: None. Impact: The prior stale-comment concern is resolved; the test commentary now accurately reflects that the Rust symbol was removed from both implementation and registration in tests/test_rust_backend.py:L107-L110. Concrete fix: None.

Tech Debt

• Severity: None. Impact: Removing the resolved compute_synthetic_weights cleanup item from TODO.md is appropriate; I do not see a remaining untracked follow-up created by this PR. Concrete fix: None.

Security

• Severity: None. Impact: The PR only removes a private extension binding and associated dead code, plus adds a regression test; no new secret exposure, unsafe input handling, or attack surface is introduced. Concrete fix: None.

Documentation/Tests

• Severity: None. Impact: The previous missing-regression-test concern is resolved by tests/test_rust_backend.py:L112-L128, which checks both from diff_diff._rust_backend import compute_synthetic_weights raising ImportError and hasattr(..., "compute_synthetic_weights") == False. Concrete fix: None.

Static-review note: I did not rerun pytest or cargo in this sandbox.