Skip to content

docs: add Deaner and Ku's duration DiD paper review - #822

Merged
igerber merged 2 commits into
mainfrom
codex/eval-paper-695220d9
Sep 6, 2026
Merged

docs: add Deaner and Ku's duration DiD paper review#822
igerber merged 2 commits into
mainfrom
codex/eval-paper-695220d9

Conversation

@igerber

@igerber igerber commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Summary

Add docs/methodology/papers/deaner-ku-2026-review.md as the methodology foundation for a future DurationDiD estimator. The review covers the entire 50-page paper and appendices A–E, including identification, estimation, bootstrap inference, diagnostics, proofs, simulations, application results, and a version-pinned reference-code audit. It reconciles consequential source discrepancies and separates paper results from proposed library choices.

  • Recommend an initial two-group estimator with common treatment timing, common-dynamics and proportional-hazards specifications, bootstrap inference, and pre-treatment diagnostics. Covariate adjustment and staggered adoption receive full methodological coverage but remain deferred from that first implementation.
  • Keep the prospective registry entry and unchecked implementation requirements inside the review. This PR introduces no estimator or public API.
  • Register the review's canonical time column vocabulary in the naming guard's existing reasoned allowlist. The guard continues to enforce renamed-surface coverage and exception reachability.

Methodology references (required if estimator / math changes)

  • Method name(s): prospective DurationDiD, common dynamics and proportional hazards.
  • Paper / source link(s): Deaner and Ku (2026), Causal Duration Analysis with Diff-in-Diff, arXiv:2405.05220v2, a working-paper revision; author software at commit 202e92ef222bf7d250c26c98e8fb42e151223c71. The review records the original PDF and six reference-file digests.
  • Any intentional deviations from the source (and why): the proposed PH estimator uses mean ratios, supported by Theorem 1 and the author code, while distinguishing both slope alternatives and Equation 3.5's incompatible denominator. Diagnostics follow Appendix B's fixed anchor, with the main text's valid moving-window alternative documented separately. Baseline normalization, post-period inference endpoints, diagnostic centering, and the two-sided maximum statistic are reconciled against equations, proofs, and pinned code. Numerical and failed-bootstrap-draw policies are labeled library recommendations; they are not represented as additional paper guarantees. No reference code is ported or modified.

Validation

  • Tests added/updated: tests/test_naming_guard.py gains one reasoned allowlist entry. The full naming guard and documentation guards were rerun after the final comment correction, including consumer completeness and allowlist reachability.

  • Executed after that correction in the frozen local environment:

    PYTHONPATH=. DIFF_DIFF_BACKEND=python .venv/bin/python -m pytest tests/test_naming_guard.py tests/test_changelog_fragments.py tests/test_docs_ia.py tests/test_doc_deps_integrity.py
    

    358 passed, with ten deprecation warnings from public-alias inspection and an existing pytest fixture.

  • .venv/bin/python -m ruff check diff_diff tests — passed in the prior local revision.

  • .venv/bin/python -m black --check diff_diff tests — passed in the prior local revision; 323 files unchanged. Ruff and Black were not rerun for the final comment-only edit; Python source and lint configuration remain unchanged since those checks.

  • .venv/bin/python .claude/scripts/premerge_scan.py --scratch .workflow/validation/premerge-scan-pr-01 — passed with zero findings; resolved the changed naming-guard test. The subsequent .venv/bin/python .claude/scripts/changelog_compile.py check passed.

  • git diff --check and direct new-file whitespace checks passed. Direct artifact checks verified Markdown fences/tables, relative links and pinned URL correspondence, all seven unchanged source digests, prospective-status wording, and the embedded calculation's exact displayed output. External links were not crawled anew.

  • Backtest / simulation / notebook evidence: independent NumPy algebra checks passed during authoring for CD/PH normalization, effect sign, PH coefficient alternatives, diagnostic maximum, resampling identities, and bootstrap-band ordering. The self-contained calculation in the review was rerun after the final correction, matching its displayed output; its executable Python AST is unchanged. These are equation checks; the paper's reported simulations and empirical results were not replicated.

  • Type-checking limitation: .venv/bin/python -m mypy diff_diff previously exited 2 at numpy/__init__.pyi:737, before further checking, because frozen NumPy 2.5.2 uses Python 3.12 type statements while the project targets Python 3.10. Local Python is 3.14.4 and mypy is 2.3.1. The issue also reproduces on unchanged input with mypy --no-incremental --python-version 3.10 -c "import numpy". The environment and configuration remain unchanged; mypy was not rerun for the comment correction. The frozen lint CI workflow pins NumPy 2.4.5 with Python 3.14 and mypy 2.3.1. No passing local type check is claimed.

  • Applicability/limitations: no estimator or Rust behavior changed, so no estimator/backend suite was run. No notebooks changed. Sphinx was not run because docs/conf.py excludes methodology/papers/*; the Markdown artifact was checked directly. Stata, MATLAB, and Octave are unavailable, and the application dataset is absent. Reference-code inspection is static; executability, numerical replication, and reuse licensing remain unverified.

Calculation and diff scope

  • The example labels .50 and .20 as “unequal initial cumulative hazards (and hence survivals),” consistent with Equations 2.6–2.8. Its calculation and displayed output are unchanged.
  • The candidate changes only the methodology review, changelog, and naming guard. pyproject.toml and .github/workflows/lint.yml are unchanged from development base 6e64e93. Main subsequently advanced Ruff from 0.16.4 to 0.16.5; comparing the two complete trees therefore displays a pin difference that is absent from this branch's changes.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes; no secrets or private individual-level records are added. The review uses public papers and public source code. Cached originals are local review evidence, with canonical public URLs and digests recorded in the document.

Changelog

  • changelog.d/ fragment added: changelog.d/20260905-deaner-ku-paper-review.md (Documentation).

@igerber igerber added ready-for-ci Triggers CI test workflows and removed ready-for-ci Triggers CI test workflows labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Overall assessment: ✅ Looks good

Executive summary

  • No estimator, public API, weighting, or runtime inference behavior changes.
  • CD/PH identification, ATT sign, baseline normalization, and bootstrap conventions are coherent with the cited methodology.
  • Intentional numerical/default choices are explicitly documented and therefore informational.
  • No P0/P1 findings; only minor documentation/tooling observations.

Methodology

  • P3 — Documented implementation choices.
    Impact: Mean-of-ratios PH estimation, fixed-anchor diagnostics, and conservative failed-bootstrap handling intentionally differ from ambiguous or alternative source formulations. These are clearly labeled and mathematically valid.
    Concrete fix: None required.
    Location: docs/methodology/papers/deaner-ku-2026-review.md:L92-L146, L291-L323, L385-L408.

Code Quality

  • P3 — Unrelated Ruff rollback.
    Impact: The PR changes Ruff from 0.16.5 to 0.16.4 despite otherwise being documentation-focused, adding avoidable scope and potentially losing patch-level fixes. CI and the dev extra remain synchronized.
    Concrete fix: Restore 0.16.5, or document why 0.16.4 is required.
    Location: .github/workflows/lint.yml:L42, pyproject.toml:L63-L69.

Performance

No findings; no runtime code changes.

Maintainability

No findings. The prospective requirements and separation between paper results and library policy are unusually explicit.

Tech Debt

No unmitigated findings. Deferred extensions are clearly identified and do not affect shipped behavior.

Security

No findings; no secrets, executable third-party code, or sensitive data are introduced.

Documentation/Tests

  • P3 — Example comment uses the wrong scale name.
    Impact: r1_base and r2_base are cumulative hazards, but the comment calls them “initial survival,” which may confuse readers even though the calculation correctly exponentiates them.
    Concrete fix: Change the comment to “unequal initial cumulative hazards (and hence survivals).”
    Location: docs/methodology/papers/deaner-ku-2026-review.md:L999-L1007.

The naming-guard exception is reasoned and reachable, and the required changelog fragment is present.

@igerber igerber changed the title docs: review Deaner and Ku's duration DiD methodology docs: add Deaner and Ku's duration DiD paper review Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 7395227a229ca1576b64ec4e76a47e30bd31fa35


Overall assessment: ✅ Looks good

Executive summary

  • No estimator, API, weighting, variance, or runtime behavior changes.
  • CD/PH equations, ATT sign, baseline normalization, and bootstrap requirements are coherent.
  • Intentional source deviations are explicitly documented and therefore informational.
  • The prior cumulative-hazard comment issue is fixed.
  • The unrelated Ruff rollback remains a P3 observation.

Methodology

  • Severity: P3 — Documented implementation choices
    • Impact: Mean-ratio PH estimation, fixed-anchor diagnostics, post-only inference, and failed-draw handling differ from source alternatives but are clearly labeled.
    • Concrete fix: None required.
    • Location: docs/methodology/papers/deaner-ku-2026-review.md:L92-L146, L291-L323, L358-L408

Code Quality

  • Severity: P3 — Ruff patch rollback remains
    • Impact: Downgrading 0.16.5 to 0.16.4 adds unrelated scope and may discard patch fixes.
    • Concrete fix: Optionally restore 0.16.5 or document the reason for pinning 0.16.4.
    • Location: .github/workflows/lint.yml:L42, pyproject.toml:L68

Performance

No findings.

Maintainability

No findings.

Tech Debt

No unmitigated findings; future estimator extensions are clearly separated from the proposed initial scope.

Security

No findings or apparent secrets.

Documentation/Tests

No findings. The previous scale-label issue is resolved at docs/methodology/papers/deaner-ku-2026-review.md:L1003. The naming exception is reasoned, and the changelog fragment follows repository conventions.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Sep 6, 2026
@igerber
igerber merged commit e15bdc6 into main Sep 6, 2026
35 of 36 checks passed
@igerber
igerber deleted the codex/eval-paper-695220d9 branch September 6, 2026 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-ci Triggers CI test workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant