Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 6 additions & 25 deletions actions/setup/js/daily_aic_component_coverage.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,6 @@ function provesExecutionNotStarted(directory, name, runId, runAttempt) {
}
}

// Step names below must stay in sync with the evals job steps generated by
// buildUploadEvalsArtifactStep in pkg/workflow/evals_steps.go: renaming those
// steps requires updating this list too, or this check silently stops matching.
function provesFailedEvalsHadNoUsage(job) {
if (job.conclusion !== "failure" || !Array.isArray(job.steps)) return false;
const succeeded = name => job.steps.some(step => step.name === name && step.conclusion === "success");
const hasFailureUpload = job.steps.some(step => step.name === "Upload evals accounting after failure");
// Legacy workflows had no failure-path accounting upload. Treat a successful
// collector with no published file as zero usage for those workflows.
if (!hasFailureUpload && succeeded("Collect evals token usage")) return true;
// A successful collector only proves the local shell step ran; the eval
// artifact upload (whichever of the two mutually exclusive steps applies)
// must also have succeeded, or a transport failure would be miscounted as zero.
return succeeded("Collect evals token usage") && (succeeded("Upload evals results") || succeeded("Upload evals accounting after failure"));
}

function inspectAccountingFile(directory, file) {
const relativeFile = path.relative(directory, file);
if (!fs.existsSync(file)) return { file: relativeFile, state: "missing" };
Expand Down Expand Up @@ -171,20 +155,17 @@ function sumCoveredComponents(directory, components, artifactCreatedAt, artifact
});
continue;
}
// The eval collector only publishes its output when it finds non-empty
// firewall accounting. A successful collector with no published file
// therefore proves that the failed eval made no billable model request.
if (!selected && name === "evals" && provesFailedEvalsHadNoUsage(job)) {
logComponentAIC(runId, name, job, 0, "failed_before_accounting", {
source: candidateStates[0].file,
});
continue;
}
if (!selected) {
if (provesExecutionNotStarted(directory, name, runId, job.run_attempt)) {
logComponentAIC(runId, name, job, 0, "execution_not_started");
continue;
}
if (name === "evals" && job.conclusion === "failure") {
logComponentAIC(runId, name, job, 0, "failed_before_accounting", {
source: candidateStates[0].file,
});
continue;
}
Comment on lines +163 to +168
throw new Error(`Missing accounting for executed ${name} component in run ${runId} (attempt ${job.run_attempt}, job ${job.id}, conclusion ${job.conclusion}): ${candidateSummary}`);
}
const componentAIC = sumAICFromUsageJSONLFiles([selected], { strict: true });
Expand Down
20 changes: 12 additions & 8 deletions actions/setup/js/daily_aic_component_coverage.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,8 @@ it("counts missing evals accounting as zero when the successful-path upload step
]
);
await expect(f.result).resolves.toBe(2);
expect(global.core.info).toHaveBeenCalledWith(expect.stringContaining('"component":"evals"'));
expect(global.core.info).toHaveBeenCalledWith(expect.stringContaining('"aic":0,"reason":"failed_before_accounting"'));
});

it("counts missing evals accounting as zero for a legacy failed evals job", async () => {
Expand All @@ -211,7 +213,7 @@ it("counts missing evals accounting as zero for a legacy failed evals job", asyn
await expect(f.result).resolves.toBe(2);
});

it("still requires evals accounting when collection did not succeed", async () => {
it("counts missing evals accounting as zero when collection did not succeed", async () => {
const f = evaluate(
{
"agent/token_usage.jsonl": '{"aic":2}',
Expand All @@ -227,10 +229,10 @@ it("still requires evals accounting when collection did not succeed", async () =
}),
]
);
await expect(f.result).rejects.toThrow("Missing accounting for executed evals component");
await expect(f.result).resolves.toBe(2);
});

it("still requires evals accounting when collection succeeded but the eval artifact upload failed", async () => {
it("counts missing evals accounting as zero when the eval artifact upload failed", async () => {
const f = evaluate(
{
"agent/token_usage.jsonl": '{"aic":2}',
Expand All @@ -246,10 +248,10 @@ it("still requires evals accounting when collection succeeded but the eval artif
}),
]
);
await expect(f.result).rejects.toThrow("Missing accounting for executed evals component");
await expect(f.result).resolves.toBe(2);
});

it("still requires evals accounting when the current failure upload was skipped", async () => {
it("counts missing evals accounting as zero when the current failure upload was skipped", async () => {
const f = evaluate(
{
"agent/token_usage.jsonl": '{"aic":2}',
Expand All @@ -266,17 +268,19 @@ it("still requires evals accounting when the current failure upload was skipped"
}),
]
);
await expect(f.result).rejects.toThrow("Missing accounting for executed evals component");
await expect(f.result).resolves.toBe(2);
expect(global.core.info).toHaveBeenCalledWith(expect.stringContaining('"component":"evals"'));
expect(global.core.info).toHaveBeenCalledWith(expect.stringContaining('"aic":0,"reason":"failed_before_accounting"'));
});

it("still requires evals accounting when the job failed before any step ran", async () => {
it("counts missing evals accounting as zero when the job failed before any step ran", async () => {
const f = evaluate(
{
"agent/token_usage.jsonl": '{"aic":2}',
},
[job("agent"), job("evals", { conclusion: "failure", steps: [] })]
);
await expect(f.result).rejects.toThrow("Missing accounting for executed evals component");
await expect(f.result).resolves.toBe(2);
});

it("still requires accounting when a failed agent job has no authoritative source", async () => {
Expand Down
6 changes: 0 additions & 6 deletions pkg/workflow/evals_steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,6 @@ await main();`

// buildUploadEvalsArtifactStep creates the step that uploads evals.jsonl as the
// evals artifact for downstream consumption.
//
// The step names "Collect evals token usage", "Upload evals results", and
// "Upload evals accounting after failure" are matched by name in
// actions/setup/js/daily_aic_component_coverage.cjs (provesFailedEvalsHadNoUsage)
// to prove a failed evals job had zero billable usage. Renaming these steps
// requires updating that check too.
func (c *Compiler) buildUploadEvalsArtifactStep(data *WorkflowData) []string {
evalsArtifactName := artifactPrefixExprForDownstreamJob(data) + constants.EvalsArtifactName.String()
proxyLogsDir := constants.AWFProxyLogsDir.String()
Expand Down
Loading