From 15003084376a65850258cf1937ba6e338f4158e7 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 23 Oct 2023 17:00:58 +1100 Subject: [PATCH] FEATURE: defer creation of bot users (#258) Also fixes it so users without bot in header can send it messages. Previous to this change we would seed all bots with database seeds. This lead to lots of confusion for people who do not enable ai bot. Instead: 1. We do not seed any bots **until** user enables the ai_bot_enabled setting 2. If it is disabled we will a. If no messages were created by bot - delete it b. Otherwise we will deactivate account --- .../after-d-editor/composer-open.hbs | 2 +- .../after-d-editor/composer-open.js | 13 +++++ .../modules/ai-bot/common/bot-replies.scss | 12 +++++ db/fixtures/ai_bot/602_bot_users.rb | 27 +--------- lib/modules/ai_bot/entry_point.rb | 13 ++++- lib/modules/ai_bot/site_settings_extension.rb | 41 +++++++++++++++ spec/lib/modules/ai_bot/anthropic_bot_spec.rb | 5 ++ spec/lib/modules/ai_bot/bot_spec.rb | 22 +++++--- .../modules/ai_bot/commands/command_spec.rb | 4 +- .../ai_bot/commands/google_command_spec.rb | 4 +- .../ai_bot/commands/image_command_spec.rb | 4 +- .../ai_bot/commands/read_command_spec.rb | 4 +- .../ai_bot/commands/search_command_spec.rb | 2 + .../ai_bot/commands/summarize_command_spec.rb | 4 +- spec/lib/modules/ai_bot/entry_point_spec.rb | 2 + .../jobs/regular/create_ai_reply_spec.rb | 2 + .../regular/update_ai_bot_pm_title_spec.rb | 5 ++ spec/lib/modules/ai_bot/open_ai_bot_spec.rb | 11 ++-- .../ai_bot/site_setting_extension_spec.rb | 50 +++++++++++++++++++ spec/requests/ai_bot/bot_controller_spec.rb | 1 + 20 files changed, 186 insertions(+), 42 deletions(-) create mode 100644 lib/modules/ai_bot/site_settings_extension.rb create mode 100644 spec/lib/modules/ai_bot/site_setting_extension_spec.rb diff --git a/assets/javascripts/discourse/connectors/after-d-editor/composer-open.hbs b/assets/javascripts/discourse/connectors/after-d-editor/composer-open.hbs index 02110038..d1d194d3 100644 --- a/assets/javascripts/discourse/connectors/after-d-editor/composer-open.hbs +++ b/assets/javascripts/discourse/connectors/after-d-editor/composer-open.hbs @@ -1,5 +1,5 @@ {{#if this.isAiBotChat}} - + {{#if this.renderChatWarning}}
{{i18n "discourse_ai.ai_bot.pm_warning" diff --git a/assets/javascripts/discourse/connectors/after-d-editor/composer-open.js b/assets/javascripts/discourse/connectors/after-d-editor/composer-open.js index da5633db..493d7c14 100644 --- a/assets/javascripts/discourse/connectors/after-d-editor/composer-open.js +++ b/assets/javascripts/discourse/connectors/after-d-editor/composer-open.js @@ -1,6 +1,7 @@ import Component from "@glimmer/component"; import { inject as service } from "@ember/service"; import { computed } from "@ember/object"; +import I18n from "discourse-i18n"; export default class extends Component { @service currentUser; @@ -14,6 +15,18 @@ export default class extends Component { return this.siteSettings.ai_bot_enable_chat_warning; } + @computed("composerModel.targetRecipients", "composerModel.title") + get aiBotClasses() { + if ( + this.composerModel?.title === + I18n.t("discourse_ai.ai_bot.default_pm_prefix") + ) { + return "ai-bot-chat"; + } else { + return "ai-bot-pm"; + } + } + @computed("composerModel.targetRecipients") get isAiBotChat() { if ( diff --git a/assets/stylesheets/modules/ai-bot/common/bot-replies.scss b/assets/stylesheets/modules/ai-bot/common/bot-replies.scss index 0c487257..b712c8e1 100644 --- a/assets/stylesheets/modules/ai-bot/common/bot-replies.scss +++ b/assets/stylesheets/modules/ai-bot/common/bot-replies.scss @@ -15,6 +15,18 @@ nav.post-controls .actions button.cancel-streaming { } } +.ai-bot-pm { + .gpt-persona { + margin-bottom: 5px; + } + #reply-control .composer-fields { + .mini-tag-chooser, + .add-warning { + display: none; + } + } +} + .ai-bot-chat-warning { color: var(--tertiary); background-color: var(--tertiary-low); diff --git a/db/fixtures/ai_bot/602_bot_users.rb b/db/fixtures/ai_bot/602_bot_users.rb index 8ddb024b..ae4d43f2 100644 --- a/db/fixtures/ai_bot/602_bot_users.rb +++ b/db/fixtures/ai_bot/602_bot_users.rb @@ -1,28 +1,3 @@ # frozen_string_literal: true -DiscourseAi::AiBot::EntryPoint::BOTS.each do |id, bot_username| - # let's not create a bot user if it already exists - # seed seems to be messing with dates on the user - # causing it to look like these bots were created at the - # wrong time - if !User.exists?(id: id) - UserEmail.seed do |ue| - ue.id = id - ue.email = "no_email_#{bot_username}" - ue.primary = true - ue.user_id = id - end - - User.seed do |u| - u.id = id - u.name = bot_username.titleize - u.username = UserNameSuggester.suggest(bot_username) - u.password = SecureRandom.hex - u.active = true - u.admin = true - u.moderator = true - u.approved = true - u.trust_level = TrustLevel[4] - end - end -end +DiscourseAi::AiBot::SiteSettingsExtension.enable_or_disable_ai_bots diff --git a/lib/modules/ai_bot/entry_point.rb b/lib/modules/ai_bot/entry_point.rb index 3ab557c7..660d908a 100644 --- a/lib/modules/ai_bot/entry_point.rb +++ b/lib/modules/ai_bot/entry_point.rb @@ -8,7 +8,11 @@ module DiscourseAi GPT4_ID = -110 GPT3_5_TURBO_ID = -111 CLAUDE_V2_ID = -112 - BOTS = [[GPT4_ID, "gpt4_bot"], [GPT3_5_TURBO_ID, "gpt3.5_bot"], [CLAUDE_V2_ID, "claude_bot"]] + BOTS = [ + [GPT4_ID, "gpt4_bot", "gpt-4"], + [GPT3_5_TURBO_ID, "gpt3.5_bot", "gpt-3.5-turbo"], + [CLAUDE_V2_ID, "claude_bot", "claude-2"], + ] def self.map_bot_model_to_user_id(model_name) case model_name @@ -48,9 +52,16 @@ module DiscourseAi require_relative "personas/settings_explorer" require_relative "personas/researcher" require_relative "personas/creative" + require_relative "site_settings_extension" end def inject_into(plugin) + plugin.on(:site_setting_changed) do |name, _old_value, _new_value| + if name == :ai_bot_enabled_chat_bots || name == :ai_bot_enabled + DiscourseAi::AiBot::SiteSettingsExtension.enable_or_disable_ai_bots + end + end + plugin.register_seedfu_fixtures( Rails.root.join("plugins", "discourse-ai", "db", "fixtures", "ai_bot"), ) diff --git a/lib/modules/ai_bot/site_settings_extension.rb b/lib/modules/ai_bot/site_settings_extension.rb new file mode 100644 index 00000000..77f17641 --- /dev/null +++ b/lib/modules/ai_bot/site_settings_extension.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module DiscourseAi::AiBot::SiteSettingsExtension + def self.enable_or_disable_ai_bots + enabled_bots = SiteSetting.ai_bot_enabled_chat_bots_map + enabled_bots = [] if !SiteSetting.ai_bot_enabled + DiscourseAi::AiBot::EntryPoint::BOTS.each do |id, bot_name, name| + active = enabled_bots.include?(name) + user = User.find_by(id: id) + + if active + if !user + user = + User.new( + id: id, + email: "no_email_#{name}", + name: bot_name.titleize, + username: UserNameSuggester.suggest(bot_name), + active: true, + approved: true, + admin: true, + moderator: true, + trust_level: TrustLevel[4], + ) + user.save!(validate: false) + else + user.update!(active: true) + end + elsif !active && user + # will include deleted + has_posts = DB.query_single("SELECT 1 FROM posts WHERE user_id = #{id} LIMIT 1").present? + + if has_posts + user.update!(active: false) if user.active + else + user.destroy + end + end + end + end +end diff --git a/spec/lib/modules/ai_bot/anthropic_bot_spec.rb b/spec/lib/modules/ai_bot/anthropic_bot_spec.rb index 73d4838e..8d1411d4 100644 --- a/spec/lib/modules/ai_bot/anthropic_bot_spec.rb +++ b/spec/lib/modules/ai_bot/anthropic_bot_spec.rb @@ -7,6 +7,11 @@ module ::DiscourseAi User.find(EntryPoint::CLAUDE_V2_ID) end + before do + SiteSetting.ai_bot_enabled_chat_bots = "claude-2" + SiteSetting.ai_bot_enabled = true + end + let(:bot) { described_class.new(bot_user) } let(:post) { Fabricate(:post) } diff --git a/spec/lib/modules/ai_bot/bot_spec.rb b/spec/lib/modules/ai_bot/bot_spec.rb index 41427fbc..3a2d654c 100644 --- a/spec/lib/modules/ai_bot/bot_spec.rb +++ b/spec/lib/modules/ai_bot/bot_spec.rb @@ -39,7 +39,12 @@ class FakeBot < DiscourseAi::AiBot::Bot end describe FakeBot do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT4_ID) } + before do + SiteSetting.ai_bot_enabled_chat_bots = "gpt-4" + SiteSetting.ai_bot_enabled = true + end + + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT4_ID) } fab!(:post) { Fabricate(:post, raw: "hello world") } it "can handle command truncation for long messages" do @@ -78,11 +83,16 @@ describe FakeBot do end describe DiscourseAi::AiBot::Bot do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT4_ID) } - fab!(:bot) { described_class.as(bot_user) } + before do + SiteSetting.ai_bot_enabled_chat_bots = "gpt-4" + SiteSetting.ai_bot_enabled = true + end + + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT4_ID) } + let(:bot) { described_class.as(bot_user) } fab!(:user) { Fabricate(:user) } - fab!(:pm) do + let!(:pm) do Fabricate( :private_message_topic, title: "This is my special PM", @@ -93,8 +103,8 @@ describe DiscourseAi::AiBot::Bot do ], ) end - fab!(:first_post) { Fabricate(:post, topic: pm, user: user, raw: "This is a reply by the user") } - fab!(:second_post) do + let!(:first_post) { Fabricate(:post, topic: pm, user: user, raw: "This is a reply by the user") } + let!(:second_post) do Fabricate(:post, topic: pm, user: user, raw: "This is a second reply by the user") end diff --git a/spec/lib/modules/ai_bot/commands/command_spec.rb b/spec/lib/modules/ai_bot/commands/command_spec.rb index 66dd6b05..e61561ee 100644 --- a/spec/lib/modules/ai_bot/commands/command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/command_spec.rb @@ -3,9 +3,11 @@ require_relative "../../../../support/openai_completions_inference_stubs" RSpec.describe DiscourseAi::AiBot::Commands::Command do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } let(:command) { DiscourseAi::AiBot::Commands::GoogleCommand.new(bot_user: bot_user, args: nil) } + before { SiteSetting.ai_bot_enabled = true } + describe "#format_results" do it "can generate efficient tables of data" do rows = [1, 2, 3, 4, 5] diff --git a/spec/lib/modules/ai_bot/commands/google_command_spec.rb b/spec/lib/modules/ai_bot/commands/google_command_spec.rb index 68e3fc3d..f932cef3 100644 --- a/spec/lib/modules/ai_bot/commands/google_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/google_command_spec.rb @@ -1,7 +1,9 @@ #frozen_string_literal: true RSpec.describe DiscourseAi::AiBot::Commands::GoogleCommand do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + + before { SiteSetting.ai_bot_enabled = true } describe "#process" do it "will not explode if there are no results" do diff --git a/spec/lib/modules/ai_bot/commands/image_command_spec.rb b/spec/lib/modules/ai_bot/commands/image_command_spec.rb index 1d780d32..c2fc7b81 100644 --- a/spec/lib/modules/ai_bot/commands/image_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/image_command_spec.rb @@ -3,7 +3,9 @@ require_relative "../../../../support/stable_difussion_stubs" RSpec.describe DiscourseAi::AiBot::Commands::ImageCommand do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + + before { SiteSetting.ai_bot_enabled = true } describe "#process" do it "can generate correct info" do diff --git a/spec/lib/modules/ai_bot/commands/read_command_spec.rb b/spec/lib/modules/ai_bot/commands/read_command_spec.rb index dc100bc0..26b04d81 100644 --- a/spec/lib/modules/ai_bot/commands/read_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/read_command_spec.rb @@ -1,7 +1,7 @@ #frozen_string_literal: true RSpec.describe DiscourseAi::AiBot::Commands::ReadCommand do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } fab!(:parent_category) { Fabricate(:category, name: "animals") } fab!(:category) { Fabricate(:category, parent_category: parent_category, name: "amazing-cat") } @@ -22,6 +22,8 @@ RSpec.describe DiscourseAi::AiBot::Commands::ReadCommand do Fabricate(:topic, category: category, tags: [tag_funny, tag_sad, tag_hidden]) end + before { SiteSetting.ai_bot_enabled = true } + describe "#process" do it "can read a topic" do topic_id = topic_with_tags.id diff --git a/spec/lib/modules/ai_bot/commands/search_command_spec.rb b/spec/lib/modules/ai_bot/commands/search_command_spec.rb index ba8728e6..2c43ce4a 100644 --- a/spec/lib/modules/ai_bot/commands/search_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/search_command_spec.rb @@ -28,6 +28,8 @@ RSpec.describe DiscourseAi::AiBot::Commands::SearchCommand do Fabricate(:topic, category: category, tags: [tag_funny, tag_sad, tag_hidden]) end + before { SiteSetting.ai_bot_enabled = true } + describe "#process" do it "can handle no results" do post1 = Fabricate(:post, topic: topic_with_tags) diff --git a/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb b/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb index 5b7888a0..078a288d 100644 --- a/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb @@ -3,7 +3,9 @@ require_relative "../../../../support/openai_completions_inference_stubs" RSpec.describe DiscourseAi::AiBot::Commands::SummarizeCommand do - fab!(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID) } + + before { SiteSetting.ai_bot_enabled = true } describe "#process" do it "can generate correct info" do diff --git a/spec/lib/modules/ai_bot/entry_point_spec.rb b/spec/lib/modules/ai_bot/entry_point_spec.rb index d6d04ebd..b3326a54 100644 --- a/spec/lib/modules/ai_bot/entry_point_spec.rb +++ b/spec/lib/modules/ai_bot/entry_point_spec.rb @@ -17,6 +17,8 @@ RSpec.describe DiscourseAi::AiBot::EntryPoint do end before do + SiteSetting.ai_bot_enabled_chat_bots = "gpt-4|claude-2" + SiteSetting.ai_bot_enabled = true SiteSetting.ai_bot_allowed_groups = bot_allowed_group.id bot_allowed_group.add(admin) end diff --git a/spec/lib/modules/ai_bot/jobs/regular/create_ai_reply_spec.rb b/spec/lib/modules/ai_bot/jobs/regular/create_ai_reply_spec.rb index 95b1f3de..212b2e00 100644 --- a/spec/lib/modules/ai_bot/jobs/regular/create_ai_reply_spec.rb +++ b/spec/lib/modules/ai_bot/jobs/regular/create_ai_reply_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Jobs::CreateAiReply do before do # got to do this cause we include times in system message freeze_time + SiteSetting.ai_bot_enabled = true end describe "#execute" do @@ -78,6 +79,7 @@ RSpec.describe Jobs::CreateAiReply do let(:deltas) { claude_response.split(" ").map { |w| "#{w} " } } before do + SiteSetting.ai_bot_enabled_chat_bots = "claude-2" bot_user = User.find(DiscourseAi::AiBot::EntryPoint::CLAUDE_V2_ID) AnthropicCompletionStubs.stub_streamed_response( diff --git a/spec/lib/modules/ai_bot/jobs/regular/update_ai_bot_pm_title_spec.rb b/spec/lib/modules/ai_bot/jobs/regular/update_ai_bot_pm_title_spec.rb index 1916c268..4477e7da 100644 --- a/spec/lib/modules/ai_bot/jobs/regular/update_ai_bot_pm_title_spec.rb +++ b/spec/lib/modules/ai_bot/jobs/regular/update_ai_bot_pm_title_spec.rb @@ -4,6 +4,11 @@ RSpec.describe Jobs::UpdateAiBotPmTitle do let(:user) { Fabricate(:admin) } let(:bot_user) { User.find(DiscourseAi::AiBot::EntryPoint::CLAUDE_V2_ID) } + before do + SiteSetting.ai_bot_enabled_chat_bots = "claude-2" + SiteSetting.ai_bot_enabled = true + end + it "will properly update title on bot PMs" do SiteSetting.ai_bot_allowed_groups = Group::AUTO_GROUPS[:staff] diff --git a/spec/lib/modules/ai_bot/open_ai_bot_spec.rb b/spec/lib/modules/ai_bot/open_ai_bot_spec.rb index 6d591496..f515c991 100644 --- a/spec/lib/modules/ai_bot/open_ai_bot_spec.rb +++ b/spec/lib/modules/ai_bot/open_ai_bot_spec.rb @@ -14,6 +14,11 @@ RSpec.describe DiscourseAi::AiBot::OpenAiBot do subject { described_class.new(bot_user) } + before do + SiteSetting.ai_bot_enabled_chat_bots = "gpt-4" + SiteSetting.ai_bot_enabled = true + end + context "when changing available commands" do it "contains all commands by default" do # this will break as we add commands, but it is important as a sanity check @@ -69,11 +74,11 @@ RSpec.describe DiscourseAi::AiBot::OpenAiBot do end context "when the topic has multiple posts" do - fab!(:post_1) { Fabricate(:post, topic: topic, raw: post_body(1), post_number: 1) } - fab!(:post_2) do + let!(:post_1) { Fabricate(:post, topic: topic, raw: post_body(1), post_number: 1) } + let!(:post_2) do Fabricate(:post, topic: topic, user: bot_user, raw: post_body(2), post_number: 2) end - fab!(:post_3) { Fabricate(:post, topic: topic, raw: post_body(3), post_number: 3) } + let!(:post_3) { Fabricate(:post, topic: topic, raw: post_body(3), post_number: 3) } it "includes them in the prompt respecting the post number order" do prompt_messages = subject.bot_prompt_with_topic_context(post_3) diff --git a/spec/lib/modules/ai_bot/site_setting_extension_spec.rb b/spec/lib/modules/ai_bot/site_setting_extension_spec.rb new file mode 100644 index 00000000..7b4144cd --- /dev/null +++ b/spec/lib/modules/ai_bot/site_setting_extension_spec.rb @@ -0,0 +1,50 @@ +#frozen_string_literal: true + +describe DiscourseAi::AiBot::SiteSettingsExtension do + it "correctly creates/deletes bot accounts as needed" do + SiteSetting.ai_bot_enabled = true + SiteSetting.ai_bot_enabled_chat_bots = "gpt-4" + + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT4_ID)).to eq(true) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID)).to eq(false) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::CLAUDE_V2_ID)).to eq(false) + + SiteSetting.ai_bot_enabled_chat_bots = "gpt-3.5-turbo" + + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT4_ID)).to eq(false) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID)).to eq(true) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::CLAUDE_V2_ID)).to eq(false) + + SiteSetting.ai_bot_enabled_chat_bots = "gpt-3.5-turbo|claude-2" + + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT4_ID)).to eq(false) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID)).to eq(true) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::CLAUDE_V2_ID)).to eq(true) + + SiteSetting.ai_bot_enabled = false + + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT4_ID)).to eq(false) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID)).to eq(false) + expect(User.exists?(id: DiscourseAi::AiBot::EntryPoint::CLAUDE_V2_ID)).to eq(false) + end + + it "leaves accounts around if they have any posts" do + SiteSetting.ai_bot_enabled = true + SiteSetting.ai_bot_enabled_chat_bots = "gpt-4" + + user = User.find(DiscourseAi::AiBot::EntryPoint::GPT4_ID) + + create_post(user: user, raw: "this is a test post") + + user.reload + SiteSetting.ai_bot_enabled = false + + user.reload + expect(user.active).to eq(false) + + SiteSetting.ai_bot_enabled = true + + user.reload + expect(user.active).to eq(true) + end +end diff --git a/spec/requests/ai_bot/bot_controller_spec.rb b/spec/requests/ai_bot/bot_controller_spec.rb index 8cebaa7c..32722e46 100644 --- a/spec/requests/ai_bot/bot_controller_spec.rb +++ b/spec/requests/ai_bot/bot_controller_spec.rb @@ -30,6 +30,7 @@ RSpec.describe DiscourseAi::AiBot::BotController do describe "#show_bot_username" do it "returns the username_lower of the selected bot" do + SiteSetting.ai_bot_enabled = true gpt_3_5_bot = "gpt-3.5-turbo" expected_username = User.find(DiscourseAi::AiBot::EntryPoint::GPT3_5_TURBO_ID).username_lower