Clamp classifier reasoningEffort per-model, fix real root cause of state-file corruption

extensions/auto-router-classify.ts

@@ -108,6 +108,12 @@ export async function classifyTurnComplexity(
     const text = hasImages
       ? `${prompt}\n\n(This turn also includes attached images.)`
       : prompt;
+    // `reasoningEffort` is expected to already be genuinely supported by `model` - the caller
+    // (auto-router.ts) resolves it through pi-ai's own `getSupportedThinkingLevels`/
+    // `clampThinkingLevel` first and warns the user directly if their config asked for something
+    // this model doesn't actually support, rather than this function quietly substituting
+    // something else with no visibility into that mismatch.
+    //
     // "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.

extensions/auto-router-health.ts

@@ -301,11 +301,23 @@ export class AutoRouterHealthStore {
   private state: AutoRouterHealthState = {};
   private classifications: ClassificationLogEntry[] = [];
   private writeTimer: ReturnType<typeof setTimeout> | undefined;
+  /**
+   * Resolved once, at construction, rather than fresh on every `flush()` - a real Pi process's
+   * agent dir never changes mid-life, and re-resolving `PI_CODING_AGENT_DIR` on each debounced
+   * flush instead means *any* code that touches that env var while a save is still pending (not
+   * just this instance's own caller) silently redirects an in-flight write to wherever the env
+   * var happens to point at that later moment - verified: this is exactly how a test suite's
+   * fixture data ended up in the real global `auto-router-state.json` on disk, repeatedly, even
+   * after each individual test file was fixed to wait out its own debounce window before its own
+   * teardown - a *different* file's `beforeEach` changing the same process-wide env var while an
+   * earlier file's save was still in flight was enough on its own.
+   */
+  private readonly path: string = statePath();
 
   async load(): Promise<void> {
     try {
       const parsed = parsePersisted(
-        JSON.parse(await readFile(statePath(), "utf8")),
+        JSON.parse(await readFile(this.path, "utf8")),
       );
       this.state = parsed.models;
       this.classifications = parsed.classifications;
@@ -392,7 +404,7 @@ export class AutoRouterHealthStore {
   }
 
   async flush(): Promise<void> {
-    const dir = dirname(statePath());
+    const dir = dirname(this.path);
     await mkdir(dir, { recursive: true });
     const tempPath = join(
       dir,
@@ -404,7 +416,7 @@ export class AutoRouterHealthStore {
         `${JSON.stringify({ models: this.state, classifications: this.classifications }, null, 2)}\n`,
         "utf8",
       );
-      await rename(tempPath, statePath());
+      await rename(tempPath, this.path);
     } finally {
       await rm(tempPath, { force: true }).catch(() => undefined);
     }

extensions/auto-router.ts

@@ -1,4 +1,9 @@
-import type { Api, Model } from "@earendil-works/pi-ai";
+import {
+  type Api,
+  clampThinkingLevel,
+  getSupportedThinkingLevels,
+  type Model,
+} from "@earendil-works/pi-ai";
 import {
   DynamicBorder,
   type ExtensionAPI,
@@ -229,6 +234,74 @@ export default async function autoRouter(pi: ExtensionAPI): Promise<void> {
     return ref?.effort ?? tier;
   }
 
+  /**
+   * `requested` if `model` genuinely supports it, or the closest level it actually does (via
+   * pi-ai's own `clampThinkingLevel` - the same resolution real turn dispatch already relies on,
+   * not a guess of our own) - warning the user directly whenever a substitution was needed, since
+   * a configured `effort` a model silently can't honor is exactly what turned a "medium (max
+   * effort)" routing decision into an unexplained empty classifier reply, undetected, for two full
+   * PRs. Applies to both the classifier's own reasoning effort and real per-turn dispatch, so
+   * fixing (or leaving) the mismatch is the user's informed choice either way, not something Auto
+   * quietly papers over in only one of the two places it happens.
+   */
+  function resolveSupportedEffort(
+    ctx: ExtensionContext,
+    model: Model<Api>,
+    requested: AutoRouterEffortLevel,
+  ): AutoRouterEffortLevel {
+    const supported = getSupportedThinkingLevels(model);
+    if (supported.includes(requested)) return requested;
+    const clamped = clampThinkingLevel(model, requested) as AutoRouterEffortLevel;
+    if (ctx.hasUI) {
+      ctx.ui.notify(
+        `Auto: ${model.provider}/${model.id} doesn't support "${requested}" effort (configured for it in ~/.pi/agent/settings.json) - it supports ${supported.join(", ")}. Using "${clamped}" instead.`,
+        "warning",
+      );
+    }
+    return clamped;
+  }
+
+  /**
+   * Whole-config check, once per session start: does every model with a configured `effort`
+   * override actually support that effort? `resolveSupportedEffort` above only warns about a
+   * mismatch once a turn happens to route to that specific model - a model configured only in a
+   * rarely-hit tier (or not yet routed to this session at all) could otherwise sit silently
+   * misconfigured indefinitely. This surfaces every mismatch in the config up front, in one
+   * notification, independent of whether anything has actually been routed yet.
+   */
+  function warnAboutUnsupportedConfiguredEfforts(
+    ctx: ExtensionContext,
+    settings: AutoRouterSettings,
+  ): void {
+    if (!ctx.hasUI) return;
+    const mismatches: string[] = [];
+    const seen = new Set<string>();
+    for (const tier of AUTO_ROUTER_EFFORT_ORDER) {
+      for (const ref of settings.efforts[tier]?.models ?? []) {
+        if (!ref.effort) continue;
+        const key = `${ref.provider}/${ref.id}:${ref.effort}`;
+        if (seen.has(key)) continue;
+        seen.add(key);
+        // Unresolvable here (no auth configured, wrong id, provider not registered, ...) is a
+        // separate, pre-existing failure mode already handled elsewhere (pickForTier's own
+        // fallback/notify path) - not this check's job to also report.
+        const model = ctx.modelRegistry.find(ref.provider, ref.id);
+        if (!model) continue;
+        const supported = getSupportedThinkingLevels(model);
+        if (supported.includes(ref.effort)) continue;
+        const clamped = clampThinkingLevel(model, ref.effort);
+        mismatches.push(
+          `${ref.provider}/${ref.id}: configured for "${ref.effort}" but only supports ${supported.join(", ")} (will run at "${clamped}")`,
+        );
+      }
+    }
+    if (mismatches.length === 0) return;
+    ctx.ui.notify(
+      `Auto: ${mismatches.length} configured model effort${mismatches.length === 1 ? "" : "s"} ${mismatches.length === 1 ? "isn't" : "aren't"} actually supported:\n${mismatches.map((line) => `  - ${line}`).join("\n")}`,
+      "warning",
+    );
+  }
+
   /** Pick the best available (resolved + healthy) model for `tier`, escalating to higher configured tiers when everything in `tier` is unhealthy, then falling back to the first available model anywhere as a last resort. */
   function pickForTier(
     ctx: ExtensionContext,
@@ -329,7 +402,7 @@ export default async function autoRouter(pi: ExtensionAPI): Promise<void> {
         }
         return;
       }
-      await pi.setThinkingLevel(effort);
+      await pi.setThinkingLevel(resolveSupportedEffort(ctx, model, effort));
     } finally {
       routingInFlight = false;
     }
@@ -369,7 +442,13 @@ export default async function autoRouter(pi: ExtensionAPI): Promise<void> {
         // 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");
+        // resolveSupportedEffort further clamps (and warns) if this model doesn't actually
+        // support that configured level at all.
+        const classifierEffort = resolveSupportedEffort(
+          ctx,
+          classifierModel,
+          resolveEffort(classifierRefs, classifierModel, "medium"),
+        );
         const result = await classifyTurnComplexity(
           ctx.modelRegistry,
           classifierModel,
@@ -491,6 +570,7 @@ export default async function autoRouter(pi: ExtensionAPI): Promise<void> {
       publishFooter();
     }
     const settings = await readAutoRouterSettings();
+    warnAboutUnsupportedConfiguredEfforts(ctx, settings);
     void reconcileAllProviders(ctx.modelRegistry, settings);
     void ensureAutoModelScopedInGlobalSettings().catch(() => undefined);
   });

tests/auto-router-classify.test.ts

@@ -230,11 +230,23 @@ test("classifyTurnComplexity caps output at CLASSIFY_MAX_TOKENS", async () => {
 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");
+  await classifyTurnComplexity(registry, CODEX_MODEL, "do something", false, "high");
 
   // 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
+  // effort: "high" for its tier should actually reason at high here too, not some unrelated
   // provider default.
+  expect(options()?.reasoningEffort).toBe("high");
+});
+
+test("classifyTurnComplexity trusts whatever reasoningEffort it's given verbatim, including \"max\"", async () => {
+  // Whether a model genuinely supports the requested level is the caller's job to resolve (via
+  // getSupportedThinkingLevels/clampThinkingLevel in auto-router.ts, which warns the user
+  // directly if their configured effort doesn't match the model's real capabilities) - not
+  // something this function should second-guess or silently substitute on its own.
+  const { registry, options } = registryCapturingOptions("medium");
+
+  await classifyTurnComplexity(registry, CODEX_MODEL, "do something", false, "max");
+
   expect(options()?.reasoningEffort).toBe("max");
 });
 

tests/auto-router-extension.test.ts

@@ -10,7 +10,6 @@ import type {
   Theme,
 } from "@earendil-works/pi-coding-agent";
 import autoRouter, { escapeTableCell } from "../extensions/auto-router.ts";
-import { SAVE_DEBOUNCE_MS } from "../extensions/auto-router-health.ts";
 import type { AutoRouterSettings } from "../extensions/auto-router-settings.ts";
 
 test("escapeTableCell neutralizes both pipes and line breaks, so one bad reply can't break the rest of the table", () => {
@@ -24,30 +23,20 @@ const ENV_VAR = "PI_CODING_AGENT_DIR";
 let agentDir: string | undefined;
 const usedDirs: string[] = [];
 
-// The extension's internal AutoRouterHealthStore debounces its writes (~2s after the last
-// record call), so a save scheduled by one test can fire well after that test's own teardown -
-// if `afterEach` restored PI_CODING_AGENT_DIR to its prior (usually unset) value in the
-// meantime, that late write would land in the real global agent directory instead of a test's
-// temp one. So the env var is never restored to anything other than a temp dir for the whole
-// run - only ever moved to a new one - and every temp dir used stays on disk until all tests
-// finish, so even a very late write can only ever land somewhere harmless.
+// Each AutoRouterHealthStore instance pins its target path at construction rather than
+// re-resolving PI_CODING_AGENT_DIR on every debounced flush, so a save scheduled by one test
+// stays pointed at that test's own temp dir no matter what this (process-wide) env var is set to
+// by the time the write actually fires - including by an unrelated later test or file. No
+// teardown coordination needed as a result; the temp dirs themselves are still kept around until
+// the whole run finishes and cleaned up together, purely so a slightly-delayed write always has
+// somewhere valid to land.
 beforeEach(async () => {
   agentDir = await mkdtemp(join(tmpdir(), "pi-kit-auto-router-agent-"));
   usedDirs.push(agentDir);
   process.env[ENV_VAR] = agentDir;
 });
 
 afterAll(async () => {
-  // The debounced save timer is unref'd, so it never blocks the process from exiting - but if
-  // this suite's own run happens to keep the process alive past SAVE_DEBOUNCE_MS anyway (e.g.
-  // a larger `bun test` invocation still running other files), a timer scheduled by one of this
-  // file's last tests can still fire *after* this hook would otherwise have already deleted
-  // PI_CODING_AGENT_DIR and removed its temp dir - at which point `statePath()` falls back to
-  // the real default `~/.pi/agent`, and the save actually corrupts the developer's real global
-  // auto-router-state.json with this suite's fixture data (verified: it happened). Waiting out
-  // the debounce window here first, before touching the env var or any directory, guarantees
-  // every such timer fires while it's still pointed at a real (about-to-be-removed) temp dir.
-  await new Promise((resolve) => setTimeout(resolve, SAVE_DEBOUNCE_MS + 500));
   delete process.env[ENV_VAR];
   await Promise.all(usedDirs.map((dir) => rm(dir, { recursive: true, force: true })));
 });
@@ -60,8 +49,17 @@ async function writeConfig(settings: AutoRouterSettings): Promise<void> {
   );
 }
 
+// reasoning: true plus explicit xhigh/max support so fixture models are "fully capable" by
+// default - these tests are about routing/escalation/pinning logic, not about effort-support
+// clamping specifically (that has its own dedicated tests below), and getSupportedThinkingLevels
+// otherwise excludes xhigh/max for any model without an explicit thinkingLevelMap entry for them.
 function model(provider: string, id: string): Model<Api> {
-  return { provider, id } as unknown as Model<Api>;
+  return {
+    provider,
+    id,
+    reasoning: true,
+    thinkingLevelMap: { xhigh: "xhigh", max: "max" },
+  } as unknown as Model<Api>;
 }
 
 const AUTO_PLACEHOLDER = model("auto", "auto");
@@ -439,6 +437,101 @@ test("a model's `effort` override sets its own thinking level, independent of th
   expect(notified).toContain("at max effort"); // classification log shows the real applied effort
 });
 
+test("routing to a model whose effort override it doesn't actually support clamps to what it does, and warns instead of silently substituting", async () => {
+  // Mirrors the real gpt-5.3-codex-spark case: reasoning-capable, and its own thinkingLevelMap
+  // confirms "xhigh" support but has no entry for "max" at all - so per pi-ai's own
+  // getSupportedThinkingLevels, this model does not actually support "max" despite it type-checking
+  // as a valid AutoRouterEffortLevel.
+  const spark = {
+    provider: "openai-codex",
+    id: "gpt-5.3-codex-spark",
+    reasoning: true,
+    thinkingLevelMap: { xhigh: "xhigh", minimal: "low" },
+  } as unknown as Model<Api>;
+  await writeConfig({
+    efforts: {
+      medium: {
+        models: [{ provider: "openai-codex", id: "gpt-5.3-codex-spark", effort: "max" }],
+      },
+    },
+  });
+
+  const fake = createFakePi();
+  await autoRouter(fake.pi);
+  const registry = fakeModelRegistry({ models: [spark], classify: () => "medium" });
+  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 and dispatches - never blocks the turn over this...
+  expect(fake.setModelCalls).toEqual([spark]);
+  // ...but clamps to what the model actually supports rather than sending "max" and getting an
+  // empty/broken response back (verified: this is exactly what was happening for real).
+  expect(fake.thinkingLevelCalls).toEqual(["xhigh"]);
+  // ...and the mismatch is surfaced twice, not silently papered over: once as a whole-config
+  // summary at session start (independent of whether anything routes there yet)...
+  const startupWarning = ctx.notifications.find(
+    (n) => n.type === "warning" && n.message.includes("configured model effort"),
+  );
+  expect(startupWarning?.message).toContain("gpt-5.3-codex-spark");
+  expect(startupWarning?.message).toContain('configured for "max"');
+  // ...and again, specifically, at the point this particular turn actually dispatched there.
+  const dispatchWarning = ctx.notifications.find(
+    (n) => n.type === "warning" && n.message.includes('doesn\'t support "max"'),
+  );
+  expect(dispatchWarning?.message).toContain("gpt-5.3-codex-spark");
+  expect(dispatchWarning?.message).toContain("xhigh");
+});
+
+test("session_start warns once per model+effort pair, even when it's configured in multiple tiers", async () => {
+  const spark = {
+    provider: "openai-codex",
+    id: "gpt-5.3-codex-spark",
+    reasoning: true,
+    thinkingLevelMap: { xhigh: "xhigh", minimal: "low" },
+  } as unknown as Model<Api>;
+  // Same model, same "max" override, configured in both low and medium - exactly the real
+  // gpt-5.3-codex-spark case.
+  await writeConfig({
+    efforts: {
+      low: { models: [{ provider: "openai-codex", id: "gpt-5.3-codex-spark", effort: "max" }] },
+      medium: { models: [{ provider: "openai-codex", id: "gpt-5.3-codex-spark", effort: "max" }] },
+    },
+  });
+
+  const fake = createFakePi();
+  await autoRouter(fake.pi);
+  const ctx = fakeCtx({ modelRegistry: fakeModelRegistry({ models: [spark] }), currentModel: fake.currentModel });
+
+  await fake.fire("session_start", {}, ctx);
+
+  const startupWarnings = ctx.notifications.filter((n) => n.message.includes("configured model effort"));
+  expect(startupWarnings).toHaveLength(1);
+  expect(startupWarnings[0]?.message).toContain("1 configured model effort isn't");
+});
+
+test("session_start does not warn when every configured effort override is genuinely supported", async () => {
+  const luna = {
+    provider: "openai-codex",
+    id: "gpt-5.6-luna",
+    reasoning: true,
+    thinkingLevelMap: { xhigh: "xhigh", max: "max", minimal: "low" },
+  } as unknown as Model<Api>;
+  await writeConfig({
+    efforts: { medium: { models: [{ provider: "openai-codex", id: "gpt-5.6-luna", effort: "max" }] } },
+  });
+
+  const fake = createFakePi();
+  await autoRouter(fake.pi);
+  const ctx = fakeCtx({ modelRegistry: fakeModelRegistry({ models: [luna] }), currentModel: fake.currentModel });
+
+  await fake.fire("session_start", {}, ctx);
+
+  expect(ctx.notifications.some((n) => n.message.includes("configured model effort"))).toBe(false);
+});
+
 test("a routed turn's classification is logged and shows up in /usage, so a routing decision can be checked against what the classifier actually said", async () => {
   const medium = model("prov", "medium-model");
   const high = model("prov", "high-model");