Repository navigation
Conversation
…measurements Admission requires a catalog context window of at least 32,768 tokens and fails smaller models with a context-window reason. Unbilled throttles and provider outages that end a run on an error event are requeued, and a CLI killed by a signal before its first event is retried once. A throttled chunk refunds its claim attempt and counts against a separate throttle-deferral budget.
… limit The internal benchmark user measures every case of each free model, so the per-account free-model rate limit does not apply to it.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change runs both throttle patterns on the ANSI-stripped Assumptions: Files Reviewed (3 files)
Fix these issues in Kilo Cloud Previous Review Summaries (4 snapshots, latest commit 9508ace)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9508ace)Status: No Issues Found | Recommendation: Merge The incremental change correctly moves the context-window check from registration into the drain: Assumptions: Files Reviewed (3 files)
Previous review (commit 885ac15)Status: No Issues Found | Recommendation: Merge Executive SummaryThe incremental change since Assumptions: Files Reviewed (3 files)
Previous review (commit a76eb50)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of the fixes since Assumptions: Files Reviewed (5 files)
Previous review (commit b585920)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe throttle/outage requeue path and the benchmark-bot rate-limit exemption are sound, but the new transient classifier ignores Assumptions: Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (16 files)
Reviewed by deepseek-flash-latest · Input: 33K · Output: 4.4K · Cached: 350K Review guidance: REVIEW.md from base branch |
…ext reason on platform rows A timed-out run stays a measured failure. Platform registry entries keep below-floor models, so the drain fails them with the context-window reason.
eshurakov
left a comment
There was a problem hiding this comment.
Bot - Light review: no critical issues found. Two minor, actionable notes.
-
packages/web-shared/src/lib/ai-gateway/handlers/llm-proxy.ts:692exempts onlyDEFAULT_BENCHMARK_USER_ID. The benchmark supports abenchmarkUserIdoverride (BenchmarkConfigSchema.benchmarkUserId, resolved byresolveBenchmarkIdentity), and the worker mints the CLI token for that override (run.tsfetchBenchmarkUserToken(env, benchmarkUserId, benchmarkOrgId)). A run configured under anotherbot-internal-*user still hits the free-model account limit, so the lane-loss failure this PR fixes remains for that configuration. Either exempt the effective/configured id (for example via a shared allowlist) or document that an override loses the exemption. -
SCHEDULER_MAX_THROTTLE_DEFERRALS = 48is per chunk andschedulerBackoffSecondscaps at 5 minutes, so one persistently throttled chunk can defer for several hours. A lane runs its chunks sequentially, and only one decider run can berunningper(router, kind, purpose), so a model that 429s for a whole profile can hold the benchmark queue for many hours before the chunk falls back to ordinary attempts. Worth confirming a per-lane or per-run throttle budget is intended rather than a per-chunk one.
…out an hour A persistently throttled chunk falls back to ordinary attempts after 16 deferrals. The gateway exemption covers the default benchmark user only.
|
bot: Both review notes are addressed in 885ac15.
|
…ion reservation The drain fails a pair whose catalog window leaves fewer than 20,000 input tokens after the completion budget the CLI reserves. Registration admits the pair, so owner and platform profiles both record the context-window reason.
eshurakov
left a comment
There was a problem hiding this comment.
Bot - Approved with minor, non-blocking notes:
-
services/auto-routing-benchmark/src/cli-runner.ts:isUnbilledTransientProviderResultrunsTHROTTLE_DIAGNOSTICagainst the rawstderrTailbutUNAVAILABLE_DIAGNOSTICagainst the ANSI-stripped text. Stripping once and matching both against the stripped string avoids an escape sequence betweenrate/limitor insidethrottlsilently turning a throttle into a graded model failure. -
services/auto-routing-benchmark/src/platform-models.ts:CLI_OUTPUT_TOKEN_MAX, theMath.ceil(context_length * 0.2)completion fallback, andMIN_BENCHMARK_INPUT_TOKENSduplicate CLI/harness internals with no shared source. If the CLI reservation or harness prompt changes, admission drifts silently; reference the CLI source/commit for these values, or cross-check them against a CLI constant in a test. -
BenchmarkModelCatalogSchemauses.catch(null)forcontext_length/max_completion_tokens, so a malformed value is treated as unknown and admitted (benchmarkPairUnavailableReasonreturns null). Consider logging malformed catalog entries so a bad context length cannot silently bypass the input-room floor. -
UNAVAILABLE_DIAGNOSTICscans the whole stderr for phrases likeinternal server error/HTTP 5xx; with a non-zero exit and a final error event, an unrelated 5xx line from an earlier step can requeue a genuinely failed case. Matching only the final error event/line would tighten this (bounded by the deferral budget, so low impact).
…ped stderr Both transient-provider patterns run on the stripped diagnostic. The mirrored CLI output values name their source files.
|
bot: Review notes on 9508ace, handled in eab438d:
|
Summary
The latest Efficient Router benchmark run completed, but some models failed or lost measurement lanes. This PR fixes each failure that comes from the benchmark or the gateway. Failures that come from model or provider behavior stay measured.
Failures in the run, by model
rekaai/reka-edge(thinking)readywith zero accuracy.poolside/laguna-xs-2.1:free(instant, thinking)xingchen-agi/xing4.0-29b-a4b:free(instant, thinking)poolside/laguna-xs-2.1:free(instant)xingchen-agi/xing4.0-29b-a4b:free(instant, thinking)Internal Server ErrorandAuthorization temporarily unavailableerrors, and upstream 429s after partial steps.moonshotai/kimi-k3.1-pro(high),xingchen-agi/xing4.0-29b-a4b:freeanthropic/claude-fable-5(low, medium, high, xhigh)poolside/laguna-xs-2.1:free(instant, thinking)<answer, and the provider returns an empty response.xingchen-agi/xing4.0-29b-a4b:free(instant, thinking)qwen/qwen3.8-2.4t-a95b(xhigh),moonshotai/kimi-k3.1-pro(high), both free modelsopenai/gpt-5(minimal) andmoonshotai/kimi-k3.1-pro(none) had no failures.Changes
platform-models.ts:benchmarkPairUnavailableReasoncomputes the input room after the CLI completion reservation, the way the CLI derives it from the gateway catalog.drainQueuestores the specific reason on the failed profile, for both the platform and the owner queue.catalogSupportsBenchmarkPairkeeps only the tool and variant rules, so owner registration admits the pair and the drain reports the reason.cli-runner.ts:isUnbilledTransientProviderResultcovers throttles and provider outages, including runs that end on the error event after partial text.isKilledBeforeEventsResultmarks a signal kill before the first event.scheduler.tsand migration0030: the newscheduler_chunks.throttle_deferralscolumn.deferThrottledChunkrefunds the claim attempt. The budget of 16 deferrals gives a throttled chunk about an hour; after that, the chunk falls back to the ordinary attempt limit, so a persistent throttle still fails its lane.llm-proxy.ts: the free-model account limit skips the default benchmark user. Other accounts, including other internal bots and abenchmarkUserIdoverride, keep the limit.Verification
rekaai/reka-edgeis rejected with the context-window reason, and the other models in the run are admitted. No model in a saved platform pool is rejected.auto-routing-benchmark: typecheck, lint, and the full Vitest suite pass, plus the deploy-script tests.auto-routing-contracts: typecheck, lint, and tests pass.web-shared: typecheck passes. The newllm-proxycases pass.Production prerequisites
0030adds one column with a default. The guarded benchmark deploy applies migrations before it deploys the worker.