-
-
Notifications
You must be signed in to change notification settings - Fork 689
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: OpenAI prompt details and completion tokens details missing from total usage #1105
Conversation
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.
👍 Looks good to me! Reviewed everything up to 60d549a in 18 seconds
More details
- Looked at
43
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
3
drafted comments based on config settings.
1. instructor/utils.py:143
- Draft comment:
Ensure the use of the walrus operator:=
is compatible with the minimum Python version supported by the project. This operator is only available in Python 3.8 and later. - Reason this comment was not posted:
Confidence changes required:50%
The code inupdate_total_usage
uses the walrus operator:=
which is only available in Python 3.8 and later. This should be noted if the codebase supports older Python versions.
2. instructor/retry.py:17
- Draft comment:
The import ofCompletionTokensDetails
andPromptTokensDetails
is necessary for the changes ininitialize_usage
. Ensure these classes are defined and available in theopenai.types.completion_usage
module. - Reason this comment was not posted:
Confidence changes required:20%
The import statement ininstructor/retry.py
forCompletionTokensDetails
andPromptTokensDetails
is correct and necessary for the changes made in theinitialize_usage
function.
3. instructor/utils.py:140
- Draft comment:
Assertions should always have an error message that is formatted well. Please add error messages to the assertions inupdate_total_usage
. This applies to other assertions in the function as well. - Reason this comment was not posted:
Confidence changes required:80%
The functionupdate_total_usage
ininstructor/utils.py
has multiple instances of assertions without error messages. This violates the rule that assertions should always have an error message that is formatted well.
Workflow ID: wflow_gfxY8ZwYYs4u5kQM
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
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.
Looks good to me! Testing it using O1 and the reasoning tokens are now updated well.
from openai import OpenAI
from pydantic import BaseModel, field_validator
import instructor
from rich import print
from instructor.mode import Mode
client = instructor.from_openai(OpenAI(), mode=Mode.JSON_O1)
class Person(BaseModel):
name: str
age: int
resp, usage = client.chat.completions.create_with_completion(
model="o1-mini",
response_model=Person,
messages=[
{
"role": "user",
"content": "What's a plausible name for a 20 year old person living in Mongolia?",
},
],
)
print(resp)
# Person(name='Bat-Erdene', age=20
print(usage.usage)
# CompletionUsage(
# completion_tokens=546,
# prompt_tokens=153,
# total_tokens=699,
# completion_tokens_details=CompletionTokensDetails(
# accepted_prediction_tokens=None,
# audio_tokens=0,
# reasoning_tokens=512,
# rejected_prediction_tokens=None,
# ),
# prompt_tokens_details=None,
# prompt_token_details=PromptTokensDetails(audio_tokens=0, cached_tokens=0),
# )
Describe your changes
The change solve
create_with_completion
failing to output the correct token usage:total_usage: CompletionUsage
now is initialized with bothPromptTokenDetails
andCompletionTokenDetails
fields. I am aware that this implies overriding to some degree the optionality of this attributes, nevertheless I think0
is a good alias for the information thatNone
in this context conveys.and this values are respectfully filled out whenever possible
Related Issue
Important
Fixes token usage reporting by initializing
CompletionUsage
with token details and updating aggregation inretry.py
andutils.py
.CompletionUsage
withCompletionTokensDetails
andPromptTokensDetails
ininitialize_usage()
inretry.py
.update_total_usage()
inutils.py
to aggregateaudio_tokens
,reasoning_tokens
, andcached_tokens
.CompletionTokensDetails
andPromptTokensDetails
to imports inretry.py
.This description was created by for 60d549a. It will automatically update as commits are pushed.