Don't report transcript usage the proxy already priced, or a turn that hasn't finished (0.23.0) - #40
Merged
Conversation
…t hasn't finished (0.23.0) Two accounting defects in the hook's transcript collector, both found while auditing our own cost numbers on 2026-09-17. 1. A proxied session was reporting turns the proxy had already priced. collectTranscriptUsage ran on every PostToolUse with no check for where the model calls actually went. It exists for plain `claude` sessions that never touch the proxy — but a `claude-acp` session sends its calls through the proxy AND fires this hook, so every call was priced twice. On our own workspace: llm.proxy.anthropic and llm.transcript.anthropic rows ~100ms apart carrying identical cost, on every sampled turn. Gated on the base URL, not on ACP_KEY. launchedViaLauncher() exists next door and would have been the obvious call, but it also returns true when ACP_KEY alone is set — a credential someone can export while running plain `claude`, whose model calls still go straight to the provider. Suppressing those would lose the only record of their spend. Only the base URL says where the tokens actually went, and a pricing decision has to follow that rather than the credential. 2. A turn still streaming was reported from its partial record. One API call is appended to the transcript as many records, one per content block. cache_read / cache_creation / input_tokens are fixed at request time and identical on every record, but output_tokens is a placeholder — commonly 1-8 — until the final record, which is the only one carrying a terminal stop_reason. PostToolUse fires between a turn's records, so a turn can straddle two read windows; the first window shipped the placeholder and the gateway's first-wins dedupe locked it in, dropping the real total when it arrived. Measured across a month of local transcripts: 2,748 calls sitting at output_tokens=3 and 2,263 at =2 on their partial rows, worst case 39.9% of output tokens lost while cache-read stayed exact. So a turn is reported only once its terminal stop_reason has been seen, and the offset is held at the first byte of the first incomplete turn so its remaining records are re-read next time. A turn that never completes — an aborted or interrupted request, ~0.7% of calls — is never reported; losing those beats billing a placeholder for them, and the stall is bounded because TRANSCRIPT_READ_CAP eventually slides the window past it. Defect 1 also has a gateway-side guard (proxy/proxySessions.ts), which is what covers harnesses and already-installed plugins that never upgrade. This half is what stops the duplicate being generated at all. One existing test fixture gained a stop_reason: the id-fallback test built turns without one, which under the new rule are incomplete by definition. Its assertion — the requestId/uuid fallback chain — is unchanged. Note for release: the hook's content hash changes with this, so the registry needs the 0.23.0 hash AFTER merge, never before: ee8de59ae945688870070640d2cb204732c6a475a73e9796f303d83f47297627
davidcrowe
added a commit
that referenced
this pull request
Sep 21, 2026
…ehind a held turn; cap the held region (0.24.0) (#41) gatewaystack-connect#1279, items 1 and 2, client half. Two accounting defects in the hook's transcript collector, both introduced or exposed by #40. 1. Silent under-count on any failure. collectTranscriptUsage wrote the new offset BEFORE the request went out; handlePostToolUse then exited on !res.ok and swallowed abort/network errors. There were no retries, and model_usage is not in the offline ledger, so a 4xx/5xx/timeout on /govern/tool-output lost that window's turns for good. Now the collector returns { turns, ack, fail } and the offset moves only in ack(), which runs after a 2xx. Any other outcome — non-2xx, abort, connection refused — calls fail(): the offset stays, `attempts` is recorded in transcript-offsets.json, and the next PostToolUse re-reads the same window and sends it again. Bounded: after 3 attempts on the same batch the offset moves past it, the loss is one JSON line in ~/.acp/lapse.log (transcript-usage-dropped, logged on the first drop per transcript and counted after), and the ids are NOT pretended acked. 2. Re-sends were the steady state. #40's holdAt pinned the offset at the first incomplete turn so its remaining records get re-read — and every completed turn behind it was re-SENT on every later tool call for the rest of the session, relying on the gateway's per-process Set to absorb it. Worse than the issue describes: holdAt was fixed at the first partial record the loop met, so a turn whose final record landed later in the SAME window still pinned the offset. The existing "withheld, then billed" test now also asserts the offset releases. The offsets file entry is now { off, acked, attempts, dropped } (a bare number, the pre-0.24.0 shape, still reads). `acked` keeps the last 200 turn ids the gateway acknowledged for that transcript; they are filtered out before a batch is built, so the steady state behind a hold is a bounded re-read and an EMPTY send. A re-send now happens in exactly one case: a 2xx whose response was lost in transit. The gateway's durable (session, id) dedupe (gatewaystack-connect PR, same issue) is what absorbs that. 3. The held region is capped at 256 KB. Past that the offset moves on, one lapse line says so (transcript-hold-cap), and the incomplete turn is abandoned — unless its final record does land later, in which case it is a complete turn in a fresh window and bills at its real total. Previously only TRANSCRIPT_READ_CAP (2 MB) bounded the stall, which a normal session never reaches, so every tool call paid the re-read. Tests (test/transcript-model-usage.test.mjs, 23 cases): a 503 leaves the offset and the next call re-sends the identical window; connection refused is a failed send too; three failures drop the batch with one lapse line and the gateway coming back does not re-send it; the legacy bare-number offsets file still reads; the second PostToolUse after a held turn sends zero already-acked ids and the hold releases when the turn completes; the acked set is capped at 200 newest; a 300 KB held region is advanced over once. Note for release: the hook's content hash changes with this, so the registry needs the 0.24.0 hash AFTER merge, never before: fddeccaa3b1db29fb48d1ebd95fc6a81333576105fc19df0ae1c11c6ea639ca5 Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two accounting defects in the hook's transcript collector, both found while auditing our own cost numbers on 2026-09-17.
1. A proxied session reported turns the proxy had already priced
collectTranscriptUsageran on every PostToolUse with no check for where the model calls actually went. It exists for plainclaudesessions that never touch the proxy — but aclaude-acpsession sends its calls through the proxy and fires this hook, so every call was priced twice.Measured on our own workspace:
llm.proxy.anthropicandllm.transcript.anthropicrows ~100ms apart carrying identical cost, on every sampled turn.Gated on the base URL, not on
ACP_KEY.launchedViaLauncher()exists next door and was the obvious call, but it also returns true whenACP_KEYalone is set — a credential someone can export while running plainclaude, whose model calls still go straight to the provider. Suppressing those would delete the only record of their spend. Only the base URL says where the tokens actually went, and a pricing decision has to follow that rather than the credential. (It would also have silently emptied every existing test intranscript-model-usage.test.mjs, since the harness setsACP_KEY— they'd have "passed" by doing nothing.)2. A turn still streaming was reported from its partial record
One API call is appended to the transcript as many records, one per content block.
cache_read/cache_creation/input_tokensare fixed at request time and identical on every record, butoutput_tokensis a placeholder — commonly 1–8 — until the final record, which is the only one carrying a terminalstop_reason.PostToolUse fires between a turn's records, so a turn can straddle two read windows. The first window shipped the placeholder, and the gateway's first-wins dedupe locked it in, dropping the real total when it arrived.
Measured across a month of local transcripts: 2,748 calls sitting at
output_tokens=3and 2,263 at=2on their partial rows. Worst case 39.9% of output tokens lost, while cache-read stayed exact — that asymmetry is the signature of the mechanism.So a turn is reported only once its terminal
stop_reasonhas been seen, and the offset is held at the first byte of the first incomplete turn so its remaining records are re-read next time. A turn that never completes — aborted or interrupted, ~0.7% of calls — is never reported; losing those beats billing a placeholder for them, and the stall is bounded becauseTRANSCRIPT_READ_CAPeventually slides the window past it.Turns that complete after an incomplete one are still sent; because the offset holds, they may be re-sent next window, where the gateway's
markSeenabsorbs them. No double-count.Relationship to the gateway fix
Defect 1 also has a gateway-side guard (
gatewaystack-connect#1236,proxy/proxySessions.ts), which is what covers harnesses and already-installed plugins that never upgrade. This half stops the duplicate being generated at all. The two are independent — either can ship first.Test note
One existing fixture gained a
stop_reason: the id-fallback test built turns without one, which under the new rule are incomplete by definition. Its assertion — therequestId/uuidfallback chain — is unchanged. Four new tests cover the withhold-then-bill sequence, the mixed complete/incomplete case, the proxy gate, and a plain session still reporting (so the gate can't pass by disabling the feature).node --test test/→ 247 passing.Release
The hook's content hash changes with this. The registry needs the 0.23.0 hash after merge, never before: