From 7edb57c005d84dd370c85282ca93214e03c419ee Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 4 Aug 2023 09:37:58 +1000 Subject: [PATCH] DEV: simplify command framework (#125) The command framework had some confusing dispatching where it would dispatch JSON blobs, this meant there was lots of parsing required in every command The refactor handles transforming the args prior to dispatch which makes consuming far simpler This is also general prep to supporting some basic command framework in other llms. --- lib/modules/ai_bot/commands/categories_command.rb | 2 +- lib/modules/ai_bot/commands/command.rb | 4 +++- lib/modules/ai_bot/commands/google_command.rb | 8 +++----- lib/modules/ai_bot/commands/image_command.rb | 4 ++-- lib/modules/ai_bot/commands/search_command.rb | 10 ++++------ lib/modules/ai_bot/commands/summarize_command.rb | 4 +--- lib/modules/ai_bot/commands/tags_command.rb | 2 +- lib/modules/ai_bot/commands/time_command.rb | 6 ++---- spec/lib/modules/ai_bot/bot_spec.rb | 4 ++-- .../ai_bot/commands/categories_command_spec.rb | 2 +- .../modules/ai_bot/commands/google_command_spec.rb | 2 +- .../modules/ai_bot/commands/search_command_spec.rb | 11 ++++++----- .../modules/ai_bot/commands/summarize_command_spec.rb | 4 ++-- spec/lib/modules/ai_bot/commands/tags_command_spec.rb | 2 +- spec/lib/modules/ai_bot/commands/time_command_spec.rb | 4 ++-- 15 files changed, 32 insertions(+), 37 deletions(-) diff --git a/lib/modules/ai_bot/commands/categories_command.rb b/lib/modules/ai_bot/commands/categories_command.rb index 7909a2b7..c796b6d3 100644 --- a/lib/modules/ai_bot/commands/categories_command.rb +++ b/lib/modules/ai_bot/commands/categories_command.rb @@ -24,7 +24,7 @@ module DiscourseAi::AiBot::Commands { count: @last_count || 0 } end - def process(_args) + def process columns = { name: "Name", slug: "Slug", diff --git a/lib/modules/ai_bot/commands/command.rb b/lib/modules/ai_bot/commands/command.rb index 916238a9..a804d9bf 100644 --- a/lib/modules/ai_bot/commands/command.rb +++ b/lib/modules/ai_bot/commands/command.rb @@ -106,7 +106,9 @@ module DiscourseAi post.post_custom_prompt ||= post.build_post_custom_prompt(custom_prompt: []) prompt = post.post_custom_prompt.custom_prompt || [] - prompt << [process(args).to_json, self.class.name, "function"] + parsed_args = JSON.parse(args).symbolize_keys + + prompt << [process(**parsed_args).to_json, self.class.name, "function"] post.post_custom_prompt.update!(custom_prompt: prompt) raw = +(<<~HTML) diff --git a/lib/modules/ai_bot/commands/google_command.rb b/lib/modules/ai_bot/commands/google_command.rb index 697b6c80..ce9daad2 100644 --- a/lib/modules/ai_bot/commands/google_command.rb +++ b/lib/modules/ai_bot/commands/google_command.rb @@ -39,13 +39,11 @@ module DiscourseAi::AiBot::Commands } end - def process(search_string) - search_string = JSON.parse(search_string)["query"] - - @last_query = search_string + def process(query:) + @last_query = query api_key = SiteSetting.ai_google_custom_search_api_key cx = SiteSetting.ai_google_custom_search_cx - query = CGI.escape(search_string) + query = CGI.escape(query) uri = URI("https://www.googleapis.com/customsearch/v1?key=#{api_key}&cx=#{cx}&q=#{query}&num=10") body = Net::HTTP.get(uri) diff --git a/lib/modules/ai_bot/commands/image_command.rb b/lib/modules/ai_bot/commands/image_command.rb index b1be713f..9439f0b5 100644 --- a/lib/modules/ai_bot/commands/image_command.rb +++ b/lib/modules/ai_bot/commands/image_command.rb @@ -43,8 +43,8 @@ module DiscourseAi::AiBot::Commands true end - def process(prompt) - @last_prompt = prompt = JSON.parse(prompt)["prompt"] + def process(prompt:) + @last_prompt = prompt results = DiscourseAi::Inference::StabilityGenerator.perform!(prompt) uploads = [] diff --git a/lib/modules/ai_bot/commands/search_command.rb b/lib/modules/ai_bot/commands/search_command.rb index 32371f40..dd7b794c 100644 --- a/lib/modules/ai_bot/commands/search_command.rb +++ b/lib/modules/ai_bot/commands/search_command.rb @@ -93,17 +93,15 @@ module DiscourseAi::AiBot::Commands } end - def process(search_args) - parsed = JSON.parse(search_args) - + def process(**search_args) limit = nil search_string = - parsed + search_args .map do |key, value| - if key == "search_query" + if key == :search_query value - elsif key == "limit" + elsif key == :limit limit = value.to_i nil else diff --git a/lib/modules/ai_bot/commands/summarize_command.rb b/lib/modules/ai_bot/commands/summarize_command.rb index 7c94a2b2..b8fcd13b 100644 --- a/lib/modules/ai_bot/commands/summarize_command.rb +++ b/lib/modules/ai_bot/commands/summarize_command.rb @@ -44,9 +44,7 @@ module DiscourseAi::AiBot::Commands { url: "#{Discourse.base_path}/t/-/#{@last_topic_id}", title: @last_topic_title || "" } end - def process(instructions) - topic_id, guidance = instructions.split(" ", 2) - + def process(topic_id:, guidance: nil) @last_topic_id = topic_id topic_id = topic_id.to_i diff --git a/lib/modules/ai_bot/commands/tags_command.rb b/lib/modules/ai_bot/commands/tags_command.rb index 61002d3a..fa8381a4 100644 --- a/lib/modules/ai_bot/commands/tags_command.rb +++ b/lib/modules/ai_bot/commands/tags_command.rb @@ -24,7 +24,7 @@ module DiscourseAi::AiBot::Commands { count: @last_count || 0 } end - def process(_args) + def process column_names = { name: "Name", public_topic_count: "Topic Count" } tags = diff --git a/lib/modules/ai_bot/commands/time_command.rb b/lib/modules/ai_bot/commands/time_command.rb index e5cb8aa8..39e2f46e 100644 --- a/lib/modules/ai_bot/commands/time_command.rb +++ b/lib/modules/ai_bot/commands/time_command.rb @@ -31,9 +31,7 @@ module DiscourseAi::AiBot::Commands { timezone: @last_timezone, time: @last_time } end - def process(args) - timezone = JSON.parse(args)["timezone"] - + def process(timezone:) time = begin Time.now.in_time_zone(timezone) @@ -45,7 +43,7 @@ module DiscourseAi::AiBot::Commands @last_timezone = timezone @last_time = time.to_s - { args: args, time: time.to_s } + { args: { timezone: timezone }, time: time.to_s } end end end diff --git a/spec/lib/modules/ai_bot/bot_spec.rb b/spec/lib/modules/ai_bot/bot_spec.rb index 289beb5f..210d5547 100644 --- a/spec/lib/modules/ai_bot/bot_spec.rb +++ b/spec/lib/modules/ai_bot/bot_spec.rb @@ -40,7 +40,7 @@ RSpec.describe DiscourseAi::AiBot::Bot do end describe "#reply_to" do - it "can respond to !search" do + it "can respond to a search command" do bot.system_prompt_style!(:simple) bot.max_commands_per_reply = 2 @@ -65,7 +65,7 @@ RSpec.describe DiscourseAi::AiBot::Bot do result = DiscourseAi::AiBot::Commands::SearchCommand .new(nil, nil) - .process({ query: "test search" }.to_json) + .process(query: "test search") .to_json prompt << { role: "function", content: result, name: "search" } diff --git a/spec/lib/modules/ai_bot/commands/categories_command_spec.rb b/spec/lib/modules/ai_bot/commands/categories_command_spec.rb index 03d42e78..aa0a90b3 100644 --- a/spec/lib/modules/ai_bot/commands/categories_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/categories_command_spec.rb @@ -7,7 +7,7 @@ RSpec.describe DiscourseAi::AiBot::Commands::CategoriesCommand do it "can generate correct info" do Fabricate(:category, name: "america", posts_year: 999) - info = DiscourseAi::AiBot::Commands::CategoriesCommand.new(nil, nil).process(nil) + info = DiscourseAi::AiBot::Commands::CategoriesCommand.new(nil, nil).process expect(info.to_s).to include("america") expect(info.to_s).to include("999") end diff --git a/spec/lib/modules/ai_bot/commands/google_command_spec.rb b/spec/lib/modules/ai_bot/commands/google_command_spec.rb index 47e0aa5f..b34ecb16 100644 --- a/spec/lib/modules/ai_bot/commands/google_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/google_command_spec.rb @@ -33,7 +33,7 @@ RSpec.describe DiscourseAi::AiBot::Commands::GoogleCommand do ).to_return(status: 200, body: json_text, headers: {}) google = described_class.new(bot_user, post) - info = google.process({ query: "some search term" }.to_json).to_json + info = google.process(query: "some search term").to_json expect(google.description_args[:count]).to eq(1) expect(info).to include("title1") diff --git a/spec/lib/modules/ai_bot/commands/search_command_spec.rb b/spec/lib/modules/ai_bot/commands/search_command_spec.rb index 13e3e998..6e6da4b8 100644 --- a/spec/lib/modules/ai_bot/commands/search_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/search_command_spec.rb @@ -13,8 +13,9 @@ RSpec.describe DiscourseAi::AiBot::Commands::SearchCommand do post1 = Fabricate(:post) search = described_class.new(bot_user, post1) - results = search.process({ query: "order:fake ABDDCDCEDGDG" }.to_json) - expect(results[:args]).to eq("{\"query\":\"order:fake ABDDCDCEDGDG\"}") + results = search.process(query: "order:fake ABDDCDCEDGDG") + + expect(results[:args]).to eq({ query: "order:fake ABDDCDCEDGDG" }) expect(results[:rows]).to eq([]) end @@ -25,7 +26,7 @@ RSpec.describe DiscourseAi::AiBot::Commands::SearchCommand do search = described_class.new(bot_user, post1) - results = search.process({ limit: 1, user: post1.user.username }.to_json) + results = search.process(limit: 1, user: post1.user.username) expect(results[:rows].to_s).to include("/subfolder" + post1.url) end @@ -37,13 +38,13 @@ RSpec.describe DiscourseAi::AiBot::Commands::SearchCommand do # search has no built in support for limit: so handle it from the outside search = described_class.new(bot_user, post1) - results = search.process({ limit: 2, user: post1.user.username }.to_json) + results = search.process(limit: 2, user: post1.user.username) expect(results[:column_names].length).to eq(4) expect(results[:rows].length).to eq(2) # just searching for everything - results = search.process({ order: "latest_topic" }.to_json) + results = search.process(order: "latest_topic") expect(results[:rows].length).to be > 1 end end diff --git a/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb b/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb index d61f9a22..3e4c336d 100644 --- a/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/summarize_command_spec.rb @@ -15,7 +15,7 @@ RSpec.describe DiscourseAi::AiBot::Commands::SummarizeCommand do ) summarizer = described_class.new(bot_user, post) - info = summarizer.process("#{post.topic_id} why did it happen?") + info = summarizer.process(topic_id: post.topic_id, guidance: "why did it happen?") expect(info).to include("Topic summarized") expect(summarizer.custom_raw).to include("summary stuff") @@ -31,7 +31,7 @@ RSpec.describe DiscourseAi::AiBot::Commands::SummarizeCommand do post = Fabricate(:post, topic: topic) summarizer = described_class.new(bot_user, post) - info = summarizer.process("#{post.topic_id} why did it happen?") + info = summarizer.process(topic_id: post.topic_id, guidance: "why did it happen?") expect(info).not_to include(post.raw) diff --git a/spec/lib/modules/ai_bot/commands/tags_command_spec.rb b/spec/lib/modules/ai_bot/commands/tags_command_spec.rb index 3b926323..4dc62389 100644 --- a/spec/lib/modules/ai_bot/commands/tags_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/tags_command_spec.rb @@ -10,7 +10,7 @@ RSpec.describe DiscourseAi::AiBot::Commands::TagsCommand do Fabricate(:tag, name: "america", public_topic_count: 100) Fabricate(:tag, name: "not_here", public_topic_count: 0) - info = DiscourseAi::AiBot::Commands::TagsCommand.new(nil, nil).process(nil) + info = DiscourseAi::AiBot::Commands::TagsCommand.new(nil, nil).process expect(info.to_s).to include("america") expect(info.to_s).not_to include("not_here") diff --git a/spec/lib/modules/ai_bot/commands/time_command_spec.rb b/spec/lib/modules/ai_bot/commands/time_command_spec.rb index 58b672ce..0cdf3317 100644 --- a/spec/lib/modules/ai_bot/commands/time_command_spec.rb +++ b/spec/lib/modules/ai_bot/commands/time_command_spec.rb @@ -7,8 +7,8 @@ RSpec.describe DiscourseAi::AiBot::Commands::TimeCommand do it "can generate correct info" do freeze_time - args = { timezone: "America/Los_Angeles" }.to_json - info = DiscourseAi::AiBot::Commands::TimeCommand.new(nil, nil).process(args) + args = { timezone: "America/Los_Angeles" } + info = DiscourseAi::AiBot::Commands::TimeCommand.new(nil, nil).process(**args) expect(info).to eq({ args: args, time: Time.now.in_time_zone("America/Los_Angeles").to_s }) expect(info.to_s).not_to include("not_here")