feat(pr-gate): word the latest-dev box as the condition the gate enforces - #5471
abhisheksharma2411 wants to merge 1 commit into
Conversation
…rces The readiness box read "I pushed my PR to the latest dev commit", which asks for the exact tip. The gate does not: `readinessClaimViolations` clears the claim while the head is at most `READINESS_LATEST_DEV_BEHIND_MAX` (10) commits behind the base. On a fast-moving `dev` that gap is a treadmill. An author who reads the box literally resyncs for unrelated commits; every resync moves the head; head-drift unticks all four boxes; and the exact-head CI evidence the previous round produced is invalidated. None of it reduces merge risk, because the gate was already satisfied — the two requirements simply pull in different directions. The sentence is now derived from the constant rather than written beside it, so the wording and the threshold cannot drift apart again: changing the tolerance rewords the box in the same commit. It also states that a maintainer may still ask for the exact tip, which keeps exact-tip sync available as a merge boundary instead of a continuously maintained condition. Rewording is safe for open pull requests, and the existing suites already prove it: `extractReviewReadiness` matches box count and checked state, never item text, and `appendReviewReadinessSection` returns a body that already carries the marker pair unchanged. Both the Node fixtures in pr-quality.test.cjs and the Bun `readinessChecklistBody` fixture build an old-worded checklist, and all of them still pass untouched — an open PR keeps its sentence and its ticks. Six new tests, including the boundary one that makes the sentence honest: the gate clears exactly the number the box promises, and one more is a violation. Mutation-tested 3/3 — restoring the hardcoded exact-tip sentence, hardcoding a number instead of deriving it, and dropping the maintainer clause each fail. All 22 .github/scripts suites pass. tests/ci-workflows/ is 1114 pass / 3 fail, the same three that fail on clean dev. Closes lidge-jun#4443
|
✅ Deterministic PR hygiene checks passed. |
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe PR-quality checklist now derives its latest-dev wording from ChangesLatest-dev readiness contract
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request has been marked Ready for Review. |
리뷰 · 우선순위 71 / 80체크리스트 한 칸이 “가장 최신 dev에 맞춰 밀어라”고 적혀 있었어요. 그런데 문이 실제로 보는 규칙은 “base보다 최대 10개만 뒤처지면 괜찮다”예요. 빠른 이 PR은 그 칸의 문장을 문 상수
메인테이너의 판단이 필요한 지점 #4443을 “문과 글자가 같아지면 충분”으로 닫을지, 겹침 확인 한 줄을 더 넣을지 정해 주세요. 10이 맞는 여유치인지는 이 PR이 일부러 안 건드렸어요. 같은 이슈를 노리는 다른 열린 PR은 없어요. 너의 추천 방향은 맞아요. 문과 칸이 서로 다른 말을 하던 구멍은 이걸로 막혀요. 테스트도 그 경계를 직접 고정해요. 겹침 확인까지 이슈에 넣을 거면 칸에 한 줄 더 쓰고, 아니면 #4443을 부분 해결로 두고 나머지를 새 이슈로 쪼개는 편이 덜 헷갈려요. 게이트 숫자를 바꾸는 커밋은 이 PR에 섞지 마세요. 이 댓글은 grok-bot이 작성했습니다 |
Closes #4443.
The two requirements disagreed
The readiness box says:
The gate says something else.
readinessClaimViolationsinpr-quality-state.cjsclears the claim while the head is at mostREADINESS_LATEST_DEV_BEHIND_MAX— 10 — commits behind the base:So the box asks for the exact tip and the enforcement tolerates ten behind. On a fast-moving
devthat gap is a treadmill, exactly as the issue describes: an author who reads the box literally resyncs for unrelated commits → the head moves → head-drift unticks all four boxes → the exact-head CI evidence from the previous round is invalidated → repeat. None of it reduces merge risk, because the gate was already satisfied.The change
The sentence is now derived from the constant instead of written next to it:
rendering as:
Deriving it is the durable half: the wording and the threshold cannot drift apart again, and raising or lowering the tolerance rewords the box in the same commit. The trailing clause keeps exact-tip sync available as a merge boundary rather than a continuously maintained condition, which is what the issue asked for.
Backward compatibility — proved by the existing suites, not just asserted
Rewording a checklist could plausibly untick every open PR. It doesn't, and the repo's own machinery is why:
extractReviewReadinessmatches/^\s*[-*]\s+\[([ xX])\]\s+/— box count and checked state, never item text. Its doc comment already says so: "The author can reword an item, but the box count and the checked state are the contract."appendReviewReadinessSectionis idempotent: a body that already carries the marker pair is returned unchanged.And this is exercised today, in both runners, against the old wording:
pr-quality.test.cjsbuilds a checked body with"- [x] I pushed my PR to the latest dev commit."and assertsuncheckReviewReadinessBoxesunticks by index while preserving the text.tests/ci-workflows/ci-workflows.test.tsdrives the real gate through thereadinessChecklistBodyfixture, which carries the same old line, and asserts on it after abehind_by: 11run.Both pass untouched on this branch. An open PR keeps its sentence and its ticks; only newly appended checklists use the new one.
Verification
node --test .github/scripts/pr-quality.test.cjs.github/scripts/*.test.cjssuitespr-quality-state43,issue-quality118,enforce-pr-target20, …)bun test --isolate tests/ci-workflows/The 3 failures are pre-existing: the same three fail on clean
dev(a bare pass run removes its preload-owned TEMP root,a bare fail run …,release shell recovers only unverified reads after acknowledged publication).One of the new tests is the one that makes the sentence honest — it checks the gate at the boundary the box now promises:
Mutation-tested 3/3:
Scope
Wording and its derivation only.
READINESS_LATEST_DEV_BEHIND_MAXkeeps its current value of 10, and no gate logic changes — whether ten is the right tolerance is a separate decision, and this PR deliberately does not make it.Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Tests