diff --git a/apps/cli/src/ui/interactive-mode.ts b/apps/cli/src/ui/interactive-mode.ts index 0190a91b..dd044edb 100644 --- a/apps/cli/src/ui/interactive-mode.ts +++ b/apps/cli/src/ui/interactive-mode.ts @@ -95,7 +95,6 @@ import { type SessionEntry, SessionImportFileNotFoundError, SessionManager, - STEP_PROVIDER_ID, type StepLoginHost, sessionEntryToContextMessages, setRegisteredThemes, @@ -143,7 +142,7 @@ import { TuiMainScreen, visibleWidth, } from "@step-harness/pi-tui"; -import type { AuthEvent, AuthPrompt, ImageContent } from "@step-harness/providers"; +import { type AuthEvent, type AuthPrompt, clampThinkingLevel, type ImageContent } from "@step-harness/providers"; import type { AssistantMessage, Message, Model, Usage } from "@step-harness/providers/compat"; import chalk from "chalk"; import { spawn } from "child_process"; @@ -4775,12 +4774,11 @@ export class InteractiveMode { }; const availableLevels = this.session.getAvailableThinkingLevels(); const globalDefault = this.settingsManager.getDefaultThinkingLevel() ?? DEFAULT_THINKING_LEVEL; - // Step models default to their highest supported effort, so mark that as - // the default rather than the global level (which may not be selectable). - const defaultMarker = - this.session.model?.provider === STEP_PROVIDER_ID - ? (availableLevels[availableLevels.length - 1] ?? globalDefault) - : globalDefault; + const model = this.session.model; + const savedDefault = model + ? (this.settingsManager.getModelThinkingLevel(model.provider, model.id) ?? globalDefault) + : globalDefault; + const defaultMarker = model ? clampThinkingLevel(model, savedDefault) : savedDefault; const selector = new ThinkingSelectorComponent( this.session.thinkingLevel ?? DEFAULT_THINKING_LEVEL, availableLevels, diff --git a/apps/cli/test/thinking-default-marker.test.ts b/apps/cli/test/thinking-default-marker.test.ts new file mode 100644 index 00000000..96a078d4 --- /dev/null +++ b/apps/cli/test/thinking-default-marker.test.ts @@ -0,0 +1,60 @@ +import type { ThinkingLevel } from "@step-harness/agent-core"; +import type { Model } from "@step-harness/providers"; +import { beforeAll, describe, expect, it, vi } from "vitest"; +import { SettingsManager } from "../../../packages/coding-agent/src/core/settings-manager.ts"; +import { stepThinkingLevelMap } from "../../../packages/coding-agent/src/features/step-provider/index.ts"; +import { initTheme } from "../../../packages/coding-agent/src/theme/theme.ts"; +import { stripAnsi } from "../../../packages/coding-agent/src/utils/ansi.ts"; +import { stepModel } from "../../../packages/coding-agent/test/utilities.ts"; +import { InteractiveMode } from "../src/ui/interactive-mode.ts"; +import type { ThinkingSelectorComponent } from "../src/ui/view/dialogs/thinking-selector.ts"; + +const showThinkingSelector = Reflect.get(InteractiveMode.prototype, "showThinkingSelector") as (this: object) => void; + +function renderSelector(settingsManager: SettingsManager, model: Model, levels: ThinkingLevel[]): string[] { + let selector!: ThinkingSelectorComponent; + showThinkingSelector.call({ + session: { model, thinkingLevel: "high", getAvailableThinkingLevels: () => levels }, + settingsManager, + selectThinkingLevel: vi.fn(), + showSelector(factory: (done: () => void) => { component: ThinkingSelectorComponent }) { + selector = factory(() => {}).component; + }, + }); + return selector.render(100).map(stripAnsi); +} + +describe("thinking selector default marker", () => { + beforeAll(() => initTheme("dark")); + + it("marks the saved global default for Step models", () => { + const lines = renderSelector( + SettingsManager.inMemory({ defaultThinkingLevel: "medium" }), + stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "medium", "high"]) }), + ["low", "medium", "high"], + ); + expect(lines.some((line) => line.includes("medium") && line.includes("default"))).toBe(true); + expect(lines.some((line) => line.includes("high") && line.includes("default"))).toBe(false); + }); + + it("marks the model-specific default ahead of the global default", () => { + const lines = renderSelector( + SettingsManager.inMemory({ + defaultThinkingLevel: "high", + modelThinkingLevels: { "step/step-5-preview": "low" }, + }), + stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "medium", "high"]) }), + ["low", "medium", "high"], + ); + expect(lines.some((line) => line.includes("low") && line.includes("default"))).toBe(true); + }); + + it("marks the supported level used when the saved default must be clamped", () => { + const lines = renderSelector( + SettingsManager.inMemory({ defaultThinkingLevel: "medium" }), + stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "high"]) }), + ["low", "high"], + ); + expect(lines.some((line) => line.includes("high") && line.includes("default"))).toBe(true); + }); +}); diff --git a/packages/coding-agent/src/features/step.ts b/packages/coding-agent/src/features/step.ts index 2c51567d..4c3ef1c1 100644 --- a/packages/coding-agent/src/features/step.ts +++ b/packages/coding-agent/src/features/step.ts @@ -1,5 +1,5 @@ import process from "node:process"; -import { type Api, getSupportedThinkingLevels, type Model } from "@step-harness/providers"; +import { type Api, clampThinkingLevel, type Model } from "@step-harness/providers"; import type { ExtensionAPI, ExtensionCommandContext, @@ -54,14 +54,13 @@ export interface StepExtensionOptions { * carries `reasoning_effort_support_list` on some profiles (e.g. step_plan); when * it does not (e.g. platform), fetch the per-model detail from the domain-root * `/v1`. Mutates the model in place — the session's active model is the same - * object the picker reads. When `applyDefault` is set (fresh activation, not a - * restore), also default the level to the highest supported effort. + * object the picker reads. Keep the session's resolved thinking preference; + * capability discovery may only clamp a level the active model cannot support. */ async function enrichStepModelEffort( model: Model | undefined, ctx: ExtensionContext, pi: ExtensionAPI, - applyDefault: boolean, ): Promise { try { if (!model || model.provider !== STEP_PROVIDER_ID) return; @@ -80,11 +79,12 @@ async function enrichStepModelEffort( } } } - if (applyDefault) { - const supported = getSupportedThinkingLevels(model).filter((level) => level !== "off"); - const highest = supported[supported.length - 1]; - if (highest) pi.setThinkingLevel(highest); - } + // Discovery may finish after a model switch or a user effort change. Only + // validate the still-active model, using the latest session preference. + if (ctx.model !== model) return; + const selected = pi.getThinkingLevel(); + const supported = clampThinkingLevel(model, selected); + if (supported !== selected) pi.setThinkingLevel(supported); } catch { // Best-effort; the model keeps its existing thinking-level defaults. } @@ -100,12 +100,11 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi // Fire-and-forget; failures leave the model's defaults untouched. // (The initial model catalog is refreshed by the launcher before the // session's model is resolved, so it is already available here.) - pi.on("session_start", (event, ctx) => { - const fresh = event.reason === "startup" || event.reason === "new"; - void enrichStepModelEffort(ctx.model, ctx, pi, fresh); + pi.on("session_start", (_event, ctx) => { + void enrichStepModelEffort(ctx.model, ctx, pi); }); pi.on("model_select", (event, ctx) => { - void enrichStepModelEffort(event.model, ctx, pi, event.source !== "restore"); + void enrichStepModelEffort(event.model, ctx, pi); }); // Declarative plugins (including StepPage) contribute MCP servers. The // bridge is loaded as part of the Step product extension so ordinary Pi diff --git a/packages/coding-agent/test/step-thinking-preferences.test.ts b/packages/coding-agent/test/step-thinking-preferences.test.ts new file mode 100644 index 00000000..e54819a6 --- /dev/null +++ b/packages/coding-agent/test/step-thinking-preferences.test.ts @@ -0,0 +1,201 @@ +import { mkdirSync, mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ThinkingLevel } from "@step-harness/agent-core"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { parseArgs } from "../src/cli/args.ts"; +import type { AgentSession } from "../src/core/agent-session.ts"; +import { AuthStorage } from "../src/core/auth-storage.ts"; +import type { ExtensionAPI, ExtensionContext } from "../src/core/extensions/types.ts"; +import { createAgentSession } from "../src/core/sdk.ts"; +import { SessionManager } from "../src/core/session-manager.ts"; +import { SettingsManager } from "../src/core/settings-manager.ts"; +import { createStepExtension } from "../src/features/step.ts"; +import { stepThinkingLevelMap } from "../src/features/step-provider/index.ts"; +import { createStepSettingsManager } from "../src/step/settings-manager.ts"; +import { createInMemoryModelRegistry, getModelRuntime } from "./model-runtime-test-utils.ts"; +import { createTestResourceLoader, stepModel } from "./utilities.ts"; + +const roots: string[] = []; +const sessions: AgentSession[] = []; +function fixture() { + const root = mkdtempSync(join(tmpdir(), "step-thinking-preferences-")); + roots.push(root); + const cwd = join(root, "project"); + const agentDir = join(root, "agent"); + mkdirSync(cwd, { recursive: true }); + mkdirSync(agentDir, { recursive: true }); + return { root, cwd, agentDir }; +} +async function makeSession(f: ReturnType, settings: SettingsManager, thinkingLevel?: ThinkingLevel) { + const registry = await createInMemoryModelRegistry(AuthStorage.inMemory()); + const result = await createAgentSession({ + cwd: f.cwd, + agentDir: f.agentDir, + settingsManager: settings, + sessionManager: SessionManager.inMemory(f.cwd), + resourceLoader: createTestResourceLoader(), + modelRuntime: getModelRuntime(registry), + model: stepModel({ thinkingLevelMap: stepThinkingLevelMap(["low", "medium", "high"]) }), + thinkingLevel, + noTools: "all", + }); + sessions.push(result.session); + return result.session; +} +function stepEffortHooks(session: AgentSession) { + const on = vi.fn(); + createStepExtension({ permission: { env: {} } })({ + registerProvider: vi.fn(), + registerCommand: vi.fn(), + sendUserMessage: vi.fn(), + on, + setThinkingLevel: (level: ThinkingLevel) => session.setThinkingLevel(level), + getThinkingLevel: () => session.thinkingLevel, + } as unknown as ExtensionAPI); + const ctx = { + get model() { + return session.model; + }, + get thinkingLevel() { + return session.thinkingLevel; + }, + modelRegistry: { + getApiKeyAndHeaders: async () => ({ ok: true, apiKey: "test-key" }), + }, + } as unknown as ExtensionContext; + return { + async start(reason = "startup") { + // The first registered session_start handler is the real effort enrichment hook. + await on.mock.calls.find(([name]) => name === "session_start")![1]({ type: "session_start", reason }, ctx); + }, + async select() { + await on.mock.calls.find(([name]) => name === "model_select")![1]( + { type: "model_select", model: session.model, source: "set" }, + ctx, + ); + }, + }; +} +afterEach(() => { + vi.restoreAllMocks(); + for (const session of sessions.splice(0)) session.dispose(); + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); +}); + +describe("Step thinking preferences", () => { + it("CLI parsing and the core SDK honor --thinking medium before the Step hook", async () => { + const f = fixture(); + const parsed = parseArgs(["--thinking", "medium"]); + expect(parsed.thinking).toBe("medium"); + const session = await makeSession(f, SettingsManager.inMemory({ defaultThinkingLevel: "high" }), parsed.thinking); + expect(session.thinkingLevel).toBe("medium"); + }); + + it("Step startup must preserve the explicit --thinking medium option", async () => { + const f = fixture(); + const parsed = parseArgs(["--thinking", "medium"]); + const session = await makeSession(f, SettingsManager.inMemory({ defaultThinkingLevel: "high" }), parsed.thinking); + expect(session.thinkingLevel).toBe("medium"); + await stepEffortHooks(session).start(); + expect(session.thinkingLevel).toBe("medium"); + }); + + it("a saved default survives disk roundtrip and must still apply after startup", async () => { + const f = fixture(); + const saved = createStepSettingsManager(f.cwd, f.agentDir); + saved.setDefaultThinkingLevel("medium"); + await saved.flush(); + const reloaded = createStepSettingsManager(f.cwd, f.agentDir); + expect(reloaded.getDefaultThinkingLevel()).toBe("medium"); + const session = await makeSession(f, reloaded); + expect(session.thinkingLevel).toBe("medium"); + await stepEffortHooks(session).start(); + expect(reloaded.getDefaultThinkingLevel()).toBe("medium"); + expect(session.thinkingLevel).toBe("medium"); + }); + + it("Step startup must preserve a saved per-model thinking preference", async () => { + const f = fixture(); + const saved = createStepSettingsManager(f.cwd, f.agentDir); + saved.setDefaultThinkingLevel("high"); + saved.setModelThinkingLevel("step", "step-5-preview", "low"); + await saved.flush(); + const reloaded = createStepSettingsManager(f.cwd, f.agentDir); + const session = await makeSession(f, reloaded); + expect(session.thinkingLevel).toBe("low"); + await stepEffortHooks(session).start(); + expect(session.thinkingLevel).toBe("low"); + }); + + it("model_select must preserve the effort already resolved by the session", async () => { + const f = fixture(); + const session = await makeSession(f, SettingsManager.inMemory(), "medium"); + await stepEffortHooks(session).select(); + expect(session.thinkingLevel).toBe("medium"); + }); + + it("an explicit resume lifecycle does not trigger the override", async () => { + const f = fixture(); + const session = await makeSession(f, SettingsManager.inMemory(), "medium"); + await stepEffortHooks(session).start("resume"); + expect(session.thinkingLevel).toBe("medium"); + }); + + it("preserves a newer user choice while capability discovery is pending", async () => { + const f = fixture(); + const session = await makeSession(f, SettingsManager.inMemory(), "medium"); + const model = session.model!; + delete model.thinkingLevelMap; + let respond!: (response: Response) => void; + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( + () => + new Promise((resolve) => { + respond = resolve; + }), + ); + await stepEffortHooks(session).start(); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce()); + session.setThinkingLevel("low"); + respond(new Response(JSON.stringify({ reasoning_effort_support_list: ["low", "medium", "high"] }))); + await vi.waitFor(() => expect(model.thinkingLevelMap).toBeDefined()); + expect(session.thinkingLevel).toBe("low"); + }); + + it("does not change the active model's effort when an earlier model's discovery finishes", async () => { + const f = fixture(); + const session = await makeSession(f, SettingsManager.inMemory(), "medium"); + const previousModel = session.model!; + delete previousModel.thinkingLevelMap; + let respond!: (response: Response) => void; + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation( + () => + new Promise((resolve) => { + respond = resolve; + }), + ); + await stepEffortHooks(session).start(); + await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce()); + session.agent.state.model = stepModel({ + id: "other-model", + thinkingLevelMap: stepThinkingLevelMap(["low", "medium"]), + }); + session.setThinkingLevel("low"); + respond(new Response(JSON.stringify({ reasoning_effort_support_list: ["high"] }))); + await vi.waitFor(() => expect(previousModel.thinkingLevelMap).toBeDefined()); + expect(session.thinkingLevel).toBe("low"); + }); + + it("clamps only unsupported choices after discovering the active model's capabilities", async () => { + const f = fixture(); + const session = await makeSession(f, SettingsManager.inMemory(), "medium"); + const model = session.model!; + delete model.thinkingLevelMap; + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ reasoning_effort_support_list: ["low", "high"] })), + ); + await stepEffortHooks(session).start(); + await vi.waitFor(() => expect(model.thinkingLevelMap).toBeDefined()); + expect(session.thinkingLevel).toBe("high"); + }); +});