fix(review): make offline coverage sandbox honor src-layout packages - #660
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes 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 (4)
📝 WalkthroughWalkthrough커버리지 이미지의 기본 Python 요구사항 설치 실패를 원인별로 처리하고, 커버리지 및 pytest 실행 시 Changes커버리지 실행 경로 보정
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CoverageWorkflow
participant Pip
participant PythonRunner
CoverageWorkflow->>Pip: 기본 요구사항 설치
Pip-->>CoverageWorkflow: 설치 결과와 로그 반환
CoverageWorkflow->>PythonRunner: 조건부 PYTHONPATH로 커버리지 실행
PythonRunner-->>CoverageWorkflow: 커버리지 결과 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 headb29b9a44ff55f31ebdaebab51912ab8db9479548. -
Head SHA:
b29b9a44ff55f31ebdaebab51912ab8db9479548 -
Workflow run: 30503230897
-
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"]
Evidence --> S2["CI script: safe_pytest_command.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: safe_pytest_command.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (2 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (2 files)"]
R3 --> V3["targeted test run"]
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"]
Evidence --> S2["CI script: safe_pytest_command.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: safe_pytest_command.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test (2 files)"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test (2 files)"]
R3 --> V3["targeted test run"]
|
…rage image Regression from the merged content-based lock discovery (#657): the coverage image now materializes every hash-pinned requirements*.txt at the base SHA, including this repo's tool-CI locks compiled for other interpreters (bandit/pip-audit 3.12, strix 3.13). The coverage image runs Python 3.14, and a transitive pin such as litellm==1.89.2 (Requires-Python >=3.10,<3.14) has no 3.14 wheel, so the --require-hashes install aborted the whole image build and coverage-evidence failed for every .github PR (observed on #660 run 30503230897: "No matching distribution found for litellm==1.89.2"). Make the base-requirements install loop skip a set that is not installable on the image interpreter instead of aborting the build. The essential coverage toolchain is installed separately and always present, and any genuinely missing repository test dependency still surfaces as a pytest import failure during the coverage run, so the coverage signal stays honest. Pins the failure-tolerant behavior in the agent contract test. 678 passed, scripts/ci coverage 100%, interrogate 100%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/opencode-review-dispatch.yml (1)
629-647: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win인터프리터 비호환 이외의
pip install실패까지 모두 삼키고 있습니다.
set -e는pip install ... || echo ...가 OR-list이므로 무력화됩니다. 해시 불일치, 손상된 requirements, 의존성 충돌, 레지스트리/네트워크 오류도 warning만 남기고 이미지가 성공적으로 빌드됩니다. 그 결과 불완전한 테스트 환경에서 coverage가 실행될 수 있습니다. 알려진 인터프리터 비호환 오류만 건너뛰고 그 외 오류는 빌드를 실패시키세요.🤖 Prompt for 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. In @.github/workflows/opencode-review-dispatch.yml around lines 629 - 647, Update the pip installation loop in the base requirements materialization step so only failures caused by interpreter incompatibility are skipped with a warning. Preserve the current successful-install behavior, but capture and inspect pip’s error output/status; re-raise or fail the image build for hash, requirements, dependency-resolution, registry, network, and other errors. Keep the cleanup and manifest iteration behavior unchanged.
🤖 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 `@tests/test_opencode_agent_contract.py`:
- Around line 657-661: Update the contract assertions for measure so the
docstring execution command is independently checked against regression to
hardcoded PYTHONPATH=.; do not rely on the existing coverage-only assertion or
the single conditional PYTHONPATH occurrence. Preserve the conditional src:.
fallback behavior for both coverage and docstring runners, and run the full test
suite.
---
Outside diff comments:
In @.github/workflows/opencode-review-dispatch.yml:
- Around line 629-647: Update the pip installation loop in the base requirements
materialization step so only failures caused by interpreter incompatibility are
skipped with a warning. Preserve the current successful-install behavior, but
capture and inspect pip’s error output/status; re-raise or fail the image build
for hash, requirements, dependency-resolution, registry, network, and other
errors. Keep the cleanup and manifest iteration 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: ce8e5db0-ea2c-4e19-814a-1ed494f2f653
📒 Files selected for processing (4)
.github/workflows/opencode-review-dispatch.ymlscripts/ci/safe_pytest_command.pytests/test_opencode_agent_contract.pytests/test_opencode_security_boundaries.py
…docstring PYTHONPATH Addresses CodeRabbit review on #660: - The base-requirements install loop previously swallowed every pip failure. Now only an interpreter incompatibility (pip's "require a different python version" report) is skipped with a warning; hash mismatches, missing distributions, dependency conflicts, and registry/network errors again fail the image build. This preserves --require-hashes supply-chain integrity and guarantees a broken environment never silently reaches the coverage run. - The agent contract test now pins the docstring runner's src-aware PYTHONPATH independently (and asserts the bare PYTHONPATH=. form is absent), so the docstring path cannot regress while the coverage path stays src-aware. 678 passed, scripts/ci coverage 100%, interrogate 100%, workflow YAML valid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HdCssGnNMhKHNu3TXFstWH
Update: broadened the skip — the regression blocks sibling coverage-evidence org-wide, not just
|
e7b769b to
9a218df
Compare
|
Current-head reconstruction for
Local evidence:
|
|
Merged-audit evidence for current head 9a218df:
Post-merge origin/main full-suite verification is being recorded separately once complete. |
|
Post-merge verification completed on the exact remote default-branch commit 7231cb7: 701 passed in 119.14s. The temporary detached worktree was removed after verification. |
Why
Follow-up to #657. The central OpenCode
coverage-evidencesandbox runs every repository's suite withPYTHONPATH=.from the project root. Repositories that keep their importable package undersrc/(a src-layout, e.g.src/<package>) therefore fail collection withModuleNotFoundError, socoverage-evidencereportsfailureand OpenCode posts a deterministicREQUEST_CHANGESthat blocks merge even when the repository's own CI is green.This is the same class of central-sandbox discovery gap that #657 fixed for hash-lock names — now for the import path. Observed on
semantic-data-portal(package undersrc/sdp,src/sdp_core; its own CI runsPYTHONPATH=src pytest), whose coverage-evidence failed on empty-import despite a green in-repoTestsworkflow.What
Prepend
srctoPYTHONPATHwhen asrcdirectory exists, in both places the sandbox runs Python evidence:scripts/ci/safe_pytest_command.py::execute_command(the shell-free configured-command executor) — via a new_project_python_pathhelper. It also hardcodedPYTHONPATH=., so even a discoveredci.ymlcommand could not import a src-layout package..github/workflows/opencode-review-dispatch.yml(the path most src-layout repos actually hit, since their CI file is notci.y*ml):PYTHONPATH="$([ -d src ] && printf src:. || printf .)".Flat-layout repositories are unaffected — with no
src/dir the path stays exactly.(the previous behavior). The.githubrepo itself has nosrc/, so its own coverage-evidence is unchanged.Verification
coverage run -m pytest tests→ 678 passed;scripts/ciline coverage 100% (safe_pytest_command.py93 stmts, 0 missed), TOTAL 100%.interrogate scripts→ PASSED (100.0%).test_safe_pytest_executor_adds_src_layout_to_pythonpathassertssrcis prepended when present; the existing flat-layout executor test still pinsPYTHONPATH == ".".[ -d src ] && printf src:. || printf .) and asserts the old barePYTHONPATH=. ... pytest testsform is gone.Scope / impact
Unblocks src-layout repositories org-wide (semantic-data-portal and any peer using
src/) so their coverage-evidence passes and the reviewer can approve on real evidence. No behavior change for flat-layout repos. Remaining coverage-sandbox gaps for pip-incompatible layouts (uv.lock-only repos; native-extension repos needing a Rust/maturin build) are separate and not addressed here.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
src레이아웃 프로젝트에서 테스트/커버리지 실행 시 모듈 탐색 경로가 리포지토리 구조에 맞게 일관되게 설정됩니다.PYTHONPATH설정에 대한 검증을 보강했으며,safe_pytest실행 환경의src포함 여부를 추가로 확인합니다.