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

[llm] Add app name for OpenRouter #4010

Merged
merged 2 commits into from
Sep 23, 2024
Merged

[llm] Add app name for OpenRouter #4010

merged 2 commits into from
Sep 23, 2024

Conversation

xingyaoww
Copy link
Contributor

@xingyaoww xingyaoww commented Sep 23, 2024

Short description of the problem this fixes or functionality that this introduces. This may be used for the CHANGELOG

OpenRouter has a way to tracking LLM usage from different App: https://openrouter.ai/


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

Following this, this PR adds the env var variable to make it easier for OpenRouter to track OpenHands' LLM usage.


Link of any specific issues this addresses

@xingyaoww xingyaoww requested review from tobitege and neubig and removed request for tobitege September 23, 2024 19:49
@enyst
Copy link
Collaborator

enyst commented Sep 23, 2024

Just a thought, I wonder if it's a bit cleaner if we add them in llm_config.py rather than llm.py. Specially since we seem to set them always, regardless if the user is using openrouter, they seem to belong in configurations. It's similar after all, with the cloud providers specific vars, of which we have a few.

@xingyaoww
Copy link
Contributor Author

@enyst good point! I've been thinking whether we need to handle then directly (e.g., check if openrouter is in the model name). But i think if the provider is not openrouter, litellm will not use these directly, which shouldn't create too much issue?

@enyst
Copy link
Collaborator

enyst commented Sep 23, 2024

I agree the overhead is not really an issue.

I just think they belong with other provider-specific stuff like aws_region_name, and those are defined in llm_config.py.

Even if we don't make them "actual" config values, just for code organization, I feel like they belong in that file.

@xingyaoww
Copy link
Contributor Author

@enyst good point! Having LLMConfig as the centralized place for all the stuff is probably a good idea. Just moved it to LLMConfig

@xingyaoww xingyaoww changed the title [llm] hard-code app name for OpenRouter [llm] Add app name for OpenRouter Sep 23, 2024
Copy link
Collaborator

@enyst enyst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking to move it completely out of llm.py 🤔 - but let's get this in, so you can focus on the fun edits stuff.

@enyst enyst merged commit 8ea2d61 into main Sep 23, 2024
13 checks passed
@enyst enyst deleted the xw/add-openrouter-appname branch September 23, 2024 22:26
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.

2 participants