Skip to content
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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ivanbelenky
Copy link

@ivanbelenky ivanbelenky commented Oct 21, 2024

Describe your changes

The change solve create_with_completion failing to output the correct token usage: total_usage: CompletionUsage now is initialized with both PromptTokenDetails and CompletionTokenDetails fields. I am aware that this implies overriding to some degree the optionality of this attributes, nevertheless I think 0 is a good alias for the information that None in this context conveys.

def initialize_usage(mode: Mode) -> CompletionUsage | Any:
    ...
    total_usage = CompletionUsage(completion_tokens=0, prompt_tokens=0, total_tokens=0,
        completion_tokens_details = CompletionTokensDetails(audio_tokens=0, reasoning_tokens=0),
        prompt_token_details = PromptTokensDetails(audio_tokens=0, cached_tokens=0)
    )
    ...

and this values are respectfully filled out whenever possible

def update_total_usage(...) -> ...:
    ...
    if isinstance(response_usage, OpenAIUsage) and isinstance(total_usage, OpenAIUsage):
        total_usage.completion_tokens += response_usage.completion_tokens or 0
        total_usage.prompt_tokens += response_usage.prompt_tokens or 0
        total_usage.total_tokens += response_usage.total_tokens or 0

        if (rtd := response_usage.completion_tokens_details) and (ttd := total_usage.completion_tokens_details):
            ttd.audio_tokens = (ttd.audio_tokens or 0) + (rtd.audio_tokens or 0)
            ttd.reasoning_tokens = (ttd.reasoning_tokens or 0) + (rtd.reasoning_tokens or 0)

        if (rpd := response_usage.prompt_tokens_details) and (tpd := total_usage.prompt_tokens_details):
            tpd.audio_tokens = (tpd.audio_tokens or 0) + (rpd.audio_tokens or 0)
            tpd.cached_tokens = (tpd.cached_tokens or 0) + (rpd.cached_tokens or 0)

        response.usage = total_usage  # Replace each response usage with the total usage

Related Issue


Important

Fixes token usage reporting by initializing CompletionUsage with token details and updating aggregation in retry.py and utils.py.

  • Behavior:
    • Fixes incorrect token usage reporting by initializing CompletionUsage with CompletionTokensDetails and PromptTokensDetails in initialize_usage() in retry.py.
    • Updates update_total_usage() in utils.py to aggregate audio_tokens, reasoning_tokens, and cached_tokens.
  • Imports:
    • Adds CompletionTokensDetails and PromptTokensDetails to imports in retry.py.

This description was created by Ellipsis for 60d549a. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a 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 in 2 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 in update_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 of CompletionTokensDetails and PromptTokensDetails is necessary for the changes in initialize_usage. Ensure these classes are defined and available in the openai.types.completion_usage module.
  • Reason this comment was not posted:
    Confidence changes required: 20%
    The import statement in instructor/retry.py for CompletionTokensDetails and PromptTokensDetails is correct and necessary for the changes made in the initialize_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 in update_total_usage. This applies to other assertions in the function as well.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The function update_total_usage in instructor/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant