FIX: various bugs in AI interface (#1430)

* FIX: improve transition logic in forms

previously back button would take you back to the /new route

* FIX: enum selection not working for persona tools

* seed information correctly in the DB

* fix broken spec

* Update assets/javascripts/discourse/components/ai-tool-editor-form.gjs

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>

---------

Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>

assets/javascripts/discourse/components/ai-llm-editor-form.gjs

@@ -202,13 +202,15 @@ export default class AiLlmEditorForm extends Component {
 
       if (isNew) {
         this.args.llms.addObject(this.args.model);
-        this.router.transitionTo("adminPlugins.show.discourse-ai-llms.index");
+        await this.router.replaceWith(
-      } else {
+          "adminPlugins.show.discourse-ai-llms.edit",
-        this.toasts.success({
+          this.args.model.id
-          data: { message: i18n("discourse_ai.llms.saved") },
+        );
-          duration: 2000,
-        });
       }
+      this.toasts.success({
+        data: { message: i18n("discourse_ai.llms.saved") },
+        duration: 2000,
+      });
     } catch (e) {
       popupAjaxError(e);
     } finally {
@@ -340,7 +342,7 @@ export default class AiLlmEditorForm extends Component {
         @format="large"
         as |field|
       >
-        <field.Password />
+        <field.Password autocomplete="off" data-1p-ignore />
       </form.Field>
 
       <form.Object @name="provider_params" as |object providerParamsData|>

assets/javascripts/discourse/components/ai-persona-editor.gjs

@@ -122,16 +122,15 @@ export default class PersonaEditor extends Component {
 
       if (isNew && this.args.model.rag_uploads.length === 0) {
         this.args.personas.addObject(personaToSave);
-        this.router.transitionTo(
+        await this.router.replaceWith(
           "adminPlugins.show.discourse-ai-personas.edit",
           personaToSave
         );
-      } else {
-        this.toasts.success({
-          data: { message: i18n("discourse_ai.ai_persona.saved") },
-          duration: 2000,
-        });
       }
+      this.toasts.success({
+        data: { message: i18n("discourse_ai.ai_persona.saved") },
+        duration: 2000,
+      });
     } catch (e) {
       popupAjaxError(e);
     } finally {

assets/javascripts/discourse/components/ai-persona-tool-options.gjs

@@ -16,16 +16,16 @@ export default class AiPersonaToolOptions extends Component {
   }
 
   get toolsMetadata() {
-    const metatada = {};
+    const metadata = {};
 
     this.args.allTools.map((t) => {
-      metatada[t.id] = {
+      metadata[t.id] = {
         name: t.name,
         ...t?.options,
       };
     });
 
-    return metatada;
+    return metadata;
   }
 
   @action
@@ -76,7 +76,7 @@ export default class AiPersonaToolOptions extends Component {
                       >
                         {{#if (eq optionMeta.type "enum")}}
                           <field.Select @includeNone={{false}} as |select|>
-                            {{#each optionsObj.values as |v|}}
+                            {{#each optionMeta.values as |v|}}
                               <select.Option @value={{v}}>{{v}}</select.Option>
                             {{/each}}
                           </field.Select>

assets/javascripts/discourse/components/ai-tool-editor-form.gjs

@@ -91,7 +91,7 @@ export default class AiToolEditorForm extends Component {
         this.args.tools.pushObject(this.args.model);
       }
 
-      this.router.transitionTo(
+      await this.router.replaceWith(
         "adminPlugins.show.discourse-ai-tools.edit",
         this.args.model
       );

db/fixtures/personas/603_ai_personas.rb

@@ -79,13 +79,8 @@ DiscourseAi::Personas::Persona.system_personas.each do |persona_class, id|
 
   persona.tools = tools.map { |name, value| [name, value] }
 
-  # Only set response_format if it's not defined as a method in the persona class
+  persona.response_format = instance.response_format
-  if !instance.class.instance_methods.include?(:response_format)
+  persona.examples = instance.examples
-    persona.response_format = instance.response_format
-  end
-
-  # Only set examples if it's not defined as a method in the persona class
-  persona.examples = instance.examples if !instance.class.instance_methods.include?(:examples)
 
   persona.system_prompt = instance.system_prompt
   persona.top_p = instance.top_p

spec/system/ai_bot/persona_spec.rb

@@ -10,6 +10,31 @@ RSpec.describe "AI personas", type: :system, js: true do
     sign_in(admin)
   end
 
+  it "can select and save persona tool options" do
+    visit "/admin/plugins/discourse-ai/ai-personas"
+    find(".ai-persona-list-editor__new-button").click
+
+    expect(page).to have_current_path("/admin/plugins/discourse-ai/ai-personas/new")
+
+    form = PageObjects::Components::FormKit.new("form")
+    form.field("name").fill_in("Test Persona")
+    form.field("description").fill_in("This is a test persona.")
+    form.field("system_prompt").fill_in("You are a helpful assistant.")
+    form.field("tools").select("Update Artifact")
+    form.field("toolOptions.UpdateArtifact.update_algorithm").select("full")
+    form.submit
+
+    expect(page).to have_current_path(%r{/admin/plugins/discourse-ai/ai-personas/\d+/edit})
+
+    persona = AiPersona.order("id desc").first
+
+    expect(persona.name).to eq("Test Persona")
+    expect(persona.description).to eq("This is a test persona.")
+    expect(persona.tools.count).to eq(1)
+    expect(persona.tools.first[0]).to eq("UpdateArtifact")
+    expect(persona.tools.first[1]["update_algorithm"]).to eq("full")
+  end
+
   it "remembers the last selected persona" do
     visit "/"
     find(".d-header .ai-bot-button").click()

spec/system/llms/ai_llm_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe "Managing LLM configurations", type: :system, js: true do
     form.field("enabled_chat_bot").toggle
     form.submit
 
-    expect(page).to have_current_path("/admin/plugins/discourse-ai/ai-llms")
+    expect(page).to have_current_path(%r{/admin/plugins/discourse-ai/ai-llms/\d+/edit})
 
     llm = LlmModel.order(:id).last
 
@@ -38,6 +38,8 @@ RSpec.describe "Managing LLM configurations", type: :system, js: true do
   end
 
   it "manually configures an LLM" do
+    llm_count = LlmModel.count
+
     visit "/admin/plugins/discourse-ai/ai-llms"
 
     expect(page_header).to be_visible
@@ -58,19 +60,32 @@ RSpec.describe "Managing LLM configurations", type: :system, js: true do
     form.field("enabled_chat_bot").toggle
     form.submit
 
-    expect(page).to have_current_path("/admin/plugins/discourse-ai/ai-llms")
+    expect(page).to have_current_path(%r{/admin/plugins/discourse-ai/ai-llms/\d+/edit})
-
     llm = LlmModel.order(:id).last
+    expect(llm.max_output_tokens.to_i).to eq(2000)
 
+    expect(page).to have_current_path("/admin/plugins/discourse-ai/ai-llms/#{llm.id}/edit")
+
+    form.field("max_output_tokens").fill_in(2001)
+    form.submit
+
+    # should go to llm list and see the llms correctly configured
+    page.go_back
+
+    expect(page).to have_selector(".ai-llms-list-editor__configured .ai-llm-list__row", count: 1)
+
+    llm.reload
     expect(llm.display_name).to eq("Self-hosted LLM")
     expect(llm.name).to eq("llava-hf/llava-v1.6-mistral-7b-hf")
     expect(llm.url).to eq("srv://self-hostest.test")
     expect(llm.tokenizer).to eq("DiscourseAi::Tokenizer::Llama3Tokenizer")
     expect(llm.max_prompt_tokens.to_i).to eq(8000)
     expect(llm.provider).to eq("vllm")
-    expect(llm.max_output_tokens.to_i).to eq(2000)
+    expect(llm.max_output_tokens.to_i).to eq(2001)
     expect(llm.vision_enabled).to eq(true)
     expect(llm.user_id).not_to be_nil
+
+    expect(LlmModel.count).to eq(llm_count + 1)
   end
 
   context "when changing the provider" do