Skip to content

fix(compact): report upstream usage for native compact turns #945

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
DevMello wants to merge 2 commits into
base: dev
Choose a base branch
from
+51 −1
Draft

fix(compact): report upstream usage for native compact turns #945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82bae06
fix(compact): report upstream usage for native compact turns
DevMello Aug 3, 2026
8192ea4
fix(compact): read usage without the debug body sampler
DevMello Aug 3, 2026
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
13 changes: 12 additions & 1 deletion src/server/responses/compact.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ import { redactSecretString } from "../../lib/redact";
import { readBoundedResponseBody } from "../../lib/bounded-body";
import { supportedLadderFor } from "../effort-policy";
import {
applyResponseLogMetadata,
beginRequestAttempt,
catalogModelSupportsServiceTier,
finishRequestAttempt,
inspectResponseLogJson,
noteAttemptSend,
readConfiguredCodexServiceTier,
requestLogSpeedLabel,
Expand Down Expand Up @@ -506,6 +506,17 @@ export async function handleResponsesCompact(
// Always record the real upstream status: a local buffering failure after a
// 200 upstream response must not soft-avoid a healthy account or rotate a thread.
recordCompactPoolOutcome(outcomeCtx, upstream.status, { retryAfter, resetAt });
// Lift usage and response metadata from the buffered upstream JSON into the
// request log; the routed branch gets the same through handleResponses. Only
// the parsed fields are read: a compact body is replacement history derived
// from the conversation, so it must never reach the usage debug body sampler.
if (buffered.ok) {
try {
applyResponseLogMetadata(logCtx, JSON.parse(await buffered.clone().text()));
} catch {
/* body may not be JSON; usage stays unreported */
}
}
return buffered;
}

Expand Down
39 changes: 39 additions & 0 deletions tests/responses-compaction-routing.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
} from "../src/codex/auth-context";
import { supportsNativeResponsesCompactEndpoint } from "../src/providers/openai-tiers";
import { acquireNativeMainProfileDrain, tryAdmitTurn } from "../src/server/lifecycle";
import type { RequestLogContext } from "../src/server/request-log";
import type { OcxConfig, OcxProviderConfig } from "../src/types";

const originalFetch = globalThis.fetch;
Expand Down Expand Up @@ -166,6 +167,44 @@ describe("supportsNativeResponsesCompactEndpoint (#422)", () => {
});
});

describe("native compact usage reporting", () => {
test("the buffered upstream body fills the request log usage and stays intact for the client", async () => {
const config = {
defaultProvider: "openai-apikey",
providers: {
"openai-apikey": {
adapter: "openai-responses",
baseUrl: "https://api.openai.com/v1",
authMode: "key",
apiKey: "sk-test",
},
},
} as unknown as OcxConfig;
globalThis.fetch = (async () => jsonResponse(completedPayload("native summary"))) as typeof fetch;
const logCtx: RequestLogContext = { model: "", provider: "" };
const previousUsageDebug = process.env.OPENCODEX_USAGE_DEBUG;
process.env.OPENCODEX_USAGE_DEBUG = "1";
let response: Response;
try {
response = await handleResponsesCompact(
compactionRequest(baseCompactionBody({ model: "openai-apikey/gpt-5.5" })),
config,
logCtx,
);
} finally {
if (previousUsageDebug === undefined) delete process.env.OPENCODEX_USAGE_DEBUG;
else process.env.OPENCODEX_USAGE_DEBUG = previousUsageDebug;
}
expect(response.status).toBe(200);
expect(await response.json()).toEqual(completedPayload("native summary"));
expect(logCtx.usage).toMatchObject({ inputTokens: 10, outputTokens: 5, totalTokens: 15 });
// The compact body is replacement history; even with usage debug on it must
// never be sampled into the debug log.
expect(logCtx.usageDebugBodyKind).toBeUndefined();
expect(logCtx.usageDebugBodySample).toBeUndefined();
});
Comment on lines +198 to +205

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the complete upstream payload.

Lines 189-190 check only body.usage. A regression that removes id, status, or output would still pass, although this PR must preserve the complete upstream body.

Proposed assertion 
-    const body = await response.json() as { usage?: Record<string, unknown> };
-    expect(body.usage).toMatchObject({ input_tokens: 10, output_tokens: 5, total_tokens: 15 });
+    const body = await response.json();
+    expect(body).toEqual(completedPayload("native summary"));
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/responses-compaction-routing.test.ts` around lines 188 - 192, Update
the response assertion in this test to validate the complete upstream payload,
not only body.usage. Assert that body also preserves the expected id, status,
and output fields while retaining the existing usage and logCtx.usage checks.

});
Comment on lines +170 to +206

@coderabbitai coderabbitai Bot Aug 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the remaining response-log branches.

This test checks usage and complete body preservation only. It does not exercise model or service_tier extraction in src/server/request-log.ts, Lines 489-504. It also does not verify the buffered.ok exclusion at src/server/responses/compact.ts, Line 513.

Add assertions for logCtx.resolvedModel and logCtx.responseServiceTier using response values that differ from the initial route metadata. Add a non-OK or synthetic buffering case and assert that existing request-log metadata remains unchanged.

As per path instructions, source behavior changes require focused regression coverage in tests/; cover these new branches here.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/responses-compaction-routing.test.ts` around lines 170 - 206, Extend
the “buffered upstream body” test around handleResponsesCompact to return
response data whose model and service_tier differ from the initial route
metadata, then assert logCtx.resolvedModel and logCtx.responseServiceTier are
populated from that response. Add a focused non-OK or synthetic buffering case
that exercises the buffered.ok exclusion and verifies existing request-log
metadata remains unchanged.

Source: Path instructions


describe("native Codex pool compaction", () => {
test("keeps a Spark reset cooldown separate from a Terra compact request (#590)", async () => {
const testDir = mkdtempSync(join(tmpdir(), "ocx-compact-scope-"));
Expand Down
Loading