From 11337167ba378caf6e0a30d6e60020c6b6ebb587 Mon Sep 17 00:00:00 2001
From: Mattias Petersson <mattias.petersson@gmail.com>
Date: Thu, 16 Jul 2026 09:59:16 +0200
Subject: [PATCH] fix benchmark provider readiness and accounting

---
 test/benchmark-provider-adapters.test.ts | 47 ++++++++++++++++++++++++
 test/benchmark-runner.test.ts            |  9 ++++-
 test/helpers/benchmark-runner.ts         | 20 ++++++----
 test/helpers/providers/claude.ts         | 16 +++++---
 test/helpers/providers/errors.ts         | 20 ++++++++++
 test/helpers/providers/gemini.ts         | 46 +++++++++++++++--------
 test/helpers/providers/gpt.ts            | 33 ++++++++++++-----
 test/helpers/providers/types.ts          |  1 +
 8 files changed, 154 insertions(+), 38 deletions(-)
 create mode 100644 test/benchmark-provider-adapters.test.ts
 create mode 100644 test/helpers/providers/errors.ts

diff --git a/test/benchmark-provider-adapters.test.ts b/test/benchmark-provider-adapters.test.ts
new file mode 100644
index 000000000..8e5f2738a
--- /dev/null
+++ b/test/benchmark-provider-adapters.test.ts
@@ -0,0 +1,47 @@
+import { describe, expect, test } from 'bun:test';
+import * as fs from 'fs';
+import * as path from 'path';
+import { isCapacityError, providerErrorDetail } from './helpers/providers/errors';
+import { GptAdapter } from './helpers/providers/gpt';
+import { geminiAuthReadiness } from './helpers/providers/gemini';
+
+describe('benchmark provider hardening', () => {
+  test('does not treat stored Gemini OAuth as non-interactive readiness', () => {
+    const oauthOnly = geminiAuthReadiness(true, false);
+    expect(oauthOnly.ok).toBe(false);
+    expect(oauthOnly.reason).toContain('UNSUPPORTED_CLIENT');
+    expect(geminiAuthReadiness(false, true)).toEqual({ ok: true });
+    expect(geminiAuthReadiness(true, true)).toEqual({ ok: true });
+  });
+
+  test('recognizes model-capacity errors without confusing auth or quota failures', () => {
+    expect(isCapacityError('Selected model is at capacity. Please try a different model')).toBe(true);
+    expect(isCapacityError('The provider is overloaded right now')).toBe(true);
+    expect(isCapacityError('Model temporarily unavailable due to high demand')).toBe(true);
+    expect(isCapacityError('Error 401: login required')).toBe(false);
+    expect(isCapacityError('429 quota exceeded')).toBe(false);
+  });
+
+  test('combines stderr and message without duplicating subprocess output', () => {
+    const stderr = 'Selected model is at capacity.';
+    expect(providerErrorDetail({ stderr: Buffer.from(stderr), message: `Command failed\n${stderr}` }))
+      .toBe(`Command failed\n${stderr}`);
+    expect(providerErrorDetail({ stderr: Buffer.from('stderr only'), message: 'message only' }))
+      .toBe('stderr only\nmessage only');
+  });
+
+  test('Codex ignores inherited stdin and separates cached input tokens', () => {
+    const source = fs.readFileSync(path.join(import.meta.dir, 'helpers/providers/gpt.ts'), 'utf8');
+    expect(source).toContain("stdio: ['ignore', 'pipe', 'pipe']");
+
+    const gpt = new GptAdapter();
+    const parseJsonl = (gpt as unknown as {
+      parseJsonl(raw: string): { tokens: { input: number; output: number; cached?: number } };
+    }).parseJsonl.bind(gpt);
+    const parsed = parseJsonl([
+      JSON.stringify({ type: 'turn.completed', usage: { input_tokens: 21_155, cached_input_tokens: 9_984, output_tokens: 5 } }),
+      JSON.stringify({ type: 'turn.completed', usage: { input_tokens: 200, cached_input_tokens: 150, output_tokens: 10 } }),
+    ].join('\n'));
+    expect(parsed.tokens).toEqual({ input: 11_221, cached: 10_134, output: 15 });
+  });
+});
diff --git a/test/benchmark-runner.test.ts b/test/benchmark-runner.test.ts
index ecd503ea8..6661d89b1 100644
--- a/test/benchmark-runner.test.ts
+++ b/test/benchmark-runner.test.ts
@@ -37,6 +37,11 @@ test('estimateCostUsd applies cached input discount alongside uncached input', (
   expect(cost2).toBeCloseTo(8.25, 2);
 });
 
+test('estimateCostUsd prices normalized Codex cache reads at the discount', () => {
+  const cost = estimateCostUsd({ input: 11_171, cached: 9_984, output: 5 }, 'gpt-5.4');
+  expect(cost).toBeCloseTo(0.030474, 6);
+});
+
 test('PRICING table covers the key model families', () => {
   expect(PRICING['claude-opus-4-7']).toBeDefined();
   expect(PRICING['claude-sonnet-4-6']).toBeDefined();
@@ -72,7 +77,7 @@ test('formatTable handles a report with mixed success/error/unavailable entries'
         available: true,
         result: {
           output: 'ok',
-          tokens: { input: 100, output: 200 },
+          tokens: { input: 100, cached: 50, output: 200 },
           durationMs: 800,
           toolCalls: 3,
           modelUsed: 'claude-opus-4-7',
@@ -107,6 +112,8 @@ test('formatTable handles a report with mixed success/error/unavailable entries'
   expect(table).toContain('ERROR auth');
   expect(table).toContain('unavailable');
   expect(table).toContain('9.2/10');
+  expect(table).toContain('100+50→200');
+  expect(formatMarkdown(report)).toContain('100+50→200');
 });
 
 test('formatJson produces parseable JSON', () => {
diff --git a/test/helpers/benchmark-runner.ts b/test/helpers/benchmark-runner.ts
index cbef4107b..60bbf5b78 100644
--- a/test/helpers/benchmark-runner.ts
+++ b/test/helpers/benchmark-runner.ts
@@ -99,21 +99,21 @@ export async function runBenchmark(input: BenchmarkInput): Promise<BenchmarkRepo
 }
 
 export function formatTable(report: BenchmarkReport): string {
-  const header = `Model                Latency   In→Out Tokens       Cost       Quality   Tool Calls   Notes`;
+  const header = `Model                Latency   Input (new+cached)→Out   Cost       Quality   Tool Calls   Notes`;
   const sep = '-'.repeat(header.length);
   const rows: string[] = [header, sep];
   for (const e of report.entries) {
     if (!e.available) {
-      rows.push(`${pad(e.provider, 20)} ${pad('-', 9)} ${pad('-', 20)} ${pad('-', 10)} ${pad('-', 9)} ${pad('-', 12)} unavailable: ${e.unavailable_reason ?? 'unknown'}`);
+      rows.push(`${pad(e.provider, 20)} ${pad('-', 9)} ${pad('-', 24)} ${pad('-', 10)} ${pad('-', 9)} ${pad('-', 12)} unavailable: ${e.unavailable_reason ?? 'unknown'}`);
       continue;
     }
     const r = e.result!;
     if (r.error) {
-      rows.push(`${pad(r.modelUsed, 20)} ${pad(msToStr(r.durationMs), 9)} ${pad(`${r.tokens.input}→${r.tokens.output}`, 20)} ${pad(fmtCost(e.costUsd), 10)} ${pad('-', 9)} ${pad(String(r.toolCalls), 12)} ERROR ${r.error.code}: ${r.error.reason.slice(0, 40)}`);
+      rows.push(`${pad(r.modelUsed, 20)} ${pad(msToStr(r.durationMs), 9)} ${pad(fmtTokens(r.tokens), 24)} ${pad(fmtCost(e.costUsd), 10)} ${pad('-', 9)} ${pad(String(r.toolCalls), 12)} ERROR ${r.error.code}: ${r.error.reason.slice(0, 40)}`);
       continue;
     }
     const quality = e.qualityScore !== undefined ? `${e.qualityScore.toFixed(1)}/10` : '-';
-    rows.push(`${pad(r.modelUsed, 20)} ${pad(msToStr(r.durationMs), 9)} ${pad(`${r.tokens.input}→${r.tokens.output}`, 20)} ${pad(fmtCost(e.costUsd), 10)} ${pad(quality, 9)} ${pad(String(r.toolCalls), 12)}`);
+    rows.push(`${pad(r.modelUsed, 20)} ${pad(msToStr(r.durationMs), 9)} ${pad(fmtTokens(r.tokens), 24)} ${pad(fmtCost(e.costUsd), 10)} ${pad(quality, 9)} ${pad(String(r.toolCalls), 12)}`);
   }
   return rows.join('\n');
 }
@@ -130,7 +130,7 @@ export function formatMarkdown(report: BenchmarkReport): string {
     `**Workdir:** \`${report.workdir}\``,
     `**Total duration:** ${msToStr(report.durationMs)}`,
     '',
-    '| Model | Latency | Tokens (in→out) | Cost | Quality | Tools | Notes |',
+    '| Model | Latency | Tokens (new+cached→out) | Cost | Quality | Tools | Notes |',
     '|-------|---------|-----------------|------|---------|-------|-------|',
   ];
   for (const e of report.entries) {
@@ -140,11 +140,11 @@ export function formatMarkdown(report: BenchmarkReport): string {
     }
     const r = e.result!;
     if (r.error) {
-      lines.push(`| ${r.modelUsed} | ${msToStr(r.durationMs)} | ${r.tokens.input}→${r.tokens.output} | ${fmtCost(e.costUsd)} | - | ${r.toolCalls} | ERROR ${r.error.code}: ${r.error.reason.slice(0, 80)} |`);
+      lines.push(`| ${r.modelUsed} | ${msToStr(r.durationMs)} | ${fmtTokens(r.tokens)} | ${fmtCost(e.costUsd)} | - | ${r.toolCalls} | ERROR ${r.error.code}: ${r.error.reason.slice(0, 80)} |`);
       continue;
     }
     const quality = e.qualityScore !== undefined ? `${e.qualityScore.toFixed(1)}/10` : '-';
-    lines.push(`| ${r.modelUsed} | ${msToStr(r.durationMs)} | ${r.tokens.input}→${r.tokens.output} | ${fmtCost(e.costUsd)} | ${quality} | ${r.toolCalls} | |`);
+    lines.push(`| ${r.modelUsed} | ${msToStr(r.durationMs)} | ${fmtTokens(r.tokens)} | ${fmtCost(e.costUsd)} | ${quality} | ${r.toolCalls} | |`);
   }
   return lines.join('\n');
 }
@@ -158,6 +158,12 @@ function msToStr(ms: number): string {
   return `${(ms / 1000).toFixed(1)}s`;
 }
 
+function fmtTokens(tokens: { input: number; output: number; cached?: number }): string {
+  return tokens.cached === undefined
+    ? `${tokens.input}→${tokens.output}`
+    : `${tokens.input}+${tokens.cached}→${tokens.output}`;
+}
+
 function fmtCost(usd?: number): string {
   if (usd === undefined) return '-';
   if (usd < 0.01) return `$${usd.toFixed(4)}`;
diff --git a/test/helpers/providers/claude.ts b/test/helpers/providers/claude.ts
index ce77767c2..07eb7e6ca 100644
--- a/test/helpers/providers/claude.ts
+++ b/test/helpers/providers/claude.ts
@@ -5,6 +5,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
 import { resolveClaudeCommand } from '../../../browse/src/claude-bin';
+import { isCapacityError, providerErrorDetail } from './errors';
 
 /**
  * Claude adapter — wraps the `claude` CLI via claude -p.
@@ -67,17 +68,20 @@ export class ClaudeAdapter implements ProviderAdapter {
     } catch (err: unknown) {
       const durationMs = Date.now() - start;
       const e = err as { code?: string; stderr?: Buffer; signal?: string; message?: string };
-      const stderr = e.stderr?.toString() ?? '';
+      const detail = providerErrorDetail(e);
       if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') {
         return this.emptyResult(durationMs, { code: 'timeout', reason: `exceeded ${opts.timeoutMs}ms` }, opts.model);
       }
-      if (/unauthorized|auth|login/i.test(stderr)) {
-        return this.emptyResult(durationMs, { code: 'auth', reason: stderr.slice(0, 400) }, opts.model);
+      if (/unauthorized|auth|login/i.test(detail)) {
+        return this.emptyResult(durationMs, { code: 'auth', reason: detail.slice(0, 400) }, opts.model);
       }
-      if (/rate[- ]?limit|429/i.test(stderr)) {
-        return this.emptyResult(durationMs, { code: 'rate_limit', reason: stderr.slice(0, 400) }, opts.model);
+      if (/rate[- ]?limit|429/i.test(detail)) {
+        return this.emptyResult(durationMs, { code: 'rate_limit', reason: detail.slice(0, 400) }, opts.model);
       }
-      return this.emptyResult(durationMs, { code: 'unknown', reason: (e.message ?? stderr ?? 'unknown').slice(0, 400) }, opts.model);
+      if (isCapacityError(detail)) {
+        return this.emptyResult(durationMs, { code: 'capacity', reason: detail.slice(0, 400) }, opts.model);
+      }
+      return this.emptyResult(durationMs, { code: 'unknown', reason: (detail || 'unknown').slice(0, 400) }, opts.model);
     }
   }
 
diff --git a/test/helpers/providers/errors.ts b/test/helpers/providers/errors.ts
new file mode 100644
index 000000000..29b6a9d23
--- /dev/null
+++ b/test/helpers/providers/errors.ts
@@ -0,0 +1,20 @@
+export interface ProviderProcessError {
+  stderr?: Buffer | string;
+  message?: string;
+}
+
+/** Combine subprocess diagnostics without duplicating identical text. */
+export function providerErrorDetail(error: ProviderProcessError): string {
+  const stderr = typeof error.stderr === 'string'
+    ? error.stderr
+    : error.stderr?.toString() ?? '';
+  const message = error.message ?? '';
+  return stderr && message.includes(stderr)
+    ? message
+    : [stderr, message].filter(Boolean).join('\n');
+}
+
+/** Provider/model saturation is transient and distinct from auth or quota. */
+export function isCapacityError(detail: string): boolean {
+  return /(?:selected\s+)?model\s+is\s+at\s+capacity|\bat\s+capacity\b|\bmodel\s+capacity\b|\boverloaded\b|temporarily unavailable due to (?:high )?(?:load|demand)/i.test(detail);
+}
diff --git a/test/helpers/providers/gemini.ts b/test/helpers/providers/gemini.ts
index b53d725d1..046dd34c1 100644
--- a/test/helpers/providers/gemini.ts
+++ b/test/helpers/providers/gemini.ts
@@ -4,6 +4,7 @@ import { execFileSync, spawnSync } from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
+import { isCapacityError, providerErrorDetail } from './errors';
 
 export type GeminiStreamParse = {
   output: string;
@@ -93,6 +94,25 @@ export function resultFromGeminiStream(
   };
 }
 
+/** Decide whether local Gemini auth can support a non-interactive benchmark. */
+export function geminiAuthReadiness(hasOauth: boolean, hasKey: boolean): AvailabilityCheck {
+  if (hasKey) {
+    return { ok: true };
+  }
+  if (hasOauth) {
+    return {
+      ok: false,
+      reason:
+        'Stored Gemini OAuth is not a reliable readiness signal and may fail with UNSUPPORTED_CLIENT. To use an AI Studio key, export GEMINI_API_KEY (or GOOGLE_API_KEY).',
+    };
+  }
+  return {
+    ok: false,
+    reason:
+      'No Gemini auth found. Get an AI Studio key from https://aistudio.google.com/app/apikey, then export GEMINI_API_KEY (or GOOGLE_API_KEY) — personal OAuth free-tier is no longer supported by gemini CLI.',
+  };
+}
+
 /**
  * Gemini adapter — wraps the `gemini` CLI.
  *
@@ -120,17 +140,10 @@ export class GeminiAdapter implements ProviderAdapter {
     const legacyCfgDir = path.join(os.homedir(), '.config', 'gemini');
     const newCfgDir = path.join(os.homedir(), '.gemini');
     const newOauth = path.join(newCfgDir, 'oauth_creds.json');
-    const hasCfg = fs.existsSync(legacyCfgDir) || fs.existsSync(newOauth);
+    const hasOauth = fs.existsSync(legacyCfgDir) || fs.existsSync(newOauth);
     // CLI accepts either name; Google AI Studio keys are usually GEMINI_API_KEY.
     const hasKey = !!(process.env.GOOGLE_API_KEY || process.env.GEMINI_API_KEY);
-    if (!hasCfg && !hasKey) {
-      return {
-        ok: false,
-        reason:
-          'No Gemini auth found. Export GEMINI_API_KEY (or GOOGLE_API_KEY) from https://aistudio.google.com/app/apikey — personal OAuth free-tier is no longer supported by gemini CLI.',
-      };
-    }
-    return { ok: true };
+    return geminiAuthReadiness(hasOauth, hasKey);
   }
 
   async run(opts: RunOpts): Promise<RunResult> {
@@ -161,17 +174,20 @@ export class GeminiAdapter implements ProviderAdapter {
     } catch (err: unknown) {
       const durationMs = Date.now() - start;
       const e = err as { code?: string; stderr?: Buffer; signal?: string; message?: string };
-      const stderr = e.stderr?.toString() ?? '';
+      const detail = providerErrorDetail(e);
       if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') {
         return this.emptyResult(durationMs, { code: 'timeout', reason: `exceeded ${opts.timeoutMs}ms` }, opts.model);
       }
-      if (/unauthorized|auth|login|api key|ineligibletier|no longer supported/i.test(stderr)) {
-        return this.emptyResult(durationMs, { code: 'auth', reason: stderr.slice(0, 400) }, opts.model);
+      if (/unauthorized|auth|login|api key|ineligibletier|no longer supported/i.test(detail)) {
+        return this.emptyResult(durationMs, { code: 'auth', reason: detail.slice(0, 400) }, opts.model);
       }
-      if (/rate[- ]?limit|429|quota/i.test(stderr)) {
-        return this.emptyResult(durationMs, { code: 'rate_limit', reason: stderr.slice(0, 400) }, opts.model);
+      if (/rate[- ]?limit|429|quota/i.test(detail)) {
+        return this.emptyResult(durationMs, { code: 'rate_limit', reason: detail.slice(0, 400) }, opts.model);
       }
-      return this.emptyResult(durationMs, { code: 'unknown', reason: (e.message ?? stderr ?? 'unknown').slice(0, 400) }, opts.model);
+      if (isCapacityError(detail)) {
+        return this.emptyResult(durationMs, { code: 'capacity', reason: detail.slice(0, 400) }, opts.model);
+      }
+      return this.emptyResult(durationMs, { code: 'unknown', reason: (detail || 'unknown').slice(0, 400) }, opts.model);
     }
   }
 
diff --git a/test/helpers/providers/gpt.ts b/test/helpers/providers/gpt.ts
index 07757dc2f..23e8bd6de 100644
--- a/test/helpers/providers/gpt.ts
+++ b/test/helpers/providers/gpt.ts
@@ -4,6 +4,7 @@ import { execFileSync, spawnSync } from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as os from 'os';
+import { isCapacityError, providerErrorDetail } from './errors';
 
 /**
  * GPT adapter — wraps the OpenAI `codex` CLI (codex exec with --json output).
@@ -46,6 +47,10 @@ export class GptAdapter implements ProviderAdapter {
         timeout: opts.timeoutMs,
         encoding: 'utf-8',
         maxBuffer: 32 * 1024 * 1024,
+        // A benchmark may itself run under an interactive agent. Never let a
+        // nested Codex process inherit that agent's stdin and wait for more
+        // input instead of executing the prompt already present in `args`.
+        stdio: ['ignore', 'pipe', 'pipe'],
       });
       const parsed = this.parseJsonl(out);
       return {
@@ -58,17 +63,20 @@ export class GptAdapter implements ProviderAdapter {
     } catch (err: unknown) {
       const durationMs = Date.now() - start;
       const e = err as { code?: string; stderr?: Buffer; signal?: string; message?: string };
-      const stderr = e.stderr?.toString() ?? '';
+      const detail = providerErrorDetail(e);
       if (e.signal === 'SIGTERM' || e.code === 'ETIMEDOUT') {
         return this.emptyResult(durationMs, { code: 'timeout', reason: `exceeded ${opts.timeoutMs}ms` }, opts.model);
       }
-      if (/unauthorized|auth|login/i.test(stderr)) {
-        return this.emptyResult(durationMs, { code: 'auth', reason: stderr.slice(0, 400) }, opts.model);
+      if (/unauthorized|auth|login/i.test(detail)) {
+        return this.emptyResult(durationMs, { code: 'auth', reason: detail.slice(0, 400) }, opts.model);
       }
-      if (/rate[- ]?limit|429/i.test(stderr)) {
-        return this.emptyResult(durationMs, { code: 'rate_limit', reason: stderr.slice(0, 400) }, opts.model);
+      if (/rate[- ]?limit|429/i.test(detail)) {
+        return this.emptyResult(durationMs, { code: 'rate_limit', reason: detail.slice(0, 400) }, opts.model);
       }
-      return this.emptyResult(durationMs, { code: 'unknown', reason: (e.message ?? stderr ?? 'unknown').slice(0, 400) }, opts.model);
+      if (isCapacityError(detail)) {
+        return this.emptyResult(durationMs, { code: 'capacity', reason: detail.slice(0, 400) }, opts.model);
+      }
+      return this.emptyResult(durationMs, { code: 'unknown', reason: (detail || 'unknown').slice(0, 400) }, opts.model);
     }
   }
 
@@ -84,9 +92,10 @@ export class GptAdapter implements ProviderAdapter {
    *   - turn.completed → usage.input_tokens, usage.output_tokens
    *   - thread.started → session id (not used here)
    */
-  private parseJsonl(raw: string): { output: string; tokens: { input: number; output: number }; toolCalls: number; modelUsed?: string } {
+  private parseJsonl(raw: string): { output: string; tokens: { input: number; output: number; cached?: number }; toolCalls: number; modelUsed?: string } {
     let output = '';
     let input = 0;
+    let cached = 0;
     let out = 0;
     let toolCalls = 0;
     let modelUsed: string | undefined;
@@ -103,7 +112,13 @@ export class GptAdapter implements ProviderAdapter {
           }
         } else if (obj.type === 'turn.completed') {
           const u = obj.usage ?? {};
-          input += u.input_tokens ?? 0;
+          // Codex reports cached_input_tokens as a subset of input_tokens.
+          // Normalize to the benchmark contract: input is uncached-only and
+          // cached is a disjoint bucket priced at the cache-read rate.
+          const turnInput = u.input_tokens ?? 0;
+          const turnCached = u.cached_input_tokens ?? 0;
+          input += Math.max(0, turnInput - turnCached);
+          cached += turnCached;
           out += u.output_tokens ?? 0;
           if (obj.model) modelUsed = obj.model;
         }
@@ -111,7 +126,7 @@ export class GptAdapter implements ProviderAdapter {
         // skip malformed lines — codex stderr can leak in
       }
     }
-    return { output, tokens: { input, output: out }, toolCalls, modelUsed };
+    return { output, tokens: { input, cached, output: out }, toolCalls, modelUsed };
   }
 
   private emptyResult(durationMs: number, error: RunResult['error'], model?: string): RunResult {
diff --git a/test/helpers/providers/types.ts b/test/helpers/providers/types.ts
index 1680d0ceb..ddaa01f09 100644
--- a/test/helpers/providers/types.ts
+++ b/test/helpers/providers/types.ts
@@ -31,6 +31,7 @@ export type RunError =
   | 'auth'       // Credentials missing or invalid.
   | 'timeout'    // Exceeded timeoutMs.
   | 'rate_limit' // Provider rate-limited us; backoff exceeded.
+  | 'capacity'   // Selected model/provider is temporarily saturated.
   | 'binary_missing' // CLI not found on PATH.
   | 'unknown';   // Catch-all with reason populated.