fix: add cache write accounting to OpenAI interceptors - #28567
Conversation
| "rejected_prediction_tokens": 7 | ||
| } | ||
| }, | ||
| "service_tier": "default", |
There was a problem hiding this comment.
Ah interesting, we already have service_tier in the fixtures 👀
| lastErr = xerrors.Errorf("marshal chunk: %w", err) | ||
| break | ||
| } | ||
| if err := events.Send(ctx, payload); err != nil { |
There was a problem hiding this comment.
I'm not sure if this is correct 🤔 so we relay the chunks back to the client, and in each iteration we keep accumulating it?
For instance, assume 3 iterations:
- Iteration 1: previousUsage = 0, prc.lastUsage = u1 => cumulativeUsage: u1 => token usage u1 send to the client
- Iteration 2: previousUsage = u1, prc.lastUsage = u2 => cumulativeUsage: u1+u2 => token usage u1+u2 send to the client
- Iteration 3: previousUsage = u1+u2, prc.lastUsage = u3 => cumulativeUsage: u1+u2+u3 => token usage u1+u2+u3 send to the client
So basically the client would receive something like:
data: {... "usage": u1}
data: {... "usage": u1+u2}
data: {... "usage": u1+u2+u3}
There was a problem hiding this comment.
This is WAI.
It used to work like that (or worked if it stream would only contain 1 usage chunk) until ad10045
Linked commit fixed issue where some OpenAI compatible providers return multiple usages for one request which resulted in old solution counting all usages from all chunks from all streams which was too much but also dropped the sum of all inner loop requests feature.
ssncferreira
left a comment
There was a problem hiding this comment.
LGTM 🤞 just a question about a case that I'm not sure can happen
| lastUsage := processor.lastUsage | ||
| // Preserve the existing recording gate, but accumulate every valid | ||
| // snapshot so later client-visible usage cannot move backwards. | ||
| if lastUsage.CompletionTokens > 0 { |
There was a problem hiding this comment.
I see this was already before 👀 but could it happen that CompletionTokens == 0 but cache read/write > 0?
There was a problem hiding this comment.
This test is kinda related to the above comment, it is only cumulative because the previous usage has a completion_tokens > 0. If all chunks have completion_tokens = 0, there would be no accumulation, which would also affect cost control, because we don't store RecordTokenUsage 🤔
87bd945 to
ea6b520
Compare
…recording token usage in chat completion interceptor and improve TestStreamProcessorZeroCompletionUsage test to accutally test something
ea6b520 to
6162b6a
Compare

Adds cache write token accounting to OpenAI interceptors.
This should automatically be picked up by const control.