From 63c9ae89dae1eac8ebf63e07dc5e3552f6c3e591 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 21 Jul 2026 14:35:36 -0700 Subject: [PATCH] fix(brainstorm): price cost preview against the configured chat model + models.brainstorm.judge config key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Takeover of PR #1855 by @starm2010, shrunk to the brainstorm-only portion (the cycle-phase hunks are superseded by the resolveModel-in- phase approach already on master). The cost preview + hard cost ceiling previously always priced anthropic:claude-sonnet-4-6 even when the configured chat_model (which the gateway actually runs) was something else; modelStr now resolves override → config.chat_model → fallback. The judge phase honors a new models.brainstorm.judge config key when no --judge-model flag is passed, resolved in the orchestrator so every caller (brainstorm, lsd, eval-brainstorm) benefits. Co-authored-by: starm2010 Co-Authored-By: Claude Fable 5 --- src/core/brainstorm/orchestrator.ts | 45 ++++++++++++++++--- src/core/config.ts | 1 + test/brainstorm/model-config.test.ts | 65 ++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 test/brainstorm/model-config.test.ts diff --git a/src/core/brainstorm/orchestrator.ts b/src/core/brainstorm/orchestrator.ts index 3ac7caef2..4d3f20799 100644 --- a/src/core/brainstorm/orchestrator.ts +++ b/src/core/brainstorm/orchestrator.ts @@ -485,9 +485,44 @@ const DEFAULT_PARALLELISM = 4; * src/core/errors.ts (the v0.19.0 envelope every new agent-facing * surface uses) rather than introducing a new BrainstormError class. */ +/** File-config slice the orchestrator reads (see loadConfig in core/config.ts). */ +export interface BrainstormRunConfig { + embedding_model?: string; + chat_model?: string; + emotional_weight?: { user_holder?: string }; +} + +/** + * Model used for the cost preview + hard cost ceiling. Mirrors what the + * gateway will actually run: explicit --model override, else the configured + * chat_model (gateway default), else the hardcoded gateway fallback. Before + * this resolved through config, a non-Sonnet chat_model got its preview + * priced against the wrong model. (Takeover of PR #1855 by @starm2010.) + */ +export function resolveBrainstormChatModel( + config: { chat_model?: string }, + modelOverride?: string, +): string { + return modelOverride ?? config.chat_model ?? 'anthropic:claude-sonnet-4-6'; +} + +/** + * Judge-phase model precedence: --judge-model flag, else the + * `models.brainstorm.judge` config key, else undefined (falls back to + * `modelOverride` then the gateway default at the runJudge callsite). + */ +export async function resolveBrainstormJudgeModel( + engine: BrainEngine, + judgeModelFlag?: string, +): Promise { + if (judgeModelFlag) return judgeModelFlag; + const configured = await engine.getConfig('models.brainstorm.judge'); + return configured ?? undefined; +} + export async function runBrainstorm( engine: BrainEngine, - config: { embedding_model?: string; emotional_weight?: { user_holder?: string } }, + config: BrainstormRunConfig, opts: BrainstormOptions ): Promise { // v0.39.3.0 (Phase 5, CV11+T4): outer try/catch around the orchestrator @@ -509,7 +544,7 @@ export async function runBrainstorm( async function runBrainstormImpl( engine: BrainEngine, - config: { embedding_model?: string; emotional_weight?: { user_holder?: string } }, + config: BrainstormRunConfig, opts: BrainstormOptions, ): Promise { // v0.39.0.0 T10: install a gateway-layer BudgetTracker scope around the @@ -529,7 +564,7 @@ async function runBrainstormImpl( async function _runBrainstormInner( engine: BrainEngine, - config: { embedding_model?: string; emotional_weight?: { user_holder?: string } }, + config: BrainstormRunConfig, opts: BrainstormOptions, ): Promise { const profile = opts.profile ?? BRAINSTORM_PROFILE; @@ -538,7 +573,7 @@ async function _runBrainstormInner( const embedFn = opts.embedQueryFn ?? embedQuery; // ---- Phase 0: cost preview + TTY grace ---- - const modelStr = opts.modelOverride ?? 'anthropic:claude-sonnet-4-6'; + const modelStr = resolveBrainstormChatModel(config, opts.modelOverride); const { aborted, estimate } = await previewCostAndWait({ profile, model: modelStr, @@ -847,7 +882,7 @@ async function _runBrainstormInner( far_slug: i.far_slug, })); const judgeResult = await runJudge(profile.judge_config, judgeInput, { - modelOverride: opts.judgeModel ?? opts.modelOverride, + modelOverride: (await resolveBrainstormJudgeModel(engine, opts.judgeModel)) ?? opts.modelOverride, chatFn: opts.chatFn, activeBiasTags: activeBiasTags ?? undefined, abortSignal: opts.abortSignal, diff --git a/src/core/config.ts b/src/core/config.ts index 4ca00cdc9..e3117f97c 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -904,6 +904,7 @@ export const KNOWN_CONFIG_KEYS: readonly string[] = [ 'models.subagent', 'models.expansion', 'models.chat', + 'models.brainstorm.judge', 'models.eval.longmemeval', 'facts.extraction_model', // #2113: output-token cap for the per-turn facts extractor (default 4000). diff --git a/test/brainstorm/model-config.test.ts b/test/brainstorm/model-config.test.ts new file mode 100644 index 000000000..407ace90f --- /dev/null +++ b/test/brainstorm/model-config.test.ts @@ -0,0 +1,65 @@ +/** + * Brainstorm model configurability (takeover of PR #1855 by @starm2010). + * + * - The cost preview + hard cost ceiling price the model that will actually + * run: --model override → configured chat_model → gateway fallback. Before + * this, the preview always priced anthropic:claude-sonnet-4-6 even when + * the configured chat_model was something else. + * - The judge phase honors the `models.brainstorm.judge` config key when no + * --judge-model flag is passed. + */ + +import { describe, test, expect } from 'bun:test'; +import { + resolveBrainstormChatModel, + resolveBrainstormJudgeModel, +} from '../../src/core/brainstorm/orchestrator.ts'; +import type { BrainEngine } from '../../src/core/engine.ts'; + +function mockEngine(configValues: Record): { engine: BrainEngine; reads: string[] } { + const reads: string[] = []; + const engine = { + async getConfig(key: string): Promise { + reads.push(key); + return configValues[key] ?? null; + }, + } as unknown as BrainEngine; + return { engine, reads }; +} + +describe('resolveBrainstormChatModel', () => { + test('--model override wins over config', () => { + expect(resolveBrainstormChatModel({ chat_model: 'openai:gpt-5' }, 'anthropic:claude-opus-4-6')) + .toBe('anthropic:claude-opus-4-6'); + }); + + test('configured chat_model wins over the hardcoded fallback', () => { + expect(resolveBrainstormChatModel({ chat_model: 'openai:gpt-5' })) + .toBe('openai:gpt-5'); + }); + + test('falls back to the gateway default model when nothing is configured', () => { + expect(resolveBrainstormChatModel({})).toBe('anthropic:claude-sonnet-4-6'); + }); +}); + +describe('resolveBrainstormJudgeModel', () => { + test('--judge-model flag wins without touching config', async () => { + const { engine, reads } = mockEngine({ 'models.brainstorm.judge': 'openai:gpt-5' }); + const out = await resolveBrainstormJudgeModel(engine, 'anthropic:claude-opus-4-6'); + expect(out).toBe('anthropic:claude-opus-4-6'); + expect(reads).toHaveLength(0); + }); + + test('models.brainstorm.judge config key is honored when no flag is passed', async () => { + const { engine, reads } = mockEngine({ 'models.brainstorm.judge': 'openai:gpt-5' }); + const out = await resolveBrainstormJudgeModel(engine); + expect(out).toBe('openai:gpt-5'); + expect(reads).toEqual(['models.brainstorm.judge']); + }); + + test('returns undefined (defer to modelOverride / gateway default) when unset', async () => { + const { engine } = mockEngine({}); + expect(await resolveBrainstormJudgeModel(engine)).toBeUndefined(); + }); +});