Skip to content

Commit f22a2ad

Browse files
christsoclaude
andauthored
feat(core): report inconclusive status when all tests have execution errors (#941)
* feat(core): report inconclusive status when all tests have execution errors When all eval tests fail due to execution errors (e.g., misconfigured model, network failures), the run now reports INCONCLUSIVE instead of a misleading PASS/FAIL verdict. - Exit code 2 for all-execution-error runs (distinct from exit 1 for threshold failures) - CLI shows yellow INCONCLUSIVE verdict with clear messaging - JUnit XML uses executionStatus to classify <error> vs <failure>, preventing double-counting of execution errors as failures Closes #894 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: clarify manual red/green UAT as blocking step in E2E checklist Make the UAT requirement more prominent with a blocking warning, clearer red/green definitions, and explicit dependency on completing UAT before proceeding to later checklist steps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4fafdd2 commit f22a2ad

7 files changed

Lines changed: 224 additions & 19 deletions

File tree

AGENTS.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -266,17 +266,18 @@ Before marking any branch as ready for review, complete this checklist:
266266

267267
2. **Run unit tests**: `bun run test` — all must pass.
268268

269-
3. **Manual red/green UAT (REQUIRED for all changes):**
270-
Automated tests are not sufficient. Every change must be manually verified from the end user's perspective using a red/green approach:
271-
- **Red (before fix):** Reproduce the bug or demonstrate the missing feature on `main` (or before your change). Confirm the undesired behavior is observable from the CLI / user-facing output.
272-
- **Green (after fix):** Run the same scenario with your changes applied. Confirm the fix or feature works correctly from the end user's perspective.
273-
- Document both the red and green results in the PR or conversation so the user can see the before/after.
269+
3. **⚠️ BLOCKING: Manual red/green UAT — must complete before steps 4-5:**
270+
Unit tests passing is NOT sufficient. Every change must be manually verified from the end user's perspective. Do NOT skip this step or proceed to step 4 until red/green evidence is documented.
271+
272+
- **Red (before your changes):** Run the scenario on `main` (or the code state before your changes). Confirm the bug or missing feature is observable from the CLI / user-facing output. Capture the output.
273+
- **Green (with your changes):** Run the identical scenario with your branch. Confirm the fix or feature works correctly from the end user's perspective. Capture the output.
274+
- **Document both** red and green results in the PR description or comments so reviewers can see the before/after evidence.
274275

275276
For evaluator changes, this means running a real eval (not `--dry-run`) and inspecting the output JSONL. For CLI/UX changes, this means running the CLI command and verifying the console output.
276277

277278
4. **Verify no regressions** in areas adjacent to your changes (e.g., if you changed evaluator parsing, run an eval that exercises different evaluator types).
278279

279-
5. **Mark PR as ready** only after all above steps pass.
280+
5. **Mark PR as ready** only after steps 1-4 have been completed AND red/green UAT evidence is included in the PR.
280281

281282
## Documentation Updates
282283

apps/cli/src/commands/eval/commands/run.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,9 @@ export const evalRunCommand = command({
238238
excludeTag: args.excludeTag,
239239
};
240240
const result = await runEvalCommand({ testFiles: resolvedPaths, rawOptions });
241+
if (result?.allExecutionErrors) {
242+
process.exit(2);
243+
}
241244
if (result?.thresholdFailed) {
242245
process.exit(1);
243246
}

apps/cli/src/commands/eval/junit-writer.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,18 @@ export class JunitWriter {
5858

5959
const suiteXmls: string[] = [];
6060
for (const [suiteName, results] of grouped) {
61-
const failures = results.filter((r) => r.score < this.threshold).length;
62-
const errors = results.filter((r) => r.error !== undefined).length;
61+
const errors = results.filter((r) => r.executionStatus === 'execution_error').length;
62+
const failures = results.filter(
63+
(r) => r.executionStatus !== 'execution_error' && r.score < this.threshold,
64+
).length;
6365

6466
const testCases = results.map((r) => {
6567
const time = r.durationMs ? (r.durationMs / 1000).toFixed(3) : '0.000';
6668

6769
let inner = '';
68-
if (r.error) {
69-
inner = `\n <error message="${escapeXml(r.error)}">${escapeXml(r.error)}</error>\n `;
70+
if (r.executionStatus === 'execution_error') {
71+
const errorMsg = r.error ?? 'Execution error';
72+
inner = `\n <error message="${escapeXml(errorMsg)}">${escapeXml(errorMsg)}</error>\n `;
7073
} else if (r.score < this.threshold) {
7174
const message = `score=${r.score.toFixed(3)}`;
7275
const failedAssertions = r.assertions.filter((a) => !a.passed);
@@ -90,8 +93,10 @@ export class JunitWriter {
9093
}
9194

9295
const totalTests = this.results.length;
93-
const totalFailures = this.results.filter((r) => r.score < this.threshold).length;
94-
const totalErrors = this.results.filter((r) => r.error !== undefined).length;
96+
const totalErrors = this.results.filter((r) => r.executionStatus === 'execution_error').length;
97+
const totalFailures = this.results.filter(
98+
(r) => r.executionStatus !== 'execution_error' && r.score < this.threshold,
99+
).length;
95100

96101
const xml = `<?xml version="1.0" encoding="UTF-8"?>\n<testsuites tests="${totalTests}" failures="${totalFailures}" errors="${totalErrors}">\n${suiteXmls.join('\n')}\n</testsuites>\n`;
97102

apps/cli/src/commands/eval/run-eval.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,8 @@ export interface RunEvalResult {
818818
readonly target?: string;
819819
/** True when --threshold is set and mean score is below the threshold */
820820
readonly thresholdFailed?: boolean;
821+
/** True when all tests had execution errors and no evaluation was performed */
822+
readonly allExecutionErrors?: boolean;
821823
}
822824

823825
export async function runEvalCommand(
@@ -1299,7 +1301,9 @@ export async function runEvalCommand(
12991301
const summary = calculateEvaluationSummary(allResults, thresholdOpts);
13001302
console.log(formatEvaluationSummary(summary, thresholdOpts));
13011303

1302-
// Exit code matches RESULT verdict: fail if any test scored below threshold.
1304+
// Exit code: 2 when all tests are execution errors (no evaluation performed),
1305+
// 1 when any test scored below threshold.
1306+
const allExecutionErrors = summary.total > 0 && summary.executionErrorCount === summary.total;
13031307
const thresholdFailed = resolvedThreshold !== undefined && summary.qualityFailureCount > 0;
13041308

13051309
// Print matrix summary when multiple targets were evaluated
@@ -1397,6 +1401,7 @@ export async function runEvalCommand(
13971401
testFiles: activeTestFiles,
13981402
target: options.target,
13991403
thresholdFailed,
1404+
allExecutionErrors,
14001405
};
14011406
} finally {
14021407
unsubscribeCodexLogs();

apps/cli/src/commands/eval/statistics.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,25 @@ export function formatEvaluationSummary(
209209
// Overall verdict: all non-error cases must score >= per-test threshold.
210210
const gradedCount = summary.total - summary.executionErrorCount;
211211
const threshold = options?.threshold ?? 0.8;
212+
const allExecutionErrors = summary.total > 0 && summary.executionErrorCount === summary.total;
212213
const overallPassed =
213-
summary.passedCount === gradedCount ||
214-
(summary.qualityFailureCount === 0 && summary.executionErrorCount === 0);
215-
const overallVerdict = overallPassed ? 'PASS' : 'FAIL';
214+
!allExecutionErrors &&
215+
(summary.passedCount === gradedCount ||
216+
(summary.qualityFailureCount === 0 && summary.executionErrorCount === 0));
216217
const useColor = !(process.env.NO_COLOR !== undefined) && (process.stdout.isTTY ?? false);
217-
const verdictColor = overallPassed ? '\x1b[32m' : '\x1b[31m';
218-
const verdictText = `RESULT: ${overallVerdict} (${summary.passedCount}/${gradedCount} scored >= ${threshold}, mean: ${formatScore(summary.mean)})`;
218+
219+
let overallVerdict: string;
220+
let verdictColor: string;
221+
let verdictText: string;
222+
if (allExecutionErrors) {
223+
overallVerdict = 'INCONCLUSIVE';
224+
verdictColor = '\x1b[33m'; // yellow
225+
verdictText = `RESULT: INCONCLUSIVE (all ${summary.total} test(s) had execution errors — no evaluation was performed)`;
226+
} else {
227+
overallVerdict = overallPassed ? 'PASS' : 'FAIL';
228+
verdictColor = overallPassed ? '\x1b[32m' : '\x1b[31m';
229+
verdictText = `RESULT: ${overallVerdict} (${summary.passedCount}/${gradedCount} scored >= ${threshold}, mean: ${formatScore(summary.mean)})`;
230+
}
219231

220232
lines.push('\n==================================================');
221233
if (useColor) {

apps/cli/test/commands/eval/output-writers.test.ts

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ function makeResult(overrides: Partial<EvaluationResult> = {}): EvaluationResult
1919
assertions: [{ text: 'criterion-1', passed: true }],
2020
output: [{ role: 'assistant' as const, content: 'answer' }],
2121
target: 'default',
22+
executionStatus: 'ok',
2223
...overrides,
2324
};
2425
}
@@ -146,7 +147,14 @@ describe('JunitWriter', () => {
146147

147148
it('should handle errors as <error> elements', async () => {
148149
const writer = await JunitWriter.open(testFilePath);
149-
await writer.append(makeResult({ testId: 'err-1', score: 0, error: 'Timeout exceeded' }));
150+
await writer.append(
151+
makeResult({
152+
testId: 'err-1',
153+
score: 0,
154+
error: 'Timeout exceeded',
155+
executionStatus: 'execution_error',
156+
}),
157+
);
150158
await writer.close();
151159

152160
const xml = await readFile(testFilePath, 'utf8');
@@ -188,6 +196,74 @@ describe('JunitWriter', () => {
188196
expect(xml).not.toContain('<failure message="score=0.600"');
189197
expect(xml).toContain('<failure message="score=0.300"');
190198
});
199+
200+
it('should use executionStatus to classify errors vs failures', async () => {
201+
const writer = await JunitWriter.open(testFilePath);
202+
203+
await writer.append(
204+
makeResult({
205+
testId: 'exec-err',
206+
score: 0,
207+
executionStatus: 'execution_error',
208+
error: 'Not Found',
209+
}),
210+
);
211+
await writer.append(
212+
makeResult({ testId: 'quality-fail', score: 0.3, executionStatus: 'quality_failure' }),
213+
);
214+
await writer.append(makeResult({ testId: 'pass', score: 0.9, executionStatus: 'ok' }));
215+
await writer.close();
216+
217+
const xml = await readFile(testFilePath, 'utf8');
218+
// Execution error produces <error>, not <failure>
219+
expect(xml).toContain('<error message="Not Found"');
220+
// Quality failure produces <failure>
221+
expect(xml).toContain('<failure message="score=0.300"');
222+
// Counts: 1 error, 1 failure (execution error excluded from failure count)
223+
expect(xml).toContain('errors="1"');
224+
expect(xml).toContain('failures="1"');
225+
});
226+
227+
it('should not double-count execution errors as failures', async () => {
228+
const writer = await JunitWriter.open(testFilePath);
229+
230+
// All execution errors — should have 0 failures, 2 errors
231+
await writer.append(
232+
makeResult({
233+
testId: 'err-1',
234+
score: 0,
235+
executionStatus: 'execution_error',
236+
error: 'Provider error',
237+
}),
238+
);
239+
await writer.append(
240+
makeResult({
241+
testId: 'err-2',
242+
score: 0,
243+
executionStatus: 'execution_error',
244+
error: 'Timeout',
245+
}),
246+
);
247+
await writer.close();
248+
249+
const xml = await readFile(testFilePath, 'utf8');
250+
expect(xml).toContain('failures="0"');
251+
expect(xml).toContain('errors="2"');
252+
});
253+
254+
it('should emit <error> for execution_error even without error message', async () => {
255+
const writer = await JunitWriter.open(testFilePath);
256+
257+
await writer.append(
258+
makeResult({ testId: 'no-msg', score: 0, executionStatus: 'execution_error' }),
259+
);
260+
await writer.close();
261+
262+
const xml = await readFile(testFilePath, 'utf8');
263+
expect(xml).toContain('<error message="Execution error"');
264+
expect(xml).toContain('errors="1"');
265+
expect(xml).toContain('failures="0"');
266+
});
191267
});
192268

193269
describe('escapeXml', () => {
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
import { describe, expect, it } from 'bun:test';
2+
3+
import type { EvaluationResult } from '@agentv/core';
4+
5+
import {
6+
calculateEvaluationSummary,
7+
formatEvaluationSummary,
8+
} from '../../../src/commands/eval/statistics.js';
9+
10+
function makeResult(overrides: Partial<EvaluationResult> = {}): EvaluationResult {
11+
return {
12+
timestamp: '2024-01-01T00:00:00Z',
13+
testId: 'test-1',
14+
score: 1.0,
15+
assertions: [{ text: 'criterion-1', passed: true }],
16+
output: [{ role: 'assistant' as const, content: 'answer' }],
17+
target: 'default',
18+
executionStatus: 'ok',
19+
...overrides,
20+
};
21+
}
22+
23+
describe('formatEvaluationSummary — inconclusive verdict', () => {
24+
it('shows INCONCLUSIVE when all tests are execution errors', () => {
25+
const results = [
26+
makeResult({
27+
testId: 'err-1',
28+
score: 0,
29+
executionStatus: 'execution_error',
30+
error: 'Not Found',
31+
}),
32+
makeResult({
33+
testId: 'err-2',
34+
score: 0,
35+
executionStatus: 'execution_error',
36+
error: 'Not Found',
37+
}),
38+
makeResult({
39+
testId: 'err-3',
40+
score: 0,
41+
executionStatus: 'execution_error',
42+
error: 'Not Found',
43+
}),
44+
];
45+
46+
const summary = calculateEvaluationSummary(results);
47+
const output = formatEvaluationSummary(summary);
48+
49+
expect(output).toContain('RESULT: INCONCLUSIVE');
50+
expect(output).toContain('all 3 test(s) had execution errors');
51+
expect(output).toContain('no evaluation was performed');
52+
});
53+
54+
it('shows PASS/FAIL when only some tests are execution errors', () => {
55+
const results = [
56+
makeResult({ testId: 'pass-1', score: 0.9, executionStatus: 'ok' }),
57+
makeResult({
58+
testId: 'err-1',
59+
score: 0,
60+
executionStatus: 'execution_error',
61+
error: 'Not Found',
62+
}),
63+
];
64+
65+
const summary = calculateEvaluationSummary(results);
66+
const output = formatEvaluationSummary(summary);
67+
68+
// Should show PASS (the one graded test passed) not INCONCLUSIVE
69+
expect(output).toContain('RESULT: PASS');
70+
expect(output).not.toContain('INCONCLUSIVE');
71+
});
72+
73+
it('shows FAIL when there are quality failures mixed with execution errors', () => {
74+
const results = [
75+
makeResult({ testId: 'fail-1', score: 0.3, executionStatus: 'quality_failure' }),
76+
makeResult({
77+
testId: 'err-1',
78+
score: 0,
79+
executionStatus: 'execution_error',
80+
error: 'Not Found',
81+
}),
82+
];
83+
84+
const summary = calculateEvaluationSummary(results, { threshold: 0.8 });
85+
const output = formatEvaluationSummary(summary, { threshold: 0.8 });
86+
87+
expect(output).toContain('RESULT: FAIL');
88+
expect(output).not.toContain('INCONCLUSIVE');
89+
});
90+
91+
it('shows PASS when all tests pass and none are errors', () => {
92+
const results = [
93+
makeResult({ testId: 'pass-1', score: 0.9, executionStatus: 'ok' }),
94+
makeResult({ testId: 'pass-2', score: 0.85, executionStatus: 'ok' }),
95+
];
96+
97+
const summary = calculateEvaluationSummary(results);
98+
const output = formatEvaluationSummary(summary);
99+
100+
expect(output).toContain('RESULT: PASS');
101+
expect(output).not.toContain('INCONCLUSIVE');
102+
});
103+
});

0 commit comments

Comments
 (0)