fix(review): Rust coverage toolchain, offline crate cache, and foreign-interpreter base-lock tolerance - #650
fix(review): Rust coverage toolchain, offline crate cache, and foreign-interpreter base-lock tolerance#650seonghobae wants to merge 12 commits into
Conversation
|
Warning Review limit reached
Next review available in: 9 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthrough커버리지 워크플로가 Rust 의존성을 격리된 환경에서 프리패치하고 Rust 1.94.1의 오프라인 실행을 검증하도록 변경되었습니다. Python 잠금 파일 탐색 범위와 base-lock 설치 오류 처리도 조정되었습니다. Changes커버리지 파이프라인 강화
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MergeTree
participant PrefetchEnv
participant CargoCache
participant OfflineRust
MergeTree->>PrefetchEnv: Cargo.toml 및 Cargo.lock 검색
PrefetchEnv->>PrefetchEnv: cargo fetch --locked 실행
PrefetchEnv->>CargoCache: Cargo registry 및 git 캐시 저장
CargoCache->>OfflineRust: 캐시 제공
OfflineRust->>OfflineRust: Rust 1.94.1 및 rustc 확인
OfflineRust-->>OfflineRust: 확인 실패 시 FAIL
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/opencode-review-dispatch.yml:
- Around line 306-307: Update the prefetch flow around the prefetch_cargo fetch
invocation to use a runner-owned temporary CARGO_HOME and HOME rather than any
PR-tree configuration. After fetching, copy only the resulting registry/ and
git/ directories into the coverage cache, excluding config.toml, credentials,
and other configuration files.
- Around line 1502-1512: Update the coverage sandbox setup near the
RUSTUP_TOOLCHAIN export to disable rustup auto-installation with
RUSTUP_AUTO_INSTALL=0, then add a prerequisite rustc --version check so
execution fails immediately when toolchain 1.94.1 is unavailable in the image.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cf8ee212-d182-4124-a674-5934ea7723de
📒 Files selected for processing (1)
.github/workflows/opencode-review-dispatch.yml
…ustup auto-install Address the two CodeRabbit findings on the offline Rust coverage lane: 1. The locked crate prefetch now runs from a runner-owned temporary HOME and CARGO_HOME with a neutral working directory, so cargo can never load pull-request-controlled configuration (including an external credential-provider executable committed to the merge tree) on the authenticated online runner. Only the content-addressed registry/ and git/ caches are copied into the tree-local sandbox CARGO_HOME; config.toml and credential files are never propagated. 2. ensure_rust_toolchain exports RUSTUP_AUTO_INSTALL=0 and probes rustc --version before measuring, so a missing pinned 1.94.1 toolchain fails fast as an image-provisioning error instead of a rustup download attempt the network-isolated sandbox cannot satisfy. Verified: workflow YAML parses; pytest tests — 674 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f
|
@coderabbitai review Generated by Claude Code |
|
✅ Action performedReview finished.
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore 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 head2a77129279623e762687fff4d81b63ad29c92d25. -
Head SHA:
2a77129279623e762687fff4d81b63ad29c92d25 -
Workflow run: 30502974830
-
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["Workflow: opencode-review-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-review-dispatch.yml"]
R1 --> V1["actionlint plus required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: opencode-review-dispatch.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: opencode-review-dispatch.yml"]
R1 --> V1["actionlint plus required checks"]
|
…mage build The content-based base-lock discovery (#657) now materializes every fully hash-pinned requirements*.txt at the PR base — including tool locks compiled for a different interpreter. In ContextualWisdomLab/.github itself that selects requirements-strix-ci-hashes.txt (Python 3.13 target), whose litellm==1.89.2 pin has no Python 3.14 distribution, so the trusted coverage image build aborts and every PR's coverage-evidence fails deterministically (observed on run 30502974830). Make the per-lock install best-effort: a lock that cannot install on the sandbox interpreter logs a warning and is skipped instead of killing the image build. Materialization is provisioning, not evidence — the offline test run remains the fail-closed gate and still surfaces any genuinely missing dependency for the measured suite, matching the crate-prefetch failure semantics elsewhere in this workflow. Verified: workflow YAML parses; pytest tests — 677 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f
seonghobae
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/opencode-review-dispatch.yml:
- Around line 309-333: Update the cargo prefetch subshell in the
prefetch_manifest loop to pass RUSTUP_HOME alongside HOME and CARGO_HOME, using
the existing RUSTUP_HOME value or the default $HOME/.rustup. Preserve reuse of
the runner’s installed rustup toolchain while keeping the temporary cargo cache
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f1dad142-4e9b-48fa-a6e9-5a798c45cc1a
📒 Files selected for processing (1)
.github/workflows/opencode-review-dispatch.yml
…lated prefetch env Redirecting HOME to a runner-owned temporary directory isolates cargo from pull-request-controlled configuration, but rustup resolves its toolchain store from RUSTUP_HOME (default $HOME/.rustup) — with HOME overridden, `rustup run 1.94.1 cargo` would look in the empty temporary directory, fail on every manifest, and silently degrade the offline crate cache to nothing. Resolve the runner's rustup home before the override and pass it into the prefetch subshell; the temporary cargo cache behavior is unchanged and rustup's toolchain store is runner-owned, never pull-request content. Reported by CodeRabbit on #650. Verified: workflow YAML parses; pytest tests — 677 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f
Content-based lock discovery still gates candidates on the basename glob requirements*.txt, so repositories that keep pip-compile outputs under a requirements/ directory with other basenames — fast-mlsirm's requirements/ci.txt and requirements/package.txt — materialize nothing. Their offline coverage suites then fail at collection with ModuleNotFoundError: numpy (observed on run 30506123636 rejecting fast-mlsirm#162), so every Python-touching PR in such repositories is blocked from approval. Accept any .txt directly inside a requirements/ directory as a name candidate; the content-based hash-pin check remains the safety gate, so the unpinned requirements/ci.in input stays excluded and PR-mutable files still never enter the networked build context. Verified: pytest tests — 679 passed; coverage on scripts/ci stays 100%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f
…effort install)
2 participants
Problem
Two coverage-sandbox provisioning defects block PR reviews:
Cargo.toml/Cargo.lock/*.rs,run_rust_test_coverageinvokescargo llvm-covand fails in the offline sandbox withfailed to find llvm-tools-preview. Live example: fast-mlsirm#196 (run 30452664727). Behind it: the distributioncargo(1.75) cannot parse version-4Cargo.lockfiles, and the--network=nonesandbox cannot download crates.requirements-strix-ci-hashes.txt(Python 3.13 target), whoselitellm==1.89.2pin has no Python 3.14 distribution — the image build dies and every PR's coverage-evidence fails deterministically (run 30502974830).Fix (fail-closed offline design unchanged)
rustup-init1.28.2 provisions Rust1.94.1withllvm-tools-previewunder/usr/local/rustup, symlinked into/usr/local/bin. No mutablesh.rustup.rsinstaller.cargo fetch --lockedfor every tracked manifest with a siblingCargo.lock(root and nested workspaces). The fetch runs from a runner-owned temporaryHOME/CARGO_HOMEwith a neutral working directory, so PR-tree cargo configuration (including external credential providers) is never loaded on the authenticated runner; only the content-addressedregistry/andgit/caches are copied into the tree-local sandboxCARGO_HOME.ensure_rust_toolchainexportsCARGO_NET_OFFLINE=true,RUSTUP_TOOLCHAIN=1.94.1, andRUSTUP_AUTO_INSTALL=0, and probesrustc --versionso a missing pinned toolchain fails fast as an image-provisioning error instead of an impossible offline download.Verification
pytest tests— 677 passed on this head (contract tests for the dispatch workflow included)docker run … --network=none, noapt-getat runtime, pinnedcargo-llvm-covretained) is unchangedmlsirm-coremeasures 100% line coverage undercargo llvm-cov --workspace --all-features, so fast-mlsirm#196 approves once this landsCARGO_HOMEisolation; rustup auto-install) addressed in 2efd782 and confirmed by the reviewerNote: required reviews execute the base branch's workflow, so this PR cannot heal its own coverage-evidence — defect 2 keeps the check red until this lands on
main(maintainer decision needed, see the report on #657).Developer experience: Rust-touching PRs across the org regain a passable coverage-evidence lane, and repos with multi-interpreter tool locks (starting with this one) stop failing coverage at image build.
User experience: no user-facing behavior change; coverage verdicts become meaningful instead of unconditionally failing on provisioning defects.
🤖 Generated with Claude Code
https://claude.ai/code/session_01ERVynrYEjd2TwYvusVpA9f
Summary by CodeRabbit