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

Create & manage LanguageModels and APIService (+add Llama 3) #389

Merged
merged 72 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
d2f16fc
Can view/edit/add language models
stephan-buckmaster May 29, 2024
c6ce6ea
Can use other OpenAI flavour services
stephan-buckmaster May 30, 2024
43a2194
Can administer API services
stephan-buckmaster May 30, 2024
5544f1c
Can associate API Service with assistants
stephan-buckmaster May 31, 2024
1a751a8
Can assign driver to API Services
stephan-buckmaster May 31, 2024
1d5eb1b
Can connect to OpenAI backend
stephan-buckmaster May 31, 2024
d7ad4f7
Fix bug, use language_model name, not assistant's name
stephan-buckmaster Jun 1, 2024
34ceb46
API Service is optional, can view API Service's access token when edi…
stephan-buckmaster Jun 2, 2024
e4ed8f2
rake test: all pass
stephan-buckmaster Jun 5, 2024
c6ea9a6
Undo changes to unrelated files
stephan-buckmaster Jun 5, 2024
d6cb747
Merge from upstream
stephan-buckmaster Jun 6, 2024
f3a9503
Recreate db/schema.rb
stephan-buckmaster Jun 10, 2024
46b6806
Review comments
stephan-buckmaster Jun 11, 2024
c9bab8d
Merge upstream
stephan-buckmaster Jun 11, 2024
8b51f04
Add hints to language model form fields
stephan-buckmaster Jun 11, 2024
b36f264
Display API services and language models in a table
stephan-buckmaster Jun 11, 2024
fa52677
Sidebar styling: API Services / Language Models links
stephan-buckmaster Jun 11, 2024
640a8f2
Styling of buttons for language models and API services
stephan-buckmaster Jun 11, 2024
9651eb6
Show language model's owner
stephan-buckmaster Jun 11, 2024
685dda6
Fix indentation
stephan-buckmaster Jun 11, 2024
2e4f6ac
Fix test failures
stephan-buckmaster Jun 12, 2024
8594d60
Merge from upstream
stephan-buckmaster Jun 12, 2024
7abc447
Add test for API Services model + controller (including updates to vi…
stephan-buckmaster Jun 12, 2024
752e115
Add test for Language Model controller (including updates to views/co…
stephan-buckmaster Jun 12, 2024
407f757
Expand LanguageModelTest
stephan-buckmaster Jun 13, 2024
23b901d
Extend soft-deletes across APIService/LanguageModel/Assistant
stephan-buckmaster Jun 13, 2024
d1fab25
Tests for APIService
stephan-buckmaster Jun 13, 2024
228dcb4
Move the TestClients out of the app directory, to test/support
stephan-buckmaster Jun 13, 2024
7e1512c
Fix lint/rubocop issues
stephan-buckmaster Jun 13, 2024
a13efbb
Merge branch 'main' into pr/stephan-buckmaster/389
krschacht Jun 13, 2024
ad1cd40
Move encrypts & normalizes to be near other attribute details
krschacht Jun 13, 2024
f6e9fb7
Reorder language_model methods
krschacht Jun 13, 2024
90f8491
Merge from upstream
stephan-buckmaster Jun 15, 2024
8061ccf
Review comments regarding db schema
stephan-buckmaster Jun 15, 2024
b8a3652
Review comments re. APIServices controller
stephan-buckmaster Jun 15, 2024
ccd8176
More review comments
stephan-buckmaster Jun 16, 2024
8c902d9
Review comment / assert_instance_of User
stephan-buckmaster Jun 16, 2024
f0190b0
Review comment / destroy vs delete
stephan-buckmaster Jun 16, 2024
8b9a7d9
Use singular for add_reference :language_models, :user in the db-migr…
stephan-buckmaster Jun 18, 2024
2d37035
Merge from upstream
stephan-buckmaster Jun 19, 2024
236eaa6
Another merge from upstream
stephan-buckmaster Jun 19, 2024
3c1b59b
Correct a bug re. access_token vs. token
stephan-buckmaster Jun 19, 2024
5c3ae61
Allow running migration 20240523080700_create_language_models.rb with…
stephan-buckmaster Jun 19, 2024
c215eec
db/schema.rb had memories table twice
stephan-buckmaster Jun 19, 2024
c6dad30
Move system language models to user level
stephan-buckmaster Jun 19, 2024
d6105c0
Fix / extend tests
stephan-buckmaster Jun 19, 2024
3594426
Fix lint/rubocop issue
stephan-buckmaster Jun 19, 2024
5e55b5d
Merge from upstream
stephan-buckmaster Jun 21, 2024
4fd47bd
Merge branch '341-manage-llms-2' into 341-manage-llms-3. More changes…
stephan-buckmaster Jun 22, 2024
c371ff9
Merge branch 'main' into pr/stephan-buckmaster/389
krschacht Jun 23, 2024
3ea3c9a
Merge branch 'main' into pr/stephan-buckmaster/425
krschacht Jun 23, 2024
55f971e
Update app/controllers/settings/language_models_controller.rb
krschacht Jun 23, 2024
1c0e70f
Merge branch 'pr/stephan-buckmaster/425' into pr/stephan-buckmaster/389
krschacht Jun 23, 2024
b7ccb75
wip
krschacht Jun 23, 2024
75ab8ff
migrations
krschacht Jun 23, 2024
fb2bb9f
Model tests all pass
krschacht Jun 23, 2024
4e19856
More model fixes
krschacht Jun 23, 2024
992160e
Get tests working for AI backends
krschacht Jun 24, 2024
4d625c1
Controller tests passing
krschacht Jun 24, 2024
885f17a
Claned up language model
krschacht Jun 24, 2024
c5e7d73
finish tweaking API services forms
krschacht Jun 24, 2024
d5a8882
Final pass changes
krschacht Jun 24, 2024
86c91f2
fix tests
krschacht Jun 24, 2024
a72c947
Fix test
krschacht Jun 24, 2024
0b673c2
correct merge misses
krschacht Jun 24, 2024
4859770
Anthropic and OpenAI are "official"
stephan-buckmaster Jun 24, 2024
361cb25
Make sure the sequence rails db:drop; rails db:create; rails db:migra…
stephan-buckmaster Jun 24, 2024
c24c8d4
flip logic of instruction link & add tests
krschacht Jun 24, 2024
042261e
correct test
krschacht Jun 24, 2024
685614d
Update the new-user pre-populate to include Groq
krschacht Jun 24, 2024
09dee26
Improve user experience for Groq & fix some bugs
krschacht Jun 24, 2024
a8a84da
Add Groq for all old users
krschacht Jun 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions app/controllers/settings/api_services_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
class Settings::APIServicesController < Settings::ApplicationController
before_action :set_api_service, only: [:edit, :update, :destroy]

def index
@api_services = APIService.all.order(updated_at: :desc)
end

def edit
end

def new
@api_service = APIService.new
end

def create
@api_service = Current.user.api_services.new(api_service_params)

if @api_service.save
redirect_to settings_api_services_path, notice: "Saved"
else
render :new, status: :unprocessable_entity
end
end

def update
if @api_service.update(api_service_params)
redirect_to edit_settings_api_service_path(@api_service), notice: "Saved", status: :see_other
else
render :edit, status: :unprocessable_entity
end
end

def destroy
@api_service.destroy!
redirect_to new_settings_api_service_url, notice: "Deleted", status: :see_other
end

private

def set_users_api_service
@api_service = Current.user.api_services.find_by(id: params[:id])
if @api_service.nil?
redirect_to new_settings_api_service_url, notice: "The API Service was deleted", status: :see_other
end
end

def set_api_service
@api_service = APIService.find_by(id: params[:id])
end

def api_service_params
params.require(:api_service).permit(:name, :url, :access_token, :driver)
end
end
58 changes: 58 additions & 0 deletions app/controllers/settings/language_models_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
class Settings::LanguageModelsController < Settings::ApplicationController
before_action :set_users_language_model, only: [:edit, :update, :destroy]
before_action :set_language_model, only: [:show]

def index
@language_models = LanguageModel.all.order(updated_at: :desc)
end

def edit
end

def show
end

def new
@language_model = LanguageModel.new
end

def create
@language_model = Current.user.language_models.new(language_model_params)

if @language_model.save
redirect_to settings_language_models_path, notice: "Saved"
else
render :new, status: :unprocessable_entity
end
end

def update
if @language_model.update(language_model_params)
redirect_to edit_settings_language_model_path(@language_model), notice: "Saved", status: :see_other
else
render :edit, status: :unprocessable_entity
end
end

def destroy
@language_model.destroy!
redirect_to new_settings_language_model_url, notice: "Deleted", status: :see_other
end

private

def set_users_language_model
@language_model = Current.user.language_models.find_by(id: params[:id])
if @language_model.nil?
redirect_to new_settings_language_model_url, notice: "The language_model was deleted", status: :see_other
end
end

def set_language_model
@language_model = LanguageModel.find_by(id: params[:id])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way I could alter the URL and succeed at viewing another user's model? Let's prevent that, just to be safe:

Suggested change
def set_language_model
@language_model = LanguageModel.find_by(id: params[:id])
def set_language_model
@language_model = LanguageModel.find_by(id: params[:id], user_id: nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a test "should not update other user's language_model" for the controller, I added now also, "should not allow viewing other user's records" So I think no code change is needed.

end

def language_model_params
params.require(:language_model).permit(:name, :description, :supports_images, :api_service_id)
end
end
3 changes: 3 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,7 @@ def viewport_tag(content)
tag.meta(name: 'viewport', content: content)
end

def n_a_if_blank(value, n_a = "Not Available")
value.blank? ? n_a : value.to_s
end
end
15 changes: 15 additions & 0 deletions app/helpers/settings/language_models_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module Settings
module LanguageModelsHelper
def display_boolean(value)
ActiveModel::Type::Boolean.new.cast(value) ? 'Yes' : 'No'
end

def language_model_edit_show_path(language_model)
if language_model.created_by_current_user?
edit_settings_language_model_path(language_model)
else
settings_language_model_path(language_model)
end
end
end
end
28 changes: 28 additions & 0 deletions app/models/api_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
class APIService < ApplicationRecord
encrypts :access_token

DRIVERS = %w(Anthropic OpenAI)

belongs_to :user

validates :url, format: URI::DEFAULT_PARSER.make_regexp(%w[http https]), if: -> { url.present? }
validates :name, :url, presence: true
validates :driver, inclusion: { in: DRIVERS }

scope :ordered, -> { order(:name) }

normalizes :url, with: -> url { url.strip }

def ai_backend
driver == 'Anthropic' ? AIBackend::Anthropic : AIBackend::OpenAI
end

def destroy
raise ActiveRecord::ReadOnlyError 'System model cannot be deleted' if user.blank?
if user.destroy_in_progress?
super
else
update!(deleted_at: Time.now)
end
end
end
2 changes: 2 additions & 0 deletions app/models/assistant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ class Assistant < ApplicationRecord

scope :ordered, -> { order(:id) }

delegate :api_service, to: :language_model

def initials
return nil if name.blank?

Expand Down
51 changes: 41 additions & 10 deletions app/models/language_model.rb
Original file line number Diff line number Diff line change
@@ -1,27 +1,58 @@
# We don"t care about large or not
class LanguageModel < ApplicationRecord

belongs_to :user, optional: true
belongs_to :api_service, optional: true

validates :name, :description, presence: true

BEST_MODELS = {
"gpt-best" => "gpt-4o-2024-05-13",
"claude-best" => "claude-3-opus-20240229"
}

scope :ordered, -> { order(:position) }

has_many :assistants
def ai_backend
if api_service.present?
api_service.ai_backend
elsif name.starts_with?('gpt-')
AIBackend::OpenAI
else
AIBackend::Anthropic
end
end

def readonly?
!new_record?
user_id.blank?
end

def destroy
raise ActiveRecord::ReadOnlyError 'System model cannot be deleted' if user.blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way that a user can trigger deletion of a language model? Is this preventing someone faking a form submission? If that's why it's here, let's move it to the controller. That feels like the better place for this check to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is the right place. There is a delete button for language models

if user.destroy_in_progress?
super
else
update!(deleted_at: Time.now)
end
end

scope :ordered, -> { order(:position) }
scope :for_user, -> (user) { where(user_id: [user.id, nil]) }
scope :system_wide, -> { where(user_id: nil) }

before_create :populate_position

has_many :assistants

def provider_name
BEST_MODELS[name] || name
end

def ai_backend
if name.starts_with?("gpt-")
AIBackend::OpenAI
else
AIBackend::Anthropic
end
def created_by_current_user?
user == Current.user
end

def populate_position
return unless position.blank?
self.position = (LanguageModel.maximum(:position) || 0) + 1
end

end
22 changes: 22 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class User < ApplicationRecord

has_many :assistants, -> { not_deleted }
has_many :assistants_including_deleted, class_name: "Assistant", inverse_of: :user, dependent: :destroy
has_many :language_models, -> { not_deleted }
has_many :language_models_including_deleted, class_name: "LanguageModel", dependent: :destroy
has_many :api_services, -> { not_deleted }
has_many :api_services_including_deleted, class_name: "APIService", dependent: :destroy
has_many :conversations, dependent: :destroy
has_many :credentials, dependent: :destroy

Expand All @@ -26,4 +30,22 @@ class User < ApplicationRecord
def preferences
attributes["preferences"].with_defaults(dark_mode: "system")
end

# Which ones can they choose for their api services
def usable_language_models
LanguageModel.for_user(self)
end

def destroy_in_progress?
@destroy_in_progress
end

def destroy
@destroy_in_progress = true
begin
super
ensure
@destroy_in_progress = false
end
end
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if you remove all of this destroy_in_process logic in all of the models? What's the symptom you're experiencing that this solves for? Because I thought we solved this in the other PR in a way that made us no longer need this extra logic.

My brief recollection was:

  • Any time we need to soft delete a model which has a deleted_at attribute, instead of overloading destroy we simply call model.deleted! and that sets the timestamp. So, for example, a controller destroy action would be updated to use deleted!
  • Any place where we want to prevent a user from deleting a model (such as in a controller) we add an explicit check there (e.g. to check if it's the last assistant)
  • With these two things in place, then all the dependent: :destroy should work just fine without the need for this extra logic

But I realize there may be some consideration that I'm missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I'll save this for another session, tomorrow ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok made change, there's probably a gem for this. Look one up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied, see last commit, f0190b0

end
9 changes: 7 additions & 2 deletions app/services/ai_backend/anthropic.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ def self.client
end

def initialize(user, assistant, conversation, message)
raise ::Anthropic::ConfigurationError if user.anthropic_key.blank?
begin
@client = self.class.client.new(access_token: user.anthropic_key)
if assistant.api_service.blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

This branching doesn't feel quite right. Why are there any assistants that do not have an api_service? Why don't we just create api_services for all of our system assistants? If we keep this inconsistency then we'll have to add these sprinkle these conditional checks throughout the code, whereas if we resolve the inconsistency then we can always assume an assistant has an api_service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure, it seems to me then we'll have the same thing with the OpenAI/Anthropic URL; we don't want to configure those URLs in our database do we?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should configure those URLs in our database. The fact that we have two different ways of getting a URL is a good clue. If some assistant’s don’t have an api_service and others do, we’ll surely end up adding more conditionals over time to account for that difference.

If someone wanted to switch from OpenAI’s servers to using Microsoft Azure for all their GPT-4 needs, this would be the place to change it. It would be really nice to be able to edit that within the app. Also, I think we’ll inevitably end up adding additional logic to the API Service abstraction. Now that it exists, it’s a good place to house certain things. For example, one thing I’m working on is voice mode for the app. I did some digging and discovered that ChatGPT layers on some additional custom instructions when you activate the microphone. The GPT is told “the text you generate will be spoken so don’t use bullet points”.

This is not something I want to do per assistant or even per language model, I think it’s a setting which I would add to the api_service model. I’ll pre-populate some “voice instructions” will get concatenated to your Assistant’s custom instructions, but if you want to tweak that setting it would get tweaked at this low level and apply to all assistants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you're thinking about changing these URLs then they would need to be user-level. So every user will have these user-level api-services, implying the existing language_models records we created with the db-migration will be moved(copied) to user-level too then? It does sound consistent, but I wanted to make sure I understand your suggestion right.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's right. So the Assistant > LanguageModel > APIService stack is all per-user. (Maybe some future "teams" features could cause us to revisit that with a more sophisticated permissions model...)

raise ::Anthropic::ConfigurationError if user.anthropic_key.blank?
@client = self.class.client.new(access_token: user.anthropic_key)
else
Rails.logger.info "Connecting to Anthropic API server at #{assistant.api_service.url} with access token of length #{assistant.api_service.access_token.to_s.length}"
@client = self.class.client.new(uri_base: assistant.api_service.url, access_token: assistant.api_service.access_token)
end
rescue ::Faraday::UnauthorizedError => e
raise ::Anthropic::ConfigurationError
end
Expand Down
9 changes: 7 additions & 2 deletions app/services/ai_backend/open_ai.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,14 @@ def self.get_tool_messages_by_calling(tool_calls_response)
end

def initialize(user, assistant, conversation, message)
raise ::OpenAI::ConfigurationError if user.openai_key.blank?
begin
@client = self.class.client.new(access_token: user.openai_key)
if assistant.api_service.blank?
raise ::OpenAI::ConfigurationError if user.openai_key.blank?
@client = self.class.client.new(access_token: user.openai_key)
else
Rails.logger.info "Connecting to API server at #{assistant.api_service.url} with access token of length #{assistant.api_service.access_token.to_s.length}"
@client = self.class.client.new(uri_base: assistant.api_service.url, access_token: assistant.api_service.access_token)
end
rescue ::Faraday::UnauthorizedError => e
raise ::OpenAI::ConfigurationError
end
Expand Down
6 changes: 6 additions & 0 deletions app/views/layouts/settings.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@
<%= render partial: "layouts/settings_item", collection: settings_items, locals: { controller: controller } %>
</div>
<% end %>
<div class="rounded-s-lg mb-1 pl-3 p-2 hover:bg-gray-100 dark:hover:bg-gray-700">
<%= link_to "API Services", settings_api_services_path %>
</div>
<div class="mt-5 rounded-s-lg mb-1 pl-3 p-2 hover:bg-gray-100 dark:hover:bg-gray-700">
<%= link_to "Language Models", settings_language_models_path %>
</div>
</section>
<% end %>

Expand Down
55 changes: 55 additions & 0 deletions app/views/settings/api_services/_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<%= form_with(model: [:settings, api_service], class: "contents") do |form| %>
<% if api_service.errors.any? %>
<div id="error_explanation" class="bg-red-50 text-red-500 px-3 py-2 font-medium rounded-lg mt-3">
<h2><%= pluralize(api_service.errors.count, "error") %> prohibited this api_service from being saved:</h2>

<ul>
<% api_service.errors.each do |error| %>
<li><%= error.full_message %></li>
<% end %>
</ul>
</div>
<% end %>

<div class="my-5">
<%= form.label :name %>
<%= form.text_field :name, class: "block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full dark:text-black" %>
</div>

<div class="my-5">
<%= form.label :driver %>
<%= form.select :driver,
APIService::DRIVERS,
{},
class: %|
block
border border-gray-200 outline-none
shadow rounded-md
px-3 py-2 mt-2
w-full
dark:text-black
|
%>
</div>

<div class="my-5">
<%= form.label :url %>
<%= form.text_field :url, class: "block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full dark:text-black" %>
</div>

<div class="my-5">
<%= form.label :access_token %>
<%= form.text_field :access_token, class: "block shadow rounded-md border border-gray-200 outline-none px-3 py-2 mt-2 w-full dark:text-black" %>
</div>

<%= form.submit "Save",
class: %|
inline-block font-bold
bg-gray-200 dark:bg-gray-900
border border-gray-400
rounded-lg py-3 px-5
cursor-pointer
|,
data: { turbo_submits_with: "Saving..." }
%>
<% end %>
11 changes: 11 additions & 0 deletions app/views/settings/api_services/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div class="mx-auto md:w-2/3 w-full">
<h1 class="font-bold text-4xl">
Editing API Service <%= @api_service.name %>
</h1>

<%= render "form", api_service: @api_service %>

<p class="float-right inline-block ml-5">
<%= link_to "Cancel", settings_api_services_path, class: "float-right inline-block ml-5 py-3" %>
</p>
</div>
Loading
Loading