-
Notifications
You must be signed in to change notification settings - Fork 14
fix: exclude cached tokens from chat usage #27
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.
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -913,10 +913,148 @@ function createFetchInterceptor( | |||||||||||||||||||||||||
| debug('Processing /v1/models response'); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| return normalizeChatUsageResponse(url, response); | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| async function normalizeChatUsageResponse(url: string, response: Response): Promise<Response> { | ||||||||||||||||||||||||||
| if (!response.ok || !url.includes('/chat/completions')) { | ||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const contentType = response.headers.get('content-type')?.toLowerCase() ?? ''; | ||||||||||||||||||||||||||
| if (contentType.includes('application/json')) { | ||||||||||||||||||||||||||
| return normalizeJsonChatUsageResponse(response); | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 The Roast: Missing 🩹 The Fix:
Suggested change
📏 Severity: critical
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 43ecd27: added the explicit await before normalizeJsonChatUsageResponse(response). Build, plugin tests, check:exports, and full npm test pass.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 The Roast: Missing 🩹 The Fix:
Suggested change
📏 Severity: critical
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 43ecd27: added the explicit await before normalizeJsonChatUsageResponse(response). Build, plugin tests, check:exports, and full npm test pass.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔥 The Roast: Missing 🩹 The Fix:
Suggested change
📏 Severity: critical
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 43ecd27: added the explicit await before normalizeJsonChatUsageResponse(response). Build, plugin tests, check:exports, and full npm test pass. |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (contentType.includes('text/event-stream')) { | ||||||||||||||||||||||||||
| return normalizeSseChatUsageResponse(response); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| async function normalizeJsonChatUsageResponse(response: Response): Promise<Response> { | ||||||||||||||||||||||||||
| let payload: unknown; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| payload = await response.clone().json(); | ||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (!isRecord(payload) || !normalizeCachedChatUsage(payload)) { | ||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const headers = cloneMutableResponseHeaders(response.headers); | ||||||||||||||||||||||||||
| headers.set('Content-Type', 'application/json'); | ||||||||||||||||||||||||||
| return new Response(JSON.stringify(payload), { | ||||||||||||||||||||||||||
| status: response.status, | ||||||||||||||||||||||||||
| statusText: response.statusText, | ||||||||||||||||||||||||||
| headers, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function normalizeSseChatUsageResponse(response: Response): Response { | ||||||||||||||||||||||||||
| if (!response.body) { | ||||||||||||||||||||||||||
| return response; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const decoder = new TextDecoder(); | ||||||||||||||||||||||||||
| const encoder = new TextEncoder(); | ||||||||||||||||||||||||||
| let pending = ''; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const stream = response.body.pipeThrough(new TransformStream<Uint8Array, Uint8Array>({ | ||||||||||||||||||||||||||
| transform(chunk, controller) { | ||||||||||||||||||||||||||
| pending += decoder.decode(chunk, { stream: true }); | ||||||||||||||||||||||||||
| const lines = pending.split('\n'); | ||||||||||||||||||||||||||
| pending = lines.pop() ?? ''; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| for (const line of lines) { | ||||||||||||||||||||||||||
| controller.enqueue(encoder.encode(`${normalizeSseChatUsageLine(line)}\n`)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
| flush(controller) { | ||||||||||||||||||||||||||
| const tail = pending + decoder.decode(); | ||||||||||||||||||||||||||
| if (tail) { | ||||||||||||||||||||||||||
| controller.enqueue(encoder.encode(`${normalizeSseChatUsageLine(tail)}\n`)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||
|
Comment on lines
+977
to
+982
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in d3466d4: the SSE flush path now appends a trailing newline for a final unterminated line. |
||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return new Response(stream, { | ||||||||||||||||||||||||||
| status: response.status, | ||||||||||||||||||||||||||
| statusText: response.statusText, | ||||||||||||||||||||||||||
| headers: cloneMutableResponseHeaders(response.headers), | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function normalizeSseChatUsageLine(line: string): string { | ||||||||||||||||||||||||||
| if (!line.startsWith('data:')) { | ||||||||||||||||||||||||||
| return line; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const rawData = line.slice(5).trim(); | ||||||||||||||||||||||||||
| if (!rawData || rawData === '[DONE]') { | ||||||||||||||||||||||||||
| return line; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| let payload: unknown; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| payload = JSON.parse(rawData); | ||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||
| return line; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (!isRecord(payload) || !normalizeCachedChatUsage(payload)) { | ||||||||||||||||||||||||||
| return line; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return `data: ${JSON.stringify(payload)}`; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function normalizeCachedChatUsage(payload: Record<string, unknown>): boolean { | ||||||||||||||||||||||||||
| const usage = payload.usage; | ||||||||||||||||||||||||||
| if (!isRecord(usage)) { | ||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const promptTokens = getNumber(usage.prompt_tokens); | ||||||||||||||||||||||||||
| const promptDetails = getRecord(usage.prompt_tokens_details); | ||||||||||||||||||||||||||
| const cachedTokens = getNumber(promptDetails?.cached_tokens); | ||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||
| promptTokens === undefined || | ||||||||||||||||||||||||||
| cachedTokens === undefined || | ||||||||||||||||||||||||||
| cachedTokens <= 0 || | ||||||||||||||||||||||||||
| cachedTokens > promptTokens | ||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // OpenCode tracks cached input separately, so prompt_tokens must be non-cached. | ||||||||||||||||||||||||||
| usage.prompt_tokens = promptTokens - cachedTokens; | ||||||||||||||||||||||||||
| const totalTokens = getNumber(usage.total_tokens); | ||||||||||||||||||||||||||
| if (totalTokens !== undefined) { | ||||||||||||||||||||||||||
| usage.total_tokens = totalTokens - cachedTokens; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||
|
Comment on lines
+1034
to
+1040
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When excluding cached tokens from
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in d3466d4: total_tokens is now reduced by cached_tokens together with prompt_tokens. |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function getNumber(value: unknown): number | undefined { | ||||||||||||||||||||||||||
| return typeof value === 'number' && Number.isFinite(value) ? value : undefined; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function getRecord(value: unknown): Record<string, unknown> | undefined { | ||||||||||||||||||||||||||
| return isRecord(value) ? value : undefined; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function cloneMutableResponseHeaders(headers: Headers): Headers { | ||||||||||||||||||||||||||
| const next = new Headers(headers); | ||||||||||||||||||||||||||
| next.delete('Content-Length'); | ||||||||||||||||||||||||||
| next.delete('Content-Encoding'); | ||||||||||||||||||||||||||
| return next; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const GEMINI_SCHEMA_KEYS_TO_REMOVE = new Set(['$schema', '$ref', 'ref', 'additionalProperties']); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| async function sanitizeGeminiToolSchemas( | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRITICAL: Missing
awaiton async function call.normalizeJsonChatUsageResponse(response)returnsPromise<Response>but the code returns the Promise without awaiting it. This causesnormalizeChatUsageResponseto return a Promise where Response is expected, breaking the response normalization.🩹 The Fix:
📏 Severity: critical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 43ecd27: added the explicit await before normalizeJsonChatUsageResponse(response). Build, plugin tests, check:exports, and full npm test pass.