fix: model fallback properly falls through to system default
- Remove Step 3 in model-resolver that forced first fallbackChain entry even when unavailable, blocking system default fallback - Add sisyphusJuniorModel option to delegate_task so agents["Sisyphus-Junior"] model override is respected in category-based delegation - Update tests to reflect new fallback behavior
This commit is contained in:
parent
2c81c8e58e
commit
f4348885f2
@ -32,30 +32,30 @@ describe("createBuiltinAgents with model overrides", () => {
|
|||||||
expect(agents.Sisyphus.thinking).toBeUndefined()
|
expect(agents.Sisyphus.thinking).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("Sisyphus uses first fallbackChain entry when no availableModels provided", async () => {
|
test("Sisyphus uses system default when no availableModels provided", async () => {
|
||||||
// #given
|
// #given
|
||||||
const systemDefaultModel = "openai/gpt-5.2"
|
const systemDefaultModel = "anthropic/claude-opus-4-5"
|
||||||
|
|
||||||
// #when
|
// #when
|
||||||
const agents = await createBuiltinAgents([], {}, undefined, systemDefaultModel)
|
const agents = await createBuiltinAgents([], {}, undefined, systemDefaultModel)
|
||||||
|
|
||||||
// #then - Sisyphus first fallbackChain entry is anthropic/claude-opus-4-5
|
// #then - falls back to system default when no availability match
|
||||||
expect(agents.Sisyphus.model).toBe("anthropic/claude-opus-4-5")
|
expect(agents.Sisyphus.model).toBe("anthropic/claude-opus-4-5")
|
||||||
expect(agents.Sisyphus.thinking).toEqual({ type: "enabled", budgetTokens: 32000 })
|
expect(agents.Sisyphus.thinking).toEqual({ type: "enabled", budgetTokens: 32000 })
|
||||||
expect(agents.Sisyphus.reasoningEffort).toBeUndefined()
|
expect(agents.Sisyphus.reasoningEffort).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("Oracle uses first fallbackChain entry when no availableModels provided", async () => {
|
test("Oracle uses system default when no availableModels provided", async () => {
|
||||||
// #given - Oracle's first fallbackChain entry is openai/gpt-5.2
|
// #given - no available models, falls back to system default
|
||||||
|
|
||||||
// #when
|
// #when
|
||||||
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL)
|
const agents = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL)
|
||||||
|
|
||||||
// #then - Oracle first fallbackChain entry is openai/gpt-5.2
|
// #then - falls back to system default (anthropic/claude-opus-4-5)
|
||||||
expect(agents.oracle.model).toBe("openai/gpt-5.2")
|
expect(agents.oracle.model).toBe("anthropic/claude-opus-4-5")
|
||||||
expect(agents.oracle.reasoningEffort).toBe("medium")
|
expect(agents.oracle.thinking).toEqual({ type: "enabled", budgetTokens: 32000 })
|
||||||
expect(agents.oracle.textVerbosity).toBe("high")
|
expect(agents.oracle.reasoningEffort).toBeUndefined()
|
||||||
expect(agents.oracle.thinking).toBeUndefined()
|
expect(agents.oracle.textVerbosity).toBeUndefined()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("Oracle with GPT model override has reasoningEffort, no thinking", async () => {
|
test("Oracle with GPT model override has reasoningEffort, no thinking", async () => {
|
||||||
|
|||||||
@ -236,6 +236,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
|
|||||||
directory: ctx.directory,
|
directory: ctx.directory,
|
||||||
userCategories: pluginConfig.categories,
|
userCategories: pluginConfig.categories,
|
||||||
gitMasterConfig: pluginConfig.git_master,
|
gitMasterConfig: pluginConfig.git_master,
|
||||||
|
sisyphusJuniorModel: pluginConfig.agents?.["Sisyphus-Junior"]?.model,
|
||||||
});
|
});
|
||||||
const disabledSkills = new Set(pluginConfig.disabled_skills ?? []);
|
const disabledSkills = new Set(pluginConfig.disabled_skills ?? []);
|
||||||
const systemMcpNames = getSystemMcpServerNames();
|
const systemMcpNames = getSystemMcpServerNames();
|
||||||
|
|||||||
@ -316,8 +316,8 @@ describe("resolveModelWithFallback", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("Step 3: First fallback entry (no availability match)", () => {
|
describe("Step 3: System default fallback (no availability match)", () => {
|
||||||
test("returns first fallbackChain entry when no availability match found", () => {
|
test("returns system default when no availability match found in fallback chain", () => {
|
||||||
// #given
|
// #given
|
||||||
const input: ExtendedModelResolutionInput = {
|
const input: ExtendedModelResolutionInput = {
|
||||||
fallbackChain: [
|
fallbackChain: [
|
||||||
@ -331,12 +331,12 @@ describe("resolveModelWithFallback", () => {
|
|||||||
const result = resolveModelWithFallback(input)
|
const result = resolveModelWithFallback(input)
|
||||||
|
|
||||||
// #then
|
// #then
|
||||||
expect(result.model).toBe("anthropic/nonexistent-model")
|
expect(result.model).toBe("google/gemini-3-pro")
|
||||||
expect(result.source).toBe("provider-fallback")
|
expect(result.source).toBe("system-default")
|
||||||
expect(logSpy).toHaveBeenCalledWith("Model resolved via fallback chain first entry (no availability match)", { model: "anthropic/nonexistent-model", variant: undefined })
|
expect(logSpy).toHaveBeenCalledWith("No available model found in fallback chain, falling through to system default")
|
||||||
})
|
})
|
||||||
|
|
||||||
test("returns first fallbackChain entry when availableModels is empty", () => {
|
test("returns system default when availableModels is empty", () => {
|
||||||
// #given
|
// #given
|
||||||
const input: ExtendedModelResolutionInput = {
|
const input: ExtendedModelResolutionInput = {
|
||||||
fallbackChain: [
|
fallbackChain: [
|
||||||
@ -350,8 +350,8 @@ describe("resolveModelWithFallback", () => {
|
|||||||
const result = resolveModelWithFallback(input)
|
const result = resolveModelWithFallback(input)
|
||||||
|
|
||||||
// #then
|
// #then
|
||||||
expect(result.model).toBe("anthropic/claude-opus-4-5")
|
expect(result.model).toBe("google/gemini-3-pro")
|
||||||
expect(result.source).toBe("provider-fallback")
|
expect(result.source).toBe("system-default")
|
||||||
})
|
})
|
||||||
|
|
||||||
test("returns system default when fallbackChain is not provided", () => {
|
test("returns system default when fallbackChain is not provided", () => {
|
||||||
@ -431,7 +431,7 @@ describe("resolveModelWithFallback", () => {
|
|||||||
expect(result.source).toBe("provider-fallback")
|
expect(result.source).toBe("provider-fallback")
|
||||||
})
|
})
|
||||||
|
|
||||||
test("falls through to first fallbackChain entry when none match availability", () => {
|
test("falls through to system default when none match availability", () => {
|
||||||
// #given
|
// #given
|
||||||
const availableModels = new Set(["other/model"])
|
const availableModels = new Set(["other/model"])
|
||||||
|
|
||||||
@ -447,8 +447,8 @@ describe("resolveModelWithFallback", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
// #then
|
// #then
|
||||||
expect(result.model).toBe("openai/gpt-5.2")
|
expect(result.model).toBe("system/default")
|
||||||
expect(result.source).toBe("provider-fallback")
|
expect(result.source).toBe("system-default")
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@ -63,15 +63,8 @@ export function resolveModelWithFallback(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// No match found in fallback chain - fall through to system default
|
||||||
// Step 3: Use first entry in fallbackChain as fallback (no availability match found)
|
log("No available model found in fallback chain, falling through to system default")
|
||||||
// This ensures category/agent intent is honored even if availableModels is incomplete
|
|
||||||
const firstEntry = fallbackChain[0]
|
|
||||||
if (firstEntry.providers.length > 0) {
|
|
||||||
const fallbackModel = `${firstEntry.providers[0]}/${firstEntry.model}`
|
|
||||||
log("Model resolved via fallback chain first entry (no availability match)", { model: fallbackModel, variant: firstEntry.variant })
|
|
||||||
return { model: fallbackModel, source: "provider-fallback", variant: firstEntry.variant }
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 4: System default
|
// Step 4: System default
|
||||||
|
|||||||
@ -360,6 +360,7 @@ describe("sisyphus-task", () => {
|
|||||||
const mockClient = {
|
const mockClient = {
|
||||||
app: { agents: async () => ({ data: [] }) },
|
app: { agents: async () => ({ data: [] }) },
|
||||||
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
||||||
|
model: { list: async () => [{ id: "anthropic/claude-opus-4-5" }] },
|
||||||
session: {
|
session: {
|
||||||
create: async () => ({ data: { id: "test-session" } }),
|
create: async () => ({ data: { id: "test-session" } }),
|
||||||
prompt: async () => ({ data: {} }),
|
prompt: async () => ({ data: {} }),
|
||||||
@ -410,6 +411,7 @@ describe("sisyphus-task", () => {
|
|||||||
const mockClient = {
|
const mockClient = {
|
||||||
app: { agents: async () => ({ data: [] }) },
|
app: { agents: async () => ({ data: [] }) },
|
||||||
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
||||||
|
model: { list: async () => [{ id: "anthropic/claude-opus-4-5" }] },
|
||||||
session: {
|
session: {
|
||||||
get: async () => ({ data: { directory: "/project" } }),
|
get: async () => ({ data: { directory: "/project" } }),
|
||||||
create: async () => ({ data: { id: "ses_sync_default_variant" } }),
|
create: async () => ({ data: { id: "ses_sync_default_variant" } }),
|
||||||
@ -958,6 +960,7 @@ describe("sisyphus-task", () => {
|
|||||||
const mockClient = {
|
const mockClient = {
|
||||||
app: { agents: async () => ({ data: [] }) },
|
app: { agents: async () => ({ data: [] }) },
|
||||||
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
||||||
|
model: { list: async () => [{ id: "google/gemini-3-pro-preview" }] },
|
||||||
session: {
|
session: {
|
||||||
get: async () => ({ data: { directory: "/project" } }),
|
get: async () => ({ data: { directory: "/project" } }),
|
||||||
create: async () => ({ data: { id: "ses_unstable_gemini" } }),
|
create: async () => ({ data: { id: "ses_unstable_gemini" } }),
|
||||||
@ -1141,6 +1144,7 @@ describe("sisyphus-task", () => {
|
|||||||
const mockClient = {
|
const mockClient = {
|
||||||
app: { agents: async () => ({ data: [] }) },
|
app: { agents: async () => ({ data: [] }) },
|
||||||
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
||||||
|
model: { list: async () => [{ id: "google/gemini-3-pro-preview" }] },
|
||||||
session: {
|
session: {
|
||||||
get: async () => ({ data: { directory: "/project" } }),
|
get: async () => ({ data: { directory: "/project" } }),
|
||||||
create: async () => ({ data: { id: "ses_artistry_gemini" } }),
|
create: async () => ({ data: { id: "ses_artistry_gemini" } }),
|
||||||
@ -1205,6 +1209,7 @@ describe("sisyphus-task", () => {
|
|||||||
const mockClient = {
|
const mockClient = {
|
||||||
app: { agents: async () => ({ data: [] }) },
|
app: { agents: async () => ({ data: [] }) },
|
||||||
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
config: { get: async () => ({ data: { model: SYSTEM_DEFAULT_MODEL } }) },
|
||||||
|
model: { list: async () => [{ id: "google/gemini-3-flash-preview" }] },
|
||||||
session: {
|
session: {
|
||||||
get: async () => ({ data: { directory: "/project" } }),
|
get: async () => ({ data: { directory: "/project" } }),
|
||||||
create: async () => ({ data: { id: "ses_writing_gemini" } }),
|
create: async () => ({ data: { id: "ses_writing_gemini" } }),
|
||||||
|
|||||||
@ -156,6 +156,7 @@ export interface DelegateTaskToolOptions {
|
|||||||
directory: string
|
directory: string
|
||||||
userCategories?: CategoriesConfig
|
userCategories?: CategoriesConfig
|
||||||
gitMasterConfig?: GitMasterConfig
|
gitMasterConfig?: GitMasterConfig
|
||||||
|
sisyphusJuniorModel?: string
|
||||||
}
|
}
|
||||||
|
|
||||||
export interface BuildSystemContentInput {
|
export interface BuildSystemContentInput {
|
||||||
@ -178,7 +179,7 @@ export function buildSystemContent(input: BuildSystemContentInput): string | und
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefinition {
|
export function createDelegateTask(options: DelegateTaskToolOptions): ToolDefinition {
|
||||||
const { manager, client, directory, userCategories, gitMasterConfig } = options
|
const { manager, client, directory, userCategories, gitMasterConfig, sisyphusJuniorModel } = options
|
||||||
|
|
||||||
const allCategories = { ...DEFAULT_CATEGORIES, ...userCategories }
|
const allCategories = { ...DEFAULT_CATEGORIES, ...userCategories }
|
||||||
const categoryNames = Object.keys(allCategories)
|
const categoryNames = Object.keys(allCategories)
|
||||||
@ -513,7 +514,7 @@ To resume this session: resume="${args.resume}"`
|
|||||||
modelInfo = { model: actualModel, type: "system-default", source: "system-default" }
|
modelInfo = { model: actualModel, type: "system-default", source: "system-default" }
|
||||||
} else {
|
} else {
|
||||||
const { model: resolvedModel, source, variant: resolvedVariant } = resolveModelWithFallback({
|
const { model: resolvedModel, source, variant: resolvedVariant } = resolveModelWithFallback({
|
||||||
userModel: userCategories?.[args.category]?.model,
|
userModel: userCategories?.[args.category]?.model ?? sisyphusJuniorModel,
|
||||||
fallbackChain: requirement.fallbackChain,
|
fallbackChain: requirement.fallbackChain,
|
||||||
availableModels,
|
availableModels,
|
||||||
systemDefaultModel,
|
systemDefaultModel,
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user