Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 61 additions & 7 deletions lib/completions/prompt_messages_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def self.messages_from_chat(
builder.push(
type: :user,
content: mapped_message,
name: m.user.username,
id: m.user.username,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the push method still takes in both name and id parameters in the method signature. Do we need to keep them both?

upload_ids: upload_ids,
)
end
Expand Down Expand Up @@ -215,11 +215,11 @@ def to_a(limit: nil, style: nil)
last_message = result[-1]

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

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)
last_message[:content] = [last_message[:content]]
Expand Down Expand Up @@ -285,6 +285,44 @@ def push(type:, content:, name: nil, upload_ids: nil, id: nil, thinking: nil)

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
raw_messages = @raw_messages.dup
content_array = []
Expand All @@ -310,19 +348,35 @@ def topic_array
content_array << "- Number of replies: #{@topic.posts_count - 1}\n\n"
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

if raw_messages.present?
content_array << "Here is the conversation so far:\n"
raw_messages.each do |message|
content_array << "#{message[:name] || "User"}: "
content_array << "#{message[:id] || "User"}: "
content_array << message[:content]
content_array << "\n\n"
end
end

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]
end

Expand All @@ -344,7 +398,7 @@ def chat_array(limit:)
buffer << "\n"

if message[:type] == :user
buffer << "#{message[:name] || "User"}: "
buffer << "#{message[:id] || "User"}: "
else
buffer << "Bot: "
end
Expand All @@ -359,7 +413,7 @@ def chat_array(limit:)
end

last_message = @raw_messages[-1]
buffer << "#{last_message[:name] || "User"}: "
buffer << "#{last_message[:id] || "User"}: "
buffer << last_message[:content]

buffer = compress_messages_buffer(buffer.flatten, max_uploads: MAX_CHAT_UPLOADS)
Expand Down
54 changes: 45 additions & 9 deletions spec/lib/completions/prompt_messages_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
end

it "correctly merges user messages with uploads" do
builder.push(type: :user, content: "Hello", name: "Alice", upload_ids: [1])
builder.push(type: :user, content: "World", name: "Bob", upload_ids: [2])
builder.push(type: :user, content: "Hello", id: "Alice", upload_ids: [1])
builder.push(type: :user, content: "World", id: "Bob", upload_ids: [2])

messages = builder.to_a

Expand All @@ -34,8 +34,8 @@
end

it "should allow merging user messages" do
builder.push(type: :user, content: "Hello", name: "Alice")
builder.push(type: :user, content: "World", name: "Bob")
builder.push(type: :user, content: "Hello", id: "Alice")
builder.push(type: :user, content: "World", id: "Bob")

expect(builder.to_a).to eq([{ type: :user, content: "Alice: Hello\nBob: World" }])
end
Expand Down Expand Up @@ -63,9 +63,9 @@
end

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: :user, content: "OK", name: "James")
builder.push(type: :user, content: "OK", id: "James")

expected = [{ type: :user, content: "Alice: Echo 123 please\nJames: OK" }]
expect(builder.to_a).to eq(expected)
Expand All @@ -80,8 +80,8 @@
topic.save!

builder.topic = topic
builder.push(type: :user, content: "I like frogs", name: "Bob")
builder.push(type: :user, content: "How do I solve this?", name: "Alice")
builder.push(type: :user, content: "I like frogs", id: "Bob")
builder.push(type: :user, content: "How do I solve this?", id: "Alice")

result = builder.to_a(style: :topic)

Expand Down Expand Up @@ -155,7 +155,7 @@
)

# 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

it "includes uploads when include_uploads is true" do
Expand Down Expand Up @@ -330,6 +330,42 @@
)
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
# Use Discourse's upload format in the post raw content
upload_markdown = "![test|658x372](#{image_upload1.short_url})"
Expand Down