-
Notifications
You must be signed in to change notification settings - Fork 872
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
Fix bug in token usage merging logic for promptflow-tracing SDK #3793
Conversation
promptflow SDK CLI Azure E2E Test Result u/zhizhu/20240929-fix-tracing-bug 4 files 4 suites 4m 18s ⏱️ Results for commit c8ef3b2. ♻️ This comment has been updated with latest results. |
SDK CLI Global Config Test Result u/zhizhu/20240929-fix-tracing-bug6 tests 6 ✅ 1m 23s ⏱️ Results for commit c8ef3b2. ♻️ This comment has been updated with latest results. |
Executor Unit Test Result u/zhizhu/20240929-fix-tracing-bug798 tests 798 ✅ 3m 43s ⏱️ Results for commit c8ef3b2. ♻️ This comment has been updated with latest results. |
Executor E2E Test Result u/zhizhu/20240929-fix-tracing-bug246 tests 239 ✅ 5m 15s ⏱️ For more details on these failures, see this check. Results for commit c8ef3b2. ♻️ This comment has been updated with latest results. |
Description
The token calculation logic in promptflow-tracing merges two dictionaries related to token usage.
i.e. Given these two dictionaries:
The merged result will be:
However, for some customized models, there might be more additional fields populated in the dictionaries, like:
Which causes the merging logic crash:
This PR is to fix the issue. It converts
null
s to0
s to avoid Python from raising errors.All Promptflow Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines