fix(proposals): stop payload precheck shadowing dead claim refs - #601
Conversation
the propose-time payload validation added a page->claim existence check to _payload_block_reason. approve() runs that precheck before its own dead-claim-refs handling, so a page citing a deleted claim raised a flat ProposalError and DeadClaimRefsError could never surface -- the drop_missing_claims strip path became unreachable and three tests/test_dead_claim_refs.py cases went red on test. approve() now passes skip_dead_claim_refs=True; check_approvable keeps the check so the batch cli precheck still flags dangling refs and stays all-or-nothing. the storage test that asserted the old wording now pins the specific DeadClaimRefsError and that the page body is untouched.
|
Warning Review limit reached
Next review available in: 13 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
picks up the approve-gate fix (#601) so this branch's ci is green on its own base rather than inheriting the three red dead-claim-refs cases from test.
What changed
approve()no longer runs the page→claim existence check from_payload_block_reason; it passes the newskip_dead_claim_refs=Trueso itsown dead-claim-refs branch is the one that decides.
check_approvablekeepsthe check unchanged, so the batch
vouch approve a bprecheck still flagsdangling refs and stays all-or-nothing.
Why
two landed changes collided semantically on
test. the propose-time payloadvalidation added a page→claim existence check to
_payload_block_reason, andapprove()runs that precheck before reaching its dead-claim-refs handling.so a page proposal citing a deleted claim raised a flat
ProposalError,DeadClaimRefsErrorcould never surface through the real entrypoint, thedrop_missing_claimsstrip path became unreachable, and the jsonl surfacereported
invalid_requestinstead ofdead_claim_refs.three cases in
tests/test_dead_claim_refs.pyare red ontest(and on everypr based on it, e.g. #599) across py3.11/3.12/3.13.
What might break
nothing on disk. no file moves, no field shape changes. the only behaviour
change is at the approve gate, and it restores the documented contract: a page
citing a missing claim raises
DeadClaimRefsError(still aProposalError, soexisting
except ProposalErrorcallers are unaffected) instead of the genericblock, and the dangling ref is still never written.
VEP
not a surface change — restores the already-specified approve behaviour.
Tests
make checkpasses locally (lint + mypy + pytest)tests/test_dead_claim_refs.pygoesgreen again;
test_approve_page_update_rejects_stale_claim_refnow pinsthe specific
DeadClaimRefsErrorand asserts the existing page body isleft untouched
CHANGELOG.mdupdated under## [Unreleased]— fixes unreleasedbreakage introduced on
test, nothing user-visible to note