Skip to content

docs(v4): 4.0 design spec + CI-enforced deprecation ledger (Phase 1)#697

Merged
igerber merged 1 commit into
mainfrom
docs/v4-phase1-design-spec
Jul 19, 2026
Merged

docs(v4): 4.0 design spec + CI-enforced deprecation ledger (Phase 1)#697
igerber merged 1 commit into
mainfrom
docs/v4-phase1-design-spec

Conversation

@igerber

@igerber igerber commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Phase 1 of the 4.0 program: the normative design spec (docs/v4-design.md) every later 4.0-program PR is reviewed against - three estimator merges (TWFE absorbs MultiPeriodDiD with a unit-FE event-study default and spec="pooled" compatibility; TripleDifference becomes a facade over the 2x2x2 and staggered DDD engines; ChangesInChanges absorbs QDiD via method=), post-fit results.aggregate() with the unified {simple, event_study, group, calendar} vocabulary, the canonical results-quintet storage flip, library-wide param/column contract rules, one inference surface, the final alias table, and the 3.9-shim-then-4.0-enforce release sequencing with the maint/3.8 patch-line rule.
  • Machine-checked deprecation ledger (docs/v4-deprecations.yaml, 69 rows): every queued deprecation - the six locked design decisions expanded per-surface plus all five pre-existing 4.0 obligations found by repo sweep (including the df_convention default flip, which carries no in-code warning and would be invisible to a warnings-grep at cut time).
  • Enforcement test (tests/test_v4_matrix.py): asserts every ledger row's status against reality at HEAD - planned rows tripwire when new surface ships unflipped, shimmed/removed rows pin both sides plus dedicated-test existence, alias rows assert identity and __all__ membership, default-flip rows pin signature defaults, and the release gate is two-sided (due rows must flip at the scheduled version; early removals before it fail). Committed scanner self-tests with negative fixtures for every schema rule. Wired into docs-tests.yml (doc-snippets job + paths) so docs-only ledger/spec edits cannot bypass enforcement; also runs in the main test matrix.
  • Support edits: docs/conf.py exclusion (internal doc, Sphinx -W safe), REGISTRY.md MultiPeriodDiD labeled Note reconciling the unit-FE parity target with the 3.x pooled default and the scheduled 4.0 migration, ROADMAP pointer, CHANGELOG [Unreleased], CLAUDE.md reference row, TODO.md lifecycle pointers, .claude/memory.md current-vs-target API conventions.

Forward-reference note for review: new locators on planned ledger rows intentionally name 3.9/4.0 surface that does NOT exist at HEAD - tests/test_v4_matrix.py asserts their ABSENCE until the owning phase ships. Nothing here is an undocumented deviation; this document set is the documentation.

Methodology references (required if estimator / math changes)

  • Method name(s): No estimator code changes. docs/methodology/REGISTRY.md gains a documentation-only labeled Note in the MultiPeriodDiD section (unit-FE target vs 3.x pooled default vs scheduled 4.0 migration).
  • Paper / source link(s): N/A - no methodology equations changed.
  • Any intentional deviations from the source (and why): None; the Note documents a scheduled 4.0 default migration (gated on the Phase 3 equivalence/divergence/parity test triple), not a current behavior change.

Validation

  • Tests added/updated: tests/test_v4_matrix.py (new; 158 checks green at HEAD, sub-0.1s, pure introspection, backend-neutral; module-level skip for installed-package CI runs, mirroring test_doc_deps_integrity.py). Workflow contract suite (tests/test_openai_review.py -m '', 254 checks) green after the docs-tests.yml edit.
  • Backtest / simulation / notebook evidence (if applicable): N/A - docs + test only; no public API or numerical behavior change. Local Sphinx -W build green with the new internal doc excluded.

Security / privacy

  • Confirm no secrets/PII in this PR: Yes

🤖 Generated with Claude Code

https://claude.ai/code/session_017eVKbBqzenDKa5idwtexQa

@github-actions

Copy link
Copy Markdown

Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • No estimator code, math, weighting, variance, or default behavior changes were introduced.
  • The MultiPeriodDiD registry addition is explicitly labeled **Note:**, so the documented pooled-vs-unit-FE migration is not a methodology defect.
  • The new matrix/test framework is directionally sound, but two enforcement gaps should be tightened before later 4.0 PRs rely on it.
  • I could not execute tests in this container because pytest and numpy are not installed.

Methodology

No P0/P1 findings.

  • Severity: P3
    Impact: The MultiPeriodDiD pooled-default vs unit-FE target discrepancy is documented in docs/methodology/REGISTRY.md:L211-L225, matching the review rule for documented deviations.
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

No separate findings.

Security

No findings. No secrets or credential material observed in the changed files.

Documentation/Tests

  • Severity: P2
    Impact: The new enforcement test is not wired into the docs workflow, so future docs-only edits to docs/v4-design.md or docs/v4-deprecations.yaml can bypass the ledger checks. docs-tests.yml triggers on docs/** but only runs test_doc_snippets.py and test_doc_deps_integrity.py; rust-test.yml runs all tests only when tests/**/code paths change. This weakens the “CI-enforced deprecation ledger” claim. See .github/workflows/docs-tests.yml:L24-L34, .github/workflows/docs-tests.yml:L69-L80, .github/workflows/rust-test.yml:L40-L44, and tests/test_v4_matrix.py:L1-L22.
    Concrete fix: Add tests/test_v4_matrix.py to the docs workflow test step, and include it in the docs-tests path filter if needed.

  • Severity: P2
    Impact: The spec says the aggregation vocabulary is closed over "simple", "event_study", "group", "calendar" and that Wooldridge’s "event" spelling dies via M-044, but M-044 only tracks to_dataframe[aggregation] -> level. It does not track WooldridgeDiDResults.aggregate(type="event") or summary(aggregation="event"), both current public surfaces. See docs/v4-design.md:L303-L314, docs/v4-deprecations.yaml:L503-L515, diff_diff/wooldridge_results.py:L188-L224, and diff_diff/wooldridge_results.py:L588-L716.
    Concrete fix: Add ledger rows for the Wooldridge aggregate(type) value migration and summary(aggregation) surface, or narrow the spec so M-044 explicitly covers only to_dataframe.

@igerber
igerber force-pushed the docs/v4-phase1-design-spec branch from 037f2f3 to 8507736 Compare July 18, 2026 19:14
@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 8507736bb355aa986a37816f62a7bfab49e904eb


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • No estimator code, math, weighting, SE, or current default behavior changed.
  • The MultiPeriodDiD pooled-vs-unit-FE migration is explicitly documented as a **Note:** in the methodology registry, so it is not a methodology defect.
  • The prior docs-workflow gap is resolved: tests/test_v4_matrix.py now runs in docs-tests.yml.
  • One prior P2 remains: the Wooldridge "event" aggregation spelling is still only partially represented in the v4 ledger.
  • I could not run tests because pytest is not installed in this container.

Methodology

No P0/P1 findings.

  • Severity: P3
    Impact: The MultiPeriodDiD default-spec difference is documented in docs/methodology/REGISTRY.md:L211-L225 and matches the current pooled design in diff_diff/estimators.py:L1812-L1841.
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

No separate findings.

Security

No findings. I did not observe secrets or credential material in the changed files.

Documentation/Tests

  • Severity: P2
    Impact: The previous CI enforcement gap is fixed in .github/workflows/docs-tests.yml:L6-L11 and .github/workflows/docs-tests.yml:L84-L89.
    Concrete fix: None required.

  • Severity: P2
    Impact: The spec still says Wooldridge’s "event" spelling dies under the unified "event_study" vocabulary, but the ledger row M-044 only tracks to_dataframe[aggregation] -> level. Current public surfaces also accept "event" in aggregate(type=...) and summary(aggregation=...), so later PRs could leave those paths unmigrated while the matrix stays green. See docs/v4-design.md:L303-L315, docs/v4-deprecations.yaml:L503-L515, diff_diff/wooldridge_results.py:L188-L224, and diff_diff/wooldridge_results.py:L588-L715.
    Concrete fix: Add ledger rows for the Wooldridge aggregate(type="event") value migration and summary(aggregation=...) surface, or narrow the spec so M-044 explicitly covers only to_dataframe.

@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: a2a28a7b0d887d7956ea0eb196a5cc0f0ace86bc


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • No estimator code, math, weighting, SE, or current default behavior changed.
  • MultiPeriodDiD is affected only through documentation; the new **Note:** in the Methodology Registry matches the current pooled 3.x implementation.
  • The prior Wooldridge ledger coverage gap is addressed by adding M-086 for aggregate(type="event") and M-087 for summary(aggregation=...).
  • Docs-only edits now run the v4 matrix enforcement in docs-tests.yml.
  • One P2 remains: param-value rows are schema-only, so M-086 can still avoid meaningful 4.0 behavior enforcement.

Methodology

No P0/P1 findings.

  • Severity: P3
    Impact: The MultiPeriodDiD note is an explicitly documented deviation/migration note, not a defect. It says 3.x defaults to the pooled treatment-main-effect + period-dummy + interaction design, while unit-FE event-study is the 4.0 target; that matches the current design matrix construction and optional FE handling. See docs/methodology/REGISTRY.md:L211-L225 and diff_diff/estimators.py:L1812-L1866.
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings. The new matrix test is pure introspection and not on estimator hot paths.

Maintainability

No findings.

Tech Debt

No blocker findings. The v4 default flips and fastpath decision are now cross-linked from TODO.md into the ledger.

Security

No findings. I did not observe secrets or credential material in the changed files.

Documentation/Tests

  • Severity: P2
    Impact: M-086 now tracks the Wooldridge "event""event_study" value migration, resolving the prior missing-ledger-row concern. However, param-value rows are explicitly schema-only and skipped by the reality test, and the due gate does not require param-value rows to become terminal at removed_in. A future PR could leave aggregate(type="event") accepted, mark or park the row without a behavior pin, and the matrix would not catch it. See docs/v4-deprecations.yaml:L845-L857, docs/v4-design.md:L453-L495, and tests/test_v4_matrix.py:L449-L457, tests/test_v4_matrix.py:L628-L629.
    Concrete fix: Give param-value rows a real lifecycle/enforcement path: require test_ref at shim/removal, add due-gate handling for removed_in, and add a focused test fixture for M-086 asserting old value warning/removal and new value acceptance.

  • Severity: P3
    Impact: The prior docs-workflow gap is fixed: tests/test_v4_matrix.py is now run by the docs workflow, so docs/spec/ledger-only edits cannot bypass this enforcement job. See .github/workflows/docs-tests.yml:L84-L89.
    Concrete fix: None required.

Validation note: I could not run the test suite in this container because pytest is not installed; a direct package import also fails here because numpy is missing.

@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: d28717469487c7f5f91cba6dbce216f796f4828f


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • No estimator code, math, weighting, SE, or current default behavior changed.
  • The MultiPeriodDiD registry note is an explicit **Note:** documenting current 3.x pooled behavior and the planned 4.0 unit-FE migration.
  • The previous P2 around param-value lifecycle enforcement is addressed: M-086 now has lifecycle/due-gate coverage and required behavioral-test linkage at shim/removal.
  • The new docs workflow step runs tests/test_v4_matrix.py, so docs/spec/ledger-only edits cannot bypass the matrix check.
  • I could not run pytest locally because pytest is not installed; static Python syntax parsing of tests/test_v4_matrix.py passed.

Methodology

  • Severity: P3
    Impact: MultiPeriodDiD is affected only by documentation. The new registry **Note:** correctly documents that 3.x defaults to the pooled treatment-main-effect + period-dummy + interaction design, with unit fixed effects only when requested via absorb/fixed_effects; this matches the implementation. Because the deviation/migration is explicitly labeled in the Methodology Registry, it is informational, not a defect. docs/methodology/REGISTRY.md:L211-L225, diff_diff/estimators.py:L1812-L1866
    Concrete fix: None required.

Code Quality

No findings.

Performance

No findings. The new matrix enforcement is introspection/schema checking only and is not on estimator hot paths.

Maintainability

No findings. The immutable row-id snapshot, row-count floor, schema self-tests, and spec cross-reference checks improve ledger maintainability. tests/test_v4_matrix.py:L112-L122, tests/test_v4_matrix.py:L385-L391, tests/test_v4_matrix.py:L855-L924

Tech Debt

No findings. Deferred 4.0 obligations are now tracked in the ledger and cross-linked from TODO where relevant. TODO.md:L43-L43, TODO.md:L120-L120

Security

No findings. I did not observe secrets or credential material in the changed files.

Documentation/Tests

No findings. The previous Wooldridge param-value gap is resolved: the spec now requires lifecycle and due-gate handling plus test_ref at shim/removal, and the test suite implements those checks with negative coverage. docs/v4-design.md:L453-L502, tests/test_v4_matrix.py:L64-L69, tests/test_v4_matrix.py:L206-L209, tests/test_v4_matrix.py:L453-L457, tests/test_v4_matrix.py:L592-L607, docs/v4-deprecations.yaml:L845-L857

Validation note: pytest is unavailable in this container (pytest: command not found), so I did not execute the test suite. I did run an AST parse check for tests/test_v4_matrix.py, which passed.

@igerber igerber added ready-for-ci Triggers CI test workflows and removed ready-for-ci Triggers CI test workflows labels Jul 18, 2026
Normative design for the 4.0 program (docs/v4-design.md): three estimator
merges (TWFE absorbs MultiPeriodDiD with a unit-FE event-study default and
spec="pooled" compatibility, TripleDifference facade over the DDD pair, CiC
absorbs QDiD via method=), post-fit results.aggregate() with the unified
{simple, event_study, group, calendar} vocabulary, canonical results quintet
storage flip, library-wide param/column contract rules (post vs time,
first_treat, covariates, no _col suffixes), one inference surface, the
final alias table, and the 3.9-shim-then-4.0-enforce release sequencing
with the maint/3.8 patch-line rule.

Every deprecation lifecycle is a row in docs/v4-deprecations.yaml (71 rows:
the six locked design decisions expanded per-surface plus all five
pre-existing 4.0 obligations found by repo sweep, including the
df_convention default flip that carries no in-code warning; Wooldridge's
existing aggregate(type=)/summary(aggregation=) surfaces are covered by
dedicated value-migration rows). tests/test_v4_matrix.py asserts every
row's status against reality at HEAD: planned rows tripwire on new surface
appearing unflipped, shimmed/removed rows pin both sides plus
dedicated-test existence, param-value rows carry the full lifecycle with
test_ref-backed value enforcement, alias rows assert identity and __all__
membership, default-flip rows pin signature defaults, and the release gate
is two-sided (due rows must flip; early removals before the scheduled
version fail). Wired into docs-tests.yml so docs-only ledger/spec edits
cannot bypass enforcement. REGISTRY.md MultiPeriodDiD gains a labeled Note
reconciling the unit-FE parity target with the 3.x pooled default and the
scheduled 4.0 migration.

Docs + test only; no public API or numerical behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017eVKbBqzenDKa5idwtexQa
@igerber
igerber force-pushed the docs/v4-phase1-design-spec branch from d287174 to 6e5dd83 Compare July 19, 2026 00:27
@github-actions

Copy link
Copy Markdown

🔁 AI review rerun (requested by @igerber)

Head SHA: 6e5dd83490a1650b478deaf7a4702d2588aa9fb5


Overall Assessment

✅ Looks good. No unmitigated P0/P1 findings.

Executive Summary

  • No estimator code, current math, weighting, SE/variance logic, or default behavior changes are introduced.
  • The only methodology-touching change is a labeled **Note:** for MultiPeriodDiD, so it is documented and informational.
  • The current implementation matches the new registry note: pooled treatment main effect + period dummies + interactions by default, with FE only via absorb/fixed_effects.
  • The prior param-value lifecycle enforcement gap remains addressed by schema, due-gate, and M-086 ledger coverage.
  • pytest is unavailable in this container, so I could not execute the suite; AST parsing of tests/test_v4_matrix.py passed.

Methodology

  • Severity: P3 informational
    Impact: MultiPeriodDiD is affected only by documentation. The registry note explicitly documents the current 3.x pooled default and the planned 4.0 unit-FE migration; that matches the implementation’s design construction. Because this is labeled **Note:** in the Methodology Registry, it is not a defect under the review rules. docs/methodology/REGISTRY.md:L212-L226, diff_diff/estimators.py:L1812-L1866, docs/v4-design.md:L178-L202
    Concrete fix: None required.

Code Quality

No findings. The matrix test keeps the parser/schema logic scoped and includes negative self-tests for schema drift. tests/test_v4_matrix.py:L176-L252, tests/test_v4_matrix.py:L855-L924

Performance

No findings. The new enforcement is import/signature/schema introspection only and is not on estimator hot paths.

Maintainability

No findings. The immutable initial row snapshot, row-count floor, spec cross-reference check, and core status-vs-reality assertions make the ledger maintainable across later 4.0 PRs. tests/test_v4_matrix.py:L112-L122, tests/test_v4_matrix.py:L372-L391, tests/test_v4_matrix.py:L643-L815

Tech Debt

No findings. Deferred default-flip and fastpath decisions are now tracked in the ledger and cross-linked from TODO. TODO.md:L45, TODO.md:L121, docs/v4-deprecations.yaml:L63-L126

Security

No findings. I did not observe secrets or credential material in the changed files.

Documentation/Tests

No findings. The previous M-086 param-value lifecycle concern is covered: param-value is a first-class kind, shim/removal requires test_ref, the due gate covers overdue value removals, and M-086 is present in the ledger. tests/test_v4_matrix.py:L64-L67, tests/test_v4_matrix.py:L206-L209, tests/test_v4_matrix.py:L453-L457, tests/test_v4_matrix.py:L592-L607, docs/v4-deprecations.yaml:L845-L857

Validation note: pytest and numpy are not installed in this container, so I could not run tests/test_v4_matrix.py. I ran a Python AST parse of tests/test_v4_matrix.py, which passed. The docs workflow installs pytest and runs the new matrix test for docs-triggered changes. .github/workflows/docs-tests.yml:L67-L89

@igerber igerber added the ready-for-ci Triggers CI test workflows label Jul 19, 2026
@igerber
igerber merged commit cffbe79 into main Jul 19, 2026
34 of 35 checks passed
@igerber
igerber deleted the docs/v4-phase1-design-spec branch July 19, 2026 09:38
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