From 2a8be6e2d72c2e05a3f0acb56f2153775241a6cd Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Fri, 21 Mar 2025 14:46:33 -0300 Subject: [PATCH] REFACTOR: Migrate Personas' form to FormKit (#1178) * REFACTOR: Migrate Personas' form to FormKit We re-arranged fields into sections so we can better differentiate which options are specific to the AI bot. * few form-kit improvements https://github.com/discourse/discourse/pull/31934 --------- Co-authored-by: Joffrey JAFFEUX --- .../discourse/admin/models/ai-persona.js | 120 +- .../ai-forced-tool-strategy-selector.gjs | 29 - .../discourse/components/ai-llm-selector.gjs | 17 + .../discourse/components/ai-llm-selector.js | 27 - .../components/ai-persona-editor.gjs | 1018 +++++++++-------- .../ai-persona-tool-option-editor.gjs | 91 -- .../components/ai-persona-tool-options.gjs | 131 ++- .../discourse/components/ai-tool-selector.gjs | 13 + .../discourse/components/ai-tool-selector.js | 18 - .../discourse/components/rag-options-fk.gjs | 81 ++ .../discourse/components/rag-uploader.gjs | 6 +- .../modules/ai-bot/common/ai-persona.scss | 32 +- config/locales/client.en.yml | 6 + spec/system/admin_ai_persona_spec.rb | 24 +- spec/system/ai_bot/tool_spec.rb | 2 +- .../unit/models/ai-persona-test.js | 103 +- 16 files changed, 788 insertions(+), 930 deletions(-) delete mode 100644 assets/javascripts/discourse/components/ai-forced-tool-strategy-selector.gjs create mode 100644 assets/javascripts/discourse/components/ai-llm-selector.gjs delete mode 100644 assets/javascripts/discourse/components/ai-llm-selector.js delete mode 100644 assets/javascripts/discourse/components/ai-persona-tool-option-editor.gjs create mode 100644 assets/javascripts/discourse/components/ai-tool-selector.gjs delete mode 100644 assets/javascripts/discourse/components/ai-tool-selector.js create mode 100644 assets/javascripts/discourse/components/rag-options-fk.gjs diff --git a/assets/javascripts/discourse/admin/models/ai-persona.js b/assets/javascripts/discourse/admin/models/ai-persona.js index 18c97a7d..98ef84ec 100644 --- a/assets/javascripts/discourse/admin/models/ai-persona.js +++ b/assets/javascripts/discourse/admin/models/ai-persona.js @@ -1,4 +1,3 @@ -import { tracked } from "@glimmer/tracking"; import { ajax } from "discourse/lib/ajax"; import RestModel from "discourse/models/rest"; @@ -63,40 +62,7 @@ const SYSTEM_ATTRIBUTES = [ "allow_chat_direct_messages", ]; -class ToolOption { - @tracked value = null; -} - export default class AiPersona extends RestModel { - // this code is here to convert the wire schema to easier to work with object - // on the wire we pass in/out tools as an Array. - // [[ToolName, {option1: value, option2: value}, force], ToolName2, ToolName3] - // So we rework this into a "tools" property and nested toolOptions - init(properties) { - this.forcedTools = []; - if (properties.tools) { - properties.tools = properties.tools.map((tool) => { - if (typeof tool === "string") { - return tool; - } else { - let [toolId, options, force] = tool; - for (let optionId in options) { - if (!options.hasOwnProperty(optionId)) { - continue; - } - this.getToolOption(toolId, optionId).value = options[optionId]; - } - if (force) { - this.forcedTools.push(toolId); - } - return toolId; - } - }); - } - super.init(properties); - this.tools = properties.tools; - } - async createUser() { const result = await ajax( `/admin/plugins/discourse-ai/ai-personas/${this.id}/create-user.json`, @@ -109,63 +75,79 @@ export default class AiPersona extends RestModel { return this.user; } - getToolOption(toolId, optionId) { - this.toolOptions ||= {}; - this.toolOptions[toolId] ||= {}; - return (this.toolOptions[toolId][optionId] ||= new ToolOption()); + flattenedToolStructure(data) { + return data.tools.map((tName) => { + return [tName, data.toolOptions[tName], data.forcedTools.includes(tName)]; + }); } - populateToolOptions(attrs) { - if (!attrs.tools) { - return; - } - let toolsWithOptions = []; - attrs.tools.forEach((toolId) => { - if (typeof toolId !== "string") { - toolId = toolId[0]; - } + // this code is here to convert the wire schema to easier to work with object + // on the wire we pass in/out tools as an Array. + // [[ToolName, {option1: value, option2: value}, force], ToolName2, ToolName3] + // We split it into tools, options and a list of forced ones. + populateTools(attrs) { + const forcedTools = []; + const toolOptions = {}; - let force = this.forcedTools.includes(toolId); - if (this.toolOptions && this.toolOptions[toolId]) { - let options = this.toolOptions[toolId]; - let optionsWithValues = {}; - for (let optionId in options) { + const flatTools = attrs.tools?.map((tool) => { + if (typeof tool === "string") { + return tool; + } else { + let [toolId, options, force] = tool; + const mappedOptions = {}; + + for (const optionId in options) { if (!options.hasOwnProperty(optionId)) { continue; } - let option = options[optionId]; - optionsWithValues[optionId] = option.value; + + mappedOptions[optionId] = options[optionId]; } - toolsWithOptions.push([toolId, optionsWithValues, force]); - } else { - toolsWithOptions.push([toolId, {}, force]); + + if (Object.keys(mappedOptions).length > 0) { + toolOptions[toolId] = mappedOptions; + } + + if (force) { + forcedTools.push(toolId); + } + + return toolId; } }); - attrs.tools = toolsWithOptions; + + attrs.tools = flatTools; + attrs.forcedTools = forcedTools; + attrs.toolOptions = toolOptions; } updateProperties() { - let attrs = this.system + const attrs = this.system ? this.getProperties(SYSTEM_ATTRIBUTES) : this.getProperties(CREATE_ATTRIBUTES); attrs.id = this.id; - this.populateToolOptions(attrs); + return attrs; } createProperties() { - let attrs = this.getProperties(CREATE_ATTRIBUTES); - this.populateToolOptions(attrs); - return attrs; + return this.getProperties(CREATE_ATTRIBUTES); } - workingCopy() { - let attrs = this.getProperties(CREATE_ATTRIBUTES); - this.populateToolOptions(attrs); + fromPOJO(data) { + const dataClone = JSON.parse(JSON.stringify(data)); + + const persona = AiPersona.create(dataClone); + persona.tools = this.flattenedToolStructure(dataClone); - const persona = AiPersona.create(attrs); - persona.forcedTools = (this.forcedTools || []).slice(); - persona.forced_tool_count = this.forced_tool_count || -1; return persona; } + + toPOJO() { + const attrs = this.getProperties(CREATE_ATTRIBUTES); + this.populateTools(attrs); + attrs.forced_tool_count = this.forced_tool_count || -1; + + return attrs; + } } diff --git a/assets/javascripts/discourse/components/ai-forced-tool-strategy-selector.gjs b/assets/javascripts/discourse/components/ai-forced-tool-strategy-selector.gjs deleted file mode 100644 index 3c60c76f..00000000 --- a/assets/javascripts/discourse/components/ai-forced-tool-strategy-selector.gjs +++ /dev/null @@ -1,29 +0,0 @@ -import { computed } from "@ember/object"; -import { i18n } from "discourse-i18n"; -import ComboBox from "select-kit/components/combo-box"; - -export default ComboBox.extend({ - content: computed(function () { - const content = [ - { - id: -1, - name: i18n("discourse_ai.ai_persona.tool_strategies.all"), - }, - ]; - - [1, 2, 5].forEach((i) => { - content.push({ - id: i, - name: i18n("discourse_ai.ai_persona.tool_strategies.replies", { - count: i, - }), - }); - }); - - return content; - }), - - selectKitOptions: { - filterable: false, - }, -}); diff --git a/assets/javascripts/discourse/components/ai-llm-selector.gjs b/assets/javascripts/discourse/components/ai-llm-selector.gjs new file mode 100644 index 00000000..f5d9d95e --- /dev/null +++ b/assets/javascripts/discourse/components/ai-llm-selector.gjs @@ -0,0 +1,17 @@ +import { hash } from "@ember/helper"; +import ComboBox from "select-kit/components/combo-box"; + +const AiLlmSelector = ; + +export default AiLlmSelector; diff --git a/assets/javascripts/discourse/components/ai-llm-selector.js b/assets/javascripts/discourse/components/ai-llm-selector.js deleted file mode 100644 index bfd6f9c0..00000000 --- a/assets/javascripts/discourse/components/ai-llm-selector.js +++ /dev/null @@ -1,27 +0,0 @@ -import { computed } from "@ember/object"; -import { observes } from "@ember-decorators/object"; -import { i18n } from "discourse-i18n"; -import ComboBox from "select-kit/components/combo-box"; -import { selectKitOptions } from "select-kit/components/select-kit"; - -@selectKitOptions({ - filterable: true, -}) -export default class AiLlmSelector extends ComboBox { - @observes("attrs.disabled") - _modelDisabledChanged() { - this.selectKit.options.set("disabled", this.get("attrs.disabled.value")); - } - - @computed - get content() { - const blankName = - this.attrs.blankName || i18n("discourse_ai.ai_persona.no_llm_selected"); - return [ - { - id: "blank", - name: blankName, - }, - ].concat(this.llms); - } -} diff --git a/assets/javascripts/discourse/components/ai-persona-editor.gjs b/assets/javascripts/discourse/components/ai-persona-editor.gjs index eb69f77d..f8c303d6 100644 --- a/assets/javascripts/discourse/components/ai-persona-editor.gjs +++ b/assets/javascripts/discourse/components/ai-persona-editor.gjs @@ -1,30 +1,24 @@ import Component from "@glimmer/component"; import { cached, tracked } from "@glimmer/tracking"; -import { Input } from "@ember/component"; -import { on } from "@ember/modifier"; +import { fn } from "@ember/helper"; import { action } from "@ember/object"; import didInsert from "@ember/render-modifiers/modifiers/did-insert"; -import didUpdate from "@ember/render-modifiers/modifiers/did-update"; import { LinkTo } from "@ember/routing"; import { later } from "@ember/runloop"; import { service } from "@ember/service"; +import { gt, or } from "truth-helpers"; import BackButton from "discourse/components/back-button"; -import DButton from "discourse/components/d-button"; -import Textarea from "discourse/components/d-textarea"; -import DToggleSwitch from "discourse/components/d-toggle-switch"; +import Form from "discourse/components/form"; import Avatar from "discourse/helpers/bound-avatar-template"; import { popupAjaxError } from "discourse/lib/ajax-error"; import Group from "discourse/models/group"; import { i18n } from "discourse-i18n"; import AdminUser from "admin/models/admin-user"; -import ComboBox from "select-kit/components/combo-box"; import GroupChooser from "select-kit/components/group-chooser"; -import DTooltip from "float-kit/components/d-tooltip"; -import AiForcedToolStrategySelector from "./ai-forced-tool-strategy-selector"; import AiLlmSelector from "./ai-llm-selector"; import AiPersonaToolOptions from "./ai-persona-tool-options"; import AiToolSelector from "./ai-tool-selector"; -import RagOptions from "./rag-options"; +import RagOptionsFk from "./rag-options-fk"; import RagUploader from "./rag-uploader"; export default class PersonaEditor extends Component { @@ -36,107 +30,81 @@ export default class PersonaEditor extends Component { @tracked allGroups = []; @tracked isSaving = false; - @tracked editingModel = null; - @tracked showDelete = false; - @tracked maxPixelsValue = null; - @tracked ragIndexingStatuses = null; + @tracked availableForcedTools = []; - @tracked selectedTools = []; - @tracked selectedToolNames = []; - @tracked forcedToolNames = []; - @tracked hasDefaultLlm = false; + @cached + get formData() { + const data = this.args.model.toPOJO(); + + if (data.tools) { + data.toolOptions = this.mapToolOptions(data.toolOptions, data.tools); + } + + return data; + } get chatPluginEnabled() { return this.siteSettings.chat_enabled; } - get allowForceTools() { - return !this.editingModel?.system && this.selectedToolNames.length > 0; + get allTools() { + return this.args.personas.resultSetMeta.tools; } - get hasForcedTools() { - return this.forcedToolNames.length > 0; - } - - @action - forcedToolsChanged(tools) { - this.forcedToolNames = tools; - this.editingModel.forcedTools = this.forcedToolNames; - } - - @action - toolsChanged(tools) { - this.selectedTools = this.args.personas.resultSetMeta.tools.filter((tool) => - tools.includes(tool.id) - ); - this.selectedToolNames = tools.slice(); - - this.forcedToolNames = this.forcedToolNames.filter( - (tool) => this.editingModel.tools.indexOf(tool) !== -1 - ); - - this.editingModel.tools = this.selectedToolNames; - this.editingModel.forcedTools = this.forcedToolNames; - } - - @action - updateModel() { - this.editingModel = this.args.model.workingCopy(); - this.hasDefaultLlm = !!this.editingModel.default_llm; - this.showDelete = !this.args.model.isNew && !this.args.model.system; - this.maxPixelsValue = this.findClosestPixelValue( - this.editingModel.vision_max_pixels - ); - - this.selectedToolNames = this.editingModel.tools || []; - this.selectedTools = this.args.personas.resultSetMeta.tools.filter((tool) => - this.selectedToolNames.includes(tool.id) - ); - this.forcedToolNames = this.editingModel.forcedTools || []; - } - - findClosestPixelValue(pixels) { - let value = "high"; - this.maxPixelValues.forEach((info) => { - if (pixels === info.pixels) { - value = info.id; - } - }); - return value; - } - - @cached get maxPixelValues() { const l = (key) => i18n(`discourse_ai.ai_persona.vision_max_pixel_sizes.${key}`); return [ - { id: "low", name: l("low"), pixels: 65536 }, - { id: "medium", name: l("medium"), pixels: 262144 }, - { id: "high", name: l("high"), pixels: 1048576 }, + { name: l("low"), id: 65536 }, + { name: l("medium"), id: 262144 }, + { name: l("high"), id: 1048576 }, ]; } + get forcedToolStrategies() { + const content = [ + { + id: -1, + name: i18n("discourse_ai.ai_persona.tool_strategies.all"), + }, + ]; + + [1, 2, 5].forEach((i) => { + content.push({ + id: i, + name: i18n("discourse_ai.ai_persona.tool_strategies.replies", { + count: i, + }), + }); + }); + + return content; + } + @action async updateAllGroups() { this.allGroups = await Group.findAll(); } @action - async save() { + async save(data) { 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(); + const personaToSave = Object.assign( + this.args.model, + this.args.model.fromPOJO(data) + ); + + await personaToSave.save(); this.#sortPersonas(); + if (isNew && this.args.model.rag_uploads.length === 0) { - this.args.personas.addObject(this.args.model); + this.args.personas.addObject(personaToSave); this.router.transitionTo( "adminPlugins.show.discourse-ai-personas.edit", - this.args.model + personaToSave ); } else { this.toasts.success({ @@ -145,7 +113,6 @@ export default class PersonaEditor extends Component { }); } } catch (e) { - this.args.model.setProperties(backupModel); popupAjaxError(e); } finally { later(() => { @@ -154,52 +121,11 @@ export default class PersonaEditor extends Component { } } - get showTemperature() { - return this.editingModel?.temperature || !this.editingModel?.system; - } - - get showTopP() { - return this.editingModel?.top_p || !this.editingModel?.system; - } - get adminUser() { - return AdminUser.create(this.editingModel?.user); - } + // Work around user not being extensible. + const userClone = Object.assign({}, this.args.model?.user); - get mappedQuestionConsolidatorLlm() { - return this.editingModel?.question_consolidator_llm_id ?? "blank"; - } - - set mappedQuestionConsolidatorLlm(value) { - if (value === "blank") { - this.editingModel.question_consolidator_llm_id = null; - } else { - this.editingModel.question_consolidator_llm_id = value; - } - } - - get mappedDefaultLlm() { - return this.editingModel?.default_llm_id ?? "blank"; - } - - set mappedDefaultLlm(value) { - if (value === "blank") { - this.editingModel.default_llm_id = null; - this.hasDefaultLlm = false; - } else { - this.editingModel.default_llm_id = value; - this.hasDefaultLlm = true; - } - } - - @action - onChangeMaxPixels(value) { - const entry = this.maxPixelValues.findBy("id", value); - if (!entry) { - return; - } - this.maxPixelsValue = value; - this.editingModel.vision_max_pixels = entry.pixels; + return AdminUser.create(userClone); } @action @@ -218,51 +144,101 @@ export default class PersonaEditor extends Component { } @action - updateAllowedGroups(ids) { - this.editingModel.set("allowed_group_ids", ids); + async toggleEnabled(value, { set }) { + set("enabled", value); + await this.persistField("enabled", value); } @action - async toggleEnabled() { - await this.toggleField("enabled"); + async togglePriority(value, { set }) { + set("priority", value); + await this.persistField("priority", value, true); } @action - async togglePriority() { - await this.toggleField("priority", true); - } - - @action - async createUser() { + async createUser(form) { try { let user = await this.args.model.createUser(); - this.editingModel.set("user", user); - this.editingModel.set("user_id", user.id); + form.set("user", user); + form.set("user_id", user.id); } catch (e) { popupAjaxError(e); } } @action - updateUploads(uploads) { - this.editingModel.rag_uploads = uploads; + updateUploads(form, newUploads) { + form.set("rag_uploads", newUploads); } @action - removeUpload(upload) { - this.editingModel.rag_uploads.removeObject(upload); + async removeUpload(form, currentUploads, upload) { + const updatedUploads = currentUploads.filter( + (file) => file.id !== upload.id + ); + + form.set("rag_uploads", updatedUploads); + if (!this.args.model.isNew) { - this.save(); + await this.persistField("rag_uploads", updatedUploads); } } - async toggleField(field, sortPersonas) { - this.args.model.set(field, !this.args.model[field]); - this.editingModel.set(field, this.args.model[field]); + @action + updateToolNames(form, currentData, updatedTools) { + const removedTools = + currentData?.tools?.filter((ct) => !updatedTools.includes(ct)) || []; + const updatedOptions = this.mapToolOptions( + currentData.toolOptions, + updatedTools + ); + + form.setProperties({ + tools: updatedTools, + toolOptions: updatedOptions, + }); + + this.availableForcedTools = this.allTools.filter((tool) => + updatedTools.includes(tool.id) + ); + + if (currentData.forcedTools?.length > 0) { + const updatedForcedTools = currentData.forcedTools.filter( + (fct) => !removedTools.includes(fct) + ); + form.set("forcedTools", updatedForcedTools); + } + } + + mapToolOptions(currentOptions, toolNames) { + const updatedOptions = Object.assign({}, currentOptions); + + toolNames.forEach((toolId) => { + const tool = this.allTools.findBy("id", toolId); + const toolOptions = tool?.options; + + if (!toolOptions || updatedOptions[toolId]) { + return; + } + + const mappedOptions = {}; + Object.keys(toolOptions).forEach((key) => { + mappedOptions[key] = null; + }); + + updatedOptions[toolId] = mappedOptions; + }); + + return updatedOptions; + } + + async persistField(field, newValue, sortPersonas) { + this.args.model.set(field, newValue); + if (!this.args.model.isNew) { try { const args = {}; - args[field] = this.args.model[field]; + args[field] = newValue; await this.args.model.update(args); if (sortPersonas) { @@ -293,369 +269,401 @@ export default class PersonaEditor extends Component { @route="adminPlugins.show.discourse-ai-personas" @label="discourse_ai.ai_persona.back" /> -
-
- -
-
- - -
-
- - -
-
- -