Skip to content

test(fitstats): fail closed without Rust after tail merge - #720

Closed
seonghobae wants to merge 1 commit into
mainfrom
test/fitstats-rust-required-red-627
Closed

test(fitstats): fail closed without Rust after tail merge#720
seonghobae wants to merge 1 commit into
mainfrom
test/fitstats-rust-required-red-627

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Post-integration ownership correction for #627

Protected main at branch creation and final pre-PR refetch: 640de7dd39d944749b92b1a88bafa8e1787a0847.

PR #646 merged Rust implementations for chi-square survival probability and Benjamini–Hochberg decisions, but fresh protected-main inspection shows both public Python functions still silently execute their complete historical Python/NumPy numerical implementations whenever _core_module() returns None or lacks the new entrypoint. That leaves two ordinary production numerical owners and does not satisfy #627's Rust-first fail-closed contract.

This Draft adds only the smallest fail-first production-boundary regressions:

  • chi2_sf() must raise a stable package-owned capability error when the compiled core is absent;
  • benjamini_hochberg() must do the same; and
  • an older/incompatible core missing these entrypoints must fail with the same bounded capability error rather than reactivate Python numerical fallback.

Current protected main is expected to fail all three tests by returning Python-computed results. Setup/import/fixture failure is not valid RED.

Minimum GREEN after exact RED proof

  1. Remove ordinary Python production fallback dispatch for these two migrated public functions. Require compatible compiled Rust entrypoints and fail closed with one stable non-reflective RuntimeError family when absent.
  2. If the historical gamma/BH Python helpers remain for reference/parity tests, mark them explicitly non-production and make them unreachable from implicit public dispatch.
  3. Preserve exact accepted Rust numerical semantics, NaN handling, p-value shape, q/df behavior, ordering/ties and caller immutability; no scientific threshold redesign in this slice.
  4. Add direct public→Rust delegation/parity plus missing/incompatible-core coverage, update doctoring/changelog wording so Python fallback is not advertised as production support, and run all same-head Python/Rust/PyO3/package/GPU/fuzz/Security/SAST/current-head review gates.
  5. Continue architecture: eliminate Python production fit-statistics and fallback numerics #627 afterward for the remaining S-X²/person-fit/quadrature support arithmetic; do not falsely close the umbrella issue just because these two kernels become single-owner.

Do not add SciPy, change fit-statistical formulas, dependencies/workflows/version/release, or canonical #604 docs. Keep Draft through RED→GREEN. Advances #627.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 200683fa-a61b-47eb-bcf5-f1bba33fb7cd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Superseded by GREEN implementation #723 (fail-closed chi2_sf/BH without pure-Python fallback).

@seonghobae seonghobae closed this Aug 11, 2026
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.

1 participant