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

Replace litellm with native API implementations. #1252

Merged
merged 10 commits into from
Mar 18, 2025
Merged

Conversation

blkt
Copy link
Contributor

@blkt blkt commented Mar 7, 2025

Refactors client architecture to use native implementations instead of litellm dependency. Adds support for OpenAPI, Ollama, OpenRouter, and fixes multiple issues with Anthropic and Copilot providers. Improves message handling and streaming responses.

Commit message brought you by Anthropic Claude 3.7.

@blkt blkt assigned blkt and jhrozek Mar 7, 2025
@JAORMX
Copy link
Contributor

JAORMX commented Mar 7, 2025

Omaigá

@blkt
Copy link
Contributor Author

blkt commented Mar 7, 2025

This pull request is humongous, sorry about that, but it turns out that almost every piece of code with the exception of the database layer was relying on litellm, implicitly or otherwise, and I could not find another way to refactor the code in steps without doubling the amount of code. All the pain this brings is my fault, apologies for that.

To make it easier to review, you can think of this refactoring as three big chunks of work

  • adding a new representation: this is fully encapsulated in codegate.types module and its submodules, that's logically ~100% new code
  • porting the pipeline to the new representation: this is all the work required to change event["messages"] to event.get_messages(); the interface should be simple enough and is based on generators
  • muxing: muxing required changing how the wiring is done, we can discuss this further on code, but it only required adding a few routines to map openai->muxed-llm and back, plus optional parameters to some functions used within providers

Feel free to pull me in a call if necessary, but my last suggestion would be to just try it out locally and see if it works.

@blkt blkt force-pushed the chore/replace-litellm branch from 6c342b7 to b67b829 Compare March 8, 2025 09:19
@lukehinds
Copy link
Contributor

I spent quite some time testing this over the weekend, mostly continue and some copilot (albeit factoring around know issues), seems good to me and I think we should move towards merging this after a rebase , is there anything in particular you wanted testing / checking over @blkt @jhrozek ?

@blkt
Copy link
Contributor Author

blkt commented Mar 17, 2025

Hey @lukehinds , thank you so much for spending time testing and reviewing this! 🙏

I would be inclined to merge this, although we have some integration tests failures due to an undesired (or unexpected) compatibility of muxing with Ollama-specific APIs.

TL;DR is that, by design, muxing was expecting to talk to OpenAI endpoints, but litellm allowed also Ollama endpoints to work because it internally maps everything to OpenAI types. We don't have that capability in this new implementation, so this would be a breaking change, but being "bug-compatible" would add more code, and I'd rather avoid that.

My proposal is to

  • rebase
  • remove the failing integration tests (or rewrite them to use OpenAI endpoints if removing them lowers coverage)
  • merge

Please folks, chime in if you don't agree. Meanwhile, I'll start making the described changes.

@blkt blkt force-pushed the chore/replace-litellm branch 5 times, most recently from 25a35ad to 4aab11c Compare March 17, 2025 19:01
lukehinds
lukehinds previously approved these changes Mar 18, 2025
blkt and others added 7 commits March 18, 2025 11:11
Refactors client architecture to use native implementations instead of
`litellm` dependency. Adds support for OpenAPI, Ollama, OpenRouter,
and fixes multiple issues with Anthropic and Copilot
providers. Improves message handling and streaming responses.

Commit message brought you by Anthropic Claude 3.7.

Co-Authored-By: Jakub Hrozek <[email protected]>
This was missed.

Signed-off-by: Juan Antonio Osorio <[email protected]>
This change aims to make it simpler to track down in which step of the
pipeline a particulare exception occurred.
lukehinds
lukehinds previously approved these changes Mar 18, 2025
@blkt blkt force-pushed the chore/replace-litellm branch from bf8d9e8 to 1452eff Compare March 18, 2025 12:31
@lukehinds lukehinds merged commit 05b134a into main Mar 18, 2025
7 of 11 checks passed
@lukehinds lukehinds deleted the chore/replace-litellm branch March 18, 2025 13:40
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.

4 participants