Skip to content

fix(utility): replay numeric trust boundary on current review workflow - #1006

Merged
seonghobae merged 7 commits into
mainfrom
fix/utility-current-review-975
Aug 24, 2026
Merged

fix(utility): replay numeric trust boundary on current review workflow#1006
seonghobae merged 7 commits into
mainfrom
fix/utility-current-review-975

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Supersedes #975 without changing its corrected product tree.

Exact source head: 92e30348cc50727d77a0a892713b2e917764eada. Repository CI, Security Scan, CodeQL, and Semgrep are terminal-success on this SHA. The CodeRabbit float-subclass and every-public-scalar-position findings were addressed; the current inline thread is resolved.

The implementation keeps BCG/Naylor-Shine/Taylor-Russell arithmetic in Rust. Python only admits trusted concrete Python/NumPy numeric scalars, rejects caller conversion providers/subclasses and booleans, checks finiteness, marshals, and dispatches.

#975's remaining formal OpenCode CHANGES_REQUESTED is solely central coverage-evidence run 32124671613. This successor creates a fresh pull-request event for the current central workflow without source churn, force-push, review dismissal, or gate weakening.

Summary by CodeRabbit

  • Bug Fixes

    • Added stricter validation for utility controls, rejecting booleans, non-real values, non-finite numbers, and unsafe numeric objects.
    • Preserved support for valid Python and NumPy real-number inputs.
    • Invalid inputs are now rejected before processing begins.
  • Documentation

    • Clarified documentation for utility gain behavior.
  • Tests

    • Added regression coverage for invalid scalar inputs and supported NumPy values.

Open in Devin Review

@coderabbitai

coderabbitai Bot commented Aug 19, 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: b948fe18-600f-429f-be8d-ad73896eb55f

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: 750e0e7f-104c-4484-8c3f-2e242180a354

📥 Commits

Reviewing files that changed from the base of the PR and between 04d0bc2 and 92e3034.

📒 Files selected for processing (3)
  • docs/changelog.d/974-utility-control-boundary.md
  • python/fast_mlsirm/utility.py
  • tests/test_utility_control_boundary.py

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


📝 Walkthrough

Walkthrough

selection_utility and taylor_russell now validate finite real scalar controls before Rust dispatch. The validation rejects booleans, unsupported objects, non-finite values, and unsafe __float__ callbacks while preserving Python and NumPy real scalars.

Changes

Utility control boundary

Layer / File(s) Summary
Validation and pre-dispatch marshalling
python/fast_mlsirm/utility.py
Utility inputs now accept trusted built-in and NumPy scalar types, reject non-real or non-finite values, and pass validated floats to the Rust core.
Boundary regression coverage
tests/test_utility_control_boundary.py, docs/changelog.d/974-utility-control-boundary.md
Tests cover hostile conversion objects, booleans, non-finite values, and NumPy scalar compatibility. The changelog documents the validation behavior.

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

Merge Risk: ⚪ Minimal · up to 92e30

The change is localized to numeric input validation, its tests, and changelog documentation; no actionable merge-blocking risk remains after normal checks and review.

Possibly related issues

  • Issue 974: Directly covers the trust-boundary validation and regression tests implemented in this pull request.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
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 identifies the utility numeric trust-boundary fix and relates to the pull request's validation and review-workflow objectives.
✨ 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/utility-current-review-975

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.

Copy link
Copy Markdown
Contributor Author

@opencode-agent @cwl-noema-review Please independently review exact current head 92e30348cc50727d77a0a892713b2e917764eada against live main. Exact-head CI 32207432046, Security Scan 32207432044, CodeQL 32207432068, and SAST Semgrep 32207432093 are terminal-success and inline review threads are empty. Verify every public utility scalar position admits only trusted concrete Python/NumPy numeric scalars before conversion, rejects booleans/subclasses/protocol providers fail-closed, preserves finiteness checks, and keeps BCG/Naylor-Shine/Taylor-Russell arithmetic Rust-owned. Do not transfer #975 predecessor evidence.

@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 92e30348cc50727d77a0a892713b2e917764eada.

  • Head SHA: 92e30348cc50727d77a0a892713b2e917764eada

  • Workflow run: 32213325180

  • 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: 974-utility-control-boundary.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: 974-utility-control-boundary.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file: utility.py"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: utility.py"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test: test_utility_control_boundary.py"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test: test_utility_control_boundary.py"]
  R3 --> V3["targeted test run"]
Loading

@opencode-agent

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 92e30348cc50727d77a0a892713b2e917764eada
  • Workflow run: 32213325180
  • 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 92e30348cc50727d77a0a892713b2e917764eada.

  • Head SHA: 92e30348cc50727d77a0a892713b2e917764eada

  • Workflow run: 32213325180

  • 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: 974-utility-control-boundary.md"]
  S1 --> I1["operator or user guidance"]
  I1 --> R1["Review risk: Docs: 974-utility-control-boundary.md"]
  R1 --> V1["docs review"]
  Evidence --> S2["Changed file: utility.py"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: utility.py"]
  R2 --> V2["required checks"]
  Evidence --> S3["Test: test_utility_control_boundary.py"]
  S3 --> I3["regression suite"]
  I3 --> R3["Review risk: Test: test_utility_control_boundary.py"]
  R3 --> V3["targeted test run"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 19, 2026 04:58

Copy link
Copy Markdown
Contributor Author

@opencode-agent review
@cwl-noema-review review

Fresh re-review request: ContextualWisdomLab/.github#1136 has merged, fixing the central coverage-evidence bug (_install_trusted_uv() target-triple validation) that caused the prior REQUEST_CHANGES verdict on this PR. Please re-review exact current head 92e30348cc50727d77a0a892713b2e917764eada — the underlying infrastructure blocker is now resolved on .github main. Do not transfer evidence from any prior head.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

@opencode-agent Please re-review unchanged exact head 92e30348cc50727d77a0a892713b2e917764eada against live main@04d0bc21a2a20693bcf16108cd76d394fe844d23. Repository CI, Security Scan, CodeQL, and Semgrep are terminal-success on this exact head and current inline threads are empty. The remaining formal CHANGES_REQUESTED is central coverage run 32213325180, created before .github main advanced to 9e9f59f3ac1e96a960c021b131d768c238f4c21a (fix(ci): verify trusted uv target-triple output (#1136), 2026-08-19T09:24:32Z), which repairs the trusted-uv materialization boundary. Please regenerate same-head coverage/formal review evidence under the current central contract.

@seonghobae
seonghobae enabled auto-merge (squash) August 19, 2026 13:22
@opencode-agent
opencode-agent Bot disabled auto-merge August 19, 2026 13:59

Copy link
Copy Markdown
Contributor Author

@opencode-agent review
@cwl-noema-review review

Re-dispatch unchanged exact head 92e30348cc50727d77a0a892713b2e917764eada under central .github@bbedc1a51ec1a2421f129955c629b3cd0507a4ec. Repository CI, Security Scan, CodeQL, and Semgrep are terminal-success and inline threads are empty. The sole formal CHANGES_REQUESTED came from central coverage run 32213325180 before #1136 corrected trusted-uv verification; central main now also includes #1140's OIDC permission repair for scheduler callers. Generate fresh same-head coverage/formal review evidence without transferring the old infrastructure verdict.

@seonghobae seonghobae closed this Aug 19, 2026
@seonghobae seonghobae reopened this Aug 19, 2026
@seonghobae
seonghobae marked this pull request as draft August 19, 2026 18:32
@seonghobae
seonghobae marked this pull request as ready for review August 19, 2026 18:32
@seonghobae seonghobae closed this Aug 19, 2026
@seonghobae seonghobae reopened this Aug 19, 2026
@seonghobae

Copy link
Copy Markdown
Contributor Author

@opencode-agent review
@cwl-noema-review review

Re-review exact current head 92e30348cc50727d77a0a892713b2e917764eada against protected main base 04d0bc21a2a20693bcf16108cd76d394fe844d23; do not transfer stale coverage-era verdicts. Local trust-boundary regression validation passed 63 tests; the single native-results test requires the compiled Rust extension, which is supplied by CI/package jobs. The current patch keeps numeric marshalling and selection arithmetic in the existing Rust path.

@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 04:26
@opencode-agent
opencode-agent Bot disabled auto-merge August 20, 2026 04:47
@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 04:55
@opencode-agent
opencode-agent Bot disabled auto-merge August 20, 2026 06:24
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review for 92e3034: required checks are terminal-success on this exact head. Existing formal review state predates the current evidence or was tied to an earlier coverage-evidence attempt; no source failure is shown. Please reacquire approval against this exact head. @opencode-agent review @cwl-noema-review review

@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 07:27
@opencode-agent
opencode-agent Bot disabled auto-merge August 20, 2026 09:09
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head re-review request for 92e30348cc50727d77a0a892713b2e917764eada against protected main. The prior REQUEST_CHANGES verdict cited predecessor or superseded coverage evidence; re-fetch the exact current-head coverage-evidence result and review the complete current diff. Do not transfer predecessor-head findings or approvals. @opencode-agent review @cwl-noema-review review

@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 17:55
@opencode-agent
opencode-agent Bot disabled auto-merge August 20, 2026 19:53
@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 20:04

Copy link
Copy Markdown
Contributor Author

@opencode-agent @cwl-noema-review Re-review the exact current head 92e30348cc50727d77a0a892713b2e917764eada.

The prior current-head REQUEST_CHANGES only reported unavailable/failed coverage evidence from an earlier run. Current CI, Security Scan, SAST Semgrep, and CodeQL runs for this exact head are terminal-success. Re-evaluate this head and publish independent approvals only from fresh evidence; do not reuse prior-head reviews. Protected merge policy remains unchanged.

@opencode-agent
opencode-agent Bot disabled auto-merge August 20, 2026 22:12
@seonghobae
seonghobae enabled auto-merge (squash) August 20, 2026 22:44

Copy link
Copy Markdown
Contributor Author

@opencode-agent Please perform a review-only formal review of exact current PR head 92e30348cc50727d77a0a892713b2e917764eada. Re-check changed-file scope, current-head findings, unresolved threads, mergeability, and every required Check. Do not reuse a stale review, mutate the branch, self-approve, or merge.

@opencode-agent
opencode-agent Bot disabled auto-merge August 21, 2026 00:48
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head fix for a6657e2:

  • Fixed the remaining trust-boundary edge: exact built-in integers outside the float range now raise the package-owned ValueError instead of leaking OverflowError.
  • Added regression coverage for every utility and Taylor-Russell scalar position; 72 boundary tests passed, Ruff, compileall, focused docstring coverage (100%), and git diff --check passed.
  • The detached checkout cannot execute the pre-existing NumPy result-parity test because the compiled fast_mlsirm._core extension is unavailable locally; hosted Rust/package checks are the execution evidence for that native path.
  • Normal squash auto-merge remains enabled; fresh hosted checks and the independent formal review gate still control merge.

@seonghobae
seonghobae enabled auto-merge (squash) August 21, 2026 00:51

@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 1 potential issue.

Open in Devin Review

Comment thread python/fast_mlsirm/utility.py Outdated
@opencode-agent
opencode-agent Bot disabled auto-merge August 21, 2026 02:53
@seonghobae
seonghobae enabled auto-merge (squash) August 21, 2026 04:46
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review disposition

Fixed the valid Devin finding about the unmatched closing parenthesis in SelectionUtilityResult documentation and pushed bf85f57c fast-forward from a6657e2d.

Exact-head evidence:

  • PYTHONPATH=.:python python -m pytest -q tests/test_utility_control_boundary.py — 72 passed; 1 native-dependent test cannot run because this checkout has no compiled fast_mlsirm._core.
  • Ruff — passed.
  • interrogate python/fast_mlsirm/utility.py — 100%.
  • git diff --check — passed.

.Jules/palette.md remains unmodified and unstaged. Fresh exact-head review and required Checks remain required for a normal merge.

@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 1 new potential issue.

Open in Devin Review

Comment thread python/fast_mlsirm/utility.py
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head disposition

The exact head bf85f57c8b6eff13a07e1f74ba0535091c83a238 has no actionable source finding in the latest review. Devin 3829113064 is informational and confirms the exact scalar-type rejection is intentional: it preserves the hostile-subclass boundary and rejects booleans without invoking caller coercion. The remaining local native-extension import limitation is environmental, not a source failure; hosted Checks remain authoritative and are currently queued with no failures. Normal squash auto-merge is requested; no bypass or stale approval reuse.

@opencode-agent
opencode-agent Bot disabled auto-merge August 21, 2026 11:59
@opencode-agent opencode-agent Bot added area: ci-cd CI, GitHub Actions, checks, release, or supply chain 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 labels Aug 22, 2026
@coderabbitai

coderabbitai Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.


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 enabled auto-merge (squash) August 24, 2026 01:08
@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 02:19
@seonghobae
seonghobae merged commit 71c4230 into main Aug 24, 2026
38 checks passed
@seonghobae
seonghobae deleted the fix/utility-current-review-975 branch August 24, 2026 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci-cd CI, GitHub Actions, checks, release, or supply chain 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.

1 participant