feat: TWFE weight diagnostics (twfeweights R port, re-scoped from #753) - #812

Thanks for the re-scope - this is a strong port. I confirmed the upstream MIT license, ran the new suites locally (572 passed, no skips, so the goldens were exercised), and lint/black/mypy are clean. The weight formulas for all three aggregations, the FWL decomposition under both base periods, and all eleven balance statistics match R at machine precision on all three fixtures, and both of your documented deviations from R are mathematically right. The public surface, Diagnostic subclassing, column names, naming versus the dCDH surface, attribution, and the docs registrations all conform to what I asked for on #753.

Most of what follows is about the paths that go beyond the R reference - the ATT(g,t)-frame input, user-supplied weights, and non-standard cohort labels - where the port needs its own input validation because R never faced those inputs. Items 1 through 5 I reproduced by running the code. Please make the changes on this PR; once your revision is pushed I will open a maintainer-side mirror so the label-gated CI and the CI reviewer can run (they do not trigger on fork PRs), and I will bring anything CI surfaces back here.

Decisions on the two open questions

• Naming: option A (as implemented). Keep it.
• AIPW deferral: accepted. method="fwl" only is fine for this PR, with the cleanup in item 10 so nothing claims AIPW is present.

Items to address

1. Cohort labels: NaN and -inf are silently treated as never-treated. _is_never uses ~np.isfinite, so a missing or -inf first_treat becomes cohort 0 and the within-unit consistency check ignores it. Setting one treated unit's label to NaN moved decompose_twfe_weights(...).estimate from 2.2796 to 2.2481 with no warning. Never-treated should be exactly 0 or +inf; reject everything else non-finite up front and include missing values in the unit-invariance check. Add a test.

2. Balance summary turns NaN into 0. _frac_treated_extreme correctly returns NaN for a covariate with fewer than three distinct values (R's rule), but covariate_balance(level="summary") rolls cells up with a pandas .sum(), which skips NaN, so a binary or constant covariate reports unweighted_frac_extreme = 0.0 and weighted_frac_extreme = 0.0. R propagates NA. REGISTRY currently says the port "reproduces this exactly, including the NA return"; the summary does not. Propagate NaN through the roll-up and test both levels with a binary covariate and a constant covariate.

3. aggregation="twfe" accepts covariate-adjusted CS fits. R's twfe_weights stops on three conditions: never-treated control, universal base period, and xformla == ~1. _guard_cs_design enforces the first two and its error text says it matches R, but nothing checks covariates because CallawaySantAnnaResults does not record them. A CS fit with covariates=[...] is accepted with no warning, and the result docstring then calls implied_att "the TWFE coefficient", which it is not. Record covariate usage on the result (the _aggregation_kit.bookkeeping dict is a reasonable home) and raise in the guard.

4. The ATT(g,t)-frame route accepts incomplete, duplicated, and non-finite input. Reproduced: dropping the (g=3, t=3) post cell before attgt_weights(..., aggregation="overall") gives post weights summing to 0.8333 with no warning, so the result is no longer ATT^O; a duplicated (group, time) row is accepted silently; an inf ATT is accepted and yields implied_att = inf with no warning (the fitted-result path uses np.isfinite, the frame path only drops NaN). Please: reject duplicate cells; apply np.isfinite to att, group, and time; fail closed on an incomplete grid for twfe; and for overall/simple either raise when a required post cell is missing or define an available-cell estimand with explicit renormalization and document it. The NaN-cell warning text ("the reported weights renormalize") describes behaviour the code does not have for a dropped post cell.

5. User weights are not validated. Only the total is checked. Negative weights (I passed -1 for the first 50 rows) are accepted and produce an implied_att; non-finite weights propagate. Require finite, non-negative weights with a positive total and positive treated and control mass, checked in one place that both entry points share.

6. The 0/0-cell deviation affects more fields than REGISTRY admits, and those fields are never asserted. On sim_staggered the two cancelling cells sit on opposite sides of the pre/post split, so versus the pinned R values pretrend_bias and post_only differ by 1.2e-4, effective_sample_size by 0.99, and cell ess/remainder at those cells by more. All are user-visible (summary() prints them). The REGISTRY note should enumerate them with magnitudes, and test_twfe_weights_parity.py should assert pretrend_bias, post_only, effective_sample_size, cell ess, and cell remainder, tight where _degenerate_mask is empty and relaxed only where it fires. Related: the scalar-split relaxation currently fires for sim_staggered/fwl_nocov too, where the observed gap is 3e-15 - restrict it to fwl_gmin1; and fwl_gmin1 cell weights/ATTs are pinned but not asserted.

7. Route the FWL linear algebra through the house helpers. _wls_coefficients uses np.linalg.lstsq, _drop_collinear a pivoted QR, and _demean_two_way its own alternating-projection loop. The library rule is that all estimation goes through diff_diff.linalg.solve_ols (with its rank-deficient handling and weights=) and diff_diff.utils.within_transform / demean_by_groups (which take weights= and a tolerance). Please replace the three with those; the parity suite will tell you immediately if anything moves.

8. Test coverage for the decomposition half and the plot. decompose_twfe_weights and covariate_balance are exercised only in the golden-gated parity file; tests/test_twfe_weights.py covers attgt_weights alone, and plot_twfe_weights has no test at all. Of the raise/warn sites in the decomposition half, two are asserted anywhere. Please add a decomposition block to test_twfe_weights.py (the REGISTRY edge cases: unbalanced panel, no never-treated group, time-varying cohort, gmin1 with a first-period cohort, covariate_balance() without balance_covariates=, bad level=; plus estimate == decomposition + remainder and implied_att == estimate on a synthetic panel), a plot_twfe_weights test next to the plot_bacon tests (both kind= branches, ax= reuse, the balance branch raising when there is no balance table, and an all-NaN balance table producing a clear error rather than a nanmax warning), and regression tests for items 1-5.

9. negative_weight_share counts pre-period cells. Over the g != 0 grid the TWFE weights sum to zero (post to +1, pre to -1), so n_negative > 0 and a share near 0.5 appear in every staggered design, including one with no negative post-period weight. The pathology the docstring describes is negative weight on post cells. Report post-only counts (or both, labelled), and add a REGISTRY Note defining the statistic - it has no R counterpart.

10. Remove the AIPW residue. TWFEDecompositionResult docstrings describe method="aipw"; the R generator header says it pins implicit_aipw_weights and "two private two-period kernels" that have no Python surface; REGISTRY says the kernels are "pinned directly by the parity suite", but the test never reads two_period.*, decompose.aipw, or balance.aipw (roughly 2,100 unused floats). Either strip those blocks from the JSON or label them in meta and the header as reserved for the AIPW follow-up, and fix the prose. For that follow-up, note that the pinned AIPW golden is covariate-adjusted (a time-invariant covariate is not a no-op in the propensity score).

11. Drop twfeweights_mpdta_panel.csv. It is benchmarks/data/mpdta_stata_panel.csv with renamed columns plus lpop_t = lpop * (period - 2002) / 5; the five shared columns are bit-identical. Have the parity fixture rename and derive from the existing file (the golden's columns map already makes the test column-name-agnostic), and record the mpdta provenance (data(mpdta, package="did")) in the JSON meta. The two simulated panels are fine to keep.

12. Label-aligned cell comparisons. The decompose and balance parity tests compare weight/att by array position without asserting (group, time, post), and the JSON mixes positional labels (decompose.*.cells, balance.*.cells) with raw labels (attgt_weights.*). Map back to original labels in the generator so every block shares one convention, then assert labels the way test_weight_column does.

13. Add the plotly backend to plot_twfe_weights. It is the only plot function in the module without backend="matplotlib"|"plotly"; its two file-mates plot_sensitivity and plot_bacon both carry it with a _render_*_mpl / _render_*_plotly split, so please follow that pattern (tests live in tests/test_visualization_plotly.py). Also type results as the result class rather than Any, and update the module docstring, which still lists only sensitivity and Bacon.

14. Landing-page discipline. The README line is about 600 characters against 130-290 for its neighbours and carries two call signatures and an argument list; cut it to the sibling shape (method, one-clause description, upstream credit). The changelog fragment reads as a PR description; keep the headline bullet and the three sub-bullets and drop the naming rationale, golden paths, and bibliography, which live in REGISTRY and the API page.

15. Weighted ECDF is quadratic in unique values. Each knot rescans the full vector, repeated per covariate, cohort, and period. Sort once and take cumulative normalized weights at unique-value boundaries.

16. Annihilated-covariate Note. It says the rounding column "amplifies noise by ~1e16 and corrupts the weights". On mpdta's lpop the demeaned column is exactly unit-constant, in the FE span, and orthogonal to the treatment residual; keeping it changes the FWL residual by ~2e-18. Dropping an exactly-zero column is still right, so keep the behaviour but rewrite the Note as numerical hygiene. Also the 1e-10 relative threshold can drop a covariate with a large level and small genuine within-variation, and the "no within variation" warning would then be false.

17. Weighted aggregation="twfe" is an extension R does not have (twfe_weights takes no w=). The algebra checks out on a balanced panel, but REGISTRY defines p_g unweighted; add a Note and a test against decompose_twfe_weights(weights=...).

18. Constructed fixtures in tests/helpers/results_foundation.py. The ATTGTWeightsResult declares negative_weight_share=0.25 while its weights give 0.143, and the TWFEDecompositionResult fixture's post_only=0.5 does not match its cells. Derive them from the cells as estimate/decomposition already are.

19. Two weak unit tests. test_implied_att_is_the_weighted_sum re-implements sum(w * att) and so cannot fail; the __all__ membership test checks three of the five exported names.

20. Fixture prose. "A real pre-trend so pretrend_bias != 0" is not what the DGP does: x1 is iid and cohorts are assigned by unit index, so the 0.093 is sampling noise. Either make x1's mean depend on cohort or soften the claim. Likewise "well-conditioned by construction" for the covariate branch: both structured terms of xtv are absorbed by the two-way FE, leaving noise as the regressor.

21. Docs housekeeping. docs/doc-deps.yaml: the new sources: block sits under the # BaconDecomposition banner above diff_diff/bacon.py; give it its own banner. llms-full.txt: every other plot function has a ### plot_x subsection; add one for plot_twfe_weights. benchmarks/R/requirements.R lacks twfeweights, BMisc, DRDID. Add docs/api/twfe_weights.rst to tests/test_doc_snippets.py::RST_FILES so its code blocks execute.

Please rebase onto current main when you push (it moved by one commit). Items 10-12 and 20 touch the R generator if you take them fully; everything else can be done against the committed goldens without re-running R.

Thanks — all 21 items are addressed, rebased onto current main (it moved by five commits; the three shared docs files merged cleanly). Items 1–5 are fixed as reproduced bugs with regression tests.

Decisions on the three you left open

Item 4 — fail closed, with two R-mirroring carve-outs. An incomplete grid now raises for all three aggregations and on both input paths; "twfe" requires the full cohort × period grid (pre cells enter h(g,t)), the CS estimands the post grid. I did not define an available-cell estimand. But testing turned up two gaps that are structural rather than user error, and raising on them would have made the function unusable on ordinary panels, so both mirror R instead and warn:

• A cohort with no estimable post cell (canonically one treated in the first observed period, which has no base period) is dropped from the table and the cohort masses, exactly as did::pre_process_did drops units already treated in the first period. The criterion is post cells, not all cells — a cohort can have an estimable universal-base pre cell and still no usable post cell, and with an "all cells" rule that cohort survives into "overall" with an available-period divisor of zero.
• Under control_group="not_yet_treated" the last cohorts run out of comparison units and CS marks those post cells zero_treated_control. For "overall"/"simple" they are treated as structurally absent and each cohort is averaged over its available post periods — what aggte(type="group"/"simple") computes on such a fit. "twfe" requires never-treated controls and never reaches this branch.

Both read skip_reason, which to_dataframe("group_time") carries, so the canonical handoff behaves identically on both paths; only a bare hand-built frame stays strict. Documented in REGISTRY as two - **Note:** entries.

Item 20 — prose only. The DGP is unchanged, so the machine-precision parity and the degenerate-cell analysis are untouched. I corrected three claims in that header, not one: the pre-trend claim, the "well-conditioned" claim, and — the one that was actually load-bearing — "3 equal cohorts of 100 so no (g,t) cell is degenerate". The equal cohorts are precisely what makes the normalizer vanish at t=3; that fixture deliberately exercises the 0/0 cells.

Item 10 — labelled reserved, not stripped. decompose.aipw, balance.aipw and two_period.* stay, labelled in meta.reserved_blocks and the generator header, so the AIPW follow-up needs no R re-run. I did not "fix" the header's two-period-kernel sentence: it is accurate (:247–:249, :275–:276 do emit them). It was mislabelled, not wrong, so it now says reserved. The AIPW golden being covariate-adjusted is recorded there too.

Two places I pushed back

Item 7 — I removed _drop_collinear rather than keeping it. I first kept it, on the theory that BMisc::drop_collinear drops "later columns first" while solve_ols pivots by norm, so replacing it would make the collinearity warning name a different covariate than R. That was wrong: _drop_collinear was itself a norm-pivoted scipy.linalg.qr — the same rule solve_ols uses — and on X = [x0, x1, x2, x0+x1] both drop index 0. Its docstring's "dropping later columns first" described neither its own behaviour nor BMisc's. So it is gone, the solve is solve_ols(..., weights=, rank_deficient_action="silent"), and the dropped names are read off the R-style NaN coefficients while the returned residual is the FWL residual. Demeaning is within_transform on the sorted long frame before the reshape, with the treatment indicator synthesized as a column (it is derived from cohorts × positional periods, not an input column) and _Panel retaining both the raw and demeaned blocks so the annihilation filter's scale-relative test survives.

Worth flagging: no parity fixture is weighted, and nothing called decompose_twfe_weights(weights=...), so the suite could not have caught a regression in exactly the branches this refactor replaces. Before touching anything I captured a frozen-numbers pin of the weighted paths on the pre-refactor code (TestWeightedRegressionPin); it is green after.

Item 3 — no frame-path warning. The guard raises on the fitted path as you asked (covariate names recorded on the kit, threaded at both build sites — staggered.py:3052 and dml_did.py:2404; DMLDiD requires covariates, so its fits now correctly raise under "twfe"). A frame carries no covariate record at all, so I documented the caller's responsibility in the docstring and REGISTRY rather than emitting a warning that would fire on every to_dataframe("group_time") handoff — including all the existing twfe parity cases — with nothing actionable. An isinstance check keeps the legacy warn-only branch reachable only by a genuine pre-change CS pickle; four other estimators build AggregationKit directly and would otherwise have fallen into it silently.

Item 6, and a correction to the magnitudes

REGISTRY now enumerates every field the 0/0 deviation moves, with magnitudes, and the suite asserts all of them. One nuance: effective_sample_size (gap ~0.99) and cell ess (up to ~0.53) are ~20× the only relaxed constant, so a single loosened tolerance would have been meaningless. Instead the expectation is rebuilt from R's own cells — R's weights and R's ess wherever the degeneracy is not detected, our limit substituted only at the detected cells. That keeps the assertion anchored to R rather than to our implementation, which would have been the tautology item 19 exists to remove. The split relaxation is restricted to fwl_gmin1 as you asked (fwl_nocov's gap is 3e-15), and fwl_gmin1 cells are now asserted.

Items 11–12

Regeneration is numerically inert, and I checked rather than assumed: a structural diff against the previous golden shows 192 changed numbers, all of them inside group/time label arrays, and zero elsewhere. Every cells block now carries original period labels (implicit_* run in positional time internally; the generator maps back), and the tests assert labels instead of array position.

twfeweights_mpdta_panel.csv is gone; the fixture reads the shared mpdta_stata_panel.csv and derives lpop_t from a derived_columns expression in the golden, applied by _fixture(). build_fixture gained emit-only data_file_out/columns_out overrides so the R calls keep using mpdta_df's own names. One correction: the generator asserts the two sources agree at CSV round-trip precision, not bit-for-bit — data(mpdta) in memory versus a 15-digit CSV differ by ~1e-15. That gap is pre-existing and unchanged, since the old fixture CSV was the same round-trip.

One thing this surfaced that neither of us had run

tests/test_naming_guard.py was failing on the pre-existing branch — the PR's new public surface (attgt_weights[time], [aggregation], ATTGTWeightsResult.aggregation) matches section-8 rename vocabulary with no ledger row, and REGISTRY.md / llms.txt became old-name readers under M-030/031/044/082/087/137/138. Your local run and mine both missed it because we ran the new suites, not the full sweep. Fixed: a local weight_col that collided with M-113's trim_weights[weight_col] is renamed (avoiding the hit rather than allowlisting it), and the genuine shared-vocabulary hits are allowlisted with reasons — our time= is the panel period column (as in CallawaySantAnna.fit[time]), not the two-period 0/1 dummy those rows rename to post; our aggregation= selects an estimand, not Wooldridge output granularity.

Verification

Full suite in pure-Python mode: 14,050 passed, 420 skipped, 3:43:18. The only remaining failure is test_allowlists_are_reachable (DMLDiDResults.groups / overall_att stale allowlist entries) — I verified it fails identically on origin/main, so it is pre-existing and unrelated; I left it alone rather than fold an unrelated fix into this diff. ruff and black clean; mypy at zero errors across 110 source files.

Ready for the maintainer-side mirror whenever you want to run the label-gated CI.

🤖 Generated with Claude Code

One logistical note: I can't add ready-for-ci myself — my permissions on this repo are pull only, so GitHub refuses the label write (wenddymacro does not have the correct permissions to execute AddLabelsToLabelable).

Not that it would help here anyway: as you noted, the label-gated CI and the CI reviewer don't trigger on fork PRs, so the label belongs on the maintainer-side mirror rather than on this PR. Whenever you open it, the branch is at 0501068d and ready.

Thanks - this was a thorough revision. I re-ran everything on 0501068d: the new suites, the naming guard, the docs tests, and the CallawaySantAnna and DML suites that cover the two small touches to existing estimator code, plus lint/black/mypy. All green. The five bugs I reproduced last round now fail closed, the linear algebra is on the house helpers (and, as you found, the old demeaning loop could never have run with more than one covariate), the carve-outs match did 2.5.1 on the designs I checked, and the golden regeneration is label-only, which I confirmed with my own structural diff. One housekeeping note: the test_allowlists_are_reachable failure you saw on main was fixed by #822 before your rebase, so it passes on your head now; nothing to do there.

Below is what is left. Most of it is one more pass over the same theme as before: the paths R never had to handle. Two of the correctness items are structural and cover several symptoms at once (a canonical time key, and balance/anticipation provenance on the aggregation kit), so the list is longer than the work. Items 1-6 and 8 I reproduced by running the code; 9-11 by reading it. Please take these on this PR; once pushed I will open the maintainer mirror for CI.

Decision

Rename pretrend_bias to pre_period_contribution. The API page says the pre-period component comes "from parallel-trends violations rather than from treatment", and the result docstring and REGISTRY say the same. Baker et al. (section 5.1.2) are explicit that pre-period estimates speak to restrictions in earlier periods, not to the post-treatment counterfactual, and that their reading depends on precision. This diagnostic carries no inference, so sampling noise alone produces a non-zero value (your own sim_staggered fixture is the example: the DGP has no pre-trend and the pinned value is 0.093). Since the API is unreleased, please rename the field and the summary label, and reword the three doc surfaces to: the sample contribution of pre-treatment cells, which can reflect differential pre-trends or sampling variation, and is diagnostic evidence rather than proof that the identifying assumption fails post-treatment.

Items to address

1. Numeric-string time labels silently corrupt the decomposition. The same panel with periods [1, 2, 10, 11, 12] gives estimate = 2.06481 as integers and 0.75460 as the strings "1", "2", "10", ..., with no error. Observations are sorted on the original representation while _positional_grid sorts on the coerced numeric value, so the positional treatment indicator, cohort map, and outcome columns no longer describe the same periods. Build one canonical numeric time key at validation and use it for sorting, reshaping, cohort mapping, and the grid; add a test asserting numeric and numeric-string labels agree on [1, 2, 10].

2. anticipation is ignored on "overall" and "simple". Post cells are defined as t >= g only. On an anticipation=1 CS fit the t = g-1 cells get post=0 and zero weight, so attgt_weights(fit, "simple").implied_att = 2.18933 against the fit's own aggregate("simple") of 2.15500. The diagnostic claims to reproduce the named estimand and does not. Read anticipation off the fitted result (record it on the kit alongside covariates), apply it to the post mask, the required-cell grid, and the available-period divisors, add an anticipation= parameter to the frame path, and pin both estimands against results.aggregate(...) with positive anticipation under both control groups. Document separately whether "twfe" deliberately keeps t >= g.

3. Unbalanced panels are accepted on the fitted path. decompose_twfe_weights correctly rejects an unbalanced panel, but attgt_weights(fit, "twfe") on an unbalanced CS fit returns 2.14277 where the actual TWFE coefficient on that panel is 2.17509. The cohort-share and E_t[D] formulas assume the same units in every period (Baker et al. state the balanced-panel assumption). Record is_balanced on the kit and reject unbalanced fitted results; on the frame path require exactly one observation per unit-period.

4. A NaN outcome silently returns an all-NaN decomposition. No error, no warning. Validate finite outcomes, covariates, and balance covariates before estimation (or define and implement a complete-case policy explicitly, then check balance after it).

5. Frame path does not honour the zero_treated_control carve-out. The carve-out is keyed on control_group == "not_yet_treated", which the frame path sets to None. On a not-yet-treated fit, attgt_weights(fit, "overall") succeeds while attgt_weights(fit.to_dataframe("group_time"), data=..., aggregation="overall") raises "needs the complete post-treatment grid ... [zero_treated_control]". Your reply and the REGISTRY Note both say the two paths behave identically. Key the carve-out on the skip_reason values present (that reason is only ever emitted under not-yet-treated), and add a not-yet-treated case to TestCSFitAndFrameAgree.

6. Fitted-path cohort drop is over-broad. Any cohort with no estimable post cell is dropped on the fitted path regardless of skip_reason. I set a mid cohort's post ATTs to NaN with skip_reason=None and it was dropped silently, with a warning saying this matches R's first-period-cohort rule. That is the non-structural case I asked to fail closed. Allow the drop only when the cohort is treated in the first observed period, or when every missing post cell of that cohort carries zero_treated_control; raise otherwise.

7. REGISTRY misdescribes R's mechanism for the second carve-out. did::pre_process_did with no never-treated group truncates the panel to t < max(g) and recodes the last cohort to never-treated; that truncation, not aggte averaging over available periods, is what produces the number. The values agree either way (I checked against did 2.5.1), so this is prose only.

8. Annihilation threshold is still too blunt, and the result then misreports. A covariate with level 1e6 and genuine within-unit sd 1e-4 is dropped (ratio 1e-10); sd 1e-3 survives. float64 rounding after demeaning a 1e6-level column is about 1e-16 relative, so 1e-10 is five orders too loose. Tighten to a rounding-noise scale (something like sqrt(n) * 64 * eps * max(raw, 1)), which still annihilates mpdta's lpop exactly. Separately, after a drop the result still reports covariates == ('big',); pass the surviving names, not the user's list, into TWFEDecompositionResult.covariates.

9. Plot title counts pre cells. _twfe_weights_payload sets n_negative over the full table and both renderers append "(N negative)" to the title, so a healthy staggered panel with n_negative_post = 0 is titled "(5 negative)". That contradicts the summary() fix from item 9 last round. Use the post-only count.

10. Signed balance plot draws only half the reference line. With absolute_value=False differences can be negative, but the "no improvement" diagonal runs from (0, 0) to (limit, limit) in both renderers. Draw it over [-limit, limit] for signed plots, and cover both backends in the tests.

11. Wrong-length weights= with an excluded cohort raises a raw IndexError. The excluded-cohort slice unit_weights[keep_units] runs before _cohort_masses checks the length. Move the length check ahead of the slice.

12. Assert the analytic ESS at degenerate cells. At a 0/0 cell the comparison weights are all 1, so cell ess == n_control exactly (100.0 on sim_staggered). Replace the isfinite assertion at the detected cells with that value; it is an anchor independent of both implementations.

13. Add a multi-covariate pin. Now that k > 1 works for the first time, the only coverage is the collinear self-consistency test. Add a frozen-numbers case (or better, a hand-checkable one) with two non-collinear covariates so the multi-column solve_ols branch is guarded.

14. Item 19 from last round was supplemented, not fixed. test_implied_att_is_the_weighted_sum still re-implements sum(w * att) and cannot fail; test_exported_from_package_root still checks three of five names. Delete or rewrite the first, extend the second.

15. Parity should assert the post-only negative fields. test_negative_weights_are_a_twfe_phenomenon asserts only the all-cells count. Add n_negative_post and negative_post_weight_share: zero for overall/simple, the observed value per fixture for twfe.

16. "Bit-for-bit" prose versus the actual tolerance. The R header, the comment at the mpdta check, and meta.mpdta_provenance say "bit-for-bit"/"bit-identical"; the assertion is identical() on the integer columns and rt_tol = 1e-14 on the float columns, as your reply correctly says. Make the prose say that.

17. llms-full subsection placement. ### plot_twfe_weights sits in the estimator catalog between the TWFE Weight Diagnostics entry and StaggeredTripleDifference; every other ### plot_* block lives under ## Visualization. Move it next to ### plot_bacon and keep the one-line mention in the estimator entry.

18. Dead lines in the plotly renderer. text=labels[mask] if annotate else None is overwritten immediately by fig.data[-1].text = labels[mask], and customdata=None does nothing.

Items 1-4 need no R re-run; nothing here changes the goldens. Rebase onto current main when you push.