diff --git a/src/core/brainstorm/orchestrator.ts b/src/core/brainstorm/orchestrator.ts index 494775c18..2c7460ad1 100644 --- a/src/core/brainstorm/orchestrator.ts +++ b/src/core/brainstorm/orchestrator.ts @@ -486,9 +486,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 @@ -510,7 +545,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 @@ -530,7 +565,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; @@ -539,7 +574,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, @@ -848,7 +883,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 bf81d77f9..e2fdc971b 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -962,6 +962,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/src/core/cycle/propose-takes.ts b/src/core/cycle/propose-takes.ts index 91cafead3..4fe1f1b2a 100644 --- a/src/core/cycle/propose-takes.ts +++ b/src/core/cycle/propose-takes.ts @@ -39,11 +39,11 @@ import { randomUUID, createHash } from 'node:crypto'; import { BaseCyclePhase, type ScopedReadOpts, type BasePhaseOpts } from './base-phase.ts'; -import { chat as gatewayChat, getChatModel } from '../ai/gateway.ts'; +import { chat as gatewayChat, getChatModel, probeChatModel } from '../ai/gateway.ts'; +import { normalizeModelId } from '../model-id.ts'; import { writeReceipt } from '../extract/receipt-writer.ts'; import { upsertExtractRollup } from '../extract/rollup-writer.ts'; import { GBrainError } from '../types.ts'; -import type { Page, PageFilters } from '../types.ts'; import type { OperationContext } from '../operations.ts'; import type { BrainEngine } from '../engine.ts'; import type { PhaseStatus, CyclePhase } from '../cycle.ts'; @@ -160,6 +160,48 @@ export interface ProposeTakesResult { warnings: string[]; } +/** Narrow projection of `pages` — the only columns this phase reads. */ +interface ProposeTakesPageRow { + slug: string; + source_id: string; + compiled_truth: string | null; +} + +/** + * Load proposal candidates with a narrow projection instead of + * `engine.listPages` (`SELECT p.*`). The phase only reads slug, source_id + * and compiled_truth — skipping timeline/frontmatter/title keeps large + * toasted columns out of the hot path. Scope precedence mirrors + * `sourceScopeOpts`: federated array (`sourceIds`) beats scalar + * (`sourceId`); ordering matches `PAGE_SORT_SQL.updated_desc` with an id + * tiebreak for determinism. (Takeover of PR #1979's projection by + * @shawnduggan.) + */ +async function listCandidatePages( + engine: BrainEngine, + scope: ScopedReadOpts, + limit: number, +): Promise { + const where = ['deleted_at IS NULL']; + const params: unknown[] = []; + if (scope.sourceIds && scope.sourceIds.length > 0) { + params.push(scope.sourceIds); + where.push(`source_id = ANY($${params.length}::text[])`); + } else if (scope.sourceId) { + params.push(scope.sourceId); + where.push(`source_id = $${params.length}`); + } + params.push(limit); + return engine.executeRaw( + `SELECT slug, source_id, compiled_truth + FROM pages + WHERE ${where.join(' AND ')} + ORDER BY updated_at DESC, id DESC + LIMIT $${params.length}`, + params, + ); +} + /** * Compute the content_hash key for the idempotency cache. SHA-256 of the * page body suffices — page slug + prompt_version are separate columns in @@ -330,6 +372,34 @@ class ProposeTakesPhase extends BaseCyclePhase { const phaseStartMs = Date.now(); const proposalRunId = `propose-${new Date().toISOString().slice(0, 19).replace(/[-:T]/g, '')}-${randomUUID().slice(0, 8)}`; + const modelId = opts.model ?? getChatModel(); + + // With the default (gateway) extractor, skip cheaply when the resolved + // model's provider can't run — same probe semantics as patterns.ts / + // think/index.ts: unknown provider/model or Anthropic-without-key skips; + // other providers' auth surfaces lazily at chat() time. An injected + // extractor bypasses the gateway, so it is never gated. (Takeover of + // PR #1979's intent by @shawnduggan.) + if (!opts.extractor) { + const probe = probeChatModel(normalizeModelId(modelId)); + if (!probe.ok) { + return { + summary: `propose_takes skipped: ${probe.detail}`, + details: { + reason: 'no_provider', + model: modelId, + pages_scanned: 0, + cache_hits: 0, + cache_misses: 0, + proposals_inserted: 0, + budget_exhausted: false, + warnings: [], + }, + status: 'skipped', + }; + } + } + const result: ProposeTakesResult = { pages_scanned: 0, cache_hits: 0, @@ -340,19 +410,12 @@ class ProposeTakesPhase extends BaseCyclePhase { }; // Load pages eligible for proposal. Source-scoped per BaseCyclePhase. - const pageFilters: PageFilters = { - ...scope, - limit: pageLimit, - sort: 'updated_desc', - }; - const pages: Page[] = await engine.listPages(pageFilters); + const pages = await listCandidatePages(engine, scope, pageLimit); if (opts.reporter) { opts.reporter.start('propose_takes.pages' as never, pages.length); } - const modelId = opts.model ?? getChatModel(); - for (const page of pages) { // Phase deadline check. Break (not throw) so the phase returns a // partial result with deadline_hit:true; work already banked stays. @@ -509,4 +572,5 @@ export const __testing = { contentHash, hasCompleteFence, extractExistingTakesForDedup, + listCandidatePages, }; 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(); + }); +}); diff --git a/test/propose-takes.test.ts b/test/propose-takes.test.ts index af5674a0d..fbe8deb26 100644 --- a/test/propose-takes.test.ts +++ b/test/propose-takes.test.ts @@ -15,6 +15,7 @@ */ import { describe, test, expect } from 'bun:test'; +import { withEnv, emptyHome } from './helpers/with-env.ts'; import { runPhaseProposeTakes, parseExtractorOutput, @@ -52,6 +53,14 @@ function buildMockEngine(opts: { }, async executeRaw(sql: string, params?: unknown[]): Promise { captured.push({ sql, params: params ?? [] }); + // Narrow candidate-page projection (replaces listPages in the phase). + if (sql.includes('SELECT slug, source_id, compiled_truth')) { + return opts.pages.map((p) => ({ + slug: p.slug, + source_id: p.source_id, + compiled_truth: p.compiled_truth, + })) as T[]; + } // SELECT idempotency check if (sql.includes('SELECT id FROM take_proposals')) { const [sourceId, slug, ch, pv] = params ?? []; @@ -476,4 +485,72 @@ New prose appended here.`; resetGateway(); } }); + + test('default extractor skips cleanly when the Anthropic chat model has no key', async () => { + // Empty GBRAIN_HOME so hasAnthropicKey's config-file fallback can't find + // the operator's real key. + await withEnv({ GBRAIN_HOME: emptyHome(), ANTHROPIC_API_KEY: undefined }, async () => { + configureGateway({ chat_model: 'anthropic:claude-sonnet-4-6', env: {} }); + try { + const { engine, captured } = buildMockEngine({ + pages: [buildPage({ slug: 'wiki/a', body: 'claim-ish prose' })], + }); + const result = await runPhaseProposeTakes(buildCtx(engine)); + + expect(result.status).toBe('skipped'); + expect((result.details as Record).reason).toBe('no_provider'); + // Skips BEFORE touching the engine — no page scan, no cache probes. + expect(captured).toHaveLength(0); + } finally { + resetGateway(); + } + }); + }); + + test('an injected extractor is never gated on provider availability', async () => { + await withEnv({ GBRAIN_HOME: emptyHome(), ANTHROPIC_API_KEY: undefined }, async () => { + configureGateway({ chat_model: 'anthropic:claude-sonnet-4-6', env: {} }); + try { + const { engine } = buildMockEngine({ + pages: [buildPage({ slug: 'wiki/b', body: 'still processed' })], + }); + const extractor: ProposeTakesExtractor = async () => []; + const result = await runPhaseProposeTakes(buildCtx(engine), { extractor }); + + expect(result.status).toBe('ok'); + expect((result.details as Record).pages_scanned).toBe(1); + } finally { + resetGateway(); + } + }); + }); + + test('loads proposal candidates with a narrow page projection', async () => { + const pages = [buildPage({ slug: 'wiki/narrow', body: 'A narrow projection avoids unrelated page columns.' })]; + const { engine, captured } = buildMockEngine({ pages }); + const extractor: ProposeTakesExtractor = async () => []; + await runPhaseProposeTakes(buildCtx(engine), { extractor }); + + const pageSelect = captured.find(c => c.sql.includes('FROM pages')); + expect(pageSelect).toBeDefined(); + expect(pageSelect!.sql).toContain('SELECT slug, source_id, compiled_truth'); + expect(pageSelect!.sql).not.toContain('*'); + // Scalar sourceId scope from ctx binds as a plain equality param. + expect(pageSelect!.params[0]).toBe('default'); + }); + + test('narrow projection: federated sourceIds beat scalar sourceId', async () => { + const { engine, captured } = buildMockEngine({ pages: [] }); + const extractor: ProposeTakesExtractor = async () => []; + const ctx = { + ...buildCtx(engine), + auth: { allowedSources: ['team-a', 'team-b'] }, + } as OperationContext; + await runPhaseProposeTakes(ctx, { extractor }); + + const pageSelect = captured.find(c => c.sql.includes('FROM pages')); + expect(pageSelect).toBeDefined(); + expect(pageSelect!.sql).toContain('source_id = ANY('); + expect(pageSelect!.params[0]).toEqual(['team-a', 'team-b']); + }); });