Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Commit 65503a5

Browse files
authored
FIX: various issues with llm and triage management (#1186)
- Fix search API to only include column_names when present to make the API less confusing - Ensure correct LLM is used in PMs by tracking and preferring the last bot user - Fix persona_id conversion from string to integer in custom fields - Add missing test for PM triage with no replies - ensure we don't try to auto title topic - Ensure bot users are properly added to PMs - Make title setting optional when replying to posts - Add ability to control stream_reply behavior These changes improve reliability and fix edge cases in bot interactions, particularly in private messages with multiple LLMs and while triaging posts using personas
1 parent b17c688 commit 65503a5

File tree

5 files changed

+123
-21
lines changed

5 files changed

+123
-21
lines changed

lib/ai_bot/playground.rb

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ module DiscourseAi
44
module AiBot
55
class Playground
66
BYPASS_AI_REPLY_CUSTOM_FIELD = "discourse_ai_bypass_ai_reply"
7+
BOT_USER_PREF_ID_CUSTOM_FIELD = "discourse_ai_bot_user_pref_id"
78

89
attr_reader :bot
910

@@ -64,6 +65,33 @@ def self.is_bot_user_id?(user_id)
6465
user_id.to_i <= 0
6566
end
6667

68+
def self.get_bot_user(post:, all_llm_users:, mentionables:)
69+
bot_user = nil
70+
if post.topic.private_message?
71+
# this ensures that we reply using the correct llm
72+
# 1. if we have a preferred llm user we use that
73+
# 2. if we don't just take first topic allowed user
74+
# 3. if we don't have that we take the first mentionable
75+
bot_user = nil
76+
if preferred_user =
77+
all_llm_users.find { |id, username|
78+
id == post.topic.custom_fields[BOT_USER_PREF_ID_CUSTOM_FIELD].to_i
79+
}
80+
bot_user = User.find_by(id: preferred_user[0])
81+
end
82+
bot_user ||=
83+
post.topic.topic_allowed_users.where(user_id: all_llm_users.map(&:first)).first&.user
84+
bot_user ||=
85+
post
86+
.topic
87+
.topic_allowed_users
88+
.where(user_id: mentionables.map { |m| m[:user_id] })
89+
.first
90+
&.user
91+
end
92+
bot_user
93+
end
94+
6795
def self.schedule_reply(post)
6896
return if is_bot_user_id?(post.user_id)
6997
mentionables = nil
@@ -75,7 +103,6 @@ def self.schedule_reply(post)
75103
mentionables = AiPersona.allowed_modalities(user: post.user, allow_topic_mentions: true)
76104
end
77105

78-
bot_user = nil
79106
mentioned = nil
80107

81108
all_llm_users =
@@ -84,18 +111,8 @@ def self.schedule_reply(post)
84111
.joins(:user)
85112
.pluck("users.id", "users.username_lower")
86113

87-
if post.topic.private_message?
88-
# this is an edge case, you started a PM with a different bot
89-
bot_user =
90-
post.topic.topic_allowed_users.where(user_id: all_llm_users.map(&:first)).first&.user
91-
bot_user ||=
92-
post
93-
.topic
94-
.topic_allowed_users
95-
.where(user_id: mentionables.map { |m| m[:user_id] })
96-
.first
97-
&.user
98-
end
114+
bot_user =
115+
get_bot_user(post: post, all_llm_users: all_llm_users, mentionables: mentionables)
99116

100117
mentions = nil
101118
if mentionables.present? || (bot_user && post.topic.private_message?)
@@ -126,6 +143,8 @@ def self.schedule_reply(post)
126143

127144
if bot_user
128145
topic_persona_id = post.topic.custom_fields["ai_persona_id"]
146+
topic_persona_id = topic_persona_id.to_i if topic_persona_id.present?
147+
129148
persona_id = mentioned&.dig(:id) || topic_persona_id
130149

131150
persona = nil
@@ -141,7 +160,7 @@ def self.schedule_reply(post)
141160
end
142161

143162
# edge case, llm was mentioned in an ai persona conversation
144-
if persona_id == topic_persona_id.to_i && post.topic.private_message? && persona &&
163+
if persona_id == topic_persona_id && post.topic.private_message? && persona &&
145164
all_llm_users.present?
146165
if !persona.force_default_llm && mentions.present?
147166
mentioned_llm_user_id, _ =
@@ -162,7 +181,15 @@ def self.schedule_reply(post)
162181
end
163182
end
164183

165-
def self.reply_to_post(post:, user: nil, persona_id: nil, whisper: nil, add_user_to_pm: false)
184+
def self.reply_to_post(
185+
post:,
186+
user: nil,
187+
persona_id: nil,
188+
whisper: nil,
189+
add_user_to_pm: false,
190+
stream_reply: false,
191+
auto_set_title: false
192+
)
166193
ai_persona = AiPersona.find_by(id: persona_id)
167194
raise Discourse::InvalidParameters.new(:persona_id) if !ai_persona
168195
persona_class = ai_persona.class_instance
@@ -178,6 +205,8 @@ def self.reply_to_post(post:, user: nil, persona_id: nil, whisper: nil, add_user
178205
whisper: whisper,
179206
context_style: :topic,
180207
add_user_to_pm: add_user_to_pm,
208+
stream_reply: stream_reply,
209+
auto_set_title: auto_set_title,
181210
)
182211
end
183212

@@ -444,6 +473,8 @@ def reply_to(
444473
whisper: nil,
445474
context_style: nil,
446475
add_user_to_pm: true,
476+
stream_reply: nil,
477+
auto_set_title: true,
447478
&blk
448479
)
449480
# this is a multithreading issue
@@ -479,13 +510,23 @@ def reply_to(
479510
reply_user = User.find_by(id: bot.persona.class.user_id) || reply_user
480511
end
481512

482-
stream_reply = post.topic.private_message?
513+
stream_reply = post.topic.private_message? if stream_reply.nil?
483514

484515
# we need to ensure persona user is allowed to reply to the pm
485516
if post.topic.private_message? && add_user_to_pm
486517
if !post.topic.topic_allowed_users.exists?(user_id: reply_user.id)
487518
post.topic.topic_allowed_users.create!(user_id: reply_user.id)
488519
end
520+
# edge case, maybe the llm user is missing?
521+
if !post.topic.topic_allowed_users.exists?(user_id: bot.bot_user.id)
522+
post.topic.topic_allowed_users.create!(user_id: bot.bot_user.id)
523+
end
524+
525+
# we store the id of the last bot_user, this is then used to give it preference
526+
if post.topic.custom_fields[BOT_USER_PREF_ID_CUSTOM_FIELD].to_i != bot.bot_user.id
527+
post.topic.custom_fields[BOT_USER_PREF_ID_CUSTOM_FIELD] = bot.bot_user.id
528+
post.topic.save_custom_fields
529+
end
489530
end
490531

491532
if stream_reply
@@ -609,7 +650,7 @@ def reply_to(
609650
end
610651
post_streamer&.finish(skip_callback: true)
611652
publish_final_update(reply_post) if stream_reply
612-
if reply_post && post.post_number == 1 && post.topic.private_message?
653+
if reply_post && post.post_number == 1 && post.topic.private_message? && auto_set_title
613654
title_playground(reply_post, post.user)
614655
end
615656
end

lib/utils/search.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ def self.perform_search(
125125

126126
def self.format_results(rows, args: nil, result_style:)
127127
rows = rows&.map { |row| yield row } if block_given?
128+
column_names = nil
128129

129130
if result_style == :compact
130131
index = -1
@@ -142,7 +143,8 @@ def self.format_results(rows, args: nil, result_style:)
142143
column_names = column_indexes.keys
143144
end
144145

145-
result = { column_names: column_names, rows: rows }
146+
result = { rows: rows }
147+
result[:column_names] = column_names if column_names
146148
result[:args] = args if args
147149
result
148150
end

spec/lib/discourse_automation/llm_persona_triage_spec.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,30 @@ def add_automation_field(name, value, type: "text")
165165
expect(context).to include("support")
166166
end
167167

168+
it "interacts correctly with a PM with no replies" do
169+
pm_topic = Fabricate(:private_message_topic, user: user, title: "Important PM")
170+
171+
# Create initial PM post
172+
pm_post =
173+
Fabricate(
174+
:post,
175+
topic: pm_topic,
176+
user: user,
177+
raw: "This is a private message that needs triage",
178+
)
179+
180+
DiscourseAi::Completions::Llm.with_prepared_responses(
181+
["I've received your private message"],
182+
) do |_, _, _prompts|
183+
automation.running_in_background!
184+
automation.trigger!({ "post" => pm_post })
185+
end
186+
187+
reply = pm_topic.posts.order(:post_number).last
188+
expect(reply.raw).to eq("I've received your private message")
189+
expect(reply.topic.reload.title).to eq("Important PM")
190+
end
191+
168192
it "interacts correctly with PMs" do
169193
# Create a private message topic
170194
pm_topic = Fabricate(:private_message_topic, user: user, title: "Important PM")

spec/lib/modules/ai_bot/playground_spec.rb

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,13 +637,14 @@
637637
create_post(
638638
title: "I just made a PM",
639639
raw: "Hey there #{persona.user.username}, can you help me?",
640-
target_usernames: "#{user.username},#{persona.user.username}",
640+
target_usernames: "#{user.username},#{persona.user.username},#{claude_2.user.username}",
641641
archetype: Archetype.private_message,
642642
user: admin,
643643
)
644644
end
645645

646-
post.topic.custom_fields["ai_persona_id"] = persona.id
646+
# note that this is a string due to custom field shananigans
647+
post.topic.custom_fields["ai_persona_id"] = persona.id.to_s
647648
post.topic.save_custom_fields
648649

649650
llm2 = Fabricate(:llm_model, enabled_chat_bot: true)
@@ -662,6 +663,22 @@
662663
expect(last_post.raw).to eq("Hi from bot two")
663664
expect(last_post.user_id).to eq(persona.user_id)
664665

666+
current_users = last_post.topic.reload.topic_allowed_users.joins(:user).pluck(:username)
667+
expect(current_users).to include(llm2.user.username)
668+
669+
# subseqent replies should come from the new llm
670+
DiscourseAi::Completions::Llm.with_prepared_responses(["Hi from bot two"], llm: llm2) do
671+
create_post(
672+
user: admin,
673+
raw: "just confirming everything switched",
674+
topic_id: post.topic_id,
675+
)
676+
end
677+
678+
last_post = post.topic.reload.posts.order("id desc").first
679+
expect(last_post.raw).to eq("Hi from bot two")
680+
expect(last_post.user_id).to eq(persona.user_id)
681+
665682
# tether llm, so it can no longer be switched
666683
persona.update!(force_default_llm: true, default_llm_id: claude_2.id)
667684

spec/lib/utils/search_spec.rb

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,26 @@
7272
GroupUser.create!(group: group, user: user)
7373

7474
# Now should find the private post
75-
results = described_class.perform_search(search_query: private_post.raw, current_user: user)
75+
results =
76+
described_class.perform_search(
77+
search_query: private_post.raw,
78+
current_user: user,
79+
result_style: :detailed,
80+
)
81+
expect(results[:rows].length).to eq(1)
82+
# so API is less confusing
83+
expect(results.key?(:column_names)).to eq(false)
84+
85+
results =
86+
described_class.perform_search(
87+
search_query: private_post.raw,
88+
current_user: user,
89+
result_style: :compact,
90+
)
91+
7692
expect(results[:rows].length).to eq(1)
93+
# so API is less confusing
94+
expect(results[:column_names]).to be_present
7795
end
7896

7997
it "properly handles subfolder URLs" do

0 commit comments

Comments
 (0)