Skip to content

Add LinkedIn carousel for HAD estimator announcement#398

Merged
igerber merged 3 commits intomainfrom
had-launch
Apr 26, 2026
Merged

Add LinkedIn carousel for HAD estimator announcement#398
igerber merged 3 commits intomainfrom
had-launch

Conversation

@igerber
Copy link
Copy Markdown
Owner

@igerber igerber commented Apr 26, 2026

Summary

Test plan

  • Generator runs without exceptions: python carousel/generate_had_carousel.py
  • Output is 11-page PDF, ~250KB
  • Black/ruff clean on the generator script
  • Code snippet on slide 8 verified against the canonical (G=2000, seed=7) example: design=continuous_at_zero, att=0.2994, CI=[0.246, 0.353]
  • Naming consistent with paper: HAD = Heterogeneous Adoption Designs; we ship the HAD estimator
  • DOI included: doi.org/10.48550/arXiv.2405.04465

🤖 Generated with Claude Code

11-slide deck launching the HeterogeneousAdoptionDiD estimator. Light
gradient theme with indigo accent, matching the v3.0 / dCDH / TROP
playbook. Hero arc: "Most methods require a control group. Reality
doesn't." through to the v3.3 R-parity validation against DIDHAD v2.0.0.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Overall Assessment

⚠️ Needs changes

Executive Summary

  • The carousel repeatedly presents HeterogeneousAdoptionDiD(design="auto") as if it always returns the WAS estimand, but the library intentionally resolves to WAS or WAS_d_lower depending on the detected design path.
  • That mislabeling shows up in multiple places: the capability slide, the code slide (result.att # WAS estimate), and the feature slide (Per-horizon WAS).
  • The R-validation slide also overstates the parity contract. The repo’s documented/tested parity is scoped to design="continuous_at_zero", not the full auto-dispatch surface.
  • I found no runtime, performance, or security issues in the generator itself; the review issues are public-facing methodology claims.

Methodology

  • P1 carousel/generate_had_carousel.py:L731-L738,L883-L940,L976-L982 The deck hardcodes WAS semantics on design='auto' surfaces. Impact: on valid Design 1 panels, the estimator returns target_parameter="WAS_d_lower", not WAS, so the carousel can tell users they estimated the wrong estimand and the wrong CI type for the resolved path. Concrete fix: either pin the example/copy to design="continuous_at_zero" everywhere it says WAS / “Bias-corrected 95% CI”, or rewrite the copy to say WAS / WAS_d_lower depending on design and show result.target_parameter. Support: docs/methodology/REGISTRY.md:L2283-L2316, diff_diff/had.py:L124-L130,L256-L259,L585-L587, docs/practitioner_decision_tree.rst:L278-L311, docs/troubleshooting.rst:L483-L499, tests/test_had.py:L1037-L1071.
  • P3 carousel/generate_had_carousel.py:L1015-L1045 The R-parity slide is too broad. Impact: the repo’s own registry/tests scope end-to-end parity to the continuous_at_zero surface, and they document intentional convention differences from R even there; the current slide implies universal “bit-exact” parity across the estimator. Concrete fix: qualify the claim to DIDHAD v2.0.0 on design="continuous_at_zero" and avoid blanket bit-exact wording unless you specify the exact fields/conventions being compared. Support: docs/methodology/REGISTRY.md:L2501-L2502, tests/test_did_had_parity.py:L15-L28,L121-L127.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

No findings. Existing TODO.md entries do not mitigate the WAS vs WAS_d_lower labeling problem; that is a new public-facing methodology mismatch, not tracked deferred work.

Security

No findings.

Documentation/Tests

No additional findings. The relevant contracts are already well locked by existing tests: tests/test_had.py:L1037-L1071 for target_parameter, and tests/test_did_had_parity.py:L15-L28,L121-L127 for the scoped R-parity claim.

Path to Approval

  1. Remove the unconditional WAS labeling from slides 4/8/9 when the example uses design="auto". Either force design="continuous_at_zero" in the example, or surface result.target_parameter and describe the estimand as WAS / WAS_d_lower depending on design.
  2. If the code slide remains on design="auto", change # Bias-corrected 95% CI to a generic # 95% CI unless you also pin the snippet to a continuous path where that statement is guaranteed.

- Slide 8 code: pin example to design="continuous_at_zero" so the
  `# WAS estimate` and `# Bias-corrected 95% CI` comments are guaranteed
  by the resolved path (was design="auto", which can resolve to
  WAS_d_lower or the mass-point Wald-IV ratio depending on dose
  distribution).
- Slide 9 features card: "Per-horizon WAS" -> "Per-horizon estimates"
  to drop the estimand-specific claim from a feature description that
  applies to all event-study paths.
- Slide 10 subtitle: drop "Bit-exact" overclaim and add the design
  qualifier. New text: "End-to-end match against DIDHAD v2.0.0 on
  continuous-at-zero designs." matches REGISTRY scope (atol=1e-8 on
  Design 1' surface with documented convention deviations).

Slides 4 (card 2 "Recover the Weighted Average Slope.") and 6 ("WAS"
hero with Design 1' equation pinned) keep paper-level naming as
defensible.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igerber
Copy link
Copy Markdown
Owner Author

igerber commented Apr 26, 2026

/ai-review

@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 3653df510c21cc19980fb5a31e7d2f844a04b0d4


Overall Assessment

⚠️ Needs changes

Executive Summary

  • Previous re-review blocker on the code sample is resolved: slide 8 now pins design="continuous_at_zero", so the WAS and bias-corrected CI labels match the actual path.
  • Previous parity-scope issue is resolved: slide 10 now limits the DIDHAD comparison to continuous-at-zero designs.
  • One blocking methodology issue remains: the generic HAD overview still presents the estimator as recovering “the Weighted Average Slope,” but the shipped no-QUG paths target WAS_d_lower, not WAS. citeturn1view0
  • I found no additional code quality, performance, or security problems in the changed generator/PDF. Review was static; I did not execute the generator in the read-only workspace.

Methodology

  • P1 carousel/generate_had_carousel.py:L731-L737 Impact: slide 4 still markets HAD generically as recovering “the Weighted Average Slope.” That overstates the estimand contract. The paper only identifies WAS on the quasi-untreated-group / Design 1' path; in no-QUG designs it identifies only the sign of WAS or a different related parameter, and the library encodes that split as target_parameter == "WAS" vs "WAS_d_lower" (docs/methodology/REGISTRY.md:L2276-L2316, docs/api/had.rst:L13-L24, diff_diff/had.py:L124-L130, diff_diff/had.py:L256-L258, tests/test_had.py:L1037-L1071). No REGISTRY.md Note/Deviation mitigates this copy. Concrete fix: change the card to “Recover WAS or WAS_d_lower,” or “Recover a weighted-average-slope estimand,” or explicitly scope the whole slide to Design 1'/design="continuous_at_zero". citeturn1view0

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

No findings. TODO.md does not already track or mitigate the remaining estimand-labeling issue.

Security

No findings.

Documentation/Tests

  • P3 carousel/generate_had_carousel.py:L1042-L1044 Impact: “coverage at nominal level” is stronger than the repo’s own validation language. The registry documents coverage only as “near nominal,” and the slow MC test uses a loose lower bar rather than a tight nominal-calibration acceptance band (docs/methodology/REGISTRY.md:L2346-L2349, tests/test_had_mc.py:L164-L204). Concrete fix: soften the copy to “coverage near nominal” or “coverage consistent with nominal in Monte Carlo.”

Path to Approval

  1. Replace slide 4’s unconditional “Recover the Weighted Average Slope.” claim with wording that matches the resolved estimand contract (WAS on continuous_at_zero, WAS_d_lower on continuous_near_d_lower / mass_point), or explicitly scope that slide to Design 1' only.

…rage claim

- Slide 4: add italic footnote below the cards spelling out the
  estimand split: "Returns WAS on continuous-at-zero designs;
  WAS_d_lower otherwise." Card 2 title kept as "Recover the Weighted
  Average Slope." since the footnote now carries the precision and the
  marketing connection to slide 6's WAS hero is preserved.
- Slide 10 card 4: "coverage at nominal level" -> "coverage near
  nominal level" to match REGISTRY language and the loose lower-bar
  in tests/test_had_mc.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igerber
Copy link
Copy Markdown
Owner Author

igerber commented Apr 26, 2026

/ai-review

@github-actions
Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 611ecd996cf692a064ef837d576f063e6d38cedf


Overall Assessment

✅ Looks good

Executive Summary

  • Previous P1 is resolved: slide 4 now explicitly discloses the shipped estimand split (WAS on continuous_at_zero, WAS_d_lower otherwise), matching the registry and HAD implementation (carousel/generate_had_carousel.py:L731-L769, docs/methodology/REGISTRY.md:L2265-L2316, docs/api/had.rst:L13-L24, diff_diff/had.py:L124-L130, tests/test_had.py:L1037-L1071).
  • Previous parity-scope concern is resolved: the DIDHAD comparison is scoped to continuous-at-zero designs, consistent with the parity harness forcing design="continuous_at_zero" against DIDHAD::did_had() (carousel/generate_had_carousel.py:L1027-L1049, tests/test_did_had_parity.py:L113-L156).
  • Previous coverage-wording issue is resolved: slide 10 now says “coverage near nominal level,” which matches the registry/test language instead of overstating exact nominal calibration (carousel/generate_had_carousel.py:L1054-L1057, docs/methodology/REGISTRY.md:L2346-L2349, tests/test_had_mc.py:L164-L204).
  • I found no new unmitigated P0/P1 issues in the changed files. Review was static; I could not text-extract the committed PDF in this environment.

Methodology

  • No findings. Affected method: HeterogeneousAdoptionDiD. The changed carousel copy now aligns with the repo’s estimand contract and documented validation scope (docs/api/had.rst:L13-L25, diff_diff/had.py:L1-L39, diff_diff/had.py:L124-L130).

Code Quality

  • No findings.

Performance

  • No findings.

Maintainability

  • No findings.

Tech Debt

  • No findings. I did not need any TODO.md mitigation to clear this diff.

Security

  • No findings.

Documentation/Tests

  • No findings. The underlying HAD claims referenced in the carousel already have supporting in-repo docs/tests (tests/test_did_had_parity.py:L1-L30, tests/test_had.py:L1037-L1071, tests/test_had_mc.py:L164-L204).

@igerber igerber added the ready-for-ci Triggers CI test workflows label Apr 26, 2026
@igerber igerber merged commit a71d8b5 into main Apr 26, 2026
4 of 5 checks passed
@igerber igerber deleted the had-launch branch April 26, 2026 21:22
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