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

Commit 67e3a61

Browse files
authored
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
1 parent df63e36 commit 67e3a61

File tree

2 files changed

+106
-16
lines changed

2 files changed

+106
-16
lines changed

lib/completions/prompt_messages_builder.rb

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ def self.messages_from_chat(
7575
builder.push(
7676
type: :user,
7777
content: mapped_message,
78-
name: m.user.username,
78+
id: m.user.username,
7979
upload_ids: upload_ids,
8080
)
8181
end
@@ -215,11 +215,11 @@ def to_a(limit: nil, style: nil)
215215
last_message = result[-1]
216216

217217
if message[:type] == :user
218-
old_name = last_message.delete(:name)
218+
old_name = last_message.delete(:id)
219219
last_message[:content] = ["#{old_name}: ", last_message[:content]].flatten if old_name
220220

221221
new_content = message[:content]
222-
new_content = ["#{message[:name]}: ", new_content].flatten if message[:name]
222+
new_content = ["#{message[:id]}: ", new_content].flatten if message[:id]
223223

224224
if !last_message[:content].is_a?(Array)
225225
last_message[:content] = [last_message[:content]]
@@ -285,6 +285,44 @@ def push(type:, content:, name: nil, upload_ids: nil, id: nil, thinking: nil)
285285

286286
private
287287

288+
def format_user_info(user)
289+
info = []
290+
info << user_role(user)
291+
info << "Trust level #{user.trust_level}" if user.trust_level > 0
292+
info << "#{account_age(user)}"
293+
info << "#{user.user_stat.post_count} posts" if user.user_stat.post_count.to_i > 0
294+
"#{user.username} (#{user.name}): #{info.compact.join(", ")}"
295+
end
296+
297+
def user_role(user)
298+
return "moderator" if user.moderator?
299+
return "admin" if user.admin?
300+
nil
301+
end
302+
303+
def account_age(user)
304+
years = ((Time.now - user.created_at) / 1.year).round
305+
months = ((Time.now - user.created_at) / 1.month).round % 12
306+
307+
output = []
308+
if years > 0
309+
output << years.to_s
310+
output << "year" if years == 1
311+
output << "years" if years > 1
312+
end
313+
if months > 0
314+
output << months.to_s
315+
output << "month" if months == 1
316+
output << "months" if months > 1
317+
end
318+
319+
if output.empty?
320+
"new account"
321+
else
322+
"account age: " + output.join(" ")
323+
end
324+
end
325+
288326
def topic_array
289327
raw_messages = @raw_messages.dup
290328
content_array = []
@@ -310,19 +348,35 @@ def topic_array
310348
content_array << "- Number of replies: #{@topic.posts_count - 1}\n\n"
311349
end
312350

351+
if raw_messages.present?
352+
usernames =
353+
raw_messages.filter { |message| message[:type] == :user }.map { |message| message[:id] }
354+
355+
if usernames.present?
356+
users_details =
357+
User
358+
.where(username: usernames)
359+
.includes(:user_stat)
360+
.map { |user| format_user_info(user) }
361+
.compact
362+
content_array << "User information:\n"
363+
content_array << "- #{users_details.join("\n- ")}\n\n" if users_details.present?
364+
end
365+
end
366+
313367
last_user_message = raw_messages.pop
314368

315369
if raw_messages.present?
316370
content_array << "Here is the conversation so far:\n"
317371
raw_messages.each do |message|
318-
content_array << "#{message[:name] || "User"}: "
372+
content_array << "#{message[:id] || "User"}: "
319373
content_array << message[:content]
320374
content_array << "\n\n"
321375
end
322376
end
323377

324378
if last_user_message
325-
content_array << "You are responding to #{last_user_message[:name] || "User"} who just said:\n"
379+
content_array << "Latest post is by #{last_user_message[:id] || "User"} who just posted:\n"
326380
content_array << last_user_message[:content]
327381
end
328382

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

346400
if message[:type] == :user
347-
buffer << "#{message[:name] || "User"}: "
401+
buffer << "#{message[:id] || "User"}: "
348402
else
349403
buffer << "Bot: "
350404
end
@@ -359,7 +413,7 @@ def chat_array(limit:)
359413
end
360414

361415
last_message = @raw_messages[-1]
362-
buffer << "#{last_message[:name] || "User"}: "
416+
buffer << "#{last_message[:id] || "User"}: "
363417
buffer << last_message[:content]
364418

365419
buffer = compress_messages_buffer(buffer.flatten, max_uploads: MAX_CHAT_UPLOADS)

spec/lib/completions/prompt_messages_builder_spec.rb

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
end
1616

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

2121
messages = builder.to_a
2222

@@ -34,8 +34,8 @@
3434
end
3535

3636
it "should allow merging user messages" do
37-
builder.push(type: :user, content: "Hello", name: "Alice")
38-
builder.push(type: :user, content: "World", name: "Bob")
37+
builder.push(type: :user, content: "Hello", id: "Alice")
38+
builder.push(type: :user, content: "World", id: "Bob")
3939

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

6565
it "should drop a tool call if it is not followed by tool" do
66-
builder.push(type: :user, content: "Echo 123 please", name: "Alice")
66+
builder.push(type: :user, content: "Echo 123 please", id: "Alice")
6767
builder.push(type: :tool_call, content: "echo(123)", name: "echo", id: 1)
68-
builder.push(type: :user, content: "OK", name: "James")
68+
builder.push(type: :user, content: "OK", id: "James")
6969

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

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

8686
result = builder.to_a(style: :topic)
8787

@@ -155,7 +155,7 @@
155155
)
156156

157157
# this is all we got cause it is assuming threading
158-
expect(context).to eq([{ type: :user, content: "How are you?", name: user.username }])
158+
expect(context).to eq([{ type: :user, content: "How are you?", id: user.username }])
159159
end
160160

161161
it "includes uploads when include_uploads is true" do
@@ -330,6 +330,42 @@
330330
)
331331
end
332332

333+
it "provides rich context for for style topic messages" do
334+
freeze_time
335+
336+
user.update!(trust_level: 2, created_at: 1.year.ago)
337+
admin.update!(trust_level: 4, created_at: 1.month.ago)
338+
user.user_stat.update!(post_count: 10, days_visited: 50)
339+
340+
reply_to_first_post =
341+
Fabricate(
342+
:post,
343+
topic: pm,
344+
user: admin,
345+
reply_to_post_number: first_post.post_number,
346+
raw: "This is a reply to the first post",
347+
)
348+
349+
context =
350+
described_class.messages_from_post(
351+
reply_to_first_post,
352+
style: :topic,
353+
max_posts: 10,
354+
bot_usernames: [bot_user.username],
355+
include_uploads: false,
356+
)
357+
358+
expect(context.length).to eq(1)
359+
content = context[0][:content]
360+
361+
expect(content).to include(user.name)
362+
expect(content).to include("Trust level 2")
363+
expect(content).to include("account age: 1 year")
364+
365+
# I am mixed on asserting everything cause the test
366+
# will be brittle, but open to changing this
367+
end
368+
333369
it "handles uploads correctly in topic style messages" do
334370
# Use Discourse's upload format in the post raw content
335371
upload_markdown = "![test|658x372](#{image_upload1.short_url})"

0 commit comments

Comments
 (0)