From 7d39f890e4aa7c12f2d71ef236fc1a339cb322de Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Wed, 2 Jul 2025 11:48:14 +0530 Subject: [PATCH 1/2] Revert "feat(revert): "feat: captain image support" (#11841)" This reverts commit a657b45bd1bb9e8ced580ad4a0de4adc77f9510e. --- .../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, 418 insertions(+), 37 deletions(-) create mode 100644 enterprise/app/services/captain/open_ai_message_builder_service.rb create 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 e5a055836..ec8e8e653 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( - params[:message_content], - message_history + additional_message: params[:message_content], + message_history: 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 f341a6e98..431945896 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -26,8 +26,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob def generate_and_process_response @response = Captain::Llm::AssistantChatService.new(assistant: @assistant).generate_response( - @conversation.messages.incoming.last.content, - collect_previous_messages + message_history: collect_previous_messages ) return process_action('handoff') if handoff_requested? @@ -43,33 +42,11 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob .where(message_type: [:incoming, :outgoing]) .where(private: false) .map do |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] + { + content: prepare_multimodal_message_content(message), + role: determine_role(message) + } end - transcriptions end def determine_role(message) @@ -78,6 +55,10 @@ 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 569931d44..ca8fafaa0 100644 --- a/enterprise/app/services/captain/llm/assistant_chat_service.rb +++ b/enterprise/app/services/captain/llm/assistant_chat_service.rb @@ -12,9 +12,16 @@ class Captain::Llm::AssistantChatService < Llm::BaseOpenAiService register_tools end - def generate_response(input, previous_messages = [], role = 'user') - @messages += previous_messages - @messages << { role: role, content: input } if input.present? + # 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? 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 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/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/assistants_controller_spec.rb index 1f6d83d80..80be6f30f 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( - valid_params[:message_content], - valid_params[:message_history] + additional_message: valid_params[:message_content], + message_history: 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( - params_without_history[:message_content], - [] + additional_message: params_without_history[:message_content], + message_history: [] ) 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 1e4a6e824..ca8d4a6c0 100644 --- a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb @@ -30,5 +30,30 @@ 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 new file mode 100644 index 000000000..13c29f756 --- /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 From de6717e7f187a8ca1fd4f53f363f22938b89abc0 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Wed, 2 Jul 2025 11:53:03 +0530 Subject: [PATCH 2/2] fix: Add wait for image response builder (#11837) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Linear https://linear.app/chatwoot/issue/CW-4559/faradaybadrequesterror-the-server-responded-with-status-400#comment-e827f872 ## Description This PR adds retry logic and wait mechanisms for image processing in the Captain conversation response builder. The fix addresses issues where image attachments might not be immediately available when the response builder tries to process multimodal message content, causing failures in the AI assistant's response generation. ## Type of change Please delete options that are not relevant. - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Captain assistant responses with image attachments - Large image processing scenarios - Network timeout conditions during image loading - Multiple retry attempts for failed image access - Conversation flow continuation after image processing failures Screenshot 2025-06-30 at 1 58 26 PM ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Muhsin Keloth --- .../conversation/response_builder_job.rb | 9 +- .../open_ai_message_builder_service.rb | 1 + .../hook_execution_service.rb | 27 ++++- .../conversation/response_builder_job_spec.rb | 112 ++++++++++++++++++ .../open_ai_message_builder_service_spec.rb | 1 + 5 files changed, 141 insertions(+), 9 deletions(-) diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 431945896..b207bd2a4 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -1,6 +1,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob MAX_MESSAGE_LENGTH = 10_000 - retry_on ActiveStorage::FileNotFoundError, attempts: 3 + retry_on ActiveStorage::FileNotFoundError, attempts: 3, wait: 2.seconds + retry_on Faraday::BadRequestError, attempts: 3, wait: 2.seconds def perform(conversation, assistant) @conversation = conversation @@ -13,7 +14,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob generate_and_process_response end rescue StandardError => e - raise e if e.is_a?(ActiveStorage::FileNotFoundError) + raise e if e.is_a?(ActiveStorage::FileNotFoundError) || e.is_a?(Faraday::BadRequestError) handle_error(e) ensure @@ -50,9 +51,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob end def determine_role(message) - return 'system' if message.content.blank? - - message.message_type == 'incoming' ? 'user' : 'system' + message.message_type == 'incoming' ? 'user' : 'assistant' end def prepare_multimodal_message_content(message) diff --git a/enterprise/app/services/captain/open_ai_message_builder_service.rb b/enterprise/app/services/captain/open_ai_message_builder_service.rb index 3320ad537..43d2851c9 100644 --- a/enterprise/app/services/captain/open_ai_message_builder_service.rb +++ b/enterprise/app/services/captain/open_ai_message_builder_service.rb @@ -42,6 +42,7 @@ class Captain::OpenAiMessageBuilderService end def get_attachment_url(attachment) + return attachment.download_url if attachment.download_url.present? return attachment.external_url if attachment.external_url.present? attachment.file.attached? ? attachment.file_url : nil diff --git a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb index 92ccba553..e8faaf74a 100644 --- a/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb +++ b/enterprise/app/services/enterprise/message_templates/hook_execution_service.rb @@ -4,10 +4,29 @@ module Enterprise::MessageTemplates::HookExecutionService return unless should_process_captain_response? return perform_handoff unless inbox.captain_active? - Captain::Conversation::ResponseBuilderJob.perform_later( - conversation, - conversation.inbox.captain_assistant - ) + schedule_captain_response + end + + private + + def schedule_captain_response + job_args = [conversation, conversation.inbox.captain_assistant] + + if message.attachments.blank? + Captain::Conversation::ResponseBuilderJob.perform_later(*job_args) + else + wait_time = calculate_attachment_wait_time + Captain::Conversation::ResponseBuilderJob.set(wait: wait_time).perform_later(*job_args) + end + end + + def calculate_attachment_wait_time + attachment_count = message.attachments.size + base_wait = 1.second + + # Wait longer for more attachments or larger files + additional_wait = [attachment_count * 1, 4].min.seconds + base_wait + additional_wait end def should_process_captain_response? 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..c21205d52 100644 --- a/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/conversation/response_builder_job_spec.rb @@ -56,4 +56,116 @@ RSpec.describe Captain::Conversation::ResponseBuilderJob, type: :job do end end end + + describe 'retry mechanisms for image processing' do + let(:conversation) { create(:conversation, inbox: inbox, account: account) } + let(:mock_llm_chat_service) { instance_double(Captain::Llm::AssistantChatService) } + let(:mock_message_builder) { instance_double(Captain::OpenAiMessageBuilderService) } + + before do + create(:message, conversation: conversation, content: 'Hello with image', message_type: :incoming) + allow(Captain::Llm::AssistantChatService).to receive(:new).and_return(mock_llm_chat_service) + allow(Captain::OpenAiMessageBuilderService).to receive(:new).with(message: anything).and_return(mock_message_builder) + allow(mock_message_builder).to receive(:generate_content).and_return('Hello with image') + allow(mock_llm_chat_service).to receive(:generate_response).and_return({ 'response' => 'Test response' }) + end + + context 'when ActiveStorage::FileNotFoundError occurs' do + it 'handles file errors and triggers handoff' do + allow(mock_message_builder).to receive(:generate_content) + .and_raise(ActiveStorage::FileNotFoundError, 'Image file not found') + + # For retryable errors, the job should handle them and proceed with handoff + described_class.perform_now(conversation, assistant) + + # Verify handoff occurred due to repeated failures + expect(conversation.reload.status).to eq('open') + end + + it 'succeeds when no error occurs' do + # Don't raise any error, should succeed normally + allow(mock_message_builder).to receive(:generate_content) + .and_return('Image content processed successfully') + + described_class.perform_now(conversation, assistant) + + expect(conversation.messages.outgoing.count).to eq(1) + expect(conversation.messages.outgoing.last.content).to eq('Test response') + end + end + + context 'when Faraday::BadRequestError occurs' do + it 'handles API errors and triggers handoff' do + allow(mock_llm_chat_service).to receive(:generate_response) + .and_raise(Faraday::BadRequestError, 'Bad request to image service') + + described_class.perform_now(conversation, assistant) + expect(conversation.reload.status).to eq('open') + end + + it 'succeeds when no error occurs' do + # Don't raise any error, should succeed normally + allow(mock_llm_chat_service).to receive(:generate_response) + .and_return({ 'response' => 'Response after retry' }) + + described_class.perform_now(conversation, assistant) + + expect(conversation.messages.outgoing.last.content).to eq('Response after retry') + end + end + + context 'when image processing fails permanently' do + before do + allow(mock_message_builder).to receive(:generate_content) + .and_raise(ActiveStorage::FileNotFoundError, 'Image permanently unavailable') + end + + it 'triggers handoff after max retries' do + # Since perform_now re-raises retryable errors, simulate the final failure after retries + allow(mock_message_builder).to receive(:generate_content) + .and_raise(StandardError, 'Max retries exceeded') + + expect(ChatwootExceptionTracker).to receive(:new).and_call_original + + described_class.perform_now(conversation, assistant) + + expect(conversation.reload.status).to eq('open') + end + end + + context 'when non-retryable error occurs' do + let(:standard_error) { StandardError.new('Generic error') } + + before do + allow(mock_llm_chat_service).to receive(:generate_response).and_raise(standard_error) + end + + it 'handles error and triggers handoff' do + expect(ChatwootExceptionTracker).to receive(:new) + .with(standard_error, account: account) + .and_call_original + + described_class.perform_now(conversation, assistant) + + expect(conversation.reload.status).to eq('open') + end + + it 'ensures Current.executed_by is reset' do + expect(Current).to receive(:executed_by=).with(assistant) + expect(Current).to receive(:executed_by=).with(nil) + + described_class.perform_now(conversation, assistant) + end + end + end + + describe 'job configuration' do + it 'has retry_on configuration for retryable errors' do + expect(described_class).to respond_to(:retry_on) + end + + it 'defines MAX_MESSAGE_LENGTH constant' do + expect(described_class::MAX_MESSAGE_LENGTH).to eq(10_000) + 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 index 13c29f756..76e91ae7a 100644 --- a/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb +++ b/spec/enterprise/services/captain/open_ai_message_builder_service_spec.rb @@ -207,6 +207,7 @@ RSpec.describe Captain::OpenAiMessageBuilderService 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') + allow(attachment).to receive(:download_url).and_return('') end it 'returns file_url' do