Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions packages/agent/src/acp-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ export const POSTHOG_NOTIFICATIONS = {

/** Permission request resolved, persisted so a reconnecting client can tell it is no longer pending */
PERMISSION_RESOLVED: "_posthog/permission_resolved",

/** RTK output-compression token savings tallied at the end of a run */
RTK_SAVINGS: "_posthog/rtk_savings",
} as const;

/**
Expand Down
39 changes: 39 additions & 0 deletions packages/agent/src/server/agent-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ import {
} from "./cloud-prompt";
import { TaskRunEventStreamSender } from "./event-stream-sender";
import { type JwtPayload, JwtValidationError, validateJwt } from "./jwt";
import { resolveRtkSavings } from "./rtk-savings";
import {
handoffLocalGitStateSchema,
jsonRpcRequestSchema,
Expand Down Expand Up @@ -3365,6 +3366,7 @@ ${signedCommitInstructions}
}

if (completeEventStream) {
await this.emitRtkSavings();
await this.eventStreamSender?.stop();
Comment on lines 3374 to 3376

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Emit RTK savings on error completion too

When a run fails, handleTurnFailure calls signalTaskComplete(..., "error"), which enqueues the error event and immediately stops eventStreamSender; the later stop() cleanup path reaches this new call only after the sender is already stopped, so enqueue drops the RTK savings event. Any failed run that used RTK-compressed commands will therefore be absent from the savings telemetry, undercounting exactly the runs that terminate through the error path.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated triage by PR Shepherd — not written by a human

Real bug — fixed in ca60d27. Confirmed the flow: on a failed run signalTaskComplete(..., "error") stops the event stream in its finally before cleanupSession runs, and enqueue is a no-op once stopped, so the savings event was dropped for error-terminating runs. Now emitRtkSavings() is also called in signalTaskComplete before that stop, and is guarded to fire at most once per run so the success-path call in cleanupSession doesn't double-emit or re-spawn rtk.

}

Expand All @@ -3373,6 +3375,43 @@ ${signedCommitInstructions}
this.session = null;
}

/**
* Emit RTK's output-compression token savings for this run as a telemetry
* event, so PostHog can report how much context RTK saved. Best-effort and
* cloud-only (no-op when the event stream isn't configured); runs just before
* the stream is completed so the event is flushed with the rest of the run.
*/
private async emitRtkSavings(): Promise<void> {
if (!this.eventStreamSender) return;

try {
const savings = await resolveRtkSavings();
if (!savings) return;

this.eventStreamSender.enqueue({
type: "notification",
timestamp: new Date().toISOString(),
notification: {
jsonrpc: "2.0",
method: POSTHOG_NOTIFICATIONS.RTK_SAVINGS,
params: {
task_id: this.config.taskId,
run_id: this.config.runId,
team_id: this.config.projectId,
total_commands: savings.totalCommands,
input_tokens: savings.inputTokens,
output_tokens: savings.outputTokens,
tokens_saved: savings.tokensSaved,
avg_savings_pct: savings.avgSavingsPct,
},
},
});
this.logger.debug("Emitted rtk savings", { ...savings });
} catch (error) {
this.logger.debug("Failed to emit rtk savings", { error });
}
}

private async captureCheckpointState(
localGitState?: HandoffLocalGitState,
): Promise<void> {
Expand Down
97 changes: 97 additions & 0 deletions packages/agent/src/server/rtk-savings.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { describe, expect, it, vi } from "vitest";
import { resolveRtkSavings } from "./rtk-savings";

const GAIN_JSON = JSON.stringify({
summary: {
total_commands: 2,
total_input: 502691,
total_output: 5835,
total_saved: 496856,
avg_savings_pct: 98.8392471717218,
total_time_ms: 3456,
avg_time_ms: 1728,
},
daily: [],
});

function gain(stdout: string) {
return vi.fn().mockResolvedValue({ stdout, stderr: "" });
}

describe("resolveRtkSavings", () => {
it("parses the rtk gain summary into a typed savings object", async () => {
const runGain = gain(GAIN_JSON);
const savings = await resolveRtkSavings({
resolveBinary: () => "/bundled/rtk",
runGain,
});

expect(runGain).toHaveBeenCalledWith("/bundled/rtk");
expect(savings).toEqual({
totalCommands: 2,
inputTokens: 502691,
outputTokens: 5835,
tokensSaved: 496856,
avgSavingsPct: 98.8392471717218,
});
});

it("does not run rtk when the binary is unresolved (disabled / not found)", async () => {
const runGain = gain(GAIN_JSON);
const savings = await resolveRtkSavings({
resolveBinary: () => undefined,
runGain,
});

expect(savings).toBeNull();
expect(runGain).not.toHaveBeenCalled();
});

it.each([
[
"nothing was tracked (zero commands)",
JSON.stringify({ summary: { total_commands: 0, total_saved: 0 } }),
],
["malformed JSON", "not json"],
["missing summary block", JSON.stringify({ daily: [] })],
])("returns null when %s", async (_label, stdout) => {
const savings = await resolveRtkSavings({
resolveBinary: () => "/bundled/rtk",
runGain: gain(stdout),
});

expect(savings).toBeNull();
});

it("returns null when rtk gain throws rather than disrupting the run", async () => {
const savings = await resolveRtkSavings({
resolveBinary: () => "/bundled/rtk",
runGain: vi.fn().mockRejectedValue(new Error("rtk exploded")),
});

expect(savings).toBeNull();
});

it("coerces missing or non-numeric fields to zero", async () => {
const savings = await resolveRtkSavings({
resolveBinary: () => "/bundled/rtk",
runGain: gain(
JSON.stringify({
summary: {
total_commands: 3,
total_saved: "lots",
avg_savings_pct: null,
},
}),
),
});

expect(savings).toEqual({
totalCommands: 3,
inputTokens: 0,
outputTokens: 0,
tokensSaved: 0,
avgSavingsPct: 0,
});
});
});
84 changes: 84 additions & 0 deletions packages/agent/src/server/rtk-savings.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { execFile } from "node:child_process";
import { promisify } from "node:util";
import { resolveRtkPrefix } from "../adapters/claude/session/rtk";

const execFileAsync = promisify(execFile);

/**
* The `summary` block of `rtk gain --format json` — RTK's own tally of how much
* command output it compressed away before it reached the model.
*/
export interface RtkSavingsSummary {
totalCommands: number;
inputTokens: number;
outputTokens: number;
tokensSaved: number;
avgSavingsPct: number;
Comment on lines +13 to +18

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Unit ambiguity in field names

RTK intercepts shell command output and compresses the bytes of that output before it reaches the model. The rtk gain JSON fields are named total_input / total_output / total_saved with no unit annotation, and the test fixture shows values of 502,691 and 5,835 for just 2 commands — consistent with bytes, not tokens. If these are byte counts, emitting them as input_tokens / output_tokens / tokens_saved (both in the interface and in the telemetry event in agent-server.ts) would mislead downstream consumers who build token-cost dashboards on top of this event. Could you confirm that rtk gain tracks model token consumption rather than raw command-output bytes?

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated triage by PR Shepherd — not written by a human

Confirmed these are tokens, not bytes. Running the bundled rtk 0.43.0 gain command directly, its own text output labels them Input tokens / Output tokens / Tokens saved (98.8%) — RTK ("Rust Token Killer") reports its own token estimates, which is exactly what we want to surface. The large values are genuine: the fixture came from a recursive rtk grep across a big tree. I kept the *_tokens names (faithful to the source) and added a doc comment on RtkSavingsSummary noting they are RTK's token estimates, not raw bytes (e137453).

}

interface GainOutput {
stdout: string;
stderr: string;
}

interface ResolveRtkSavingsOptions {
env?: NodeJS.ProcessEnv;
/** Resolves the rtk binary to invoke; undefined disables reporting. Overridable for tests. */
resolveBinary?: (env: NodeJS.ProcessEnv) => string | undefined;
/** Runs `rtk gain` and returns its stdio; overridable for tests. */
runGain?: (binary: string) => Promise<GainOutput>;
}

function toFiniteNumber(value: unknown): number {
return typeof value === "number" && Number.isFinite(value) ? value : 0;
}

function parseGainSummary(stdout: string): RtkSavingsSummary | null {
const parsed = JSON.parse(stdout) as { summary?: Record<string, unknown> };
const summary = parsed.summary;
if (!summary || typeof summary !== "object") return null;
Comment on lines +33 to +39

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 JSON.parse result not guarded before property access

If stdout is the string "null", JSON.parse returns JavaScript null, and parsed.summary immediately throws TypeError: Cannot read properties of null. This is caught by the outer try/catch in resolveRtkSavings, so it cannot disrupt a run, but the TypeScript type assertion as { summary?: ... } gives a false sense of safety that parsed is an object. A guard like if (!parsed || typeof parsed !== 'object') return null; before line 38 would make the defensive intent explicit and avoid the throw entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

🤖 Automated triage by PR Shepherd — not written by a human

Good call — fixed in e137453. parseGainSummary now guards if (!parsed || typeof parsed !== "object") return null; before indexing summary, so a "null" (or any non-object) payload returns null explicitly instead of throwing into the outer catch. Added test cases for "null" and a non-object payload.

return {
totalCommands: toFiniteNumber(summary.total_commands),
inputTokens: toFiniteNumber(summary.total_input),
outputTokens: toFiniteNumber(summary.total_output),
tokensSaved: toFiniteNumber(summary.total_saved),
avgSavingsPct: toFiniteNumber(summary.avg_savings_pct),
};
}

async function defaultRunGain(binary: string): Promise<GainOutput> {
const { stdout, stderr } = await execFileAsync(
binary,
["gain", "--format", "json"],
{ timeout: 5_000 },
);
return { stdout, stderr };
}

/**
* Reads RTK's own token-savings tally (`rtk gain --format json`).
*
* Best-effort: returns null when RTK is disabled or unavailable, when it has
* tracked nothing, or on any exec/parse failure — reporting savings must never
* disrupt a run. The tally is machine-global, which equals a single run's
* savings in the ephemeral cloud sandbox (fresh DB per run). A long-lived host
* would need to snapshot-and-diff instead.
*/
export async function resolveRtkSavings({
env = process.env,
resolveBinary = resolveRtkPrefix,
runGain = defaultRunGain,
}: ResolveRtkSavingsOptions = {}): Promise<RtkSavingsSummary | null> {
const binary = resolveBinary(env);
if (!binary) return null;

try {
const { stdout } = await runGain(binary);
const summary = parseGainSummary(stdout);
// Nothing was compressed this run — no point emitting a zero.
if (!summary || summary.totalCommands <= 0) return null;
return summary;
} catch {
return null;
}
}
Loading