FEATURE: Allow deleting custom LLMs. (#643)

This change allows us to delete custom models. It checks if there is no module using them.

It also fixes a bug where the after-create transition wasn't working. While this prevents a model from being saved multiple times, endpoint validations are still needed (will be added in a separate PR).:
This commit is contained in:
Roman Rizzi 2024-05-27 16:44:08 -03:00 committed by GitHub
parent 90c5e4bb0e
commit 333b331eb9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 92 additions and 10 deletions

View File

@ -49,6 +49,36 @@ module DiscourseAi
end end
end end
def destroy
llm_model = LlmModel.find(params[:id])
dependant_settings = %i[ai_helper_model ai_embeddings_semantic_search_hyde_model]
in_use_by = []
dependant_settings.each do |s_name|
in_use_by << s_name if SiteSetting.public_send(s_name) == "custom:#{llm_model.id}"
end
if !in_use_by.empty?
return(
render_json_error(
I18n.t(
"discourse_ai.llm.delete_failed",
settings: in_use_by.join(", "),
count: in_use_by.length,
),
status: 409,
)
)
end
if llm_model.destroy
head :no_content
else
render_json_error llm_model
end
end
def test def test
RateLimiter.new(current_user, "llm_test_#{current_user.id}", 3, 1.minute).performed! RateLimiter.new(current_user, "llm_test_#{current_user.id}", 3, 1.minute).performed!

View File

@ -16,6 +16,7 @@ import DTooltip from "float-kit/components/d-tooltip";
export default class AiLlmEditor extends Component { export default class AiLlmEditor extends Component {
@service toasts; @service toasts;
@service router; @service router;
@service dialog;
@tracked isSaving = false; @tracked isSaving = false;
@ -45,10 +46,7 @@ export default class AiLlmEditor extends Component {
if (isNew) { if (isNew) {
this.args.llms.addObject(this.args.model); this.args.llms.addObject(this.args.model);
this.router.transitionTo( this.router.transitionTo("adminPlugins.show.discourse-ai-llms.index");
"adminPlugins.show.discourse-ai-llms.show",
this.args.model
);
} else { } else {
this.toasts.success({ this.toasts.success({
data: { message: I18n.t("discourse_ai.llms.saved") }, data: { message: I18n.t("discourse_ai.llms.saved") },
@ -94,6 +92,24 @@ export default class AiLlmEditor extends Component {
return this.testRunning || this.testResult !== null; return this.testRunning || this.testResult !== null;
} }
@action
delete() {
return this.dialog.confirm({
message: I18n.t("discourse_ai.llms.confirm_delete"),
didConfirm: () => {
return this.args.model
.destroyRecord()
.then(() => {
this.args.llms.removeObject(this.args.model);
this.router.transitionTo(
"adminPlugins.show.discourse-ai-llms.index"
);
})
.catch(popupAjaxError);
},
});
}
<template> <template>
<BackButton <BackButton
@route="adminPlugins.show.discourse-ai-llms" @route="adminPlugins.show.discourse-ai-llms"
@ -182,6 +198,14 @@ export default class AiLlmEditor extends Component {
> >
{{I18n.t "discourse_ai.llms.save"}} {{I18n.t "discourse_ai.llms.save"}}
</DButton> </DButton>
{{#unless @model.isNew}}
<DButton
@action={{this.delete}}
class="btn-danger ai-llm-editor__delete"
>
{{I18n.t "discourse_ai.llms.delete"}}
</DButton>
{{/unless}}
</div> </div>
<div class="control-group ai-llm-editor-tests"> <div class="control-group ai-llm-editor-tests">

View File

@ -217,7 +217,10 @@ en:
edit: "Edit" edit: "Edit"
saved: "LLM Model Saved" saved: "LLM Model Saved"
back: "Back" back: "Back"
tests: confirm_delete: Are you sure you want to delete this model?
delete: Delete
tests:
title: "Run Test" title: "Run Test"
running: "Running test..." running: "Running test..."
success: "Success!" success: "Success!"

View File

@ -319,6 +319,10 @@ en:
configuration_hint: configuration_hint:
one: "Make sure the `%{settings}` setting was configured." one: "Make sure the `%{settings}` setting was configured."
other: "Make sure these settings were configured: %{settings}" other: "Make sure these settings were configured: %{settings}"
delete_failed:
one: "We couldn't delete this model because %{settings} is using it. Update the setting and try again."
other: "We couldn't delete this model because %{settings} are using it. Update the settings and try again."
embeddings: embeddings:
configuration: configuration:

View File

@ -47,7 +47,7 @@ Discourse::Application.routes.draw do
get "/ai-personas/:id/files/status", to: "discourse_ai/admin/ai_personas#indexing_status_check" get "/ai-personas/:id/files/status", to: "discourse_ai/admin/ai_personas#indexing_status_check"
resources :ai_llms, resources :ai_llms,
only: %i[index create show update], only: %i[index create show update destroy],
path: "ai-llms", path: "ai-llms",
controller: "discourse_ai/admin/ai_llms" do controller: "discourse_ai/admin/ai_llms" do
collection { get :test } collection { get :test }

View File

@ -28,11 +28,11 @@ module DiscourseAi
@endpoint = endpoint @endpoint = endpoint
return false return false
end end
end
if !can_talk_to_model?(val) if !can_talk_to_model?(val)
@unreachable = true @unreachable = true
return false return false
end
end end
true true

View File

@ -112,4 +112,25 @@ RSpec.describe DiscourseAi::Admin::AiLlmsController do
end end
end end
end end
describe "DELETE #destroy" do
fab!(:llm_model)
it "destroys the requested ai_persona" do
expect {
delete "/admin/plugins/discourse-ai/ai-llms/#{llm_model.id}.json"
expect(response).to have_http_status(:no_content)
}.to change(LlmModel, :count).by(-1)
end
it "validates the model is not in use" do
SiteSetting.ai_helper_model = "custom:#{llm_model.id}"
delete "/admin/plugins/discourse-ai/ai-llms/#{llm_model.id}.json"
expect(response.status).to eq(409)
expect(llm_model.reload).to eq(llm_model)
end
end
end end