Skip to content

dynamic model list #107

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

Merged
merged 6 commits into from
Jun 16, 2023
Merged

dynamic model list #107

merged 6 commits into from
Jun 16, 2023

Conversation

tarasglek
Copy link
Owner

@tarasglek tarasglek commented Jun 14, 2023

Addresses #106 by querying openai for available models, also gets us ready for claude support.

new chatgpt model is using date suffix, so mark that as chatgpt and mark old one with date. Will change that after 27th.

Left for followup:

  • Need to list models for various places in UI. Need to decide on what we wanna do to reduce calls to models endpoint. (probably persist in cache if not called from options dialog)
  • Retry-with needs to use this list
  • Ask GPT-4/ChatGPT
  • Should also use this api endpoint for validating api key
  • Costs
image

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jun 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5554b61
Status: ✅  Deploy successful!
Preview URL: https://287f8767.console-overthinker-dev.pages.dev
Branch Preview URL: https://taras-gpt-models.console-overthinker-dev.pages.dev

View logs

@tarasglek tarasglek requested a review from humphd June 14, 2023 06:11
@humphd
Copy link
Collaborator

humphd commented Jun 14, 2023

This is awesome. I've pulled your branch locally, and started doing some review, fixes, and additions on top of it. I'll push up the changes in the next day or so.

@humphd
Copy link
Collaborator

humphd commented Jun 15, 2023

I've added a new commit to this. It does a number of things:

  • Adds a new React context/hook, useModels() which returns a list of models from the OpenAI call, cached in memory. It also returns an error if the API can't be called (e.g., API key is invalid). I use this to show an error toast, which validates the key.
  • Everywhere we used a GptModel before, which was essentially a string, I've switched to ChatCraftModel. This required some major changes to how the data serialization/deserialization works, though I needed to do this anyway, so it was a good time to pay off some tech debt. In the db and in JSON the model gets serialized to a string, but rehydrated to ChatCraftModel in the app whenever we use it.
  • With the db cleanup, I've moved a bunch of db operations to ChatCraftChat and ChatCraftMessage.
  • I've updated the constructor to ChatCraftModel to take an optional vendor, which we can start using later with other model vendors. For now, it will default to OpenAI, freeing us from having to add it to the modelId.

I think this needs a bunch of testing, and I want to see make sure i won't break current databases with this--I'm 95% sure I won't, but let's not land this yet.

One cool thing about this PR is that it helps guide the user to see exactly which models their API key supports. For example, my OpenAI key does not support GPT-4, but @tarasglek's does, and when I switch keys, I get a different list of models everywhere we show them. That's very cool.

@tarasglek
Copy link
Owner Author

tarasglek commented Jun 16, 2023

Thanks for this extensive followup. I been using this, have only one observations:

  1. I would prefer "retry with" to have a submenu instead of listing ever-increasing model count inline.
image
  1. Once you have a model explosion, kinda interesting to summarize the differences.
image In this case all 3 models generate same content..but they took vastly different time delays to do that. Something to consider would be to include hash of output and time to generate somewhere.

@humphd
Copy link
Collaborator

humphd commented Jun 16, 2023

I think both of these could be follow-ups.

For 1. we'll have to figure out a good way to do this via Chakra-UI, which doesn't natively support submenus (first time it's let me down so far). Thinking about this connected to other discussions, I wonder if we should have a Retry with different model... menu that opens a model where you can pick a model + set model params. The submenu idea won't give us enough UI surface to support setting different model params, so maybe it's time to split it out into its own thing?

For 2., would a hash really help here? I find that responses from different models are often very, very similar but usually not identical, so a hash would basically always be different and hide the similarity. Maybe there is a way to do similarity scoring? Not sure how you do that without setting one of them as a baseline (maybe the current version is baseline?). Also, is time a useful indicator, since it's dependent on a bunch of things unrelated to the model (e.g., network and API congestion, speed of things in user's browser, etc). When I query these models, I find that GPT-4 can be faster than GPT-3.5 on some days, which is clearly not due to the speed of the model.

@humphd
Copy link
Collaborator

humphd commented Jun 16, 2023

OK, I've spent some more time reading this change, and I think we're safe to land it. From a data-loss perspective, this change is widening the type of GptModel from "gpt-4" | "gpt-3.5-turbo" to string in two places: one in db schema for ChatCraftMessageTable's model and model in versions records, another in the JSON format SerializedChatCraftMessage. This won't cause data loss, and doesn't need a migration/db version update.

@tarasglek
Copy link
Owner Author

pushed a fix to "retry with". it was using model from settings, not from menu.

Copy link
Collaborator

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Nice catch

@tarasglek
Copy link
Owner Author

I think both of these could be follow-ups.

For 1. we'll have to figure out a good way to do this via Chakra-UI, which doesn't natively support submenus (first time it's let me down so far). Thinking about this connected to other discussions, I wonder if we should have a Retry with different model... menu that opens a model where you can pick a model + set model params. The submenu idea won't give us enough UI surface to support setting different model params, so maybe it's time to split it out into its own thing?

About time we hit some limits :)

For 2., would a hash really help here? I find that responses from different models are often very, very similar but usually not identical, so a hash would basically always be different and hide the similarity. Maybe there is a way to do similarity scoring? Not sure how you do that without setting one of them as a baseline (maybe the current version is baseline?). Also, is time a useful indicator, since it's dependent on a bunch of things unrelated to the model (e.g., network and API congestion, speed of things in user's browser, etc). When I query these models, I find that GPT-4 can be faster than GPT-3.5 on some days, which is clearly not due to the speed of the model.

I was getting identical results due to above bug :)

@humphd
Copy link
Collaborator

humphd commented Jun 16, 2023

I was getting identical results due to above bug :)

This explains so much, I couldn't understand how the models could be so close or how GPT-4 got so fast! Glad you caught this.

@tarasglek tarasglek merged commit 7ed4fd6 into main Jun 16, 2023
@humphd
Copy link
Collaborator

humphd commented Jun 16, 2023

Prod seems happy with this change, loading my current stuff. Let me know if you see anything breaking.

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