From 84646914964470e4d2e6aeaf4e50e6a85e6e52c8 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 2 Jun 2026 21:57:29 -0700 Subject: [PATCH] fix(skillopt): feed the scorer's success criteria to the optimizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaced by the SkillOpt real-LLM eval (Track B). The reflect step was shown only a pass/fail score and the agent transcript — never WHAT the benchmark judge rewards. On a skill judged by structure (e.g. "must include a Confidence: line") the optimizer proposed plausible-but-off edits ("close with a synthesis") that never satisfied the literal check; every candidate scored 0 on D_sel, the validation gate rejected them all, and the skill text never changed (optimized === baseline === 0). Fix: render each benchmark Judge (rule checks / llm rubric / qrels) into plain-English criteria via new exported describeJudge / describeJudges, and thread them into the reflect prompt (a SUCCESS CRITERIA block) for both the loop reflect calls and the one-shot-rewrite path. The orchestrator computes the distinct criteria across train+sel+test once. The optimizer system prompt now instructs it to satisfy the criteria through genuine content, never empty keywords — reward-hacking stays defended by the independent held-out gate (cat32 confirms the gate catches a keyword-stuffing hack). End-to-end this took a deficient skill from 0.00 to 1.00 on a held-out set it never trained on. Pinned by test/skillopt/reflect.test.ts (describeJudge per kind, describeJudges dedup, criteria present/absent in the prompt). Folds into the open v0.42.9.0 PR (#1759). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 10 +++++ src/core/skillopt/orchestrator.ts | 13 +++++- src/core/skillopt/reflect.ts | 69 ++++++++++++++++++++++++++--- test/skillopt/reflect.test.ts | 72 ++++++++++++++++++++++++++++++- 4 files changed, 157 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f01d711b6..1f22264dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,16 @@ command, no migration. score — a pricing crash that looked exactly like a real "0 out of N" measurement. The pricing entry is added, and the gate now re-throws budget/pricing errors loudly instead of recording a hollow zero. Surfaced by the SkillOpt real-LLM eval. +- **The optimizer now knows what the scorer rewards.** `gbrain skillopt`'s reflect step + was only shown a pass/fail score and the agent's transcript, never the benchmark's + success criteria — so on a skill judged by structure (e.g. "must include a Confidence: + line") it proposed plausible-but-off edits that never satisfied the check, every + candidate scored 0, the validation gate rejected them all, and the skill never changed. + The reflect prompt now includes a plain-English description of exactly how the output is + scored, with an instruction to satisfy it through genuine content, not empty keywords. + In an end-to-end run this took a deficient skill from 0.00 to 1.00 on a held-out set it + never trained on. Reward-hacking is still defended by the independent held-out gate. + Surfaced by the SkillOpt real-LLM eval. - **`--no-mutate` now writes `proposed.md`** with the winning rewrite (was a stub that wrote nothing). - **`--max-runtime-min` is enforced** via a wall-clock deadline between optimization steps. diff --git a/src/core/skillopt/orchestrator.ts b/src/core/skillopt/orchestrator.ts index 58964ab8c..ffb858a83 100644 --- a/src/core/skillopt/orchestrator.ts +++ b/src/core/skillopt/orchestrator.ts @@ -92,7 +92,7 @@ import { withSkilloptLock } from './lock.ts'; import { resolveLrSchedule } from './lr-schedule.ts'; import { preflight, formatPreflightReport } from './preflight.ts'; import { isRejected, loadRejectedBuffer, makeRejectedEntry, saveRejectedBuffer } from './rejected-buffer.ts'; -import { runReflect, runOneShotRewrite } from './reflect.ts'; +import { runReflect, runOneShotRewrite, describeJudges } from './reflect.ts'; import { acceptCandidate, bestPath, revertAllPending, skillPath, writeProposed } from './version-store.ts'; import { runValidationGate, scoreSkillOnTasks } from './validate-gate.ts'; import { ROLLOUT_SUCCESS_THRESHOLD } from './types.ts'; @@ -248,6 +248,15 @@ async function runOptimizationLoop( const { skillName, skillsDir } = opts; const skillFile = skillPath(skillsDir, skillName); + // Plain-English success criteria from the benchmark judges, fed to the + // optimizer's reflect step so it targets WHAT the scorer rewards instead of + // guessing from a bare pass/fail score. Without this, rule-judged benchmarks + // (e.g. "must include a Confidence: line") are near-unoptimizable: the + // optimizer proposes plausible-but-off edits, the candidate scores 0, the gate + // rejects it, and the skill never changes. The held-out gate defends against + // the optimizer gaming these criteria at the expense of real quality. + const benchmarkCriteria = describeJudges([...split.train, ...split.sel, ...split.test]); + // Crash-recovery sweep (D8): revert any pending rows from a prior crashed run. revertAllPending(skillsDir, skillName); @@ -369,6 +378,7 @@ async function runOptimizationLoop( successes: fwd.scoredRollouts.filter((r) => r.score >= ROLLOUT_SUCCESS_THRESHOLD), failures: fwd.scoredRollouts.filter((r) => r.score < ROLLOUT_SUCCESS_THRESHOLD), rejected: [], + criteria: benchmarkCriteria, optimizerModel: opts.optimizerModel, }); if (rewrite.newBody) { @@ -456,6 +466,7 @@ async function runOptimizationLoop( successes, failures, rejected, + criteria: benchmarkCriteria, optimizerModel: opts.optimizerModel, ...(opts.reflectMode ? { reflectMode: opts.reflectMode } : {}), abortSignal: undefined, diff --git a/src/core/skillopt/reflect.ts b/src/core/skillopt/reflect.ts index 24f672f67..1a48fb82c 100644 --- a/src/core/skillopt/reflect.ts +++ b/src/core/skillopt/reflect.ts @@ -14,9 +14,56 @@ */ import { chat as gatewayChat } from '../ai/gateway.ts'; -import type { EditOp, ScoredRollout } from './types.ts'; +import type { EditOp, ScoredRollout, Judge, RuleCheck } from './types.ts'; import type { RejectedEntry } from './rejected-buffer.ts'; +/** + * Render ONE rule check as a plain-English requirement the optimizer can target. + */ +function describeCheck(c: RuleCheck): string { + switch (c.op) { + case 'contains': return `the output must contain the exact text \`${c.arg}\``; + case 'regex': return `the output must match the regular expression \`/${c.arg}/\``; + case 'section_present': return `the output must include a markdown heading titled "${c.arg}" (any heading level, case-insensitive)`; + case 'max_chars': return `the output must be at most ${c.arg} characters long`; + case 'min_citations': return `the output must include at least ${c.arg} citation(s)`; + case 'tool_called': return `the agent must call the \`${c.arg}\` tool at least once`; + case 'tool_not_called': return `the agent must NOT call the \`${c.arg}\` tool`; + } +} + +/** + * Render a Judge into the plain-English criteria the scorer rewards, so the + * optimizer knows WHAT it is optimizing toward. Without this the optimizer only + * sees a pass/fail score and has to reverse-engineer the target from behavior + * alone — which fails for rule judges that require a specific structure (e.g. a + * literal "Confidence:" line): it proposes plausible-but-off edits that never + * satisfy the rule, the candidate scores 0, the gate rejects it, and the skill + * never changes. Reward-hacking is defended separately by the held-out gate. + */ +export function describeJudge(judge: Judge): string { + switch (judge.kind) { + case 'rule': return judge.checks.map((c) => `- ${describeCheck(c)}`).join('\n'); + case 'llm': return `- the output is graded 0..1 by an LLM judge against this rubric:\n "${judge.rubric}"`; + case 'qrels': return `- the agent must retrieve the expected pages (scored recall@${judge.k})`; + } +} + +/** + * Describe the DISTINCT success criteria across a set of benchmark tasks. Most + * benchmarks use one judge shape for every task, so this collapses to a single + * block; heterogeneous benchmarks list each distinct shape once. + */ +export function describeJudges(tasks: ReadonlyArray<{ judge: Judge }>): string { + const seen = new Set(); + const blocks: string[] = []; + for (const t of tasks) { + const desc = describeJudge(t.judge); + if (!seen.has(desc)) { seen.add(desc); blocks.push(desc); } + } + return blocks.join('\n'); +} + const FAILURE_REFLECT_SYSTEM = `You are SkillOpt's optimizer. You analyze AGENT FAILURE TRAJECTORIES and propose specific edits to a SKILL document so the agent does better next time. Output ONLY a single JSON object on one or more lines: @@ -33,7 +80,8 @@ Rules: - Do NOT propose edits already in the rejected-edit history — those were tried and didn't help. - Be SURGICAL. Small targeted edits outperform large rewrites. - Do NOT modify the YAML frontmatter (triggers, brain_first, etc.) — that's out of scope. -- Output at MOST 8 edits. The orchestrator's LR budget will rank-and-clip further.`; +- Output at MOST 8 edits. The orchestrator's LR budget will rank-and-clip further. +- You may be given SUCCESS CRITERIA describing exactly how the agent's output is scored. Make your edits cause the agent to SATISFY those criteria, through genuine, high-quality content (a real section with real substance, a justified confidence level) — never by inserting empty keywords. An independent held-out check rejects edits that game the score while hurting real quality.`; const SUCCESS_REFLECT_SYSTEM = `You are SkillOpt's optimizer. You analyze AGENT SUCCESS TRAJECTORIES and propose specific edits to a SKILL document so the agent CONSISTENTLY does what worked here. @@ -51,6 +99,12 @@ export interface ReflectOpts { failures: ScoredRollout[]; /** Rejected-edit buffer for anti-bias context. */ rejected: readonly RejectedEntry[]; + /** + * Plain-English description of how the agent's output is scored (from + * `describeJudges(benchmarkTasks)`). Threaded into the reflect prompt so the + * optimizer targets the actual criteria instead of guessing from score alone. + */ + criteria?: string; optimizerModel: string; /** * Ablation (cat31 config B): 'failure-only' skips the D7 success-reflect call @@ -119,7 +173,7 @@ export interface OneShotRewriteResult { export async function runOneShotRewrite(opts: ReflectOpts): Promise { const usage = { input_tokens: 0, output_tokens: 0, cache_read_tokens: 0, cache_creation_tokens: 0 }; const chat = opts.chatFn ?? gatewayChat; - const userMsg = buildReflectUserMessage(opts.skillBodyText, [...opts.failures, ...opts.successes], opts.rejected); + const userMsg = buildReflectUserMessage(opts.skillBodyText, [...opts.failures, ...opts.successes], opts.rejected, opts.criteria); try { const result = await chat({ model: opts.optimizerModel, @@ -155,7 +209,7 @@ async function callReflect( errors: string[], ): Promise { const chat = opts.chatFn ?? gatewayChat; - const userMsg = buildReflectUserMessage(opts.skillBodyText, scoredRollouts, opts.rejected); + const userMsg = buildReflectUserMessage(opts.skillBodyText, scoredRollouts, opts.rejected, opts.criteria); try { const result = await chat({ model: opts.optimizerModel, @@ -181,6 +235,7 @@ function buildReflectUserMessage( skillBody: string, rollouts: ScoredRollout[], rejected: readonly RejectedEntry[], + criteria?: string, ): string { const trajectoryBlocks = rollouts.map((r, i) => { const tcSummary = r.trajectory.tool_calls @@ -199,8 +254,12 @@ ${r.rationale ? `JUDGE RATIONALE: ${r.rationale}` : ''}`; ? `\n\n--- PREVIOUSLY REJECTED EDITS (do not re-propose) ---\n${rejected.slice(0, 20).map((r) => `- ${r.reason}: ${JSON.stringify(r.edits)}`).join('\n')}` : ''; + const criteriaBlock = criteria + ? `\n\nSUCCESS CRITERIA (exactly how the agent's output is scored — make the agent satisfy these through genuine, high-quality content, never empty keywords):\n${criteria}` + : ''; + return `CURRENT SKILL BODY: -${truncate(skillBody, 5000)} +${truncate(skillBody, 5000)}${criteriaBlock} OBSERVED ROLLOUTS: ${trajectoryBlocks}${rejectedSummary} diff --git a/test/skillopt/reflect.test.ts b/test/skillopt/reflect.test.ts index 9eb431307..c645741ce 100644 --- a/test/skillopt/reflect.test.ts +++ b/test/skillopt/reflect.test.ts @@ -18,7 +18,7 @@ */ import { describe, expect, test } from 'bun:test'; -import { parseEditsResponse, runReflect, runOneShotRewrite } from '../../src/core/skillopt/reflect.ts'; +import { parseEditsResponse, runReflect, runOneShotRewrite, describeJudge, describeJudges } from '../../src/core/skillopt/reflect.ts'; import type { ChatOpts, ChatResult } from '../../src/core/ai/gateway.ts'; import type { ScoredRollout, Trajectory } from '../../src/core/skillopt/types.ts'; @@ -327,3 +327,73 @@ describe('runOneShotRewrite', () => { expect(r.error).toContain('one_shot_rewrite_failed'); }); }); + +// ─── Success criteria threading (v0.42.9.0) ────────────────────────────────── +// The optimizer must be TOLD how its output is scored, or it optimizes blind: +// on a rule-judged benchmark it proposes plausible-but-off edits, every +// candidate scores 0, the gate rejects them, and the skill never changes. These +// pin that the judge criteria render to plain English AND reach the reflect prompt. +describe('describeJudge / criteria threading', () => { + test('describeJudge renders each rule check as a plain requirement', () => { + const d = describeJudge({ kind: 'rule', checks: [ + { op: 'section_present', arg: 'Key Risks' }, + { op: 'regex', arg: '[Cc]onfidence\\s*[:=]' }, + { op: 'max_chars', arg: 1200 }, + { op: 'tool_called', arg: 'search' }, + ] }); + expect(d).toContain('Key Risks'); + expect(d).toContain('[Cc]onfidence'); + expect(d).toContain('1200'); + expect(d).toContain('search'); + }); + + test('describeJudge renders an llm rubric', () => { + expect(describeJudge({ kind: 'llm', rubric: 'reward genuine substance' })) + .toContain('reward genuine substance'); + }); + + test('describeJudges dedupes identical judge shapes across tasks', () => { + const j = { kind: 'rule' as const, checks: [{ op: 'contains' as const, arg: 'X' }] }; + const out = describeJudges([{ judge: j }, { judge: j }, { judge: j }]); + // One distinct shape → one block, not three. + expect(out.match(/contain the exact text/g)).toHaveLength(1); + }); + + test('runReflect injects the criteria block into the optimizer prompt', async () => { + let seenUser = ''; + const chatFn = async (opts: ChatOpts): Promise => { + const u = opts.messages[0]?.content; + seenUser = typeof u === 'string' ? u : ''; + return makeChatResult(JSON.stringify({ edits: [] })); + }; + await runReflect({ + skillBodyText: '# Test', + successes: [], + failures: [makeScored('f-1', 0.0)], + rejected: [], + criteria: 'CRITERIA: must include a Confidence: line', + optimizerModel: 'anthropic:claude-opus-4-7', + chatFn, + }); + expect(seenUser).toContain('SUCCESS CRITERIA'); + expect(seenUser).toContain('must include a Confidence: line'); + }); + + test('runReflect omits the criteria block when none is given', async () => { + let seenUser = ''; + const chatFn = async (opts: ChatOpts): Promise => { + const u = opts.messages[0]?.content; + seenUser = typeof u === 'string' ? u : ''; + return makeChatResult(JSON.stringify({ edits: [] })); + }; + await runReflect({ + skillBodyText: '# Test', + successes: [], + failures: [makeScored('f-1', 0.0)], + rejected: [], + optimizerModel: 'anthropic:claude-opus-4-7', + chatFn, + }); + expect(seenUser).not.toContain('SUCCESS CRITERIA'); + }); +});