Skip to content

fix(queen): a busy reviewer lane must try the next one, not end the review - #514

Merged
gHashTag merged 1 commit into
feat/queen-supervisorfrom
fix/reviewer-tries-another-lane
Sep 23, 2026
Merged

gHashTag merged 1 commit into
feat/queen-supervisorfrom
fix/reviewer-tries-another-lane

Conversation

@gHashTag

Copy link
Copy Markdown
Owner

What was happening

Twelve bees in flight, twenty-nine dispatches an hour, zero accepts, for hours on 2026-09-23.

The bees were fine. They were passing every machine check and still getting wait:

issue=4614 criteria=5 criteriaPassed=5 criteriaFailed=0 oracle="pass" verdict="wait"
  reviewerSkipped="the reviewer call failed: [1302] Rate limit reached for requests"

Sixteen reviewer calls in one log window, sixteen failures, every one of them ZAI's 1302/1305 — while fifteen NVIDIA credentials sat idle. reviewerModel=null on all 23 verdicts.

Why

chooseReviewerLane deliberately prefers a lane of a different vendor from the bee — that is the adversarial part. Bees ran NVIDIA, so every review went to the only other vendor, ZAI, which the pool-2 bees were already saturating. The reviewer competed for the quota it gates.

And then one line gave up:

if (answer.transient) break
markReviewerLaneFailed(lane)
choice = pick()

A transient failure ended the loop on the first refusal; only a non-transient one tried another lane. That is backwards. A rate limit belongs to one vendor's account, so it is exactly the case where the next lane answers. A 404 model name is the case where it does not.

What this changes

  • A refusal of any kind moves to the next lane. Only a broken lane is backed off for the half hour — busy is not broken, and burning a lane for thirty minutes turns a provider's bad minute into the Queen's bad half-hour.
  • A per-review set of lanes already tried. pick() is deterministic, so without it the retry is three calls to the endpoint that just said no.
  • lastTransient → sawTransient, sticky. It meant "did any lane say not-now", and was only the same as "was the last failure transient" while a transient failure ended the loop. Without this, a busy lane followed by a broken one reads as "no lane was busy" and charges the bee a miss for an outage it had no part in.

Evidence it is the right fix

Pinning the reviewer away from the exhausted vendor (TRIOS_QUEEN_REVIEW_POOL=1, TRIOS_QUEEN_REVIEW_MODEL=nvidia/nemotron-3-super-120b-a12b) in production was enough to restore it, which is the same effect this makes automatic:

verdicts accepted in wait reviewer calls failed
before 40 5 (13%) 4 16 of 16
pinned 12 3 (25%) 0 0

The pin is a workaround for one configuration; this fixes the behaviour for any.

Tests

Two new cases in queen-adversarial-review.test.ts:

  • a busy lane falls through to the next one, the calls are [pool 2, pool 1] and never the same lane twice, and the second lane's answer is written;
  • a busy lane followed by a broken one records no miss.

bun test apps/server/tests/api/queen-adversarial-review.test.ts — 46 pass, 0 fail. Three failures elsewhere in apps/server/tests/api/ (queen-tree-load, queen/public-research) are present on feat/queen-supervisor without this change.

🤖 Generated with Claude Code

…eview

On 2026-09-23 the swarm ran twelve bees and accepted nothing for hours. The
bees were not the problem: they passed every machine check
(`criteriaPassed=5 criteriaFailed=0 oracle="pass"`) and the verdict was still
`wait`, because the adversarial reviewer never answered. Sixteen reviewer
calls in one window, sixteen failures, all of them ZAI's 1302 and 1305 - while
fifteen NVIDIA credentials sat idle.

The reason is one line. A transient failure did `break`, ending the retry loop
on the first refusal; only a NON-transient failure fell through to another
lane. That is backwards. A rate limit belongs to one vendor's account, so it
is precisely the case where the next lane answers - and a 404 model name is
the case where it does not. The comment above the loop already promised "a
lane refused for good is backed off and the next one is tried"; the code did
that only for the failures where it helps least.

So a refusal of any kind now moves to the next lane, and only a lane that is
BROKEN is backed off for the half hour. Busy is not broken: a lane answering a
rate limit will take work again in a minute, and burning it for thirty turns a
provider's bad minute into the Queen's bad half-hour.

Two smaller things the fall-through makes necessary:

  * `chooseReviewerLane` is deterministic, so without a record of what was
    already tried the retry is three calls to the one endpoint that just said
    no. A per-review set, not the half-hour backoff, which belongs to a lane
    that is broken.

  * `lastTransient` was "was the LAST failure transient", which was the same
    thing as "did any lane say not-now" only while a transient failure ended
    the loop. Now a busy lane followed by a broken one would read as "no lane
    was busy" and charge the bee a miss for an outage it had no part in. The
    flag is sticky and named for what it means.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the fix label Sep 23, 2026
@github-actions

Copy link
Copy Markdown

⚠️ No test results were produced

View workflow run

@gHashTag
gHashTag merged commit c25e1b0 into feat/queen-supervisor Sep 23, 2026
3 of 18 checks passed
@github-actions
github-actions Bot deleted the fix/reviewer-tries-another-lane branch September 27, 2026 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant