From 23b49c4a5cfba505f029bb69eb56e2c9ed0e7036 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 29 Jan 2026 19:46:34 +0900 Subject: [PATCH] fix: expand override.category and explicit reasoningEffort priority (#1219) (#1235) * fix: expand override.category and explicit reasoningEffort priority (#1219) Two bugs fixed: 1. createBuiltinAgents(): override.category was never expanded into concrete config properties (model, variant, reasoningEffort, etc.). Added applyCategoryOverride() helper and applied it in the standard agent loop, Sisyphus path, and Atlas path. 2. Prometheus config-handler: reasoningEffort/textVerbosity/thinking from direct override now use explicit priority chains (direct > category) matching the existing variant pattern, instead of relying on spread ordering. Priority order (highest to lowest): 1. Direct override properties 2. Override category properties 3. Resolved variant from model fallback chain 4. Factory base defaults Closes #1219 * fix: use undefined check for thinking to allow explicit false --- src/agents/utils.test.ts | 116 +++++++++++++++++++ src/agents/utils.ts | 58 ++++++++-- src/plugin-handlers/config-handler.test.ts | 124 +++++++++++++++++++++ src/plugin-handlers/config-handler.ts | 42 ++++--- 4 files changed, 314 insertions(+), 26 deletions(-) diff --git a/src/agents/utils.test.ts b/src/agents/utils.test.ts index 9b28590f..bb24f2f8 100644 --- a/src/agents/utils.test.ts +++ b/src/agents/utils.test.ts @@ -407,3 +407,119 @@ describe("buildAgent with category and skills", () => { expect(agent.prompt).not.toContain("agent-browser open") }) }) + +describe("override.category expansion in createBuiltinAgents", () => { + test("standard agent override with category expands category properties", async () => { + // #given + const overrides = { + oracle: { category: "ultrabrain" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + + // #then - ultrabrain category: model=openai/gpt-5.2-codex, variant=xhigh + expect(agents.oracle).toBeDefined() + expect(agents.oracle.model).toBe("openai/gpt-5.2-codex") + expect(agents.oracle.variant).toBe("xhigh") + }) + + test("standard agent override with category AND direct variant - direct wins", async () => { + // #given - ultrabrain has variant=xhigh, but direct override says "max" + const overrides = { + oracle: { category: "ultrabrain", variant: "max" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + + // #then - direct variant overrides category variant + expect(agents.oracle).toBeDefined() + expect(agents.oracle.variant).toBe("max") + }) + + test("standard agent override with category AND direct reasoningEffort - direct wins", async () => { + // #given - custom category has reasoningEffort=xhigh, direct override says "low" + const categories = { + "test-cat": { + model: "openai/gpt-5.2", + reasoningEffort: "xhigh" as const, + }, + } + const overrides = { + oracle: { category: "test-cat", reasoningEffort: "low" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, categories) + + // #then - direct reasoningEffort wins over category + expect(agents.oracle).toBeDefined() + expect(agents.oracle.reasoningEffort).toBe("low") + }) + + test("standard agent override with category applies reasoningEffort from category when no direct override", async () => { + // #given - custom category has reasoningEffort, no direct reasoningEffort in override + const categories = { + "reasoning-cat": { + model: "openai/gpt-5.2", + reasoningEffort: "high" as const, + }, + } + const overrides = { + oracle: { category: "reasoning-cat" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL, categories) + + // #then - category reasoningEffort is applied + expect(agents.oracle).toBeDefined() + expect(agents.oracle.reasoningEffort).toBe("high") + }) + + test("sisyphus override with category expands category properties", async () => { + // #given + const overrides = { + sisyphus: { category: "ultrabrain" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + + // #then - ultrabrain category: model=openai/gpt-5.2-codex, variant=xhigh + expect(agents.sisyphus).toBeDefined() + expect(agents.sisyphus.model).toBe("openai/gpt-5.2-codex") + expect(agents.sisyphus.variant).toBe("xhigh") + }) + + test("atlas override with category expands category properties", async () => { + // #given + const overrides = { + atlas: { category: "ultrabrain" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + + // #then - ultrabrain category: model=openai/gpt-5.2-codex, variant=xhigh + expect(agents.atlas).toBeDefined() + expect(agents.atlas.model).toBe("openai/gpt-5.2-codex") + expect(agents.atlas.variant).toBe("xhigh") + }) + + test("override with non-existent category has no effect on config", async () => { + // #given + const overrides = { + oracle: { category: "non-existent-category" } as any, + } + + // #when + const agents = await createBuiltinAgents([], overrides, undefined, TEST_DEFAULT_MODEL) + + // #then - no category-specific variant/reasoningEffort applied from non-existent category + expect(agents.oracle).toBeDefined() + const agentsWithoutOverride = await createBuiltinAgents([], {}, undefined, TEST_DEFAULT_MODEL) + expect(agents.oracle.model).toBe(agentsWithoutOverride.oracle.model) + }) +}) diff --git a/src/agents/utils.ts b/src/agents/utils.ts index dc96f6ff..7e9f6169 100644 --- a/src/agents/utils.ts +++ b/src/agents/utils.ts @@ -120,6 +120,33 @@ export function createEnvContext(): string { ` } +/** + * Expands a category reference from an agent override into concrete config properties. + * Category properties are applied unconditionally (overwriting factory defaults), + * because the user's chosen category should take priority over factory base values. + * Direct override properties applied later via mergeAgentConfig() will supersede these. + */ +function applyCategoryOverride( + config: AgentConfig, + categoryName: string, + mergedCategories: Record +): AgentConfig { + const categoryConfig = mergedCategories[categoryName] + if (!categoryConfig) return config + + const result = { ...config } as AgentConfig & Record + if (categoryConfig.model) result.model = categoryConfig.model + if (categoryConfig.variant !== undefined) result.variant = categoryConfig.variant + if (categoryConfig.temperature !== undefined) result.temperature = categoryConfig.temperature + if (categoryConfig.reasoningEffort !== undefined) result.reasoningEffort = categoryConfig.reasoningEffort + if (categoryConfig.textVerbosity !== undefined) result.textVerbosity = categoryConfig.textVerbosity + if (categoryConfig.thinking !== undefined) result.thinking = categoryConfig.thinking + if (categoryConfig.top_p !== undefined) result.top_p = categoryConfig.top_p + if (categoryConfig.maxTokens !== undefined) result.maxTokens = categoryConfig.maxTokens + + return result as AgentConfig +} + function mergeAgentConfig( base: AgentConfig, override: AgentOverrideConfig @@ -210,18 +237,23 @@ export async function createBuiltinAgents( let config = buildAgent(source, model, mergedCategories, gitMasterConfig, browserProvider) - // Apply variant from override or resolved fallback chain - if (override?.variant) { - config = { ...config, variant: override.variant } - } else if (resolvedVariant) { + // Apply resolved variant from model fallback chain + if (resolvedVariant) { config = { ...config, variant: resolvedVariant } } + // Expand override.category into concrete properties (higher priority than factory/resolved) + const overrideCategory = (override as Record | undefined)?.category as string | undefined + if (overrideCategory) { + config = applyCategoryOverride(config, overrideCategory, mergedCategories) + } + if (agentName === "librarian" && directory && config.prompt) { const envContext = createEnvContext() config = { ...config, prompt: config.prompt + envContext } } + // Direct override properties take highest priority if (override) { config = mergeAgentConfig(config, override) } @@ -261,12 +293,15 @@ export async function createBuiltinAgents( availableCategories ) - if (sisyphusOverride?.variant) { - sisyphusConfig = { ...sisyphusConfig, variant: sisyphusOverride.variant } - } else if (sisyphusResolvedVariant) { + if (sisyphusResolvedVariant) { sisyphusConfig = { ...sisyphusConfig, variant: sisyphusResolvedVariant } } + const sisOverrideCategory = (sisyphusOverride as Record | undefined)?.category as string | undefined + if (sisOverrideCategory) { + sisyphusConfig = applyCategoryOverride(sisyphusConfig, sisOverrideCategory, mergedCategories) + } + if (directory && sisyphusConfig.prompt) { const envContext = createEnvContext() sisyphusConfig = { ...sisyphusConfig, prompt: sisyphusConfig.prompt + envContext } @@ -302,12 +337,15 @@ export async function createBuiltinAgents( userCategories: categories, }) - if (orchestratorOverride?.variant) { - orchestratorConfig = { ...orchestratorConfig, variant: orchestratorOverride.variant } - } else if (atlasResolvedVariant) { + if (atlasResolvedVariant) { orchestratorConfig = { ...orchestratorConfig, variant: atlasResolvedVariant } } + const atlasOverrideCategory = (orchestratorOverride as Record | undefined)?.category as string | undefined + if (atlasOverrideCategory) { + orchestratorConfig = applyCategoryOverride(orchestratorConfig, atlasOverrideCategory, mergedCategories) + } + if (orchestratorOverride) { orchestratorConfig = mergeAgentConfig(orchestratorConfig, orchestratorOverride) } diff --git a/src/plugin-handlers/config-handler.test.ts b/src/plugin-handlers/config-handler.test.ts index bff7d762..d4f6fe06 100644 --- a/src/plugin-handlers/config-handler.test.ts +++ b/src/plugin-handlers/config-handler.test.ts @@ -280,3 +280,127 @@ describe("Prometheus category config resolution", () => { expect(config?.tools).toEqual({ tool1: true, tool2: false }) }) }) + +describe("Prometheus direct override priority over category", () => { + test("direct reasoningEffort takes priority over category reasoningEffort", async () => { + // #given - category has reasoningEffort=xhigh, direct override says "low" + const pluginConfig: OhMyOpenCodeConfig = { + sisyphus_agent: { + planner_enabled: true, + }, + categories: { + "test-planning": { + model: "openai/gpt-5.2", + reasoningEffort: "xhigh", + }, + }, + agents: { + prometheus: { + category: "test-planning", + reasoningEffort: "low", + }, + }, + } + const config: Record = { + model: "anthropic/claude-opus-4-5", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + // #when + await handler(config) + + // #then - direct override's reasoningEffort wins + const agents = config.agent as Record + expect(agents.prometheus).toBeDefined() + expect(agents.prometheus.reasoningEffort).toBe("low") + }) + + test("category reasoningEffort applied when no direct override", async () => { + // #given - category has reasoningEffort but no direct override + const pluginConfig: OhMyOpenCodeConfig = { + sisyphus_agent: { + planner_enabled: true, + }, + categories: { + "reasoning-cat": { + model: "openai/gpt-5.2", + reasoningEffort: "high", + }, + }, + agents: { + prometheus: { + category: "reasoning-cat", + }, + }, + } + const config: Record = { + model: "anthropic/claude-opus-4-5", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + // #when + await handler(config) + + // #then - category's reasoningEffort is applied + const agents = config.agent as Record + expect(agents.prometheus).toBeDefined() + expect(agents.prometheus.reasoningEffort).toBe("high") + }) + + test("direct temperature takes priority over category temperature", async () => { + // #given + const pluginConfig: OhMyOpenCodeConfig = { + sisyphus_agent: { + planner_enabled: true, + }, + categories: { + "temp-cat": { + model: "openai/gpt-5.2", + temperature: 0.8, + }, + }, + agents: { + prometheus: { + category: "temp-cat", + temperature: 0.1, + }, + }, + } + const config: Record = { + model: "anthropic/claude-opus-4-5", + agent: {}, + } + const handler = createConfigHandler({ + ctx: { directory: "/tmp" }, + pluginConfig, + modelCacheState: { + anthropicContext1MEnabled: false, + modelContextLimitsCache: new Map(), + }, + }) + + // #when + await handler(config) + + // #then - direct temperature wins over category + const agents = config.agent as Record + expect(agents.prometheus).toBeDefined() + expect(agents.prometheus.temperature).toBe(0.1) + }) +}) diff --git a/src/plugin-handlers/config-handler.ts b/src/plugin-handlers/config-handler.ts index 9039c771..37f7451f 100644 --- a/src/plugin-handlers/config-handler.ts +++ b/src/plugin-handlers/config-handler.ts @@ -227,7 +227,17 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { ); const prometheusOverride = pluginConfig.agents?.["prometheus"] as - | (Record & { category?: string; model?: string; variant?: string }) + | (Record & { + category?: string + model?: string + variant?: string + reasoningEffort?: string + textVerbosity?: string + thinking?: { type: string; budgetTokens?: number } + temperature?: number + top_p?: number + maxTokens?: number + }) | undefined; const categoryConfig = prometheusOverride?.category @@ -248,12 +258,18 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { userModel: prometheusOverride?.model ?? categoryConfig?.model, fallbackChain: prometheusRequirement?.fallbackChain, availableModels, - systemDefaultModel: undefined, // let fallback chain handle this + systemDefaultModel: undefined, }); const resolvedModel = modelResolution?.model; const resolvedVariant = modelResolution?.variant; const variantToUse = prometheusOverride?.variant ?? resolvedVariant; + const reasoningEffortToUse = prometheusOverride?.reasoningEffort ?? categoryConfig?.reasoningEffort; + const textVerbosityToUse = prometheusOverride?.textVerbosity ?? categoryConfig?.textVerbosity; + const thinkingToUse = prometheusOverride?.thinking ?? categoryConfig?.thinking; + const temperatureToUse = prometheusOverride?.temperature ?? categoryConfig?.temperature; + const topPToUse = prometheusOverride?.top_p ?? categoryConfig?.top_p; + const maxTokensToUse = prometheusOverride?.maxTokens ?? categoryConfig?.maxTokens; const prometheusBase = { name: "prometheus", ...(resolvedModel ? { model: resolvedModel } : {}), @@ -263,22 +279,16 @@ export function createConfigHandler(deps: ConfigHandlerDeps) { permission: PROMETHEUS_PERMISSION, description: `${configAgent?.plan?.description ?? "Plan agent"} (Prometheus - OhMyOpenCode)`, color: (configAgent?.plan?.color as string) ?? "#FF6347", - ...(categoryConfig?.temperature !== undefined - ? { temperature: categoryConfig.temperature } - : {}), - ...(categoryConfig?.top_p !== undefined - ? { top_p: categoryConfig.top_p } - : {}), - ...(categoryConfig?.maxTokens !== undefined - ? { maxTokens: categoryConfig.maxTokens } - : {}), + ...(temperatureToUse !== undefined ? { temperature: temperatureToUse } : {}), + ...(topPToUse !== undefined ? { top_p: topPToUse } : {}), + ...(maxTokensToUse !== undefined ? { maxTokens: maxTokensToUse } : {}), ...(categoryConfig?.tools ? { tools: categoryConfig.tools } : {}), - ...(categoryConfig?.thinking ? { thinking: categoryConfig.thinking } : {}), - ...(categoryConfig?.reasoningEffort !== undefined - ? { reasoningEffort: categoryConfig.reasoningEffort } + ...(thinkingToUse ? { thinking: thinkingToUse } : {}), + ...(reasoningEffortToUse !== undefined + ? { reasoningEffort: reasoningEffortToUse } : {}), - ...(categoryConfig?.textVerbosity !== undefined - ? { textVerbosity: categoryConfig.textVerbosity } + ...(textVerbosityToUse !== undefined + ? { textVerbosity: textVerbosityToUse } : {}), };