Skip to content

feat: auto model routing for SkillFlows#51

Open
RiteshTiwari1 wants to merge 1 commit into
open-gitagent:mainfrom
RiteshTiwari1:feat/auto-model-routing
Open

feat: auto model routing for SkillFlows#51
RiteshTiwari1 wants to merge 1 commit into
open-gitagent:mainfrom
RiteshTiwari1:feat/auto-model-routing

Conversation

@RiteshTiwari1

Copy link
Copy Markdown

Addresses #48

Summary

Adds automatic model routing for SkillFlows. Each step is classified by
complexity at runtime and routed to the right model — lightweight tasks
(summarize / extract / classify / transform) run on a cheaper model, while
reasoning-intensive tasks (planning / decision-making / tool orchestration)
stay on the configured reasoning model. Reduces token usage and cost without
sacrificing quality on complex steps.

How it works

Model resolution per step, in priority order:

  1. Explicit per-step model: in workflows/*.yaml
  2. Per-skill model: in SKILL.md
  3. Automatic classification via model.routing tiers
  4. Fallback to the preferred model

Unknown / ambiguous tasks default to reasoning, so cost optimization never
silently degrades quality. Routing is opt-in — active only when a
model.routing block is present, so existing agents are unaffected.

Configuration (agent.yaml)

model:
  preferred: "openai:gpt-5-reasoning"
  routing:
    lightweight: "openai:gpt-4o-mini"
    reasoning:   "openai:gpt-5-reasoning"

Scope note

Routing is applied at the SkillFlow step level — where GitAgent runs
discrete tasks in sequence. Per-turn routing inside a single agent loop would
need deeper pi-agent-core changes; happy to follow up if that's preferred.

@Nivesh353

Copy link
Copy Markdown
Contributor

Hi @RiteshTiwari1 — apologies for the lack of review, and thanks for putting this together. The routing logic in model-routing.ts looks solid — clean priority chain, and defaults to the reasoning tier when a task is ambiguous so cost savings don't quietly hurt quality.

One thing that's changed since this was opened: src/voice/server.ts, where the routing gets wired into the SkillFlow execution loop, was removed in the 2.0 split (#56) — voice moved into its own @open-gitagent/voice package. That's why this now shows conflicts. The loader.ts, skills.ts, and workflows.ts changes still apply cleanly; it's just the execution wiring that needs a new home.

Let us know if you'd like to pick this back up against the new structure — happy to help figure out where it should hook in. Thanks again for the contribution.

@RiteshTiwari1
RiteshTiwari1 force-pushed the feat/auto-model-routing branch from e99c8ac to f3ea30f Compare July 20, 2026 12:01
@RiteshTiwari1

Copy link
Copy Markdown
Author

Thanks @Nivesh353. Rebased onto main (v2.0.2) and dropped the src/voice/server.ts changes, so the conflict's cleared. This PR now has: model-routing.ts (engine), the loader/skills/workflows config changes, exports.ts (re-exports resolveRoutedModel for the voice package), and unit tests.

The per-step wiring belongs in executeFlow() in @open-gitagent/voice — swap query({ model: opts.model }) for resolveRoutedModel(...). That's a follow-up importing from this change.

Two questions:

  1. Should I open the voice-side PR, or will the team take it?
  2. Keep model-routing.ts in core, or move it into the voice package (its only consumer)?

@Nivesh353

Copy link
Copy Markdown
Contributor

Thanks @RiteshTiwari1

  1. Please go ahead and open the voice-side PR yourself — you clearly know
    this code well, and it's a small, well-scoped swap per what you
    described. Happy to review it as soon as it's up.

  2. Keep model-routing.ts in core. Voice isn't going to be its only
    consumer for much longer.

@shreyas-lyzr shreyas-lyzr 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.

Four issues across the new model-routing module. The most significant is the classifier design — covered in the inline comments below.

  1. Keyword classifier produces systematic misclassifications (blocking — see inline).
  2. 'search' is bucketed as reasoning; common lightweight use (grep/find/lookup) will always overpay.
  3. resolveModelAlias returns undefined when a tier alias is configured but the routing block omits that tier's model string — silently skips the tier instead of warning. This can silently downgrade to fallback with no observability.
  4. No integration/end-to-end test exercises resolveRoutedModel being called from actual SkillFlow execution — the unit tests cover the logic in isolation but there's no test proving the plumbing from workflows.ts through to the routed model call.

Security pass: no CVEs in changed deps (no new packages added), no hardcoded secrets, no injection surface — the classifyText input is only used in a regex match, not in any eval/exec/query path.

Comment thread src/model-routing.ts
const DEFAULT_LIGHTWEIGHT = [
"summ", "extract", "classif", "transform", "format", "convert",
"parse", "fetch", "read", "load", "lookup", "normaliz", "translat",
"rephrase", "rewrite", "tag", "label", "render",

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.

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):

  • "verify the file exists" → matches 'verif' → reasoning (but this is a trivial existence check)
  • "review the spelling in this string" → matches 'review' → reasoning
  • "validate that the field is not empty" → matches 'validat' → reasoning
  • "search for the word hello in the file" → matches 'search' → reasoning

Reasoning classified as lightweight (silent quality downgrade, the worse case):

  • "read the code and identify bugs" → matches 'read' → lightweight (but this is a debugging task)
  • "fetch user data and detect anomalies" → matches 'fetch' → lightweight (complex analysis)
  • "parse and understand the business requirements" → matches 'parse' → lightweight
  • "load the config, then decide which deployment strategy to use" → matches 'load' → lightweight

The read and fetch entries 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:

async function classifyTaskTierLLM(
  classifyText: string,
  lightweightModel: string,   // the cheap model is appropriate to classify itself
): Promise<ModelTier> {
  const prompt = [
    "Classify the following task as either 'lightweight' (summarize, extract, format,",
    "convert, parse, fetch, render — mechanical, no reasoning required) or 'reasoning'",
    "(plan, analyze, decide, debug, review, validate logic, orchestrate — needs judgment).",
    "Reply with exactly one word: lightweight or reasoning.",
    "",
    "Task: " + classifyText,
  ].join("\n");

  const response = await callModel(lightweightModel, prompt, { max_tokens: 5 });
  const word = response.trim().toLowerCase();
  return word === "lightweight" ? "lightweight" : "reasoning"; // unknown → reasoning (safe default)
}

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.

Comment thread src/model-routing.ts
];
const DEFAULT_REASONING = [
"search", "analy", "plan", "decid", "decision", "orchestrat", "solve",
"reason", "validat", "evaluat", "review", "audit", "diagnos", "debug",

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.

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.

Comment thread src/model-routing.ts
if (!ref) return undefined;
if (ref === "lightweight") return routing?.lightweight || undefined;
if (ref === "reasoning") return routing?.reasoning || undefined;
return ref;

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.

Silent fallback when a tier alias has no backing model.

If a SKILL.md specifies model: lightweight but the agent.yaml routing block omits lightweight:, resolveModelAlias returns undefined, and the caller falls through to primaryModel without any log or warning. The operator's explicit intention ("use a lightweight model for this skill") is silently ignored.

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;
}

});
assert.equal(r.model, "openai:gpt-5-reasoning");
assert.equal(r.source, "fallback");
});

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.

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.

@RiteshTiwari1

Copy link
Copy Markdown
Author

Thanks @shreyas-lyzr , all four are fair.

2, 3, 4 I'll fix: drop search from the reasoning defaults, warn when a tier alias resolves to no configured model, and add a save→load roundtrip test for step model.

On 1: agreed keywords can't capture semantic complexity. Quick check before I rework —> an LLM classify call has to be async, so it'd live in voice's executeFlow() rather than the pure resolver in core (adds a per-step call on the cheap model). Good with that split, or would you rather the classifier stay in core behind an injected query fn?

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.

3 participants