Fix classifier sending an invalid reasoningEffort value to OpenAI-family models

extensions/auto-router-classify.ts

@@ -4,6 +4,21 @@ 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. 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 = 8_000;
 const VALID_LEVELS: readonly AutoRouterEffortLevel[] = [
   "minimal",
   "low",
@@ -14,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<string> = 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.
@@ -41,29 +72,49 @@ 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 {
   return typeof value === "number" && Number.isFinite(value) ? value : 0;
 }
 
 /**
- * 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.
+ * 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<Api>,
   prompt: string,
   hasImages: boolean,
+  reasoningEffort: AutoRouterEffortLevel,
 ): Promise<ClassificationResult> {
   const controller = new AbortController();
   const timeout = setTimeout(() => controller.abort(), CLASSIFY_TIMEOUT_MS);
   try {
     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,
       {
@@ -78,16 +129,12 @@ export async function classifyTurnComplexity(
       },
       {
         signal: controller.signal,
-        reasoningEffort: "off",
         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.
+        maxTokens: CLASSIFY_MAX_TOKENS,
+        ...(rawReasoningEffort !== undefined
+          ? { reasoningEffort: rawReasoningEffort }
+          : {}),
       },
     );
     const reply = response.content
@@ -115,10 +162,30 @@ export async function classifyTurnComplexity(
           cost: numeric(response.usage.cost?.total),
         }
       : undefined;
-    return { level, usage, reply: reply || "(empty reply)" };
+    // `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)"}${reasonSuffix}`,
+      failed,
+    };
   } 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);
   }

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

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 {
@@ -352,10 +353,8 @@ export default async function autoRouter(pi: ExtensionAPI): Promise<void> {
       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,17 +366,35 @@ export default async function autoRouter(pi: ExtensionAPI): Promise<void> {
       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;
         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) {
+          // 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 (${truncateForLog(result.reply, 500)}); defaulting to ${classifiedLevel} for this turn. Check /usage for details.`,
+            "warning",
+          );
+        }
       }
       level = classifiedLevel;
       tier = resolveEffortTier(settings, classifiedLevel);