fix(gateway): meter Anthropic streaming input tokens from message_start - #148
Conversation
Merge the message_start usage source so streamed input and cache tokens reach the request row and the budget counter. Closes theam#54.
|
I followed the EOF case separately from this fix. #148 looks good for the message_start and merge changes, but there’s one edge case left in the shared SSE path. |
|
Good catch, and I reproduced it. With the last frame valid but unterminated, the bytes still pass through byte-exact and usage comes back with On which behaviour Facility wants, I would argue for recovering it. The repo already takes that position elsewhere: in Strict SSE framing governs what you forward, and the tee already forwards untouched, so counting a frame it fully received breaks no contract with the caller. And the consequence is not a cosmetic metric: The change is small: route the flush remainder through |
adrian-lorenzo
left a comment
There was a problem hiding this comment.
Thanks for the solid contribution!
|
Thanks! @adrian-lorenzo. Happy to dig into the codebase. CI workflows still need a maintainer to approve the run since it's my first contribution, could someone hit the approve button when you get a chance? |
|
Not asking to reopen anything — the production fix is right and the approval looks right to me. This is about the test corpus, and it's a follow-up rather than a blocker.
export interface RawMessageDeltaEvent {
delta: RawMessageDeltaEvent.Delta; // stop_reason, stop_sequence, …
type: 'message_delta';
usage: MessageDeltaUsage; // <- sibling, not inside delta
}Two places still assume the nested shape. One is
That one's worth a two-word edit before it lands, since it's the explanation the next person will read. Why it matters beyond tidiness — two mutations to
So the streaming tests are load-bearing on a frame that never arrives, and the source that actually meters streamed output is pinned only by the non-streamed case. One practical note: I checked that my stub PR and this branch don't collide. Merged together locally, the gateway suite is 57/57, and your new I also have the fixture corrections for |
… shape Review follow-up on theam#181, after theam#148 landed: the integration stub now matches Anthropic's wire format, but the `MESSAGE_DELTA` unit fixture in `usage.test.ts` still nested `usage` inside `delta`, and the comment in `usage.ts` still documented output usage as living under `delta.usage`. The fixture now emits the real frame — `usage` as a sibling of `delta`, with `delta` carrying only the stop fields — and the comment states that shape. The `parsed.delta` merge source is deliberately kept as tolerance for relays that nest usage inside the delta, and the comment now says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…its (#181) * test(gateway): correct the Anthropic SSE stub to the shape the API emits anthropicSseBody() puts `usage` inside `delta` on message_delta. The API puts it at the top level, as a sibling of `delta`, and `delta` carries only the stop fields — see RawMessageDeltaEvent in @anthropic-ai/sdk, already a dependency of services/api. message_start is also thin: the real frame carries a full Message envelope, and usage there reports output_tokens alongside the input and cache counts. The shape matters because the stub is what every Anthropic streaming assertion in this suite is measured against. Reading the top-level `usage` is what meters streamed output tokens in production, and no test exercised that path: with the old stub, deleting the top-level source from UsageTee left the streaming tests green. The corrected stub also sends the null cache counters an uncached request reports, so the merge behaviour is exercised rather than assumed. No production behaviour changes; the suite passes unchanged (52/52). * test(gateway): align the unit fixture and usage comment with the wire shape Review follow-up on #181, after #148 landed: the integration stub now matches Anthropic's wire format, but the `MESSAGE_DELTA` unit fixture in `usage.test.ts` still nested `usage` inside `delta`, and the comment in `usage.ts` still documented output usage as living under `delta.usage`. The fixture now emits the real frame — `usage` as a sibling of `delta`, with `delta` carrying only the stop fields — and the comment states that shape. The `parsed.delta` merge source is deliberately kept as tolerance for relays that nest usage inside the delta, and the comment now says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Carlos Santana <1540596+ophiocus@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
What changes
message_start.message.usageinto the Anthropic streaming usage sources, so input and cache tokens are metered alongside the output tokensmessage_deltaalready reported.Why
Closes #54.
Anthropic reports streamed input and cache usage under
message_start.message.usage, whileUsageTeeread only the top-levelusageanddelta.usage. Streamed input tokens never reached metering, so with the existing 1M-input/1M-output fixture the gateway recorded 1500 cents against the 1800 cents owed. That undercounts thellm_requestsrow and thespend_countersrow a hard budget enforces against, so a project can outspend its cap on streamed traffic.The gap survived because the streaming test asserted only
outputTokens. It now asserts the input tokens, the row cost, and the counter, so an undercount fails the suite rather than the invoice.Verification
pnpm verifypasses locally — it reached the critical integration step and stopped on API timeout flakes unrelated to this change, detailed below.usage.tsalone and re-ran the new unit file: 4 of 5 tests fail with input tokens at 0, and the single pass is the non-streamed top-levelusagecase the change does not touch. Restored, all pass. The integration assertion moves 1500 → 1800 cents on both the request row and the spend counter.Commands run:
pnpm --filter @facility/gateway test— 57/57 passed against an isolatedfacility_gwpnpm --filter @facility/gateway typecheckpnpm lintpnpm guards— 2 guards ran, 0 failedgit diff --checkOn the
pnpm verifybox: the failures are inservices/api, which does not depend on@facility/gatewayand cannot import the changed file. The gateway suite also runs after the API suite, against a separate database. Running the same 42-file API suite three times gave three different results:test/assistant.test.tsfailed duringpnpm verify, unmodifiedmainpassed 42/42, and this branch produced two 5-second timeouts intest/watchtower.test.ts. That is the same class of unrelated 5-second API timeout reported in #143. Happy to re-run anything on request.Acceptance criteria from #54:
message_startinput and cache fields are metered; a latermessage_deltamerges output without erasing them; unit coverage spans valid, split-chunk, and malformed frames; and the integration proof runs against local Postgres and a stub provider, with no live credentials or network.