Repository navigation
feat: enrich auto-router model capabilities from lowest common denominator - #44
Rahulsharma0810 wants to merge 1 commit into
Conversation
…nator Resolves Alph4d0g#43 — OmniRoute's zero-config `auto` router model isn't listed in /api/combos like user-defined combos, so it previously had no computed context window / capabilities, requiring a manual modelMetadata override. Adds isAutoModel()/enrichAutoModel() in src/omniroute-combos.ts, following the same lowest-common-denominator approach already used for combo enrichment: contextWindow/maxTokens are set to the minimum across all other fetched models, capability booleans use AND (vision/tools/streaming/ temperature/attachment) or OR (reasoning) as appropriate. Existing values (from the API or user overrides) are never overwritten. 5 new tests in test/auto-model.test.mjs, 71/71 total passing.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ...autoModel, | ||
| ...(computedContextWindow !== undefined ? { contextWindow: computedContextWindow } : {}), | ||
| ...(computedMaxTokens !== undefined ? { maxTokens: computedMaxTokens } : {}), | ||
| supportsVision: autoModel.supportsVision ?? others.every((m) => m.supportsVision === true), |
There was a problem hiding this comment.
🔥 The Roast: The "lowest common denominator" AND logic treats undefined as false. Every other model in your catalog that merely lacks a resolved capability — e.g. a custom provider model not found in models.dev, which leaves supportsVision/supportsTools/etc. undefined — poisons the AND into false. So adding any single model with unknown capabilities silently flips vision/tools/streaming off for the auto router. Meanwhile the combo path this claims to mirror (calculateLowestCommonCapabilities in models-dev.ts:444) explicitly guards with hasTemperatureMetadata/hasReasoningMetadata so missing metadata doesn't drag results down. This isn't even consistent with its own inspiration.
🩹 The Fix: Ignore models whose capability is unknown when computing the AND, e.g. const visionKnown = others.filter(m => m.supportsVision !== undefined); supportsVision: autoModel.supportsVision ?? (visionKnown.length ? visionKnown.every(m => m.supportsVision === true) : undefined). Apply the same pattern to the other five booleans (reasoning should stay OR, also ignoring unknown). This matches the combo behavior's "don't punish missing metadata" intent.
📏 Severity: warning
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| * "other models" pool already has resolved capabilities where possible. | ||
| */ | ||
| export function enrichAutoModel(models: OmniRouteModel[]): OmniRouteModel[] { | ||
| const autoIndex = models.findIndex(isAutoModel); |
There was a problem hiding this comment.
🔥 The Roast: findIndex(isAutoModel) enriches only the first auto model, then the others filter excludes every other auto model via !isAutoModel(model). So if the catalog ever contains two auto routers (e.g. auto and omniroute/auto, or two providers' auto), the first wins and the rest are quietly left as empty shells — excluded from the pool and never enriched themselves. It's a coin-flip which one gets capabilities.
🩹 The Fix: Pick a single canonical auto model (prefer bare auto over provider/auto, or merge all matched auto models) instead of enriching only findIndex's first hit and discarding the rest.
📏 Severity: suggestion
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| OmniRoute itself ever starts returning explicit capabilities for `auto` (or you set them via | ||
| `modelMetadata` overrides), those take precedence and this calculation is skipped. | ||
|
|
||
| ### API Mode |
There was a problem hiding this comment.
🔥 The Roast: The new Auto-Router sectiongot a little too eager and stamped ### API Mode twice. Two identical headings now sit back-to-back like a copy-paste that forgot to press delete. Anyone following the TOC will think they've fallen into a documentation time loop.
🩹 The Fix: Remove the duplicate ### API Mode heading this PR added (keep the original one that introduces the API Modes content).
📏 Severity: nitpick
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review Roast 🔥Verdict: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The overall approach is genuinely clean — reusing 💀 Worst part: The capability AND logic poisons itself on 📊 Overall: Like a soufflé that rose beautifully but collapsed because one ingredient was "unknown" — the recipe's right, but the Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by free · Input: 80.1K · Output: 12.1K · Cached: 187.1K |
2 participants
Resolves #43.
OmniRoute's zero-config
autorouter model isn't listed in/api/comboslike user-defined combos, so it previously surfaced with no computed context window or capabilities — requiring a manualmodelMetadataoverride, exactly the kind of hardcoded workaround #43 is about.Changes
src/omniroute-combos.ts: addsisAutoModel(model)andenrichAutoModel(models). When anautomodel is present and missingcontextWindow/maxTokens, computes them as the minimum across all other fetched models (same lowest-common-denominator approach as combo enrichment). Capability booleans: AND for vision/tools/streaming/temperature/attachment, OR for reasoning. Never overrides values already set (from the API or user overrides) — only fills gaps.src/models.ts: wiresenrichAutoModelin as the final step ofenrichModelMetadata(), afterenrichComboModels.test/auto-model.test.mjs: 5 new test cases covering matching, LCD computation, no-op when no auto model, non-override behavior, and partial-fill behavior.Test plan
npm run build— 0 errorsnpm test— 71/71 passing (66 pre-existing + 5 new)