From c2e96d1e190b57e82b40d7bbd253673e0b5352f7 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Mon, 27 Jan 2025 08:59:17 +1100 Subject: [PATCH 01/19] work in progress move artifact creation to background llm job --- config/locales/server.en.yml | 2 +- lib/ai_bot/tools/create_artifact.rb | 275 ++++++++++------- lib/ai_bot/tools/update_artifact.rb | 284 +++++++++++------- lib/completions/endpoints/base.rb | 5 +- .../ai_bot/tools/create_artifact_spec.rb | 88 +++--- 5 files changed, 404 insertions(+), 250 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index e490ef24c..db8a6d815 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -359,7 +359,7 @@ en: 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" + 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/lib/ai_bot/tools/create_artifact.rb b/lib/ai_bot/tools/create_artifact.rb index 627131d4e..0fc098e1b 100644 --- a/lib/ai_bot/tools/create_artifact.rb +++ b/lib/ai_bot/tools/create_artifact.rb @@ -8,38 +8,47 @@ def self.name "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 - - To include them ensure they are the last tag in your HTML body. - Example: - TIP - end - - def self.js_script_tag_tip - <<~TIP - if you need a custom script tag, you can use the following format: - - - - If you only need a regular script tag, you can use the following format: - - // your script here - TIP + def self.specification_description + <<~DESC + A detailed description of the web artifact you want to create. Your specification should include: + + 1. Purpose and functionality + 2. Visual design requirements + 3. Interactive elements and behavior + 4. Data handling (if applicable) + 5. Specific requirements or constraints + + 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" + + Poor specification example: + "Make a website that looks nice and does cool stuff" + (Too vague, lacks specific requirements and functionality details) + + 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 + 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,58 +57,25 @@ def self.signature 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 + + + 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 From dab4f886c0a76cde3256702a98209945bfa4f761 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Sat, 1 Feb 2025 10:20:08 +1100 Subject: [PATCH 18/19] debugging code, will remove --- lib/ai_bot/artifact_update_strategies/diff.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/ai_bot/artifact_update_strategies/diff.rb b/lib/ai_bot/artifact_update_strategies/diff.rb index ecc072773..efc60669c 100644 --- a/lib/ai_bot/artifact_update_strategies/diff.rb +++ b/lib/ai_bot/artifact_update_strategies/diff.rb @@ -50,12 +50,19 @@ def apply_changes(changes) content = source.public_send(section == :javascript ? :js : section) blocks.each do |block| - content = - DiscourseAi::Utils::DiffUtils::SimpleDiff.apply( - content, - block[:search], - block[:replace], - ) + begin + content = + DiscourseAi::Utils::DiffUtils::SimpleDiff.apply( + content, + block[:search], + block[:replace], + ) + rescue StandardError => e + File.write("/tmp/x/content", content) + File.write("/tmp/x/search", block[:search]) + File.write("/tmp/x/replace", block[:replace]) + raise e + end end updated_content[section == :javascript ? :js : section] = content end From 59a499f961dc1a4dfd96476ef36ff5605cb50b10 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Sat, 1 Feb 2025 16:19:16 +1100 Subject: [PATCH 19/19] make it much more forgiving. still need to decide about "no match" problems --- config/locales/server.en.yml | 6 +- lib/ai_bot/artifact_update_strategies/diff.rb | 3 + lib/utils/diff_utils/simple_diff.rb | 114 ++++++------------ spec/lib/utils/diff_utils/simple_diff_spec.rb | 31 +++-- 4 files changed, 64 insertions(+), 90 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 3e5e351c5..2cac7fac3 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -298,9 +298,6 @@ en: creator_llm: name: "LLM" description: "Language model to use for artifact creation" - echo_artifact: - name: "Echo Artifact" - description: "Include full artifact source in subsequent replies" update_artifact: editor_llm: name: "LLM" @@ -308,6 +305,9 @@ en: update_algorithm: name: "Update Algorithm" description: "Ask LLM to fully replace, or use diff to update" + echo_artifact: + name: "Echo Artifact" + description: "Include full artifact source in subsequent replies" google: base_query: name: "Base Search Query" diff --git a/lib/ai_bot/artifact_update_strategies/diff.rb b/lib/ai_bot/artifact_update_strategies/diff.rb index efc60669c..5639bc2bf 100644 --- a/lib/ai_bot/artifact_update_strategies/diff.rb +++ b/lib/ai_bot/artifact_update_strategies/diff.rb @@ -49,6 +49,7 @@ def apply_changes(changes) next unless blocks content = source.public_send(section == :javascript ? :js : section) + original_content = content.dup blocks.each do |block| begin content = @@ -58,6 +59,8 @@ def apply_changes(changes) block[:replace], ) rescue StandardError => e + File.write("/tmp/x/original", original_content) + File.write("/tmp/x/blocks", blocks.inspect) File.write("/tmp/x/content", content) File.write("/tmp/x/search", block[:search]) File.write("/tmp/x/replace", block[:replace]) diff --git a/lib/utils/diff_utils/simple_diff.rb b/lib/utils/diff_utils/simple_diff.rb index 360279e16..29c6951ff 100644 --- a/lib/utils/diff_utils/simple_diff.rb +++ b/lib/utils/diff_utils/simple_diff.rb @@ -8,8 +8,6 @@ class SimpleDiff class Error < StandardError end - class AmbiguousMatchError < Error - end class NoMatchError < Error end @@ -23,111 +21,73 @@ def apply(content, search, replace) raise ArgumentError, "replace cannot be nil" if replace.nil? lines = content.split("\n") - result = [] - i = 0 - found_match = false - match_positions = find_possible_matches(lines, search) + search_lines = search.split("\n") + # First try exact matching + match_positions = + find_matches(lines, search_lines) { |line, search_line| line == search_line } + + # sripped match if match_positions.empty? - raise NoMatchError, "Could not find a match for the search content" + match_positions = + find_matches(lines, search_lines) do |line, search_line| + line.strip == search_line.strip + end end - if match_positions.length > 1 - raise AmbiguousMatchError, - "Found multiple different potential matches for the search content" + # Fallback to fuzzy matching if no exact matches found + if match_positions.empty? + match_positions = + find_matches(lines, search_lines) do |line, search_line| + fuzzy_match?(line, search_line) + end end - while i < lines.length - if match_positions.include?(i) - found_match = true - # Skip the matched lines and add replacement - i += search.split("\n").length - result.concat(replace.split("\n")) - else - result << lines[i] - i += 1 - end + if match_positions.empty? + raise NoMatchError, "Could not find a match for the search content" end - raise NoMatchError, "Failed to apply the replacement" unless found_match + # Replace every occurrence (process in descending order to avoid shifting indices) + match_positions.sort.reverse.each do |pos| + lines.slice!(pos, search_lines.length) + lines.insert(pos, *replace.split("\n")) + end - result.join("\n") + lines.join("\n") end private - def find_possible_matches(lines, search) - search_lines = search.split("\n") - - # First try exact matches - exact_matches = [] - (0..lines.length - search_lines.length).each do |i| - if lines[i].strip == search_lines.first.strip - # Check if all subsequent lines match exactly in sequence - if search_lines.each_with_index.all? { |search_line, idx| - lines[i + idx].strip == search_line.strip - } - exact_matches << i - end + 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 - return exact_matches if exact_matches.any? - - # Fall back to fuzzy matches if no exact matches found - fuzzy_matches = [] - (0..lines.length - search_lines.length).each do |i| - if fuzzy_match?(lines[i], search_lines.first, LEVENSHTEIN_THRESHOLD) - # Check if all subsequent lines match fuzzily in sequence - if search_lines.each_with_index.all? { |search_line, idx| - fuzzy_match?(lines[i + idx], search_line, LEVENSHTEIN_THRESHOLD) - } - fuzzy_matches << i - end - end - end - - fuzzy_matches - end - - def fuzzy_match?(line1, line2, threshold) - return true if line1.strip == line2.strip - - # Remove leading whitespace for comparison - s1 = line1.lstrip - s2 = line2.lstrip - - distance = levenshtein_distance(s1, s2) - distance <= threshold + matches end - def match_block?(lines, search) - search_lines = search.split("\n") - return false if lines.length < search_lines.length - - search_lines.each_with_index.all? do |search_line, idx| - fuzzy_match?(lines[idx], search_line, LEVENSHTEIN_THRESHOLD) - 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, # deletion - d[i][j - 1] + 1, # insertion - d[i - 1][j - 1] + cost, # substitution - ].min + 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 end diff --git a/spec/lib/utils/diff_utils/simple_diff_spec.rb b/spec/lib/utils/diff_utils/simple_diff_spec.rb index 1dbf8546a..029f011ba 100644 --- a/spec/lib/utils/diff_utils/simple_diff_spec.rb +++ b/spec/lib/utils/diff_utils/simple_diff_spec.rb @@ -10,23 +10,39 @@ expect { subject.apply("content", "search", nil) }.to raise_error(ArgumentError) end + it "prioritizes exact matches over all fuzzy matches" do + content = <<~TEXT + line 1 + line 1 + lin 1 + TEXT + + search = " line 1" + replace = " new_line" + expected = <<~TEXT + line 1 + new_line + lin 1 + TEXT + + expect(subject.apply(content, search, replace)).to eq(expected.strip) + end + it "raises error when no match is found" do content = "line1\ncompletely_different\nline3" search = "nothing_like_this" replace = "new_line" - expect { subject.apply(content, search, replace) }.to raise_error( DiscourseAi::Utils::DiffUtils::SimpleDiff::NoMatchError, ) end - it "raises error for ambiguous matches" do + it "replaces all matching occurrences" do content = "line1\nline2\nmiddle\nline2\nend" search = "line2" replace = "new_line2" - - expect { subject.apply(content, search, replace) }.to raise_error( - DiscourseAi::Utils::DiffUtils::SimpleDiff::AmbiguousMatchError, + expect(subject.apply(content, search, replace)).to eq( + "line1\nnew_line2\nmiddle\nnew_line2\nend", ) end @@ -34,7 +50,6 @@ content = "line1\nline2\nline3" search = "line2" replace = "new_line2" - expect(subject.apply(content, search, replace)).to eq("line1\nnew_line2\nline3") end @@ -42,7 +57,6 @@ content = "start\nline1\nline2\nend" search = "line1\nline2" replace = "new_line" - expect(subject.apply(content, search, replace)).to eq("start\nnew_line\nend") end @@ -50,7 +64,6 @@ content = "line1\n line2\nline3" search = "line2" replace = "new_line2" - expect(subject.apply(content, search, replace)).to eq("line1\nnew_line2\nline3") end @@ -58,7 +71,6 @@ content = "line one one one\nlin2\nline three three" # Notice 'lin2' instead of 'line2' search = "line2" replace = "new_line2" - expect(subject.apply(content, search, replace)).to eq( "line one one one\nnew_line2\nline three three", ) @@ -68,7 +80,6 @@ content = "def method\n line1\n line2\nend" search = "line1\nline2" replace = "new_content" - expect(subject.apply(content, search, replace)).to eq("def method\nnew_content\nend") end end