mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): cycle to the only model in scope when the active model is outside it (#11832)
Ctrl+P did nothing when the scope held exactly one model and the active
model was a different one. It showed "Only one model in scope" and kept
the current model, because `_cycleScopedModel` returned early for any
list with one entry. `_cycleAvailableModel` had the same early return,
so it also failed when the active model was no longer available and one
other model was.
A second bug had the same cause. When the active model was not in the
list, the code treated it as if it were at index 0, so cycling forward
skipped the first scoped model. With `--models A,B` and another model
active, Ctrl+P went to B.
This change adds a small `getNextCycleIndex` helper that both functions
now use:
- When the active model is not in the list, cycling forward selects the
first entry and cycling backward selects the last.
- It returns `undefined` (the "Only one model…" message) only when the
list is empty, or when its single entry is already the active model.
**Testing**
- Added a regression test to
`test/suite/agent-session-model-extension.test.ts`. It covers a
one-model scope, a repeated cycle once that model is active, and forward
and backward cycling into a two-model scope from outside it. The test
fails without the change ("expected undefined to be 'faux-2'") and
passes with it.
- `npm run check` passes.
- Manual check: `pi --model anthropic/claude-sonnet-4-5 --models
anthropic/claude-opus-4-5`, then Ctrl+P.
- Before: "Only one model in scope".
- After: "Switched to Claude Opus 4.5".
- With `--models anthropic/claude-opus-4-5,anthropic/claude-haiku-4-5`,
Ctrl+P now selects opus instead of haiku.
Fixes #11806
Requested by @davidbrai
Co-authored-by: triagent <triagent@localhost>
Co-authored-by: David Brailovsky <davidbrai@gmail.com>
This commit is contained in:
parent
469d8651b5
commit
5732bac060
3 changed files with 52 additions and 14 deletions
|
|
@ -32,6 +32,7 @@
|
|||
- Fixed grep match lines, codemode output, compaction summaries of tool results, and MCP error and server summaries cutting an emoji or other non-BMP character in half when truncating, which left an invalid lone surrogate in the text ([#7121](https://github.com/earendil-works/pi/issues/7121))
|
||||
- Fixed the read tool loading whole files into memory, which ran out of memory on large files even with `offset`/`limit`; it now streams the file and keeps only the shown lines. Custom `ReadOperations` can stream too by providing the new optional `createReadStream` ([#9654](https://github.com/earendil-works/pi/issues/9654))
|
||||
- Fixed OAuth logins such as `openai` and `anthropic` failing with `invalid_grant` after pi was stopped while it refreshed an expired token, for example an RPC client that stops pi right after startup while the background catalog refresh runs. The token endpoint had already replaced the refresh token, but pi exited before saving the new one. On exit, pi now waits for a token refresh in progress to be saved ([#11692](https://github.com/earendil-works/pi/issues/11692))
|
||||
- Fixed Ctrl+P not switching to the only model in scope (or the only available model) when the active model is a different one; it showed "Only one model in scope" instead. Cycling forward from a model outside the scope now also selects the first scoped model instead of skipping it ([#11806](https://github.com/earendil-works/pi/issues/11806))
|
||||
|
||||
|
||||
## [1.5.0] - 2026-11-02
|
||||
|
|
|
|||
|
|
@ -390,6 +390,22 @@ function estimateMessagesTokens(messages: AgentMessage[]): number {
|
|||
// AgentSession Class
|
||||
// ============================================================================
|
||||
|
||||
/**
|
||||
* Index of the model to cycle to, or undefined when there is nothing to switch to.
|
||||
* When the current model is not in the list (currentIndex -1), forward picks the first
|
||||
* entry and backward the last, so a one-entry list still selects its only model.
|
||||
*/
|
||||
function getNextCycleIndex(
|
||||
currentIndex: number,
|
||||
length: number,
|
||||
direction: "forward" | "backward",
|
||||
): number | undefined {
|
||||
if (length === 0) return undefined;
|
||||
if (currentIndex === -1) return direction === "forward" ? 0 : length - 1;
|
||||
if (length === 1) return undefined;
|
||||
return direction === "forward" ? (currentIndex + 1) % length : (currentIndex - 1 + length) % length;
|
||||
}
|
||||
|
||||
export class AgentSession {
|
||||
readonly agent: Agent;
|
||||
readonly sessionManager: SessionManager;
|
||||
|
|
@ -2662,14 +2678,10 @@ export class AgentSession {
|
|||
const scopedModels = this._scopedModels.filter((scoped) =>
|
||||
availableIds.has(`${scoped.model.provider}\0${scoped.model.id}`),
|
||||
);
|
||||
if (scopedModels.length <= 1) return undefined;
|
||||
|
||||
const currentModel = this.model;
|
||||
let currentIndex = scopedModels.findIndex((sm) => modelsAreEqual(sm.model, currentModel));
|
||||
|
||||
if (currentIndex === -1) currentIndex = 0;
|
||||
const len = scopedModels.length;
|
||||
const nextIndex = direction === "forward" ? (currentIndex + 1) % len : (currentIndex - 1 + len) % len;
|
||||
const currentIndex = scopedModels.findIndex((sm) => modelsAreEqual(sm.model, currentModel));
|
||||
const nextIndex = getNextCycleIndex(currentIndex, scopedModels.length, direction);
|
||||
if (nextIndex === undefined) return undefined;
|
||||
const next = scopedModels[nextIndex];
|
||||
const thinkingLevel = this._getThinkingLevelForModelSwitch(next.model, next.thinkingLevel);
|
||||
|
||||
|
|
@ -2698,14 +2710,10 @@ export class AgentSession {
|
|||
options: ModelMutationOptions,
|
||||
): Promise<ModelCycleResult | undefined> {
|
||||
const availableModels = this._modelRuntime.getAvailableSnapshot();
|
||||
if (availableModels.length <= 1) return undefined;
|
||||
|
||||
const currentModel = this.model;
|
||||
let currentIndex = availableModels.findIndex((m) => modelsAreEqual(m, currentModel));
|
||||
|
||||
if (currentIndex === -1) currentIndex = 0;
|
||||
const len = availableModels.length;
|
||||
const nextIndex = direction === "forward" ? (currentIndex + 1) % len : (currentIndex - 1 + len) % len;
|
||||
const currentIndex = availableModels.findIndex((m) => modelsAreEqual(m, currentModel));
|
||||
const nextIndex = getNextCycleIndex(currentIndex, availableModels.length, direction);
|
||||
if (nextIndex === undefined) return undefined;
|
||||
const nextModel = availableModels[nextIndex];
|
||||
|
||||
const thinkingLevel = this._getThinkingLevelForModelSwitch(nextModel);
|
||||
|
|
|
|||
|
|
@ -197,6 +197,35 @@ describe("AgentSession model and extension characterization", () => {
|
|||
expect(harness.session.thinkingLevel).toBe("high");
|
||||
});
|
||||
|
||||
// Regression test for https://github.com/earendil-works/pi/issues/11806
|
||||
it("cycles to the only scoped model and to the first scoped model from outside the scope", async () => {
|
||||
const harness = await createHarness({
|
||||
models: [
|
||||
{ id: "faux-1", name: "One" },
|
||||
{ id: "faux-2", name: "Two" },
|
||||
{ id: "faux-3", name: "Three" },
|
||||
],
|
||||
});
|
||||
harnesses.push(harness);
|
||||
const modelTwo = harness.getModel("faux-2")!;
|
||||
const modelThree = harness.getModel("faux-3")!;
|
||||
|
||||
harness.session.setScopedModels([{ model: modelTwo }]);
|
||||
expect((await harness.session.cycleModel())?.model.id).toBe("faux-2");
|
||||
expect(harness.session.model?.id).toBe("faux-2");
|
||||
expect(await harness.session.cycleModel()).toBeUndefined();
|
||||
expect(harness.session.model?.id).toBe("faux-2");
|
||||
|
||||
await harness.session.setModel(harness.getModel("faux-1")!);
|
||||
harness.session.setScopedModels([{ model: modelTwo }, { model: modelThree }]);
|
||||
await harness.session.cycleModel();
|
||||
expect(harness.session.model?.id).toBe("faux-2");
|
||||
|
||||
await harness.session.setModel(harness.getModel("faux-1")!);
|
||||
await harness.session.cycleModel("backward");
|
||||
expect(harness.session.model?.id).toBe("faux-3");
|
||||
});
|
||||
|
||||
it("clamps thinking levels to model capabilities and cycles available levels", async () => {
|
||||
const harness = await createHarness({ models: [{ id: "faux-1", reasoning: false }] });
|
||||
harnesses.push(harness);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue