Skip to content

fix(crm): harden public numeric controls - #857

Merged
seonghobae merged 8 commits into
mainfrom
fix/crm-control-callbacks-856
Aug 17, 2026
Merged

fix(crm): harden public numeric controls#857
seonghobae merged 8 commits into
mainfrom
fix/crm-control-callbacks-856

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Scope

Fix #856 at the public Samejima continuous-response-model control-marshalling boundary without changing CRM psychometric arithmetic.

Protected main discovered the compiled core before validating public scalar controls. It also compared or normalized q_theta, max_iter and tol through caller-dispatchable numeric operations before establishing trusted scalar identities. Caller-defined Python/NumPy scalar subclasses could therefore execute comparison, conversion, ufunc or representation callbacks before the PyO3 boundary.

Test-first lineage

  • RED 129cc24cb9768a5cfc87bcf26cf511145fb0ace8 requires hostile Python integer, NumPy-integer and float subclasses to fail before caller callbacks and native-core discovery, while preserving genuine NumPy controls and all Rust-embedded quadrature orders.
  • RED fd51394091bdeb50745dc4757310610268f91ce4 specifies package-owned failure for a trusted but non-representable oversized tolerance.
  • GREEN d53a89384165b959fda6de76b12fee687c87bc12 admits only exact built-in or supported genuine NumPy scalar identities, validates controls before native discovery, preserves the Rust quadrature domain {7, 11, 15, 21, 31, 41}, retains the existing iteration limit and normalizes only admitted values.
  • c97f6c3af5b237f09d054426b902b4fa3b943c6f and exact current head b1a43d47864018a76a5705bf650eba77b67b815c add canonical changelog and APA-traced doctoring evidence.

Scientific and architecture boundary

  • Samejima CRM logit transform, Gauss-Hermite nodes/weights, observed-data likelihood, EM posterior bookkeeping, WLS item M-step, convergence rule, sign identification, derived discrimination/difficulty, EAP scoring, estimands and interpretation remain Rust-owned and unchanged.
  • Python remains validation/marshalling only; no fallback estimator or result-affecting formula is added.
  • Existing q_theta, max_iter and finite strictly-positive tolerance semantics are preserved.
  • No dependency, provider, workflow, database, package-version, downstream ownership or gate change is introduced.

Exact current state

  • Protected base: fb67ced09d8ee00542c05d56374537a9a7239751.
  • Exact source head: b1a43d47864018a76a5705bf650eba77b67b815c.
  • GitHub reports the PR mergeable and Ready for review.
  • The exact-head check snapshot is terminal with no failing, queued or in-progress check; no formal review or unresolved review thread is present.
  • Protected integration still requires every then-live required context and a qualifying independent non-author approval. Automated checks do not replace that decision.

No gate is weakened or bypassed. Any future source-head movement invalidates this evidence.

Fixes #856.

Summary by CodeRabbit

  • Bug Fixes

    • Added stricter validation for CRM fitting controls, including iteration limits, tolerances, and supported quadrature settings.
    • Invalid values, booleans, and unsupported scalar types are now rejected before processing or native execution.
    • Accepted numeric controls are normalized consistently for reliable execution.
  • Documentation

    • Added guidance on CRM control validation, safety behavior, and supported parameter constraints.
  • Tests

    • Added coverage for invalid controls, scalar normalization, callback safety, and supported quadrature options.

@coderabbitai

coderabbitai Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 83a0a065-baa4-4d07-84ca-e09c54bd69bd

📥 Commits

Reviewing files that changed from the base of the PR and between b1d45a4 and 0903072.

📒 Files selected for processing (4)
  • docs/changelog.d/856-crm-control-callback-safety.md
  • docs/doctoring/crm_control_callback_safety.md
  • python/fast_mlsirm/crm.py
  • tests/test_crm_control_callbacks.py

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

fit_crm now validates and normalizes CRM control parameters before response preprocessing and native-core discovery. Tests cover hostile subclasses, malformed controls, NumPy scalar normalization, and supported quadrature values. Documentation records the validation contract and regression evidence.

Changes

CRM control callback safety

Layer / File(s) Summary
Strict validation and native dispatch
python/fast_mlsirm/crm.py, tests/test_crm_control_callbacks.py
fit_crm accepts only supported exact scalar types, enforces control bounds, normalizes NumPy scalars, and dispatches trusted built-in values. Tests cover hostile callbacks, invalid controls, native dispatch, and supported quadrature orders.
Validation contract documentation
docs/doctoring/crm_control_callback_safety.md, docs/changelog.d/856-crm-control-callback-safety.md
Documentation describes pre-dispatch validation, callback rejection, preserved numerical contracts, regression evidence, and the changelog entry.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 09030

The change is limited to hardening public numeric-control validation without changing CRM calculations or runtime configuration, and no actionable merge-blocking risk remains after normal checks and review.

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the CRM numeric-control hardening implemented by the pull request.
Linked Issues check ✅ Passed The changes validate and normalize CRM controls before dispatch, preserve existing contracts, reject hostile subclasses, and add focused regression tests for issue #856.
Out of Scope Changes check ✅ Passed The code, tests, documentation, and changelog changes directly support the CRM control-marshalling hardening objectives in issue #856.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/crm-control-callbacks-856

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
seonghobae marked this pull request as ready for review August 14, 2026 19:03
@seonghobae
seonghobae enabled auto-merge (squash) August 14, 2026 19:08

@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 b1a43d47864018a76a5705bf650eba77b67b815c.

  • Head SHA: b1a43d47864018a76a5705bf650eba77b67b815c

  • Workflow run: 31924075594

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

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

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: b1a43d47864018a76a5705bf650eba77b67b815c
  • Workflow run: 31924075594
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

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 b1a43d47864018a76a5705bf650eba77b67b815c.

  • Head SHA: b1a43d47864018a76a5705bf650eba77b67b815c

  • Workflow run: 31924075594

  • Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

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

@opencode-agent
opencode-agent Bot disabled auto-merge August 16, 2026 04:06
@seonghobae
seonghobae marked this pull request as draft August 16, 2026 04:06
@seonghobae
seonghobae marked this pull request as ready for review August 16, 2026 22:22
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

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 16, 2026 22:22
@seonghobae
seonghobae merged commit 437b5c4 into main Aug 17, 2026
35 checks passed
@seonghobae
seonghobae deleted the fix/crm-control-callbacks-856 branch August 17, 2026 02:18
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.

Harden CRM numeric controls before native dispatch

1 participant