From 2e554500e248a765e022c17beb09a42bc4fe0672 Mon Sep 17 00:00:00 2001 From: Paolo Belcastro <1436372+p3ob7o@users.noreply.github.com> Date: Sun, 2 Aug 2026 08:26:51 -0400 Subject: [PATCH] =?UTF-8?q?fix(cycle):=20grade=5Ftakes=20and=20calibration?= =?UTF-8?q?=5Fprofile=20record=20the=20model=20they=20actually=20run=20?= =?UTF-8?q?=E2=80=94=20follow=20the=20gateway=20chat=20model=20(#3726)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cycle phases still carried the label-vs-actual defect that propose_takes shed in v0.42.62 (and that the #2805 review noted was worth tracking so it isn't lost): - grade_takes hardcoded 'claude-sonnet-4-6' into judge_model_id, the evidence signature, and budget metering, while the default judge call passed NO model hint and rode the gateway's chat_model. On any brain with a non-default chat_model, the verdict cache and telemetry recorded a model that never ran. - calibration_profile persisted TIER_DEFAULTS.reasoning to model_id but never passed a model to the patterns generator at all, so the recorded model and the executed one were unrelated. Fix, matching the propose_takes convention: each phase resolves ONE string via explicit override > getChatModel(), and that string drives the chat call, the cache key, and the stored id. - grade_takes: the judge hint gets the FULL provider-prefixed string; the stored judge_model_id and evidence signature keep the historical bare tail. Stock installs are unchanged: getChatModel() defaults to 'anthropic:claude-sonnet-4-6', whose tail equals the old hardcoded value, so no verdict cache invalidates. A genuinely different chat_model invalidates, which is correct — the judge really changed. - calibration_profile: getChatModel() is provider-prefixed, preserving the #2451 contract, and its default IS the old TIER_DEFAULTS.reasoning value — stock behavior unchanged. The generator now receives the same resolved string that is persisted to model_id. Tests: regression per phase pinning configured-chat-model routing (full string to the call, bare tail to the cache key for grade_takes, full string persisted for calibration); all existing suites pass unchanged, pinning the no-change-on-stock property. 112 tests across the four affected suites; typecheck clean. Co-authored-by: Paolo Belcastro Co-authored-by: Claude Fable 5 --- src/core/cycle/calibration-profile.ts | 17 ++++++++++--- src/core/cycle/grade-takes.ts | 21 +++++++++++++--- test/calibration-profile.test.ts | 30 ++++++++++++++++++++++ test/grade-takes.test.ts | 36 +++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/core/cycle/calibration-profile.ts b/src/core/cycle/calibration-profile.ts index 56e995d54..857ffeaa6 100644 --- a/src/core/cycle/calibration-profile.ts +++ b/src/core/cycle/calibration-profile.ts @@ -27,8 +27,7 @@ import { BaseCyclePhase, type ScopedReadOpts, type BasePhaseOpts } from './base-phase.ts'; import { resolveOwnerHolder } from '../owner-holder.ts'; -import { chat as gatewayChat } from '../ai/gateway.ts'; -import { TIER_DEFAULTS } from '../model-config.ts'; +import { chat as gatewayChat, getChatModel } from '../ai/gateway.ts'; import { gateVoice, type VoiceGateGenerator, type VoiceGateJudge } from '../calibration/voice-gate.ts'; import { patternStatementTemplate, type PatternStatementSlots } from '../calibration/templates.ts'; // v0.41 T10 — domain widening. The aggregator module resolves the active @@ -91,6 +90,8 @@ export type PatternStatementsGenerator = (input: { holder: string; attempt: number; feedback?: string; + /** Provider-prefixed model the phase resolved; drives the generator's chat call. */ + modelHint?: string; }) => Promise; /** Generator function for bias tags (test seam). */ @@ -233,7 +234,14 @@ class CalibrationProfilePhase extends BaseCyclePhase { configValue: await engine.getConfig('emotional_weight.user_holder'), }); const promptVersion = opts.promptVersion ?? CALIBRATION_PROFILE_PROMPT_VERSION; - const modelId = opts.model ?? TIER_DEFAULTS.reasoning; + // Follow the gateway's configured chat model, matching propose_takes + // (v0.42.62) and grade_takes: previously the generator stayed pinned to + // the TIER_DEFAULTS.reasoning constant, ignoring a configured + // chat_model. getChatModel() is provider-prefixed, preserving the #2451 + // contract (a bare id fed back into gateway.chat() throws), and its + // default IS 'anthropic:claude-sonnet-4-6' — identical to the old + // constant — so stock installs are unchanged. + const modelId = opts.model ?? getChatModel(); const gradeCompletion = opts.gradeCompletion ?? 1.0; const patternsGenerator = opts.patternsGenerator ?? defaultPatternsGenerator; const biasTagsGenerator = opts.biasTagsGenerator ?? defaultBiasTagsGenerator; @@ -269,6 +277,9 @@ class CalibrationProfilePhase extends BaseCyclePhase { scorecard, holder, attempt, + // The same resolved string that is persisted to model_id drives the + // generator's chat call — the phase can't record a model it didn't run. + modelHint: modelId, ...(feedback !== undefined ? { feedback } : {}), }); return lines.join('\n'); diff --git a/src/core/cycle/grade-takes.ts b/src/core/cycle/grade-takes.ts index e5e1d491f..ac47e8826 100644 --- a/src/core/cycle/grade-takes.ts +++ b/src/core/cycle/grade-takes.ts @@ -36,7 +36,8 @@ import { createHash } from 'node:crypto'; import { BaseCyclePhase, type ScopedReadOpts, type BasePhaseOpts } from './base-phase.ts'; -import { chat as gatewayChat } from '../ai/gateway.ts'; +import { chat as gatewayChat, getChatModel } from '../ai/gateway.ts'; +import { splitProviderModelId } from '../model-id.ts'; import { GBrainError } from '../types.ts'; import type { OperationContext } from '../operations.ts'; import type { BrainEngine, Take, TakeResolution } from '../engine.ts'; @@ -395,7 +396,21 @@ class GradeTakesPhase extends BaseCyclePhase { const autoResolve = opts.autoResolve ?? false; // D17 default OFF const autoResolveThreshold = opts.autoResolveThreshold ?? 0.95; // D12 conservative const resolvedByLabel = opts.resolvedByLabel ?? 'gbrain:grade_takes'; - const judgeModelId = opts.model ?? 'claude-sonnet-4-6'; + // One resolved string drives the judge call, the verdict-cache key, and + // the stored judge_model_id — the convention propose_takes adopted in + // v0.42.62. Previously the default judge call passed NO model hint (it + // rode the gateway's chat_model) while 'claude-sonnet-4-6' was hardcoded + // into judge_model_id, the evidence signature, and budget metering — on + // brains with a different chat_model, telemetry priced and recorded a + // model that never ran. + const judgeModelFull = opts.model ?? getChatModel(); + // Bare tail for the stored judge_model_id + evidence signature + // (historical convention). Stock installs are unchanged: getChatModel() + // defaults to 'anthropic:claude-sonnet-4-6', whose tail equals the old + // hardcoded value — zero verdict-cache invalidation. A genuinely + // different chat_model invalidates, which is correct: the judge really + // changed. + const judgeModelId = splitProviderModelId(judgeModelFull).model || judgeModelFull; const useEnsemble = opts.useEnsemble ?? false; const ensembleThreshold = opts.ensembleThreshold ?? 0.85; @@ -468,7 +483,7 @@ class GradeTakesPhase extends BaseCyclePhase { // Call the single-model judge. Errors on a single take log warning + continue. let verdict: JudgeVerdict; try { - verdict = await judge({ take, evidence, modelHint: opts.model }); + verdict = await judge({ take, evidence, modelHint: judgeModelFull }); } catch (err) { const msg = err instanceof Error ? err.message : String(err); result.warnings.push(`judge failed on take ${take.id}: ${msg}`); diff --git a/test/calibration-profile.test.ts b/test/calibration-profile.test.ts index ff40c1afd..ae5566607 100644 --- a/test/calibration-profile.test.ts +++ b/test/calibration-profile.test.ts @@ -355,3 +355,33 @@ describe('runPhaseCalibrationProfile — phase integration', () => { expect(result.summary).toContain('holder=people/charlie-example'); }); }); + +describe('generator model follows the gateway chat model', () => { + test('configured chat_model drives the generator hint and the persisted model_id', async () => { + // Regression: the generator previously stayed pinned to the + // TIER_DEFAULTS.reasoning constant, ignoring a configured chat_model — + // unlike propose_takes (v0.42.62 convention). Stock behavior is + // unchanged (the gateway default equals the old constant). + const { configureGateway, resetGateway } = await import('../src/core/ai/gateway.ts'); + configureGateway({ chat_model: 'openai:gpt-5', env: { OPENAI_API_KEY: 'test-key' } }); + try { + const { engine, captured } = buildMockEngine({ scorecard: ENOUGH_RESOLVED_SCORECARD }); + const hints: Array = []; + const patternsGenerator: PatternStatementsGenerator = async ({ modelHint }) => { + hints.push(modelHint); + return ['You call early-stage tactics well — 8 of 10 held up.']; + }; + await runPhaseCalibrationProfile(buildCtx(engine), { + patternsGenerator, + biasTagsGenerator: async () => [], + voiceGateJudge: passJudge, + }); + expect(hints).toEqual(['openai:gpt-5']); + const insert = captured.find(c => c.sql.includes('INSERT INTO calibration_profiles')); + expect(insert).toBeDefined(); + expect(insert!.params).toContain('openai:gpt-5'); // persisted model_id = full configured string + } finally { + resetGateway(); + } + }); +}); diff --git a/test/grade-takes.test.ts b/test/grade-takes.test.ts index c8bd3e3d8..bc1819e0e 100644 --- a/test/grade-takes.test.ts +++ b/test/grade-takes.test.ts @@ -328,3 +328,39 @@ describe('runPhaseGradeTakes — phase integration', () => { expect((details.warnings as string[])[0]).toContain('judge timeout'); }); }); + +// ─── judge model follows the gateway chat model ───────────────────── + +describe('judge model follows the gateway chat model (label = actual)', () => { + test('configured chat_model drives the judge hint (full string) and the stored judge_model_id (bare tail)', async () => { + // Regression: the default judge call previously passed NO model hint + // (riding the gateway's chat_model) while 'claude-sonnet-4-6' was + // hardcoded into judge_model_id + the evidence signature — on brains + // with a different chat_model, the cache and telemetry recorded a model + // that never ran. + const { configureGateway, resetGateway } = await import('../src/core/ai/gateway.ts'); + configureGateway({ chat_model: 'openai:gpt-5', env: { OPENAI_API_KEY: 'test-key' } }); + try { + const takes = [buildTake({ id: 1, sinceDate: '2023-01-01' })]; + const { engine, captured } = buildMockEngine({ takes }); + const hints: Array = []; + const judge: JudgeFn = async ({ modelHint }) => { + hints.push(modelHint); + return { verdict: 'correct', confidence: 0.9, reasoning: 'held' }; + }; + const evidenceRetriever: EvidenceRetrieverFn = async () => 'evidence body'; + const result = await runPhaseGradeTakes(buildCtx(engine), { judge, evidenceRetriever }); + expect(result.status).toBe('ok'); + expect(hints).toEqual(['openai:gpt-5']); // the chat call gets the FULL string + const inserts = captured.filter(c => c.sql.includes('INSERT INTO take_grade_cache')); + expect(inserts).toHaveLength(1); + expect(inserts[0]!.params[2]).toBe('gpt-5'); // stored judge_model_id is the bare tail + // The evidence signature is keyed on the same bare tail, so a stock + // install (default chat model tail == the old hardcoded value) sees + // zero cache invalidation from this change. + expect(inserts[0]!.params[3]).toBe(evidenceSignature('evidence body', 'gpt-5')); + } finally { + resetGateway(); + } + }); +});