fix(opencode-review): build PyO3/maturin extensions offline before coverage - #2225
Merged
Merged
Conversation
…verage Adds maturin==1.15.0 to the hash-pinned opencode-review CI lock (hashes verified against PyPI JSON metadata) and teaches run_python_test_coverage's new build_maturin_extension_if_needed helper to run `maturin build --offline --release` with CARGO_NET_OFFLINE=true and CARGO_BUILD_JOBS=1 against the vendored base-commit Cargo dependencies before pytest, for every tracked Python project whose pyproject.toml declares build-backend = "maturin". Closes the gap materialize_base_rust_dependencies.py (#2222, #2223) left open: the base Cargo dependency graph was vendored for `cargo llvm-cov`, but nothing ever built the PyO3 extension itself, so `coverage run -m pytest` kept failing collection with `ImportError: cannot import name '_core'` on 8 of the last 10 fast-mlsirm fallbacks. tests/test_maturin_offline_build_contract.py proves both halves against a real PyO3 fixture crate: the vendored-offline build produces an importable `_core` extension, and a dependency only a pull request's head added (never seen by the base-commit materializer) is never fetched -- the offline build fails closed on the missing crate. Refs ContextualWisdomLab/fast-mlsirm#1907 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpnJ9UuQwdTUfhzmqueXCz
|
Warning Review limit reachedNext included review available in 25 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
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 |
This was referenced Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
maturin==1.15.0to the hash-pinnedrequirements-opencode-review-ci.txt/requirements-opencode-review-ci-hashes.txtlock (hashes regenerated with./scripts/ci/compile_opencode_review_lock.sh, verified byte-for-byte against PyPI JSON metadata for the exact release)..github/workflows/opencode-review-dispatch.yml'srun_python_test_coveragenow calls a newbuild_maturin_extension_if_neededhelper for every tracked Python project whosepyproject.tomldeclaresbuild-backend = "maturin": it runsmaturin build --offline --releasewithCARGO_NET_OFFLINE=true CARGO_BUILD_JOBS=1against the Cargo dependenciesmaterialize_base_rust_dependencies.py(fix(opencode-review): coverage-failure fallback must publish a formal receipt, not a bare COMMENT #2222, fix(opencode-review): vendor Cargo deps offline for the coverage sandbox #2223) already vendors from the base commit, thenpip install --user --no-index --no-depsinstalls the built wheel before pytest runs -- entirely inside the existing--network=nonesandbox.cargo llvm-cov, but nothing ever built the PyO3 extension itself, sopython3 -m coverage run -m pytestkept failing collection withImportError: cannot import name '_core'on 8 of the last 10 fast-mlsirm fallbacks. Refs opencode-review가 dispatch 직후 fail-closed 되어 체크 완료된 PR 43건 중 29건이 병합 불가 fast-mlsirm#1907.--offline/CARGO_NET_OFFLINE=truekeeps the build inside the network-isolated sandbox exactly ascargo llvm-covalready runs; thepip install --require-hashes --only-binary=:all:install path is unchanged.Test plan
tests/test_maturin_offline_build_contract.pybuilds a real minimal PyO3 crate againstmaterialize_base_rust_dependencies.py's vendored output and asserts (a) the offline build produces an importablefixture_core._coreextension, and (b) a Cargo dependency only a simulated pull-request head added (never seen by the base-commit materializer) is never fetched -- the offline build fails closed on the missing crate instead of reaching the network.tests/test_opencode_workflow_shell_syntax.py(bash -nover every workflowrun:block) passes with the new helper functions.tests/test_opencode_rust_coverage_toolchain_contract.py::test_review_dispatch_blob_sha_stays_paired_with_trusted_workflowupdated and passing (workflow blob SHA repinned intests/test_pr_review_autofix_nvidia_nim_contract.py).coverage run -m pytest tests): 3134 passed, 3 skipped, 36 subtests passed on a clean run; the two new tests independently reverified passing undercoverage run -m pytest tests/test_maturin_offline_build_contract.pyafter fixing an environment-only interpreter mismatch (this dev machine has two pyenv Pythons;maturinneeded installing under both). Two later attempts to re-run the entire suite together withcoverage report/interrogatewere killed by the OS's low-memory protector -- confirmed viaps auxto be unrelated concurrent load on this shared machine (another workspace'srustcbuild, several other agent sessions, a Parallels VM), not this change. Noscripts/ci/production code changed in this PR, so its 100% coverage/docstring gates are structurally unaffected.Not merging this PR myself, per repo policy (single shared review identity cannot self-approve).
🤖 Generated with Claude Code
https://claude.ai/code/session_01FpnJ9UuQwdTUfhzmqueXCz