From cb6832b055dc766052798fc69f24cad9ea0116b2 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:30:23 -0400 Subject: [PATCH 1/4] Fix the same real-state-corrupting test leak in auto-router-health.test.ts This file independently duplicates the PI_CODING_AGENT_DIR isolation pattern from auto-router-extension.test.ts (same debounced-save race, same explanatory comment) but didn't get the fix applied to that file earlier - and it just corrupted the real ~/.pi/agent/auto-router-state.json again, verified while investigating an unrelated question. Same fix: wait out AutoRouterHealthStore's SAVE_DEBOUNCE_MS in afterAll before deleting the env var or removing any temp dir. Co-Authored-By: Claude Sonnet 5 --- tests/auto-router-health.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/auto-router-health.test.ts b/tests/auto-router-health.test.ts index d019694..bd5d12b 100644 --- a/tests/auto-router-health.test.ts +++ b/tests/auto-router-health.test.ts @@ -12,6 +12,7 @@ import { modelKey, parseRetryAfterMs, pickHealthy, + SAVE_DEBOUNCE_MS, } from "../extensions/auto-router-health.ts"; const NOW = 1_000_000_000_000; @@ -34,6 +35,18 @@ beforeEach(async () => { }); 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 - twice, since + // this file duplicates the same env-var isolation as auto-router-extension.test.ts but didn't + // get this fix the first time). 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 }))); }); From 02c15935b9c61601ec1ac83d8b5f61c3c5324ed4 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:44:47 -0400 Subject: [PATCH 2/4] Clamp classifier reasoningEffort "max" per-model, and fix the real root cause of the state-file leak Two independent findings from a live retest after PR #11 merged: 1. The classifier still came back empty for gpt-5.3-codex-spark even with a valid, model-configured effort ("max") and the higher token cap. Root cause: Model.thinkingLevelMap's own doc says "missing keys use provider defaults" - in practice an unmapped level is forwarded to the raw API as its own literal name, and "max" is pi's own extended vocabulary that several models (this one included, per its own map only covering xhigh and minimal) don't actually understand despite it type-checking. This is exactly why the very first footer-badge mismatch existed: Pi's own real-turn dispatch already knows to clamp such a model down to what it actually supports (observably "xhigh"), but the classifier's raw completion call bypassed that entirely. Now mirrors it: "max" clamps to "xhigh" unless the model's own thinkingLevelMap explicitly confirms support. 2. My earlier per-test-file "wait out the debounce before teardown" fixes only protected each file against its *own* later teardown - they did nothing against a *different* test file's beforeEach changing the same process-wide PI_CODING_AGENT_DIR while an earlier file's save was still pending, which is exactly how the real ~/.pi/agent/auto-router-state.json kept getting corrupted with test fixture data even after those fixes landed. Root fix: AutoRouterHealthStore now pins its target path once, at construction, instead of re-resolving the env var on every debounced flush - making each instance immune to any later change to that var from anywhere, not just careful test teardown. This makes the ad-hoc per-file debounce waits unnecessary; removed them (verified corruption no longer reproduces, twice, with the real full test suite run twice against the actual global state file). Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 15 ++++++++++++++- extensions/auto-router-health.ts | 18 +++++++++++++++--- tests/auto-router-classify.test.ts | 29 +++++++++++++++++++++++++++-- tests/auto-router-extension.test.ts | 25 +++++++------------------ tests/auto-router-health.test.ts | 27 +++++++-------------------- 5 files changed, 70 insertions(+), 44 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index 08452c4..b102597 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -111,10 +111,23 @@ export async function classifyTurnComplexity( // "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 = + const safeEffort = reasoningEffort !== "off" && REASONING_EFFORT_SAFE_APIS.has(model.api) ? reasoningEffort : undefined; + // `Model.thinkingLevelMap`'s own doc: "Missing keys use provider defaults. null marks a level + // as unsupported." In practice that means an unmapped level gets forwarded to the API as its + // own literal name - fine for the standard low/medium/high/xhigh scale most providers + // recognize directly, but "max" is the one level several models (verified: this is exactly + // what was happening) don't actually understand even though it type-checks, since it only + // exists as pi's own extended vocabulary. Pi's own real-turn dispatch already knows this and + // clamps such a model down to whatever it actually supports (observably "xhigh" here) - this + // mirrors that, but only for "max" specifically and only when the model's own map doesn't + // explicitly confirm support for it, rather than guessing at every model's real ceiling. + const rawReasoningEffort = + safeEffort === "max" && model.thinkingLevelMap?.max == null + ? "xhigh" + : safeEffort; const response = await modelRegistry.complete( model, { diff --git a/extensions/auto-router-health.ts b/extensions/auto-router-health.ts index 73e4748..bb9f565 100644 --- a/extensions/auto-router-health.ts +++ b/extensions/auto-router-health.ts @@ -301,11 +301,23 @@ export class AutoRouterHealthStore { private state: AutoRouterHealthState = {}; private classifications: ClassificationLogEntry[] = []; private writeTimer: ReturnType | 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 { 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 { - 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); } diff --git a/tests/auto-router-classify.test.ts b/tests/auto-router-classify.test.ts index 79b0175..6543df0 100644 --- a/tests/auto-router-classify.test.ts +++ b/tests/auto-router-classify.test.ts @@ -15,6 +15,10 @@ const UNLISTED_API_MODEL = { id: "classifier", api: "anthropic-messages", } as unknown as Model; +const CODEX_MODEL_WITH_MAX_SUPPORT = { + ...CODEX_MODEL, + thinkingLevelMap: { max: "max" }, +} as unknown as Model; function registryReplying( text: string, @@ -230,11 +234,32 @@ 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 clamps \"max\" to \"xhigh\" when the model's thinkingLevelMap doesn't confirm support for it", async () => { + const { registry, options } = registryCapturingOptions("medium"); + + // CODEX_MODEL has no thinkingLevelMap at all - exactly the real-world case that produced empty + // classifier replies: "max" type-checks but isn't necessarily a level this specific model + // understands, unlike the standard low/medium/high/xhigh scale most providers recognize + // directly. Pi's own real-turn dispatch already clamps such a model down to "xhigh" - this + // mirrors that instead of blindly forwarding an unconfirmed value. + await classifyTurnComplexity(registry, CODEX_MODEL, "do something", false, "max"); + + expect(options()?.reasoningEffort).toBe("xhigh"); +}); + +test("classifyTurnComplexity does not clamp \"max\" when the model's thinkingLevelMap explicitly confirms support for it", async () => { + const { registry, options } = registryCapturingOptions("medium"); + + await classifyTurnComplexity(registry, CODEX_MODEL_WITH_MAX_SUPPORT, "do something", false, "max"); + expect(options()?.reasoningEffort).toBe("max"); }); diff --git a/tests/auto-router-extension.test.ts b/tests/auto-router-extension.test.ts index 51d3ddc..e22625d 100644 --- a/tests/auto-router-extension.test.ts +++ b/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,13 +23,13 @@ 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); @@ -38,16 +37,6 @@ beforeEach(async () => { }); 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 }))); }); diff --git a/tests/auto-router-health.test.ts b/tests/auto-router-health.test.ts index bd5d12b..cec8f84 100644 --- a/tests/auto-router-health.test.ts +++ b/tests/auto-router-health.test.ts @@ -12,7 +12,6 @@ import { modelKey, parseRetryAfterMs, pickHealthy, - SAVE_DEBOUNCE_MS, } from "../extensions/auto-router-health.ts"; const NOW = 1_000_000_000_000; @@ -21,13 +20,13 @@ const ENV_VAR = "PI_CODING_AGENT_DIR"; let agentDir: string | undefined; const usedDirs: string[] = []; -// 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-health-")); usedDirs.push(agentDir); @@ -35,18 +34,6 @@ beforeEach(async () => { }); 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 - twice, since - // this file duplicates the same env-var isolation as auto-router-extension.test.ts but didn't - // get this fix the first time). 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 }))); }); From 2b11cc960a4c38760bfb4f74fec22e3da03715e3 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:02:52 -0400 Subject: [PATCH 3/4] Warn instead of silently clamping unsupported effort, using pi-ai's own capability check The previous fix quietly clamped a classifier's "max" request down to "xhigh" with a hand-rolled, "max"-only heuristic - working around the symptom without telling the user their config asked for something the model can't actually do. Replaced with the real thing: pi-ai exports getSupportedThinkingLevels()/clampThinkingLevel(), the exact same capability resolution real turn dispatch already relies on (this is why real turns silently ran spark at "xhigh" instead of the configured "max" all along, invisibly). Added resolveSupportedEffort() in auto-router.ts, used at both call sites that dispatch a model at a configured effort: - applyRouting(), before pi.setThinkingLevel() - the real per-turn dispatch - routeForPrompt()'s classifier effort resolution Whenever the configured effort isn't in the model's actual supported set, it now warns explicitly (model, requested effort, what it does support, what's being used instead) rather than substituting silently. This surfaces the mismatch in general - not just for the classifier's "max" case that started this investigation, and not just when it happens to produce an empty reply. auto-router-classify.ts no longer does its own clamping: the caller guarantees an effort the model actually supports before calling it, so classifyTurnComplexity just trusts what it's given (still gating the raw reasoningEffort field to APIs verified to accept it at all). Also fixed the extension test suite's shared `model()` fixture helper, which didn't set `reasoning: true` or a thinkingLevelMap - meaning every existing fixture model looked entirely non-reasoning to getSupportedThinkingLevels and broke 6 unrelated tests. Now defaults to "fully capable" (reasoning: true, xhigh/max both mapped) so existing routing/escalation tests are unaffected; the new clamp-and-warn test uses its own narrower fixture modeled directly on the real spark model. Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router-classify.ts | 21 ++++-------- extensions/auto-router.ts | 44 ++++++++++++++++++++++-- tests/auto-router-classify.test.ts | 23 +++---------- tests/auto-router-extension.test.ts | 52 ++++++++++++++++++++++++++++- 4 files changed, 104 insertions(+), 36 deletions(-) diff --git a/extensions/auto-router-classify.ts b/extensions/auto-router-classify.ts index b102597..306183b 100644 --- a/extensions/auto-router-classify.ts +++ b/extensions/auto-router-classify.ts @@ -108,26 +108,19 @@ 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. - const safeEffort = + const rawReasoningEffort = reasoningEffort !== "off" && REASONING_EFFORT_SAFE_APIS.has(model.api) ? reasoningEffort : undefined; - // `Model.thinkingLevelMap`'s own doc: "Missing keys use provider defaults. null marks a level - // as unsupported." In practice that means an unmapped level gets forwarded to the API as its - // own literal name - fine for the standard low/medium/high/xhigh scale most providers - // recognize directly, but "max" is the one level several models (verified: this is exactly - // what was happening) don't actually understand even though it type-checks, since it only - // exists as pi's own extended vocabulary. Pi's own real-turn dispatch already knows this and - // clamps such a model down to whatever it actually supports (observably "xhigh" here) - this - // mirrors that, but only for "max" specifically and only when the model's own map doesn't - // explicitly confirm support for it, rather than guessing at every model's real ceiling. - const rawReasoningEffort = - safeEffort === "max" && model.thinkingLevelMap?.max == null - ? "xhigh" - : safeEffort; const response = await modelRegistry.complete( model, { diff --git a/extensions/auto-router.ts b/extensions/auto-router.ts index b394aae..a220184 100644 --- a/extensions/auto-router.ts +++ b/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,33 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { 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, + 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; + } + /** 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 +361,7 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { } return; } - await pi.setThinkingLevel(effort); + await pi.setThinkingLevel(resolveSupportedEffort(ctx, model, effort)); } finally { routingInFlight = false; } @@ -369,7 +401,13 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { // 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, diff --git a/tests/auto-router-classify.test.ts b/tests/auto-router-classify.test.ts index 6543df0..1c163c9 100644 --- a/tests/auto-router-classify.test.ts +++ b/tests/auto-router-classify.test.ts @@ -15,10 +15,6 @@ const UNLISTED_API_MODEL = { id: "classifier", api: "anthropic-messages", } as unknown as Model; -const CODEX_MODEL_WITH_MAX_SUPPORT = { - ...CODEX_MODEL, - thinkingLevelMap: { max: "max" }, -} as unknown as Model; function registryReplying( text: string, @@ -242,24 +238,15 @@ test("classifyTurnComplexity passes the requested reasoningEffort through for a expect(options()?.reasoningEffort).toBe("high"); }); -test("classifyTurnComplexity clamps \"max\" to \"xhigh\" when the model's thinkingLevelMap doesn't confirm support for it", async () => { +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"); - // CODEX_MODEL has no thinkingLevelMap at all - exactly the real-world case that produced empty - // classifier replies: "max" type-checks but isn't necessarily a level this specific model - // understands, unlike the standard low/medium/high/xhigh scale most providers recognize - // directly. Pi's own real-turn dispatch already clamps such a model down to "xhigh" - this - // mirrors that instead of blindly forwarding an unconfirmed value. await classifyTurnComplexity(registry, CODEX_MODEL, "do something", false, "max"); - expect(options()?.reasoningEffort).toBe("xhigh"); -}); - -test("classifyTurnComplexity does not clamp \"max\" when the model's thinkingLevelMap explicitly confirms support for it", async () => { - const { registry, options } = registryCapturingOptions("medium"); - - await classifyTurnComplexity(registry, CODEX_MODEL_WITH_MAX_SUPPORT, "do something", false, "max"); - expect(options()?.reasoningEffort).toBe("max"); }); diff --git a/tests/auto-router-extension.test.ts b/tests/auto-router-extension.test.ts index e22625d..f3d8b08 100644 --- a/tests/auto-router-extension.test.ts +++ b/tests/auto-router-extension.test.ts @@ -49,8 +49,17 @@ async function writeConfig(settings: AutoRouterSettings): Promise { ); } +// 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 { - return { provider, id } as unknown as Model; + return { + provider, + id, + reasoning: true, + thinkingLevelMap: { xhigh: "xhigh", max: "max" }, + } as unknown as Model; } const AUTO_PLACEHOLDER = model("auto", "auto"); @@ -428,6 +437,47 @@ 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; + 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, not silently papered over. + const warning = ctx.notifications.find( + (n) => n.type === "warning" && n.message.includes("gpt-5.3-codex-spark"), + ); + expect(warning?.message).toContain('doesn\'t support "max"'); + expect(warning?.message).toContain("xhigh"); +}); + 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"); From b5adaca7fa02d36a603313bdaeee94899e155129 Mon Sep 17 00:00:00 2001 From: Ian Walter <122028+ianwalter@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:12:16 -0400 Subject: [PATCH 4/4] Validate the whole autoRouter config's effort overrides at session start resolveSupportedEffort 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 one nothing has routed to yet this session, could otherwise sit silently misconfigured indefinitely. warnAboutUnsupportedConfiguredEfforts now checks every model+effort override in the config once at session start (deduplicated across tiers) and reports every mismatch in one notification, independent of routing activity. Verified against the real ~/.pi/agent/settings.json: correctly flags gpt-5.3-codex-spark, minimax/MiniMax-M3, and zai/glm-5.3 (all configured for "max" without actually supporting it) while correctly leaving gpt-5.6-luna, gpt-5.6-sol, zai/glm-5.2, and opencode-go/kimi-k3 alone (all genuinely support their configured effort). Co-Authored-By: Claude Sonnet 5 --- extensions/auto-router.ts | 42 +++++++++++++++++++ tests/auto-router-extension.test.ts | 64 ++++++++++++++++++++++++++--- 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/extensions/auto-router.ts b/extensions/auto-router.ts index a220184..47ddbfb 100644 --- a/extensions/auto-router.ts +++ b/extensions/auto-router.ts @@ -261,6 +261,47 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { 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(); + 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, @@ -529,6 +570,7 @@ export default async function autoRouter(pi: ExtensionAPI): Promise { publishFooter(); } const settings = await readAutoRouterSettings(); + warnAboutUnsupportedConfiguredEfforts(ctx, settings); void reconcileAllProviders(ctx.modelRegistry, settings); void ensureAutoModelScopedInGlobalSettings().catch(() => undefined); }); diff --git a/tests/auto-router-extension.test.ts b/tests/auto-router-extension.test.ts index f3d8b08..2087466 100644 --- a/tests/auto-router-extension.test.ts +++ b/tests/auto-router-extension.test.ts @@ -470,12 +470,66 @@ test("routing to a model whose effort override it doesn't actually support clamp // ...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, not silently papered over. - const warning = ctx.notifications.find( - (n) => n.type === "warning" && n.message.includes("gpt-5.3-codex-spark"), + // ...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(warning?.message).toContain('doesn\'t support "max"'); - expect(warning?.message).toContain("xhigh"); + 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; + // 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; + 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 () => {