linting and basic functioning version

SamSaffron · SamSaffron · commit 8c30b7c40782 · 2024-12-01T17:21:11.000+11:00
even though it does not perform fancy edits it at least works
consistently
diff --git a/app/controllers/discourse_ai/ai_bot/artifacts_controller.rb b/app/controllers/discourse_ai/ai_bot/artifacts_controller.rb
@@ -19,13 +19,20 @@ def show
           raise Discourse::NotFound if !guardian.can_see?(post)
         end
 
+        name = artifact.name
+
+        if params[:version].present?
+          artifact = artifact.versions.find_by(version_number: params[:version])
+          raise Discourse::NotFound if !artifact
+        end
+
         # Prepare the inner (untrusted) HTML document
         untrusted_html = <<~HTML
           <!DOCTYPE html>
           <html>
             <head>
               <meta charset="UTF-8">
-              <title>#{ERB::Util.html_escape(artifact.name)}</title>
+              <title>#{ERB::Util.html_escape(name)}</title>
               <style>
                 #{artifact.css}
               </style>
@@ -45,7 +52,7 @@ def show
           <html>
             <head>
               <meta charset="UTF-8">
-              <title>#{ERB::Util.html_escape(artifact.name)}</title>
+              <title>#{ERB::Util.html_escape(name)}</title>
               <style>
                 html, body, iframe {
                   margin: 0;
diff --git a/app/models/ai_artifact.rb b/app/models/ai_artifact.rb
@@ -8,17 +8,22 @@ class AiArtifact < ActiveRecord::Base
   validates :css, length: { maximum: 65_535 }
   validates :js, length: { maximum: 65_535 }
 
-  def self.iframe_for(id)
+  def self.iframe_for(id, version = nil)
     <<~HTML
       <div class='ai-artifact'>
-        <iframe src='#{url(id)}' frameborder="0" height="100%" width="100%"></iframe>
-        <a href='#{url(id)}' target='_blank'>#{I18n.t("discourse_ai.ai_artifact.link")}</a>
+        <iframe src='#{url(id, version)}' frameborder="0" height="100%" width="100%"></iframe>
+        <a href='#{url(id, version)}' target='_blank'>#{I18n.t("discourse_ai.ai_artifact.link")}</a>
       </div>
     HTML
   end
 
-  def self.url(id)
-    Discourse.base_url + "/discourse-ai/ai-bot/artifacts/#{id}"
+  def self.url(id, version = nil)
+    url = Discourse.base_url + "/discourse-ai/ai-bot/artifacts/#{id}"
+    if version
+      "#{url}/#{version}"
+    else
+      url
+    end
   end
 
   def self.share_publicly(id:, post:)
@@ -48,23 +53,22 @@ def apply_diff(html_diff: nil, css_diff: nil, js_diff: nil, change_description:
   def create_new_version(html: nil, css: nil, js: nil, change_description: nil)
     latest_version = versions.order(version_number: :desc).first
     new_version_number = latest_version ? latest_version.version_number + 1 : 1
+    version = nil
 
     transaction do
       # Create the version record
-      versions.create!(
-        version_number: new_version_number,
-        html: self.html,
-        css: self.css,
-        js: self.js,
-        change_description: change_description,
-      )
-
-      # Update the main artifact
-      self.html = html if html.present?
-      self.css = css if css.present?
-      self.js = js if js.present?
+      version =
+        versions.create!(
+          version_number: new_version_number,
+          html: html || self.html,
+          css: css || self.css,
+          js: js || self.js,
+          change_description: change_description,
+        )
       save!
     end
+
+    version
   end
 end
 
diff --git a/app/models/shared_ai_conversation.rb b/app/models/shared_ai_conversation.rb
@@ -190,9 +190,11 @@ def self.cook_artifacts(post)
       .css("div.ai-artifact")
       .each do |node|
         id = node["data-ai-artifact-id"].to_i
+        version = node["data-ai-artifact-version"]
+        version_number = version.to_i if version
         if id > 0
           AiArtifact.share_publicly(id: id, post: post)
-          node.replace(AiArtifact.iframe_for(id))
+          node.replace(AiArtifact.iframe_for(id, version_number))
         end
       end
 
diff --git a/assets/javascripts/discourse/components/ai-artifact.gjs b/assets/javascripts/discourse/components/ai-artifact.gjs
@@ -37,7 +37,13 @@ export default class AiArtifactComponent extends Component {
   }
 
   get artifactUrl() {
-    return getURL(`/discourse-ai/ai-bot/artifacts/${this.args.artifactId}`);
+    const url = getURL(`/discourse-ai/ai-bot/artifacts/${this.args.artifactId}`);
+
+    if (this.args.artifactVersion) {
+      return `${url}?version=${this.args.artifactVersion}`;
+    } else {
+      return url;
+    }
   }
 
   @action
diff --git a/assets/javascripts/initializers/ai-artifacts.gjs b/assets/javascripts/initializers/ai-artifacts.gjs
@@ -14,8 +14,15 @@ function initializeAiArtifacts(api) {
             "data-ai-artifact-id"
           );
 
+          const artifactVersion = artifactElement.getAttribute(
+            "data-ai-artifact-version"
+          );
+
           helper.renderGlimmer(artifactElement, <template>
-            <AiArtifact @artifactId={{artifactId}} />
+            <AiArtifact
+              @artifactId={{artifactId}}
+              @artifactVersion={{artifactVersion}}
+            />
           </template>);
         }
       );
diff --git a/assets/javascripts/lib/discourse-markdown/ai-tags.js b/assets/javascripts/lib/discourse-markdown/ai-tags.js
@@ -1,4 +1,8 @@
 export function setup(helper) {
   helper.allowList(["details[class=ai-quote]"]);
-  helper.allowList(["div[class=ai-artifact]", "div[data-ai-artifact-id]"]);
+  helper.allowList([
+    "div[class=ai-artifact]",
+    "div[data-ai-artifact-id]",
+    "div[data-ai-artifact-version]",
+  ]);
 }
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
@@ -310,6 +310,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:
+        update_artifact: "Update a web artifact"
         create_artifact: "Create web artifact"
         web_browser: "Browse Web"
         github_search_files: "GitHub search files"
@@ -332,6 +333,7 @@ en:
         search_meta_discourse: "Search Meta Discourse"
         javascript_evaluator: "Evaluate JavaScript"
       tool_help:
+        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"
         github_search_code: "Search for code in a GitHub repository"
@@ -354,6 +356,7 @@ en:
         search_meta_discourse: "Search Meta Discourse"
         javascript_evaluator: "Evaluate JavaScript"
       tool_description:
+        update_artifact: "Updated a web artifact using the AI Bot"
         create_artifact: "Created a web artifact using the AI Bot"
         web_browser: "Reading <a href='%{url}'>%{url}</a>"
         github_search_files: "Searched for '%{keywords}' in %{repo}/%{branch}"
diff --git a/config/routes.rb b/config/routes.rb
@@ -36,6 +36,7 @@
 
   scope module: :ai_bot, path: "/ai-bot/artifacts" do
     get "/:id" => "artifacts#show"
+    get "/:id/:version" => "artifacts#show"
   end
 
   scope module: :summarization, path: "/summarization", defaults: { format: :json } do
diff --git a/db/migrate/20241130003808_add_artifact_versions.rb b/db/migrate/20241130003808_add_artifact_versions.rb
@@ -4,14 +4,14 @@ def change
     create_table :ai_artifact_versions do |t|
       t.bigint :ai_artifact_id, null: false
       t.integer :version_number, null: false
-      t.string :html, limit: 65535
-      t.string :css, limit: 65535
-      t.string :js, limit: 65535
+      t.string :html, limit: 65_535
+      t.string :css, limit: 65_535
+      t.string :js, limit: 65_535
       t.jsonb :metadata
       t.string :change_description
       t.timestamps
 
-      t.index [:ai_artifact_id, :version_number], unique: true
+      t.index %i[ai_artifact_id version_number], unique: true
     end
   end
 end
diff --git a/lib/ai_bot/tools/update_artifact.rb b/lib/ai_bot/tools/update_artifact.rb
@@ -8,6 +8,7 @@ def self.name
           "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:
@@ -50,7 +51,7 @@ def self.signature
           {
             name: "update_artifact",
             description:
-              "Updates an existing web artifact with new HTML, CSS, or JavaScript content.\n#{unified_diff_tip}",
+              "Updates an existing web artifact with new HTML, CSS, or JavaScript content.",
             parameters: [
               {
                 name: "artifact_id",
@@ -59,18 +60,18 @@ def self.signature
                 required: true,
               },
               {
-                name: "html_diff",
-                description: "A unified diff of HTML changes to apply",
+                name: "html",
+                description: "(Optional) new HTML content for the artifact",
                 type: "string",
               },
               {
-                name: "css_diff",
-                description: "A unified diff of CSS changes to apply",
+                name: "css",
+                description: "(Optional) new CSS content for the artifact",
                 type: "string",
               },
               {
-                name: "js_diff",
-                description: "A unified diff of JavaScript changes to apply",
+                name: "js",
+                description: "(Optional) new JavaScript content for the artifact",
                 type: "string",
               },
               {
@@ -79,14 +80,6 @@ def self.signature
                 type: "string",
                 required: true,
               },
-              {
-                name: "diff_mode",
-                description:
-                  "How would you like to apply the diff? (replace, append or diff) default is diff",
-                type: "string",
-                required: false,
-                enum: %w[replace append diff],
-              },
             ],
           }
         end
@@ -100,7 +93,7 @@ def chain_next_response?
         end
 
         def partial_invoke
-          @selected_tab = :html_diff
+          @selected_tab = :html
           if @prev_parameters
             @selected_tab = parameters.keys.find { |k| @prev_parameters[k] != parameters[k] }
           end
@@ -122,31 +115,16 @@ def invoke
           end
 
           begin
-            if parameters[:diff_mode] == "diff"
-              artifact.apply_diff(
-                html_diff: parameters[:html_diff],
-                css_diff: parameters[:css_diff],
-                js_diff: parameters[:js_diff],
-                change_description: parameters[:change_description],
-              )
-            elsif parameters[:diff_mode] == "replace"
-              artifact.create_new_version(
-                html: parameters[:html_diff] || artifact.html,
-                css: parameters[:css_diff] || artifact.css,
-                js: parameters[:js_diff] || artifact.js,
-                change_description: parameters[:change_description],
-              )
-            else
+            version =
               artifact.create_new_version(
-                html: (artifact.html + "\n" + parameters[:html_diff].to_s).strip,
-                css: (artifact.css + "\n" + parameters[:css_diff].to_s).strip,
-                js: (artifact.js + "\n" + parameters[:js_diff].to_s).strip,
+                html: parameters[:html] || artifact.html,
+                css: parameters[:css] || artifact.css,
+                js: parameters[:js] || artifact.js,
                 change_description: parameters[:change_description],
               )
-            end
 
-            update_custom_html(artifact)
-            success_response(artifact)
+            update_custom_html(artifact, version)
+            success_response(artifact, version)
           rescue DiscourseAi::Utils::DiffUtils::DiffError => e
             error_response(e.to_llm_message)
           rescue => e
@@ -156,19 +134,19 @@ def invoke
 
         private
 
-        def update_custom_html(artifact = nil)
+        def update_custom_html(artifact = nil, version = nil)
           content = []
 
-          if parameters[:html_diff].present?
-            content << [:html_diff, "### HTML Changes\n\n```diff\n#{parameters[:html_diff]}\n```"]
+          if parameters[:html].present?
+            content << [:html, "### HTML Changes\n\n```html\n#{parameters[:html]}\n```"]
           end
 
-          if parameters[:css_diff].present?
-            content << [:css_diff, "### CSS Changes\n\n```diff\n#{parameters[:css_diff]}\n```"]
+          if parameters[:css].present?
+            content << [:css, "### CSS Changes\n\n```css\n#{parameters[:css]}\n```"]
           end
 
-          if parameters[:js_diff].present?
-            content << [:js_diff, "### JavaScript Changes\n\n```diff\n#{parameters[:js_diff]}\n```"]
+          if parameters[:js].present?
+            content << [:js, "### JavaScript Changes\n\n```javascript\n#{parameters[:js]}\n```"]
           end
 
           if parameters[:change_description].present?
@@ -184,7 +162,7 @@ def update_custom_html(artifact = nil)
             content << [nil, "[/details]"]
             content << [
               :preview,
-              "### Preview\n\n<div class=\"ai-artifact\" data-ai-artifact-id=\"#{artifact.id}\"></div>",
+              "### Preview\n\n<div class=\"ai-artifact\" data-ai-artifact-version=\"#{version.version_number}\" data-ai-artifact-id=\"#{artifact.id}\"></div>",
             ]
           end
 
@@ -193,28 +171,16 @@ def update_custom_html(artifact = nil)
           self.custom_raw = content.map { |c| c[1] }.join("\n\n")
         end
 
-        def success_response(artifact)
+        def success_response(artifact, version)
           @chain_next_response = false
 
           hash = {
             status: "success",
             artifact_id: artifact.id,
-            version: artifact.versions.last.version_number,
+            version: version.version_number,
             message: "Artifact updated successfully and rendered to user.",
           }
 
-          if parameters[:html_diff].present?
-            hash[:new_artifact_html] = artifact.html
-          end
-
-          if parameters[:css_diff].present?
-            hash[:new_artifact_css] = artifact.css
-          end
-
-          if parameters[:js_diff].present?
-            hash[:new_artifact_js] = artifact.js
-          end
-
           hash
         end
 
diff --git a/lib/utils/diff_utils.rb b/lib/utils/diff_utils.rb
diff --git a/spec/lib/modules/ai_bot/tools/update_artifact_spec.rb b/spec/lib/modules/ai_bot/tools/update_artifact_spec.rb
diff --git a/spec/lib/utils/diff_utils_spec.rb b/spec/lib/utils/diff_utils_spec.rb
diff --git a/spec/requests/ai_bot/artifacts_controller_spec.rb b/spec/requests/ai_bot/artifacts_controller_spec.rb

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,13 @@ export default class AiArtifactComponent extends Component {`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`get artifactUrl() {`
`40`		- return getURL(`/discourse-ai/ai-bot/artifacts/${this.args.artifactId}`);
	`40`	+ const url = getURL(`/discourse-ai/ai-bot/artifacts/${this.args.artifactId}`);
	`41`	`+`
	`42`	`+ if (this.args.artifactVersion) {`
	`43`	+ return `${url}?version=${this.args.artifactVersion}`;
	`44`	`+ } else {`
	`45`	`+ return url;`
	`46`	`+ }`
`41`	`47`	`}`
`42`	`48`
`43`	`49`	`@action`