FIX: invalid context construction for responders (#1257)

Previous to this fix we assumed the name field contained usernames
when in fact it was stored in the id field.

This fixes the context contruction and also adds some basic user
information to the context to assist responders in understanding
the cast of chars
This commit is contained in:
Sam 2025-04-12 08:15:31 +10:00 committed by GitHub
parent df63e36ad8
commit 67e3a610cb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 106 additions and 16 deletions

View File

@ -75,7 +75,7 @@ module DiscourseAi
builder.push( builder.push(
type: :user, type: :user,
content: mapped_message, content: mapped_message,
name: m.user.username, id: m.user.username,
upload_ids: upload_ids, upload_ids: upload_ids,
) )
end end
@ -215,11 +215,11 @@ module DiscourseAi
last_message = result[-1] last_message = result[-1]
if message[:type] == :user if message[:type] == :user
old_name = last_message.delete(:name) old_name = last_message.delete(:id)
last_message[:content] = ["#{old_name}: ", last_message[:content]].flatten if old_name last_message[:content] = ["#{old_name}: ", last_message[:content]].flatten if old_name
new_content = message[:content] new_content = message[:content]
new_content = ["#{message[:name]}: ", new_content].flatten if message[:name] new_content = ["#{message[:id]}: ", new_content].flatten if message[:id]
if !last_message[:content].is_a?(Array) if !last_message[:content].is_a?(Array)
last_message[:content] = [last_message[:content]] last_message[:content] = [last_message[:content]]
@ -285,6 +285,44 @@ module DiscourseAi
private private
def format_user_info(user)
info = []
info << user_role(user)
info << "Trust level #{user.trust_level}" if user.trust_level > 0
info << "#{account_age(user)}"
info << "#{user.user_stat.post_count} posts" if user.user_stat.post_count.to_i > 0
"#{user.username} (#{user.name}): #{info.compact.join(", ")}"
end
def user_role(user)
return "moderator" if user.moderator?
return "admin" if user.admin?
nil
end
def account_age(user)
years = ((Time.now - user.created_at) / 1.year).round
months = ((Time.now - user.created_at) / 1.month).round % 12
output = []
if years > 0
output << years.to_s
output << "year" if years == 1
output << "years" if years > 1
end
if months > 0
output << months.to_s
output << "month" if months == 1
output << "months" if months > 1
end
if output.empty?
"new account"
else
"account age: " + output.join(" ")
end
end
def topic_array def topic_array
raw_messages = @raw_messages.dup raw_messages = @raw_messages.dup
content_array = [] content_array = []
@ -310,19 +348,35 @@ module DiscourseAi
content_array << "- Number of replies: #{@topic.posts_count - 1}\n\n" content_array << "- Number of replies: #{@topic.posts_count - 1}\n\n"
end end
if raw_messages.present?
usernames =
raw_messages.filter { |message| message[:type] == :user }.map { |message| message[:id] }
if usernames.present?
users_details =
User
.where(username: usernames)
.includes(:user_stat)
.map { |user| format_user_info(user) }
.compact
content_array << "User information:\n"
content_array << "- #{users_details.join("\n- ")}\n\n" if users_details.present?
end
end
last_user_message = raw_messages.pop last_user_message = raw_messages.pop
if raw_messages.present? if raw_messages.present?
content_array << "Here is the conversation so far:\n" content_array << "Here is the conversation so far:\n"
raw_messages.each do |message| raw_messages.each do |message|
content_array << "#{message[:name] || "User"}: " content_array << "#{message[:id] || "User"}: "
content_array << message[:content] content_array << message[:content]
content_array << "\n\n" content_array << "\n\n"
end end
end end
if last_user_message if last_user_message
content_array << "You are responding to #{last_user_message[:name] || "User"} who just said:\n" content_array << "Latest post is by #{last_user_message[:id] || "User"} who just posted:\n"
content_array << last_user_message[:content] content_array << last_user_message[:content]
end end
@ -344,7 +398,7 @@ module DiscourseAi
buffer << "\n" buffer << "\n"
if message[:type] == :user if message[:type] == :user
buffer << "#{message[:name] || "User"}: " buffer << "#{message[:id] || "User"}: "
else else
buffer << "Bot: " buffer << "Bot: "
end end
@ -359,7 +413,7 @@ module DiscourseAi
end end
last_message = @raw_messages[-1] last_message = @raw_messages[-1]
buffer << "#{last_message[:name] || "User"}: " buffer << "#{last_message[:id] || "User"}: "
buffer << last_message[:content] buffer << last_message[:content]
buffer = compress_messages_buffer(buffer.flatten, max_uploads: MAX_CHAT_UPLOADS) buffer = compress_messages_buffer(buffer.flatten, max_uploads: MAX_CHAT_UPLOADS)

View File

@ -15,8 +15,8 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do
end end
it "correctly merges user messages with uploads" do it "correctly merges user messages with uploads" do
builder.push(type: :user, content: "Hello", name: "Alice", upload_ids: [1]) builder.push(type: :user, content: "Hello", id: "Alice", upload_ids: [1])
builder.push(type: :user, content: "World", name: "Bob", upload_ids: [2]) builder.push(type: :user, content: "World", id: "Bob", upload_ids: [2])
messages = builder.to_a messages = builder.to_a
@ -34,8 +34,8 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do
end end
it "should allow merging user messages" do it "should allow merging user messages" do
builder.push(type: :user, content: "Hello", name: "Alice") builder.push(type: :user, content: "Hello", id: "Alice")
builder.push(type: :user, content: "World", name: "Bob") builder.push(type: :user, content: "World", id: "Bob")
expect(builder.to_a).to eq([{ type: :user, content: "Alice: Hello\nBob: World" }]) expect(builder.to_a).to eq([{ type: :user, content: "Alice: Hello\nBob: World" }])
end end
@ -63,9 +63,9 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do
end end
it "should drop a tool call if it is not followed by tool" do it "should drop a tool call if it is not followed by tool" do
builder.push(type: :user, content: "Echo 123 please", name: "Alice") builder.push(type: :user, content: "Echo 123 please", id: "Alice")
builder.push(type: :tool_call, content: "echo(123)", name: "echo", id: 1) builder.push(type: :tool_call, content: "echo(123)", name: "echo", id: 1)
builder.push(type: :user, content: "OK", name: "James") builder.push(type: :user, content: "OK", id: "James")
expected = [{ type: :user, content: "Alice: Echo 123 please\nJames: OK" }] expected = [{ type: :user, content: "Alice: Echo 123 please\nJames: OK" }]
expect(builder.to_a).to eq(expected) expect(builder.to_a).to eq(expected)
@ -80,8 +80,8 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do
topic.save! topic.save!
builder.topic = topic builder.topic = topic
builder.push(type: :user, content: "I like frogs", name: "Bob") builder.push(type: :user, content: "I like frogs", id: "Bob")
builder.push(type: :user, content: "How do I solve this?", name: "Alice") builder.push(type: :user, content: "How do I solve this?", id: "Alice")
result = builder.to_a(style: :topic) result = builder.to_a(style: :topic)
@ -155,7 +155,7 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do
) )
# this is all we got cause it is assuming threading # this is all we got cause it is assuming threading
expect(context).to eq([{ type: :user, content: "How are you?", name: user.username }]) expect(context).to eq([{ type: :user, content: "How are you?", id: user.username }])
end end
it "includes uploads when include_uploads is true" do it "includes uploads when include_uploads is true" do
@ -330,6 +330,42 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do
) )
end end
it "provides rich context for for style topic messages" do
freeze_time
user.update!(trust_level: 2, created_at: 1.year.ago)
admin.update!(trust_level: 4, created_at: 1.month.ago)
user.user_stat.update!(post_count: 10, days_visited: 50)
reply_to_first_post =
Fabricate(
:post,
topic: pm,
user: admin,
reply_to_post_number: first_post.post_number,
raw: "This is a reply to the first post",
)
context =
described_class.messages_from_post(
reply_to_first_post,
style: :topic,
max_posts: 10,
bot_usernames: [bot_user.username],
include_uploads: false,
)
expect(context.length).to eq(1)
content = context[0][:content]
expect(content).to include(user.name)
expect(content).to include("Trust level 2")
expect(content).to include("account age: 1 year")
# I am mixed on asserting everything cause the test
# will be brittle, but open to changing this
end
it "handles uploads correctly in topic style messages" do it "handles uploads correctly in topic style messages" do
# Use Discourse's upload format in the post raw content # Use Discourse's upload format in the post raw content
upload_markdown = "![test|658x372](#{image_upload1.short_url})" upload_markdown = "![test|658x372](#{image_upload1.short_url})"