From f90d5a990e594a8c08d72a44bdb27a16757414b4 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 12 Aug 2026 10:44:47 -0700 Subject: [PATCH] fix(evals): 300s budget floor for the two 90s design-consultation SDK tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of PR #2533's e2e-design CI failure: design-consultation-preview failed 3 attempts at 0 turns/$0.00/93s — the session was up but the model's first completion queued past the 90s inner budget under concurrent API load (11 matrix jobs; the sibling research test booted its first tool at 4s, so this is API-side queuing, not CPU boot contention). The test was selected only because touchfiles.ts is a global touchfile; the tested behavior is untouched by this branch. 90s budgets cannot absorb one slow first completion. Both 90s tests in the file move to the repo's saturated-runner standard (300s inner / 360s outer, matching review-dashboard-via and retro-base-branch). Deliberately NOT re-arming the runner's inner timer on first stream event: an audit found ~100 outer bun-timeout literals sized inner+30-60s that a re-arm would silently break — the structural options are written up in TODOS.md. Co-Authored-By: Claude Fable 5 --- TODOS.md | 25 +++++++++++++++++++++++++ test/skill-e2e-design.test.ts | 16 ++++++++++++---- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/TODOS.md b/TODOS.md index cddd7c9be..d62f0ad29 100644 --- a/TODOS.md +++ b/TODOS.md @@ -233,6 +233,31 @@ SKILL.md untouched). `bun test` is green again. ## Scope-gate follow-ups (filed via /plan-eng-review on the plan-mode auto-select-B change) +### P2: SDK eval budgets charge API-queue latency to the work budget — pick a structural fix + +**What:** `runSkillTest`'s single `setTimeout(timeout)` arms at spawn, so session +startup AND the model's first-completion queue time are charged against the +test's work budget. Under concurrent load (11 CI matrix jobs, or local eval +runs sharing the org API), a first completion can queue 60-90s+, producing the +deterministic `0 turns / $0.00 / s x3 attempts` failure shape. Observed: +`review-dashboard-via` (PR #2472, 180s→300s), `retro-base-branch` (240s→360s), +`plan-ceo-plan-mode` (300s→420s, 2026-08-12), `design-consultation-preview` +(90s→300s, PR #2533 CI). Every fix so far is a per-test budget bump. + +**Why not just re-arm the timer on first stream event:** an audit (2026-08-12) +found ~100 outer bun-timeout literals sized as inner+30-60s; re-arming the inner +clock breaks every outer/inner relationship and needs a codemod of all of them. + +**Options:** (a) two-phase timer in session-runner (startup grace, re-arm on +first NDJSON line) + codemod outer literals to inner+grace+slack; (b) adopt a +300s floor for all CI SDK budgets (statically enforceable — a free test can +assert no `timeout: <300_000` in skill-e2e files) and stop re-litigating per +test; (c) startup-spawn semaphore in the runner (bounds the boot stampede but +not API-side queuing — evidence says queuing dominates, so likely insufficient +alone). Recommend (b) short-term + (a) properly sequenced with the codemod. + +**Depends on / blocked by:** none. + ### P2: Wire the four demoted plan-mode/finding-floor PTY tests into periodic CI **What:** `evals-periodic.yml` runs an explicit 9-file matrix; the four tests diff --git a/test/skill-e2e-design.test.ts b/test/skill-e2e-design.test.ts index ec2b83728..2966412dc 100644 --- a/test/skill-e2e-design.test.ts +++ b/test/skill-e2e-design.test.ts @@ -176,7 +176,13 @@ Include: color trends, typography patterns, and layout conventions you observed. Do NOT generate a full DESIGN.md — just research notes.`, workingDirectory: researchDir, maxTurns: 8, - timeout: 90_000, + // 300s, not 90s: saturated-runner class (same as review-dashboard-via / + // retro-base-branch). PR #2533 CI observed the sibling preview test at + // 0 turns/$0.00 for 93s x3 attempts — session up, first completion + // queued past the budget under concurrent API load. 90s budgets cannot + // absorb one slow first completion; 300s is the repo's standard floor + // for CI SDK tests. Outer timeout below rises to 360s for headroom. + timeout: 300_000, testName: 'design-consultation-research', runId, }); @@ -206,7 +212,7 @@ Do NOT generate a full DESIGN.md — just research notes.`, } try { fs.rmSync(researchDir, { recursive: true, force: true }); } catch {} - }, 120_000); + }, 360_000); testConcurrentIfSelected('design-consultation-existing', async () => { // Pre-create a minimal DESIGN.md (independent of core test) @@ -274,7 +280,9 @@ Write a single HTML file to ${previewDir}/design-preview.html that shows: Do NOT write DESIGN.md — only the preview HTML.`, workingDirectory: previewDir, maxTurns: 8, - timeout: 90_000, + // 300s, not 90s: this is the test that failed 3x at 0 turns/$0.00/93s + // on PR #2533 CI — see the research test's comment for the class. + timeout: 300_000, testName: 'design-consultation-preview', runId, }); @@ -303,7 +311,7 @@ Do NOT write DESIGN.md — only the preview HTML.`, } try { fs.rmSync(previewDir, { recursive: true, force: true }); } catch {} - }, 120_000); + }, 360_000); }); // --- Plan Design Review E2E (plan-mode) ---