diff --git a/trios/agent-server/apps/server/src/api/services/queen-tick.ts b/trios/agent-server/apps/server/src/api/services/queen-tick.ts index 8a7ab44a02..d543a4cceb 100644 --- a/trios/agent-server/apps/server/src/api/services/queen-tick.ts +++ b/trios/agent-server/apps/server/src/api/services/queen-tick.ts @@ -95,6 +95,7 @@ import { reviewerAnswers, reviewerFingerprint, reviewerLaneBackedOff, + reviewerLaneKey, reviewerMessage, sameModelAs, visiblePatchPaths, @@ -3188,11 +3189,20 @@ export async function reviewFinishedDispatches( // `runRound` counts them before it hands out a key. takenKeys ??= await runningKeys(pool) const taken = takenKeys + // A LANE THAT ALREADY REFUSED THIS REVIEW IS NOT OFFERED AGAIN. + // + // `chooseReviewerLane` is deterministic, so without this the same lane + // is handed back every time round the loop and the retry is three + // calls to the one endpoint that just said no. Scoped to this one + // review, unlike `markReviewerLaneFailed`, which is a half-hour + // backoff and belongs only to a lane that is broken rather than busy. + const triedLanes = new Set() const pick = () => chooseReviewerLane( deps .laneCandidates(taken) - .filter((lane) => !reviewerLaneBackedOff(lane)), + .filter((lane) => !reviewerLaneBackedOff(lane)) + .filter((lane) => !triedLanes.has(reviewerLaneKey(lane))), bee, ) let choice = pick() @@ -3225,13 +3235,20 @@ export async function reviewFinishedDispatches( // deterministic, so a lane that can never answer was chosen again // every round; a lane refused for good is backed off and the next // one is tried, down to the bee's own model as a last resort. - let lastTransient = false + // DID ANY LANE SAY "NOT NOW". Sticky, and it used to be "was the + // LAST failure transient" - which was the same thing only while a + // transient failure ended the loop. Now that a busy lane falls + // through to the next one, 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. + let sawTransient = false for ( let tries = 0; choice && tries < REVIEWER_LANE_TRIES; tries++ ) { const lane: WorkerProvider = choice.lane + triedLanes.add(reviewerLaneKey(lane)) const answer = await deps.llm( lane, REVIEWER_SYSTEM_PROMPT, @@ -3241,7 +3258,7 @@ export async function reviewFinishedDispatches( // Nothing spent: a 1302 or a timeout is the provider saying // "not now", and it must not read as a finding about the work. reviewerSkipped = `the reviewer call failed: ${answer.error}` - lastTransient = answer.transient + sawTransient ||= answer.transient logger.warn('Queen reviewer call failed; nothing was spent', { issue, reviewerModel: lane.model, @@ -3249,12 +3266,25 @@ export async function reviewFinishedDispatches( transient: answer.transient, error: answer.error, }) - if (answer.transient) break - markReviewerLaneFailed(lane) + // A TRANSIENT REFUSAL TRIES THE NEXT LANE TOO. This used to + // `break`, which gave up on the whole review the moment one + // provider said "not now" - and that is exactly the case where + // another lane answers, because a rate limit belongs to ONE + // vendor's account and the others are idle. On 2026-09-23 the + // reviewer ran 16 calls and lost 16 of them to ZAI's 1302 and + // 1305 while fifteen NVIDIA credentials sat unused; every + // finished bee went to `wait` and the swarm accepted nothing + // for hours. + // + // Only a lane that is broken is backed off for the half hour. + // Busy is not broken: a lane that answered a rate limit will + // answer work again in a minute, and burning it for thirty + // would turn a provider's bad minute into the Queen's bad + // half-hour. + if (!answer.transient) markReviewerLaneFailed(lane) choice = pick() continue } - lastTransient = false // THE COMPILER'S LINES ONLY, as `reviewerMessage` is given // them. `citesEvidence` harvests every file-shaped token out of // a met machine line and makes it citable for EVERY criterion, @@ -3319,7 +3349,7 @@ export async function reviewFinishedDispatches( }) break } - if (!reviewer && !reviewerMissed && !lastTransient) { + if (!reviewer && !reviewerMissed && !sawTransient) { reviewerMissed = true } } diff --git a/trios/agent-server/apps/server/tests/api/queen-adversarial-review.test.ts b/trios/agent-server/apps/server/tests/api/queen-adversarial-review.test.ts index d7da11b34f..af5464728f 100644 --- a/trios/agent-server/apps/server/tests/api/queen-adversarial-review.test.ts +++ b/trios/agent-server/apps/server/tests/api/queen-adversarial-review.test.ts @@ -586,6 +586,58 @@ describe('the verdict, with an adversary', () => { expect(update.params[6]).toBe(0) }) + /** + * 2026-09-23: the reviewer made 16 calls and lost all 16 to ZAI answering + * 1302 and 1305, while fifteen NVIDIA credentials sat idle. Every finished + * bee went to `wait` and the swarm accepted nothing for hours - because a + * transient failure used to END the loop instead of trying the next lane, + * which is the one case where another lane certainly would have answered: a + * rate limit belongs to ONE vendor's account. + */ + it('tries another lane when a provider says "not now"', async () => { + const { pool, queries } = sweepPool(finishedRow()) + const { deps, calls } = fakes({ + llm: async (lane) => { + calls.push({ lane, system: '', message: '' }) + if (lane.poolNumber === 2) + return { + ok: false, + error: '[1305] The service may be temporarily overloaded', + transient: true, + } + return { ok: true, text: allMet } + }, + }) + await reviewFinishedDispatches(pool, deps) + // The busy lane first, because it is the one of another vendor; then the + // other, which answers. Two lanes, and never the same one twice - `pick` + // is deterministic, so without the tried-set the retry would be three + // calls to the endpoint that just refused. + expect(calls.map((c) => c.lane.poolNumber)).toEqual([2, 1]) + // The second lane's answer is what got written, so the review happened + // rather than being skipped for the round. + const cached = queries.filter((q) => + q.sql.includes('SET reviewer_fingerprint'), + ) + expect(cached).toHaveLength(1) + expect(cached[0].params[3]).toBe('glm-5.3') + }) + + it('does not charge a miss when a busy lane is followed by a broken one', async () => { + const { pool, verdictUpdates } = sweepPool(finishedRow()) + const { deps } = fakes({ + llm: async (lane) => + lane.poolNumber === 2 + ? { ok: false, error: '[1302] Rate limit reached', transient: true } + : { ok: false, error: 'model not found', transient: false }, + }) + const reviewed = await reviewFinishedDispatches(pool, deps) + expect(reviewed.acted).toEqual([`#${ISSUE}:wait`]) + // One lane was merely busy, so the review never had its chance - the bee + // had no part in that and must not wear the miss. + expect(verdictUpdates()[0].params[6]).toBe(0) + }) + it.if(present)( 'sends back and spends the budget when the reviewer refutes with a reason', async () => {