FIX: array arguments not parsed correctly (#405)

DALL E command accepts an Array as a tool argument, this was not
parsed correctly by the invoker leading to errors generating
images with DALL E

Side quest ... don't use update! it calls validations and will now
fail due to email validation

lib/ai_bot/personas/persona.rb

@@ -122,16 +122,22 @@ module DiscourseAi
           tool_klass = available_tools.find { |c| c.signature.dig(:name) == function_name }
           return false if tool_klass.nil?
 
-          arguments =
+          arguments = {}
-            tool_klass.signature[:parameters]
+          tool_klass.signature[:parameters].to_a.each do |param|
-              .to_a
+            name = param[:name]
-              .reduce({}) do |memo, p|
+            value = parsed_function.at(name)&.text
-                argument = parsed_function.at(p[:name])&.text
-                next(memo) unless argument
 
-                memo[p[:name].to_sym] = argument
+            if param[:type] == "array" && value
-                memo
+              value =
-              end
+                begin
+                  JSON.parse(value)
+                rescue JSON::ParserError
+                  nil
+                end
+            end
+
+            arguments[name.to_sym] = value if value
+          end
 
           tool_klass.new(
             arguments,

lib/ai_bot/site_settings_extension.rb

@@ -24,14 +24,14 @@ module DiscourseAi::AiBot::SiteSettingsExtension
             )
           user.save!(validate: false)
         else
-          user.update!(active: true)
+          user.update_columns(active: true)
         end
       elsif !active && user
         # will include deleted
         has_posts = DB.query_single("SELECT 1 FROM posts WHERE user_id = #{id} LIMIT 1").present?
 
         if has_posts
-          user.update!(active: false) if user.active
+          user.update_columns(active: false) if user.active
         else
           user.destroy
         end

lib/ai_bot/tools/dall_e.rb

@@ -99,17 +99,17 @@ module DiscourseAi
           end
 
           self.custom_raw = <<~RAW
-    
+
-          [grid]
+            [grid]
-          #{
+            #{
             uploads
               .map do |item|
                 "![#{item[:prompt].gsub(/\|\'\"/, "")}|512x512, 50%](#{item[:upload].short_url})"
               end
               .join(" ")
           }
-          [/grid]
+            [/grid]
-        RAW
+          RAW
 
           { prompts: uploads.map { |item| item[:prompt] } }
         end

spec/lib/modules/ai_bot/personas/persona_spec.rb

@@ -20,131 +20,157 @@ class TestPersona < DiscourseAi::AiBot::Personas::Persona
   end
 end
 
-module DiscourseAi::AiBot::Personas
+RSpec.describe DiscourseAi::AiBot::Personas::Persona do
-  RSpec.describe Persona do
+  let :persona do
-    let :persona do
+    TestPersona.new
-      TestPersona.new
+  end
-    end
 
-    let :topic_with_users do
+  let :topic_with_users do
-      topic = Topic.new
+    topic = Topic.new
-      topic.allowed_users = [User.new(username: "joe"), User.new(username: "jane")]
+    topic.allowed_users = [User.new(username: "joe"), User.new(username: "jane")]
-      topic
+    topic
-    end
+  end
 
-    after do
+  after do
-      # we are rolling back transactions so we can create poison cache
+    # we are rolling back transactions so we can create poison cache
-      AiPersona.persona_cache.flush!
+    AiPersona.persona_cache.flush!
-    end
+  end
 
-    let(:context) do
+  let(:context) do
-      {
+    {
-        site_url: Discourse.base_url,
+      site_url: Discourse.base_url,
-        site_title: "test site title",
+      site_title: "test site title",
-        site_description: "test site description",
+      site_description: "test site description",
-        time: Time.zone.now,
+      time: Time.zone.now,
-        participants: topic_with_users.allowed_users.map(&:username).join(", "),
+      participants: topic_with_users.allowed_users.map(&:username).join(", "),
-      }
+    }
-    end
+  end
 
-    fab!(:user)
+  fab!(:user)
 
-    it "renders the system prompt" do
+  it "renders the system prompt" do
-      freeze_time
+    freeze_time
 
-      rendered = persona.craft_prompt(context)
+    rendered = persona.craft_prompt(context)
 
-      expect(rendered[:insts]).to include(Discourse.base_url)
+    expect(rendered[:insts]).to include(Discourse.base_url)
-      expect(rendered[:insts]).to include("test site title")
+    expect(rendered[:insts]).to include("test site title")
-      expect(rendered[:insts]).to include("test site description")
+    expect(rendered[:insts]).to include("test site description")
-      expect(rendered[:insts]).to include("joe, jane")
+    expect(rendered[:insts]).to include("joe, jane")
-      expect(rendered[:insts]).to include(Time.zone.now.to_s)
+    expect(rendered[:insts]).to include(Time.zone.now.to_s)
 
-      tools = rendered[:tools]
+    tools = rendered[:tools]
 
-      expect(tools.find { |t| t[:name] == "search" }).to be_present
+    expect(tools.find { |t| t[:name] == "search" }).to be_present
-      expect(tools.find { |t| t[:name] == "tags" }).to be_present
+    expect(tools.find { |t| t[:name] == "tags" }).to be_present
 
-      # needs to be configured so it is not available
+    # needs to be configured so it is not available
-      expect(tools.find { |t| t[:name] == "image" }).to be_nil
+    expect(tools.find { |t| t[:name] == "image" }).to be_nil
-    end
+  end
 
-    describe "custom personas" do
+  it "can correctly parse arrays in tools" do
-      it "is able to find custom personas" do
+    SiteSetting.ai_openai_api_key = "123"
-        Group.refresh_automatic_groups!
 
-        # define an ai persona everyone can see
+    # Dall E tool uses an array for params
-        persona =
+    xml = <<~XML
-          AiPersona.create!(
+      <function_calls>
-            name: "zzzpun_bot",
+        <invoke>
-            description: "you write puns",
+        <tool_name>dall_e</tool_name>
-            system_prompt: "you are pun bot",
+        <tool_id>call_JtYQMful5QKqw97XFsHzPweB</tool_id>
-            commands: ["ImageCommand"],
+        <parameters>
-            allowed_group_ids: [Group::AUTO_GROUPS[:trust_level_0]],
+        <prompts>["cat oil painting", "big car"]</prompts>
-          )
+        </parameters>
+        </invoke>
+      </function_calls>
+    XML
+    dall_e = DiscourseAi::AiBot::Personas::DallE3.new.find_tool(xml)
+    expect(dall_e.parameters[:prompts]).to eq(["cat oil painting", "big car"])
+  end
 
-        custom_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
+  describe "custom personas" do
-        expect(custom_persona.name).to eq("zzzpun_bot")
+    it "is able to find custom personas" do
-        expect(custom_persona.description).to eq("you write puns")
+      Group.refresh_automatic_groups!
 
-        instance = custom_persona.new
+      # define an ai persona everyone can see
-        expect(instance.tools).to eq([DiscourseAi::AiBot::Tools::Image])
+      persona =
-        expect(instance.craft_prompt(context).dig(:insts)).to eq("you are pun bot\n\n")
+        AiPersona.create!(
-
+          name: "zzzpun_bot",
-        # should update
+          description: "you write puns",
-        persona.update!(name: "zzzpun_bot2")
+          system_prompt: "you are pun bot",
-        custom_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
+          commands: ["ImageCommand"],
-        expect(custom_persona.name).to eq("zzzpun_bot2")
+          allowed_group_ids: [Group::AUTO_GROUPS[:trust_level_0]],
-
-        # can be disabled
-        persona.update!(enabled: false)
-        last_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
-        expect(last_persona.name).not_to eq("zzzpun_bot2")
-
-        persona.update!(enabled: true)
-        # no groups have access
-        persona.update!(allowed_group_ids: [])
-
-        last_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
-        expect(last_persona.name).not_to eq("zzzpun_bot2")
-      end
-    end
-
-    describe "available personas" do
-      it "includes all personas by default" do
-        Group.refresh_automatic_groups!
-
-        # must be enabled to see it
-        SiteSetting.ai_stability_api_key = "abc"
-        SiteSetting.ai_google_custom_search_api_key = "abc"
-        SiteSetting.ai_google_custom_search_cx = "abc123"
-
-        # should be ordered by priority and then alpha
-        expect(DiscourseAi::AiBot::Personas::Persona.all(user: user)).to eq(
-          [General, Artist, Creative, Researcher, SettingsExplorer, SqlHelper],
         )
 
-        # omits personas if key is missing
+      custom_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
-        SiteSetting.ai_stability_api_key = ""
+      expect(custom_persona.name).to eq("zzzpun_bot")
-        SiteSetting.ai_google_custom_search_api_key = ""
+      expect(custom_persona.description).to eq("you write puns")
 
-        expect(DiscourseAi::AiBot::Personas::Persona.all(user: user)).to contain_exactly(
+      instance = custom_persona.new
-          General,
+      expect(instance.tools).to eq([DiscourseAi::AiBot::Tools::Image])
-          SqlHelper,
+      expect(instance.craft_prompt(context).dig(:insts)).to eq("you are pun bot\n\n")
-          SettingsExplorer,
-          Creative,
-        )
 
-        AiPersona.find(DiscourseAi::AiBot::Personas::Persona.system_personas[General]).update!(
+      # should update
-          enabled: false,
+      persona.update!(name: "zzzpun_bot2")
-        )
+      custom_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
+      expect(custom_persona.name).to eq("zzzpun_bot2")
 
-        expect(DiscourseAi::AiBot::Personas::Persona.all(user: user)).to contain_exactly(
+      # can be disabled
-          SqlHelper,
+      persona.update!(enabled: false)
-          SettingsExplorer,
+      last_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
-          Creative,
+      expect(last_persona.name).not_to eq("zzzpun_bot2")
-        )
+
-      end
+      persona.update!(enabled: true)
+      # no groups have access
+      persona.update!(allowed_group_ids: [])
+
+      last_persona = DiscourseAi::AiBot::Personas::Persona.all(user: user).last
+      expect(last_persona.name).not_to eq("zzzpun_bot2")
+    end
+  end
+
+  describe "available personas" do
+    it "includes all personas by default" do
+      Group.refresh_automatic_groups!
+
+      # must be enabled to see it
+      SiteSetting.ai_stability_api_key = "abc"
+      SiteSetting.ai_google_custom_search_api_key = "abc"
+      SiteSetting.ai_google_custom_search_cx = "abc123"
+
+      # should be ordered by priority and then alpha
+      expect(DiscourseAi::AiBot::Personas::Persona.all(user: user)).to eq(
+        [
+          DiscourseAi::AiBot::Personas::General,
+          DiscourseAi::AiBot::Personas::Artist,
+          DiscourseAi::AiBot::Personas::Creative,
+          DiscourseAi::AiBot::Personas::Researcher,
+          DiscourseAi::AiBot::Personas::SettingsExplorer,
+          DiscourseAi::AiBot::Personas::SqlHelper,
+        ],
+      )
+
+      # omits personas if key is missing
+      SiteSetting.ai_stability_api_key = ""
+      SiteSetting.ai_google_custom_search_api_key = ""
+
+      expect(DiscourseAi::AiBot::Personas::Persona.all(user: user)).to contain_exactly(
+        DiscourseAi::AiBot::Personas::General,
+        DiscourseAi::AiBot::Personas::SqlHelper,
+        DiscourseAi::AiBot::Personas::SettingsExplorer,
+        DiscourseAi::AiBot::Personas::Creative,
+      )
+
+      AiPersona.find(
+        DiscourseAi::AiBot::Personas::Persona.system_personas[
+          DiscourseAi::AiBot::Personas::General
+        ],
+      ).update!(enabled: false)
+
+      expect(DiscourseAi::AiBot::Personas::Persona.all(user: user)).to contain_exactly(
+        DiscourseAi::AiBot::Personas::SqlHelper,
+        DiscourseAi::AiBot::Personas::SettingsExplorer,
+        DiscourseAi::AiBot::Personas::Creative,
+      )
     end
   end
 end