Skip to content

4.0 migration guide + R-equivalents argument table (phase 4) - #759

Merged
igerber merged 2 commits into
mainfrom
feat/v4-migration-guide
Aug 10, 2026
Merged

4.0 migration guide + R-equivalents argument table (phase 4)#759
igerber merged 2 commits into
mainfrom
feat/v4-migration-guide

Conversation

@igerber

@igerber igerber commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Ships the two un-rowed obligations from the 3.9-cut checklist (docs/v4-design.md §9, items 2 and 3) — the last thing between 3.9 being code-complete and the formal cut.

  • docs/migration-4.0.md (new): a 21-row orientation table, a 108-row per-symbol appendix carrying the ledger's own Old/New locators plus a one-line fix each, worked before/after for the three merges, a codemod regex table, and a separate "already shipped in 3.9" section.
  • docs/r_comparison.rst: the rule-8 Argument mapping table — the library's stated alternative to shipping R-spelling parameter aliases.
  • Nine new gates across tests/test_v4_matrix.py and tests/test_docs_ia.py, including three vacuity guards.

Sequencing: this lands before the 3.9.0 release commit, which is the order the design already mandates in two normative places (the phase table at :828 and the hand-assertion sentence at :885). The release PR then verifies items 2 and 3 rather than shipping them. No ordering amendment was needed.

The appendix has no derived advice columns — the load-bearing decision

An Available since column was designed, gated, and then removed. Every label it would carry rests on a single ledger field while the fact asserted is compound:

derived from would publish why it is false
introduced_in an availability date tracks the dataclass storage flip — the nine field-flip rows say 4.0 while .att resolves today
new is null "pure removal, nothing to do" 12 of the 38 are default flips / behaviour changes that move numbers
successor resolves "already available" 5 of the 13 aggregate-postfit successors raise on bootstrapped fits

No better probe fixes that, so the appendix carries only what is mechanically checkable and the compound claim moved into a hand-written Fix cell sourced from each row's notes.

This was validated during implementation, not assumed. The generator's null-successor guard refused to emit a mechanical rename five times (M-087, then M-011/M-012/M-014/M-016). Executing the robust= family produced three distinct rules across four rows — drop-or-translate on DifferenceInDifferences/LinearRegression, drop-only on TripleDifference (it accepts only hc1, so vcov_type="classical" raises), and the inverse on HAD. One pre-written rule would have been wrong on all four.

What local review caught

Three defects, all in hand-written prose rather than the derived table — the same failure mode one level down:

  • The auto-cluster opt-out was inverted. cluster=None and omission are the same thing under the 4.0 policy; cluster=False disables (v4-design.md:681). The documented "reproduce 3.x exactly" recipe would have silently returned clustered SEs, p-values and CIs.
  • M-139 pointed at a method that does not exist. HADPretestReport.aggregate is a metadata field; the successor is panel-shape inference.
  • All three merge examples carried an invalid keywordtreated=/partition=/post= for treatment=/eligibility=/time=. Note the surviving TripleDifference really does take partition=, which is what made the mistake look right.

That third one is the risk this page is structurally exposed to (markdown isn't snippet-executed), so rather than leave the acceptance bare, test_migration_guide_examples_bind_to_real_signatures AST-parses the guide's python blocks and asserts every constructor/fit() keyword exists on the target signature — binding examples to reality without executing 4.0-only behaviour. Verified non-vacuous: replayed against the four original broken keywords it flags all four, and it checks 33 keywords on the live guide.

Deliberately not here

The llms.txt and README.md catalog entries. Both list pages as readthedocs.io/en/stable/…, and stable builds the newest tag (v3.8.0), so an entry added before the cut is a 404 in the wheel-bundled agent contract. They go with the release commit.

Methodology references (required if estimator / math changes)

  • Method name(s): N/A — zero diff_diff/ source files changed. This is documentation plus test gates; no estimator, equation, weighting, variance, or inference code is touched.
  • Paper / source link(s): the guide's methodology claims were cross-checked against docs/methodology/REGISTRY.md (pooled-vs-within MPD/TWFE, the staggered DDD merge) and the Athey–Imbens 2006 review (the CiC-over-QDiD recommendation).
  • Any intentional deviations from the source (and why): none. docs/v4-design.md §10 carries a dated amendment recording five deviations from its own skeleton (two tables not one; no availability column, with the reason; the §7b and §9 additions; the codemod's identifier-only scope).

Validation

  • Tests added/updated: tests/test_v4_matrix.py (8 migration-guide gates + 3 parser/vacuity tests), tests/test_docs_ia.py (the two-directional R-table gate).
  • Backtest / simulation / notebook evidence: N/A — no notebooks changed.
  • Verified: 469 gate tests pass (matrix, docs IA, doc-deps integrity); naming guard clean; test_doc_snippets.py 119 passed / 5 skipped with r_comparison block indices unshifted; SPHINXOPTS="-W" make -C docs html reports build succeeded with zero warnings; black, ruff and mypy clean at the CI-pinned toolchain.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

Ships the two un-rowed obligations from the 3.9-cut checklist
(docs/v4-design.md section 9, items 2 and 3) - the last thing between 3.9 being
code-complete and the formal cut. Lands BEFORE the release commit, the order the
design already mandates in two normative places (the phase table at :828 and the
hand-assertion sentence at :885); the release PR verifies these rather than
shipping them.

docs/migration-4.0.md is a 21-row orientation table plus a 108-row per-symbol
appendix, worked before/after for the three merges, a codemod regex table, and a
separate "already shipped in 3.9" section. The appendix row set is derived from
the ledger by _changes_at_4_0 and asserted in both directions, so a rescheduled
or added row fails CI until the guide is updated.

THE APPENDIX HAS NO DERIVED ADVICE COLUMNS, and that is the load-bearing
decision. An `Available since` column was designed, gated, and then removed:
every label it would carry rests on a single ledger field while the fact asserted
is compound. `introduced_in` tracks the dataclass STORAGE flip, so the nine
field-flip rows say 4.0 while `.att` resolves today; a null `new` covers
removals, default flips AND translations; a successor that resolves can still
raise (the five bootstrapped-fit aggregate() families). No better probe fixes
that, so the appendix carries only what is mechanically checkable and the
compound claim moved into a hand-written Fix cell sourced from each row's notes.

That choice was validated during implementation, not assumed. The generator's
null-successor guard refused to emit a mechanical rename for M-087, then
M-011/M-012/M-014/M-016 - each needed text written from its notes. Executing the
robust= family produced THREE distinct rules across four rows: drop-or-translate
on DifferenceInDifferences/LinearRegression, drop-only on TripleDifference (it
accepts only hc1, so vcov_type="classical" RAISES), and the inverse on HAD, whose
legacy default is non-robust. One pre-written rule would have been wrong on all
four.

Local review then found three defects that prove the same point one level down,
in the hand-written prose rather than the derived table:

- The auto-cluster opt-out was INVERTED. `cluster=None` and omission are the same
  thing under the 4.0 policy - both auto-cluster at unit - so the documented
  "reproduce 3.x exactly" recipe would have silently returned clustered SEs,
  p-values and CIs. `cluster=False` disables (v4-design.md:681). Also scoped: the
  TWFE event-study mode has auto-clustered since 3.9, so the flip changes the
  static and other panel paths, not that one.
- M-139 pointed at a method that does not exist. `HADPretestReport.aggregate` is
  a metadata field; the successor is panel-shape inference. The advice raised.
- All three merge examples carried an invalid keyword: `treated=` for
  `treatment=`, `partition=` for `eligibility=` on the RETIRED staggered class
  (the surviving TripleDifference does take `partition=`, which is what made it
  look right), and `treated=`/`post=` for `treatment=`/`time=` on both CiC sides.

The third is the failure mode this page is structurally exposed to - it is
markdown, and test_doc_snippets.py discovers only RST, so a plausible-but-wrong
keyword ships silently. Rather than leave that acceptance bare,
test_migration_guide_examples_bind_to_real_signatures ast-parses the guide's
python blocks and asserts every constructor/fit() keyword exists on the target
signature. It binds examples to reality WITHOUT executing 4.0-only behaviour,
which is why the snippet harness could not do this job. Verified non-vacuous
rather than assumed: replayed against the four original broken keywords it flags
all four, and it checks 33 keywords on the live guide.

The R argument table is scoped to the three packages r_comparison.rst actually
evidences with paired R/Python blocks - did, HonestDiD, synthdid. fixest,
DIDmultiplegtDYN and DIDHAD appear there in prose only, so they are named as out
of scope rather than mapped from memory. Non-1:1 cases get a Notes cell instead
of a fabricated row: aggte(type="dynamic") -> aggregate(type="event_study")
(AGGREGATION_VOCABULARY has no "dynamic"), synthdid's block structure ->
treatment indicator plus post_periods, and HonestDiD's betahat/sigma carried by
the results object. Its gate is two-directional and resolves each name against
the SPECIFIC callable the row names - resolving against any public callable would
let `time` or `unit` be satisfied by an unrelated signature. The R side itself is
ungateable (no R signatures in the repo) and the docstring says so. The gate
earned itself immediately by catching a real defect in the table it guards: a
cell read `aggregate(type=)` where the parameter is `type`.

Two deviations from the approved plan, both forced by execution. The doc-deps
union needed a private-module filter the plan did not specify - the mechanical
recipe resolved `_reporting_helpers.py` to a key that does not exist; excluding
private modules loses no coverage (both public owners are already in the union)
and avoids a schema edit. And the no-op Fix denylist had to become word-boundary
regex: a bare "none" fired on the legitimate `cluster=None` and
`summary(alpha=None)` that two cells must state.

TODO.md's aggregate= row is re-graded Quick -> Heavy with a measured inventory:
28 executable code-cell sites across 9 notebooks (14_continuous_did and
15_efficient_did match only in markdown prose; 21_had_pretest_workflow uses a
spaced `aggregate =` that a naive grep misses), plus six docs files of which two
API pages cannot migrate at all today because their report consumers read the raw
event_study_effects field post-fit aggregate() never populates.

Also corrects two false statements on the R page that no snippet gate can reach:
`.ci` (the field is conf_int) and the claim that aggregation is requested at fit
time.

The llms.txt and README.md catalog entries are deliberately NOT here: both list
pages as readthedocs.io/en/stable/..., and `stable` builds the newest tag
(v3.8.0), so an entry added before the cut is a 404 in the wheel-bundled agent
contract. They go with the release commit.

Cross-references to TODO.md/DEFERRED.md/v4-design.md/the ledger YAML are inline
literals, never links - none is a Sphinx document, so a MyST link to any of them
fails the -W build.

Verified: 469 gate tests pass (matrix incl. 8 new guide gates + 3 parser vacuity
tests, docs IA incl. the new R-table gate, doc-deps integrity); naming guard
clean (the guide is outside every scan lane); doc snippets 119 passed / 5 skipped
with r_comparison block indices unshifted; Sphinx -W build succeeded with zero
warnings; black, ruff, and mypy clean at the CI-pinned toolchain.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

Overall assessment

⚠️ Needs changes — one unmitigated P1 migration error could silently change SyntheticDiD estimates.

Executive summary

  • SyntheticDiD’s migration advice incorrectly suggests replacing ignored parameters with active regularization overrides.
  • The estimator merges, inference-default descriptions, and 108-row ledger parity otherwise align with the Methodology Registry.
  • The codemod and results-field sections omit several migrations.
  • No performance, security, or estimator-code regressions found.

Methodology

P1 — SyntheticDiD migration can change estimates

  • Location: docs/migration-4.0.md:L301-L302
  • Affected method: SyntheticDiD
  • Impact: lambda_reg and zeta are ignored, leaving regularization auto-computed. Replacing their values with zeta_omega or zeta_lambda, as instructed, activates explicit overrides and can change weights, ATT, and inference. This contradicts diff_diff/synthetic_did.py:L216-L234, diff_diff/synthetic_did.py:L720-L733, and the auto-regularization equations in docs/methodology/REGISTRY.md:L2640-L2644.
  • Concrete fix: Say to remove the ignored arguments and leave zeta_omega/zeta_lambda unset to preserve 3.x results. Explain that the new parameters should only be supplied intentionally, not populated mechanically from the old values.

Code Quality

P2 — Example-signature gate is vacuous

  • Location: tests/test_v4_matrix.py:L1242-L1312
  • Impact: Removing all Python blocks or changing their fence label makes the loops empty and the test pass.
  • Concrete fix: Assert the expected four blocks and required estimator/keyword pairs—or at least a nonzero pair count—before validation.

Performance

No findings.

Maintainability

No findings.

Tech Debt

P3 — Tracked documentation limitations

  • Impact: Markdown examples are not executed, and deprecated fit-time aggregation remains elsewhere.
  • Tracking: DEFERRED.md:L130 and TODO.md:L81.
  • Concrete fix: No action required in this PR; follow the tracked rows.

Security

No findings.

Documentation/Tests

P2 — Codemod table is incomplete

  • Location: docs/migration-4.0.md:L197-L215
  • Impact: It claims to cover identifier renames but omits mappings including treatment_col→takeup and aggregation→level, plus contextual group, controls, and time migrations listed at docs/migration-4.0.md:L314-L346.
  • Concrete fix: Add safe mappings and explicitly enumerate contextual renames excluded from global regex replacement.

P2 — Results-field migration omits inference siblings

  • Location: docs/migration-4.0.md:L155-L164
  • Impact: Users are told only about overall_att→att, while the ledger also flips overall_se, overall_t_stat, overall_p_value, and overall_conf_int; ContinuousDiD uses the overall_att_* variants (docs/v4-deprecations.yaml:L611-L616, L724-L734).
  • Concrete fix: Document the full inference-field mapping, including the ContinuousDiD outlier.

Runtime tests could not run because this checkout lacks pytest, NumPy, and Sphinx. Static parsing and custom ledger checks confirmed 108 appendix rows, 21 groups, correct counts, and no duplicates.

Path to Approval

  1. Correct the M-001/M-002 SyntheticDiD fixes so preserving 3.x behavior means dropping the ignored arguments and retaining auto-regularization.

…arry their values

Addresses the CI review on #759 (1 P1 + 3 P2, all verified against source before
acting).

P1 - THE SYNTHETICDID ADVICE WOULD HAVE CHANGED ESTIMATES. The appendix told
readers to drop `lambda_reg`/`zeta` and "set `zeta_omega=`/`zeta_lambda=`
instead". But those params have been IGNORED since 3.0.0 - `synthetic_did.py:216`
warns "deprecated and ignored. Regularization is now auto-computed from data. Use
zeta_omega to OVERRIDE". So 3.x behaviour is auto-computed regularization, and
copying the old value into the new parameter activates an override that was
previously inert, changing unit weights, ATT and inference. The value-preserving
migration is to delete the argument and leave the new ones unset. Both cells now
say that explicitly and warn against the mechanical copy. The ledger's own "use
zeta_omega" note describes where the CAPABILITY moved, not a value migration -
reading it as the latter is what produced the error.

P2 - the results-field section documented only `overall_att` -> `att`, but the
whole inference quintet moves (`overall_se`, `overall_t_stat`, `overall_p_value`,
`overall_conf_int`). Added the mapping table, plus the ContinuousDiD outlier that
M-058's notes call out: its siblings carry an `att` infix
(`overall_att_se`, `overall_att_t_stat`, ...), so anyone grepping `overall_se`
misses them entirely.

P2 - the codemod table claimed to cover identifier renames but omitted
`treatment_col` -> `takeup` (the one `*_col` param whose replacement is not just
the suffix dropped), `aggregation` -> `level`, and `controls` -> `covariates`.
Added, and the three renames that CANNOT be global replacements are now
enumerated with the reason each is contextual: `group=` -> `unit=` would corrupt
`TripleDifference.fit`, which takes a `group=` that is not renamed; `time=` ->
`post=` survives everywhere else as the calendar column; `aggregate=` is not a
rename at all.

P2 - the signature-binding gate added in the previous round was itself vacuous:
deleting every example, or renaming the ```python fence, emptied its loop and it
passed. It now asserts a floor of 4 blocks and 20 resolvable keywords, and names
the six estimators whose worked examples must be present. Verified by mutation
rather than assumed - renaming the fence and deleting the QDiD example each fire
the expected assertion, and the guide is restored afterwards.

The gate I wrote to catch documentation errors had the same defect I built three
parser-vacuity tests to prevent on the table parsers. Adding a floor to a new
gate is not optional just because the gate is small.

Verified: 469 gate tests pass; Sphinx -W build succeeded with zero warnings;
black and ruff clean.
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: eff369c84a91e118df56687d38dc218c8941ce42


Overall assessment

Looks good — no unmitigated P0/P1 findings.

Executive summary

  • Prior SyntheticDiD P1 is resolved: ignored arguments are now dropped, not translated into active overrides.
  • Previous codemod, inference-field, and vacuity-test gaps are resolved.
  • Merge and inference guidance aligns with the Methodology Registry.
  • Appendix checks confirm 108 unique qualifying rows across 21 groups.
  • Remaining aggregation and test limitations are explicitly tracked.

Methodology

No findings. The corrected SyntheticDiD advice at docs/migration-4.0.md:L334-L335 matches its ignored-parameter behavior and Registry auto-regularization equations.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

P3 — Tracked aggregation limitations

  • Impact: Some bootstrapped post-fit aggregations and staggered-DDD aggregation remain unavailable, but fail explicitly rather than returning incorrect inference.
  • Concrete fix: No action required here; tracked in TODO.md:L24 and TODO.md:L54.

Security

No findings.

Documentation/Tests

P3 — Markdown examples are not executed

  • Impact: Values and call sequences are not runtime-tested, though signatures are checked with non-vacuous guards at tests/test_v4_matrix.py:L1297-L1313.
  • Concrete fix: No action required; tracked in DEFERRED.md:L130.

Static ledger, parser, documentation-dependency, and syntax checks passed. Full pytest/Sphinx execution was unavailable because the environment lacks pytest, NumPy, pandas, SciPy, and Sphinx.

@igerber igerber added the ready-for-ci Triggers CI test workflows label Aug 9, 2026
@igerber
igerber merged commit 54df914 into main Aug 10, 2026
35 of 36 checks passed
@igerber
igerber deleted the feat/v4-migration-guide branch August 10, 2026 00:19
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