feat(auto-routing): owner-configurable Efficient model pools - #4846
Open
iscekic wants to merge 20 commits into
Open
feat(auto-routing): owner-configurable Efficient model pools#4846iscekic wants to merge 20 commits into
iscekic wants to merge 20 commits into
Conversation
A bare session target intermittently resolves to a same-named window and fails with "index 0 in use". Use the session: colon form and record the learning.
… contracts Add Pool entry / Efficient model pool schemas with a collision-safe poolEntryKey, optional canonical variant on routing candidates and benchmark decisions with legacy reasoningEffort compatibility, the sparse CustomRoutingTableSchema, benchmark profile registry wire types, and owner pool settings contracts. Extend ADR 0002 with owner-pool invariants and record Auto Routing scope and canonical terms in CONTEXT.md.
…results Composite (model, variant) primary keys across run_models, model_summaries, and case_results with a generated migration that backfills variant from reasoning_effort; exact-pair queue messages, container lanes, carry matching, and summaries; routing-table candidates self-describe their variant. The published platform routing table JSON is unchanged in shape. Bounded contract extensions: optional variant on BenchmarkModelSummary and BenchmarkDeciderModel with legacy reasoningEffort compatibility.
…-e6e3 # Conflicts: # .kilo_workflow/dispatch-role.sh
When kilo-auto/efficient decisions carry a canonical variant, apply the full getModelVariants settings (reasoning + verbosity). Missing catalog keys fall back to balanced; legacy effort-only decisions stay unchanged.
Store Efficient model pools on the routing-mode DO, admit profiles before settings PUT, and resolve custom tables with exact-pair sticky and custom-only isActive fail-closed.
…atomic admission Add benchmark_profiles and profile_request_events D1 tables, one all-or-nothing admission batch with 10-per-owner-per-24h quota and global dedupe, concurrency-safe guarded upserts and conditional charges, and the internal register/status endpoints.
Cover the charged-event, register-upsert, and status-insert builders against node:sqlite so column-count or guard regressions fail the suite behind the drizzle mock.
Validate Efficient pool entries against the owner's effective managed catalog, proxy worker settings, annotate unavailable saved entries, and preserve benchmark quota 429 retryAt for the UI.
…, sparse custom tables Wire startRun(purpose, entries), terminal profile transitions, pending-batch drain on the existing one-active-decider slot with platform priority, POST /admin/custom-routing-table with provenance-bound assembly, and ops docs. Platform artifact path unchanged.
Extend AutoRoutingModeCard to manage mode + 1-10 pool entries via /api/auto-routing/settings, with benchmark status, polling, retry, and org billing-manager edit permission isolation.
…-e6e3 # Conflicts: # .kilo_workflow/dispatch-role.sh
A mixed save (mode set + pool cleared, or the reverse) fanned out to independent multi-key put and delete ops via Promise.all; one succeeding while the other threw diverged mode and pool despite the 'commit or fail together' contract. Wrap the mutation in ctx.storage.transaction and cover rollback with an honest transactional storage stub.
Production promotes web before workers, so every merge runs new web against workers that lack /admin/routing-settings (JSON 404). Load now falls back to the legacy mode route and reports poolSupported: false, hiding pool controls instead of failing the card; mode saves from that state use the legacy mode endpoint, which never touches pool keys at any worker version. A settings PUT that 404s (rollback mid-session from a supported UI) answers a retryable 503 rather than silently mis-saving.
…gration truth ADR 0002 and the benchmark README now match the shipped behavior: owner mode+pool commit in one DO storage transaction; the web-before-worker deploy window serves the legacy-synthesis fallback (poolSupported: false, mode-only legacy saves, retryable 503 on settings PUT 404); admission's DO-after-admit semantics are written down; and the rollback section stops claiming the D1 schema is additive now that 0005 rebuilds profile tables with backfill.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThis incremental review confirms the prior admin-client/route.ts graceful-502 findings are fixed (JSON parse errors and schema-mismatched 2xx bodies now safely degrade to Files Reviewed (4 files)
Previous Review Summary (commit 608e3d5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 608e3d5)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryThe most notable risk is in Overview
Issue Details (click to expand)WARNING
Files Reviewed (60 files)
Reviewed by claude-sonnet-5 · Input: 20 · Output: 5K · Cached: 419.7K Review guidance: REVIEW.md from base branch |
Verifier-authored from the owner-pool E2E run: deterministic benchmark profile D1 fixture facts (engine identity, switch_cost_factor floor, config prerequisites, provenance binding, uncharged stale re-admission) and local gateway/auth facts (org-context bearer path, balance top-up counter, fake-login cookie behavior, org deny-list fixture lever).
fetchAutoRoutingSettingsAdmin parsed success bodies with .parse(), so an unexpected-shape or non-JSON 2xx (old/rolled-back worker, proxy error page) threw through the route and surfaced as an unhandled 500 instead of the intended graceful 502. safeParse plus a guarded .json() now return the 502 result the route already maps, making the deploy-order safety net reachable. Addresses two Kilobot review threads on PR #4846.
Contributor
Author
|
(bot) @kilocode-bot please review the latest head 3239b9d — it addresses your two malformed-response findings from the previous head; the third thread was replied to with rationale and resolved. |
Contributor
Follow-up review at
|
Contributor
|
Review already posted at #4846 (comment) — both findings confirmed fixed, third thread confirmed unchanged/out of scope. No further action needed from my side; the non-blocking observations (404-body edge case, missing logging, |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What.
kilo-auto/efficientcan now be customized per owner: a personal user or an organization configures 1–10 exact(managed model, canonical catalog variant)pairs on the existing Auto routing card. Missing exact pairs are benchmarked on demand through the existing benchmark Worker; routing then decides only among ready, compatible entries and keeps the balanced fallback whenever none are usable.Why. Owners want the Efficient auto model constrained to the exact models and variants they trust (same-model
xhighvsmaxstay distinct end-to-end) without losing the platform default when their pool cannot serve.How.
packages/auto-routing-contracts), ADR 0002 updates, canonical terms in CONTEXT.md.reasoningEffortdecisions stay parseable.poolSupported: falsehides pool controls; mode saves stay mode-only at every worker version; a mid-session rollback answers a retryable 503 instead of mis-saving). Malformed worker settings responses degrade to a graceful 502.Verification
claude-haiku-4.5; clearing restored personalclaude-sonnet-4.5, instrumented proof; mode and pool saves independent).xhigh+maxpersist and render as distinct rows and distinct D1 PKs.retryAtcopy, atomically (zero partial rows).kilo-auto/efficientcalls only): seeded ready pool constrained the decision to pool entries (custom table, 18 routes, threshold-winner xhigh); stale profiles → balanced fallback with uncharged re-admission; no pool → default platform path.Visual Changes
Reviewer Notes
getModelVariants); admission vs save atomicity across the two workers (quota 429 withretryAt, all-or-nothing D1 admission, one DO storage transaction); the deploy-order fallback matrix (old web + new worker, new web + old worker, rollback mid-session).Follow-ups noted during review (non-blocking)
createWorkerAdminFetchkeeps the throw-on-malformed convention for the sibling admin routes; the settings client now sets the safer safeParse→502 precedent those can adopt.502rather than the upstream status; genuine worker error responses are always JSON, so this only affects edge cases that surface as the same retryable load/save error either way.