Skip to content

fix(gateway): meter OpenAI usage from the shapes the wire actually sends - #262

Merged
javiertoledo merged 2 commits into
theam:mainfrom
ophiocus:fix/gateway-openai-usage-shapes
Sep 4, 2026
Merged

fix(gateway): meter OpenAI usage from the shapes the wire actually sends#262
javiertoledo merged 2 commits into
theam:mainfrom
ophiocus:fix/gateway-openai-usage-shapes

Conversation

@ophiocus

@ophiocus ophiocus commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Closes #232 — and full credit to @Julian-Genuario, whose diagnosis was exact down to the sentence "the tests encode the wrong shapes." I claimed this in the issue thread with right of way offered; the courtesy window has passed, so here is the fix, built the way #181 was.

Stubs first, so the tests bite (the #181 pattern)

Both OpenAI stubs now emit what the wire actually sends — Chat Completions speaks prompt_tokens/completion_tokens (with prompt_tokens_details.cached_tokens on the final include_usage chunk), and the /responses stub streams the real envelope: usage nested inside {"type":"response.completed","response":{...}}, never at the frame's top level. With the corrected stubs alone, tests 3 and 3b fail against the old parser exactly as the issue predicted:

AssertionError: expected +0 to be 600000   // Chat streaming metered to zero
AssertionError: expected +0 to be 750000   // Codex/Responses streaming metered to zero

Then the parser

  • usageFromJson accepts both naming schemes; the stream path also reads the response.completed envelope (the wire Codex speaks) alongside top-level usage.
  • The double-charge is fixed at its root: OpenAI reports input INCLUSIVE of cached tokens while costCents sums buckets additively in the Anthropic convention — so cached tokens are subtracted from input, or they get billed at both the input and cache-read rates.

Pinned money math

The assertions now check the full buckets, not just presence: 1M prompt with 400k cached + 1M completion on gpt-5.5-mini = 216¢ (was silently 225 with cached double-billed and names lucky-matched); the streamed Codex path (1M/250k cached/200k out on gpt-5.6-sol) = 987.5¢ — previously zero, which is the hard-budget bypass in the issue title.

Gateway suite: 57 tests, 0 failures, tsc clean. Adjacent, not overlapping: #248/#251/#255 handle incomplete usage reconciliation; this handles usage that arrives complete but was unparseable.

The gateway parsed OpenAI usage with Responses-only, top-level names, so
Chat Completions (prompt_tokens/completion_tokens) and streamed Responses
(usage nested in the response.completed envelope - the wire Codex speaks)
both metered to zero: hard budgets never accumulated for Codex runs. The
stubs encoded the same wrong shapes, so the assertions could not catch it
(theam#232). Both stubs now emit the wire truth - with those stubs alone, tests
3 and 3b fail against the old parser at "expected +0 to be 600000/750000".
usageFromJson accepts both naming schemes, the stream path also reads the
response envelope, and cached tokens are subtracted from input: OpenAI
reports input INCLUSIVE of cached while costCents sums buckets additively
in the Anthropic convention, so the old accounting billed cached tokens
at both the input and cache-read rates. Metering assertions now pin the
full bucket math (600k/400k cached at 216 cents; 750k/250k/200k at 987.5).

Closes theam#232

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ophiocus
ophiocus force-pushed the fix/gateway-openai-usage-shapes branch from f7b2d46 to b8fe764 Compare September 3, 2026 18:25
@javiertoledo
javiertoledo self-requested a review September 3, 2026 20:56
usage.test.ts is the unit suite for this module and covered Anthropic only:
five cases including split frames and malformed ones, and nothing for OpenAI.
So the parsing this branch adds had integration coverage but no unit coverage,
and CLAUDE.md asks for both on a change to billing.

That gap matters more than the rule alone suggests. The integration test that
does cover these shapes lives in gateway.test.ts, which needs Postgres and
skips silently when it is unreachable - a local run reports "4 passed" with 32
tests skipped, so on a developer machine nothing exercises this at all. These
cases run everywhere.

Three cases, one per shape the branch teaches the parser:

- Chat Completions naming, with prompt_tokens inclusive of cached, so 1000
  reported with 400 cached meters 600 input and 400 cache-read. Reading only
  input_tokens/output_tokens meters the stream at zero, which is the
  hard-budget bypass; leaving input at 1000 bills the cached tokens at the
  input rate on top of the cache-read rate.
- The response.completed envelope Codex streams, where usage never appears at
  the frame's top level. The preceding response.created frame carries no usage
  and must not erase the total.
- A non-streamed body using the chat naming, asserting input stays as reported
  when no cached details accompany it.

All three fail against the previous parser with inputTokens 0. The five
Anthropic cases are untouched and still pass.

Addresses: review finding - services/gateway/test/usage.test.ts

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LyXT6m6npUH46VBuY1mPGM
@javiertoledo

Copy link
Copy Markdown
Member

Reviewed this and pushed one commit to the branch (maintainer_can_modify): bf57d4d, unit coverage only. Your usage.ts is untouched — I went looking for a reason to disagree with the approach and did not find one.

What I verified independently, driving the old and new parser directly rather than trusting the description:

main this branch
Chat Completions streaming {input:0, output:0, cacheRead:0} 216¢
Responses streaming (response.completed) {input:0, output:0, cacheRead:0} 987.5¢
Cached at input rate 1112.50¢ 987.50¢

Both streaming paths meter to exactly zero on main, so "hard budgets never accumulate for Codex runs" is precise, not rhetorical. I also confirmed costCents sums the four buckets additively — so the subtraction is right — and that every OpenAI model in MODEL_PRICES_USD_PER_1M carries a cacheRead price, which was the obvious risk of subtracting: cached tokens do not silently become free. Your 216¢ and 987.5¢ both reproduce exactly.

The one gap: no unit coverage. usage.test.ts is the unit suite for this module and covers Anthropic thoroughly — five cases, including frames split across chunk boundaries and malformed ones — and nothing for OpenAI. CLAUDE.md asks for unit and deterministic integration tests on a change to billing.

That gap is worse than the rule alone implies. The integration test that does cover these shapes needs Postgres and skips silently when it is unreachable: a local run here reported 4 passed with 32 tests skipped and exit 0. On a developer machine without the database, nothing exercised this at all.

So bf57d4d adds three cases, one per shape you taught the parser — Chat Completions naming with cached subtraction, the response.completed envelope (with the preceding response.created frame proving it does not erase the total), and a non-streamed body using the chat naming. All three fail against the previous parser with inputTokens: 0; the five Anthropic cases are untouched and still pass. usage.test.ts needs no database, so these run everywhere.

Gateway suite 28 passed / 1 skipped, tsc --noEmit, biome check and node guards/run.mjs all clean.

Two things I looked at and deliberately left alone: cacheWriteTokens reads only input_tokens_details while cacheRead reads both naming schemes — but neither OpenAI API reports a cache-creation count, so no wire shape reaches it, and the line predates this branch. And Math.max(0, …) clamps silently if cached ever exceeded input, which no observed shape produces. Neither is worth a commit.

@ophiocus

ophiocus commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Thank you — this is the kind of review a branch dreams about: an independent verification table, a commit that closes a real gap, and explicit notes on what you chose not to touch.

The unit-coverage point lands especially well. The silently-skipping integration suite is a masked failure of exactly the class this fix targets: 4 passed with 32 skipped reads as green on any machine without Postgres — the same shape as main metering streams to zero while looking alive. Putting the three wire shapes into usage.test.ts where they run everywhere is the right fix for that, and pairing each with the frame that could erase it (response.created before response.completed) is a nice touch.

On the two things you left alone, I read them the same way: no OpenAI wire shape reports a cache-creation count, so the asymmetric naming on cacheWriteTokens has nothing to receive; and the Math.max clamp can only fire on a shape no observed API emits. If you ever want that clamp loud instead of silent, a debug-level log inside it would make a future impossible-shape visible without changing behavior — but I agree neither belongs in this PR.

From my side the branch is exactly as you left it — b8fe764 + bf57d4d — and good to go.

@javiertoledo

Copy link
Copy Markdown
Member

Thanks — and agreed on leaving both out.

On the clamp: I'd go one step further and say a debug log is the version I'd argue against, precisely for the reason you spotted. 4 passed with 32 skipped and main metering a live stream to zero are the same failure — something reporting health while measuring nothing — and a debug line inherits that shape rather than breaking it. It fires on a branch nothing reaches, into a level nobody watches, so the first time it ever mattered it would already have scrolled past.

If that state is worth knowing about, the useful version is louder and lives elsewhere: metered usage that cannot be reconciled against what the provider reported is a signal for whatever already watches spend, not a line in the parser. That is a design question rather than a line change, and it belongs to whoever owns budget reconciliation — adjacent to the #248/#251/#255 work you flagged as out of scope here. Happy to open an issue framed that way if you want it tracked; I'd rather not file "add a debug log" as a ticket, since that is the kind that sits open forever.

Branch confirmed as you describe: b8fe764 + bf57d4d, CI green across all six checks — including verify, which is the one that actually stands Postgres up and runs the integration cases that skip locally.

One thing worth flagging for whoever merges: bf57d4d is test: and your commit is fix(gateway):, so the range still releases a patch and the PR title's impact matches. Nothing to change — just so the release classification isn't a surprise.

@javiertoledo
javiertoledo merged commit 81e101d into theam:main Sep 4, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Gateway meters OpenAI traffic wrong: streamed Responses and Chat usage parse to zero, and cached tokens are charged on top of input

2 participants