From cd708ac4dcf0e56791f81b9080f6c578fcce9ba2 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:30:10 -0400 Subject: [PATCH 1/6] Fix classifier sending an invalid reasoningEffort value to OpenAI-family models auto-router-classify.ts passed reasoningEffort: "off" straight through ModelRegistry.complete(), which forwards it verbatim to each API module's own raw request builder. "off" isn't a valid value in any OpenAI-family reasoningEffort enum: openai-completions/-responses/azure-responses only accept minimal..max, and openai-codex-responses accepts "none" instead of "off". This only type-checked because classifyTurnComplexity's `model: Model` is the broad provider union rather than the specific API a given model actually uses. Verified this is why classification was silently defaulting to "medium" on every turn instead of ever running: routing the classifier through a configured openai-codex-responses model (as happens whenever the medium tier's first model is unhealthy) sent a literally invalid `reasoning.effort: "off"` in the request body, and the model came back with no text content at all - indistinguishable from a genuine "medium" verdict without checking the raw reply. The previous fix in this area (dropping the classifier's maxTokens cap) addressed a real but secondary risk - it didn't touch this root cause, which is why the same empty-reply failure persisted after that merge. There's no reasoningEffort value valid across every provider's raw enum, so the safe fix is to not specify one at all and let each model use its own default; the existing 15s abort timeout still bounds worst-case cost/latency. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index f1b05fc..5ff044b 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -78,16 +78,21 @@ export async function classifyTurnComplexity( }, { signal: controller.signal, - reasoningEffort: "off", + // No `reasoningEffort` here, deliberately: `ModelRegistry.complete()` passes it straight + // through to each API module's own raw request builder, and "off" isn't a valid value in + // any OpenAI-family reasoningEffort enum (openai-completions/-responses/azure-responses + // accept only minimal..max; openai-codex-responses accepts "none" instead of "off"). It + // only type-checked here because `model: Model` is the broad provider union, not the + // specific API this model actually uses. Concretely, passing "off" to an + // openai-codex-responses model sends a literally invalid `reasoning.effort: "off"` in the + // request body - verified: this is why the classifier came back with no text at all every + // time it landed on a codex model, silently defaulting every turn to "medium" instead of + // ever actually classifying it. Since there's no single sentinel valid across every + // provider's raw enum, the safe provider-agnostic choice is to not specify one at all and + // let each model use its own default; `CLASSIFY_TIMEOUT_MS` above still bounds worst-case + // cost/latency regardless of how much a model decides to reason before answering. cacheRetention: "none", sessionId: uuidv7(), - // No maxTokens cap: a small fixed budget (previously 20) is plenty for a non-reasoning - // model's one-word reply, but a reasoning-capable model can spend the entire budget on - // reasoning content before ever emitting the answer - `reasoningEffort: "off"` should - // suppress that, but isn't honored the same way by every provider/model, and when it - // isn't, a tight cap starves the visible reply to nothing rather than just some reasoning - // tokens (verified: this is what was happening). `CLASSIFY_TIMEOUT_MS` above is the real - // bound on how long/expensive a stuck classification call can get. }, ); const reply = response.content From 86c3e3876913c6c0fe304cb793d6fea4dd576839 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:40:47 -0400 Subject: [PATCH 2/6] Surface classifier failures instead of silently defaulting to medium An empty or unparseable classifier reply already fell back to `medium`, but with nothing distinguishing that from a genuine medium verdict - which is exactly how the reasoningEffort bug fixed earlier in this PR went unnoticed across multiple real turns. classifyTurnComplexity now returns `failed: true` whenever `level` is that fallback rather than an actual parsed answer (empty reply, unparseable reply, or the call erroring out), and routeForPrompt surfaces it as a warning notification rather than routing silently. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 26 +++++++++++++++++++++++--- extensions/auto-router.ts | 10 ++++++++++ tests/auto-router-classify.test.ts | 21 ++++++++++++++++++++- tests/auto-router-extension.test.ts | 27 +++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index 5ff044b..fc14826 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -41,6 +41,14 @@ export type ClassificationResult = { * reply" after the fact, since the model call itself is never persisted anywhere else. */ reply: string; + /** + * True when `level` is the `medium` default because the classifier call errored, timed out, or + * came back with no recognizable level word - not because the model actually judged the turn to + * be medium complexity. Callers should surface this (a notification, a log line) rather than let + * it pass as an ordinary classification: silently defaulting with no visible signal is exactly + * what made a real, sustained classifier failure indistinguishable from normal routing. + */ + failed: boolean; }; function numeric(value: unknown): number { @@ -50,7 +58,9 @@ function numeric(value: unknown): number { /** * Classify a turn's complexity using the given (default/medium-tier) model. Never throws and * never blocks indefinitely: a bounded timeout, a provider error, or an unparseable reply all - * fall back to `medium` so classification can never stall or break the user's turn. + * fall back to `medium` so classification can never stall or break the user's turn - but that + * fallback is reported via `failed: true` rather than silently, so a caller can still tell a real + * judgment apart from a classifier that never actually answered. */ export async function classifyTurnComplexity( modelRegistry: ModelRegistry, @@ -120,10 +130,20 @@ export async function classifyTurnComplexity( cost: numeric(response.usage.cost?.total), } : undefined; - return { level, usage, reply: reply || "(empty reply)" }; + return { + level, + usage, + reply: reply || "(empty reply)", + // `String.match()` returns `null`, not `undefined`, when nothing matches. + failed: match === null, + }; } catch (error) { const message = error instanceof Error ? error.message : String(error); - return { level: DEFAULT_LEVEL, reply: `(classification failed: ${message})` }; + return { + level: DEFAULT_LEVEL, + reply: `(classification failed: ${message})`, + failed: true, + }; } finally { clearTimeout(timeout); } diff --git a/extensions/auto-router.ts b/extensions/auto-router.ts index 35486c7..a3560f9 100644 --- a/extensions/auto-router.ts +++ b/extensions/auto-router.ts @@ -378,6 +378,16 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { if (result.usage) { healthStore.recordSuccess(modelKey(classifierModel), result.usage); } + // The classifier defaulting to `medium` isn't a real judgment of this turn's complexity + // when it failed to answer at all - that's silently indistinguishable from a genuine + // medium verdict otherwise, which is exactly what let a sustained classifier failure go + // unnoticed. Surface it visibly rather than let it pass as ordinary routing. + if (result.failed && ctx.hasUI) { + ctx.ui.notify( + `Auto: classifier gave no usable answer (${result.reply}); defaulting to ${classifiedLevel} for this turn. Check /usage for details.`, + "warning", + ); + } } level = classifiedLevel; tier = resolveEffortTier(settings, classifiedLevel); diff --git a/tests/auto-router-classify.test.ts b/tests/auto-router-classify.test.ts index 6e417b8..dd23abf 100644 --- a/tests/auto-router-classify.test.ts +++ b/tests/auto-router-classify.test.ts @@ -67,9 +67,27 @@ test("classifyTurnComplexity picks the level word the model actually led with, e expect(result.level).toBe("medium"); }); -test("classifyTurnComplexity falls back to medium on an unparseable reply", async () => { +test("classifyTurnComplexity falls back to medium on an unparseable reply, and flags it as failed", async () => { const result = await classifyTurnComplexity(registryReplying("uh, tricky one"), MODEL, "do something", false); expect(result.level).toBe("medium"); + // `failed: true` is what lets a caller tell "the model actually said medium" apart from "the + // model said nothing usable, so this is just the fallback" - a distinction that matters because + // the two look identical if only `level` is checked, which is exactly what let a real, sustained + // classifier failure pass as ordinary routing undetected. + expect(result.failed).toBe(true); +}); + +test("classifyTurnComplexity flags a completely empty reply as failed too, not just unparseable text", async () => { + const result = await classifyTurnComplexity(registryReplying(""), MODEL, "do something", false); + expect(result.level).toBe("medium"); + expect(result.reply).toBe("(empty reply)"); + expect(result.failed).toBe(true); +}); + +test("classifyTurnComplexity does not flag a genuine parsed verdict as failed", async () => { + const result = await classifyTurnComplexity(registryReplying("medium"), MODEL, "do something", false); + expect(result.level).toBe("medium"); + expect(result.failed).toBe(false); }); test("classifyTurnComplexity falls back to medium when the provider call throws, and records why in reply", async () => { @@ -77,6 +95,7 @@ test("classifyTurnComplexity falls back to medium when the provider call throws, expect(result.level).toBe("medium"); expect(result.usage).toBeUndefined(); expect(result.reply).toContain("provider down"); + expect(result.failed).toBe(true); }); test("classifyTurnComplexity surfaces the raw reply text alongside the parsed level, for diagnosing mismatches later", async () => { diff --git a/tests/auto-router-extension.test.ts b/tests/auto-router-extension.test.ts index 8e63183..51d3ddc 100644 --- a/tests/auto-router-extension.test.ts +++ b/tests/auto-router-extension.test.ts @@ -378,6 +378,33 @@ test("before_agent_start routes to the classified tier, and the picker shows Aut expect(lastFooterBadge(fake.footerEvents)).toBe("🔀 Auto (auto)"); }); +test("before_agent_start notifies when the classifier gives no usable answer, instead of silently defaulting", async () => { + const medium = model("prov", "medium-model"); + await writeConfig({ + efforts: { medium: { models: [{ provider: "prov", id: "medium-model" }] } }, + }); + + const fake = createFakePi(); + await autoRouter(fake.pi); + // An empty classifier reply parses to no level word at all - exactly what happened for real + // when a codex-family model was sent an invalid reasoningEffort value and came back with no + // text content. + const registry = fakeModelRegistry({ models: [medium], classify: () => "" }); + const ctx = fakeCtx({ modelRegistry: registry, currentModel: fake.currentModel }); + + await fake.fire("session_start", {}, ctx); + await selectAuto(fake, ctx); + await fake.fire("before_agent_start", { prompt: "anything" }, ctx); + + // Still routes (defaulting to medium) rather than blocking the turn... + expect(fake.setModelCalls).toEqual([medium]); + // ...but the failure is visible, not silent. + const warning = ctx.notifications.find( + (n) => n.type === "warning" && n.message.includes("classifier"), + ); + expect(warning?.message).toContain("(empty reply)"); +}); + test("a model's `effort` override sets its own thinking level, independent of the tier that routed to it", async () => { const high = model("opencode-go", "kimi-k3"); await writeConfig({ From 08e972ef058b8e916595dacc0e5162de93690f34 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:42:35 -0400 Subject: [PATCH 3/6] Address review: cap classifier output tokens, add request-shape regression test CodeRabbit correctly flagged that dropping maxTokens entirely (in the prior PR) left the classification call with no bound on output/cost at all - CLASSIFY_TIMEOUT_MS only bounds wall-clock time, not tokens, so a model that reasons at length but still answers quickly could run up real per-turn cost on what's supposed to be a cheap triage call. Reintroduce a cap, but a much larger one (2000, exported as CLASSIFY_MAX_TOKENS) than the previous fixed 20 that was starving reasoning-capable models to an empty reply. Also add the requested regression test asserting the actual options object passed to modelRegistry.complete() has no reasoningEffort field and the new token cap - coverage this suite didn't have before, which is exactly how the invalid reasoningEffort: "off" went unnoticed in the first place. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 14 ++++++++++-- tests/auto-router-classify.test.ts | 36 +++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index fc14826..91b8030 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -4,6 +4,16 @@ import type { ModelRegistry } from "@earendil-works/pi-coding-agent"; import type { AutoRouterEffortLevel } from "./auto-router-settings.js"; const CLASSIFY_TIMEOUT_MS = 15_000; +/** + * Generous compared to the single word this call actually needs, but bounded: a reasoning-capable + * classifier model can spend real output budget on reasoning content before ever emitting the + * answer, and the previous fixed cap of 20 starved that to nothing (verified). Uncapped isn't the + * right fix either though - `CLASSIFY_TIMEOUT_MS` only bounds wall-clock time, not tokens, so a + * model that reasons at length but still finishes quickly could otherwise run up real cost on what + * is supposed to be a cheap per-turn triage call. This leaves room for a few hundred tokens of + * reasoning plus the answer while still capping the worst case. + */ +export const CLASSIFY_MAX_TOKENS = 2_000; const VALID_LEVELS: readonly AutoRouterEffortLevel[] = [ "minimal", "low", @@ -99,10 +109,10 @@ export async function classifyTurnComplexity( // time it landed on a codex model, silently defaulting every turn to "medium" instead of // ever actually classifying it. Since there's no single sentinel valid across every // provider's raw enum, the safe provider-agnostic choice is to not specify one at all and - // let each model use its own default; `CLASSIFY_TIMEOUT_MS` above still bounds worst-case - // cost/latency regardless of how much a model decides to reason before answering. + // let each model use its own default. cacheRetention: "none", sessionId: uuidv7(), + maxTokens: CLASSIFY_MAX_TOKENS, }, ); const reply = response.content diff --git a/tests/auto-router-classify.test.ts b/tests/auto-router-classify.test.ts index dd23abf..ff0fc6c 100644 --- a/tests/auto-router-classify.test.ts +++ b/tests/auto-router-classify.test.ts @@ -1,7 +1,7 @@ import { expect, test } from "bun:test"; import type { Api, Model } from "@earendil-works/pi-ai"; import type { ModelRegistry } from "@earendil-works/pi-coding-agent"; -import { classifyTurnComplexity } from "../extensions/auto-router-classify.ts"; +import { CLASSIFY_MAX_TOKENS, classifyTurnComplexity } from "../extensions/auto-router-classify.ts"; import type { AutoRouterEffortLevel } from "../extensions/auto-router-settings.ts"; const MODEL = { provider: "prov", id: "classifier" } as unknown as Model; @@ -18,6 +18,27 @@ function registryReplying( } as unknown as ModelRegistry; } +/** Captures the raw options object passed to `complete()`, so a test can assert on the actual + * request shape rather than just the parsed result - the only way to catch a regression like the + * invalid `reasoningEffort: "off"` this suite didn't previously guard against. */ +function registryCapturingOptions(text: string): { + registry: ModelRegistry; + options: () => Record | undefined; +} { + let captured: Record | undefined; + const registry = { + complete: async ( + _model: Model, + _context: unknown, + options: Record, + ) => { + captured = options; + return { content: [{ type: "text", text }], usage: undefined }; + }, + } as unknown as ModelRegistry; + return { registry, options: () => captured }; +} + function throwingRegistry(): ModelRegistry { return { complete: async () => { @@ -135,3 +156,16 @@ test("classifyTurnComplexity notes attached images in the classification prompt" await classifyTurnComplexity(registry, MODEL, "describe this screenshot", true); expect(capturedText).toContain("attached images"); }); + +test("classifyTurnComplexity does not send reasoningEffort, and caps output at CLASSIFY_MAX_TOKENS", async () => { + const { registry, options } = registryCapturingOptions("medium"); + + await classifyTurnComplexity(registry, MODEL, "do something", false); + + // "off" isn't a valid reasoningEffort value for any OpenAI-family API - regression coverage + // for that bug: the field must be entirely absent, not just falsy, since some raw request + // builders treat "any value present" as "send a reasoning object" regardless of its content. + expect(options()).not.toHaveProperty("reasoningEffort"); + // Bounded, but not the old fixed 20 that starved reasoning-capable models to an empty reply. + expect(options()?.maxTokens).toBe(CLASSIFY_MAX_TOKENS); +}); From 17daa810282f045e192b4d3d9010683802d9c5b0 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:03:17 -0400 Subject: [PATCH 4/6] Raise CLASSIFY_MAX_TOKENS from 2000 to 8000 2000 was an untested guess. Since the classifier no longer sets an explicit reasoningEffort, a reasoning-capable model's default reasoning depth for this trivial a prompt is unmeasured - sized up generously to hedge against that uncertainty rather than tuned from real data. Still negligible next to a real agent turn's own token usage, which is dominated by input context this call never carries. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index 91b8030..45eb749 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -10,10 +10,15 @@ const CLASSIFY_TIMEOUT_MS = 15_000; * answer, and the previous fixed cap of 20 starved that to nothing (verified). Uncapped isn't the * right fix either though - `CLASSIFY_TIMEOUT_MS` only bounds wall-clock time, not tokens, so a * model that reasons at length but still finishes quickly could otherwise run up real cost on what - * is supposed to be a cheap per-turn triage call. This leaves room for a few hundred tokens of - * reasoning plus the answer while still capping the worst case. + * is supposed to be a cheap per-turn triage call. There's no explicit `reasoningEffort` set (see + * below), so a reasoning model's default reasoning depth for this trivial a prompt is unmeasured - + * sized generously to hedge against that uncertainty rather than tuned from real data. Still tiny + * next to a real agent turn's own token usage (input context there dwarfs this call's output cap + * by orders of magnitude - the two aren't comparable). If replies still come back empty/truncated + * at this size, that's a signal to control reasoning effort per-provider instead of just raising + * this further. */ -export const CLASSIFY_MAX_TOKENS = 2_000; +export const CLASSIFY_MAX_TOKENS = 8_000; const VALID_LEVELS: readonly AutoRouterEffortLevel[] = [ "minimal", "low", From 3efd3b5bea8d39164ed8dc95c1cc5f533541a2e9 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:08:58 -0400 Subject: [PATCH 5/6] Classify at the model's actually-configured effort, not an unrelated default The classifier previously omitted reasoningEffort entirely, letting each provider fall back to whatever it does by default - reasonable as a safe stopgap, but not what the medium tier's own config says a given model should run at. routeForPrompt now resolves the same effort the classifier model would actually be dispatched at for real work (its own configured override, or the tier name) via the existing resolveEffort() helper, and passes that through. The raw per-API reasoningEffort field is still provider-specific (that's the whole reason "off" broke things), so classifyTurnComplexity only forwards it for APIs verified to accept the full AutoRouterEffortLevel vocabulary (the OpenAI family), and never forwards "off" even there since it's invalid everywhere. Every other provider keeps using its own default, same as before. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 51 +++++++++++++------- extensions/auto-router.ts | 11 +++-- tests/auto-router-classify.test.ts | 76 +++++++++++++++++++++++++----- 3 files changed, 105 insertions(+), 33 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index 45eb749..f72bc92 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -29,6 +29,22 @@ const VALID_LEVELS: readonly AutoRouterEffortLevel[] = [ ]; const DEFAULT_LEVEL: AutoRouterEffortLevel = "medium"; +/** + * APIs whose raw `reasoningEffort` field is verified (against each module's own type + * declaration) to accept the full `AutoRouterEffortLevel` vocabulary (minus "off", handled + * separately below). Passing it to any other API either does nothing - most providers (Anthropic, + * Z.ai, MiniMax, OpenCode Go, ...) have no such field at all - or, worse, sends a value invalid + * for that API's own narrower enum: Mistral's `reasoningEffort` only accepts "none" | "high", so + * "medium" would be exactly the kind of invalid-value bug this allowlist exists to avoid + * repeating. + */ +const REASONING_EFFORT_SAFE_APIS: ReadonlySet = new Set([ + "openai-completions", + "openai-responses", + "azure-openai-responses", + "openai-codex-responses", +]); + const SYSTEM_PROMPT = `You triage the complexity of a single upcoming coding-agent turn so it can be routed to an appropriately capable model. Reply with exactly one word, lowercase, no punctuation: minimal, low, medium, high, xhigh, or max. - minimal: rote, no real reasoning needed. A one-word answer, a pure formatting pass, a trivial rename, echoing back something already known. @@ -71,17 +87,20 @@ function numeric(value: unknown): number { } /** - * Classify a turn's complexity using the given (default/medium-tier) model. Never throws and - * never blocks indefinitely: a bounded timeout, a provider error, or an unparseable reply all - * fall back to `medium` so classification can never stall or break the user's turn - but that - * fallback is reported via `failed: true` rather than silently, so a caller can still tell a real - * judgment apart from a classifier that never actually answered. + * Classify a turn's complexity using the given (default/medium-tier) model, reasoning at + * `reasoningEffort` - the same effort this model is actually dispatched at for real work (its own + * configured override, or its tier's name), so the classify call doesn't reason at some unrelated + * provider default. Never throws and never blocks indefinitely: a bounded timeout, a provider + * error, or an unparseable reply all fall back to `medium` so classification can never stall or + * break the user's turn - but that fallback is reported via `failed: true` rather than silently, + * so a caller can still tell a real judgment apart from a classifier that never actually answered. */ export async function classifyTurnComplexity( modelRegistry: ModelRegistry, model: Model, prompt: string, hasImages: boolean, + reasoningEffort: AutoRouterEffortLevel, ): Promise { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), CLASSIFY_TIMEOUT_MS); @@ -89,6 +108,13 @@ export async function classifyTurnComplexity( const text = hasImages ? `${prompt}\n\n(This turn also includes attached images.)` : prompt; + // "off" isn't a valid raw reasoningEffort value on any API observed (that's the bug this + // whole thing started from), and only route it through at all on APIs verified to accept our + // effort vocabulary - see REASONING_EFFORT_SAFE_APIS. + const rawReasoningEffort = + reasoningEffort !== "off" && REASONING_EFFORT_SAFE_APIS.has(model.api) + ? reasoningEffort + : undefined; const response = await modelRegistry.complete( model, { @@ -103,21 +129,12 @@ export async function classifyTurnComplexity( }, { signal: controller.signal, - // No `reasoningEffort` here, deliberately: `ModelRegistry.complete()` passes it straight - // through to each API module's own raw request builder, and "off" isn't a valid value in - // any OpenAI-family reasoningEffort enum (openai-completions/-responses/azure-responses - // accept only minimal..max; openai-codex-responses accepts "none" instead of "off"). It - // only type-checked here because `model: Model` is the broad provider union, not the - // specific API this model actually uses. Concretely, passing "off" to an - // openai-codex-responses model sends a literally invalid `reasoning.effort: "off"` in the - // request body - verified: this is why the classifier came back with no text at all every - // time it landed on a codex model, silently defaulting every turn to "medium" instead of - // ever actually classifying it. Since there's no single sentinel valid across every - // provider's raw enum, the safe provider-agnostic choice is to not specify one at all and - // let each model use its own default. cacheRetention: "none", sessionId: uuidv7(), maxTokens: CLASSIFY_MAX_TOKENS, + ...(rawReasoningEffort !== undefined + ? { reasoningEffort: rawReasoningEffort } + : {}), }, ); const reply = response.content diff --git a/extensions/auto-router.ts b/extensions/auto-router.ts index a3560f9..eec4c58 100644 --- a/extensions/auto-router.ts +++ b/extensions/auto-router.ts @@ -352,10 +352,8 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { level = "(pinned)"; classifierReply = `pinned to ${pinnedTier} - not classified`; } else { - const classifierPool = resolveAvailableModels( - ctx.modelRegistry, - settings.efforts.medium?.models ?? allConfiguredModels(settings), - ); + const classifierRefs = settings.efforts.medium?.models ?? allConfiguredModels(settings); + const classifierPool = resolveAvailableModels(ctx.modelRegistry, classifierRefs); const classifierRef = healthStore.pickHealthy(classifierPool); const classifierModel = classifierRef ? classifierPool.find( @@ -367,11 +365,16 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { let classifiedLevel: AutoRouterEffortLevel = "medium"; classifierReply = "(no classifier available)"; if (classifierModel) { + // Same effort this model would actually be dispatched at for real work in the medium + // tier - its own configured override, or "medium" itself - so the classify call reasons + // at the level the user configured for it rather than an unrelated provider default. + const classifierEffort = resolveEffort(classifierRefs, classifierModel, "medium"); const result = await classifyTurnComplexity( ctx.modelRegistry, classifierModel, prompt, hasImages, + classifierEffort, ); classifiedLevel = result.level; classifierReply = result.reply; diff --git a/tests/auto-router-classify.test.ts b/tests/auto-router-classify.test.ts index ff0fc6c..40ef55a 100644 --- a/tests/auto-router-classify.test.ts +++ b/tests/auto-router-classify.test.ts @@ -5,6 +5,16 @@ import { CLASSIFY_MAX_TOKENS, classifyTurnComplexity } from "../extensions/auto- import type { AutoRouterEffortLevel } from "../extensions/auto-router-settings.ts"; const MODEL = { provider: "prov", id: "classifier" } as unknown as Model; +const CODEX_MODEL = { + provider: "openai-codex", + id: "classifier", + api: "openai-codex-responses", +} as unknown as Model; +const UNLISTED_API_MODEL = { + provider: "anthropic", + id: "classifier", + api: "anthropic-messages", +} as unknown as Model; function registryReplying( text: string, @@ -51,7 +61,7 @@ const ALL_LEVELS: AutoRouterEffortLevel[] = ["minimal", "low", "medium", "high", for (const level of ALL_LEVELS) { test(`classifyTurnComplexity parses a bare "${level}" reply`, async () => { - const result = await classifyTurnComplexity(registryReplying(level), MODEL, "do something", false); + const result = await classifyTurnComplexity(registryReplying(level), MODEL, "do something", false, "medium"); expect(result.level).toBe(level); }); } @@ -62,6 +72,7 @@ test("classifyTurnComplexity parses the reply case-insensitively with surroundin MODEL, "do something", false, + "medium", ); expect(result.level).toBe("max"); }); @@ -74,6 +85,7 @@ test("classifyTurnComplexity picks the level word the model actually led with, n MODEL, "do something", false, + "medium", ); expect(result.level).toBe("high"); }); @@ -84,12 +96,19 @@ test("classifyTurnComplexity picks the level word the model actually led with, e MODEL, "do something", false, + "medium", ); expect(result.level).toBe("medium"); }); test("classifyTurnComplexity falls back to medium on an unparseable reply, and flags it as failed", async () => { - const result = await classifyTurnComplexity(registryReplying("uh, tricky one"), MODEL, "do something", false); + const result = await classifyTurnComplexity( + registryReplying("uh, tricky one"), + MODEL, + "do something", + false, + "medium", + ); expect(result.level).toBe("medium"); // `failed: true` is what lets a caller tell "the model actually said medium" apart from "the // model said nothing usable, so this is just the fallback" - a distinction that matters because @@ -99,20 +118,20 @@ test("classifyTurnComplexity falls back to medium on an unparseable reply, and f }); test("classifyTurnComplexity flags a completely empty reply as failed too, not just unparseable text", async () => { - const result = await classifyTurnComplexity(registryReplying(""), MODEL, "do something", false); + const result = await classifyTurnComplexity(registryReplying(""), MODEL, "do something", false, "medium"); expect(result.level).toBe("medium"); expect(result.reply).toBe("(empty reply)"); expect(result.failed).toBe(true); }); test("classifyTurnComplexity does not flag a genuine parsed verdict as failed", async () => { - const result = await classifyTurnComplexity(registryReplying("medium"), MODEL, "do something", false); + const result = await classifyTurnComplexity(registryReplying("medium"), MODEL, "do something", false, "medium"); expect(result.level).toBe("medium"); expect(result.failed).toBe(false); }); test("classifyTurnComplexity falls back to medium when the provider call throws, and records why in reply", async () => { - const result = await classifyTurnComplexity(throwingRegistry(), MODEL, "do something", false); + const result = await classifyTurnComplexity(throwingRegistry(), MODEL, "do something", false, "medium"); expect(result.level).toBe("medium"); expect(result.usage).toBeUndefined(); expect(result.reply).toContain("provider down"); @@ -125,6 +144,7 @@ test("classifyTurnComplexity surfaces the raw reply text alongside the parsed le MODEL, "do something", false, + "medium", ); expect(result.level).toBe("high"); expect(result.reply).toBe("high complexity, more than a medium task"); @@ -136,6 +156,7 @@ test("classifyTurnComplexity surfaces usage from the response when present", asy MODEL, "do something", false, + "medium", ); expect(result.level).toBe("low"); expect(result.usage).toEqual({ input: 42, output: 7, cost: 0.002 }); @@ -153,19 +174,50 @@ test("classifyTurnComplexity notes attached images in the classification prompt" }, } as unknown as ModelRegistry; - await classifyTurnComplexity(registry, MODEL, "describe this screenshot", true); + await classifyTurnComplexity(registry, MODEL, "describe this screenshot", true, "medium"); expect(capturedText).toContain("attached images"); }); -test("classifyTurnComplexity does not send reasoningEffort, and caps output at CLASSIFY_MAX_TOKENS", async () => { +test("classifyTurnComplexity caps output at CLASSIFY_MAX_TOKENS", async () => { const { registry, options } = registryCapturingOptions("medium"); - await classifyTurnComplexity(registry, MODEL, "do something", false); + await classifyTurnComplexity(registry, MODEL, "do something", false, "medium"); - // "off" isn't a valid reasoningEffort value for any OpenAI-family API - regression coverage - // for that bug: the field must be entirely absent, not just falsy, since some raw request - // builders treat "any value present" as "send a reasoning object" regardless of its content. - expect(options()).not.toHaveProperty("reasoningEffort"); // Bounded, but not the old fixed 20 that starved reasoning-capable models to an empty reply. expect(options()?.maxTokens).toBe(CLASSIFY_MAX_TOKENS); }); + +test("classifyTurnComplexity passes the requested reasoningEffort through for a model on a known-safe API", async () => { + const { registry, options } = registryCapturingOptions("medium"); + + await classifyTurnComplexity(registry, CODEX_MODEL, "do something", false, "max"); + + // This is the whole point of threading an effort through at all: a model configured with + // effort: "max" for its tier should actually reason at max here too, not some unrelated + // provider default. + expect(options()?.reasoningEffort).toBe("max"); +}); + +test("classifyTurnComplexity never sends reasoningEffort \"off\", even for a known-safe API", async () => { + const { registry, options } = registryCapturingOptions("medium"); + + await classifyTurnComplexity(registry, CODEX_MODEL, "do something", false, "off"); + + // "off" isn't a valid reasoningEffort value for any OpenAI-family API - regression coverage for + // the bug that started all this: the field must be entirely absent, not just falsy, since some + // raw request builders treat "any value present" as "send a reasoning object" regardless of + // its content. + expect(options()).not.toHaveProperty("reasoningEffort"); +}); + +test("classifyTurnComplexity does not send reasoningEffort for a model on an unlisted API", async () => { + const { registry, options } = registryCapturingOptions("medium"); + + // "high" is a perfectly valid AutoRouterEffortLevel, but this model's API was never verified to + // accept it as a raw reasoningEffort value - Mistral's own enum, for example, is only + // "none" | "high", so blindly forwarding an untested value risks repeating the exact bug this + // allowlist exists to prevent. + await classifyTurnComplexity(registry, UNLISTED_API_MODEL, "do something", false, "high"); + + expect(options()).not.toHaveProperty("reasoningEffort"); +}); From 2e9e769567a6d5f743c2fc4e56e8b687438fee79 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:15:35 -0400 Subject: [PATCH 6/6] Name CLASSIFY_MAX_TOKENS truncation specifically, and bound the notification If a reasoning model hits CLASSIFY_MAX_TOKENS before ever producing its answer, that's directly actionable (raise the cap, or this model needs less reasoning effort) - but it was indistinguishable from any other unusable reply. classifyTurnComplexity now checks the response's stopReason and names truncation explicitly when that's what happened. Also addresses review: a failed reply can now legitimately run up to CLASSIFY_MAX_TOKENS long, and was being interpolated into the warning notification whole - long enough to flood the UI and bury the warning itself. Reused (and exported) the existing truncateForLog helper that already bounds this same text for /usage, rather than adding a second copy of the same logic. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 16 +++++++++--- extensions/auto-router-health.ts | 12 ++++++--- extensions/auto-router.ts | 6 ++++- tests/auto-router-classify.test.ts | 40 ++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index f72bc92..08452c4 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -162,12 +162,22 @@ export async function classifyTurnComplexity( cost: numeric(response.usage.cost?.total), } : undefined; + // `String.match()` returns `null`, not `undefined`, when nothing matches. + const failed = match === null; + // `stopReason: "length"` means the model hit CLASSIFY_MAX_TOKENS before finishing - distinct + // from a model that finished cleanly but just didn't say a recognizable level word. Naming + // the actual cause here means a diagnosis doesn't have to be guessed at: it's directly + // actionable (raise CLASSIFY_MAX_TOKENS, or this model needs less reasoning effort) rather + // than indistinguishable from any other reason the reply came back empty. + const reasonSuffix = + failed && response.stopReason === "length" + ? ` (hit CLASSIFY_MAX_TOKENS=${CLASSIFY_MAX_TOKENS} before answering)` + : ""; return { level, usage, - reply: reply || "(empty reply)", - // `String.match()` returns `null`, not `undefined`, when nothing matches. - failed: match === null, + reply: `${reply || "(empty reply)"}${reasonSuffix}`, + failed, }; } catch (error) { const message = error instanceof Error ? error.message : String(error); diff --git a/extensions/auto-router-health.ts b/extensions/auto-router-health.ts index fdd1c2e..73e4748 100644 --- a/extensions/auto-router-health.ts +++ b/extensions/auto-router-health.ts @@ -57,11 +57,15 @@ export type ClassificationLogEntry = { model: ModelIdentity; }; -function truncateForLog(text: string): string { +/** Collapses whitespace and bounds length - shared with the live "classifier failed" UI + * notification, since an unusable reply can now run up to `CLASSIFY_MAX_TOKENS` long and would + * otherwise flood a one-line notification the same way it would this log. */ +export function truncateForLog( + text: string, + limit: number = CLASSIFICATION_LOG_TEXT_LIMIT, +): string { const collapsed = text.replace(/\s+/g, " ").trim(); - return collapsed.length > CLASSIFICATION_LOG_TEXT_LIMIT - ? `${collapsed.slice(0, CLASSIFICATION_LOG_TEXT_LIMIT)}…` - : collapsed; + return collapsed.length > limit ? `${collapsed.slice(0, limit)}…` : collapsed; } export type UsageDelta = { input: number; output: number; cost: number }; diff --git a/extensions/auto-router.ts b/extensions/auto-router.ts index eec4c58..b394aae 100644 --- a/extensions/auto-router.ts +++ b/extensions/auto-router.ts @@ -16,6 +16,7 @@ import { type ModelHealthEntry, type ModelIdentity, modelKey, + truncateForLog, } from "./auto-router-health.js"; import { normalizeModelId, reconcileProviderQuota } from "./auto-router-quota.js"; import { @@ -386,8 +387,11 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { // medium verdict otherwise, which is exactly what let a sustained classifier failure go // unnoticed. Surface it visibly rather than let it pass as ordinary routing. if (result.failed && ctx.hasUI) { + // A failed reply can now run up to CLASSIFY_MAX_TOKENS long (e.g. a reasoning model + // that never got to its answer) - bound it here the same way it's already bounded for + // /usage below, so one long reply can't flood this notification and bury the warning. ctx.ui.notify( - `Auto: classifier gave no usable answer (${result.reply}); defaulting to ${classifiedLevel} for this turn. Check /usage for details.`, + `Auto: classifier gave no usable answer (${truncateForLog(result.reply, 500)}); defaulting to ${classifiedLevel} for this turn. Check /usage for details.`, "warning", ); } diff --git a/tests/auto-router-classify.test.ts b/tests/auto-router-classify.test.ts index 40ef55a..79b0175 100644 --- a/tests/auto-router-classify.test.ts +++ b/tests/auto-router-classify.test.ts @@ -28,6 +28,19 @@ function registryReplying( } as unknown as ModelRegistry; } +function registryStoppingAt( + text: string, + stopReason: string, +): ModelRegistry { + return { + complete: async () => ({ + content: [{ type: "text", text }], + usage: undefined, + stopReason, + }), + } as unknown as ModelRegistry; +} + /** Captures the raw options object passed to `complete()`, so a test can assert on the actual * request shape rather than just the parsed result - the only way to catch a regression like the * invalid `reasoningEffort: "off"` this suite didn't previously guard against. */ @@ -124,6 +137,33 @@ test("classifyTurnComplexity flags a completely empty reply as failed too, not j expect(result.failed).toBe(true); }); +test("classifyTurnComplexity names CLASSIFY_MAX_TOKENS truncation specifically, not just \"(empty reply)\"", async () => { + // stopReason "length" means the model hit the token cap before finishing - directly actionable + // (raise the cap, or this model needs less reasoning effort) rather than indistinguishable from + // any other reason the reply came back unusable. + const result = await classifyTurnComplexity( + registryStoppingAt("", "length"), + MODEL, + "do something", + false, + "medium", + ); + expect(result.failed).toBe(true); + expect(result.reply).toContain(`CLASSIFY_MAX_TOKENS=${CLASSIFY_MAX_TOKENS}`); +}); + +test("classifyTurnComplexity does not claim truncation when the reply is just empty for some other reason", async () => { + const result = await classifyTurnComplexity( + registryStoppingAt("", "stop"), + MODEL, + "do something", + false, + "medium", + ); + expect(result.failed).toBe(true); + expect(result.reply).not.toContain("CLASSIFY_MAX_TOKENS"); +}); + test("classifyTurnComplexity does not flag a genuine parsed verdict as failed", async () => { const result = await classifyTurnComplexity(registryReplying("medium"), MODEL, "do something", false, "medium"); expect(result.level).toBe("medium");