From 30abe158c840849dd118bfd9e2e2cfb927d0f84c Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 31 Oct 2026 13:11:19 +0000 Subject: [PATCH] fix(eval-plan): lint loadout arms by their model; refuse arms sharing a ledger attempt Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-eval-plan | 4 ++-- lib/eval-plan.ts | 9 +++++---- test/eval-models.test.ts | 13 +++++++++++++ test/eval-plan-no-gstack-arm.test.ts | 11 +++++++++++ 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/bin/gstack-eval-plan b/bin/gstack-eval-plan index c6ba29d43..1dc081a76 100755 --- a/bin/gstack-eval-plan +++ b/bin/gstack-eval-plan @@ -31,7 +31,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; -import { checkPrereg, DEFAULT_TOKENS, PREREG_SECTIONS, dryRun, pilotReport, preregTemplate, priceGate, releasePilot, renderArms, renderDryRun, renderGatePrice, renderPilotReport, renderPreregDiagnostics, resolveArms, resolvePlanSource, runPreregDiagnostics, settlePilot, startPilot } from '../lib/eval-plan'; +import { armModel, checkPrereg, DEFAULT_TOKENS, PREREG_SECTIONS, dryRun, pilotReport, preregTemplate, priceGate, releasePilot, renderArms, renderDryRun, renderGatePrice, renderPilotReport, renderPreregDiagnostics, resolveArms, resolvePlanSource, runPreregDiagnostics, settlePilot, startPilot } from '../lib/eval-plan'; import { checkModelRules, EVAL_SCOPES, type EvalScope, type ModelRulesCheck } from '../lib/eval-models'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { resultLine } from '../lib/result-codes'; @@ -90,7 +90,7 @@ function lintModels(args: string[], uses: Array<{ model: string; scope: EvalScop if (from !== 'base' && from !== 'head') throw new Error('--models-from needs base or head'); const top = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 30_000 }); const repoRoot = top.status === 0 ? top.stdout.trim() : null; - const check = checkModelRules(repoRoot, uses, { from, base: repoRoot ? resolveBaseBranch(repoRoot, flag(args, '--base')) : 'main' }); + const check = checkModelRules(repoRoot, uses.map(u => (armModel(u.model) === u.model ? u : { ...u, model: armModel(u.model), where: `${u.where} (${u.model})` })), { from, base: repoRoot ? resolveBaseBranch(repoRoot, flag(args, '--base')) : 'main' }); if (!args.includes('--json')) process.stdout.write(check.lines.join('\n') + '\n'); for (const e of check.errors) process.stderr.write(resultLine(`gstack-eval-plan ${command}`, e.code, e.message) + '\n'); return check; diff --git a/lib/eval-plan.ts b/lib/eval-plan.ts index 4ff4605a7..6bb74db94 100644 --- a/lib/eval-plan.ts +++ b/lib/eval-plan.ts @@ -375,8 +375,10 @@ export function startPilot(o: PilotStartOptions): { record: PilotRecord; error?: if (!dry.pass) return { record: null as unknown as PilotRecord, error: { code: 'EVAL_DRY_RUN_FAILED', message: `${o.dryRun} did not pass; the pilot never starts on a failed dry run` } }; const tokens = o.tokens ?? { input: dry.rows ? Math.ceil(dry.est_input_tokens / dry.rows) : DEFAULT_TOKENS.input, output: dry.checks.output_budget.budget }; const record: PilotRecord = { schema_version: 1, n: o.n, arms: o.arms, cap_usd: o.cap, dry_run: o.dryRun, attempts: [], started_at: (o.now ?? new Date()).toISOString(), status: 'reserved' }; - const duplicates = [...new Set(o.arms.filter((a, i) => o.arms.indexOf(a) !== i))]; - if (duplicates.length) return { record, error: { code: 'EVAL_ARM_DUPLICATE', message: `arm ${duplicates.join(', ')} is listed more than once; give each arm its own # identity` } }; + const keyOf = (a: string) => `${armModel(a).replace(/[^A-Za-z0-9.-]/g, '_')}${armLoadout(a) !== null ? `.${armLoadout(a)!.replace(/[^A-Za-z0-9.-]/g, '_')}` : ''}`; + const keys = o.arms.map(keyOf); + const duplicates = [...new Set(o.arms.filter((_, i) => keys.indexOf(keys[i]!) !== i).map(a => o.arms.filter(b => keyOf(b) === keyOf(a)).join(' = ')))]; + if (duplicates.length) return { record, error: { code: 'EVAL_ARM_DUPLICATE', message: `arm ${duplicates.join(', ')} is listed more than once or shares one ledger attempt name; give each arm its own # identity` } }; fs.mkdirSync(o.out, { recursive: true }); const ledgerFile = path.join(o.out, PILOT_FILES.spend); let error: { code: ResultCodeName; message: string } | undefined; @@ -384,8 +386,7 @@ export function startPilot(o: PilotStartOptions): { record: PilotRecord; error?: if (!hasPricing(armModel(model))) { error = { code: 'EVAL_PRICE_MISSING', message: `${armModel(model)} has no row in lib/pricing.ts` }; break; } for (let i = 1; i <= o.n; i++) { const estimate = estimateCostUsd(tokens, armModel(model)); - const loadout = armLoadout(model); - const attempt = `pilot-${armModel(model).replace(/[^A-Za-z0-9.-]/g, '_')}${loadout ? `.${loadout.replace(/[^A-Za-z0-9.-]/g, '_')}` : ''}-${i}`; + const attempt = `pilot-${keyOf(model)}-${i}`; try { reserve(ledgerFile, attempt, estimate, { cap: o.cap, label: `pilot ${model} #${i}`, now: o.now }); } catch (e) { if (e instanceof SpendCapExceeded) { record.status = 'budget_exhausted'; error = { code: 'EVAL_BUDGET_EXHAUSTED', message: e.message }; break outer; } diff --git a/test/eval-models.test.ts b/test/eval-models.test.ts index 6e22a4e03..e907b7e6e 100644 --- a/test/eval-models.test.ts +++ b/test/eval-models.test.ts @@ -142,6 +142,19 @@ describe('gstack-eval-plan lints every arm (bin, fixture repo with an origin)', expect(head.err).toContain('(EVAL_MODEL_GENERATION_UNRESOLVED)'); }); + test('a # arm is linted as its model: a deny still refuses it and an exact allow accepts it', () => { + seed(GBRAIN_RULES); + const r = run(['arms', '--arms', 'claude-opus-5-5#no-gstack,gpt-5.4-mini#no-gstack']); + expect(r.code).toBe(1); + expect(r.out).toContain('EVAL_MODEL: claude-opus-5-5 scope=full ok (allow claude-*; newest in families.claude-opus) where=flag:--arms (claude-opus-5-5#no-gstack)'); + expect(r.out).toContain('EVAL_MODEL: gpt-5.4-mini scope=full denied (deny gpt-5.4-mini) where=flag:--arms (gpt-5.4-mini#no-gstack)'); + expect(r.err).toContain('(EVAL_MODEL_DENIED)'); + fs.writeFileSync(path.join(repo, '.gstack', 'eval-models.json'), JSON.stringify({ schema_version: 1, allow: ['claude-opus-5-5'], deny: [] })); + const exact = run(['arms', '--arms', 'claude-opus-5-5,claude-opus-5-5#no-gstack', '--models-from', 'head']); + expect(exact.code).toBe(0); + expect(exact.out).toContain('where=flag:--arms (claude-opus-5-5#no-gstack)'); + }); + test('dry-run and pilot start: Fable fails the full scope (no reservation), passes --scope smoke; the dry-run record carries the check', () => { seed(GBRAIN_RULES); const out = path.join(host.base, 'out'); diff --git a/test/eval-plan-no-gstack-arm.test.ts b/test/eval-plan-no-gstack-arm.test.ts index 2eeb42daa..c04e4de9b 100644 --- a/test/eval-plan-no-gstack-arm.test.ts +++ b/test/eval-plan-no-gstack-arm.test.ts @@ -149,6 +149,17 @@ describe('arm identities in pricing and the pilot', () => { expect(r.error?.code).toBe('EVAL_ARM_DUPLICATE'); expect(r.record.attempts).toEqual([]); }); + + test('distinct identities that share one ledger attempt name are refused before anything is reserved', () => { + for (const arms of [['claude-opus-5-5#review/a', 'claude-opus-5-5#review_a'], ['gpt-6#1-sol', 'gpt-6.1-sol']]) { + const d = tmp(); + const r = startPilot({ out: path.join(d, 'pilot'), cap: 100, n: 1, arms, dryRun: passingDryRun(d) }); + expect(r.error?.code).toBe('EVAL_ARM_DUPLICATE'); + expect(r.error?.message).toContain(arms.join(' = ')); + expect(r.record.attempts).toEqual([]); + expect(fs.existsSync(path.join(d, 'pilot'))).toBe(false); + } + }); }); describe('gstack-eval-plan prereg --check', () => {