From a657b45bd1bb9e8ced580ad4a0de4adc77f9510e Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Tue, 1 Jul 2025 00:12:07 +0530 Subject: [PATCH 1/2] feat(revert): "feat: captain image support" (#11841) Reverts chatwoot/chatwoot#11730 --- .../accounts/captain/assistants_controller.rb | 4 +- .../conversation/response_builder_job.rb | 37 ++- .../captain/llm/assistant_chat_service.rb | 13 +- .../open_ai_message_builder_service.rb | 59 ---- .../captain/assistants_controller_spec.rb | 8 +- .../conversation/response_builder_job_spec.rb | 25 -- .../open_ai_message_builder_service_spec.rb | 309 ------------------ 7 files changed, 37 insertions(+), 418 deletions(-) delete mode 100644 enterprise/app/services/captain/open_ai_message_builder_service.rb delete mode 100644 spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb diff --git a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb index ec8e8e653..e5a055836 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/assistants_controller.rb @@ -25,8 +25,8 @@ class Api::V1::Accounts::Captain::AssistantsController < Api::V1::Accounts::Base def playground response = Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( - additional_message: params[:message_content], - message_history: message_history + params[:message_content], + message_history ) render json: response diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 431945896..f341a6e98 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -26,7 +26,8 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob def generate_and_process_response @response = Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( - message_history: collect_previous_messages + @conversation.messages.incoming.last.content, + collect_previous_messages ) return process_action('handoff') if handoff_requested? @@ -42,11 +43,33 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob .where(message_type: [:incoming, :outgoing]) .where(private: false) .map do |message| - { - content: prepare_multimodal_message_content(message), - role: determine_role(message) - } + { + content: message_content(message), + role: determine_role(message) + } + end + end + + def message_content(message) + return message.content if message.content.present? + return 'User has shared a message without content' unless message.attachments.any? + + audio_transcriptions = extract_audio_transcriptions(message.attachments) + return audio_transcriptions if audio_transcriptions.present? + + 'User has shared an attachment' + 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 def determine_role(message) @@ -55,10 +78,6 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob message.message_type == 'incoming' ? 'user' : 'system' end - def prepare_multimodal_message_content(message) - Captain::OpenAiMessageBuilderService.new(message: message).generate_content - end - def handoff_requested? @response['response'] == 'conversation_handoff' end diff --git a/enterprise/app/services/captain/llm/assistant_chat_service.rb b/enterprise/app/services/captain/llm/assistant_chat_service.rb index ca8fafaa0..569931d44 100644 --- a/enterprise/app/services/captain/llm/assistant_chat_service.rb +++ b/enterprise/app/services/captain/llm/assistant_chat_service.rb @@ -12,16 +12,9 @@ class Captain::Llm::AssistantChatService < Llm::BaseOpenAiService register_tools end - # additional_message: A single message (String) from the user that should be appended to the chat. - # It can be an empty String or nil when you only want to supply historical messages. - # message_history: An Array of already formatted messages that provide the previous context. - # role: The role for the additional_message (defaults to `user`). - # - # NOTE: Parameters are provided as keyword arguments to improve clarity and avoid relying on - # positional ordering. - def generate_response(additional_message: nil, message_history: [], role: 'user') - @messages += message_history - @messages << { role: role, content: additional_message } if additional_message.present? + def generate_response(input, previous_messages = [], role = 'user') + @messages += previous_messages + @messages << { role: role, content: input } if input.present? request_chat_completion end diff --git a/enterprise/app/services/captain/open_ai_message_builder_service.rb b/enterprise/app/services/captain/open_ai_message_builder_service.rb deleted file mode 100644 index 3320ad537..000000000 --- a/enterprise/app/services/captain/open_ai_message_builder_service.rb +++ /dev/null @@ -1,59 +0,0 @@ -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/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb index 80be6f30f..1f6d83d80 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb @@ -211,8 +211,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do expect(response).to have_http_status(:success) expect(chat_service).to have_received(:generate_response).with( - additional_message: valid_params[:message_content], - message_history: valid_params[:message_history] + valid_params[:message_content], + valid_params[:message_history] ) expect(json_response[:content]).to eq('Assistant response') end @@ -232,8 +232,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Assistants', type: :request do expect(response).to have_http_status(:success) expect(chat_service).to have_received(:generate_response).with( - additional_message: params_without_history[:message_content], - message_history: [] + params_without_history[:message_content], + [] ) end end diff --git a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb index ca8d4a6c0..1e4a6e824 100644 --- a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb @@ -30,30 +30,5 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do account.reload expect(account.usage_limits[:captain][:responses][:consumed]).to eq(1) end - - context 'when message contains an image' do - let(:message_with_image) { create(:message, conversation: conversation, message_type: :incoming, content: 'Can you help with this error?') } - let(:image_attachment) { message_with_image.attachments.create!(account: account, file_type: :image, external_url: 'https://example.com/error.jpg') } - - before do - image_attachment - end - - it 'includes image URL directly in the message content for OpenAI vision analysis' do - # Expect the generate_response to receive multimodal content with image URL - expect(mock_llm_chat_service).to receive(:generate_response) do |**kwargs| - history = kwargs[:message_history] - last_entry = history.last - expect(last_entry[:content]).to be_an(Array) - expect(last_entry[:content].any? { |part| part[:type] == 'text' && part[:text] == 'Can you help with this error?' }).to be true - expect(last_entry[:content].any? do |part| - part[:type] == 'image_url' && part[:image_url][:url] == 'https://example.com/error.jpg' - end).to be true - { 'response' => 'I can see the error in your image. It appears to be a database connection issue.' } - end - - described_class.perform_now(conversation, assistant) - end - end end end 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 deleted file mode 100644 index 13c29f756..000000000 --- a/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb +++ /dev/null @@ -1,309 +0,0 @@ -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 From 58da92a252bd03437b42b1c2d281bb4cb0b2164e Mon Sep 17 00:00:00 2001 From: Pranav Date: Mon, 30 Jun 2025 19:06:25 -0700 Subject: [PATCH 2/2] chore: Disable copilot usage after the response count is over (#11845) Disable copilot if the response usage is over. --- config/locales/en.yml | 1 + .../captain/copilot_threads_controller.rb | 15 ++++++++-- .../copilot_threads_controller_spec.rb | 30 ++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index a41a009cf..221f83472 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -270,6 +270,7 @@ en: short_description: 'Sync your contacts and conversations with LeadSquared CRM.' description: 'Sync your contacts and conversations with LeadSquared CRM. This integration automatically creates leads in LeadSquared when new contacts are added, and logs conversation activity to provide your sales team with complete context.' captain: + copilot_message_required: Message is required copilot_error: 'Please connect an assistant to this inbox to use Copilot' copilot_limit: 'You are out of Copilot credits. You can buy more credits from the billing section.' copilot: diff --git a/enterprise/app/controllers/api/v1/accounts/captain/copilot_threads_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/copilot_threads_controller.rb index 533859b95..8ea85145e 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/copilot_threads_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/copilot_threads_controller.rb @@ -23,14 +23,25 @@ class Api::V1::Accounts::Captain::CopilotThreadsController < Api::V1::Accounts:: message: { content: copilot_thread_params[:message] } ) - copilot_message.enqueue_response_job(copilot_thread_params[:conversation_id], Current.user.id) + build_copilot_response(copilot_message) end end private + def build_copilot_response(copilot_message) + if Current.account.usage_limits[:captain][:responses][:current_available].positive? + copilot_message.enqueue_response_job(copilot_thread_params[:conversation_id], Current.user.id) + else + copilot_message.copilot_thread.copilot_messages.create!( + message_type: :assistant, + message: { content: I18n.t('captain.copilot_limit') } + ) + end + end + def ensure_message - return render_could_not_create_error('Message is required') if copilot_thread_params[:message].blank? + return render_could_not_create_error(I18n.t('captain.copilot_message_required')) if copilot_thread_params[:message].blank? end def assistant diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/copilot_threads_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/copilot_threads_controller_spec.rb index b8fc628d1..8b561f22a 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/copilot_threads_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/copilot_threads_controller_spec.rb @@ -86,7 +86,28 @@ RSpec.describe 'Api::V1::Accounts::Captain::CopilotThreads', type: :request do end context 'with valid params' do + it 'returns error when usage limit is exceeded' do + account.limits = { captain_responses: 2 } + account.custom_attributes = { captain_responses_usage: 2 } + account.save! + + post "/api/v1/accounts/#{account.id}/captain/copilot_threads", + params: valid_params, + headers: agent.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + + expect(CopilotMessage.last.message['content']).to eq( + 'You are out of Copilot credits. You can buy more credits from the billing section.' + ) + end + it 'creates a new copilot thread with initial message' do + account.limits = { captain_responses: 2 } + account.custom_attributes = { captain_responses_usage: 0 } + account.save! + expect do post "/api/v1/accounts/#{account.id}/captain/copilot_threads", params: valid_params, @@ -103,8 +124,15 @@ RSpec.describe 'Api::V1::Accounts::Captain::CopilotThreads', type: :request do expect(thread.assistant_id).to eq(assistant.id) message = thread.copilot_messages.last - expect(message.message_type).to eq('user') expect(message.message).to eq({ 'content' => valid_params[:message] }) + + expect(Captain::Copilot::ResponseJob).to have_been_enqueued.with( + assistant: assistant, + conversation_id: valid_params[:conversation_id], + user_id: agent.id, + copilot_thread_id: thread.id, + message: valid_params[:message] + ) end end end