Skip to content

fix(scoring): harden policy integer callback boundary - #869

Merged
seonghobae merged 9 commits into
mainfrom
fix/scoring-policy-integer-callbacks-868
Aug 17, 2026
Merged

fix(scoring): harden policy integer callback boundary#869
seonghobae merged 9 commits into
mainfrom
fix/scoring-policy-integer-callbacks-868

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Scope

Fix #868 by requiring scoring-policy positive-integer controls to establish a package-trusted scalar identity before any caller-dispatchable coercion, without changing scoring or psychometric arithmetic.

Test-first lineage

  • RED ea9dc8d0d14753efe2bc0596bbc6e64355775f8d exercises EnginePolicy.minimum_raters_per_response and the shared scoring integer validators, requiring arbitrary __index__ providers and integer subclasses to fail with zero callback execution while preserving genuine NumPy integer compatibility.
  • GREEN c6feb3769c7d9eb5cd37a5d795d442235fda1b91 rejects policy integer callbacks before coercion.
  • 2d9fce97c1bdcac86350e19dfd207b79a90234cb applies the same exact-type boundary to the shared positive-integer validator.
  • 8089f3f2682f3beb2ccf46fafe865fdcdabfe054 records the boundary in canonical documentation; exact current head 4c7afb86237ebe33e5c5468aa6b43f24e368dbab preserves the five-file validation/test/release slice.

Preserved boundary

  • Exact built-in Python integers and exact supported NumPy integer scalar types remain accepted.
  • Booleans, caller-defined Python/NumPy integer subclasses, and arbitrary __index__ providers are rejected before conversion or representation callbacks.
  • Package-owned bounds and non-reflective error messages remain unchanged.
  • Scoring, calibration, likelihood, estimation, uncertainty and all other result-affecting arithmetic remain Rust-owned; Python performs validation/marshalling only.
  • No provider, dependency, workflow, database, package-version, authorization or downstream-ownership change is introduced.

Exact current state

  • Protected base: fb67ced09d8ee00542c05d56374537a9a7239751.
  • Exact source head: 4c7afb86237ebe33e5c5468aa6b43f24e368dbab.
  • GitHub reports the PR mergeable and Ready for review.
  • Exact-head CI, security, static-analysis, Python/Rust/package/fuzz, coverage/source-tree and OpenCode contexts are terminal with no failing or queued check.
  • No formal submitted review or unresolved review thread is present.
  • Squash auto-merge is enabled and remains protected by the live independent non-author approval, last-push, thread-resolution and required-check rules.

Integration still requires a qualifying independent non-author approval on this unchanged exact head. Automated checks do not replace that decision, and predecessor-head evidence does not transfer. No gate is weakened or bypassed.

Fixes #868.

@coderabbitai

coderabbitai Bot commented Aug 14, 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: b1c5a717-1ef3-410c-aabd-78c29f747982

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

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: dc706a9e-7ccc-4601-87dd-af7bae557426

📥 Commits

Reviewing files that changed from the base of the PR and between fb67ced and 4c7afb8.

📒 Files selected for processing (5)
  • docs/changelog.d/868-scoring-policy-integer-callback-safety.md
  • python/fast_mlsirm/scoring/_contract_safety.py
  • python/fast_mlsirm/scoring/_validation.py
  • tests/test_scoring_contract_public_callback_safety.py
  • tests/test_scoring_policy_integer_callback_safety.py

📝 Walkthrough

Walkthrough

Scoring-policy integer validation now rejects caller-defined coercion callbacks and integer subclasses before dispatch. Exact built-in integers and genuine NumPy integer scalars remain supported and normalize to Python int. Regression tests cover errors, callback counts, and compatibility.

Changes

Scoring integer safety

Layer / File(s) Summary
Trusted integer validation
python/fast_mlsirm/scoring/_contract_safety.py, python/fast_mlsirm/scoring/_validation.py
Both validators use exact built-in and approved NumPy integer types. Accepted NumPy values convert to Python int. Other values are rejected without invoking __index__.
Callback-safety regression coverage
tests/test_scoring_policy_integer_callback_safety.py, tests/test_scoring_contract_public_callback_safety.py, docs/changelog.d/868-scoring-policy-integer-callback-safety.md
Tests cover hostile callbacks, integer subclasses, trusted scalar compatibility, callback counts, and field-specific AssessmentSpecError behavior. The changelog records the change.

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

Merge Risk: ⚪ Minimal · up to 4c7af

The PR hardens integer validation at the scoring-policy boundary without changing scoring arithmetic; no actionable merge-blocking risk remains beyond normal checks and review.

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: hardening the scoring-policy integer callback boundary.
✨ 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 fix/scoring-policy-integer-callbacks-868

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.

@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 4c7afb86237ebe33e5c5468aa6b43f24e368dbab.

  • Head SHA: 4c7afb86237ebe33e5c5468aa6b43f24e368dbab

  • Workflow run: 31935795128

  • 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: 868-scoring-policy-integer-callback-safety.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: 868-scoring-policy-integer-callback-safety.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file (2 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (2 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (2 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (2 files)"]
  R3 --> V3["targeted test run"]
Loading

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 4c7afb86237ebe33e5c5468aa6b43f24e368dbab
  • Workflow run: 31935795128
  • 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 4c7afb86237ebe33e5c5468aa6b43f24e368dbab.

  • Head SHA: 4c7afb86237ebe33e5c5468aa6b43f24e368dbab

  • Workflow run: 31935795128

  • 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: 868-scoring-policy-integer-callback-safety.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: 868-scoring-policy-integer-callback-safety.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file (2 files)"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file (2 files)"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test (2 files)"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test (2 files)"]
  R3 --> V3["targeted test run"]
Loading

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

cursor Bot commented Aug 17, 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 dismissed opencode-agent[bot]’s stale review August 17, 2026 10:09

Superseded infrastructure-only review. This REQUEST_CHANGES is explicitly anchored to predecessor head 4c7afb8 and failed coverage-evidence run 31935795128. The live source head is f38bfe6; its exact-head hosted checks observed before the Ready transition are terminal-success with no failure, and the review contains no source-code finding. Dismissing only the obsolete predecessor-head blocker; this is not approval and fresh Ready-state required workflows remain authoritative.

@seonghobae
seonghobae enabled auto-merge (squash) August 17, 2026 10:09
@seonghobae
seonghobae merged commit 32705eb into main Aug 17, 2026
37 checks passed
@seonghobae
seonghobae deleted the fix/scoring-policy-integer-callbacks-868 branch August 17, 2026 12:36
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 scoring policy integer controls before caller callbacks

1 participant