Skip to content

fix(mmle): wrap stale marginal ABI TypeError as RuntimeError - #933

Merged
seonghobae merged 17 commits into
mainfrom
cursor/bc-fdce5e1e-163c-4424-9d96-e5213933ebe1-8551
Aug 17, 2026
Merged

fix(mmle): wrap stale marginal ABI TypeError as RuntimeError#933
seonghobae merged 17 commits into
mainfrom
cursor/bc-fdce5e1e-163c-4424-9d96-e5213933ebe1-8551

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Scope

Close the residual fail-closed hole on #823: a version-matched fit_marginal callable that rejects the current keyword contract must raise the package-owned ABI RuntimeError, never a leaked TypeError and never NumPy production arithmetic.

Test-first lineage

#823 already rejects missing, malformed, stale, and future MARGINAL_CAPABILITY_VERSION before dispatch. Review of head 59332c93 found that after capability == 1 and callable(fit_marginal), only ValueError was wrapped. A stale keyword signature still leaked TypeError.

Implemented GREEN

  • Catch TypeError from native fit_marginal and raise the existing ABI RuntimeError with causal chaining.
  • Add a public fit(..., estimator="mmle", model="MLS2PLM") regression that injects a version-1 callable raising keyword TypeError and asserts no NumPy fallback.
  • Make the missing-entrypoint ownership test inject the incomplete core so it does not depend on a compiled extension being importable.
  • Compare the compiled export assertion to fit._MARGINAL_CAPABILITY_VERSION.

Boundaries

No NumPy production fallback, no formula redesign, and no gate weakening. Prefer this branch over merging #823 as-is.

Next action

Review this successor, wait for exact-head required checks, then merge this PR and close #823 as superseded.

Open in Web View Automation 

seonghobae and others added 15 commits August 14, 2026 01:36
A version-1 fit_marginal callable that rejects the current keyword
contract now raises the package-owned ABI RuntimeError instead of
leaking TypeError, and ownership tests inject the incomplete core so
the fail-closed path does not depend on a compiled extension.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Document the fail-closed RuntimeError for a version-matched stale
fit_marginal keyword signature in Unreleased.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
State that rust/auto marginal MMLE requires a matching
MARGINAL_CAPABILITY_VERSION and never falls back to NumPy, with the
Jeon et al. (2021) APA 7 citation on the estimator docstring.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: #933 head 094616e7

This successor closes the residual on #823 59332c93. After MARGINAL_CAPABILITY_VERSION == 1 and a callable fit_marginal, a stale keyword signature is now raised as the package-owned ABI RuntimeError (with causal chaining) and does not leak TypeError or enter fit_marginal_numpy.

No merge-blocking source defect on this head.

What was checked

  • python/fast_mlsirm/fit.py fail-closed import, integer capability gate, callable gate, and except TypeError wrap around the native call.
  • Rust export of MARGINAL_CAPABILITY_VERSION = 1 next to the existing fit_marginal keyword contract (pop_kind, anchors, covariates, zero-inflation).
  • Ownership tests inject the incomplete / stale _core via fast_mlsirm._core (honored by from . import _core). Local PYTHONPATH=python pytest on the three new files: 9 passed, 1 skipped (test_compiled_rust_core_exports_current_marginal_capability without a compiled extension; CI builds the extension).
  • backend="auto" is still fail-closed at resolve_backend when _core is missing; the new auto test covers the production path once it has already resolved to rust.

Not blocking (do not open a competing MMLE PR)

  • The TypeError handler is broader than unexpected-keyword failures. That is acceptable fail-closed behavior; the original exception remains on __cause__.
  • A version-1 callable that accepts the keywords but returns an incomplete dict can still leak KeyError after the call. Out of scope for this wrap.
  • Pre-existing: Rust fit_marginal accepts singlefree while the invalid-pop_kind error text omits it. Not introduced here.

Next action

  1. Mark this draft ready for review once required checks are green on this exact SHA (094616e7454fbec8241ada1405f06306f5dfd4ad).
  2. Get a non-author approval. This automation will not approve or merge its own lineage.
  3. Merge #933, then close #823 as superseded. Do not merge #823.
  4. Do not open another marginal-MMLE ABI PR while this is the landing vehicle.
Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

@seonghobae
seonghobae marked this pull request as ready for review August 17, 2026 04:20
@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@seonghobae
seonghobae enabled auto-merge (squash) August 17, 2026 04:20
@seonghobae
seonghobae merged commit 8dcbd24 into main Aug 17, 2026
35 checks passed
@seonghobae
seonghobae deleted the cursor/bc-fdce5e1e-163c-4424-9d96-e5213933ebe1-8551 branch August 17, 2026 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants