Back model benchmark with Braintrust, drop in-house scoring#13
Merged
time-attack merged 5 commits intoJul 21, 2026
Merged
Conversation
Backing the model benchmark with Braintrust's local eval runner and its autoevals scorer library instead of an in-house scoring engine. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The contract previously named Context.dev the only authorized external service and barred provider marketplaces. Loosen it so optional, off-by- default, consent-gated eval backends (Braintrust and the like) are allowed; keep the consent + typed-failure hygiene. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The benchmark adapter only recognized GOOGLE_API_KEY and parsed the legacy text/usage event shape. Recognize GEMINI_API_KEY and ~/.gemini/.env, parse the current content/stats schema, ignore echoed user messages, and run report-only plan mode with --skip-trust for disposable workspaces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Braintrust now owns benchmark scoring, experiments, comparison, and reporting. GStack keeps only the CLI-agent adapters (the unavoidable shim) plus operational metrics the CLIs report. runProviderBenchmark wraps each adapter as a Braintrust Eval task; a deterministic required-terms scorer replaces the in-house evaluation logic and the optional autoevals ClosedQA judge replaces judge.ts. Runs local by default under bun: with no BRAINTRUST_API_KEY it sets noSendLogs and ships nothing; setting the key opts into the cloud dashboard. An empty output with zero tokens is thrown so a silent auth/CLI failure can't masquerade as a 0.0 score. Deletes runner.ts and judge.ts (and their test-helper re-export shims); rewires bin/gstack-model-benchmark and adapts the benchmark tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
The adapters parsed each vendor's proprietary JSON stream (Claude json, Codex JSONL, Gemini stream-json) to extract tokens/tool-calls, and a per-model pricing table turned tokens into cost. That coupling was the brittle hardcoding GStack 2 exists to avoid — it broke every time a vendor reshuffled its output, and it duplicated what any tool that instruments the real model call already does. Braintrust owns scoring; it can't see a CLI subprocess's tokens anyway, so computing cost ourselves meant maintaining both a parser and a price table forever. Now each adapter runs the CLI in plain-text mode and returns stdout. Scoring is unchanged (Braintrust reads the text). RunResult drops tokens/toolCalls; the comparison table drops the Tokens/Cost columns. Deletes pricing.ts, all three JSON parsers, and the Gemini stream-schema parser + its test. Gemini auth detection (env/OAuth/.env) is kept — that's not schema parsing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="package.json">
<violation number="1" location="package.json:102">
P1: The benchmark shipped in the managed runtime will fail immediately with `Cannot find package 'braintrust'`; `--judge` likewise cannot resolve `autoevals`. These packages and their runtime dependency closures need entries in `DEFAULT_RUNTIME_BUNDLE` (plus an installed-runtime smoke test), rather than only package declarations.</violation>
</file>
<file name="evals/model-benchmark/corpus.json">
<violation number="1" location="evals/model-benchmark/corpus.json:4">
P2: This case cannot measure security-review ability because the referenced synthetic patch is never supplied; the empty-workdir task can earn full credit by echoing the three vulnerabilities named in the prompt. Including an actual diff or fixture would make the result reflect diagnosis instead of keyword repetition.</violation>
<violation number="2" location="evals/model-benchmark/corpus.json:5">
P2: Correct report-only responses are penalized unless they literally print “no mutation,” even though mutation compliance is behavior rather than required answer content. Removing this phrase from the substring expectations avoids this false negative; mutation should be verified separately if it is part of the benchmark.</violation>
</file>
<file name="CLAUDE.md">
<violation number="1" location="CLAUDE.md:36">
P2: Agents now receive contradictory security policy: CLAUDE.md broadly authorizes consent-gated external services, while AGENTS.md permits only Context.dev and requires an accepted issue plus maintainer approval. Please synchronize both canonical contracts and explicitly scope authorization to the approved Braintrust integration rather than “and the like.”</violation>
</file>
<file name="evals/model-benchmark/gstack.eval.ts">
<violation number="1" location="evals/model-benchmark/gstack.eval.ts:15">
P1: Conductor benchmark runs lose their configured Claude/OpenAI credentials because this entrypoint omits the side-effecting `conductor-env-shim` import used by the CLI. Importing the shim before the benchmark module preserves the documented `GSTACK_*` credential promotion.</violation>
<violation number="2" location="evals/model-benchmark/gstack.eval.ts:18">
P2: A malformed or empty `GSTACK_BENCH_TIMEOUT_MS` can disable the timeout or fail inside `execFileSync`, allowing CI benchmark processes to hang or abort with an argument error. Validate a finite positive timeout before starting paid provider calls.</violation>
<violation number="3" location="evals/model-benchmark/gstack.eval.ts:21">
P2: This benchmark entrypoint is never selected by the repository eval runner, so periodic/CI evals do not execute the newly added corpus benchmark. Register it in the periodic `test:evals` harness with touchfiles and detached execution rather than leaving it as an unreferenced standalone script.</violation>
</file>
<file name="lib/model-benchmark/providers/gemini.ts">
<violation number="1" location="lib/model-benchmark/providers/gemini.ts:28">
P3: Gemini availability can now throw for an unreadable/raced `~/.gemini/.env`, and an empty key assignment is treated as valid auth. Wrapping the read and requiring a non-empty value would preserve the adapter's non-throwing availability contract.</violation>
</file>
<file name="lib/model-benchmark/braintrust-eval.ts">
<violation number="1" location="lib/model-benchmark/braintrust-eval.ts:91">
P2: Normal benchmark runs invoke providers even when their binary or auth is unavailable, so an auth prompt/failure can consume the full 300-second timeout instead of being skipped. Checking `available()` before entering `Eval` would retain the documented clean-skip behavior.</violation>
<violation number="2" location="lib/model-benchmark/braintrust-eval.ts:95">
P2: Repeated prompts in a custom corpus cause every matching `CaseOps` row to receive the last case's ID, corrupting per-case reporting. Passing `{ id, prompt }` as the Braintrust input would preserve identity without relying on prompt uniqueness.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| "@huggingface/transformers": "^4.1.0" | ||
| "@huggingface/transformers": "^4.1.0", | ||
| "autoevals": "^0.3.0", | ||
| "braintrust": "^3.24.0" |
There was a problem hiding this comment.
P1: The benchmark shipped in the managed runtime will fail immediately with Cannot find package 'braintrust'; --judge likewise cannot resolve autoevals. These packages and their runtime dependency closures need entries in DEFAULT_RUNTIME_BUNDLE (plus an installed-runtime smoke test), rather than only package declarations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 102:
<comment>The benchmark shipped in the managed runtime will fail immediately with `Cannot find package 'braintrust'`; `--judge` likewise cannot resolve `autoevals`. These packages and their runtime dependency closures need entries in `DEFAULT_RUNTIME_BUNDLE` (plus an installed-runtime smoke test), rather than only package declarations.</comment>
<file context>
@@ -97,6 +97,8 @@
- "@huggingface/transformers": "^4.1.0"
+ "@huggingface/transformers": "^4.1.0",
+ "autoevals": "^0.3.0",
+ "braintrust": "^3.24.0"
}
}
</file context>
| * export BRAINTRUST_API_KEY=... | ||
| * for p in claude gpt gemini; do GSTACK_BENCH_PROVIDER=$p bun run evals/model-benchmark/gstack.eval.ts; done | ||
| */ | ||
| import { loadCorpus, runProviderBenchmark, type ProviderName } from '../../lib/model-benchmark/braintrust-eval'; |
There was a problem hiding this comment.
P1: Conductor benchmark runs lose their configured Claude/OpenAI credentials because this entrypoint omits the side-effecting conductor-env-shim import used by the CLI. Importing the shim before the benchmark module preserves the documented GSTACK_* credential promotion.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At evals/model-benchmark/gstack.eval.ts, line 15:
<comment>Conductor benchmark runs lose their configured Claude/OpenAI credentials because this entrypoint omits the side-effecting `conductor-env-shim` import used by the CLI. Importing the shim before the benchmark module preserves the documented `GSTACK_*` credential promotion.</comment>
<file context>
@@ -0,0 +1,21 @@
+ * export BRAINTRUST_API_KEY=...
+ * for p in claude gpt gemini; do GSTACK_BENCH_PROVIDER=$p bun run evals/model-benchmark/gstack.eval.ts; done
+ */
+import { loadCorpus, runProviderBenchmark, type ProviderName } from '../../lib/model-benchmark/braintrust-eval';
+
+const provider = (process.env.GSTACK_BENCH_PROVIDER ?? 'claude') as ProviderName;
</file context>
| { | ||
| "id": "security-review", | ||
| "input": "Review a synthetic patch containing command injection, unsafe recursive delete, and path traversal. Do not mutate files. Report only.", | ||
| "required": ["command injection", "recursive delete", "path traversal", "no mutation"] |
There was a problem hiding this comment.
P2: Correct report-only responses are penalized unless they literally print “no mutation,” even though mutation compliance is behavior rather than required answer content. Removing this phrase from the substring expectations avoids this false negative; mutation should be verified separately if it is part of the benchmark.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At evals/model-benchmark/corpus.json, line 5:
<comment>Correct report-only responses are penalized unless they literally print “no mutation,” even though mutation compliance is behavior rather than required answer content. Removing this phrase from the substring expectations avoids this false negative; mutation should be verified separately if it is part of the benchmark.</comment>
<file context>
@@ -0,0 +1,17 @@
+ {
+ "id": "security-review",
+ "input": "Review a synthetic patch containing command injection, unsafe recursive delete, and path traversal. Do not mutate files. Report only.",
+ "required": ["command injection", "recursive delete", "path traversal", "no mutation"]
+ },
+ {
</file context>
| [ | ||
| { | ||
| "id": "security-review", | ||
| "input": "Review a synthetic patch containing command injection, unsafe recursive delete, and path traversal. Do not mutate files. Report only.", |
There was a problem hiding this comment.
P2: This case cannot measure security-review ability because the referenced synthetic patch is never supplied; the empty-workdir task can earn full credit by echoing the three vulnerabilities named in the prompt. Including an actual diff or fixture would make the result reflect diagnosis instead of keyword repetition.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At evals/model-benchmark/corpus.json, line 4:
<comment>This case cannot measure security-review ability because the referenced synthetic patch is never supplied; the empty-workdir task can earn full credit by echoing the three vulnerabilities named in the prompt. Including an actual diff or fixture would make the result reflect diagnosis instead of keyword repetition.</comment>
<file context>
@@ -0,0 +1,17 @@
+[
+ {
+ "id": "security-review",
+ "input": "Review a synthetic patch containing command injection, unsafe recursive delete, and path traversal. Do not mutate files. Report only.",
+ "required": ["command injection", "recursive delete", "path traversal", "no mutation"]
+ },
</file context>
| as locked/atomically written JSON and JSONL. Network mode defaults off. | ||
| Context.dev is the only newly authorized external service, may receive only | ||
| public URLs after explicit consent, and must use the exact typed failure codes. | ||
| New external services (Context.dev, benchmark/eval backends, and the like) are |
There was a problem hiding this comment.
P2: Agents now receive contradictory security policy: CLAUDE.md broadly authorizes consent-gated external services, while AGENTS.md permits only Context.dev and requires an accepted issue plus maintainer approval. Please synchronize both canonical contracts and explicitly scope authorization to the approved Braintrust integration rather than “and the like.”
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLAUDE.md, line 36:
<comment>Agents now receive contradictory security policy: CLAUDE.md broadly authorizes consent-gated external services, while AGENTS.md permits only Context.dev and requires an accepted issue plus maintainer approval. Please synchronize both canonical contracts and explicitly scope authorization to the approved Braintrust integration rather than “and the like.”</comment>
<file context>
@@ -33,13 +33,15 @@ then regenerate. Preserve the source-blob and normalized-render evidence.
as locked/atomically written JSON and JSONL. Network mode defaults off.
-Context.dev is the only newly authorized external service, may receive only
-public URLs after explicit consent, and must use the exact typed failure codes.
+New external services (Context.dev, benchmark/eval backends, and the like) are
+allowed, but each must be optional, off by default, and consent-gated: send only
+what the user approves, and use typed failure codes. Context.dev specifically may
</file context>
| import { loadCorpus, runProviderBenchmark, type ProviderName } from '../../lib/model-benchmark/braintrust-eval'; | ||
|
|
||
| const provider = (process.env.GSTACK_BENCH_PROVIDER ?? 'claude') as ProviderName; | ||
| const timeoutMs = Number(process.env.GSTACK_BENCH_TIMEOUT_MS ?? 300_000); |
There was a problem hiding this comment.
P2: A malformed or empty GSTACK_BENCH_TIMEOUT_MS can disable the timeout or fail inside execFileSync, allowing CI benchmark processes to hang or abort with an argument error. Validate a finite positive timeout before starting paid provider calls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At evals/model-benchmark/gstack.eval.ts, line 18:
<comment>A malformed or empty `GSTACK_BENCH_TIMEOUT_MS` can disable the timeout or fail inside `execFileSync`, allowing CI benchmark processes to hang or abort with an argument error. Validate a finite positive timeout before starting paid provider calls.</comment>
<file context>
@@ -0,0 +1,21 @@
+import { loadCorpus, runProviderBenchmark, type ProviderName } from '../../lib/model-benchmark/braintrust-eval';
+
+const provider = (process.env.GSTACK_BENCH_PROVIDER ?? 'claude') as ProviderName;
+const timeoutMs = Number(process.env.GSTACK_BENCH_TIMEOUT_MS ?? 300_000);
+const judge = process.env.GSTACK_BENCH_JUDGE === '1';
+
</file context>
| @@ -0,0 +1,21 @@ | |||
| /** | |||
There was a problem hiding this comment.
P2: This benchmark entrypoint is never selected by the repository eval runner, so periodic/CI evals do not execute the newly added corpus benchmark. Register it in the periodic test:evals harness with touchfiles and detached execution rather than leaving it as an unreferenced standalone script.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At evals/model-benchmark/gstack.eval.ts, line 21:
<comment>This benchmark entrypoint is never selected by the repository eval runner, so periodic/CI evals do not execute the newly added corpus benchmark. Register it in the periodic `test:evals` harness with touchfiles and detached execution rather than leaving it as an unreferenced standalone script.</comment>
<file context>
@@ -0,0 +1,21 @@
+const timeoutMs = Number(process.env.GSTACK_BENCH_TIMEOUT_MS ?? 300_000);
+const judge = process.env.GSTACK_BENCH_JUDGE === '1';
+
+await runProviderBenchmark(provider, loadCorpus(), { timeoutMs, judge });
</file context>
| const timeoutMs = opts.timeoutMs ?? 300_000; | ||
|
|
||
| const ops: CaseOps[] = []; | ||
| const byInput = new Map(cases.map((c) => [c.input, c])); |
There was a problem hiding this comment.
P2: Repeated prompts in a custom corpus cause every matching CaseOps row to receive the last case's ID, corrupting per-case reporting. Passing { id, prompt } as the Braintrust input would preserve identity without relying on prompt uniqueness.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/braintrust-eval.ts, line 99:
<comment>Repeated prompts in a custom corpus cause every matching `CaseOps` row to receive the last case's ID, corrupting per-case reporting. Passing `{ id, prompt }` as the Braintrust input would preserve identity without relying on prompt uniqueness.</comment>
<file context>
@@ -0,0 +1,160 @@
+ const timeoutMs = opts.timeoutMs ?? 300_000;
+
+ const ops: CaseOps[] = [];
+ const byInput = new Map(cases.map((c) => [c.input, c]));
+
+ // Braintrust calls scorers with { input, output, expected, metadata }.
</file context>
| ): Promise<ProviderBenchmark> { | ||
| const factory = ADAPTERS[provider]; | ||
| if (!factory) throw new Error(`unknown provider '${provider}' (claude|gpt|gemini)`); | ||
| const adapter = factory(); |
There was a problem hiding this comment.
P2: Normal benchmark runs invoke providers even when their binary or auth is unavailable, so an auth prompt/failure can consume the full 300-second timeout instead of being skipped. Checking available() before entering Eval would retain the documented clean-skip behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/braintrust-eval.ts, line 95:
<comment>Normal benchmark runs invoke providers even when their binary or auth is unavailable, so an auth prompt/failure can consume the full 300-second timeout instead of being skipped. Checking `available()` before entering `Eval` would retain the documented clean-skip behavior.</comment>
<file context>
@@ -0,0 +1,160 @@
+): Promise<ProviderBenchmark> {
+ const factory = ADAPTERS[provider];
+ if (!factory) throw new Error(`unknown provider '${provider}' (claude|gpt|gemini)`);
+ const adapter = factory();
+ const timeoutMs = opts.timeoutMs ?? 300_000;
+
</file context>
Comment on lines
+28
to
+29
| const hasEnvFileKey = fs.existsSync(geminiEnv) | ||
| && /^(?:GOOGLE_API_KEY|GEMINI_API_KEY)\s*=/m.test(fs.readFileSync(geminiEnv, 'utf-8')); |
There was a problem hiding this comment.
P3: Gemini availability can now throw for an unreadable/raced ~/.gemini/.env, and an empty key assignment is treated as valid auth. Wrapping the read and requiring a non-empty value would preserve the adapter's non-throwing availability contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/gemini.ts, line 31:
<comment>Gemini availability can now throw for an unreadable/raced `~/.gemini/.env`, and an empty key assignment is treated as valid auth. Wrapping the read and requiring a non-empty value would preserve the adapter's non-throwing availability contract.</comment>
<file context>
@@ -25,19 +26,22 @@ export class GeminiAdapter implements ProviderAdapter {
+ const geminiEnv = path.join(newCfgDir, '.env');
const hasCfg = fs.existsSync(legacyCfgDir) || fs.existsSync(newOauth);
- const hasKey = !!process.env.GOOGLE_API_KEY;
+ const hasEnvFileKey = fs.existsSync(geminiEnv)
+ && /^(?:GOOGLE_API_KEY|GEMINI_API_KEY)\s*=/m.test(fs.readFileSync(geminiEnv, 'utf-8'));
+ const hasKey = !!process.env.GOOGLE_API_KEY || !!process.env.GEMINI_API_KEY || hasEnvFileKey;
</file context>
Suggested change
| const hasEnvFileKey = fs.existsSync(geminiEnv) | |
| && /^(?:GOOGLE_API_KEY|GEMINI_API_KEY)\s*=/m.test(fs.readFileSync(geminiEnv, 'utf-8')); | |
| const hasEnvFileKey = (() => { | |
| try { | |
| return fs.existsSync(geminiEnv) | |
| && /^\s*(?:GOOGLE_API_KEY|GEMINI_API_KEY)\s*=\s*(?:"[^"\r\n]+"|'[^'\r\n]+'|[^\s#][^\r\n]*)\s*$/m.test(fs.readFileSync(geminiEnv, 'utf-8')); | |
| } catch { | |
| return false; | |
| } | |
| })(); |
There was a problem hiding this comment.
7 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/model-benchmark/providers/claude.ts">
<violation number="1" location="lib/model-benchmark/providers/claude.ts:74">
P3: Timeout results no longer show which limit was exceeded, making benchmark failure output less actionable when runs use different `--timeout-ms` values. Consider passing `opts.timeoutMs` into `errorResult` and retaining it in the reason.</violation>
</file>
<file name="lib/model-benchmark/providers/types.ts">
<violation number="1" location="lib/model-benchmark/providers/types.ts:6">
P2: Benchmark results no longer include the advertised token or cost comparison: these subprocess calls are not Braintrust-instrumented, so removing usage/cost from `ProviderAdapter` leaves no component able to collect them. Consider retaining normalized usage/cost telemetry in the adapter contract (with unknown values when unavailable) or explicitly changing the benchmark’s documented feature contract.</violation>
</file>
<file name="lib/model-benchmark/providers/gpt.ts">
<violation number="1" location="lib/model-benchmark/providers/gpt.ts:64">
P3: A direct run (or binary-removal race after `available()`) reports a missing `codex` executable as `unknown`, despite the adapter contract's `binary_missing` code. Classifying `ENOENT` preserves the structured failure users and callers expect.</violation>
<violation number="2" location="lib/model-benchmark/providers/gpt.ts:65">
P2: Failed Codex runs can return the full prompt while truncating the useful failure detail because `execFileSync`'s command-containing `e.message` takes precedence over `stderr`. Prefer nonempty `stderr` so auth/rate-limit/unknown errors remain actionable and avoid unnecessarily echoing prompt text.</violation>
</file>
<file name="lib/model-benchmark/providers/gemini.ts">
<violation number="1" location="lib/model-benchmark/providers/gemini.ts:42">
P1: For users with an auto-approving Gemini configuration, an arbitrary benchmark prompt can now invoke mutating tools instead of staying report-only because `-p` only selects non-interactive execution. Keeping an explicit restricted approval policy avoids inheriting user-specific execution permissions.</violation>
<violation number="2" location="lib/model-benchmark/providers/gemini.ts:70">
P3: A missing Gemini executable is reported as `unknown` rather than the contract's `binary_missing`, making JSON/ops diagnostics less actionable. Mapping `ENOENT` before the fallback preserves the provider error semantics.</violation>
</file>
<file name="lib/model-benchmark/braintrust-eval.ts">
<violation number="1" location="lib/model-benchmark/braintrust-eval.ts:134">
P2: Empty provider responses fail the Braintrust case but still appear successful in the CLI/JSON operational results because the already-pushed `CaseOps.error` remains unset. Recording the synthetic failure on that op would keep the comparison report consistent with the eval result.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| // --skip-trust lets the CLI run in disposable/non-git benchmark workdirs. | ||
| // Plain -p is non-interactive; benchmark prompts are reasoning tasks, not file | ||
| // ops, and the workdir is a throwaway temp dir. | ||
| const args = ['-p', opts.prompt, '--skip-trust']; |
There was a problem hiding this comment.
P1: For users with an auto-approving Gemini configuration, an arbitrary benchmark prompt can now invoke mutating tools instead of staying report-only because -p only selects non-interactive execution. Keeping an explicit restricted approval policy avoids inheriting user-specific execution permissions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/gemini.ts, line 42:
<comment>For users with an auto-approving Gemini configuration, an arbitrary benchmark prompt can now invoke mutating tools instead of staying report-only because `-p` only selects non-interactive execution. Keeping an explicit restricted approval policy avoids inheriting user-specific execution permissions.</comment>
<file context>
@@ -39,9 +36,10 @@ export class GeminiAdapter implements ProviderAdapter {
+ // --skip-trust lets the CLI run in disposable/non-git benchmark workdirs.
+ // Plain -p is non-interactive; benchmark prompts are reasoning tasks, not file
+ // ops, and the workdir is a throwaway temp dir.
+ const args = ['-p', opts.prompt, '--skip-trust'];
if (opts.model) args.push('--model', opts.model);
if (opts.extraArgs) args.push(...opts.extraArgs);
</file context>
Suggested change
| const args = ['-p', opts.prompt, '--skip-trust']; | |
| const args = ['-p', opts.prompt, '--approval-mode', 'plan', '--skip-trust']; |
| * The benchmark runner only talks to adapters through this interface. | ||
| * Adapters shell out to the provider's CLI and return its plain-text output. We | ||
| * deliberately do NOT parse each vendor's proprietary JSON stream for tokens or | ||
| * cost: that coupling is brittle (it breaks every time a vendor reshuffles its |
There was a problem hiding this comment.
P2: Benchmark results no longer include the advertised token or cost comparison: these subprocess calls are not Braintrust-instrumented, so removing usage/cost from ProviderAdapter leaves no component able to collect them. Consider retaining normalized usage/cost telemetry in the adapter contract (with unknown values when unavailable) or explicitly changing the benchmark’s documented feature contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/types.ts, line 6:
<comment>Benchmark results no longer include the advertised token or cost comparison: these subprocess calls are not Braintrust-instrumented, so removing usage/cost from `ProviderAdapter` leaves no component able to collect them. Consider retaining normalized usage/cost telemetry in the adapter contract (with unknown values when unavailable) or explicitly changing the benchmark’s documented feature contract.</comment>
<file context>
@@ -1,8 +1,12 @@
- * The benchmark runner only talks to adapters through this interface.
+ * Adapters shell out to the provider's CLI and return its plain-text output. We
+ * deliberately do NOT parse each vendor's proprietary JSON stream for tokens or
+ * cost: that coupling is brittle (it breaks every time a vendor reshuffles its
+ * output) and redundant — Braintrust owns scoring, and token/cost tracking
+ * belongs to whatever instruments the actual model call. The CLI's text answer
</file context>
| else if (/unauthorized|auth|login/i.test(stderr)) code = 'auth'; | ||
| else if (/rate[- ]?limit|429/i.test(stderr)) code = 'rate_limit'; | ||
| else code = 'unknown'; | ||
| const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400); |
There was a problem hiding this comment.
P2: Failed Codex runs can return the full prompt while truncating the useful failure detail because execFileSync's command-containing e.message takes precedence over stderr. Prefer nonempty stderr so auth/rate-limit/unknown errors remain actionable and avoid unnecessarily echoing prompt text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/gpt.ts, line 65:
<comment>Failed Codex runs can return the full prompt while truncating the useful failure detail because `execFileSync`'s command-containing `e.message` takes precedence over `stderr`. Prefer nonempty `stderr` so auth/rate-limit/unknown errors remain actionable and avoid unnecessarily echoing prompt text.</comment>
<file context>
@@ -47,81 +44,25 @@ export class GptAdapter implements ProviderAdapter {
+ else if (/unauthorized|auth|login/i.test(stderr)) code = 'auth';
+ else if (/rate[- ]?limit|429/i.test(stderr)) code = 'rate_limit';
+ else code = 'unknown';
+ const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400);
+ return { output: '', durationMs, modelUsed: model ?? 'gpt', error: { code, reason } };
}
</file context>
Suggested change
| const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400); | |
| const reason = code === 'timeout' ? 'exceeded timeout' : (stderr || e.message || 'unknown').slice(0, 400); |
| if (r.error) throw new Error(`${provider} failed: ${r.error.code} — ${r.error.reason}`); | ||
| // Empty output = provider never answered (silent auth/CLI failure). Fail | ||
| // loud so it can't masquerade as a 0.0 score. | ||
| if (!r.output.trim()) { |
There was a problem hiding this comment.
P2: Empty provider responses fail the Braintrust case but still appear successful in the CLI/JSON operational results because the already-pushed CaseOps.error remains unset. Recording the synthetic failure on that op would keep the comparison report consistent with the eval result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/braintrust-eval.ts, line 134:
<comment>Empty provider responses fail the Braintrust case but still appear successful in the CLI/JSON operational results because the already-pushed `CaseOps.error` remains unset. Recording the synthetic failure on that op would keep the comparison report consistent with the eval result.</comment>
<file context>
@@ -126,23 +122,17 @@ export async function runProviderBenchmark(
- throw new Error(`${provider} returned empty output with zero tokens (likely auth/CLI failure, not a real result)`);
+ // Empty output = provider never answered (silent auth/CLI failure). Fail
+ // loud so it can't masquerade as a 0.0 score.
+ if (!r.output.trim()) {
+ throw new Error(`${provider} returned empty output (likely auth/CLI failure, not a real result)`);
}
</file context>
| else if (/unauthorized|auth|login/i.test(stderr)) code = 'auth'; | ||
| else if (/rate[- ]?limit|429/i.test(stderr)) code = 'rate_limit'; | ||
| else code = 'unknown'; | ||
| const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400); |
There was a problem hiding this comment.
P3: Timeout results no longer show which limit was exceeded, making benchmark failure output less actionable when runs use different --timeout-ms values. Consider passing opts.timeoutMs into errorResult and retaining it in the reason.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/claude.ts, line 74:
<comment>Timeout results no longer show which limit was exceeded, making benchmark failure output less actionable when runs use different `--timeout-ms` values. Consider passing `opts.timeoutMs` into `errorResult` and retaining it in the reason.</comment>
<file context>
@@ -56,70 +53,25 @@ export class ClaudeAdapter implements ProviderAdapter {
+ else if (/unauthorized|auth|login/i.test(stderr)) code = 'auth';
+ else if (/rate[- ]?limit|429/i.test(stderr)) code = 'rate_limit';
+ else code = 'unknown';
+ const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400);
+ return { output: '', durationMs, modelUsed: model ?? 'claude', error: { code, reason } };
}
</file context>
| if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') code = 'timeout'; | ||
| else if (/unauthorized|auth|login/i.test(stderr)) code = 'auth'; | ||
| else if (/rate[- ]?limit|429/i.test(stderr)) code = 'rate_limit'; | ||
| else code = 'unknown'; |
There was a problem hiding this comment.
P3: A direct run (or binary-removal race after available()) reports a missing codex executable as unknown, despite the adapter contract's binary_missing code. Classifying ENOENT preserves the structured failure users and callers expect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/gpt.ts, line 64:
<comment>A direct run (or binary-removal race after `available()`) reports a missing `codex` executable as `unknown`, despite the adapter contract's `binary_missing` code. Classifying `ENOENT` preserves the structured failure users and callers expect.</comment>
<file context>
@@ -47,81 +44,25 @@ export class GptAdapter implements ProviderAdapter {
+ if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') code = 'timeout';
+ else if (/unauthorized|auth|login/i.test(stderr)) code = 'auth';
+ else if (/rate[- ]?limit|429/i.test(stderr)) code = 'rate_limit';
+ else code = 'unknown';
+ const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400);
+ return { output: '', durationMs, modelUsed: model ?? 'gpt', error: { code, reason } };
</file context>
Suggested change
| else code = 'unknown'; | |
| else if (e.code === 'ENOENT') code = 'binary_missing'; | |
| else code = 'unknown'; |
| if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') code = 'timeout'; | ||
| else if (/unauthorized|auth|login|api key/i.test(stderr)) code = 'auth'; | ||
| else if (/rate[- ]?limit|429|quota/i.test(stderr)) code = 'rate_limit'; | ||
| else code = 'unknown'; |
There was a problem hiding this comment.
P3: A missing Gemini executable is reported as unknown rather than the contract's binary_missing, making JSON/ops diagnostics less actionable. Mapping ENOENT before the fallback preserves the provider error semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/model-benchmark/providers/gemini.ts, line 70:
<comment>A missing Gemini executable is reported as `unknown` rather than the contract's `binary_missing`, making JSON/ops diagnostics less actionable. Mapping `ENOENT` before the fallback preserves the provider error semantics.</comment>
<file context>
@@ -52,83 +50,25 @@ export class GeminiAdapter implements ProviderAdapter {
+ if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') code = 'timeout';
+ else if (/unauthorized|auth|login|api key/i.test(stderr)) code = 'auth';
+ else if (/rate[- ]?limit|429|quota/i.test(stderr)) code = 'rate_limit';
+ else code = 'unknown';
+ const reason = code === 'timeout' ? 'exceeded timeout' : (e.message ?? stderr ?? 'unknown').slice(0, 400);
+ return { output: '', durationMs, modelUsed: model ?? 'gemini', error: { code, reason } };
</file context>
Suggested change
| else code = 'unknown'; | |
| else if (e.code === 'ENOENT') code = 'binary_missing'; | |
| else code = 'unknown'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the in-house model-benchmark scoring engine with Braintrust, running fully local by default. GStack keeps only the unavoidable piece — the CLI-agent adapters that shell out to
claude/codex/gemini— and hands scoring, experiments, comparison, and reporting to Braintrust.Why not a zero-code drop-in: off-the-shelf eval tools score a model completion, but this benchmark drives a CLI agent (tool use, report-only mode, mutation boundaries). Braintrust's
Eval()closes the gap by taking the adapter as itstask, so the adapters stay and everything above them (therunner.tsaggregation/formatters and thejudge.tsLLM judge) is deleted.What changed
lib/model-benchmark/braintrust-eval.ts(new) —runProviderBenchmark()wraps each adapter as a BraintrustEvaltask. A deterministic required-terms scorer replaces the old in-house scoring; an optionalautoevalsClosedQAjudge replacesjudge.ts.bin/gstack-model-benchmark— rewired to Braintrust. Prints a comparison table (score + latency/tokens/cost). Keeps--dry-run,--models,--prompt,--judge,--output.evals/model-benchmark/(new) — synthetic corpus + abun run/ CI entrypoint.lib/model-benchmark/runner.ts,judge.ts, and their test-helper re-export shims.GEMINI_API_KEY/~/.gemini/.env, parses the currentcontent/statsstream schema, runs report-only plan mode with--skip-trust.CLAUDE.mdloosened so optional, off-by-default, consent-gated eval backends are allowed (previously only Context.dev was).Privacy / consent
Local by default: with no
BRAINTRUST_API_KEY, the eval runs withnoSendLogsand uploads nothing. Setting the key is the opt-in to the cloud dashboard. An empty output with zero tokens is thrown, so a silent auth/CLI failure can never masquerade as a0.0score.Verification
Benchmark unit + boundary tests: 27 pass, 0 fail (
bun test test/benchmark-*.test.ts test/gemini-benchmark-adapter.test.ts).Live end-to-end through the new CLI, local, nothing uploaded:
Known gaps
--judge(autoevalsClosedQA) needsOPENAI_API_KEY; without it, it warns and runs the deterministic scorer only.No VERSION bump or CHANGELOG entry in this PR by request.
🤖 Generated with Claude Code
Summary by cubic
Switched the benchmark to
braintrustEval and moved adapters to plain‑text output, removing token/cost parsing and pricing. Runs local by default; optionalautoevalsjudge remains.New Features
runProviderBenchmark()wraps each provider as the Eval task; deterministic required-terms scorer by default;--judgeenablesautoevalsClosedQA (needsOPENAI_API_KEY).--prompt, falls back to a built-in corpus, outputs table or JSON, and shows score and latency. Local-only unlessBRAINTRUST_API_KEYis set; use bun. Addedevals/model-benchmark/corpus and abun run/CI entrypoint.Refactors
pricing.ts. Dropped Tokens/Cost/ToolCall metrics from results and the comparison table.GEMINI_API_KEYand~/.gemini/.env, run report‑only plan mode with--skip-trust, and fail loudly on zero output.Written for commit 7f4574e. Summary will update on new commits.