* 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
This commit is contained in:
parent
b4973954e3
commit
23b49c4a5c
@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
@ -120,6 +120,33 @@ export function createEnvContext(): string {
|
||||
</omo-env>`
|
||||
}
|
||||
|
||||
/**
|
||||
* 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<string, CategoryConfig>
|
||||
): AgentConfig {
|
||||
const categoryConfig = mergedCategories[categoryName]
|
||||
if (!categoryConfig) return config
|
||||
|
||||
const result = { ...config } as AgentConfig & Record<string, unknown>
|
||||
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<string, unknown> | 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<string, unknown> | 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<string, unknown> | undefined)?.category as string | undefined
|
||||
if (atlasOverrideCategory) {
|
||||
orchestratorConfig = applyCategoryOverride(orchestratorConfig, atlasOverrideCategory, mergedCategories)
|
||||
}
|
||||
|
||||
if (orchestratorOverride) {
|
||||
orchestratorConfig = mergeAgentConfig(orchestratorConfig, orchestratorOverride)
|
||||
}
|
||||
|
||||
@ -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<string, unknown> = {
|
||||
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<string, { reasoningEffort?: string }>
|
||||
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<string, unknown> = {
|
||||
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<string, { reasoningEffort?: string }>
|
||||
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<string, unknown> = {
|
||||
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<string, { temperature?: number }>
|
||||
expect(agents.prometheus).toBeDefined()
|
||||
expect(agents.prometheus.temperature).toBe(0.1)
|
||||
})
|
||||
})
|
||||
|
||||
@ -227,7 +227,17 @@ export function createConfigHandler(deps: ConfigHandlerDeps) {
|
||||
);
|
||||
const prometheusOverride =
|
||||
pluginConfig.agents?.["prometheus"] as
|
||||
| (Record<string, unknown> & { category?: string; model?: string; variant?: string })
|
||||
| (Record<string, unknown> & {
|
||||
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 }
|
||||
: {}),
|
||||
};
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user