Repository navigation
fix(ext): normalize Semantic Kernel token usage - #8327
Thota Shashank (thotashashank302) wants to merge 3 commits into
Conversation
Christopher Krah (chrikrah)
left a comment
There was a problem hiding this comment.
Thota Shashank (@thotashashank302), approving at e587009. Usage now arrives from the real Semantic Kernel Anthropic connector on both paths. The six Anthropic and dict-usage cases fail without the fix.
$ cd python/packages/autogen-ext && pytest -q tests/models/test_sk_chat_completion_adapter.py
# e587009 (head), semantic-kernel 1.19.0 and anthropic 0.49.0 as in uv.lock
26 passed, 1 warning in 2.42s
# head tests, _sk_chat_completion_adapter.py restored from 027ecf0 (the PR's base)
6 failed, 20 passed, 1 warning in 2.15s
FAILED ...test_sk_chat_completion_token_usage[anthropic-create]
FAILED ...test_sk_chat_completion_token_usage[anthropic-create_stream]
FAILED ...test_sk_chat_completion_token_usage[anthropic-dict-create]
FAILED ...test_sk_chat_completion_token_usage[anthropic-dict-create_stream]
FAILED ...test_sk_chat_completion_token_usage[openai-dict-create]
FAILED ...test_sk_chat_completion_token_usage[openai-dict-create_stream]
$ python sk_probe.py # SKChatCompletionAdapter(AnthropicChatCompletion) over a stubbed AsyncAnthropic
# e587009
create RequestUsage(prompt_tokens=147, completion_tokens=1024)
create_stream RequestUsage(prompt_tokens=147, completion_tokens=1024)
# 027ecf0
create RequestUsage(prompt_tokens=0, completion_tokens=0)
create_stream RequestUsage(prompt_tokens=0, completion_tokens=0)
# not run: a live Anthropic call, and the rest of the autogen-ext suite
The probe sends message_start with input_tokens=147, a text event, then message_delta with output_tokens=1024. They pass through _send_chat_stream_request in the connector. The streamed count is right because the connector writes into one shared metadata["usage"] dict for all chunks.
non-blocking: the create_stream test puts usage on the final chunk only. A case that yields one shared metadata dict on every chunk, with input_tokens set before the first yield and output_tokens before the last, would pin that behaviour.
Victor Dibia (@victordibia), could you take a look and merge it if the shared-dict reading holds for you? cc Eric Zhu (@ekzhu)
|
Thanks for the review and the detailed probe, @chrikrah! I've added a test case in 321a096 that covers the shared metadata dictionary across chunks (setting input_tokens before the first yield and output_tokens before the last). Everything is passing locally and ready for review whenever @victordibia or @ekzhu have a moment. |
Christopher Krah (chrikrah)
left a comment
There was a problem hiding this comment.
Thota Shashank (@thotashashank302), approving at 321a096. The new shared-dict case fails against the base adapter, and it is the only test that catches an adapter which keeps the first usage it sees.
$ cd python/packages/autogen-ext && pytest -q -p no:cacheprovider tests/models/test_sk_chat_completion_adapter.py
# semantic-kernel 1.19.0, anthropic 0.49.0, Python 3.12.6
27 passed # head 321a096
7 failed, 20 passed # _sk_chat_completion_adapter.py from base 027ecf0, the new case included
1 failed, 26 passed # head, create_stream reads usage only while both counts are still 0
FAILED ...test_sk_chat_completion_stream_shared_usage_metadata
E assert RequestUsage(...tion_tokens=0) == RequestUsage(...n_tokens=1024)
$ mypy --config-file ../../pyproject.toml --python-version 3.12 <the two changed files>
Success: no issues found in 2 source files # mypy 1.13.0, strict = true
$ pyright <the two changed files>
0 errors, 0 warnings, 0 informations # pyright 1.1.414, typeCheckingMode strict
# not run: the rest of the autogen-ext suite
The mutant adds and not (prompt_tokens or completion_tokens) to the usage check in create_stream. The older stream cases put usage on the last chunk only, so they still pass with it.
Victor Dibia (@victordibia), every workflow on this PR is still waiting for approval, Checks included. Could you approve the run?
|
Thanks for running the mutant tests and for the approval, Christopher Krah (@chrikrah)! Appreciate the detailed verification. |
|
Hi Victor Dibia (@victordibia) Eric Zhu (@ekzhu), gentle follow-up on this PR when you get a chance. Christopher Krah has approved the changes and verified the tests/coverage locally. The branch is up-to-date, CLA is signed, and all required checks are ready to run once approved. Happy to address any feedback or questions! |
Why are these changes needed?
SKChatCompletionAdapterreports zero token usage for Anthropic responses because it only reads OpenAI'sprompt_tokensandcompletion_tokensattributes. For example, Claude usage withinput_tokens=147andoutput_tokens=1024becomesRequestUsage(0, 0), affecting the returned usage, cumulative counters, and logged usage.Normalize usage in both
create()andcreate_stream(). Accept SDK usage objects and mappings: Semantic Kernel's Anthropic connector returns an object for ordinary responses and a dictionary for streaming responses. Prefer OpenAI field names when present, including explicit zero values, and fall back to Anthropic names. Missing usage retains the existing zero-count behavior.Add 17 offline regression cases covering both request paths, Anthropic and OpenAI usage objects/dictionaries, field precedence, missing/null usage, accumulated totals across two calls, and usage events. The streaming coverage also reuses one shared metadata dictionary across chunks, setting input tokens before the first yield and adding output tokens before the last yield, as suggested in review.
Validation on macOS with Python 3.12.14, using
uv, lockfile-pinned Semantic Kernel/Anthropic dependencies and check tools:autogen-extwheel build andgit diff --check: passed.The full workspace/optional-provider test matrix and documentation build were not run locally. Tests use mocked responses; no live model APIs were called. Implementation and local validation were assisted by OpenAI Codex.
Related issue number
Fixes #5464
Checks