docs: add Deaner and Ku's duration DiD paper review - #822
Conversation
Overall assessment: ✅ Looks goodExecutive summary
Methodology
Code Quality
PerformanceNo findings; no runtime code changes. MaintainabilityNo findings. The prospective requirements and separation between paper results and library policy are unusually explicit. Tech DebtNo unmitigated findings. Deferred extensions are clearly identified and do not affect shipped behavior. SecurityNo findings; no secrets, executable third-party code, or sensitive data are introduced. Documentation/Tests
The naming-guard exception is reasoned and reachable, and the required changelog fragment is present. |
|
🔁 AI review rerun (requested by @igerber) Head SHA: Overall assessment: ✅ Looks goodExecutive summary
Methodology
Code Quality
PerformanceNo findings. MaintainabilityNo findings. Tech DebtNo unmitigated findings; future estimator extensions are clearly separated from the proposed initial scope. SecurityNo findings or apparent secrets. Documentation/TestsNo findings. The previous scale-label issue is resolved at |
Summary
Add
docs/methodology/papers/deaner-ku-2026-review.mdas the methodology foundation for a futureDurationDiDestimator. 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.timecolumn 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)
DurationDiD, common dynamics and proportional hazards.Validation
Tests added/updated:
tests/test_naming_guard.pygains 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:
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 checkpassed.git diff --checkand 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_diffpreviously exited 2 atnumpy/__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 withmypy --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.pyexcludesmethodology/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
.50and.20as “unequal initial cumulative hazards (and hence survivals),” consistent with Equations 2.6–2.8. Its calculation and displayed output are unchanged.pyproject.tomland.github/workflows/lint.ymlare unchanged from development base6e64e93. 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
Changelog
changelog.d/20260905-deaner-ku-paper-review.md(Documentation).