Claude 3 Opus as a coding assistant

I want to make a PR to discourse/discourse-ai that changes the behavior of ai bot.

• When a user replies directly to another user on an ai bot PM, do not reply as bot
• When a user replies to another persona on the PM, reply as other persona

Can you help me create this PR, I am going to need tests as well.

Start by searching through the repo so you find the names of all the files involved, then read the important files.

Okay, let’s break this down step-by-step. First, I’ll search the discourse/discourse-ai repository for relevant files:

GitHub code search

Searched for 'ChatbotReplyJob' in discourse/discourse-ai

Hmm, the search for “ChatbotReplyJob” in the discourse/discourse-ai repo did not return any results. Let me try a broader search.

calls

GitHub code search

Searched for 'bot reply' in discourse/discourse-ai

Based on the code search results, it looks like the main files involved in handling bot replies are:

1. lib/ai_bot/bot.rb - Defines the AiBot::Bot class which handles bot replies
2. app/jobs/regular/create_ai_reply.rb - Job that creates a bot reply by calling AiBot::Playground#reply_to
3. lib/ai_bot/playground.rb - AiBot::Playground class which handles the process of generating a bot reply
4. lib/ai_bot/entry_point.rb - Registers a callback to schedule a bot reply when a post is created

To implement the requested changes, we’ll need to modify the logic in these files to:

1. Check if the post being replied to is a direct reply to another user (not the bot)
2. If so, do not generate a bot reply
3. If the post is a reply to a different persona, reply as that persona instead

Let’s take a closer look at the relevant files:

GitHub file content

Retrieved content of lib/ai_bot/bot.rb, app/jobs/regular/create_ai_reply.rb, lib/ai_bot/playground.rb, lib/ai_bot/entry_point.rb from discourse/discourse-ai@main

Based on the code in the discourse-ai repository, here are the key changes needed to implement the desired behavior:

1. In lib/ai_bot/entry_point.rb, modify the on(:post_created) callback to pass the post to a new method, e.g. AiBot::Playground.handle_new_post(post) instead of directly calling schedule_reply.

2. In lib/ai_bot/playground.rb, implement the handle_new_post method:

def self.handle_new_post(post)
  return if post.user.bot?

  if post.reply_to_post_number.present?
    replied_to_post = Post.find_by(topic_id: post.topic_id, post_number: post.reply_to_post_number)
    
    if replied_to_post.user.bot?
      # User replied to bot, don't reply
      return
    elsif replied_to_post.ai_persona_name != post.ai_persona_name
      # User replied to a different persona, reply as that persona  
      bot_user = User.find_by(username: replied_to_post.ai_persona_name)
      persona = AiPersona.find_by(user: post.user, name: replied_to_post.ai_persona_name)
    end
  end

  bot_user ||= get_bot_user(post)
  persona ||= get_persona(post)

  return unless bot_user && persona

  bot = DiscourseAi::AiBot::Bot.as(bot_user, persona: persona.new)
  DiscourseAi::AiBot::Playground.new(bot).reply_to(post)  
end

3. Add helper methods in playground.rb to determine the bot user and persona:

def self.get_bot_user(post)
  if post.topic.private_message?
    post.topic.topic_allowed_users.where(user_id: DiscourseAi::AiBot::EntryPoint::BOT_USER_IDS).first&.user
  end
end

def self.get_persona(post)
  persona_id = post.topic.custom_fields["ai_persona_id"]
  DiscourseAi::AiBot::Personas::Persona.find_by(user: post.user, id: persona_id) if persona_id
end

4. Add tests in spec/lib/ai_bot/playground_spec.rb to verify the new behavior. Test cases should cover:

• When a user replies to the bot, no reply is sent
• When a user replies to another user in a PM with the bot, the bot replies as its default persona
• When a user replies to a specific bot persona, the bot replies as that persona

This covers the main changes needed in the bot logic. Let me know if you have any other questions!

Mister I think you got this the opposite way around… if we reply to someone that is not a bot, we should exit. this is saying any reply to bot is ignored which is wrong.

Also write me a test or 2

You’re absolutely right, I got that condition backwards. Here’s the corrected version:

def self.handle_new_post(post)
  return if post.user.bot?

  if post.reply_to_post_number.present?
    replied_to_post = Post.find_by(topic_id: post.topic_id, post_number: post.reply_to_post_number)
    
    if replied_to_post.user.bot?
      # User replied to bot, continue with reply
      bot_user = replied_to_post.user
      persona = AiPersona.find_by(user: post.user, name: replied_to_post.ai_persona_name)
    else 
      # User replied to another user, don't reply
      return
    end
  end

  bot_user ||= get_bot_user(post)
  persona ||= get_persona(post)

  return unless bot_user && persona

  bot = DiscourseAi::AiBot::Bot.as(bot_user, persona: persona.new)
  DiscourseAi::AiBot::Playground.new(bot).reply_to(post)  
end

And here are a couple of test cases for spec/lib/ai_bot/playground_spec.rb:

require 'rails_helper'

describe DiscourseAi::AiBot::Playground do
  fab!(:user) { Fabricate(:user) }
  fab!(:bot_user) { Fabricate(:user, id: DiscourseAi::AiBot::EntryPoint::GPT4_ID) }
  fab!(:persona) { DiscourseAi::AiBot::Personas::Persona.create(user: user, name: 'Test Persona') }
  
  before do
    SiteSetting.ai_bot_enabled = true
    SiteSetting.ai_bot_allowed_groups = Group::AUTO_GROUPS[:everyone]
  end

  describe '.handle_new_post' do
    fab!(:topic) { Fabricate(:private_message_topic, topic_allowed_users: [user, bot_user]) }
    fab!(:bot_post) { Fabricate(:post, topic: topic, user: bot_user) }
    fab!(:user_post) { Fabricate(:post, topic: topic, user: user) }
    
    it 'does not reply when user replies to another user' do
      reply = Fabricate(:post, topic: topic, user: user, reply_to_post_number: user_post.post_number)
      
      expect(DiscourseAi::AiBot::Playground).not_to receive(:new)
      DiscourseAi::AiBot::Playground.handle_new_post(reply)
    end

    it 'replies as the replied-to persona when user replies to bot' do
      persona.update!(user: bot_user)
      bot_post.update!(raw: "This is the Test Persona")
      reply = Fabricate(:post, topic: topic, user: user, reply_to_post_number: bot_post.post_number)

      playground = DiscourseAi::AiBot::Playground.new(nil)
      expect(DiscourseAi::AiBot::Playground).to receive(:new).and_return(playground)
      expect(playground).to receive(:reply_to).with(reply)

      DiscourseAi::AiBot::Playground.handle_new_post(reply)
    end
  end
end

The first test checks that no reply is sent when a user replies to another user in the bot PM.

The second test verifies that when a user replies to a specific bot persona, the bot replies as that persona.

Let me know if you have any other questions or if you’d like me to modify the tests further.

I don’t like this… can you just recommend surgical changes to “schedule_reply” instead.

revise tests to account for this.

Certainly! Instead of creating a new handle_new_post method, we can modify the existing schedule_reply method in lib/ai_bot/playground.rb to handle the new requirements. Here’s the updated code:

def self.schedule_reply(post)
  return if post.user.bot?

  bot_user = nil
  persona = nil

  if post.reply_to_post_number.present?
    replied_to_post = Post.find_by(topic_id: post.topic_id, post_number: post.reply_to_post_number)
    
    if replied_to_post.user.bot?
      # User replied to bot, continue with reply
      bot_user = replied_to_post.user
      persona = AiPersona.find_by(user: post.user, name: replied_to_post.ai_persona_name)
    else 
      # User replied to another user, don't reply
      return
    end
  end

  bot_user ||= post.topic.topic_allowed_users.where(user_id: DiscourseAi::AiBot::EntryPoint::BOT_USER_IDS).first&.user
  persona ||= DiscourseAi::AiBot::Personas::Persona.find_by(user: post.user, id: post.topic.custom_fields["ai_persona_id"])

  return unless bot_user && persona

  bot = DiscourseAi::AiBot::Bot.as(bot_user, persona: persona.new)
  DiscourseAi::AiBot::Playground.new(bot).reply_to(post)
end

And here are the updated tests for spec/lib/ai_bot/playground_spec.rb:

require 'rails_helper'

describe DiscourseAi::AiBot::Playground do
  fab!(:user) { Fabricate(:user) }
  fab!(:bot_user) { Fabricate(:user, id: DiscourseAi::AiBot::EntryPoint::GPT4_ID) }
  fab!(:persona) { DiscourseAi::AiBot::Personas::Persona.create(user: user, name: 'Test Persona') }
  
  before do
    SiteSetting.ai_bot_enabled = true
    SiteSetting.ai_bot_allowed_groups = Group::AUTO_GROUPS[:everyone]
  end

  describe '.schedule_reply' do
    fab!(:topic) { Fabricate(:private_message_topic, topic_allowed_users: [user, bot_user]) }
    fab!(:bot_post) { Fabricate(:post, topic: topic, user: bot_user) }
    fab!(:user_post) { Fabricate(:post, topic: topic, user: user) }
    
    it 'does not reply when user replies to another user' do
      reply = Fabricate(:post, topic: topic, user: user, reply_to_post_number: user_post.post_number)
      
      expect(DiscourseAi::AiBot::Playground).not_to receive(:new)
      DiscourseAi::AiBot::Playground.schedule_reply(reply)
    end

    it 'replies as the replied-to persona when user replies to bot' do
      persona.update!(user: bot_user)
      bot_post.update!(raw: "This is the Test Persona")
      reply = Fabricate(:post, topic: topic, user: user, reply_to_post_number: bot_post.post_number)

      playground = DiscourseAi::AiBot::Playground.new(nil)
      expect(DiscourseAi::AiBot::Playground).to receive(:new).and_return(playground)
      expect(playground).to receive(:reply_to).with(reply)

      DiscourseAi::AiBot::Playground.schedule_reply(reply)
    end
  end
end

The changes are similar to the previous version, but now we’re modifying the existing schedule_reply method instead of creating a new one.

The tests remain the same, but they now call schedule_reply instead of handle_new_post.

Let me know if you have any further questions or if you’d like me to make any other changes!