Skip to content

fix(auto-routing): harden benchmark admission and daily aliases - #7937

Merged
iscekic merged 11 commits into
mainfrom
fix/benchmark-reliability-daily-aliases
Oct 19, 2026
Merged

iscekic merged 11 commits into
mainfrom
fix/benchmark-reliability-daily-aliases

Conversation

@iscekic

@iscekic iscekic commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Treat exact CLI model lookup failures as unavailable profiles rather than quality scores, and validate admission against the CLI-visible authenticated catalog.
  • Reconcile platform and saved owner aliases daily. Reuse compatible measurements, retain previous serving targets during replacement, and avoid hidden retargeting during dispatch or publication.
  • Preserve active paid calls and fencing during failures, retries, subscription removal, and quota admission.
  • Confirm container response completion before suppressing recovery callbacks, bound delivery waits, and report safe failure categories.
  • Add explicit targeted refresh and repair support without discarding history or invalidating unrelated measurements.
  • Default new or omitted Efficient pools to ~deepseek/deepseek-flash-latest and Frontier pools to ~openai/gpt-sol-latest. Preserve explicit saved pools.

Verification

  • Scoped contract, benchmark-worker, routing-worker, and web type checks on an isolated VM.
  • Scoped lint and contract, benchmark, and routing regression suites.
  • Real Worker HTTP smoke with native local D1, KV, and Secrets Store bindings. Exercised daily-only target changes, cached reuse, replacement publication, failure retention, removal, refresh, and router-specific defaults.
  • Real CLI and Node HTTP container smoke. Verified exact lookup-error classification and recovered outcomes after caller disconnection without provider inference.
  • Actual admin component browser smoke for both default aliases and explicit saved-pool precedence.
  • Independent review findings addressed with concurrency regressions.

Rollout

  • Apply the additive benchmark D1 migration before resuming new worker consumers.
  • Deploy both routing workers together. Existing owner pools enroll during settings or custom-table reads; the authenticated subscription endpoint supports explicit backfill of known owners.
  • Untouched historical Durable Objects are not centrally enumerable. Backfill known owners before relying on daily coverage for their pools.
  • Use targeted repair after deployment for confirmed unavailable-model measurements that were previously marked ready.
  • This PR does not deploy workers, reset production profiles, or change explicitly saved production pools.

@iscekic iscekic self-assigned this Oct 18, 2026
Comment thread services/auto-routing/src/decide.ts Resolved
@kilo-code-bot

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

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The follow-up commit correctly distinguishes an explicitly empty supported_parameters list from an omitted one, rejecting models without advertised tool support while preserving the documented legacy fallback, and adds catalog and queue-admission regression coverage.

Files Reviewed (4 files)
  • services/auto-routing-benchmark/src/platform-models.ts
  • services/auto-routing-benchmark/src/platform-models.test.ts
  • services/auto-routing-benchmark/src/executable-models.test.ts
  • services/auto-routing-benchmark/src/profile-runs.test.ts
Previous Review Summaries (6 snapshots, latest commit 9a36e73)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 9a36e73)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

The incremental commits add poolOwner provenance to the legacy-pool overlap resolver (so an unmigrated organization pool keeps its owner-scoped custom routing table) and extend tests; the resolver logic is correct, and one previously raised admission concern remains open on an unchanged file.

Overview

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

WARNING

File Line Issue
services/auto-routing-benchmark/src/platform-models.ts 43 Carried forward from the existing review thread: catalogModelSupportsTools treats an explicit supported_parameters: [] as tool-capable, so a model with no advertised tool support can pass executable-catalog/queue admission and be sent paid tool-use benchmark tasks. Still present at HEAD; no duplicate inline comment added.
Files Reviewed (5 files)
  • services/auto-routing/src/legacy-pool-overlap.ts
  • services/auto-routing/src/legacy-pool-overlap.test.ts
  • services/auto-routing/src/index.test.ts
  • services/auto-routing-benchmark/src/profile-refresh.test.ts
  • services/auto-routing-benchmark/src/profile-runs.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 04ac3dc)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change aligns the settled-run sweep in dispatchScheduler with the canonical SCHEDULER_UNSETTLED_SQL/hasUnsettledChunks predicate so an active chunk keeps its run unsettled even when a lane failure already exists, and adds a targeted regression test; the new SQL is correct and consistent with the existing policy.

Files Reviewed (2 files)
  • services/auto-routing-benchmark/src/run.ts
  • services/auto-routing-benchmark/src/scheduler-initialization.test.ts

Previous review (commit 81be3e2)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only change since the previous review is an admin test fixture update that adds the run_lane_failures table and the model/variant/rep chunk columns required by the settled-run query; the columns match the production schema and the fixture is not populated by this suite, so there is no behavioral risk.

Files Reviewed (1 file)
  • services/auto-routing-benchmark/src/admin.test.ts

Previous review (commit 50264c3)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change at 50264c3621 aligns dispatch-sweep run settlement with the existing lane-aware finalization logic; the new correlated lane-failure guard is correct, matches the storage-form variant key used elsewhere, and is covered by a targeted regression test.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0
Verification notes
  • dispatchScheduler's settled-run query now ignores pending/queued/active chunks whose (model, variant, rep) lane is recorded in run_lane_failures. This matches the finalization contract in finalizeRunIfComplete/loadLaneAccounting (a lane with a recorded failure is accounted for) and the claim query in scheduler.ts:353, which already refuses to dispatch failed lanes — so no duplicate inference is introduced.
  • The new inner comparison keys on storage-form variant ('' for the null/default variant, NOT NULL on both sides per db-schema.ts), the same key used by failScheduledEntry and recoverChunks, and is served by the run_lane_failures primary-key prefix.
  • A run that still has runnable (non-failed-lane) chunks is not selected, so in-progress work is not finalized prematurely.
  • README addition at services/auto-routing-benchmark/README.md:841 matches the implemented behavior.
Files Reviewed (3 files)
  • services/auto-routing-benchmark/README.md
  • services/auto-routing-benchmark/src/run.ts
  • services/auto-routing-benchmark/src/scheduler-initialization.test.ts

Reviewed at commit 50264c3621465a52ff4b8891936af11d09a671b6. Verification was read-only (no test/build execution); findings are based on source and diff inspection.

Previous review (commit 3bb14a1)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental fix set at 3bb14a178d resolves all three prior findings (run-initialization isolation, mode-only pool statuses, and the unbounded owner-table cache); no new issues were found in the changed lines.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 0
Prior Findings Re-verified as Fixed
  • services/auto-routing-benchmark/src/run.ts - the missing-execution-identity throw is replaced by marking the run and its profiles failed and returning false, and initializeScheduledRuns wraps each run in its own try/catch, so one bad run no longer aborts the pass or skips the second dispatch.
  • services/auto-routing/src/routing-mode.ts - the unchanged-pool fast path now fetches and returns the current pool statuses for a non-null pool, keeping the poolStatuses contract; a failed lookup leaves the saved mode unchanged.
  • services/auto-routing/src/decide.ts - customTableLoaders is now bounded by CUSTOM_ROUTING_TABLE_CACHE_MAX_ENTRIES with oldest-entry eviction, removing the unbounded per-isolate growth.
Files Reviewed (7 files)
  • services/auto-routing-benchmark/README.md
  • services/auto-routing-benchmark/src/run.ts
  • services/auto-routing-benchmark/src/scheduler-initialization.test.ts
  • services/auto-routing/src/decide.ts
  • services/auto-routing/src/index.test.ts
  • services/auto-routing/src/routing-mode.test.ts
  • services/auto-routing/src/routing-mode.ts

Reviewed at commit 3bb14a178d64b912628c907c4f7cfb965e819bc8. Verification was read-only (no test/build execution); findings are based on source and diff inspection.

Previous review (commit 23ec8a4)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The benchmark admission/daily-alias rework is broadly sound (SQL guards, owner-scoped snapshots, and compensation logic all check out), but one changed line in run.ts can abort run initialization for unrelated runs, plus two lower-severity contract/cache concerns.

Overview

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

WARNING

File Line Issue
services/auto-routing-benchmark/src/run.ts 1058 New throw for a missing benchmark_user_id propagates out of initializeScheduledRuns's unguarded loop, aborting initialization of the other selected runs and skipping the second dispatch pass. Contain to the affected run.

SUGGESTION

File Line Issue
services/auto-routing/src/routing-mode.ts 117 Unchanged-pool fast path returns { statuses: [] } for a non-null pool, contradicting the poolStatuses "empty only when configuredPool is null" contract and briefly showing ready entries as "Benchmarking" in the admin card.
services/auto-routing/src/decide.ts 84 Adding ownerType/ownerId to the never-evicted module-level customTableLoaders key changes growth from distinct pools to distinct owner+pool pairs; unbounded per-isolate growth (memory leak).
Files Reviewed (49 files)
  • apps/web/src/app/admin/auto-routing/BenchmarksSection.tsx
  • packages/auto-routing-contracts/src/benchmark.ts - reviewed
  • packages/auto-routing-contracts/src/contracts.test.ts
  • services/auto-routing-benchmark/README.md
  • services/auto-routing-benchmark/container/server.mjs - no issues (checked new response-completion/timer paths for leaks; none found)
  • services/auto-routing-benchmark/migrations/0023_black_black_tom.sql - no issues (additive, nullable add)
  • services/auto-routing-benchmark/migrations/meta/0023_snapshot.json (generated)
  • services/auto-routing-benchmark/migrations/meta/_journal.json
  • services/auto-routing-benchmark/src/admin-profile-refresh.ts
  • services/auto-routing-benchmark/src/admin.test.ts
  • services/auto-routing-benchmark/src/admin.ts
  • services/auto-routing-benchmark/src/benchmark-token.ts
  • services/auto-routing-benchmark/src/cli-runner.test.ts
  • services/auto-routing-benchmark/src/cli-runner.ts
  • services/auto-routing-benchmark/src/container-auth.test.ts
  • services/auto-routing-benchmark/src/db-schema.ts
  • services/auto-routing-benchmark/src/db.ts
  • services/auto-routing-benchmark/src/executable-models.test.ts
  • services/auto-routing-benchmark/src/executable-models.ts
  • services/auto-routing-benchmark/src/index-queue.test.ts
  • services/auto-routing-benchmark/src/index.ts
  • services/auto-routing-benchmark/src/orphan-claim-sql.test.ts
  • services/auto-routing-benchmark/src/owner-alias-sql.test.ts (deleted)
  • services/auto-routing-benchmark/src/platform-models.test.ts
  • services/auto-routing-benchmark/src/platform-models.ts
  • services/auto-routing-benchmark/src/prior-case-reuse-sql.test.ts
  • services/auto-routing-benchmark/src/profile-entry-claims.ts
  • services/auto-routing-benchmark/src/profile-refresh.test.ts
  • services/auto-routing-benchmark/src/profile-refresh.ts
  • services/auto-routing-benchmark/src/profile-runs.test.ts
  • services/auto-routing-benchmark/src/profiles-sql.test.ts
  • services/auto-routing-benchmark/src/profiles.test.ts
  • services/auto-routing-benchmark/src/profiles.ts
  • services/auto-routing-benchmark/src/run-dead-letter.test.ts
  • services/auto-routing-benchmark/src/run-process-job.test.ts
  • services/auto-routing-benchmark/src/run.ts - 1 issue
  • services/auto-routing-benchmark/src/scheduler-initialization.test.ts
  • services/auto-routing-benchmark/src/scheduler.test.ts
  • services/auto-routing-benchmark/src/scheduler.ts
  • services/auto-routing-benchmark/src/subscriptions.test.ts
  • services/auto-routing-benchmark/src/subscriptions.ts
  • services/auto-routing-benchmark/src/system-optimal.test.ts
  • services/auto-routing-benchmark/src/system-optimal.ts
  • services/auto-routing/src/admin-routing-settings.ts
  • services/auto-routing/src/benchmark-origin.ts
  • services/auto-routing/src/decide.ts - 1 issue
  • services/auto-routing/src/index.test.ts
  • services/auto-routing/src/routing-mode.test.ts
  • services/auto-routing/src/routing-mode.ts - 1 issue

Reviewed at commit 23ec8a47eff610c95cb9f6d38538fff48d2fa319. Verification was read-only (no test/build execution); findings are based on source and diff inspection.

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash-1017 · Input: 66.4K · Output: 8.9K · Cached: 1.4M

Review guidance: REVIEW.md from base branch main

@iscekic

iscekic commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Added recovery for runs whose remaining pending chunks belong to failed lanes. Active claims still keep the run open until they settle. The regression failed before the fix and passes after it. A standalone scheduler smoke confirmed per-entry settlement without duplicate inference. CI passes, and the current review has no unresolved threads. This change does not alter production until merge and deployment.

…lity-daily-aliases

# Conflicts:
#	services/auto-routing-benchmark/migrations/meta/0023_snapshot.json
#	services/auto-routing-benchmark/migrations/meta/_journal.json
#	services/auto-routing-benchmark/src/admin.ts
#	services/auto-routing-benchmark/src/cli-runner.test.ts
#	services/auto-routing-benchmark/src/db.ts
#	services/auto-routing-benchmark/src/index-queue.test.ts
#	services/auto-routing-benchmark/src/platform-models.ts
#	services/auto-routing-benchmark/src/profiles-sql.test.ts
#	services/auto-routing-benchmark/src/profiles.ts
#	services/auto-routing-benchmark/src/run.ts
#	services/auto-routing-benchmark/src/scheduler.test.ts
#	services/auto-routing/src/admin-routing-settings.ts
#	services/auto-routing/src/decide.ts
#	services/auto-routing/src/index.test.ts
#	services/auto-routing/src/routing-mode.ts
…e tables

Unmigrated legacy pools that reach the platform decision path now carry their owner, so the owner-scoped custom routing table and its subscription sync still apply after the custom-router merge. Update the benchmark tests for the merged re-measure queue and reuse parameters.
An explicit empty supported_parameters list now means the model advertises no tool support, so executable-catalog and queue admission reject it. Only an omitted list keeps the legacy fallback.
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