Skip to content

Commit 8df07e0

Browse files
authored
Fix misleading explain hint when no explain artifact exists (#31)
1 parent a8b1d1e commit 8df07e0

2 files changed

Lines changed: 124 additions & 3 deletions

File tree

src/reporters/standard.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import path from "node:path";
22
import process from "node:process";
3+
import { existsSync } from "node:fs";
34
import cliSpinners from "cli-spinners";
45
import { printBanner } from "../cli/branding.js";
56
import pc from "picocolors";
@@ -261,12 +262,20 @@ export function createStandardReporter(options: StandardReporterOptions = {}): B
261262
stdout,
262263
);
263264

264-
if (failures.length > 0) {
265+
if (failures.some(hasExplainArtifact)) {
265266
writeLine("", stdout);
266267
writeLine(
267268
colors.yellow("Explain failed executions with `skillgym explain <artifactDir>`."),
268269
stdout,
269270
);
271+
} else if (failures.length > 0) {
272+
writeLine("", stdout);
273+
writeLine(
274+
colors.yellow(
275+
"No explain questions were recorded for this failure. Add `explain.question` to relevant assertions to enable deferred explain.",
276+
),
277+
stdout,
278+
);
270279
}
271280
},
272281
onError() {
@@ -280,6 +289,10 @@ export function createStandardReporter(options: StandardReporterOptions = {}): B
280289
};
281290
}
282291

292+
function hasExplainArtifact(failure: FailureEntry): boolean {
293+
return existsSync(path.join(failure.executionArtifactDir, "explain.json"));
294+
}
295+
283296
function formatCaseRow(result: CaseResult, symbols: ReturnType<typeof getSymbols>): string {
284297
const passed = result.passed;
285298
const color = passed ? pc.green : pc.red;

test/reporters/standard.test.ts

Lines changed: 110 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import fs from "node:fs/promises";
2+
import os from "node:os";
13
import path from "node:path";
24
import { afterEach, expect, test, vi } from "vitest";
35
import type {
@@ -174,6 +176,106 @@ test("standard reporter prints runner-grouped results and failure artifacts", as
174176
expect(output).not.toContain("Run did not complete because the runner crashed");
175177
expect(output).not.toContain("Artifacts:");
176178
expect(output).not.toContain("Artifact Root:");
179+
expect(output).not.toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
180+
expect(output).toContain(
181+
"No explain questions were recorded for this failure. Add `explain.question` to relevant assertions to enable deferred explain.",
182+
);
183+
});
184+
185+
test("standard reporter shows explain hint when a failed execution has explain.json", async () => {
186+
const writes: string[] = [];
187+
const reporter = createStandardReporter({
188+
stdout: {
189+
isTTY: false,
190+
columns: 120,
191+
write(chunk: string) {
192+
writes.push(chunk);
193+
return true;
194+
},
195+
},
196+
isInteractive: false,
197+
isUnicode: true,
198+
});
199+
200+
const executionArtifactDir = await fs.mkdtemp(
201+
path.join(os.tmpdir(), "skillgym-standard-reporter-"),
202+
);
203+
await fs.writeFile(
204+
path.join(executionArtifactDir, "explain.json"),
205+
JSON.stringify({ questions: ["why"] }),
206+
);
207+
208+
const runner = createRunnerInfo("code-main", { type: "codex", model: "gpt-5.4" });
209+
const context = {
210+
isInteractive: false,
211+
cwd: "/workspace",
212+
workspaceMode: "shared" as const,
213+
suitePath: "examples/basic-suite.ts",
214+
suiteRunArtifactDir: ".skillgym-results/run-1",
215+
selectedCaseCount: 1,
216+
selectedRunnerCount: 1,
217+
selectedExecutionCount: 1,
218+
scheduleMode: "serial" as const,
219+
maxParallel: 1,
220+
declaredTags: [],
221+
};
222+
const suiteResult: SuiteRunResult = {
223+
suitePath: context.suitePath,
224+
startedAt: "2026-04-02T12:00:00.000Z",
225+
endedAt: "2026-04-02T12:01:42.000Z",
226+
durationMs: 102_000,
227+
suiteRunArtifactDir: context.suiteRunArtifactDir,
228+
declaredTags: [],
229+
selectedTags: [],
230+
cases: [
231+
createCaseResult({
232+
caseId: "case-a",
233+
runnerResults: [
234+
createRunnerResult({
235+
runner,
236+
passed: false,
237+
executionArtifactDir,
238+
totalTokens: 12_000,
239+
}),
240+
],
241+
}),
242+
],
243+
runners: [
244+
createRunnerSummary({
245+
runner,
246+
passedCases: 0,
247+
totalCases: 1,
248+
averageDurationMs: 19_300,
249+
averageTotalTokens: 13_500,
250+
}),
251+
],
252+
};
253+
254+
await reporter.onSuiteStart?.({
255+
context,
256+
cases: [],
257+
runners: [runner],
258+
startedAt: suiteResult.startedAt,
259+
});
260+
await reporter.onRunnerFinish?.({
261+
context,
262+
case: { id: "case-a", prompt: "", assert() {} },
263+
runner,
264+
result: suiteResult.cases[0]!.runnerResults[0]!,
265+
caseIndex: 1,
266+
totalCases: 1,
267+
});
268+
await reporter.onCaseFinish?.({
269+
context,
270+
case: { id: "case-a", prompt: "", assert() {} },
271+
result: suiteResult.cases[0]!,
272+
caseIndex: 1,
273+
totalCases: 1,
274+
});
275+
await reporter.onSuiteFinish?.({ context, result: suiteResult });
276+
277+
const output = writes.join("");
278+
177279
expect(output).toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
178280
});
179281

@@ -738,7 +840,10 @@ test("standard reporter prints friendly runner crash message with log path", asy
738840
expect(output).toContain("AssertionError: expected skill to be loaded before command execution");
739841
expect(output).toContain("Log: .skillgym-results/run-1/case-a/code-main/stderr.log");
740842
expect(output).not.toContain("Artifacts:");
741-
expect(output).toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
843+
expect(output).not.toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
844+
expect(output).toContain(
845+
"No explain questions were recorded for this failure. Add `explain.question` to relevant assertions to enable deferred explain.",
846+
);
742847
});
743848

744849
test("standard reporter points workspace bootstrap failures to bootstrap logs", async () => {
@@ -841,7 +946,10 @@ test("standard reporter points workspace bootstrap failures to bootstrap logs",
841946
expect(output).toContain("Error: Workspace bootstrap failed: sh ./bootstrap.sh (exit 4)");
842947
expect(output).toContain("Log: .skillgym-results/run-1/case-a/open-main/bootstrap.stderr.log");
843948
expect(output).not.toContain("Artifacts:");
844-
expect(output).toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
949+
expect(output).not.toContain("Explain failed executions with `skillgym explain <artifactDir>`.");
950+
expect(output).toContain(
951+
"No explain questions were recorded for this failure. Add `explain.question` to relevant assertions to enable deferred explain.",
952+
);
845953
});
846954

847955
test("standard reporter renders max-steps failures with a clear message", async () => {

0 commit comments

Comments
 (0)