feat: enhance token usage resolution for OpenAI and Anthropic compati…#223
feat: enhance token usage resolution for OpenAI and Anthropic compati…#223AlapinEnjoyer wants to merge 2 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "usage.prompt_tokens_details.cache_creation_tokens", | ||
| "data.usage.prompt_tokens_details.cache_creation_tokens", |
There was a problem hiding this comment.
Minor maintainability issue: These two new entries lack comments describing which providers or API formats use cache_creation_tokens under prompt_tokens_details. All other entries in these path lists have explanatory comments. Adding a comment (e.g., // OpenAI-compatible providers or similar) would keep the style consistent and help future maintainers understand when this path applies.
Suggestion:
| "usage.prompt_tokens_details.cache_creation_tokens", | |
| "data.usage.prompt_tokens_details.cache_creation_tokens", | |
| "usage.prompt_tokens_details.cache_creation_tokens", // OpenAI-compatible providers | |
| "data.usage.prompt_tokens_details.cache_creation_tokens", // wrapped OpenAI-compatible providers |
lizhengfeng101
left a comment
lizhengfeng101
left a comment
There was a problem hiding this comment.
Hey @AlapinEnjoyer, thanks for this PR! 🎉 Great catch on the missing cached_tokens field — that's the standard OpenAI field name and it's definitely needed. The tests are solid too. A few thoughts below:
1. Potential double-counting of TotalTokens (pre-existing, not introduced by this PR)
When total_tokens is absent from the response, the fallback calculation at usage_resolver.go:77-79 does:
ui.TotalTokens = prompt + completion + cacheRead + cacheWriteIn OpenAI's semantics, prompt_tokens already includes cached_tokens (cached tokens are a subset of prompt tokens), so the correct total would be prompt + completion — adding cache tokens on top double-counts them. In contrast, Anthropic's input_tokens excludes cache tokens, so the addition is correct there.
The test TestResolveUsageWrappedCachedTokens asserts TotalTokens == 205 (100+20+75+10), which would be a double-count under OpenAI semantics.
This is a pre-existing design choice, not something you introduced — but your PR does make more OpenAI-format responses reach this code path. Might be worth a follow-up to handle the two semantics differently, or at least a comment noting the trade-off. No action needed in this PR if you'd prefer to keep the scope focused.
2. Where does cache_creation_tokens come from?
The newly added paths:
usage.prompt_tokens_details.cache_creation_tokens
data.usage.prompt_tokens_details.cache_creation_tokens
This field name isn't part of OpenAI's standard response format (OpenAI doesn't expose cache creation), and it differs from Anthropic's cache_creation_input_tokens (which lives at the usage top level, not under prompt_tokens_details). Could you add a short comment noting which provider or proxy uses this field? It would help future readers understand where it comes from.
3. Minor: test idea for path priority
Since probePath uses first-match-wins, it might be worth adding a small test where both cached_tokens and cache_read_input_tokens (or cache_tokens_hit) are present in the same response, to verify the expected path takes priority. This would document the intended ordering.
Overall this is a solid improvement — the gap in OpenAI cached token tracking was real and this fixes it cleanly. Nice work on the test coverage too! 👍
…bility Recognize OpenAI cached_tokens and wrapped proxy response paths, add tests for path priority and provider-specific total token fallback semantics. Co-authored-by: Cursor <cursoragent@cursor.com>
AlapinEnjoyer
force-pushed
the
feat/recognize-openai-cached-tokens
branch
from
8e2443c to
c0b2d88
Compare
2 participants
Description
Cached token usage was not getting tracked for the OpenAI provider, the MR adds a way to recognize the common OpenAI-compatible cached-token fields for custom providers
Type of Change
How Has This Been Tested?
make testpasses locallyTested with a custom provider run
Checklist
go fmt,go vet)