Skip to content

Commit b02dff6

Browse files
committed
fix(did): harden /did-event-study from Card-Krueger validation (real bug found by running it)
Validated against Pedro's DiD_book Card-Krueger to his 1e-6 standard: - PRINCIPLE PASSES: DRDID reproduces his canonical 2x2 DiD (2.913982357…) to ~1e-14. - REAL BUG: the skill prescribed DRDID(panel=TRUE, idname=id), which ERRORS on Card-Krueger — an unbalanced panel (409 stores, 389 in both waves) with 2 duplicate (id,wave) rows. Fixes: - Phase 3: pre-flight idname-unique-by-period + balance check; full-sample 2x2 via panel=FALSE + row-unique id (matches feols ~1e-10); balancing → panel=TRUE flagged as a DIFFERENT estimand (2.914 full-sample vs 2.972 balanced — recorded as a named alternative, the EXPLAINED pattern). - did-conventions: idname-unique-by-period + check-balance-before-panel-mode now HARD. - quality_reports/did_validation_card_krueger.md: the verification record. Not yet validated: staggered att_gt path + HonestDiD/didFF sensitivity (packages not installed).
1 parent 91f874e commit b02dff6

3 files changed

Lines changed: 42 additions & 2 deletions

File tree

.claude/rules/did-conventions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ The opinionated defaults here reflect **Pedro Sant'Anna's sign-off** (2026-06-09
2020
## Data & coding — HARD
2121
- Data MUST be **LONG**: one row per unit-period.
2222
- `gname` (group) = the **first period a unit is treated**; **never-treated coded EXACTLY `0`**.
23-
- `idname` must be **time-invariant and numeric**.
23+
- `idname` must be **time-invariant and numeric**, and **unique within each period**. **Check panel balance before any `panel = TRUE` estimation** — unbalanced or duplicate-`(id,period)` data errors in `DRDID`/`did` or silently changes the estimand (balancing drops attriters → a different target). For the full-sample textbook 2×2, use `panel = FALSE` with a row-unique id.
2424
- These estimators are **staggered-adoption / absorbing only** — once treated, always treated. No reversal.
2525
- `ATT(g,t)` is identified only for `t ≥ g`; `t < g` estimates are **pseudo-ATTs for pre-testing only** (valid only under no-anticipation).
2626

.claude/skills/did-event-study/SKILL.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,17 @@ Follow the decision logic in §Estimator selection. Output: which estimator, `es
4444

4545
### Phase 3 — Estimation (drive the package; do not reimplement)
4646
- **2×2 (one pre / one post):**
47-
`DRDID::drdid(yname, tname, idname, dname, xformla = ~covs, data, panel = TRUE, estMethod = "imp")``summary()`.
47+
```r
48+
DRDID::drdid(yname, tname, idname, dname, xformla = ~covs, data, panel = TRUE, estMethod = "imp")
49+
```
4850
IPW-only: `DRDID::ipwdid(..., normalized = TRUE)`; OR-only: `DRDID::ordid(...)`.
51+
- **Pre-flight (learned from validating Card–Krueger):** `panel = TRUE` requires `idname` **unique within each period** AND a **balanced** panel. Real datasets often aren't — check first:
52+
```r
53+
stopifnot(nrow(dplyr::count(data, .data[[idname]], .data[[tname]]) |> dplyr::filter(n > 1)) == 0) # idname unique by period
54+
balanced <- all(table(data[[idname]]) == length(unique(data[[tname]])))
55+
```
56+
- **If unbalanced** (the common case): either (a) reproduce the **full-sample textbook 2×2** with `panel = FALSE` and a **row-unique id** (`data$rowid <- seq_len(nrow(data))`; this equals `feols(y ~ d*post)` to ~1e-10but RC SEs treat the waves as independent, so for a true panel report the clustered/panel SE separately); or (b) **balance** the panel (`keep ids present in all periods`) and use `panel = TRUE`but this is a **different estimand** (the balanced subpopulation), so record it as a named alternative (`EXPLAINED`), e.g. "full-sample 2×2 = 2.914; balanced-panel DR = 2.972 (19 attriting stores dropped)."
57+
- DR with **no covariates reduces to the simple 2×2**DRDID earns its keep once `xformla` adds covariates.
4958
- **Staggered / multi-period (G×T or 2×T):**
5059
```r
5160
out <- did::att_gt(
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# `/did-event-study` validation — Card & Krueger (1994)
2+
3+
**Date:** 2026-06-09 · **Model:** Opus 4.8 · **Standard:** `DiD_book` 1e-6 tolerance
4+
**Source of truth:** Pedro Sant'Anna's `DiD_book/Card_Krueger_American_Economic_Review_1994` (his code + recorded outputs).
5+
6+
## Result: PASS (principle) + 1 real bug found & fixed (prescription)
7+
8+
| Check | Value | vs target | Status |
9+
|---|---|---|---|
10+
| Reproduce his canonical 2×2 DiD (`feols(fte ~ treated*post)`) | `2.913982357430426` | his `…376` | **1e-14**|
11+
| Skill's estimator `DRDID::drdid` reproduces it (fixed: RC + row-id) | `2.913982357430…` | his 2×2 | **2.7e-14**|
12+
| Skill's *original* prescription `DRDID(panel=TRUE, idname=id)` | **ERROR** || ✗ bug |
13+
14+
## The bug (why this validation mattered)
15+
Card–Krueger is an **unbalanced** panel (409 stores; only 389 in both waves) with **2 duplicate `(id,wave)` rows**. The skill prescribed `DRDID::drdid(panel = TRUE)` with the panel id, which **errors** ("idname must be unique by tname") on exactly this common real-world shape.
16+
17+
**Fixes applied:**
18+
- Skill Phase 3: a pre-flight balance/uniqueness check; full-sample 2×2 via `panel = FALSE` + a **row-unique id** (matches `feols` to ~1e-10); balancing → `panel = TRUE` is flagged as a **different estimand**.
19+
- `did-conventions` rule: idname-unique-by-period + check-balance-before-panel-mode is now HARD.
20+
21+
## Estimand note (the `EXPLAINED` pattern, live)
22+
- Full-sample textbook 2×2 (his target): **ATT = 2.914**
23+
- Balanced-panel DR (389 stores, 19 attriters dropped): **ATT = 2.972**
24+
25+
Both are defensible — they answer different questions. The skill now records this as a named alternative rather than presenting one number as "the" answer.
26+
27+
## Caveat
28+
SE comparison is not 1e-6: `DRDID` RC SE (1.73) treats waves as independent; `feols` clustered SE (1.29) uses the panel. For a true panel, report the clustered/panel SE. Point-estimate equivalence is the validation test.
29+
30+
## Not yet validated
31+
`HonestDiD`/`didFF`/`contdid` are not installed → the staggered `att_gt` path + sensitivity suite (HonestDiD breakdown, `didFF`) and continuous-treatment path remain to be validated on a genuinely staggered `DiD_book` application.

0 commit comments

Comments
 (0)