fix(config,backend): harden controls and close auto-backend buyer surfaces - #951
Conversation
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>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis 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. ChangesRuntime contract and validation hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to 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)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Preserve current simulation resource caps while replaying the hostile integer-control boundary and focused evidence from #939 onto the protected-main tree.
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
|
Scheduled review-feedback autofix for this PR head.
|
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. |
There was a problem hiding this comment.
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
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore 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 head5d3db09d747e89018aec7c8f156b060c56f57323. -
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"]
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.
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/rustonly. NumPy parity is reached through the namedfast_mlsirm.fit_reference(...)Python API orfast-mlsirm fit --reference; public productionfast_mlsirm.fit(...)rejects NumPy as a production backend.Review-driven contract repair
tests/test_backend_reference_guidance_contract.pyto lock the actual Python/CLI reference entry points and preserve the protected-main Accepted ADR decision.fit --helpto direct CLI users to--reference, and updated the governed changelog to distinguish CLI--referencefrom Pythonfast_mlsirm.fit_reference.fast_mlsirm.fit_referencein the runtime error as not applicable:_fit_public.fitclears reference authority and rejectsFitConfig(backend="numpy"); the named reference API is the intentional NumPy parity path.c63744bc97a19beef61434e56c1f89f1075a4acbexposed two stale contract assertions intests/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.” Current033e10df4a5249c8a2314c0eacd9099a68db9e21replaces those string assertions with the actual accepted ADR semantics and--referenceCLI 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
033e10df4a5249c8a2314c0eacd9099a68db9e21.c26dac4bcdb7d600a2688c3e74eb4a68c0b45620.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).CHANGES_REQUESTEDreviewed predecessor head5d3db09d747e89018aec7c8f156b060c56f57323and failed on centralcoverage-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.