diff --git a/evaluation/benchmarks/swe_bench/scripts/run_infer.sh b/evaluation/benchmarks/swe_bench/scripts/run_infer.sh index 2edf91b721cc..d6b14916000d 100755 --- a/evaluation/benchmarks/swe_bench/scripts/run_infer.sh +++ b/evaluation/benchmarks/swe_bench/scripts/run_infer.sh @@ -23,6 +23,11 @@ # COMMAND_EXEC_TIMEOUT per-bash-command timeout in seconds # DIVERSIFY_TOOL_NAMES optional: rename tools for RL diversity # CAMEL_CASE_TOOL_NAMES optional: camelCase tool names +# PATCH_MODE optional: how the model patch is extracted. +# `worktree` (default) = `git diff` of the +# working tree; `committed` = diff of what the +# agent committed, for task families whose +# prompt asks the agent to commit its work. set -eo pipefail @@ -91,6 +96,7 @@ echo "INSTANCE_DICT_PATH: $INSTANCE_DICT_PATH" echo "CONFIG_FILE: $CONFIG_FILE" echo "WORKSPACE_ROOT: $WORKSPACE_ROOT" echo "USER_MESSAGE_PATH: $USER_MESSAGE_PATH" +echo "PATCH_MODE: ${PATCH_MODE:-worktree (default)}" echo "SYSTEM_PROMPT_PATH: $SYSTEM_PROMPT_PATH" echo "MODEL_SERVER: $NEMO_GYM_MODEL_SERVER_NAME @ $NEMO_GYM_MODEL_SERVER_BASE_URL" @@ -113,6 +119,10 @@ fi if [ "${ENABLE_SUBAGENTS:-0}" = "1" ] || [ "${ENABLE_SUBAGENTS:-}" = "true" ]; then cmd+=(--enable-subagents) fi +# Omitted entirely when unset so cli.ts keeps its own default (`worktree`). +if [ -n "${PATCH_MODE:-}" ]; then + cmd+=(--patch-mode "$PATCH_MODE") +fi echo "Executing: ${cmd[*]}" exec "${cmd[@]}" diff --git a/packages/opencode/src/bench/cli.ts b/packages/opencode/src/bench/cli.ts index a38ef8a3877b..a7c00024cb4b 100644 --- a/packages/opencode/src/bench/cli.ts +++ b/packages/opencode/src/bench/cli.ts @@ -16,7 +16,8 @@ * * Trajectory capture: the nemo-gym provider (registered via this config) * writes `/.json` per LLM call BEFORE returning. On - * exit we capture `git diff` and write `output.jsonl`. + * exit we capture the model patch (see ./patch.ts for the two modes) and write + * `output.jsonl`. */ import { existsSync, promises as fs, readFileSync } from "node:fs" @@ -25,6 +26,7 @@ import os from "node:os" import { spawn } from "node:child_process" import { runDeepReset } from "./deep_reset" import { bootstrapRepoIfMissing } from "./bootstrap_repo" +import { capturePatch, ensureCommitIdentity, parsePatchMode, recordBaselineCommit, type PatchMode } from "./patch" import * as BenchTerminalError from "./terminal_error" // opencode's built-in anthropic system prompt — Bun bundles .txt as a string. // Used as the default when no --system-prompt override is passed. @@ -46,6 +48,8 @@ interface CliArgs { systemPromptPath?: string /** Enable opencode's `task` tool (spawns subagent sessions). */ enableSubagents: boolean + /** How the model patch is extracted at the end of the run. See ./patch.ts. */ + patchMode: PatchMode } function parseArgs(argv: string[]): CliArgs { @@ -55,6 +59,7 @@ function parseArgs(argv: string[]): CliArgs { dataset: "", split: "test", enableSubagents: false, + patchMode: parsePatchMode(undefined), } for (let i = 0; i < argv.length; i++) { const a = argv[i] @@ -96,6 +101,9 @@ function parseArgs(argv: string[]): CliArgs { case "--enable-subagents": out.enableSubagents = true break + case "--patch-mode": + out.patchMode = parsePatchMode(next()) + break default: if (a.startsWith("--")) throw new Error(`Unknown flag: ${a}`) } @@ -332,16 +340,6 @@ async function buildConfigDir(args: { return { tmpRoot, configFile } } -// Some SIFs ship with bare PATH lookups that ENOENT on bare program names -// through Bun's posix_spawn. Resolve to an absolute path up front for any -// binary we shell out to. -function detectBin(candidates: string[]): string | null { - for (const p of candidates) { - if (existsSync(p)) return p - } - return null -} - function runOpencode(args: { workspaceRoot: string modelName: string @@ -416,25 +414,6 @@ function runOpencode(args: { }) } -async function captureGitDiff(workspaceRoot: string): Promise { - const gitPath = detectBin(["/usr/bin/git", "/bin/git", "/usr/local/bin/git"]) ?? "git" - const runGit = (args: string[], capture: boolean): Promise => - new Promise((resolve) => { - const child = spawn(gitPath, ["-C", workspaceRoot, ...args], { - env: { ...process.env, GIT_PAGER: "cat" }, - }) - let stdout = "" - if (capture) child.stdout?.on("data", (b) => (stdout += b.toString("utf8"))) - child.on("close", () => resolve(stdout)) - child.on("error", () => resolve("")) - }) - // Mark untracked files as intent-to-add so newly-created files appear in - // `git diff` without being committed. Plain `git diff` only shows changes - // to tracked files, which silently drops new-file patches the agent wrote. - await runGit(["add", "-AN"], false) - return runGit(["diff", "--binary"], true) -} - interface OutputJsonl { instance_id: string test_result: { git_patch: string } @@ -538,7 +517,7 @@ async function main() { } // Bootstrap a git repo if the SIF shipped a flat source tree (swe-bench-ext - // and some SWE-rebench variants). Without this, captureGitDiff returns "" + // and some SWE-rebench variants). Without this, the patch capture returns "" // and every patch is recorded as 0 bytes. const { freshInit } = await bootstrapRepoIfMissing(workspaceRoot) @@ -551,6 +530,16 @@ async function main() { await runDeepReset(workspaceRoot, String(instance.base_commit ?? "")) } + // Snapshot the pristine HEAD *after* bootstrap/deep-reset — it is the diff + // base for `--patch-mode committed`. Recorded unconditionally so the log + // always shows what the agent started from. + const baselineCommit = await recordBaselineCommit(workspaceRoot) + console.log(`[bench] patch_mode=${args.patchMode} baseline=${baselineCommit || ""}`) + if (args.patchMode === "committed") { + // The agent is expected to commit; make sure git will let it. + await ensureCommitIdentity(workspaceRoot) + } + const opencodeBin = detectOpencodeBin() const result = await runOpencode({ workspaceRoot, @@ -561,7 +550,7 @@ async function main() { agent: "swe-bench", }) - const patch = await captureGitDiff(workspaceRoot) + const patch = await capturePatch(workspaceRoot, args.patchMode, baselineCommit) const benchRunTime = (Date.now() - startedAt) / 1000 const error = BenchTerminalError.toGymError(result.exitCode, result.terminalError) @@ -572,11 +561,14 @@ async function main() { metrics: { bench_run_time: benchRunTime, opencode_exit_code: result.exitCode, + patch_mode: args.patchMode, }, error, }) - console.log(`[bench] wrote ${outPath} (patch=${patch.length} bytes, error=${error ?? "none"})`) + console.log( + `[bench] wrote ${outPath} (patch=${patch.length} bytes, mode=${args.patchMode}, error=${error ?? "none"})`, + ) // Mirror opencode's exit code explicitly. Falling off the end of main() and // letting Bun drain the event loop produced a flaky exit=1 even when the diff --git a/packages/opencode/src/bench/patch.ts b/packages/opencode/src/bench/patch.ts new file mode 100644 index 000000000000..135ba335028b --- /dev/null +++ b/packages/opencode/src/bench/patch.ts @@ -0,0 +1,218 @@ +/** + * Model-patch capture for the bench driver. + * + * Two modes, selected by `--patch-mode` (gym passes it through the + * `PATCH_MODE` env var in run_infer.sh): + * + * - `worktree` (DEFAULT, the historical behaviour): mark untracked files + * intent-to-add and take `git diff` of the working tree. Correct for the + * SWE-bench-style prompts that explicitly tell the agent *not* to commit. + * + * - `committed`: ignore the working tree and extract what the agent + * COMMITTED. Required by task families whose problem statement ends with + * "work on this in a new branch from main and commit everything when you + * are done" (e.g. the DeepSWE set). There, the agent commits its solution, + * leaving a clean tree — `git diff` returns "" and every rollout would be + * recorded as a 0-byte patch. + * + * In `committed` mode we diff `baseline..tip`, where `baseline` is the HEAD sha + * captured *before* the agent starts (after bootstrap/deep-reset, so it is the + * dataset's base commit) and `tip` is the most-advanced commit the agent left + * behind. `tip` is searched across HEAD *and* every local branch, because the + * agent may commit on a side branch and then switch back to main — HEAD alone + * would silently yield an empty patch. + * + * The resulting patch is still a plain `baseline -> final tree` unified diff, + * so the eval side (`git reset --hard ` + `git apply`) is + * unchanged regardless of mode. + */ + +import { spawn } from "node:child_process" +import { existsSync } from "node:fs" + +export type PatchMode = "worktree" | "committed" + +export const PATCH_MODES: PatchMode[] = ["worktree", "committed"] + +export const DEFAULT_PATCH_MODE: PatchMode = "worktree" + +export function parsePatchMode(raw: string | undefined): PatchMode { + if (!raw) return DEFAULT_PATCH_MODE + const v = raw.trim().toLowerCase() + if ((PATCH_MODES as string[]).includes(v)) return v as PatchMode + throw new Error(`Invalid --patch-mode "${raw}" (expected one of: ${PATCH_MODES.join(", ")})`) +} + +// git's canonical empty-tree object id. Used as the diff base when the repo has +// no commits at all (unborn HEAD), so a first commit still produces a patch. +const EMPTY_TREE = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" + +// Same rationale as cli.ts/deep_reset.ts: some SIFs ENOENT on bare program +// names through Bun's posix_spawn, so resolve an absolute path up front. +function detectGit(): string { + for (const p of ["/usr/bin/git", "/bin/git", "/usr/local/bin/git"]) { + if (existsSync(p)) return p + } + return "git" +} + +interface GitResult { + stdout: string + exitCode: number +} + +function git(workspaceRoot: string, args: string[]): Promise { + const gitPath = detectGit() + return new Promise((resolve) => { + const child = spawn(gitPath, ["-C", workspaceRoot, ...args], { + env: { ...process.env, GIT_PAGER: "cat" }, + stdio: ["ignore", "pipe", "pipe"], + }) + let stdout = "" + child.stdout?.on("data", (b) => (stdout += b.toString("utf8"))) + // Swallow stderr: every call here is best-effort and a noisy `git` on a + // half-broken repo must not pollute the gym log or fail the rollout. + child.stderr?.on("data", () => {}) + child.on("close", (code) => resolve({ stdout, exitCode: code ?? 0 })) + child.on("error", () => resolve({ stdout: "", exitCode: 1 })) + }) +} + +/** + * Snapshot HEAD before the agent runs. Returns "" when the repo has no commit + * yet (unborn HEAD) or isn't a repo at all; `capturePatch` then falls back to + * the empty tree. + */ +export async function recordBaselineCommit(workspaceRoot: string): Promise { + const res = await git(workspaceRoot, ["rev-parse", "--verify", "--quiet", "HEAD^{commit}"]) + return res.exitCode === 0 ? res.stdout.trim() : "" +} + +/** + * Give git a committer identity if the image doesn't ship one. Without this the + * agent's very first `git commit` dies with "Author identity unknown" and the + * whole rollout scores zero for a reason that has nothing to do with the model. + * Repo-local (`--local`) so we don't mutate anything outside the workspace, and + * only when unset, so a task-provided identity always wins. + */ +export async function ensureCommitIdentity(workspaceRoot: string): Promise { + for (const [key, value] of [ + ["user.email", "agent@opencode.local"], + ["user.name", "opencode agent"], + ]) { + const existing = await git(workspaceRoot, ["config", "--get", key]) + if (existing.exitCode === 0 && existing.stdout.trim()) continue + await git(workspaceRoot, ["config", "--local", key, value]) + } +} + +async function worktreePatch(workspaceRoot: string): Promise { + // Mark untracked files as intent-to-add so newly-created files appear in + // `git diff` without being committed. Plain `git diff` only shows changes + // to tracked files, which silently drops new-file patches the agent wrote. + await git(workspaceRoot, ["add", "-AN"]) + const res = await git(workspaceRoot, ["diff", "--binary"]) + return res.stdout +} + +interface Candidate { + /** commit sha of the ref tip */ + sha: string + /** human label for logging: "HEAD" or the branch name */ + label: string + /** commits reachable from `sha` but not from the baseline */ + ahead: number +} + +/** + * Every commit the agent could have left its work on: HEAD (covers detached + * HEAD and "still on the branch it committed to") plus every local branch + * (covers "committed on a side branch, then checked main back out"). + */ +async function candidateTips(workspaceRoot: string, baseline: string): Promise { + const tips: { sha: string; label: string }[] = [] + + const head = await git(workspaceRoot, ["rev-parse", "--verify", "--quiet", "HEAD^{commit}"]) + if (head.exitCode === 0 && head.stdout.trim()) tips.push({ sha: head.stdout.trim(), label: "HEAD" }) + + const branches = await git(workspaceRoot, ["for-each-ref", "--format=%(objectname) %(refname:short)", "refs/heads"]) + for (const line of branches.stdout.split("\n")) { + const [sha, ...rest] = line.trim().split(" ") + if (!sha) continue + if (tips.some((t) => t.sha === sha)) continue // HEAD already covers this tip + tips.push({ sha, label: rest.join(" ") || sha.slice(0, 8) }) + } + + const out: Candidate[] = [] + for (const tip of tips) { + // `baseline..tip` counts commits reachable from tip but not from baseline. + // Deliberately NOT gated on `merge-base --is-ancestor`: an agent that + // amended or rebased its work leaves a tip that no longer descends from + // baseline, and we still want that work. + const res = await git(workspaceRoot, ["rev-list", "--count", `${baseline}..${tip.sha}`]) + const ahead = res.exitCode === 0 ? parseInt(res.stdout.trim(), 10) : 0 + out.push({ ...tip, ahead: Number.isFinite(ahead) ? ahead : 0 }) + } + return out +} + +/** Count of dirty/untracked paths, reported so a dropped worktree is visible in the log. */ +async function dirtyPathCount(workspaceRoot: string): Promise { + const res = await git(workspaceRoot, ["status", "--porcelain", "--untracked-files=all"]) + return res.stdout.split("\n").filter((l) => l.trim()).length +} + +async function committedPatch(workspaceRoot: string, baselineCommit: string): Promise { + const baseline = baselineCommit || EMPTY_TREE + if (!baselineCommit) { + console.log(`[bench] patch_mode=committed: no pre-run HEAD; diffing against the empty tree`) + } + + const candidates = await candidateTips(workspaceRoot, baseline) + // Most commits past the baseline wins. `candidateTips` puts HEAD first, and + // Array.prototype.sort is stable in every JS engine we ship on, so HEAD wins + // ties against a side branch holding the identical work. + const ranked = candidates.filter((c) => c.ahead > 0).sort((a, b) => b.ahead - a.ahead) + const dirty = await dirtyPathCount(workspaceRoot) + + if (ranked.length === 0) { + console.log( + `[bench] patch_mode=committed: agent left no commit past baseline ${baseline.slice(0, 8)} ` + + `(refs checked: ${candidates.length}, uncommitted paths: ${dirty}) -> empty patch`, + ) + return "" + } + + const chosen = ranked[0]! + const others = ranked + .slice(1) + .map((c) => `${c.label}+${c.ahead}`) + .join(",") + console.log( + `[bench] patch_mode=committed: baseline=${baseline.slice(0, 8)} tip=${chosen.label}@${chosen.sha.slice(0, 8)} ` + + `commits=${chosen.ahead} uncommitted_paths=${dirty}${others ? ` other_refs=[${others}]` : ""}`, + ) + if (dirty > 0) { + console.log( + `[bench] patch_mode=committed: ${dirty} uncommitted path(s) are NOT in the patch ` + + `(the task asked the agent to commit its work)`, + ) + } + + const res = await git(workspaceRoot, ["diff", "--binary", baseline, chosen.sha]) + return res.stdout +} + +/** + * Produce the model patch for `mode`. + * + * @param baselineCommit HEAD sha captured before the agent ran; only used by + * `committed` mode. + */ +export async function capturePatch( + workspaceRoot: string, + mode: PatchMode, + baselineCommit: string = "", +): Promise { + return mode === "committed" ? committedPatch(workspaceRoot, baselineCommit) : worktreePatch(workspaceRoot) +} diff --git a/packages/opencode/test/bench/patch.test.ts b/packages/opencode/test/bench/patch.test.ts new file mode 100644 index 000000000000..5aae8f5b1f2e --- /dev/null +++ b/packages/opencode/test/bench/patch.test.ts @@ -0,0 +1,154 @@ +import { $ } from "bun" +import { afterEach, describe, expect, test } from "bun:test" +import fs from "fs/promises" +import os from "os" +import path from "path" +import { capturePatch, ensureCommitIdentity, parsePatchMode, recordBaselineCommit } from "../../src/bench/patch" + +// Deliberately NOT using ../fixture/fixture: src/bench/* is a standalone leaf +// (node:child_process + node:fs only) that runs inside minimal SIF images, and +// this suite should stay runnable without booting the instance/effect stack. +const created: string[] = [] + +afterEach(async () => { + await Promise.all(created.splice(0).map((dir) => fs.rm(dir, { recursive: true, force: true }).catch(() => {}))) +}) + +async function repo(withBaseline = true) { + const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "bench-patch-"))) + created.push(dir) + await $`git init -q -b main`.cwd(dir).quiet() + await $`git config user.email bench@opencode.local`.cwd(dir).quiet() + await $`git config user.name bench`.cwd(dir).quiet() + await $`git config commit.gpgsign false`.cwd(dir).quiet() + if (withBaseline) { + await Bun.write(path.join(dir, "app.py"), "def solve():\n return 0\n") + await commit(dir, "baseline") + } + return dir +} + +async function commit(dir: string, message: string) { + await $`git add -A`.cwd(dir).quiet() + await $`git commit -q -m ${message}`.cwd(dir).quiet() +} + +describe("bench patch mode", () => { + test("parsePatchMode defaults to worktree and rejects garbage", () => { + expect(parsePatchMode(undefined)).toBe("worktree") + expect(parsePatchMode("")).toBe("worktree") + expect(parsePatchMode("Committed")).toBe("committed") + expect(() => parsePatchMode("staged")).toThrow() + }) + + test("worktree mode captures uncommitted edits and new files", async () => { + const dir = await repo() + const baseline = await recordBaselineCommit(dir) + + await Bun.write(path.join(dir, "app.py"), "def solve():\n return 1\n") + await Bun.write(path.join(dir, "new_file.py"), "X = 1\n") + + const patch = await capturePatch(dir, "worktree", baseline) + expect(patch).toContain("+ return 1") + expect(patch).toContain("new_file.py") + }) + + test("worktree mode misses work the agent committed (the DeepSWE failure)", async () => { + const dir = await repo() + const baseline = await recordBaselineCommit(dir) + + await Bun.write(path.join(dir, "app.py"), "def solve():\n return 1\n") + await commit(dir, "fix") + + expect(await capturePatch(dir, "worktree", baseline)).toBe("") + }) + + test("committed mode captures a side-branch commit from either HEAD position", async () => { + const dir = await repo() + const baseline = await recordBaselineCommit(dir) + + await $`git checkout -q -b fix/solve`.cwd(dir).quiet() + await Bun.write(path.join(dir, "app.py"), "def solve():\n return 1\n") + await Bun.write(path.join(dir, "new_file.py"), "X = 1\n") + await commit(dir, "fix") + + const onBranch = await capturePatch(dir, "committed", baseline) + expect(onBranch).toContain("+ return 1") + expect(onBranch).toContain("new_file.py") + + // Agent switched back to main after committing: HEAD sits at the baseline, + // so only the local-branch scan finds the work. + await $`git checkout -q main`.cwd(dir).quiet() + expect(await capturePatch(dir, "committed", baseline)).toBe(onBranch) + }) + + test("committed mode picks the ref with the most commits past baseline", async () => { + const dir = await repo() + const baseline = await recordBaselineCommit(dir) + + await $`git checkout -q -b scratch`.cwd(dir).quiet() + await Bun.write(path.join(dir, "scratch.txt"), "debug\n") + await commit(dir, "scratch") + + await $`git checkout -q -b fix/solve main`.cwd(dir).quiet() + for (const n of [1, 2]) { + await Bun.write(path.join(dir, "app.py"), `def solve():\n return ${n}\n`) + await commit(dir, `step${n}`) + } + await $`git checkout -q main`.cwd(dir).quiet() + + const patch = await capturePatch(dir, "committed", baseline) + expect(patch).toContain("+ return 2") + expect(patch).not.toContain("scratch.txt") + }) + + test("committed mode excludes uncommitted leftovers", async () => { + const dir = await repo() + const baseline = await recordBaselineCommit(dir) + + await Bun.write(path.join(dir, "app.py"), "def solve():\n return 1\n") + await commit(dir, "fix") + await Bun.write(path.join(dir, "repro_scratch.py"), "print('debug')\n") + + const patch = await capturePatch(dir, "committed", baseline) + expect(patch).toContain("+ return 1") + expect(patch).not.toContain("repro_scratch.py") + }) + + test("committed mode yields an empty patch when the agent never committed", async () => { + const dir = await repo() + const baseline = await recordBaselineCommit(dir) + + await Bun.write(path.join(dir, "app.py"), "def solve():\n return 1\n") + + expect(await capturePatch(dir, "committed", baseline)).toBe("") + }) + + test("committed mode falls back to the empty tree for an unborn HEAD", async () => { + const dir = await repo(false) + const baseline = await recordBaselineCommit(dir) + expect(baseline).toBe("") + + await Bun.write(path.join(dir, "app.py"), "X = 1\n") + await commit(dir, "first") + + const patch = await capturePatch(dir, "committed", baseline) + expect(patch).toContain("app.py") + expect(patch).toContain("+X = 1") + }) + + test("ensureCommitIdentity fills a missing identity and keeps an existing one", async () => { + const dir = await fs.realpath(await fs.mkdtemp(path.join(os.tmpdir(), "bench-patch-"))) + created.push(dir) + await $`git init -q -b main`.cwd(dir).quiet() + // A user-level identity would mask the "missing" case on a dev machine. + await $`git config --local user.useConfigOnly true`.cwd(dir).quiet() + + await ensureCommitIdentity(dir) + expect((await $`git config --get user.email`.cwd(dir).quiet().text()).trim()).not.toBe("") + + await $`git config --local user.email task@example.com`.cwd(dir).quiet() + await ensureCommitIdentity(dir) + expect((await $`git config --get user.email`.cwd(dir).quiet().text()).trim()).toBe("task@example.com") + }) +})