Skip to content

fix(config,backend): harden controls and close auto-backend buyer surfaces - #951

Merged
seonghobae merged 44 commits into
mainfrom
cursor/bc-9ea40d9c-829d-4fd3-8383-f346575fc77f-51c7
Aug 25, 2026
Merged

fix(config,backend): harden controls and close auto-backend buyer surfaces#951
seonghobae merged 44 commits into
mainfrom
cursor/bc-9ea40d9c-829d-4fd3-8383-f346575fc77f-51c7

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Closes #872.
Advances #621 and #626.

Single landing unit

This branch combines configuration integer-safety with Rust-required production-backend authority. It admits exact supported integer scalars without caller callbacks, normalizes trusted controls before arithmetic, bounds simulation/fit resources, rejects backend/device string subclasses before callbacks or native discovery, and normalizes an installed-but-unloadable Rust core to a stable package-owned error.

The production backend contract is auto/rust only. NumPy parity is reached through the named fast_mlsirm.fit_reference(...) Python API or fast-mlsirm fit --reference; public production fast_mlsirm.fit(...) rejects NumPy as a production backend.

Review-driven contract repair

  • Added tests/test_backend_reference_guidance_contract.py to lock the actual Python/CLI reference entry points and preserve the protected-main Accepted ADR decision.
  • Restored ADR-0002 byte-for-byte to the protected-main decision instead of silently rewriting an Accepted ADR or creating an unnecessary successor decision.
  • Corrected fit --help to direct CLI users to --reference, and updated the governed changelog to distinguish CLI --reference from Python fast_mlsirm.fit_reference.
  • Revalidated the CodeRabbit request to replace fast_mlsirm.fit_reference in the runtime error as not applicable: _fit_public.fit clears reference authority and rejects FitConfig(backend="numpy"); the named reference API is the intentional NumPy parity path.
  • Exact-head CI on c63744bc97a19beef61434e56c1f89f1075a4acb exposed two stale contract assertions in tests/test_claude_runtime_contract.py: one required wording that is not present in protected-main ADR-0002, and one required the obsolete CLI instruction to “pass numpy.” Current 033e10df4a5249c8a2314c0eacd9099a68db9e21 replaces those string assertions with the actual accepted ADR semantics and --reference CLI contract. The failed run otherwise completed 5,949 tests successfully with Rust/package/GPU/fuzz jobs green.

Preserved ownership

This remains Python validation, marshalling, backend orchestration, stable errors, buyer-facing documentation, and release evidence only. No likelihood, estimator, quadrature, optimizer, fit statistic, or other psychometric arithmetic moves out of Rust.

Current-head verification boundary

  • Exact PR head: 033e10df4a5249c8a2314c0eacd9099a68db9e21.
  • Current protected main: c26dac4bcdb7d600a2688c3e74eb4a68c0b45620.
  • Ancestry: 43 commits ahead / 1 behind; merge base 7400e9780942f31188d77ad21e17594bf4dba5b2. The one protected-main commit behind is the unrelated polytomous GRM/GPCM/CAT/FIPC recovery suite (test: add polytomous GRM/GPCM/CAT/FIPC parameter-recovery suite #1313).
  • New exact-head CI, CodeQL, Security Scan, SAST Semgrep, and ClusterFuzzLite runs are regenerating; predecessor-head results are historical only.
  • The latest formal OpenCode CHANGES_REQUESTED reviewed predecessor head 5d3db09d747e89018aec7c8f156b060c56f57323 and failed on central coverage-evidence; it remains formal review history. Fresh current-head central coverage/review evidence and independent approval are still required before merge.

Hosted checks and current-head review are authoritative; predecessor-head approvals/checks are not reused. No force push, gate weakening, or self-approval is intended.

Supersedes #939, #946, and #831. Do not merge those predecessor lanes separately.

seonghobae and others added 19 commits August 14, 2026 10:13
Strix VULN-0001 failed the current head because MLS2PLMConfig and
FitConfig could be constructed with values that bypass memory-safety
bounds until a later explicit validate() call. Run the existing
validators from __post_init__ so invalid objects cannot exist, and keep
validate() public and idempotent.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Write admitted NumPy integers back as built-in ints so n_items and
seed + restart cannot wrap, and reject untrusted seed/verbose values
before construction returns.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Keep the Claude runtime-contract TOML, and stop README, commercial
readiness, PRD, TRD, and ADR-0002 from advertising a silent NumPy
fallback for backend=auto. Purchasers should install the compiled
Rust core or pass explicit backend=numpy for parity testing.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Close the buyer-facing leftovers that #945 missed: README CLI examples,
fit --help, FitConfig comments, commercial Operational Notes, and the
auto fail-closed error now name the next action. Contract tests bind to
resolve_backend and scan those surfaces.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
The auto-unavailable message uses "reference/parity" as policy language,
not a filesystem path. fit --help wraps the next-action clause, so the
lock now compares whitespace-normalized help text.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Buyer demo copy, README layout, sales --check-import help, and
release-acceptance still treated Rust as optional or accepted NumPy
as an automatic fit outcome. Fail those surfaces closed and lock them.

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

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

Auto incremental reviews are disabled on this repository.

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: 3733d06d-10ba-4877-ad66-2258955dcd31

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
📝 Walkthrough

Walkthrough

This PR makes Rust the required automatic backend, limits NumPy to explicit reference/parity use, hardens integer validation in config and diagnostics, moves config validation to construction time, and updates acceptance, fuzz, and documentation checks to match.

Changes

Runtime contract and validation hardening

Layer / File(s) Summary
Runtime contract and buyer-facing policy
CLAUDE.md, README.md, docs/PRD.md, docs/TRD.md, docs/adr/*, docs/buyer_demo_storyboard.md, docs/commercial_readiness.md, docs/doctoring/automatic_backend_rust_authority.md, docs/prd_trd_summary.md, python/fast_mlsirm/cli.py, python/fast_mlsirm/estimators/marginal.py, scripts/sales_readiness.py, docs/changelog.d/833-runtime-contract-buyer-docs.md, tests/test_claude_runtime_contract.py
The documented backend contract now says auto resolves to Rust or fails closed, while backend="numpy" remains explicit reference/parity behavior. Runtime-contract tests enforce the same wording across guidance, docs, CLI help, and sales or release surfaces.
Fail-closed core loading and acceptance checks
python/fast_mlsirm/backend.py, scripts/release_acceptance.py, docs/changelog.d/831-rust-core-loader-fail-closed.md, tests/test_backend.py, tests/test_backend_import_failure_contract.py, tests/test_release_acceptance_*
Backend loading now exposes a stable unavailable message and converts discovered-core import failures into package-owned RuntimeError values with preserved causes. Release acceptance checks and tests require auto fits to resolve to Rust consistently.
Trusted integer normalization
python/fast_mlsirm/config.py, python/fast_mlsirm/diagnostics.py, docs/doctoring/config_integer_callback_safety.md, docs/changelog.d/872-config-integer-callback-safety.md, tests/test_config_integer_callback_safety.py
Validation now accepts exact built-in int values and supported NumPy integer scalars, rejects booleans, int subclasses, and arbitrary __index__ providers, and stores normalized built-in integers before later arithmetic. Diagnostics use the same trusted-integer path for parameter counts, quadrature sizes, fold counts, seeds, and latent-dimension candidates.
Construction-time validation rollout
python/fast_mlsirm/config.py, fuzz/README.md, fuzz/atheris/fuzz_config.py, tests/test_config.py, tests/test_fuzz_properties.py, tests/test_model_estimator_compatibility.py, tests/test_objective.py, tests/test_simulation_resource_limits.py
MLS2PLMConfig and FitConfig now validate in __post_init__. Fuzz harnesses and regression tests were updated so invalid inputs are expected to fail during construction instead of only through later validate() calls.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: 🟡 Moderate · up to f8fac

The PR currently directs some users to an obsolete reference API and an invalid CLI option, which can make documented recovery and reference workflows fail, and it changes the accepted backend policy without the required successor decision record. These bounded documentation and runtime-contract issues should be fixed before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes substantial backend authority, native import, buyer documentation, release tooling, and sales-surface changes beyond #872. Split unrelated backend and buyer-facing changes into separate pull requests, or link issues that explicitly cover those objectives.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #872 by blocking unsafe integer callbacks, preserving trusted NumPy integers, normalizing values, and adding focused regression tests.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the configuration hardening and automatic-backend authority changes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/bc-9ea40d9c-829d-4fd3-8383-f346575fc77f-51c7

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.

Preserve current simulation resource caps while replaying the hostile integer-control boundary and focused evidence from #939 onto the protected-main tree.
@opencode-agent

Copy link
Copy Markdown
Contributor

Scheduled review-feedback autofix for this PR head.

  • Head SHA: 286bd2dbba5da8348643b6ad8145967972813ae9

@opencode-agent

Copy link
Copy Markdown
Contributor

Scheduled review-feedback autofix for this PR head.

  • Head SHA: 286bd2dbba5da8348643b6ad8145967972813ae9

@opencode-agent

Copy link
Copy Markdown
Contributor

Scheduled review-feedback autofix for this PR head.

  • Head SHA: 286bd2dbba5da8348643b6ad8145967972813ae9

@opencode-agent

Copy link
Copy Markdown
Contributor

Scheduled review-feedback autofix for this PR head.

  • Head SHA: 286bd2dbba5da8348643b6ad8145967972813ae9

@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 14:26
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head merge-result verification

The PR remains on normal auto-merge. No stale review or predecessor-head evidence is being reused; hosted checks and current-head review are authoritative.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 5 new potential issues.

Open in Devin Review

Comment thread python/fast_mlsirm/config.py
Comment thread python/fast_mlsirm/config.py
Comment thread python/fast_mlsirm/diagnostics.py
Comment thread python/fast_mlsirm/config.py
Comment thread python/fast_mlsirm/backend.py
coderabbitai[bot]

This comment was marked as resolved.

@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 14:55

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

  • Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

  • Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

  • Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

  • Result: REQUEST_CHANGES

  • Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 5d3db09d747e89018aec7c8f156b060c56f57323.

  • Head SHA: 5d3db09d747e89018aec7c8f156b060c56f57323

  • Workflow run: 32770339449

  • Workflow attempt: 1

Coverage evidence

Coverage Decision

  • Result: FAIL
  • Test evidence: not proven passing
  • Docstring evidence: not proven passing when configured
  • Failure count: 2

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Changed file (11 files)"]
  S1 --> I1["repository behavior"]
  I1 --> R1["Review risk: Changed file (11 files)"]
  R1 --> V1["required checks"]
  Evidence --> S2["Docs (13 files)"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs (13 files)"]
  R2 --> V2["docs review"]
  Evidence --> S3["Test (11 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (11 files)"]
  R3 --> V3["targeted test run"]
Loading

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae closed this Aug 25, 2026
@seonghobae seonghobae reopened this Aug 25, 2026
@seonghobae seonghobae closed this Aug 25, 2026
@seonghobae seonghobae reopened this Aug 25, 2026
@seonghobae
seonghobae merged commit 9c12eab into main Aug 25, 2026
89 of 102 checks passed
@seonghobae
seonghobae deleted the cursor/bc-9ea40d9c-829d-4fd3-8383-f346575fc77f-51c7 branch August 25, 2026 02:49
seonghobae added a commit that referenced this pull request Aug 25, 2026
Pin the baseline to protected main@9c12eab1 (v0.9.0). The open-PR queue
collapsed from 74 to 6 since the previous snapshot; replace the stale PR
table with the current six lanes (#1363, #1345, #1279, #1029, #1005, #998)
including their merge-forward state. Update gap-matrix evidence columns for
landed work: automatic Rust backend (#951/#1070), crossed multiple-membership
(#1014), the Figma design-boundary ADR now at docs/adr/0016 via merged
#1130, and cross-engine conformance (#1082) after #1077 closed as completed.
No production code, formula, estimator, or release surface changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: security Security boundary, hardening, or vulnerability prevention priority: medium Normal-priority or P2 work status: needs-review Open pull request requiring current-head review or checks type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden FitConfig and simulation integer validation against caller callbacks

3 participants