From 3d96fd0d9e8be8a135d1f8caf7951f47e80c40c3 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 21 May 2026 16:37:44 +0900 Subject: [PATCH] fix(coding-agent): fixed compaction to prefer active session model over role default - Added the active session model to compaction candidate selection before role-based candidates. - Updated compaction routing so role-based models are only considered after the current chat model. - Added a regression test proving an Anthropic session prefers its active model over `modelRoles.default` on OpenAI. --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/session/agent-session.ts | 6 ++ .../compaction-prefer-current-model.test.ts | 100 ++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 packages/coding-agent/test/compaction-prefer-current-model.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a39be4e0d..0576514f7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed compaction routing to the wrong provider when `modelRoles.default` is set to a different model than the active chat. Auto- and manual compaction now prefer the active session's model and only fall back to role-based candidates when the current model has no usable credentials. Previously, an Anthropic chat with `modelRoles.default = openai/gpt-5` would compact through OpenAI (including the remote-compaction endpoint), even though the live conversation never used OpenAI. + ## [15.2.0] - 2026-05-21 ### Changed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 006e69ca1..2a77d4f3e 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -6171,6 +6171,12 @@ export class AgentSession { }; const currentModel = this.model; + // Prefer the active session's model: it's what the user is actively using, + // and routing compaction to a different provider (e.g. an OpenAI default + // model while the chat is on Anthropic) changes provider-specific behavior + // like remote compaction endpoints. Role-based candidates only kick in + // as auth fallbacks when the current model has no usable credentials. + addCandidate(currentModel); for (const role of MODEL_ROLE_IDS) { addCandidate(this.#resolveRoleModelFull(role, availableModels, currentModel).model); } diff --git a/packages/coding-agent/test/compaction-prefer-current-model.test.ts b/packages/coding-agent/test/compaction-prefer-current-model.test.ts new file mode 100644 index 000000000..d555bc91d --- /dev/null +++ b/packages/coding-agent/test/compaction-prefer-current-model.test.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; +import * as path from "node:path"; +import { Agent } from "@oh-my-pi/pi-agent-core"; +import * as compactionModule from "@oh-my-pi/pi-agent-core/compaction"; +import { getBundledModel } from "@oh-my-pi/pi-ai"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +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 { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; +import { assistantMsg, userMsg } from "./utilities"; + +/** + * Regression: when the user sets `modelRoles.default` to a model on a different + * provider than the current chat, compaction must still pick the active chat's + * model first. Otherwise an Anthropic chat would route compaction through the + * OpenAI remote-compaction endpoint (gated by `shouldUseOpenAiRemoteCompaction`), + * even though the live conversation never used OpenAI. + */ +describe("compaction prefers the current session model over modelRoles.default", () => { + let tempDir: TempDir; + let authStorage: AuthStorage; + let session: AgentSession; + let modelRegistry: ModelRegistry; + + beforeEach(() => { + tempDir = TempDir.createSync("@pi-compact-current-"); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + if (session) { + await session.dispose(); + } + authStorage?.close(); + tempDir.removeSync(); + }); + + it("uses the active Anthropic chat model when modelRoles.default points at an OpenAI model", async () => { + const currentModel = getBundledModel("anthropic", "claude-sonnet-4-5"); + const defaultRoleModel = getBundledModel("openai", "gpt-5"); + if (!currentModel || !defaultRoleModel) { + throw new Error("Expected bundled test models to exist"); + } + + const settings = Settings.isolated({ "compaction.keepRecentTokens": 1 }); + settings.setModelRole("default", `${defaultRoleModel.provider}/${defaultRoleModel.id}`); + + const agent = new Agent({ + initialState: { + model: currentModel, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + }); + + authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); + // Both providers have credentials so an "auth failure" wouldn't be the + // reason a candidate is skipped — order alone must drive the choice. + authStorage.setRuntimeApiKey(currentModel.provider, "anthropic-token"); + authStorage.setRuntimeApiKey(defaultRoleModel.provider, "openai-token"); + modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml")); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + }); + session.subscribe(() => {}); + + for (const [userText, assistantText] of [ + ["first question", "first answer"], + ["second question", "second answer"], + ] as const) { + const user = userMsg(userText); + const assistant = assistantMsg(assistantText); + session.agent.appendMessage(user); + session.sessionManager.appendMessage(user); + session.agent.appendMessage(assistant); + session.sessionManager.appendMessage(assistant); + } + + const compactSpy = vi.spyOn(compactionModule, "compact").mockImplementation(async (preparation, model) => ({ + summary: "ok", + shortSummary: "ok short", + firstKeptEntryId: preparation.firstKeptEntryId, + tokensBefore: 1, + details: { provider: model.provider }, + })); + + await session.compact(); + + expect(compactSpy).toHaveBeenCalled(); + const [, firstCandidate] = compactSpy.mock.calls[0]!; + expect(`${firstCandidate.provider}/${firstCandidate.id}`).toBe(`${currentModel.provider}/${currentModel.id}`); + }); +});