refactor: fix the codeclimate

enterprise/app/controllers/api/v1/accounts/captain/documents_controller.rb

@@ -87,15 +87,37 @@ class Api::V1::Accounts::Captain::DocumentsController < Api::V1::Accounts::BaseC
 
   def validate_pdf_file
     file = pdf_params[:pdf_document]
-    return { valid: false, error: 'Invalid file object' } unless file.respond_to?(:content_type) && file.respond_to?(:size)
 
-    return { valid: false, error: 'Invalid file type. Only PDF files are allowed.' } unless ALLOWED_PDF_CONTENT_TYPES.include?(file.content_type)
+    file_object_error = validate_file_object(file)
+    return file_object_error if file_object_error
 
-    return { valid: false, error: "File size too large. Maximum size is #{MAX_PDF_SIZE / 1.megabyte}MB." } if file.size > MAX_PDF_SIZE
+    file_type_error = validate_file_type(file)
+    return file_type_error if file_type_error
+
+    file_size_error = validate_file_size(file)
+    return file_size_error if file_size_error
 
     { valid: true }
   end
 
+  def validate_file_object(file)
+    return nil if file.respond_to?(:content_type) && file.respond_to?(:size)
+
+    { valid: false, error: 'Invalid file object' }
+  end
+
+  def validate_file_type(file)
+    return nil if ALLOWED_PDF_CONTENT_TYPES.include?(file.content_type)
+
+    { valid: false, error: 'Invalid file type. Only PDF files are allowed.' }
+  end
+
+  def validate_file_size(file)
+    return nil if file.size <= MAX_PDF_SIZE
+
+    { valid: false, error: "File size too large. Maximum size is #{MAX_PDF_SIZE / 1.megabyte}MB." }
+  end
+
   def create_pdf_blob
     file = pdf_params[:pdf_document]
 

enterprise/app/jobs/captain/documents/crawl_job.rb

@@ -22,14 +22,19 @@ class Captain::Documents::CrawlJob < ApplicationJob
   include Captain::FirecrawlHelper
 
   def pdf_by_metadata?(document)
-    return true if document.respond_to?(:source_type) && document.source_type == 'pdf_upload'
-    return true if document.respond_to?(:content_type) && document.content_type&.include?('application/pdf')
-
-    false
+    pdf_by_source_type?(document) || pdf_by_content_type?(document)
   rescue StandardError
     false
   end
 
+  def pdf_by_source_type?(document)
+    document.respond_to?(:source_type) && document.source_type == 'pdf_upload'
+  end
+
+  def pdf_by_content_type?(document)
+    document.respond_to?(:content_type) && document.content_type&.include?('application/pdf')
+  end
+
   def pdf_by_url?(document)
     return false if document.external_link.blank?
 

enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb

@@ -152,16 +152,19 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob
   end
 
   def add_page_chunk_info(base_title, page_number, chunk_index, total_chunks)
-    title_with_info = if total_chunks > 1
-                        "#{base_title} (Page #{page_number}, Part #{chunk_index}/#{total_chunks})"
-                      elsif page_number && page_number > 1
-                        "#{base_title} (Page #{page_number})"
-                      else
-                        base_title
-                      end
+    title_with_info = build_title_with_chunk_info(base_title, page_number, chunk_index, total_chunks)
+    truncate_title(title_with_info)
+  end
 
-    # Ensure title doesn't exceed database limit
-    title_with_info.length > 255 ? "#{title_with_info[0, 252]}..." : title_with_info
+  def build_title_with_chunk_info(base_title, page_number, chunk_index, total_chunks)
+    return "#{base_title} (Page #{page_number}, Part #{chunk_index}/#{total_chunks})" if total_chunks > 1
+    return "#{base_title} (Page #{page_number})" if page_number && page_number > 1
+
+    base_title
+  end
+
+  def truncate_title(title)
+    title.length > 255 ? "#{title[0, 252]}..." : title
   end
 
   def limit_exceeded?(account)
@@ -179,33 +182,45 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob
   def handle_processing_error(error, assistant_id, document_id)
     Rails.logger.error "Failed to parse PDF content for assistant #{assistant_id}: #{error.message}"
 
-    # If we have a document_id, try to update its status to indicate error
-    if document_id.present?
-      begin
-        document = Captain::Document.find_by(id: document_id)
-        # Only update if document exists and update doesn't cause another error
-        document&.update(status: 'in_progress') unless error.message.include?('Database error')
-      rescue StandardError => e
-        Rails.logger.error "Failed to update document status: #{e.message}"
-      end
-    end
+    update_document_error_status(document_id, error) if document_id.present?
 
     raise "Failed to parse PDF data: #{error.message}"
   end
 
+  def update_document_error_status(document_id, error)
+    return if should_skip_document_update?(error)
+
+    document = Captain::Document.find_by(id: document_id)
+    document&.update(status: 'in_progress')
+  rescue StandardError => e
+    Rails.logger.error "Failed to update document status: #{e.message}"
+  end
+
+  def should_skip_document_update?(error)
+    error.message.include?('Database error')
+  end
+
   def process_document(assistant, content_data, document_id)
-    if document_id.present?
-      existing_document = Captain::Document.find_by(id: document_id)
-      if existing_document
-        log_chunk_processing(document_id, content_data)
-        update_document_content(existing_document, content_data)
-      else
-        result = create_new_document(assistant, content_data, document_id)
-        return false if result == false  # Limits exceeded
-      end
+    return create_new_document_with_limit_check(assistant, content_data, document_id) if document_id.blank?
+
+    process_existing_document(assistant, content_data, document_id)
+  end
+
+  def process_existing_document(assistant, content_data, document_id)
+    existing_document = Captain::Document.find_by(id: document_id)
+
+    if existing_document
+      log_chunk_processing(document_id, content_data)
+      update_document_content(existing_document, content_data)
     else
-      result = create_new_document(assistant, content_data, document_id)
-      return false if result == false  # Limits exceeded
+      create_new_document_with_limit_check(assistant, content_data, document_id)
     end
   end
+
+  def create_new_document_with_limit_check(assistant, content_data, document_id)
+    result = create_new_document(assistant, content_data, document_id)
+    return false if result == false  # Limits exceeded
+
+    result
+  end
 end

enterprise/app/services/captain/tools/pdf_content_chunking_concern.rb

@@ -26,48 +26,70 @@ module Captain::Tools::PdfContentChunkingConcern
     return [content] if content.length <= max_size
 
     paragraphs = content.split(/\n\s*\n/)
+    process_paragraphs_into_chunks(paragraphs, max_size, content)
+  end
+
+  def process_paragraphs_into_chunks(paragraphs, max_size, original_content)
     chunks = []
     current_chunk = ''
 
     paragraphs.each do |paragraph|
-      if paragraph.length > max_size
-        add_chunk_if_present(chunks, current_chunk)
-        chunks.concat(split_paragraph_into_chunks(paragraph, max_size))
-        current_chunk = ''
-      elsif ("#{current_chunk}\n\n#{paragraph}").length > max_size
-        add_chunk_if_present(chunks, current_chunk)
-        current_chunk = paragraph
-      else
-        current_chunk = current_chunk.blank? ? paragraph : "#{current_chunk}\n\n#{paragraph}"
-      end
+      chunks, current_chunk = process_single_paragraph(chunks, current_chunk, paragraph, max_size)
     end
 
+    finalize_chunks(chunks, current_chunk, original_content)
+  end
+
+  def process_single_paragraph(chunks, current_chunk, paragraph, max_size)
+    if paragraph.length > max_size
+      add_chunk_if_present(chunks, current_chunk)
+      chunks.concat(split_paragraph_into_chunks(paragraph, max_size))
+      [chunks, '']
+    elsif ("#{current_chunk}\n\n#{paragraph}").length > max_size
+      add_chunk_if_present(chunks, current_chunk)
+      [chunks, paragraph]
+    else
+      combined = current_chunk.blank? ? paragraph : "#{current_chunk}\n\n#{paragraph}"
+      [chunks, combined]
+    end
+  end
+
+  def finalize_chunks(chunks, current_chunk, original_content)
     add_chunk_if_present(chunks, current_chunk)
-    chunks.presence || [content]
+    chunks.presence || [original_content]
   end
 
   def split_paragraph_into_chunks(paragraph, max_size)
     sentences = paragraph.split(/(?<=[.!?])\s+/)
+    process_sentences_into_chunks(sentences, max_size)
+  end
+
+  def process_sentences_into_chunks(sentences, max_size)
     chunks = []
     current_chunk = ''
 
     sentences.each do |sentence|
-      if sentence.length > max_size
-        add_chunk_if_present(chunks, current_chunk)
-        chunks << sentence[0, max_size]
-        current_chunk = ''
-      elsif ("#{current_chunk} #{sentence}").length > max_size
-        add_chunk_if_present(chunks, current_chunk)
-        current_chunk = sentence
-      else
-        current_chunk = current_chunk.blank? ? sentence : "#{current_chunk} #{sentence}"
-      end
+      chunks, current_chunk = process_single_sentence(chunks, current_chunk, sentence, max_size)
     end
 
     add_chunk_if_present(chunks, current_chunk)
     chunks
   end
 
+  def process_single_sentence(chunks, current_chunk, sentence, max_size)
+    if sentence.length > max_size
+      add_chunk_if_present(chunks, current_chunk)
+      chunks << sentence[0, max_size]
+      [chunks, '']
+    elsif ("#{current_chunk} #{sentence}").length > max_size
+      add_chunk_if_present(chunks, current_chunk)
+      [chunks, sentence]
+    else
+      combined = current_chunk.blank? ? sentence : "#{current_chunk} #{sentence}"
+      [chunks, combined]
+    end
+  end
+
   def add_chunk_if_present(chunks, chunk)
     chunks << chunk.strip if chunk.present?
   end

enterprise/app/services/captain/tools/pdf_extraction_service.rb

@@ -23,6 +23,10 @@ class Captain::Tools::PdfExtractionService
     validation_result = validate_pdf_source
     return validation_result unless validation_result[:success]
 
+    extract_pdf_with_error_handling
+  end
+
+  def extract_pdf_with_error_handling
     process_pdf_extraction
   rescue PDF::Reader::MalformedPDFError => e
     handle_malformed_pdf_error(e)
@@ -131,25 +135,27 @@ class Captain::Tools::PdfExtractionService
 
     PDF::Reader.open(file_path) do |reader|
       reader.pages.each_with_index do |page, index|
-        page_text = page.text
-        next if page_text.blank?
-
-        cleaned_text = clean_text(page_text)
-        if cleaned_text.present?
-          text_content << {
-            page_number: index + 1,
-            content: cleaned_text
-          }
-        end
-      rescue StandardError => e
-        Rails.logger.warn "Failed to extract text from page #{index + 1}: #{e.message}"
-        next
+        page_content = extract_page_content(page, index)
+        text_content << page_content if page_content
       end
     end
 
     text_content
   end
 
+  def extract_page_content(page, index)
+    page_text = page.text
+    return nil if page_text.blank?
+
+    cleaned_text = clean_text(page_text)
+    return nil if cleaned_text.blank?
+
+    { page_number: index + 1, content: cleaned_text }
+  rescue StandardError => e
+    Rails.logger.warn "Failed to extract text from page #{index + 1}: #{e.message}"
+    nil
+  end
+
   def clean_text(text)
     # Remove form feeds and normalize whitespace
     cleaned = text.tr("\f", "\n")
@@ -179,9 +185,6 @@ class Captain::Tools::PdfExtractionService
   def log_extraction_success(chunked_content)
     total_chars = chunked_content.sum { |chunk| chunk[:content].length }
     Rails.logger.info "PDF extraction completed: #{chunked_content.length} chunks, #{total_chars} characters"
-    Rails.logger.info "PDF chunks breakdown: #{chunked_content.map do |chunk|
-      "Page #{chunk[:page_number]} (#{chunk[:content].length} chars)"
-    end.join(', ')}"
   end
 
   def process_pdf_extraction

enterprise/app/services/captain/tools/pdf_validation_concern.rb

@@ -17,26 +17,78 @@ module Captain::Tools::PdfValidationConcern
   end
 
   def validate_url
-    uri = URI.parse(pdf_source)
-    raise StandardError, 'Invalid URL format' unless uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS)
-    raise StandardError, 'URL too long' if pdf_source.length > 2048
-    raise StandardError, 'Invalid URL scheme' unless %w[http https].include?(uri.scheme)
+    uri = parse_url
+    validate_url_format(uri)
+    validate_url_length
+    validate_url_scheme(uri)
+  end
+
+  def parse_url
+    URI.parse(pdf_source)
   rescue URI::InvalidURIError
     raise StandardError, 'Malformed URL'
   end
 
+  def validate_url_format(uri)
+    raise StandardError, 'Invalid URL format' unless uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS)
+  end
+
+  def validate_url_length
+    raise StandardError, 'URL too long' if pdf_source.length > 2048
+  end
+
+  def validate_url_scheme(uri)
+    raise StandardError, 'Invalid URL scheme' unless %w[http https].include?(uri.scheme)
+  end
+
   def validate_uploaded_file
+    validate_file_object
+    validate_file_size
+    validate_file_type
+    validate_file_not_empty
+  end
+
+  def validate_file_object
     raise StandardError, 'File object is invalid' unless pdf_source.respond_to?(:size) && pdf_source.respond_to?(:content_type)
+  end
+
+  def validate_file_size
     raise StandardError, "File too large (max #{self.class::MAX_PDF_SIZE / 1.megabyte}MB)" if pdf_source.size > self.class::MAX_PDF_SIZE
+  end
+
+  def validate_file_type
     raise StandardError, 'Invalid file type' unless pdf_source.content_type == 'application/pdf'
+  end
+
+  def validate_file_not_empty
     raise StandardError, 'Empty file' if pdf_source.respond_to?(:empty?) && pdf_source.empty?
   end
 
   def validate_file_path
+    validate_path_presence
+    validate_path_existence
+    validate_path_file_size
+    validate_path_not_empty
+    validate_path_readable
+  end
+
+  def validate_path_presence
     raise StandardError, 'File path is blank' if pdf_source.blank?
+  end
+
+  def validate_path_existence
     raise StandardError, 'File does not exist' unless File.exist?(pdf_source)
+  end
+
+  def validate_path_file_size
     raise StandardError, "File too large (max #{self.class::MAX_PDF_SIZE / 1.megabyte}MB)" if File.size(pdf_source) > self.class::MAX_PDF_SIZE
+  end
+
+  def validate_path_not_empty
     raise StandardError, 'Empty file' if File.empty?(pdf_source)
+  end
+
+  def validate_path_readable
     raise StandardError, 'File is not readable' unless File.readable?(pdf_source)
   end
 end