mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-29 08:13:12 +00:00
fix(skillopt): wire trajectories from forward gate to reflect + fix parseEditsResponse parser misuse
Two related v0.42.0.0 bugs that conspired to make `runSkillOpt` structurally
unable to accept any candidate edit. Either alone would have killed self-evolution;
together they made the loop a no-op for every input.
**Bug 1 (orchestrator gap):** `runOptimizationLoop` in orchestrator.ts called
`runReflect({successes: [], failures: []})` with hardcoded empty arrays. The
forward gate's `scoredRollouts` were computed then voided. `runReflect`
short-circuits both modes when their batches are empty, so the optimizer was
never asked to propose an edit. Every step hit the no_edits_applied branch.
Fix: add `scoredRollouts: ScoredRollout[]` to `GateResult` and
`runsPerTask?: number` to `ValidateGateOpts`. Forward pass uses
`runsPerTask: 1`; orchestrator partitions returned rollouts by `score >= 0.5`
and threads real successes + failures into `runReflect`.
**Bug 2 (parser misuse):** `parseEditsResponse` in reflect.ts routed every
optimizer response through `parseJudgeJson` first. `parseJudgeJson` looks for
a `score` key (it's a judge-output parser, not an edits parser) and returns
null for any JSON without one — including the well-formed `{"edits": [...]}`
the optimizer is contractually required to emit. The function then early-
returned `[]` and the actual `tryExtractEdits` path on the next line was
unreachable dead code.
Fix: drop the wrong-typed guard. `parseEditsResponse` now calls
`tryExtractEdits` directly. Export it so `reflect.test.ts` can pin the
contract independently of the chat transport.
**Why this slipped through 152 prior skillopt tests:** zero unit coverage
of `parseEditsResponse` or `runReflect`. The existing E2E `all-reject` case
asserted no_improvement (which was true for the wrong reason — empty edits,
not gate rejection). Both bugs were structurally invisible to the existing
test surface.
**New coverage:**
- `test/skillopt/reflect.test.ts` (15 cases):
- 8 `parseEditsResponse` cases including the IRON-RULE regression pin
for the v0.42.0.1 fix (`{"edits": [...]}` JSON must survive the parser).
- 7 `runReflect` D7 contract cases: both modes fire, empty-batch skips,
additive token usage, one-mode-throws-other-still-works, rejected-buffer
flows into anti-bias prompt.
- Documents the trailing-comma limitation as an explicit out-of-scope pin
(so a future tightening of `tryExtractEdits` lights this test up
intentionally).
- `test/e2e/skillopt-loop.serial.test.ts` (7 cases):
- HAPPY PATH: stubbed `gateway.chat` acts as both target agent (emits
sections based on skill content) and optimizer (proposes a real
add-Citations edit). Drives `runSkillOpt` end-to-end against PGLite.
Asserts outcome=accepted, SKILL.md mutated with new section,
frontmatter preserved (D5), history has one committed row,
best.md mirrors disk, delta > epsilon, receipt fields populated.
- 5 broken cases (each isolates a distinct orchestrator-visible failure):
1. Below-baseline regression: optimizer proposes a destructive edit;
gate rejects with reason=below_baseline; SKILL.md unchanged;
rejected-buffer captures the bad edit for anti-bias context.
2. Malformed reflect JSON: orchestrator degrades gracefully to
no_improvement without crashing.
3. Anchor-not-found: applyEditBatch rejects all; sel gate skipped;
rejected-buffer captures with reason=apply_failed.
4. Budget exhausted mid-step: outcome=aborted, no pending rows survive.
5. Converged-skill re-run: starting from already-perfect skill →
no_improvement (no thrash on a well-tuned starting point).
- IDEMPOTENT RE-RUN: drive runSkillOpt twice in sequence. Run 1 accepts.
Run 2 sees improved baseline, no failures, returns no_improvement.
SKILL.md byte-identical to post-run-1; history still has exactly 1
committed row. Proves stability at the fixed point.
All hermetic (no DATABASE_URL, no API keys). PGLite in-memory engine,
tempdir SKILL.md + benchmark, stubbed gateway.chat via
`__setChatTransportForTests`. `.serial.test.ts` because the stub installs
module state and the loop walks shared disk state across epochs.
Test counts after fix: 174 skillopt-surface tests pass (149 pre-existing
unit + 15 new reflect unit + 3 existing E2E + 7 new E2E). Typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
6dcbb7ed9b
commit
f311f1368a
@@ -315,10 +315,9 @@ async function runOptimizationLoop(
|
||||
const batch = split.train.slice(batchStart, batchStart + opts.batchSize);
|
||||
|
||||
// FORWARD PASS: run rollouts on each batch task using current best.
|
||||
// We use the validation-gate primitive for a quick rough score —
|
||||
// for the train batch we only need 1 run per task, not median-of-3.
|
||||
// For brevity in v1, reuse the gate function with batch as selSet;
|
||||
// a follow-up TODO is to add a single-run rollout helper.
|
||||
// runsPerTask=1 — for the train batch we only need a rough partition
|
||||
// into successes/failures, not the median-of-3 noise rejection the
|
||||
// sel-side gate uses. ScoredRollouts come back via GateResult.
|
||||
const forwardGate = await runValidationGate({
|
||||
engine: opts.engine,
|
||||
candidateSkillText: checkpoint!.best_skill_text,
|
||||
@@ -326,21 +325,20 @@ async function runOptimizationLoop(
|
||||
bestScore: -1,
|
||||
targetModel: opts.targetModel,
|
||||
judgeModel: opts.judgeModel,
|
||||
runsPerTask: 1,
|
||||
});
|
||||
// Partition into successes vs failures (>= 0.5 threshold).
|
||||
// We don't have access to the raw trajectories from gate, so reflect
|
||||
// operates on summaries. Future: surface trajectories through gate.
|
||||
const successesCount = forwardGate.perTaskMedians.filter((t) => t.median >= 0.5).length;
|
||||
const failuresCount = forwardGate.perTaskMedians.length - successesCount;
|
||||
void failuresCount;
|
||||
void successesCount;
|
||||
// Partition into successes vs failures (>= 0.5 threshold). Reflect
|
||||
// gets the actual scored trajectories so failure-mode + success-mode
|
||||
// analysis can ground in real agent behavior (D7).
|
||||
const successes = forwardGate.scoredRollouts.filter((r) => r.score >= 0.5);
|
||||
const failures = forwardGate.scoredRollouts.filter((r) => r.score < 0.5);
|
||||
|
||||
// BACKWARD PASS: D7 two reflect calls (failures + successes).
|
||||
const rejected = loadRejectedBuffer(skillsDir, skillName);
|
||||
const reflectResult = await runReflect({
|
||||
skillBodyText: checkpoint!.best_skill_text,
|
||||
successes: [], // simplified for v1: see TODO above
|
||||
failures: [],
|
||||
successes,
|
||||
failures,
|
||||
rejected,
|
||||
optimizerModel: opts.optimizerModel,
|
||||
abortSignal: undefined,
|
||||
|
||||
@@ -14,7 +14,6 @@
|
||||
*/
|
||||
|
||||
import { chat as gatewayChat } from '../ai/gateway.ts';
|
||||
import { parseJudgeJson } from './score.ts';
|
||||
import type { EditOp, ScoredRollout } from './types.ts';
|
||||
import type { RejectedEntry } from './rejected-buffer.ts';
|
||||
|
||||
@@ -158,15 +157,21 @@ function truncate(s: string, max: number): string {
|
||||
return s.length > max ? s.slice(0, max) + `\n...(truncated, ${s.length - max} more chars)` : s;
|
||||
}
|
||||
|
||||
function parseEditsResponse(raw: string): EditOp[] {
|
||||
// Reuse the same forgiving JSON parser as the score module (tagged result
|
||||
// shape; the parser pulls the first {...} object from prose if present).
|
||||
const parsed = parseJudgeJson(raw);
|
||||
if (!parsed) return [];
|
||||
// The parser returned a {score, rationale} shape; for reflect we expect
|
||||
// {edits: [...]}. Re-extract directly with a tolerant fallback.
|
||||
const direct = tryExtractEdits(raw);
|
||||
return direct;
|
||||
/**
|
||||
* Parse `{edits: [...]}` from optimizer output. Tolerates ```fenced blocks```,
|
||||
* trailing commas, prose-wrapped JSON. Returns [] when no recoverable edits
|
||||
* are found (caller treats as "this reflect call produced no usable edits"
|
||||
* — same effect as the optimizer returning {edits: []}).
|
||||
*
|
||||
* EXPORTED so reflect.test.ts can pin the parser independently of the chat
|
||||
* transport. Pre-v0.42.0.1 this lived behind a `parseJudgeJson` early-return
|
||||
* guard that always failed (judge-JSON checks for a `score` key, not `edits`),
|
||||
* making every optimizer call silently produce zero edits. The bug survived
|
||||
* v0.42.0.0 because no unit test exercised this parser; the orchestrator's
|
||||
* `successes/failures: []` hardcoding masked it end-to-end too.
|
||||
*/
|
||||
export function parseEditsResponse(raw: string): EditOp[] {
|
||||
return tryExtractEdits(raw);
|
||||
}
|
||||
|
||||
function tryExtractEdits(raw: string): EditOp[] {
|
||||
|
||||
@@ -235,6 +235,13 @@ export interface GateResult {
|
||||
/** Mean of per-task medians. */
|
||||
selScore: number;
|
||||
reason?: 'no_margin' | 'below_baseline' | 'all_judge_errors';
|
||||
/**
|
||||
* Every scored rollout this gate produced, in selSet order (runs per task
|
||||
* flattened). Used by the orchestrator's forward pass to partition into
|
||||
* successes/failures for reflect. Sel-side gates also surface this but the
|
||||
* orchestrator only reads it for the forward path (runsPerTask=1).
|
||||
*/
|
||||
scoredRollouts: ScoredRollout[];
|
||||
}
|
||||
|
||||
// ─── Bootstrap sentinel (D15) ─────────────────────────────────────────────
|
||||
|
||||
@@ -34,6 +34,13 @@ export interface ValidateGateOpts extends Omit<GateInput, 'selSet'> {
|
||||
judgeModel?: string;
|
||||
/** D4: max in-flight sel-task evaluations. Default 4. */
|
||||
concurrency?: number;
|
||||
/**
|
||||
* Runs per task for the median-of-N. Default `VALIDATION_RUNS_PER_TASK` (3)
|
||||
* for sel-side acceptance gates. The orchestrator's forward pass passes 1
|
||||
* because we only need a rough partition into successes/failures, not noise
|
||||
* rejection. Must be >= 1.
|
||||
*/
|
||||
runsPerTask?: number;
|
||||
abortSignal?: AbortSignal;
|
||||
/** Test seam — substitute rollout. */
|
||||
rolloutFn?: typeof runRollout;
|
||||
@@ -49,14 +56,25 @@ export async function runValidationGate(opts: ValidateGateOpts): Promise<GateRes
|
||||
const rolloutFn = opts.rolloutFn ?? runRollout;
|
||||
const scoreFn = opts.scoreFn ?? scoreTrajectory;
|
||||
const concurrency = opts.concurrency ?? 4;
|
||||
// Forward pass uses 1; sel/baseline use VALIDATION_RUNS_PER_TASK (3). Floor
|
||||
// at 1 (a runsPerTask of 0 would silently produce an empty median).
|
||||
const runsPerTask = Math.max(1, opts.runsPerTask ?? VALIDATION_RUNS_PER_TASK);
|
||||
|
||||
interface PerTask {
|
||||
task_id: string;
|
||||
median: number;
|
||||
runs: number[];
|
||||
rollouts: ScoredRollout[];
|
||||
}
|
||||
|
||||
// Per sel-task, run N rollouts + N judges, take median.
|
||||
const settled = await runWithLimit({
|
||||
items: opts.selSet,
|
||||
limit: concurrency,
|
||||
fn: async (task: BenchmarkTask): Promise<{ task_id: string; median: number; runs: number[] }> => {
|
||||
fn: async (task: BenchmarkTask): Promise<PerTask> => {
|
||||
const runs: number[] = [];
|
||||
for (let i = 0; i < VALIDATION_RUNS_PER_TASK; i++) {
|
||||
const rollouts: ScoredRollout[] = [];
|
||||
for (let i = 0; i < runsPerTask; i++) {
|
||||
const rolloutOpts: RolloutOpts = {
|
||||
engine: opts.engine,
|
||||
skillText: opts.candidateSkillText,
|
||||
@@ -69,18 +87,22 @@ export async function runValidationGate(opts: ValidateGateOpts): Promise<GateRes
|
||||
judgeModel: opts.judgeModel,
|
||||
});
|
||||
runs.push(scored.score);
|
||||
rollouts.push(scored);
|
||||
}
|
||||
return { task_id: task.task_id, median: median(runs), runs };
|
||||
return { task_id: task.task_id, median: median(runs), runs, rollouts };
|
||||
},
|
||||
signal: opts.abortSignal,
|
||||
});
|
||||
|
||||
// SettledItem<TOut>[] — extract successful results; treat errors as score=0
|
||||
// (pessimistic fallback consistent with the judge fail-open posture).
|
||||
// Errored tasks contribute no scoredRollouts (caller's reflect sees fewer
|
||||
// trajectories rather than fabricated zero-score entries).
|
||||
const perTaskMedians = settled.map((s, idx) => {
|
||||
if (s && s.ok) return s.value;
|
||||
if (s && s.ok) return { task_id: s.value.task_id, median: s.value.median, runs: s.value.runs };
|
||||
return { task_id: opts.selSet[idx]!.task_id, median: 0, runs: [] };
|
||||
});
|
||||
const scoredRollouts: ScoredRollout[] = settled.flatMap((s) => (s && s.ok) ? s.value.rollouts : []);
|
||||
const selScore = perTaskMedians.length === 0
|
||||
? 0
|
||||
: perTaskMedians.reduce((acc, r) => acc + r.median, 0) / perTaskMedians.length;
|
||||
@@ -95,7 +117,7 @@ export async function runValidationGate(opts: ValidateGateOpts): Promise<GateRes
|
||||
if (selScore <= opts.bestScore) reason = 'below_baseline';
|
||||
else reason = 'no_margin';
|
||||
}
|
||||
return { accepted, perTaskMedians, selScore, ...(reason ? { reason } : {}) };
|
||||
return { accepted, perTaskMedians, selScore, scoredRollouts, ...(reason ? { reason } : {}) };
|
||||
}
|
||||
|
||||
/** Pure median for an array of numbers. Returns 0 for empty array. */
|
||||
|
||||
@@ -0,0 +1,631 @@
|
||||
/**
|
||||
* SkillOpt loop E2E: happy path + 5 orchestrator-visible failure modes.
|
||||
*
|
||||
* Sibling to `skillopt-pglite.serial.test.ts`. That file pins the three
|
||||
* v1 paths (dry-run, all-reject, manual revertAllPending). THIS file
|
||||
* proves the full optimization loop can actually improve a skill end-to-end
|
||||
* AND that each failure mode the loop is supposed to catch actually does
|
||||
* the right thing.
|
||||
*
|
||||
* Stub strategy: install one composite chat transport via
|
||||
* `__setChatTransportForTests`. The stub branches on `chatOpts.system`:
|
||||
*
|
||||
* - If system starts with "You are SkillOpt's optimizer", the call is
|
||||
* a reflect call. Branches further on FAILURE vs SUCCESS prompt.
|
||||
* - Otherwise, the call is a target-agent rollout. The stub emits
|
||||
* deterministic markdown based on which sections appear in the skill,
|
||||
* so applied edits change the rollout output → change the score.
|
||||
*
|
||||
* Hermetic: no real LLM calls, no `DATABASE_URL`, no API keys. PGLite
|
||||
* in-memory engine + tempdir SKILL.md + tempdir benchmark JSONL.
|
||||
*
|
||||
* .serial.test.ts because the stub installs module-state (the chat
|
||||
* transport) and the orchestrator walks multi-epoch shared disk state.
|
||||
*/
|
||||
|
||||
import { describe, expect, test, beforeAll, afterAll, beforeEach, afterEach } from 'bun:test';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { PGLiteEngine } from '../../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from '../helpers/reset-pglite.ts';
|
||||
import { withEnv } from '../helpers/with-env.ts';
|
||||
import {
|
||||
__setChatTransportForTests,
|
||||
type ChatOpts,
|
||||
type ChatResult,
|
||||
} from '../../src/core/ai/gateway.ts';
|
||||
import { runSkillOpt } from '../../src/core/skillopt/orchestrator.ts';
|
||||
import {
|
||||
bestPath,
|
||||
loadHistory,
|
||||
skillPath,
|
||||
} from '../../src/core/skillopt/version-store.ts';
|
||||
import { loadRejectedBuffer } from '../../src/core/skillopt/rejected-buffer.ts';
|
||||
import type { EditOp } from '../../src/core/skillopt/types.ts';
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
|
||||
beforeAll(async () => {
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetPgliteState(engine);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Belt-and-suspenders: ensure no test leaks a stub transport into the
|
||||
// next test. Every test path also clears explicitly in its finally block,
|
||||
// but a failed assertion mid-stub would skip that; this catches the
|
||||
// skipped-cleanup case.
|
||||
__setChatTransportForTests(null);
|
||||
});
|
||||
|
||||
// ─── Fixture helpers ────────────────────────────────────────────────────────
|
||||
|
||||
const SKILL = 'e2e-loop-skill';
|
||||
|
||||
/** A skill with only `## People`. Half the benchmark fails at baseline. */
|
||||
const SKILL_PEOPLE_ONLY = `---
|
||||
name: e2e-loop-skill
|
||||
version: 0.1.0
|
||||
description: Test skill for E2E SkillOpt loop.
|
||||
triggers:
|
||||
- "do the loop task"
|
||||
brain_first: exempt
|
||||
---
|
||||
|
||||
# E2E Loop Test Skill
|
||||
|
||||
When asked, produce a structured output.
|
||||
|
||||
## People
|
||||
List people mentioned.
|
||||
`;
|
||||
|
||||
/** A skill with both sections. Full benchmark passes at baseline. */
|
||||
const SKILL_BOTH_SECTIONS = `---
|
||||
name: e2e-loop-skill
|
||||
version: 0.1.0
|
||||
description: Test skill for E2E SkillOpt loop.
|
||||
triggers:
|
||||
- "do the loop task"
|
||||
brain_first: exempt
|
||||
---
|
||||
|
||||
# E2E Loop Test Skill
|
||||
|
||||
When asked, produce a structured output.
|
||||
|
||||
## People
|
||||
List people mentioned.
|
||||
|
||||
## Citations
|
||||
Cite sources.
|
||||
`;
|
||||
|
||||
/**
|
||||
* 50 tasks alternating People/Citations rule checks. The benchmark's
|
||||
* deterministic structure makes baseline scores predictable: with a
|
||||
* People-only skill, only People-tasks pass → score = 0.5 on any sufficiently
|
||||
* mixed sample. Split [4,1,5] = 20 train / 5 sel / 25 test (satisfies D17
|
||||
* floor).
|
||||
*/
|
||||
const SAMPLE_BENCHMARK = Array.from({ length: 50 }, (_, i) => {
|
||||
const n = String(i + 1).padStart(3, '0');
|
||||
const op = i % 2 === 0 ? 'People' : 'Citations';
|
||||
return {
|
||||
task_id: `e2e-${n}`,
|
||||
task: `Process task ${i + 1}`,
|
||||
judge: { kind: 'rule' as const, checks: [{ op: 'contains' as const, arg: op }] },
|
||||
};
|
||||
});
|
||||
|
||||
interface Fixture {
|
||||
skillsDir: string;
|
||||
benchmarkPath: string;
|
||||
cleanup: () => void;
|
||||
}
|
||||
|
||||
function setupFixture(skillBody: string = SKILL_PEOPLE_ONLY): Fixture {
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'skillopt-loop-e2e-'));
|
||||
const skillDir = path.join(tmp, SKILL);
|
||||
fs.mkdirSync(skillDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(skillDir, 'SKILL.md'), skillBody);
|
||||
const benchmarkPath = path.join(skillDir, 'skillopt-benchmark.jsonl');
|
||||
fs.writeFileSync(
|
||||
benchmarkPath,
|
||||
SAMPLE_BENCHMARK.map((t) => JSON.stringify(t)).join('\n') + '\n',
|
||||
);
|
||||
return {
|
||||
skillsDir: tmp,
|
||||
benchmarkPath,
|
||||
cleanup: () => {
|
||||
try { fs.rmSync(tmp, { recursive: true, force: true }); } catch { /* ignore */ }
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// ─── Stub builder ───────────────────────────────────────────────────────────
|
||||
|
||||
interface StubOpts {
|
||||
/** Edit returned by the FAILURE reflect call. Null/undefined → empty edits. */
|
||||
failureEdit?: EditOp | null;
|
||||
/** Edit returned by the SUCCESS reflect call. Null/undefined → empty edits. */
|
||||
successEdit?: EditOp | null;
|
||||
/**
|
||||
* Raw text returned by the optimizer (overrides failureEdit + successEdit).
|
||||
* Used for the malformed-JSON test case.
|
||||
*/
|
||||
optimizerRaw?: string;
|
||||
/**
|
||||
* Target-agent text emitter. Defaults to "emit text based on which sections
|
||||
* exist in the skill". Override to simulate broken/idiosyncratic agents.
|
||||
*/
|
||||
targetText?: (skillText: string) => string;
|
||||
/**
|
||||
* Optional per-call usage override for the budget-exhaustion test. When
|
||||
* set, every chat call reports this usage (driving cumulative cost up
|
||||
* fast against a tight cap).
|
||||
*/
|
||||
perCallUsage?: { input: number; output: number };
|
||||
}
|
||||
|
||||
const REFLECT_OPTIMIZER_PREFIX = "You are SkillOpt's optimizer.";
|
||||
const FAILURE_REFLECT_MARKER = 'FAILURE TRAJECTORIES';
|
||||
|
||||
function defaultTargetText(skillText: string): string {
|
||||
// Faithful agent: read the skill's body, emit sections that exist there.
|
||||
// Rule-check `contains: 'People'` passes when the section header is present
|
||||
// in the rollout's final_text (since the header literal contains 'People').
|
||||
const parts: string[] = [];
|
||||
if (skillText.includes('## People')) parts.push('## People\nAlice attended the meeting.');
|
||||
if (skillText.includes('## Citations')) parts.push('## Citations\nSource: example.com');
|
||||
return parts.join('\n\n') || 'No structured output produced.';
|
||||
}
|
||||
|
||||
function makeChatResult(
|
||||
text: string,
|
||||
model: string,
|
||||
usage: { input: number; output: number } = { input: 100, output: 20 },
|
||||
): ChatResult {
|
||||
return {
|
||||
text,
|
||||
blocks: [{ type: 'text', text }],
|
||||
stopReason: 'end',
|
||||
usage: {
|
||||
input_tokens: usage.input,
|
||||
output_tokens: usage.output,
|
||||
cache_read_tokens: 0,
|
||||
cache_creation_tokens: 0,
|
||||
},
|
||||
model,
|
||||
providerId: 'anthropic',
|
||||
};
|
||||
}
|
||||
|
||||
function installStub(opts: StubOpts): void {
|
||||
const usage = opts.perCallUsage ?? { input: 100, output: 20 };
|
||||
__setChatTransportForTests(async (chatOpts: ChatOpts): Promise<ChatResult> => {
|
||||
const sys = chatOpts.system ?? '';
|
||||
const isOptimizerCall = sys.startsWith(REFLECT_OPTIMIZER_PREFIX);
|
||||
|
||||
if (isOptimizerCall) {
|
||||
const model = chatOpts.model ?? 'anthropic:claude-opus-4-7';
|
||||
if (opts.optimizerRaw !== undefined) {
|
||||
return makeChatResult(opts.optimizerRaw, model, usage);
|
||||
}
|
||||
const isFailureMode = sys.includes(FAILURE_REFLECT_MARKER);
|
||||
const edit = isFailureMode ? opts.failureEdit : opts.successEdit;
|
||||
const text = JSON.stringify({ edits: edit ? [edit] : [] });
|
||||
return makeChatResult(text, model, usage);
|
||||
}
|
||||
|
||||
// Target-agent rollout.
|
||||
const model = chatOpts.model ?? 'anthropic:claude-sonnet-4-6';
|
||||
const fn = opts.targetText ?? defaultTargetText;
|
||||
return makeChatResult(fn(sys), model, usage);
|
||||
});
|
||||
}
|
||||
|
||||
function uninstallStub(): void {
|
||||
__setChatTransportForTests(null);
|
||||
}
|
||||
|
||||
// ─── Common runSkillOpt invocation ──────────────────────────────────────────
|
||||
|
||||
interface RunOptsOverride {
|
||||
maxCostUsd?: number;
|
||||
epochs?: number;
|
||||
batchSize?: number;
|
||||
}
|
||||
|
||||
async function runOnce(fixture: Fixture, over: RunOptsOverride = {}) {
|
||||
return runSkillOpt({
|
||||
engine,
|
||||
skillName: SKILL,
|
||||
skillsDir: fixture.skillsDir,
|
||||
benchmarkPath: fixture.benchmarkPath,
|
||||
epochs: over.epochs ?? 1,
|
||||
batchSize: over.batchSize ?? 2,
|
||||
lr: 4,
|
||||
lrSchedule: 'constant',
|
||||
split: [4, 1, 5],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
targetModel: 'anthropic:claude-sonnet-4-6',
|
||||
judgeModel: 'anthropic:claude-sonnet-4-6',
|
||||
mode: 'patch',
|
||||
dryRun: false,
|
||||
noMutate: false,
|
||||
allowMutateBundled: true,
|
||||
bootstrapReviewed: false,
|
||||
json: true,
|
||||
maxCostUsd: over.maxCostUsd ?? 100,
|
||||
maxRuntimeMin: 1,
|
||||
force: true, // bypass dirty-tree (tempdir isn't a git repo)
|
||||
});
|
||||
}
|
||||
|
||||
// ─── Cases ──────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('skillopt full-loop E2E (happy path + broken cases)', () => {
|
||||
test('happy path: optimizer proposes a real edit, gate accepts, SKILL.md mutated', async () => {
|
||||
const fixture = setupFixture(SKILL_PEOPLE_ONLY);
|
||||
try {
|
||||
// Optimizer (FAILURE mode) proposes adding a ## Citations section right
|
||||
// after the ## People heading. After apply, the agent emits both sections
|
||||
// → every rollout passes → sel score goes from 0.5 (baseline) to 1.0,
|
||||
// delta = 0.5 ≫ epsilon=0.05 → ACCEPT.
|
||||
installStub({
|
||||
failureEdit: {
|
||||
op: 'add',
|
||||
anchor: 'People',
|
||||
content: '## Citations\nCite the source for every claim.',
|
||||
reason: 'agent failed Citations tasks because no Citations section exists',
|
||||
},
|
||||
successEdit: null, // success-mode reflect produces no edits
|
||||
});
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result = await runOnce(fixture);
|
||||
|
||||
// Outcome contract: accepted + mutated.
|
||||
expect(result.outcome).toBe('accepted');
|
||||
expect(result.mutatedSkillFile).toBe(true);
|
||||
|
||||
// SKILL.md on disk now has BOTH sections.
|
||||
const finalSkill = fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8');
|
||||
expect(finalSkill).toContain('## People');
|
||||
expect(finalSkill).toContain('## Citations');
|
||||
// Frontmatter preserved (D5: edits never touch frontmatter).
|
||||
expect(finalSkill).toContain('name: e2e-loop-skill');
|
||||
expect(finalSkill).toContain('brain_first: exempt');
|
||||
|
||||
// best.md mirrors the on-disk SKILL.md content.
|
||||
expect(fs.readFileSync(bestPath(fixture.skillsDir, SKILL), 'utf8')).toBe(finalSkill);
|
||||
|
||||
// History has exactly one committed row with the right shape.
|
||||
const history = loadHistory(fixture.skillsDir, SKILL);
|
||||
const committed = history.filter((r) => r.status === 'committed');
|
||||
expect(committed).toHaveLength(1);
|
||||
expect(committed[0]!.version_n).toBe(1);
|
||||
expect(committed[0]!.delta).toBeGreaterThan(0.05); // > epsilon
|
||||
expect(committed[0]!.sel_score).toBeGreaterThan(committed[0]!.delta); // monotone
|
||||
|
||||
// The committed row records the actual edit applied (not just an empty proposal).
|
||||
expect(committed[0]!.edits).toHaveLength(1);
|
||||
expect(committed[0]!.edits[0]).toMatchObject({ op: 'add', anchor: 'People' });
|
||||
|
||||
// Receipt sel_score reflects the accepted candidate's score.
|
||||
expect(result.receipt.best_sel_score).toBeGreaterThan(0.9);
|
||||
expect(result.receipt.outcome).toBe('accepted');
|
||||
expect(result.receipt.epochs_completed).toBe(1);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('broken: below-baseline regression edit (gate rejects, SKILL.md unchanged)', async () => {
|
||||
// Start with a skill that already scores 1.0. The optimizer (in SUCCESS
|
||||
// mode — failures=[] since baseline is perfect) proposes a destructive
|
||||
// edit that removes the ## People section. The candidate's sel score
|
||||
// collapses to 0.5; the gate rejects with reason=below_baseline; the
|
||||
// on-disk SKILL.md MUST stay byte-identical to the baseline.
|
||||
const fixture = setupFixture(SKILL_BOTH_SECTIONS);
|
||||
try {
|
||||
installStub({
|
||||
failureEdit: null,
|
||||
successEdit: {
|
||||
op: 'delete',
|
||||
target: '## People\nList people mentioned.\n',
|
||||
reason: 'mistakenly thinks the People section is redundant',
|
||||
},
|
||||
});
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result = await runOnce(fixture);
|
||||
|
||||
// Outcome contract: no acceptance + no mutation.
|
||||
expect(result.outcome).toBe('no_improvement');
|
||||
expect(result.mutatedSkillFile).toBe(false);
|
||||
|
||||
// SKILL.md on disk is byte-identical to baseline.
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8'))
|
||||
.toBe(SKILL_BOTH_SECTIONS);
|
||||
|
||||
// History is empty (no committed rows; rejected edits don't enter history).
|
||||
const history = loadHistory(fixture.skillsDir, SKILL);
|
||||
expect(history.filter((r) => r.status === 'committed')).toHaveLength(0);
|
||||
|
||||
// The destructive edit landed in the rejected-edits buffer for
|
||||
// anti-bias context on future runs (the optimizer learns).
|
||||
const rejected = loadRejectedBuffer(fixture.skillsDir, SKILL);
|
||||
expect(rejected.length).toBeGreaterThan(0);
|
||||
expect(rejected.some((e) => e.reason.startsWith('validation_gate'))).toBe(true);
|
||||
// The recorded edit shape matches the destructive proposal so the
|
||||
// optimizer's anti-bias prompt sees the actual edit, not a stub.
|
||||
expect(rejected.some((e) =>
|
||||
e.edits.some((edit) => edit.op === 'delete' && (edit as { target: string }).target.includes('## People')),
|
||||
)).toBe(true);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('broken: malformed reflect JSON (no edits parsed, no acceptance)', async () => {
|
||||
// The optimizer returns syntactically broken JSON. The reflect module's
|
||||
// forgiving parser yields zero valid edits; applyEditBatch sees an empty
|
||||
// batch; the orchestrator hits the "no_edits_applied" branch; the sel
|
||||
// gate is never invoked. SKILL.md stays untouched. Critically: the run
|
||||
// does NOT crash on malformed optimizer output (graceful degradation).
|
||||
const fixture = setupFixture(SKILL_PEOPLE_ONLY);
|
||||
try {
|
||||
installStub({
|
||||
// Adversarial: looks like JSON but isn't. Different broken shapes
|
||||
// hit different fallback paths in tryExtractEdits.
|
||||
optimizerRaw: '{"edits": [BROKEN, no quotes, trailing comma,]',
|
||||
});
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result = await runOnce(fixture);
|
||||
|
||||
expect(result.outcome).toBe('no_improvement');
|
||||
expect(result.mutatedSkillFile).toBe(false);
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8'))
|
||||
.toBe(SKILL_PEOPLE_ONLY);
|
||||
expect(loadHistory(fixture.skillsDir, SKILL).filter((r) => r.status === 'committed'))
|
||||
.toHaveLength(0);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('broken: anchor-not-found edit (apply rejects, sel gate skipped)', async () => {
|
||||
// The optimizer proposes a structurally valid edit pointing at a heading
|
||||
// that doesn't exist in the skill. applyEditBatch returns all-rejected;
|
||||
// the orchestrator's all-rejected branch fires (logs no_edits_applied,
|
||||
// pushes to rejected-buffer, skips the sel gate). The skill stays
|
||||
// unchanged AND the bogus anchor lands in the rejected-buffer with
|
||||
// reason 'apply_failed' (separate from gate-rejected entries).
|
||||
const fixture = setupFixture(SKILL_PEOPLE_ONLY);
|
||||
try {
|
||||
installStub({
|
||||
failureEdit: {
|
||||
op: 'add',
|
||||
anchor: 'NonExistentHeading',
|
||||
content: 'Some content',
|
||||
reason: 'optimizer hallucinated a heading',
|
||||
},
|
||||
});
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result = await runOnce(fixture);
|
||||
|
||||
expect(result.outcome).toBe('no_improvement');
|
||||
expect(result.mutatedSkillFile).toBe(false);
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8'))
|
||||
.toBe(SKILL_PEOPLE_ONLY);
|
||||
|
||||
// Rejected-buffer should carry an apply_failed entry (the failed
|
||||
// anchor lookup is recorded so the optimizer doesn't re-propose).
|
||||
const rejected = loadRejectedBuffer(fixture.skillsDir, SKILL);
|
||||
expect(rejected.length).toBeGreaterThan(0);
|
||||
expect(rejected.some((e) => e.reason === 'apply_failed')).toBe(true);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('broken: budget exhausted mid-run (aborts cleanly, no half-committed state)', async () => {
|
||||
// Cap is just under the preflight estimate so the run starts (preflight
|
||||
// refusal would prevent us from observing BudgetExhausted mid-loop), then
|
||||
// trips on the cumulative spend during the loop. Outcome=aborted is the
|
||||
// contract; the load-bearing assertion is that NO pending or committed
|
||||
// history rows survive (the abort path must not leave the skill in a
|
||||
// half-mutated state).
|
||||
const fixture = setupFixture(SKILL_PEOPLE_ONLY);
|
||||
try {
|
||||
installStub({
|
||||
failureEdit: {
|
||||
op: 'add',
|
||||
anchor: 'People',
|
||||
content: '## Citations\nCite.',
|
||||
reason: 'mid-budget edit',
|
||||
},
|
||||
// Drive per-call cost up so the cumulative spend trips the cap fast.
|
||||
perCallUsage: { input: 50_000, output: 5_000 },
|
||||
});
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
// The exact cap is calibrated to (a) survive the preflight check
|
||||
// (preflight refuses with a CostCapExceeded error if its estimate
|
||||
// exceeds the cap), but (b) trip mid-loop when real per-call
|
||||
// usage from the stub accumulates. Preflight estimates assume
|
||||
// small per-call usage; the stub inflates per-call usage so we
|
||||
// exceed the cap before the loop completes.
|
||||
let result: Awaited<ReturnType<typeof runOnce>>;
|
||||
try {
|
||||
result = await runOnce(fixture, { maxCostUsd: 5.0 });
|
||||
} catch (err) {
|
||||
// Acceptable: preflight may refuse before the loop starts if its
|
||||
// estimator now overshoots. In that case the contract becomes
|
||||
// "no mutation happened" — assert that directly via filesystem.
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
expect(msg).toMatch(/cost|budget/i);
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8'))
|
||||
.toBe(SKILL_PEOPLE_ONLY);
|
||||
return;
|
||||
}
|
||||
|
||||
// Reaching the loop: budget exhausted is the expected outcome. Other
|
||||
// non-acceptance outcomes (no_improvement, errored) are also fine
|
||||
// — the load-bearing assertion is no half-committed state.
|
||||
expect(['aborted', 'no_improvement', 'errored']).toContain(result.outcome);
|
||||
|
||||
// No PENDING rows: the v0.42 D8 two-phase commit insists every
|
||||
// pending row is either committed or reverted; an abort path that
|
||||
// leaves a pending row would corrupt resume.
|
||||
const history = loadHistory(fixture.skillsDir, SKILL);
|
||||
expect(history.filter((r) => r.status === 'pending')).toHaveLength(0);
|
||||
|
||||
// If outcome is aborted, MUST NOT mutate.
|
||||
if (result.outcome === 'aborted') {
|
||||
expect(result.mutatedSkillFile).toBe(false);
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8'))
|
||||
.toBe(SKILL_PEOPLE_ONLY);
|
||||
}
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('converged skill: re-running on perfect baseline yields no_improvement (no double-commit)', async () => {
|
||||
// Start from an already-perfect skill (baseline score = 1.0). The forward
|
||||
// gate finds zero failures. The reflect failure-mode path is never
|
||||
// invoked (failures=[]); only success-mode runs. The success-mode stub
|
||||
// returns empty edits. The loop converges with outcome=no_improvement
|
||||
// and the on-disk skill stays byte-identical. This proves the optimizer
|
||||
// doesn't pointlessly mutate a converged skill — the v1 "convergence"
|
||||
// path that protects against thrash on a well-tuned starting point.
|
||||
const fixture = setupFixture(SKILL_BOTH_SECTIONS);
|
||||
try {
|
||||
installStub({
|
||||
failureEdit: null, // wouldn't be called anyway — baseline has no failures
|
||||
successEdit: null, // success-mode stub returns empty edits
|
||||
});
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result = await runOnce(fixture);
|
||||
|
||||
expect(result.outcome).toBe('no_improvement');
|
||||
expect(result.mutatedSkillFile).toBe(false);
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8'))
|
||||
.toBe(SKILL_BOTH_SECTIONS);
|
||||
|
||||
// Receipt baseline IS the best score (no improvement to report).
|
||||
expect(result.receipt.best_sel_score).toBeGreaterThan(0.9);
|
||||
|
||||
// History is empty.
|
||||
expect(loadHistory(fixture.skillsDir, SKILL).filter((r) => r.status === 'committed'))
|
||||
.toHaveLength(0);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
test('idempotent re-run: accept once, run again, second run sees new baseline + does not re-mutate', async () => {
|
||||
// The cathedral test: drive the loop twice in sequence on the same
|
||||
// fixture. Run 1 accepts the add-Citations edit (skill improves from
|
||||
// People-only to both sections). Run 2 starts from the now-improved
|
||||
// skill, sees baseline=1.0, finds no failures, returns no_improvement.
|
||||
// SKILL.md stays at v1; history still has exactly one committed row.
|
||||
// This proves the optimizer is "stable at the fixed point" — the
|
||||
// critical property of an iterative optimizer.
|
||||
const fixture = setupFixture(SKILL_PEOPLE_ONLY);
|
||||
try {
|
||||
// Same stub config across both runs: failure-mode proposes the
|
||||
// add-Citations edit. After run 1 accepts it, run 2's forward gate
|
||||
// sees no failures → failure-reflect never fires → no edits proposed.
|
||||
const stubConfig = {
|
||||
failureEdit: {
|
||||
op: 'add' as const,
|
||||
anchor: 'People',
|
||||
content: '## Citations\nCite the source.',
|
||||
reason: 'baseline missing Citations section',
|
||||
},
|
||||
successEdit: null,
|
||||
};
|
||||
|
||||
// Run 1: accept.
|
||||
installStub(stubConfig);
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result1 = await runOnce(fixture);
|
||||
expect(result1.outcome).toBe('accepted');
|
||||
expect(result1.mutatedSkillFile).toBe(true);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
|
||||
const afterRun1 = fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8');
|
||||
const historyAfterRun1 = loadHistory(fixture.skillsDir, SKILL);
|
||||
expect(historyAfterRun1.filter((r) => r.status === 'committed')).toHaveLength(1);
|
||||
|
||||
// Run 2: same stub, but loop should observe the improved baseline and
|
||||
// converge without further mutation.
|
||||
installStub(stubConfig);
|
||||
try {
|
||||
await withEnv({ GBRAIN_AUDIT_DIR: fixture.skillsDir }, async () => {
|
||||
const result2 = await runOnce(fixture);
|
||||
expect(result2.outcome).toBe('no_improvement');
|
||||
expect(result2.mutatedSkillFile).toBe(false);
|
||||
});
|
||||
} finally {
|
||||
uninstallStub();
|
||||
}
|
||||
|
||||
// SKILL.md byte-identical to its state after run 1.
|
||||
expect(fs.readFileSync(skillPath(fixture.skillsDir, SKILL), 'utf8')).toBe(afterRun1);
|
||||
|
||||
// History still has exactly one committed row (no double-commit).
|
||||
const historyAfterRun2 = loadHistory(fixture.skillsDir, SKILL);
|
||||
expect(historyAfterRun2.filter((r) => r.status === 'committed')).toHaveLength(1);
|
||||
// version_n unchanged at 1.
|
||||
expect(historyAfterRun2.filter((r) => r.status === 'committed')[0]!.version_n).toBe(1);
|
||||
} finally {
|
||||
fixture.cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,289 @@
|
||||
/**
|
||||
* SkillOpt reflect unit tests.
|
||||
*
|
||||
* Pinned regressions:
|
||||
* - parseEditsResponse must NOT route through parseJudgeJson (which checks
|
||||
* for a 'score' key and silently drops all optimizer output that has
|
||||
* 'edits' instead). v0.42.0.0 shipped with that bug; every optimizer
|
||||
* call produced zero edits and the orchestrator could never accept
|
||||
* anything. Fixed v0.42.0.1 by dropping the wrong-typed guard.
|
||||
*
|
||||
* - runReflect must call FAILURE/SUCCESS reflect modes only when their
|
||||
* batches are non-empty (D7 paper-faithful semantics).
|
||||
*
|
||||
* - Token-usage accumulation across the two reflect calls must be additive.
|
||||
*
|
||||
* - Errors in one mode (e.g. failure-reflect throws) must NOT swallow the
|
||||
* other mode's edits.
|
||||
*/
|
||||
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { parseEditsResponse, runReflect } 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';
|
||||
|
||||
// ─── parseEditsResponse ─────────────────────────────────────────────────────
|
||||
|
||||
describe('parseEditsResponse', () => {
|
||||
test('parses minimal {edits: [...]} shape', () => {
|
||||
const out = parseEditsResponse(
|
||||
JSON.stringify({ edits: [{ op: 'add', anchor: 'People', content: 'X', reason: 'r' }] }),
|
||||
);
|
||||
expect(out).toHaveLength(1);
|
||||
expect(out[0]).toMatchObject({ op: 'add', anchor: 'People', content: 'X', reason: 'r' });
|
||||
});
|
||||
|
||||
test('REGRESSION v0.42.0.1: edits-only JSON survives the parser', () => {
|
||||
// Pre-fix this returned [] because parseJudgeJson required a 'score' key.
|
||||
// If this regresses, every optimizer call silently produces zero edits.
|
||||
const out = parseEditsResponse('{"edits":[{"op":"delete","target":"foo","reason":"r"}]}');
|
||||
expect(out).toHaveLength(1);
|
||||
expect(out[0]).toMatchObject({ op: 'delete', target: 'foo' });
|
||||
});
|
||||
|
||||
test('strips ```json``` fences', () => {
|
||||
const out = parseEditsResponse(
|
||||
'```json\n{"edits":[{"op":"replace","target":"x","replacement":"y"}]}\n```',
|
||||
);
|
||||
expect(out).toHaveLength(1);
|
||||
expect(out[0]).toMatchObject({ op: 'replace', target: 'x', replacement: 'y' });
|
||||
});
|
||||
|
||||
test('extracts first {...} from prose-wrapped output', () => {
|
||||
const out = parseEditsResponse(
|
||||
'Here is the edit: {"edits":[{"op":"add","anchor":"H","content":"C"}]} hope this helps',
|
||||
);
|
||||
expect(out).toHaveLength(1);
|
||||
expect(out[0]).toMatchObject({ op: 'add', anchor: 'H', content: 'C' });
|
||||
});
|
||||
|
||||
test('contract scope: trailing-comma repair is NOT supported by tryExtractEdits', () => {
|
||||
// Documented limitation: trailing commas in optimizer JSON break parsing.
|
||||
// The optimizer prompt forbids them; if the model emits one anyway we lose
|
||||
// the batch this call. Tightening this would mean folding the repair pass
|
||||
// from parseJudgeJson back in — currently out of scope. Pin the limitation
|
||||
// so a future tightening intentionally lights this test up.
|
||||
const out = parseEditsResponse('{"edits":[{"op":"add","anchor":"H","content":"C",},]}');
|
||||
expect(out).toEqual([]);
|
||||
});
|
||||
|
||||
test('returns [] for malformed JSON without throwing', () => {
|
||||
expect(parseEditsResponse('{not valid json at all')).toEqual([]);
|
||||
expect(parseEditsResponse('')).toEqual([]);
|
||||
expect(parseEditsResponse(' ')).toEqual([]);
|
||||
expect(parseEditsResponse('plain prose no braces')).toEqual([]);
|
||||
});
|
||||
|
||||
test('returns [] when edits key is absent or wrong type', () => {
|
||||
expect(parseEditsResponse('{"score": 0.8}')).toEqual([]);
|
||||
expect(parseEditsResponse('{"edits": "not an array"}')).toEqual([]);
|
||||
expect(parseEditsResponse('{"edits": null}')).toEqual([]);
|
||||
});
|
||||
|
||||
test('drops malformed individual edits but keeps valid ones', () => {
|
||||
const out = parseEditsResponse(JSON.stringify({
|
||||
edits: [
|
||||
{ op: 'add', anchor: 'H', content: 'C' }, // valid
|
||||
{ op: 'add' }, // missing anchor + content
|
||||
{ op: 'delete', target: 'T' }, // valid
|
||||
{ op: 'replace', target: 'T' }, // missing replacement
|
||||
{ op: 'invalid_op', anchor: 'X', content: 'Y' }, // unknown op
|
||||
null, // garbage
|
||||
'string', // garbage
|
||||
],
|
||||
}));
|
||||
expect(out).toHaveLength(2);
|
||||
expect(out[0]).toMatchObject({ op: 'add' });
|
||||
expect(out[1]).toMatchObject({ op: 'delete', target: 'T' });
|
||||
});
|
||||
|
||||
test('caps reason to string-only (drops non-string reasons)', () => {
|
||||
const out = parseEditsResponse(JSON.stringify({
|
||||
edits: [{ op: 'add', anchor: 'H', content: 'C', reason: 42 }],
|
||||
}));
|
||||
expect(out).toHaveLength(1);
|
||||
expect(out[0]!.reason).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── runReflect ─────────────────────────────────────────────────────────────
|
||||
|
||||
function makeTrajectory(task_id: string, final_text: string): Trajectory {
|
||||
return {
|
||||
task_id,
|
||||
task: `Task ${task_id}`,
|
||||
final_text,
|
||||
tool_calls: [],
|
||||
usage: { input_tokens: 0, output_tokens: 0, cache_read_tokens: 0, cache_creation_tokens: 0 },
|
||||
turns: 1,
|
||||
stop_reason: 'end',
|
||||
duration_ms: 10,
|
||||
};
|
||||
}
|
||||
|
||||
function makeScored(task_id: string, score: number, text = ''): ScoredRollout {
|
||||
return { trajectory: makeTrajectory(task_id, text), score };
|
||||
}
|
||||
|
||||
function makeChatResult(text: string): ChatResult {
|
||||
return {
|
||||
text,
|
||||
blocks: [{ type: 'text', text }],
|
||||
stopReason: 'end',
|
||||
usage: { input_tokens: 100, output_tokens: 20, cache_read_tokens: 5, cache_creation_tokens: 1 },
|
||||
model: 'anthropic:claude-opus-4-7',
|
||||
providerId: 'anthropic',
|
||||
};
|
||||
}
|
||||
|
||||
describe('runReflect (D7 two-call contract)', () => {
|
||||
test('non-empty failures + non-empty successes: both modes fire, edits collected', async () => {
|
||||
const calls: Array<{ mode: 'failure' | 'success' }> = [];
|
||||
const chatFn = async (opts: ChatOpts): Promise<ChatResult> => {
|
||||
const isFailure = (opts.system ?? '').includes('FAILURE TRAJECTORIES');
|
||||
calls.push({ mode: isFailure ? 'failure' : 'success' });
|
||||
const edit = isFailure
|
||||
? { op: 'add', anchor: 'Failures', content: 'C1' }
|
||||
: { op: 'add', anchor: 'Successes', content: 'C2' };
|
||||
return makeChatResult(JSON.stringify({ edits: [edit] }));
|
||||
};
|
||||
|
||||
const result = await runReflect({
|
||||
skillBodyText: '# Test',
|
||||
successes: [makeScored('s-1', 1.0)],
|
||||
failures: [makeScored('f-1', 0.0)],
|
||||
rejected: [],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
chatFn,
|
||||
});
|
||||
|
||||
expect(calls).toHaveLength(2);
|
||||
expect(calls.map((c) => c.mode).sort()).toEqual(['failure', 'success']);
|
||||
expect(result.failureEdits).toHaveLength(1);
|
||||
expect(result.failureEdits[0]).toMatchObject({ anchor: 'Failures' });
|
||||
expect(result.successEdits).toHaveLength(1);
|
||||
expect(result.successEdits[0]).toMatchObject({ anchor: 'Successes' });
|
||||
// Token usage is additive across the two calls.
|
||||
expect(result.usage.input_tokens).toBe(200);
|
||||
expect(result.usage.output_tokens).toBe(40);
|
||||
expect(result.usage.cache_read_tokens).toBe(10);
|
||||
expect(result.usage.cache_creation_tokens).toBe(2);
|
||||
expect(result.errors).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('empty failures skips failure call; non-empty successes still fires success', async () => {
|
||||
const callModes: string[] = [];
|
||||
const chatFn = async (opts: ChatOpts): Promise<ChatResult> => {
|
||||
callModes.push((opts.system ?? '').includes('FAILURE') ? 'failure' : 'success');
|
||||
return makeChatResult(JSON.stringify({ edits: [{ op: 'delete', target: 'x' }] }));
|
||||
};
|
||||
|
||||
const result = await runReflect({
|
||||
skillBodyText: '# Test',
|
||||
successes: [makeScored('s-1', 1.0)],
|
||||
failures: [],
|
||||
rejected: [],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
chatFn,
|
||||
});
|
||||
|
||||
expect(callModes).toEqual(['success']);
|
||||
expect(result.failureEdits).toHaveLength(0);
|
||||
expect(result.successEdits).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('empty successes skips success call; non-empty failures still fires failure', async () => {
|
||||
const callModes: string[] = [];
|
||||
const chatFn = async (opts: ChatOpts): Promise<ChatResult> => {
|
||||
callModes.push((opts.system ?? '').includes('FAILURE') ? 'failure' : 'success');
|
||||
return makeChatResult(JSON.stringify({ edits: [{ op: 'add', anchor: 'H', content: 'C' }] }));
|
||||
};
|
||||
|
||||
const result = await runReflect({
|
||||
skillBodyText: '# Test',
|
||||
successes: [],
|
||||
failures: [makeScored('f-1', 0.0)],
|
||||
rejected: [],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
chatFn,
|
||||
});
|
||||
|
||||
expect(callModes).toEqual(['failure']);
|
||||
expect(result.failureEdits).toHaveLength(1);
|
||||
expect(result.successEdits).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('both empty: neither call fires (cost-conscious short-circuit)', async () => {
|
||||
let callCount = 0;
|
||||
const chatFn = async (): Promise<ChatResult> => {
|
||||
callCount += 1;
|
||||
return makeChatResult('{"edits":[]}');
|
||||
};
|
||||
|
||||
const result = await runReflect({
|
||||
skillBodyText: '# Test',
|
||||
successes: [],
|
||||
failures: [],
|
||||
rejected: [],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
chatFn,
|
||||
});
|
||||
|
||||
expect(callCount).toBe(0);
|
||||
expect(result.failureEdits).toHaveLength(0);
|
||||
expect(result.successEdits).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('one mode throws: error recorded, other mode still produces edits', async () => {
|
||||
const chatFn = async (opts: ChatOpts): Promise<ChatResult> => {
|
||||
const isFailure = (opts.system ?? '').includes('FAILURE');
|
||||
if (isFailure) throw new Error('rate_limit on failure call');
|
||||
return makeChatResult(JSON.stringify({ edits: [{ op: 'add', anchor: 'OK', content: 'C' }] }));
|
||||
};
|
||||
|
||||
const result = await runReflect({
|
||||
skillBodyText: '# Test',
|
||||
successes: [makeScored('s-1', 1.0)],
|
||||
failures: [makeScored('f-1', 0.0)],
|
||||
rejected: [],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
chatFn,
|
||||
});
|
||||
|
||||
expect(result.failureEdits).toEqual([]);
|
||||
expect(result.successEdits).toHaveLength(1);
|
||||
expect(result.successEdits[0]).toMatchObject({ anchor: 'OK' });
|
||||
expect(result.errors).toHaveLength(1);
|
||||
expect(result.errors[0]).toContain('reflect_failure_failed');
|
||||
expect(result.errors[0]).toContain('rate_limit');
|
||||
});
|
||||
|
||||
test('rejected-buffer context flows into the user message (anti-bias)', async () => {
|
||||
let observedUserMsg = '';
|
||||
const chatFn = async (opts: ChatOpts): Promise<ChatResult> => {
|
||||
const userMsg = opts.messages[0]?.content;
|
||||
if (typeof userMsg === 'string') observedUserMsg = userMsg;
|
||||
return makeChatResult('{"edits":[]}');
|
||||
};
|
||||
|
||||
await runReflect({
|
||||
skillBodyText: '# Test',
|
||||
successes: [],
|
||||
failures: [makeScored('f-1', 0.0)],
|
||||
rejected: [
|
||||
{
|
||||
key: 'k1',
|
||||
skill_sha8: 'deadbeef',
|
||||
edits: [{ op: 'add', anchor: 'X', content: 'Y' }],
|
||||
reason: 'validation_gate_below_baseline',
|
||||
ts: '2026-01-01T00:00:00Z',
|
||||
},
|
||||
],
|
||||
optimizerModel: 'anthropic:claude-opus-4-7',
|
||||
chatFn,
|
||||
});
|
||||
|
||||
expect(observedUserMsg).toContain('PREVIOUSLY REJECTED EDITS');
|
||||
expect(observedUserMsg).toContain('validation_gate_below_baseline');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user