FEATURE: Display bot in feature list (#1466)

- allows features to have multiple llms and multiple personas
- sorts module list
- adds Bot as a first class module
- fixes issue where search module was always configured
- some tests

admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-features-edit.js

@@ -1,3 +1,4 @@
+import { action } from "@ember/object";
 import { ajax } from "discourse/lib/ajax";
 import DiscourseRoute from "discourse/routes/discourse";
 import SiteSetting from "admin/models/site-setting";
@@ -24,4 +25,11 @@ export default class AdminPluginsShowDiscourseAiFeaturesEdit extends DiscourseRo
 
     return currentFeature;
   }
+
+  @action
+  willTransition() {
+    // site settings may amend if a feature is enabled or disabled, so refresh the model
+    // even on back button
+    this.router.refresh("adminPlugins.show.discourse-ai-features");
+  }
 }

app/controllers/discourse_ai/admin/ai_features_controller.rb

@@ -37,11 +37,11 @@ module DiscourseAi
       def serialize_feature(feature)
         {
           name: feature.name,
-          persona: serialize_persona(persona_id_obj_hash[feature.persona_id]),
-          llm_model: {
-            id: feature.llm_model&.id,
-            name: feature.llm_model&.name,
-          },
+          personas: feature.persona_ids.map { |id| serialize_persona(persona_id_obj_hash[id]) },
+          llm_models:
+            feature.llm_models.map do |llm_model|
+              { id: llm_model.id, name: llm_model.display_name }
+            end,
           enabled: feature.enabled?,
         }
       end
@@ -57,9 +57,7 @@ module DiscourseAi
       def persona_id_obj_hash
         @persona_id_obj_hash ||=
           begin
-            setting_names = DiscourseAi::Configuration::Feature.all_persona_setting_names
-            ids = setting_names.map { |sn| SiteSetting.public_send(sn) }
-
+            ids = DiscourseAi::Configuration::Feature.all.map(&:persona_ids).flatten.uniq
             AiPersona.where(id: ids).index_by(&:id)
           end
       end

assets/javascripts/discourse/components/ai-features-list.gjs

@@ -1,11 +1,100 @@
+import Component from "@glimmer/component";
+import { tracked } from "@glimmer/tracking";
 import { concat } from "@ember/helper";
-import { gt } from "truth-helpers";
+import { action } from "@ember/object";
 import DButton from "discourse/components/d-button";
 import { i18n } from "discourse-i18n";
 
-const AiFeaturesList = <template>
+class ExpandableList extends Component {
+  @tracked isExpanded = false;
+
+  get maxItemsToShow() {
+    return this.args.maxItemsToShow ?? 5;
+  }
+
+  get hasMore() {
+    return this.args.items?.length > this.maxItemsToShow;
+  }
+
+  get visibleItems() {
+    if (!this.args.items) {
+      return [];
+    }
+    return this.isExpanded
+      ? this.args.items
+      : this.args.items.slice(0, this.maxItemsToShow);
+  }
+
+  get remainingCount() {
+    return this.args.items?.length - this.maxItemsToShow;
+  }
+
+  get expandToggleLabel() {
+    if (this.isExpanded) {
+      return i18n("discourse_ai.features.collapse_list");
+    } else {
+      return i18n("discourse_ai.features.expand_list", {
+        count: this.remainingCount,
+      });
+    }
+  }
+
+  @action
+  toggleExpanded() {
+    this.isExpanded = !this.isExpanded;
+  }
+
+  <template>
+    {{#each this.visibleItems as |item index|}}
+      {{yield item index}}
+    {{/each}}
+
+    {{#if this.hasMore}}
+      <DButton
+        class="btn-flat btn-small ai-expanded-list__toggle-button"
+        @translatedLabel={{this.expandToggleLabel}}
+        @action={{this.toggleExpanded}}
+      />
+    {{/if}}
+  </template>
+}
+
+export default class AiFeaturesList extends Component {
+  get sortedModules() {
+    return this.args.modules.sort((a, b) => {
+      const nameA = i18n(`discourse_ai.features.${a.module_name}.name`);
+      const nameB = i18n(`discourse_ai.features.${b.module_name}.name`);
+      return nameA.localeCompare(nameB);
+    });
+  }
+
+  @action
+  hasGroups(feature) {
+    return this.groupList(feature).length > 0;
+  }
+
+  @action
+  groupList(feature) {
+    const groups = [];
+    const groupIds = new Set();
+    if (feature.personas) {
+      feature.personas.forEach((persona) => {
+        if (persona.allowed_groups) {
+          persona.allowed_groups.forEach((group) => {
+            if (!groupIds.has(group.id)) {
+              groupIds.add(group.id);
+              groups.push(group);
+            }
+          });
+        }
+      });
+    }
+    return groups;
+  }
+
+  <template>
     <div class="ai-features-list">
-    {{#each @modules as |module|}}
+      {{#each this.sortedModules as |module|}}
         <div class="ai-module" data-module-name={{module.module_name}}>
           <div class="ai-module__header">
             <div class="ai-module__module-title">
@@ -47,37 +136,57 @@ const AiFeaturesList = <template>
                     {{/unless}}
                   </div>
                   <div class="ai-feature-card__persona">
-                  <span>{{i18n "discourse_ai.features.persona"}}</span>
-                  {{#if feature.persona}}
+                    <span>{{i18n
+                        "discourse_ai.features.persona"
+                        count=feature.personas.length
+                      }}</span>
+                    {{#if feature.personas}}
+                      <ExpandableList
+                        @items={{feature.personas}}
+                        @maxItemsToShow={{5}}
+                        as |persona|
+                      >
                         <DButton
                           class="btn-flat btn-small ai-feature-card__persona-button"
-                      @translatedLabel={{feature.persona.name}}
+                          @translatedLabel={{persona.name}}
                           @route="adminPlugins.show.discourse-ai-personas.edit"
-                      @routeModels={{feature.persona.id}}
+                          @routeModels={{persona.id}}
                         />
+                      </ExpandableList>
                     {{else}}
                       {{i18n "discourse_ai.features.no_persona"}}
                     {{/if}}
                   </div>
                   <div class="ai-feature-card__llm">
-                  <span>{{i18n "discourse_ai.features.llm"}}</span>
-                  {{#if feature.llm_model.name}}
+                    {{#if feature.llm_models}}
+                      <span>{{i18n
+                          "discourse_ai.features.llm"
+                          count=feature.llm_models.length
+                        }}</span>
+                    {{/if}}
+                    {{#if feature.llm_models}}
+                      <ExpandableList
+                        @items={{feature.llm_models}}
+                        @maxItemsToShow={{5}}
+                        as |llm|
+                      >
                         <DButton
                           class="btn-flat btn-small ai-feature-card__llm-button"
-                      @translatedLabel={{feature.llm_model.name}}
+                          @translatedLabel={{llm.name}}
                           @route="adminPlugins.show.discourse-ai-llms.edit"
-                      @routeModels={{feature.llm_model.id}}
+                          @routeModels={{llm.id}}
                         />
+                      </ExpandableList>
                     {{else}}
                       {{i18n "discourse_ai.features.no_llm"}}
                     {{/if}}
                   </div>
-                {{#if feature.persona}}
+                  {{#if feature.personas}}
                     <div class="ai-feature-card__groups">
                       <span>{{i18n "discourse_ai.features.groups"}}</span>
-                    {{#if (gt feature.persona.allowed_groups.length 0)}}
+                      {{#if (this.hasGroups feature)}}
                         <ul class="ai-feature-card__item-groups">
-                        {{#each feature.persona.allowed_groups as |group|}}
+                          {{#each (this.groupList feature) as |group|}}
                             <li>{{group.name}}</li>
                           {{/each}}
                         </ul>
@@ -93,6 +202,5 @@ const AiFeaturesList = <template>
         </div>
       {{/each}}
     </div>
-</template>;
-
-export default AiFeaturesList;
+  </template>
+}

assets/stylesheets/common/ai-features.scss

@@ -22,12 +22,18 @@
   background: var(--primary-very-low);
   border: 1px solid var(--primary-low);
   padding: 0.5rem;
-  display: block;
+  display: flex;
+  flex-direction: column;
 
   &__llm,
   &__persona,
   &__groups {
     font-size: var(--font-down-1-rem);
+    display: flex;
+    flex-flow: row wrap;
+    gap: 0.1em;
+    margin-top: 0.5rem;
+    align-items: center;
   }
 
   &__persona {
@@ -36,7 +42,7 @@
 
   &__persona-button,
   &__llm-button {
-    padding-left: 0;
+    padding-left: 0.2em;
   }
 
   &__groups {

config/locales/client.en.yml

@@ -186,13 +186,25 @@ en:
         description: "These are the AI features available to visitors on your site. These can be configured to use specific personas and LLMs, and can be access controlled by groups."
         back: "Back"
         disabled: "(disabled)"
-        persona: "Persona:"
+        persona:
+          one: "Persona:"
+          other: "Personas:"
         groups: "Groups:"
-        llm: "LLM:"
+        llm:
+          one: "LLM:"
+          other: "LLMs:"
         no_llm: "No LLM selected"
         no_persona: "Not set"
         no_groups: "None"
         edit: "Edit"
+        expand_list:
+          one: "(%{count} more)"
+          other: "(%{count} more)"
+        collapse_list: "(show less)"
+        bot:
+          bot: "Chatbot"
+          name: "Bot"
+          description: "A chat bot that can answer questions and assist users in private messagges, forum and in chat"
         nav:
           configured: "Configured"
           unconfigured: "Unconfigured"

config/settings.yml

@@ -350,19 +350,22 @@ discourse_ai:
   ai_bot_enabled:
     default: false
     client: true
-    area: "ai-features/search"
+    area: "ai-features/bot"
   ai_bot_enable_chat_warning:
     default: false
     client: true
+    area: "ai-features/bot"
   ai_bot_debugging_allowed_groups:
     type: group_list
     list_type: compact
     default: ""
     allow_any: false
+    area: "ai-features/bot"
   ai_bot_allowed_groups:
     type: group_list
     list_type: compact
     default: "3|14" # 3: @staff, 14: @trust_level_4
+    area: "ai-features/bot"
   ai_bot_public_sharing_allowed_groups:
     client: false
     type: group_list
@@ -370,17 +373,21 @@ discourse_ai:
     default: "1|2" # 1: admins, 2: moderators
     allow_any: false
     refresh: true
+    area: "ai-features/bot"
   ai_bot_add_to_header:
     default: true
     client: true
+    area: "ai-features/bot"
   ai_bot_github_access_token:
     default: ""
     secret: true
+    area: "ai-features/bot"
   ai_bot_allowed_seeded_models:
     default: ""
     hidden: true
     type: list
     list_type: compact
+    area: "ai-features/bot"
   ai_bot_discover_persona:
     default: ""
     type: enum

lib/configuration/feature.rb

@@ -118,6 +118,32 @@ module DiscourseAi
           ]
         end
 
+        def bot_features
+          feature_cache[:bot] ||= [
+            new(
+              "bot",
+              nil,
+              DiscourseAi::Configuration::Module::BOT_ID,
+              DiscourseAi::Configuration::Module::BOT,
+              persona_ids_lookup: -> { lookup_bot_persona_ids },
+              llm_models_lookup: -> { lookup_bot_llms },
+            ),
+          ]
+        end
+
+        def lookup_bot_persona_ids
+          AiPersona
+            .where(enabled: true)
+            .where(
+              "allow_chat_channel_mentions OR allow_chat_direct_messages OR allow_topic_mentions OR allow_personal_messages",
+            )
+            .pluck(:id)
+        end
+
+        def lookup_bot_llms
+          LlmModel.where(enabled_chat_bot: true).to_a
+        end
+
         def translation_features
           feature_cache[:translation] ||= [
             new(
@@ -155,29 +181,41 @@ module DiscourseAi
             inference_features,
             ai_helper_features,
             translation_features,
+            bot_features,
           ].flatten
         end
 
-        def all_persona_setting_names
-          all.map(&:persona_setting)
-        end
-
         def find_features_using(persona_id:)
-          all.select { |feature| feature.persona_id == persona_id }
+          all.select { |feature| feature.persona_ids.include?(persona_id) }
         end
       end
 
-      def initialize(name, persona_setting, module_id, module_name, enabled_by_setting: "")
+      def initialize(
+        name,
+        persona_setting,
+        module_id,
+        module_name,
+        enabled_by_setting: "",
+        persona_ids_lookup: nil,
+        llm_models_lookup: nil
+      )
         @name = name
         @persona_setting = persona_setting
         @module_id = module_id
         @module_name = module_name
         @enabled_by_setting = enabled_by_setting
+        @persona_ids_lookup = persona_ids_lookup
+        @llm_models_lookup = llm_models_lookup
       end
 
-      def llm_model
-        persona = AiPersona.find_by(id: persona_id)
-        return if persona.blank?
+      def llm_models
+        return @llm_models_lookup.call if @llm_models_lookup
+        return if !persona_ids
+
+        llm_models = []
+        personas = AiPersona.where(id: persona_ids)
+        personas.each do |persona|
+          next if persona.blank?
 
           persona_klass = persona.class_instance
 
@@ -194,7 +232,11 @@ module DiscourseAi
           if llm_model.blank? && persona.default_llm_id
             llm_model = LlmModel.find_by(id: persona.default_llm_id)
           end
-        llm_model
+
+          llm_models << llm_model if llm_model
+        end
+
+        llm_models.compact.uniq
       end
 
       attr_reader :name, :persona_setting, :module_id, :module_name
@@ -203,8 +245,17 @@ module DiscourseAi
         @enabled_by_setting.blank? || SiteSetting.get(@enabled_by_setting)
       end
 
-      def persona_id
-        SiteSetting.get(persona_setting).to_i
+      def persona_ids
+        if @persona_ids_lookup
+          @persona_ids_lookup.call
+        else
+          id = SiteSetting.get(persona_setting).to_i
+          if id != 0
+            [id]
+          else
+            []
+          end
+        end
       end
     end
   end

lib/configuration/module.rb

@@ -9,8 +9,9 @@ module DiscourseAi
       INFERENCE = "inference"
       AI_HELPER = "ai_helper"
       TRANSLATION = "translation"
+      BOT = "bot"
 
-      NAMES = [SUMMARIZATION, SEARCH, DISCORD, INFERENCE, AI_HELPER, TRANSLATION]
+      NAMES = [SUMMARIZATION, SEARCH, DISCORD, INFERENCE, AI_HELPER, TRANSLATION, BOT].freeze
 
       SUMMARIZATION_ID = 1
       SEARCH_ID = 2
@@ -18,6 +19,7 @@ module DiscourseAi
       INFERENCE_ID = 4
       AI_HELPER_ID = 5
       TRANSLATION_ID = 6
+      BOT_ID = 7
 
       class << self
         def all
@@ -33,6 +35,7 @@ module DiscourseAi
               SEARCH,
               "ai_bot_enabled",
               features: DiscourseAi::Configuration::Feature.search_features,
+              extra_check: -> { SiteSetting.ai_bot_discover_persona.present? },
             ),
             new(
               DISCORD_ID,
@@ -58,6 +61,12 @@ module DiscourseAi
               "ai_translation_enabled",
               features: DiscourseAi::Configuration::Feature.translation_features,
             ),
+            new(
+              BOT_ID,
+              BOT,
+              "ai_bot_enabled",
+              features: DiscourseAi::Configuration::Feature.bot_features,
+            ),
           ]
         end
 
@@ -66,17 +75,24 @@ module DiscourseAi
         end
       end
 
-      def initialize(id, name, enabled_by_setting, features: [])
+      def initialize(id, name, enabled_by_setting, features: [], extra_check: nil)
         @id = id
         @name = name
         @enabled_by_setting = enabled_by_setting
         @features = features
+        @extra_check = extra_check
       end
 
       attr_reader :id, :name, :enabled_by_setting, :features
 
       def enabled?
-        SiteSetting.get(enabled_by_setting)
+        enabled_setting = SiteSetting.get(enabled_by_setting)
+
+        if @extra_check
+          enabled_setting && @extra_check.call
+        else
+          enabled_setting
+        end
       end
     end
   end

spec/configuration/feature_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe DiscourseAi::Configuration::Feature do
           )
 
         SiteSetting.ai_summarization_persona = 999_999
-        expect(ai_feature.llm_model).to be_nil
+        expect(ai_feature.llm_models).to eq([])
       end
     end
 
@@ -39,7 +39,7 @@ RSpec.describe DiscourseAi::Configuration::Feature do
       it "returns the configured llm model" do
         SiteSetting.ai_summarization_persona = ai_persona.id
         allow_configuring_setting { SiteSetting.ai_summarization_model = "custom:#{llm_model.id}" }
-        expect(ai_feature.llm_model).to eq(llm_model)
+        expect(ai_feature.llm_models).to eq([llm_model])
       end
     end
 
@@ -57,7 +57,7 @@ RSpec.describe DiscourseAi::Configuration::Feature do
         SiteSetting.ai_helper_proofreader_persona = ai_persona.id
         SiteSetting.ai_helper_model = ""
 
-        expect(ai_feature.llm_model).to eq(llm_model)
+        expect(ai_feature.llm_models).to eq([llm_model])
       end
     end
 
@@ -80,7 +80,7 @@ RSpec.describe DiscourseAi::Configuration::Feature do
           SiteSetting.ai_translation_model = "custom:#{translation_model.id}"
         end
 
-        expect(ai_feature.llm_model).to eq(translation_model)
+        expect(ai_feature.llm_models).to eq([translation_model])
       end
     end
   end
@@ -116,7 +116,85 @@ RSpec.describe DiscourseAi::Configuration::Feature do
     end
   end
 
-  describe "#persona_id" do
+  describe ".bot_features" do
+    fab!(:bot_llm) { Fabricate(:llm_model, enabled_chat_bot: true) }
+    fab!(:non_bot_llm) { Fabricate(:llm_model, enabled_chat_bot: false) }
+    fab!(:chat_persona) do
+      Fabricate(
+        :ai_persona,
+        default_llm_id: bot_llm.id,
+        allow_chat_channel_mentions: true,
+        allow_chat_direct_messages: false,
+      )
+    end
+    fab!(:dm_persona) do
+      Fabricate(
+        :ai_persona,
+        default_llm_id: bot_llm.id,
+        allow_chat_channel_mentions: false,
+        allow_chat_direct_messages: true,
+      )
+    end
+    fab!(:topic_persona) do
+      Fabricate(
+        :ai_persona,
+        default_llm_id: bot_llm.id,
+        allow_topic_mentions: true,
+        allow_personal_messages: false,
+      )
+    end
+    fab!(:pm_persona) do
+      Fabricate(:ai_persona, allow_topic_mentions: false, allow_personal_messages: true)
+    end
+    fab!(:inactive_persona) do
+      Fabricate(
+        :ai_persona,
+        enabled: false,
+        allow_chat_channel_mentions: false,
+        allow_chat_direct_messages: false,
+        allow_topic_mentions: false,
+        allow_personal_messages: true,
+      )
+    end
+
+    let(:bot_feature) { described_class.bot_features.first }
+
+    it "returns bot features with correct configuration" do
+      expect(bot_feature.name).to eq("bot")
+      expect(bot_feature.persona_setting).to be_nil
+      expect(bot_feature.module_id).to eq(DiscourseAi::Configuration::Module::BOT_ID)
+      expect(bot_feature.module_name).to eq(DiscourseAi::Configuration::Module::BOT)
+    end
+
+    it "returns only LLMs with enabled_chat_bot" do
+      expect(bot_feature.llm_models).to contain_exactly(bot_llm)
+      expect(bot_feature.llm_models).not_to include(non_bot_llm)
+    end
+
+    it "returns only personas with at least one bot permission enabled" do
+      expected_ids = [chat_persona.id, dm_persona.id, topic_persona.id, pm_persona.id]
+      AiPersona.where("id not in (:ids)", ids: expected_ids).update_all(enabled: false)
+      expect(bot_feature.persona_ids).to match_array(expected_ids)
+      expect(bot_feature.persona_ids).not_to include(inactive_persona.id)
+    end
+
+    it "includes personas with multiple permissions enabled" do
+      multi_permission_persona =
+        Fabricate(
+          :ai_persona,
+          enabled: true,
+          default_llm_id: bot_llm.id,
+          allow_chat_channel_mentions: true,
+          allow_chat_direct_messages: true,
+          allow_topic_mentions: true,
+          allow_personal_messages: true,
+        )
+
+      expect(bot_feature.persona_ids).to include(multi_permission_persona.id)
+    end
+  end
+
+  describe "#persona_ids" do
     it "returns the persona id from site settings" do
       ai_feature =
         described_class.new(
@@ -127,7 +205,7 @@ RSpec.describe DiscourseAi::Configuration::Feature do
         )
 
       SiteSetting.ai_summarization_persona = ai_persona.id
-      expect(ai_feature.persona_id).to eq(ai_persona.id)
+      expect(ai_feature.persona_ids).to eq([ai_persona.id])
     end
   end
 

spec/requests/admin/ai_features_controller_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe DiscourseAi::Admin::AiFeaturesController do
       get "/admin/plugins/discourse-ai/ai-features.json"
 
       expect(response.status).to eq(200)
-      expect(response.parsed_body["ai_features"].count).to eq(6)
+      expect(response.parsed_body["ai_features"].count).to eq(7)
     end
   end
 

spec/system/admin_ai_features_spec.rb

@@ -27,7 +27,8 @@ RSpec.describe "Admin AI features configuration", type: :system, js: true do
 
     ai_features_page.toggle_unconfigured
 
-    expect(ai_features_page).to have_listed_modules(5)
+    # this changes as we add more AI features
+    expect(ai_features_page).to have_listed_modules(6)
   end
 
   it "lists the persona used for the corresponding AI feature" do