From 67e3a610cb71182a07f5b78e223e38fd46cfd283 Mon Sep 17 00:00:00 2001 From: Sam Date: Sat, 12 Apr 2025 08:15:31 +1000 Subject: [PATCH] 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 --- lib/completions/prompt_messages_builder.rb | 68 +++++++++++++++++-- .../prompt_messages_builder_spec.rb | 54 ++++++++++++--- 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/lib/completions/prompt_messages_builder.rb b/lib/completions/prompt_messages_builder.rb index 56d2154e..840fecf1 100644 --- a/lib/completions/prompt_messages_builder.rb +++ b/lib/completions/prompt_messages_builder.rb @@ -75,7 +75,7 @@ module DiscourseAi builder.push( type: :user, content: mapped_message, - name: m.user.username, + id: m.user.username, upload_ids: upload_ids, ) end @@ -215,11 +215,11 @@ module DiscourseAi 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]] @@ -285,6 +285,44 @@ module DiscourseAi 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 = [] @@ -310,19 +348,35 @@ module DiscourseAi 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 @@ -344,7 +398,7 @@ module DiscourseAi buffer << "\n" if message[:type] == :user - buffer << "#{message[:name] || "User"}: " + buffer << "#{message[:id] || "User"}: " else buffer << "Bot: " end @@ -359,7 +413,7 @@ module DiscourseAi 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) diff --git a/spec/lib/completions/prompt_messages_builder_spec.rb b/spec/lib/completions/prompt_messages_builder_spec.rb index 5fd7eb1d..d2bb2a21 100644 --- a/spec/lib/completions/prompt_messages_builder_spec.rb +++ b/spec/lib/completions/prompt_messages_builder_spec.rb @@ -15,8 +15,8 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do 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 @@ -34,8 +34,8 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do 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 @@ -63,9 +63,9 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do 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) @@ -80,8 +80,8 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do 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) @@ -155,7 +155,7 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do ) # 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 @@ -330,6 +330,42 @@ describe DiscourseAi::Completions::PromptMessagesBuilder do ) 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})"