Skip to content

fix(auto-routing-benchmark): keep transient provider failures out of measurements - #9352

Open
iscekic wants to merge 7 commits into
mainfrom
fix/benchmark-run-failures
Open

iscekic wants to merge 7 commits into
mainfrom
fix/benchmark-run-failures

Conversation

@iscekic

@iscekic iscekic commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

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

Model (efforts) Failure Cause Change in this PR
rekaai/reka-edge (thinking) Every case failed; each rejection was billed The CLI reserves the catalog's completion budget on every request. That leaves too little of the 16,384-token window for the harness prompt. The profile still became ready with zero accuracy. The drain requires at least 20,000 input tokens after the CLI completion reservation. A smaller pair fails with a context-window reason before any paid call.
poolside/laguna-xs-2.1:free (instant, thinking) Every lane died; the profiles failed The gateway per-account free-model limit throttled the internal benchmark bot. Each throttle deferral consumed one of the six chunk attempts. The gateway exempts the benchmark bot from the free-model account limit. A throttled chunk refunds its attempt and counts against a separate throttle budget.
xingchen-agi/xing4.0-29b-a4b:free (instant, thinking) Every lane died; the profiles failed The same free-model account limit, plus upstream provider 429s. Same as above.
poolside/laguna-xs-2.1:free (instant) One case was graded as a model failure A 429 ended a multi-step run after partial text. The throttle check required empty text. An unbilled run that ends on a throttle error event is requeued.
xingchen-agi/xing4.0-29b-a4b:free (instant, thinking) A few cases were graded as model failures One-off Internal Server Error and Authorization temporarily unavailable errors, and upstream 429s after partial steps. Unbilled provider outages and throttles that end a run on an error event are requeued.
moonshotai/kimi-k3.1-pro (high), xingchen-agi/xing4.0-29b-a4b:free A few cases were graded as model failures The CLI process was killed by a signal before it emitted any event. That result is retried once, like an empty completion.
anthropic/claude-fable-5 (low, medium, high, xhigh) A small share of cases failed The provider content filter blocks the same cases at every effort. None: users see the same block, so the score is valid.
poolside/laguna-xs-2.1:free (instant, thinking) Two cases failed at every repetition Both cases expect a bare < answer, and the provider returns an empty response. None: provider output handling.
xingchen-agi/xing4.0-29b-a4b:free (instant, thinking) A few long-context cases failed The upstream provider rejects those requests as invalid. None: upstream capability limit.
qwen/qwen3.8-2.4t-a95b (xhigh), moonshotai/kimi-k3.1-pro (high), both free models A few cases timed out The model did not finish within the case timeout. None: a timeout is a valid measurement.

openai/gpt-5 (minimal) and moonshotai/kimi-k3.1-pro (none) had no failures.

Changes

  • platform-models.ts: benchmarkPairUnavailableReason computes the input room after the CLI completion reservation, the way the CLI derives it from the gateway catalog. drainQueue stores the specific reason on the failed profile, for both the platform and the owner queue. catalogSupportsBenchmarkPair keeps only the tool and variant rules, so owner registration admits the pair and the drain reports the reason.
  • cli-runner.ts: isUnbilledTransientProviderResult covers throttles and provider outages, including runs that end on the error event after partial text. isKilledBeforeEventsResult marks a signal kill before the first event.
  • scheduler.ts and migration 0030: the new scheduler_chunks.throttle_deferrals column. deferThrottledChunk refunds 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 a benchmarkUserId override, keep the limit.

Verification

  • A replay of every failed case from the run through the new classifiers: only the throttle, outage, and pre-event kill results change outcome. Content-filter, timeout, context-overflow, and invalid-request results stay graded.
  • The admission rule against the live catalog: rekaai/reka-edge is 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 new llm-proxy cases pass.
  • No end-to-end run of the changed worker: a decider pass needs Cloudflare Containers, the CLI image, and paid gateway calls. The replay of the run's failed cases and the D1-backed scheduler tests cover the changed paths. The first production run after the deploy is the end-to-end check.

Production prerequisites

  • D1 migration 0030 adds one column with a default. The guarded benchmark deploy applies migrations before it deploys the worker.
  • No environment variables, secrets, or feature flags.

…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.
@kilo-code-bot

kilo-code-bot Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change runs both throttle patterns on the ANSI-stripped stderrTail, so a throttle split by terminal control codes is now detected by hasThrottleDiagnostic and by the partial-text branch of isUnbilledTransientProviderResult; the added Vitest case covers that exact shape.

Assumptions: [INFERENCE] The referenced CLI sources (packages/opencode/src/provider/transform.ts and packages/kilo-gateway/src/api/models.ts) live outside this repository, so the updated CLI_OUTPUT_TOKEN_MAX doc comment was checked for consistency with the code and README only. Tests were not executed in this read-only review.

Files Reviewed (3 files)
  • services/auto-routing-benchmark/src/cli-runner.ts
  • services/auto-routing-benchmark/src/platform-models.ts
  • services/auto-routing-benchmark/src/run-process-job.test.ts

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: catalogModelInputRoom subtracts the CLI completion reservation (min(top_provider.max_completion_tokens || max_completion_tokens || 20% of window, 32,000)) so rekaai/reka-edge is rejected with the context-window reason while the previously admitted pairs still pass, and catalogSupportsBenchmarkPair now carries only tool/variant admission as the README and tests describe.

Assumptions: [INFERENCE] The OUTPUT_TOKEN_MAX = 32_000 cap and the 20% fallback are the CLI's actual catalog-derived reservation; that code lives outside this repository, so the formula was verified only against the README, tests, and catalog schema. Tests were not executed in this read-only review.

Files Reviewed (3 files)
  • services/auto-routing-benchmark/README.md
  • services/auto-routing-benchmark/src/platform-models.test.ts
  • services/auto-routing-benchmark/src/platform-models.ts

Previous review (commit 885ac15)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change since a76eb50 only tightens the throttle-deferral budget, its documentation, and the exemption comment in llm-proxy.ts; the new budget of 16 matches the 15–300 second backoff (~63 minutes), the six-attempt fallback path in run.ts:1703, and the DEFAULT_BENCHMARK_USER_ID comparison.

Assumptions: [INFERENCE] Tests were not executed in this read-only review; behavior was verified against current code (schedulerBackoffSeconds, deferThrottledChunk, drainQueue throttle branch, DEFAULT_BENCHMARK_USER_ID in auto-routing-contracts).

Files Reviewed (3 files)
  • packages/web-shared/src/lib/ai-gateway/handlers/llm-proxy.ts - comment update only, matches exemption code
  • services/auto-routing-benchmark/README.md - budget and exemption wording matches implementation
  • services/auto-routing-benchmark/src/scheduler.ts - constant 48 -> 16 consistent with backoff and fallback

Previous review (commit a76eb50)

Status: No Issues Found | Recommendation: Merge

Executive Summary

Incremental review of the fixes since b585920 confirms both prior findings are resolved: isUnbilledTransientProviderResult now returns false for a timed-out run, and platformRegistryEntries keeps below-floor models so their pending rows reach drainQueue and fail with CATALOG_CONTEXT_TOO_SMALL_REASON. No new issues found in the changed code.

Assumptions: [INFERENCE] Tests were not executed in this read-only review; behavior was verified against the changed code, its callers (run.ts drain/grade paths, subscriptions.ts resolution and promotion), and the new tests.

Files Reviewed (5 files)
  • services/auto-routing-benchmark/src/cli-runner.ts - prior issue resolved, no new issues
  • services/auto-routing-benchmark/src/platform-models.ts - prior issue resolved, no new issues
  • services/auto-routing-benchmark/src/platform-models.test.ts - no issues
  • services/auto-routing-benchmark/src/profile-runs.test.ts - no issues
  • services/auto-routing-benchmark/src/run-process-job.test.ts - no issues

Previous review (commit b585920)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The throttle/outage requeue path and the benchmark-bot rate-limit exemption are sound, but the new transient classifier ignores timedOut, so a timed-out unbilled run can be requeued instead of graded, and the new context-window reason never reaches platform-queue profiles.

Assumptions: [INFERENCE] No database, worker, or gateway process was executed in this read-only review; behavior was verified against the changed code and its callers. Generated migration snapshot/journal files were excluded from line review.

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 1
Issue Details (click to expand)

WARNING

File Line Issue
services/auto-routing-benchmark/src/cli-runner.ts 100 isUnbilledTransientProviderResult does not exclude timedOut, so a run killed by the case timeout after an error event with a throttle/5xx diagnostic is requeued as a throttle instead of graded, contradicting the stated timeout-is-measured policy.

SUGGESTION

File Line Issue
services/auto-routing-benchmark/src/platform-models.ts 119 platformRegistryEntries drops below-floor models before syncPlatformRegistryRows clears platform_requested, so platform-queue profiles never reach the new CATALOG_CONTEXT_TOO_SMALL_REASON branch in drainQueue.
Files Reviewed (16 files)
  • packages/web-shared/src/lib/ai-gateway/handlers/llm-proxy.ts - no issues
  • packages/web-shared/src/lib/ai-gateway/handlers/llm-proxy.test.ts - no issues
  • services/auto-routing-benchmark/README.md - no issues
  • services/auto-routing-benchmark/migrations/0030_simple_xorn.sql - no issues
  • services/auto-routing-benchmark/migrations/meta/0030_snapshot.json - generated, no line review
  • services/auto-routing-benchmark/migrations/meta/_journal.json - generated, no line review
  • services/auto-routing-benchmark/src/cli-runner.ts - 1 issue
  • services/auto-routing-benchmark/src/db-schema.ts - no issues
  • services/auto-routing-benchmark/src/platform-models.ts - 1 issue
  • services/auto-routing-benchmark/src/platform-models.test.ts - no issues
  • services/auto-routing-benchmark/src/profile-runs.test.ts - no issues
  • services/auto-routing-benchmark/src/run-dead-letter.test.ts - no issues
  • services/auto-routing-benchmark/src/run-process-job.test.ts - no issues
  • services/auto-routing-benchmark/src/run.ts - no issues
  • services/auto-routing-benchmark/src/scheduler.test.ts - no issues
  • services/auto-routing-benchmark/src/scheduler.ts - no issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-flash-latest · Input: 33K · Output: 4.4K · Cached: 350K

Review guidance: REVIEW.md from base branch main

…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 eshurakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot - Light review: no critical issues found. Two minor, actionable notes.

  1. packages/web-shared/src/lib/ai-gateway/handlers/llm-proxy.ts:692 exempts only DEFAULT_BENCHMARK_USER_ID. The benchmark supports a benchmarkUserId override (BenchmarkConfigSchema.benchmarkUserId, resolved by resolveBenchmarkIdentity), and the worker mints the CLI token for that override (run.ts fetchBenchmarkUserToken(env, benchmarkUserId, benchmarkOrgId)). A run configured under another bot-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.

  2. SCHEDULER_MAX_THROTTLE_DEFERRALS = 48 is per chunk and schedulerBackoffSeconds caps 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 be running per (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.
@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Both review notes are addressed in 885ac15.

  1. The gateway cannot read the benchmark config, and exempting every internal bot would also lift the limit for unrelated bots. The code comment and the README now state that only the default benchmark user is exempt and that a benchmarkUserId override keeps the limit.
  2. The throttle budget is now 16 deferrals, about an hour of backoff per chunk. A persistent throttle then falls back to the ordinary attempt limit and fails its lane. Lanes run in parallel, so the extra wait for a genuine failure is bounded by about an hour per run.

…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 eshurakov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bot - Approved with minor, non-blocking notes:

  1. services/auto-routing-benchmark/src/cli-runner.ts: isUnbilledTransientProviderResult runs THROTTLE_DIAGNOSTIC against the raw stderrTail but UNAVAILABLE_DIAGNOSTIC against the ANSI-stripped text. Stripping once and matching both against the stripped string avoids an escape sequence between rate/limit or inside throttl silently turning a throttle into a graded model failure.

  2. services/auto-routing-benchmark/src/platform-models.ts: CLI_OUTPUT_TOKEN_MAX, the Math.ceil(context_length * 0.2) completion fallback, and MIN_BENCHMARK_INPUT_TOKENS duplicate 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.

  3. BenchmarkModelCatalogSchema uses .catch(null) for context_length/max_completion_tokens, so a malformed value is treated as unknown and admitted (benchmarkPairUnavailableReason returns null). Consider logging malformed catalog entries so a bad context length cannot silently bypass the input-room floor.

  4. UNAVAILABLE_DIAGNOSTIC scans the whole stderr for phrases like internal 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.
@iscekic

iscekic commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Review notes on 9508ace, handled in eab438d:

  1. Fixed. Both patterns now run on the ANSI-stripped diagnostic, and a test covers an escape sequence inside "rate limit".
  2. Fixed. The mirrored output cap and completion fallback name their source files in the kilo CLI.
  3. No change. An unknown or malformed context length is admitted on purpose, which matches how the catalog schema treats other optional fields. A pair that still overflows is measured, as before this PR.
  4. No change. The CLI prints its final error last, and the throttle budget bounds any wrong requeue to about an hour before the lane falls back to ordinary attempts.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants