[imerad]

Description

• Moves the ASI validation, power-tuning and mean/standard-error computation out of ASIMeanEstimator into a new internal ASIMeanEngine (glide/engines/asi.py), so a monitor can reuse it per batch. ASIMeanEstimator becomes a thin shell over the engine, with no change of behavior.
• The engine leaves the y_true NaNs in its preprocessed dataset and handles them downstream, and the effective sample size now comes from IPWClassicalMeanEngine directly instead of building a second estimator result.
• Unit tests for the moved logic now live in tests/unit/engines/test_asi.py; tests/unit/estimators/test_asi.py is reduced to estimate-level tests, including delegation to both engines.
• Closes Migrate ASIMeanEstimator onto the engine pattern with a new ASIMeanEngine #507

Checklist

Quality gates that must be satisfied before requesting a review:

• I have read CONTRIBUTING.md
• make lint passes
• make type-check passes
• make tests passes
• make coverage reports 100% coverage
• make test-notebooks passes
• New public API has numpy-style docstrings
• New public API is inserted in the API reference section of the documentation
• Docs build without warnings (make doc)
• CHANGELOG.md updated if the change is user-facing

LLM usage

Disclose if an LLM was used in writing this PR:

• No LLM used
• I used an LLM and I went through and validated all the code myself

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[imerad]

Code review

Six findings, none a functional bug. Ordered by relevance.

Tests

1. Weak delegation test (tests/unit/estimators/test_asi.py:28): test_estimate_delegates only checks y_true on the engine datasets and never y_proxy or sampling_probability. Its fixtures have no zero-probability rows, so the filtered-versus-raw handoff to the classical engine is untested. Passing the unfiltered sampling_probability to the IPW classical dataset would still pass, and the old estimator-level n_proxy / n_labeled checks after filtering were deleted.
2. Convention mismatch (tests/unit/estimators/test_asi.py:28): the test is named test_estimate_delegates, but CLAUDE.md asks for a single test_<function>_delegates_to_validation test. It wraps real methods with patch.object(..., wraps=...) instead of asserting exact outputs, and the mock names don't follow mock_<function>. tests/unit/estimators/test_ppi.py is the reference pattern.

Engine and estimator code

3. Repeated computation (glide/engines/asi.py:33): the labeled indicator, the inverse weights and the nan_to_num(y_true) fill are recomputed in _preprocess, _fit_tuning_parameter and _compute_mean_and_std, and the estimator recomputes the labeled count a fourth time. y_proxy * (labeled_indicator / sampling_probability - 1) appears in both _preprocess and _fit_tuning_parameter, so a change to the rectifier has to be made twice. The dataset tuple carries a NaN-laden y_true that every consumer must re-mask. Carrying the indicator or a filled y_true in the dataset would remove this, as the old xi return value did.
4. Validation bypass (glide/estimators/asi.py:117): the effective-sample-size baseline now calls the IPW classical engine's private _compute_mean_and_std directly, so IPW's own _preprocess checks no longer run for ASI. Before, IPWClassicalMeanEstimator().estimate(...) ran the full validation, including _validate_min_samples(y, 'y'). No input was found where this changes behaviour today, since ASI's own non-constant-rectifier check covers it, but the two validation paths are now silently decoupled: if the IPW engine gains a precondition, ASI will skip it.
5. Effective sample size rebuilt by hand (glide/estimators/asi.py:119): it is computed as classical_std**2 / std**2 from two engines' std values, re-squaring what CLTConfidenceInterval.var already stores. The old code compared .var attributes. This adds a second place that must stay in sync with the IPW estimator's std definition.
6. Missing docstring and protocol link (glide/engines/asi.py:18): ASIMeanEngine and its methods have no docstring, and nothing ties the class to the MeanEstimationEngine protocol in glide/engines/base.py, so signature drift would go unnoticed. The _fit_tuning_parameter early-return branch also puts a local assignment before return, against the CLAUDE.md return-statement rule. The IPW engine's _fit_tuning_parameter returns None from a signature that doesn't match the protocol, which is the same kind of drift.

[gmartinon-ed]

C'est plus propre comme ça !