diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 94756279d..e7aef8031 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed plan approval applying the wrong execution model when the model-tier slider sat on the model that exit would restore. The match check now compares the selected role's effective thinking level against the pre-plan thinking level, so picking the active planning tier is retained and picking a same-model tier with an explicit thinking suffix (e.g. `default = sonnet:off` while plan-mode raised thinking to `high`) goes through `applyRoleModel` instead of silently restoring the pre-plan level. ([#3554](https://github.com/can1357/oh-my-pi/issues/3554)) +- Fixed plan approval applying the wrong execution model when the model-tier slider sat on the model that exit would restore. The match check now compares the selected role's effective thinking level against the pre-plan thinking level, and a singleton cycle (only `modelRoles.plan` configured, default unset, so `getRoleModelCycle` synthesizes a lone `default` entry from the active plan model and the slider stays hidden) is no longer pinned as the execution tier — approval falls through to the pre-plan restore instead of silently switching back to the plan model. ([#3554](https://github.com/can1357/oh-my-pi/issues/3554)) - Fixed the `eval` Julia kernel showing only runner-internal backtrace frames (`at top-level scope (./none:N)`, `at main (…runner-…jl:635)`) with no exception type or message, making cell errors undebuggable. The host renderer (`packages/coding-agent/src/eval/kernel-base.ts`) displays a non-empty `traceback` verbatim and only falls back to `ename: evalue` when it is empty; the Python and Ruby runners embed the rendered error in `traceback`, but the Julia runner (`packages/coding-agent/src/eval/jl/runner.jl`) built `traceback` from stack frames only, so the message was dropped. `emit_error` now seeds `traceback` with the `showerror` output (matching the REPL's `ERROR:` text) ahead of the frames. diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 4ed306b44..440324795 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -3022,6 +3022,13 @@ export class InteractiveMode implements InteractiveModeContext { // applies it AFTER #exitPlanMode. #exitPlanMode normally restores // #planModePreviousModelState (the model from before plan mode), so // applying the slider choice any earlier would be silently reverted. + // Pass executionModel only when the slider was actually shown — a + // singleton cycle (e.g. only modelRoles.plan is configured, so + // getRoleModelCycle synthesizes a lone `default` entry from the + // currently active plan model) hides the slider, the operator made + // no selection, and the pre-plan model is not in the cycle. Pinning + // that singleton would silently switch the session back to the plan + // model after #exitPlanMode restored the pre-plan model. // Treat the choice as implicit only when applying the selected role // would land on the same end state as the restore — same model AND // the same effective thinking level. A role with an explicit thinking @@ -3038,7 +3045,7 @@ export class InteractiveMode implements InteractiveModeContext { }) : -1; const executionModel = - cycle && selectedTierIndex !== restoredIndex ? cycle.models[selectedTierIndex] : undefined; + slider && cycle && selectedTierIndex !== restoredIndex ? cycle.models[selectedTierIndex] : undefined; await this.#approvePlan(latestPlanContent, { planFilePath, title: details.title, 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 55e0bc2e3..428e596e3 100644 --- a/packages/coding-agent/test/interactive-mode-plan-review.test.ts +++ b/packages/coding-agent/test/interactive-mode-plan-review.test.ts @@ -835,6 +835,46 @@ describe("InteractiveMode plan review rendering", () => { expect(defaultApply?.[0]?.explicitThinkingLevel).toBe(true); }); + 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"); + if (!sonnet || !opus) throw new Error("Expected sonnet + opus to exist in registry"); + expect(session.model?.id).toBe(sonnet.id); + + // Only the plan role is configured. getRoleModelCycle synthesizes a + // singleton `default` entry from the active plan model (opus), so the + // slider is hidden — the operator made no selection and approval must + // fall through to the pre-plan sonnet restore instead of pinning the + // lone plan tier. + session.settings.setModelRole("plan", "anthropic/claude-opus-4-5"); + + const planFilePath = "local://PLAN.md"; + const resolvedPlanPath = resolveLocalUrlToPath(planFilePath, { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }); + await Bun.write(resolvedPlanPath, "# Plan\n\nNo slider, restore default."); + + await mode.handlePlanModeCommand(); + expect(session.model?.id).toBe(opus.id); + + vi.spyOn(session, "getContextUsage").mockReturnValue(undefined); + vi.spyOn(session, "prompt").mockResolvedValue(undefined as never); + + let sliderShown: HookSelectorSlider | undefined; + vi.spyOn(mode, "showPlanReview").mockImplementation( + async (_planContent, _title, _options, _dialogOptions, extra?: { slider?: HookSelectorSlider }) => { + sliderShown = extra?.slider; + return "Approve and keep context"; + }, + ); + + await mode.handlePlanApproval({ planFilePath, planExists: true, title: "PLAN" }); + + expect(sliderShown).toBeUndefined(); + expect(session.model?.id).toBe(sonnet.id); + }); + it("compaction runs on the plan model and restores the pre-plan model after success", async () => { const planModel = session.modelRegistry.find("anthropic", "claude-opus-4-5"); const prePlanModel = session.modelRegistry.find("anthropic", "claude-sonnet-4-5");