From 250d9d4deed8d6081e7c6db3454df4bfdb24f1ee Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Thu, 3 Jul 2025 13:57:38 +0530 Subject: [PATCH] refactor: review changes --- .../conversation/response_builder_job.rb | 9 ++++-- .../captain/documents/pdf_extraction_job.rb | 32 ++++++++++++------- .../tools/pdf_extraction_parser_job.rb | 14 +++++--- .../captain/tools/pdf_validation_concern.rb | 22 +++++++++++-- 4 files changed, 56 insertions(+), 21 deletions(-) diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index f341a6e98..2d618ae89 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -52,12 +52,15 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob def message_content(message) return message.content if message.content.present? + + handle_message_without_content(message) + end + + def handle_message_without_content(message) 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' + audio_transcriptions.presence || 'User has shared an attachment' end def extract_audio_transcriptions(attachments) diff --git a/enterprise/app/jobs/captain/documents/pdf_extraction_job.rb b/enterprise/app/jobs/captain/documents/pdf_extraction_job.rb index aef02dfb1..da7f945b7 100644 --- a/enterprise/app/jobs/captain/documents/pdf_extraction_job.rb +++ b/enterprise/app/jobs/captain/documents/pdf_extraction_job.rb @@ -4,23 +4,33 @@ class Captain::Documents::PdfExtractionJob < ApplicationJob def perform(document) return unless document.pdf_document? - document.update(status: 'in_progress') - - pdf_source = document.file.attached? ? document.file : document.external_link - pdf_extraction_service = Captain::Tools::PdfExtractionService.new(pdf_source) - result = pdf_extraction_service.perform - - if result[:success] && result[:content].present? - process_pdf_content_chunks(document, result[:content]) - else - handle_pdf_extraction_failure(document, result) - end + initialize_document_processing(document) + result = extract_pdf_content(document) + process_extraction_result(document, result) rescue Captain::Tools::PdfExtractionService::ExtractionError => e handle_pdf_extraction_error(document, e) end private + def initialize_document_processing(document) + document.update(status: 'in_progress') + end + + def extract_pdf_content(document) + pdf_source = document.file.attached? ? document.file : document.external_link + pdf_extraction_service = Captain::Tools::PdfExtractionService.new(pdf_source) + pdf_extraction_service.perform + end + + def process_extraction_result(document, result) + if result[:success] && result[:content].present? + process_pdf_content_chunks(document, result[:content]) + else + handle_pdf_extraction_failure(document, result) + end + end + def process_pdf_content_chunks(document, content_chunks) Rails.logger.info "PDF extraction successful for document #{document.id}: #{content_chunks.length} chunks will be processed" diff --git a/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb b/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb index 546e2c4ba..2815cac3c 100644 --- a/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb +++ b/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb @@ -3,12 +3,10 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob def perform(assistant_id:, pdf_content:, document_id: nil) assistant = Captain::Assistant.find(assistant_id) - content = pdf_content[:content] - - return if content.blank? || limit_exceeded?(assistant.account) + return unless should_process_content?(pdf_content[:content], assistant.account) document = create_document(assistant, pdf_content, document_id) - Captain::Documents::ResponseBuilderJob.perform_later(document) if document + enqueue_response_builder_job(document) rescue ActiveRecord::RecordNotFound => e Rails.logger.error "PDF parser job failed - Assistant not found: #{e.message}" rescue ActiveRecord::RecordInvalid => e @@ -69,6 +67,14 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob end end + def should_process_content?(content, account) + content.present? && !limit_exceeded?(account) + end + + def enqueue_response_builder_job(document) + Captain::Documents::ResponseBuilderJob.perform_later(document) if document + end + def limit_exceeded?(account) limits = account.usage_limits.dig(:captain, :documents) limits && limits[:current_available].to_i <= 0 diff --git a/enterprise/app/services/captain/tools/pdf_validation_concern.rb b/enterprise/app/services/captain/tools/pdf_validation_concern.rb index ff5eed33b..ff613f2c6 100644 --- a/enterprise/app/services/captain/tools/pdf_validation_concern.rb +++ b/enterprise/app/services/captain/tools/pdf_validation_concern.rb @@ -20,13 +20,29 @@ module Captain::Tools::PdfValidationConcern end def validate_url_format - uri = URI.parse(pdf_source) - raise StandardError, 'Invalid URL scheme' unless %w[http https].include?(uri.scheme) - raise StandardError, 'URL too long' if pdf_source.length > 2000 + uri = parse_and_validate_uri + validate_url_scheme(uri) + validate_url_length + end + + def parse_and_validate_uri + URI.parse(pdf_source) rescue URI::InvalidURIError raise StandardError, 'Malformed URL' end + def validate_url_scheme(uri) + return if %w[http https].include?(uri.scheme) + + raise StandardError, 'Invalid URL scheme' + end + + def validate_url_length + return if pdf_source.length <= 2000 + + raise StandardError, 'URL too long' + end + def validate_file_type_and_size raise StandardError, 'Invalid file type' unless pdf_source.content_type == 'application/pdf' raise StandardError, 'File too large' if pdf_source.size > 25.megabytes