diff --git a/app/services/llm_formatter/conversation_llm_formatter.rb b/app/services/llm_formatter/conversation_llm_formatter.rb index d7e7a1cdb..1444d75c1 100644 --- a/app/services/llm_formatter/conversation_llm_formatter.rb +++ b/app/services/llm_formatter/conversation_llm_formatter.rb @@ -5,29 +5,22 @@ class LlmFormatter::ConversationLlmFormatter < LlmFormatter::DefaultLlmFormatter sections << "Channel: #{@record.inbox.channel.name}" sections << 'Message History:' sections << if @record.messages.any? - build_messages(config) + build_messages else 'No messages in this conversation' end sections << "Contact Details: #{@record.contact.to_llm_text}" if config[:include_contact_details] - sections << 'Conversation Attributes:' - sections << build_attributes sections.join("\n") end private - def build_messages(config = {}) + def build_messages return "No messages in this conversation\n" if @record.messages.empty? message_text = '' - messages = @record.messages.where.not(message_type: :activity).order(created_at: :asc) - - messages.each do |message| - # Skip private messages unless explicitly included in config - next if message.private? && !config[:include_private_messages] - + @record.messages.chat.order(created_at: :asc).each do |message| message_text << format_message(message) end message_text @@ -35,14 +28,6 @@ class LlmFormatter::ConversationLlmFormatter < LlmFormatter::DefaultLlmFormatter def format_message(message) sender = message.message_type == 'incoming' ? 'User' : 'Support agent' - sender = "[Private Note] #{sender}" if message.private? "#{sender}: #{message.content}\n" end - - def build_attributes - attributes = @record.account.custom_attribute_definitions.with_attribute_model('conversation_attribute').map do |attribute| - "#{attribute.attribute_display_name}: #{@record.custom_attributes[attribute.attribute_key]}" - end - attributes.join("\n") - end end diff --git a/app/services/openai_multimodal_content_service.rb b/app/services/openai_multimodal_content_service.rb deleted file mode 100644 index 11bf3c98e..000000000 --- a/app/services/openai_multimodal_content_service.rb +++ /dev/null @@ -1,74 +0,0 @@ -class OpenaiMultimodalContentService - def initialize(message) - @message = message - end - - def generate_content - parts = [] - - parts << text_part(@message.content) if @message.content.present? - parts.concat(attachment_parts(@message.attachments)) if @message.attachments.any? - - finalize_content_parts(parts) - end - - private - - def text_part(text) - { type: 'text', text: text } - end - - def attachment_parts(attachments) - [].tap do |parts| - parts.concat(image_parts(attachments.where(file_type: :image))) - - transcription = extract_audio_transcriptions(attachments) - parts << text_part(transcription) if transcription.present? - - parts << text_part('User has shared an attachment') if attachments.where.not(file_type: %i[image audio]).exists? - end - end - - def image_parts(image_attachments) - image_attachments.each_with_object([]) do |attachment, parts| - url = get_attachment_url(attachment) - next if url.blank? - - parts << { - type: 'image_url', - image_url: { url: url } - } - end - end - - def finalize_content_parts(parts) - return 'Message without content' if parts.blank? - return parts.first[:text] if single_text_part?(parts) - - parts - end - - def single_text_part?(parts) - parts.one? && parts.first[:type] == 'text' - end - - def get_attachment_url(attachment) - return attachment.external_url if attachment.external_url.present? - - return unless attachment.file.attached? - - attachment.file_url - end - - def extract_audio_transcriptions(attachments) - audio_attachments = attachments.where(file_type: :audio) - return '' if audio_attachments.blank? - - transcriptions = '' - audio_attachments.each do |attachment| - result = Messages::AudioTranscriptionService.new(attachment).perform - transcriptions += result[:transcriptions] if result[:success] - end - transcriptions - end -end \ No newline at end of file diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 976743f74..bceece6c1 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -45,7 +45,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob .where(private: false) .map do |message| { - content: message_content_multimodal(message), + content: prepare_multimodal_message_content(message), role: determine_role(message) } end @@ -57,8 +57,8 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob message.message_type == 'incoming' ? 'user' : 'system' end - def message_content_multimodal(message) - OpenaiMultimodalContentService.new(message).generate_content + def prepare_multimodal_message_content(message) + OpenAiMessageBuilderService.new(message: message).generate_content end def handoff_requested? diff --git a/enterprise/app/services/captain/open_ai_message_builder_service.rb b/enterprise/app/services/captain/open_ai_message_builder_service.rb new file mode 100644 index 000000000..3320ad537 --- /dev/null +++ b/enterprise/app/services/captain/open_ai_message_builder_service.rb @@ -0,0 +1,59 @@ +class Captain::OpenAiMessageBuilderService + pattr_initialize [:message!] + + def generate_content + parts = [] + parts << text_part(@message.content) if @message.content.present? + parts.concat(attachment_parts(@message.attachments)) if @message.attachments.any? + + return 'Message without content' if parts.blank? + return parts.first[:text] if parts.one? && parts.first[:type] == 'text' + + parts + end + + private + + def text_part(text) + { type: 'text', text: text } + end + + def image_part(image_url) + { type: 'image_url', image_url: { url: image_url } } + end + + def attachment_parts(attachments) + image_attachments = attachments.where(file_type: :image) + image_content = image_parts(image_attachments) + + transcription = extract_audio_transcriptions(attachments) + transcription_part = text_part(transcription) if transcription.present? + + attachment_part = text_part('User has shared an attachment') if attachments.where.not(file_type: %i[image audio]).exists? + + [image_content, transcription_part, attachment_part].flatten.compact + end + + def image_parts(image_attachments) + image_attachments.each_with_object([]) do |attachment, parts| + url = get_attachment_url(attachment) + parts << image_part(url) if url.present? + end + end + + def get_attachment_url(attachment) + return attachment.external_url if attachment.external_url.present? + + attachment.file.attached? ? attachment.file_url : nil + end + + def extract_audio_transcriptions(attachments) + audio_attachments = attachments.where(file_type: :audio) + return '' if audio_attachments.blank? + + audio_attachments.map do |attachment| + result = Messages::AudioTranscriptionService.new(attachment).perform + result[:success] ? result[:transcriptions] : '' + end.join + end +end \ No newline at end of file diff --git a/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb b/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb index 6f942ee8e..64b52d012 100644 --- a/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb +++ b/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb @@ -30,7 +30,7 @@ class Captain::Tools::Copilot::GetConversationService < Captain::Tools::BaseServ conversation = Conversation.find_by(display_id: conversation_id, account_id: @assistant.account_id) return 'Conversation not found' if conversation.blank? - conversation.to_llm_text(include_private_messages: true) + conversation.to_llm_text end def active? diff --git a/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb new file mode 100644 index 000000000..1cbc41955 --- /dev/null +++ b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb @@ -0,0 +1,309 @@ +require 'rails_helper' + +RSpec.describe Captain::OpenAiMessageBuilderService do + subject(:service) { described_class.new(message: message) } + + let(:message) { create(:message, content: 'Hello world') } + + describe '#generate_content' do + context 'when message has only text content' do + it 'returns the text content directly' do + expect(service.generate_content).to eq('Hello world') + end + end + + context 'when message has no content and no attachments' do + let(:message) { create(:message, content: nil) } + + it 'returns default message' do + expect(service.generate_content).to eq('Message without content') + end + end + + context 'when message has text content and attachments' do + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + end + + it 'returns an array of content parts' do + result = service.generate_content + expect(result).to be_an(Array) + expect(result).to include({ type: 'text', text: 'Hello world' }) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + end + end + + context 'when message has only non-text attachments' do + let(:message) { create(:message, content: nil) } + + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + end + + it 'returns an array of content parts without text' do + result = service.generate_content + expect(result).to be_an(Array) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + expect(result).not_to include(hash_including(type: 'text', text: 'Hello world')) + end + end + end + + describe '#attachment_parts' do + let(:message) { create(:message, content: nil) } + let(:attachments) { message.attachments } + + context 'with image attachments' do + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + end + + it 'includes image parts' do + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + end + end + + context 'with audio attachments' do + let(:audio_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio_attachment).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'Audio transcription text' }) + ) + end + + it 'includes transcription text part' do + audio_attachment # trigger creation + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'text', text: 'Audio transcription text' }) + end + end + + context 'with other file types' do + before do + attachment = message.attachments.build(account_id: message.account_id, file_type: :file) + attachment.save! + end + + it 'includes generic attachment message' do + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'text', text: 'User has shared an attachment' }) + end + end + + context 'with mixed attachment types' do + let(:image_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image.jpg') + attachment.save! + attachment + end + + let(:audio_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + let(:document_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :file) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio_attachment).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'Audio text' }) + ) + end + + it 'includes all relevant parts' do + image_attachment # trigger creation + audio_attachment # trigger creation + document_attachment # trigger creation + + result = service.send(:attachment_parts, attachments) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + expect(result).to include({ type: 'text', text: 'Audio text' }) + expect(result).to include({ type: 'text', text: 'User has shared an attachment' }) + end + end + end + + describe '#image_parts' do + let(:message) { create(:message, content: nil) } + + context 'with valid image attachments' do + let(:image1) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image1.jpg') + attachment.save! + attachment + end + + let(:image2) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: 'https://example.com/image2.jpg') + attachment.save! + attachment + end + + it 'returns image parts for all valid images' do + image1 # trigger creation + image2 # trigger creation + + image_attachments = message.attachments.where(file_type: :image) + result = service.send(:image_parts, image_attachments) + + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image1.jpg' } }) + expect(result).to include({ type: 'image_url', image_url: { url: 'https://example.com/image2.jpg' } }) + end + end + + context 'with image attachments without URLs' do + let(:image_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image, external_url: nil) + attachment.save! + attachment + end + + before do + allow(image_attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: false)) + end + + it 'skips images without valid URLs' do + image_attachment # trigger creation + + image_attachments = message.attachments.where(file_type: :image) + result = service.send(:image_parts, image_attachments) + + expect(result).to be_empty + end + end + end + + describe '#get_attachment_url' do + let(:attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :image) + attachment.save! + attachment + end + + context 'when attachment has external_url' do + before { attachment.update(external_url: 'https://example.com/image.jpg') } + + it 'returns external_url' do + expect(service.send(:get_attachment_url, attachment)).to eq('https://example.com/image.jpg') + end + end + + context 'when attachment has attached file' do + before do + attachment.update(external_url: nil) + allow(attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: true)) + allow(attachment).to receive(:file_url).and_return('https://local.com/file.jpg') + end + + it 'returns file_url' do + expect(service.send(:get_attachment_url, attachment)).to eq('https://local.com/file.jpg') + end + end + + context 'when attachment has no URL or file' do + before do + attachment.update(external_url: nil) + allow(attachment).to receive(:file).and_return(instance_double(ActiveStorage::Attached::One, attached?: false)) + end + + it 'returns nil' do + expect(service.send(:get_attachment_url, attachment)).to be_nil + end + end + end + + describe '#extract_audio_transcriptions' do + let(:message) { create(:message, content: nil) } + + context 'with no audio attachments' do + it 'returns empty string' do + result = service.send(:extract_audio_transcriptions, message.attachments) + expect(result).to eq('') + end + end + + context 'with successful audio transcriptions' do + let(:audio1) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + let(:audio2) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio1).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'First audio text. ' }) + ) + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio2).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: true, transcriptions: 'Second audio text.' }) + ) + end + + it 'concatenates all successful transcriptions' do + audio1 # trigger creation + audio2 # trigger creation + + attachments = message.attachments + result = service.send(:extract_audio_transcriptions, attachments) + expect(result).to eq('First audio text. Second audio text.') + end + end + + context 'with failed audio transcriptions' do + let(:audio_attachment) do + attachment = message.attachments.build(account_id: message.account_id, file_type: :audio) + attachment.save! + attachment + end + + before do + allow(Messages::AudioTranscriptionService).to receive(:new).with(audio_attachment).and_return( + instance_double(Messages::AudioTranscriptionService, perform: { success: false, transcriptions: nil }) + ) + end + + it 'returns empty string for failed transcriptions' do + audio_attachment # trigger creation + + attachments = message.attachments + result = service.send(:extract_audio_transcriptions, attachments) + expect(result).to eq('') + end + end + end + + describe 'private helper methods' do + describe '#text_part' do + it 'returns correct text part format' do + result = service.send(:text_part, 'Hello world') + expect(result).to eq({ type: 'text', text: 'Hello world' }) + end + end + + describe '#image_part' do + it 'returns correct image part format' do + result = service.send(:image_part, 'https://example.com/image.jpg') + expect(result).to eq({ type: 'image_url', image_url: { url: 'https://example.com/image.jpg' } }) + end + end + end +end \ No newline at end of file