From 5b5edb22c6a3230451af8759cfd39bc0daa83f2e Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 21 Nov 2023 16:56:43 +1100 Subject: [PATCH] FEATURE: UI to update ai personas on admin page (#290) Introduces a UI to manage customizable personas (admin only feature) Part of the change was some extensive internal refactoring: - AIBot now has a persona set in the constructor, once set it never changes - Command now takes in bot as a constructor param, so it has the correct persona and is not generating AIBot objects on the fly - Added a .prettierignore file, due to the way ALE is configured in nvim it is a pre-req for prettier to work - Adds a bunch of validations on the AIPersona model, system personas (artist/creative etc...) are all seeded. We now ensure - name uniqueness, and only allow certain properties to be touched for system personas. - (JS note) the client side design takes advantage of nested routes, the parent route for personas gets all the personas via this.store.findAll("ai-persona") then child routes simply reach into this model to find a particular persona. - (JS note) data is sideloaded into the ai-persona model the meta property supplied from the controller, resultSetMeta - This removes ai_bot_enabled_personas and ai_bot_enabled_chat_commands, both should be controlled from the UI on a per persona basis - Fixes a long standing bug in token accounting ... we were doing to_json.length instead of to_json.to_s.length - Amended it so {commands} are always inserted at the end unconditionally, no need to add it to the template of the system message as it just confuses things - Adds a concept of required_commands to stock personas, these are commands that must be configured for this stock persona to show up. - Refactored tests so we stop requiring inference_stubs, it was very confusing to need it, added to plugin.rb for now which at least is clearer - Migrates the persona selector to gjs --------- Co-authored-by: Joffrey JAFFEUX Co-authored-by: Martin Brennan --- .prettierignore | 21 ++ .../admin/ai_personas_controller.rb | 71 ++++++ app/models/ai_persona.rb | 160 +++++++++---- .../localized_ai_persona_serializer.rb | 23 ++ .../admin-discourse-ai-personas-route-map.js | 14 ++ .../discourse/admin/adapters/ai-persona.js | 17 ++ .../discourse/admin/models/ai-persona.js | 28 +++ .../components/ai-command-selector.gjs | 18 ++ .../components/ai-persona-editor.gjs | 224 ++++++++++++++++++ .../components/ai-persona-list-editor.gjs | 66 ++++++ .../components/ai-suggestion-dropdown.gjs | 2 +- ...rsona-selector.js => persona-selector.gjs} | 25 +- .../composer-fields/persona-selector.hbs | 7 - ...-plugins-discourse-ai-ai-personas-index.js | 7 + ...in-plugins-discourse-ai-ai-personas-new.js | 18 ++ ...n-plugins-discourse-ai-ai-personas-show.js | 17 ++ .../admin-plugins-discourse-ai-ai-personas.js | 7 + .../admin-plugins-discourse-ai-index.js | 7 + .../discourse-ai/ai-personas/index.hbs | 1 + .../discourse-ai/ai-personas/new.hbs | 4 + .../discourse-ai/ai-personas/show.hbs | 4 + .../modules/ai-bot/common/ai-persona.scss | 45 ++++ config/locales/client.en.yml | 29 +++ config/locales/server.en.yml | 6 +- config/routes.rb | 7 + config/settings.yml | 23 -- db/fixtures/ai_bot/603_bot_ai_personas.rb | 38 +++ ..._add_system_and_priority_to_ai_personas.rb | 8 + .../20231120033747_remove_site_settings.rb | 12 + lib/modules/ai_bot/anthropic_bot.rb | 7 +- lib/modules/ai_bot/bot.rb | 81 ++++--- lib/modules/ai_bot/commands/command.rb | 11 +- lib/modules/ai_bot/entry_point.rb | 12 +- .../ai_bot/jobs/regular/create_ai_reply.rb | 18 +- lib/modules/ai_bot/open_ai_bot.rb | 13 +- lib/modules/ai_bot/personas/artist.rb | 7 +- lib/modules/ai_bot/personas/general.rb | 12 +- lib/modules/ai_bot/personas/persona.rb | 98 +++++--- lib/modules/ai_bot/personas/researcher.rb | 7 +- .../ai_bot/personas/settings_explorer.rb | 3 - lib/modules/ai_bot/personas/sql_helper.rb | 2 - plugin.rb | 9 + spec/fabricators/ai_persona_fabricator.rb | 6 + spec/lib/modules/ai_bot/anthropic_bot_spec.rb | 5 +- spec/lib/modules/ai_bot/bot_spec.rb | 10 +- .../commands/categories_command_spec.rb | 4 +- .../modules/ai_bot/commands/command_spec.rb | 5 +- .../ai_bot/commands/db_schema_command_spec.rb | 2 +- .../ai_bot/commands/google_command_spec.rb | 9 +- .../ai_bot/commands/image_command_spec.rb | 3 +- .../ai_bot/commands/read_command_spec.rb | 3 +- .../ai_bot/commands/search_command_spec.rb | 15 +- .../commands/search_settings_command_spec.rb | 4 +- .../ai_bot/commands/summarize_command_spec.rb | 7 +- .../ai_bot/commands/tags_command_spec.rb | 4 +- .../ai_bot/commands/time_command_spec.rb | 4 +- .../jobs/regular/create_ai_reply_spec.rb | 13 +- spec/lib/modules/ai_bot/open_ai_bot_spec.rb | 26 +- .../modules/ai_bot/personas/persona_spec.rb | 63 ++--- spec/lib/modules/ai_helper/llm_prompt_spec.rb | 2 - spec/lib/modules/ai_helper/painter_spec.rb | 5 +- .../embeddings/semantic_search_spec.rb | 1 - .../summarization/models/anthropic_spec.rb | 2 - .../summarization/models/open_ai_spec.rb | 2 - spec/models/ai_persona_spec.rb | 2 +- .../admin/ai_personas_controller_spec.rb | 191 +++++++++++++++ .../ai_helper/assistant_controller_spec.rb | 2 - .../inference/anthropic_completions_spec.rb | 2 - .../inference/openai_completions_spec.rb | 2 - ...ion_stubs.rb => stable_diffusion_stubs.rb} | 0 spec/system/ai_bot/persona_spec.rb | 49 ++++ .../ai_helper/ai_composer_helper_spec.rb | 2 - spec/system/ai_helper/ai_post_helper_spec.rb | 2 - 73 files changed, 1310 insertions(+), 326 deletions(-) create mode 100644 .prettierignore create mode 100644 app/controllers/discourse_ai/admin/ai_personas_controller.rb create mode 100644 app/serializers/localized_ai_persona_serializer.rb create mode 100644 assets/javascripts/discourse/admin-discourse-ai-personas-route-map.js create mode 100644 assets/javascripts/discourse/admin/adapters/ai-persona.js create mode 100644 assets/javascripts/discourse/admin/models/ai-persona.js create mode 100644 assets/javascripts/discourse/components/ai-command-selector.gjs create mode 100644 assets/javascripts/discourse/components/ai-persona-editor.gjs create mode 100644 assets/javascripts/discourse/components/ai-persona-list-editor.gjs rename assets/javascripts/discourse/connectors/composer-fields/{persona-selector.js => persona-selector.gjs} (63%) delete mode 100644 assets/javascripts/discourse/connectors/composer-fields/persona-selector.hbs create mode 100644 assets/javascripts/discourse/routes/admin-plugins-discourse-ai-ai-personas-index.js create mode 100644 assets/javascripts/discourse/routes/admin-plugins-discourse-ai-ai-personas-new.js create mode 100644 assets/javascripts/discourse/routes/admin-plugins-discourse-ai-ai-personas-show.js create mode 100644 assets/javascripts/discourse/routes/admin-plugins-discourse-ai-ai-personas.js create mode 100644 assets/javascripts/discourse/routes/admin-plugins-discourse-ai-index.js create mode 100644 assets/javascripts/discourse/templates/admin-plugins/discourse-ai/ai-personas/index.hbs create mode 100644 assets/javascripts/discourse/templates/admin-plugins/discourse-ai/ai-personas/new.hbs create mode 100644 assets/javascripts/discourse/templates/admin-plugins/discourse-ai/ai-personas/show.hbs create mode 100644 assets/stylesheets/modules/ai-bot/common/ai-persona.scss create mode 100644 db/fixtures/ai_bot/603_bot_ai_personas.rb create mode 100644 db/migrate/20231117050928_add_system_and_priority_to_ai_personas.rb create mode 100644 db/migrate/20231120033747_remove_site_settings.rb create mode 100644 spec/fabricators/ai_persona_fabricator.rb create mode 100644 spec/requests/admin/ai_personas_controller_spec.rb rename spec/support/{stable_difussion_stubs.rb => stable_diffusion_stubs.rb} (100%) create mode 100644 spec/system/ai_bot/persona_spec.rb diff --git a/.prettierignore b/.prettierignore new file mode 100644 index 00000000..2c3b88d4 --- /dev/null +++ b/.prettierignore @@ -0,0 +1,21 @@ +app/assets/stylesheets/vendor/ +documentation/ +package.json +config/locales/**/*.yml +!config/locales/**/*.en*.yml +script/import_scripts/**/*.yml + +plugins/**/lib/javascripts/locale +public/ +!/app/assets/javascripts/discourse/public +vendor/ +app/assets/javascripts/discourse/tests/fixtures +spec/ +node_modules/ +dist/ +tmp/ + +**/*.rb +**/*.html +**/*.json +**/*.md diff --git a/app/controllers/discourse_ai/admin/ai_personas_controller.rb b/app/controllers/discourse_ai/admin/ai_personas_controller.rb new file mode 100644 index 00000000..f2e5b161 --- /dev/null +++ b/app/controllers/discourse_ai/admin/ai_personas_controller.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +module DiscourseAi + module Admin + class AiPersonasController < ::Admin::AdminController + before_action :find_ai_persona, only: %i[show update destroy] + + def index + ai_personas = + AiPersona.ordered.map do |persona| + # we use a special serializer here cause names and descriptions are + # localized for system personas + LocalizedAiPersonaSerializer.new(persona, root: false) + end + commands = + DiscourseAi::AiBot::Personas::Persona.all_available_commands.map do |command| + { id: command.to_s.split("::").last, name: command.name.humanize.titleize } + end + render json: { ai_personas: ai_personas, meta: { commands: commands } } + end + + def show + render json: LocalizedAiPersonaSerializer.new(@ai_persona) + end + + def create + ai_persona = AiPersona.new(ai_persona_params) + if ai_persona.save + render json: { ai_persona: ai_persona }, status: :created + else + render_json_error ai_persona + end + end + + def update + if @ai_persona.update(ai_persona_params) + render json: @ai_persona + else + render_json_error @ai_persona + end + end + + def destroy + if @ai_persona.destroy + head :no_content + else + render_json_error @ai_persona + end + end + + private + + def find_ai_persona + @ai_persona = AiPersona.find(params[:id]) + end + + def ai_persona_params + params.require(:ai_persona).permit( + :name, + :description, + :enabled, + :system_prompt, + :enabled, + :priority, + allowed_group_ids: [], + commands: [], + ) + end + end + end +end diff --git a/app/models/ai_persona.rb b/app/models/ai_persona.rb index 5d19fee3..679e9599 100644 --- a/app/models/ai_persona.rb +++ b/app/models/ai_persona.rb @@ -4,6 +4,13 @@ class AiPersona < ActiveRecord::Base # places a hard limit, so per site we cache a maximum of 500 classes MAX_PERSONAS_PER_SITE = 500 + validates :name, presence: true, uniqueness: true, length: { maximum: 100 } + validates :description, presence: true, length: { maximum: 2000 } + validates :system_prompt, presence: true, length: { maximum: 10_000_000 } + validate :system_persona_unchangeable, on: :update, if: :system + + before_destroy :ensure_not_system + class MultisiteHash def initialize(id) @hash = Hash.new { |h, k| h[k] = {} } @@ -38,61 +45,15 @@ class AiPersona < ActiveRecord::Base @persona_cache ||= MultisiteHash.new("persona_cache") end + scope :ordered, -> { order("priority DESC, lower(name) ASC") } + def self.all_personas persona_cache[:value] ||= AiPersona - .order(:name) + .ordered .where(enabled: true) .all .limit(MAX_PERSONAS_PER_SITE) - .map do |ai_persona| - name = ai_persona.name - description = ai_persona.description - ai_persona_id = ai_persona.id - allowed_group_ids = ai_persona.allowed_group_ids - commands = - ai_persona.commands.filter_map do |inner_name| - begin - ("DiscourseAi::AiBot::Commands::#{inner_name}").constantize - rescue StandardError - nil - end - end - - Class.new(DiscourseAi::AiBot::Personas::Persona) do - define_singleton_method :name do - name - end - - define_singleton_method :description do - description - end - - define_singleton_method :allowed_group_ids do - allowed_group_ids - end - - define_singleton_method :to_s do - "#" - end - - define_singleton_method :inspect do - "#" - end - - define_method :initialize do |*args, **kwargs| - @ai_persona = AiPersona.find_by(id: ai_persona_id) - super(*args, **kwargs) - end - - define_method :commands do - commands - end - - define_method :system_prompt do - @ai_persona&.system_prompt || "You are a helpful bot." - end - end - end + .map(&:class_instance) end after_commit :bump_cache @@ -100,6 +61,99 @@ class AiPersona < ActiveRecord::Base def bump_cache self.class.persona_cache.flush! end + + def class_instance + allowed_group_ids = self.allowed_group_ids + id = self.id + system = self.system + + persona_class = DiscourseAi::AiBot::Personas.system_personas_by_id[self.id] + if persona_class + persona_class.define_singleton_method :allowed_group_ids do + allowed_group_ids + end + + persona_class.define_singleton_method :id do + id + end + + persona_class.define_singleton_method :system do + system + end + + return persona_class + end + + name = self.name + description = self.description + ai_persona_id = self.id + commands = + self.commands.filter_map do |inner_name| + begin + ("DiscourseAi::AiBot::Commands::#{inner_name}").constantize + rescue StandardError + nil + end + end + + Class.new(DiscourseAi::AiBot::Personas::Persona) do + define_singleton_method :id do + id + end + + define_singleton_method :name do + name + end + + define_singleton_method :description do + description + end + + define_singleton_method :system do + system + end + + define_singleton_method :allowed_group_ids do + allowed_group_ids + end + + define_singleton_method :to_s do + "#" + end + + define_singleton_method :inspect do + "#" + end + + define_method :initialize do |*args, **kwargs| + @ai_persona = AiPersona.find_by(id: ai_persona_id) + super(*args, **kwargs) + end + + define_method :commands do + commands + end + + define_method :system_prompt do + @ai_persona&.system_prompt || "You are a helpful bot." + end + end + end + + private + + def system_persona_unchangeable + if system_prompt_changed? || commands_changed? || name_changed? || description_changed? + errors.add(:base, I18n.t("discourse_ai.ai_bot.personas.cannot_edit_system_persona")) + end + end + + def ensure_not_system + if system + errors.add(:base, I18n.t("discourse_ai.ai_bot.personas.cannot_delete_system_persona")) + throw :abort + end + end end # == Schema Information @@ -110,10 +164,14 @@ end # name :string(100) not null # description :string(2000) not null # commands :string default([]), not null, is an Array -# system_prompt :string not null +# system_prompt :string(10000000) not null # allowed_group_ids :integer default([]), not null, is an Array +# created_by_id :integer +# enabled :boolean default(TRUE), not null # created_at :datetime not null # updated_at :datetime not null +# system :boolean default(FALSE), not null +# priority :integer default(0), not null # # Indexes # diff --git a/app/serializers/localized_ai_persona_serializer.rb b/app/serializers/localized_ai_persona_serializer.rb new file mode 100644 index 00000000..1ecd3239 --- /dev/null +++ b/app/serializers/localized_ai_persona_serializer.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class LocalizedAiPersonaSerializer < ApplicationSerializer + root "ai_persona" + + attributes :id, + :name, + :description, + :enabled, + :system, + :priority, + :commands, + :system_prompt, + :allowed_group_ids + + def name + object.class_instance.name + end + + def description + object.class_instance.description + end +end diff --git a/assets/javascripts/discourse/admin-discourse-ai-personas-route-map.js b/assets/javascripts/discourse/admin-discourse-ai-personas-route-map.js new file mode 100644 index 00000000..91f67648 --- /dev/null +++ b/assets/javascripts/discourse/admin-discourse-ai-personas-route-map.js @@ -0,0 +1,14 @@ +export default { + resource: "admin.adminPlugins", + + path: "/plugins", + + map() { + this.route("discourse-ai", { path: "discourse-ai" }, function () { + this.route("ai-personas", { path: "ai_personas" }, function () { + this.route("new", { path: "/new" }); + this.route("show", { path: "/:id" }); + }); + }); + }, +}; diff --git a/assets/javascripts/discourse/admin/adapters/ai-persona.js b/assets/javascripts/discourse/admin/adapters/ai-persona.js new file mode 100644 index 00000000..309eb273 --- /dev/null +++ b/assets/javascripts/discourse/admin/adapters/ai-persona.js @@ -0,0 +1,17 @@ +import RestAdapter from "discourse/adapters/rest"; + +export default class Adapter extends RestAdapter { + jsonMode = true; + + basePath() { + return "/admin/plugins/discourse-ai/"; + } + + pathFor() { + return super.pathFor(...arguments) + ".json"; + } + + apiNameFor() { + return "ai-persona"; + } +} diff --git a/assets/javascripts/discourse/admin/models/ai-persona.js b/assets/javascripts/discourse/admin/models/ai-persona.js new file mode 100644 index 00000000..08fc8d28 --- /dev/null +++ b/assets/javascripts/discourse/admin/models/ai-persona.js @@ -0,0 +1,28 @@ +import RestModel from "discourse/models/rest"; + +const ATTRIBUTES = [ + "name", + "description", + "commands", + "system_prompt", + "allowed_group_ids", + "enabled", + "system", + "priority", +]; + +export default class AiPersona extends RestModel { + updateProperties() { + let attrs = this.getProperties(ATTRIBUTES); + attrs.id = this.id; + return attrs; + } + + createProperties() { + return this.getProperties(ATTRIBUTES); + } + + workingCopy() { + return AiPersona.create(this.createProperties()); + } +} diff --git a/assets/javascripts/discourse/components/ai-command-selector.gjs b/assets/javascripts/discourse/components/ai-command-selector.gjs new file mode 100644 index 00000000..5dea2fad --- /dev/null +++ b/assets/javascripts/discourse/components/ai-command-selector.gjs @@ -0,0 +1,18 @@ +import { computed, observer } from "@ember/object"; +import MultiSelectComponent from "select-kit/components/multi-select"; + +export default MultiSelectComponent.extend({ + _modelDisabledChanged: observer("attrs.disabled", function () { + this.selectKit.options.set("disabled", this.get("attrs.disabled.value")); + }), + + content: computed(function () { + return this.attrs.commands.value; + }), + + value: "", + + selectKitOptions: { + filterable: true, + }, +}); diff --git a/assets/javascripts/discourse/components/ai-persona-editor.gjs b/assets/javascripts/discourse/components/ai-persona-editor.gjs new file mode 100644 index 00000000..05960f28 --- /dev/null +++ b/assets/javascripts/discourse/components/ai-persona-editor.gjs @@ -0,0 +1,224 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { Input } from "@ember/component"; +import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import didInsert from "@ember/render-modifiers/modifiers/did-insert"; +import didUpdate from "@ember/render-modifiers/modifiers/did-update"; +import { later } from "@ember/runloop"; +import { inject as service } from "@ember/service"; +import DButton from "discourse/components/d-button"; +import Textarea from "discourse/components/d-textarea"; +import DToggleSwitch from "discourse/components/d-toggle-switch"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import Group from "discourse/models/group"; +import i18n from "discourse-common/helpers/i18n"; +import I18n from "discourse-i18n"; +import GroupChooser from "select-kit/components/group-chooser"; +import DTooltip from "float-kit/components/d-tooltip"; +import AiCommandSelector from "./ai-command-selector"; + +export default class PersonaEditor extends Component { + @service router; + @service store; + @service dialog; + @service toasts; + + @tracked allGroups = []; + @tracked isSaving = false; + @tracked editingModel = null; + @tracked showDelete = false; + + @action + updateModel() { + this.editingModel = this.args.model.workingCopy(); + this.showDelete = !this.args.model.isNew && !this.args.model.system; + } + + @action + async updateAllGroups() { + this.allGroups = await Group.findAll(); + } + + @action + async save() { + const isNew = this.args.model.isNew; + this.isSaving = true; + + const backupModel = this.args.model.workingCopy(); + + this.args.model.setProperties(this.editingModel); + try { + await this.args.model.save(); + this.#sortPersonas(); + if (isNew) { + this.args.personas.addObject(this.args.model); + this.router.transitionTo( + "adminPlugins.discourse-ai.ai-personas.show", + this.args.model + ); + } else { + this.toasts.success({ + data: { message: I18n.t("discourse_ai.ai_persona.saved") }, + duration: 2000, + }); + } + } catch (e) { + this.args.model.setProperties(backupModel); + popupAjaxError(e); + } finally { + later(() => { + this.isSaving = false; + }, 1000); + } + } + + @action + delete() { + return this.dialog.confirm({ + message: I18n.t("discourse_ai.ai_persona.confirm_delete"), + didConfirm: () => { + return this.args.model.destroyRecord().then(() => { + this.args.personas.removeObject(this.args.model); + this.router.transitionTo( + "adminPlugins.discourse-ai.ai-personas.index" + ); + }); + }, + }); + } + + @action + updateAllowedGroups(ids) { + this.editingModel.set("allowed_group_ids", ids); + } + + @action + async toggleEnabled() { + this.args.model.set("enabled", !this.args.model.enabled); + if (!this.args.model.isNew) { + try { + await this.args.model.update({ enabled: this.args.model.enabled }); + } catch (e) { + popupAjaxError(e); + } + } + } + + @action + async togglePriority() { + this.args.model.set("priority", !this.args.model.priority); + if (!this.args.model.isNew) { + try { + await this.args.model.update({ priority: this.args.model.priority }); + + this.#sortPersonas(); + } catch (e) { + popupAjaxError(e); + } + } + } + + #sortPersonas() { + const sorted = this.args.personas.toArray().sort((a, b) => { + if (a.priority && !b.priority) { + return -1; + } else if (!a.priority && b.priority) { + return 1; + } else { + return a.name.localeCompare(b.name); + } + }); + this.args.personas.clear(); + this.args.personas.setObjects(sorted); + } + +