-
Notifications
You must be signed in to change notification settings - Fork 118
feat: auto model routing for SkillFlows #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| // Classifies each SkillFlow step by complexity and resolves the model it should | ||
| // run on: lightweight tasks (summarize/extract/classify/transform) to a cheap | ||
| // model, reasoning-heavy tasks to the configured reasoning model. Explicit | ||
| // per-step / per-skill settings win; anything unresolved falls back to primary. | ||
|
|
||
| export type ModelTier = "lightweight" | "reasoning"; | ||
|
|
||
| export interface RoutingConfig { | ||
| /** Defaults to true when a routing block is present. */ | ||
| enabled?: boolean; | ||
| /** Model id for lightweight tasks, e.g. "openai:gpt-4o-mini". */ | ||
| lightweight?: string; | ||
| /** Model id for reasoning tasks, e.g. "openai:gpt-4o". */ | ||
| reasoning?: string; | ||
| /** Classification overrides — first matching rule wins. */ | ||
| rules?: Array<{ tier: ModelTier; match: string[] }>; | ||
| } | ||
|
|
||
| export interface RouteInput { | ||
| /** Explicit per-step model (highest priority); alias or model id. */ | ||
| stepModel?: string; | ||
| /** Per-skill default from SKILL.md frontmatter; alias or model id. */ | ||
| skillModel?: string; | ||
| /** Text used to classify the task (skill name + step prompt). */ | ||
| classifyText: string; | ||
| routing?: RoutingConfig; | ||
| /** The agent's preferred model — the ultimate fallback. */ | ||
| primaryModel?: string; | ||
| } | ||
|
|
||
| export interface RouteResult { | ||
| /** Resolved "provider:model" (undefined → let the runtime decide). */ | ||
| model?: string; | ||
| /** Tier, when the model came from automatic classification. */ | ||
| tier: ModelTier | null; | ||
| source: "step" | "skill" | "auto" | "fallback"; | ||
| } | ||
|
|
||
| // Matched against word starts, so "summarize"/"summary"/"summarization" all hit | ||
| // "summ" without "already" matching "read". | ||
| const DEFAULT_LIGHTWEIGHT = [ | ||
| "summ", "extract", "classif", "transform", "format", "convert", | ||
| "parse", "fetch", "read", "load", "lookup", "normaliz", "translat", | ||
| "rephrase", "rewrite", "tag", "label", "render", | ||
| ]; | ||
| const DEFAULT_REASONING = [ | ||
| "search", "analy", "plan", "decid", "decision", "orchestrat", "solve", | ||
| "reason", "validat", "evaluat", "review", "audit", "diagnos", "debug", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The 'search' keyword in DEFAULT_REASONING catches all uses of the word — including trivial lookup/grep steps that should route to the lightweight model. Consider removing it or narrowing it to more specific terms like 'research', 'investigate', or 'deep-search'. As-is, any step prompt containing 'search' (e.g. 'search the file for pattern X', 'full-text search the index') pays the reasoning model rate. |
||
| "architect", "design", "strateg", "investigat", "assess", "judge", | ||
| "verify", "critique", "infer", "deduc", | ||
| ]; | ||
|
|
||
| function matchesAny(text: string, keywords: string[]): boolean { | ||
| for (const kw of keywords) { | ||
| const re = new RegExp(`\\b${kw.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`, "i"); | ||
| if (re.test(text)) return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Classify a task into a complexity tier. User rules take precedence over the | ||
| * built-in defaults. A task that matches neither — or both — resolves to | ||
| * "reasoning", so cost optimization never silently degrades quality. | ||
| */ | ||
| export function classifyTaskTier( | ||
| classifyText: string, | ||
| rules?: Array<{ tier: ModelTier; match: string[] }>, | ||
| ): ModelTier { | ||
| const text = classifyText || ""; | ||
|
|
||
| if (rules) { | ||
| for (const rule of rules) { | ||
| if (Array.isArray(rule.match) && matchesAny(text, rule.match)) { | ||
| return rule.tier; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (matchesAny(text, DEFAULT_REASONING)) return "reasoning"; | ||
| if (matchesAny(text, DEFAULT_LIGHTWEIGHT)) return "lightweight"; | ||
| return "reasoning"; | ||
| } | ||
|
|
||
| /** Resolve a tier alias ("lightweight"/"reasoning") or pass a model id through. */ | ||
| export function resolveModelAlias(ref: string | undefined, routing?: RoutingConfig): string | undefined { | ||
| if (!ref) return undefined; | ||
| if (ref === "lightweight") return routing?.lightweight || undefined; | ||
| if (ref === "reasoning") return routing?.reasoning || undefined; | ||
| return ref; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Silent fallback when a tier alias has no backing model. If a SKILL.md specifies Consider emitting a warning — or throwing — when an alias resolves to undefined: export function resolveModelAlias(ref: string | undefined, routing?: RoutingConfig): string | undefined {
if (!ref) return undefined;
if (ref === "lightweight") {
if (!routing?.lightweight) {
console.warn(`[routing] tier alias 'lightweight' used but routing.lightweight is not configured; falling through`);
}
return routing?.lightweight || undefined;
}
if (ref === "reasoning") {
if (!routing?.reasoning) {
console.warn(`[routing] tier alias 'reasoning' used but routing.reasoning is not configured; falling through`);
}
return routing?.reasoning || undefined;
}
return ref;
} |
||
| } | ||
|
|
||
| /** | ||
| * Decide which model a task runs on, in precedence order: explicit per-step | ||
| * model, per-skill model, automatic classification (only when a routing block | ||
| * is present and enabled), then the primary model. | ||
| */ | ||
| export function resolveRoutedModel(input: RouteInput): RouteResult { | ||
| const { stepModel, skillModel, classifyText, routing, primaryModel } = input; | ||
|
|
||
| const fromStep = resolveModelAlias(stepModel, routing); | ||
| if (fromStep) return { model: fromStep, tier: null, source: "step" }; | ||
|
|
||
| const fromSkill = resolveModelAlias(skillModel, routing); | ||
| if (fromSkill) return { model: fromSkill, tier: null, source: "skill" }; | ||
|
|
||
| const autoEnabled = !!routing && routing.enabled !== false && !!(routing.lightweight || routing.reasoning); | ||
| if (autoEnabled) { | ||
| const tier = classifyTaskTier(classifyText, routing!.rules); | ||
| const model = tier === "lightweight" ? routing!.lightweight : routing!.reasoning; | ||
| if (model) return { model, tier, source: "auto" }; | ||
| } | ||
|
|
||
| return { model: primaryModel, tier: null, source: "fallback" }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| // Tests for the resolution priority chain (step > skill > auto > fallback) and | ||
| // the classifier's safety default (ambiguous tasks resolve to reasoning). | ||
|
|
||
| import test from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
|
|
||
| import { | ||
| classifyTaskTier, | ||
| resolveModelAlias, | ||
| resolveRoutedModel, | ||
| type RoutingConfig, | ||
| } from "../src/model-routing.ts"; | ||
|
|
||
| const routing: RoutingConfig = { | ||
| lightweight: "openai:gpt-4o-mini", | ||
| reasoning: "openai:gpt-4o", | ||
| }; | ||
|
|
||
| // ── classifyTaskTier ─────────────────────────────────────────────────── | ||
|
|
||
| test("classifies lightweight task types as lightweight", () => { | ||
| assert.equal(classifyTaskTier("summarize the pull request diff"), "lightweight"); | ||
| assert.equal(classifyTaskTier("extract the linked issue number"), "lightweight"); | ||
| assert.equal(classifyTaskTier("format the report as markdown"), "lightweight"); | ||
| }); | ||
|
|
||
| test("classifies reasoning task types as reasoning", () => { | ||
| assert.equal(classifyTaskTier("analyze the security implications"), "reasoning"); | ||
| assert.equal(classifyTaskTier("plan a multi-step remediation"), "reasoning"); | ||
| assert.equal(classifyTaskTier("validate the truth score"), "reasoning"); | ||
| }); | ||
|
|
||
| test("unknown tasks default to reasoning (never silently downgrade quality)", () => { | ||
| assert.equal(classifyTaskTier("frobnicate the widget"), "reasoning"); | ||
| assert.equal(classifyTaskTier(""), "reasoning"); | ||
| }); | ||
|
|
||
| test("a task matching both tiers resolves to reasoning", () => { | ||
| // "summarize" (lightweight) + "analyze" (reasoning) → reasoning wins. | ||
| assert.equal(classifyTaskTier("summarize and analyze the results"), "reasoning"); | ||
| }); | ||
|
|
||
| test("user rules take precedence over the built-in defaults", () => { | ||
| // "analyze" would default to reasoning, but a user rule forces lightweight. | ||
| const rules = [{ tier: "lightweight" as const, match: ["analyze"] }]; | ||
| assert.equal(classifyTaskTier("analyze the log lines", rules), "lightweight"); | ||
| }); | ||
|
|
||
| // ── resolveModelAlias ────────────────────────────────────────────────── | ||
|
|
||
| test("resolves tier aliases and passes literal model ids through", () => { | ||
| assert.equal(resolveModelAlias("lightweight", routing), "openai:gpt-4o-mini"); | ||
| assert.equal(resolveModelAlias("reasoning", routing), "openai:gpt-4o"); | ||
| assert.equal(resolveModelAlias("anthropic:claude-sonnet-4-5", routing), "anthropic:claude-sonnet-4-5"); | ||
| assert.equal(resolveModelAlias(undefined, routing), undefined); | ||
| }); | ||
|
|
||
| // ── resolveRoutedModel priority chain ────────────────────────────────── | ||
|
|
||
| test("explicit per-step model wins over everything", () => { | ||
| const r = resolveRoutedModel({ | ||
| stepModel: "openai:gpt-4o", | ||
| skillModel: "openai:gpt-4o-mini", | ||
| classifyText: "summarize the diff", | ||
| routing, | ||
| primaryModel: "openai:gpt-5-reasoning", | ||
| }); | ||
| assert.equal(r.model, "openai:gpt-4o"); | ||
| assert.equal(r.source, "step"); | ||
| }); | ||
|
|
||
| test("per-skill model wins when no step model is set", () => { | ||
| const r = resolveRoutedModel({ | ||
| skillModel: "lightweight", | ||
| classifyText: "analyze the diff", | ||
| routing, | ||
| primaryModel: "openai:gpt-5-reasoning", | ||
| }); | ||
| assert.equal(r.model, "openai:gpt-4o-mini"); | ||
| assert.equal(r.source, "skill"); | ||
| }); | ||
|
|
||
| test("auto classification routes lightweight tasks to the cheap model", () => { | ||
| const r = resolveRoutedModel({ | ||
| classifyText: "summarize the pull request", | ||
| routing, | ||
| primaryModel: "openai:gpt-5-reasoning", | ||
| }); | ||
| assert.equal(r.model, "openai:gpt-4o-mini"); | ||
| assert.equal(r.tier, "lightweight"); | ||
| assert.equal(r.source, "auto"); | ||
| }); | ||
|
|
||
| test("routing stays opt-in — no routing block falls back to primary", () => { | ||
| const r = resolveRoutedModel({ | ||
| classifyText: "summarize the pull request", | ||
| primaryModel: "openai:gpt-5-reasoning", | ||
| }); | ||
| assert.equal(r.model, "openai:gpt-5-reasoning"); | ||
| assert.equal(r.source, "fallback"); | ||
| }); | ||
|
|
||
| test("disabled routing falls back to primary", () => { | ||
| const r = resolveRoutedModel({ | ||
| classifyText: "summarize the pull request", | ||
| routing: { ...routing, enabled: false }, | ||
| primaryModel: "openai:gpt-5-reasoning", | ||
| }); | ||
| assert.equal(r.model, "openai:gpt-5-reasoning"); | ||
| assert.equal(r.source, "fallback"); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test suite only covers the pure functions in model-routing.ts. There is no test that wires a SkillFlowStep with a model field through loadFlowDefinition/discoverWorkflows and verifies that the model field is preserved and passed to resolveRoutedModel correctly. A roundtrip test (serialize → deserialize via saveFlowDefinition → loadFlowDefinition, assert step.model survives) would catch regressions in the YAML plumbing. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking: the keyword-prefix classifier is fundamentally fragile for this use case.
The core problem is that task complexity is a semantic property that does not reliably map to a fixed prefix vocabulary. Real SkillFlow prompts will break this in both directions:
Lightweight classified as reasoning (false-positive tax):
Reasoning classified as lightweight (silent quality downgrade, the worse case):
The
readandfetchentries in DEFAULT_LIGHTWEIGHT are especially risky: they fire on the first verb in a compound prompt, classifying the whole task by only part of it.Suggested fix — replace classifyTaskTier with a single cheap model call:
This costs one gpt-4o-mini call per auto-classified step (~0.0001 USD at current pricing), which is negligible compared to the step itself, and it handles compound prompts, novel verbs, and context correctly. The existing user-rule override path (
rules:) can stay as-is for deterministic overrides where operators want guaranteed behavior.If a synchronous API is required (e.g. the call site can't be made async), the keyword approach is acceptable as a degraded fallback, but the DEFAULT_LIGHTWEIGHT list should at minimum remove 'read', 'fetch', 'load', and 'search' — these verbs are too context-sensitive to use as tier signals.