From 51da3add8328641c78bf86254eefaf890d9ecc1d Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 30 Jun 2026 14:28:07 +0000 Subject: [PATCH] fix(coding-agent): preserved auto thinking across plan approval Captured the configured thinking selector when entering plan mode so approving a plan restores auto instead of the provisional concrete effort. Reloaded DEFAULT(auto) badges from defaultThinkingLevel and covered the plan-approval handoff plus /model display. Fixes #3901 --- .../src/modes/components/model-selector.ts | 30 ++++++++++---- .../src/modes/interactive-mode.ts | 9 ++-- .../coding-agent/src/session/agent-session.ts | 2 +- .../test/interactive-mode-plan-review.test.ts | 41 +++++++++++++++++++ ...model-selector-role-badge-thinking.test.ts | 22 +++++++++- 5 files changed, 89 insertions(+), 15 deletions(-) diff --git a/packages/coding-agent/src/modes/components/model-selector.ts b/packages/coding-agent/src/modes/components/model-selector.ts index 4eed39f75..26a467502 100644 --- a/packages/coding-agent/src/modes/components/model-selector.ts +++ b/packages/coding-agent/src/modes/components/model-selector.ts @@ -24,7 +24,12 @@ import { getKnownRoleIds, getRoleInfo, MODEL_ROLE_IDS, MODEL_ROLES } from "../.. import type { Settings } from "../../config/settings"; import { type ThemeColor, theme } from "../../modes/theme/theme"; import { matchesSelectDown, matchesSelectUp } from "../../modes/utils/keybinding-matchers"; -import { AUTO_THINKING, type ConfiguredThinkingLevel, getConfiguredThinkingLevelMetadata } from "../../thinking"; +import { + AUTO_THINKING, + type ConfiguredThinkingLevel, + getConfiguredThinkingLevelMetadata, + parseConfiguredThinkingLevel, +} from "../../thinking"; import { getTabBarTheme } from "../shared"; import { DynamicBorder } from "./dynamic-border"; @@ -342,10 +347,7 @@ export class ModelSelectorComponent extends Container { if (resolved.model) { nextRoles[role] = { model: resolved.model, - thinkingLevel: - resolved.explicitThinkingLevel && resolved.thinkingLevel !== undefined - ? resolved.thinkingLevel - : ThinkingLevel.Inherit, + thinkingLevel: this.#getResolvedRoleThinkingLevel(role, resolved), autoSelected: false, }; } @@ -363,10 +365,7 @@ export class ModelSelectorComponent extends Container { if (!resolved.model) continue; nextRoles[role] = { model: resolved.model, - thinkingLevel: - resolved.explicitThinkingLevel && resolved.thinkingLevel !== undefined - ? resolved.thinkingLevel - : ThinkingLevel.Inherit, + thinkingLevel: this.#getResolvedRoleThinkingLevel(role, resolved), autoSelected: true, }; } @@ -1059,6 +1058,19 @@ export class ModelSelectorComponent extends Container { ); } } + #getResolvedRoleThinkingLevel( + role: string, + resolved: { explicitThinkingLevel: boolean; thinkingLevel?: ThinkingLevel }, + ): ConfiguredThinkingLevel { + if (resolved.explicitThinkingLevel && resolved.thinkingLevel !== undefined) { + return resolved.thinkingLevel; + } + if (role === "default") { + return parseConfiguredThinkingLevel(this.#settings.get("defaultThinkingLevel")) ?? ThinkingLevel.Inherit; + } + return ThinkingLevel.Inherit; + } + #getThinkingLevelsForModel(model: Model): ReadonlyArray { return [ThinkingLevel.Inherit, ThinkingLevel.Off, AUTO_THINKING, ...getSupportedEfforts(model)]; } diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index e62de17ee..b30f5e03f 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -100,6 +100,7 @@ import { formatDuration } from "../slash-commands/helpers/format"; import { STTController, type SttState } from "../stt"; import { discoverTitleSystemPromptFile, resolvePromptInput } from "../system-prompt"; import { formatTaskId } from "../task/render"; +import type { ConfiguredThinkingLevel } from "../thinking"; import type { LspStartupServerInfo } from "../tools"; import { normalizeLocalScheme } from "../tools/path-utils"; import { replaceTabs, TRUNCATE_LENGTHS, truncateToWidth } from "../tools/render-utils"; @@ -493,8 +494,8 @@ export class InteractiveMode implements InteractiveModeContext { #goalTurnHadToolCalls = false; #goalContinuationTurnInFlight = false; #goalSuppressNextContinuation = false; - #planModePreviousModelState: { model: Model; thinkingLevel?: ThinkingLevel } | undefined; - #pendingModelSwitch: { model: Model; thinkingLevel?: ThinkingLevel } | undefined; + #planModePreviousModelState: { model: Model; thinkingLevel?: ConfiguredThinkingLevel } | undefined; + #pendingModelSwitch: { model: Model; thinkingLevel?: ConfiguredThinkingLevel } | undefined; #planModeHasEntered = false; #planReviewOverlay: PlanReviewOverlay | undefined; #planReviewOverlayHandle: OverlayHandle | undefined; @@ -1917,7 +1918,7 @@ export class InteractiveMode implements InteractiveModeContext { const planThinkingLevel = resolved.explicitThinkingLevel ? resolved.thinkingLevel : undefined; this.#planModePreviousModelState = currentModel - ? { model: currentModel, thinkingLevel: this.session.thinkingLevel } + ? { model: currentModel, thinkingLevel: this.session.configuredThinkingLevel() } : undefined; if (!sameModel) { @@ -2125,7 +2126,7 @@ export class InteractiveMode implements InteractiveModeContext { }); } - async #restorePlanPreviousModel(prev: { model: Model; thinkingLevel?: ThinkingLevel }): Promise { + async #restorePlanPreviousModel(prev: { model: Model; thinkingLevel?: ConfiguredThinkingLevel }): Promise { if (modelsAreEqual(this.session.model, prev.model)) { // Same model — only thinking level may differ. Avoid setModelTemporary() // which would reset provider-side sessions and break continuity. diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index cf694a150..1c8251684 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -8087,7 +8087,7 @@ export class AgentSession { */ async setModelTemporary( model: Model, - thinkingLevel?: ThinkingLevel, + thinkingLevel?: ConfiguredThinkingLevel, options?: { ephemeral?: boolean }, ): Promise { const previousEditMode = this.#resolveActiveEditMode(); diff --git a/packages/coding-agent/test/interactive-mode-plan-review.test.ts b/packages/coding-agent/test/interactive-mode-plan-review.test.ts index c95071b11..a59fd2f2f 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -17,6 +17,7 @@ import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SILENT_ABORT_MARKER, USER_INTERRUPT_LABEL } from "@oh-my-pi/pi-coding-agent/session/messages"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { AUTO_THINKING } from "@oh-my-pi/pi-coding-agent/thinking"; import { setKeybindings } from "@oh-my-pi/pi-tui"; import { formatNumber, TempDir } from "@oh-my-pi/pi-utils"; @@ -836,6 +837,46 @@ describe("InteractiveMode plan review rendering", () => { expect(defaultApply?.[0]?.explicitThinkingLevel).toBe(true); }); + it("preserves DEFAULT(auto) when plan approval restores the default tier", async () => { + const sonnet = session.modelRegistry.find("anthropic", "claude-sonnet-4-5"); + const opus = session.modelRegistry.find("anthropic", "claude-opus-4-5"); + if (!sonnet || !opus) throw new Error("Expected sonnet + opus to exist in registry"); + + session.settings.setModelRole("default", "anthropic/claude-sonnet-4-5"); + session.settings.setModelRole("slow", "anthropic/claude-opus-4-5"); + session.settings.setModelRole("plan", "anthropic/claude-opus-4-5"); + session.setThinkingLevel(AUTO_THINKING, true); + + const planFilePath = "local://PLAN.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nPreserve the configured auto selector."); + + await mode.handlePlanModeCommand(); + expect(session.model?.id).toBe(opus.id); + + vi.spyOn(session, "getContextUsage").mockReturnValue(undefined); + vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); + + vi.spyOn(mode, "showPlanReview").mockImplementation( + async (_planContent, _title, _options, _dialogOptions, extra?: { slider?: HookSelectorSlider }) => { + const slider = extra?.slider; + expect(slider).toBeDefined(); + const defaultIndex = slider!.segments.findIndex(segment => segment.label === "default"); + expect(defaultIndex).toBeGreaterThanOrEqual(0); + slider!.onChange?.(defaultIndex); + return "Approve and keep context"; + }, + ); + + await mode.handlePlanApproval({ planFilePath, planExists: true, title: "PLAN" }); + + expect(session.model?.id).toBe(sonnet.id); + expect(session.configuredThinkingLevel()).toBe(AUTO_THINKING); + }); + it("falls back to the pre-plan model when only plan is configured and the slider is hidden", async () => { const sonnet = session.modelRegistry.find("anthropic", "claude-sonnet-4-5"); const opus = session.modelRegistry.find("anthropic", "claude-opus-4-5"); diff --git a/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts b/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts index 529908858..e11d2d8c6 100644 --- a/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts +++ b/packages/coding-agent/test/model-selector-role-badge-thinking.test.ts @@ -7,7 +7,7 @@ import type { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-regis import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { ModelSelectorComponent } from "@oh-my-pi/pi-coding-agent/modes/components/model-selector"; import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; -import type { ConfiguredThinkingLevel } from "@oh-my-pi/pi-coding-agent/thinking"; +import { AUTO_THINKING, type ConfiguredThinkingLevel } from "@oh-my-pi/pi-coding-agent/thinking"; import type { TUI } from "@oh-my-pi/pi-tui"; function normalizeRenderedText(text: string): string { @@ -156,6 +156,26 @@ describe("ModelSelector role badge thinking display", () => { expect(rendered).not.toContain("low medium high max"); }); + test("reloads DEFAULT(auto) from defaultThinkingLevel", async () => { + installTestTheme(); + const model = getBundledModel("openai", "gpt-5.5"); + if (!model) throw new Error("Expected bundled model openai/gpt-5.5"); + + const settings = Settings.isolated({ + defaultThinkingLevel: AUTO_THINKING, + modelRoles: { + default: `${model.provider}/${model.id}`, + }, + }); + + const selector = createSelector(model, settings); + await Bun.sleep(0); + installTestTheme(); + + const rendered = normalizeRenderedText(selector.render(220).join("\n")); + expect(rendered).toContain("DEFAULT (auto)"); + }); + test("shows compact auto badges for unconfigured role defaults", async () => { installTestTheme(); const settings = Settings.isolated({});