testing and prompt engineering

SamSaffron · SamSaffron · commit 72559c76ab65 · 2025-02-19T14:32:57.000+11:00
diff --git a/app/models/ai_artifact_version.rb b/app/models/ai_artifact_version.rb
@@ -4,6 +4,19 @@ class AiArtifactVersion < ActiveRecord::Base
   validates :html, length: { maximum: 65_535 }
   validates :css, length: { maximum: 65_535 }
   validates :js, length: { maximum: 65_535 }
+
+  # used when generating test cases
+  def write_to(path)
+    css_path = "#{path}/main.css"
+    html_path = "#{path}/main.html"
+    js_path = "#{path}/main.js"
+    instructions_path = "#{path}/instructions.txt"
+
+    File.write(css_path, css)
+    File.write(html_path, html)
+    File.write(js_path, js)
+    File.write(instructions_path, change_description)
+  end
 end
 
 # == Schema Information
diff --git a/lib/ai_bot/artifact_update_strategies/diff.rb b/lib/ai_bot/artifact_update_strategies/diff.rb
@@ -58,12 +58,16 @@ def apply_changes(changes)
             content = source.public_send(section == :javascript ? :js : section)
             blocks.each do |block|
               begin
-                content =
-                  DiscourseAi::Utils::DiffUtils::SimpleDiff.apply(
-                    content,
-                    block[:search],
-                    block[:replace],
-                  )
+                if !block[:search]
+                  content = block[:replace]
+                else
+                  content =
+                    DiscourseAi::Utils::DiffUtils::SimpleDiff.apply(
+                      content,
+                      block[:search],
+                      block[:replace],
+                    )
+                end
               rescue DiscourseAi::Utils::DiffUtils::SimpleDiff::NoMatchError
                 @failed_searches << { section: section, search: block[:search] }
                 # TODO, we may need to inform caller here, LLM made a mistake which it
@@ -85,7 +89,8 @@ def apply_changes(changes)
         private
 
         def extract_search_replace_blocks(content)
-          return nil if content.blank?
+          return nil if content.blank? || content.to_s.strip.downcase.match?(/^\(?no changes?\)?$/m)
+          return [{ replace: content }] if !content.match?(/<<+\s*SEARCH/)
 
           blocks = []
           remaining = content
@@ -116,25 +121,26 @@ def system_prompt
             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. Keep changes minimal and focused
-            6. HTML should not include <html>, <head>, or <body> tags, it is injected into a template
-            7. When specifying a SEARCH block, ALWAYS keep it 8 lines or less, you will be interrupted and a retry will be required if you exceed this limit
-            8. NEVER EVER ask followup questions, ALL changes must be performed in a single response, you are consumed via an API, there is no opportunity for humans in the loop
-            9. When performing a non-contiguous search, ALWAYS use ... to denote the skipped lines
-            10. Be mindful that ... non-contiguous search is not greedy, the following line will only match the first occurrence of the search block
+            5. HTML should not include <html>, <head>, or <body> tags, it is injected into a template
+            6. When specifying a SEARCH block, ALWAYS keep it 8 lines or less, you will be interrupted and a retry will be required if you exceed this limit
+            7. NEVER EVER ask followup questions, ALL changes must be performed in a single response, you are consumed via an API, there is no opportunity for humans in the loop
+            8. When performing a non-contiguous search, ALWAYS use ... to denote the skipped lines
+            9. Be mindful that ... non-contiguous search is not greedy, the following line will only match the first occurrence of the search block
+            10. Never mix a full section replacement with a search/replace block in the same section
+            11. ALWAYS skip sections you to not want to change, do not include them in the response
 
             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)
+            (changes or empty if no changes or entire HTML)
             [/HTML]
             [CSS]
-            (changes or empty if no changes)
+            (changes or empty if no changes or entire CSS)
             [/CSS]
             [JavaScript]
-            (changes or empty if no changes)
+            (changes or empty if no changes or entire JavaScript)
             [/JavaScript]
 
             Example - Multiple changes in one file:
@@ -209,6 +215,25 @@ def system_prompt
             }
             [/CSS]
 
+            Example - full HTML replacement:
+
+            [HTML]
+            <div>something old</div>
+            <div>another somethin old</div>
+            [/HTML]
+
+            output:
+
+            [HTML]
+            <div>something new</div>
+            [/HTML]
+
+            result:
+            [HTML]
+            <div>something new</div>
+            [/HTML]
+
+
           PROMPT
         end
 
diff --git a/spec/fabricators/ai_artifact_fabricator.rb b/spec/fabricators/ai_artifact_fabricator.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+Fabricator(:ai_artifact) do
+  user
+  post
+  name { sequence(:name) { |i| "artifact_#{i}" } }
+  html { "<div>Test Content</div>" }
+  css { ".test { color: blue; }" }
+  js { "console.log('test');" }
+  metadata { { public: false } }
+end
+
+Fabricator(:ai_artifact_version) do
+  ai_artifact
+  version_number { sequence(:version_number) { |i| i } }
+  html { "<div>Version Content</div>" }
+  css { ".version { color: red; }" }
+  js { "console.log('version');" }
+  change_description { "Test change" }
+end
diff --git a/spec/lib/modules/ai_bot/artifact_update_strategies/diff_spec.rb b/spec/lib/modules/ai_bot/artifact_update_strategies/diff_spec.rb
@@ -0,0 +1,214 @@
+# frozen_string_literal: true
+
+RSpec.describe DiscourseAi::AiBot::ArtifactUpdateStrategies::Diff do
+  fab!(:user)
+  fab!(:post)
+  fab!(:artifact) { Fabricate(:ai_artifact) }
+  fab!(:llm_model)
+
+  let(:llm) { llm_model.to_llm }
+  let(:instructions) { "Update the button color to red" }
+
+  let(:strategy) do
+    described_class.new(
+      llm: llm,
+      post: post,
+      user: user,
+      artifact: artifact,
+      artifact_version: nil,
+      instructions: instructions,
+    )
+  end
+
+  describe "#apply" do
+    it "processes simple search/replace blocks" do
+      original_css = ".button { color: blue; }"
+      artifact.update!(css: original_css)
+
+      response = <<~RESPONSE
+        [CSS]
+        <<<<<<< SEARCH
+        .button { color: blue; }
+        =======
+        .button { color: red; }
+        >>>>>>> REPLACE
+        [/CSS]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      expect(artifact.versions.last.css).to eq(".button { color: red; }")
+    end
+
+    it "handles multiple search/replace blocks in the same section" do
+      original_css = <<~CSS
+        .button { color: blue; }
+        .text { font-size: 12px; }
+      CSS
+
+      artifact.update!(css: original_css)
+
+      response = <<~RESPONSE
+        [CSS]
+        <<<<<<< SEARCH
+        .button { color: blue; }
+        =======
+        .button { color: red; }
+        >>>>>>> REPLACE
+        <<<<<<< SEARCH
+        .text { font-size: 12px; }
+        =======
+        .text { font-size: 16px; }
+        >>>>>>> REPLACE
+        [/CSS]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      expected = <<~CSS.strip
+        .button { color: red; }
+        .text { font-size: 16px; }
+      CSS
+
+      expect(artifact.versions.last.css.strip).to eq(expected.strip)
+    end
+
+    it "handles non-contiguous search/replace using ..." do
+      original_css = <<~CSS
+        body {
+          color: red;
+        }
+        .button {
+          color: blue;
+        }
+        .alert {
+          background-color: green;
+        }
+      CSS
+
+      artifact.update!(css: original_css)
+
+      response = <<~RESPONSE
+        [CSS]
+        <<<<<<< SEARCH
+        body {
+        ...
+        background-color: green;
+        }
+        =======
+        body {
+          color: red;
+        }
+        >>>>>>> REPLACE
+        [/CSS]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      expect(artifact.versions.last.css).to eq("body {\n  color: red;\n}")
+    end
+
+    it "tracks failed searches" do
+      original_css = ".button { color: blue; }"
+      artifact.update!(css: original_css)
+
+      response = <<~RESPONSE
+        [CSS]
+        <<<<<<< SEARCH
+        .button { color: green; }
+        =======
+        .button { color: red; }
+        >>>>>>> REPLACE
+        [/CSS]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      expect(strategy.failed_searches).to contain_exactly(
+        { section: :css, search: ".button { color: green; }" },
+      )
+      expect(artifact.versions.last.css).to eq(original_css)
+    end
+
+    it "handles complete section replacements" do
+      original_html = "<div>old content</div>"
+      artifact.update!(html: original_html)
+
+      response = <<~RESPONSE
+        [HTML]
+        <div>new content</div>
+        [/HTML]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      expect(artifact.versions.last.html.strip).to eq("<div>new content</div>")
+    end
+
+    it "ignores empty or 'no changes' sections part 1" do
+      original = {
+        html: "<div>content</div>",
+        css: ".button { color: blue; }",
+        js: "console.log('test');",
+      }
+
+      artifact.update!(html: original[:html], css: original[:css], js: original[:js])
+
+      response = <<~RESPONSE
+        [HTML]
+        no changes
+        [/HTML]
+        [CSS]
+        (NO CHANGES)
+        [/CSS]
+        [JavaScript]
+        <<<<<<< SEARCH
+        console.log('test');
+        =======
+        console.log('(no changes)');
+        >>>>>>> REPLACE
+        [/JavaScript]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      version = artifact.versions.last
+      expect(version.html).to eq(original[:html])
+      expect(version.css).to eq(original[:css])
+      expect(version.js).to eq("console.log('(no changes)');")
+    end
+
+    it "ignores empty or 'no changes' section part 2" do
+      original = {
+        html: "<div>content</div>",
+        css: ".button { color: blue; }",
+        js: "console.log('test');",
+      }
+
+      artifact.update!(html: original[:html], css: original[:css], js: original[:js])
+
+      response = <<~RESPONSE
+        [HTML]
+        (no changes)
+        [/HTML]
+        [CSS]
+
+        [/CSS]
+        [JavaScript]
+        <<<<<<< SEARCH
+        console.log('test');
+        =======
+        console.log('updated');
+        >>>>>>> REPLACE
+        [/JavaScript]
+      RESPONSE
+
+      DiscourseAi::Completions::Llm.with_prepared_responses([response]) { strategy.apply }
+
+      version = artifact.versions.last
+      expect(version.html).to eq(original[:html])
+      expect(version.css).to eq(original[:css])
+      expect(version.js).to eq("console.log('updated');")
+    end
+  end
+end
