mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(cycle): grade_takes and calibration_profile record the model they actually run — follow the gateway chat model (#3726)
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 <p3ob7o@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Paolo Belcastro
Claude Fable 5
parent
630786dc65
commit
2e554500e2
@@ -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<string[]>;
|
||||
|
||||
/** 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');
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
@@ -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<string | undefined> = [];
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string | undefined> = [];
|
||||
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();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user