Skip to content

fix(opencode-review): build PyO3/maturin extensions offline before coverage - #2225

Merged
seonghobae merged 1 commit into
mainfrom
seonghobae/fix-1907-maturin-lock
Sep 16, 2026
Merged

seonghobae merged 1 commit into
mainfrom
seonghobae/fix-1907-maturin-lock

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds maturin==1.15.0 to the hash-pinned requirements-opencode-review-ci.txt / requirements-opencode-review-ci-hashes.txt lock (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's run_python_test_coverage now calls a new build_maturin_extension_if_needed helper for every tracked Python project whose pyproject.toml declares build-backend = "maturin": it runs maturin build --offline --release with CARGO_NET_OFFLINE=true CARGO_BUILD_JOBS=1 against the Cargo dependencies materialize_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, then pip install --user --no-index --no-deps installs the built wheel before pytest runs -- entirely inside the existing --network=none sandbox.
  • Closes the gap those two PRs left open: the base Cargo dependency graph was vendored for cargo llvm-cov, but nothing ever built the PyO3 extension itself, so python3 -m coverage run -m pytest kept failing collection with ImportError: 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.
  • No gate is weakened: --offline/CARGO_NET_OFFLINE=true keeps the build inside the network-isolated sandbox exactly as cargo llvm-cov already runs; the pip install --require-hashes --only-binary=:all: install path is unchanged.

Test plan

  • New tests/test_maturin_offline_build_contract.py builds a real minimal PyO3 crate against materialize_base_rust_dependencies.py's vendored output and asserts (a) the offline build produces an importable fixture_core._core extension, 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 -n over every workflow run: block) passes with the new helper functions.
  • tests/test_opencode_rust_coverage_toolchain_contract.py::test_review_dispatch_blob_sha_stays_paired_with_trusted_workflow updated and passing (workflow blob SHA repinned in tests/test_pr_review_autofix_nvidia_nim_contract.py).
  • Full local suite (coverage run -m pytest tests): 3134 passed, 3 skipped, 36 subtests passed on a clean run; the two new tests independently reverified passing under coverage run -m pytest tests/test_maturin_offline_build_contract.py after fixing an environment-only interpreter mismatch (this dev machine has two pyenv Pythons; maturin needed installing under both). Two later attempts to re-run the entire suite together with coverage report/interrogate were killed by the OS's low-memory protector -- confirmed via ps aux to be unrelated concurrent load on this shared machine (another workspace's rustc build, several other agent sessions, a Parallels VM), not this change. No scripts/ci/ production code changed in this PR, so its 100% coverage/docstring gates are structurally unaffected.
  • Live sandbox run against a real maturin/PyO3 PR (e.g. a fast-mlsirm PR) is still open -- this PR's local fixture proves the mechanism but not the exact Docker sandbox path end to end.

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

…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
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 25 minutes.

Check out review usage here.

View limit details

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

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: d2379663-5fb9-4d6a-a96d-af90a47b75fa

📥 Commits

Reviewing files that changed from the base of the PR and between 346b46d and 11a5630.

📒 Files selected for processing (6)
  • .github/workflows/opencode-review-dispatch.yml
  • CHANGELOG.d/20260917-maturin-offline-coverage-build.md
  • requirements-opencode-review-ci-hashes.txt
  • requirements-opencode-review-ci.txt
  • tests/test_maturin_offline_build_contract.py
  • tests/test_pr_review_autofix_nvidia_nim_contract.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 merged commit 89b225d into main Sep 16, 2026
8 of 20 checks passed
@seonghobae
seonghobae deleted the seonghobae/fix-1907-maturin-lock branch September 16, 2026 18:29
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.

1 participant