From 9b12d43764c3b469b0ba3b8bdffe9dbb568a0787 Mon Sep 17 00:00:00 2001 From: akumrazor Date: Tue, 28 Jul 2026 14:01:37 -0600 Subject: [PATCH] fix(eval): make the harness runnable on Windows and price runs correctly The L1 harness could not run on Windows at all, and the cost preflight was wrong twice over on any model outside the two in the pricing table. Portability: * render-bridge.mjs passed a filesystem path to dynamic import(). On POSIX a bare absolute path happens to work; on Windows `E:\...` parses as the scheme `e:` and is rejected outright. Use pathToFileURL(). * anthropic-client.mjs hardcoded ~/.claude/local/claude and never passed it to cci.py, which reads its own CCI_CLAUDE_BIN. npm/global installs land elsewhere, so the harness failed with a confusing "exited 1" rather than a missing-binary error. Resolve env -> legacy path -> PATH, and pass it down. * cci.py drives the interactive TUI through a pty, which pexpect only provides on POSIX -- pexpect.spawn does not exist on win32, so the shim can never run there. Added a headless `claude -p` transport, automatic on win32 and forcable anywhere with CCI_HEADLESS=1. It also returns a real server usage block the TUI panels do not, so accounting improves. Cost estimator: * No entry for claude-opus-5 / claude-fable-5, and the lookup silently fell back to another model's rates behind a "$?/Mtok" label -- a confidently wrong dollar figure. Added both, plus derived cache read/write rates, and made the unknown-model path say so instead of guessing. * printCostEstimate assumed 2 calls per block while L1 runs one per variant, understating the bill 5.5x at the current 11. It now takes the real count. eval-l1-ocr.mjs was scaling externally to compensate; that is removed so the two do not double-apply. * The variant names moved to eval/lib/variant-names.mjs so the estimator can count them without importing (and thereby running) the eval. eval-l1-ocr.mjs asserts its VARIANTS array matches at startup, so a variant added in one place and not the other fails loudly instead of drifting back into the bug this fixes. run-eval.mjs: the combined report read baselineAgg/reflowAgg, fields that disappeared when L1 became an N-variant sweep, so step 3 crashed even on a dry run. Rebuilt over perVariant. Co-Authored-By: Claude Opus 5 (1M context) --- eval/eval-l1-ocr.mjs | 23 +++++++++++--- eval/lib/anthropic-client.mjs | 60 +++++++++++++++++++++++++++++++---- eval/lib/cost.mjs | 55 ++++++++++++++++++++++++++++++-- eval/lib/render-bridge.mjs | 13 +++++--- eval/lib/variant-names.mjs | 28 ++++++++++++++++ eval/run-eval.mjs | 46 +++++++++++++++++++-------- 6 files changed, 194 insertions(+), 31 deletions(-) create mode 100644 eval/lib/variant-names.mjs diff --git a/eval/eval-l1-ocr.mjs b/eval/eval-l1-ocr.mjs index 7054be88..85cb1204 100644 --- a/eval/eval-l1-ocr.mjs +++ b/eval/eval-l1-ocr.mjs @@ -258,15 +258,30 @@ if (MAX_BLOCKS > 0) blocks = blocks.slice(0, MAX_BLOCKS); console.log(`[L1] Loaded ${blocks.length} text blocks from corpus`); console.log(`[L1] Variants: ${VARIANTS.map(v => v.name).join(', ')}`); +// Fail loudly if the shared name list (used by run-eval.mjs to price the run) +// has drifted from the variants actually defined here. +{ + const { L1_VARIANT_NAMES } = await import('./lib/variant-names.mjs'); + const declared = VARIANTS_FILTER ? null : VARIANTS.map(v => v.name).join(','); + if (declared !== null && declared !== L1_VARIANT_NAMES.join(',')) { + console.error( + `[L1] variant list drift: eval/lib/variant-names.mjs is out of sync. +` + + ` here: ${declared} +` + + ` there: ${L1_VARIANT_NAMES.join(',')}`, + ); + process.exit(1); + } +} + // --------------------------------------------------------------------------- // Cost estimate gate // --------------------------------------------------------------------------- -// printCostEstimate models the legacy 2-call (baseline + reflow) shape. Actual -// spend scales by the variant count, so adjust the headline figure. +// printCostEstimate scales by the variant count it is handed. const corpus = { l1Blocks: blocks, l2Sessions: [] }; -const baseUsd = printCostEstimate(corpus, MODEL); -const totalUsd = baseUsd * (VARIANTS.length / 2); +const totalUsd = printCostEstimate(corpus, MODEL, VARIANTS.length); console.log(`[L1] ${VARIANTS.length} variants/block → estimated ~$${totalUsd.toFixed(4)} USD\n`); if (!DRY_RUN && !CONFIRMED) { diff --git a/eval/lib/anthropic-client.mjs b/eval/lib/anthropic-client.mjs index 0735c0a6..21f748c9 100644 --- a/eval/lib/anthropic-client.mjs +++ b/eval/lib/anthropic-client.mjs @@ -33,12 +33,34 @@ import { spawn } from 'node:child_process'; import { writeFileSync, unlinkSync, existsSync } from 'node:fs'; import { tmpdir, homedir } from 'node:os'; -import { join, dirname } from 'node:path'; +import { join, dirname, delimiter } from 'node:path'; import { fileURLToPath } from 'node:url'; import { randomUUID } from 'node:crypto'; // Real claude binary — NOT the shell alias, which injects the proxy base URL. -const CLAUDE_BIN = join(homedir(), '.claude', 'local', 'claude'); +// The historical hardcoded path only exists for installs that used the bundled +// local installer; npm/global installs land elsewhere (e.g. ~/.local/bin/claude), +// which made the harness unrunnable with a confusing "exited 1" instead of a +// missing-binary error. Resolution order: explicit env → legacy path → PATH. +function resolveClaudeBin() { + if (process.env.CCI_CLAUDE_BIN) return process.env.CCI_CLAUDE_BIN; + + const legacy = join(homedir(), '.claude', 'local', 'claude'); + if (existsSync(legacy)) return legacy; + + const exts = process.platform === 'win32' ? ['.cmd', '.exe', '.ps1', ''] : ['']; + for (const dir of (process.env.PATH ?? '').split(delimiter)) { + if (!dir) continue; + for (const ext of exts) { + const candidate = join(dir, `claude${ext}`); + if (existsSync(candidate)) return candidate; + } + } + throw new Error( + 'claude binary not found. Set CCI_CLAUDE_BIN to its full path ' + + `(looked in ${legacy} and on PATH).`, + ); +} // Interactive shim: drives the real TUI (Max auth) instead of headless `claude -p`. const CCI_PY = join(dirname(fileURLToPath(import.meta.url)), 'cci.py'); @@ -132,7 +154,9 @@ async function callClaudeCli(body, model) { // Child env: strip the proxy override so the CLI hits the real API directly // with the subscription OAuth token. Tall buffer so long transcriptions are // not truncated by the visible screen height. - const env = { ...process.env, CCI_ROWS: process.env.CCI_ROWS || '1500' }; + // cci.py reads CCI_CLAUDE_BIN; without this it falls back to the legacy path + // and fails on any install that did not use the bundled local installer. + const env = { ...process.env, CCI_ROWS: process.env.CCI_ROWS || '1500', CCI_CLAUDE_BIN: resolveClaudeBin() }; delete env.ANTHROPIC_BASE_URL; const args = [ @@ -142,10 +166,31 @@ async function callClaudeCli(body, model) { ]; if (imageCount > 0) args.push('--allowedTools', 'Read'); + // Transport choice. cci.py drives the interactive TUI through a pty, which + // pexpect only provides on POSIX — on Windows `pexpect.spawn` does not exist at + // all, so the shim can never run there. Headless `claude -p` authenticates + // against the same subscription and accepts the same --model/--allowedTools, so + // it is used automatically on win32 (and can be forced anywhere with + // CCI_HEADLESS=1). The trade-off is that -p reports usage per call while the + // TUI panels do not, so the headless path actually yields BETTER accounting. + const headless = process.env.CCI_HEADLESS === '1' || process.platform === 'win32'; + + const bin = resolveClaudeBin(); + const cmd = headless ? bin : PYTHON; + const cmdArgs = headless + ? ['-p', '--model', model, '--output-format', 'json', + ...(imageCount > 0 ? ['--allowedTools', 'Read'] : [])] + : args; + let stdout = '', stderr = ''; try { await new Promise((resolveP, rejectP) => { - const child = spawn(PYTHON, args, { env }); + // shell:true on Windows: the resolved binary is claude.cmd, which CreateProcess + // refuses to execute directly since the CVE-2024-27980 fix. + const child = spawn(cmd, cmdArgs, { + env, + shell: headless && process.platform === 'win32', + }); child.stdout.on('data', d => { stdout += d; }); child.stderr.on('data', d => { stderr += d; }); child.on('error', rejectP); @@ -182,8 +227,11 @@ async function callClaudeCli(body, model) { // /context estimate; total_cost_usd is the /cost server total. output_tokens // is not separately reported by the interactive panels. usage: { - input_tokens: parsed.context_tokens ?? 0, - output_tokens: 0, + // Headless -p returns a real server usage block; the interactive shim has + // only the /context estimate, hence the fallback chain rather than a plain read. + input_tokens: parsed.usage?.input_tokens ?? parsed.context_tokens ?? 0, + output_tokens: parsed.usage?.output_tokens ?? 0, + cache_read_input_tokens: parsed.usage?.cache_read_input_tokens ?? 0, }, total_cost_usd: parsed.total_cost_usd ?? null, }; diff --git a/eval/lib/cost.mjs b/eval/lib/cost.mjs index f7a34643..ab3812b7 100644 --- a/eval/lib/cost.mjs +++ b/eval/lib/cost.mjs @@ -14,6 +14,16 @@ // These are approximate public rates; update if pricing changes. // --------------------------------------------------------------------------- export const MODELS = { + 'claude-opus-5': { + inputPerMtok: 5.00, + outputPerMtok: 25.00, + imageTileTokens: 4761, + }, + 'claude-fable-5': { + inputPerMtok: 10.00, + outputPerMtok: 50.00, + imageTileTokens: 4761, + }, 'claude-sonnet-4-5': { inputPerMtok: 3.00, outputPerMtok: 15.00, @@ -26,6 +36,24 @@ export const MODELS = { }, }; +/** Cache multipliers against the model's base input rate. Reads are ~0.1×; writes + * are 1.25× at the default 5-minute TTL and 2× at 1 hour. Exposed so callers can + * price a cached prefix without hardcoding the ratios. */ +export const CACHE_MULTIPLIERS = { read: 0.1, write5m: 1.25, write1h: 2.0 }; + +/** Per-Mtok rates for a model including the derived cache rates. */ +export function modelRates(model) { + const m = MODELS[model]; + if (!m) return null; + return { + input: m.inputPerMtok, + output: m.outputPerMtok, + cacheRead: m.inputPerMtok * CACHE_MULTIPLIERS.read, + cacheWrite5m: m.inputPerMtok * CACHE_MULTIPLIERS.write5m, + cacheWrite1h: m.inputPerMtok * CACHE_MULTIPLIERS.write1h, + }; +} + /** Characters per token for Claude Code transcripts (empirical, N=354). */ const CHARS_PER_TOKEN = 1.17; @@ -171,15 +199,31 @@ export function estimateL2SessionCost( * @param {string} model * @returns {number} total USD */ -export function printCostEstimate(corpus, model = DEFAULT_MODEL) { +export function printCostEstimate(corpus, model = DEFAULT_MODEL, l1VariantCount = 2) { const { l1Blocks, l2Sessions } = corpus; let totalUsd = 0; + // L1 runs one call per VARIANT per block, not the fixed baseline+reflow pair this + // estimator was written against — with the current 11 variants that understated + // the bill 5.5×. Callers pass the real count; the per-block cost below still + // prices that pair, so scale it by variants/2. + const variants = Math.max(1, l1VariantCount | 0); + const variantScale = variants / 2; console.log('\n╔══════════════════════════════════════════════════╗'); console.log('║ COST ESTIMATE (before real run) ║'); console.log('╚══════════════════════════════════════════════════╝'); console.log(` Model: ${model}`); - console.log(` Pricing: $${MODELS[model]?.inputPerMtok ?? '?'}/Mtok input, $${MODELS[model]?.outputPerMtok ?? '?'}/Mtok output`); + const rates = modelRates(model); + if (rates) { + console.log(` Pricing: $${rates.input}/Mtok input, $${rates.output}/Mtok output`); + console.log(` $${rates.cacheRead.toFixed(2)}/Mtok cache read, ` + + `$${rates.cacheWrite5m.toFixed(2)} (5m) / $${rates.cacheWrite1h.toFixed(2)} (1h) cache write`); + } else { + // Previously this printed "$?/Mtok" and then silently priced the run at + // DEFAULT_MODEL's rates — a confidently wrong number. Say so instead. + console.log(` Pricing: UNKNOWN for ${model} — no entry in MODELS.`); + console.log(` Totals below use ${DEFAULT_MODEL} rates and are NOT this model's cost.`); + } // L1 let l1Total = { inputTokens: 0, outputTokens: 0, usd: 0, calls: 0 }; @@ -194,9 +238,14 @@ export function printCostEstimate(corpus, model = DEFAULT_MODEL) { l1Total.usd += base.usd + refl.usd; l1Total.calls += 2; } + // Scale the baseline+reflow pair up to the real variant count. + l1Total.inputTokens = Math.round(l1Total.inputTokens * variantScale); + l1Total.outputTokens = Math.round(l1Total.outputTokens * variantScale); + l1Total.usd *= variantScale; + l1Total.calls = l1Blocks.length * variants; totalUsd += l1Total.usd; - console.log(`\n ── L1 OCR Fidelity (${l1Blocks.length} blocks × 2 calls) ──`); + console.log(`\n ── L1 OCR Fidelity (${l1Blocks.length} blocks x ${variants} variants) ──`); console.log(` API calls: ${l1Total.calls}`); console.log(` Input tokens: ${l1Total.inputTokens.toLocaleString()}`); console.log(` Output tokens: ${l1Total.outputTokens.toLocaleString()}`); diff --git a/eval/lib/render-bridge.mjs b/eval/lib/render-bridge.mjs index 197244fe..19425f7f 100644 --- a/eval/lib/render-bridge.mjs +++ b/eval/lib/render-bridge.mjs @@ -16,7 +16,7 @@ import { createRequire } from 'node:module'; import { existsSync } from 'node:fs'; import { resolve, dirname } from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { fileURLToPath, pathToFileURL } from 'node:url'; const __dirname = dirname(fileURLToPath(import.meta.url)); const ROOT = resolve(__dirname, '..', '..'); @@ -34,10 +34,13 @@ if (!existsSync(RENDER_PATH)) { ); } -const renderModule = await import(RENDER_PATH); -const pngModule = await import(PNG_PATH); -const profileModule = await import(PROFILE_PATH); -const visionModule = await import(VISION_PATH); +// Dynamic import() takes a URL, not a filesystem path. On POSIX a bare absolute +// path happens to work; on Windows `E:\...` is parsed as the scheme `e:` and +// rejected outright, so the eval harness could not run there at all. +const renderModule = await import(pathToFileURL(RENDER_PATH).href); +const pngModule = await import(pathToFileURL(PNG_PATH).href); +const profileModule = await import(pathToFileURL(PROFILE_PATH).href); +const visionModule = await import(pathToFileURL(VISION_PATH).href); export const { renderTextToPngs, diff --git a/eval/lib/variant-names.mjs b/eval/lib/variant-names.mjs new file mode 100644 index 00000000..6bf90df0 --- /dev/null +++ b/eval/lib/variant-names.mjs @@ -0,0 +1,28 @@ +/** + * eval/lib/variant-names.mjs + * + * The L1 render variants, by name, in run order. + * + * Why this exists as its own module: the cost estimator needs the variant COUNT + * (L1 makes one API call per variant per block), but the variants themselves live + * in eval-l1-ocr.mjs as closures over the render bridge — importing that file to + * count them would execute the whole eval. Duplicating the list instead risks the + * two drifting apart silently, which is exactly the bug this fixes: run-eval.mjs + * assumed 2 calls per block while L1 was running 11, understating the bill 5.5×. + * + * eval-l1-ocr.mjs asserts its VARIANTS array matches this list at startup, so a + * variant added in one place and not the other fails loudly instead of drifting. + */ +export const L1_VARIANT_NAMES = [ + 'baseline', + 'reflow', + 'reflow-6x9', + 'reflow-7x9', + 'reflow-6x10', + 'reflow-7x10', + 'reflow-8x10', + 'aa-5x8', + 'aa-5x8-color', + 'aa-7x10', + 'reflow-inimage', +]; diff --git a/eval/run-eval.mjs b/eval/run-eval.mjs index 25643795..ef41cd42 100644 --- a/eval/run-eval.mjs +++ b/eval/run-eval.mjs @@ -135,7 +135,12 @@ if (ESTIMATE_ONLY) { const l1Blocks = existsSync(blocksPath) ? JSON.parse(readFileSync(blocksPath, 'utf8')) : []; const l2Sessions = existsSync(sessionsPath) ? JSON.parse(readFileSync(sessionsPath, 'utf8')) : []; - printCostEstimate({ l1Blocks, l2Sessions }, args.model); + // L1 makes one call per variant per block, not the 2 this used to assume — with + // the full sweep that understated the bill 5.5×. This orchestrator has no + // --variants filter of its own, so it always prices the full sweep; run + // eval-l1-ocr.mjs directly (it takes --variants) for a narrowed estimate. + const { L1_VARIANT_NAMES } = await import('./lib/variant-names.mjs'); + printCostEstimate({ l1Blocks, l2Sessions }, args.model, L1_VARIANT_NAMES.length); process.exit(0); } @@ -211,18 +216,33 @@ const combinedLines = [ ``, `## Overview`, ``, - l1Results ? [ - `### L1: OCR Fidelity`, - ``, - `| | Baseline | Reflow | Δ |`, - `|--|---------|--------|---|`, - `| Mean char accuracy | ${(l1Results.baselineAgg.meanAccuracy * 100).toFixed(2)}% | ${(l1Results.reflowAgg.meanAccuracy * 100).toFixed(2)}% | ${((l1Results.reflowAgg.meanAccuracy - l1Results.baselineAgg.meanAccuracy) * 100).toFixed(2)}pp |`, - `| Macro accuracy | ${(l1Results.baselineAgg.macroAccuracy * 100).toFixed(2)}% | ${(l1Results.reflowAgg.macroAccuracy * 100).toFixed(2)}% | ${((l1Results.reflowAgg.macroAccuracy - l1Results.baselineAgg.macroAccuracy) * 100).toFixed(2)}pp |`, - `| Image savings | — | ${l1Results.imageSavingsPct.toFixed(1)}% | |`, - ``, - `Full L1 report: [l1-report.md](l1-report.md)`, - ``, - ].join('\n') : '*(L1 not run)*', + // L1 grew from a fixed baseline/reflow pair into an N-variant sweep, so the + // summary is now built from `perVariant` (keyed by variant name) instead of the + // long-gone `baselineAgg`/`reflowAgg` fields. Deltas are stated against + // `baseline` when present, which keeps the old two-column reading intact. + l1Results ? (() => { + const perVariant = l1Results.perVariant ?? {}; + const names = Object.keys(perVariant); + if (names.length === 0) return '### L1: OCR Fidelity\n\n*(no variant results)*\n'; + const base = perVariant.baseline?.agg; + const pct = (v) => `${(v * 100).toFixed(2)}%`; + const delta = (v, b) => + b === undefined ? '—' : `${((v - b) * 100 >= 0 ? '+' : '')}${((v - b) * 100).toFixed(2)}pp`; + return [ + `### L1: OCR Fidelity`, + ``, + `| Variant | Mean char accuracy | Δ vs baseline | Macro accuracy | Images |`, + `|---------|-------------------:|--------------:|---------------:|-------:|`, + ...names.map((n) => { + const a = perVariant[n].agg; + return `| ${n} | ${pct(a.meanAccuracy)} | ${delta(a.meanAccuracy, base?.meanAccuracy)} ` + + `| ${pct(a.macroAccuracy)} | ${perVariant[n].imageCount ?? '—'} |`; + }), + ``, + `Full L1 report: [l1-report.md](l1-report.md)`, + ``, + ].join('\n'); + })() : '*(L1 not run)*', ``,