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 issue #2947: Feat: make use of litellm's response "usage" data #5248

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

enyst
Copy link
Collaborator

@enyst enyst commented Nov 25, 2024

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions


Link of any specific issues this addresses
Fix #2947


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:23cab89-nikolaik   --name openhands-app-23cab89   docker.all-hands.dev/all-hands-ai/openhands:23cab89

@enyst enyst marked this pull request as draft November 25, 2024 04:32
@enyst
Copy link
Collaborator Author

enyst commented Nov 25, 2024

@openhands-agent Python linting is failing on this PR. You know how to run python linting in this project.

Important: run linting only on the files changed in this PR.

IMPORTANT:
You have access to a GITHUB_TOKEN, and so you can use the github API to get the diff of the PR, then run lint only on the files changed by it.

Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@All-Hands-AI All-Hands-AI deleted a comment from openhands-agent Nov 26, 2024
@All-Hands-AI All-Hands-AI deleted a comment from github-actions bot Nov 26, 2024
tests/unit/test_llm.py Outdated Show resolved Hide resolved
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@All-Hands-AI All-Hands-AI deleted a comment from openhands-agent Nov 26, 2024
@All-Hands-AI All-Hands-AI deleted a comment from github-actions bot Nov 26, 2024
@enyst
Copy link
Collaborator Author

enyst commented Nov 27, 2024

We have a problem. In get_token_count method, what type is the messages parameter? Verify three things:

  • what type(s) does the litellm token counter function take. You have options how to do that, such as a small script to inspect it.
  • what type(s) does our code, that uses get_token_count, send over
  • are they both compatible with the type(s) we get in Usage?

Verify and fix accordingly.

@enyst enyst added the fix-me Attempt to fix this issue with OpenHands label Nov 27, 2024
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@All-Hands-AI All-Hands-AI deleted a comment from github-actions bot Nov 27, 2024
@All-Hands-AI All-Hands-AI deleted a comment from openhands-agent Nov 27, 2024
@enyst
Copy link
Collaborator Author

enyst commented Nov 27, 2024

@openhands-agent python linting is failing on this PR. You know how to do python lint in this project. Do it.

Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@openhands-agent
Copy link
Contributor

New OpenHands update

Copy link
Contributor

The workflow to fix this issue encountered an error. Please check the workflow logs for more information.

@enyst
Copy link
Collaborator Author

enyst commented Nov 27, 2024

You're doing great, but there is now one unit test that's failing. You know how to run unit tests in your environment. Run them, see what is failing, and fix it. Take into account previous comments and the issue we're fixing in this PR, so you understand the desired behavior of that test.

@enyst enyst added fix-me Attempt to fix this issue with OpenHands and removed fix-me Attempt to fix this issue with OpenHands labels Nov 27, 2024
Copy link
Contributor

OpenHands started fixing the pr! You can monitor the progress here.

@openhands-agent
Copy link
Contributor

New OpenHands update

Copy link
Contributor

The workflow to fix this issue encountered an error. Please check the workflow logs for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix-me Attempt to fix this issue with OpenHands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: make use of litellm's response "usage" data
2 participants