Skip to content

fix(review): coverage evidence tooling for numpy and Rust repositories#611

Open
seonghobae wants to merge 6 commits into
mainfrom
claude/fast-mlsirm-pr-review-mt2e1z
Open

fix(review): coverage evidence tooling for numpy and Rust repositories#611
seonghobae wants to merge 6 commits into
mainfrom
claude/fast-mlsirm-pr-review-mt2e1z

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Problem

Since 2026-07-14 the offline coverage sandbox has structurally been unable to produce passing evidence for any repository that needs third-party Python packages or a modern Rust toolchain. Every such PR (all 47 open fast-mlsirm PRs, including the #160 model-design PR) is stuck at REQUEST_CHANGES with "coverage-evidence result was failure":

  • Python: the sandbox runs the repository's pytest suite with only the trusted toolchain installed (coverage, interrogate, pytest, pytest-cov, uv). Target dependencies are never installed (scripts/ci/install_python_requirements_for_coverage.py exists but is not wired into any workflow), so every test module fails collection with ModuleNotFoundError: No module named 'numpy' — see run 29875093850 for fast-mlsirm#160.
  • Rust: cargo llvm-cov aborts with failed to find llvm-tools-preview. The image only has the Ubuntu-noble rustc/cargo 1.75, which additionally cannot parse version-4 Cargo.lock files (fast-mlsirm, wardnet, …), and the --network=none sandbox has no way to download crates even if it could build.

Fix (fail-closed offline design unchanged)

  1. numpy in the trusted toolchainnumpy==2.5.1 added to requirements-opencode-review-ci.txt; the hashes file regenerated with uv pip compile --generate-hashes --no-header --no-annotate --python-version 3.12 --python-platform x86_64-manylinux_2_28. Empirically verified against fast-mlsirm main: with numpy alone the suite passes (156 passed, 72 skipped — the Rust-parity tests importorskip cleanly when fast_mlsirm._core is absent).
  2. Pinned rustup toolchain in the image — sha256-pinned rustup-init 1.28.2 provisions Rust 1.94.1 with llvm-tools-preview under /usr/local/rustup, symlinked into /usr/local/bin so it shadows the distribution rustc. No mutable sh.rustup.rs installer is used (the existing contract test forbidding it still passes).
  3. Offline crate cache — the online coverage-source-tree job now runs cargo fetch --locked into the tree-local CARGO_HOME (.opencode-sandbox-home/.cargo) that the sandbox already sets, and the sandbox exports CARGO_NET_OFFLINE=true so cargo resolves strictly from that cache. cargo fetch downloads content-addressed crates and executes no build scripts; a failed prefetch keeps today's behavior (the offline build surfaces the missing dependencies).

Verification

  • coverage run -m pytest tests634 passed, coverage report100% over scripts/ci
  • interrogate100.0%
  • Workflow YAML parses; contract assertions about the sandbox (docker build … --network=default before docker run … --network=none, https://sh.rustup.rs absent, pinned cargo-llvm-cov retained, pytest-cov==7.1.0 hash retained) all hold.

Developer experience: unblocks the fast-mlsirm review/merge queue (and every other numpy/Rust repository) without weakening any gate — evidence is still produced offline, from pinned tooling, under the same sandbox identity.

User experience: no user-facing behavior change; review verdicts for scientific-Python/Rust repositories become meaningful instead of unconditionally failing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f


Generated by Claude Code

pytest ran with no target dependencies installed and cargo llvm-cov had
no llvm-tools or crate cache, so coverage evidence always failed for
scientific-Python and Rust repositories (every fast-mlsirm PR has been
stuck at REQUEST_CHANGES since 2026-07-14).

Fixes, keeping the fail-closed offline sandbox design intact:

1. Add numpy==2.5.1 to the trusted hash-pinned coverage toolchain
   (hashes regenerated with uv pip compile --generate-hashes
   --no-header --no-annotate --python-version 3.12
   --python-platform x86_64-manylinux_2_28). Verified against
   fast-mlsirm main: suite passes with numpy alone (156 passed,
   72 skipped; Rust-parity tests skip cleanly).
2. Provision a pinned rustup toolchain (1.94.1 + llvm-tools-preview)
   via the sha256-pinned rustup-init 1.28.2 binary, symlinked into
   /usr/local/bin so it shadows the distribution rustc (1.75), which
   cannot parse version-4 Cargo.lock files.
3. Pre-fetch the locked crate graph in the online coverage-source-tree
   job into the tree-local CARGO_HOME the sandbox already uses
   (cargo fetch --locked executes no build scripts), and export
   CARGO_NET_OFFLINE=true in the sandbox so cargo resolves strictly
   from that cache.

Local gates: 634 contract tests pass, scripts/ci coverage 100%,
interrogate 100%, workflow YAML parses.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f
Copilot AI review requested due to automatic review settings July 21, 2026 23:16
This was referenced Jul 21, 2026
pip-audit fails on main: pyasn1 0.6.3 in requirements-strix-ci-hashes.txt carries CVE-2026-59886 (univ.Real big-exponent DoS), fixed in 0.6.4. Add the floor to requirements-strix-ci.txt and regenerate the lockfile with the exact header command; only pyasn1 changes.

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates CI dependency sets and the opencode-review workflow to improve security posture and enable offline Rust coverage runs.

Changes:

  • Bump pyasn1 to address a referenced CVE in the Strix CI requirements (and update its pinned hash entry).
  • Add numpy to the opencode-review CI Python requirements (and regenerate the hashed lock file).
  • Enhance the opencode-review workflow to prefetch Rust crates online and run Rust coverage fully offline, including installing a pinned Rust toolchain with llvm-tools-preview.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
requirements-strix-ci.txt Adds a pyasn1 constraint and CVE note for Strix CI.
requirements-strix-ci-hashes.txt Updates pinned pyasn1 version+hashes to match the intended security bump.
requirements-opencode-review-ci.txt Adds numpy to opencode-review CI dependencies.
requirements-opencode-review-ci-hashes.txt Regenerates hashes/locks to include numpy and updated resolved artifacts.
.github/workflows/opencode-review.yml Adds cargo prefetch + offline flags and installs a pinned rustup toolchain for Rust coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/opencode-review.yml Outdated
Comment thread .github/workflows/opencode-review.yml
Copilot AI review requested due to automatic review settings July 21, 2026 23:21
Address review feedback: export RUSTUP_TOOLCHAIN so a repository rust-toolchain.toml cannot make the offline sandbox attempt a toolchain download, and emit an explicit warning when the crate prefetch is skipped because cargo is unavailable on the runner.

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Comment thread requirements-strix-ci.txt Outdated
Comment thread .github/workflows/opencode-review.yml Outdated
Comment thread .github/workflows/opencode-review.yml Outdated
Comment thread .github/workflows/opencode-review.yml
Comment thread .github/workflows/opencode-review.yml
Copilot AI review requested due to automatic review settings July 21, 2026 23:25
…drop global image CARGO_HOME

Address second review round: requirements-strix-ci.txt now pins pyasn1==0.6.4 to match the lock; the crate prefetch prefers rustup run 1.94.1 so lockfile parsing and cache layout match the sandbox toolchain, falling back to the runner cargo; the image no longer bakes a global CARGO_HOME so the tree-local cache set per measured command is the only cargo home the sandbox sees.

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 21, 2026 23:29

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comment thread .github/workflows/opencode-review.yml Outdated
Address review feedback: the crate prefetch now walks every tracked Cargo.toml with a sibling Cargo.lock (root and nested workspaces alike, matching the sandbox's nested-manifest coverage discovery) instead of only the repository root.
Copilot AI review requested due to automatic review settings July 21, 2026 23:34

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

.github/workflows/opencode-review.yml:306

  • git ls-files 'Cargo.toml' '*/Cargo.toml' only matches Cargo.toml at repo root or one directory deep, so nested workspaces like crates/foo/Cargo.toml (and any deeper) won’t have their locked crate graphs prefetched. That can leave the offline sandbox without a complete crate cache even though the comment says nested manifests are covered.
          prefetch_manifests="$(git -C "$COVERAGE_SOURCE_WORKDIR" ls-files 'Cargo.toml' '*/Cargo.toml')"

seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…or re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change in the changelog and produces a fresh head so the
scheduler dispatches a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…resh head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…esh head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…efresh head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…ad for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…s; refresh head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…fresh head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
seonghobae added a commit to ContextualWisdomLab/fast-mlsirm that referenced this pull request Jul 22, 2026
…es; refresh head for re-review

The previous CHANGES_REQUESTED verdict on this head was an infrastructure
failure (the central coverage-evidence sandbox could not install numpy —
fixed by ContextualWisdomLab/.github#611), not a code judgment. This commit
documents the change and produces a fresh head so the scheduler dispatches
a new review under the repaired pipeline.
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.

3 participants