fix(coding-agent): preserve explicit default model on session resume

buildSessionContext walked the entry path and unconditionally overwrote
models.default from every assistant message's reported model. Temporary
fallbacks (retry fallback, context promotion) and codex-side model
downgrades both produce assistant messages tagged with a different model
id, which clobbered the user's explicit /model pick on resume and made
the session silently revert to the older model.

Treat assistant-message inference as a legacy fallback that only fills
in models.default when no explicit `model_change` with role="default"
has been seen on the path.

Fixes #849
This commit is contained in:
can1357
2026-04-30 04:32:24 +02:00
parent 9caf025937
commit 08403be71f
4 changed files with 131 additions and 6 deletions
@@ -566,6 +566,14 @@ export function buildSessionContext(
let hasPersistedMCPToolSelection = false;
let mode = "none";
let modeData: Record<string, unknown> | undefined;
// Track whether an explicit `model_change` with role="default" has been
// seen on this path. Once a user (or the agent itself) records an
// explicit default, later assistant-message inference must NOT overwrite
// it: temporary fallbacks (retry fallback, context promotion) and
// server-side model downgrades both produce assistant messages tagged
// with the wrong model id, which previously clobbered the user's pick on
// resume (issue #849).
let hasExplicitDefaultModel = false;
for (const entry of path) {
if (entry.type === "thinking_level_change") {
@@ -575,12 +583,21 @@ export function buildSessionContext(
if (entry.model) {
const role = entry.role ?? "default";
models[role] = entry.model;
if (role === "default") {
hasExplicitDefaultModel = true;
}
}
} else if (entry.type === "service_tier_change") {
serviceTier = entry.serviceTier ?? undefined;
} else if (entry.type === "message" && entry.message.role === "assistant") {
// Infer default model from assistant messages
models.default = `${entry.message.provider}/${entry.message.model}`;
// Legacy fallback: infer default model from assistant messages only
// when no explicit `model_change` (role=default) entry has been
// recorded yet. Newer sessions always record an explicit default
// model_change at the start of the conversation, so this branch is
// only used to keep pre-model_change sessions working.
if (!hasExplicitDefaultModel) {
models.default = `${entry.message.provider}/${entry.message.model}`;
}
} else if (entry.type === "compaction") {
compaction = entry;
} else if (entry.type === "ttsr_injection") {
@@ -895,8 +895,8 @@ describe("buildSessionContext", () => {
];
const loaded = buildSessionContext(entries);
// model_change is later overwritten by assistant message's model info
expect(loaded.models.default).toBe("anthropic/claude-sonnet-4-5");
// Issue #849: explicit model_change wins over assistant-message inference.
expect(loaded.models.default).toBe("openai/gpt-4");
expect(loaded.thinkingLevel).toBe("high");
});
});
@@ -0,0 +1,105 @@
import { describe, expect, it } from "bun:test";
import type { AssistantMessage } from "@oh-my-pi/pi-ai";
import {
buildSessionContext,
type ModelChangeEntry,
type SessionEntry,
type SessionMessageEntry,
} from "@oh-my-pi/pi-coding-agent/session/session-manager";
/**
* Issue #849: After a user explicitly switches to gpt-5.5, the session reverts
* to gpt-5.4 on resume.
*
* Root cause hypothesis: buildSessionContext walks entries in path order and
* overwrites `models.default` from every assistant message's reported model.
* When a temporary fallback (e.g. retry fallback or a server-side downgrade
* in the codex provider) emits an assistant message tagged with the older
* model id, that id clobbers the user's explicitly chosen default.
*
* Contract under test: an explicit `model_change` with role="default" must
* win over assistant-message inference from later messages produced under a
* temporary or downgraded model.
*/
describe("issue #849: explicit default model survives later assistant-message inference", () => {
function makeAssistantEntry(
id: string,
parentId: string | null,
provider: string,
model: string,
): SessionMessageEntry {
const message: AssistantMessage = {
role: "assistant",
content: [{ type: "text", text: "ok" }],
api: "openai-codex-responses",
provider: provider as AssistantMessage["provider"],
model,
usage: {
input: 0,
output: 0,
cacheRead: 0,
cacheWrite: 0,
totalTokens: 0,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
},
stopReason: "stop",
timestamp: Date.parse(`2026-04-30T00:00:0${id.slice(-1)}Z`),
};
return {
type: "message",
id,
parentId,
timestamp: new Date(message.timestamp).toISOString(),
message,
};
}
function makeModelChange(id: string, parentId: string | null, model: string, role: string): ModelChangeEntry {
return {
type: "model_change",
id,
parentId,
timestamp: new Date().toISOString(),
model,
role,
};
}
it("preserves explicit user-selected default when a later assistant message reports a downgraded model", () => {
// User explicitly picks gpt-5.5 as default.
// Then a temporary fallback (retry / context promotion) appends a
// model_change with role="temporary" pointing at gpt-5.4, and the
// next assistant message is produced under that temporary model.
const entries: SessionEntry[] = [
makeModelChange("a1", null, "openai-codex/gpt-5.5", "default"),
makeAssistantEntry("a2", "a1", "openai-codex", "gpt-5.5"),
makeModelChange("a3", "a2", "openai-codex/gpt-5.4", "temporary"),
makeAssistantEntry("a4", "a3", "openai-codex", "gpt-5.4"),
];
const ctx = buildSessionContext(entries);
expect(ctx.models.default).toBe("openai-codex/gpt-5.5");
});
it("preserves explicit user-selected default when the codex backend reports a different model id", () => {
// User picks gpt-5.5; the assistant message returned by the upstream
// codex backend is tagged "gpt-5.4" (server-side downgrade /
// stale id mapping). Resume must still restore what the user picked.
const entries: SessionEntry[] = [
makeModelChange("b1", null, "openai-codex/gpt-5.5", "default"),
makeAssistantEntry("b2", "b1", "openai-codex", "gpt-5.4"),
];
const ctx = buildSessionContext(entries);
expect(ctx.models.default).toBe("openai-codex/gpt-5.5");
});
it("still infers default from assistant messages when no model_change entry exists", () => {
// Backwards compatibility: legacy sessions have no model_change entries
// and rely on assistant-message inference.
const entries: SessionEntry[] = [makeAssistantEntry("c1", null, "openai-codex", "gpt-5.4")];
const ctx = buildSessionContext(entries);
expect(ctx.models.default).toBe("openai-codex/gpt-5.4");
});
});
@@ -151,8 +151,11 @@ describe("buildSessionContext", () => {
msg("3", "2", "assistant", "hi"),
];
const ctx = buildSessionContext(entries);
// Assistant message overwrites model change
expect(ctx.models.default).toBe("anthropic/claude-test");
// Issue #849: an explicit model_change with role="default" must NOT
// be silently overwritten by a later assistant message tagged with a
// different model id. Temporary fallbacks and provider-side
// downgrades both produce such mismatched messages.
expect(ctx.models.default).toBe("openai/gpt-4");
});
});