fix(ci): repoint stale scheduler_file self-test assertions at the core module - #1820
Conversation
…e module scripts/ci/test_strix_quick_gate.sh's scheduler_file variable pointed at pr_review_merge_scheduler.py, which the #1803 facade/core split reduced to a ~98-line thin facade. Ten assert_file_contains checks (branch-update SHA guard, squash-fallback retry, subprocess shell=False/check=True safety, same-head Strix/OpenCode dispatch markers, and the pr_head_ref repository_dispatch payload) still targeted that facade for content that now lives in pr_review_merge_scheduler_core.py, so they had been silently failing on every run since the split -- confirmed identical on a from-scratch clean main checkout before this fix. Add a core_scheduler_file variable alongside the existing scheduler_file and repoint the 9 in-scope assertions plus one separately hardcoded reference at it. Left the one scheduler_file assertion that already passes ("update-branch", present in both the facade and core) untouched, and left two other already-passing hardcoded-path assertions (event_type/dispatches strings, still present in the facade) untouched -- only the 10 confirmed-broken checks moved. Verified: bash scripts/ci/test_strix_quick_gate.sh now prints "test_strix_quick_gate: PASS" with zero FAIL lines (down from 10). Full Python suite unaffected: coverage run -m pytest tests -q (2775 passed, 1 skipped, 21 subtests), coverage report --fail-under=100 (100%), interrogate (100%). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 9 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team 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. 📝 WalkthroughWalkthroughCI self-test의 스케줄러 검증 대상을 facade에서 core 구현 파일로 변경했습니다. PR head 보호, 재시도, subprocess, Strix dispatch 및 workflow 조회 검증이 새 경로를 사용합니다. 변경 내역을 Changes스케줄러 코어 검증
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This updates CI self-tests to validate scheduler behavior in its current core module rather than the facade, eliminating stale false failures without changing production behavior. The change is ready to merge. Suggested reviewers: 🚥 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 |
…adence test_reconciled_scheduler_preserves_current_main_control_plane_fixes still asserted `- cron: "0 * * * *"` (hourly), a leftover from before pr-review-merge-scheduler.yml's own "Daily missed-event recovery" comments describe moving repository and org-wide missed-event recovery to a once-daily, staggered cadence (`47 3 * * *` / `17 3 * * *`) -- the same class of congestion-reduction cadence change this file's CHANGELOG already documents twice (*/30 -> hourly, 15-minute -> hourly). Confirmed pre-existing and blocking every PR org-wide regardless of diff: reproduced on a fresh, unmodified clone of main with zero other changes.
… daily cadence" This reverts commit 41411c1.
|
Updated this branch to current main (new head `10b28ffb`) — was `BEHIND`, not actually `CONFLICTING`; the merge itself was clean, no conflict markers. While verifying, found (and reproduced identically on a fresh, unmodified clean `origin/main` checkout — confirming it's pre-existing and unrelated to this PR's own diff) one currently-live regression: `tests/test_noema_repair_attempt_telemetry.py::test_served_model_is_annotation_safe` expects `_extract_served_model`/`_safe_model_identifier` (introduced by #1831) to return a scrubbed-but-non-`None` model id for a string containing control characters, a GitHub Actions `::error::` injection pattern, and a lone UTF-16 surrogate — but the actual implementation only does reject-or-accept via `SAFE_MODEL_IDENTIFIER_RE.fullmatch`, with no scrubbing step, so it returns `None`. This also leaves 4 lines in the sibling `_extract_http_error_served_model` uncovered (99% instead of 100%) and one inner closure (`_bounded_allowed_locations_json.render`) missing a docstring. Reported to the peer session network coordinating today's other required-suite regressions (#1823/#1826/#1828); not fixed here since it's unrelated to this PR's scope. Post-merge verification (excluding that one known pre-existing failure): `coverage run -m pytest tests -q` → 2771 passed, 1 skipped, 21 subtests, 1 pre-existing unrelated failure. 🤖 Generated with Claude Code |
github-project-automation
Bot
moved this from In Progress
to Done
in naruon Platform Roadmap
1 participant
Summary
scripts/ci/test_strix_quick_gate.sh'sscheduler_filevariable points atpr_review_merge_scheduler.py, which thepr_review_merge_scheduler.py/pr_review_merge_scheduler_core.pyfacade/core split (#1803) reduced to a ~98-line thin facade. Tenassert_file_containschecks — the branch-update SHA guard, the squash-fallback retry, subprocessshell=False/check=Truesafety, the same-head Strix/OpenCode dispatch markers, and thepr_head_refrepository_dispatch payload — still targeted that facade for content that now lives inpr_review_merge_scheduler_core.py, so they had been silently failing on every run since the split. A coverage/docstring version of the same gap was already fixed via #1810; this bash contract script was missed.Discovered incidentally while resolving PR #939's merge conflict in a separate stale-PR-triage session. Confirmed via a from-scratch clean
origin/mainworktree (no branch, no PR) that these exact 10 failures already exist on unmodifiedmaintoday, unrelated to any in-flight PR.Fix
Added a
core_scheduler_filevariable alongside the existingscheduler_fileand repointed the 9 in-scopeassert_file_containscalls plus one separately hardcoded reference (pr_head_ref, outside thescheduler_filevariable's scope) at it. Left untouched:scheduler_fileassertion that already passes ("update-branch", present in both the facade and core files)"event_type": "opencode-review"andrepos/{dispatch_repo}/dispatches, still present in the facade)Only the 10 confirmed-broken checks moved.
Verification
bash scripts/ci/test_strix_quick_gate.sh— now printstest_strix_quick_gate: PASSwith zero FAIL lines (down from 10 on unmodifiedmain).coverage run -m pytest tests -q— 2775 passed, 1 skipped, 21 subtests.coverage report --fail-under=100— 100%.interrogate— 100%.Developer experience
Restores real signal to a bash contract test that's been silently broken since #1803 — anyone running it locally was seeing 10 false failures with no clear indication they were pre-existing rather than caused by their own change.
User experience
No behavior change to any production workflow or script; this only fixes the self-test's own file references.
🤖 Generated with Claude Code
Summary by CodeRabbit
테스트
pr_head_refpayload 검증을 최신 계약에 맞게 조정했습니다.문서