Render one ordered review queue on the dashboard#38
Conversation
A round in awaiting_retry whose RetryAt has not passed is correctly not
fire-eligible, so QueuedRounds excluded it — and then it vanished from the
dashboard entirely. Two PRs parked until 00:07Z rendered byte-identically to
an empty queue ("_Nothing queued._", title "idle"), so a reader concluded
their enqueue had been dropped.
Add parkedRounds: the active rounds that no other part of the dashboard
accounts for — not fire-eligible, and not named in the "In flight"/"Feedback
wait" rows. That also catches two neighbouring blind spots: every reviewing
round past the first (only reviewing[0] gets a row) and a reserved round whose
fire slot was cleared. Every active round now appears in exactly one of "In
flight", "Feedback wait", "Queue" or "Parked".
The queue heading gains a "· N parked" suffix and the empty-state text is
suppressed whenever work is parked; RenderTitle reports parked counts so a
parked-only state is never "idle". FireEligible/QueuedRounds and firing
behaviour are untouched — this is rendering only.
Adds internal/state/dashboard_test.go, the first test file in the package.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
@codex review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The dashboard had two ways to be waiting for a fire slot. "Queue" listed fire-eligible rounds; a round in awaiting_retry whose RetryAt had not passed fell out of QueuedRounds and needed a separate "Parked" table to be visible at all. Rendering live fleet state showed why that is the wrong cut: two rounds sat parked with near-identical retry times and nothing said which went first, or how they related to the rounds listed above them. They were never racing. NextEligible takes the lowest Seq among eligible rounds, Seq survives AwaitRetry untouched, and the FireSlot admits one fire fleet-wide — a cooling-down round is already queued, in order. What was wrong is the model: a round's position in line (Seq) and its permission to run (RetryAt) are independent properties, and crq rendered the first as a queue and the second as a lifecycle phase. State.Queue is the single view: every round waiting for a fire, ordered by (ReadyAt, Seq) — the order they actually reach the slot. ReadyAt folds in the account-wide quota block, which fixes a real bug: the parked table printed the round's own RetryAt while DecideFire also gates on Account.BlockedUntil, so a window CodeRabbit later extended left the dashboard promising a retry time the fire gate would refuse. FireEligible, QueuedRounds and NextEligible are untouched; this is a view, not a schema or a rule. The single "In flight"/"Feedback wait" summary rows are replaced by an In flight section listing every reserved/fired/reviewing round. Those rows showed one round each, which hid every reviewing round past the first and any reserved round whose FireSlot Normalize had cleared. In flight and Queue now partition Active() by phase, so "every active round is on the dashboard" holds by construction rather than by a catch-all — pinned by a test.
|
@coderabbitai review |
✅ Action performedReview finished.
|
kristofferR
changed the title
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 603cb0778a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case slotBusy: | ||
| e.Why = WaitSlotBusy |
There was a problem hiding this comment.
When another PR holds the fire slot, this branch sets Why to slot busy but leaves ReadyAt at zero, which the dashboard renders as ready: now. Such an entry cannot fire until the current holder releases the account-wide slot, so the new table presents contradictory and incorrect availability information; render the ready time as unknown/slot-dependent instead of now in this case.
AGENTS.md reference: AGENTS.md:L71-L73
Useful? React with 👍 / 👎.
| // MinInterval is deliberately NOT reflected: it is sub-minute pacing, so | ||
| // surfacing it would only churn DashboardSHA on every render. |
There was a problem hiding this comment.
After the slot is released shortly following a fire, queued entries are rendered as ready: now even though DecideFire still rejects them until LastFired + MinInterval; this occurs with the default 90-second interval and can last arbitrarily longer when CRQ_MIN_INTERVAL is customized. Unlike a relative countdown, that absolute boundary is stable and would not churn the dashboard hash, so either include it in ReadyAt or avoid claiming the entry is ready now.
AGENTS.md reference: AGENTS.md:L84-L87
Useful? React with 👍 / 👎.
| case len(inFlight) > 0: | ||
| fmt.Fprintf(&b, "### 🟡 Awaiting feedback for %s#%d\n\n", inFlight[0].Repo, inFlight[0].PR) |
There was a problem hiding this comment.
When a round is reserved but has no matching FireSlot—the exact stranded case inFlightRounds now includes—slot is nil and this branch reports that crq is “Awaiting feedback.” A reserved round precedes command posting, so no feedback can arrive, and Pump cannot progress it without the slot; the same misleading state is also emitted by RenderTitle. Surface this as a stranded reservation or generic in-flight state rather than a feedback wait.
AGENTS.md reference: AGENTS.md:L54-L59
Useful? React with 👍 / 👎.
1 participant
Problem
The dashboard had two ways to be waiting for a fire slot.
## ⏳ Queuelisted fire-eligible rounds; a round inawaiting_retrywhoseRetryAthad not passed fell out ofQueuedRoundsand was invisible. The first commit here added a separate "Parked" table for those. Rendering live fleet state showed that was the wrong cut:Two lists, near-identical retry times, and nothing saying which goes first or how they relate.
They were never racing
NextEligibletakes the lowestSeqamong eligible rounds,SeqsurvivesAwaitRetryuntouched, and theFireSlotis a CAS admitting one fire fleet-wide. A cooling-down round is already queued, in order — crq just rendered it as a separate species of work.The defect is the model: a round's position in line (
Seq) and its permission to run (RetryAt) are independent properties, and crq rendered the first as a queue and the second as a lifecycle phase.Change
State.Queue(now)is the single view: every round waiting for a fire —queuedandawaiting_retryalike — ordered by(ReadyAt, Seq), the order they actually reach the slot. Each entry carries an absolute ready time and the gate holding it (cooling down/account blocked/slot busy).A real bug fixed on the way. The parked table printed the round's own
RetryAt, butDecideFirealso gates onAccount.BlockedUntil. If CodeRabbit extended the account window after a round parked, the dashboard promised a retry time the fire gate would refuse.ReadyAtnow folds in the account block — the samemax(...)State.AccountBlockedUntilalready computes for the wait path.## 🔬 In flightreplaces the singleIn flight/Feedback waitsummary rows, which showed one round each and so hid every reviewing round past the first plus any reserved round whoseFireSlotNormalizehad cleared. In flight (reserved|fired|reviewing) and Queue (queued|awaiting_retry) now partitionActive()by phase, so "every active round is on the dashboard" holds by construction rather than by a catch-all.RenderTitlecounts the whole queue, so a state whose only work is cooling down reads🐰 crq — 2 queued, neveridle.FireEligible,QueuedRounds,NextEligible, queue ordering, firing behaviour and the hiddencrq:stateblock are untouched — this is a view, not a schema or a rule.Tests
internal/state/dashboard_test.gois new to the package (dashboard rendering was untested): cooling-down-only renders no empty state and a correct header, genuinely-empty keeps_Nothing queued._and theidletitle,(ReadyAt, Seq)ordering including an elapsedRetryAtsorting among the ready rounds, the account block dominating a round's ownRetryAt,slot busy, theActive()partition, andCRQ_TZformatting.gofmt -l .clean,go vet ./...andgo test ./...pass. Verified against live fleet state withcrq status(read-only); thecrq-stateissue was not hand-edited.Note:
crq preflightcould not be re-run before this push — the CodeRabbit account was rate-limited and the autoreview daemon won each window. The branch's first push had a clean preflight, and this PR's own review covers the change.