docs: Firpo & Possebom (2018) paper review — SCM CI by test inversion (PR-A)

[igerber]

Summary

• Add docs/methodology/papers/firpo-possebom-2018-review.md — a faithful, paper-sourced fidelity review (PR-A) of Firpo & Possebom (2018), the Step-1 artifact for the forthcoming SCM confidence-set / CI-by-test-inversion track (PR-B) layered on the existing SyntheticControl estimator (classic SCM has no analytical SE; se/p_value/conf_int are NaN).
• Full-paper fidelity: benchmark RMSPE-ratio permutation test (Eqs 4–6), sensitivity-analysis parametric p-value weights with worst/best-case φ̲/φ̄ (Eqs 7–9), sharp-null RMSPE^f test (Eqs 10–13), confidence sets by test inversion (Eq 14) with the operational constant-effect CI (Eqs 15–16) and linear-effect CS (Eqs 17–18), the general test-statistic framework + Monte Carlo size/power (Eq 19, §5), and the multiple-outcome FWER (Eqs 23–24) / multiple-treated-unit pooled (Eqs 25–26) extensions. The requirements checklist flags the PR-B target vs deferred items.
• Docs-only; no source/estimator change. Registered in docs/references.rst (Synthetic Control Method), docs/doc-deps.yaml, the REGISTRY ## SyntheticControl reviews-on-file pointer, and CHANGELOG [Unreleased].

Methodology references (required if estimator / math changes)

• Method name(s): SyntheticControl inference — sensitivity analysis + confidence sets by test inversion (documentation only; no estimator/math code changed in this PR).
• Paper / source link(s): Firpo, S. & Possebom, V. (2018), "Synthetic Control Method: Inference, Sensitivity Analysis and Confidence Sets," Journal of Causal Inference 6(2), https://doi.org/10.1515/jci-2016-0026
• Any intentional deviations from the source (and why): None — this is a paper-sourced Step-1 fidelity artifact with no implementation. The CI grid bounds/resolution and the inversion search are noted as implementation choices deferred to PR-B (the paper does not specify them).

Validation

• Tests added/updated: None (docs-only). tests/test_doc_deps_integrity.py passes with the new review path mapped (144 passed).
• Backtest / simulation / notebook evidence (if applicable): N/A — docs-only PR-A. Local Codex review (agentic, CI-quality) is ✅ clean after one revision round (CI-inversion boundary sharpened to the strict p^f > γ; disputed license tag dropped).

Security / privacy

• Confirm no secrets/PII in this PR: Yes. The paper PDF is kept outside the repository (per the PDFs-never-committed convention); only the public DOI is cited.

---

🤖 Generated with Claude Code

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: 03587c78aafe3bb13ab5a792f1f4b1e16bde94c7

---

PR Review Report

Overall Assessment

✅ Looks good — No unmitigated P0 or P1 findings.

Executive Summary

• Current supplied diff is docs-only: changelog, doc dependency map, REGISTRY pointer, references, and a new Firpo–Possebom paper review. No estimator, variance, SE, weighting, or default behavior code changed.
• Prior EfficientDiD P0 is resolved for this re-review scope because EfficientDiD source files are no longer modified.
• Prior Firpo confidence-set boundary concern is resolved: the review now explicitly documents p < γ, p ≤ γ, and strict p > γ conventions.
• Source cross-check supports the new review’s main claims: parametric p-value weights, sharp-null RMSPE, confidence sets by inversion, and multiple-outcome / multiple-treated extensions. (doi.org)

Methodology

Finding M1 — Existing SCM placebo conventions are documented implementation choices

Severity: P3 informational
Location: docs/methodology/papers/firpo-possebom-2018-review.md:L135-L149, docs/methodology/papers/firpo-possebom-2018-review.md:L167-L168, docs/methodology/REGISTRY.md:L1993-L2015

Impact: The PR-B guidance says to reuse existing in_space_placebo() helpers. Those helpers use documented project conventions: root-scale RMSPE ratio, treated-unit exclusion from placebo donor pools, effective-count handling for failed placebos, and a denominator floor. These differ from the paper’s literal presentation in small implementation details, but the differences are already documented in REGISTRY Notes and are rank-equivalent or explicit project conventions.

Concrete fix: No action required for this PR. When PR-B lands, cross-reference the REGISTRY Notes in the new Firpo implementation subsection.

Code Quality

Finding: None.
Severity: N/A
Impact: No code files are modified in the supplied diff.
Concrete fix: N/A

Performance

Finding: None.
Severity: N/A
Impact: Docs-only change; no runtime behavior affected.
Concrete fix: N/A

Maintainability

Finding: None.
Severity: N/A
Impact: The new paper review is registered in docs/doc-deps.yaml:L616-L617 and REGISTRY at docs/methodology/REGISTRY.md:L1977, so methodology drift tracking is maintained.
Concrete fix: N/A

Tech Debt

Finding: None.
Severity: N/A
Impact: Deferred Firpo implementation items are explicitly marked in the review checklist at docs/methodology/papers/firpo-possebom-2018-review.md:L134-L141; no silent correctness issue is being deferred.
Concrete fix: N/A

Security

Finding: None.
Severity: N/A
Impact: No secrets or sensitive data found in the changed documentation.
Concrete fix: N/A

Documentation/Tests

Finding: None.
Severity: N/A
Impact: Prior changelog mismatch is resolved: CHANGELOG.md:L12 now truthfully describes the current supplied diff as docs-only. The new reference is also added under Synthetic Control at docs/references.rst:L111.
Concrete fix: N/A

Tests not run; review-only pass.

[github-actions]

🔁 AI review rerun (requested by @igerber)

Head SHA: e9550092b84877f6200b778246744e5bcf2c7264

---

PR Review Report

Overall Assessment

✅ Looks good — No unmitigated P0 or P1 findings.

Executive Summary

• Re-review scope is docs-only: CHANGELOG.md, docs/doc-deps.yaml, docs/methodology/REGISTRY.md, a new Firpo–Possebom paper review, and docs/references.rst.
• No estimator, inference, weighting, variance/SE, or default behavior code changed.
• The prior P0/P1 concerns are resolved for this scope; the changed files do not modify the earlier affected estimator code paths.
• Methodology spot-check supports the review’s core claims: Firpo–Possebom add sensitivity-weighted SCM permutation p-values, sharp-null RMSPE tests, confidence sets by test inversion, and extensions to other statistics / multiple outcomes / multiple treated units. (degruyterbrill.com)
• Existing SyntheticControl placebo conventions are documented in the REGISTRY as implementation notes, so they are informational, not defects.

Methodology

Finding M1 — Existing SCM Placebo Conventions Are Documented Implementation Choices

Severity: P3 informational
Location: docs/methodology/papers/firpo-possebom-2018-review.md:L135-L153, docs/methodology/papers/firpo-possebom-2018-review.md:L167-L168, docs/methodology/REGISTRY.md:L1993-L2015

Impact: The new review’s PR-B guidance points toward reusing existing in_space_placebo() machinery. That machinery uses documented project conventions: root-scale RMSPE ratio, treated-unit exclusion from placebo donor pools, effective-count handling for failed placebos, and a denominator floor. These are already labeled as REGISTRY notes/deviations, so they are not methodology defects under the review rules.

Concrete fix: No action required for this docs-only PR. When PR-B implements Firpo–Possebom inversion, cross-reference the existing REGISTRY notes in the new implementation subsection.

Code Quality

Finding: None.
Severity: N/A
Impact: No Python source files are modified.
Concrete fix: N/A

Performance

Finding: None.
Severity: N/A
Impact: Documentation-only change; no runtime behavior affected.
Concrete fix: N/A

Maintainability

Finding: None.
Severity: N/A
Impact: The new review is registered in docs/doc-deps.yaml:L616-L617, docs/references.rst:L111, and the SyntheticControl REGISTRY pointer at docs/methodology/REGISTRY.md:L1977.
Concrete fix: N/A

Tech Debt

Finding: None.
Severity: N/A
Impact: Deferred implementation scope is explicitly marked as PR-B/deferred in docs/methodology/papers/firpo-possebom-2018-review.md:L134-L141; no silent correctness bug is introduced.
Concrete fix: N/A

Security

Finding: None.
Severity: N/A
Impact: No secrets, credentials, or sensitive data found in the changed documentation.
Concrete fix: N/A

Documentation/Tests

Finding: None.
Severity: N/A
Impact: Changelog, reference list, doc dependency map, and methodology registry are consistent with the docs-only scope.
Concrete fix: N/A

Tests not run; review-only pass.