From 584ed99466299985df07bab8f10e98c5ceca09ef Mon Sep 17 00:00:00 2001 From: Josh Owens Date: Tue, 22 Sep 2026 03:34:50 +0000 Subject: [PATCH] feat: allow an empty baseline skill list The most basic question a compare can answer is whether adding a skill changes anything at all, which wants a baseline arm with no skill. Config load rejected `"baselineSkills": []`, so the workaround was a placeholder skill file. That placeholder contaminates the baseline it is standing in for: its text lands in the system prompt under inline delivery, and its description is visible in the sandbox registry under install delivery. An explicit empty array now loads. Omitting or misspelling the key still fails in normalizeSkills, so a typo cannot silently become a no-skill baseline. The proposed arm still requires at least one skill for a real compare, but not under single-arm loading, where proposed is a mirror of baseline that no run exercises: without that, measure on a no-skill scenario failed with an error naming a proposed arm the user never wrote. Closes #38 Co-Authored-By: Claude Opus 5 --- README.md | 7 ++++ SPEC.md | 2 +- src/engine/compare.ts | 6 +++- src/engine/config.ts | 17 +++++---- test/config.test.ts | 73 ++++++++++++++++++++++++++++++++++++++ test/delivery.test.ts | 60 +++++++++++++++++++++++++++++++ test/measure.test.ts | 28 +++++++++++++++ test/skill-install.test.ts | 18 ++++++++++ 8 files changed, 203 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index bd4eaa9..941b2fd 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,13 @@ Create a scenario file: Paths inside a scenario file are resolved relative to that scenario file. +`baselineSkills` may be `[]`, which compares "no skill" against a proposed +skill: the question of whether adding the skill changes anything at all. This +is preferable to pointing baseline at a placeholder skill file, which +contaminates the baseline arm: its text lands in the system prompt under +inline delivery, and its description is visible in the registry under install +delivery. + Run it: ```bash diff --git a/SPEC.md b/SPEC.md index b74d0e5..7330732 100644 --- a/SPEC.md +++ b/SPEC.md @@ -158,7 +158,7 @@ exactly one arm is openai). A compare file defines: - agent file -- baseline skill files (or a shared skill set both arms inherit) +- baseline skill files, empty to compare against no skill, or a shared skill set both arms inherit - proposed skill files - model (shared, or per arm for model comparisons) - run count diff --git a/src/engine/compare.ts b/src/engine/compare.ts index bec1851..160838d 100644 --- a/src/engine/compare.ts +++ b/src/engine/compare.ts @@ -495,7 +495,11 @@ async function runArm( if (config.delivery === "install") { const { installed, warnings } = installSkills(armSkills, sandbox.dir); if (index === 0) { - onProgress?.(` ${label} installed skills: ${installed.map((skill) => skill.name).join(", ")}`); + onProgress?.( + installed.length === 0 + ? ` ${label} installed skills: (none)` + : ` ${label} installed skills: ${installed.map((skill) => skill.name).join(", ")}`, + ); for (const warning of warnings) { onProgress?.(` WARNING: ${warning}`); } diff --git a/src/engine/config.ts b/src/engine/config.ts index f5793ac..1f5cea0 100644 --- a/src/engine/config.ts +++ b/src/engine/config.ts @@ -217,7 +217,7 @@ export function loadCompareConfig( cases: rawCases.map((rawCase, index) => normalizeCase(baseDir, recordValue(rawCase, `scenarios[${index}]`), index)), }; - validateCompareConfig(config); + validateCompareConfig(config, options); return config; } @@ -242,11 +242,16 @@ function normalizeCase(baseDir: string, raw: RawCase, index: number): EvalCaseCo }; } -function validateCompareConfig(config: CompareConfig): void { - if (config.baselineSkills.length === 0) { - throw new Error("compare requires at least one baseline skill"); - } - if (config.proposedSkills.length === 0) { +function validateCompareConfig(config: CompareConfig, options: LoadOptions = {}): void { + // An explicit empty baselineSkills is a valid comparison: "does adding this + // skill change anything at all" needs a no-skill baseline arm. normalizeSkills + // already rejects an omitted or misspelled key, so a typo still fails loudly. + // + // In single-arm (measure) mode the proposed arm is a mirror of baseline that + // no run ever exercises, so an empty mirror is not a missing proposed set. + // Requiring content there would reject "characterize the agent with no + // skills," the measure-side version of the question this baseline change enables. + if (!options.singleArm && config.proposedSkills.length === 0) { throw new Error("compare requires at least one proposed skill"); } if (config.runs < 1) { diff --git a/test/config.test.ts b/test/config.test.ts index 8e8ff5c..8435215 100644 --- a/test/config.test.ts +++ b/test/config.test.ts @@ -249,3 +249,76 @@ test("loadCompareConfig parses maxTurns at both levels and rejects bad caps", () rmSync(dir, { recursive: true, force: true }); } }); + +test("loadCompareConfig accepts an explicit empty baselineSkills but still requires proposedSkills", () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-config-empty-baseline-")); + try { + writeFileSync(join(dir, "agent.md"), "Agent", "utf8"); + writeFileSync(join(dir, "proposed.md"), "Proposed", "utf8"); + + // An explicit `[]` answers "does adding this skill change anything at all" + // and loads with an empty baseline arm. + writeFileSync( + join(dir, "empty-baseline.json"), + JSON.stringify({ + agent: "./agent.md", + baselineSkills: [], + proposedSkills: ["./proposed.md"], + model: "sonnet", + scenarios: [{ name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } }], + }), + "utf8", + ); + const config = loadCompareConfig(join(dir, "empty-baseline.json")); + expect(config.baselineSkills).toEqual([]); + expect(config.proposedSkills).toEqual([join(dir, "proposed.md")]); + + // A shared top-level `skills` set must not backfill an explicit `[]`: + // normalizeSkills returns on the Array.isArray branch before it ever + // reaches the shared-skills fallback, so pin that here. + writeFileSync(join(dir, "shared.md"), "Shared", "utf8"); + writeFileSync( + join(dir, "empty-baseline-with-shared.json"), + JSON.stringify({ + agent: "./agent.md", + skills: ["./shared.md"], + baselineSkills: [], + proposedSkills: ["./proposed.md"], + model: "sonnet", + scenarios: [{ name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } }], + }), + "utf8", + ); + expect(loadCompareConfig(join(dir, "empty-baseline-with-shared.json")).baselineSkills).toEqual([]); + + // Omitting the key entirely is a different failure than supplying `[]`: + // a typo'd or forgotten key must not silently become a no-skill baseline. + writeFileSync( + join(dir, "omitted-baseline.json"), + JSON.stringify({ + agent: "./agent.md", + proposedSkills: ["./proposed.md"], + model: "sonnet", + scenarios: [{ name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } }], + }), + "utf8", + ); + expect(() => loadCompareConfig(join(dir, "omitted-baseline.json"))).toThrow(/baseline skill paths/); + + // A compare with nothing proposed is meaningless even when the baseline is empty. + writeFileSync( + join(dir, "empty-proposed.json"), + JSON.stringify({ + agent: "./agent.md", + baselineSkills: [], + proposedSkills: [], + model: "sonnet", + scenarios: [{ name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } }], + }), + "utf8", + ); + expect(() => loadCompareConfig(join(dir, "empty-proposed.json"))).toThrow("compare requires at least one proposed skill"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); diff --git a/test/delivery.test.ts b/test/delivery.test.ts index 9260320..40715e8 100644 --- a/test/delivery.test.ts +++ b/test/delivery.test.ts @@ -81,6 +81,66 @@ test("install delivery puts arm skills in each sandbox registry and keeps them o } }); +test("inline delivery with an empty baseline sends the agent body alone, with no SKILL marker", async () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-delivery-empty-baseline-")); + try { + const agent = join(dir, "agent.md"); + writeFileSync(agent, "Agent persona", "utf8"); + // Inline delivery reads skill paths as files directly (assembleSystemPrompt + // maps over them and reads each one), unlike install delivery's directories. + const proposed = join(dir, "proposed-skill.md"); + writeFileSync(proposed, "PROPOSED_MARKER", "utf8"); + + const seenPrompts: { arm: string; prompt: string }[] = []; + const runner: Runner = { + name: "mock", + capabilities: { sandboxTools: true, skillRegistry: true, images: false, streamEvents: false }, + async run(options: RunnerRunOptions) { + const arm = options.systemPrompt.includes("PROPOSED_MARKER") ? "proposed" : "baseline"; + seenPrompts.push({ arm, prompt: options.systemPrompt }); + return { output: "ok", costUsd: 0.1, turns: 1, durationMs: 10, models: ["sonnet"], raw: {} }; + }, + }; + + const config: CompareConfig = { + name: "inline delivery, empty baseline", + agent, + baselineSkills: [], + proposedSkills: [proposed], + delivery: "inline", + arms: { + baseline: { model: "sonnet", runner: "claude-p" }, + proposed: { model: "sonnet", runner: "claude-p" }, + }, + runs: 1, + timeoutMs: 1_000, + maxBudgetUsd: 1, + addDirs: [], + sandboxRoot: join(dir, "runs"), + keepSandbox: false, + cases: [ + { + name: "target", + kind: "target", + prompt: "do the task", + grader: { type: "text", contains: ["ok"] }, + images: [], + addDirs: [], + }, + ], + }; + + await runCompare({ config, runners: { baseline: runner, proposed: runner } }); + const baselinePrompt = seenPrompts.find((entry) => entry.arm === "baseline")?.prompt; + // No baseline skills means assembleSystemPrompt has nothing to join in. + // The baseline arm's prompt is the agent file, verbatim, with no skill section. + expect(baselinePrompt).toBe("Agent persona"); + expect(baselinePrompt).not.toContain("===== SKILL"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("scenario files parse delivery and reject install with disabled tools", () => { const dir = mkdtempSync(join(tmpdir(), "promptdiff-delivery-config-")); try { diff --git a/test/measure.test.ts b/test/measure.test.ts index 59db4ce..adec557 100644 --- a/test/measure.test.ts +++ b/test/measure.test.ts @@ -94,6 +94,34 @@ test("singleArm loading accepts baselineSkills-only scenarios; compare still req } }); +test("singleArm loading accepts an empty baselineSkills; the mirrored empty proposed arm is never a validation error", () => { + const dir = mkdtempSync(join(tmpdir(), "promptdiff-measure-empty-config-")); + try { + writeFileSync(join(dir, "agent.md"), "Agent.", "utf8"); + const scenario = { + agent: "./agent.md", + baselineSkills: [], + model: "sonnet", + scenarios: [{ name: "t", prompt: "p", grader: { type: "text", contains: ["ok"] } }], + }; + const path = join(dir, "s.json"); + writeFileSync(path, JSON.stringify(scenario), "utf8"); + + // "Characterize the agent with no skills" is the measure-side version of + // the question an empty baseline enables in compare. Proposed mirrors + // baseline here and no run ever touches it, so an empty mirror must load. + const config = loadCompareConfig(path, {}, { singleArm: true }); + expect(config.baselineSkills).toEqual([]); + expect(config.proposedSkills).toEqual([]); + + // The same scenario as a real compare has no proposed arm at all: that + // still throws, since a compare with nothing proposed is meaningless. + expect(() => loadCompareConfig(path)).toThrow(/proposed skill paths/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("measure warns when the measured model diverges from productionModel", async () => { const dir = mkdtempSync(join(tmpdir(), "promptdiff-measure-prod-")); try { diff --git a/test/skill-install.test.ts b/test/skill-install.test.ts index 7073a96..39db10b 100644 --- a/test/skill-install.test.ts +++ b/test/skill-install.test.ts @@ -100,6 +100,24 @@ test("installSkills warns when a description exceeds the registry limit", () => } }); +test("installSkills with an empty list installs nothing and creates no skills directory", () => { + const root = mkdtempSync(join(tmpdir(), "skill-install-test-")); + try { + const sandbox = join(root, "sandbox"); + mkdirSync(sandbox); + + const result = installSkills([], sandbox, join(root, "no-user-skills")); + + expect(result.installed).toEqual([]); + expect(result.warnings).toEqual([]); + // An empty baseline arm must leave the registry untouched, not an empty + // .claude/skills directory a later check could mistake for "installed but empty". + expect(existsSync(join(sandbox, ".claude", "skills"))).toBe(false); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); + test("deliveryValue validates the delivery axis", () => { expect(deliveryValue(undefined, "inline")).toBe("inline"); expect(deliveryValue("install", "inline")).toBe("install");