diff --git a/config/locales/en.yml b/config/locales/en.yml index b7eb17ada..32d19a6dc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -297,8 +297,21 @@ en: pdf_format_error: 'must be a PDF file' pdf_size_error: 'must be less than 10MB' pdf_upload_failed: 'Failed to upload PDF to OpenAI' + pdf_upload_success: 'PDF uploaded successfully with file_id: %{file_id}' pdf_processing_failed: 'Failed to process PDF document %{document_id}: %{error}' pdf_processing_success: 'Successfully processed PDF document %{document_id}' + faq_generation_complete: 'FAQ generation complete. Total FAQs created: %{count}' + using_paginated_faq: 'Using paginated FAQ generation for document %{document_id}' + using_standard_faq: 'Using standard FAQ generation for document %{document_id}' + response_creation_error: 'Error in creating response document: %{error}' + missing_openai_file_id: 'Document must have openai_file_id for paginated processing' + openai_api_error: 'OpenAI API Error: %{error}' + starting_paginated_faq: 'Starting paginated FAQ generation (%{pages_per_chunk} pages per chunk)' + stopping_faq_generation: 'Stopping processing. Reason: %{reason}' + paginated_faq_complete: 'Paginated generation complete. Total FAQs: %{total_faqs}, Pages processed: %{pages_processed}' + processing_pages: 'Processing pages %{start}-%{end} (iteration %{iteration})' + chunk_generated: 'Chunk generated %{chunk_faqs} FAQs. Total so far: %{total_faqs}' + page_processing_error: 'Error processing pages %{start}-%{end}: %{error}' public_portal: search: search_placeholder: Search for article by title or body... diff --git a/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb b/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb index 26ca7c47b..32fa0a7b6 100644 --- a/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb @@ -2,15 +2,16 @@ class Api::V1::Accounts::Captain::DocumentsController < Api::V1::Accounts::BaseC before_action :current_account before_action -> { check_authorization(Captain::Assistant) } + before_action :set_current_page, only: [:index] + before_action :set_documents, except: [:create] before_action :set_document, only: [:show, :destroy] - + before_action :set_assistant, only: [:create] RESULTS_PER_PAGE = 25 def index - base_query = account_documents.includes(:assistant) + base_query = @documents base_query = base_query.where(assistant_id: permitted_params[:assistant_id]) if permitted_params[:assistant_id].present? - @current_page = (permitted_params[:page] || 1).to_i @documents_count = base_query.count @documents = base_query.page(@current_page).per(RESULTS_PER_PAGE) end @@ -18,7 +19,9 @@ class Api::V1::Accounts::Captain::DocumentsController < Api::V1::Accounts::BaseC def show; end def create - @document = account_documents.build(document_params) + return render_could_not_create_error('Missing Assistant') if @assistant.nil? + + @document = @assistant.documents.build(document_params) @document.save! rescue Captain::Document::LimitExceededError => e render_could_not_create_error(e.message) @@ -27,18 +30,26 @@ class Api::V1::Accounts::Captain::DocumentsController < Api::V1::Accounts::BaseC end def destroy - @document.destroy! + @document.destroy head :no_content end private - def account_documents - @account_documents ||= Current.account.captain_documents.ordered + def set_documents + @documents = Current.account.captain_documents.includes(:assistant).ordered end def set_document - @document = account_documents.find(permitted_params[:id]) + @document = @documents.find(permitted_params[:id]) + end + + def set_assistant + @assistant = Current.account.captain_assistants.find_by(id: document_params[:assistant_id]) + end + + def set_current_page + @current_page = permitted_params[:page] || 1 end def permitted_params diff --git a/enterprise/app/jobs/captain/documents/crawl_job.rb b/enterprise/app/jobs/captain/documents/crawl_job.rb index dd0091df6..3fa5b6d56 100644 --- a/enterprise/app/jobs/captain/documents/crawl_job.rb +++ b/enterprise/app/jobs/captain/documents/crawl_job.rb @@ -16,10 +16,8 @@ class Captain::Documents::CrawlJob < ApplicationJob include Captain::FirecrawlHelper def perform_pdf_processing(document) - pdf_processor = Captain::Llm::PdfProcessingService.new(document) - pdf_processor.process + Captain::Llm::PdfProcessingService.new(document).process document.update!(status: :available) - Rails.logger.info I18n.t('captain.documents.pdf_processing_success', document_id: document.id) rescue StandardError => e Rails.logger.error I18n.t('captain.documents.pdf_processing_failed', document_id: document.id, error: e.message) raise # Re-raise to let job framework handle retry logic diff --git a/enterprise/app/jobs/captain/documents/response_builder_job.rb b/enterprise/app/jobs/captain/documents/response_builder_job.rb index 9971b6063..41b9f0e42 100644 --- a/enterprise/app/jobs/captain/documents/response_builder_job.rb +++ b/enterprise/app/jobs/captain/documents/response_builder_job.rb @@ -6,10 +6,10 @@ class Captain::Documents::ResponseBuilderJob < ApplicationJob faqs = generate_faqs(document, options) create_responses_from_faqs(faqs, document) - - Rails.logger.info "FAQ generation complete. Total FAQs created: #{faqs.size}" end + private + def generate_faqs(document, options) if should_use_pagination?(document) generate_paginated_faqs(document, options) @@ -19,7 +19,6 @@ class Captain::Documents::ResponseBuilderJob < ApplicationJob end def generate_paginated_faqs(document, options) - Rails.logger.info "Using paginated FAQ generation for document #{document.id}" service = build_paginated_service(document, options) faqs = service.generate store_paginated_metadata(document, service) @@ -27,7 +26,6 @@ class Captain::Documents::ResponseBuilderJob < ApplicationJob end def generate_standard_faqs(document) - Rails.logger.info "Using standard FAQ generation for document #{document.id}" Captain::Llm::FaqGeneratorService.new(document.content).generate end @@ -56,8 +54,6 @@ class Captain::Documents::ResponseBuilderJob < ApplicationJob faqs.each { |faq| create_response(faq, document) } end - private - def should_use_pagination?(document) # Auto-detect when to use pagination # For now, use pagination for PDFs with OpenAI file ID @@ -76,6 +72,6 @@ class Captain::Documents::ResponseBuilderJob < ApplicationJob documentable: document ) rescue ActiveRecord::RecordInvalid => e - Rails.logger.error "Error in creating response document: #{e.message}" + Rails.logger.error I18n.t('captain.documents.response_creation_error', error: e.message) end end diff --git a/enterprise/app/models/captain/document.rb b/enterprise/app/models/captain/document.rb index 944686c26..c278eb879 100644 --- a/enterprise/app/models/captain/document.rb +++ b/enterprise/app/models/captain/document.rb @@ -54,7 +54,9 @@ class Captain::Document < ApplicationRecord scope :for_assistant, ->(assistant_id) { where(assistant_id: assistant_id) } def pdf_document? - (external_link&.ends_with?('.pdf')) || (pdf_file.attached? && pdf_file.blob.content_type == 'application/pdf') + return true if pdf_file.attached? && pdf_file.blob.content_type == 'application/pdf' + + external_link&.ends_with?('.pdf') end def content_type diff --git a/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb b/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb index a87afeec8..006a01b62 100644 --- a/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb +++ b/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb @@ -16,7 +16,7 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::BaseOpenAiService end def generate - raise 'Document must have openai_file_id for paginated processing' if @document&.openai_file_id.blank? + raise CustomExceptions::PdfFaqGenerationError, I18n.t('captain.documents.missing_openai_file_id') if @document&.openai_file_id.blank? generate_paginated_faqs end @@ -45,13 +45,11 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::BaseOpenAiService response = @client.chat(parameters: standard_chat_parameters) parse_response(response) rescue OpenAI::Error => e - Rails.logger.error "OpenAI API Error: #{e.message}" + Rails.logger.error I18n.t('captain.documents.openai_api_error', error: e.message) [] end def generate_paginated_faqs - Rails.logger.info "Starting paginated FAQ generation (#{@pages_per_chunk} pages per chunk)" - all_faqs = [] current_page = 1 @@ -59,15 +57,11 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::BaseOpenAiService end_page = calculate_end_page(current_page) chunk_result = process_chunk_and_update_state(current_page, end_page, all_faqs) - unless should_continue_processing?(chunk_result) - Rails.logger.info "Stopping processing. Reason: #{determine_stop_reason(chunk_result)}" - break - end + break unless should_continue_processing?(chunk_result) current_page = end_page + 1 end - Rails.logger.info "Paginated generation complete. Total FAQs: #{all_faqs.size}, Pages processed: #{@total_pages_processed}" deduplicate_faqs(all_faqs) end @@ -77,8 +71,6 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::BaseOpenAiService end def process_chunk_and_update_state(current_page, end_page, all_faqs) - Rails.logger.info "Processing pages #{current_page}-#{end_page} (iteration #{@iterations_completed + 1})" - chunk_result = process_page_chunk(current_page, end_page) chunk_faqs = chunk_result[:faqs] @@ -86,7 +78,6 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::BaseOpenAiService @total_pages_processed = end_page @iterations_completed += 1 - Rails.logger.info "Chunk generated #{chunk_faqs.size} FAQs. Total so far: #{all_faqs.size}" chunk_result end @@ -96,7 +87,7 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::BaseOpenAiService result = parse_chunk_response(response) { faqs: result['faqs'] || [], has_content: result['has_content'] != false } rescue OpenAI::Error => e - Rails.logger.error "Error processing pages #{start_page}-#{end_page}: #{e.message}" + Rails.logger.error I18n.t('captain.documents.page_processing_error', start: start_page, end: end_page, error: e.message) { faqs: [], has_content: false } end diff --git a/enterprise/app/services/captain/llm/pdf_processing_service.rb b/enterprise/app/services/captain/llm/pdf_processing_service.rb index ab7af212d..026ef4e48 100644 --- a/enterprise/app/services/captain/llm/pdf_processing_service.rb +++ b/enterprise/app/services/captain/llm/pdf_processing_service.rb @@ -1,11 +1,16 @@ class Captain::Llm::PdfProcessingService < Llm::BaseOpenAiService def initialize(document) - @document = document super() + @document = document end def process - process_for_pagination + return if document.openai_file_id.present? + + file_id = upload_pdf_to_openai + raise CustomExceptions::PdfUploadError, I18n.t('captain.documents.pdf_upload_failed') if file_id.blank? + + document.store_openai_file_id(file_id) end private @@ -13,34 +18,23 @@ class Captain::Llm::PdfProcessingService < Llm::BaseOpenAiService attr_reader :document def upload_pdf_to_openai - pdf_file = document.pdf_file - - Tempfile.create(['pdf_upload', '.pdf'], binmode: true) do |temp_file| - temp_file.write(pdf_file.download) - temp_file.close - - File.open(temp_file.path, 'rb') do |file| - @client.files.upload( - parameters: { - file: file, - purpose: 'assistants' - } - ) - end + with_tempfile do |temp_file| + response = @client.files.upload( + parameters: { + file: temp_file, + purpose: 'assistants' + } + ) + response['id'] end end - def process_for_pagination - return 'PDF ready for paginated processing' if document.openai_file_id.present? + def with_tempfile(&) + Tempfile.create(['pdf_upload', '.pdf'], binmode: true) do |temp_file| + temp_file.write(document.pdf_file.download) + temp_file.close - openai_response = upload_pdf_to_openai - file_id = openai_response['id'] - - raise I18n.t('captain.documents.pdf_upload_failed') if file_id.blank? - - document.store_openai_file_id(file_id) - Rails.logger.info "PDF uploaded successfully with file_id: #{file_id}" - - "PDF ready for paginated processing (file_id: #{file_id})" + File.open(temp_file.path, 'rb', &) + end end end diff --git a/lib/custom_exceptions/pdf_processing_error.rb b/lib/custom_exceptions/pdf_processing_error.rb index 116bf3ead..2c81f95d8 100644 --- a/lib/custom_exceptions/pdf_processing_error.rb +++ b/lib/custom_exceptions/pdf_processing_error.rb @@ -1,23 +1,25 @@ -class PdfProcessingError < CustomExceptions::Base - def initialize(message = 'PDF processing failed') - super(message) +module CustomExceptions + class PdfProcessingError < Base + def initialize(message = 'PDF processing failed') + super(message) + end end -end -class PdfUploadError < PdfProcessingError - def initialize(message = 'PDF upload failed') - super(message) + class PdfUploadError < PdfProcessingError + def initialize(message = 'PDF upload failed') + super(message) + end end -end -class PdfValidationError < PdfProcessingError - def initialize(message = 'PDF validation failed') - super(message) + class PdfValidationError < PdfProcessingError + def initialize(message = 'PDF validation failed') + super(message) + end end -end -class PdfFaqGenerationError < PdfProcessingError - def initialize(message = 'PDF FAQ generation failed') - super(message) + class PdfFaqGenerationError < PdfProcessingError + def initialize(message = 'PDF FAQ generation failed') + super(message) + end end end diff --git a/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb b/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb index b7ab1a7f1..a2dffcf04 100644 --- a/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb +++ b/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb @@ -1,157 +1,132 @@ require 'rails_helper' RSpec.describe Captain::Documents::CrawlJob, type: :job do - let(:account) { create(:account) } - let(:assistant) { create(:captain_assistant, account: account) } - let(:url_document) { create(:captain_document, assistant: assistant, account: account, external_link: 'https://example.com') } - let(:pdf_document) do - doc = create(:captain_document, assistant: assistant, account: account) - doc.pdf_file.attach( - io: File.open(Rails.root.join('spec/fixtures/files/sample.pdf')), - filename: 'sample.pdf', - content_type: 'application/pdf' - ) - doc - end + let(:document) { create(:captain_document, external_link: 'https://example.com/page') } + let(:assistant_id) { document.assistant_id } + let(:webhook_url) { Rails.application.routes.url_helpers.enterprise_webhooks_firecrawl_url } describe '#perform' do - context 'when document has external link' do - it 'performs simple crawling when no firecrawl API key' do - allow(InstallationConfig).to receive(:find_by).and_return(nil) - job = described_class.new - expect(job).to receive(:perform_simple_crawl).with(url_document) - job.perform(url_document) + context 'when CAPTAIN_FIRECRAWL_API_KEY is configured' do + let(:firecrawl_service) { instance_double(Captain::Tools::FirecrawlService) } + let(:account) { document.account } + let(:token) { Digest::SHA256.hexdigest("-key#{document.assistant_id}#{document.account_id}") } + + before do + allow(Captain::Tools::FirecrawlService).to receive(:new).and_return(firecrawl_service) + allow(firecrawl_service).to receive(:perform) + create(:installation_config, name: 'CAPTAIN_FIRECRAWL_API_KEY', value: 'test-key') end - it 'does not perform PDF processing' do - allow(InstallationConfig).to receive(:find_by).and_return(nil) - # Mock the HTTP request made by SimplePageCrawlService - stub_request(:get, 'https://example.com/').to_return(status: 200, body: '', headers: {}) + context 'with account usage limits' do + before do + allow(account).to receive(:usage_limits).and_return({ captain: { documents: { current_available: 20 } } }) + end - simple_crawl_service = instance_double(Captain::Tools::SimplePageCrawlService) - allow(Captain::Tools::SimplePageCrawlService).to receive(:new).and_return(simple_crawl_service) - allow(simple_crawl_service).to receive(:page_links).and_return([]) + it 'uses FirecrawlService with the correct crawl limit' do + expect(firecrawl_service).to receive(:perform).with( + document.external_link, + "#{webhook_url}?assistant_id=#{assistant_id}&token=#{token}", + 20 + ) - job = described_class.new - expect(job).not_to receive(:perform_pdf_processing) - job.perform(url_document) - end - end - - context 'when document has PDF file' do - it 'performs PDF processing' do - job = described_class.new - expect(job).to receive(:perform_pdf_processing).with(pdf_document) - job.perform(pdf_document) + described_class.perform_now(document) + end end - it 'does not perform web crawling' do - job = described_class.new - allow(job).to receive(:perform_pdf_processing) - expect(job).not_to receive(:perform_simple_crawl) - expect(job).not_to receive(:perform_firecrawl_crawl) - job.perform(pdf_document) + context 'when crawl limit exceeds maximum' do + before do + allow(account).to receive(:usage_limits).and_return({ captain: { documents: { current_available: 1000 } } }) + end + + it 'caps the crawl limit at 500' do + expect(firecrawl_service).to receive(:perform).with( + document.external_link, + "#{webhook_url}?assistant_id=#{assistant_id}&token=#{token}", + 500 + ) + + described_class.perform_now(document) + end end - it 'calls PDF processing service' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - expect(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - expect(pdf_processing_service).to receive(:process) + context 'with no usage limits configured' do + before do + allow(account).to receive(:usage_limits).and_return({}) + end - described_class.perform_now(pdf_document) - end + it 'uses default crawl limit of 10' do + expect(firecrawl_service).to receive(:perform).with( + document.external_link, + "#{webhook_url}?assistant_id=#{assistant_id}&token=#{token}", + 10 + ) - it 'marks document as available after processing' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - allow(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - allow(pdf_processing_service).to receive(:process) - - expect { described_class.perform_now(pdf_document) } - .to change { pdf_document.reload.status }.to('available') - end - - it 'logs successful PDF processing' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - allow(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - allow(pdf_processing_service).to receive(:process) - allow(Rails.logger).to receive(:info) - - described_class.perform_now(pdf_document) - - expect(Rails.logger).to have_received(:info).with(I18n.t('captain.documents.pdf_processing_success', document_id: pdf_document.id)) - end - - context 'when PDF processing fails' do - it 'logs error and re-raises' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - allow(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - allow(pdf_processing_service).to receive(:process).and_raise(StandardError, 'Processing failed') - - allow(Rails.logger).to receive(:error) - - expect { described_class.perform_now(pdf_document) } - .to raise_error(StandardError, 'Processing failed') - - expect(Rails.logger).to have_received(:error).with(I18n.t('captain.documents.pdf_processing_failed', document_id: pdf_document.id, - error: 'Processing failed')) + described_class.perform_now(document) end end end - context 'when document is nil' do - it 'raises NoMethodError' do - expect { described_class.perform_now(nil) } - .to raise_error(NoMethodError) + context 'when CAPTAIN_FIRECRAWL_API_KEY is not configured' do + let(:page_links) { ['https://example.com/page1', 'https://example.com/page2'] } + let(:simple_crawler) { instance_double(Captain::Tools::SimplePageCrawlService) } + + before do + allow(Captain::Tools::SimplePageCrawlService) + .to receive(:new) + .with(document.external_link) + .and_return(simple_crawler) + + allow(simple_crawler).to receive(:page_links).and_return(page_links) + end + + it 'enqueues SimplePageCrawlParserJob for each discovered link' do + page_links.each do |link| + expect(Captain::Tools::SimplePageCrawlParserJob) + .to receive(:perform_later) + .with( + assistant_id: assistant_id, + page_link: link + ) + end + + # Should also crawl the original link + expect(Captain::Tools::SimplePageCrawlParserJob) + .to receive(:perform_later) + .with( + assistant_id: assistant_id, + page_link: document.external_link + ) + + described_class.perform_now(document) + end + + it 'uses SimplePageCrawlService to discover page links' do + expect(simple_crawler).to receive(:page_links) + described_class.perform_now(document) end end - context 'when document has external link but no PDF file' do - let(:web_document) { create(:captain_document, assistant: assistant, account: account, external_link: 'https://example.com') } - - it 'performs simple crawl when no firecrawl API key' do - allow(InstallationConfig).to receive(:find_by).and_return(nil) - job = described_class.new - expect(job).to receive(:perform_simple_crawl).with(web_document) - expect(job).not_to receive(:perform_pdf_processing) - - job.perform(web_document) + context 'when document is a PDF' do + let(:pdf_document) do + doc = create(:captain_document, external_link: 'https://example.com/document') + allow(doc).to receive(:pdf_document?).and_return(true) + allow(doc).to receive(:update!).and_return(true) + doc end - end - end - describe '#perform_pdf_processing' do - let(:job_instance) { described_class.new } + it 'processes PDF using PdfProcessingService' do + pdf_service = instance_double(Captain::Llm::PdfProcessingService) + expect(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_service) + expect(pdf_service).to receive(:process) + expect(pdf_document).to receive(:update!).with(status: :available) - it 'creates PDF processing service with correct parameters' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - expect(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - expect(pdf_processing_service).to receive(:process) + described_class.perform_now(pdf_document) + end - job_instance.send(:perform_pdf_processing, pdf_document) - end + it 'handles PDF processing errors' do + allow(Captain::Llm::PdfProcessingService).to receive(:new).and_raise(StandardError, 'Processing failed') - it 'marks document as available after processing' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - allow(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - allow(pdf_processing_service).to receive(:process) - - expect { job_instance.send(:perform_pdf_processing, pdf_document) } - .to change { pdf_document.reload.status }.to('available') - end - - context 'when processing fails' do - it 'logs error and re-raises' do - pdf_processing_service = instance_double(Captain::Llm::PdfProcessingService) - allow(Captain::Llm::PdfProcessingService).to receive(:new).with(pdf_document).and_return(pdf_processing_service) - allow(pdf_processing_service).to receive(:process).and_raise(StandardError, 'Test error') - - allow(Rails.logger).to receive(:error) - - expect { job_instance.send(:perform_pdf_processing, pdf_document) } - .to raise_error(StandardError, 'Test error') - - expect(Rails.logger).to have_received(:error).with(I18n.t('captain.documents.pdf_processing_failed', document_id: pdf_document.id, - error: 'Test error')) + expect { described_class.perform_now(pdf_document) }.to raise_error(StandardError, 'Processing failed') end end end diff --git a/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb b/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb index 6182fcbd1..59d9abe85 100644 --- a/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb @@ -1,229 +1,83 @@ require 'rails_helper' RSpec.describe Captain::Documents::ResponseBuilderJob, type: :job do - let(:account) { create(:account) } - let(:assistant) { create(:captain_assistant, account: account) } - let(:document) { create(:captain_document, assistant: assistant, account: account) } + let(:assistant) { create(:captain_assistant) } + let(:document) { create(:captain_document, assistant: assistant) } + let(:faq_generator) { instance_double(Captain::Llm::FaqGeneratorService) } + let(:faqs) do + [ + { 'question' => 'What is Ruby?', 'answer' => 'A programming language' }, + { 'question' => 'What is Rails?', 'answer' => 'A web framework' } + ] + end + + before do + allow(Captain::Llm::FaqGeneratorService).to receive(:new) + .with(document.content) + .and_return(faq_generator) + allow(faq_generator).to receive(:generate).and_return(faqs) + end describe '#perform' do - before do - # Mock the InstallationConfig for services that need it - allow(InstallationConfig).to receive(:find_by!) - .with(name: 'CAPTAIN_OPEN_AI_API_KEY') - .and_return(instance_double(InstallationConfig, value: 'test-api-key')) + context 'when processing a document' do + it 'deletes previous responses' do + existing_response = create(:captain_assistant_response, documentable: document) + + described_class.new.perform(document) + + expect { existing_response.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'creates new responses for each FAQ' do + expect do + described_class.new.perform(document) + end.to change(Captain::AssistantResponse, :count).by(2) + + responses = document.responses.reload + expect(responses.count).to eq(2) + + first_response = responses.first + expect(first_response.question).to eq('What is Ruby?') + expect(first_response.answer).to eq('A programming language') + expect(first_response.assistant).to eq(assistant) + expect(first_response.documentable).to eq(document) + end end - context 'when document requires pagination' do + context 'when processing a PDF document' do + let(:pdf_document) do + doc = create(:captain_document, assistant: assistant) + allow(doc).to receive(:pdf_document?).and_return(true) + allow(doc).to receive(:openai_file_id).and_return('file-123') + allow(doc).to receive(:update!).and_return(true) + allow(doc).to receive(:metadata).and_return({}) + doc + end + let(:paginated_service) { instance_double(Captain::Llm::PaginatedFaqGeneratorService) } + let(:pdf_faqs) do + [{ 'question' => 'What is in the PDF?', 'answer' => 'Important content' }] + end + before do - allow(document).to receive(:pdf_document?).and_return(true) - allow(document).to receive(:openai_file_id).and_return('file-123') - allow(document).to receive(:update!).and_return(true) - allow(document).to receive(:metadata).and_return({}) - end - - it 'uses paginated FAQ generator service' do - paginated_service = instance_double(Captain::Llm::PaginatedFaqGeneratorService) - - expect(Captain::Llm::PaginatedFaqGeneratorService).to receive(:new) - .with(document, hash_including(:pages_per_chunk, :max_pages)) - .and_return(paginated_service) - - expect(paginated_service).to receive(:generate).and_return([]) - expect(paginated_service).to receive(:total_pages_processed).and_return(10) - expect(paginated_service).to receive(:iterations_completed).and_return(1) - - described_class.perform_now(document) - end - - it 'accepts pagination options' do - options = { pages_per_chunk: 5, max_pages: 20 } - paginated_service = instance_double(Captain::Llm::PaginatedFaqGeneratorService) - - expect(Captain::Llm::PaginatedFaqGeneratorService).to receive(:new) - .with(document, pages_per_chunk: 5, max_pages: 20) - .and_return(paginated_service) - - expect(paginated_service).to receive(:generate).and_return([]) - expect(paginated_service).to receive(:total_pages_processed).and_return(20) - expect(paginated_service).to receive(:iterations_completed).and_return(4) - - described_class.perform_now(document, options) - end - - it 'stores metadata about paginated generation' do - paginated_service = instance_double(Captain::Llm::PaginatedFaqGeneratorService) - allow(Captain::Llm::PaginatedFaqGeneratorService).to receive(:new) + .with(pdf_document, anything) .and_return(paginated_service) - allow(paginated_service).to receive(:generate).and_return([]) - allow(paginated_service).to receive(:total_pages_processed).and_return(15) - allow(paginated_service).to receive(:iterations_completed).and_return(2) - - expect(document).to receive(:update!).with( - metadata: hash_including( - 'faq_generation' => hash_including( - 'method' => 'paginated', - 'pages_processed' => 15, - 'iterations' => 2 - ) - ) - ) - - described_class.perform_now(document) - end - end - - context 'when document uses standard generation' do - before do - allow(document).to receive(:pdf_document?).and_return(false) - allow(document).to receive(:content).and_return('Document content') + allow(paginated_service).to receive(:generate).and_return(pdf_faqs) + allow(paginated_service).to receive(:total_pages_processed).and_return(10) + allow(paginated_service).to receive(:iterations_completed).and_return(1) end - it 'uses standard FAQ generator service' do - standard_service = instance_double(Captain::Llm::FaqGeneratorService) + it 'uses paginated FAQ generator for PDFs' do + expect(Captain::Llm::PaginatedFaqGeneratorService).to receive(:new).with(pdf_document, anything) - expect(Captain::Llm::FaqGeneratorService).to receive(:new) - .with('Document content') - .and_return(standard_service) - - expect(standard_service).to receive(:generate).and_return([]) - - described_class.perform_now(document) - end - end - - context 'when FAQs are generated successfully' do - let(:faqs) do - [ - { 'question' => 'What is this?', 'answer' => 'This is a test' }, - { 'question' => 'How does it work?', 'answer' => 'It works well' } - ] + described_class.new.perform(pdf_document) end - before do - allow(document).to receive(:pdf_document?).and_return(false) - allow(document).to receive(:content).and_return('Document content') + it 'stores pagination metadata' do + expect(pdf_document).to receive(:update!).with(hash_including(metadata: hash_including('faq_generation'))) - standard_service = instance_double(Captain::Llm::FaqGeneratorService) - allow(Captain::Llm::FaqGeneratorService).to receive(:new).and_return(standard_service) - allow(standard_service).to receive(:generate).and_return(faqs) + described_class.new.perform(pdf_document) end - - it 'creates response records from FAQs' do - expect { described_class.perform_now(document) } - .to change { document.responses.count }.by(2) - end - - it 'creates responses with correct attributes' do - described_class.perform_now(document) - - response = document.responses.first - expect(response.question).to eq('What is this?') - expect(response.answer).to eq('This is a test') - expect(response.assistant).to eq(assistant) - expect(response.documentable).to eq(document) - end - - it 'removes previous responses before creating new ones' do - # Create existing responses - document.responses.create!( - question: 'Old question', - answer: 'Old answer', - assistant: assistant, - documentable: document - ) - - expect(document.responses.count).to eq(1) - - described_class.perform_now(document) - - expect(document.responses.count).to eq(2) - expect(document.responses.pluck(:question)).not_to include('Old question') - end - end - - context 'when FAQ generation fails' do - before do - allow(document).to receive(:pdf_document?).and_return(false) - allow(document).to receive(:content).and_return('Document content') - end - - it 'propagates the error' do - standard_service = instance_double(Captain::Llm::FaqGeneratorService) - allow(Captain::Llm::FaqGeneratorService).to receive(:new).and_return(standard_service) - allow(standard_service).to receive(:generate).and_raise(StandardError, 'Generation failed') - - expect { described_class.perform_now(document) } - .to raise_error(StandardError, 'Generation failed') - end - end - - context 'when creating response fails' do - let(:faqs) do - [ - { 'question' => nil, 'answer' => 'Invalid FAQ' } # Invalid due to nil question - ] - end - - before do - allow(document).to receive(:pdf_document?).and_return(false) - allow(document).to receive(:content).and_return('Document content') - - standard_service = instance_double(Captain::Llm::FaqGeneratorService) - allow(Captain::Llm::FaqGeneratorService).to receive(:new).and_return(standard_service) - allow(standard_service).to receive(:generate).and_return(faqs) - end - - it 'logs the error and continues' do - expect(Rails.logger).to receive(:error).with(/Error in creating response document/) - - expect { described_class.perform_now(document) } - .not_to raise_error - end - end - end - - describe 'job queuing' do - it 'is enqueued in the low queue' do - expect do - described_class.perform_later(document) - end.to have_enqueued_job(described_class).on_queue('low') - end - - it 'can be scheduled for later execution' do - expect do - described_class.set(wait: 5.minutes).perform_later(document) - end.to have_enqueued_job(described_class).at(a_value_within(1.second).of(5.minutes.from_now)) - end - end - - describe '#should_use_pagination?' do - subject { described_class.new.send(:should_use_pagination?, document) } - - context 'when document is a PDF with openai_file_id' do - before do - allow(document).to receive(:pdf_document?).and_return(true) - allow(document).to receive(:openai_file_id).and_return('file-123') - end - - it { is_expected.to be true } - end - - context 'when document is a PDF without openai_file_id' do - before do - allow(document).to receive(:pdf_document?).and_return(true) - allow(document).to receive(:openai_file_id).and_return(nil) - end - - it { is_expected.to be false } - end - - context 'when document is not a PDF' do - before do - allow(document).to receive(:pdf_document?).and_return(false) - end - - it { is_expected.to be false } end end end diff --git a/spec/enterprise/models/captain/document_spec.rb b/spec/enterprise/models/captain/document_spec.rb index ef503a171..56dc1727c 100644 --- a/spec/enterprise/models/captain/document_spec.rb +++ b/spec/enterprise/models/captain/document_spec.rb @@ -4,240 +4,81 @@ RSpec.describe Captain::Document, type: :model do let(:account) { create(:account) } let(:assistant) { create(:captain_assistant, account: account) } - describe 'validations' do - context 'with PDF file' do - let(:document) { build(:captain_document, assistant: assistant, account: account) } - - before do - document.pdf_file.attach( - io: StringIO.new('PDF content'), - filename: 'test.pdf', - content_type: 'application/pdf' - ) - end - - it 'is valid with PDF file and no external link' do - document.external_link = nil - expect(document).to be_valid - end - - # NOTE: Current implementation allows both PDF and external link - it 'is valid with both PDF file and external link' do - document.external_link = 'https://example.com' - expect(document).to be_valid - end - - it 'validates PDF file format' do - document.pdf_file.purge - document.pdf_file.attach( - io: StringIO.new('Text content'), - filename: 'test.txt', - content_type: 'text/plain' - ) - # The validation actually checks if content_type is application/pdf - # Since pdf_document? returns true for attached files with pdf content type, - # and validate_pdf_format only runs if pdf_document? is true, - # this test shows current behavior - expect(document).to be_valid # Because it has external_link and pdf_document? returns false - end - end - - context 'without PDF file' do - let(:document) { build(:captain_document, assistant: assistant, account: account, external_link: 'https://example.com') } - - it 'is valid with external link and no PDF file' do - expect(document).to be_valid - end - - it 'is invalid without both PDF file and external link' do - document.external_link = nil - expect(document).not_to be_valid - expect(document.errors[:external_link]).to include("can't be blank") - end - end - end - - describe '#pdf_document?' do - let(:document) { create(:captain_document, assistant: assistant, account: account) } - - context 'when document has PDF file attached' do - before do - document.pdf_file.attach( - io: StringIO.new('PDF content'), - filename: 'test.pdf', - content_type: 'application/pdf' - ) - end - - it 'returns true' do - expect(document.pdf_document?).to be true - end - end - - context 'when document has no PDF file' do - it 'returns false' do - expect(document.pdf_document?).to be false - end - end - end - - describe '#content_type' do - let(:document) { create(:captain_document, assistant: assistant, account: account) } - - context 'with PDF file' do - before do - document.pdf_file.attach( - io: StringIO.new('PDF content'), - filename: 'test.pdf', - content_type: 'application/pdf' - ) - end - - it 'returns PDF content type' do - expect(document.content_type).to eq('application/pdf') - end - end - - context 'without PDF file' do - it 'returns nil' do - expect(document.content_type).to be_nil - end - end - end - - describe '#file_size' do - let(:document) { create(:captain_document, assistant: assistant, account: account) } - - context 'with PDF file' do - let(:file_content) { 'PDF content with some size' } - - before do - document.pdf_file.attach( - io: StringIO.new(file_content), - filename: 'test.pdf', - content_type: 'application/pdf' - ) - end - - it 'returns the file size in bytes' do - expect(document.file_size).to eq(file_content.bytesize) - end - end - - context 'without PDF file' do - it 'returns nil' do - expect(document.file_size).to be_nil - end - end - end - - describe '#set_external_link_for_pdf' do - let(:document) { build(:captain_document, assistant: assistant, account: account, external_link: nil) } - - context 'when called before save' do - before do - document.pdf_file.attach( - io: StringIO.new('PDF content'), - filename: 'test.pdf', - content_type: 'application/pdf' - ) - end - - it 'generates a unique external link' do - document.save! - expect(document.external_link).to be_present - expect(document.external_link).to match(/^PDF: /) - end - - it 'generates different links for different documents' do - document.save! - another_document = build(:captain_document, assistant: assistant, account: account, external_link: nil) - another_document.pdf_file.attach( - io: StringIO.new('Different PDF'), - filename: 'another.pdf', - content_type: 'application/pdf' - ) - another_document.save! - - expect(document.external_link).not_to eq(another_document.external_link) - end - end - - context 'when document already has external link' do - before do - document.external_link = 'https://example.com' - end - - it 'does not override existing external link' do - document.save! - expect(document.external_link).to eq('https://example.com') - end - end - - context 'when document has no PDF file' do - it 'does not set external link' do - expect { document.save! }.to raise_error(ActiveRecord::RecordInvalid) - end - end - end - - describe 'metadata column' do - let(:document) { create(:captain_document, assistant: assistant, account: account) } - - it 'has metadata column with default empty hash' do - expect(document.metadata).to eq({}) - end - - it 'can store OpenAI file ID' do - document.metadata['openai_file_id'] = 'file-123' - document.save! - document.reload - expect(document.metadata['openai_file_id']).to eq('file-123') - end - - it 'can store FAQ generation metadata' do - document.metadata['faq_generation'] = { - 'faqs_generated' => 10, - 'pages_processed' => 3, - 'generation_time' => 5.2 - } - document.save! - document.reload - expect(document.metadata['faq_generation']).to include( - 'faqs_generated' => 10, - 'pages_processed' => 3, - 'generation_time' => 5.2 + describe 'PDF support' do + let(:pdf_document) do + doc = build(:captain_document, assistant: assistant, account: account) + doc.pdf_file.attach( + io: StringIO.new('PDF content'), + filename: 'test.pdf', + content_type: 'application/pdf' ) + doc end - it 'preserves existing metadata when adding new keys' do - document.metadata['openai_file_id'] = 'file-123' - document.save! + describe 'validations' do + it 'allows PDF file without external link' do + pdf_document.external_link = nil + expect(pdf_document).to be_valid + end - document.metadata['faq_generation'] = { 'count' => 5 } - document.save! - document.reload - - expect(document.metadata).to include( - 'openai_file_id' => 'file-123', - 'faq_generation' => { 'count' => 5 } - ) - end - end - - describe 'callbacks' do - context 'when before_validation callback runs' do - let(:document) { build(:captain_document, assistant: assistant, account: account, external_link: nil) } - - it 'sets external link for PDF documents' do - document.pdf_file.attach( - io: StringIO.new('PDF content'), - filename: 'test.pdf', + it 'validates PDF file size' do + doc = build(:captain_document, assistant: assistant, account: account) + doc.pdf_file.attach( + io: StringIO.new('x' * 11.megabytes), + filename: 'large.pdf', content_type: 'application/pdf' ) + doc.external_link = nil + expect(doc).not_to be_valid + expect(doc.errors[:pdf_file]).to include(I18n.t('captain.documents.pdf_size_error')) + end + end - expect { document.valid? }.to change(document, :external_link).from(nil) - expect(document.external_link).to match(/^PDF: /) + describe '#pdf_document?' do + it 'returns true for attached PDF' do + expect(pdf_document.pdf_document?).to be true + end + + it 'returns true for .pdf external links' do + doc = build(:captain_document, external_link: 'https://example.com/document.pdf') + expect(doc.pdf_document?).to be true + end + + it 'returns false for non-PDF documents' do + doc = build(:captain_document, external_link: 'https://example.com') + expect(doc.pdf_document?).to be false + end + end + + describe '#display_url' do + it 'returns Rails blob URL for attached PDFs' do + pdf_document.save! + # The display_url method calls rails_blob_url which returns a URL containing 'rails/active_storage' + url = pdf_document.display_url + expect(url).to be_present + end + + it 'returns external_link for web documents' do + doc = create(:captain_document, external_link: 'https://example.com') + expect(doc.display_url).to eq('https://example.com') + end + end + + describe '#store_openai_file_id' do + it 'stores the file ID in metadata' do + pdf_document.save! + pdf_document.store_openai_file_id('file-abc123') + + expect(pdf_document.reload.openai_file_id).to eq('file-abc123') + end + end + + describe 'automatic external_link generation' do + it 'generates unique external_link for PDFs' do + pdf_document.external_link = nil + pdf_document.save! + + expect(pdf_document.external_link).to start_with('PDF: test_') end end end diff --git a/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb b/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb index 036ba2459..5215a2d40 100644 --- a/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb +++ b/spec/enterprise/services/captain/llm/paginated_faq_generator_service_spec.rb @@ -1,315 +1,106 @@ require 'rails_helper' +require 'custom_exceptions/pdf_processing_error' RSpec.describe Captain::Llm::PaginatedFaqGeneratorService do - let(:account) { create(:account) } - let(:service) do - described_class.new(document, { pages_per_chunk: 10 }) - end - let(:assistant) { create(:captain_assistant, account: account) } - let(:document) { create(:captain_document, assistant: assistant, account: account) } + let(:document) { create(:captain_document) } + let(:service) { described_class.new(document, pages_per_chunk: 5) } let(:openai_client) { instance_double(OpenAI::Client) } before do - # Mock the InstallationConfig for base class initialization - allow(InstallationConfig).to receive(:find_by!).with(name: 'CAPTAIN_OPEN_AI_API_KEY') - .and_return(instance_double(InstallationConfig, value: 'test-api-key')) - # Allow any find_by calls to return nil by default - allow(InstallationConfig).to receive(:find_by).and_return(nil) + # Mock OpenAI configuration + installation_config = instance_double(InstallationConfig, value: 'test-api-key') + allow(InstallationConfig).to receive(:find_by!) + .with(name: 'CAPTAIN_OPEN_AI_API_KEY') + .and_return(installation_config) + allow(OpenAI::Client).to receive(:new).and_return(openai_client) end - describe '#initialize' do - it 'initializes with document and options' do - expect(service.instance_variable_get(:@document)).to eq(document) - expect(service.instance_variable_get(:@pages_per_chunk)).to eq(10) - end - - it 'uses default pages per chunk when not specified' do - default_service = described_class.new(document) - expect(default_service.instance_variable_get(:@pages_per_chunk)).to eq(10) - end - - it 'accepts max_pages option' do - service_with_max = described_class.new(document, { max_pages: 50 }) - expect(service_with_max.instance_variable_get(:@max_pages)).to eq(50) - end - end - describe '#generate' do - context 'when document has no openai_file_id' do + context 'when document lacks OpenAI file ID' do before do allow(document).to receive(:openai_file_id).and_return(nil) end it 'raises an error' do - expect { service.generate }.to raise_error('Document must have openai_file_id for paginated processing') + expect { service.generate }.to raise_error(CustomExceptions::PdfFaqGenerationError) end end - context 'when document has openai_file_id' do + context 'when generating FAQs from PDF pages' do + let(:faq_response) do + { + 'choices' => [{ + 'message' => { + 'content' => JSON.generate({ + 'faqs' => [ + { 'question' => 'What is this document about?', 'answer' => 'It explains key concepts.' } + ], + 'has_content' => true + }) + } + }] + } + end + + let(:empty_response) do + { + 'choices' => [{ + 'message' => { + 'content' => JSON.generate({ + 'faqs' => [], + 'has_content' => false + }) + } + }] + } + end + before do allow(document).to receive(:openai_file_id).and_return('file-123') end - context 'when generating FAQs successfully' do - let(:faq_response) do - { - 'choices' => [ - { - 'message' => { - 'content' => JSON.generate({ - 'faqs' => [ - { 'question' => 'What is the product?', 'answer' => 'It is a great product.' }, - { 'question' => 'How does it work?', 'answer' => 'It works seamlessly.' } - ], - 'has_content' => true - }) - } - } - ] - } - end + it 'generates FAQs from paginated content' do + allow(openai_client).to receive(:chat).and_return(faq_response, empty_response) - let(:empty_response) do - { - 'choices' => [ - { - 'message' => { - 'content' => JSON.generate({ - 'faqs' => [], - 'has_content' => false - }) - } - } - ] - } - end + faqs = service.generate - it 'generates FAQs from document' do - allow(openai_client).to receive(:chat).and_return(faq_response, empty_response) - - result = service.generate - expect(result).to be_an(Array) - expect(result.first).to include('question', 'answer') - end - - it 'processes multiple chunks until no more content' do - allow(openai_client).to receive(:chat).and_return(faq_response, faq_response, empty_response) - - result = service.generate - expect(result.count).to eq(2) # Deduplication removes duplicates - end - - it 'deduplicates similar FAQs' do - duplicate_response = { - 'choices' => [ - { - 'message' => { - 'content' => JSON.generate({ - 'faqs' => [ - { 'question' => 'What is the product?', 'answer' => 'It is a great product.' }, - { 'question' => 'What is the product?', 'answer' => 'Different answer.' } - ], - 'has_content' => false - }) - } - } - ] - } - - allow(openai_client).to receive(:chat).and_return(duplicate_response) - - result = service.generate - expect(result.count).to eq(1) - end - - it 'tracks total pages processed' do - allow(openai_client).to receive(:chat).and_return(faq_response, faq_response, empty_response) - - service.generate - expect(service.total_pages_processed).to eq(30) # 3 chunks of 10 pages each - end - - it 'tracks iterations completed' do - allow(openai_client).to receive(:chat).and_return(faq_response, faq_response, empty_response) - - service.generate - expect(service.iterations_completed).to eq(3) - end + expect(faqs).to have_attributes(size: 1) + expect(faqs.first['question']).to eq('What is this document about?') end - context 'when API returns error' do - before do - allow(openai_client).to receive(:chat).and_raise(OpenAI::Error, 'API error') - end + it 'stops when no more content' do + allow(openai_client).to receive(:chat).and_return(empty_response) - it 'returns empty array and logs error' do - expect(Rails.logger).to receive(:error).with(/Error processing pages/) + faqs = service.generate - result = service.generate - expect(result).to eq([]) - end + expect(faqs).to be_empty end - context 'when response is malformed' do - let(:malformed_response) do - { - 'choices' => [ - { - 'message' => { - 'content' => 'Invalid JSON' - } - } - ] - } - end + it 'respects max iterations limit' do + allow(openai_client).to receive(:chat).and_return(faq_response) - before do - allow(openai_client).to receive(:chat).and_return(malformed_response) - end + # Force max iterations + service.instance_variable_set(:@iterations_completed, 19) - it 'handles JSON parsing errors gracefully' do - expect(Rails.logger).to receive(:error).with(/Error parsing chunk response/) - - result = service.generate - expect(result).to eq([]) - end + service.generate + expect(service.iterations_completed).to eq(20) end end end describe '#should_continue_processing?' do - let(:chunk_result_with_faqs) { { faqs: [{ 'question' => 'Q1', 'answer' => 'A1' }], has_content: true } } - let(:chunk_result_empty) { { faqs: [], has_content: false } } - - it 'returns false when max iterations reached' do + it 'stops at max iterations' do service.instance_variable_set(:@iterations_completed, 20) - expect(service.should_continue_processing?(chunk_result_with_faqs)).to be false + expect(service.should_continue_processing?(faqs: ['faq'], has_content: true)).to be false end - it 'returns false when max pages reached' do - service.instance_variable_set(:@max_pages, 30) - service.instance_variable_set(:@total_pages_processed, 30) - expect(service.should_continue_processing?(chunk_result_with_faqs)).to be false + it 'stops when no FAQs returned' do + expect(service.should_continue_processing?(faqs: [], has_content: true)).to be false end - it 'returns false when no FAQs returned' do - expect(service.should_continue_processing?({ faqs: [], has_content: true })).to be false - end - - it 'returns false when has_content is false' do - expect(service.should_continue_processing?({ faqs: ['faq'], has_content: false })).to be false - end - - it 'returns true when should continue' do - expect(service.should_continue_processing?(chunk_result_with_faqs)).to be true - end - end - - describe 'private methods' do - describe '#similarity_score' do - it 'returns 1 for identical strings' do - score = service.send(:similarity_score, 'Hello world', 'Hello world') - expect(score).to eq(1.0) - end - - it 'returns 0 for completely different strings' do - score = service.send(:similarity_score, 'Hello', 'Goodbye') - expect(score).to eq(0.0) - end - - it 'calculates partial similarity correctly' do - score = service.send(:similarity_score, 'Hello world', 'Hello there') - expect(score).to be_between(0.3, 0.4) - end - end - - describe '#deduplicate_faqs' do - let(:faqs_with_duplicates) do - [ - { 'question' => 'What is the product?', 'answer' => 'Answer 1' }, - { 'question' => 'What is the product?', 'answer' => 'Answer 2' }, - { 'question' => 'What is the product really?', 'answer' => 'Answer 3' }, - { 'question' => 'How does it work?', 'answer' => 'Answer 4' } - ] - end - - it 'removes exact duplicates' do - result = service.send(:deduplicate_faqs, faqs_with_duplicates) - questions = result.map { |f| f['question'] } - expect(questions.count('What is the product?')).to eq(1) - end - - it 'removes similar questions' do - result = service.send(:deduplicate_faqs, faqs_with_duplicates) - expect(result.count).to be < faqs_with_duplicates.count - end - end - - describe '#determine_stop_reason' do - it 'identifies max iterations reason' do - service.instance_variable_set(:@iterations_completed, 20) - reason = service.send(:determine_stop_reason, { faqs: ['faq'] }) - expect(reason).to eq('Maximum iterations reached') - end - - it 'identifies max pages reason' do - service.instance_variable_set(:@max_pages, 30) - service.instance_variable_set(:@total_pages_processed, 30) - reason = service.send(:determine_stop_reason, { faqs: ['faq'] }) - expect(reason).to eq('Maximum pages processed') - end - - it 'identifies no content reason' do - reason = service.send(:determine_stop_reason, { faqs: [] }) - expect(reason).to eq('No content found in last chunk') - end - - it 'identifies end of document reason' do - reason = service.send(:determine_stop_reason, { faqs: ['faq'], has_content: false }) - expect(reason).to eq('End of document reached') - end - end - - describe '#calculate_end_page' do - it 'calculates end page correctly' do - service.instance_variable_set(:@pages_per_chunk, 10) - expect(service.send(:calculate_end_page, 1)).to eq(10) - expect(service.send(:calculate_end_page, 11)).to eq(20) - end - - it 'respects max_pages limit' do - service.instance_variable_set(:@pages_per_chunk, 10) - service.instance_variable_set(:@max_pages, 15) - expect(service.send(:calculate_end_page, 11)).to eq(15) - end - end - - describe '#build_chunk_parameters' do - before do - allow(document).to receive(:openai_file_id).and_return('file-123') - allow(Captain::Llm::SystemPromptsService).to receive(:paginated_faq_generator) - .and_return('Generate FAQs from pages') - end - - it 'builds correct parameters structure' do - params = service.send(:build_chunk_parameters, 1, 10) - - expect(params[:model]).to eq('gpt-4.1-mini') - expect(params[:response_format]).to eq({ type: 'json_object' }) - expect(params[:messages]).to be_an(Array) - expect(params[:messages].first[:role]).to eq('user') - end - - it 'includes file reference and prompt' do - params = service.send(:build_chunk_parameters, 1, 10) - user_content = params[:messages].first[:content] - - expect(user_content).to be_an(Array) - expect(user_content.first[:type]).to eq('file') - expect(user_content.first[:file][:file_id]).to eq('file-123') - expect(user_content.last[:type]).to eq('text') - end + it 'continues when FAQs exist and under limits' do + expect(service.should_continue_processing?(faqs: ['faq'], has_content: true)).to be true end end end diff --git a/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb b/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb index 154ecf078..9dc416685 100644 --- a/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb +++ b/spec/enterprise/services/captain/llm/pdf_processing_service_spec.rb @@ -1,13 +1,12 @@ require 'rails_helper' +require 'custom_exceptions/pdf_processing_error' RSpec.describe Captain::Llm::PdfProcessingService do - let(:account) { create(:account) } - let(:assistant) { create(:captain_assistant, account: account) } - let(:document) { create(:captain_document, account: account, assistant: assistant) } + let(:document) { create(:captain_document) } let(:service) { described_class.new(document) } before do - # Mock the base class dependencies + # Mock OpenAI configuration installation_config = instance_double(InstallationConfig, value: 'test-api-key') allow(InstallationConfig).to receive(:find_by!) .with(name: 'CAPTAIN_OPEN_AI_API_KEY') @@ -15,43 +14,44 @@ RSpec.describe Captain::Llm::PdfProcessingService do end describe '#process' do - context 'when document already has openai_file_id' do + context 'when document already has OpenAI file ID' do before do allow(document).to receive(:openai_file_id).and_return('existing-file-id') end - it 'returns success message without uploading' do - result = service.process - expect(result).to eq('PDF ready for paginated processing') + it 'skips upload' do + expect(document).not_to receive(:store_openai_file_id) + service.process end end - context 'when document needs upload' do + context 'when uploading PDF to OpenAI' do let(:mock_client) { instance_double(OpenAI::Client) } + let(:pdf_content) { 'PDF content' } before do allow(document).to receive(:openai_file_id).and_return(nil) - # Create a mock for pdf_file that responds to download - pdf_file = Struct.new(:download).new('pdf content') + # Use a simple double for ActiveStorage since it's a complex Rails object + pdf_file = double('pdf_file', download: pdf_content) # rubocop:disable RSpec/VerifiedDoubles allow(document).to receive(:pdf_file).and_return(pdf_file) - # Mock OpenAI client allow(OpenAI::Client).to receive(:new).and_return(mock_client) - files_api = Object.new - files_api.define_singleton_method(:upload) { |_params| { 'id' => 'file-abc123' } } + # Use a simple double for OpenAI::Files as it may not be loaded + files_api = double('files_api') # rubocop:disable RSpec/VerifiedDoubles + allow(files_api).to receive(:upload).and_return({ 'id' => 'file-abc123' }) allow(mock_client).to receive(:files).and_return(files_api) - - allow(document).to receive(:store_openai_file_id) end - it 'uploads PDF and returns success message' do + it 'uploads PDF and stores file ID' do expect(document).to receive(:store_openai_file_id).with('file-abc123') + service.process + end - result = service.process + it 'raises error when upload fails' do + allow(mock_client.files).to receive(:upload).and_return({ 'id' => nil }) - expect(result).to include('file-abc123') - expect(result).to include('PDF ready for paginated processing') + expect { service.process }.to raise_error(CustomExceptions::PdfUploadError) end end end