From 0554f3d959a6ffa0f00de6675f7ac5103765057f Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 29 Sep 2026 16:55:55 +0000 Subject: [PATCH] fix(evals): plan CI-unrunnable cases as excluded entries, not empty case shards design-review-fix drives the Aside browser and registers test.skip on Linux runners, so its case shard executed zero cases and failed the exact-one-case check in proof census 36597762183 (eval-slices 6). CASE_CI_EXCLUDE (reason + tracking, beside PERIODIC_CI_EXCLUDE) now turns such cases into excluded manifest entries that --list and the manifest surface; every planned case shard still must execute exactly its case. --- scripts/test-paid-shards.ts | 28 +++++++++++++++++++---- test/helpers/periodic-exclude-data.ts | 15 ++++++++++++ test/periodic-exclude-policy.test.ts | 33 +++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 7 deletions(-) diff --git a/scripts/test-paid-shards.ts b/scripts/test-paid-shards.ts index 3221b23be..8790ac71d 100644 --- a/scripts/test-paid-shards.ts +++ b/scripts/test-paid-shards.ts @@ -66,7 +66,7 @@ import { type ShardChildResult, } from './test-strict-output'; import { PAID_TEST_GLOBS, isPaidTestFile } from '../test/helpers/paid-test-set'; -import { PERIODIC_CI_EXCLUDE } from '../test/helpers/periodic-exclude-data'; +import { CASE_CI_EXCLUDE, PERIODIC_CI_EXCLUDE } from '../test/helpers/periodic-exclude-data'; import { FILE_RETRY_BUDGETS, SHORT_CASE_RETRY_FILES, STRICT_RETRY_CASE_BUDGETS } from '../test/helpers/eval-budgets'; import { getProjectEvalDir, getClaudeCliVersion, isFinalizedEvalResultFile, evalEntryOutcome } from '../test/helpers/eval-store'; import { manualReviewProblem } from '../test/helpers/cookie-workflow-manual-review'; @@ -180,6 +180,20 @@ export function expandCaseShards(files: string[], tier: PaidTier, rootDir = ROOT }); } +/** + * Split expanded shard keys into runnable keys and CI-unrunnable cases + * (CASE_CI_EXCLUDE), each with its surfaced reason; never an empty shard. + */ +export function partitionCaseExclusions(keys: string[]): { runnable: string[]; excluded: Array<{ file: string; reason: string }> } { + const excluded: Array<{ file: string; reason: string }> = []; + const runnable = keys.filter(key => { + const exclusion = CASE_CI_EXCLUDE[normalizeRelativePath(key)]; + if (exclusion) excluded.push({ file: key, reason: `excluded: ${exclusion.reason} [${exclusion.tracking}]` }); + return !exclusion; + }); + return { runnable, excluded }; +} + /** Compatibility helper for callers that only need the effective wall. */ export function resolvePaidShardTimeoutMs(files: string[], explicitTimeoutMs?: number): number { return resolvePaidShardBudget(files, explicitTimeoutMs).timeoutMs; @@ -1406,9 +1420,10 @@ export function buildRunManifest(opts: { const tierSelection = selectPaidTestFiles(discovered, opts.tier, rootDir, env); const judge = (file: string) => /^test\/skill-llm-eval[^/]*\.test\.ts$/.test(normalizeRelativePath(file)); const selected = opts.skipJudges ? tierSelection.selected.filter(file => !judge(file)) : tierSelection.selected; + const caseKeys = partitionCaseExclusions(expandCaseShards(selected, opts.tier, rootDir)); const excluded = [...tierSelection.excluded, ...(opts.skipJudges ? tierSelection.selected.filter(judge) - .map(file => ({ file, reason: 'skipped: LLM judges run in the periodic census and PR gate lanes' })) : [])]; - const shards = planPaidShards(expandCaseShards(selected, opts.tier, rootDir), { maxFilesPerShard: 1 }); + .map(file => ({ file, reason: 'skipped: LLM judges run in the periodic census and PR gate lanes' })) : []), ...caseKeys.excluded]; + const shards = planPaidShards(caseKeys.runnable, { maxFilesPerShard: 1 }); const cases = computePaidCaseSelection({ profile, env, rootDir, changedFiles: opts.changedFiles }); const fast = cases.coverage?.mode === 'pr'; const profileShards = fast ? shards.filter(files => prProfileFileSelected(files[0], cases.selection)) : shards; @@ -2182,8 +2197,11 @@ async function main(): Promise { return 0; } - const { selected, excluded } = selectPaidTestFiles(discovered, options.tier); - const shards = planPaidShards(expandCaseShards(selected, options.tier), { maxFilesPerShard: options.maxFilesPerShard }); + const tierSelection = selectPaidTestFiles(discovered, options.tier); + const caseKeys = partitionCaseExclusions(expandCaseShards(tierSelection.selected, options.tier)); + const selected = tierSelection.selected; + const excluded = [...tierSelection.excluded, ...caseKeys.excluded]; + const shards = planPaidShards(caseKeys.runnable, { maxFilesPerShard: options.maxFilesPerShard }); // Parent-side diff selection (D9): skip whole shards whose mapped tests are // all unselected. Fail-open everywhere — the child's self-skip stays diff --git a/test/helpers/periodic-exclude-data.ts b/test/helpers/periodic-exclude-data.ts index a3f7ccb31..7538b4ccc 100644 --- a/test/helpers/periodic-exclude-data.ts +++ b/test/helpers/periodic-exclude-data.ts @@ -44,3 +44,18 @@ export const PERIODIC_CI_EXCLUDE: Record#`), same + * contract as above: a case lands here only when a CI runner cannot execute it + * (it self-skips), with reason + tracking. The planner records each as an + * excluded manifest entry instead of an empty case shard, so the exact + * one-case check stays strict for every planned case. Pinned by + * test/periodic-exclude-policy.test.ts. + */ +export const CASE_CI_EXCLUDE: Record = { + 'test/skill-e2e-design.test.ts#design-review-fix': { + reason: '/design-review drives the Aside browser; CI runners are Linux without Aside, so the case registers test.skip("needs Aside")', + tracking: 'TODOS.md "CI-unrunnable paid evals" (re-entry: the CLI/device is available in the CI image; review by 2026-12-28)', + }, +}; diff --git a/test/periodic-exclude-policy.test.ts b/test/periodic-exclude-policy.test.ts index 64af9596c..8e4946507 100644 --- a/test/periodic-exclude-policy.test.ts +++ b/test/periodic-exclude-policy.test.ts @@ -9,9 +9,10 @@ import { describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; -import { PERIODIC_CI_EXCLUDE } from './helpers/periodic-exclude-data'; +import { CASE_CI_EXCLUDE, PERIODIC_CI_EXCLUDE } from './helpers/periodic-exclude-data'; +import { E2E_TOUCHFILES } from './helpers/touchfiles'; import { isPaidTestFile } from './helpers/paid-test-set'; -import { selectPaidTestFiles } from '../scripts/test-paid-shards'; +import { buildRunManifest, CASE_SHARDED_FILES, expandCaseShards, partitionCaseExclusions, selectPaidTestFiles, shardCaseId, shardFile } from '../scripts/test-paid-shards'; const ROOT = path.resolve(__dirname, '..'); @@ -42,4 +43,32 @@ describe('periodic exclude policy', () => { expect(reason).not.toStartWith('excluded: '); } }); + + test('case exclusions name a registered case of a case-sharded file and carry reason + tracking', () => { + const entries = Object.entries(CASE_CI_EXCLUDE); + expect(entries.length).toBeGreaterThan(0); + for (const [key, meta] of entries) { + const file = shardFile(key), id = shardCaseId(key); + expect(CASE_SHARDED_FILES, `${key}: not a case-sharded file`).toContain(file); + expect(id !== null && E2E_TOUCHFILES[id]?.includes(file), `${key}: not a registered case of ${file}`).toBe(true); + expect(meta.reason.length, `${key}: empty reason`).toBeGreaterThan(20); + expect(meta.tracking.length, `${key}: empty tracking pointer`).toBeGreaterThan(5); + } + }); + + test('an excluded case is an excluded manifest entry with its reason, never a planned empty case shard', () => { + for (const [key] of Object.entries(CASE_CI_EXCLUDE)) { + const tiers = (['gate', 'periodic', 'marathon'] as const).filter(tier => expandCaseShards([shardFile(key)], tier).includes(key)); + expect(tiers.length, `${key} belongs to no tier`).toBeGreaterThan(0); + for (const tier of tiers) { + const { runnable, excluded } = partitionCaseExclusions(expandCaseShards([shardFile(key)], tier)); + expect(runnable).not.toContain(key); + expect(excluded.find(entry => entry.file === key)?.reason).toStartWith('excluded: '); + const manifest = buildRunManifest({ tier, sliceBudgetMs: 540_000, jobs: 2, evalsAll: true, env: { EVALS_ALL: '1' } }); + const entry = manifest.entries.find(entry => entry.file === key)!; + expect(entry).toMatchObject({ status: 'excluded', slice: 0 }); + expect(entry.reason).toContain(CASE_CI_EXCLUDE[key]!.tracking); + } + } + }); });