diff --git a/.changeset/nested-eval-results.md b/.changeset/nested-eval-results.md new file mode 100644 index 0000000..cf9ed20 --- /dev/null +++ b/.changeset/nested-eval-results.md @@ -0,0 +1,11 @@ +--- +'@vercel/agent-eval': patch +--- + +Fix housekeeping and result reuse for nested eval directories + +Results for nested evals (e.g. `caching/cache-bypass`) are now discovered at any +depth, so they are deduplicated, reused, and cleaned up under their full name +instead of being skipped. Group directories are pruned only once every eval +beneath them is gone, and results that housekeeping keeps are no longer walked +into or modified. diff --git a/README.md b/README.md index 402601a..d9e19b8 100644 --- a/README.md +++ b/README.md @@ -738,7 +738,12 @@ Classification uses Claude Sonnet 4.5 via the Vercel AI Gateway with sandboxed r After each experiment completes, the framework automatically: - Removes duplicate results for the same eval (keeps the newest) - Removes incomplete results (missing `summary.json` or transcripts) -- Removes empty timestamp directories +- Prunes group and timestamp directories left empty by those removals + +Nested evals are handled throughout: a result stored at `caching/cache-bypass/` +is deduplicated and reused under that full name, and the `caching/` group +directory is removed only once every eval beneath it is gone. Results that +housekeeping keeps are never modified. ## Environment Variables diff --git a/packages/agent-eval/src/cli.ts b/packages/agent-eval/src/cli.ts index 61e4229..071e513 100644 --- a/packages/agent-eval/src/cli.ts +++ b/packages/agent-eval/src/cli.ts @@ -26,7 +26,7 @@ import { computeReuseCompatibilityFingerprint, decideRefingerprint, } from './lib/fingerprint.js'; -import { scanReusableResults } from './lib/results.js'; +import { scanReusableResults, findEvalResultDirs } from './lib/results.js'; import { isClassifierEnabled, classifyFailure } from './lib/classifier.js'; import { housekeep } from './lib/housekeeping.js'; import { spawnSync } from 'child_process'; @@ -830,7 +830,7 @@ async function carryForwardConfigChanges( for (const timestamp of readdirSync(expResultsDir)) { const tsDir = join(expResultsDir, timestamp); if (!statSync(tsDir).isDirectory()) continue; - for (const evalName of readdirSync(tsDir)) { + for (const evalName of findEvalResultDirs(tsDir)) { const summaryPath = join(tsDir, evalName, 'summary.json'); const evalPath = join(evalsDir, evalName); if (!existsSync(summaryPath) || !existsSync(evalPath)) continue; diff --git a/packages/agent-eval/src/lib/housekeeping.test.ts b/packages/agent-eval/src/lib/housekeeping.test.ts index c4104dd..6802dd5 100644 --- a/packages/agent-eval/src/lib/housekeeping.test.ts +++ b/packages/agent-eval/src/lib/housekeeping.test.ts @@ -171,4 +171,83 @@ describe('housekeep', () => { expect(stats.removedNonModelFailures).toBe(0); expect(existsSync(evalDir)).toBe(true); }); + + it('handles nested eval directories without deleting parent groups', () => { + // Newer timestamp with two nested evals under 'caching' + createResult(join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z', 'caching', 'cache-bypass'), {}); + createResult(join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z', 'caching', 'cached-handler'), {}); + + // Older timestamp with duplicate + createResult(join(TEST_DIR, 'exp', '2024-01-25T12-00-00.000Z', 'caching', 'cache-bypass'), {}); + + const stats = housekeep(TEST_DIR, 'exp'); + + expect(stats.removedDuplicates).toBe(1); + expect(stats.removedIncomplete).toBe(0); + + // Newer results should exist + expect(existsSync(join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z', 'caching', 'cache-bypass'))).toBe(true); + expect(existsSync(join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z', 'caching', 'cached-handler'))).toBe(true); + + // Older duplicate should be removed + expect(existsSync(join(TEST_DIR, 'exp', '2024-01-25T12-00-00.000Z', 'caching', 'cache-bypass'))).toBe(false); + // And empty old timestamp dir should be removed + expect(existsSync(join(TEST_DIR, 'exp', '2024-01-25T12-00-00.000Z'))).toBe(false); + }); + + it('removes incomplete nested results and cleans up empty parent directories', () => { + const incompleteDir = join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z', 'group', 'subgroup', 'eval-1'); + mkdirSync(incompleteDir, { recursive: true }); + // Write run-1 without summary.json (crashed/incomplete run) + mkdirSync(join(incompleteDir, 'run-1'), { recursive: true }); + + const stats = housekeep(TEST_DIR, 'exp'); + + expect(stats.removedIncomplete).toBe(1); + // subgroup, group, and the timestamp dir + expect(stats.removedEmptyDirs).toBe(3); + expect(existsSync(join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z'))).toBe(false); + }); + + it('leaves kept results untouched, including empty and hidden contents', () => { + const evalDir = join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z', 'eval-1'); + createResult(evalDir, {}); + // saveResults creates run-N/outputs/ unconditionally, often with nothing in it + const outputs = join(evalDir, 'run-1', 'outputs'); + mkdirSync(outputs, { recursive: true }); + // A copied fixture whose only contents are dotfiles + const workflows = join(evalDir, 'run-1', 'project', '.github', 'workflows'); + mkdirSync(workflows, { recursive: true }); + writeFileSync(join(workflows, 'ci.yml'), 'name: ci\n'); + + const stats = housekeep(TEST_DIR, 'exp'); + + expect(stats.removedEmptyDirs).toBe(0); + expect(existsSync(outputs)).toBe(true); + expect(existsSync(join(workflows, 'ci.yml'))).toBe(true); + }); + + it('removes crashed eval directories that never got a run dir', () => { + const tsDir = join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z'); + const crashed = join(tsDir, 'group', 'eval-1'); + mkdirSync(crashed, { recursive: true }); + writeFileSync(join(crashed, 'partial.log'), 'boom\n'); + + const stats = housekeep(TEST_DIR, 'exp'); + + expect(stats.removedIncomplete).toBe(1); + expect(existsSync(tsDir)).toBe(false); + }); + + it('does not delete a group directory whose eval is named like a run', () => { + const tsDir = join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z'); + createResult(join(tsDir, 'caching', 'run-1'), {}); + createResult(join(tsDir, 'caching', 'cache-bypass'), {}); + + const stats = housekeep(TEST_DIR, 'exp'); + + expect(stats.removedIncomplete).toBe(0); + expect(existsSync(join(tsDir, 'caching', 'run-1', 'summary.json'))).toBe(true); + expect(existsSync(join(tsDir, 'caching', 'cache-bypass', 'summary.json'))).toBe(true); + }); }); diff --git a/packages/agent-eval/src/lib/housekeeping.ts b/packages/agent-eval/src/lib/housekeeping.ts index 32a9d66..c4a4542 100644 --- a/packages/agent-eval/src/lib/housekeeping.ts +++ b/packages/agent-eval/src/lib/housekeeping.ts @@ -4,12 +4,13 @@ * After experiments complete, consolidate results: * - For each (experiment, eval) pair: keep only the latest valid result * - Remove older duplicates and dangling/incomplete results - * - Remove empty timestamp directories + * - Prune group and timestamp directories left empty by those removals */ import { readdirSync, rmSync, existsSync, readFileSync, statSync } from 'fs'; import { join } from 'path'; import { isClassifierEnabled, isNonModelFailure } from './classifier.js'; +import { findEvalResultDirs, isRunDirName } from './results.js'; interface HousekeepingStats { removedDuplicates: number; @@ -23,7 +24,8 @@ interface HousekeepingStats { * * For each eval: keeps the newest complete result (has summary.json and * at least one transcript), removes older duplicates and incomplete results. - * Removes empty timestamp directories afterward. + * Group and timestamp directories left empty by those removals are then pruned; + * results that were kept are never modified. */ export function housekeep( resultsDir: string, @@ -66,18 +68,14 @@ export function housekeep( for (const timestamp of timestamps) { const tsDir = join(experimentDir, timestamp); - let evalDirs: string[]; - try { - evalDirs = readdirSync(tsDir).filter((d) => !d.startsWith('.')); - } catch { - continue; - } + const evalDirs = findEvalResultDirs(tsDir); + + // Group directories that a removal below may have left empty. + const orphanedGroups = new Set(); for (const evalDir of evalDirs) { const evalResultDir = join(tsDir, evalDir); - if (!statSync(evalResultDir).isDirectory()) continue; - // Read fingerprint to distinguish different configs (e.g. smoke vs full) const fingerprint = readFingerprint(evalResultDir); const dedupeKey = fingerprint ? `${evalDir}:${fingerprint}` : evalDir; @@ -87,6 +85,7 @@ export function housekeep( if (!options?.dry) { rmSync(evalResultDir, { recursive: true }); } + markGroupsOrphaned(evalDir, orphanedGroups); stats.removedDuplicates++; continue; } @@ -100,33 +99,77 @@ export function housekeep( if (!options?.dry) { rmSync(evalResultDir, { recursive: true }); } + markGroupsOrphaned(evalDir, orphanedGroups); stats.removedNonModelFailures++; } else { // Incomplete or smoke — remove if (!options?.dry) { rmSync(evalResultDir, { recursive: true }); } + markGroupsOrphaned(evalDir, orphanedGroups); stats.removedIncomplete++; } } - // Check if timestamp dir is now empty - try { - const remaining = readdirSync(tsDir).filter((d) => !d.startsWith('.')); - if (remaining.length === 0) { - if (!options?.dry) { - rmSync(tsDir, { recursive: true }); - } - stats.removedEmptyDirs++; - } - } catch { - // Directory already removed or inaccessible + // Prune group directories the removals above emptied, deepest first, then + // the timestamp directory itself. Only ancestors of something we deleted are + // considered — results we chose to keep are never walked into. + const deepestFirst = [...orphanedGroups].sort( + (a, b) => b.split('/').length - a.split('/').length + ); + for (const group of deepestFirst) { + if (removeIfEmpty(join(tsDir, group), options?.dry)) stats.removedEmptyDirs++; } + if (removeIfEmpty(tsDir, options?.dry)) stats.removedEmptyDirs++; } return stats; } +/** + * OS metadata files that should not keep an otherwise-empty directory alive. + * Deliberately an allowlist: any other dotfile (.gitignore, .github/, .env) is + * real content and must not be swept away with the directory holding it. + */ +const IGNORABLE_ENTRIES = new Set(['.DS_Store', 'Thumbs.db']); + +/** + * Record every group directory between `evalDir` and the timestamp root, so the + * ones a removal just emptied can be pruned. + */ +function markGroupsOrphaned(evalDir: string, orphaned: Set): void { + const segments = evalDir.split('/'); + segments.pop(); + while (segments.length > 0) { + orphaned.add(segments.join('/')); + segments.pop(); + } +} + +/** + * Remove `dir` if it holds nothing but OS metadata. Returns whether it went. + * + * A directory that cannot be read is left alone: never delete contents that + * were never inspected. + */ +function removeIfEmpty(dir: string, dry = false): boolean { + let entries: string[]; + try { + entries = readdirSync(dir); + } catch { + return false; + } + if (entries.some((e) => !IGNORABLE_ENTRIES.has(e))) return false; + if (!dry) { + try { + rmSync(dir, { recursive: true }); + } catch { + return false; + } + } + return true; +} + /** * Check if an eval result is from a smoke test. */ @@ -163,7 +206,7 @@ function isComplete(evalResultDir: string): boolean { try { const entries = readdirSync(evalResultDir); for (const entry of entries) { - if (!entry.startsWith('run-')) continue; + if (!isRunDirName(entry)) continue; const runDir = join(evalResultDir, entry); if ( existsSync(join(runDir, 'transcript-raw.jsonl')) || diff --git a/packages/agent-eval/src/lib/results.test.ts b/packages/agent-eval/src/lib/results.test.ts index 2011a25..e05f2af 100644 --- a/packages/agent-eval/src/lib/results.test.ts +++ b/packages/agent-eval/src/lib/results.test.ts @@ -10,6 +10,8 @@ import { formatResultsTable, formatRunResult, scanReusableResults, + findEvalResultDirs, + isEvalResultDir, } from './results.js'; import type { AgentRunResult } from './agents/types.js'; import type { EvalRunResult, EvalRunData, ResolvedExperimentConfig } from './types.js'; @@ -620,5 +622,92 @@ describe('results utilities', () => { expect(result.size).toBe(1); expect(result.get('eval-1')?.timestamp).toBe('2024-01-26T00-00-00.000Z'); }); + + it('finds reusable results for nested eval directories', () => { + const expDir = join(TEST_DIR, 'my-exp', '2024-01-26T12-00-00.000Z', 'caching', 'cache-bypass'); + mkdirSync(expDir, { recursive: true }); + writeFileSync( + join(expDir, 'summary.json'), + JSON.stringify({ totalRuns: 1, passedRuns: 1, passRate: '100%', meanDuration: 10, fingerprint: 'nested-hash' }) + ); + + const result = scanReusableResults(TEST_DIR, 'my-exp', { 'caching/cache-bypass': 'nested-hash' }); + expect(result.size).toBe(1); + expect(result.get('caching/cache-bypass')?.fingerprint).toBe('nested-hash'); + }); + }); + + describe('findEvalResultDirs', () => { + it('finds flat and nested eval result directories', () => { + const tsDir = join(TEST_DIR, 'exp', '2024-01-26T12-00-00.000Z'); + mkdirSync(join(tsDir, 'flat-eval'), { recursive: true }); + writeFileSync(join(tsDir, 'flat-eval', 'summary.json'), '{}'); + + mkdirSync(join(tsDir, 'group', 'nested-eval'), { recursive: true }); + writeFileSync(join(tsDir, 'group', 'nested-eval', 'summary.json'), '{}'); + + mkdirSync(join(tsDir, 'a', 'b', 'c', 'deep-eval'), { recursive: true }); + writeFileSync(join(tsDir, 'a', 'b', 'c', 'deep-eval', 'summary.json'), '{}'); + + // Crashed run (has run-* but no summary.json) + mkdirSync(join(tsDir, 'group', 'crashed-eval', 'run-1'), { recursive: true }); + + const found = findEvalResultDirs(tsDir).sort(); + expect(found).toEqual([ + 'a/b/c/deep-eval', + 'flat-eval', + 'group/crashed-eval', + 'group/nested-eval', + ]); + }); + + it('identifies eval result directories with isEvalResultDir', () => { + const withSummary = join(TEST_DIR, 'eval-with-summary'); + mkdirSync(withSummary, { recursive: true }); + writeFileSync(join(withSummary, 'summary.json'), '{}'); + expect(isEvalResultDir(withSummary)).toBe(true); + + const withRun = join(TEST_DIR, 'eval-with-run', 'run-1'); + mkdirSync(withRun, { recursive: true }); + expect(isEvalResultDir(join(TEST_DIR, 'eval-with-run'))).toBe(true); + + const empty = join(TEST_DIR, 'empty-dir'); + mkdirSync(empty, { recursive: true }); + expect(isEvalResultDir(empty)).toBe(false); + }); + + it('treats a nested eval named run-N as an eval, not a run directory', () => { + const tsDir = join(TEST_DIR, 'ts-run-named-eval'); + mkdirSync(join(tsDir, 'caching', 'run-1'), { recursive: true }); + writeFileSync(join(tsDir, 'caching', 'run-1', 'summary.json'), '{}'); + + expect(isEvalResultDir(join(tsDir, 'caching'))).toBe(false); + expect(findEvalResultDirs(tsDir)).toEqual(['caching/run-1']); + }); + + it('ignores files that merely look like run directories', () => { + const tsDir = join(TEST_DIR, 'ts-run-named-file'); + mkdirSync(join(tsDir, 'group', 'eval-1'), { recursive: true }); + writeFileSync(join(tsDir, 'group', 'eval-1', 'summary.json'), '{}'); + writeFileSync(join(tsDir, 'group', 'run-1.log'), 'noise'); + + expect(isEvalResultDir(join(tsDir, 'group'))).toBe(false); + expect(findEvalResultDirs(tsDir)).toEqual(['group/eval-1']); + }); + + it('reports crashed debris so housekeeping can still clean it up', () => { + const tsDir = join(TEST_DIR, 'ts-debris'); + mkdirSync(join(tsDir, 'group', 'eval-1'), { recursive: true }); + writeFileSync(join(tsDir, 'group', 'eval-1', 'partial.log'), 'boom'); + + expect(findEvalResultDirs(tsDir)).toEqual(['group/eval-1']); + }); + + it('reports nothing for a tree of empty directories', () => { + const tsDir = join(TEST_DIR, 'ts-empty'); + mkdirSync(join(tsDir, 'group', 'subgroup'), { recursive: true }); + + expect(findEvalResultDirs(tsDir)).toEqual([]); + }); }); }); diff --git a/packages/agent-eval/src/lib/results.ts b/packages/agent-eval/src/lib/results.ts index 9380cf1..5792ae0 100644 --- a/packages/agent-eval/src/lib/results.ts +++ b/packages/agent-eval/src/lib/results.ts @@ -8,9 +8,9 @@ import { readdirSync, readFileSync, existsSync, - statSync, unlinkSync, } from 'fs'; +import type { Dirent } from 'fs'; import { join, dirname } from 'path'; import chalk from 'chalk'; import type { @@ -523,16 +523,11 @@ export function scanReusableResults( for (const timestamp of timestamps) { const tsDir = join(experimentDir, timestamp); - if (!statSync(tsDir).isDirectory()) continue; - let evalDirs: string[]; - try { - evalDirs = readdirSync(tsDir).filter((d) => !d.startsWith('.')); - } catch { - continue; - } - - for (const evalDir of evalDirs) { + // Walk the results actually on disk rather than probing every configured + // eval: an experiment with many evals and many timestamps would otherwise + // cost a stat per (timestamp x eval) pair on every scan. + for (const evalDir of findEvalResultDirs(tsDir)) { // Already found a reusable result for this eval if (reusable.has(evalDir)) continue; @@ -587,3 +582,79 @@ export function scanReusableResults( return reusable; } + +/** + * Matches the run directories saveResults creates ("run-1", "run-2", ...). + * + * Shared so everything that asks "is this a run?" agrees: when the discovery + * predicate and housekeeping's completeness check drift apart, a group + * directory can be mistaken for a result and deleted wholesale. + */ +export function isRunDirName(name: string): boolean { + return /^run-\d+$/.test(name); +} + +/** + * Check whether a directory is an eval result directory, as opposed to a group + * directory that merely contains nested evals. + * + * A result has summary.json, or holds run-N/ directories from a run that + * crashed before the summary was written. Entries are read as dirents, so a + * plain file named "run-1.log" never makes its parent look like a result. + */ +export function isEvalResultDir(absPath: string): boolean { + if (existsSync(join(absPath, 'summary.json'))) return true; + try { + return readdirSync(absPath, { withFileTypes: true }).some((e) => { + if (!e.isDirectory() || !isRunDirName(e.name)) return false; + // An eval can itself be named "run-1". If the child is a result in its + // own right, this directory is the group above it, not a result. + return !isEvalResultDir(join(absPath, e.name)); + }); + } catch { + return false; + } +} + +/** + * Find every eval-result directory under a timestamp directory, returning paths + * relative to it (e.g. "eval-1" or "caching/cache-bypass"). + * + * Recurses through group directories until it reaches a result and does not + * descend past it. A directory holding files but no result anywhere beneath is + * reported as a result itself: it is debris from a run that crashed before + * writing run-N/, and housekeeping has to see it in order to clean it up. + * + * Entries come from dirents rather than statSync, so symlinks are not followed + * and a link pointing at an ancestor cannot send the walk into a loop. + */ +export function findEvalResultDirs(tsDir: string, prefix = ''): string[] { + const out: string[] = []; + let entries: Dirent[]; + try { + entries = readdirSync(join(tsDir, prefix), { withFileTypes: true }); + } catch { + return out; + } + + let hasContent = false; + for (const entry of entries) { + if (entry.name.startsWith('.')) continue; + if (!entry.isDirectory()) { + hasContent = true; + continue; + } + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (isEvalResultDir(join(tsDir, rel))) { + out.push(rel); + } else { + out.push(...findEvalResultDirs(tsDir, rel)); + } + } + + // Debris from a crashed run: files, but no result directory beneath. The + // timestamp root is never reported this way — it is not itself an eval. + if (prefix && out.length === 0 && hasContent) return [prefix]; + + return out; +}