fix(coverage): discover hash-pinned requirements lock files - #1157
fix(coverage): discover hash-pinned requirements lock files#1157seonghobae wants to merge 10 commits into
Conversation
|
@opencode-agent review |
|
Warning Review limit reached
Next review available in: 52 minutes 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. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day 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)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Changesrequirements 잠금 파일 후보 확장
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change broadens coverage-materializer discovery to include hash-pinned lock files and adds a focused regression test; no actionable merge-blocking risk remains beyond normal checks and review. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Current-head verification for |
|
@opencode-agent Review exact current head |
|
@opencode-agent review exact current head f0823bb against base c47bee5, including the candidate-lock naming boundary and regression tests. |
4ff27bf to
666153a
Compare
|
@opencode-agent Review exact current head 666153a against main@2cce96f8. Rebased the lock-discovery change onto current main; requirements*.lock and existing requirements*.txt discovery remain content/hash gated. Verified: 43 focused tests passed, compileall, interrogate, and git diff --check. |
|
@opencode-agent review exact head |
|
@opencode-agent Review current exact head |
|
@opencode-agent Review the current exact head for the trusted Python lock-materializer repair. Verify |
|
@opencode-agent review this exact current HEAD. Use the current commit SHA, current Checks, and current diff; do not reuse prior approvals or prior-head evidence. |
|
Independent current-head verification for |
|
Please perform an independent review against exact current head 107c572 only. Current read-only inventory: all nine current-head workflow runs completed successfully; no review submission is present. Do not self-approve or transfer predecessor-head evidence; protected merge still requires exact-head approval and terminal Checks. |
|
Remote agents advanced this PR; exact current HEAD is |
|
Correction: the exact current HEAD |
|
Current HEAD 8c8c70a: reviewed the discovery predicate and its callers; added a real service/requirements-dev.lock materialization case plus complete docstrings for the changed test module. Focused tests 58 passed, source and test interrogate 100%, Ruff, compileall, and diff check passed. Merge remains gated on independent approval and required Checks. |
|
Exact-head verification for 8c8c70a on 2026-08-21 Asia/Seoul: uv run --group dev pytest -q passed (1234 passed, 16 subtests); coverage passed at 100% statements and branches; interrogate scripts/ci --fail-under=100, compileall, and git diff --check passed. Fresh PR Checks for this exact HEAD are authoritative; cancelled runs for dda5cc5 do not carry forward. Devin's current review reports no issues. |
|
@opencode-agent please review exact current head 8c8c70a. Fresh Checks are running; independently validate the lock-discovery change and current coverage evidence. |
|
@opencode-agent Please review exact current PR head 8c8c70a. Re-check changed-file scope, current-head findings, unresolved threads, mergeability, and all required checks. Do not approve a stale head. |
|
Current-head review (2026-08-21):
|
Exact-head failure RCA
|
Exact-head audit update\n\n- Observed hosted pip-audit failure: run 96850603561 reported PYSEC-2026-3721 through pip 26.1.2.\n- Root-cause fix: refreshed the hash-locked CI dependency to pip 26.2.1 with both package hashes and updated the stale scheduler contract assertion to the current repository-dispatch scope.\n- Exact head: 7229a38\n- Exact base: 6a79f07\n- Local verification: 43 focused tests passed; pip-audit reported no known vulnerabilities; shell syntax and diff checks passed.\n- Hosted required checks and exact-head approval remain pending; decision remains WAIT_AND_REMEDIATE. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
7229a38 to
800d345
Compare
Current-head verification update
|
|
Exact-head audit update\n\nObserved hosted exact-head-path-policy failure on the prior head: the scheduler assertion still expected the shorter cancellation expression. The pip lock had already been refreshed on the current head.\n\nRoot-cause fix: aligned the assertion with the current scheduler behavior, which also cancels orphaned workflow-run groups while preserving active workflow-run evidence.\n\nExact head: 7fd8724\nExact base: fce028b\nLocal verification: 95 materialization/scheduler contract tests passed; shell syntax and diff checks passed.\nHosted required checks and exact-head approval remain pending; decision remains WAIT_AND_REMEDIATE. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Exact-head security follow-upThe required Python Security gate on sibling exact head |
|
The exact-head Strix run reported zero vulnerabilities before Caido guest bootstrap failed after 10 attempts because 127.0.0.1:48080 refused connections. This is runner infrastructure failure, not a source finding; the security gate remains fail-closed. Rerunning the failed hosted job. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@claude Review exact current head against base . Confirm coverage, security, data-loss, and workflow behavior from this head only. Do not self-approve, bypass, force-push, or merge. |
| if ' "requirements/ci.txt",\n' not in test_text: | ||
| if ' "services/account_unification/requirements-dev.lock",\n' in test_text: | ||
| replace_once( | ||
| TEST, | ||
| ' "requirements-test.txt",\n' | ||
| ' "services/account_unification/requirements-dev.lock",\n' | ||
| ' "services/account_unification/requirements-dev.txt",\n', | ||
| ' "requirements-test.txt",\n' | ||
| ' "requirements/ci.txt",\n' | ||
| ' "services/account_unification/requirements-dev.lock",\n' | ||
| ' "services/account_unification/requirements-dev.txt",\n', | ||
| ) | ||
| else: | ||
| replace_once( | ||
| TEST, | ||
| ' "requirements-test.txt",\n' | ||
| ' "services/account_unification/requirements-dev.txt",\n', | ||
| ' "requirements-test.txt",\n' | ||
| ' "requirements/ci.txt",\n' | ||
| ' "services/account_unification/requirements-dev.txt",\n', | ||
| ) |
There was a problem hiding this comment.
🟡 Untested migration branch breaks 100% coverage gate
The new module-level if only runs its body when the staged test file lacks requirements/ci.txt. The one test that executes this driver, test_bounded_repair_driver_runs_against_a_staged_fixture, copies the current files, which already contain that line, so the condition is always false and the enclosed lines never run. scripts/ci is gated at 100% coverage, so the uncovered branch fails CI.
Prompt for agents
The repair driver at scripts/ci/repair_pr827_coderabbit_comments.py now has a module-level if/elif/else (lines 269-290) whose body only executes when the target test file does not yet contain the line for requirements/ci.txt. The only test that runs this driver, test_bounded_repair_driver_runs_against_a_staged_fixture in tests/test_materialize_base_python_requirements.py, stages the current already-migrated repo files (which already contain requirements/ci.txt and the requirements-dev.lock source line), so the branch is never taken. Because scripts/ci is measured under a hard 100% coverage gate (pyproject.toml fail_under=100, reinforced by AGENTS.md/CLAUDE.md), the unexecuted lines and the untaken branch will drop coverage below 100% and fail CI. Fix by making that test stage a pre-migration fixture that forces the migration branch (e.g., rewrite the copied test file's expected-source list back to a state without requirements/ci.txt, in both the .lock-present and .lock-absent variants) so both branches execute, or otherwise ensure the new branches are exercised.
Was this helpful? React with 👍 or 👎 to provide feedback.
| def _is_candidate_lock_name(name: str) -> bool: | ||
| """Return whether a file name is a possible pip requirements lock.""" | ||
| return name == "requirements.lock" or ( | ||
| fnmatch.fnmatch(name, "requirements*.txt") | ||
| and not fnmatch.fnmatch(name, "requirements-*-ci-hashes.txt") | ||
| return ( | ||
| ( | ||
| fnmatch.fnmatch(name, "requirements*.txt") | ||
| and not fnmatch.fnmatch(name, "requirements-*-ci-hashes.*") | ||
| ) | ||
| or ( | ||
| fnmatch.fnmatch(name, "requirements*.lock") | ||
| and not fnmatch.fnmatch(name, "requirements-*-ci-hashes.*") | ||
| ) | ||
| ) |
There was a problem hiding this comment.
📝 Info: Lock candidate change is a safe superset
_is_candidate_lock_name adds requirements*.lock and widens the hash-file exclusion to requirements-*-ci-hashes.*. For .txt names this matches the prior behavior exactly, and content still must pass _is_hash_pinned in base_hash_locks before materialization. The repo ships no .lock requirements files, so its own coverage build is unaffected.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
The failed Strix attempt was reviewed against exact head 0aba0b4. The report contained no actionable source vulnerability and the provider then failed during scan completion, so the required check remains subject to rerun rather than being neutralized. A failed-only rerun is queued; no security gate was weakened. |
1 participant
Root cause
The trusted base Python coverage materializer only recognized
requirements.lockandrequirements*.txt. Repositories usingrequirements-dev.lockwere therefore measured without their async test plugin; pg-erd-cloud #931 recorded 133 async test failures and coverage evidence failed.Change
requirements*.lockalongside existing hash-pinned.txtcandidatesVerification
python -m pytest -q tests/test_materialize_base_python_requirements.py(43 passed)ruff check scripts/ci/materialize_base_python_requirements.py tests/test_materialize_base_python_requirements.pygit diff --checkCurrent branch head:
f0823bb.Summary by CodeRabbit
개선 사항
requirements-dev.lock과 같은 다양한 잠금 파일을 요구사항 파일로 인식하도록 지원 범위를 확장했습니다.테스트
Hosted exact-head lock-materializer evidence
8c8c70ae506e777b3a21885173f8c86b2e5f2a31in both Python 3.14 and Python 3.10 jobs.96459071429: focused materializer coverage reached 100%, the full central suite passed1234tests plus16subtests, and the central source inventory measured7838statements /3046branches at 100%.96459071725also checked out the same exact head.