fix(rust): deterministic clustered vcov — first-appearance cluster aggregation

[igerber]

Summary

• Resolves the run-to-run nondeterminism discovered this session: Rust compute_robust_vcov (also reached via solve_ols(return_vcov=True)) built its (G, k) cluster-scores matrix in HashMap iteration order — SipHash-randomized per call. Mathematically identical, but the GEMM accumulation order changed on every invocation, wobbling the clustered vcov at ~1e-14 (3 distinct values across 8 identical calls; the Python backend is bit-stable; thread pinning had no effect, which pointed away from BLAS and at the map).
• Rows now accumulate in first-appearance order — ascending for the factorized 0..G-1 ids the Python dispatcher passes, matching NumPy's groupby order. Side benefit: the per-row to_owned + map-entry churn is gone (direct accumulation into the matrix).
• Verified: 1 distinct value across 50 identical calls (was 3-in-8); NumPy parity unchanged (~1e-14, the normal cross-backend GEMM tolerance per the repo's assert_allclose convention). Bit-identity regression tests cover contiguous ids (the dispatcher path) and non-contiguous unsorted ids (the direct-kernel path exercising the remap).
• The TODO row documenting this finding is currently in flight on perf(utils): precompute r-independent wild-bootstrap inversion pieces (~7x, outputs unchanged) #647; its deletion follows in the next TODO-touching PR once both merge (noted here to avoid a cross-PR conflict).

Methodology references (required if estimator / math changes)

• Method name(s): CR1 cluster-robust sandwich (no formula change — summation-order determinism only; Python numpy implementation is canonical per repo convention)
• Paper / source link(s): N/A — numerical implementation detail
• Any intentional deviations from the source (and why): None

Validation

• Tests added/updated: tests/test_rust_backend.py::TestClusterVcovDeterminism (2 bit-identity tests, 20 repeat-calls each).
• Evidence: 50-call probe → 1 unique value; tests/test_rust_backend.py + tests/test_linalg.py pass (253); cargo fmt/check clean.

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 method: CR1 cluster-robust sandwich vcov in Rust compute_robust_vcov_internal, reached by compute_robust_vcov and solve_ols(return_vcov=True).
• Methodology matches the registry/docstring contract: cluster meat remains sum_g score_g score_g' and CR1 adjustment remains (G/(G-1))*((N-1)/(N-k)).
• The change is a deterministic accumulation-order implementation detail, not a formula/default/assumption change.
• No NaN/Inf inference anti-patterns, control-group changes, parameter propagation issues, or security concerns found.
• One P3 test/documentation note: the new public-wrapper test factorizes “non-contiguous” IDs before Rust, so it does not directly test raw Rust non-contiguous IDs.

Methodology

No blocking findings.

Severity: N/A
Impact: The Rust code still computes the same CR1 cluster-robust sandwich specified in docs/methodology/REGISTRY.md:L126-L128 and diff_diff/linalg.py:L1693-L1696. The only changed behavior is deterministic cluster-row ordering before cluster_scores.t().dot(&cluster_scores) at rust/src/linalg.rs:L284-L318.
Concrete fix: None required.

Code Quality

No findings.

Severity: N/A
Impact: The new cluster_index map is only used for lookup, not iteration, so randomized HashMap ordering no longer affects vcov row order.
Concrete fix: None required.

Performance

No findings.

Severity: N/A
Impact: The change removes per-row to_owned() accumulation into map values and uses direct matrix row accumulation at rust/src/linalg.rs:L309-L314.
Concrete fix: None required.

Maintainability

No findings.

Severity: N/A
Impact: The Rust comment explains why deterministic ordering matters and preserves the CR1 formula context.
Concrete fix: None required.

Tech Debt

No findings.

Severity: N/A
Impact: No new deferred correctness limitation introduced.
Concrete fix: None required.

Security

No findings.

Severity: N/A
Impact: No secrets, unsafe code, or input-handling security concern introduced by the diff.
Concrete fix: None required.

Documentation/Tests

Severity: P3
Impact: The added test tests/test_rust_backend.py:L3535-L3545 imports diff_diff.linalg.compute_robust_vcov, whose wrapper factorizes cluster_ids before calling Rust at diff_diff/linalg.py:L1722-L1727. So the test covers public API non-contiguous IDs, but not raw _rust_backend.compute_robust_vcov receiving non-contiguous unsorted IDs directly.
Concrete fix: Optional: add a small raw Rust-backend regression importing diff_diff._rust_backend.compute_robust_vcov with the same [7, 3, 11, ...] cluster IDs and asserting repeated-call bit identity.

Tests not run in this read-only review sandbox.