Skip to content

fix(parallel): validate controls before native discovery - #880

Closed
seonghobae wants to merge 4 commits into
mainfrom
fix/parallel-analysis-control-callbacks-879
Closed

fix(parallel): validate controls before native discovery#880
seonghobae wants to merge 4 commits into
mainfrom
fix/parallel-analysis-control-callbacks-879

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Scope

Fix #879 at the public parallel-analysis control-marshalling boundary without changing Horn/Glorfeld factor-retention mathematics.

Protected main accepted broad Python/NumPy integer subclasses through isinstance(...) and then normalized them with int(value). Caller-defined subclasses could therefore execute conversion code before trusted controls were established. The public wrapper also discovered the compiled core before validating n_iterations, centile, and seed.

Test-first lineage

  • RED 99fbf30ce03fac9472264901b37412fdb61f17ed adds public regressions for hostile Python and NumPy integer subclasses and proves malformed explicit controls must fail before _core_module() discovery.
  • The predecessor implementation reproduced both failures: hostile subclasses escaped through __int__, and malformed controls reached native-core discovery first.
  • GREEN ad9c7f54d05b0fd774c95a83d21ee1495df4e502 admits exact built-in Python integers and exact supported NumPy integer scalar types, validates every explicit control before native discovery, and preserves genuine NumPy scalar normalization.
  • 2323c629a4102680e87399276f573fc760bcc0df adds canonical changelog evidence.
  • Exact current head 9dd71c00e4ce2688ec007877d0d0f73386c8b184 adds APA-traced scientific/security doctoring while keeping the implementation scope unchanged.

Preserved contract

  • n_iterations remains positive and defaults to 30 * n_items when omitted.
  • centile remains bounded to 0..=99.
  • seed remains bounded to Rust/PyO3 u64.
  • The 128 MiB random-eigenvalue workspace ceiling remains unchanged.
  • Observed/random eigenvalue calculation, bias adjustment, retention scan, deterministic random stream, result shape, and all result-affecting arithmetic remain Rust-owned and unchanged.
  • Factor retention remains distinct from structural model selection under Implement factor-retention and structural measurement-model selection workflow #608.
  • No factor-count recommendation, Python numerical fallback, dependency, provider, workflow, database, or release-version change is introduced.

Verification

  • A focused fail-first harness reproduced the expected predecessor failures: caller __int__ execution and native discovery before validation.
  • The focused GREEN harness passes hostile Python/NumPy subclass rejection, invalid-control-before-discovery, and genuine np.int64 preservation.
  • The exact-head CI, Security Scan, and SAST Semgrep workflow runs completed successfully.
  • The fresh exact-head check-run inventory contains 34 checks with no failure, queued, or in-progress check; required Python/Rust/package/fuzz, coverage evidence, OpenCode, Strix, dependency/security, and provenance contexts are complete on this unchanged head.
  • No formal submitted review or unresolved inline review thread is present on the exact head.

Exact current state

  • Protected base: fb67ced09d8ee00542c05d56374537a9a7239751.
  • Exact source head: 9dd71c00e4ce2688ec007877d0d0f73386c8b184.
  • Direct descendant of protected main; net diff is four bounded files: production wrapper, focused regressions, changelog fragment, and doctoring note.
  • GitHub reports the PR mergeable and Ready for review.
  • Integration is blocked only by the then-live qualifying independent non-author approval requirement. Automated checks and author action do not substitute for that approval.

Any future source-head movement invalidates this evidence and requires a fresh exact-head sweep. No gate is weakened or bypassed.

Fixes #879. Advances #608.

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 63035c2d-2094-4278-a42e-05361016ade7

📥 Commits

Reviewing files that changed from the base of the PR and between fb67ced and 9dd71c0.

📒 Files selected for processing (4)
  • docs/changelog.d/879-parallel-analysis-control-callbacks.md
  • docs/doctoring/parallel_analysis_control_callback_safety.md
  • python/fast_mlsirm/parallel_analysis.py
  • tests/test_parallel_analysis_control_bounds.py

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 15, 2026 07:01
@seonghobae
seonghobae enabled auto-merge (squash) August 15, 2026 07:02
@seonghobae seonghobae closed this Aug 15, 2026
auto-merge was automatically disabled August 15, 2026 23:58

Pull request was closed

Copy link
Copy Markdown
Contributor Author

Closed as superseded by #923 after fresh head/diff comparison. The live #923 lane preserves #879’s exact-type integer control contract and Rust-owned Horn/Glorfeld arithmetic, while additionally validating the public data/control boundary before native discovery and carrying explicit arbitrary integer-protocol/callback regressions. Keep #923 as the sole active implementation lane for #879; no source from this branch was merged.

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.

security: validate parallel-analysis controls before native discovery

1 participant