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

Ensure the -best model points to the appropriate language model #604

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

stephan-buckmaster
Copy link
Contributor

@stephan-buckmaster stephan-buckmaster commented Jan 19, 2025

Added new method LanguageModel#effective_api_name. When the api_name of a LanguageModel ends in -best this method looks up the language model of its APIService that has the best flag set.

  1. I don't think the Export class needs to resolve the (new) effective_api_name.
  2. The Export functionality seems the only where the Assistant code references api_name. So no need to change the Assistant model code.
  3. Added comment about the language model Test link (such a nice feature) that it should also take into account the API Service (File app/controllers/settings/language_models_controller.rb)

Fixes issue #592

@krschacht krschacht changed the title Using "best" model Ensure the -best model points to another language model Jan 27, 2025
@krschacht krschacht changed the title Ensure the -best model points to another language model Ensure the -best model points to the appropriate language model Jan 27, 2025
@krschacht
Copy link
Contributor

I reviewed this PR but I'm torn with regards to the implementation. The challenge with replacing all the api_name calls with effective_api_name is that we're not fully replacing this "virtual" language_model with the resulting language_model that we want it to act like. So code that references any other attributes of this language_model will potentially get wrong info (e.g. cost per token and flags like supports_image).

My first thought was: can we make the call to .language_model somehow recognize that it's a special "virtual/pointer/best" model and actually return the model that we want it to act like. But since language_model is an association, I couldn't figure out an appropriate way to hook in there.

So then I was thinking that we could let language_model be the virtual one, but within it, any call to any attribute would do a lookup of the actual best: true model and return that attribute instead. But @attributes is an ivar, not a method, and I couldn't figure out an elegant way to tap into that and intercept any attempt to reference an attribute.

I see how you ended up where you did. I can't think of a better solution. But I worry that this solution will be too brittle. There may already be subtle bugs in the code for places where we aren't using the correct language_model attribute. I don't have a good idea for how to resolve... I'm stuck... :)

@stephan-buckmaster
Copy link
Contributor Author

I don't know, this seems to get very general. How often does the "best" of today change features compared to the "best" of yesterday?

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