From a7d032fa28ee4c2e19b2a7b36769e4ccdc656ec3 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 4 Feb 2025 16:27:27 +1100 Subject: [PATCH] DEV: artifact system update (#1096) ### Why This pull request fundamentally restructures how AI bots create and update web artifacts to address critical limitations in the previous approach: 1. **Improved Artifact Context for LLMs**: Previously, artifact creation and update tools included the *entire* artifact source code directly in the tool arguments. This overloaded the Language Model (LLM) with raw code, making it difficult for the LLM to maintain a clear understanding of the artifact's current state when applying changes. The LLM would struggle to differentiate between the base artifact and the requested modifications, leading to confusion and less effective updates. 2. **Reduced Token Usage and History Bloat**: Including the full artifact source code in every tool interaction was extremely token-inefficient. As conversations progressed, this redundant code in the history consumed a significant number of tokens unnecessarily. This not only increased costs but also diluted the context for the LLM with less relevant historical information. 3. **Enabling Updates for Large Artifacts**: The lack of a practical diff or targeted update mechanism made it nearly impossible to efficiently update larger web artifacts. Sending the entire source code for every minor change was both computationally expensive and prone to errors, effectively blocking the use of AI bots for meaningful modifications of complex artifacts. **This pull request addresses these core issues by**: * Introducing methods for the AI bot to explicitly *read* and understand the current state of an artifact. * Implementing efficient update strategies that send *targeted* changes rather than the entire artifact source code. * Providing options to control the level of artifact context included in LLM prompts, optimizing token usage. ### What The main changes implemented in this PR to resolve the above issues are: 1. **`Read Artifact` Tool for Contextual Awareness**: - A new `read_artifact` tool is introduced, enabling AI bots to fetch and process the current content of a web artifact from a given URL (local or external). - This provides the LLM with a clear and up-to-date representation of the artifact's HTML, CSS, and JavaScript, improving its understanding of the base to be modified. - By cloning local artifacts, it allows the bot to work with a fresh copy, further enhancing context and control. 2. **Refactored `Update Artifact` Tool with Efficient Strategies**: - The `update_artifact` tool is redesigned to employ more efficient update strategies, minimizing token usage and improving update precision: - **`diff` strategy**: Utilizes a search-and-replace diff algorithm to apply only the necessary, targeted changes to the artifact's code. This significantly reduces the amount of code sent to the LLM and focuses its attention on the specific modifications. - **`full` strategy**: Provides the option to replace the entire content sections (HTML, CSS, JavaScript) when a complete rewrite is required. - Tool options enhance the control over the update process: - `editor_llm`: Allows selection of a specific LLM for artifact updates, potentially optimizing for code editing tasks. - `update_algorithm`: Enables choosing between `diff` and `full` update strategies based on the nature of the required changes. - `do_not_echo_artifact`: Defaults to true, and by *not* echoing the artifact in prompts, it further reduces token consumption in scenarios where the LLM might not need the full artifact context for every update step (though effectiveness might be slightly reduced in certain update scenarios). 3. **System and General Persona Tool Option Visibility and Customization**: - Tool options, including those for system personas, are made visible and editable in the admin UI. This allows administrators to fine-tune the behavior of all personas and their tools, including setting specific LLMs or update algorithms. This was previously limited or hidden for system personas. 4. **Centralized and Improved Content Security Policy (CSP) Management**: - The CSP for AI artifacts is consolidated and made more maintainable through the `ALLOWED_CDN_SOURCES` constant. This improves code organization and future updates to the allowed CDN list, while maintaining the existing security posture. 5. **Codebase Improvements**: - Refactoring of diff utilities, introduction of strategy classes, enhanced error handling, new locales, and comprehensive testing all contribute to a more robust, efficient, and maintainable artifact management system. By addressing the issues of LLM context confusion, token inefficiency, and the limitations of updating large artifacts, this pull request significantly improves the practicality and effectiveness of AI bots in managing web artifacts within Discourse. --- .../ai_bot/artifacts_controller.rb | 4 +- app/models/ai_artifact.rb | 13 + app/models/ai_persona.rb | 34 +- app/serializers/ai_tool_serializer.rb | 5 +- .../discourse/admin/models/ai-persona.js | 1 + .../discourse/components/ai-llm-selector.js | 4 +- .../components/ai-persona-editor.gjs | 13 +- .../ai-persona-tool-option-editor.gjs | 49 ++- .../components/ai-persona-tool-options.gjs | 5 +- config/locales/client.en.yml | 1 + config/locales/server.en.yml | 20 +- db/fixtures/ai_bot/603_bot_ai_personas.rb | 14 +- lib/ai_bot/artifact_update_strategies/base.rb | 53 +++ lib/ai_bot/artifact_update_strategies/diff.rb | 183 ++++++++ lib/ai_bot/artifact_update_strategies/full.rb | 148 +++++++ lib/ai_bot/bot.rb | 7 +- lib/ai_bot/personas/persona.rb | 5 +- lib/ai_bot/personas/web_artifact_creator.rb | 4 +- lib/ai_bot/playground.rb | 4 +- lib/ai_bot/tools/create_artifact.rb | 315 ++++++++++---- lib/ai_bot/tools/option.rb | 6 +- lib/ai_bot/tools/read_artifact.rb | 211 ++++++++++ lib/ai_bot/tools/tool.rb | 12 +- lib/ai_bot/tools/update_artifact.rb | 289 ++++++++----- lib/completions/endpoints/base.rb | 5 +- lib/completions/endpoints/open_ai.rb | 2 +- lib/utils/diff_utils.rb | 184 -------- lib/utils/diff_utils/hunk_diff.rb | 228 ++++++++++ lib/utils/diff_utils/simple_diff.rb | 178 ++++++++ .../ai_bot/tools/create_artifact_spec.rb | 88 ++-- .../ai_bot/tools/read_artifact_spec.rb | 143 +++++++ .../ai_bot/tools/update_artifact_spec.rb | 393 ++++++++++++++---- .../hunk_diff_spec.rb} | 10 +- spec/lib/utils/diff_utils/simple_diff_spec.rb | 168 ++++++++ spec/models/ai_persona_spec.rb | 28 ++ 35 files changed, 2272 insertions(+), 555 deletions(-) create mode 100644 lib/ai_bot/artifact_update_strategies/base.rb create mode 100644 lib/ai_bot/artifact_update_strategies/diff.rb create mode 100644 lib/ai_bot/artifact_update_strategies/full.rb create mode 100644 lib/ai_bot/tools/read_artifact.rb delete mode 100644 lib/utils/diff_utils.rb create mode 100644 lib/utils/diff_utils/hunk_diff.rb create mode 100644 lib/utils/diff_utils/simple_diff.rb create mode 100644 spec/lib/modules/ai_bot/tools/read_artifact_spec.rb rename spec/lib/utils/{diff_utils_spec.rb => diff_utils/hunk_diff_spec.rb} (94%) create mode 100644 spec/lib/utils/diff_utils/simple_diff_spec.rb diff --git a/app/controllers/discourse_ai/ai_bot/artifacts_controller.rb b/app/controllers/discourse_ai/ai_bot/artifacts_controller.rb index aaec3b26..4e489659 100644 --- a/app/controllers/discourse_ai/ai_bot/artifacts_controller.rb +++ b/app/controllers/discourse_ai/ai_bot/artifacts_controller.rb @@ -12,7 +12,7 @@ module DiscourseAi artifact = AiArtifact.find(params[:id]) post = Post.find_by(id: artifact.post_id) - if artifact.metadata&.dig("public") + if artifact.public? # no guardian needed else raise Discourse::NotFound if !post&.topic&.private_message? @@ -81,7 +81,7 @@ module DiscourseAi response.headers.delete("X-Frame-Options") response.headers[ "Content-Security-Policy" - ] = "script-src 'self' 'unsafe-inline' 'wasm-unsafe-eval' https://unpkg.com https://cdnjs.cloudflare.com https://ajax.googleapis.com https://cdn.jsdelivr.net;" + ] = "script-src 'self' 'unsafe-inline' 'wasm-unsafe-eval' #{AiArtifact::ALLOWED_CDN_SOURCES.join(" ")};" response.headers["X-Robots-Tag"] = "noindex" # Render the content diff --git a/app/models/ai_artifact.rb b/app/models/ai_artifact.rb index 355e8384..f3faddb3 100644 --- a/app/models/ai_artifact.rb +++ b/app/models/ai_artifact.rb @@ -8,6 +8,15 @@ class AiArtifact < ActiveRecord::Base validates :css, length: { maximum: 65_535 } validates :js, length: { maximum: 65_535 } + ALLOWED_CDN_SOURCES = %w[ + https://cdn.jsdelivr.net + https://cdnjs.cloudflare.com + https://unpkg.com + https://ajax.googleapis.com + https://d3js.org + https://code.jquery.com + ] + def self.iframe_for(id, version = nil) <<~HTML
@@ -70,6 +79,10 @@ class AiArtifact < ActiveRecord::Base version end + + def public? + !!metadata&.dig("public") + end end # == Schema Information diff --git a/app/models/ai_persona.rb b/app/models/ai_persona.rb index 681ea7a8..cc1df7e0 100644 --- a/app/models/ai_persona.rb +++ b/app/models/ai_persona.rb @@ -130,8 +130,6 @@ class AiPersona < ActiveRecord::Base tool_details ] - persona_class = DiscourseAi::AiBot::Personas::Persona.system_personas_by_id[self.id] - instance_attributes = {} attributes.each do |attr| value = self.read_attribute(attr) @@ -140,14 +138,6 @@ class AiPersona < ActiveRecord::Base instance_attributes[:username] = user&.username_lower - if persona_class - instance_attributes.each do |key, value| - # description/name are localized - persona_class.define_singleton_method(key) { value } if key != :description && key != :name - end - return persona_class - end - options = {} force_tool_use = [] @@ -180,6 +170,16 @@ class AiPersona < ActiveRecord::Base klass end + persona_class = DiscourseAi::AiBot::Personas::Persona.system_personas_by_id[self.id] + if persona_class + instance_attributes.each do |key, value| + # description/name are localized + persona_class.define_singleton_method(key) { value } if key != :description && key != :name + end + persona_class.define_method(:options) { options } + return persona_class + end + ai_persona_id = self.id Class.new(DiscourseAi::AiBot::Personas::Persona) do @@ -264,9 +264,19 @@ class AiPersona < ActiveRecord::Base end def system_persona_unchangeable - if top_p_changed? || temperature_changed? || system_prompt_changed? || tools_changed? || - name_changed? || description_changed? + if top_p_changed? || temperature_changed? || system_prompt_changed? || name_changed? || + description_changed? errors.add(:base, I18n.t("discourse_ai.ai_bot.personas.cannot_edit_system_persona")) + elsif tools_changed? + old_tools = tools_change[0] + new_tools = tools_change[1] + + old_tool_names = old_tools.map { |t| t.is_a?(Array) ? t[0] : t }.to_set + new_tool_names = new_tools.map { |t| t.is_a?(Array) ? t[0] : t }.to_set + + if old_tool_names != new_tool_names + errors.add(:base, I18n.t("discourse_ai.ai_bot.personas.cannot_edit_system_persona")) + end end end diff --git a/app/serializers/ai_tool_serializer.rb b/app/serializers/ai_tool_serializer.rb index 8cb7bf72..0d0c48a3 100644 --- a/app/serializers/ai_tool_serializer.rb +++ b/app/serializers/ai_tool_serializer.rb @@ -22,11 +22,14 @@ class AiToolSerializer < ApplicationSerializer def options options = {} object.accepted_options.each do |option| - options[option.name] = { + processed_option = { name: option.localized_name, description: option.localized_description, type: option.type, } + processed_option[:values] = option.values if option.values.present? + processed_option[:default] = option.default if option.default.present? + options[option.name] = processed_option end options end diff --git a/assets/javascripts/discourse/admin/models/ai-persona.js b/assets/javascripts/discourse/admin/models/ai-persona.js index c649efbd..2c9c7383 100644 --- a/assets/javascripts/discourse/admin/models/ai-persona.js +++ b/assets/javascripts/discourse/admin/models/ai-persona.js @@ -41,6 +41,7 @@ const SYSTEM_ATTRIBUTES = [ "enabled", "system", "priority", + "tools", "user_id", "default_llm", "force_default_llm", diff --git a/assets/javascripts/discourse/components/ai-llm-selector.js b/assets/javascripts/discourse/components/ai-llm-selector.js index 4937f3ac..bfd6f9c0 100644 --- a/assets/javascripts/discourse/components/ai-llm-selector.js +++ b/assets/javascripts/discourse/components/ai-llm-selector.js @@ -15,10 +15,12 @@ export default class AiLlmSelector extends ComboBox { @computed get content() { + const blankName = + this.attrs.blankName || i18n("discourse_ai.ai_persona.no_llm_selected"); return [ { id: "blank", - name: i18n("discourse_ai.ai_persona.no_llm_selected"), + 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 4fd6d8d0..121ad852 100644 --- a/assets/javascripts/discourse/components/ai-persona-editor.gjs +++ b/assets/javascripts/discourse/components/ai-persona-editor.gjs @@ -420,13 +420,12 @@ export default class PersonaEditor extends Component {
{{/if}} {{/if}} - {{#unless this.editingModel.system}} - - {{/unless}} +
- {{#if this.isBoolean}} + {{#if this.isEnum}} + + {{else if this.isLlm}} + + {{else if this.isBoolean}}
{{#each toolOption.options as |option|}} - + {{/each}}
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 300d30c1..203ac460 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -213,6 +213,7 @@ en: edit: "Edit" description: "Description" no_llm_selected: "No language model selected" + use_parent_llm: "Use personas language model" max_context_posts: "Max context posts" max_context_posts_help: "The maximum number of posts to use as context for the AI when responding to a user. (empty for default)" vision_enabled: Vision enabled diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e490ef24..a00b56e2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -199,6 +199,7 @@ en: link: "Show Artifact in new tab" view_source: "View Source" view_changes: "View Changes" + change_description: "Change Description" unknown_model: "Unknown AI model" tools: @@ -293,6 +294,20 @@ en: summarizing: "Summarizing topic" searching: "Searching for: '%{query}'" tool_options: + create_artifact: + creator_llm: + name: "LLM" + description: "Language model to use for artifact creation" + update_artifact: + editor_llm: + name: "LLM" + description: "Language model to use for artifact edits" + update_algorithm: + name: "Update Algorithm" + description: "Ask LLM to fully replace, or use diff to update" + do_not_echo_artifact: + name: "Do Not Echo Artifact" + description: "Will limit costs however effectiveness of Artifact updates will be reduced" google: base_query: name: "Base Search Query" @@ -312,6 +327,7 @@ en: name: "Base Search Query" description: "Base query to use when searching. Example: '#urgent' will prepend '#urgent' to the search query and only include topics with the urgent category or tag." tool_summary: + read_artifact: "Read a web artifact" update_artifact: "Update a web artifact" create_artifact: "Create web artifact" web_browser: "Browse Web" @@ -335,6 +351,7 @@ en: search_meta_discourse: "Search Meta Discourse" javascript_evaluator: "Evaluate JavaScript" tool_help: + read_artifact: "Read a web artifact using the AI Bot" update_artifact: "Update a web artifact using the AI Bot" create_artifact: "Create a web artifact using the AI Bot" web_browser: "Browse web page using the AI Bot" @@ -358,8 +375,9 @@ en: search_meta_discourse: "Search Meta Discourse" javascript_evaluator: "Evaluate JavaScript" tool_description: + read_artifact: "Read a web artifact using the AI Bot" update_artifact: "Updated a web artifact using the AI Bot" - create_artifact: "Created a web artifact using the AI Bot" + create_artifact: "Created a web artifact: %{name} - %{specification}" web_browser: "Reading %{url}" github_search_files: "Searched for '%{keywords}' in %{repo}/%{branch}" github_search_code: "Searched for '%{query}' in %{repo}" diff --git a/db/fixtures/ai_bot/603_bot_ai_personas.rb b/db/fixtures/ai_bot/603_bot_ai_personas.rb index 90ba535e..06c11169 100644 --- a/db/fixtures/ai_bot/603_bot_ai_personas.rb +++ b/db/fixtures/ai_bot/603_bot_ai_personas.rb @@ -37,7 +37,19 @@ DiscourseAi::AiBot::Personas::Persona.system_personas.each do |persona_class, id persona.system = true instance = persona_class.new - persona.tools = instance.tools.map { |tool| tool.to_s.split("::").last } + tools = {} + instance.tools.map { |tool| tool.to_s.split("::").last }.each { |name| tools[name] = nil } + existing_tools = persona.tools || [] + + existing_tools.each do |tool| + if tool.is_a?(Array) + name, value = tool + tools[name] = value if tools.key?(name) + end + end + + persona.tools = tools.map { |name, value| [name, value] } + persona.system_prompt = instance.system_prompt persona.top_p = instance.top_p persona.temperature = instance.temperature diff --git a/lib/ai_bot/artifact_update_strategies/base.rb b/lib/ai_bot/artifact_update_strategies/base.rb new file mode 100644 index 00000000..91bcb306 --- /dev/null +++ b/lib/ai_bot/artifact_update_strategies/base.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true +module DiscourseAi + module AiBot + module ArtifactUpdateStrategies + class InvalidFormatError < StandardError + end + class Base + attr_reader :post, :user, :artifact, :artifact_version, :instructions, :llm + + def initialize(llm:, post:, user:, artifact:, artifact_version:, instructions:) + @llm = llm + @post = post + @user = user + @artifact = artifact + @artifact_version = artifact_version + @instructions = instructions + end + + def apply(&progress) + changes = generate_changes(&progress) + parsed_changes = parse_changes(changes) + apply_changes(parsed_changes) + end + + private + + def generate_changes(&progress) + response = +"" + llm.generate(build_prompt, user: user) do |partial| + progress.call(partial) if progress + response << partial + end + response + end + + def build_prompt + # To be implemented by subclasses + raise NotImplementedError + end + + def parse_changes(response) + # To be implemented by subclasses + raise NotImplementedError + end + + def apply_changes(changes) + # To be implemented by subclasses + raise NotImplementedError + end + end + end + end +end diff --git a/lib/ai_bot/artifact_update_strategies/diff.rb b/lib/ai_bot/artifact_update_strategies/diff.rb new file mode 100644 index 00000000..fc2c2345 --- /dev/null +++ b/lib/ai_bot/artifact_update_strategies/diff.rb @@ -0,0 +1,183 @@ +# frozen_string_literal: true +module DiscourseAi + module AiBot + module ArtifactUpdateStrategies + class Diff < Base + private + + def build_prompt + DiscourseAi::Completions::Prompt.new( + system_prompt, + messages: [{ type: :user, content: user_prompt }], + post_id: post.id, + topic_id: post.topic_id, + ) + end + + def parse_changes(response) + sections = { html: nil, css: nil, javascript: nil } + current_section = nil + lines = [] + + response.each_line do |line| + case line + when /^\[(HTML|CSS|JavaScript)\]$/ + sections[current_section] = lines.join if current_section && !lines.empty? + current_section = line.match(/^\[(.+)\]$/)[1].downcase.to_sym + lines = [] + when %r{^\[/(?:HTML|CSS|JavaScript)\]$} + sections[current_section] = lines.join if current_section && !lines.empty? + current_section = nil + else + lines << line if current_section + end + end + + sections.each do |section, content| + sections[section] = extract_search_replace_blocks(content) + end + + sections + end + + def apply_changes(changes) + source = artifact_version || artifact + updated_content = { js: source.js, html: source.html, css: source.css } + + %i[html css javascript].each do |section| + blocks = changes[section] + next unless blocks + + content = source.public_send(section == :javascript ? :js : section) + blocks.each do |block| + begin + content = + DiscourseAi::Utils::DiffUtils::SimpleDiff.apply( + content, + block[:search], + block[:replace], + ) + rescue DiscourseAi::Utils::DiffUtils::SimpleDiff::NoMatchError + # TODO, we may need to inform caller here, LLM made a mistake which it + # should correct + end + end + updated_content[section == :javascript ? :js : section] = content + end + + artifact.create_new_version( + html: updated_content[:html], + css: updated_content[:css], + js: updated_content[:js], + change_description: instructions, + ) + end + + private + + def extract_search_replace_blocks(content) + return nil if content.blank? + + blocks = [] + remaining = content + + pattern = /<<+\s*SEARCH\s*\n(.*?)\n=+\s*\n(.*?)\n>>+\s*REPLACE/m + while remaining =~ pattern + blocks << { search: $1.strip, replace: $2.strip } + remaining = $' + end + + blocks.empty? ? nil : blocks + end + + def system_prompt + <<~PROMPT + You are a web development expert generating precise search/replace changes for updating HTML, CSS, and JavaScript code. + + Important rules: + + 1. Use EXACTLY this format for changes: + <<<<<<< SEARCH + (exact code to find) + ======= + (replacement code) + >>>>>>> REPLACE + 2. DO NOT modify the markers or add spaces around them + 3. DO NOT add explanations or comments within sections + 4. ONLY include [HTML], [CSS], and [JavaScript] sections if they have changes + 5. Ensure search text matches EXACTLY - partial matches will fail + 6. Keep changes minimal and focused + 7. HTML should not include , , or tags, it is injected into a template + + JavaScript libraries must be sourced from the following CDNs, otherwise CSP will reject it: + #{AiArtifact::ALLOWED_CDN_SOURCES.join("\n")} + + Reply Format: + [HTML] + (changes or empty if no changes) + [/HTML] + [CSS] + (changes or empty if no changes) + [/CSS] + [JavaScript] + (changes or empty if no changes) + [/JavaScript] + + Example - Multiple changes in one file: + + [JavaScript] + <<<<<<< SEARCH + console.log('old1'); + ======= + console.log('new1'); + >>>>>>> REPLACE + <<<<<<< SEARCH + console.log('old2'); + ======= + console.log('new2'); + >>>>>>> REPLACE + [/JavaScript] + + Example - CSS with multiple blocks: + + [CSS] + <<<<<<< SEARCH + .button { color: blue; } + ======= + .button { color: red; } + >>>>>>> REPLACE + <<<<<<< SEARCH + .text { font-size: 12px; } + ======= + .text { font-size: 16px; } + >>>>>>> REPLACE + [/CSS] + PROMPT + end + + def user_prompt + source = artifact_version || artifact + <<~CONTENT + Artifact code: + + [HTML] + #{source.html} + [/HTML] + + [CSS] + #{source.css} + [/CSS] + + [JavaScript] + #{source.js} + [/JavaScript] + + Instructions: + + #{instructions} + CONTENT + end + end + end + end +end diff --git a/lib/ai_bot/artifact_update_strategies/full.rb b/lib/ai_bot/artifact_update_strategies/full.rb new file mode 100644 index 00000000..26f02a84 --- /dev/null +++ b/lib/ai_bot/artifact_update_strategies/full.rb @@ -0,0 +1,148 @@ +# frozen_string_literal: true +module DiscourseAi + module AiBot + module ArtifactUpdateStrategies + class Full < Base + private + + def build_prompt + DiscourseAi::Completions::Prompt.new( + system_prompt, + messages: [ + { type: :user, content: "#{current_artifact_content}\n\n\n#{instructions}" }, + ], + post_id: post.id, + topic_id: post.topic_id, + ) + end + + def parse_changes(response) + sections = { html: nil, css: nil, javascript: nil } + current_section = nil + lines = [] + + response.each_line do |line| + case line + when /^\[(HTML|CSS|JavaScript)\]$/ + sections[current_section] = lines.join if current_section && !lines.empty? + current_section = line.match(/^\[(.+)\]$/)[1].downcase.to_sym + lines = [] + when %r{^\[/(HTML|CSS|JavaScript)\]$} + sections[current_section] = lines.join if current_section && !lines.empty? + current_section = nil + lines = [] + else + lines << line if current_section + end + end + + sections + end + + def apply_changes(changes) + source = artifact_version || artifact + updated_content = { js: source.js, html: source.html, css: source.css } + + %i[html css javascript].each do |section| + content = changes[section]&.strip + next if content.blank? + updated_content[section == :javascript ? :js : section] = content + end + + artifact.create_new_version( + html: updated_content[:html], + css: updated_content[:css], + js: updated_content[:js], + change_description: instructions, + ) + end + + private + + def system_prompt + <<~PROMPT + You are a web development expert generating updated HTML, CSS, and JavaScript code. + + Important rules: + 1. Provide full source code for each changed section + 2. Generate up to three sections: HTML, CSS, and JavaScript + 3. Only include sections that need changes + 4. Keep changes focused on the requirements + 5. NEVER EVER BE LAZY, always include ALL the source code with any update you make. If you are lazy you will break the artifact. + 6. Do not print out any reasoning, just the changed code, you will be parsed via a program. + 7. Sections must start and end with exact tags: [HTML] [/HTML], [CSS] [/CSS], [JavaScript] [/JavaScript] + 8. HTML should not include , , or tags, it is injected into a template + + JavaScript libraries must be sourced from the following CDNs, otherwise CSP will reject it: + #{AiArtifact::ALLOWED_CDN_SOURCES.join("\n")} + + Always adhere to the format when replying: + + [HTML] + complete html code, omit if no changes + [/HTML] + + [CSS] + complete css code, omit if no changes + [/CSS] + + [JavaScript] + complete js code, omit if no changes + [/JavaScript] + + Examples: + + Example 1 (HTML only change): + [HTML] +
+

Title

+
+ [/HTML] + + Example 2 (CSS and JavaScript changes): + [CSS] + .container { padding: 20px; } + .title { color: blue; } + [/CSS] + [JavaScript] + function init() { + console.log("loaded"); + } + [/JavaScript] + + Example 3 (All sections): + [HTML] +
+ [/HTML] + [CSS] + #app { margin: 0; } + [/CSS] + [JavaScript] + const app = document.getElementById("app"); + [/JavaScript] + + PROMPT + end + + def current_artifact_content + source = artifact_version || artifact + <<~CONTENT + Current artifact code: + + [HTML] + #{source.html} + [/HTML] + + [CSS] + #{source.css} + [/CSS] + + [JavaScript] + #{source.js} + [/JavaScript] + CONTENT + end + end + end + end +end diff --git a/lib/ai_bot/bot.rb b/lib/ai_bot/bot.rb index 6160bff6..f2b1d53c 100644 --- a/lib/ai_bot/bot.rb +++ b/lib/ai_bot/bot.rb @@ -220,8 +220,11 @@ module DiscourseAi update_blk.call("", cancel, build_placeholder(tool.summary, "")) if show_placeholder result = - tool.invoke do |progress| - if show_placeholder + tool.invoke do |progress, render_raw| + if render_raw + update_blk.call("", cancel, tool.custom_raw, :partial_invoke) + show_placeholder = false + elsif show_placeholder placeholder = build_placeholder(tool.summary, progress) update_blk.call("", cancel, placeholder) end diff --git a/lib/ai_bot/personas/persona.rb b/lib/ai_bot/personas/persona.rb index 63220e71..7fe1b9ac 100644 --- a/lib/ai_bot/personas/persona.rb +++ b/lib/ai_bot/personas/persona.rb @@ -102,6 +102,7 @@ module DiscourseAi if SiteSetting.ai_artifact_security.in?(%w[lax strict]) tools << Tools::CreateArtifact tools << Tools::UpdateArtifact + tools << Tools::ReadArtifact end tools << Tools::GithubSearchCode if SiteSetting.ai_bot_github_access_token.present? @@ -201,7 +202,9 @@ module DiscourseAi prompt.max_pixels = self.class.vision_max_pixels if self.class.vision_enabled prompt.tools = available_tools.map(&:signature) if available_tools - + available_tools.each do |tool| + tool.inject_prompt(prompt: prompt, context: context, persona: self) + end prompt end diff --git a/lib/ai_bot/personas/web_artifact_creator.rb b/lib/ai_bot/personas/web_artifact_creator.rb index 9297371e..ec99a778 100644 --- a/lib/ai_bot/personas/web_artifact_creator.rb +++ b/lib/ai_bot/personas/web_artifact_creator.rb @@ -5,11 +5,11 @@ module DiscourseAi module Personas class WebArtifactCreator < Persona def tools - [Tools::CreateArtifact, Tools::UpdateArtifact] + [Tools::CreateArtifact, Tools::UpdateArtifact, Tools::ReadArtifact] end def required_tools - [Tools::CreateArtifact, Tools::UpdateArtifact] + [Tools::CreateArtifact, Tools::UpdateArtifact, Tools::ReadArtifact] end def system_prompt diff --git a/lib/ai_bot/playground.rb b/lib/ai_bot/playground.rb index 8f95cf57..65719870 100644 --- a/lib/ai_bot/playground.rb +++ b/lib/ai_bot/playground.rb @@ -479,7 +479,9 @@ module DiscourseAi raw = reply.dup raw << "\n\n" << placeholder if placeholder.present? - blk.call(partial) if blk && type != :tool_details && type != :partial_tool + if blk && type != :tool_details && type != :partial_tool && type != :partial_invoke + blk.call(partial) + end if stream_reply && !Discourse.redis.get(redis_stream_key) cancel&.call diff --git a/lib/ai_bot/tools/create_artifact.rb b/lib/ai_bot/tools/create_artifact.rb index 627131d4..b9175e8d 100644 --- a/lib/ai_bot/tools/create_artifact.rb +++ b/lib/ai_bot/tools/create_artifact.rb @@ -8,38 +8,49 @@ module DiscourseAi "create_artifact" end - def self.js_dependency_tip - <<~TIP - If you need to include a JavaScript library, you may include assets from: - - unpkg.com - - cdnjs.com - - jsdelivr.com - - ajax.googleapis.com + def self.specification_description + <<~DESC + A detailed description of the web artifact you want to create. Your specification should include: - To include them ensure they are the last tag in your HTML body. - Example: - TIP - end + 1. Purpose and functionality + 2. Visual design requirements + 3. Interactive elements and behavior + 4. Data handling (if applicable) + 5. Specific requirements or constraints + 6. DO NOT include full source code of the artifact, just very clear requirements - def self.js_script_tag_tip - <<~TIP - if you need a custom script tag, you can use the following format: + Good specification examples: - + Example: (Calculator): + "Create a modern calculator with a dark theme. It should: + - Have a large display area showing current and previous calculations + - Include buttons for numbers 0-9, basic operations (+,-,*,/), and clear + - Use a grid layout with subtle hover effects on buttons + - Show button press animations + - Keep calculation history visible above current input + - Use a monospace font for numbers + - Support keyboard input for numbers and operations" - If you only need a regular script tag, you can use the following format: + Poor specification example: + "Make a website that looks nice and does cool stuff" + (Too vague, lacks specific requirements and functionality details) - // your script here - TIP + Tips for good specifications: + - Be specific about layout and design preferences + - Describe all interactive elements and their behavior + - Include any specific visual effects or animations + - Mention responsive design requirements if needed + - List any specific libraries or frameworks to use/avoid + - Describe error states and edge cases + - Include accessibility requirements + - Include code snippets to help ground the specification + DESC end def self.signature { name: "create_artifact", - description: - "Creates a web artifact with HTML, CSS, and JavaScript that can be displayed in an iframe", + description: "Creates a web artifact based on a specification", parameters: [ { name: "name", @@ -48,121 +59,243 @@ module DiscourseAi required: true, }, { - name: "html_body", - description: - "The HTML content for the BODY tag (do not include the BODY tag). #{js_dependency_tip}", + name: "specification", type: "string", + description: specification_description, required: true, }, - { name: "css", description: "Optional CSS styles for the artifact", type: "string" }, - { - name: "js", - description: - "Optional - JavaScript code for the artifact, this will be the last + + Required response format: + + [HTML] +
+ [/HTML] + + [CSS] + #app { /* Your complete CSS here */ } + [/CSS] + + [JavaScript] + // Your complete JavaScript here + [/JavaScript] + + Important: + - All three sections are required + - Sections must use exact tags shown above + - Focus on simplicity and reliability + - Include basic error handling + - Follow accessibility guidelines + - No explanatory text, only code + PROMPT + end + + def update_custom_html(artifact) + html_preview = <<~MD + [details="View Source"] + ### HTML + ```html + #{artifact.html} + ``` + + ### CSS + ```css + #{artifact.css} + ``` + + ### JavaScript + ```javascript + #{artifact.js} + ``` + [/details] + + ### Preview +
+ MD + + self.custom_raw = html_preview end def success_response(artifact) - @chain_next_response = false - - { - status: "success", - artifact_id: artifact.id, - message: "Artifact created successfully and rendered to user.", - } + { status: "success", artifact_id: artifact.id, message: "Artifact created successfully." } end def error_response(message) - @chain_next_response = false - { status: "error", error: message } end - - def help - "Creates a web artifact with HTML, CSS, and JavaScript that can be displayed in an iframe. " \ - "Requires a name and HTML content. CSS and JavaScript are optional. " \ - "The artifact will be associated with the current post and can be displayed using an iframe." - end end end end diff --git a/lib/ai_bot/tools/option.rb b/lib/ai_bot/tools/option.rb index eb0b6e1f..b772ab8c 100644 --- a/lib/ai_bot/tools/option.rb +++ b/lib/ai_bot/tools/option.rb @@ -4,12 +4,14 @@ module DiscourseAi module AiBot module Tools class Option - attr_reader :tool, :name, :type + attr_reader :tool, :name, :type, :values, :default - def initialize(tool:, name:, type:) + def initialize(tool:, name:, type:, values: nil, default: nil) @tool = tool @name = name.to_s @type = type + @values = values + @default = default end def localized_name diff --git a/lib/ai_bot/tools/read_artifact.rb b/lib/ai_bot/tools/read_artifact.rb new file mode 100644 index 00000000..7fb5646c --- /dev/null +++ b/lib/ai_bot/tools/read_artifact.rb @@ -0,0 +1,211 @@ +# frozen_string_literal: true + +module DiscourseAi + module AiBot + module Tools + class ReadArtifact < Tool + MAX_HTML_SIZE = 30.kilobytes + MAX_CSS_FILES = 5 + + def self.name + "read_artifact" + end + + def self.signature + { + name: "read_artifact", + description: "Read an artifact from a URL and convert to a local artifact", + parameters: [ + { + name: "url", + type: "string", + description: "URL of the artifact to read", + required: true, + }, + ], + } + end + + def invoke + return error_response("Unknown context, feature only works in PMs") if !post + + uri = URI.parse(parameters[:url]) + return error_response("Invalid URL") unless uri.is_a?(URI::HTTP) + + if discourse_artifact?(uri) + handle_discourse_artifact(uri) + else + handle_external_page(uri) + end + end + + def chain_next_response? + @chain_next_response + end + + private + + def error_response(message) + @chain_next_response = true + { status: "error", error: message } + end + + def success_response(artifact) + { status: "success", artifact_id: artifact.id, message: "Artifact created successfully." } + end + + def discourse_artifact?(uri) + uri.path.include?("/discourse-ai/ai-bot/artifacts/") + end + + def post + @post ||= Post.find_by(id: context[:post_id]) + end + + def handle_discourse_artifact(uri) + if uri.path =~ %r{/discourse-ai/ai-bot/artifacts/(\d+)(?:/(\d+))?} + artifact_id = $1.to_i + version = $2&.to_i + else + return error_response("Invalid artifact URL format") + end + + if uri.host == Discourse.current_hostname + source_artifact = AiArtifact.find_by(id: artifact_id) + return error_response("Artifact not found") if !source_artifact + + if !source_artifact.public? && !Guardian.new(post.user).can_see?(source_artifact.post) + return error_response("Access denied") + end + new_artifact = clone_artifact(source_artifact, version) + else + response = fetch_page(uri) + return error_response("Failed to fetch artifact") unless response + + html, css, js = extract_discourse_artifact(response.body) + return error_response("Invalid artifact format") unless html + + new_artifact = + create_artifact_from_web( + html: html, + css: css, + js: js, + name: "Imported Discourse Artifact", + ) + end + + if new_artifact&.persisted? + update_custom_html(new_artifact) + success_response(new_artifact) + else + error_response( + new_artifact&.errors&.full_messages&.join(", ") || "Failed to create artifact", + ) + end + end + + def extract_discourse_artifact(html) + doc = Nokogiri.HTML(html) + iframe = doc.at_css("body > iframe") + return nil unless iframe + + # parse srcdoc attribute of iframe + iframe_doc = Nokogiri.HTML(iframe["srcdoc"]) + return nil unless iframe_doc + + body = iframe_doc.at_css("body") + last_script_tag = body&.at_css("script:last-of-type") + script = last_script_tag&.content.to_s[0...MAX_HTML_SIZE] + last_script_tag.remove if last_script_tag + content = body&.inner_html.to_s[0...MAX_HTML_SIZE] + style = iframe_doc.at_css("style")&.content.to_s[0...MAX_HTML_SIZE] + + [content, style, script] + end + + def handle_external_page(uri) + response = fetch_page(uri) + return error_response("Failed to fetch page") unless response + + html, css, js = extract_content(response, uri) + new_artifact = + create_artifact_from_web(html: html, css: css, js: js, name: "external artifact") + + if new_artifact&.persisted? + update_custom_html(new_artifact) + success_response(new_artifact) + else + error_response( + new_artifact&.errors&.full_messages&.join(", ") || "Failed to create artifact", + ) + end + end + + def extract_content(response, uri) + doc = Nokogiri.HTML(response.body) + + html = doc.at_css("body").to_html.to_s[0...MAX_HTML_SIZE] + + css_files = + doc + .css('link[rel="stylesheet"]') + .map { |link| URI.join(uri, link["href"]).to_s } + .first(MAX_CSS_FILES) + css = download_css_files(css_files).to_s[0...MAX_HTML_SIZE] + + js = doc.css("script:not([src])").map(&:content).join("\n").to_s[0...MAX_HTML_SIZE] + + [html, css, js] + end + + def clone_artifact(source, version = nil) + source_version = version ? source.versions.find_by(version_number: version) : nil + content = source_version || source + + AiArtifact.create!( + user: post.user, + post: post, + name: source.name, + html: content.html, + css: content.css, + js: content.js, + metadata: { + cloned_from: source.id, + cloned_version: source_version&.version_number, + }, + ) + end + + def create_artifact_from_web(html:, css:, js:, name:) + AiArtifact.create( + user: post.user, + post: post, + name: name, + html: html, + css: css, + js: js, + metadata: { + imported_from: parameters[:url], + }, + ) + end + + def update_custom_html(artifact) + self.custom_raw = <<~HTML + ### Artifact created successfully + +
+ HTML + end + + def fetch_page(uri) + send_http_request(uri.to_s) { |response| response if response.code == "200" } + end + + def download_css_files(urls) + urls.map { |url| fetch_page(URI.parse(url)).body }.join("\n") + end + end + end + end +end diff --git a/lib/ai_bot/tools/tool.rb b/lib/ai_bot/tools/tool.rb index 89cbb762..ff95f16e 100644 --- a/lib/ai_bot/tools/tool.rb +++ b/lib/ai_bot/tools/tool.rb @@ -27,8 +27,8 @@ module DiscourseAi [] end - def option(name, type:) - Option.new(tool: self, name: name, type: type) + def option(name, type:, values: nil, default: nil) + Option.new(tool: self, name: name, type: type, values: values, default: default) end def help @@ -42,6 +42,9 @@ module DiscourseAi def allow_partial_tool_calls? false end + + def inject_prompt(prompt:, context:, persona:) + end end attr_accessor :custom_raw, :parameters @@ -89,8 +92,13 @@ module DiscourseAi val = (val.to_s == "true") when :integer val = val.to_i + when :enum + val = val.to_s + val = option.default if option.values && !option.values.include?(val) end result[option.name] = val + elsif val.nil? + result[option.name] = option.default end end result diff --git a/lib/ai_bot/tools/update_artifact.rb b/lib/ai_bot/tools/update_artifact.rb index 0ea6b846..4b62bd01 100644 --- a/lib/ai_bot/tools/update_artifact.rb +++ b/lib/ai_bot/tools/update_artifact.rb @@ -8,50 +8,10 @@ module DiscourseAi "update_artifact" end - # this is not working that well, we support it, but I am leaving it dormant for now - def self.unified_diff_tip - <<~TIP - When updating and artifact in diff mode unified diffs can be applied: - - If editing: - -
-

Some text

-
- - You can provide a diff like: - -
- -

Some text

- +

Some new text

-
- - This will result in: - -
-

Some new text

-
- - If you need to supply multiple hunks for a diff use a @@ separator, for example: - - @@ -1,3 +1,3 @@ - -

Some text

- +

Some new text

- @@ -5,3 +5,3 @@ - -
- +

more text

-
- - If you supply text without @@ seperators or + and - prefixes, the entire text will be appended to the artifact section. - - TIP - end - def self.signature { name: "update_artifact", - description: - "Updates an existing web artifact with new HTML, CSS, or JavaScript content. Note either html, css, or js MUST be provided. You may provide all three if desired.", + description: "Updates an existing web artifact", parameters: [ { name: "artifact_id", @@ -59,138 +19,241 @@ module DiscourseAi type: "integer", required: true, }, - { name: "html", description: "new HTML content for the artifact", type: "string" }, - { name: "css", description: "new CSS content for the artifact", type: "string" }, { - name: "js", - description: "new JavaScript content for the artifact", + name: "instructions", + description: "Clear instructions on what changes need to be made to the artifact.", type: "string", + required: true, }, { - name: "change_description", + name: "version", description: - "A brief description of the changes being made. Note: This only documents the change - you must provide the actual content in html/css/js parameters to make changes.", - type: "string", + "The version number of the artifact to update, if not supplied latest version will be updated", + type: "integer", + required: false, }, ], } end + def self.inject_prompt(prompt:, context:, persona:) + return if persona.options["do_not_echo_artifact"].to_s == "true" + # we inject the current artifact content into the last user message + if topic_id = context[:topic_id] + posts = Post.where(topic_id: topic_id) + artifact = AiArtifact.order("id desc").where(post: posts).first + if artifact + latest_version = artifact.versions.order(version_number: :desc).first + current = latest_version || artifact + + artifact_source = <<~MSG + Current Artifact: + + ### HTML + ```html + #{current.html} + ``` + + ### CSS + ```css + #{current.css} + ``` + + ### JavaScript + ```javascript + #{current.js} + ``` + + MSG + + last_message = prompt.messages.last + last_message[:content] = "#{artifact_source}\n\n#{last_message[:content]}" + end + end + end + + def self.accepted_options + [ + option(:editor_llm, type: :llm), + option(:update_algorithm, type: :enum, values: %w[diff full], default: "diff"), + option(:do_not_echo_artifact, type: :boolean, default: true), + ] + end + def self.allow_partial_tool_calls? true end - def chain_next_response? - @chain_next_response + def partial_invoke + in_progress(instructions: parameters[:instructions]) if parameters[:instructions].present? end - def partial_invoke - @selected_tab = :html - if @prev_parameters - @selected_tab = parameters.keys.find { |k| @prev_parameters[k] != parameters[k] } - end - update_custom_html - @prev_parameters = parameters.dup + def in_progress(instructions:, source: nil) + source = (<<~HTML) if source.present? + ### Source + + ```` + #{source} + ```` + HTML + + self.custom_raw = <<~HTML +
+ Thinking... + + ### Instructions + ```` + #{instructions} + ```` + + #{source} + +
+ HTML end def invoke - yield "Updating Artifact" - post = Post.find_by(id: context[:post_id]) return error_response("No post context found") unless post artifact = AiArtifact.find_by(id: parameters[:artifact_id]) return error_response("Artifact not found") unless artifact + artifact_version = nil + if version = parameters[:version] + artifact_version = artifact.versions.find_by(version_number: version) + # we could tell llm it is confused here if artifact version is not there + # but let's just fix it transparently which saves an llm call + end + + artifact_version ||= artifact.versions.order(version_number: :desc).first + if artifact.post.topic.id != post.topic.id return error_response("Attempting to update an artifact you are not allowed to") end - last_version = artifact.versions.order(version_number: :desc).first + llm = + ( + options[:editor_llm].present? && + LlmModel.find_by(id: options[:editor_llm].to_i)&.to_llm + ) || self.llm + + strategy = + ( + if options[:update_algorithm] == "diff" + ArtifactUpdateStrategies::Diff + else + ArtifactUpdateStrategies::Full + end + ) begin - version = - artifact.create_new_version( - html: parameters[:html] || last_version&.html || artifact.html, - css: parameters[:css] || last_version&.css || artifact.css, - js: parameters[:js] || last_version&.js || artifact.js, - change_description: parameters[:change_description].to_s, - ) + instructions = parameters[:instructions] + partial_response = +"" + new_version = + strategy + .new( + llm: llm, + post: post, + user: post.user, + artifact: artifact, + artifact_version: artifact_version, + instructions: instructions, + ) + .apply do |progress| + partial_response << progress + in_progress(instructions: instructions, source: partial_response) + # force in progress to render + yield nil, true + end - update_custom_html(artifact, version) - success_response(artifact, version) - rescue DiscourseAi::Utils::DiffUtils::DiffError => e - error_response(e.to_llm_message) - rescue => e + update_custom_html( + artifact: artifact, + artifact_version: artifact_version, + new_version: new_version, + ) + success_response(artifact, new_version) + rescue StandardError => e error_response(e.message) end end + def chain_next_response? + false + end + private - def update_custom_html(artifact = nil, version = nil) + def line_based_markdown_diff(before, after) + # Split into lines + before_lines = before.split("\n") + after_lines = after.split("\n") + + # Use ONPDiff for line-level comparison + diff = ONPDiff.new(before_lines, after_lines).diff + + # Build markdown output + result = ["```diff"] + + diff.each do |line, status| + case status + when :common + result << " #{line}" + when :delete + result << "-#{line}" + when :add + result << "+#{line}" + end + end + + result << "```" + result.join("\n") + end + + def update_custom_html(artifact:, artifact_version:, new_version:) content = [] - if parameters[:html].present? - content << [:html, "### HTML Changes\n\n```html\n#{parameters[:html]}\n```"] - end - - if parameters[:css].present? - content << [:css, "### CSS Changes\n\n```css\n#{parameters[:css]}\n```"] - end - - if parameters[:js].present? - content << [:js, "### JavaScript Changes\n\n```javascript\n#{parameters[:js]}\n```"] - end - - if parameters[:change_description].present? - content.unshift( - [:description, "### Change Description\n\n#{parameters[:change_description]}"], - ) - end - - content.sort_by! { |c| c[0] === @selected_tab ? 1 : 0 } if !artifact - - if artifact - content.unshift([nil, "[details='#{I18n.t("discourse_ai.ai_artifact.view_changes")}']"]) - content << [nil, "[/details]"] + if new_version.change_description.present? content << [ - :preview, - "### Preview\n\n
", + :description, + "[details='#{I18n.t("discourse_ai.ai_artifact.change_description")}']\n\n````\n#{new_version.change_description}\n````\n\n[/details]", ] end + content << [nil, "[details='#{I18n.t("discourse_ai.ai_artifact.view_changes")}']"] - content.unshift("\n\n") + %w[html css js].each do |type| + source = artifact_version || artifact + old_content = source.public_send(type) + new_content = new_version.public_send(type) + + if old_content != new_content + diff = line_based_markdown_diff(old_content, new_content) + content << [nil, "### #{type.upcase} Changes\n#{diff}"] + end + end + + content << [nil, "[/details]"] + content << [ + :preview, + "### Preview\n\n
", + ] self.custom_raw = content.map { |c| c[1] }.join("\n\n") end def success_response(artifact, version) - @chain_next_response = false - - hash = { + { status: "success", artifact_id: artifact.id, version: version.version_number, message: "Artifact updated successfully and rendered to user.", } - - hash end def error_response(message) - @chain_next_response = true self.custom_raw = "" - { status: "error", error: message } end - - def help - "Updates an existing web artifact with changes to its HTML, CSS, or JavaScript content. " \ - "Requires the artifact ID and at least one change diff. " \ - "Changes are applied using unified diff format. " \ - "A description of the changes is required for version history." - end end end end diff --git a/lib/completions/endpoints/base.rb b/lib/completions/endpoints/base.rb index 58ff9c2a..a4b6050f 100644 --- a/lib/completions/endpoints/base.rb +++ b/lib/completions/endpoints/base.rb @@ -87,10 +87,11 @@ module DiscourseAi partial_tool_calls: partial_tool_calls, ) - result = [result] if !result.is_a?(Array) + wrapped = result + wrapped = [result] if !result.is_a?(Array) cancelled_by_caller = false cancel_proc = -> { cancelled_by_caller = true } - result.each do |partial| + wrapped.each do |partial| blk.call(partial, cancel_proc) break if cancelled_by_caller end diff --git a/lib/completions/endpoints/open_ai.rb b/lib/completions/endpoints/open_ai.rb index f34f29db..4523c1c0 100644 --- a/lib/completions/endpoints/open_ai.rb +++ b/lib/completions/endpoints/open_ai.rb @@ -132,7 +132,7 @@ module DiscourseAi end def decode_chunk_finish - @processor.finish + processor.finish end def xml_tools_enabled? diff --git a/lib/utils/diff_utils.rb b/lib/utils/diff_utils.rb deleted file mode 100644 index 21e78f7e..00000000 --- a/lib/utils/diff_utils.rb +++ /dev/null @@ -1,184 +0,0 @@ -# frozen_string_literal: true -# Inspired by Aider https://github.com/Aider-AI/aider - -module DiscourseAi - module Utils - module DiffUtils - # Custom errors with detailed information for LLM feedback - class DiffError < StandardError - attr_reader :original_text, :diff_text, :context - - def initialize(message, original_text:, diff_text:, context: {}) - @original_text = original_text - @diff_text = diff_text - @context = context - super(message) - end - - def to_llm_message - original_text = @original_text - original_text = @original_text[0..1000] + "..." if @original_text.length > 1000 - - <<~MESSAGE - #{message} - - Original text: - ``` - #{original_text} - ``` - - Attempted diff: - ``` - #{diff_text} - ``` - - #{context_message} - - Please provide a corrected diff that: - 1. Has the correct context lines - 2. Contains all necessary removals (-) and additions (+) - MESSAGE - end - - private - - def context_message - return "" if context.empty? - - context.map { |key, value| "#{key}: #{value}" }.join("\n") - end - end - - class NoMatchingContextError < DiffError - def initialize(original_text:, diff_text:) - super( - "Could not find the context lines in the original text", - original_text: original_text, - diff_text: diff_text, - ) - end - end - - class AmbiguousMatchError < DiffError - def initialize(original_text:, diff_text:) - super( - "Found multiple possible locations for this change", - original_text: original_text, - diff_text: diff_text, - ) - end - end - - class MalformedDiffError < DiffError - def initialize(original_text:, diff_text:, issue:) - super( - "The diff format is invalid", - original_text: original_text, - diff_text: diff_text, - context: { - "Issue" => issue, - }, - ) - end - end - - def self.apply_hunk(text, diff) - # we need to handle multiple hunks just in case - if diff.match?(/^\@\@.*\@\@$\n/) - hunks = diff.split(/^\@\@.*\@\@$\n/) - if hunks.present? - hunks.each do |hunk| - next if hunk.blank? - text = apply_hunk(text, hunk) - end - return text - end - end - - text = text.encode(universal_newline: true) - diff = diff.encode(universal_newline: true) - # we need this for matching - text = text + "\n" unless text.end_with?("\n") - - diff_lines = parse_diff_lines(diff, text) - - validate_diff_format!(text, diff, diff_lines) - - return text.strip + "\n" + diff.strip if diff_lines.all? { |marker, _| marker == " " } - - lines_to_match = diff_lines.select { |marker, _| ["-", " "].include?(marker) }.map(&:last) - match_start, match_end = find_unique_match(text, lines_to_match, diff) - new_hunk = diff_lines.select { |marker, _| ["+", " "].include?(marker) }.map(&:last).join - - new_hunk = +"" - - diff_lines_index = 0 - text[match_start..match_end].lines.each do |line| - diff_marker, diff_content = diff_lines[diff_lines_index] - - while diff_marker == "+" - new_hunk << diff_content - diff_lines_index += 1 - diff_marker, diff_content = diff_lines[diff_lines_index] - end - - new_hunk << line if diff_marker == " " - - diff_lines_index += 1 - end - - # leftover additions - diff_marker, diff_content = diff_lines[diff_lines_index] - while diff_marker == "+" - diff_lines_index += 1 - new_hunk << diff_content - diff_marker, diff_content = diff_lines[diff_lines_index] - end - - (text[0...match_start].to_s + new_hunk + text[match_end..-1].to_s).strip - end - - private_class_method def self.parse_diff_lines(diff, text) - diff.lines.map do |line| - marker = line[0] - content = line[1..] - - if !["-", "+", " "].include?(marker) - marker = " " - content = line - end - - [marker, content] - end - end - - private_class_method def self.validate_diff_format!(text, diff, diff_lines) - if diff_lines.empty? - raise MalformedDiffError.new(original_text: text, diff_text: diff, issue: "Diff is empty") - end - end - - private_class_method def self.find_unique_match(text, context_lines, diff) - return 0 if context_lines.empty? && removals.empty? - - pattern = context_lines.map { |line| "^\\s*" + Regexp.escape(line.strip) + "\s*$\n" }.join - matches = - text - .enum_for(:scan, /#{pattern}/m) - .map do - match = Regexp.last_match - [match.begin(0), match.end(0)] - end - - case matches.length - when 0 - raise NoMatchingContextError.new(original_text: text, diff_text: diff) - when 1 - matches.first - else - raise AmbiguousMatchError.new(original_text: text, diff_text: diff) - end - end - end - end -end diff --git a/lib/utils/diff_utils/hunk_diff.rb b/lib/utils/diff_utils/hunk_diff.rb new file mode 100644 index 00000000..dbe6743a --- /dev/null +++ b/lib/utils/diff_utils/hunk_diff.rb @@ -0,0 +1,228 @@ +# frozen_string_literal: true +# Inspired by Aider https://github.com/Aider-AI/aider + +module DiscourseAi + module Utils + module DiffUtils + class HunkDiff + class DiffError < StandardError + attr_reader :original_text, :diff_text, :context + + def initialize(message, original_text:, diff_text:, context: {}) + @original_text = original_text + @diff_text = diff_text + @context = context + super(message) + end + + def to_llm_message + original_text = @original_text + original_text = @original_text[0..1000] + "..." if @original_text.length > 1000 + + <<~MESSAGE + #{message} + + Original text: + ``` + #{original_text} + ``` + + Attempted diff: + ``` + #{diff_text} + ``` + + #{context_message} + + Please provide a corrected diff that: + 1. Has the correct context lines + 2. Contains all necessary removals (-) and additions (+) + MESSAGE + end + + private + + def context_message + return "" if context.empty? + + context.map { |key, value| "#{key}: #{value}" }.join("\n") + end + end + + class NoMatchingContextError < DiffError + def initialize(original_text:, diff_text:) + super( + "Could not find the context lines in the original text", + original_text: original_text, + diff_text: diff_text, + ) + end + end + + class AmbiguousMatchError < DiffError + def initialize(original_text:, diff_text:) + super( + "Found multiple possible locations for this change", + original_text: original_text, + diff_text: diff_text, + ) + end + end + + class MalformedDiffError < DiffError + def initialize(original_text:, diff_text:, issue:) + super( + "The diff format is invalid", + original_text: original_text, + diff_text: diff_text, + context: { + "Issue" => issue, + }, + ) + end + end + + def self.apply(text, diff) + new(text, diff).apply + end + + def initialize(text, diff) + @text = text.encode(universal_newline: true) + @diff = diff.encode(universal_newline: true) + @text = @text + "\n" unless @text.end_with?("\n") + end + + def apply + if multiple_hunks? + apply_multiple_hunks + else + apply_single_hunk + end + end + + private + + attr_reader :text, :diff + + def multiple_hunks? + diff.match?(/^\@\@.*\@\@$\n/) + end + + def apply_multiple_hunks + result = text + hunks = diff.split(/^\@\@.*\@\@$\n/) + + hunks.each do |hunk| + next if hunk.blank? + result = self.class.new(result, hunk).apply + end + + result + end + + def apply_single_hunk + diff_lines = parse_diff_lines + validate_diff_format!(diff_lines) + + return text.strip + "\n" + diff.strip if context_only?(diff_lines) + + lines_to_match = extract_context_lines(diff_lines) + match_start, match_end = find_unique_match(lines_to_match) + + build_result(match_start, match_end, diff_lines) + end + + def parse_diff_lines + diff.lines.map do |line| + marker = line[0] + content = line[1..] + + if !["-", "+", " "].include?(marker) + marker = " " + content = line + end + + [marker, content] + end + end + + def validate_diff_format!(diff_lines) + if diff_lines.empty? + raise MalformedDiffError.new( + original_text: text, + diff_text: diff, + issue: "Diff is empty", + ) + end + end + + def context_only?(diff_lines) + diff_lines.all? { |marker, _| marker == " " } + end + + def extract_context_lines(diff_lines) + diff_lines.select { |marker, _| ["-", " "].include?(marker) }.map(&:last) + end + + def find_unique_match(context_lines) + return 0, 0 if context_lines.empty? + + pattern = context_lines.map { |line| "^\\s*" + Regexp.escape(line.strip) + "\s*$\n" }.join + matches = + text + .enum_for(:scan, /#{pattern}/m) + .map do + match = Regexp.last_match + [match.begin(0), match.end(0)] + end + + case matches.length + when 0 + raise NoMatchingContextError.new(original_text: text, diff_text: diff) + when 1 + matches.first + else + raise AmbiguousMatchError.new(original_text: text, diff_text: diff) + end + end + + def build_result(match_start, match_end, diff_lines) + new_hunk = +"" + diff_lines_index = 0 + + text[match_start..match_end].lines.each do |line| + diff_marker, diff_content = diff_lines[diff_lines_index] + + while diff_marker == "+" + new_hunk << diff_content + diff_lines_index += 1 + diff_marker, diff_content = diff_lines[diff_lines_index] + end + + new_hunk << line if diff_marker == " " + + diff_lines_index += 1 + end + + # Handle any remaining additions + append_remaining_additions(new_hunk, diff_lines, diff_lines_index) + + combine_result(match_start, match_end, new_hunk) + end + + def append_remaining_additions(new_hunk, diff_lines, diff_lines_index) + diff_marker, diff_content = diff_lines[diff_lines_index] + while diff_marker == "+" + diff_lines_index += 1 + new_hunk << diff_content + diff_marker, diff_content = diff_lines[diff_lines_index] + end + end + + def combine_result(match_start, match_end, new_hunk) + (text[0...match_start].to_s + new_hunk + text[match_end..-1].to_s).strip + end + end + end + end +end diff --git a/lib/utils/diff_utils/simple_diff.rb b/lib/utils/diff_utils/simple_diff.rb new file mode 100644 index 00000000..ca67629e --- /dev/null +++ b/lib/utils/diff_utils/simple_diff.rb @@ -0,0 +1,178 @@ +# frozen_string_literal: true + +module DiscourseAi + module Utils + module DiffUtils + class SimpleDiff + LEVENSHTEIN_THRESHOLD = 2 + + class Error < StandardError + end + class NoMatchError < Error + end + + def self.apply(content, search, replace) + new.apply(content, search, replace) + end + + def apply(content, search, replace) + raise ArgumentError, "content cannot be nil" if content.nil? + raise ArgumentError, "search cannot be nil" if search.nil? + raise ArgumentError, "replace cannot be nil" if replace.nil? + + lines = content.split("\n") + search_lines = search.split("\n") + + # 1. Try exact matching + match_positions = + find_matches(lines, search_lines) { |line, search_line| line == search_line } + + # 2. Try stripped matching + if match_positions.empty? + match_positions = + find_matches(lines, search_lines) do |line, search_line| + line.strip == search_line.strip + end + end + + # 3. Try fuzzy matching + if match_positions.empty? + match_positions = + find_matches(lines, search_lines) do |line, search_line| + fuzzy_match?(line, search_line) + end + end + + # 4. Try block matching as last resort + if match_positions.empty? + if block_matches = find_block_matches(content, search) + return replace_blocks(content, block_matches, replace) + end + end + + if match_positions.empty? + raise NoMatchError, "Could not find a match for the search content" + end + + # Replace matches in reverse order + match_positions.sort.reverse.each do |pos| + lines.slice!(pos, search_lines.length) + lines.insert(pos, *replace.split("\n")) + end + + lines.join("\n") + end + + private + + def find_matches(lines, search_lines) + matches = [] + max_index = lines.length - search_lines.length + + (0..max_index).each do |i| + if (0...search_lines.length).all? { |j| yield(lines[i + j], search_lines[j]) } + matches << i + end + end + + matches + end + + def fuzzy_match?(line, search_line) + return true if line.strip == search_line.strip + s1 = line.lstrip + s2 = search_line.lstrip + levenshtein_distance(s1, s2) <= LEVENSHTEIN_THRESHOLD + end + + def levenshtein_distance(s1, s2) + m = s1.length + n = s2.length + d = Array.new(m + 1) { Array.new(n + 1, 0) } + + (0..m).each { |i| d[i][0] = i } + (0..n).each { |j| d[0][j] = j } + + (1..m).each do |i| + (1..n).each do |j| + cost = s1[i - 1] == s2[j - 1] ? 0 : 1 + d[i][j] = [d[i - 1][j] + 1, d[i][j - 1] + 1, d[i - 1][j - 1] + cost].min + end + end + + d[m][n] + end + + def find_block_matches(content, search) + content_blocks = extract_blocks(content) + search_blocks = extract_blocks(search) + + return nil if content_blocks.empty? || search_blocks.empty? + + matches = [] + search_blocks.each do |search_block| + content_blocks.each do |content_block| + matches << content_block if content_block[:text] == search_block[:text] + end + end + + matches.empty? ? nil : matches + end + + def extract_blocks(text) + lines = text.split("\n") + blocks = [] + current_block = [] + block_start = nil + + lines.each_with_index do |line, index| + if line =~ /^[^\s]/ + # Save previous block if exists + if !current_block.empty? + current_block << line + blocks << { + start: block_start, + length: current_block.length, + text: current_block.join("\n").strip, + } + current_block = [] + else + current_block = [line] + block_start = index + end + else + # Continue current block + current_block << line if current_block.any? + end + end + + # Add final block + if !current_block.empty? + blocks << { + start: block_start, + length: current_block.length, + text: current_block.join("\n").strip, + } + end + + blocks + end + + def replace_blocks(content, blocks, replace) + lines = content.split("\n") + + # Sort blocks in reverse order to maintain correct positions + blocks + .sort_by { |b| -b[:start] } + .each_with_index do |block, index| + replacement = index.zero? ? replace : "" + lines.slice!(block[:start], block[:length]) + lines.insert(block[:start], *replacement.split("\n")) + end + + lines.join("\n") + end + end + end + end +end diff --git a/spec/lib/modules/ai_bot/tools/create_artifact_spec.rb b/spec/lib/modules/ai_bot/tools/create_artifact_spec.rb index 785088d1..058e28d2 100644 --- a/spec/lib/modules/ai_bot/tools/create_artifact_spec.rb +++ b/spec/lib/modules/ai_bot/tools/create_artifact_spec.rb @@ -2,7 +2,6 @@ RSpec.describe DiscourseAi::AiBot::Tools::CreateArtifact do fab!(:llm_model) - let(:bot_user) { DiscourseAi::AiBot::EntryPoint.find_user_from_model(llm_model.name) } let(:llm) { DiscourseAi::Completions::Llm.proxy("custom:#{llm_model.id}") } fab!(:post) @@ -10,61 +9,70 @@ RSpec.describe DiscourseAi::AiBot::Tools::CreateArtifact do describe "#process" do it "correctly adds details block on final invoke" do - tool = - described_class.new( - { html_body: "hello" }, - bot_user: Fabricate(:user), - llm: llm, - context: { - post_id: post.id, - }, - ) + responses = [<<~TXT.strip] + [HTML] +
+ hello +
+ [/HTML] + [CSS] + .hello { + color: red; + } + [/CSS] + [JavaScript] + console.log("hello"); + console.log("world"); + [/JavaScript] + TXT - tool.parameters = { html_body: "hello" } + tool = nil - tool.invoke {} + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do + tool = + described_class.new( + { html_body: "hello" }, + bot_user: Fabricate(:user), + llm: llm, + context: { + post_id: post.id, + }, + ) + + tool.parameters = { name: "hello", specification: "hello spec" } + + tool.invoke {} + end artifact_id = AiArtifact.order("id desc").limit(1).pluck(:id).first expected = <<~MD - [details='View Source'] - + [details="View Source"] ### HTML - ```html - hello +
+ hello +
``` + ### CSS + ```css + .hello { + color: red; + } + ``` + + ### JavaScript + ```javascript + console.log("hello"); + console.log("world"); + ``` [/details] ### Preview -
MD expect(tool.custom_raw.strip).to eq(expected.strip) end - - it "can correctly handle partial updates" do - tool = described_class.new({}, bot_user: bot_user, llm: llm) - - tool.parameters = { css: "a { }" } - tool.partial_invoke - - expect(tool.custom_raw).to eq("### CSS\n\n```css\na { }\n```") - - tool.parameters = { css: "a { }", html_body: "hello" } - tool.partial_invoke - - expect(tool.custom_raw).to eq( - "### CSS\n\n```css\na { }\n```\n\n### HTML\n\n```html\nhello\n```", - ) - - tool.parameters = { css: "a { }", html_body: "hello world" } - tool.partial_invoke - - expect(tool.custom_raw).to eq( - "### CSS\n\n```css\na { }\n```\n\n### HTML\n\n```html\nhello world\n```", - ) - end end end diff --git a/spec/lib/modules/ai_bot/tools/read_artifact_spec.rb b/spec/lib/modules/ai_bot/tools/read_artifact_spec.rb new file mode 100644 index 00000000..252e7eec --- /dev/null +++ b/spec/lib/modules/ai_bot/tools/read_artifact_spec.rb @@ -0,0 +1,143 @@ +# frozen_string_literal: true + +RSpec.describe DiscourseAi::AiBot::Tools::ReadArtifact do + fab!(:llm_model) + let(:bot_user) { DiscourseAi::AiBot::EntryPoint.find_user_from_model(llm_model.name) } + fab!(:post) + fab!(:post2) { Fabricate(:post, user: post.user) } + fab!(:artifact) do + AiArtifact.create!( + user: post.user, + post: post, + name: "Test Artifact", + html: "
Test Content
", + css: ".test { color: blue; }", + js: "console.log('test');", + ) + end + + before { SiteSetting.ai_bot_enabled = true } + + describe "#invoke" do + it "successfully reads a local artifact" do + tool = + described_class.new( + { url: "#{Discourse.base_url}/discourse-ai/ai-bot/artifacts/#{artifact.id}" }, + bot_user: bot_user, + llm: llm_model.to_llm, + context: { + post_id: post2.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("success") + + new_artifact = AiArtifact.last + expect(new_artifact.html).to eq(artifact.html) + expect(new_artifact.css).to eq(artifact.css) + expect(new_artifact.js).to eq(artifact.js) + expect(new_artifact.metadata["cloned_from"]).to eq(artifact.id) + end + + it "handles invalid URLs" do + tool = + described_class.new( + { url: "invalid-url" }, + bot_user: bot_user, + llm: llm_model.to_llm, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("error") + expect(result[:error]).to eq("Invalid URL") + end + + it "handles non-existent artifacts" do + tool = + described_class.new( + { url: "#{Discourse.base_url}/discourse-ai/ai-bot/artifacts/99999" }, + bot_user: bot_user, + llm: llm_model.to_llm, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("error") + expect(result[:error]).to eq("Artifact not found") + end + + it "handles external web pages" do + stub_request(:get, "https://example.com").to_return(status: 200, body: <<~HTML) + + + + + +
+
External Content
+
+ + + + HTML + + stub_request(:get, "https://example.com/style.css").to_return( + status: 200, + body: ".external { color: red; }", + ) + + tool = + described_class.new( + { url: "https://example.com" }, + bot_user: bot_user, + llm: llm_model.to_llm, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("success") + + new_artifact = AiArtifact.last + expect(new_artifact.html).to include("
External Content
") + expect(new_artifact.css).to include(".external { color: red; }") + expect(new_artifact.js).to include("console.log('test');") + expect(new_artifact.metadata["imported_from"]).to eq("https://example.com") + end + + it "respects MAX_HTML_SIZE limit" do + large_content = "x" * (described_class::MAX_HTML_SIZE + 1000) + + stub_request(:get, "https://example.com").to_return(status: 200, body: <<~HTML) + + +
#{large_content}
+ + + HTML + + tool = + described_class.new( + { url: "https://example.com" }, + bot_user: bot_user, + llm: llm_model.to_llm, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("success") + + new_artifact = AiArtifact.last + expect(new_artifact.html.length).to be <= described_class::MAX_HTML_SIZE + end + end +end diff --git a/spec/lib/modules/ai_bot/tools/update_artifact_spec.rb b/spec/lib/modules/ai_bot/tools/update_artifact_spec.rb index e1683e34..17833a0b 100644 --- a/spec/lib/modules/ai_bot/tools/update_artifact_spec.rb +++ b/spec/lib/modules/ai_bot/tools/update_artifact_spec.rb @@ -1,108 +1,140 @@ -# spec/lib/modules/ai_bot/tools/update_artifact_spec.rb # frozen_string_literal: true RSpec.describe DiscourseAi::AiBot::Tools::UpdateArtifact do fab!(:llm_model) let(:bot_user) { DiscourseAi::AiBot::EntryPoint.find_user_from_model(llm_model.name) } - let(:llm) { DiscourseAi::Completions::Llm.proxy("custom:#{llm_model.id}") } fab!(:post) fab!(:artifact) do AiArtifact.create!( user: Fabricate(:user), post: post, name: "Test Artifact", - html: "
\nOriginal\n
", - css: "div {\n color: blue; \n}", - js: "console.log('hello');", + html: "
Original
", + css: ".test { color: blue; }", + js: "console.log('original');\nconsole.log('world');\nconsole.log('hello');", ) end before { SiteSetting.ai_bot_enabled = true } describe "#process" do - let(:html) { <<~DIFF } -
- Updated -
- DIFF - - let(:css) { <<~DIFF } - div { - color: red; - } - DIFF - - let(:js) { <<~DIFF } + it "correctly updates artifact using section markers" do + responses = [<<~TXT.strip] + [HTML] +
Updated
+ [/HTML] + [CSS] + .test { color: red; } + [/CSS] + [JavaScript] + console.log('updated'); console.log('world'); - DIFF + console.log('updated2'); + [/JavaScript] + TXT - it "updates artifact content when supplied" do - tool = - described_class.new( - { - artifact_id: artifact.id, - html: html, - css: css, - js: js, - change_description: "Updated colors and text", - }, - bot_user: bot_user, - llm: llm, - context: { - post_id: post.id, - }, - ) + tool = nil - result = tool.invoke {} + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do + tool = + described_class.new( + { + artifact_id: artifact.id, + instructions: "Change the text to Updated and color to red", + }, + bot_user: bot_user, + llm: llm_model.to_llm, + persona_options: { + "update_algorithm" => "full", + }, + context: { + post_id: post.id, + }, + ) - expect(result[:status]).to eq("success") - expect(result[:version]).to eq(1) + result = tool.invoke {} + expect(result[:status]).to eq("success") + end - version = artifact.versions.find_by(version_number: 1) - expect(version.html).to include("Updated") - expect(version.css).to include("color: red") - expect(version.js).to include("'world'") - expect(artifact.versions.count).to eq(1) - expect(version.change_description).to eq("Updated colors and text") + version = artifact.versions.order(:version_number).last + expect(version.html).to eq("
Updated
") + expect(version.css).to eq(".test { color: red; }") + expect(version.js).to eq(<<~JS.strip) + console.log('updated'); + console.log('world'); + console.log('updated2'); + JS - # updating again should update the correct version - tool.parameters = { - artifact_id: artifact.id, - html: nil, - css: nil, - js: "updated", - change_description: "Updated colors and text again", - } - - result = tool.invoke {} - - version = artifact.versions.find_by(version_number: 2) - - expect(result[:status]).to eq("success") - expect(result[:version]).to eq(2) - - expect(version.html).to include("Updated") - expect(version.css).to include("color: red") - expect(version.js).to include("updated") + expect(tool.custom_raw).to include("Change Description") + expect(tool.custom_raw).to include("[details='View Changes']") + expect(tool.custom_raw).to include("### HTML Changes") + expect(tool.custom_raw).to include("### CSS Changes") + expect(tool.custom_raw).to include("### JS Changes") + expect(tool.custom_raw).to include("
"full", + }, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("success") + end + + version = artifact.versions.order(:version_number).last + expect(version.html).to eq("
Original
") + expect(version.css).to eq(".test { color: blue; }") + expect(version.js).to eq( + "console.log('updated');\nconsole.log('world');\nconsole.log('hello');", + ) + end + + it "handles invalid section format" do + responses = ["Invalid format without proper section markers"] + + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do + tool = + described_class.new( + { artifact_id: artifact.id, instructions: "Invalid update" }, + bot_user: bot_user, + llm: llm_model.to_llm, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("success") # The strategy will just keep original content + end end it "handles invalid artifact ID" do tool = described_class.new( - { artifact_id: -1, html: html, change_description: "Test change" }, + { artifact_id: -1, instructions: "Update something" }, bot_user: bot_user, - llm: llm, + llm: llm_model.to_llm, context: { post_id: post.id, }, @@ -113,36 +145,233 @@ RSpec.describe DiscourseAi::AiBot::Tools::UpdateArtifact do expect(result[:error]).to eq("Artifact not found") end - it "requires at least one change" do + it "preserves unchanged sections in the diff output" do + responses = [<<~TXT.strip] + [HTML] +
Updated
+ [/HTML] + TXT + + tool = nil + + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do + tool = + described_class.new( + { artifact_id: artifact.id, instructions: "Just update the HTML" }, + bot_user: bot_user, + llm: llm_model.to_llm, + persona_options: { + "update_algorithm" => "full", + }, + context: { + post_id: post.id, + }, + ) + + tool.invoke {} + end + + version = artifact.versions.order(:version_number).last + expect(version.css).to eq(artifact.css) + expect(version.js).to eq(artifact.js) + expect(tool.custom_raw).to include("### HTML Changes") + expect(tool.custom_raw).not_to include("### CSS Changes") + expect(tool.custom_raw).not_to include("### JavaScript Changes") + end + + it "handles updates to specific versions" do + # Create first version + responses = [<<~TXT.strip] + [HTML] +
Version 1
+ [/HTML] + TXT + + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do + described_class + .new( + { artifact_id: artifact.id, instructions: "Update to version 1" }, + bot_user: bot_user, + llm: llm_model.to_llm, + persona_options: { + "update_algorithm" => "full", + }, + context: { + post_id: post.id, + }, + ) + .invoke {} + end + + first_version = artifact.versions.order(:version_number).last + + responses = [<<~TXT.strip] + [HTML] +
Updated from version 1
+ [/HTML] + TXT + + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do + tool = + described_class.new( + { + artifact_id: artifact.id, + version: first_version.version_number, + instructions: "Update from version 1", + }, + bot_user: bot_user, + llm: llm_model.to_llm, + persona_options: { + "update_algorithm" => "full", + }, + context: { + post_id: post.id, + }, + ) + + result = tool.invoke {} + expect(result[:status]).to eq("success") + end + + latest_version = artifact.versions.order(:version_number).last + expect(latest_version.html).to eq("
Updated from version 1
") + end + end + + it "correctly updates artifact using diff strategy (partial diff)" do + responses = [<<~TXT.strip] + + [HTML] + nonsense + <<<<<<< SEARCH +
Original
+ ======= +
Updated
+ >>>>>>> REPLACE + garbage llm injects + [/HTML] + + [CSS] + garbage llm injects + <<<<<<< SEARCH + .test { color: blue; } + ======= + .test { color: red; } + >>>>>>> REPLACE + nonsense + [/CSS] + + [JavaScript] + nothing to do + [/JavaScript] + + LLMs like to say nonsense that we can ignore here as well + TXT + + tool = nil + + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do tool = described_class.new( - { artifact_id: artifact.id, change_description: "No changes" }, + { artifact_id: artifact.id, instructions: "Change the text to Updated and color to red" }, bot_user: bot_user, - llm: llm, + llm: llm_model.to_llm, context: { post_id: post.id, }, + persona_options: { + "update_algorithm" => "diff", + }, ) result = tool.invoke {} expect(result[:status]).to eq("success") - expect(artifact.versions.count).to eq(1) end - it "correctly renders changes in message" do + version = artifact.versions.order(:version_number).last + expect(version.html).to eq("
Updated
") + expect(version.css).to eq(".test { color: red; }") + expect(version.js).to eq(<<~JS.strip) + console.log('original'); + console.log('world'); + console.log('hello'); + JS + + expect(tool.custom_raw).to include("Change Description") + expect(tool.custom_raw).to include("[details='View Changes']") + expect(tool.custom_raw).to include("### HTML Changes") + expect(tool.custom_raw).to include("### CSS Changes") + expect(tool.custom_raw).to include("
Original
+ ======= +
Updated
+ >>>>>>> REPLACE + [/HTML] + + [CSS] + <<<<<<< SEARCH + .test { color: blue; } + ======= + .test { color: red; } + >>>>>>> REPLACE + [/CSS] + + [JavaScript] + <<<<<<< SEARCH + console.log('original'); + console.log('world'); + console.log('hello'); + ======= + console.log('updated'); + console.log('world'); + console.log('updated sam'); + >>>>>>> REPLACE + [/JavaScript] + + LLMs like to say nonsense that we can ignore here + TXT + + tool = nil + + DiscourseAi::Completions::Llm.with_prepared_responses(responses) do tool = described_class.new( - { artifact_id: artifact.id, html: html, change_description: "Updated content" }, + { artifact_id: artifact.id, instructions: "Change the text to Updated and color to red" }, bot_user: bot_user, - llm: llm, + llm: llm_model.to_llm, context: { post_id: post.id, }, + persona_options: { + "update_algorithm" => "diff", + }, ) - tool.invoke {} - - expect(tool.custom_raw.strip).to include(html.strip) + result = tool.invoke {} + expect(result[:status]).to eq("success") end + + version = artifact.versions.order(:version_number).last + expect(version.html).to eq("
Updated
") + expect(version.css).to eq(".test { color: red; }") + expect(version.js).to eq(<<~JS.strip) + console.log('updated'); + console.log('world'); + console.log('updated sam'); + JS + + expect(tool.custom_raw).to include("Change Description") + expect(tool.custom_raw).to include("[details='View Changes']") + expect(tool.custom_raw).to include("### HTML Changes") + expect(tool.custom_raw).to include("### CSS Changes") + expect(tool.custom_raw).to include("### JS Changes") + expect(tool.custom_raw).to include("
"abc" }], ["Time"]] + system_persona.update!(tools: tools) + + system_persona.reload + expect(system_persona.tools).to eq(tools) + + invalid_tools = ["Time"] + system_persona.update(tools: invalid_tools) + expect(system_persona.errors[:base]).to include( + I18n.t("discourse_ai.ai_bot.personas.cannot_edit_system_persona"), + ) + end + end + end end