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(settings): Add fallback in case models httpRequest returns a JsonArray instead of JsonObject #146

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ETdoFresh
Copy link

I wanted to have support for https://www.together.ai/. However, AI Commits did not work with this endpoint out the box. I found out that the results from the "models" call differed.

From OpenAI, a call to https://api.openai.com/v1/models looked something like this:

{
    "object": "list",
    "data": [
        {
            "id": "dall-e-3",
            "object": "model",
            "created": 1698785189,
            "owned_by": "system"
        },
        ...
    ]
}

Where as Together.ai, a call to https://api.together.xyz/v1/models looked something like this:

[
    {
        "id": "Austism/chronos-hermes-13b",
        "object": "model",
        "created": 1692896905
    },
    ...
]

So, I ended up trying openai.models() first, and if it failed, fall back to parsing the jsonarray manually. Which leads me to another pain point, which was that owned_by is also not present in together.ai's model. So fallback only requires id and created.

If this is helpful to others, feel free to use this. Here's a video of it in action...

java_zuqwE6Wfgr.mp4

@Blarc
Copy link
Owner

Blarc commented Mar 4, 2024

Hey @ETdoFresh,

thanks for opening this PR. I was thinking about supporting different providers anyway, so I was planning to implement this a bit more generically via interface class, so new providers can easily be added. (Since I already had requests for other providers #132)

I'll look into this ASAP.

@ETdoFresh
Copy link
Author

ETdoFresh commented Mar 4, 2024

Excellent! Yeah, feel free to close this PR if you already have plans for this. I found out I had to make another tweak because created was required on openAI models call as well, but when I use LLM Studio, it did not have a created field when calling models. Sooo... yeah!

And I went ahead and ran "trim" on the commit message as well, because sometimes I had leading spaces.

My changes will be on my personal modified version of main branch... but yeah, let me know if there is anything I can do to help.

@Blarc
Copy link
Owner

Blarc commented Jun 8, 2024

Hey @ETdoFresh,

I have refactored the code. I now use langchain4j for creating LLM API clients. Even though together.ai is currently not supported by langchain4j, adding it to the plugin should now be easier.

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