Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DEFERRED.md
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ decisions (refactor waivers, perf trade-offs, test-infrastructure calls) are rec
|----------|----------|----------|
| **Fixed-B Stata SE-golden comparisons in the LWDiD suite run unmarked in default CI** (no `ci_params.bootstrap()` scaling, no `slow` marker): the SE tolerance derives from BOTH fixed rep counts (ours B=999, Stata's R from the golden meta) so scaling either side would invalidate the committed-golden comparison — the ci_params convention governs convergence-style tests, not fixed-B golden comparisons — and these tests are the PR #588 acceptance bar, which must run in default CI; module-scoped fit memoization bounds the cost (~1-2 s per fit locally) | `tests/test_methodology_lwdid.py` | 2026-08-16 |
| **The 4.0 migration guide's code blocks are not snippet-executed.** `tests/test_doc_snippets.py` discovers a hardcoded list of `.rst` files and only `.. code-block:: python` / RST `::` bodies, so `docs/migration-4.0.md` gets no coverage. Deliberate: the guide is a MIXED document - most "after" examples (the renames, `results.att`) run on the current release, but the `field-flip` and `df-convention-flip` examples describe 4.0 behaviour that cannot run until 4.0, so a blanket execution lane would fail by construction. Closing the gap fully means a markdown-fence extractor plus a skip-marker convention for the future-API blocks - a harness change, out of scope for a docs PR. What IS gated: the appendix's ledger parity (`test_migration_guide_*`), which pins the row set and every mechanically checkable cell; and, since the first local review found all three merge examples carrying invalid keywords, `test_migration_guide_examples_bind_to_real_signatures`, which ast-parses the guide's python blocks and asserts every constructor/`fit()` keyword exists on the target signature. That is signature binding, NOT execution - it deliberately skips calls whose owner it cannot resolve (e.g. `results.aggregate(...)`), and it cannot catch a wrong VALUE or a wrong sequence of calls. The hand-written `Fix` prose remains unverifiable by any available means. | `docs/migration-4.0.md`, `tests/test_doc_snippets.py` | Phase 4 / 2026-08-09 |
| **MMM launch carousel carries scoped marketing claims, not exporter-contract documentation.** The deck (a LinkedIn marketing artifact, not a docs surface) states capabilities in scoped-but-punchy form: guardrail copy says "the easy mistakes fail loudly" / "you own the design, it owns the math" (the caller-owned estimand/population/window/outcome-scale alignment lives in REGISTRY.md's MMM section and the exporter docstrings, not on slides), and slide 8 shows tutorial 29's worked staggered-boost lift row WITHOUT an on-slide linearity qualifier - the compression's linear-channel scoping is the tutorial's job, which the CTA points to. Absolutes ("no silent mis-calibration", "any estimate exports", "anything with an estimate + SE") are ban-listed by `tests/test_mmm_carousel_claims.py`; local review R1/R2 pressed for compliance-style scoping language on the slides and the marketing-appropriate scoped copy was chosen instead. | `carousel/generate_mmm_carousel.py`, `tests/test_mmm_carousel_claims.py` | mmm-carousel / 2026-08-20 |
| **MultiPeriodDiD deprecation shim loses static constructor-arg checking (3.9 window).** The M-010 shim is `__init__(*args, **kwargs)` + an import-time `__signature__` mirror of DiD's constructor: runtime introspection (get_params/set_params, `inspect.signature`) and eager validation are fully preserved, but static type checkers / IDEs cannot check constructor arguments for the deprecated class until its 4.0 removal. Accepted: the alternative (hand-mirroring ~20 parameters) is a drift magnet on a class with one minor version of remaining life. | `diff_diff/estimators.py` | 3(a) / 2026-08-07 |
| **DCDH `sklearn.base.clone` param-identity failure won't-fix.** `ChaisemartinDHaultfoeuille._validate_paths_of_interest` unconditionally canonicalizes `paths_of_interest` into a fresh `List[Tuple[int, ...]]`, so sklearn `clone()`'s post-construction `param1 is param2` identity check fails for configured instances - a pre-existing normalization the BaseEstimator mixin PR documented rather than changed (get_params/set_params signatures are clone-compatible; the dependency-free `cls(**est.get_params())` config-equality contract is the enforced one, `tests/test_base_estimator.py`). Fixing would mean returning the caller's raw object from a validator whose job is canonicalization. | `chaisemartin_dhaultfoeuille.py` | mixin PR / 2026-08-01 |
| **scikit-learn stays out of dev deps; clone-identity tests remain importorskip-only.** The sklearn-`clone()` round-trip tests (`test_base_estimator.py`, had/rdd/cic suites) run only where scikit-learn happens to be installed - deliberate, matching the numpy/pandas/scipy-only dependency posture; the always-running contract is the dependency-free re-instantiation config-equality test. | `tests/test_base_estimator.py` | mixin PR / 2026-08-01 |
Expand Down
1 change: 1 addition & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m
| df-provenance completion follow-up (PR C descoping): a Wooldridge public per-row df channel (a per-key dict that can also carry the hc2_bm BM contrast dofs the scalar `_df_analytic_fallback` cannot represent — the unified surface's Wooldridge df column stays NaN until then); ImputationDiD lead-horizon `event_study_df` (needs the `_compute_lead_coefficients`/`_aggregate_event_study` return-path change plus bootstrap-clearing semantics for post rows); and the `inference_df` (DiD/MPD/SA/StackedDiD) vs `df_inference` (CS/dCDH) adapter-naming unification. NOTE (M-024): StackedDiD's `df_survey=None` on analytical fits is DELIBERATE - the container resolver's `df_inference` fallback not matching `inference_df` is load-bearing (normal-theory FLCI; see the REGISTRY StackedDiD M-024 Note and the pin in `tests/test_event_study_consumers.py`) - the unification must preserve that outcome or renegotiate it explicitly. | `diff_diff/wooldridge_results.py`, `diff_diff/imputation.py`, `diff_diff/results_base.py` | #variance-PR-C | Mid | Low |
| `worktree-rm` safety via a tested argv helper: the prose rewrite (ask-before-remove confirmation gate, detached-HEAD reachability/rescue, tip-identity force-delete guard) was reverted to the main version because editing the prose repeatedly reintroduced shell-injection (last: sourcing a state file built from a git-derived branch name). Restore those guards in a `worktree_rm.py` that takes the name via file ingress, invokes git through argv arrays, and has a metacharacter-branch/path injection regression test — the pattern that worked for `pr_prepare.py`/`premerge_scan.py`. | `.claude/commands/worktree-rm.md`, `.claude/scripts/` | skill-audit | Heavy | Medium |
| `premerge_scan.py` should scan the staged blob (`git show :path`) for staged methodology files, not the working-tree copy — a stage-then-revert-working-copy edit currently reads the safe working version and misses the staged violation. Union staged-index findings with unstaged/untracked filesystem findings. | `.claude/scripts/premerge_scan.py` | skill-audit | Mid | Low |
| Carousel PDF drift guard: the committed deck PDFs are build artifacts of their `generate_*.py` scripts (README rule: regenerate in the same commit), but nothing verifies a committed PDF matches its generator — a source edit without regeneration, or a stale raster chart, passes every claims test. Applies to ALL decks, not just the MMM one. fpdf embeds creation-date metadata, so a byte-compare needs fixed metadata or a per-page rendered-text manifest. Origin: mmm-carousel local review R1. | `carousel/`, `tests/` | mmm-carousel | Mid | Low |
| Re-add committed-range methodology scanning to `/push-pr-update` §3b (clean tree, commits ahead) using `premerge_scan.py --range`, with the comparison ref passed as **data** (resolved into a quoted variable in one Bash call, never a raw `<placeholder>`). It was removed to avoid ref interpolation; the helper already implements and tests `--range`. | `.claude/commands/push-pr-update.md` | skill-audit | Quick | Low |
| Codex reviewer isolation (repo-wide): `codex --sandbox read-only` blocks writes but does NOT confine READS to the worktree/repo (verified: a probe read `/etc/hosts`), and prompt guards are not a security boundary. Affects `/ai-review-local` (which already documents and accepts this surface) and plan-review-eval dual arms equally. Evaluate OS-level isolation for codex invocations (container / `sandbox-exec` profile / dedicated low-privilege account exposing only the worktree). | `.claude/scripts/openai_review.py`, `tools/plan-review-eval/` | plan-review-eval local review R6 | Heavy | Medium |
| plan-review-eval dual arms: terminate the surviving peer subprocess when the first reviewer of a dual pair fails (today an early Claude failure still waits out the codex peer — up to its 3600s ceiling — before the run becomes INFRA_ERROR; needs process handles threaded through `_call_claude`/`call_codex` or a shared cancellation event). | `tools/plan-review-eval/plan_adapters/plan_reviewer.py` | plan-review-eval local review R3 | Mid | Low |
Expand Down
Binary file added carousel/diff-diff-mmm-carousel.pdf
Binary file not shown.
Loading
Loading