Let the fire slot serialize the metered review and nothing else - #47
Conversation
Pump returned as soon as it had progressed the round holding the fire slot. The rescue scan further down said it covered "blocked or slot-busy" starvation, and its condition tested both — but control never reached it with the slot held, so the slot-busy half had never run. A summary-only round, whose CodeRabbit review is never coming at all, therefore sat queued for as long as an unrelated PR's review took. The slot exists to serialize one account-metered review; the account window bounds that same allowance. Neither has anything to say about a round that spends no quota. So progressing the slot round no longer ends the pump: it reloads, then sweeps the quota-free rounds behind it. A pump still never both progresses and fires, because a quota-free verdict is not a fire. The scan is now one function used by both paths that can starve, rather than a loop inlined into the tail of the account-blocked case, which is how one of its two callers came to be unreachable.
|
Warning Review limit reached
Next review available in: 24 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: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Roadmap item F2: the queue exists for one thing, and this makes the code say so.
The bug
Pumpreturned as soon as it had progressed the round holding the fire slot:Further down, the bounded rescue scan guarded itself with
so it claimed to cover both starvation causes. The
!global.SlotFreehalf had never executedonce — control cannot reach that line with the slot held. A summary-only round, whose CodeRabbit
review is never coming at all, therefore stayed queued for as long as an unrelated PR's review ran.
TestWaitResolvesSummaryOnlyWithoutTheQueuedocumented the short-circuit and routed around it inWait; nothing fixed it in the pump.The rule
The fire slot serializes one account-metered review. The account window bounds that same
allowance. Neither has any authority over a round that spends no quota — a co-reviewer defer, a
dedupe, a summary-only round.
So progressing the slot round no longer ends the pump. It reloads state (progressing may have
released the slot) and sweeps the quota-free rounds behind it. A pump still never both progresses
and fires, because a quota-free verdict is not a fire, takes no slot, and posts no
@coderabbitai review.The scan is now one
sweepQuotaFreecalled from both paths that can starve, instead of a loopinlined into the tail of the account-blocked case — which is how one of its two callers came to be
unreachable in the first place. The scan budget and the rotating start survive unchanged; they are
cost control, and the rotation still answers the case where the rounds behind the head all need
CodeRabbit.
Verification
TestPumpSweepsQuotaFreeRoundWhileTheSlotIsHeldstages it directly: an unrelated PR holds the slot,a summary-only round is queued behind it, one
Pumpmust move it. Confirmed to fail onmain(
the summary-only round must move while the slot is held ... Phase:"queued") and pass here. It alsoasserts the slot still belongs to the holder afterwards and that no CodeRabbit command was posted —
otherwise "it moved" could mean it moved by spending the very quota it must not.
gofmt -l .clean,go vet ./...clean,go test ./... -count=1green across all six packages.Ref #42