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
39 changes: 39 additions & 0 deletions apps/api/e2e/repo-task.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,4 +260,43 @@ describe("repo-task e2e", () => {
const { body: logsBody } = await api<{ logs: LogRow[] }>(`/api/tasks/${taskId}/logs`);
expect(logsBody.logs).toHaveLength(0);
});

it("accumulates cost and tokens across a resume instead of overwriting (issue #541)", async () => {
// First run reports $0.05 and no PR → needs_attention, recording cost 0.05.
const taskId = await createTask("Resume accumulates cost [[mock:cost:0.05]]");
const first = await waitForTaskState(taskId, [
"needs_attention",
"completed",
"failed",
"pr_opened",
]);
expect(first.state).toBe("needs_attention");
expect(first.costUsd).toBe("0.05");
expect(first.inputTokens).toBe(100);
expect(first.outputTokens).toBe(25);

// Resume with a fresh $0.03 invocation. The resumed claude process reports
// only its OWN spend (0.03), so the persisted task total must ACCUMULATE to
// 0.05 + 0.03 = 0.08 — not be overwritten to 0.03 (the pre-fix undercount).
// The [[mock:cost:0.03]] directive leads the resume prompt, so it is the
// first match the fake sees even though the original 0.05 prompt is appended
// for context.
const { status } = await api(`/api/tasks/${taskId}/resume`, {
method: "POST",
body: JSON.stringify({ prompt: "Please continue [[mock:cost:0.03]]" }),
});
expect(status).toBe(200);

const resumed = await waitFor(
async () => {
const t = await getTask(taskId);
return t.costUsd === "0.08" ? t : null;
},
{ timeoutMs: 90_000, label: `task ${taskId} cost accumulates to 0.08` },
);
expect(resumed.costUsd).toBe("0.08");
// Tokens accumulate the same way: 100+100 input, 25+25 output.
expect(resumed.inputTokens).toBe(200);
expect(resumed.outputTokens).toBe(50);
});
});
45 changes: 41 additions & 4 deletions apps/api/src/workers/task-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
parseRepoUrl,
parsePrUrl,
parseIntEnv,
addCostStrings,
addTokenCounts,
} from "@optio/shared";
import { getAdapter } from "@optio/agent-adapters";
import { parseClaudeEvent } from "../services/agent-event-parser.js";
Expand Down Expand Up @@ -1086,11 +1088,46 @@ export function startTaskWorker() {

await taskService.updateTaskResult(taskId, result.summary, result.error);

// Persist cost, token usage, and model data
// Persist cost, token usage, and model data.
//
// On a resume or force-restart, Claude runs as a FRESH process (either
// `claude --resume <session>` or a brand-new session on the existing
// branch). Its result reports only its OWN turns' total_cost_usd / token
// usage — it has no knowledge of what the prior run already spent. So the
// recorded value must ACCUMULATE (prior + this run), not overwrite.
// Overwriting is what caused issue #541: /api/analytics/costs sums
// tasks.cost_usd, so replacing the original cost with just the resumed
// invocation's spend undercounts total spend.
//
// A first run (no resume/restart signal) has no prior spend to preserve,
// so it writes its value directly — this also keeps "redo from scratch"
// semantics for a fresh run. Accumulating never double-counts here: each
// relaunch is a distinct process reporting only its own cost, so
// prior + current is always the true total.
//
// Continuation signals: `resumeSessionId` (/resume, --resume), a
// `restartFromBranch` fresh session on the existing PR (/force-restart,
// auto-resume), or a `resumePrompt` (set by every relaunch path —
// including message-resume where the stored session id may be absent).
// Any of the three means a prior run's cost is already recorded and must
// be preserved; only a genuine first run has none of them.
const isContinuation = !!(resumeSessionId || restartFromBranch || resumePrompt);
const costFields: Record<string, unknown> = {};
if (result.costUsd != null) costFields.costUsd = String(result.costUsd);
if (result.inputTokens != null) costFields.inputTokens = result.inputTokens;
if (result.outputTokens != null) costFields.outputTokens = result.outputTokens;
if (result.costUsd != null) {
costFields.costUsd = isContinuation
? addCostStrings(taskAfterExec.costUsd, result.costUsd)
: String(result.costUsd);
}
if (result.inputTokens != null) {
costFields.inputTokens = isContinuation
? addTokenCounts(taskAfterExec.inputTokens, result.inputTokens)
: result.inputTokens;
}
if (result.outputTokens != null) {
costFields.outputTokens = isContinuation
? addTokenCounts(taskAfterExec.outputTokens, result.outputTokens)
: result.outputTokens;
}
if (result.model) costFields.modelUsed = result.model;
if (Object.keys(costFields).length > 0) {
await db.update(tasks).set(costFields).where(eq(tasks.id, taskId));
Expand Down
26 changes: 26 additions & 0 deletions packages/agent-adapters/src/claude-code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,32 @@ describe("ClaudeCodeAdapter", () => {
expect(result.costUsd).toBe(0.0534);
});

it("uses the LAST result event's cost on a multi-turn run, not the first (issue #541)", () => {
// With --input-format stream-json each mid-task user message produces its
// own result event whose total_cost_usd is cumulative for the process.
// Taking the first match would drop everything after turn one.
const logs = [
'{"type":"assistant","message":{"usage":{"input_tokens":100,"output_tokens":50}}}',
'{"type":"result","subtype":"success","is_error":false,"total_cost_usd":0.0534,"result":"turn 1 done"}',
'{"type":"assistant","message":{"usage":{"input_tokens":200,"output_tokens":75}}}',
'{"type":"result","subtype":"success","is_error":false,"total_cost_usd":0.1289,"result":"turn 2 done"}',
].join("\n");
const result = adapter.parseResult(0, logs);
expect(result.costUsd).toBe(0.1289);
});

it("falls back to the LAST regex cost match when result events are not JSON", () => {
// Truncated/garbled result lines still expose total_cost_usd in the raw
// text; the fallback must also prefer the last occurrence.
const logs = [
'noise "total_cost_usd": 0.01 more noise',
"<partial> total_cost_usd was 0.02 ...",
'trailing "total_cost_usd": 0.0789 end',
].join("\n");
const result = adapter.parseResult(0, logs);
expect(result.costUsd).toBe(0.0789);
});

it("extracts model from system init event", () => {
const logs = [
'{"type":"system","subtype":"init","model":"claude-sonnet-4-20250514"}',
Expand Down
21 changes: 19 additions & 2 deletions packages/agent-adapters/src/claude-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,17 @@ export class ClaudeCodeAdapter implements AgentAdapter {
const prMatch = logs.match(
/https:\/\/(?![\w.-]+\/api\/)[^\s"]+\/(?:pull\/\d+|-\/merge_requests\/\d+)/,
);
const costMatch = logs.match(/"total_cost_usd":\s*([\d.]+)/);
// Cost: take the LAST total_cost_usd, not the first. In stream-json mode a
// multi-turn run (mid-task user messages) emits a `result` event PER TURN,
// and each result's total_cost_usd is the CUMULATIVE cost of the process so
// far — so the final result event carries the full spend. Grabbing the first
// match would drop every turn after the first (issue #541). The authoritative
// value comes from the last result event (captured in the loop below); this
// global-regex last-match is only a fallback for logs whose result event
// didn't parse as JSON.
const costMatches = [...logs.matchAll(/"total_cost_usd":\s*([\d.]+)/g)];
const costFromRegex =
costMatches.length > 0 ? parseFloat(costMatches[costMatches.length - 1][1]) : undefined;

// Extract error, token usage, model, and result text from Claude's NDJSON events
let totalInputTokens = 0;
Expand All @@ -128,6 +138,8 @@ export class ClaudeCodeAdapter implements AgentAdapter {
let lastResultIsError = false;
let lastResultText: string | undefined;
let lastResultSubtype: string | undefined;
// Cumulative cost reported by the most recent result event (last wins).
let lastResultCostUsd: number | undefined;
// Synthetic assistant text Claude Code emits when an API call fails
// ("API Error: ..."). Only used as a failure signal when the run never
// produced a result event (i.e. the error was terminal, not recovered).
Expand Down Expand Up @@ -175,6 +187,11 @@ export class ClaudeCodeAdapter implements AgentAdapter {
lastResultSubtype = typeof event.subtype === "string" ? event.subtype : undefined;
lastResultText =
typeof event.result === "string" && event.result ? event.result : undefined;
// Each result event's total_cost_usd is cumulative for the process,
// so the last one seen holds the authoritative final cost.
if (typeof event.total_cost_usd === "number") {
lastResultCostUsd = event.total_cost_usd;
}
}
} catch {
// Not JSON, skip
Expand Down Expand Up @@ -230,7 +247,7 @@ export class ClaudeCodeAdapter implements AgentAdapter {
return {
success,
prUrl: prMatch?.[0],
costUsd: costMatch ? parseFloat(costMatch[1]) : undefined,
costUsd: lastResultCostUsd ?? costFromRegex,
inputTokens: totalInputTokens > 0 ? totalInputTokens : undefined,
outputTokens: totalOutputTokens > 0 ? totalOutputTokens : undefined,
model,
Expand Down
1 change: 1 addition & 0 deletions packages/shared/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export * from "./types/git-platform.js";
export * from "./utils/parse-repo-url.js";
export * from "./utils/is-stalled.js";
export * from "./utils/parse-int-env.js";
export * from "./utils/cost.js";
export * from "./optio-tools.js";
export * from "./types/pr-review.js";
export * from "./reconcile/types.js";
Expand Down
65 changes: 65 additions & 0 deletions packages/shared/src/utils/cost.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { describe, it, expect } from "vitest";
import { addCostStrings, addTokenCounts } from "./cost.js";

describe("addCostStrings", () => {
it("accumulates prior + current cost (the resume undercount fix)", () => {
// Original run cost $0.0534; resumed run reports only its own $0.0212.
// The task total must reflect both, not just the resumed invocation.
expect(addCostStrings("0.0534", "0.0212")).toBe("0.0746");
});

it("treats a null prior cost as zero (first run)", () => {
expect(addCostStrings(null, "0.0123")).toBe("0.0123");
expect(addCostStrings(undefined, "0.0123")).toBe("0.0123");
});

it("treats an empty-string prior cost as zero", () => {
expect(addCostStrings("", "0.5")).toBe("0.5");
});

it("returns the prior cost when the new cost is missing", () => {
expect(addCostStrings("0.42", null)).toBe("0.42");
});

it("is decimal-safe: 0.1 + 0.2 does not drift to 0.30000000000000004", () => {
expect(addCostStrings("0.1", "0.2")).toBe("0.3");
});

it("handles differing decimal precision without drift", () => {
expect(addCostStrings("0.001", "0.00012345")).toBe("0.00112345");
});

it("accepts numbers as well as strings", () => {
expect(addCostStrings(0.0534, 0.0212)).toBe("0.0746");
});

it("never emits scientific notation for tiny costs", () => {
const sum = addCostStrings("0.0000001", "0.0000002");
expect(sum).not.toMatch(/e/i);
expect(Number(sum)).toBeCloseTo(0.0000003, 12);
});

it("treats non-numeric junk as zero rather than NaN", () => {
expect(addCostStrings("not-a-number", "0.25")).toBe("0.25");
expect(addCostStrings("0.25", "junk")).toBe("0.25");
});

it("accumulates repeatedly across multiple resumes", () => {
let total = addCostStrings(null, "0.05");
total = addCostStrings(total, "0.03");
total = addCostStrings(total, "0.02");
expect(total).toBe("0.1");
});
});

describe("addTokenCounts", () => {
it("adds two token counts", () => {
expect(addTokenCounts(300, 125)).toBe(425);
});

it("treats nullish operands as zero", () => {
expect(addTokenCounts(null, 125)).toBe(125);
expect(addTokenCounts(300, undefined)).toBe(300);
expect(addTokenCounts(null, null)).toBe(0);
});
});
59 changes: 59 additions & 0 deletions packages/shared/src/utils/cost.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* Cost accounting helpers.
*
* `tasks.cost_usd` (and the equivalent columns on workflow_runs / pr_review_runs
* / persistent_agents) is stored as a **string** — see the schema comment
* "stored as string to avoid float precision issues". Analytics reads it back
* with `CAST(cost_usd AS NUMERIC)` and sums across rows, so the string must be a
* plain decimal literal (no scientific notation, no thousands separators).
*/

/** Count the digits after the decimal point in a numeric string. */
function decimalPlaces(s: string): number {
const m = /\.(\d+)/.exec(s);
return m ? m[1].length : 0;
}

/**
* Decimal-safe addition of two cost values, returning a plain decimal string.
*
* Naive float addition drifts (`0.1 + 0.2 === 0.30000000000000004`), which would
* accumulate error every time a resumed run's cost is added to the prior total.
* To avoid that, both operands are scaled to integers at the finer of the two
* inputs' decimal precision (capped so the scale can't overflow), added exactly
* as integers, then rescaled.
*
* Nullish, empty, or non-numeric operands are treated as `0`, so this is safe to
* call with a task's current `costUsd` (which may be `null` on the first run).
*/
export function addCostStrings(a?: string | number | null, b?: string | number | null): string {
const sa = a == null ? "" : String(a);
const sb = b == null ? "" : String(b);
const na = Number(sa || 0);
const nb = Number(sb || 0);
const va = Number.isFinite(na) ? na : 0;
const vb = Number.isFinite(nb) ? nb : 0;

// Cap precision at 12 decimals: more than enough for per-run USD costs and
// keeps 10 ** decimals well within safe-integer range.
const decimals = Math.min(12, Math.max(decimalPlaces(sa), decimalPlaces(sb)));
const scale = 10 ** decimals;
const sum = (Math.round(va * scale) + Math.round(vb * scale)) / scale;

// Format with toFixed (never scientific notation, unlike String() for values
// below 1e-6) at the operands' precision, then trim trailing zeros so the
// result is a clean plain-decimal literal for CAST(... AS NUMERIC).
let out = sum.toFixed(decimals);
if (out.includes(".")) {
out = out.replace(/0+$/, "").replace(/\.$/, "");
}
return out;
}

/**
* Add two token counts, treating nullish operands as 0. Tokens are integers, so
* plain addition is exact — this just centralizes the null handling.
*/
export function addTokenCounts(a?: number | null, b?: number | null): number {
return (a ?? 0) + (b ?? 0);
}
Loading