From 73768ce9206a99b04b78c9c0c66fb6fb24648d5a Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 27 Jun 2025 12:35:41 +1000 Subject: [PATCH] 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 --- ...plugins-show-discourse-ai-features-edit.js | 8 + .../admin/ai_features_controller.rb | 14 +- .../discourse/components/ai-features-list.gjs | 284 ++++++++++++------ assets/stylesheets/common/ai-features.scss | 10 +- config/locales/client.en.yml | 16 +- config/settings.yml | 9 +- lib/configuration/feature.rb | 97 ++++-- lib/configuration/module.rb | 22 +- spec/configuration/feature_spec.rb | 90 +++++- .../admin/ai_features_controller_spec.rb | 2 +- spec/system/admin_ai_features_spec.rb | 3 +- 11 files changed, 420 insertions(+), 135 deletions(-) diff --git a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-features-edit.js b/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-features-edit.js index c6d8ae41..a0eac213 100644 --- a/admin/assets/javascripts/discourse/routes/admin-plugins-show-discourse-ai-features-edit.js +++ b/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"); + } } diff --git a/app/controllers/discourse_ai/admin/ai_features_controller.rb b/app/controllers/discourse_ai/admin/ai_features_controller.rb index 587ef700..d66e5b8b 100644 --- a/app/controllers/discourse_ai/admin/ai_features_controller.rb +++ b/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 diff --git a/assets/javascripts/discourse/components/ai-features-list.gjs b/assets/javascripts/discourse/components/ai-features-list.gjs index d3643dc6..8253990e 100644 --- a/assets/javascripts/discourse/components/ai-features-list.gjs +++ b/assets/javascripts/discourse/components/ai-features-list.gjs @@ -1,98 +1,206 @@ +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 = +} diff --git a/assets/stylesheets/common/ai-features.scss b/assets/stylesheets/common/ai-features.scss index 11c0835a..40590f94 100644 --- a/assets/stylesheets/common/ai-features.scss +++ b/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 { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 64b4ab4f..971c1c8b 100644 --- a/config/locales/client.en.yml +++ b/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" diff --git a/config/settings.yml b/config/settings.yml index 6230d201..e0b0cc69 100644 --- a/config/settings.yml +++ b/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 diff --git a/lib/configuration/feature.rb b/lib/configuration/feature.rb index f1e6261a..d3f71056 100644 --- a/lib/configuration/feature.rb +++ b/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,46 +181,62 @@ 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 - persona_klass = persona.class_instance + llm_models = [] + personas = AiPersona.where(id: persona_ids) + personas.each do |persona| + next if persona.blank? - llm_model = - case module_name - when DiscourseAi::Configuration::Module::SUMMARIZATION - DiscourseAi::Summarization.find_summarization_model(persona_klass) - when DiscourseAi::Configuration::Module::AI_HELPER - DiscourseAi::AiHelper::Assistant.find_ai_helper_model(name, persona_klass) - when DiscourseAi::Configuration::Module::TRANSLATION - DiscourseAi::Translation::BaseTranslator.preferred_llm_model(persona_klass) + persona_klass = persona.class_instance + + llm_model = + case module_name + when DiscourseAi::Configuration::Module::SUMMARIZATION + DiscourseAi::Summarization.find_summarization_model(persona_klass) + when DiscourseAi::Configuration::Module::AI_HELPER + DiscourseAi::AiHelper::Assistant.find_ai_helper_model(name, persona_klass) + when DiscourseAi::Configuration::Module::TRANSLATION + DiscourseAi::Translation::BaseTranslator.preferred_llm_model(persona_klass) + end + + if llm_model.blank? && persona.default_llm_id + llm_model = LlmModel.find_by(id: persona.default_llm_id) end - if llm_model.blank? && persona.default_llm_id - llm_model = LlmModel.find_by(id: persona.default_llm_id) + llm_models << llm_model if llm_model end - llm_model + + 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 diff --git a/lib/configuration/module.rb b/lib/configuration/module.rb index 1d99643a..74921e1e 100644 --- a/lib/configuration/module.rb +++ b/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 diff --git a/spec/configuration/feature_spec.rb b/spec/configuration/feature_spec.rb index bab8bba2..19368d12 100644 --- a/spec/configuration/feature_spec.rb +++ b/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 diff --git a/spec/requests/admin/ai_features_controller_spec.rb b/spec/requests/admin/ai_features_controller_spec.rb index 68062ecc..7bb1bcc0 100644 --- a/spec/requests/admin/ai_features_controller_spec.rb +++ b/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 diff --git a/spec/system/admin_ai_features_spec.rb b/spec/system/admin_ai_features_spec.rb index f24dd9ed..be111785 100644 --- a/spec/system/admin_ai_features_spec.rb +++ b/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